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

When a higher version of a package is already in the EXPath registry,
calling repo:install-and-deploy(name, "<lower>", url) would correctly
fetch the requested XAR and install it into the registry, but the
subsequent deploy() step would re-resolve by name via packages.latest()
and deploy the existing higher version's directory instead. The user
sees a successful "ok" status, but the deployed application is the
higher version, not the one they asked for.

Two changes:

1. Deployment.deploy(...) now has a Package-based overload that takes
   the freshly-installed Package directly. The by-name overload is kept
   and delegates to the new one via getPackage(pkgName, repo).

2. installAndDeploy(...) looks up the package by the version extracted
   from the XAR descriptor (via Packages.version(...)) and passes it to
   the Package-based deploy overload, bypassing packages.latest().

Includes XQSuite regression test (deploy:install-lower-version-after-higher):
install 1.0.0, then install 0.5.0 over it, and verify the deployed
expath-pkg.xml in the target collection is 0.5.0's, not 1.0.0's.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: joewiz

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 @@ public Optional<String> installAndDeploy(final DBBroker broker, final Txn transa
             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 void checkProcessorVersion(final PackageLoader.Version version) throws P
         }
     }
 
+    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 @@ public Optional<String> undeploy(final DBBroker broker, final Txn transaction, f
         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 @@ public Optional<String> deploy(final DBBroker broker, final Txn transaction, fin
                 }
                 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() {
@@ -192,3 +225,56 @@ declare
 function deploy:remove-unknown-throws() {
     repo:remove("http://exist-db.org/apps/this-package-does-not-exist-and-never-did")
 };
+
+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)
+    )
+};