You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2018/12/03 03:51:00 UTC

[GitHub] wu-sheng closed pull request #1989: refactoring PropertyPlaceholderHelper class.

wu-sheng closed pull request #1989: refactoring PropertyPlaceholderHelper class.
URL: https://github.com/apache/incubator-skywalking/pull/1989
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/PropertyPlaceholderHelper.java b/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/PropertyPlaceholderHelper.java
index 41957f4de..10de12200 100644
--- a/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/PropertyPlaceholderHelper.java
+++ b/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/PropertyPlaceholderHelper.java
@@ -29,14 +29,11 @@
  * ${name}}. Using {@code PropertyPlaceholderHelper} these placeholders can be substituted for user-supplied values. <p>
  * Values for substitution can be supplied using a {@link Properties} instance or using a {@link PlaceholderResolver}.
  */
-public class PropertyPlaceholderHelper {
-    private static final Map<String, String> WELL_KNOWN_SIMPLE_PREFIXES = new HashMap<String, String>(4);
+public enum PropertyPlaceholderHelper {
 
-    static {
-        WELL_KNOWN_SIMPLE_PREFIXES.put("}", "{");
-        WELL_KNOWN_SIMPLE_PREFIXES.put("]", "[");
-        WELL_KNOWN_SIMPLE_PREFIXES.put(")", "(");
-    }
+    INSTANCE(PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_PREFIX,
+        PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_SUFFIX,
+        PlaceholderConfigurerSupport.DEFAULT_VALUE_SEPARATOR, true);
 
     private final String placeholderPrefix;
 
@@ -58,14 +55,21 @@
      * @param ignoreUnresolvablePlaceholders indicates whether unresolvable placeholders should be ignored ({@code
      * true}) or cause an exception ({@code false})
      */
-    public PropertyPlaceholderHelper(String placeholderPrefix, String placeholderSuffix,
+    PropertyPlaceholderHelper(String placeholderPrefix, String placeholderSuffix,
         String valueSeparator, boolean ignoreUnresolvablePlaceholders) {
         if (StringUtil.isEmpty(placeholderPrefix) || StringUtil.isEmpty(placeholderSuffix)) {
             throw new UnsupportedOperationException("'placeholderPrefix or placeholderSuffix' must not be null");
         }
+
+        final Map<String, String> wellKnownSimplePrefixes = new HashMap<String, String>(4);
+
+        wellKnownSimplePrefixes.put("}", "{");
+        wellKnownSimplePrefixes.put("]", "[");
+        wellKnownSimplePrefixes.put(")", "(");
+
         this.placeholderPrefix = placeholderPrefix;
         this.placeholderSuffix = placeholderSuffix;
-        String simplePrefixForSuffix = WELL_KNOWN_SIMPLE_PREFIXES.get(this.placeholderSuffix);
+        String simplePrefixForSuffix = wellKnownSimplePrefixes.get(this.placeholderSuffix);
         if (simplePrefixForSuffix != null && this.placeholderPrefix.endsWith(simplePrefixForSuffix)) {
             this.simplePrefix = simplePrefixForSuffix;
         } else {
@@ -85,7 +89,8 @@ public PropertyPlaceholderHelper(String placeholderPrefix, String placeholderSuf
      */
     public String replacePlaceholders(String value, final Properties properties) {
         return replacePlaceholders(value, new PlaceholderResolver() {
-            @Override public String resolvePlaceholder(String placeholderName) {
+            @Override
+            public String resolvePlaceholder(String placeholderName) {
                 return PropertyPlaceholderHelper.this.getConfigValue(placeholderName, properties);
             }
         });
diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/SnifferConfigInitializer.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/SnifferConfigInitializer.java
index 0c4b8ba72..f17336b81 100644
--- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/SnifferConfigInitializer.java
+++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/SnifferConfigInitializer.java
@@ -34,7 +34,6 @@
 import org.apache.skywalking.apm.agent.core.logging.api.ILog;
 import org.apache.skywalking.apm.agent.core.logging.api.LogManager;
 import org.apache.skywalking.apm.util.ConfigInitializer;
-import org.apache.skywalking.apm.util.PlaceholderConfigurerSupport;
 import org.apache.skywalking.apm.util.PropertyPlaceholderHelper;
 import org.apache.skywalking.apm.util.StringUtil;
 
@@ -51,8 +50,9 @@
     private static boolean IS_INIT_COMPLETED = false;
 
     /**
-     * If the specified agent config path is set, the agent will try to locate the specified agent config.
-     * If the specified agent config path is not set , the agent will try to locate `agent.config`, which should be in the /config dictionary of agent package.
+     * If the specified agent config path is set, the agent will try to locate the specified agent config. If the
+     * specified agent config path is not set , the agent will try to locate `agent.config`, which should be in the
+     * /config dictionary of agent package.
      * <p>
      * Also try to override the config by system.env and system.properties. All the keys in these two places should
      * start with {@link #ENV_KEY_PREFIX}. e.g. in env `skywalking.agent.application_code=yourAppName` to override
@@ -67,14 +67,10 @@ public static void initialize(String agentOptions) throws ConfigNotFoundExceptio
             configFileStream = loadConfig();
             Properties properties = new Properties();
             properties.load(configFileStream);
-            PropertyPlaceholderHelper helper =
-                new PropertyPlaceholderHelper(PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_PREFIX,
-                    PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_SUFFIX,
-                    PlaceholderConfigurerSupport.DEFAULT_VALUE_SEPARATOR, true);
             for (String key : properties.stringPropertyNames()) {
                 String value = (String)properties.get(key);
                 //replace the key's value. properties.replace(key,value) in jdk8+
-                properties.put(key, helper.replacePlaceholders(value, properties));
+                properties.put(key, PropertyPlaceholderHelper.INSTANCE.replacePlaceholders(value, properties));
             }
             ConfigInitializer.initialize(properties, Config.class);
         } catch (Exception e) {
@@ -153,11 +149,10 @@ public static boolean isInitCompleted() {
     }
 
     /**
-     * Override the config by system properties. The env key must start with `skywalking`, the reuslt should be as same as in
-     * `agent.config`
+     * Override the config by system properties. The env key must start with `skywalking`, the reuslt should be as same
+     * as in `agent.config`
      * <p>
-     * such as:
-     * Env key of `agent.application_code` shoule be `skywalking.agent.application_code`
+     * such as: Env key of `agent.application_code` shoule be `skywalking.agent.application_code`
      *
      * @return the config file {@link InputStream}, or null if not needEnhance.
      */
diff --git a/apm-sniffer/apm-agent-core/src/test/java/org/apache/skywalking/apm/agent/core/conf/SnifferConfigInitializerTest.java b/apm-sniffer/apm-agent-core/src/test/java/org/apache/skywalking/apm/agent/core/conf/SnifferConfigInitializerTest.java
index 305466ef2..0ed5d4e52 100644
--- a/apm-sniffer/apm-agent-core/src/test/java/org/apache/skywalking/apm/agent/core/conf/SnifferConfigInitializerTest.java
+++ b/apm-sniffer/apm-agent-core/src/test/java/org/apache/skywalking/apm/agent/core/conf/SnifferConfigInitializerTest.java
@@ -24,7 +24,6 @@
 import org.apache.skywalking.apm.agent.core.boot.AgentPackageNotFoundException;
 import org.apache.skywalking.apm.agent.core.logging.core.LogLevel;
 import org.apache.skywalking.apm.util.ConfigInitializer;
-import org.apache.skywalking.apm.util.PlaceholderConfigurerSupport;
 import org.apache.skywalking.apm.util.PropertyPlaceholderHelper;
 import org.junit.After;
 import org.junit.Rule;
@@ -81,11 +80,7 @@ public void testConfigOverridingFromSystemEnv() throws IllegalAccessException {
         properties.put("agent.service_name", "${AGENT_SERVICE_NAME:testAppFromSystem}");
         properties.put("collector.backend_service", "${AGENT_COLLECTOR_SERVER:127.0.0.1:8090}");
         properties.put("logging.level", "INFO");
-
-        PropertyPlaceholderHelper placeholderHelper =
-            new PropertyPlaceholderHelper(PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_PREFIX,
-                PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_SUFFIX,
-                PlaceholderConfigurerSupport.DEFAULT_VALUE_SEPARATOR, true);
+        PropertyPlaceholderHelper placeholderHelper = PropertyPlaceholderHelper.INSTANCE;
         properties.put("agent.service_name", placeholderHelper.replacePlaceholders((String)properties.get("agent.service_name"), properties));
         properties.put("collector.backend_service", placeholderHelper.replacePlaceholders((String)properties.get("collector.backend_service"), properties));
         ConfigInitializer.initialize(properties, Config.class);
diff --git a/oap-server/server-library/library-util/src/test/java/org/apache/skywalking/oap/server/library/util/PropertyPlaceholderHelperTest.java b/oap-server/server-library/library-util/src/test/java/org/apache/skywalking/oap/server/library/util/PropertyPlaceholderHelperTest.java
index 6c72092f6..bd88be2d1 100644
--- a/oap-server/server-library/library-util/src/test/java/org/apache/skywalking/oap/server/library/util/PropertyPlaceholderHelperTest.java
+++ b/oap-server/server-library/library-util/src/test/java/org/apache/skywalking/oap/server/library/util/PropertyPlaceholderHelperTest.java
@@ -22,7 +22,6 @@
 import java.io.Reader;
 import java.util.Map;
 import java.util.Properties;
-import org.apache.skywalking.apm.util.PlaceholderConfigurerSupport;
 import org.apache.skywalking.apm.util.PropertyPlaceholderHelper;
 import org.junit.After;
 import org.junit.Assert;
@@ -63,10 +62,7 @@ public void init() throws FileNotFoundException {
                 }
             });
         }
-        placeholderHelper =
-            new PropertyPlaceholderHelper(PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_PREFIX,
-                PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_SUFFIX,
-                PlaceholderConfigurerSupport.DEFAULT_VALUE_SEPARATOR, true);
+        placeholderHelper = PropertyPlaceholderHelper.INSTANCE;
     }
 
     @Test
diff --git a/oap-server/server-starter/src/main/java/org/apache/skywalking/oap/server/starter/config/ApplicationConfigLoader.java b/oap-server/server-starter/src/main/java/org/apache/skywalking/oap/server/starter/config/ApplicationConfigLoader.java
index 80e965902..fdc2e7fe6 100644
--- a/oap-server/server-starter/src/main/java/org/apache/skywalking/oap/server/starter/config/ApplicationConfigLoader.java
+++ b/oap-server/server-starter/src/main/java/org/apache/skywalking/oap/server/starter/config/ApplicationConfigLoader.java
@@ -22,10 +22,9 @@
 import java.io.Reader;
 import java.util.Map;
 import java.util.Properties;
+import org.apache.skywalking.apm.util.PropertyPlaceholderHelper;
 import org.apache.skywalking.oap.server.library.module.ApplicationConfiguration;
 import org.apache.skywalking.oap.server.library.util.CollectionUtils;
-import org.apache.skywalking.apm.util.PlaceholderConfigurerSupport;
-import org.apache.skywalking.apm.util.PropertyPlaceholderHelper;
 import org.apache.skywalking.oap.server.library.util.ResourceUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -68,11 +67,8 @@ private void loadConfig(ApplicationConfiguration configuration) throws ConfigFil
                             if (propertiesConfig != null) {
                                 propertiesConfig.forEach((key, value) -> {
                                     properties.put(key, value);
-                                    PropertyPlaceholderHelper helper =
-                                        new PropertyPlaceholderHelper(PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_PREFIX,
-                                            PlaceholderConfigurerSupport.DEFAULT_PLACEHOLDER_SUFFIX,
-                                            PlaceholderConfigurerSupport.DEFAULT_VALUE_SEPARATOR, true);
-                                    final Object replaceValue = yaml.load(helper.replacePlaceholders(value + "", properties));
+                                    final Object replaceValue = yaml.load(PropertyPlaceholderHelper.INSTANCE
+                                        .replacePlaceholders(value + "", properties));
                                     properties.replace(key, replaceValue);
                                     logger.info("The property with key: {}, value: {}, in {} provider", key, replaceValue.toString(), name);
                                 });


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services