Skip to content

Commit d4ddee9

Browse files
joewizclaude
andcommitted
[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>
1 parent 9057c82 commit d4ddee9

2 files changed

Lines changed: 143 additions & 10 deletions

File tree

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

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -255,8 +255,16 @@ public Optional<String> installAndDeploy(final DBBroker broker, final Txn transa
255255
broker.getBrokerPool().reportStatus("Installing app: " + pkg.getAbbrev());
256256
repo.get().reportAction(ExistRepository.Action.INSTALL, pkg.getName());
257257

258-
LOG.info("Deploying package {}", pkgName);
259-
return deploy(broker, transaction, pkgName, repo, null);
258+
// installPackage may return packages.latest() (the highest-version
259+
// entry by semver) rather than the just-installed package when the
260+
// registry already had a higher version with the same name. Deploy
261+
// the freshly-installed package specifically (looked up by the
262+
// version we extracted from the XAR descriptor above) so an explicit
263+
// repo:install-and-deploy(name, "<older>", url) is not silently
264+
// upgraded to packages.latest().
265+
final Package deployTarget = resolveInstalledVersion(repo.get(), pkg, pkgVersion);
266+
LOG.info("Deploying package {} (version {})", pkgName, deployTarget.getVersion());
267+
return deploy(broker, transaction, deployTarget, null);
260268
}
261269

262270
// Totally unnecessary to do the above if repo is unavailable.
@@ -273,6 +281,18 @@ private void checkProcessorVersion(final PackageLoader.Version version) throws P
273281
}
274282
}
275283

284+
private Package resolveInstalledVersion(final ExistRepository repo, final Package installed, final String requestedVersion) {
285+
if (requestedVersion == null) {
286+
return installed;
287+
}
288+
final Packages allVersions = repo.getParentRepo().getPackages(installed.getName());
289+
if (allVersions == null) {
290+
return installed;
291+
}
292+
final Package match = allVersions.version(requestedVersion);
293+
return match != null ? match : installed;
294+
}
295+
276296
public Optional<String> undeploy(final DBBroker broker, final Txn transaction, final String pkgName, final Optional<ExistRepository> repo) throws PackageException {
277297
final Optional<Path> maybePackageDir = getPackageDir(pkgName, repo);
278298
if (maybePackageDir.isEmpty()) {
@@ -317,14 +337,38 @@ public Optional<String> undeploy(final DBBroker broker, final Txn transaction, f
317337
return Optional.empty();
318338
}
319339

340+
/**
341+
* Deploy a package by name. Resolves the package via {@code packages.latest()} for
342+
* that name (see {@link #getPackage(String, Optional)}), which is appropriate for
343+
* callers that explicitly want "the latest installed version of this package" —
344+
* e.g., the user invoking {@code repo:deploy(name)} directly.
345+
*
346+
* NOT appropriate for the install-and-deploy flow when a specific version has just
347+
* been installed alongside an existing higher version; that path should call
348+
* {@link #deploy(DBBroker, Txn, Package, String)} with the
349+
* freshly-installed package itself.
350+
*/
320351
public Optional<String> deploy(final DBBroker broker, final Txn transaction, final String pkgName, final Optional<ExistRepository> repo, final String userTarget) throws PackageException, IOException {
321-
final Optional<Path> maybePackageDir = getPackageDir(pkgName, repo);
322-
if (maybePackageDir.isEmpty()) {
352+
final Optional<Package> pkg = getPackage(pkgName, repo);
353+
if (pkg.isEmpty()) {
323354
throw new PackageException("Package not found: " + pkgName);
324355
}
356+
return deploy(broker, transaction, pkg.get(), userTarget);
357+
}
325358

326-
final Path packageDir = maybePackageDir.get();
327-
359+
/**
360+
* Deploy a specific {@link Package}. Use this overload when the caller knows exactly
361+
* which package version to deploy (e.g., the one it just installed) rather than
362+
* wanting the {@code packages.latest()} for the given name.
363+
*
364+
* Introduced to fix the bug where {@code repo:install-and-deploy} with an explicit
365+
* version argument would correctly install the requested version but then deploy the
366+
* existing higher version instead, because the name-based lookup always returned
367+
* {@code packages.latest()}.
368+
*/
369+
public Optional<String> deploy(final DBBroker broker, final Txn transaction, final Package targetPkg, final String userTarget) throws PackageException, IOException {
370+
final Path packageDir = getPackageDir(targetPkg);
371+
final String pkgName = targetPkg.getName();
328372
final DocumentImpl repoXML = getRepoXML(broker, packageDir);
329373
if (repoXML == null) {
330374
return Optional.empty();
@@ -363,10 +407,13 @@ public Optional<String> deploy(final DBBroker broker, final Txn transaction, fin
363407
}
364408
if (targetCollection == null) {
365409
// no target means: package does not need to be deployed into database
366-
// however, we need to preserve a copy for backup purposes
367-
final Optional<Package> pkg = getPackage(pkgName, repo);
368-
pkg.orElseThrow(() -> new XPathException((Expression) null, "expath repository is not available so the package was not stored."));
369-
final String pkgColl = pkg.get().getAbbrev() + "-" + pkg.get().getVersion();
410+
// however, we need to preserve a copy for backup purposes.
411+
// Use targetPkg's abbrev+version directly rather than re-resolving
412+
// packages.latest() — when install-and-deploy was called with an
413+
// explicit older version, the registry's latest may be a different
414+
// (higher) version, and re-resolving would point this status target
415+
// at the wrong version's collection.
416+
final String pkgColl = targetPkg.getAbbrev() + "-" + targetPkg.getVersion();
370417
targetCollection = XmldbURI.SYSTEM.append("repo/" + pkgColl);
371418
}
372419

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

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,39 @@ declare variable $deploy:entries-library := (
7070
<entry name="test.xml" type="xml"><test><foo/></test></entry>
7171
);
7272

73+
(: Separate fixture for the version-routing regression test — uses a distinct
74+
package name so leftover state doesn't pollute the dtest tests above. :)
75+
declare variable $deploy:expathxml-vtest-high :=
76+
<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">
77+
<title>Deployment Test (versioning)</title>
78+
<dependency package="http://exist-db.org/html-templating" semver-min="1.0.2"/>
79+
</package>;
80+
81+
declare variable $deploy:expathxml-vtest-low :=
82+
<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">
83+
<title>Deployment Test (versioning, older)</title>
84+
<dependency package="http://exist-db.org/html-templating" semver-min="1.0.2"/>
85+
</package>;
86+
87+
declare variable $deploy:repoxml-vtest :=
88+
<meta xmlns="http://exist-db.org/xquery/repo">
89+
<description>Deployment Test (versioning)</description>
90+
<type>application</type>
91+
<target>dtest-versioning</target>
92+
</meta>;
93+
94+
declare variable $deploy:entries-vtest-high := (
95+
<entry name="expath-pkg.xml" type="xml">{$deploy:expathxml-vtest-high}</entry>,
96+
<entry name="repo.xml" type="xml">{$deploy:repoxml-vtest}</entry>,
97+
<entry name="test-high.xml" type="xml"><test><foo/></test></entry>
98+
);
99+
100+
declare variable $deploy:entries-vtest-low := (
101+
<entry name="expath-pkg.xml" type="xml">{$deploy:expathxml-vtest-low}</entry>,
102+
<entry name="repo.xml" type="xml">{$deploy:repoxml-vtest}</entry>,
103+
<entry name="test-low.xml" type="xml"><test><foo/></test></entry>
104+
);
105+
73106
declare
74107
%test:setUp
75108
function deploy:setup() {
@@ -192,3 +225,56 @@ declare
192225
function deploy:remove-unknown-throws() {
193226
repo:remove("http://exist-db.org/apps/this-package-does-not-exist-and-never-did")
194227
};
228+
229+
declare
230+
%test:name("install-and-deploy respects the freshly installed package version (regression for Deployment.installAndDeploy deploying packages.latest() instead of the version just installed)")
231+
%test:assertEquals("ok", "ok", "0.5.0", "true")
232+
function deploy:install-lower-version-after-higher() {
233+
(: Use a distinct package name from the other tests (`dtest-versioning`,
234+
not `dtest`) so leftover state doesn't pollute the rest of the suite. :)
235+
let $pkg-name := "http://exist-db.org/apps/dtest-versioning"
236+
let $target-coll := "/db/apps/dtest-versioning"
237+
238+
(: Install version 1.0.0 first. :)
239+
let $zip-high := compression:zip($deploy:entries-vtest-high, false())
240+
let $stored-high := xmldb:store("/db/deployment-test", "dtest-versioning-high", $zip-high)
241+
let $deployed-high := repo:install-and-deploy-from-db($stored-high)
242+
243+
(: Then install version 0.5.0 (lower) while 1.0.0 is still present.
244+
Before the fix, Deployment.installAndDeploy correctly fetched 0.5.0
245+
but the subsequent deploy() step ran against packages.latest() (1.0.0)
246+
and the user-visible deployed version stayed at 1.0.0. :)
247+
let $zip-low := compression:zip($deploy:entries-vtest-low, false())
248+
let $stored-low := xmldb:store("/db/deployment-test", "dtest-versioning-low", $zip-low)
249+
let $deployed-low := repo:install-and-deploy-from-db($stored-low)
250+
251+
(: After the second install, the descriptor copied into the deployed target
252+
collection should be 0.5.0's (the just-installed version), not 1.0.0's
253+
(the existing higher version). Before the fix the second deploy step
254+
used packages.latest() and copied 1.0.0's descriptor instead.
255+
NOTE: we read /db/apps/<target>/expath-pkg.xml, NOT
256+
repo:get-resource(name, "expath-pkg.xml") — the latter reads from
257+
packages.latest()'s dir, not from the deployed target. :)
258+
let $deployed-version := doc($target-coll || "/expath-pkg.xml")/*:package/@version/string()
259+
260+
(: Sanity check: the just-installed 0.5.0 XAR's distinctive resource
261+
test-low.xml should be present in the deployed target collection.
262+
(test-high.xml may also still be there from the prior 1.0.0 deploy —
263+
deploy does not clean up old files — so its presence is not
264+
informative either way.) :)
265+
let $low-resource-deployed := exists(doc($target-coll || "/test-low.xml"))
266+
267+
(: Cleanup — best effort; loop until repo:list no longer contains the package
268+
(both versions are now in the registry, so a single remove may not suffice). :)
269+
let $_cleanup :=
270+
for $attempt in 1 to 5
271+
where exists(repo:list()[. = $pkg-name])
272+
return (repo:undeploy($pkg-name), try { repo:remove($pkg-name) } catch * { () })
273+
274+
return (
275+
$deployed-high/@result/string(),
276+
$deployed-low/@result/string(),
277+
$deployed-version,
278+
string($low-resource-deployed)
279+
)
280+
};

0 commit comments

Comments
 (0)