Skip to content

Commit

Permalink
Fixes bug where the number of labels was at max 5. (#653)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mrm9084
mrm9084 authored Mar 27, 2020
1 parent 6124db4 commit ae32e9a
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 51 deletions.
39 changes: 2 additions & 37 deletions ...src/main/java/com/microsoft/azure/spring/cloud/config/AppConfigurationPropertySource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,9 @@
*/
package com.microsoft.azure.spring.cloud.config;

import static com.microsoft.azure.spring.cloud.config.Constants.CONFIGURATION_SUFFIX;
import static com.microsoft.azure.spring.cloud.config.Constants.FEATURE_FLAG_CONTENT_TYPE;
import static com.microsoft.azure.spring.cloud.config.Constants.FEATURE_FLAG_PREFIX;
import static com.microsoft.azure.spring.cloud.config.Constants.FEATURE_MANAGEMENT_KEY;
import static com.microsoft.azure.spring.cloud.config.Constants.FEATURE_STORE_WATCH_KEY;
import static com.microsoft.azure.spring.cloud.config.Constants.FEATURE_SUFFIX;
import static com.microsoft.azure.spring.cloud.config.Constants.KEY_VAULT_CONTENT_TYPE;

import java.io.IOException;
Expand All @@ -27,7 +24,6 @@
import org.slf4j.LoggerFactory;
import org.springframework.core.env.EnumerablePropertySource;
import org.springframework.util.ReflectionUtils;
import org.springframework.util.StringUtils;

import com.azure.data.appconfiguration.ConfigurationClient;
import com.azure.data.appconfiguration.models.ConfigurationSetting;
Expand Down Expand Up @@ -66,12 +62,9 @@ public class AppConfigurationPropertySource extends EnumerablePropertySource<Con

private ConfigStore configStore;

private Map<String, List<String>> storeContextsMap;

AppConfigurationPropertySource(String context, ConfigStore configStore, String label,
AppConfigurationProperties appConfigurationProperties, ClientStore clients,
AppConfigurationProviderProperties appProperties, KeyVaultCredentialProvider keyVaultCredentialProvider,
Map<String, List<String>> storeContextsMap) {
AppConfigurationProviderProperties appProperties, KeyVaultCredentialProvider keyVaultCredentialProvider) {
// The context alone does not uniquely define a PropertySource, append storeName
// and label to uniquely define a PropertySource
super(context + configStore.getEndpoint() + "/" + label);
Expand All @@ -83,7 +76,6 @@ public class AppConfigurationPropertySource extends EnumerablePropertySource<Con
this.keyVaultClients = new HashMap<String, KeyVaultClient>();
this.clients = clients;
this.keyVaultCredentialProvider = keyVaultCredentialProvider;
this.storeContextsMap = storeContextsMap;
}

@Override
Expand Down Expand Up @@ -146,34 +138,7 @@ FeatureSet initProperties(FeatureSet featureSet) throws IOException {
}
}

featureSet = addToFeatureSet(featureSet, features, date);

// Setting new ETag values for Watch
String watchedKeyNames = clients.watchedKeyNames(configStore, storeContextsMap);
settingSelector = new SettingSelector().setKeyFilter(watchedKeyNames)
.setLabelFilter(StringUtils.arrayToCommaDelimitedString(configStore.getLabels()));

List<ConfigurationSetting> configurationRevisions = clients.listSettingRevisons(settingSelector, storeName);

settingSelector = new SettingSelector().setKeyFilter(FEATURE_STORE_WATCH_KEY)
.setLabelFilter(StringUtils.arrayToCommaDelimitedString(configStore.getLabels()));

List<ConfigurationSetting> featureRevisions = clients.listSettingRevisons(settingSelector, storeName);

if (configurationRevisions != null && !configurationRevisions.isEmpty()) {
StateHolder.setEtagState(configStore.getEndpoint() + CONFIGURATION_SUFFIX, configurationRevisions.get(0));
} else {
StateHolder.setEtagState(configStore.getEndpoint() + CONFIGURATION_SUFFIX, new ConfigurationSetting());
}

if (featureRevisions != null && !featureRevisions.isEmpty()) {
StateHolder.setEtagState(configStore.getEndpoint() + FEATURE_SUFFIX, featureRevisions.get(0));
} else {
StateHolder.setEtagState(configStore.getEndpoint() + FEATURE_SUFFIX, new ConfigurationSetting());
}
StateHolder.setLoadState(configStore.getEndpoint(), true);

return featureSet;
return addToFeatureSet(featureSet, features, date);
}

/**
Expand Down
36 changes: 34 additions & 2 deletions ...n/java/com/microsoft/azure/spring/cloud/config/AppConfigurationPropertySourceLocator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
*/
package com.microsoft.azure.spring.cloud.config;

import static com.microsoft.azure.spring.cloud.config.Constants.CONFIGURATION_SUFFIX;
import static com.microsoft.azure.spring.cloud.config.Constants.FEATURE_STORE_WATCH_KEY;
import static com.microsoft.azure.spring.cloud.config.Constants.FEATURE_SUFFIX;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -26,6 +30,8 @@
import org.springframework.util.ReflectionUtils;
import org.springframework.util.StringUtils;

import com.azure.data.appconfiguration.models.ConfigurationSetting;
import com.azure.data.appconfiguration.models.SettingSelector;
import com.microsoft.azure.spring.cloud.config.feature.management.entity.FeatureSet;
import com.microsoft.azure.spring.cloud.config.stores.ClientStore;
import com.microsoft.azure.spring.cloud.config.stores.ConfigStore;
Expand Down Expand Up @@ -96,7 +102,7 @@ public PropertySource<?> locate(Environment environment) {
}
}

startup = false;
startup = false;

return composite;
}
Expand Down Expand Up @@ -201,14 +207,40 @@ private List<AppConfigurationPropertySource> create(String context, ConfigStore
for (String label : store.getLabels()) {
putStoreContext(store.getEndpoint(), context, storeContextsMap);
AppConfigurationPropertySource propertySource = new AppConfigurationPropertySource(context, store,
label, properties, clients, appProperties, keyVaultCredentialProvider, storeContextsMap);
label, properties, clients, appProperties, keyVaultCredentialProvider);

propertySource.initProperties(featureSet);
if (initFeatures) {
propertySource.initFeatures(featureSet);
}
sourceList.add(propertySource);
}

// Setting new ETag values for Watch
String watchedKeyNames = clients.watchedKeyNames(store, storeContextsMap);
SettingSelector settingSelector = new SettingSelector().setKeyFilter(watchedKeyNames).setLabelFilter("*");

List<ConfigurationSetting> configurationRevisions = clients.listSettingRevisons(settingSelector,
store.getEndpoint());

settingSelector = new SettingSelector().setKeyFilter(FEATURE_STORE_WATCH_KEY).setLabelFilter("*");

List<ConfigurationSetting> featureRevisions = clients.listSettingRevisons(settingSelector,
store.getEndpoint());

if (configurationRevisions != null && !configurationRevisions.isEmpty()) {
StateHolder.setEtagState(store.getEndpoint() + CONFIGURATION_SUFFIX,
configurationRevisions.get(0));
} else {
StateHolder.setEtagState(store.getEndpoint() + CONFIGURATION_SUFFIX, new ConfigurationSetting());
}

if (featureRevisions != null && !featureRevisions.isEmpty()) {
StateHolder.setEtagState(store.getEndpoint() + FEATURE_SUFFIX, featureRevisions.get(0));
} else {
StateHolder.setEtagState(store.getEndpoint() + FEATURE_SUFFIX, new ConfigurationSetting());
}
StateHolder.setLoadState(store.getEndpoint(), true);
} catch (Exception e) {
delayException();
throw e;
Expand Down
3 changes: 1 addition & 2 deletions ...config/src/main/java/com/microsoft/azure/spring/cloud/config/AppConfigurationRefresh.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.springframework.cloud.endpoint.event.RefreshEvent;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.ApplicationEventPublisherAware;
import org.springframework.util.StringUtils;

import com.azure.data.appconfiguration.models.ConfigurationSetting;
import com.azure.data.appconfiguration.models.SettingSelector;
Expand Down Expand Up @@ -133,7 +132,7 @@ private boolean refreshStores() {
private boolean refresh(ConfigStore store, String storeSuffix, String watchedKeyNames) {
String storeNameWithSuffix = store.getEndpoint() + storeSuffix;
SettingSelector settingSelector = new SettingSelector().setKeyFilter(watchedKeyNames)
.setLabelFilter(StringUtils.arrayToCommaDelimitedString(store.getLabels()));
.setLabelFilter("*");

List<ConfigurationSetting> items = clientStore.listSettingRevisons(settingSelector, store.getEndpoint());

Expand Down
6 changes: 1 addition & 5 deletions ...a/com/microsoft/azure/spring/cloud/config/AppConfigurationPropertySourceKeyVaultTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@
import java.io.IOException;
import java.net.URI;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

import org.junit.Before;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -132,12 +130,10 @@ public void setup() {
appProperties.setMaxRetryTime(0);
ConfigStore testStore = new ConfigStore();
testStore.setEndpoint(TEST_STORE_NAME);
Map<String, List<String>> storeContextsMap = new HashMap<String, List<String>>();
ArrayList<String> contexts = new ArrayList<String>();
contexts.add("/application/*");
storeContextsMap.put(TEST_STORE_NAME, contexts);
propertySource = new AppConfigurationPropertySource(TEST_CONTEXT, testStore, "\0",
appConfigurationProperties, clientStoreMock, appProperties, tokenCredentialProvider, storeContextsMap);
appConfigurationProperties, clientStoreMock, appProperties, tokenCredentialProvider);

testItems = new ArrayList<ConfigurationSetting>();
testItems.add(item1);
Expand Down
53 changes: 53 additions & 0 deletions ...va/com/microsoft/azure/spring/cloud/config/AppConfigurationPropertySourceLocatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,19 @@
import static com.microsoft.azure.spring.cloud.config.TestConstants.FEATURE_VALUE;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_CONN_STRING;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_CONN_STRING_2;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_CONTEXT;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_KEY_1;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_KEY_2;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_KEY_3;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_LABEL_1;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_LABEL_2;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_LABEL_3;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_STORE_NAME;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_STORE_NAME_1;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_STORE_NAME_2;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_VALUE_1;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_VALUE_2;
import static com.microsoft.azure.spring.cloud.config.TestConstants.TEST_VALUE_3;
import static com.microsoft.azure.spring.cloud.config.TestUtils.createItem;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -115,6 +125,19 @@ public class AppConfigurationPropertySourceLocatorTest {
private AppConfigurationProviderProperties appProperties;

private KeyVaultCredentialProvider tokenCredentialProvider = null;

private static final String EMPTY_CONTENT_TYPE = "";

public List<ConfigurationSetting> testItems = new ArrayList<>();

private static final ConfigurationSetting item1 = createItem(TEST_CONTEXT, TEST_KEY_1, TEST_VALUE_1, TEST_LABEL_1,
EMPTY_CONTENT_TYPE);

private static final ConfigurationSetting item2 = createItem(TEST_CONTEXT, TEST_KEY_2, TEST_VALUE_2, TEST_LABEL_2,
EMPTY_CONTENT_TYPE);

private static final ConfigurationSetting item3 = createItem(TEST_CONTEXT, TEST_KEY_3, TEST_VALUE_3, TEST_LABEL_3,
EMPTY_CONTENT_TYPE);

@BeforeClass
public static void init() {
Expand Down Expand Up @@ -149,6 +172,11 @@ public void setup() {
appProperties.setVersion("1.0");
appProperties.setMaxRetries(12);
appProperties.setMaxRetryTime(0);

testItems = new ArrayList<ConfigurationSetting>();
testItems.add(item1);
testItems.add(item2);
testItems.add(item3);
}

@After
Expand Down Expand Up @@ -182,6 +210,31 @@ public void compositeSourceIsCreated() {
assertThat(sources.size()).isEqualTo(6);
assertThat(sources.stream().map(s -> s.getName()).toArray()).containsExactly(expectedSourceNames);
}

@Test
public void revisionsCheck() {
String[] labels = new String[1];
labels[0] = "\0";
when(configStoreMock.getLabels()).thenReturn(labels);
when(properties.getDefaultContext()).thenReturn("application");
when(clientStoreMock.listSettingRevisons(Mockito.any(), Mockito.anyString())).thenReturn(testItems)
.thenReturn(FEATURE_ITEMS);

locator = new AppConfigurationPropertySourceLocator(properties, appProperties, clientStoreMock,
tokenCredentialProvider);
PropertySource<?> source = locator.locate(environment);
assertThat(source).isInstanceOf(CompositePropertySource.class);

Collection<PropertySource<?>> sources = ((CompositePropertySource) source).getPropertySources();
// Application name: foo and active profile: dev,prod, should construct below
// composite Property Source:
// [/foo_prod/, /foo_dev/, /foo/, /application_prod/, /application_dev/,
// /application/]
String[] expectedSourceNames = new String[] { "/foo_prod/store1/\0", "/foo_dev/store1/\0", "/foo/store1/\0",
"/application_prod/store1/\0", "/application_dev/store1/\0", "/application/store1/\0" };
assertThat(sources.size()).isEqualTo(6);
assertThat(sources.stream().map(s -> s.getName()).toArray()).containsExactly(expectedSourceNames);
}

@Test
public void compositeSourceIsCreatedForPrefixedConfig() {
Expand Down
7 changes: 2 additions & 5 deletions ...test/java/com/microsoft/azure/spring/cloud/config/AppConfigurationPropertySourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,10 @@ public void setup() {
appProperties = new AppConfigurationProviderProperties();
ConfigStore configStore = new ConfigStore();
configStore.setEndpoint(TEST_STORE_NAME);
Map<String, List<String>> storeContextsMap = new HashMap<String, List<String>>();
ArrayList<String> contexts = new ArrayList<String>();
contexts.add("/application/*");
storeContextsMap.put(TEST_STORE_NAME, contexts);
propertySource = new AppConfigurationPropertySource(TEST_CONTEXT, configStore, "\0",
appConfigurationProperties, clientStoreMock, appProperties, tokenCredentialProvider, storeContextsMap);
appConfigurationProperties, clientStoreMock, appProperties, tokenCredentialProvider);

testItems = new ArrayList<ConfigurationSetting>();
testItems.add(item1);
Expand All @@ -181,8 +179,7 @@ public void setup() {
public void testPropCanBeInitAndQueried() throws IOException {
when(clientStoreMock.listSettings(Mockito.any(), Mockito.anyString())).thenReturn(testItems)
.thenReturn(FEATURE_ITEMS);
when(clientStoreMock.listSettingRevisons(Mockito.any(), Mockito.anyString())).thenReturn(testItems)
.thenReturn(FEATURE_ITEMS);

FeatureSet featureSet = new FeatureSet();
try {
propertySource.initProperties(featureSet);
Expand Down

0 comments on commit ae32e9a

Please sign in to comment.