This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git
The following commit(s) were added to refs/heads/main by this push:
new d542652 Associate cache entries with profile and tool version used
d542652 is described below
commit d54265227edf5794891411ff482840662ec63bf1
Author: remm <[email protected]>
AuthorDate: Wed Sep 9 11:30:53 2026 +0200
Associate cache entries with profile and tool version used
Testing GLM for code review.
---
CHANGES.md | 2 +
.../apache/tomcat/jakartaee/MigrationCache.java | 49 ++++++++++++--
.../tomcat/jakartaee/MigrationCacheTest.java | 77 ++++++++++++++++++++++
3 files changed, 121 insertions(+), 7 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index dae9ba0..9f26f72 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -11,6 +11,8 @@
- Improve processing of relevant version numbers in manifests. (remm/markt)
- Avoid shallow copy style issue when converting manifests attributes. (remm)
- Make file extension check more robust. (markt)
+- Avoid shallow copy style issue when converting manifests attributes. (remm)
+- Associate cache entries with profile and tool version used. (remm)
## 1.0.12
- Add Maven Wrapper Plugin to manage the Maven wrapper. (markt)
diff --git a/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
b/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
index 499a8db..de79083 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
@@ -55,11 +55,15 @@ import java.util.regex.Pattern;
* <h2>Cache Key</h2>
* <p>Each cache entry is keyed by a SHA-256 hash computed from:</p>
* <ul>
- * <li>The migration profile name (e.g., "TOMCAT", "EE")</li>
+ * <li>The version of the migration tool (a different version may convert
+ * the same content differently)</li>
+ * <li>The name and definition (source, target and pattern) of the
+ * migration profile (e.g., "TOMCAT", "EE")</li>
* <li>The pre-conversion archive content (as bytes)</li>
* </ul>
- * <p>This ensures that the same archive converted with different profiles
- * produces different cache entries.</p>
+ * <p>This ensures that the same archive converted with different profiles,
+ * or with a different version of the tool, produces different cache
+ * entries.</p>
*
* <h2>Metadata Format</h2>
* <p>The {@code cache-metadata.txt} file tracks access times for cache
pruning:</p>
@@ -316,8 +320,11 @@ public class MigrationCache {
}
/**
- * Compute SHA-256 hash of the given bytes combined with the profile name.
- * The profile is included to ensure different profiles produce different
cache entries.
+ * Compute SHA-256 hash of the given bytes combined with the version of
+ * the tool and the profile definition.
+ * The tool version is included because a new version may convert the
+ * same content differently. The profile is included to ensure different
+ * profiles produce different cache entries.
*
* @param bytes the bytes to hash
* @param profile the migration profile
@@ -327,8 +334,7 @@ public class MigrationCache {
private String computeHash(byte[] bytes, EESpecProfile profile) throws
IOException {
try {
MessageDigest digest = MessageDigest.getInstance("SHA-256");
- // Include profile name in hash to differentiate between profiles
-
digest.update(profile.toString().getBytes(java.nio.charset.StandardCharsets.UTF_8));
+ digest.update(getHashKeyData(profile));
digest.update(bytes);
byte[] hashBytes = digest.digest();
@@ -343,6 +349,35 @@ public class MigrationCache {
}
}
+ /**
+ * Build the keying data that, together with the pre-conversion content,
+ * determines the cache hash. Cache entries outlive the process that
+ * created them so the key must include every input that determines the
+ * conversion output:
+ * <ul>
+ * <li>the tool version - so entries created by an older version of the
+ * tool are never used by a newer one (the two may convert the same
+ * content differently and the version is embedded in manifest
+ * attributes)</li>
+ * <li>the profile definition, in addition to the profile name - so
+ * that two EESpecProfile implementations that share a name but use
+ * different conversion definitions never collide</li>
+ * </ul>
+ * This must be kept consistent with any other implementation that
+ * computes cache hashes.
+ *
+ * @param profile the migration profile
+ * @return the keying data as UTF-8 bytes
+ */
+ static byte[] getHashKeyData(EESpecProfile profile) {
+ // Note that Pattern.toString() returns the pattern source and is
+ // deterministic
+ String key = Info.getVersion() + '-' + profile.toString() + '-' +
+ profile.getSource() + '-' + profile.getTarget() + '-' +
+ profile.getPattern();
+ return key.getBytes(java.nio.charset.StandardCharsets.UTF_8);
+ }
+
/**
* Clear the cache directory.
*
diff --git a/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
b/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
index df5dcf3..bbdadc5 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
@@ -25,6 +25,7 @@ import java.io.OutputStream;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.time.LocalDate;
+import java.util.regex.Pattern;
import org.apache.commons.io.FileUtils;
import org.junit.After;
@@ -388,6 +389,82 @@ public class MigrationCacheTest {
assertTrue("Should be cache hit for same profile", entry3.exists());
}
+ /**
+ * Two profiles that report the same name (toString()) but have different
+ * conversion definitions must not share cache entries. Without the
+ * profile definition (source, target, pattern) in the hash, one would
+ * corrupt the other's results. This is a regression test for that
+ * behaviour - previously only the profile name was part of the hash.
+ *
+ * @throws Exception if the test fails
+ */
+ @Test
+ public void testCacheDifferentProfilesWithSameName() throws Exception {
+ MigrationCache cache = new MigrationCache(tempCacheDir, 30);
+
+ byte[] sourceData = "test source
content".getBytes(StandardCharsets.UTF_8);
+ byte[] convertedData = "converted
content".getBytes(StandardCharsets.UTF_8);
+
+ EESpecProfile profileA = new EESpecProfile() {
+ @Override
+ public String getSource() {
+ return "javax";
+ }
+
+ @Override
+ public String getTarget() {
+ return "jakarta";
+ }
+
+ @Override
+ public Pattern getPattern() {
+ return Pattern.compile("javax([/\\.](servlet))");
+ }
+
+ @Override
+ public String toString() {
+ // Deliberately the same name as the other profile
+ return "SAME-NAME";
+ }
+ };
+
+ EESpecProfile profileB = new EESpecProfile() {
+ @Override
+ public String getSource() {
+ return "jakarta";
+ }
+
+ @Override
+ public String getTarget() {
+ return "javax";
+ }
+
+ @Override
+ public Pattern getPattern() {
+ return Pattern.compile("jakarta([/\\.](servlet))");
+ }
+
+ @Override
+ public String toString() {
+ // Deliberately the same name as the other profile
+ return "SAME-NAME";
+ }
+ };
+
+ // Store with one profile
+ CacheEntry entry1 = cache.getCacheEntry(sourceData, profileA);
+ try (OutputStream os = entry1.beginStore()) {
+ os.write(convertedData);
+ }
+ entry1.commitStore();
+
+ // The same name but a different definition - must not be served
+ // from the cache
+ CacheEntry entry2 = cache.getCacheEntry(sourceData, profileB);
+ assertFalse("Profile with same name but different definition must not
use another profile's cache entry",
+ entry2.exists());
+ }
+
@Test
public void testCacheCorruptMetadata() throws Exception {
// Create a corrupt metadata file
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
