Re-seed bundled core packages when content changes at the same version

beast-pkgmgmt/src/main/java/beast/pkgmgmt/launcher/BeastLauncher.java

@@ -12,6 +12,8 @@
 import java.lang.reflect.Method;
 import java.net.URL;
 import java.net.URLDecoder;
+import java.nio.file.Files;
+import java.security.MessageDigest;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -191,14 +193,33 @@ private static void copyFileUsingStream(File source, File dest) throws IOExcepti
 		}
 	}
 
+	/** Name of the marker file holding the checksum of the bundled package zip
+	 *  that was last extracted into a user package directory. */
+	private static final String BUNDLED_CHECKSUM_FILE = ".bundled.checksum";
+
 	/**
 	 * Seed a bundled core package (e.g. BEAST.base, BEAST.app) into the user
 	 * package directory. The application bundle ships these as full package zips
-	 * under {@code <app dir>/packages/}. On first run -- or whenever the bundled
-	 * version is newer than the copy already in the user dir -- the zip is
-	 * extracted into {@code <user package dir>/<packageName>/}. A user copy that
-	 * is the same or newer (e.g. an in-place upgrade applied by the package
-	 * manager) is left untouched, so upgrades survive application relaunches.
+	 * under {@code <app dir>/packages/}. The zip is extracted into
+	 * {@code <user package dir>/<packageName>/} on first run, and re-extracted
+	 * whenever its contents differ from the copy already seeded there.
+	 *
+	 * <p>The version number alone is not enough to decide, because the package
+	 * version string ({@code version.xml}) does not change between development
+	 * rebuilds (it stays e.g. {@code 2.8.0} for the whole release cycle) nor
+	 * between re-released betas. A purely version-based check would leave a stale
+	 * {@code lib/} in place after {@code mvn package} + a fresh application build,
+	 * so the running app would keep loading the old classes even though the
+	 * source was fixed. So when the user copy reports the <em>same</em> version
+	 * as the bundled one, we additionally compare a checksum of the bundled zip
+	 * against the checksum recorded when the user copy was seeded (stored in
+	 * {@link #BUNDLED_CHECKSUM_FILE}); a mismatch triggers re-seeding.
+	 *
+	 * <p>The other cases are decided by version alone: a user copy reporting a
+	 * strictly newer version (e.g. an in-place upgrade applied by the package
+	 * manager) is never downgraded, and a strictly older one is always replaced.
+	 * The (potentially large) checksum is therefore only computed when the
+	 * versions match, or when re-seeding, to record the marker for next time.
 	 *
 	 * @return true if a bundled package zip was found and handled (callers can
 	 *         then skip the legacy single-jar seeding); false if no bundled zip
@@ -215,13 +236,29 @@ static boolean seedBundledPackage(String packageName) {
 
 			File pkgDir = new File(getPackageUserDir() + pathDelimiter + packageName);
 			File userVersionFile = new File(pkgDir, "version.xml");
+			File checksumFile = new File(pkgDir, BUNDLED_CHECKSUM_FILE);
+			String bundledChecksum = null; // computed lazily -- only when needed
 			if (userVersionFile.exists()) {
 				double userVersion = parseVersion(readVersion(new FileReader(userVersionFile)));
-				if (userVersion >= bundledVersion) {
-					// user dir already holds a same-or-newer copy -- keep it
+				if (userVersion > bundledVersion) {
+					// user dir holds a strictly newer copy (e.g. a package-manager
+					// upgrade) -- never downgrade it
 					return true;
 				}
-				// bundled copy is newer (app was updated): replace the stale lib
+				if (userVersion == bundledVersion) {
+					// Same version string, but content may still differ (a
+					// development rebuild or re-released beta). Fall back to a
+					// content checksum to decide whether the seeded copy is stale.
+					bundledChecksum = checksum(bundledZip);
+					if (bundledChecksum != null
+							&& bundledChecksum.equals(readChecksumFile(checksumFile))) {
+						// already seeded from this exact zip -- nothing to do
+						return true;
+					}
+				}
+				// The bundled copy is a newer version (app was updated), or it is
+				// the same version with different content. Replace the stale lib so
+				// the freshly built classes are the ones that load.
 				File libDir = new File(pkgDir, "lib");
 				if (libDir.exists()) {
 					deleteDir(libDir);
@@ -232,6 +269,10 @@ static boolean seedBundledPackage(String packageName) {
 				return false;
 			}
 			PackageManager.doUnzip(bundledZip.getAbsolutePath(), pkgDir.getAbsolutePath());
+			if (bundledChecksum == null) {
+				bundledChecksum = checksum(bundledZip);
+			}
+			writeChecksumFile(checksumFile, bundledChecksum);
 			return true;
 		} catch (Exception e) {
 			// never let seeding hold up launch of BEAST & friends
@@ -240,6 +281,50 @@ static boolean seedBundledPackage(String packageName) {
 		}
 	}
 
+	/** SHA-256 of a file's bytes as a lowercase hex string, or null on failure.
+	 *  Used to detect when a bundled package zip differs from the copy already
+	 *  seeded into the user dir, even when the version string is unchanged. */
+	private static String checksum(File file) {
+		try (InputStream in = new BufferedInputStream(new FileInputStream(file))) {
+			MessageDigest md = MessageDigest.getInstance("SHA-256");
+			byte[] buf = new byte[1 << 16];
+			int n;
+			while ((n = in.read(buf)) > 0) {
+				md.update(buf, 0, n);
+			}
+			StringBuilder sb = new StringBuilder();
+			for (byte b : md.digest()) {
+				sb.append(Character.forDigit((b >> 4) & 0xf, 16));
+				sb.append(Character.forDigit(b & 0xf, 16));
+			}
+			return sb.toString();
+		} catch (Exception e) {
+			return null;
+		}
+	}
+
+	/** Read a previously written checksum marker, or null if absent/unreadable. */
+	private static String readChecksumFile(File f) {
+		try {
+			return f.exists() ? Files.readString(f.toPath()).trim() : null;
+		} catch (IOException e) {
+			return null;
+		}
+	}
+
+	/** Record the bundled zip's checksum alongside the extracted package. A
+	 *  failure here is non-fatal: the worst case is a needless re-seed next launch. */
+	private static void writeChecksumFile(File f, String checksum) {
+		if (checksum == null) {
+			return;
+		}
+		try {
+			Files.writeString(f.toPath(), checksum);
+		} catch (IOException e) {
+			// non-fatal
+		}
+	}
+
 	/**
 	 * Locate a bundled package zip (named {@code <packageName>.package*.zip}) by
 	 * walking up from the launcher jar looking for a {@code packages} directory.

beast-pkgmgmt/src/test/java/beast/pkgmgmt/launcher/SeedBundledPackageReseedTest.java

@@ -0,0 +1,147 @@
+package beast.pkgmgmt.launcher;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Regression test for the BEAUti-template staleness bug (issue #112): a bundled
+ * core package whose contents changed but whose version string did not (the
+ * normal situation during a development cycle, where {@code version.xml} stays
+ * e.g. {@code 2.8.0}) must still replace the stale copy already seeded into the
+ * user package directory. A purely version-based check left the old {@code lib/}
+ * in place, so the running app kept loading pre-fix classes.
+ *
+ * <p>Self-contained: it fabricates its own bundled package zips under a
+ * {@code packages/} directory in the launcher-jar parent chain, using a private
+ * package name so it never collides with real staged core packages, and removes
+ * them afterwards.
+ */
+public class SeedBundledPackageReseedTest {
+
+    private static final String PKG = "BEAST.reseedtest";
+
+    private Path userDir;
+    private String prevUserDir;
+    private File packagesDir;
+    private boolean createdPackagesDir;
+
+    @BeforeEach
+    public void setUp() throws Exception {
+        userDir = Files.createTempDirectory("beast-reseed-user");
+        prevUserDir = System.getProperty("beast.user.package.dir");
+        System.setProperty("beast.user.package.dir", userDir.toString());
+
+        // seedBundledPackage discovers zips by walking up from the launcher class
+        // location looking for a packages/ directory; create one there.
+        File codeSource = new File(BeastLauncher.class.getProtectionDomain()
+                .getCodeSource().getLocation().toURI());
+        packagesDir = new File(codeSource.getParentFile(), "packages");
+        createdPackagesDir = !packagesDir.exists();
+        packagesDir.mkdirs();
+    }
+
+    @AfterEach
+    public void tearDown() throws Exception {
+        File[] mine = packagesDir.listFiles((d, n) -> n.startsWith(PKG + ".package"));
+        if (mine != null) {
+            for (File f : mine) f.delete();
+        }
+        if (createdPackagesDir) {
+            packagesDir.delete(); // only removes it if now empty
+        }
+        if (prevUserDir == null) {
+            System.clearProperty("beast.user.package.dir");
+        } else {
+            System.setProperty("beast.user.package.dir", prevUserDir);
+        }
+        deleteRecursively(userDir.toFile());
+    }
+
+    @Test
+    public void sameVersionDifferentContentTriggersReseed() throws Exception {
+        writeBundle("2.8.0", "OLD");
+        assertTrue(BeastLauncher.seedBundledPackage(PKG), "first seed should succeed");
+        assertEquals("OLD", markerContent(), "first run extracts the bundled content");
+
+        // rebuild: same version string, different content (the #112 scenario)
+        writeBundle("2.8.0", "NEW");
+        assertTrue(BeastLauncher.seedBundledPackage(PKG), "re-seed should succeed");
+        assertEquals("NEW", markerContent(),
+                "same version with new content must re-seed, not keep the stale copy");
+    }
+
+    @Test
+    public void identicalBundleIsNotReExtracted() throws Exception {
+        writeBundle("2.8.0", "A");
+        BeastLauncher.seedBundledPackage(PKG);
+
+        // mutate the extracted copy, then seed the *same* zip again
+        Files.writeString(markerPath(), "LOCALLY-CHANGED");
+        BeastLauncher.seedBundledPackage(PKG);
+        assertEquals("LOCALLY-CHANGED", markerContent(),
+                "an unchanged bundled zip must not be re-extracted");
+    }
+
+    @Test
+    public void strictlyNewerUserCopyIsNotDowngraded() throws Exception {
+        writeBundle("2.9.0", "USER_NEWER");
+        BeastLauncher.seedBundledPackage(PKG);
+        Files.writeString(markerPath(), "KEEP_ME");
+
+        // bundled package is an older version -- must not clobber the user copy
+        writeBundle("2.8.0", "BUNDLED_OLDER");
+        BeastLauncher.seedBundledPackage(PKG);
+        assertEquals("KEEP_ME", markerContent(),
+                "a strictly newer user copy must never be downgraded");
+    }
+
+    // ---- helpers ----
+
+    private Path markerPath() {
+        return userDir.resolve(PKG).resolve("lib").resolve("marker.txt");
+    }
+
+    private String markerContent() throws Exception {
+        return Files.readString(markerPath(), StandardCharsets.UTF_8);
+    }
+
+    /** Write <packagesDir>/BEAST.reseedtest.package.v<version>.zip holding a
+     *  version.xml and a lib/marker.txt with the given content. The app ships
+     *  exactly one version of each core package, so older test zips are removed. */
+    private void writeBundle(String version, String content) throws Exception {
+        File[] old = packagesDir.listFiles((d, n) -> n.startsWith(PKG + ".package"));
+        if (old != null) {
+            for (File f : old) f.delete();
+        }
+        File zip = new File(packagesDir, PKG + ".package.v" + version + ".zip");
+        try (ZipOutputStream z = new ZipOutputStream(new FileOutputStream(zip))) {
+            z.putNextEntry(new ZipEntry("version.xml"));
+            z.write(("<package name=\"" + PKG + "\" version=\"" + version + "\"></package>")
+                    .getBytes(StandardCharsets.UTF_8));
+            z.closeEntry();
+            z.putNextEntry(new ZipEntry("lib/marker.txt"));
+            z.write(content.getBytes(StandardCharsets.UTF_8));
+            z.closeEntry();
+        }
+    }
+
+    private static void deleteRecursively(File f) {
+        File[] children = f.listFiles();
+        if (children != null) {
+            for (File c : children) deleteRecursively(c);
+        }
+        f.delete();
+    }
+}