App Configuration Bug Fixes (#723)

* Fixes Refresh not being disabled when provider is, fixed Muliple profile issue, fixed muliple label issue, fixed feature management name issue.

* Fixed issue where switching from on/off to conditional didn't work after a refresh

* Updated so profiles had seprate etag stored

spring-cloud-azure-appconfiguration-config-web/src/main/java/com/microsoft/azure/spring/cloud/config/web/AppConfigurationWebAutoConfiguration.java

@@ -5,6 +5,7 @@
  */
 package com.microsoft.azure.spring.cloud.config.web;
 
+import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.cloud.endpoint.RefreshEndpoint;
 import org.springframework.context.annotation.Bean;
@@ -13,6 +14,7 @@
 import com.microsoft.azure.spring.cloud.config.AppConfigurationRefresh;
 
 @Configuration
+@ConditionalOnBean(AppConfigurationRefresh.class)
 public class AppConfigurationWebAutoConfiguration {
 
     @Configuration

spring-cloud-azure-appconfiguration-config/src/main/java/com/microsoft/azure/spring/cloud/config/AppConfigurationPropertySourceLocator.java

@@ -58,7 +58,7 @@ public class AppConfigurationPropertySourceLocator implements PropertySourceLoca
     private ClientStore clients;
 
     private KeyVaultCredentialProvider keyVaultCredentialProvider;
-    
+
     private SecretClientBuilderSetup keyVaultClientProvider;
 
     private static Boolean startup = true;
@@ -214,8 +214,8 @@ private List<AppConfigurationPropertySource> create(String context, ConfigStore
         List<AppConfigurationPropertySource> sourceList = new ArrayList<>();
 
         try {
+            putStoreContext(store.getEndpoint(), context, storeContextsMap);
             for (String label : store.getLabels()) {
-                putStoreContext(store.getEndpoint(), context, storeContextsMap);
                 AppConfigurationPropertySource propertySource = new AppConfigurationPropertySource(context, store,
                         label, properties, clients, appProperties, keyVaultCredentialProvider, keyVaultClientProvider);
 
@@ -227,7 +227,7 @@ private List<AppConfigurationPropertySource> create(String context, ConfigStore
             }
 
             // Setting new ETag values for Watch
-            String watchedKeyNames = clients.watchedKeyNames(store, storeContextsMap);
+            String watchedKeyNames = clients.watchedKeyNames(store, context);
             SettingSelector settingSelector = new SettingSelector().setKeyFilter(watchedKeyNames).setLabelFilter("*");
 
             ConfigurationSetting configurationRevision = clients.getRevison(settingSelector,
@@ -238,10 +238,13 @@ private List<AppConfigurationPropertySource> create(String context, ConfigStore
             ConfigurationSetting featureRevision = clients.getRevison(settingSelector,
                     store.getEndpoint());
 
+            String prefix = "_" + context;
+
             if (configurationRevision != null) {
-                StateHolder.setEtagState(store.getEndpoint() + CONFIGURATION_SUFFIX, configurationRevision);
+                StateHolder.setEtagState(store.getEndpoint() + CONFIGURATION_SUFFIX + prefix, configurationRevision);
             } else {
-                StateHolder.setEtagState(store.getEndpoint() + CONFIGURATION_SUFFIX, new ConfigurationSetting());
+                StateHolder.setEtagState(store.getEndpoint() + CONFIGURATION_SUFFIX + prefix,
+                        new ConfigurationSetting());
             }
 
             if (featureRevision != null) {

spring-cloud-azure-appconfiguration-config/src/main/java/com/microsoft/azure/spring/cloud/config/AppConfigurationRefresh.java

@@ -10,6 +10,7 @@
 import static com.microsoft.azure.spring.cloud.config.Constants.FEATURE_SUFFIX;
 
 import java.time.Duration;
+import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
@@ -51,6 +52,8 @@ public class AppConfigurationRefresh implements ApplicationEventPublisherAware {
 
     private String eventDataInfo;
 
+    private List<String> featureWatchKey = new ArrayList<String>();
+
     public AppConfigurationRefresh(AppConfigurationProperties properties, Map<String, List<String>> storeContextsMap,
             ClientStore clientStore) {
         this.configStores = properties.getStores();
@@ -59,6 +62,7 @@ public AppConfigurationRefresh(AppConfigurationProperties properties, Map<String
         this.lastCheckedTime = new Date();
         this.clientStore = clientStore;
         this.eventDataInfo = "";
+        featureWatchKey.add(FEATURE_STORE_WATCH_KEY);
     }
 
     @Override
@@ -100,11 +104,10 @@ private boolean refreshStores() {
                 if (notCachedTime == null || date.after(notCachedTime)) {
                     for (ConfigStore configStore : configStores) {
                         if (StateHolder.getLoadState(configStore.getEndpoint())) {
-                            String watchedKeyNames = clientStore.watchedKeyNames(configStore, storeContextsMap);
-                            willRefresh = refresh(configStore, CONFIGURATION_SUFFIX, watchedKeyNames) ? true
+                            willRefresh = refresh_configurations(configStore) ? true
                                     : willRefresh;
                             // Refresh Feature Flags
-                            willRefresh = refresh(configStore, FEATURE_SUFFIX, FEATURE_STORE_WATCH_KEY) ? true
+                            willRefresh = refreshFeatureFlag(configStore) ? true
                                     : willRefresh;
                         } else {
                             LOGGER.debug("Skipping refresh check for " + configStore.getEndpoint()
@@ -137,15 +140,36 @@ private boolean refreshStores() {
      * published.
      * 
      * @param store the {@code store} for which to composite watched key names
-     * @param storeSuffix Suffix used to distinguish between Settings and Features
-     * @param watchedKeyNames Key used to check if refresh should occur
      * @return Refresh event was triggered. No other sources need to be checked.
      */
-    private boolean refresh(ConfigStore store, String storeSuffix, String watchedKeyNames) {
-        String storeNameWithSuffix = store.getEndpoint() + storeSuffix;
-        SettingSelector settingSelector = new SettingSelector().setKeyFilter(watchedKeyNames)
-                .setLabelFilter("*");
+    private boolean refresh_configurations(ConfigStore store) {
+        for (String context : storeContextsMap.get(store.getEndpoint())) {
+            // Checking every Profile
+            String storeNameWithSuffix = store.getEndpoint() + CONFIGURATION_SUFFIX + "_" + context;
+            String watchedKeyName = clientStore.watchedKeyNames(store, context);
+
+            if (checkETagChange(store, storeNameWithSuffix, watchedKeyName)) {
+                return true;
+            }
+        }
+        return false;
+    }
 
+    /**
+     * Checks un-cached items for etag changes for feature flags. If they have changed a
+     * RefreshEventData is published.
+     * 
+     * @param store the {@code store} for which to composite watched key names
+     * @return Refresh event was triggered. No other sources need to be checked.
+     */
+    private boolean refreshFeatureFlag(ConfigStore store) {
+        String storeNameWithSuffix = store.getEndpoint() + FEATURE_SUFFIX;
+        return checkETagChange(store, storeNameWithSuffix, FEATURE_STORE_WATCH_KEY);
+    }
+
+    private boolean checkETagChange(ConfigStore store, String storeNameWithSuffix, String watchKey) {
+        SettingSelector settingSelector = new SettingSelector().setKeyFilter(watchKey)
+                .setLabelFilter("*");
         ConfigurationSetting revision = clientStore.getRevison(settingSelector, store.getEndpoint());
 
         String etag = null;
@@ -166,11 +190,11 @@ private boolean refresh(ConfigStore store, String storeSuffix, String watchedKey
 
         if (etag != null && !etag.equals(StateHolder.getEtagState(storeNameWithSuffix).getETag())) {
             LOGGER.trace("Some keys in store [{}] matching [{}] is updated, will send refresh event.",
-                    store.getEndpoint(), watchedKeyNames);
+                    store.getEndpoint(), watchKey);
             if (this.eventDataInfo.isEmpty()) {
-                this.eventDataInfo = watchedKeyNames;
+                this.eventDataInfo = watchKey;
             } else {
-                this.eventDataInfo += ", " + watchedKeyNames;
+                this.eventDataInfo += ", " + watchKey;
             }
 
             // Don't need to refresh here will be done in Property Source

spring-cloud-azure-appconfiguration-config/src/main/java/com/microsoft/azure/spring/cloud/config/stores/ClientStore.java

@@ -7,6 +7,7 @@
 
 import java.io.IOException;
 import java.time.Duration;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
@@ -25,10 +26,9 @@
 import com.azure.data.appconfiguration.models.ConfigurationSetting;
 import com.azure.data.appconfiguration.models.SettingSelector;
 import com.azure.identity.ManagedIdentityCredentialBuilder;
-import com.microsoft.azure.spring.cloud.config.ConfigurationClientBuilderSetup;
 import com.microsoft.azure.spring.cloud.config.AppConfigurationCredentialProvider;
 import com.microsoft.azure.spring.cloud.config.AppConfigurationProviderProperties;
-import com.microsoft.azure.spring.cloud.config.AppConfigurationRefresh;
+import com.microsoft.azure.spring.cloud.config.ConfigurationClientBuilderSetup;
 import com.microsoft.azure.spring.cloud.config.pipline.policies.BaseAppConfigurationPolicy;
 import com.microsoft.azure.spring.cloud.config.resource.Connection;
 import com.microsoft.azure.spring.cloud.config.resource.ConnectionPool;
@@ -157,12 +157,28 @@ public final List<ConfigurationSetting> listSettings(SettingSelector settingSele
      * @param storeContextsMap map storing store name and List of context key-value pair
      * @return the full name of the key mapping to the configuration store
      */
-    public String watchedKeyNames(ConfigStore store, Map<String, List<String>> storeContextsMap) {
+    public List<String> watchedKeyNames(ConfigStore store, Map<String, List<String>> storeContextsMap) {
+        List<String> watchedKeys = new ArrayList<String>();
         String watchedKey = store.getWatchedKey().trim();
         List<String> contexts = storeContextsMap.get(store.getEndpoint());
 
-        String watchedKeys = contexts.stream().map(ctx -> genKey(ctx, watchedKey))
-                .collect(Collectors.joining(","));
+        for (String context : contexts) {
+            String key = genKey(context, watchedKey);
+            if (key.contains(",") && key.contains("*")) {
+                // Multi keys including one or more key patterns is not supported by API,
+                // will
+                // watch all keys(*) instead
+                key = "*";
+            }
+            watchedKeys.add(key);
+        }
+
+        return watchedKeys;
+    }
+
+    public String watchedKeyNames(ConfigStore store, String context) {
+        String watchedKey = store.getWatchedKey().trim();
+        String watchedKeys = genKey(context, watchedKey);
 
         if (watchedKeys.contains(",") && watchedKeys.contains("*")) {
             // Multi keys including one or more key patterns is not supported by API, will

spring-cloud-azure-appconfiguration-config/src/test/java/com/microsoft/azure/spring/cloud/config/AppConfigurationRefreshTest.java

@@ -60,7 +60,9 @@ public class AppConfigurationRefreshTest {
     @Mock
     private ClientStore clientStoreMock;
 
-    private static final String WATCHED_KEYS = "/application/*";
+    private static final String WATCHED_KEY = "/application/*";
+
+    private List<String> watchedKeys = new ArrayList<String>();
 
     @Before
     public void setup() {
@@ -69,7 +71,10 @@ public void setup() {
         ConfigStore store = new ConfigStore();
         store.setEndpoint(TEST_STORE_NAME);
         store.setConnectionString(TEST_CONN_STRING);
-        store.setWatchedKey(WATCHED_KEYS);
+        store.setWatchedKey(WATCHED_KEY);
+
+        watchedKeys = new ArrayList<String>();
+        watchedKeys.add(WATCHED_KEY);
 
         properties = new AppConfigurationProperties();
         properties.setStores(Arrays.asList(store));
@@ -88,21 +93,21 @@ public void setup() {
         item.setKey("fake-etag/application/test.key");
         item.setETag("fake-etag");
 
-        when(clientStoreMock.watchedKeyNames(Mockito.any(), Mockito.any())).thenReturn(WATCHED_KEYS);
+        when(clientStoreMock.watchedKeyNames(Mockito.any(), Mockito.anyString())).thenReturn(WATCHED_KEY);
 
         configRefresh = new AppConfigurationRefresh(properties, contextsMap, clientStoreMock);
         StateHolder.setLoadState(TEST_STORE_NAME, true);
     }
 
     @After
     public void cleanupMethod() {
-        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX, new ConfigurationSetting());
+        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX + "_" + TEST_ETAG, new ConfigurationSetting());
         StateHolder.setEtagState(TEST_STORE_NAME + FEATURE_SUFFIX, new ConfigurationSetting());
     }
 
     @Test
     public void nonUpdatedEtagShouldntPublishEvent() throws Exception {
-        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX, initialResponse());
+        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX + "_" + TEST_ETAG, initialResponse());
         StateHolder.setEtagState(TEST_STORE_NAME + FEATURE_SUFFIX, initialResponse());
 
         configRefresh.setApplicationEventPublisher(eventPublisher);
@@ -115,7 +120,7 @@ public void nonUpdatedEtagShouldntPublishEvent() throws Exception {
 
     @Test
     public void updatedEtagShouldPublishEvent() throws Exception {
-        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX, initialResponse());
+        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX + "_" + TEST_ETAG, initialResponse());
         StateHolder.setEtagState(TEST_STORE_NAME + FEATURE_SUFFIX, initialResponse());
 
         when(clientStoreMock.getRevison(Mockito.any(), Mockito.anyString())).thenReturn(initialResponse());
@@ -131,7 +136,7 @@ public void updatedEtagShouldPublishEvent() throws Exception {
         assertTrue(configRefresh.refreshConfigurations().get());
         verify(eventPublisher, times(1)).publishEvent(any(RefreshEvent.class));
 
-        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX, updatedResponse());
+        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX + "_" + TEST_ETAG, updatedResponse());
         StateHolder.setEtagState(TEST_STORE_NAME + FEATURE_SUFFIX, updatedResponse());
 
         HashMap<String, String> map = new HashMap<String, String>();
@@ -141,7 +146,7 @@ public void updatedEtagShouldPublishEvent() throws Exception {
         ConfigurationSetting updated = new ConfigurationSetting();
         updated.setETag("fake-etag-updated");
 
-        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX, updated);
+        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX + "_" + TEST_ETAG, updated);
 
         // If there is no change it shouldn't update
         assertFalse(configRefresh.refreshConfigurations().get());
@@ -150,7 +155,7 @@ public void updatedEtagShouldPublishEvent() throws Exception {
 
     @Test
     public void noEtagReturned() throws Exception {
-        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX, initialResponse());
+        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX + "_" + TEST_ETAG, initialResponse());
         StateHolder.setEtagState(TEST_STORE_NAME + FEATURE_SUFFIX, initialResponse());
 
         when(clientStoreMock.getRevison(Mockito.any(), Mockito.anyString()))
@@ -164,7 +169,7 @@ public void noEtagReturned() throws Exception {
 
     @Test
     public void nullItemsReturned() throws Exception {
-        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX, initialResponse());
+        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX + "_" + TEST_ETAG, initialResponse());
         StateHolder.setEtagState(TEST_STORE_NAME + FEATURE_SUFFIX, initialResponse());
 
         when(clientStoreMock.getRevison(Mockito.any(), Mockito.anyString())).thenReturn(null);
@@ -180,13 +185,17 @@ public void noInitialStateNoEtag() throws Exception {
         ConfigStore store = new ConfigStore();
         store.setEndpoint(TEST_STORE_NAME + "_LOST");
         store.setConnectionString(TEST_CONN_STRING);
-        store.setWatchedKey(WATCHED_KEYS);
-        
+        store.setWatchedKey(WATCHED_KEY);
+
         AppConfigurationProperties propertiesLost = new AppConfigurationProperties();
         propertiesLost.setStores(Arrays.asList(store));
 
         propertiesLost.setCacheExpiration(Duration.ofMinutes(-60));
-        AppConfigurationRefresh configRefreshLost = new AppConfigurationRefresh(propertiesLost, contextsMap,
+
+        Map<String, List<String>> contextsMapLost = new ConcurrentHashMap<>();
+        contextsMapLost.put(TEST_STORE_NAME + "_LOST", Arrays.asList(TEST_ETAG));
+
+        AppConfigurationRefresh configRefreshLost = new AppConfigurationRefresh(propertiesLost, contextsMapLost,
                 clientStoreMock);
         StateHolder.setLoadState(TEST_STORE_NAME + "_LOST", true);
         when(clientStoreMock.getRevison(Mockito.any(), Mockito.anyString())).thenReturn(null);
@@ -202,16 +211,20 @@ public void noInitialStateHasEtag() throws Exception {
         ConfigStore store = new ConfigStore();
         store.setEndpoint(TEST_STORE_NAME + "_LOST");
         store.setConnectionString(TEST_CONN_STRING);
-        store.setWatchedKey(WATCHED_KEYS);
-        
+        store.setWatchedKey(WATCHED_KEY);
+
         AppConfigurationProperties propertiesLost = new AppConfigurationProperties();
         propertiesLost.setStores(Arrays.asList(store));
 
         propertiesLost.setCacheExpiration(Duration.ofMinutes(-60));
-        AppConfigurationRefresh configRefreshLost = new AppConfigurationRefresh(propertiesLost, contextsMap,
+
+        Map<String, List<String>> contextsMapLost = new ConcurrentHashMap<>();
+        contextsMapLost.put(TEST_STORE_NAME + "_LOST", Arrays.asList(TEST_ETAG));
+
+        AppConfigurationRefresh configRefreshLost = new AppConfigurationRefresh(propertiesLost, contextsMapLost,
                 clientStoreMock);
         StateHolder.setLoadState(TEST_STORE_NAME + "_LOST", true);
-        
+
         when(clientStoreMock.getRevison(Mockito.any(), Mockito.anyString())).thenReturn(updatedResponse());
         configRefreshLost.setApplicationEventPublisher(eventPublisher);
 
@@ -222,7 +235,7 @@ public void noInitialStateHasEtag() throws Exception {
 
     @Test
     public void notRefreshTime() throws Exception {
-        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX, initialResponse());
+        StateHolder.setEtagState(TEST_STORE_NAME + CONFIGURATION_SUFFIX + "_" + TEST_ETAG, initialResponse());
         StateHolder.setEtagState(TEST_STORE_NAME + FEATURE_SUFFIX, initialResponse());
 
         properties.setCacheExpiration(Duration.ofMinutes(60));

spring-cloud-azure-appconfiguration-config/src/test/java/com/microsoft/azure/spring/cloud/config/stores/ClientStoreTest.java

@@ -191,7 +191,7 @@ public void watchedKeyNamesWildcardTest() {
 
         storeContextsMap.put(TEST_ENDPOINT, contexts);
 
-        assertEquals("/application/*", clientStore.watchedKeyNames(store, storeContextsMap));
+        assertEquals("/application/*", clientStore.watchedKeyNames(store, "/application/"));
     }
 
     private PagedFlux<ConfigurationSetting> getConfigurationPagedFlux(int noOfPages) throws MalformedURLException {

spring-cloud-azure-feature-management/src/main/java/com/microsoft/azure/spring/cloud/feature/manager/FeatureManager.java

@@ -145,6 +145,14 @@ public void putAll(Map<? extends String, ? extends Object> m) {
             return;
         }
 
+        // Need to reset or switch between on/off to conditional doesn't work
+        featureManagement = new HashMap<String, Feature>();
+        onOff = new HashMap<String, Boolean>();
+
+        if (m.size() == 1 && m.containsKey("featureManagement")) {
+            m = (Map<? extends String, ? extends Object>) m.get("featureManagement");
+        }
+
         for (String key : m.keySet()) {
             addToFeatures(m, key, "");
         }