From 572a79015bfad68aff94afff00de34f6a7bdfbae Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Mon, 7 Nov 2022 09:53:49 +0100 Subject: [PATCH] Fix logging with panache in interfaces The logging transformation used to add a `private static final` field to each transformed class. This is invalid in case the transformed class is in fact an interface, because the JVMS requires interface fields to always be `public static final`. With this commit, the logging transformation still adds `private` fields to regular classes, but when the transformed class is an interface, the generated field is `public`. This is not very nice, but working code trumps aesthetically pleasing code. --- .../logging/LoggingWithPanacheProcessor.java | 20 ++++++++++++++++--- .../java/io/quarkus/logging/LoggingBean.java | 4 +++- .../io/quarkus/logging/LoggingInterface.java | 9 +++++++++ .../logging/LoggingWithPanacheTest.java | 5 +++-- 4 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingInterface.java diff --git a/core/deployment/src/main/java/io/quarkus/deployment/logging/LoggingWithPanacheProcessor.java b/core/deployment/src/main/java/io/quarkus/deployment/logging/LoggingWithPanacheProcessor.java index 8cc8b42b2825e..6e7890d8bb2df 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/logging/LoggingWithPanacheProcessor.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/logging/LoggingWithPanacheProcessor.java @@ -43,7 +43,8 @@ public void process(CombinedIndexBuildItem index, BuildProducer - *
  • adds a {@code private static final} field of type {@code org.jboss.logging.Logger};
    • + *
  • adds a {@code private static final} field of type {@code org.jboss.logging.Logger} + * ({@code public} in case the class is an interface, to obey the JVMS rules);
    • *
  • initializes the field (to {@code Logger.getLogger(className)}) at the beginning of the * static initializer (creating one if missing);
    • *
  • rewrites all invocations of {@code static} methods on {@code io.quarkus.logging.Log} @@ -57,6 +58,8 @@ private static class AddLoggerFieldAndRewriteInvocations extends ClassVisitor { private final String className; private final String classNameBinary; + private boolean isInterface; + private boolean generatedLoggerField; private boolean generatedLoggerFieldInitialization; @@ -66,6 +69,14 @@ public AddLoggerFieldAndRewriteInvocations(ClassVisitor visitor, String classNam this.classNameBinary = className.replace(".", "/"); } + @Override + public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { + super.visit(version, access, name, signature, superName, interfaces); + if ((access & Opcodes.ACC_INTERFACE) != 0) { + isInterface = true; + } + } + @Override public FieldVisitor visitField(int access, String name, String descriptor, String signature, Object value) { if (!generatedLoggerField) { @@ -190,8 +201,11 @@ public void visitEnd() { } private void generateLoggerField() { - super.visitField(Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL, - SYNTHETIC_LOGGER_FIELD_NAME, JBOSS_LOGGER_DESCRIPTOR, null, null); + // interface fields must be public static final per the JVMS + int access = isInterface + ? Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL + : Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL; + super.visitField(access, SYNTHETIC_LOGGER_FIELD_NAME, JBOSS_LOGGER_DESCRIPTOR, null, null); generatedLoggerField = true; } } diff --git a/integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingBean.java b/integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingBean.java index d137a47414e51..f3ddfdd4f0c96 100644 --- a/integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingBean.java +++ b/integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingBean.java @@ -4,7 +4,7 @@ import javax.inject.Singleton; @Singleton -public class LoggingBean { +public class LoggingBean implements LoggingInterface { // not final to prevent constant inlining private static String msg = "Heya!"; @@ -18,6 +18,8 @@ public void setup() { } public void doSomething() { + inheritedMethod("abc"); + if (Log.isDebugEnabled()) { Log.debug("starting massive computation"); } diff --git a/integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingInterface.java b/integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingInterface.java new file mode 100644 index 0000000000000..14a0e0bb99dc2 --- /dev/null +++ b/integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingInterface.java @@ -0,0 +1,9 @@ +package io.quarkus.logging; + +public interface LoggingInterface { + default void inheritedMethod(String param) { + if (Log.isInfoEnabled()) { + Log.infof("Default method from interface: %s", param); + } + } +} diff --git a/integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingWithPanacheTest.java b/integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingWithPanacheTest.java index 11573084be736..563b5f15b15f6 100644 --- a/integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingWithPanacheTest.java +++ b/integration-tests/logging-panache/src/test/java/io/quarkus/logging/LoggingWithPanacheTest.java @@ -17,8 +17,8 @@ public class LoggingWithPanacheTest { @RegisterExtension static final QuarkusUnitTest test = new QuarkusUnitTest() - .withApplicationRoot((jar) -> jar - .addClasses(LoggingBean.class, LoggingEntity.class, NoStackTraceTestException.class)) + .withApplicationRoot((jar) -> jar.addClasses(LoggingBean.class, LoggingInterface.class, LoggingEntity.class, + NoStackTraceTestException.class)) .overrideConfigKey("quarkus.log.category.\"io.quarkus.logging\".min-level", "TRACE") .overrideConfigKey("quarkus.log.category.\"io.quarkus.logging\".level", "TRACE") .setLogRecordPredicate(record -> record.getLoggerName().startsWith("io.quarkus.logging.Logging")) @@ -29,6 +29,7 @@ public class LoggingWithPanacheTest { assertThat(lines).containsExactly( "[INFO] Heya!", "[TRACE] LoggingBean created", + "[INFO] Default method from interface: abc", "[DEBUG] starting massive computation", "[DEBUG] one: 42", "[TRACE] two: 42 | 13",