Safeguard against JAX-RS app modifications after start. (#1486)

Signed-off-by: Tomas Langer <tomas.langer@oracle.com>

microprofile/server/src/main/java/io/helidon/microprofile/server/JaxRsCdiExtension.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2020 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,11 +22,15 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
+import javax.annotation.Priority;
+import javax.enterprise.context.ApplicationScoped;
+import javax.enterprise.context.Initialized;
 import javax.enterprise.event.Observes;
 import javax.enterprise.inject.spi.Extension;
 import javax.enterprise.inject.spi.ProcessAnnotatedType;
@@ -45,6 +49,8 @@
 import org.eclipse.microprofile.config.ConfigProvider;
 import org.glassfish.jersey.server.ResourceConfig;
 
+import static javax.interceptor.Interceptor.Priority.PLATFORM_BEFORE;
+
 /**
  * Configure Jersey related things.
  */
@@ -59,6 +65,7 @@ public class JaxRsCdiExtension implements Extension {
 
     private final Set<Class<? extends Application>> applications = new LinkedHashSet<>();
     private final Set<Class<?>> resources = new HashSet<>();
+    private final AtomicBoolean setInStone = new AtomicBoolean(false);
 
     private void collectApplications(@Observes ProcessAnnotatedType<? extends Application> applicationType) {
         applications.add(applicationType.getAnnotatedType().getJavaClass());
@@ -74,12 +81,26 @@ private void collectResourceClasses(@Observes @WithAnnotations(Path.class) Proce
         resources.add(resourceClass);
     }
 
+    // once application scoped starts, we do not allow modification of applications
+    void fixApps(@Observes @Priority(PLATFORM_BEFORE) @Initialized(ApplicationScoped.class) Object event) {
+        this.setInStone.set(true);
+    }
+
     /**
      * List of applications including discovered and explicitly configured applications.
+     * <p>
+     * This method should only be called in {@code Initialized(ApplicationScoped.class)} observer methods,
+     *  that have a higher priority than {@link io.helidon.microprofile.server.ServerCdiExtension} start server
+     *  method.
      *
      * @return list of applications found by CDI
+     * @throws java.lang.IllegalStateException in case the list of applications is not yet fixed
      */
-    public List<JaxRsApplication> applicationsToRun() {
+    public List<JaxRsApplication> applicationsToRun() throws IllegalStateException {
+        if (!setInStone.get()) {
+            throw new IllegalStateException("Applications are not yet fixed. This method is only available in "
+                                                    + "@Initialized(ApplicationScoped.class) event, before server is started");
+        }
         if (applications.isEmpty() && applicationMetas.isEmpty()) {
             // create a synthetic application from all resource classes
             // the classes set must be created before the lambda, as resources are cleared later on
@@ -117,15 +138,21 @@ public Set<Class<?>> getClasses() {
 
     /**
      * Remove all discovered applications (configured applications are not removed).
+     *
+     * @throws java.lang.IllegalStateException in case applications are already started
      */
-    public void removeApplications() {
+    public void removeApplications() throws IllegalStateException {
+        mutateApps();
         this.applications.clear();
     }
 
     /**
      * Remove all discovered and configured resource classes.
+     *
+     * @throws java.lang.IllegalStateException in case applications are already started
      */
-    public void removeResourceClasses() {
+    public void removeResourceClasses() throws IllegalStateException {
+        mutateApps();
         this.resources.clear();
     }
 
@@ -135,18 +162,23 @@ public void removeResourceClasses() {
      * on other configuration.
      *
      * @param resourceClasses resource classes to add
+     * @throws java.lang.IllegalStateException in case applications are already started
      */
-    public void addResourceClasses(List<Class<?>> resourceClasses) {
+    public void addResourceClasses(List<Class<?>> resourceClasses) throws IllegalStateException {
+        mutateApps();
         this.resources.addAll(resourceClasses);
     }
 
     /**
      * Add all application metadata from the provided list.
      *
      * @param applications application metadata
+     * @throws java.lang.IllegalStateException in case applications are already started
+     *
      * @see io.helidon.microprofile.server.JaxRsApplication
      */
-    public void addApplications(List<JaxRsApplication> applications) {
+    public void addApplications(List<JaxRsApplication> applications) throws IllegalStateException {
+        mutateApps();
         this.applicationMetas.addAll(applications);
     }
 
@@ -155,8 +187,10 @@ public void addApplications(List<JaxRsApplication> applications) {
      * You can also use {@link #addApplication(String, Application)}.
      *
      * @param application configured as needed
+     * @throws java.lang.IllegalStateException in case applications are already started
      */
-    public void addApplication(Application application) {
+    public void addApplication(Application application) throws IllegalStateException {
+        mutateApps();
         this.applicationMetas.add(JaxRsApplication.create(application));
     }
 
@@ -165,25 +199,16 @@ public void addApplication(Application application) {
      *
      * @param contextRoot Context root to use for this application ({@link javax.ws.rs.ApplicationPath} is ignored)
      * @param application configured as needed
+     * @throws java.lang.IllegalStateException in case applications are already started
      */
-    public void addApplication(String contextRoot, Application application) {
+    public void addApplication(String contextRoot, Application application) throws IllegalStateException {
+        mutateApps();
         this.applicationMetas.add(JaxRsApplication.builder()
                                           .application(application)
                                           .contextRoot(contextRoot)
                                           .build());
     }
 
-    /**
-     * Access existing applications explicitly configured. Does not include discovered applications.
-     *
-     * @return list of all applications
-     */
-    public List<ResourceConfig> applications() {
-        return applicationMetas.stream()
-                .map(JaxRsApplication::resourceConfig)
-                .collect(Collectors.toList());
-    }
-
     /**
      * Makes an attempt to "guess" the service name.
      * <p>
@@ -215,8 +240,10 @@ private Optional<String> guessServiceName() {
      * Create an application from the provided resource classes and add it to the list of applications.
      *
      * @param resourceClasses resource classes to create a synthetic application from
+     * @throws java.lang.IllegalStateException in case applications are already started
      */
-    public void addSyntheticApplication(List<Class<?>> resourceClasses) {
+    public void addSyntheticApplication(List<Class<?>> resourceClasses) throws IllegalStateException {
+        mutateApps();
         this.applicationMetas.add(JaxRsApplication.builder()
                                           .applicationClass(Application.class)
                                           .config(ResourceConfig.forApplication(new Application() {
@@ -265,4 +292,11 @@ boolean isNamedRoutingRequired(io.helidon.config.Config config, JaxRsApplication
                 .asBoolean()
                 .orElseGet(jaxRsApplication::routingNameRequired);
     }
+
+    private void mutateApps() {
+        if (setInStone.get()) {
+            throw new IllegalStateException("You are attempting to modify applications in JAX-RS after they were registered "
+                                                    + "with the server");
+        }
+    }
 }

microprofile/server/src/test/java/io/helidon/microprofile/server/JaxRsCdiExtensionTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2020 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,6 +15,7 @@
  */
 package io.helidon.microprofile.server;
 
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
@@ -28,6 +29,7 @@
 import static io.helidon.config.testing.OptionalMatcher.value;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Unit test for {@link io.helidon.microprofile.server.JaxRsCdiExtension}.
@@ -157,4 +159,18 @@ void testContextRootNoConfigWithTrailingSlash() {
         Optional<String> contextRoot = extension.findContextRoot(EMPTY_CONFIG, app);
         assertThat(contextRoot, value(is("/myApp")));
     }
+
+    @Test
+    void testAppsNotModifiableAfterUse() {
+        JaxRsCdiExtension ext = new JaxRsCdiExtension();
+        ext.fixApps(new Object());
+
+        assertThrows(IllegalStateException.class, () -> ext.addApplication(new JaxRsApplicationTest.MyApplication()));
+        assertThrows(IllegalStateException.class, () -> ext.addApplication("/", new JaxRsApplicationTest.MyApplication()));
+        assertThrows(IllegalStateException.class, () -> ext.addApplications(List.of()));
+        assertThrows(IllegalStateException.class, () -> ext.addResourceClasses(List.of()));
+        assertThrows(IllegalStateException.class, () -> ext.addSyntheticApplication(List.of()));
+        assertThrows(IllegalStateException.class, ext::removeApplications);
+        assertThrows(IllegalStateException.class, ext::removeResourceClasses);
+    }
 }