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) + ) +};