Merge pull request #1673 from lilai23/fix_code

Optimize code and improve readability
sermant-io · Nov 22, 2024 · 6dc2fcc · 6dc2fcc
2 parents 76b68a4 + bd0c4c7
commit 6dc2fcc
Show file tree

Hide file tree

Showing 13 changed files with 107 additions and 77 deletions.
diff --git a/...rmant-agentcore-core/src/main/java/io/sermant/core/plugin/config/PluginConfigManager.java b/...rmant-agentcore-core/src/main/java/io/sermant/core/plugin/config/PluginConfigManager.java
@@ -60,31 +60,40 @@ private PluginConfigManager() {
     public static void loadPluginConfigs(Plugin plugin) {
         File pluginConfigFile = getPluginConfigFile(plugin.getPath());
         ClassLoader classLoader = plugin.getPluginClassLoader();
+
         for (BaseConfig config : ServiceLoader.load(PluginConfig.class, classLoader)) {
-            Class<? extends BaseConfig> pluginConfigCls = config.getClass();
-            String pluginConfigKey = ConfigKeyUtil.getTypeKeyWithClassloader(ConfigKeyUtil.getTypeKey(pluginConfigCls),
-                    pluginConfigCls.getClassLoader());
-            final BaseConfig retainedConfig = PLUGIN_CONFIG_MAP.get(pluginConfigKey);
-            if (pluginConfigFile.isFile()) {
-                if (retainedConfig == null) {
-                    PLUGIN_CONFIG_MAP.put(pluginConfigKey,
-                            ConfigManager.doLoad(pluginConfigFile, config, plugin.isDynamic()));
-                    plugin.getConfigs().add(pluginConfigKey);
-                } else if (retainedConfig.getClass() == pluginConfigCls) {
-                    LOGGER.log(Level.FINE, "Skip load config [{0}] repeatedly. ",
-                            pluginConfigCls.getName());
-                } else {
-                    LOGGER.log(Level.WARNING, "Type key of {0} is {1}, same as {2}'s. ",
-                            new String[]{pluginConfigCls.getName(), pluginConfigKey,
-                                    retainedConfig.getClass().getName()});
-                }
-                continue;
-            }
-            if (PLUGIN_CONFIG_MAP.containsKey(pluginConfigKey)) {
-                continue;
-            }
+            processConfig(plugin, pluginConfigFile, config);
+        }
+    }
+
+    private static void processConfig(Plugin plugin, File pluginConfigFile, BaseConfig config) {
+        Class<? extends BaseConfig> pluginConfigCls = config.getClass();
+        String pluginConfigKey = ConfigKeyUtil.getTypeKeyWithClassloader(ConfigKeyUtil.getTypeKey(pluginConfigCls),
+                pluginConfigCls.getClassLoader());
+
+        if (pluginConfigFile.isFile()) {
+            handleFileConfig(plugin, pluginConfigFile, config, pluginConfigCls, pluginConfigKey);
+        } else {
+            loadDefaultConfig(plugin, config, pluginConfigKey);
+        }
+    }
+
+    private static void handleFileConfig(Plugin plugin, File pluginConfigFile, BaseConfig config,
+            Class<? extends BaseConfig> pluginConfigCls, String pluginConfigKey) {
+        final BaseConfig retainedConfig = PLUGIN_CONFIG_MAP.get(pluginConfigKey);
+        if (retainedConfig == null) {
+            PLUGIN_CONFIG_MAP.put(pluginConfigKey, ConfigManager.doLoad(pluginConfigFile, config, plugin.isDynamic()));
+            plugin.getConfigs().add(pluginConfigKey);
+        } else if (retainedConfig.getClass() == pluginConfigCls) {
+            LOGGER.log(Level.FINE, "Skip load config [{0}] repeatedly.", pluginConfigCls.getName());
+        } else {
+            LOGGER.log(Level.WARNING, "Type key of {0} is {1}, same as {2}'s.",
+                    new String[]{pluginConfigCls.getName(), pluginConfigKey, retainedConfig.getClass().getName()});
+        }
+    }
 
-            // If it cannot be loaded from file, it is the default configuration
+    private static void loadDefaultConfig(Plugin plugin, BaseConfig config, String pluginConfigKey) {
+        if (!PLUGIN_CONFIG_MAP.containsKey(pluginConfigKey)) {
             PLUGIN_CONFIG_MAP.put(pluginConfigKey, config);
             plugin.getConfigs().add(pluginConfigKey);
         }

diff --git a/...tcore-core/src/main/java/io/sermant/core/service/xds/entity/match/RegexMatchStrategy.java b/...tcore-core/src/main/java/io/sermant/core/service/xds/entity/match/RegexMatchStrategy.java
@@ -27,6 +27,8 @@
  * @since 2024-08-15
  **/
 public class RegexMatchStrategy implements MatchStrategy {
+    private static final Pattern DEFAULT_PATTERN = Pattern.compile(".*");
+
     private final Pattern regex;
 
     /**
@@ -35,7 +37,7 @@ public class RegexMatchStrategy implements MatchStrategy {
      * @param regex regex
      */
     public RegexMatchStrategy(String regex) {
-        this.regex = StringUtils.isEmpty(regex) ? Pattern.compile(".*") : Pattern.compile(regex);
+        this.regex = StringUtils.isEmpty(regex) ? DEFAULT_PATTERN : Pattern.compile(regex);
     }
 
     @Override

diff --git a/...gentcore-implement/src/main/java/io/sermant/implement/service/xds/handler/XdsHandler.java b/...gentcore-implement/src/main/java/io/sermant/implement/service/xds/handler/XdsHandler.java
@@ -56,7 +56,7 @@
  * @since 2024-05-13
  **/
 public abstract class XdsHandler<T> implements XdsServiceAction {
-    protected static final Logger LOGGER = LoggerFactory.getLogger();
+    private static final Logger LOGGER = LoggerFactory.getLogger();
 
     private static final int DELAY_TIME = 3000;
 
@@ -180,40 +180,49 @@ protected StreamObserver<DiscoveryResponse> getResponseStreamObserver(String req
             @Override
             public void onNext(DiscoveryResponse response) {
                 handleResponse(requestKey, response);
-                if (countDownLatch != null) {
-                    countDownLatch.countDown();
-                }
+                countDown(countDownLatch);
             }
 
             @Override
             public void onError(Throwable throwable) {
-                if (countDownLatch != null) {
-                    countDownLatch.countDown();
-                }
-                initExecutor();
-                executor.submit(() -> {
-                    try {
-                        Thread.sleep(DELAY_TIME);
-                    } catch (InterruptedException e) {
-                        LOGGER.log(Level.WARNING, "An error occurred in thread sleeping.", e);
-                    }
-                    client.updateChannel();
-                    subscribe(requestKey, null);
-                });
-                LOGGER.log(Level.SEVERE, "An error occurred in Xds communication with istiod.", throwable);
+                countDown(countDownLatch);
+                handleError(throwable, requestKey);
             }
 
             @Override
             public void onCompleted() {
-                if (countDownLatch != null) {
-                    countDownLatch.countDown();
-                }
-                subscribe(requestKey, null);
-                LOGGER.log(Level.WARNING, "Xds stream is closed, new stream has been created for communication.");
+                countDown(countDownLatch);
+                handleCompletion(requestKey);
             }
         };
     }
 
+    private void countDown(CountDownLatch countDownLatch) {
+        if (countDownLatch != null) {
+            countDownLatch.countDown();
+        }
+    }
+
+    private void handleError(Throwable throwable, String requestKey) {
+        initExecutor();
+        executor.submit(() -> {
+            try {
+                Thread.sleep(DELAY_TIME);
+            } catch (InterruptedException e) {
+                LOGGER.log(Level.WARNING, "An error occurred in thread sleeping.", e);
+                Thread.currentThread().interrupt();
+            }
+            client.updateChannel();
+            subscribe(requestKey, null);
+        });
+        LOGGER.log(Level.SEVERE, "An error occurred in Xds communication with istiod.", throwable);
+    }
+
+    private void handleCompletion(String requestKey) {
+        subscribe(requestKey, null);
+        LOGGER.log(Level.WARNING, "Xds stream is closed, new stream has been created for communication.");
+    }
+
     /**
      * handle response from istiod
      *

diff --git a/...ntcore-implement/src/main/java/io/sermant/implement/service/xds/utils/XdsCommonUtils.java b/...ntcore-implement/src/main/java/io/sermant/implement/service/xds/utils/XdsCommonUtils.java
@@ -41,7 +41,7 @@ public class XdsCommonUtils {
     private XdsCommonUtils() {
     }
 
-    /***
+    /**
      * get service name from cluster
      *
      * @param clusterName cluster name

diff --git a/sermant-backend/src/main/java/io/sermant/backend/service/PageTemplateService.java b/sermant-backend/src/main/java/io/sermant/backend/service/PageTemplateService.java
@@ -103,20 +103,30 @@ private void init() {
     private void loadTemplateFile(String templatePath) {
         try (DirectoryStream<Path> stream = Files.newDirectoryStream(Paths.get(templatePath), "*.yml")) {
             for (Path entry : stream) {
-                if (!Files.isRegularFile(entry)) {
-                    continue;
-                }
-                try (InputStream inputStream = Files.newInputStream(entry)) {
-                    PageTemplateInfo pageTemplateInfo = yaml.loadAs(inputStream, PageTemplateInfo.class);
-                    if (pageTemplateInfo == null || pageTemplateInfo.getPlugin() == null) {
-                        LOGGER.warn("The page template file {} is missing plugin information.", entry.getFileName());
-                        continue;
-                    }
-                    pageTemplateInfoMap.put(pageTemplateInfo.getPlugin().getEnglishName(), pageTemplateInfo);
-                }
+                loadTemplateEntry(entry);
             }
         } catch (IOException e) {
             LOGGER.error("An error occurred while retrieving template file information", e);
         }
     }
+
+    private void loadTemplateEntry(Path entry) {
+        if (!Files.isRegularFile(entry)) {
+            return;
+        }
+        try (InputStream inputStream = Files.newInputStream(entry)) {
+            PageTemplateInfo pageTemplateInfo = yaml.loadAs(inputStream, PageTemplateInfo.class);
+            processTemplateInfo(entry, pageTemplateInfo);
+        } catch (IOException e) {
+            LOGGER.warn("Failed to load page template from file {}", entry.getFileName(), e);
+        }
+    }
+
+    private void processTemplateInfo(Path entry, PageTemplateInfo pageTemplateInfo) {
+        if (pageTemplateInfo == null || pageTemplateInfo.getPlugin() == null) {
+            LOGGER.warn("The page template file {} is missing plugin information.", entry.getFileName());
+            return;
+        }
+        pageTemplateInfoMap.put(pageTemplateInfo.getPlugin().getEnglishName(), pageTemplateInfo);
+    }
 }
diff --git a/sermant-backend/src/main/webapp/frontend/src/composables/translations.ts b/sermant-backend/src/main/webapp/frontend/src/composables/translations.ts
@@ -543,7 +543,7 @@ const i18n = createI18n({
 });
 
 export const localeLanguage = reactive({
-    value: getSavedLanguage() == 'zh' ? zh : en,
+    value: getSavedLanguage() === 'zh' ? zh : en,
     language: getSavedLanguage()
 });
 

diff --git a/...-grayscale/mq-config-common/src/main/java/io/sermant/mq/grayscale/config/GrayTagItem.java b/...-grayscale/mq-config-common/src/main/java/io/sermant/mq/grayscale/config/GrayTagItem.java
@@ -81,7 +81,7 @@ public boolean matchPropertiesByServiceMeta(Map<String, String> properties) {
      * @param grayscale grayscale
      */
     public void updateTrafficTags(List<GrayTagItem> grayscale) {
-        for (GrayTagItem item: grayscale) {
+        for (GrayTagItem item : grayscale) {
             if (StringUtils.equals(consumerGroupTag, item.getConsumerGroupTag())) {
                 setTrafficTag(item.getTrafficTag());
                 return;

diff --git a/...cale/mq-config-common/src/main/java/io/sermant/mq/grayscale/config/MqGrayscaleConfig.java b/...cale/mq-config-common/src/main/java/io/sermant/mq/grayscale/config/MqGrayscaleConfig.java
@@ -109,7 +109,7 @@ public Optional<GrayTagItem> getMatchedGrayTagByServiceMeta(Map<String, String>
      * @return gray tag item
      */
     public Optional<GrayTagItem> getGrayTagByGroupTag(String grayGroupTag) {
-        for (GrayTagItem item: grayscale) {
+        for (GrayTagItem item : grayscale) {
             if (grayGroupTag.equals(item.getConsumerGroupTag())) {
                 return Optional.of(item);
             }

diff --git a/...rc/main/java/io/sermant/mq/grayscale/rocketmq/service/RocketMqConsumerGroupAutoCheck.java b/...rc/main/java/io/sermant/mq/grayscale/rocketmq/service/RocketMqConsumerGroupAutoCheck.java
@@ -143,7 +143,7 @@ public static void findGrayConsumerGroupAndUpdateGrayTags() {
                 continue;
             }
             Set<String> grayTags = findGrayConsumerGroupAndGetTags(clientConfig);
-            LOGGER.log(Level.INFO,"[auto-check] current find gray tags: {0}.", grayTags);
+            LOGGER.log(Level.INFO, "[auto-check] current find gray tags: {0}.", grayTags);
             resetAutoCheckGrayTagItems(grayTags, clientConfig);
         }
     }
@@ -163,7 +163,7 @@ private static Set<String> findGrayConsumerGroupAndGetTags(RocketMqConsumerClien
             return getGrayTagsByConsumerGroup(groupList, brokerAddress, mqClientApi,
                     clientConfig.getConsumerGroup());
         } catch (MQClientException | InterruptedException | RemotingTimeoutException | RemotingSendRequestException
-                 | RemotingConnectException | MQBrokerException e) {
+                | RemotingConnectException | MQBrokerException e) {
             LOGGER.log(Level.FINE, String.format(Locale.ENGLISH, "[auto-check] error, message: %s",
                     e.getMessage()), e);
         }
@@ -185,7 +185,7 @@ private static Set<String> getGrayTagsByConsumerGroup(GroupList groupList, Strin
                     grayTags.add(grayTag);
                 }
             } catch (RemotingConnectException | RemotingSendRequestException | RemotingTimeoutException
-                     | MQBrokerException | InterruptedException e) {
+                    | MQBrokerException | InterruptedException e) {
                 LOGGER.warning(String.format(Locale.ENGLISH, "[auto-check] can not find ids in group: [%s].",
                         group));
             }

diff --git a/...in/src/main/java/io/sermant/mq/grayscale/rocketmq/utils/RocketMqGrayscaleConfigUtils.java b/...in/src/main/java/io/sermant/mq/grayscale/rocketmq/utils/RocketMqGrayscaleConfigUtils.java
@@ -43,7 +43,7 @@ public class RocketMqGrayscaleConfigUtils {
     /**
      * serviceMeta info
      */
-    public static final Map<String, String> MICRO_SERVICE_PROPERTIES = new HashMap<>();
+    private static final Map<String, String> MICRO_SERVICE_PROPERTIES = new HashMap<>();
 
     /**
      * consumerGroup name rule: ^[%|a-zA-Z0-9_-]+$

diff --git a/...n/src/main/java/io/sermant/mq/grayscale/rocketmq/utils/RocketMqSubscriptionDataUtils.java b/...n/src/main/java/io/sermant/mq/grayscale/rocketmq/utils/RocketMqSubscriptionDataUtils.java
@@ -20,9 +20,9 @@
 import io.sermant.mq.grayscale.config.ConsumeModeEnum;
 import io.sermant.mq.grayscale.config.GrayTagItem;
 import io.sermant.mq.grayscale.config.MqGrayConfigCache;
+import io.sermant.mq.grayscale.config.MqGrayscaleConfig;
 import io.sermant.mq.grayscale.config.rocketmq.RocketMqConfigUtils;
 import io.sermant.mq.grayscale.rocketmq.config.RocketMqConsumerClientConfig;
-import io.sermant.mq.grayscale.config.MqGrayscaleConfig;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.rocketmq.client.impl.consumer.RebalanceImpl;
@@ -244,7 +244,7 @@ private static String rebuildWithoutGrayTagSubData(String originSubData) {
         }
         String[] originConditions = PATTERN.split(originSubData);
         List<String> refactorConditions = new ArrayList<>();
-        for (String condition: originConditions) {
+        for (String condition : originConditions) {
             if (!containsGrayTags(condition) && !condition.contains("_message_tag_")) {
                 refactorConditions.add(condition);
             }

diff --git a/...nt-router/router-common/src/main/java/io/sermant/router/common/metric/MetricsManager.java b/...nt-router/router-common/src/main/java/io/sermant/router/common/metric/MetricsManager.java
@@ -57,13 +57,13 @@ public class MetricsManager {
     static {
         try {
             metricService = ServiceManager.getService(MetricService.class);
-            TAG_KEY_MAP.put("service","service_meta_service");
-            TAG_KEY_MAP.put("version","service_meta_version");
-            TAG_KEY_MAP.put("application","service_meta_application");
-            TAG_KEY_MAP.put("zone","service_meta_zone");
-            TAG_KEY_MAP.put("project","service_meta_project");
-            TAG_KEY_MAP.put("environment","service_meta_environment");
-            TAG_KEY_MAP.put("parameters","service_meta_parameters");
+            TAG_KEY_MAP.put("service", "service_meta_service");
+            TAG_KEY_MAP.put("version", "service_meta_version");
+            TAG_KEY_MAP.put("application", "service_meta_application");
+            TAG_KEY_MAP.put("zone", "service_meta_zone");
+            TAG_KEY_MAP.put("project", "service_meta_project");
+            TAG_KEY_MAP.put("environment", "service_meta_environment");
+            TAG_KEY_MAP.put("parameters", "service_meta_parameters");
         } catch (IllegalArgumentException e) {
             LOGGER.log(Level.SEVERE, "Failed to load metrics service", e);
         }
@@ -92,7 +92,7 @@ public static void addOrUpdateCounterMetricValue(String metricName, Map<String,
         } else {
             tagsMap = new HashMap<>();
         }
-        tagsMap.put(RouterConstant.SCOPE,"service-router");
+        tagsMap.put(RouterConstant.SCOPE, "service-router");
         Counter counter = COUNT_MAP.computeIfAbsent(new MetricInfo(metricName, tagsMap),
                 metricInfo -> metricService.counter(metricName, Tags.of(tagsMap)));
         counter.increment(value);

diff --git a/...outer-common/src/main/java/io/sermant/router/common/xds/lb/XdsRoundRobinLoadBalancer.java b/...outer-common/src/main/java/io/sermant/router/common/xds/lb/XdsRoundRobinLoadBalancer.java
@@ -39,7 +39,7 @@ public XdsRoundRobinLoadBalancer() {
 
     @Override
     public ServiceInstance selectInstance(List<ServiceInstance> instances) {
-        synchronized (this.getClass()) {
+        synchronized (XdsRoundRobinLoadBalancer.class) {
             // safely calculate the index based on the current size of the instances list
             int currentIndex = index.getAndUpdate(i -> (i + 1) % instances.size());
-Original file line number
+Diff line change
@@ Expand Up / @@ -41,7 +41,7 @@ public class XdsCommonUtils { @@
         private XdsCommonUtils() {
         }
-        /***
+        /**
          * get service name from cluster
          *
          * @param clusterName cluster name
@@ Expand Down @@