This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git
commit 066f71df6bcd647b26af96079235861a035032cb
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Feb 9 15:18:48 2021 +0000
Fix #13 refactor logging
INFO - Lists archives and dropped signature files
FINE - INFO plus modified files and dropped manifets entries
FINEST - Every file
---
CHANGES.md | 1 +
.../apache/tomcat/jakartaee/ClassConverter.java | 22 +++++++--
.../org/apache/tomcat/jakartaee/Converter.java | 3 +-
.../apache/tomcat/jakartaee/ManifestConverter.java | 2 +-
.../org/apache/tomcat/jakartaee/Migration.java | 55 +++++++++-------------
.../tomcat/jakartaee/PassThroughConverter.java | 10 +++-
.../org/apache/tomcat/jakartaee/TextConverter.java | 18 ++++++-
.../tomcat/jakartaee/LocalStrings.properties | 13 +++--
.../org/apache/tomcat/jakartaee/MigrationTest.java | 3 +-
.../tomcat/jakartaee/PassThroughConverterTest.java | 6 ++-
.../apache/tomcat/jakartaee/TextConverterTest.java | 4 +-
11 files changed, 88 insertions(+), 49 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 99d84e1..9b23746 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -22,6 +22,7 @@
- Ensure that all the Manifest attributes are processed during the migration
process. (markt)
- Include `.properties` and `.json` files in the conversion process. (markt)
- Replace `-verbose` with `-logLevel=` to provide more control over logging
level. (markt)
+- Fix [#13](https://github.com/apache/tomcat-jakartaee-migration/issues/13).
Refactor mapping of log messages to log levels. (markt)
## 0.1.0
diff --git a/src/main/java/org/apache/tomcat/jakartaee/ClassConverter.java
b/src/main/java/org/apache/tomcat/jakartaee/ClassConverter.java
index cc06bde..173ea8e 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/ClassConverter.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/ClassConverter.java
@@ -19,6 +19,8 @@ package org.apache.tomcat.jakartaee;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import org.apache.bcel.classfile.ClassParser;
import org.apache.bcel.classfile.Constant;
@@ -27,6 +29,9 @@ import org.apache.bcel.classfile.JavaClass;
public class ClassConverter implements Converter {
+ private static final Logger logger =
Logger.getLogger(ClassConverter.class.getCanonicalName());
+ private static final StringManager sm =
StringManager.getManager(ClassConverter.class);
+
@Override
public boolean accepts(String filename) {
String extension = Util.getExtension(filename);
@@ -35,26 +40,37 @@ public class ClassConverter implements Converter {
@Override
- public void convert(InputStream src, OutputStream dest, EESpecProfile
profile) throws IOException {
+ public void convert(String path, InputStream src, OutputStream dest,
EESpecProfile profile) throws IOException {
ClassParser parser = new ClassParser(src, "unknown");
JavaClass javaClass = parser.parse();
+ boolean converted = false;
+
// Loop through constant pool
Constant[] constantPool =
javaClass.getConstantPool().getConstantPool();
for (short i = 0; i < constantPool.length; i++) {
if (constantPool[i] instanceof ConstantUtf8) {
ConstantUtf8 c = (ConstantUtf8) constantPool[i];
String str = c.getBytes();
- String converted = profile.convert(str);
+ String newString = profile.convert(str);
// Object comparison is deliberate
- if (converted != str) {
+ if (newString != str) {
c = new ConstantUtf8(profile.convert(str));
constantPool[i] = c;
+ converted = true;
}
}
}
+ if (logger.isLoggable(Level.FINE)) {
+ if (converted) {
+ logger.log(Level.FINE,
sm.getString("classConverter.converted", path));
+ } else if (logger.isLoggable(Level.FINEST)) {
+ logger.log(Level.FINEST,
sm.getString("classConverter.noConversion", path));
+ }
+ }
+
javaClass.dump(dest);
}
}
diff --git a/src/main/java/org/apache/tomcat/jakartaee/Converter.java
b/src/main/java/org/apache/tomcat/jakartaee/Converter.java
index 5a62b5c..e05c232 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/Converter.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/Converter.java
@@ -28,11 +28,12 @@ public interface Converter {
* Copies the source to the destination, converting it if necessary,
* according to the requirements of the given profile.
*
+ * @param path The path to the data being converted
* @param src The source data to convert
* @param dest The destination to write the converted data
* @param profile The profile that defines the conversion required
*
* @throws IOException If the conversion fails
*/
- void convert(InputStream src, OutputStream dest, EESpecProfile profile)
throws IOException;
+ void convert(String path, InputStream src, OutputStream dest,
EESpecProfile profile) throws IOException;
}
diff --git a/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
b/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
index e177217..30e4ec0 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
@@ -48,7 +48,7 @@ public class ManifestConverter implements Converter {
}
@Override
- public void convert(InputStream src, OutputStream dest, EESpecProfile
profile) throws IOException {
+ public void convert(String path, InputStream src, OutputStream dest,
EESpecProfile profile) throws IOException {
Manifest srcManifest = new Manifest(src);
Manifest destManifest = new Manifest(srcManifest);
diff --git a/src/main/java/org/apache/tomcat/jakartaee/Migration.java
b/src/main/java/org/apache/tomcat/jakartaee/Migration.java
index e970a08..153345a 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/Migration.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/Migration.java
@@ -128,8 +128,7 @@ public class Migration {
}
- private boolean migrateDirectory(File src, File dest) throws IOException {
- boolean result = true;
+ private void migrateDirectory(File src, File dest) throws IOException {
// Won't return null because src is known to be a directory
String[] files = src.list();
for (String file : files) {
@@ -137,71 +136,61 @@ public class Migration {
File destFile = new File(dest, file);
if (srcFile.isDirectory()) {
if ((destFile.exists() && destFile.isDirectory()) ||
destFile.mkdir()) {
- result = result && migrateDirectory(srcFile, destFile);
+ migrateDirectory(srcFile, destFile);
} else {
- logger.log(Level.WARNING,
sm.getString("migration.mkdirError", destFile.getAbsolutePath()));
- result = false;
+ throw new IOException(sm.getString("migration.mkdirError",
destFile.getAbsolutePath()));
}
} else {
- result = result && migrateFile(srcFile, destFile);
+ migrateFile(srcFile, destFile);
}
}
- return result;
}
- private boolean migrateFile(File src, File dest) throws IOException {
- boolean result = false;
-
+ private void migrateFile(File src, File dest) throws IOException {
boolean inplace = src.equals(dest);
if (!inplace) {
try (InputStream is = new FileInputStream(src);
OutputStream os = new FileOutputStream(dest)) {
- result = migrateStream(src.getName(), is, os);
+ migrateStream(src.getName(), is, os);
}
} else {
ByteArrayOutputStream buffer = new ByteArrayOutputStream((int)
(src.length() * 1.05));
try (InputStream is = new FileInputStream(src)) {
- result = migrateStream(src.getName(), is, buffer);
+ migrateStream(src.getName(), is, buffer);
}
try (OutputStream os = new FileOutputStream(dest)) {
os.write(buffer.toByteArray());
}
}
-
- return result;
}
- private boolean migrateArchiveStreaming(String name, InputStream src,
OutputStream dest) throws IOException {
- boolean result = true;
+ private void migrateArchiveStreaming(String name, InputStream src,
OutputStream dest) throws IOException {
try (ZipInputStream zipIs = new ZipInputStream(new
CloseShieldInputStream(src));
ZipOutputStream zipOs = new ZipOutputStream(new
CloseShieldOutputStream(dest))) {
ZipEntry zipEntry;
while ((zipEntry = zipIs.getNextEntry()) != null) {
String sourceName = zipEntry.getName();
- logger.log(Level.FINE, sm.getString("migration.entry",
sourceName));
if (isSignatureFile(sourceName)) {
- logger.log(Level.FINE,
sm.getString("migration.skipSignatureFile", sourceName));
+ logger.log(Level.WARNING,
sm.getString("migration.skipSignatureFile", sourceName));
continue;
}
String destName = profile.convert(sourceName);
JarEntry destEntry = new JarEntry(destName);
zipOs.putNextEntry(destEntry);
- result = result && migrateStream(destEntry.getName(), zipIs,
zipOs);
+ migrateStream(destEntry.getName(), zipIs, zipOs);
}
} catch (ZipException ze) {
logger.log(Level.SEVERE, sm.getString("migration.archiveFailed",
name), ze);
throw ze;
}
- return result;
}
- private boolean migrateArchiveInMemory(InputStream src, OutputStream dest)
throws IOException {
- boolean result = true;
+ private void migrateArchiveInMemory(InputStream src, OutputStream dest)
throws IOException {
// Read the source into memory
ByteArrayOutputStream baos = new ByteArrayOutputStream();
IOUtils.copy(src, baos);
@@ -216,16 +205,15 @@ public class Migration {
while (entries.hasMoreElements()) {
ZipArchiveEntry srcZipEntry = entries.nextElement();
String srcName = srcZipEntry.getName();
- logger.log(Level.FINE, sm.getString("migration.entry",
srcName));
if (isSignatureFile(srcName)) {
- logger.log(Level.FINE,
sm.getString("migration.skipSignatureFile", srcName));
+ logger.log(Level.WARNING,
sm.getString("migration.skipSignatureFile", srcName));
continue;
}
String destName = profile.convert(srcName);
RenamableZipArchiveEntry destZipEntry = new
RenamableZipArchiveEntry(srcZipEntry);
destZipEntry.setName(destName);
destZipStream.putArchiveEntry(destZipEntry);
- result = result && migrateStream(srcName,
srcZipFile.getInputStream(srcZipEntry), destZipStream);
+ migrateStream(srcName, srcZipFile.getInputStream(srcZipEntry),
destZipStream);
destZipStream.closeArchiveEntry();
}
}
@@ -233,8 +221,6 @@ public class Migration {
// Write the destination back to the stream
ByteArrayInputStream bais = new
ByteArrayInputStream(destByteChannel.array(), 0, (int) destByteChannel.size());
IOUtils.copy(bais, dest);
-
- return result;
}
@@ -244,23 +230,24 @@ public class Migration {
}
- private boolean migrateStream(String name, InputStream src, OutputStream
dest) throws IOException {
+ private void migrateStream(String name, InputStream src, OutputStream
dest) throws IOException {
if (isArchive(name)) {
- logger.log(Level.INFO, sm.getString("migration.archive", name));
if (zipInMemory) {
- return migrateArchiveInMemory(src, dest);
+ logger.log(Level.INFO,
sm.getString("migration.archive.memory", name));
+ migrateArchiveInMemory(src, dest);
+ logger.log(Level.INFO,
sm.getString("migration.archive.complete", name));
} else {
- return migrateArchiveStreaming(name, src, dest);
+ logger.log(Level.INFO,
sm.getString("migration.archive.stream", name));
+ migrateArchiveStreaming(name, src, dest);
+ logger.log(Level.INFO,
sm.getString("migration.archive.complete", name));
}
} else {
- logger.log(Level.FINE, sm.getString("migration.stream", name));
for (Converter converter : converters) {
if (converter.accepts(name)) {
- converter.convert(src, dest, profile);
+ converter.convert(name, src, dest, profile);
break;
}
}
- return true;
}
}
diff --git
a/src/main/java/org/apache/tomcat/jakartaee/PassThroughConverter.java
b/src/main/java/org/apache/tomcat/jakartaee/PassThroughConverter.java
index 24c01f9..8da2d9b 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/PassThroughConverter.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/PassThroughConverter.java
@@ -19,9 +19,14 @@ package org.apache.tomcat.jakartaee;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.util.logging.Level;
+import java.util.logging.Logger;
public class PassThroughConverter implements Converter {
+ private static final Logger logger =
Logger.getLogger(PassThroughConverter.class.getCanonicalName());
+ private static final StringManager sm =
StringManager.getManager(PassThroughConverter.class);
+
@Override
public boolean accepts(String filename) {
// Accepts everything
@@ -29,8 +34,11 @@ public class PassThroughConverter implements Converter {
}
@Override
- public void convert(InputStream src, OutputStream dest, EESpecProfile
profile) throws IOException {
+ public void convert(String path, InputStream src, OutputStream dest,
EESpecProfile profile) throws IOException {
// This simply copies the source to the destination
Util.copy(src, dest);
+ if (logger.isLoggable(Level.FINEST)) {
+ logger.log(Level.FINEST,
sm.getString("passThroughConverter.noConversion", path));
+ }
}
}
diff --git a/src/main/java/org/apache/tomcat/jakartaee/TextConverter.java
b/src/main/java/org/apache/tomcat/jakartaee/TextConverter.java
index 57cf1ae..b3c7857 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/TextConverter.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/TextConverter.java
@@ -23,9 +23,14 @@ import java.io.OutputStream;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
+import java.util.logging.Level;
+import java.util.logging.Logger;
public class TextConverter implements Converter {
+ private static final Logger logger =
Logger.getLogger(TextConverter.class.getCanonicalName());
+ private static final StringManager sm =
StringManager.getManager(TextConverter.class);
+
private static final List<String> supportedExtensions;
static {
@@ -60,10 +65,21 @@ public class TextConverter implements Converter {
* execution.
*/
@Override
- public void convert(InputStream src, OutputStream dest, EESpecProfile
profile) throws IOException {
+ public void convert(String path, InputStream src, OutputStream dest,
EESpecProfile profile) throws IOException {
String srcString = Util.toString(src, StandardCharsets.ISO_8859_1);
String destString = profile.convert(srcString);
+ // Object comparison is deliberate here
+ if (srcString == destString) {
+ if (logger.isLoggable(Level.FINEST)) {
+ logger.log(Level.FINEST,
sm.getString("classConverter.noConversion", path));
+ }
+ } else {
+ if (logger.isLoggable(Level.FINE)) {
+ logger.log(Level.FINE, sm.getString("textConverter.converted",
path));
+ }
+ }
+
ByteArrayInputStream bais = new
ByteArrayInputStream(destString.getBytes(StandardCharsets.ISO_8859_1));
Util.copy(bais, dest);
}
diff --git
a/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
b/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
index d8150c0..5515874 100644
--- a/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
+++ b/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
@@ -13,7 +13,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-migration.archive=Migrating archive [{0}]
+classConverter.converted=Migrated class [{0}]
+classConverter.noConversion=No conversion necessary for [{0}]
+
+migration.archive.complete=Migration of archive [{0}] is complete
+migration.archive.memory=Migrating archive [{0}] using in memory copy
+migration.archive.stream=Migrating archive [{0}] using streaming
migration.archiveFailed=Failed to migrate archive [{0}]. Using the
"-zipInMemory" option may help.
migration.cannotReadSource=Cannot read source location [{0}]
migration.done=Migration completed successfully in [{0}] milliseconds
@@ -23,7 +28,6 @@ migration.execute=Performing migration from source [{0}] to
destination [{1}] wi
migration.mkdirError=Error creating destination directory [{0}]
migration.removeSignature=Remove cryptographic signature for [{0}]
migration.skipSignatureFile=Drop cryptographic signature file [{0}]
-migration.stream=Migrating stream [{0}]
migration.usage=Usage: Migration [options] <source> <destination>\n\
where options includes:\n\
\ -logLevel=<name of java.util.logging.level enum value>\n\
@@ -37,6 +41,9 @@ where options includes:\n\
\ compatible with some zip archive structures. If you see an\n\
\ exception while processing a zip file, enabling this option\n\
\ may workaround the issue.
+migration.warnSignatureRemoval=Removed cryptographic signature from JAR file
+passThroughConverter.noConversion=No conversion necessary for [{0}]
-migration.warnSignatureRemoval=Removed cryptographic signature from JAR file
+textConverter.converted=Migrated text file [{0}]
+textConverter.noConversion=No conversion necessary for [{0}]
diff --git a/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
b/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
index 0e0ba2a..e4384be 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
@@ -115,9 +115,8 @@ public class MigrationTest {
Migration migration = new Migration();
migration.setSource(sourceDirectory);
migration.setDestination(destinationDirectory);
- boolean success = migration.execute();
+ migration.execute();
- assertTrue("Migration failed", success);
assertTrue("Destination directory not found",
destinationDirectory.exists());
File migratedFile = new
File("target/test-classes/migration/HelloServlet.java");
diff --git
a/src/test/java/org/apache/tomcat/jakartaee/PassThroughConverterTest.java
b/src/test/java/org/apache/tomcat/jakartaee/PassThroughConverterTest.java
index 5df0687..fb8fa91 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/PassThroughConverterTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/PassThroughConverterTest.java
@@ -26,6 +26,8 @@ import static org.junit.Assert.*;
public class PassThroughConverterTest {
+ private static final String TEST_FILENAME = "project.properties";
+
@Test
public void testConverter() throws Exception {
String content = "javax.servlet";
@@ -35,9 +37,9 @@ public class PassThroughConverterTest {
Converter converter = new PassThroughConverter();
- assertTrue(converter.accepts("project.properties"));
+ assertTrue(converter.accepts(TEST_FILENAME));
- converter.convert(in, out, null);
+ converter.convert(TEST_FILENAME, in, out, null);
assertArrayEquals(content.getBytes(), out.toByteArray());
}
diff --git a/src/test/java/org/apache/tomcat/jakartaee/TextConverterTest.java
b/src/test/java/org/apache/tomcat/jakartaee/TextConverterTest.java
index 2d015f3..f8d029b 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/TextConverterTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/TextConverterTest.java
@@ -10,6 +10,8 @@ import org.junit.Test;
public class TextConverterTest {
+ private static final String TEST_FILENAME = "text.txt";
+
private static final String INPUT =
"javax.servlet.http.HttpServletRequest";
private static final String OUTPUT =
"jakarta.servlet.http.HttpServletRequest";
@@ -23,7 +25,7 @@ public class TextConverterTest {
EESpecProfile profile = EESpecProfile.EE;
// test
- converter.convert(in, out, profile);
+ converter.convert(TEST_FILENAME, in, out, profile);
// assert
String result = new String(out.toByteArray(),
StandardCharsets.ISO_8859_1);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
