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 062f48f Minor fixes from code review
062f48f is described below
commit 062f48f13d043d6080a259cdae316a59b41646c1
Author: remm <[email protected]>
AuthorDate: Fri Jun 5 12:04:12 2026 +0200
Minor fixes from code review
---
.../org/apache/tomcat/jakartaee/CacheEntry.java | 16 ++++++++++------
.../org/apache/tomcat/jakartaee/GlobMatcher.java | 4 ++--
.../apache/tomcat/jakartaee/ManifestConverter.java | 6 +++++-
.../java/org/apache/tomcat/jakartaee/Migration.java | 21 +++++++++++++--------
.../org/apache/tomcat/jakartaee/MigrationTask.java | 8 +++++++-
.../org/apache/tomcat/jakartaee/StringManager.java | 14 ++++++++------
.../org/apache/tomcat/jakartaee/TextConverter.java | 2 +-
.../apache/tomcat/jakartaee/LocalStrings.properties | 5 ++++-
8 files changed, 50 insertions(+), 26 deletions(-)
diff --git a/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
b/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
index 8cb8110..525f5ae 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
@@ -90,7 +90,9 @@ class CacheEntry {
* @throws IOException if an I/O error occurs
*/
public void commitStore() throws IOException {
- fos.close();
+ if (fos != null) {
+ fos.close();
+ }
if (!tempFile.exists()) {
throw new IOException(sm.getString("cacheEntry.tempNotExist",
tempFile));
}
@@ -99,7 +101,7 @@ class CacheEntry {
if (!parentDir.exists()) {
parentDir.mkdirs();
}
- // Atomic rename
+ // Rename temp file to final cache location (usually atomic)
if (!tempFile.renameTo(cacheFile)) {
throw new IOException(sm.getString("cacheEntry.tempRenameFail",
tempFile, cacheFile));
}
@@ -117,10 +119,12 @@ class CacheEntry {
* Rollback the store operation - delete temp file.
*/
public void rollbackStore() {
- try {
- fos.close();
- } catch (IOException ioe) {
- // Ignore
+ if (fos != null) {
+ try {
+ fos.close();
+ } catch (IOException ioe) {
+ // Ignore
+ }
}
if (tempFile.exists()) {
tempFile.delete();
diff --git a/src/main/java/org/apache/tomcat/jakartaee/GlobMatcher.java
b/src/main/java/org/apache/tomcat/jakartaee/GlobMatcher.java
index 5f25a34..2e706e6 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/GlobMatcher.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/GlobMatcher.java
@@ -31,9 +31,9 @@ public final class GlobMatcher {
/**
- * Construct the matcher.
+ * All static.
*/
- public GlobMatcher() {}
+ private GlobMatcher() {}
/**
diff --git a/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
b/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
index cc93965..b316c41 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
@@ -132,7 +132,11 @@ public class ManifestConverter implements Converter {
}
// Update package names in values
for (Entry<Object, Object> entry : attributes.entrySet()) {
- String newValue = profile.convert((String) entry.getValue());
+ Object value = entry.getValue();
+ if (!(value instanceof String)) {
+ continue;
+ }
+ String newValue = profile.convert((String) value);
String header = entry.getKey().toString();
try {
// Need to be careful with OSGI headers.
diff --git a/src/main/java/org/apache/tomcat/jakartaee/Migration.java
b/src/main/java/org/apache/tomcat/jakartaee/Migration.java
index 9f15698..4c1c7d6 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/Migration.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/Migration.java
@@ -162,7 +162,7 @@ public class Migration {
/**
* Enable the default exclusion list for the tool.
- * @param enableDefaultExcludes true to enable the default
+ * @param enableDefaultExcludes true to enable the default excludes
*/
public void setEnableDefaultExcludes(boolean enableDefaultExcludes) {
this.enableDefaultExcludes = enableDefaultExcludes;
@@ -222,10 +222,12 @@ public class Migration {
/**
- * <b>NOTE</b>:
- * this method is not to indicate that no changes were made,
- * but that the source can be used and satisfy the selected profile.
- * @return true if converted occurs
+ * Returns whether any files were converted during migration.
+ * Note: a return value of {@code false} means the source already
+ * satisfied the selected profile and no changes were necessary.
+ *
+ * @return true if at least one file was converted
+ * @throws IllegalStateException if migration has not completed
*/
public boolean hasConverted() {
if (state != State.COMPLETE) {
@@ -238,6 +240,7 @@ public class Migration {
/**
* Execute migration operation.
* @throws IOException when an exception occurs
+ * @throws IllegalStateException if migration is already running
*/
public void execute() throws IOException {
if (state == State.RUNNING) {
@@ -296,7 +299,7 @@ public class Migration {
private void migrateFile(File src, File dest) throws IOException {
if (src.equals(dest)) {
- ByteArrayOutputStream buffer = new ByteArrayOutputStream((int)
(src.length() * 1.05));
+ ByteArrayOutputStream buffer = new
ByteArrayOutputStream(Math.toIntExact((long) (src.length() * 1.05)));
try (InputStream is = new FileInputStream(src)) {
if (migrateStream(src.getAbsolutePath(), is, buffer)) {
@@ -342,7 +345,8 @@ public class Migration {
}
String destName = profile.convert(srcName);
if (srcZipEntry.getMethod() == ZipEntry.STORED) {
- ByteArrayOutputStream tempBuffer = new
ByteArrayOutputStream((int) (srcZipEntry.getSize() * 1.05));
+ ByteArrayOutputStream tempBuffer =
+ new ByteArrayOutputStream(Math.toIntExact((long)
(srcZipEntry.getSize() * 1.05)));
convertedStream = migrateStream(srcName, srcZipStream,
tempBuffer);
crc32.update(tempBuffer.toByteArray(), 0,
tempBuffer.size());
MigrationZipArchiveEntry destZipEntry = new
MigrationZipArchiveEntry(srcZipEntry);
@@ -407,7 +411,7 @@ public class Migration {
}
// Write the destination back to the stream
- ByteArrayInputStream bais = new
ByteArrayInputStream(destByteChannel.array(), 0, (int) destByteChannel.size());
+ ByteArrayInputStream bais = new
ByteArrayInputStream(destByteChannel.array(), 0,
Math.toIntExact(destByteChannel.size()));
IOUtils.copy(bais, dest);
return convertedArchive;
@@ -448,6 +452,7 @@ public class Migration {
// Cache hit! Copy cached result to dest and return
logger.log(Level.INFO, sm.getString("cache.hit", name,
cacheEntry.getHash()));
cacheEntry.copyToDestination(dest);
+ // Although it is from the cache, this still counts as
converting the source
return true;
}
diff --git a/src/main/java/org/apache/tomcat/jakartaee/MigrationTask.java
b/src/main/java/org/apache/tomcat/jakartaee/MigrationTask.java
index e3e5d5b..c39ce09 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/MigrationTask.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/MigrationTask.java
@@ -108,6 +108,12 @@ public class MigrationTask extends Task {
} catch (IllegalArgumentException e) {
throw new BuildException("Invalid profile specified: " +
this.profile, getLocation()); // todo i18n
}
+ if (src == null || !src.exists() || !src.canRead()) {
+ throw new BuildException("Invalid or missing source parameter: " +
src); // todo i18n
+ }
+ if (dest == null) {
+ throw new BuildException("Missing destination parameter"); // todo
i18n
+ }
Migration migration = new Migration();
migration.setSource(src);
@@ -118,7 +124,7 @@ public class MigrationTask extends Task {
if (this.excludes != null) {
String[] excludes= this.excludes.split(",");
for (String exclude : excludes) {
- migration.addExclude(exclude);
+ migration.addExclude(exclude.trim());
}
}
diff --git a/src/main/java/org/apache/tomcat/jakartaee/StringManager.java
b/src/main/java/org/apache/tomcat/jakartaee/StringManager.java
index 4187348..1813186 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/StringManager.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/StringManager.java
@@ -60,10 +60,10 @@ public class StringManager {
/**
- * Creates a new StringManager for a given package. This is a
- * private method and all access to it is arbitrated by the
- * static getManager method call so that only one StringManager
- * per package will be created.
+ * Creates a new StringManager for a given package and locale.
+ * Access is arbitrated by the static {@link #getManager(Class)}
+ * method which caches instances per package/locale combination
+ * using an LRU cache.
*
* @param packageName Name of package to create StringManager for.
*/
@@ -75,7 +75,7 @@ public class StringManager {
// use of the ROOT Locale else incorrect results may be obtained if
// the system default locale is not English and translations are
// available for the system default locale.
- if (locale.getLanguage().equals(Locale.ENGLISH.getLanguage())) {
+ if (Locale.ENGLISH.getLanguage().equals(locale.getLanguage())) {
locale = Locale.ROOT;
}
bnd = ResourceBundle.getBundle(bundleName, locale);
@@ -167,7 +167,9 @@ public class StringManager {
}
MessageFormat mf = new MessageFormat(value);
- mf.setLocale(locale);
+ if (locale != null) {
+ mf.setLocale(locale);
+ }
return mf.format(args, new StringBuffer(), null).toString();
}
diff --git a/src/main/java/org/apache/tomcat/jakartaee/TextConverter.java
b/src/main/java/org/apache/tomcat/jakartaee/TextConverter.java
index cee3917..7421165 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/TextConverter.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/TextConverter.java
@@ -87,7 +87,7 @@ public class TextConverter implements Converter {
}
} else {
if (logger.isLoggable(Level.FINEST)) {
- logger.log(Level.FINEST,
sm.getString("classConverter.noConversion", path));
+ logger.log(Level.FINEST,
sm.getString("textConverter.noConversion", path));
}
}
diff --git
a/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
b/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
index f493afc..e87a484 100644
--- a/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
+++ b/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
@@ -27,7 +27,7 @@ migration.done=Migration completed successfully in [{0}]
milliseconds
migration.error=Error performing migration
migration.execute=Performing migration from source [{0}] to destination [{1}]
with Jakarta EE specification profile [{2}]
migration.jdk8303866=Due to size of [{0}], migrated JAR will fail if used in a
JDK without the fix for https://bugs.openjdk.org/browse/JDK-8303866 - Using an
in memory migration rather than a streaming migration may work-around the issue.
-migration.mkdirError=Error creating destination directory [{0}]a
+migration.mkdirError=Error creating destination directory [{0}]
migration.skip=Migration skipped for archive [{0}] because it is excluded (the
archive was copied unchanged)
migration.skipSignatureFile=Drop cryptographic signature file [{0}]
migration.usage=Usage: Migration [options] <source> <destination>\n\
@@ -100,6 +100,9 @@ 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)
+cache.tempfile.cleaned=Cache temporary file [{0}] was cleaned
+cache.tempfile.cleanFailed=Cache temporary file [{0}] cleaning failed
+cache.tempfiles.cleaned=[{0}] cache temporary files were cleaned
cacheEntry.copyNotExist=Cannot copy - cache entry does not exist
cacheEntry.tempNotExist=Temporary file [{0}] does not exist
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
