Check duplicate providers in bootstrap (#2976)

* Enhance module manager

oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/storage/StorageInstallerTestCase.java

@@ -18,12 +18,14 @@
 
 package org.apache.skywalking.oap.server.core.storage;
 
-import java.util.LinkedList;
-import org.apache.skywalking.oap.server.core.*;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.CoreModuleProvider;
 import org.apache.skywalking.oap.server.core.remote.define.StreamDataMapping;
-import org.apache.skywalking.oap.server.core.storage.model.*;
+import org.apache.skywalking.oap.server.core.storage.model.Model;
+import org.apache.skywalking.oap.server.core.storage.model.ModelInstaller;
 import org.apache.skywalking.oap.server.library.client.Client;
-import org.apache.skywalking.oap.server.library.module.*;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.module.ServiceNotProvidedException;
 import org.junit.Test;
 import org.mockito.Mockito;
 import org.powermock.reflect.Whitebox;
@@ -40,8 +42,7 @@ public class StorageInstallerTestCase {
         CoreModule moduleDefine = Mockito.spy(CoreModule.class);
         ModuleManager moduleManager = Mockito.mock(ModuleManager.class);
 
-        LinkedList<ModuleProvider> moduleProviders = Whitebox.getInternalState(moduleDefine, "loadedProviders");
-        moduleProviders.add(moduleProvider);
+        Whitebox.setInternalState(moduleDefine, "loadedProvider", moduleProvider);
 
         Mockito.when(moduleManager.find(CoreModule.NAME)).thenReturn(moduleDefine);
         Mockito.when(moduleProvider.getService(StreamDataMapping.class)).thenReturn(streamDataMapping);

oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/BootstrapFlow.java

@@ -18,9 +18,13 @@
 
 package org.apache.skywalking.oap.server.library.module;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
 import org.apache.skywalking.oap.server.library.util.CollectionUtils;
-import org.slf4j.*;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * @author wu-sheng, peng-yongsheng
@@ -66,7 +70,7 @@ class BootstrapFlow {
 
     private void makeSequence() throws CycleDependencyException {
         List<ModuleProvider> allProviders = new ArrayList<>();
-        loadedModules.forEach((moduleName, module) -> allProviders.addAll(module.providers()));
+        loadedModules.forEach((moduleName, module) -> allProviders.add(module.provider()));
 
         do {
             int numOfToBeSequenced = allProviders.size();

oap-server/server-library/library-module/src/main/java/org/apache/skywalking/oap/server/library/module/ModuleDefine.java

@@ -19,8 +19,11 @@
 package org.apache.skywalking.oap.server.library.module;
 
 import java.lang.reflect.Field;
-import java.util.*;
-import org.slf4j.*;
+import java.util.Enumeration;
+import java.util.Properties;
+import java.util.ServiceLoader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * A module definition.
@@ -31,7 +34,7 @@ public abstract class ModuleDefine implements ModuleProviderHolder {
 
     private static final Logger logger = LoggerFactory.getLogger(ModuleDefine.class);
 
-    private final LinkedList<ModuleProvider> loadedProviders = new LinkedList<>();
+    private ModuleProvider loadedProvider = null;
 
     private final String name;
 
@@ -60,39 +63,35 @@ public abstract class ModuleDefine implements ModuleProviderHolder {
      */
     void prepare(ModuleManager moduleManager, ApplicationConfiguration.ModuleConfiguration configuration,
         ServiceLoader<ModuleProvider> moduleProviderLoader) throws ProviderNotFoundException, ServiceNotProvidedException, ModuleConfigException, ModuleStartException {
-        boolean providerExist = false;
         for (ModuleProvider provider : moduleProviderLoader) {
             if (!configuration.has(provider.name())) {
                 continue;
             }
 
-            providerExist = true;
             if (provider.module().equals(getClass())) {
-                ModuleProvider newProvider;
-                try {
-                    newProvider = provider.getClass().newInstance();
-                } catch (InstantiationException | IllegalAccessException e) {
-                    throw new ProviderNotFoundException(e);
+                if (loadedProvider == null) {
+                    loadedProvider = provider;
+                    loadedProvider.setManager(moduleManager);
+                    loadedProvider.setModuleDefine(this);
+                } else {
+                    throw new DuplicateProviderException(this.name() + " module has one " + loadedProvider.name() + "[" + loadedProvider.getClass().getName() + "] provider already, "
+                        + provider.name() + "[" + provider.getClass().getName() + "] is defined as 2nd provider.");
                 }
-                newProvider.setManager(moduleManager);
-                newProvider.setModuleDefine(this);
-                loadedProviders.add(newProvider);
             }
+
         }
 
-        if (!providerExist) {
+        if (loadedProvider == null) {
             throw new ProviderNotFoundException(this.name() + " module no provider exists.");
         }
 
-        for (ModuleProvider moduleProvider : loadedProviders) {
-            logger.info("Prepare the {} provider in {} module.", moduleProvider.name(), this.name());
-            try {
-                copyProperties(moduleProvider.createConfigBeanIfAbsent(), configuration.getProviderConfiguration(moduleProvider.name()), this.name(), moduleProvider.name());
-            } catch (IllegalAccessException e) {
-                throw new ModuleConfigException(this.name() + " module config transport to config bean failure.", e);
-            }
-            moduleProvider.prepare();
+        logger.info("Prepare the {} provider in {} module.", loadedProvider.name(), this.name());
+        try {
+            copyProperties(loadedProvider.createConfigBeanIfAbsent(), configuration.getProviderConfiguration(loadedProvider.name()), this.name(), loadedProvider.name());
+        } catch (IllegalAccessException e) {
+            throw new ModuleConfigException(this.name() + " module config transport to config bean failure.", e);
         }
+        loadedProvider.prepare();
     }
 
     private void copyProperties(ModuleConfig dest, Properties src, String moduleName,
@@ -129,20 +128,11 @@ public abstract class ModuleDefine implements ModuleProviderHolder {
         throw new NoSuchFieldException();
     }
 
-    /**
-     * @return providers of this module
-     */
-    final List<ModuleProvider> providers() {
-        return loadedProviders;
-    }
-
     @Override public final ModuleProvider provider() throws DuplicateProviderException, ProviderNotFoundException {
-        if (loadedProviders.size() > 1) {
-            throw new DuplicateProviderException(this.name() + " module exist " + loadedProviders.size() + " providers");
-        } else if (loadedProviders.size() == 0) {
+        if (loadedProvider == null) {
             throw new ProviderNotFoundException("There is no module provider in " + this.name() + " module!");
         }
 
-        return loadedProviders.getFirst();
+        return loadedProvider;
     }
 }