From 1d06f173ae7dd8dc67a01b5a9ce774805164f832 Mon Sep 17 00:00:00 2001 From: Joe Wicentowski Date: Tue, 2 Jun 2026 15:18:44 -0400 Subject: [PATCH] [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, "", 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 --- .../main/java/org/exist/repo/Deployment.java | 68 +++++++++++--- .../xquery/modules/expathrepo/deployment.xql | 88 ++++++++++++++++++- 2 files changed, 145 insertions(+), 11 deletions(-) diff --git a/exist-core/src/main/java/org/exist/repo/Deployment.java b/exist-core/src/main/java/org/exist/repo/Deployment.java index 15c0ac43f71..be64decdab9 100644 --- a/exist-core/src/main/java/org/exist/repo/Deployment.java +++ b/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 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, "", 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 undeploy(final DBBroker broker, final Txn transaction, final String pkgName, final Optional repo) throws PackageException { final Optional maybePackageDir = getPackageDir(pkgName, repo); if (maybePackageDir.isEmpty()) { @@ -317,14 +338,38 @@ public Optional 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 deploy(final DBBroker broker, final Txn transaction, final String pkgName, final Optional repo, final String userTarget) throws PackageException, IOException { - final Optional maybePackageDir = getPackageDir(pkgName, repo); - if (maybePackageDir.isEmpty()) { + final Optional 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 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 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 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); } diff --git a/extensions/modules/expathrepo/src/test/xquery/modules/expathrepo/deployment.xql b/extensions/modules/expathrepo/src/test/xquery/modules/expathrepo/deployment.xql index ae1aa283b85..da48aa8ec55 100644 --- a/extensions/modules/expathrepo/src/test/xquery/modules/expathrepo/deployment.xql +++ b/extensions/modules/expathrepo/src/test/xquery/modules/expathrepo/deployment.xql @@ -70,6 +70,39 @@ declare variable $deploy:entries-library := ( ); +(: 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 := + + Deployment Test (versioning) + + ; + +declare variable $deploy:expathxml-vtest-low := + + Deployment Test (versioning, older) + + ; + +declare variable $deploy:repoxml-vtest := + + Deployment Test (versioning) + application + dtest-versioning + ; + +declare variable $deploy:entries-vtest-high := ( + {$deploy:expathxml-vtest-high}, + {$deploy:repoxml-vtest}, + +); + +declare variable $deploy:entries-vtest-low := ( + {$deploy:expathxml-vtest-low}, + {$deploy:repoxml-vtest}, + +); + 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//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) + ) +};