Merge pull request quarkusio#31666 from ppalaga/i4192

Validate the default JAXBContext at build time only if it is really u…

extensions/jaxb/deployment/pom.xml

@@ -28,7 +28,12 @@
 
         <dependency>
             <groupId>io.quarkus</groupId>
-            <artifactId>quarkus-junit5</artifactId>
+            <artifactId>quarkus-junit5-internal</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
             <scope>test</scope>
         </dependency>
     </dependencies>

extensions/jaxb/deployment/src/main/java/io/quarkus/jaxb/deployment/FilteredJaxbClassesToBeBoundBuildItem.java

@@ -0,0 +1,70 @@
+package io.quarkus.jaxb.deployment;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import io.quarkus.builder.item.SimpleBuildItem;
+import io.quarkus.jaxb.deployment.utils.JaxbType;
+
+/**
+ * List of classes to be bound in the JAXB context. Aggregates all classes passed via
+ * {@link JaxbClassesToBeBoundBuildItem}. All class names excluded via {@code quarkus.jaxb.exclude-classes} are not
+ * present in this list.
+ */
+public final class FilteredJaxbClassesToBeBoundBuildItem extends SimpleBuildItem {
+
+    private final List<Class<?>> classes;
+
+    public static Builder builder() {
+        return new Builder();
+    }
+
+    private FilteredJaxbClassesToBeBoundBuildItem(List<Class<?>> classes) {
+        this.classes = classes;
+    }
+
+    public List<Class<?>> getClasses() {
+        return new ArrayList<>(classes);
+    }
+
+    public static class Builder {
+        private final Set<String> classNames = new LinkedHashSet<>();
+        private final Set<String> classNameExcludes = new LinkedHashSet<>();
+
+        public Builder classNameExcludes(Collection<String> classNameExcludes) {
+            for (String className : classNameExcludes) {
+                this.classNameExcludes.add(className);
+            }
+            return this;
+        }
+
+        public Builder classNames(Collection<String> classNames) {
+            for (String className : classNames) {
+                this.classNames.add(className);
+            }
+            return this;
+        }
+
+        public FilteredJaxbClassesToBeBoundBuildItem build() {
+            final List<Class<?>> classes = classNames.stream()
+                    .filter(className -> !this.classNameExcludes.contains(className))
+                    .map(FilteredJaxbClassesToBeBoundBuildItem::getClassByName)
+                    .filter(JaxbType::isValidType)
+                    .collect(Collectors.toList());
+
+            return new FilteredJaxbClassesToBeBoundBuildItem(classes);
+        }
+    }
+
+    private static Class<?> getClassByName(String name) {
+        try {
+            return Class.forName(name, false, Thread.currentThread().getContextClassLoader());
+        } catch (ClassNotFoundException e) {
+            throw new RuntimeException(e);
+        }
+    }
+}

extensions/jaxb/deployment/src/main/java/io/quarkus/jaxb/deployment/JaxbClassesToBeBoundBuildItem.java

@@ -1,19 +1,24 @@
 package io.quarkus.jaxb.deployment;
 
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 
 import io.quarkus.builder.item.MultiBuildItem;
 
 /**
- * List of classes to be bound in the JAXB context.
+ * List of class names to be bound in the JAXB context. Note that some of the class names can be removed via
+ * {@code quarkus.jaxb.exclude-classes}.
+ *
+ * @see FilteredJaxbClassesToBeBoundBuildItem
  */
 public final class JaxbClassesToBeBoundBuildItem extends MultiBuildItem {
 
     private final List<String> classes;
 
     public JaxbClassesToBeBoundBuildItem(List<String> classes) {
-        this.classes = Objects.requireNonNull(classes);
+        this.classes = Objects.requireNonNull(Collections.unmodifiableList(new ArrayList<>(classes)));
     }
 
     public List<String> getClasses() {

extensions/jaxb/deployment/src/main/java/io/quarkus/jaxb/deployment/JaxbProcessor.java

@@ -1,19 +1,16 @@
 package io.quarkus.jaxb.deployment;
 
-import static io.quarkus.jaxb.deployment.utils.JaxbType.isValidType;
-
 import java.io.IOError;
 import java.io.IOException;
 import java.lang.annotation.Annotation;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Stream;
 
+import jakarta.enterprise.inject.spi.DeploymentException;
 import jakarta.xml.bind.JAXBContext;
 import jakarta.xml.bind.JAXBException;
 import jakarta.xml.bind.annotation.XmlAccessOrder;
@@ -58,6 +55,9 @@
 import org.jboss.logging.Logger;
 
 import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
+import io.quarkus.arc.deployment.SynthesisFinishedBuildItem;
+import io.quarkus.arc.processor.BeanInfo;
+import io.quarkus.arc.processor.BeanResolver;
 import io.quarkus.deployment.ApplicationArchive;
 import io.quarkus.deployment.annotations.BuildProducer;
 import io.quarkus.deployment.annotations.BuildStep;
@@ -297,29 +297,62 @@ void registerClasses(
     }
 
     @BuildStep
-    @Record(ExecutionTime.STATIC_INIT)
-    void setupJaxbContextConfig(JaxbConfig config,
-            List<JaxbClassesToBeBoundBuildItem> classesToBeBoundBuildItems,
-            JaxbContextConfigRecorder jaxbContextConfig) {
-        Set<String> classNamesToBeBound = new HashSet<>();
-        for (JaxbClassesToBeBoundBuildItem classesToBeBoundBuildItem : classesToBeBoundBuildItems) {
-            classNamesToBeBound.addAll(classesToBeBoundBuildItem.getClasses());
-        }
+    FilteredJaxbClassesToBeBoundBuildItem filterBoundClasses(
+            JaxbConfig config,
+            List<JaxbClassesToBeBoundBuildItem> classesToBeBoundBuildItems) {
+
+        FilteredJaxbClassesToBeBoundBuildItem.Builder builder = FilteredJaxbClassesToBeBoundBuildItem.builder();
+        classesToBeBoundBuildItems.stream()
+                .map(JaxbClassesToBeBoundBuildItem::getClasses)
+                .forEach(builder::classNames);
 
         // remove classes that have been excluded by users
         if (config.excludeClasses.isPresent()) {
-            classNamesToBeBound.removeAll(config.excludeClasses.get());
+            builder.classNameExcludes(config.excludeClasses.get());
         }
+        return builder.build();
+    }
+
+    @BuildStep
+    @Record(ExecutionTime.STATIC_INIT)
+    void setupJaxbContextConfig(
+            FilteredJaxbClassesToBeBoundBuildItem filteredClassesToBeBound,
+            JaxbContextConfigRecorder jaxbContextConfig) {
+        jaxbContextConfig.addClassesToBeBound(filteredClassesToBeBound.getClasses());
+    }
+
+    @BuildStep
+    @Record(ExecutionTime.STATIC_INIT)
+    void validateDefaultJaxbContext(
+            JaxbConfig config,
+            FilteredJaxbClassesToBeBoundBuildItem filteredClassesToBeBound,
+            SynthesisFinishedBuildItem beanContainerState,
+            JaxbContextConfigRecorder jaxbContextConfig /* Force the build time container to invoke this method */) {
 
-        // parse class names to class
-        Set<Class<?>> classes = getAllClassesFromClassNames(classNamesToBeBound);
         if (config.validateJaxbContext) {
-            // validate the context to fail at build time if it's not valid
-            validateContext(classes);
+            final BeanResolver beanResolver = beanContainerState.getBeanResolver();
+            final Set<BeanInfo> beans = beanResolver
+                    .resolveBeans(Type.create(DotName.createSimple(JAXBContext.class), org.jboss.jandex.Type.Kind.CLASS));
+            if (!beans.isEmpty()) {
+                final BeanInfo bean = beanResolver.resolveAmbiguity(beans);
+                if (bean.isDefaultBean()) {
+                    /*
+                     * Validate the default JAXB context at build time and fail early.
+                     * Do this only if the user application actually requires the default JAXBContext bean
+                     */
+                    try {
+                        JAXBContext.newInstance(filteredClassesToBeBound.getClasses().toArray(new Class[0]));
+                    } catch (JAXBException e) {
+                        /*
+                         * Producing a ValidationErrorBuildItem would perhaps be more natural here,
+                         * but doing so causes a cycle between this and reactive JAXB extension
+                         * Throwing from here works well too
+                         */
+                        throw new DeploymentException("Failed to create or validate the default JAXBContext", e);
+                    }
+                }
+            }
         }
-
-        // register the classes to be used at runtime
-        jaxbContextConfig.addClassesToBeBound(classes);
     }
 
     @BuildStep
@@ -388,31 +421,4 @@ private void addResourceBundle(BuildProducer<NativeImageResourceBundleBuildItem>
         resourceBundle.produce(new NativeImageResourceBundleBuildItem(bundle));
     }
 
-    private void validateContext(Set<Class<?>> classes) {
-        try {
-            JAXBContext.newInstance(classes.toArray(new Class[0]));
-        } catch (JAXBException e) {
-            throw new IllegalStateException("Failed to configure JAXB context", e);
-        }
-    }
-
-    private Set<Class<?>> getAllClassesFromClassNames(Collection<String> classNames) {
-        Set<Class<?>> classes = new HashSet<>();
-        for (String className : classNames) {
-            Class<?> clazz = getClassByName(className);
-            if (isValidType(clazz)) {
-                classes.add(clazz);
-            }
-        }
-
-        return classes;
-    }
-
-    private Class<?> getClassByName(String name) {
-        try {
-            return Class.forName(name, false, Thread.currentThread().getContextClassLoader());
-        } catch (ClassNotFoundException e) {
-            throw new RuntimeException(e);
-        }
-    }
 }

extensions/jaxb/deployment/src/test/java/io/quarkus/jaxb/deployment/ConflictingModelClassesMarshalerOnlyTest.java

@@ -0,0 +1,54 @@
+package io.quarkus.jaxb.deployment;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.List;
+
+import jakarta.enterprise.context.control.ActivateRequestContext;
+import jakarta.enterprise.inject.spi.DeploymentException;
+import jakarta.inject.Inject;
+import jakarta.xml.bind.JAXBContext;
+import jakarta.xml.bind.Marshaller;
+
+import org.assertj.core.api.Assertions;
+import org.glassfish.jaxb.core.v2.runtime.IllegalAnnotationException;
+import org.glassfish.jaxb.runtime.v2.runtime.IllegalAnnotationsException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+
+/**
+ * Make sure that the validation of the default JAXB context fails if there conflicting model classes and there is only
+ * a {@link Marshaller} injection point (which actually requires a {@link JAXBContext} bean to be available too).
+ */
+public class ConflictingModelClassesMarshalerOnlyTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest config = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(
+                            io.quarkus.jaxb.deployment.one.Model.class,
+                            io.quarkus.jaxb.deployment.two.Model.class))
+            .assertException(e -> {
+                assertThat(e).isInstanceOf(DeploymentException.class);
+                assertThat(e.getMessage()).isEqualTo("Failed to create or validate the default JAXBContext");
+                Throwable cause = e.getCause();
+                assertThat(cause).isInstanceOf(IllegalAnnotationsException.class);
+                assertThat(cause.getMessage()).isEqualTo("1 counts of IllegalAnnotationExceptions");
+                List<IllegalAnnotationException> errors = ((IllegalAnnotationsException) cause).getErrors();
+                assertThat(errors.size()).isEqualTo(1);
+                assertThat(errors.get(0).getMessage()).contains("Two classes have the same XML type name \"model\"");
+
+            });
+
+    @Inject
+    Marshaller marshaller;
+
+    @Test
+    @ActivateRequestContext
+    public void shouldFail() {
+        Assertions.fail("The application should fail at boot");
+    }
+
+}

extensions/jaxb/deployment/src/test/java/io/quarkus/jaxb/deployment/ConflictingModelClassesTest.java

@@ -0,0 +1,53 @@
+package io.quarkus.jaxb.deployment;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.List;
+
+import jakarta.enterprise.context.control.ActivateRequestContext;
+import jakarta.enterprise.inject.spi.DeploymentException;
+import jakarta.inject.Inject;
+import jakarta.xml.bind.JAXBContext;
+
+import org.assertj.core.api.Assertions;
+import org.glassfish.jaxb.core.v2.runtime.IllegalAnnotationException;
+import org.glassfish.jaxb.runtime.v2.runtime.IllegalAnnotationsException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+
+/**
+ * Make sure that the validation of the default JAXB context fails if there conflicting model classes and there actually
+ * is a {@link JAXBContext} injection point.
+ */
+public class ConflictingModelClassesTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest config = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(
+                            io.quarkus.jaxb.deployment.one.Model.class,
+                            io.quarkus.jaxb.deployment.two.Model.class))
+            .assertException(e -> {
+                assertThat(e).isInstanceOf(DeploymentException.class);
+                assertThat(e.getMessage()).isEqualTo("Failed to create or validate the default JAXBContext");
+                Throwable cause = e.getCause();
+                assertThat(cause).isInstanceOf(IllegalAnnotationsException.class);
+                assertThat(cause.getMessage()).isEqualTo("1 counts of IllegalAnnotationExceptions");
+                List<IllegalAnnotationException> errors = ((IllegalAnnotationsException) cause).getErrors();
+                assertThat(errors.size()).isEqualTo(1);
+                assertThat(errors.get(0).getMessage()).contains("Two classes have the same XML type name \"model\"");
+
+            });
+
+    @Inject
+    JAXBContext jaxbContext;
+
+    @Test
+    @ActivateRequestContext
+    public void shouldFail() {
+        Assertions.fail("The application should fail at boot");
+    }
+
+}