refactor code based on Intellij's check result (#27474)

* refactor code (#27287) merge change of appconfiguration
Azure · Mar 10, 2022 · 223834a · 223834a
1 parent 6846985
commit 223834a
Show file tree

Hide file tree

Showing 35 changed files with 113 additions and 127 deletions.
diff --git a/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml b/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml
@@ -340,6 +340,7 @@ the main ServiceBusClientBuilder. -->
   <suppress checks="IllegalImport"
             files="com.azure.cosmos.ClientSideRequestStatistics"/> <!-- Need OperatingSystemMXBean from sun to obtain cpu info -->
   <suppress checks="EnforceFinalFields" files="com.azure.spring.cloud.config.AppConfigurationPropertySourceLocator"/>
+  <suppress checks="ConstantName" files="com.azure.spring.cloud.config.AppConfigurationPropertySourceLocator"/>
 
   <!-- Checkstyle suppressions for resource manager package -->
   <suppress checks="com.azure.tools.checkstyle.checks.ServiceClientCheck" files="com.azure.resourcemanager.*"/>

diff --git a/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml b/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml
@@ -2318,6 +2318,12 @@
     <Bug pattern="UPM_UNCALLED_PRIVATE_METHOD"/>
   </Match>
 
+  <!-- Exception needs to be caught here.-->
+  <Match>
+    <Class name="com.azure.spring.cloud.config.AppConfigurationPropertySourceLocator"/>
+    <Bug pattern="REC_CATCH_EXCEPTION"/>
+  </Match>
+
   <!-- Exclude from spring related classes -->
   <Match>
     <Or>

diff --git a/...g/src/main/java/com/azure/spring/cloud/config/AppConfigurationBootstrapConfiguration.java b/...g/src/main/java/com/azure/spring/cloud/config/AppConfigurationBootstrapConfiguration.java
@@ -2,10 +2,13 @@
 // Licensed under the MIT License.
 package com.azure.spring.cloud.config;
 
-import java.util.Arrays;
-import java.util.List;
-import java.util.Optional;
-
+import com.azure.spring.cloud.config.properties.AppConfigurationProperties;
+import com.azure.spring.cloud.config.properties.AppConfigurationProviderProperties;
+import com.azure.spring.cloud.config.properties.ConfigStore;
+import com.azure.spring.cloud.config.resource.AppConfigManagedIdentityProperties;
+import com.azure.spring.cloud.config.resource.Connection;
+import com.azure.spring.cloud.config.resource.ConnectionPool;
+import com.azure.spring.cloud.config.stores.ClientStore;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
@@ -17,13 +20,8 @@
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
-import com.azure.spring.cloud.config.properties.AppConfigurationProperties;
-import com.azure.spring.cloud.config.properties.AppConfigurationProviderProperties;
-import com.azure.spring.cloud.config.properties.ConfigStore;
-import com.azure.spring.cloud.config.resource.AppConfigManagedIdentityProperties;
-import com.azure.spring.cloud.config.resource.Connection;
-import com.azure.spring.cloud.config.resource.ConnectionPool;
-import com.azure.spring.cloud.config.stores.ClientStore;
+import java.util.List;
+import java.util.Optional;
 
 /**
  * Setup ConnectionPool, AppConfigurationPropertySourceLocator, and ClientStore when
@@ -157,11 +155,10 @@ public ClientStore buildClientStores(AppConfigurationProperties properties,
         boolean isDev = false;
         boolean isKeyVaultConfigured = false;
 
-        List<String> profiles = Arrays.asList(env.getActiveProfiles());
-
-        for (String profile : profiles) {
+        for (String profile : env.getActiveProfiles()) {
             if ("dev".equalsIgnoreCase(profile)) {
                 isDev = true;
+                break;
             }
         }
 

diff --git a/...on-config/src/main/java/com/azure/spring/cloud/config/AppConfigurationPropertySource.java b/...on-config/src/main/java/com/azure/spring/cloud/config/AppConfigurationPropertySource.java
@@ -114,7 +114,7 @@ public final class AppConfigurationPropertySource extends EnumerablePropertySour
         this.profiles = profiles;
         this.appConfigurationProperties = appConfigurationProperties;
         this.appProperties = appProperties;
-        this.keyVaultClients = new HashMap<String, KeyVaultClient>();
+        this.keyVaultClients = new HashMap<>();
         this.clients = clients;
         this.keyVaultCredentialProvider = keyVaultCredentialProvider;
         this.keyVaultClientProvider = keyVaultClientProvider;
@@ -167,9 +167,6 @@ FeatureSet initProperties(FeatureSet featureSet) throws IOException {
                 .setLabelFilter(configStore.getFeatureFlags().getLabelFilter());
             features = clients.listSettings(settingSelector, storeName);
 
-            if (features == null) {
-                throw new IOException("Unable to load properties from App Configuration Store.");
-            }
         }
 
         List<String> labels = Arrays.asList(selectedKeys.getLabelFilter(profiles));
@@ -182,10 +179,6 @@ FeatureSet initProperties(FeatureSet featureSet) throws IOException {
             // * for wildcard match
             PagedIterable<ConfigurationSetting> settings = clients.listSettings(settingSelector, storeName);
 
-            if (settings == null) {
-                throw new IOException("Unable to load properties from App Configuration Store.");
-            }
-
             for (ConfigurationSetting setting : settings) {
                 String key = setting.getKey().trim().substring(selectedKeys.getKeyFilter().length()).replace('/', '.');
                 if (setting instanceof SecretReferenceConfigurationSetting) {
@@ -259,18 +252,15 @@ void initFeatures(FeatureSet featureSet) {
             FEATURE_MAPPER.convertValue(featureSet.getFeatureManagement(), LinkedHashMap.class));
     }
 
-    private FeatureSet addToFeatureSet(FeatureSet featureSet, PagedIterable<ConfigurationSetting> features, Date date)
-        throws IOException {
+    private FeatureSet addToFeatureSet(FeatureSet featureSet, PagedIterable<ConfigurationSetting> features, Date date) {
         if (features == null) {
             return featureSet;
         }
         // Reading In Features
         for (ConfigurationSetting setting : features) {
             if (setting instanceof FeatureFlagConfigurationSetting) {
                 Object feature = createFeature((FeatureFlagConfigurationSetting) setting);
-                if (feature != null) {
-                    featureSet.addFeature(setting.getKey().trim().substring(FEATURE_FLAG_PREFIX.length()), feature);
-                }
+                featureSet.addFeature(setting.getKey().trim().substring(FEATURE_FLAG_PREFIX.length()), feature);
             }
         }
         return featureSet;

diff --git a/...ig/src/main/java/com/azure/spring/cloud/config/AppConfigurationPropertySourceLocator.java b/...ig/src/main/java/com/azure/spring/cloud/config/AppConfigurationPropertySourceLocator.java
@@ -56,9 +56,9 @@ public final class AppConfigurationPropertySourceLocator implements PropertySour
 
     private final KeyVaultSecretProvider keyVaultSecretProvider;
 
-    private static AtomicBoolean configloaded = new AtomicBoolean(false);
+    private static final AtomicBoolean configloaded = new AtomicBoolean(false);
 
-    private static AtomicBoolean startup = new AtomicBoolean(true);
+    private static final AtomicBoolean startup = new AtomicBoolean(true);
 
     /**
      * Loads all Azure App Configuration Property Sources configured.
@@ -103,7 +103,7 @@ public PropertySource<?> locate(Environment environment) {
         while (configStoreIterator.hasNext()) {
             ConfigStore configStore = configStoreIterator.next();
 
-            Boolean loadNewPropertySources = startup.get() || StateHolder.getLoadState(configStore.getEndpoint());
+            boolean loadNewPropertySources = startup.get() || StateHolder.getLoadState(configStore.getEndpoint());
 
             if (configStore.isEnabled() && loadNewPropertySources) {
                 addPropertySource(composite, configStore, profiles, !configStoreIterator.hasNext());
@@ -192,8 +192,8 @@ private List<AppConfigurationPropertySource> create(ConfigStore store, List<Stri
             }
 
             // Setting new ETag values for Watch
-            List<ConfigurationSetting> watchKeysSettings = new ArrayList<ConfigurationSetting>();
-            List<ConfigurationSetting> watchKeysFeatures = new ArrayList<ConfigurationSetting>();
+            List<ConfigurationSetting> watchKeysSettings = new ArrayList<>();
+            List<ConfigurationSetting> watchKeysFeatures = new ArrayList<>();
 
             for (AppConfigurationStoreTrigger trigger : store.getMonitoring().getTriggers()) {
                 ConfigurationSetting watchKey = clients.getWatchKey(trigger.getKey(), trigger.getLabel(),
@@ -213,9 +213,7 @@ private List<AppConfigurationPropertySource> create(ConfigStore store, List<Stri
                 PagedIterable<ConfigurationSetting> watchKeys = clients.getFeatureFlagWatchKey(settingSelector,
                     store.getEndpoint());
 
-                watchKeys.forEach(watchKey -> {
-                    watchKeysFeatures.add(NormalizeNull.normalizeNullLabel(watchKey));
-                });
+                watchKeys.forEach(watchKey -> watchKeysFeatures.add(NormalizeNull.normalizeNullLabel(watchKey)));
 
                 StateHolder.setStateFeatureFlag(store.getEndpoint(), watchKeysFeatures,
                     store.getMonitoring().getFeatureFlagRefreshInterval());
@@ -224,9 +222,6 @@ private List<AppConfigurationPropertySource> create(ConfigStore store, List<Stri
 
             StateHolder.setState(store.getEndpoint(), watchKeysSettings, store.getMonitoring().getRefreshInterval());
             StateHolder.setLoadState(store.getEndpoint(), true);
-        } catch (RuntimeException e) {
-            delayException();
-            throw e;
         } catch (Exception e) {
             delayException();
             throw e;

diff --git a/...iguration-config/src/main/java/com/azure/spring/cloud/config/AppConfigurationRefresh.java b/...iguration-config/src/main/java/com/azure/spring/cloud/config/AppConfigurationRefresh.java
@@ -62,7 +62,7 @@ public AppConfigurationRefresh(AppConfigurationProperties properties, ClientStor
         this.clientStore = clientStore;
         this.eventDataInfo = "";
         this.clientHealth = new HashMap<>();
-        configStores.stream().forEach(store -> {
+        configStores.forEach(store -> {
             if (getStoreHealthState(store)) {
                 this.clientHealth.put(store.getEndpoint(), AppConfigurationStoreHealth.UP);
             } else {
@@ -114,7 +114,7 @@ private boolean refreshStores() {
         if (running.compareAndSet(false, true)) {
             BaseAppConfigurationPolicy.setWatchRequests(true);
             Map<String, AppConfigurationStoreHealth> clientHealthUpdate = new HashMap<>();
-            configStores.stream().forEach(store -> {
+            configStores.forEach(store -> {
                 if (getStoreHealthState(store)) {
                     clientHealthUpdate.put(store.getEndpoint(), AppConfigurationStoreHealth.DOWN);
                 } else {

diff --git a/...iguration-config/src/main/java/com/azure/spring/cloud/config/JsonConfigurationParser.java b/...iguration-config/src/main/java/com/azure/spring/cloud/config/JsonConfigurationParser.java
@@ -2,18 +2,16 @@
 // Licensed under the MIT License.
 package com.azure.spring.cloud.config;
 
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-
-import org.springframework.util.StringUtils;
-
 import com.azure.data.appconfiguration.models.ConfigurationSetting;
 import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.JsonMappingException;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import org.springframework.util.StringUtils;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
 
 final class JsonConfigurationParser {
     private static final ObjectMapper MAPPER = new ObjectMapper();
@@ -34,8 +32,8 @@ static boolean isJsonContentType(String contentType) {
                 if (subType.contains("+")) {
                     List<String> subtypes = Arrays.asList(subType.split("\\+"));
                     return subtypes.contains(acceptedSubType);
-                } else if (subType.equalsIgnoreCase(acceptedSubType)) {
-                    return true;
+                } else {
+                    return subType.equalsIgnoreCase(acceptedSubType);
                 }
             }
         }
@@ -44,8 +42,8 @@ static boolean isJsonContentType(String contentType) {
     }
 
     static HashMap<String, Object> parseJsonSetting(ConfigurationSetting setting)
-        throws JsonMappingException, JsonProcessingException {
-        HashMap<String, Object> settings = new HashMap<String, Object>();
+        throws JsonProcessingException {
+        HashMap<String, Object> settings = new HashMap<>();
         JsonNode json = MAPPER.readTree(setting.getValue());
         parseSetting(setting.getKey(), json, settings);
         return settings;

diff --git a/...loud-appconfiguration-config/src/main/java/com/azure/spring/cloud/config/StateHolder.java b/...loud-appconfiguration-config/src/main/java/com/azure/spring/cloud/config/StateHolder.java
@@ -39,7 +39,7 @@ static State getStateFeatureFlag(String endpoint) {
     /**
      * @param endpoint the stores endpoint
      * @param watchKeys list of configuration watch keys that can trigger a refresh event
-     * @param monitoring refresh configurations
+     * @param duration refresh duration.
      */
     static void setState(String endpoint, List<ConfigurationSetting> watchKeys,
         Duration duration) {
@@ -49,7 +49,7 @@ static void setState(String endpoint, List<ConfigurationSetting> watchKeys,
     /**
      * @param endpoint the stores endpoint
      * @param watchKeys list of configuration watch keys that can trigger a refresh event
-     * @param monitoring refresh configurations
+     * @param duration refresh duration.
      */
     static void setStateFeatureFlag(String endpoint, List<ConfigurationSetting> watchKeys,
         Duration duration) {
@@ -65,13 +65,12 @@ static void setState(State state, Duration duration) {
     }
 
     static void expireState(String endpoint) {
-        String key = endpoint;
-        State oldState = STATE.get(key);
+        State oldState = STATE.get(endpoint);
         SecureRandom random = new SecureRandom();
         long wait = (long) (random.nextDouble() * MAX_JITTER);
         long timeLeft = (int) ((oldState.getNextRefreshCheck().getTime() - (new Date().getTime())) / 1000);
         if (wait < timeLeft) {
-            STATE.put(key, new State(oldState.getWatchKeys(), (int) wait, oldState.getKey()));
+            STATE.put(endpoint, new State(oldState.getWatchKeys(), (int) wait, oldState.getKey()));
         }
     }
 
@@ -90,14 +89,14 @@ static boolean getLoadStateFeatureFlag(String name) {
     }
 
     /**
-     * @param LOAD_STATE the loadState to set
+     * @param name the loadState name to set
      */
     static void setLoadState(String name, Boolean loaded) {
         LOAD_STATE.put(name, loaded);
     }
 
     /**
-     * @param LOAD_STATE the loadState to set
+     * @param name the loadState feature flag name to set
      */
     static void setLoadStateFeatureFlag(String name, Boolean loaded) {
         setLoadState(name + FEATURE_ENDPOINT, loaded);

diff --git a/...config/src/main/java/com/azure/spring/cloud/config/feature/management/entity/Feature.java b/...config/src/main/java/com/azure/spring/cloud/config/feature/management/entity/Feature.java
@@ -40,7 +40,7 @@ public Feature(String key, FeatureFlagConfigurationSetting featureItem) {
         this.key = key;
         List<FeatureFlagFilter> filterMapper = featureItem.getClientFilters();
 
-        enabledFor = new HashMap<Integer, FeatureFlagFilter>();
+        enabledFor = new HashMap<>();
 
         for (int i = 0; i < filterMapper.size(); i++) {
             enabledFor.put(i, filterMapper.get(i));

diff --git a/...fig/src/main/java/com/azure/spring/cloud/config/feature/management/entity/FeatureSet.java b/...fig/src/main/java/com/azure/spring/cloud/config/feature/management/entity/FeatureSet.java
@@ -33,7 +33,7 @@ public HashMap<String, Object> getFeatureManagement() {
      */
     public void addFeature(String key, Object feature) {
         if (featureManagement == null) {
-            featureManagement = new HashMap<String, Object>();
+            featureManagement = new HashMap<>();
         }
         if (feature != null) {
             featureManagement.put(key, feature);

diff --git a/...g/src/main/java/com/azure/spring/cloud/config/health/AppConfigurationHealthIndicator.java b/...g/src/main/java/com/azure/spring/cloud/config/health/AppConfigurationHealthIndicator.java
@@ -25,7 +25,7 @@ public AppConfigurationHealthIndicator(AppConfigurationRefresh refresh) {
     @Override
     public Health health() {
         Health.Builder healthBuilder = new Health.Builder();
-        Boolean healthy = true;
+        boolean healthy = true;
 
         for (String store : refresh.getAppConfigurationStoresHealth().keySet()) {
             if (AppConfigurationStoreHealth.DOWN.equals(refresh.getAppConfigurationStoresHealth().get(store))) {

diff --git a/.../main/java/com/azure/spring/cloud/config/pipline/policies/BaseAppConfigurationPolicy.java b/.../main/java/com/azure/spring/cloud/config/pipline/policies/BaseAppConfigurationPolicy.java
@@ -59,13 +59,13 @@ public BaseAppConfigurationPolicy(Boolean isDev, Boolean isKeyVaultConfigured) {
      */
     private String getTracingInfo(HttpRequest request) {
         String track = System.getenv(RequestTracingConstants.REQUEST_TRACING_DISABLED_ENVIRONMENT_VARIABLE.toString());
-        if (track != null && "false".equalsIgnoreCase(track)) {
+        if ("false".equalsIgnoreCase(track)) {
             return "";
         }
 
         RequestType requestTypeValue = watchRequests ? RequestType.WATCH : RequestType.STARTUP;
 
-        String tracingInfo = RequestTracingConstants.REQUEST_TYPE_KEY.toString() + "=" + requestTypeValue;
+        String tracingInfo = RequestTracingConstants.REQUEST_TYPE_KEY + "=" + requestTypeValue;
         String hostType = getHostType();
 
         if (!hostType.isEmpty()) {

diff --git a/.../src/main/java/com/azure/spring/cloud/config/properties/AppConfigurationStoreSelects.java b/.../src/main/java/com/azure/spring/cloud/config/properties/AppConfigurationStoreSelects.java
@@ -65,7 +65,7 @@ public String[] getLabelFilter(List<String> profiles) {
 
         // The use of trim makes label= dev,prod and label= dev, prod equal.
         List<String> labels = Arrays.stream(labelFilter.split(LABEL_SEPARATOR))
-            .map(label -> mapLabel(label))
+            .map(this::mapLabel)
             .distinct()
             .collect(Collectors.toList());
 

diff --git a/...configuration-config/src/main/java/com/azure/spring/cloud/config/resource/Connection.java b/...configuration-config/src/main/java/com/azure/spring/cloud/config/resource/Connection.java
@@ -43,7 +43,7 @@ public final class Connection {
      * @throws IllegalStateException Connection String format is invalid.
      */
     public Connection(String connectionString) {
-        Assert.hasText(connectionString, String.format("Connection string cannot be empty."));
+        Assert.hasText(connectionString, "Connection string cannot be empty.");
 
         Matcher matcher = CONN_STRING_PATTERN.matcher(connectionString);
         if (!matcher.find()) {

diff --git a/...pconfiguration-config/src/main/java/com/azure/spring/cloud/config/stores/ClientStore.java b/...pconfiguration-config/src/main/java/com/azure/spring/cloud/config/stores/ClientStore.java
@@ -64,7 +64,7 @@ public ClientStore(AppConfigurationProviderProperties appProperties, ConnectionP
         this.pool = pool;
         this.tokenCredentialProvider = tokenCredentialProvider;
         this.clientProvider = clientProvider;
-        this.clients = new HashMap<String, ConfigurationClient>();
+        this.clients = new HashMap<>();
         this.isDev = isDev;
         this.isKeyVaultConfigurated = isKeyVaultConfigured;
     }

diff --git a/...rc/test/java/com/azure/spring/cloud/config/AppConfigurationPropertySourceLocatorTest.java b/...rc/test/java/com/azure/spring/cloud/config/AppConfigurationPropertySourceLocatorTest.java
@@ -25,6 +25,7 @@
 
 import java.io.IOException;
 import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.net.MalformedURLException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -210,6 +211,9 @@ public Object getProperty(String name) {
     public void cleanup() throws Exception {
         MockitoAnnotations.openMocks(this).close();
         Field field = AppConfigurationPropertySourceLocator.class.getDeclaredField("startup");
+        Field modifiersField = Field.class.getDeclaredField("modifiers");
+        modifiersField.setAccessible(true);
+        modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL);
         field.setAccessible(true);
         field.set(null, new AtomicBoolean(true));
         StateHolder.setLoadState(TEST_STORE_NAME, false);
@@ -355,6 +359,9 @@ public void defaultFailFastThrowException() throws IOException {
     public void refreshThrowException() throws IOException, NoSuchFieldException, SecurityException,
         IllegalArgumentException, IllegalAccessException {
         Field field = AppConfigurationPropertySourceLocator.class.getDeclaredField("startup");
+        Field modifiersField = Field.class.getDeclaredField("modifiers");
+        modifiersField.setAccessible(true);
+        modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL);
         field.setAccessible(true);
         field.set(null, new AtomicBoolean(false));
         StateHolder.setLoadState(TEST_STORE_NAME, true);

diff --git a/...ment-web/src/main/java/com/azure/spring/cloud/feature/manager/FeatureManagerSnapshot.java b/...ment-web/src/main/java/com/azure/spring/cloud/feature/manager/FeatureManagerSnapshot.java
@@ -26,7 +26,7 @@ public class FeatureManagerSnapshot {
      */
     public FeatureManagerSnapshot(FeatureManager featureManager) {
         this.featureManager = featureManager;
-        this.requestMap = new HashMap<String, Boolean>();
+        this.requestMap = new HashMap<>();
     }
 
     /**