From 224af7f6256c7926455b5ecc5acd2cf9047e81ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=90=B4=E6=99=9F=20Wu=20Sheng?= Date: Tue, 15 Sep 2020 16:32:41 +0800 Subject: [PATCH] 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. --- .../server/library/module/BootstrapFlow.java | 34 ++++++----- .../server/library/module/ModuleDefine.java | 2 +- .../server/library/module/ModuleManager.java | 11 +--- .../library/module/ModuleA2Provider.java | 58 ++++++++++++++++++ .../library/module/ModuleB2Provider.java | 59 +++++++++++++++++++ .../library/module/ModuleB3Provider.java | 59 +++++++++++++++++++ .../library/module/ModuleManagerTest.java | 20 +++++++ ...g.oap.server.library.module.ModuleProvider | 5 +- .../e2e/docker/DockerComposeFileTest.java | 1 - 9 files changed, 223 insertions(+), 26 deletions(-) create mode 100644 oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleA2Provider.java create mode 100644 oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleB2Provider.java create mode 100644 oap-server/server-library/library-module/src/test/java/org/apache/skywalking/oap/server/library/module/ModuleB3Provider.java 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 a5ac9be12..7a9f6e99a 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 loadedModules; private List startupSequence; - BootstrapFlow(Map loadedModules) throws CycleDependencyException { + BootstrapFlow(Map 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 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 e9c7b97c9..ad04576c1 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 12e8f6b71..481813376 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 ModuleManager takes charge of all {@link ModuleDefine}s in collector. */ public class ModuleManager implements ModuleDefineHolder { - private boolean isInPrepareStage = true; private final Map 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 000000000..aca758a63 --- /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 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 000000000..0eda621b3 --- /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 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 000000000..b42732629 --- /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 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 4f2308357..8071f8b6c 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 8efb813c8..2501a8dc4 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 5562c8f7f..8dd84f276 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