resolving comments from 27.5.2019

Signed-off-by: Maxim Nesen <maxim.nesen@oracle.com>

bom/pom.xml

@@ -150,7 +150,7 @@
             </dependency>
             <dependency>
                 <groupId>org.glassfish.jersey.ext</groupId>
-                <artifactId>config</artifactId>
+                <artifactId>jersey-mp-config</artifactId>
                 <version>${project.version}</version>
             </dependency>
             <dependency>

core-common/src/main/java/org/glassfish/jersey/internal/config/ExternalPropertiesAutoDiscoverable.java

@@ -28,9 +28,9 @@
 public class ExternalPropertiesAutoDiscoverable implements AutoDiscoverable {
     @Override
     public void configure(FeatureContext context) {
-        if (!context.getConfiguration().isRegistered(ExternalPropertiesConfigurationFactoryFeature.class)) {
-            ExternalPropertiesConfigurationFactoryFeature.getFactory().configure(context);
-            context.register(ExternalPropertiesConfigurationFactoryFeature.getFactory());
+        if (!context.getConfiguration().isRegistered(ExternalPropertiesConfigurationFeature.class)) {
+            ExternalPropertiesConfigurationFactory.configure(context);
+            context.register(ExternalPropertiesConfigurationFeature.class);
         }
     }
 }

core-common/src/main/java/org/glassfish/jersey/internal/config/ExternalPropertiesConfigurationFactoryFeature.java → core-common/src/main/java/org/glassfish/jersey/internal/config/ExternalPropertiesConfigurationFactory.java

@@ -21,11 +21,11 @@
 import org.glassfish.jersey.spi.ExternalConfigurationProvider;
 
 import javax.annotation.Priority;
+import javax.ws.rs.Priorities;
 import javax.ws.rs.core.Configurable;
-import javax.ws.rs.core.Feature;
-import javax.ws.rs.core.FeatureContext;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -38,26 +38,20 @@
  * Offers methods to work with properties loaded from providers or
  * just configure Jersey's Configurables with loaded properties from providers
  */
-public class ExternalPropertiesConfigurationFactoryFeature implements Feature {
+public class ExternalPropertiesConfigurationFactory {
 
-    private ExternalPropertiesConfigurationFactoryFeature() {
-    }
-
-    private static final ExternalPropertiesConfigurationFactoryFeature factory
-            = new ExternalPropertiesConfigurationFactoryFeature();
+    private static final List<ExternalConfigurationProvider> EXTERNAL_CONFIGURATION_PROVIDERS =
+            getExternalConfigurations();
 
-    public static ExternalPropertiesConfigurationFactoryFeature getFactory() {
-        return factory;
-    }
 
     /**
      * Map of merged properties from all found providers
      *
      * @return map of merged properties from all found/plugged providers
      */
-    public Map<String, Object> readExternalPropertiesMap() {
+    protected static Map<String, Object> readExternalPropertiesMap() {
 
-        final ExternalConfigurationProvider provider = mergeConfigs(getExternalConfigurations());
+        final ExternalConfigurationProvider provider = mergeConfigs(EXTERNAL_CONFIGURATION_PROVIDERS);
         return provider == null ? Collections.emptyMap() : provider.getProperties();
     }
 
@@ -69,7 +63,7 @@ public Map<String, Object> readExternalPropertiesMap() {
      * @return true if configured false otherwise
      */
 
-    public boolean configure(Configurable config) {
+    public static boolean configure(Configurable config) {
 
         if (config instanceof ExternalConfigurationModel) {
             return false; //shall not configure itself
@@ -82,17 +76,12 @@ public boolean configure(Configurable config) {
         return true;
     }
 
-    @Override
-    public boolean configure(FeatureContext configurableContext) {
-        return configure((Configurable) configurableContext);
-    }
-
     /**
      * Merged config model from all found configuration models
      *
      * @return merged Model object with all properties
      */
-    public ExternalConfigurationModel getConfig() {
+    protected static ExternalConfigurationModel getConfig() {
         final ExternalConfigurationProvider provider = mergeConfigs(getExternalConfigurations());
         return provider == null ? null : provider.getConfiguration();
     }
@@ -102,7 +91,7 @@ public ExternalConfigurationModel getConfig() {
      *
      * @return list of models (or empty list)
      */
-    public List<ExternalConfigurationProvider> getExternalConfigurations() {
+    private static List<ExternalConfigurationProvider> getExternalConfigurations() {
         final List<ExternalConfigurationProvider> providers = new ArrayList<>();
         final ServiceFinder<ExternalConfigurationProvider> finder =
                 ServiceFinder.find(ExternalConfigurationProvider.class);
@@ -114,7 +103,7 @@ public List<ExternalConfigurationProvider> getExternalConfigurations() {
         return providers;
     }
 
-    private ExternalConfigurationProvider mergeConfigs(List<ExternalConfigurationProvider> configurations) {
+    private static ExternalConfigurationProvider mergeConfigs(List<ExternalConfigurationProvider> configurations) {
         final Set<ExternalConfigurationProvider> orderedConfigurations = orderConfigs(configurations);
         final Iterator<ExternalConfigurationProvider> configurationIterator = orderedConfigurations.iterator();
         if (!configurationIterator.hasNext()) {
@@ -129,25 +118,35 @@ private ExternalConfigurationProvider mergeConfigs(List<ExternalConfigurationPro
         return firstConfig;
     }
 
-    private Set<ExternalConfigurationProvider> orderConfigs(List<ExternalConfigurationProvider> configurations) {
-
-        final SortedSet<ExternalConfigurationProvider> sortedSet = new TreeSet<>((config1, config2) -> {
+    private static Set<ExternalConfigurationProvider> orderConfigs(List<ExternalConfigurationProvider> configurations) {
 
-            if (
-                    config1.getClass().isAnnotationPresent(Priority.class)
-                           && config2.getClass().isAnnotationPresent(Priority.class)
-            ) {
-                int priority1 = config1.getClass().getAnnotation(Priority.class).value();
-                int priority2 = config2.getClass().getAnnotation(Priority.class).value();
-                if (priority1 == priority2) {
-                    return config1.getClass().getName().compareTo(config2.getClass().getName());
-                }
-                return Integer.compare(priority1, priority2);
-            }
-            return config1.getClass().getName().compareTo(config2.getClass().getName());
-        });
+        final SortedSet<ExternalConfigurationProvider> sortedSet = new TreeSet<>(new ConfigComparator());
         sortedSet.addAll(configurations);
         return Collections.unmodifiableSortedSet(sortedSet);
     }
 
+    private static class ConfigComparator implements Comparator<ExternalConfigurationProvider> {
+
+        @Override
+        public int compare(ExternalConfigurationProvider config1, ExternalConfigurationProvider config2) {
+
+            boolean config1PriorityPresent = config1.getClass().isAnnotationPresent(Priority.class);
+            boolean config2PriorityPresent = config2.getClass().isAnnotationPresent(Priority.class);
+
+            int priority1 = Priorities.USER;
+            int priority2 = Priorities.USER;
+
+            if (config1PriorityPresent) {
+                priority1 = config1.getClass().getAnnotation(Priority.class).value();
+            }
+            if (config2PriorityPresent) {
+                priority2 = config2.getClass().getAnnotation(Priority.class).value();
+            }
+
+            if (priority1 == priority2) {
+                return config1.getClass().getName().compareTo(config2.getClass().getName());
+            }
+            return Integer.compare(priority1, priority2);
+        }
+    }
 }

core-common/src/main/java/org/glassfish/jersey/internal/config/ExternalPropertiesConfigurationFeature.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0, which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the
+ * Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
+ * version 2 with the GNU Classpath Exception, which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ */
+
+package org.glassfish.jersey.internal.config;
+
+import javax.ws.rs.core.Feature;
+import javax.ws.rs.core.FeatureContext;
+
+public class ExternalPropertiesConfigurationFeature implements Feature {
+
+    @Override
+    public boolean configure(FeatureContext configurableContext) {
+        return ExternalPropertiesConfigurationFactory.configure(configurableContext);
+    }
+
+}

core-common/src/main/java/org/glassfish/jersey/internal/config/SystemPropertiesConfigurationModel.java

@@ -36,7 +36,7 @@
 import java.util.logging.Logger;
 
 
-public class SystemPropertiesConfigurationModel implements ExternalConfigurationModel<Void> {
+class SystemPropertiesConfigurationModel implements ExternalConfigurationModel<Void> {
 
     private static final Logger log = Logger.getLogger(SystemPropertiesConfigurationModel.class.getName());
 

core-common/src/main/java/org/glassfish/jersey/internal/config/SystemPropertiesConfigurationProvider.java

@@ -21,7 +21,7 @@
 
 import java.util.Map;
 
-public class SystemPropertiesConfigurationProvider implements ExternalConfigurationProvider {
+class SystemPropertiesConfigurationProvider implements ExternalConfigurationProvider {
 
     private final SystemPropertiesConfigurationModel model = new SystemPropertiesConfigurationModel();
     @Override

core-common/src/test/java/org/glassfish/jersey/internal/config/ExternalPropertiesConfigurationFactoryTest.java

@@ -17,16 +17,17 @@
 package org.glassfish.jersey.internal.config;
 
 import org.glassfish.jersey.CommonProperties;
-import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Assert;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
 import java.util.HashMap;
 import java.util.Map;
 
+import static org.glassfish.jersey.internal.config.ExternalPropertiesConfigurationFactory.getConfig;
+import static org.glassfish.jersey.internal.config.ExternalPropertiesConfigurationFactory.readExternalPropertiesMap;
+
 public class ExternalPropertiesConfigurationFactoryTest {
 
     /**
@@ -50,36 +51,33 @@ public static void tearDown() {
 
     @Test
     public void readSystemPropertiesTest() {
-        final ExternalPropertiesConfigurationFactoryFeature factory = ExternalPropertiesConfigurationFactoryFeature.getFactory();
         final Object result =
-                factory.readExternalPropertiesMap().get("jersey.config.server.provider.scanning.recursive");
+                readExternalPropertiesMap().get("jersey.config.server.provider.scanning.recursive");
         Assert.assertNull(result);
         Assert.assertEquals(Boolean.TRUE,
-                factory.getConfig().as(CommonProperties.JSON_PROCESSING_FEATURE_DISABLE, Boolean.class));
+                getConfig().as(CommonProperties.JSON_PROCESSING_FEATURE_DISABLE, Boolean.class));
         Assert.assertEquals(Boolean.FALSE,
-                factory.getConfig().as("jersey.config.client.readTimeout", Boolean.class));
+                getConfig().as("jersey.config.client.readTimeout", Boolean.class));
         Assert.assertEquals(1,
-                factory.getConfig().as(CommonProperties.JSON_PROCESSING_FEATURE_DISABLE, Integer.class));
+                getConfig().as(CommonProperties.JSON_PROCESSING_FEATURE_DISABLE, Integer.class));
         Assert.assertEquals(10,
-                factory.getConfig().as("jersey.config.client.readTimeout", Integer.class));
+                getConfig().as("jersey.config.client.readTimeout", Integer.class));
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void unsupportedMapperTest() {
-        ExternalPropertiesConfigurationFactoryFeature.getFactory()
-                .getConfig().as(CommonProperties.JSON_PROCESSING_FEATURE_DISABLE, Double.class);
+        getConfig().as(CommonProperties.JSON_PROCESSING_FEATURE_DISABLE, Double.class);
     }
 
     @Test
     public void mergePropertiesTest() {
         final Map<String, Object> inputProperties = new HashMap<>();
         inputProperties.put("jersey.config.server.provider.scanning.recursive", "MODIFIED");
         inputProperties.put("org.jersey.microprofile.config.added", "ADDED");
-        final ExternalPropertiesConfigurationFactoryFeature factory = ExternalPropertiesConfigurationFactoryFeature.getFactory();
-        factory.getConfig().mergeProperties(inputProperties);
-        final Object result = factory.readExternalPropertiesMap().get("jersey.config.server.provider.scanning.recursive");
+        getConfig().mergeProperties(inputProperties);
+        final Object result = readExternalPropertiesMap().get("jersey.config.server.provider.scanning.recursive");
         Assert.assertNull(result);
-        Assert.assertNull(factory.readExternalPropertiesMap().get("org.jersey.microprofile.config.added"));
+        Assert.assertNull(readExternalPropertiesMap().get("org.jersey.microprofile.config.added"));
     }
 
 }

core-server/src/main/java/org/glassfish/jersey/server/ResourceConfig.java

@@ -38,7 +38,7 @@
 import javax.ws.rs.core.Feature;
 
 import org.glassfish.jersey.internal.Errors;
-import org.glassfish.jersey.internal.config.ExternalPropertiesConfigurationFactoryFeature;
+import org.glassfish.jersey.internal.config.ExternalPropertiesConfigurationFactory;
 import org.glassfish.jersey.internal.inject.Binder;
 import org.glassfish.jersey.internal.inject.InjectionManager;
 import org.glassfish.jersey.internal.spi.AutoDiscoverable;
@@ -57,6 +57,7 @@
 import org.glassfish.jersey.server.internal.scanning.PackageNamesScanner;
 import org.glassfish.jersey.server.model.Resource;
 
+
 /**
  * The resource configuration for configuring a web application.
  *
@@ -714,7 +715,7 @@ final void lock() {
         final State current = state;
         if (!(current instanceof ImmutableState)) {
             setupApplicationName();
-            configure();
+            ExternalPropertiesConfigurationFactory.configure(state);
             state = new ImmutableState(current);
         }
     }
@@ -1300,7 +1301,4 @@ private void setupApplicationName() {
         }
     }
 
-    private void configure() {
-        ExternalPropertiesConfigurationFactoryFeature.getFactory().configure(state);
-    }
 }

ext/microprofile/config/pom.xml

@@ -28,7 +28,7 @@
     </parent>
     <modelVersion>4.0.0</modelVersion>
 
-    <artifactId>config</artifactId>
+    <artifactId>jersey-mp-config</artifactId>
 
     <dependencies>
 

tests/e2e-server/src/test/java/org/glassfish/jersey/tests/e2e/server/RuntimeConfigTest.java

@@ -98,7 +98,7 @@ public Object aroundReadFrom(final ReaderInterceptorContext context) throws IOEx
             assertThat(config.getProperty(InternalProperties.JSON_FEATURE_CLIENT), notNullValue());
 
             // MetaInfAutoDiscoverable
-            assertThat(config.getInstances().size(), is(2));
+            assertThat(config.getInstances().size(), is(1));
             assertTrue(config.isEnabled(ClientFeature.class));
 
             context.getHeaders().add("CustomHeader", "ClientReaderInterceptor");

tests/integration/microprofile/config/helidon/pom.xml

@@ -39,7 +39,7 @@
         </dependency>
         <dependency>
             <groupId>org.glassfish.jersey.ext</groupId>
-            <artifactId>config</artifactId>
+            <artifactId>jersey-mp-config</artifactId>
             <scope>test</scope>
         </dependency>
         <dependency>