From 5c79a1e7a8ed4e89a42778f088e8028c0a20f8da Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sat, 21 Jun 2025 10:49:54 +0200 Subject: [PATCH] fix: Make `GraalVmProcessor` Arguments Optional Fixes #3771 This PR makes the `-Alog4j.graalvm.groupId` and `-Alog4j.graalvm.artifactId` arguments optional. * If **no arguments** are provided, metadata is stored in: ``` META-INF/native-image/log4j-generated ``` Previously an error was thrown. * If **arguments are provided**, files go to: ``` META-INF/native-image/log4j-generated// ``` Previously `META-INF/native-image//` was used. The new path prevents collisions with user-provided metadata. --- .../processor/GraalVmProcessorTest.java | 164 ++++++++++++++---- .../java/FakeAnnotations.java | 83 +++++++++ .../GraalVmProcessorTest/java/FakePlugin.java | 90 ++++++++++ .../plugins/processor/GraalVmProcessor.java | 24 ++- src/changelog/.2.x.x/3771_graalvm-params.xml | 10 ++ 5 files changed, 330 insertions(+), 41 deletions(-) create mode 100644 log4j-core-test/src/test/resources/GraalVmProcessorTest/java/FakeAnnotations.java create mode 100644 log4j-core-test/src/test/resources/GraalVmProcessorTest/java/FakePlugin.java create mode 100644 src/changelog/.2.x.x/3771_graalvm-params.xml diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessorTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessorTest.java index 9f8b51a7b75..933d2cc4340 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessorTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessorTest.java @@ -16,31 +16,50 @@ */ package org.apache.logging.log4j.core.config.plugins.processor; +import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; +import static java.util.Objects.requireNonNull; import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json; import static org.assertj.core.api.Assertions.assertThat; +import java.io.File; import java.io.IOException; import java.net.URL; -import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.stream.Collectors; import java.util.stream.Stream; -import org.apache.commons.io.IOUtils; +import javax.tools.Diagnostic; +import javax.tools.DiagnosticCollector; +import javax.tools.JavaCompiler; +import javax.tools.JavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.StandardLocation; +import javax.tools.ToolProvider; +import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; class GraalVmProcessorTest { + private static final String FAKE_PLUGIN_NAME = "example.FakePlugin"; private static final Object FAKE_PLUGIN = asMap( "name", - FakePlugin.class.getName(), + FAKE_PLUGIN_NAME, "methods", asList( asMap("name", "", "parameterTypes", emptyList()), @@ -57,9 +76,10 @@ class GraalVmProcessorTest { "java.lang.String"))), "fields", emptyList()); + private static final String FAKE_PLUGIN_BUILDER_NAME = FAKE_PLUGIN_NAME + "$Builder"; private static final Object FAKE_PLUGIN_BUILDER = asMap( "name", - FakePlugin.Builder.class.getName(), + FAKE_PLUGIN_BUILDER_NAME, "methods", emptyList(), "fields", @@ -71,21 +91,26 @@ class GraalVmProcessorTest { asMap("name", "loggerContext"), asMap("name", "node"), asMap("name", "value"))); - private static final Object FAKE_PLUGIN_NESTED = onlyNoArgsConstructor(FakePlugin.Nested.class); - private static final Object FAKE_CONSTRAINT_VALIDATOR = - onlyNoArgsConstructor(FakeAnnotations.FakeConstraintValidator.class); - private static final Object FAKE_PLUGIN_VISITOR = onlyNoArgsConstructor(FakeAnnotations.FakePluginVisitor.class); + private static final String FAKE_PLUGIN_NESTED_NAME = FAKE_PLUGIN_NAME + "$Nested"; + private static final Object FAKE_PLUGIN_NESTED = onlyNoArgsConstructor(FAKE_PLUGIN_NESTED_NAME); + private static final String FAKE_CONSTRAINT_VALIDATOR_NAME = "example.FakeAnnotations$FakeConstraintValidator"; + private static final Object FAKE_CONSTRAINT_VALIDATOR = onlyNoArgsConstructor(FAKE_CONSTRAINT_VALIDATOR_NAME); + private static final String FAKE_PLUGIN_VISITOR_NAME = "example.FakeAnnotations$FakePluginVisitor"; + private static final Object FAKE_PLUGIN_VISITOR = onlyNoArgsConstructor(FAKE_PLUGIN_VISITOR_NAME); + + private static final String GROUP_ID = "groupId"; + private static final String ARTIFACT_ID = "artifactId"; /** * Generates a metadata element with just a single no-arg constructor. * - * @param clazz The name of the metadata element. + * @param className The name of the metadata element. * @return A GraalVM metadata element. */ - private static Object onlyNoArgsConstructor(Class clazz) { + private static Object onlyNoArgsConstructor(String className) { return asMap( "name", - clazz.getName(), + className, "methods", singletonList(asMap("name", "", "parameterTypes", emptyList())), "fields", @@ -103,43 +128,114 @@ private static Object onlyNoArgsConstructor(Class clazz) { return map; } - private static String reachabilityMetadata; + private static Path sourceDir; + + @TempDir + private static Path outputDir; @BeforeAll - static void setup() throws IOException { - // There are two descriptors, choose the one in `test-classes` - URL reachabilityMetadataUrl = null; - for (URL url : Collections.list(GraalVmProcessor.class - .getClassLoader() - .getResources("META-INF/native-image/org.apache.logging.log4j/log4j-core-test/reflect-config.json"))) { - if (url.getPath().contains("test-classes")) { - reachabilityMetadataUrl = url; - break; - } - } - assertThat(reachabilityMetadataUrl).isNotNull(); - reachabilityMetadata = IOUtils.toString(reachabilityMetadataUrl, StandardCharsets.UTF_8); + static void setup() throws Exception { + URL sourceUrl = requireNonNull(GraalVmProcessorTest.class.getResource("/GraalVmProcessorTest/java")); + sourceDir = Paths.get(sourceUrl.toURI()); + // Generate metadata + List diagnostics = generateDescriptor(sourceDir, GROUP_ID, ARTIFACT_ID, outputDir); + assertThat(diagnostics).isEmpty(); } static Stream containsSpecificEntries() { return Stream.of( - Arguments.of(FakePlugin.class, FAKE_PLUGIN), - Arguments.of(FakePlugin.Builder.class, FAKE_PLUGIN_BUILDER), - Arguments.of(FakePlugin.Nested.class, FAKE_PLUGIN_NESTED), - Arguments.of(FakeAnnotations.FakeConstraintValidator.class, FAKE_CONSTRAINT_VALIDATOR), - Arguments.of(FakeAnnotations.FakePluginVisitor.class, FAKE_PLUGIN_VISITOR)); + Arguments.of(FAKE_PLUGIN_NAME, FAKE_PLUGIN), + Arguments.of(FAKE_PLUGIN_BUILDER_NAME, FAKE_PLUGIN_BUILDER), + Arguments.of(FAKE_PLUGIN_NESTED_NAME, FAKE_PLUGIN_NESTED), + Arguments.of(FAKE_CONSTRAINT_VALIDATOR_NAME, FAKE_CONSTRAINT_VALIDATOR), + Arguments.of(FAKE_PLUGIN_VISITOR_NAME, FAKE_PLUGIN_VISITOR)); } @ParameterizedTest @MethodSource - void containsSpecificEntries(Class clazz, Object expectedJson) { + void containsSpecificEntries(String className, Object expectedJson) throws IOException { + // Read metadata + Path reachabilityMetadataPath = + outputDir.resolve(GraalVmProcessor.getReachabilityMetadataPath(GROUP_ID, ARTIFACT_ID)); + String reachabilityMetadata = new String(Files.readAllBytes(reachabilityMetadataPath), UTF_8); assertThatJson(reachabilityMetadata) - .inPath(filterByName(clazz)) + .inPath(String.format("$[?(@.name == '%s')]", className)) .isArray() .contains(json(expectedJson)); } - private String filterByName(Class clazz) { - return String.format("$[?(@.name == '%s')]", clazz.getName()); + static Stream reachabilityMetadataPath() { + return Stream.of( + Arguments.of( + "groupId", + "artifactId", + "META-INF/native-image/log4j-generated/groupId/artifactId/reflect-config.json"), + Arguments.of(null, "artifactId", "META-INF/native-image/log4j-generated/reflect-config.json"), + Arguments.of("groupId", null, "META-INF/native-image/log4j-generated/reflect-config.json"), + Arguments.of(null, null, "META-INF/native-image/log4j-generated/reflect-config.json")); + } + + @ParameterizedTest + @MethodSource + void reachabilityMetadataPath(@Nullable String groupId, @Nullable String artifactId, String expected) { + assertThat(GraalVmProcessor.getReachabilityMetadataPath(groupId, artifactId)) + .isEqualTo(expected); + } + + @Test + void whenNoGroupIdAndArtifactId_thenWarningIsPrinted(@TempDir Path outputDir) throws Exception { + List diagnostics = generateDescriptor(sourceDir, null, null, outputDir); + assertThat(diagnostics).hasSize(1); + // The warning message should contain the information about the missing groupId and artifactId arguments + assertThat(diagnostics.get(0)) + .contains( + "recommended", + "-A" + GraalVmProcessor.GROUP_ID + "=", + "-A" + GraalVmProcessor.ARTIFACT_ID + "="); + Path reachabilityMetadataPath = outputDir.resolve(GraalVmProcessor.getReachabilityMetadataPath(null, null)); + assertThat(Files.exists(reachabilityMetadataPath)).isTrue(); + } + + private static List generateDescriptor( + Path sourceDir, @Nullable String groupId, @Nullable String artifactId, Path outputDir) throws Exception { + // Instantiate the tooling + final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + final StandardJavaFileManager fileManager = compiler.getStandardFileManager(null, Locale.ROOT, UTF_8); + + // Populate sources + final Iterable sources; + try (final Stream files = Files.walk(sourceDir)) { + File[] sourceFiles = + files.filter(Files::isRegularFile).map(Path::toFile).toArray(File[]::new); + sources = fileManager.getJavaFileObjects(sourceFiles); + } + + // Set the target path used by `DescriptorGenerator` to dump the generated files + fileManager.setLocation(StandardLocation.CLASS_OUTPUT, Collections.singleton(outputDir.toFile())); + + // Prepare the compiler options + List options = new ArrayList<>(); + options.add("-proc:only"); + options.add("-processor"); + options.add(GraalVmProcessor.class.getName()); + if (groupId != null) { + options.add("-A" + GraalVmProcessor.GROUP_ID + "=" + groupId); + } + if (artifactId != null) { + options.add("-A" + GraalVmProcessor.ARTIFACT_ID + "=" + artifactId); + } + + // Compile the sources + final Path descriptorFilePath = outputDir.resolve("plugins.xml"); + final DiagnosticCollector diagnosticCollector = new DiagnosticCollector<>(); + final JavaCompiler.CompilationTask task = + compiler.getTask(null, fileManager, diagnosticCollector, options, null, sources); + task.call(); + + // Verify successful compilation + return diagnosticCollector.getDiagnostics().stream() + .filter(d -> d.getKind() != Diagnostic.Kind.NOTE) + .map(d -> d.getMessage(Locale.ROOT)) + .collect(Collectors.toList()); } } diff --git a/log4j-core-test/src/test/resources/GraalVmProcessorTest/java/FakeAnnotations.java b/log4j-core-test/src/test/resources/GraalVmProcessorTest/java/FakeAnnotations.java new file mode 100644 index 00000000000..c3223781f49 --- /dev/null +++ b/log4j-core-test/src/test/resources/GraalVmProcessorTest/java/FakeAnnotations.java @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package example; + +import java.lang.annotation.Annotation; +import java.lang.reflect.Member; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.Node; +import org.apache.logging.log4j.core.config.plugins.PluginVisitorStrategy; +import org.apache.logging.log4j.core.config.plugins.validation.Constraint; +import org.apache.logging.log4j.core.config.plugins.validation.ConstraintValidator; +import org.apache.logging.log4j.core.config.plugins.visitors.PluginVisitor; +import org.apache.logging.log4j.core.lookup.StrSubstitutor; + +/** + * Fake constraint and plugin visitor that are accessed through reflection. + */ +public class FakeAnnotations { + + @Constraint(FakeConstraintValidator.class) + public @interface FakeConstraint {} + + public static class FakeConstraintValidator implements ConstraintValidator { + @Override + public void initialize(FakeConstraint annotation) {} + + @Override + public boolean isValid(String name, Object value) { + return false; + } + } + + @PluginVisitorStrategy(FakePluginVisitor.class) + public @interface FakeAnnotation {} + + public static class FakePluginVisitor implements PluginVisitor { + + @Override + public PluginVisitor setAnnotation(Annotation annotation) { + return null; + } + + @Override + public PluginVisitor setAliases(String... aliases) { + return null; + } + + @Override + public PluginVisitor setStrSubstitutor(StrSubstitutor substitutor) { + return null; + } + + @Override + public PluginVisitor setMember(Member member) { + return null; + } + + @Override + public Object visit(Configuration configuration, Node node, LogEvent event, StringBuilder log) { + return null; + } + + @Override + public PluginVisitor setConversionType(Class conversionType) { + return null; + } + } +} diff --git a/log4j-core-test/src/test/resources/GraalVmProcessorTest/java/FakePlugin.java b/log4j-core-test/src/test/resources/GraalVmProcessorTest/java/FakePlugin.java new file mode 100644 index 00000000000..90b872c0a31 --- /dev/null +++ b/log4j-core-test/src/test/resources/GraalVmProcessorTest/java/FakePlugin.java @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package example; + +import java.io.Serializable; +import org.apache.logging.log4j.core.Layout; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.Node; +import org.apache.logging.log4j.core.config.plugins.Plugin; +import org.apache.logging.log4j.core.config.plugins.PluginAliases; +import org.apache.logging.log4j.core.config.plugins.PluginAttribute; +import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute; +import org.apache.logging.log4j.core.config.plugins.PluginConfiguration; +import org.apache.logging.log4j.core.config.plugins.PluginElement; +import org.apache.logging.log4j.core.config.plugins.PluginFactory; +import org.apache.logging.log4j.core.config.plugins.PluginLoggerContext; +import org.apache.logging.log4j.core.config.plugins.PluginNode; +import org.apache.logging.log4j.core.config.plugins.PluginValue; + +/** + * Test plugin class for unit tests. + */ +@Plugin(name = "Fake", category = "Test") +@PluginAliases({"AnotherFake", "StillFake"}) +public class FakePlugin { + + @Plugin(name = "Nested", category = "Test") + public static class Nested {} + + @PluginFactory + public static FakePlugin newPlugin( + @PluginAttribute("attribute") int attribute, + @PluginElement("layout") Layout layout, + @PluginConfiguration Configuration config, + @PluginNode Node node, + @PluginLoggerContext LoggerContext loggerContext, + @PluginValue("value") String value) { + return null; + } + + public static Builder newBuilder() { + return new Builder(); + } + + public static class Builder implements org.apache.logging.log4j.core.util.Builder { + + @PluginBuilderAttribute + @SuppressWarnings("log4j.public.setter") + private int attribute; + + @PluginBuilderAttribute + @SuppressWarnings("log4j.public.setter") + private int attributeWithoutPublicSetterButWithSuppressAnnotation; + + @PluginElement("layout") + private Layout layout; + + @PluginConfiguration + private Configuration config; + + @PluginNode + private Node node; + + @PluginLoggerContext + private LoggerContext loggerContext; + + @PluginValue("value") + private String value; + + @Override + public FakePlugin build() { + return null; + } + } +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessor.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessor.java index 3b1f9bd5cb6..cbe27449411 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessor.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessor.java @@ -75,8 +75,8 @@ @SupportedOptions({"log4j.graalvm.groupId", "log4j.graalvm.artifactId"}) public class GraalVmProcessor extends AbstractProcessor { - private static final String GROUP_ID = "log4j.graalvm.groupId"; - private static final String ARTIFACT_ID = "log4j.graalvm.artifactId"; + static final String GROUP_ID = "log4j.graalvm.groupId"; + static final String ARTIFACT_ID = "log4j.graalvm.artifactId"; private static final String PROCESSOR_NAME = GraalVmProcessor.class.getSimpleName(); private final Map reachableTypes = new HashMap<>(); @@ -212,18 +212,28 @@ private void writeReachabilityMetadata() { } } + // package-private for testing + static String getReachabilityMetadataPath(@Nullable String groupId, @Nullable String artifactId) { + StringBuilder pathBuilder = new StringBuilder("META-INF/native-image/log4j-generated"); + if (groupId != null && artifactId != null) { + pathBuilder.append('/').append(groupId).append('/').append(artifactId); + } + return pathBuilder.append("/reflect-config.json").toString(); + } + private String getReachabilityMetadataPath() { String groupId = processingEnv.getOptions().get(GROUP_ID); String artifactId = processingEnv.getOptions().get(ARTIFACT_ID); if (groupId == null || artifactId == null) { String message = String.format( - "The `%s` annotation processor is missing the required `%s` and `%s` options.%n" - + "The generation of GraalVM reflection metadata for your Log4j Plugins will be disabled.", + "The `%1$s` annotation processor is missing the recommended `%2$s` and `%3$s` options.%n" + + "To follow the GraalVM recommendations, please add the following options to your build tool:%n" + + " -A%2$s=%n" + + " -A%3$s=%n", PROCESSOR_NAME, GROUP_ID, ARTIFACT_ID); - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, message); - throw new IllegalArgumentException(message); + processingEnv.getMessager().printMessage(Diagnostic.Kind.WARNING, message); } - return String.format("META-INF/native-image/%s/%s/reflect-config.json", groupId, artifactId); + return getReachabilityMetadataPath(groupId, artifactId); } private void addField(TypeElement parent, VariableElement element) { diff --git a/src/changelog/.2.x.x/3771_graalvm-params.xml b/src/changelog/.2.x.x/3771_graalvm-params.xml new file mode 100644 index 00000000000..ff3c79b1efa --- /dev/null +++ b/src/changelog/.2.x.x/3771_graalvm-params.xml @@ -0,0 +1,10 @@ + + + + + Make `-Alog4j.graalvm.groupId` and `-Alog4j.graalvm.artifactId` arguments optional. + +