You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2021/03/23 09:27:40 UTC

[servicecomb-java-chassis] branch master updated: [SCB-2234] configuration for instance properties should support placeholder (#2310)

This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git


The following commit(s) were added to refs/heads/master by this push:
     new 1156705  [SCB-2234] configuration for instance properties should support placeholder (#2310)
1156705 is described below

commit 11567053d21dcf08f9292d2996c2594ca11348f0
Author: wujimin <wu...@huawei.com>
AuthorDate: Tue Mar 23 17:27:29 2021 +0800

    [SCB-2234] configuration for instance properties should support placeholder (#2310)
---
 .../org/apache/servicecomb/core/SCBEngine.java     |   9 +-
 .../servicecomb/config/BootStrapProperties.java    |  33 +++--
 .../org/apache/servicecomb/config/ConfigUtil.java  |   6 +-
 .../config/BootStrapPropertiesTest.java            | 148 +++++++++++++++++++++
 .../servicecomb/registry/RegistrationManager.java  |   6 +-
 foundations/foundation-test-scaffolding/pom.xml    |   5 +
 .../foundation/test/scaffolding/AssertUtils.java   |  33 +++++
 .../org/apache/servicecomb/zeroconfig/Config.java  |   2 +-
 8 files changed, 218 insertions(+), 24 deletions(-)

diff --git a/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java b/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
index 39640f2..5c6d3d5 100644
--- a/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
+++ b/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
@@ -342,8 +342,13 @@ public class SCBEngine {
     serviceInfo.append("Service information is shown below:\n");
     for (BootUpInformationCollector bootUpInformationCollector : bootUpInformationCollectors) {
       String info = bootUpInformationCollector.collect(this);
-      if (!StringUtils.isEmpty(info)) {
-        serviceInfo.append(info).append('\n');
+      if (StringUtils.isEmpty(info)) {
+        continue;
+      }
+
+      serviceInfo.append(info);
+      if (!info.endsWith("\n")) {
+        serviceInfo.append('\n');
       }
     }
     LOGGER.info(serviceInfo.toString());
diff --git a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/BootStrapProperties.java b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/BootStrapProperties.java
index 97c18a8..f1b5158 100644
--- a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/BootStrapProperties.java
+++ b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/BootStrapProperties.java
@@ -17,8 +17,7 @@
 
 package org.apache.servicecomb.config;
 
-import java.util.HashMap;
-import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -263,24 +262,24 @@ public class BootStrapProperties {
   }
 
   private static Map<String, String> readProperties(Configuration configuration, String newKey, String oldKey) {
-    Map<String, String> propertiesMap = new HashMap<>();
-
-    String prefix = newKey;
-
-    Iterator<String> keysIterator = configuration.getKeys(prefix);
-
-    if (!keysIterator.hasNext()) {
-      prefix = oldKey;
-      keysIterator = configuration.getKeys(prefix);
+    Configuration subset = configuration.subset(newKey);
+    if (subset.isEmpty()) {
+      subset = configuration.subset(oldKey);
     }
-
-    while (keysIterator.hasNext()) {
-      String key = keysIterator.next();
-      propertiesMap.put(key.substring(prefix.length() + 1), String.valueOf(configuration.getProperty(key)));
-    }
-    return propertiesMap;
+    return toStringMap(subset);
   }
 
+  private static Map<String, String> toStringMap(Configuration configuration) {
+    Map<String, String> map = new LinkedHashMap<>();
+    configuration.getKeys().forEachRemaining(key -> {
+      try {
+        map.put(key, configuration.getString(key));
+      } catch (Exception e) {
+        map.put(key, String.valueOf(configuration.getProperty(key)));
+      }
+    });
+    return map;
+  }
 
   private static void checkMicroserviceName(String name) {
     // the configuration we used
diff --git a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/ConfigUtil.java b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/ConfigUtil.java
index 7b76fc4..2b288d4 100644
--- a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/ConfigUtil.java
+++ b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/ConfigUtil.java
@@ -91,7 +91,7 @@ public final class ConfigUtil {
   }
 
   public static Object getProperty(String key) {
-    Object config = DynamicPropertyFactory.getBackingConfigurationSource();
+    AbstractConfiguration config = ConfigurationManager.getConfigInstance();
     return getProperty(config, key);
   }
 
@@ -103,6 +103,10 @@ public final class ConfigUtil {
     return null;
   }
 
+  public static List<String> getStringList(@Nonnull String key) {
+    return getStringList(ConfigurationManager.getConfigInstance(), key);
+  }
+
   public static List<String> getStringList(@Nonnull Configuration config, @Nonnull String key) {
     return config.getList(key).stream()
         .map(v -> Objects.toString(v, null))
diff --git a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/BootStrapPropertiesTest.java b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/BootStrapPropertiesTest.java
new file mode 100644
index 0000000..488546a
--- /dev/null
+++ b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/BootStrapPropertiesTest.java
@@ -0,0 +1,148 @@
+/*
+ * 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 org.apache.servicecomb.config;
+
+import static org.apache.servicecomb.foundation.test.scaffolding.AssertUtils.assertPrettyJson;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Map;
+
+import org.apache.commons.configuration.MapConfiguration;
+import org.junit.jupiter.api.Test;
+
+class BootStrapPropertiesTest {
+  private Map<String, String> readInstanceProperties(String yaml) {
+    Map<String, Object> properties = YAMLUtil.yaml2Properties(yaml);
+    MapConfiguration configuration = new MapConfiguration(properties);
+
+    return BootStrapProperties.readServiceInstanceProperties(configuration);
+  }
+
+  @Test
+  void should_be_empty_when_old_and_new_key_not_exists() {
+    Map<String, String> properties = readInstanceProperties("k: v");
+
+    assertThat(properties).isEmpty();
+  }
+
+  @Test
+  void should_read_boolean_to_string() {
+    Map<String, String> properties = readInstanceProperties(""
+        + "servicecomb:\n"
+        + "  instance:\n"
+        + "    properties:\n"
+        + "      k: true");
+
+    assertPrettyJson(properties).isEqualTo(""
+        + "{\n"
+        + "  \"k\" : \"true\"\n"
+        + "}");
+  }
+
+  @Test
+  void should_read_number_to_string() {
+    Map<String, String> properties = readInstanceProperties(""
+        + "servicecomb:\n"
+        + "  instance:\n"
+        + "    properties:\n"
+        + "      k: 1");
+
+    assertPrettyJson(properties).isEqualTo(""
+        + "{\n"
+        + "  \"k\" : \"1\"\n"
+        + "}");
+  }
+
+  @Test
+  void should_resolve_placeholder_for_instance_properties() {
+    Map<String, String> properties = readInstanceProperties(""
+        + "k1: new\n"
+        + "servicecomb:\n"
+        + "  instance:\n"
+        + "    properties:\n"
+        + "      k: ${k1}");
+
+    assertPrettyJson(properties).isEqualTo(""
+        + "{\n"
+        + "  \"k\" : \"new\"\n"
+        + "}");
+  }
+
+  @Test
+  void should_resolve_boolean_to_string() {
+    Map<String, String> properties = readInstanceProperties(""
+        + "k: true\n"
+        + "servicecomb:\n"
+        + "  instance:\n"
+        + "    properties:\n"
+        + "      k: ${k}");
+
+    assertPrettyJson(properties).isEqualTo(""
+        + "{\n"
+        + "  \"k\" : \"true\"\n"
+        + "}");
+  }
+
+  @Test
+  void should_resolve_number_to_string() {
+    Map<String, String> properties = readInstanceProperties(""
+        + "k: 1\n"
+        + "servicecomb:\n"
+        + "  instance:\n"
+        + "    properties:\n"
+        + "      k: ${k}");
+
+    assertPrettyJson(properties).isEqualTo(""
+        + "{\n"
+        + "  \"k\" : \"1\"\n"
+        + "}");
+  }
+
+  @Test
+  void should_read_by_old_prefix_when_new_prefix_not_exists() {
+    Map<String, String> properties = readInstanceProperties(""
+        + "instance_description:\n"
+        + "  properties:\n"
+        + "    k: v\n"
+        + "    k1: v1");
+
+    assertPrettyJson(properties).isEqualTo(""
+        + "{\n"
+        + "  \"k\" : \"v\",\n"
+        + "  \"k1\" : \"v1\"\n"
+        + "}");
+  }
+
+  @Test
+  void should_ignore_old_prefix_when_new_prefix_exists() {
+    Map<String, String> properties = readInstanceProperties(""
+        + "instance_description:\n"
+        + "  properties:\n"
+        + "    k: v\n"
+        + "    k1: v1\n"
+        + "servicecomb:\n"
+        + "  instance:\n"
+        + "    properties:\n"
+        + "      k: new");
+
+    assertPrettyJson(properties).isEqualTo(""
+        + "{\n"
+        + "  \"k\" : \"new\"\n"
+        + "}");
+  }
+}
\ No newline at end of file
diff --git a/foundations/foundation-registry/src/main/java/org/apache/servicecomb/registry/RegistrationManager.java b/foundations/foundation-registry/src/main/java/org/apache/servicecomb/registry/RegistrationManager.java
index ecf6d5a..065800d 100644
--- a/foundations/foundation-registry/src/main/java/org/apache/servicecomb/registry/RegistrationManager.java
+++ b/foundations/foundation-registry/src/main/java/org/apache/servicecomb/registry/RegistrationManager.java
@@ -315,9 +315,9 @@ public class RegistrationManager {
         result.append("Registration implementations:\n");
       }
 
-      result.append("\tname:" + registration.name() + "\n");
-      result.append("\t\tService ID: " + registration.getMicroservice().getServiceId() + "\n");
-      result.append("\t\tInstance ID: " + registration.getMicroserviceInstance().getInstanceId() + "\n");
+      result.append("  name:" + registration.name() + "\n");
+      result.append("    Service ID: " + registration.getMicroservice().getServiceId() + "\n");
+      result.append("    Instance ID: " + registration.getMicroserviceInstance().getInstanceId() + "\n");
     });
     return result.toString();
   }
diff --git a/foundations/foundation-test-scaffolding/pom.xml b/foundations/foundation-test-scaffolding/pom.xml
index 5532fba..720d47b 100644
--- a/foundations/foundation-test-scaffolding/pom.xml
+++ b/foundations/foundation-test-scaffolding/pom.xml
@@ -68,5 +68,10 @@
       <groupId>org.springframework</groupId>
       <artifactId>spring-aspects</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.assertj</groupId>
+      <artifactId>assertj-core</artifactId>
+      <scope>compile</scope>
+    </dependency>
   </dependencies>
 </project>
diff --git a/foundations/foundation-test-scaffolding/src/main/java/org/apache/servicecomb/foundation/test/scaffolding/AssertUtils.java b/foundations/foundation-test-scaffolding/src/main/java/org/apache/servicecomb/foundation/test/scaffolding/AssertUtils.java
new file mode 100644
index 0000000..8456054
--- /dev/null
+++ b/foundations/foundation-test-scaffolding/src/main/java/org/apache/servicecomb/foundation/test/scaffolding/AssertUtils.java
@@ -0,0 +1,33 @@
+/*
+ * 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 org.apache.servicecomb.foundation.test.scaffolding;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.assertj.core.api.AbstractStringAssert;
+
+import io.netty.handler.codec.string.LineSeparator;
+import io.vertx.core.json.Json;
+
+public interface AssertUtils {
+  static AbstractStringAssert<?> assertPrettyJson(Object value) {
+    String json = Json.encodePrettily(value)
+        .replace(LineSeparator.WINDOWS.value(), LineSeparator.UNIX.value());
+    return assertThat(json);
+  }
+}
\ No newline at end of file
diff --git a/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/Config.java b/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/Config.java
index 9b69e04..2356c41 100644
--- a/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/Config.java
+++ b/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/Config.java
@@ -35,7 +35,7 @@ import org.apache.servicecomb.config.DynamicProperties;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
-@Component
+@Component("zero-config-model")
 public class Config {
   private DynamicProperties dynamicProperties;