You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2020/09/15 08:32:57 UTC
[skywalking] branch master updated: Correct the module missing
check in the ModuleManagerCore (#5484)
This is an automated email from the ASF dual-hosted git repository.
wusheng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking.git
The following commit(s) were added to refs/heads/master by this push:
new 224af7f Correct the module missing check in the ModuleManagerCore (#5484)
224af7f is described below
commit 224af7f6256c7926455b5ecc5acd2cf9047e81ea
Author: 吴晟 Wu Sheng <wu...@foxmail.com>
AuthorDate: Tue Sep 15 16:32:41 2020 +0800
Correct the module missing check in the ModuleManagerCore (#5484)
* Correct the module missing check.
* Don't check the docker compose version, as the GitHub installed 3.x in default.
---
.../oap/server/library/module/BootstrapFlow.java | 34 ++++++++-----
.../oap/server/library/module/ModuleDefine.java | 2 +-
.../oap/server/library/module/ModuleManager.java | 11 +---
.../server/library/module/ModuleA2Provider.java | 58 +++++++++++++++++++++
.../server/library/module/ModuleB2Provider.java | 59 ++++++++++++++++++++++
.../server/library/module/ModuleB3Provider.java | 59 ++++++++++++++++++++++
.../server/library/module/ModuleManagerTest.java | 20 ++++++++
...alking.oap.server.library.module.ModuleProvider | 5 +-
.../e2e/docker/DockerComposeFileTest.java | 1 -
9 files changed, 223 insertions(+), 26 deletions(-)
diff --git a/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/BootstrapFlow.java b/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/BootstrapFlow.java
index a5ac9be..7a9f6e9 100644
--- a/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/BootstrapFlow.java
+++ b/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/BootstrapFlow.java
@@ -32,7 +32,7 @@ class BootstrapFlow {
private Map<String, ModuleDefine> loadedModules;
private List<ModuleProvider> startupSequence;
- BootstrapFlow(Map<String, ModuleDefine> loadedModules) throws CycleDependencyException {
+ BootstrapFlow(Map<String, ModuleDefine> loadedModules) throws CycleDependencyException, ModuleNotFoundException {
this.loadedModules = loadedModules;
startupSequence = new LinkedList<>();
@@ -43,15 +43,6 @@ class BootstrapFlow {
void start(
ModuleManager moduleManager) throws ModuleNotFoundException, ServiceNotProvidedException, ModuleStartException {
for (ModuleProvider provider : startupSequence) {
- String[] requiredModules = provider.requiredModules();
- if (requiredModules != null) {
- for (String module : requiredModules) {
- if (!moduleManager.has(module)) {
- throw new ModuleNotFoundException(module + " is required by " + provider.getModuleName() + "." + provider
- .name() + ", but not found.");
- }
- }
- }
LOGGER.info("start the provider {} in {} module.", provider.name(), provider.getModuleName());
provider.requiredCheck(provider.getModule().services());
@@ -65,9 +56,23 @@ class BootstrapFlow {
}
}
- private void makeSequence() throws CycleDependencyException {
+ private void makeSequence() throws CycleDependencyException, ModuleNotFoundException {
List<ModuleProvider> allProviders = new ArrayList<>();
- loadedModules.forEach((moduleName, module) -> allProviders.add(module.provider()));
+ for (final ModuleDefine module : loadedModules.values()) {
+ String[] requiredModules = module.provider().requiredModules();
+ if (requiredModules != null) {
+ for (String requiredModule : requiredModules) {
+ if (!loadedModules.containsKey(requiredModule)) {
+ throw new ModuleNotFoundException(
+ requiredModule + " module is required by "
+ + module.provider().getModuleName() + "."
+ + module.provider().name() + ", but not found.");
+ }
+ }
+ }
+
+ allProviders.add(module.provider());
+ }
do {
int numOfToBeSequenced = allProviders.size();
@@ -109,8 +114,9 @@ class BootstrapFlow {
.append("[provider=")
.append(provider.getClass().getName())
.append("]\n"));
- throw new CycleDependencyException("Exist cycle module dependencies in \n" + unSequencedProviders.substring(0, unSequencedProviders
- .length() - 1));
+ throw new CycleDependencyException(
+ "Exist cycle module dependencies in \n" + unSequencedProviders.substring(0, unSequencedProviders
+ .length() - 1));
}
}
while (allProviders.size() != 0);
diff --git a/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/ModuleDefine.java b/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/ModuleDefine.java
index e9c7b97..ad04576 100644
--- a/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/ModuleDefine.java
+++ b/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/ModuleDefine.java
@@ -83,7 +83,7 @@ public abstract class ModuleDefine implements ModuleProviderHolder {
}
if (loadedProvider == null) {
- throw new ProviderNotFoundException(this.name() + " module no provider exists.");
+ throw new ProviderNotFoundException(this.name() + " module no provider found.");
}
LOGGER.info("Prepare the {} provider in {} module.", loadedProvider.name(), this.name());
diff --git a/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/ModuleManager.java b/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/ModuleManager.java
index 12e8f6b..4818133 100644
--- a/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/ModuleManager.java
+++ b/oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/ModuleManager.java
@@ -28,7 +28,6 @@ import java.util.ServiceLoader;
* The <code>ModuleManager</code> takes charge of all {@link ModuleDefine}s in collector.
*/
public class ModuleManager implements ModuleDefineHolder {
-
private boolean isInPrepareStage = true;
private final Map<String, ModuleDefine> loadedModules = new HashMap<>();
@@ -45,14 +44,8 @@ public class ModuleManager implements ModuleDefineHolder {
for (ModuleDefine module : moduleServiceLoader) {
for (String moduleName : moduleNames) {
if (moduleName.equals(module.name())) {
- ModuleDefine newInstance;
- try {
- newInstance = module.getClass().newInstance();
- } catch (InstantiationException | IllegalAccessException e) {
- throw new ModuleNotFoundException(e);
- }
- newInstance.prepare(this, applicationConfiguration.getModuleConfiguration(moduleName), moduleProviderLoader);
- loadedModules.put(moduleName, newInstance);
+ module.prepare(this, applicationConfiguration.getModuleConfiguration(moduleName), moduleProviderLoader);
+ loadedModules.put(moduleName, module);
moduleList.remove(moduleName);
}
}
diff --git a/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleA2Provider.java b/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleA2Provider.java
new file mode 100644
index 0000000..aca758a
--- /dev/null
+++ b/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleA2Provider.java
@@ -0,0 +1,58 @@
+/*
+ * 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.skywalking.oap.server.library.module;
+
+public class ModuleA2Provider extends ModuleProvider {
+ @Override
+ public String name() {
+ return "P-A2";
+ }
+
+ @Override
+ public ModuleConfig createConfigBeanIfAbsent() {
+ return null;
+ }
+
+ @Override
+ public Class<? extends ModuleDefine> module() {
+ return BaseModuleA.class;
+ }
+
+ @Override
+ public void prepare() throws ServiceNotProvidedException {
+ this.registerServiceImplementation(BaseModuleA.ServiceABusiness1.class, new ModuleABusiness1Impl());
+ this.registerServiceImplementation(BaseModuleA.ServiceABusiness2.class, new ModuleABusiness2Impl());
+ }
+
+ @Override
+ public void start() {
+ }
+
+ @Override
+ public void notifyAfterCompleted() {
+ }
+
+ @Override
+ public String[] requiredModules() {
+ return new String[] {"BaseB"};
+ }
+
+ class Config {
+ }
+}
diff --git a/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleB2Provider.java b/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleB2Provider.java
new file mode 100644
index 0000000..0eda621
--- /dev/null
+++ b/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleB2Provider.java
@@ -0,0 +1,59 @@
+/*
+ * 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.skywalking.oap.server.library.module;
+
+public class ModuleB2Provider extends ModuleProvider {
+
+ @Override
+ public String name() {
+ return "P-B2";
+ }
+
+ @Override
+ public ModuleConfig createConfigBeanIfAbsent() {
+ return null;
+ }
+
+ @Override
+ public Class<? extends ModuleDefine> module() {
+ return BaseModuleB.class;
+ }
+
+ @Override
+ public void prepare() throws ServiceNotProvidedException {
+ this.registerServiceImplementation(BaseModuleB.ServiceBBusiness1.class, new ModuleBBusiness1Impl());
+ this.registerServiceImplementation(BaseModuleB.ServiceBBusiness2.class, new ModuleBBusiness2Impl());
+ }
+
+ @Override
+ public void start() {
+ }
+
+ @Override
+ public void notifyAfterCompleted() {
+ }
+
+ @Override
+ public String[] requiredModules() {
+ return new String[] {"dummy"};
+ }
+
+ class Config {
+ }
+}
diff --git a/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleB3Provider.java b/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleB3Provider.java
new file mode 100644
index 0000000..b427326
--- /dev/null
+++ b/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleB3Provider.java
@@ -0,0 +1,59 @@
+/*
+ * 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.skywalking.oap.server.library.module;
+
+public class ModuleB3Provider extends ModuleProvider {
+
+ @Override
+ public String name() {
+ return "P-B3";
+ }
+
+ @Override
+ public ModuleConfig createConfigBeanIfAbsent() {
+ return null;
+ }
+
+ @Override
+ public Class<? extends ModuleDefine> module() {
+ return BaseModuleB.class;
+ }
+
+ @Override
+ public void prepare() throws ServiceNotProvidedException {
+ this.registerServiceImplementation(BaseModuleB.ServiceBBusiness1.class, new ModuleBBusiness1Impl());
+ this.registerServiceImplementation(BaseModuleB.ServiceBBusiness2.class, new ModuleBBusiness2Impl());
+ }
+
+ @Override
+ public void start() {
+ }
+
+ @Override
+ public void notifyAfterCompleted() {
+ }
+
+ @Override
+ public String[] requiredModules() {
+ return new String[] {"BaseA"};
+ }
+
+ class Config {
+ }
+}
diff --git a/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleManagerTest.java b/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleManagerTest.java
index 4f23083..8071f8b 100644
--- a/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleManagerTest.java
+++ b/oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleManagerTest.java
@@ -38,4 +38,24 @@ public class ModuleManagerTest {
.getService(BaseModuleA.ServiceABusiness1.class);
Assert.assertTrue(serviceABusiness1 != null);
}
+
+ @Test(expected = ModuleNotFoundException.class)
+ public void testModuleMissing() throws ModuleConfigException, ModuleNotFoundException, ModuleStartException {
+ ApplicationConfiguration configuration = new ApplicationConfiguration();
+ configuration.addModule("BaseA").addProviderConfiguration("P-A", new Properties());
+ configuration.addModule("BaseB").addProviderConfiguration("P-B2", new Properties());
+
+ ModuleManager manager = new ModuleManager();
+ manager.init(configuration);
+ }
+
+ @Test(expected = CycleDependencyException.class)
+ public void testCycleDependency() throws ModuleConfigException, ModuleNotFoundException, ModuleStartException {
+ ApplicationConfiguration configuration = new ApplicationConfiguration();
+ configuration.addModule("BaseA").addProviderConfiguration("P-A2", new Properties());
+ configuration.addModule("BaseB").addProviderConfiguration("P-B3", new Properties());
+
+ ModuleManager manager = new ModuleManager();
+ manager.init(configuration);
+ }
}
diff --git a/oap-server/server-library/library-module/src/test/resources/META-INF/services/org.apache.skywalking.oap.server.library.module.ModuleProvider b/oap-server/server-library/library-module/src/test/resources/META-INF/services/org.apache.skywalking.oap.server.library.module.ModuleProvider
index 8efb813..2501a8d 100644
--- a/oap-server/server-library/library-module/src/test/resources/META-INF/services/org.apache.skywalking.oap.server.library.module.ModuleProvider
+++ b/oap-server/server-library/library-module/src/test/resources/META-INF/services/org.apache.skywalking.oap.server.library.module.ModuleProvider
@@ -18,4 +18,7 @@
org.apache.skywalking.oap.server.library.module.TestModuleProvider
org.apache.skywalking.oap.server.library.module.ModuleAProvider
-org.apache.skywalking.oap.server.library.module.ModuleBProvider
\ No newline at end of file
+org.apache.skywalking.oap.server.library.module.ModuleA2Provider
+org.apache.skywalking.oap.server.library.module.ModuleBProvider
+org.apache.skywalking.oap.server.library.module.ModuleB2Provider
+org.apache.skywalking.oap.server.library.module.ModuleB3Provider
\ No newline at end of file
diff --git a/test/e2e/e2e-common/src/test/java/org/apache/skywalking/e2e/docker/DockerComposeFileTest.java b/test/e2e/e2e-common/src/test/java/org/apache/skywalking/e2e/docker/DockerComposeFileTest.java
index 5562c8f..8dd84f2 100644
--- a/test/e2e/e2e-common/src/test/java/org/apache/skywalking/e2e/docker/DockerComposeFileTest.java
+++ b/test/e2e/e2e-common/src/test/java/org/apache/skywalking/e2e/docker/DockerComposeFileTest.java
@@ -59,7 +59,6 @@ class DockerComposeFileTest {
Assert.assertNotNull(testFile.getServices());
Assert.assertEquals(COMPOSE_FILE_ONE.getServices().size() + COMPOSE_FILE_TWO.getServices().size(),
testFile.getServices().size());
- Assert.assertEquals(COMPOSE_FILE_ONE.getVersion(), testFile.getVersion());
}
@Test