This is an automated email from the ASF dual-hosted git repository.
markt 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 d52d7c6 Follow-on to PR #87
d52d7c6 is described below
commit d52d7c6dc4478d486de5d7f635b5d940aba74b6a
Author: Mark Thomas <[email protected]>
AuthorDate: Thu Nov 20 17:35:16 2025 +0000
Follow-on to PR #87
---
CHANGES.md | 1 +
.../org/apache/tomcat/jakartaee/CacheEntry.java | 9 ++-
.../org/apache/tomcat/jakartaee/MigrationCLI.java | 6 +-
.../apache/tomcat/jakartaee/MigrationCache.java | 72 ++++++++--------------
.../tomcat/jakartaee/LocalStrings.properties | 8 ++-
.../tomcat/jakartaee/MigrationCacheTest.java | 32 ++--------
6 files changed, 46 insertions(+), 82 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 72aeb6b..3695dc6 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -4,6 +4,7 @@
- When migrating files in place, don't replace the original file if no
conversion has taken place. Based on PR[#78] by Semiao Marco.
- When converting a file in an archive, update the last modified time for that
archive entry. Based on PR[#78] by Semiao Marco.
- Correctly handle OSGi headers. PR[#54] by Kyle Smith.
+- Add an option to cache migrated JARs. PR[#87] by Aaron Cosand.
- Update ASF parent POM 34. (dependabot/markt)
- Update Commons BCEL to 6.11.0. (dependabot/remm)
- Update Commons Compress to 1.28.0. (dependabot/remm)
diff --git a/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
b/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
index 710de8d..b49ca97 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
@@ -29,6 +29,9 @@ import org.apache.commons.io.IOUtils;
* Package-private - only created by MigrationCache.
*/
class CacheEntry {
+
+ private static final StringManager sm =
StringManager.getManager(CacheEntry.class);
+
private final String hash;
private final boolean exists;
private final File cacheFile;
@@ -64,7 +67,7 @@ class CacheEntry {
*/
public void copyToDestination(OutputStream dest) throws IOException {
if (!exists) {
- throw new IllegalStateException("Cannot copy - cache entry does
not exist");
+ throw new
IllegalStateException(sm.getString("cacheEntry.copyNotExist"));
}
try (FileInputStream fis = new FileInputStream(cacheFile)) {
IOUtils.copy(fis, dest);
@@ -86,7 +89,7 @@ class CacheEntry {
*/
public void commitStore() throws IOException {
if (!tempFile.exists()) {
- throw new IOException("Temp file does not exist: " + tempFile);
+ throw new IOException(sm.getString("cacheEntry.tempNotExist",
tempFile));
}
// Ensure parent directory exists
File parentDir = cacheFile.getParentFile();
@@ -95,7 +98,7 @@ class CacheEntry {
}
// Atomic rename
if (!tempFile.renameTo(cacheFile)) {
- throw new IOException("Failed to rename temp file to cache file: "
+ tempFile + " -> " + cacheFile);
+ throw new IOException(sm.getString("cacheEntry.tempRenameFail",
tempFile, cacheFile));
}
}
diff --git a/src/main/java/org/apache/tomcat/jakartaee/MigrationCLI.java
b/src/main/java/org/apache/tomcat/jakartaee/MigrationCLI.java
index 6f1a117..b2c90dd 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/MigrationCLI.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/MigrationCLI.java
@@ -58,12 +58,12 @@ public class MigrationCLI {
System.setProperty("java.util.logging.SimpleFormatter.format",
"%5$s%n");
Migration migration = new Migration();
- // Cache settings - opt-in by default is false
+ // Cache settings - disabled by default
File cacheDir = null;
boolean enableCache = false;
int cacheRetentionDays = 30; // Default retention period
- // Process argumnets
+ // Process arguments
List<String> arguments = new ArrayList<>(Arrays.asList(args));
// Process the custom log level if present
@@ -112,6 +112,7 @@ public class MigrationCLI {
}
} else if (argument.startsWith(CACHE_LOCATION_ARG)) {
iter.remove();
+ enableCache = true;
String cachePath =
argument.substring(CACHE_LOCATION_ARG.length());
cacheDir = new File(cachePath);
} else if (argument.startsWith(CACHE_RETENTION_ARG)) {
@@ -138,7 +139,6 @@ public class MigrationCLI {
migration.setSource(new File(source));
migration.setDestination(new File(dest));
- // Only enable cache if -cache argument was provided
if (enableCache) {
MigrationCache migrationCache = new MigrationCache(cacheDir,
cacheRetentionDays);
migration.setCache(migrationCache);
diff --git a/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
b/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
index d7bde18..1af6070 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
@@ -97,26 +97,29 @@ public class MigrationCache {
this.cacheDir = cacheDir;
this.metadataFile = cacheDir == null ? null : new File(cacheDir,
METADATA_FILE);
- if (cacheDir != null) {
- // Create cache directory if it doesn't exist
- if (!cacheDir.exists()) {
- if (!cacheDir.mkdirs()) {
- throw new IOException(sm.getString("cache.cannotCreate",
cacheDir.getAbsolutePath()));
- }
- }
+ if (cacheDir == null) {
+ throw new
IllegalStateException(sm.getString("cache.nullDirectory"));
+ }
- if (!cacheDir.isDirectory()) {
- throw new IOException(sm.getString("cache.notDirectory",
cacheDir.getAbsolutePath()));
+ // Create cache directory if it doesn't exist
+ if (!cacheDir.exists()) {
+ if (!cacheDir.mkdirs()) {
+ throw new IOException(sm.getString("cache.cannotCreate",
cacheDir.getAbsolutePath()));
}
+ }
+
+ if (!cacheDir.isDirectory()) {
+ throw new IOException(sm.getString("cache.notDirectory",
cacheDir.getAbsolutePath()));
+ }
- // Load existing metadata
- loadMetadata();
+ // Load existing metadata
+ loadMetadata();
- // Clean up any orphaned temp files from previous crashes
- cleanupTempFiles();
+ // Clean up any orphaned temp files from previous crashes
+ cleanupTempFiles();
- logger.log(Level.INFO, sm.getString("cache.enabled",
cacheDir.getAbsolutePath(), retentionDays));
- }
+ logger.log(Level.INFO,
+ sm.getString("cache.enabled", cacheDir.getAbsolutePath(),
Integer.valueOf(retentionDays)));
}
/**
@@ -138,7 +141,7 @@ public class MigrationCache {
}
}
if (cleanedCount > 0) {
- logger.log(Level.INFO, sm.getString("cache.tempfiles.cleaned",
cleanedCount));
+ logger.log(Level.INFO, sm.getString("cache.tempfiles.cleaned",
Integer.valueOf(cleanedCount)));
}
}
}
@@ -188,7 +191,7 @@ public class MigrationCache {
}
}
- logger.log(Level.FINE, sm.getString("cache.metadata.loaded",
cacheMetadata.size()));
+ logger.log(Level.FINE, sm.getString("cache.metadata.loaded",
Integer.valueOf(cacheMetadata.size())));
} catch (IOException e) {
// Corrupt or unreadable - assume all cached files accessed today
logger.log(Level.WARNING,
sm.getString("cache.metadata.loadError"), e);
@@ -240,10 +243,6 @@ public class MigrationCache {
* @throws IOException if an I/O error occurs
*/
public CacheEntry getCacheEntry(byte[] sourceBytes, EESpecProfile profile)
throws IOException {
- if (cacheDir == null) {
- throw new IllegalStateException("Cache is not enabled");
- }
-
// Compute hash once (includes profile)
String hash = computeHash(sourceBytes, profile);
@@ -297,7 +296,7 @@ public class MigrationCache {
// Convert to hex string
StringBuilder sb = new StringBuilder();
for (byte b : hashBytes) {
- sb.append(String.format("%02x", b));
+ sb.append(String.format("%02x", Byte.valueOf(b)));
}
return sb.toString();
} catch (NoSuchAlgorithmException e) {
@@ -311,10 +310,6 @@ public class MigrationCache {
* @throws IOException if an I/O error occurs
*/
public void clear() throws IOException {
- if (cacheDir == null) {
- return;
- }
-
deleteDirectory(cacheDir);
cacheDir.mkdirs();
logger.log(Level.INFO, sm.getString("cache.cleared"));
@@ -356,10 +351,6 @@ public class MigrationCache {
* @throws IOException if an I/O error occurs
*/
private void saveMetadata() throws IOException {
- if (cacheDir == null) {
- return;
- }
-
try (BufferedWriter writer = new BufferedWriter(new
FileWriter(metadataFile))) {
writer.write("# Migration cache metadata -
hash|last_access_date\n");
for (Map.Entry<String, LocalDate> entry :
cacheMetadata.entrySet()) {
@@ -370,7 +361,7 @@ public class MigrationCache {
}
}
- logger.log(Level.FINE, sm.getString("cache.metadata.saved",
cacheMetadata.size()));
+ logger.log(Level.FINE, sm.getString("cache.metadata.saved",
Integer.valueOf(cacheMetadata.size())));
}
/**
@@ -380,10 +371,6 @@ public class MigrationCache {
* @throws IOException if an I/O error occurs
*/
public void pruneCache() throws IOException {
- if (cacheDir == null) {
- return;
- }
-
LocalDate cutoffDate = LocalDate.now().minusDays(retentionDays);
int prunedCount = 0;
long prunedSize = 0;
@@ -422,9 +409,10 @@ public class MigrationCache {
saveMetadata();
if (prunedCount > 0) {
- logger.log(Level.INFO, sm.getString("cache.pruned.summary",
prunedCount, prunedSize / 1024 / 1024, retentionDays));
+ logger.log(Level.INFO, sm.getString("cache.pruned.summary",
Integer.valueOf(prunedCount),
+ Long.valueOf(prunedSize / 1024 / 1024),
Integer.valueOf(retentionDays)));
} else {
- logger.log(Level.FINE, sm.getString("cache.pruned.none",
retentionDays));
+ logger.log(Level.FINE, sm.getString("cache.pruned.none",
Integer.valueOf(retentionDays)));
}
}
@@ -435,10 +423,6 @@ public class MigrationCache {
* @throws IOException if an I/O error occurs
*/
public void finalizeCacheOperations() throws IOException {
- if (cacheDir == null) {
- return;
- }
-
// Save updated metadata
saveMetadata();
@@ -452,10 +436,6 @@ public class MigrationCache {
* @return a string describing cache size and entry count
*/
public String getStats() {
- if (cacheDir == null) {
- return sm.getString("cache.disabled");
- }
-
long totalSize = 0;
int entryCount = 0;
@@ -476,6 +456,6 @@ public class MigrationCache {
}
}
- return sm.getString("cache.stats", entryCount, totalSize / 1024 /
1024);
+ return sm.getString("cache.stats", Integer.valueOf(entryCount),
Long.valueOf(totalSize / 1024 / 1024));
}
}
diff --git
a/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
b/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
index 89c0395..f493afc 100644
--- a/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
+++ b/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
@@ -82,8 +82,8 @@ manifestConverter.noConversion=No manifest conversion
necessary for [{0}]
cache.cannotCreate=Cannot create cache directory [{0}]
cache.notDirectory=[{0}] is not a directory
+cache.nullDirectory=The cache storage directory may not be null
cache.enabled=Migration cache enabled at [{0}] with {1} day retention period
-cache.disabled=Cache is disabled
cache.hit=Cache hit for archive [{0}] (hash: {1})
cache.miss=Cache miss for archive [{0}] (hash: {1})
cache.store=Stored converted archive in cache (hash: {0}, size: {1} bytes)
@@ -99,4 +99,8 @@ cache.metadata.invalidDate=Invalid date in cache metadata: {0}
cache.pruned.entry=Pruned cache entry {0} (last accessed: {1})
cache.pruned.failed=Failed to delete cache entry {0}
cache.pruned.summary=Pruned {0} cache entries totaling {1} MB (retention
period: {2} days)
-cache.pruned.none=No cache entries to prune (retention period: {0} days)
\ No newline at end of file
+cache.pruned.none=No cache entries to prune (retention period: {0} days)
+
+cacheEntry.copyNotExist=Cannot copy - cache entry does not exist
+cacheEntry.tempNotExist=Temporary file [{0}] does not exist
+cacheEntry.tempRenameFail=Failed to rename temporary file [{0}] to cache file
[{1}]
\ No newline at end of file
diff --git a/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
b/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
index 331edb8..91cbfe9 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
@@ -50,7 +50,8 @@ public class MigrationCacheTest {
@Test
public void testCacheEnabledWithValidDirectory() throws Exception {
- new MigrationCache(tempCacheDir, 30);
+ @SuppressWarnings("unused")
+ MigrationCache unused = new MigrationCache(tempCacheDir, 30);
assertTrue("Cache directory should exist", tempCacheDir.exists());
}
@@ -59,7 +60,8 @@ public class MigrationCacheTest {
File newCacheDir = new File(tempCacheDir, "new-cache");
assertFalse("Cache directory should not exist yet",
newCacheDir.exists());
- new MigrationCache(newCacheDir, 30);
+ @SuppressWarnings("unused")
+ MigrationCache unused = new MigrationCache(newCacheDir, 30);
assertTrue("Cache directory should be created", newCacheDir.exists());
}
@@ -180,15 +182,6 @@ public class MigrationCacheTest {
assertTrue("Stats should contain entry count", stats.contains("0"));
}
- @Test
- public void testCacheStatsDisabled() throws Exception {
- MigrationCache cache = new MigrationCache(null, 30);
-
- String stats = cache.getStats();
- assertNotNull("Stats should not be null", stats);
- assertTrue("Stats should indicate cache is disabled",
stats.toLowerCase().contains("disabled"));
- }
-
@Test
public void testCacheWithLargeContent() throws Exception {
MigrationCache cache = new MigrationCache(tempCacheDir, 30);
@@ -249,21 +242,4 @@ public class MigrationCacheTest {
expectedConverted, destOutput.toByteArray());
}
}
-
- @Test
- public void testCacheDisabledNoOperations() throws Exception {
- MigrationCache cache = new MigrationCache(null, 30);
-
- byte[] sourceData = "test content".getBytes(StandardCharsets.UTF_8);
-
- // getCacheEntry should throw exception when cache is disabled
- try {
- cache.getCacheEntry(sourceData, EESpecProfiles.TOMCAT);
- fail("Should throw exception when cache is disabled");
- } catch (IllegalStateException e) {
- // Expected
- assertTrue("Error message should mention cache not enabled",
- e.getMessage().contains("not enabled"));
- }
- }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
