[bugfix] repo:install-and-deploy respect explicitly-requested version

exist-core/src/main/java/org/exist/repo/Deployment.java

@@ -46,6 +46,7 @@
 import org.exist.xquery.value.SequenceIterator;
 import org.exist.xquery.value.Type;
 import org.expath.pkg.repo.Package;
+import org.expath.pkg.repo.Packages;
 import org.expath.pkg.repo.*;
 import org.expath.pkg.repo.deps.DependencyVersion;
 import org.expath.pkg.repo.tui.BatchUserInteraction;
@@ -255,8 +256,16 @@
             broker.getBrokerPool().reportStatus("Installing app: " + pkg.getAbbrev());
             repo.get().reportAction(ExistRepository.Action.INSTALL, pkg.getName());
 
-            LOG.info("Deploying package {}", pkgName);
-            return deploy(broker, transaction, pkgName, repo, null);
+            // installPackage may return packages.latest() (the highest-version
+            // entry by semver) rather than the just-installed package when the
+            // registry already had a higher version with the same name. Deploy
+            // the freshly-installed package specifically (looked up by the
+            // version we extracted from the XAR descriptor above) so an explicit
+            // repo:install-and-deploy(name, "<older>", url) is not silently
+            // upgraded to packages.latest().
+            final Package deployTarget = resolveInstalledVersion(repo.get(), pkg, pkgVersion);
+            LOG.info("Deploying package {} (version {})", pkgName, deployTarget.getVersion());
+            return deploy(broker, transaction, deployTarget, null);
         }
 
         // Totally unnecessary to do the above if repo is unavailable.
@@ -273,6 +282,18 @@
         }
     }
 
+    private Package resolveInstalledVersion(final ExistRepository repo, final Package installed, final String requestedVersion) {
+        if (requestedVersion == null) {
+            return installed;
+        }
+        final Packages allVersions = repo.getParentRepo().getPackages(installed.getName());
+        if (allVersions == null) {
+            return installed;
+        }
+        final Package match = allVersions.version(requestedVersion);
+        return match != null ? match : installed;
+    }
+
     public Optional<String> undeploy(final DBBroker broker, final Txn transaction, final String pkgName, final Optional<ExistRepository> repo) throws PackageException {
         final Optional<Path> maybePackageDir = getPackageDir(pkgName, repo);
         if (maybePackageDir.isEmpty()) {
@@ -317,14 +338,38 @@
         return Optional.empty();
     }
 
+    /**
+     * Deploy a package by name. Resolves the package via {@code packages.latest()} for
+     * that name (see {@link #getPackage(String, Optional)}), which is appropriate for
+     * callers that explicitly want "the latest installed version of this package" —
+     * e.g., the user invoking {@code repo:deploy(name)} directly.
+     *
+     * NOT appropriate for the install-and-deploy flow when a specific version has just
+     * been installed alongside an existing higher version; that path should call
+     * {@link #deploy(DBBroker, Txn, Package, String)} with the
+     * freshly-installed package itself.
+     */
     public Optional<String> deploy(final DBBroker broker, final Txn transaction, final String pkgName, final Optional<ExistRepository> repo, final String userTarget) throws PackageException, IOException {
-        final Optional<Path> maybePackageDir = getPackageDir(pkgName, repo);
-        if (maybePackageDir.isEmpty()) {
+        final Optional<Package> pkg = getPackage(pkgName, repo);
+        if (pkg.isEmpty()) {
             throw new PackageException("Package not found: " + pkgName);
         }
+        return deploy(broker, transaction, pkg.get(), userTarget);
+    }
 
-        final Path packageDir = maybePackageDir.get();
-
+    /**
+     * Deploy a specific {@link Package}. Use this overload when the caller knows exactly
+     * which package version to deploy (e.g., the one it just installed) rather than
+     * wanting the {@code packages.latest()} for the given name.
+     *
+     * Introduced to fix the bug where {@code repo:install-and-deploy} with an explicit
+     * version argument would correctly install the requested version but then deploy the
+     * existing higher version instead, because the name-based lookup always returned
+     * {@code packages.latest()}.
+     */
+    public Optional<String> deploy(final DBBroker broker, final Txn transaction, final Package targetPkg, final String userTarget) throws PackageException, IOException {
+        final Path packageDir = getPackageDir(targetPkg);
+        final String pkgName = targetPkg.getName();
         final DocumentImpl repoXML = getRepoXML(broker, packageDir);
         if (repoXML == null) {
             return Optional.empty();
@@ -363,10 +408,13 @@
                 }
                 if (targetCollection == null) {
                     // no target means: package does not need to be deployed into database
-                    // however, we need to preserve a copy for backup purposes
-                    final Optional<Package> pkg = getPackage(pkgName, repo);
-                    pkg.orElseThrow(() -> new XPathException((Expression) null, "expath repository is not available so the package was not stored."));
-                    final String pkgColl = pkg.get().getAbbrev() + "-" + pkg.get().getVersion();
+                    // however, we need to preserve a copy for backup purposes.
+                    // Use targetPkg's abbrev+version directly rather than re-resolving
+                    // packages.latest() — when install-and-deploy was called with an
+                    // explicit older version, the registry's latest may be a different
+                    // (higher) version, and re-resolving would point this status target
+                    // at the wrong version's collection.
+                    final String pkgColl = targetPkg.getAbbrev() + "-" + targetPkg.getVersion();
                     targetCollection = XmldbURI.SYSTEM.append("repo/" + pkgColl);
                 }
 

extensions/modules/expathrepo/src/test/xquery/modules/expathrepo/deployment.xql

@@ -70,6 +70,39 @@ declare variable $deploy:entries-library := (
     <entry name="test.xml" type="xml"><test><foo/></test></entry>
 );
 
+(: Separate fixture for the version-routing regression test — uses a distinct
+   package name so leftover state doesn't pollute the dtest tests above. :)
+declare variable $deploy:expathxml-vtest-high :=
+    <package xmlns="http://expath.org/ns/pkg" name="http://exist-db.org/apps/dtest-versioning" abbrev="dtest-versioning" version="1.0.0" spec="1.0">
+        <title>Deployment Test (versioning)</title>
+        <dependency package="http://exist-db.org/html-templating" semver-min="1.0.2"/>
+    </package>;
+
+declare variable $deploy:expathxml-vtest-low :=
+    <package xmlns="http://expath.org/ns/pkg" name="http://exist-db.org/apps/dtest-versioning" abbrev="dtest-versioning" version="0.5.0" spec="1.0">
+        <title>Deployment Test (versioning, older)</title>
+        <dependency package="http://exist-db.org/html-templating" semver-min="1.0.2"/>
+    </package>;
+
+declare variable $deploy:repoxml-vtest :=
+    <meta xmlns="http://exist-db.org/xquery/repo">
+        <description>Deployment Test (versioning)</description>
+        <type>application</type>
+        <target>dtest-versioning</target>
+    </meta>;
+
+declare variable $deploy:entries-vtest-high := (
+    <entry name="expath-pkg.xml" type="xml">{$deploy:expathxml-vtest-high}</entry>,
+    <entry name="repo.xml" type="xml">{$deploy:repoxml-vtest}</entry>,
+    <entry name="test-high.xml" type="xml"><test><foo/></test></entry>
+);
+
+declare variable $deploy:entries-vtest-low := (
+    <entry name="expath-pkg.xml" type="xml">{$deploy:expathxml-vtest-low}</entry>,
+    <entry name="repo.xml" type="xml">{$deploy:repoxml-vtest}</entry>,
+    <entry name="test-low.xml" type="xml"><test><foo/></test></entry>
+);
+
 declare
     %test:setUp
 function deploy:setup() {
@@ -180,8 +213,61 @@ function deploy:install-uninstall-library() {
         $inList,
         $avail1,
         $avail2,
-        $undeploy/@result/string(), 
+        $undeploy/@result/string(),
         $remove,
         $avail3
     )
 };
+
+declare
+    %test:name("install-and-deploy respects the freshly installed package version (regression for Deployment.installAndDeploy deploying packages.latest() instead of the version just installed)")
+    %test:assertEquals("ok", "ok", "0.5.0", "true")
+function deploy:install-lower-version-after-higher() {
+    (: Use a distinct package name from the other tests (`dtest-versioning`,
+       not `dtest`) so leftover state doesn't pollute the rest of the suite. :)
+    let $pkg-name := "http://exist-db.org/apps/dtest-versioning"
+    let $target-coll := "/db/apps/dtest-versioning"
+
+    (: Install version 1.0.0 first. :)
+    let $zip-high := compression:zip($deploy:entries-vtest-high, false())
+    let $stored-high := xmldb:store("/db/deployment-test", "dtest-versioning-high", $zip-high)
+    let $deployed-high := repo:install-and-deploy-from-db($stored-high)
+
+    (: Then install version 0.5.0 (lower) while 1.0.0 is still present.
+       Before the fix, Deployment.installAndDeploy correctly fetched 0.5.0
+       but the subsequent deploy() step ran against packages.latest() (1.0.0)
+       and the user-visible deployed version stayed at 1.0.0. :)
+    let $zip-low := compression:zip($deploy:entries-vtest-low, false())
+    let $stored-low := xmldb:store("/db/deployment-test", "dtest-versioning-low", $zip-low)
+    let $deployed-low := repo:install-and-deploy-from-db($stored-low)
+
+    (: After the second install, the descriptor copied into the deployed target
+       collection should be 0.5.0's (the just-installed version), not 1.0.0's
+       (the existing higher version). Before the fix the second deploy step
+       used packages.latest() and copied 1.0.0's descriptor instead.
+       NOTE: we read /db/apps/<target>/expath-pkg.xml, NOT
+       repo:get-resource(name, "expath-pkg.xml") — the latter reads from
+       packages.latest()'s dir, not from the deployed target. :)
+    let $deployed-version := doc($target-coll || "/expath-pkg.xml")/*:package/@version/string()
+
+    (: Sanity check: the just-installed 0.5.0 XAR's distinctive resource
+       test-low.xml should be present in the deployed target collection.
+       (test-high.xml may also still be there from the prior 1.0.0 deploy —
+       deploy does not clean up old files — so its presence is not
+       informative either way.) :)
+    let $low-resource-deployed := exists(doc($target-coll || "/test-low.xml"))
+
+    (: Cleanup — best effort; loop until repo:list no longer contains the package
+       (both versions are now in the registry, so a single remove may not suffice). :)
+    let $_cleanup :=
+        for $attempt in 1 to 5
+        where exists(repo:list()[. = $pkg-name])
+        return (repo:undeploy($pkg-name), try { repo:remove($pkg-name) } catch * { () })
+
+    return (
+        $deployed-high/@result/string(),
+        $deployed-low/@result/string(),
+        $deployed-version,
+        string($low-resource-deployed)
+    )
+};