[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
URL:
https://github.com/apache/incubator-shardingsphere/pull/3620#discussion_r353145575
##
File path:
sharding-orchestration/sharding-orchestration-center/src/main/java/org/apache/shardingsphere/orchestration/center/instance/node/ConfigTreeNode.java
##
@@ -53,29 +53,29 @@
private Set childrenNodes;
/**
- * init tree.
+ * init config tree.
*
* @param instanceKeys instance Key set
* @param keySeparator key separator
*/
-public void initTree(final Set instanceKeys, final String
keySeparator) {
-initKeysRelationship(instanceKeys, keySeparator);
-initTree(this, SHARDING_SPHERE_KEY_ROOT);
+public void init(final Set instanceKeys, final String
keySeparator) {
+initKeysRelationships(instanceKeys, keySeparator);
+init(this, SHARDING_SPHERE_KEY_ROOT);
}
-private void initTree(final ConfigTreeNode parentNode, final String
shardingSphereKey) {
+private void init(final ConfigTreeNode parentNode, final String
shardingSphereKey) {
Review comment:
fix it, initTreeNode()
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
URL:
https://github.com/apache/incubator-shardingsphere/pull/3620#discussion_r352271164
##
File path:
sharding-orchestration/sharding-orchestration-center/src/main/java/org/apache/shardingsphere/orchestration/center/instance/node/ConfigTreeNode.java
##
@@ -0,0 +1,130 @@
+/*
+ * 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.shardingsphere.orchestration.center.instance.node;
+
+import com.google.common.base.Joiner;
+import com.google.common.collect.Sets;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Config Tree Node.
+ *
+ * @author dongzonglei
+ * @author wangguangyuan
+ * @author sunbufu
+ */
+@Getter
+@Setter
+@AllArgsConstructor
+public final class ConfigTreeNode {
+
+private static final String SHARDING_SPHERE_KEY_ROOT = "/";
+
+private static final String SHARDING_SPHERE_KEY_SEPARATOR = "/";
+
+private static Map> childrenKeyMap = new
ConcurrentHashMap<>();
Review comment:
No, The `Map` store the relationships between all the key and it's
children's keys, so I use the Map structure.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
URL:
https://github.com/apache/incubator-shardingsphere/pull/3620#discussion_r352271039
##
File path:
sharding-orchestration/sharding-orchestration-center/src/test/java/org/apache/shardingsphere/orchestration/center/instance/ApolloInstanceTest.java
##
@@ -0,0 +1,80 @@
+/*
+ * 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.shardingsphere.orchestration.center.instance;
+
+import com.ctrip.framework.apollo.Config;
+import com.ctrip.framework.apollo.openapi.client.ApolloOpenApiClient;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import lombok.SneakyThrows;
+import org.apache.shardingsphere.orchestration.center.api.ConfigCenter;
+import
org.apache.shardingsphere.orchestration.center.instance.node.ConfigTreeNode;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.internal.util.reflection.FieldSetter;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public final class ApolloInstanceTest {
+
+private static ConfigCenter configCenter = new ApolloInstance();
+
+@BeforeClass
+@SneakyThrows
+public static void init() {
+Config apolloConfig = mock(Config.class);
+when(apolloConfig.getProperty("test.children.1",
"")).thenReturn("value1");
+FieldSetter.setField(configCenter,
ApolloInstance.class.getDeclaredField("apolloConfig"), apolloConfig);
+Set instanceKeys = Sets.newHashSet();
+instanceKeys.add("test.children.1");
+instanceKeys.add("test.children.2");
+instanceKeys.add("test1.children.3");
+ConfigTreeNode root = new ConfigTreeNode(null, "/",
Sets.newHashSet());
+root.initTree(instanceKeys, ".");
Review comment:
It's a good suggestion? But what's your concrete suggestion? How to make it
more easily?
used static method?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
URL:
https://github.com/apache/incubator-shardingsphere/pull/3620#discussion_r352270951
##
File path:
sharding-orchestration/sharding-orchestration-center/src/test/java/org/apache/shardingsphere/orchestration/center/instance/ApolloInstanceTest.java
##
@@ -0,0 +1,80 @@
+/*
+ * 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.shardingsphere.orchestration.center.instance;
+
+import com.ctrip.framework.apollo.Config;
+import com.ctrip.framework.apollo.openapi.client.ApolloOpenApiClient;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import lombok.SneakyThrows;
+import org.apache.shardingsphere.orchestration.center.api.ConfigCenter;
+import
org.apache.shardingsphere.orchestration.center.instance.node.ConfigTreeNode;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.internal.util.reflection.FieldSetter;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public final class ApolloInstanceTest {
Review comment:
I'm sorry, Those methods are unit test unfriendly, `watch` is Apollo‘s
`ConfigChangeListener`, now I make unit test as Mockito's mock method, It's a
little difficult. I want to test it in dev mode.
`close` only a blank method, There’s no need `close` method for Apollo.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
URL:
https://github.com/apache/incubator-shardingsphere/pull/3620#discussion_r352270517
##
File path:
sharding-orchestration/sharding-orchestration-center/src/main/java/org/apache/shardingsphere/orchestration/center/instance/node/ConfigTreeNode.java
##
@@ -0,0 +1,130 @@
+/*
+ * 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.shardingsphere.orchestration.center.instance.node;
+
+import com.google.common.base.Joiner;
+import com.google.common.collect.Sets;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Config Tree Node.
+ *
+ * @author dongzonglei
+ * @author wangguangyuan
+ * @author sunbufu
+ */
+@Getter
+@Setter
+@AllArgsConstructor
+public final class ConfigTreeNode {
+
+private static final String SHARDING_SPHERE_KEY_ROOT = "/";
+
+private static final String SHARDING_SPHERE_KEY_SEPARATOR = "/";
+
+private static Map> childrenKeyMap = new
ConcurrentHashMap<>();
+
+private ConfigTreeNode parentNode;
+
+private String absolutePath;
+
+private Set childrenNodes;
+
+/**
+ * init tree.
+ *
+ * @param instanceKeys instance Key set
+ * @param keySeparator key separator
+ */
+public void initTree(final Set instanceKeys, final String
keySeparator) {
+initKeysRelationship(instanceKeys, keySeparator);
+initTree(this, SHARDING_SPHERE_KEY_ROOT);
+}
+
+private void initTree(final ConfigTreeNode parentNode, final String
shardingSphereKey) {
+Set childrenKeys = childrenKeyMap.get(shardingSphereKey);
+if (childrenKeys == null || childrenKeys.isEmpty()) {
+return;
+}
+for (String each : childrenKeys) {
+ConfigTreeNode child = new ConfigTreeNode(parentNode, each,
Sets.newHashSet());
+parentNode.getChildrenNodes().add(child);
+initTree(child, each);
+}
+}
+
+private void initKeysRelationship(final Set instanceKeys, final
String keySeparator) {
+for (String each : instanceKeys) {
+initKeysRelationship(each, keySeparator);
+}
+}
+
+private void initKeysRelationship(final String instanceKey, final String
keySeparator) {
+if (!instanceKey.contains(keySeparator)) {
Review comment:
Yes, I add this sentence's unit test at `ApolloInstanceTest` class.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
URL:
https://github.com/apache/incubator-shardingsphere/pull/3620#discussion_r352270277
##
File path:
sharding-orchestration/sharding-orchestration-center/src/main/java/org/apache/shardingsphere/orchestration/center/instance/ApolloInstance.java
##
@@ -0,0 +1,209 @@
+/*
+ * 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.shardingsphere.orchestration.center.instance;
+
+import com.ctrip.framework.apollo.Config;
+import com.ctrip.framework.apollo.ConfigChangeListener;
+import com.ctrip.framework.apollo.ConfigService;
+import com.ctrip.framework.apollo.core.ConfigConsts;
+import com.ctrip.framework.apollo.enums.PropertyChangeType;
+import com.ctrip.framework.apollo.model.ConfigChange;
+import com.ctrip.framework.apollo.model.ConfigChangeEvent;
+import com.ctrip.framework.apollo.openapi.client.ApolloOpenApiClient;
+import
com.ctrip.framework.apollo.openapi.client.constant.ApolloOpenApiConstants;
+import com.ctrip.framework.apollo.openapi.dto.NamespaceReleaseDTO;
+import com.ctrip.framework.apollo.openapi.dto.OpenItemDTO;
+import com.google.common.collect.Sets;
+import com.google.common.primitives.Ints;
+import lombok.Getter;
+import lombok.Setter;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shardingsphere.orchestration.center.api.ConfigCenter;
+import
org.apache.shardingsphere.orchestration.center.configuration.InstanceConfiguration;
+import
org.apache.shardingsphere.orchestration.center.instance.node.ConfigTreeNode;
+import
org.apache.shardingsphere.orchestration.center.listener.DataChangedEvent;
+import
org.apache.shardingsphere.orchestration.center.listener.DataChangedEventListener;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+import java.util.Set;
+
+/**
+ * Registry center for Apollo.
+ *
+ * @author dongzonglei
+ */
+@Slf4j
+public final class ApolloInstance implements ConfigCenter {
+
+private static final String SHARDING_SPHERE_KEY_ROOT = "/";
+
+private static final String SHARDING_SPHERE_KEY_SEPARATOR = "/";
+
+private static final String APOLLO_KEY_SEPARATOR = ".";
+
+private static final String APOLLO_KEY_APP_ID = "app.id";
+
+private static final String APOLLO_KEY_ENV = "env";
+
+private static final String APOLLO_KEY_CLUSTER =
ConfigConsts.APOLLO_CLUSTER_KEY;
+
+private static final String APOLLO_KEY_META = ConfigConsts.APOLLO_META_KEY;
+
+private String namespace;
+
+private String appId;
+
+private String env;
+
+private String clusterName;
+
+private String administrator;
+
+private Config apolloConfig;
+
+private ApolloOpenApiClient client;
+
+private ConfigTreeNode tree = new ConfigTreeNode(null, "/",
Sets.newHashSet());
+
+@Getter
+@Setter
+private Properties properties = new Properties();
+
+@Override
+public void init(final InstanceConfiguration config) {
+initApolloConfig(config);
+initApolloOpenApiClient();
+initKeysRelationship();
+}
+
+private void initApolloConfig(final InstanceConfiguration config) {
Review comment:
This is only a `private` method and inner used. The `public` method is
`init(final InstanceConfiguration config)`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
URL:
https://github.com/apache/incubator-shardingsphere/pull/3620#discussion_r352270215
##
File path:
sharding-orchestration/sharding-orchestration-center/src/main/java/org/apache/shardingsphere/orchestration/center/instance/node/ConfigTreeNode.java
##
@@ -0,0 +1,130 @@
+/*
+ * 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.shardingsphere.orchestration.center.instance.node;
+
+import com.google.common.base.Joiner;
+import com.google.common.collect.Sets;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Config Tree Node.
+ *
+ * @author dongzonglei
+ * @author wangguangyuan
+ * @author sunbufu
+ */
+@Getter
+@Setter
+@AllArgsConstructor
+public final class ConfigTreeNode {
+
+private static final String SHARDING_SPHERE_KEY_ROOT = "/";
+
+private static final String SHARDING_SPHERE_KEY_SEPARATOR = "/";
+
+private static Map> childrenKeyMap = new
ConcurrentHashMap<>();
+
+private ConfigTreeNode parentNode;
+
+private String absolutePath;
+
+private Set childrenNodes;
+
+/**
+ * init tree.
+ *
+ * @param instanceKeys instance Key set
+ * @param keySeparator key separator
+ */
+public void initTree(final Set instanceKeys, final String
keySeparator) {
+initKeysRelationship(instanceKeys, keySeparator);
+initTree(this, SHARDING_SPHERE_KEY_ROOT);
+}
+
+private void initTree(final ConfigTreeNode parentNode, final String
shardingSphereKey) {
+Set childrenKeys = childrenKeyMap.get(shardingSphereKey);
+if (childrenKeys == null || childrenKeys.isEmpty()) {
+return;
+}
+for (String each : childrenKeys) {
+ConfigTreeNode child = new ConfigTreeNode(parentNode, each,
Sets.newHashSet());
+parentNode.getChildrenNodes().add(child);
+initTree(child, each);
+}
+}
+
+private void initKeysRelationship(final Set instanceKeys, final
String keySeparator) {
Review comment:
OK.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
dongzl commented on a change in pull request #3620: Apollo as a ConfigCenter.
URL:
https://github.com/apache/incubator-shardingsphere/pull/3620#discussion_r352270210
##
File path:
sharding-orchestration/sharding-orchestration-center/src/main/java/org/apache/shardingsphere/orchestration/center/instance/node/ConfigTreeNode.java
##
@@ -0,0 +1,130 @@
+/*
+ * 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.shardingsphere.orchestration.center.instance.node;
+
+import com.google.common.base.Joiner;
+import com.google.common.collect.Sets;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Config Tree Node.
+ *
+ * @author dongzonglei
+ * @author wangguangyuan
+ * @author sunbufu
+ */
+@Getter
+@Setter
+@AllArgsConstructor
+public final class ConfigTreeNode {
+
+private static final String SHARDING_SPHERE_KEY_ROOT = "/";
+
+private static final String SHARDING_SPHERE_KEY_SEPARATOR = "/";
+
+private static Map> childrenKeyMap = new
ConcurrentHashMap<>();
+
+private ConfigTreeNode parentNode;
+
+private String absolutePath;
+
+private Set childrenNodes;
+
+/**
+ * init tree.
+ *
+ * @param instanceKeys instance Key set
+ * @param keySeparator key separator
+ */
+public void initTree(final Set instanceKeys, final String
keySeparator) {
Review comment:
OK, I fix it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
