From d30e0b627d995c011a97ad8dd662629b242ac6ed Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 13 Apr 2023 09:51:29 -0700 Subject: [PATCH 1/6] Adds Requirement Type All --- .../feature/management/FeatureManager.java | 26 ++++--- .../implementation/models/Feature.java | 16 +++++ .../management/FeatureManagerTest.java | 72 +++++++++++++++++++ 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManager.java b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManager.java index 72d6b751d616..1805b0367627 100644 --- a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManager.java +++ b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManager.java @@ -6,6 +6,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Stream; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,9 +51,9 @@ public class FeatureManager { } /** - * Checks to see if the feature is enabled. If enabled it check each filter, once a single filter - * returns true it returns true. If no filter returns true, it returns false. If there are no - * filters, it returns true. If feature isn't found it returns false. + * Checks to see if the feature is enabled. If enabled it check each filter, once a single filter returns true it + * returns true. If no filter returns true, it returns false. If there are no filters, it returns true. If feature + * isn't found it returns false. * * @param feature Feature being checked. * @return state of the feature @@ -63,9 +64,9 @@ public Mono isEnabledAsync(String feature) { } /** - * Checks to see if the feature is enabled. If enabled it check each filter, once a single filter - * returns true it returns true. If no filter returns true, it returns false. If there are no - * filters, it returns true. If feature isn't found it returns false. + * Checks to see if the feature is enabled. If enabled it check each filter, once a single filter returns true it + * returns true. If no filter returns true, it returns false. If there are no filters, it returns true. If feature + * isn't found it returns false. * * @param feature Feature being checked. * @return state of the feature @@ -93,9 +94,16 @@ private boolean checkFeature(String feature) throws FilterNotFoundException { return false; } - return featureItem.getEnabledFor().values().stream().filter(Objects::nonNull) - .filter(featureFilter -> featureFilter.getName() != null) - .anyMatch(featureFilter -> isFeatureOn(featureFilter, feature)); + Stream filters = featureItem.getEnabledFor().values().stream() + .filter(Objects::nonNull).filter(featureFilter -> featureFilter.getName() != null); + + // All Filters must be true + if (featureItem.getRequirementType().equals("All")) { + return filters.allMatch(featureFilter -> isFeatureOn(featureFilter, feature)); + } + + // Any Filter must be true + return filters.anyMatch(featureFilter -> isFeatureOn(featureFilter, feature)); } private boolean isFeatureOn(FeatureFilterEvaluationContext filter, String feature) { diff --git a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java index f105e57f7b9a..21c930dd59f1 100644 --- a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java +++ b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java @@ -19,6 +19,8 @@ public class Feature { @JsonProperty("evaluate") private Boolean evaluate = true; + private String requirementType = "Any"; + @JsonProperty("enabled-for") private HashMap enabledFor; @@ -64,4 +66,18 @@ public void setEnabledFor(HashMap enabl this.enabledFor = enabledFor; } + /** + * @return the requirementType + */ + public String getRequirementType() { + return requirementType; + } + + /** + * @param requirementType the requirementType to set + */ + public void setRequirementType(String requirementType) { + this.requirementType = requirementType; + } + } diff --git a/sdk/spring/spring-cloud-azure-feature-management/src/test/java/com/azure/spring/cloud/feature/management/FeatureManagerTest.java b/sdk/spring/spring-cloud-azure-feature-management/src/test/java/com/azure/spring/cloud/feature/management/FeatureManagerTest.java index 38c4fcbd89ef..9933722a018e 100644 --- a/sdk/spring/spring-cloud-azure-feature-management/src/test/java/com/azure/spring/cloud/feature/management/FeatureManagerTest.java +++ b/sdk/spring/spring-cloud-azure-feature-management/src/test/java/com/azure/spring/cloud/feature/management/FeatureManagerTest.java @@ -139,6 +139,69 @@ public void noFilter() throws FilterNotFoundException { assertThat(e).hasMessage("Fail fast is set and a Filter was unable to be found: AlwaysOff"); } + @Test + public void allOn() { + HashMap features = new HashMap<>(); + Feature onFeature = new Feature(); + onFeature.setKey("On"); + HashMap filters = new HashMap(); + FeatureFilterEvaluationContext alwaysOn = new FeatureFilterEvaluationContext(); + alwaysOn.setName("AlwaysOn"); + filters.put(0, alwaysOn); + filters.put(1, alwaysOn); + onFeature.setEnabledFor(filters); + onFeature.setRequirementType("All"); + features.put("On", onFeature); + when(featureManagementPropertiesMock.getFeatureManagement()).thenReturn(features); + + when(context.getBean(Mockito.matches("AlwaysOn"))).thenReturn(new AlwaysOnFilter()) + .thenReturn(new AlwaysOnFilter()); + + assertTrue(featureManager.isEnabledAsync("On").block()); + } + + @Test + public void oneOffAny() { + HashMap features = new HashMap<>(); + Feature onFeature = new Feature(); + onFeature.setKey("On"); + HashMap filters = new HashMap(); + FeatureFilterEvaluationContext alwaysOn = new FeatureFilterEvaluationContext(); + alwaysOn.setName("AlwaysOn"); + filters.put(0, alwaysOn); + filters.put(1, alwaysOn); + onFeature.setEnabledFor(filters); + onFeature.setRequirementType("Any"); + features.put("On", onFeature); + when(featureManagementPropertiesMock.getFeatureManagement()).thenReturn(features); + + when(context.getBean(Mockito.matches("AlwaysOn"))).thenReturn(new AlwaysOnFilter()) + .thenReturn(new AlwaysOffFilter()); + + assertTrue(featureManager.isEnabledAsync("On").block()); + } + + @Test + public void oneOffAll() { + HashMap features = new HashMap<>(); + Feature onFeature = new Feature(); + onFeature.setKey("On"); + HashMap filters = new HashMap(); + FeatureFilterEvaluationContext alwaysOn = new FeatureFilterEvaluationContext(); + alwaysOn.setName("AlwaysOn"); + filters.put(0, alwaysOn); + filters.put(1, alwaysOn); + onFeature.setEnabledFor(filters); + onFeature.setRequirementType("All"); + features.put("On", onFeature); + when(featureManagementPropertiesMock.getFeatureManagement()).thenReturn(features); + + when(context.getBean(Mockito.matches("AlwaysOn"))).thenReturn(new AlwaysOnFilter()) + .thenReturn(new AlwaysOffFilter()); + + assertFalse(featureManager.isEnabledAsync("On").block()); + } + class AlwaysOnFilter implements FeatureFilter { @Override @@ -148,4 +211,13 @@ public boolean evaluate(FeatureFilterEvaluationContext context) { } + class AlwaysOffFilter implements FeatureFilter { + + @Override + public boolean evaluate(FeatureFilterEvaluationContext context) { + return false; + } + + } + } From 327d45dd600da5db94b5a36ee12f99da87d5a6d8 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 11 May 2023 19:37:27 -0700 Subject: [PATCH 2/6] Adding Requirement Type --- ...rationFeatureManagementPropertySource.java | 17 ++++++- .../feature/entity/Feature.java | 9 ++-- ...onfigurationPropertySourceLocatorTest.java | 47 +++++++++++++++++++ .../implementation/models/Feature.java | 11 +++-- 4 files changed, 75 insertions(+), 9 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java index f60d47a12d9f..26c0ee9ecfad 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java @@ -32,7 +32,10 @@ import com.azure.data.appconfiguration.models.SettingSelector; import com.azure.spring.cloud.appconfiguration.config.implementation.feature.entity.Feature; import com.azure.spring.cloud.appconfiguration.config.implementation.http.policy.TracingInfo; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.json.JsonMapper; @@ -127,7 +130,17 @@ List getFeatureFlagSettings() { @SuppressWarnings("unchecked") private Object createFeature(FeatureFlagConfigurationSetting item) { String key = getFeatureSimpleName(item); - Feature feature = new Feature(key, item); + String requirementType = "Any"; + try { + JsonNode node = CASE_INSENSITIVE_MAPPER.readTree(item.getValue()); + JsonNode conditions = node.get("conditions"); + if (conditions != null && conditions.get("requirement_type") != null) { + requirementType = conditions.get("requirement_type").asText(); + } + } catch (JsonProcessingException e) { + + } + Feature feature = new Feature(key, item, requirementType); Map featureEnabledFor = feature.getEnabledFor(); // Setting Enabled For to null, but enabled = true will result in the feature @@ -170,7 +183,7 @@ private Object createFeature(FeatureFlagConfigurationSetting item) { return feature; } - + /** * Looks at each filter used in a Feature Flag to check what types it is using. * diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java index f26a40541ac0..47c720fcbf32 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java @@ -8,7 +8,6 @@ import com.azure.data.appconfiguration.models.FeatureFlagConfigurationSetting; import com.azure.data.appconfiguration.models.FeatureFlagFilter; -import com.fasterxml.jackson.annotation.JsonAlias; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; @@ -21,8 +20,11 @@ public final class Feature { @JsonProperty("key") private String key; - @JsonAlias("enabled-for") + @JsonProperty("enabled-for") private Map enabledFor; + + @JsonProperty("requirement-type") + private String requirementType = "Any"; /** * Feature Flag object. @@ -36,7 +38,7 @@ public Feature() { * @param key Name of the Feature Flag * @param featureItem Configurations of the Feature Flag. */ - public Feature(String key, FeatureFlagConfigurationSetting featureItem) { + public Feature(String key, FeatureFlagConfigurationSetting featureItem, String requirementType) { this.key = key; List filterMapper = featureItem.getClientFilters(); @@ -45,6 +47,7 @@ public Feature(String key, FeatureFlagConfigurationSetting featureItem) { for (int i = 0; i < filterMapper.size(); i++) { enabledFor.put(i, filterMapper.get(i)); } + this.requirementType = requirementType; } /** diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySourceLocatorTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySourceLocatorTest.java index 150bfe5e019e..b352521ef355 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySourceLocatorTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySourceLocatorTest.java @@ -45,6 +45,7 @@ import com.azure.data.appconfiguration.ConfigurationAsyncClient; import com.azure.data.appconfiguration.models.ConfigurationSetting; import com.azure.data.appconfiguration.models.FeatureFlagConfigurationSetting; +import com.azure.data.appconfiguration.models.FeatureFlagFilter; import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationKeyValueSelector; import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationProperties; import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationProviderProperties; @@ -347,7 +348,50 @@ public void storeCreatedWithFeatureFlags() { featureFlagStore.setEnabled(true); featureFlagStore.validateAndInit(); + List featureList = new ArrayList<>(); + FeatureFlagConfigurationSetting featureFlag = new FeatureFlagConfigurationSetting("Alpha", false); + featureFlag.setValue("{}"); + featureList.add(featureFlag); + + when(configStoreMock.getFeatureFlags()).thenReturn(featureFlagStore); + when(replicaClientMock.listSettings(Mockito.any())).thenReturn(featureList); + + locator = new AppConfigurationPropertySourceLocator(appProperties, clientFactoryMock, keyVaultClientFactory, + null, stores); + + try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { + stateHolderMock.when(() -> StateHolder.updateState(Mockito.any())).thenReturn(null); + PropertySource source = locator.locate(emptyEnvironment); + assertTrue(source instanceof CompositePropertySource); + + Collection> 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[] { + "FM_store1/", + KEY_FILTER + "store1/\0" + }; + assertEquals(expectedSourceNames.length, sources.size()); + assertArrayEquals((Object[]) expectedSourceNames, sources.stream().map(PropertySource::getName).toArray()); + } + } + + @Test + public void storeCreatedWithFeatureFlagsRequireAll() { + FeatureFlagStore featureFlagStore = new FeatureFlagStore(); + featureFlagStore.setEnabled(true); + featureFlagStore.validateAndInit(); + + List featureList = new ArrayList<>(); + FeatureFlagConfigurationSetting featureFlag = new FeatureFlagConfigurationSetting("Alpha", true); + featureFlag.setValue("{\"conditions\":{\"requirement_type\":\"All\"}}"); + featureFlag.addClientFilter(new FeatureFlagFilter("AlwaysOn")); + featureList.add(featureFlag); + when(configStoreMock.getFeatureFlags()).thenReturn(featureFlagStore); + when(replicaClientMock.listSettings(Mockito.any())).thenReturn(featureList); locator = new AppConfigurationPropertySourceLocator(appProperties, clientFactoryMock, keyVaultClientFactory, null, stores); @@ -367,7 +411,9 @@ public void storeCreatedWithFeatureFlags() { KEY_FILTER + "store1/\0" }; assertEquals(expectedSourceNames.length, sources.size()); + Object[] propertSources = sources.stream().map(c -> c.getProperty("feature-management.Alpha")).toArray(); assertArrayEquals((Object[]) expectedSourceNames, sources.stream().map(PropertySource::getName).toArray()); + } } @@ -381,6 +427,7 @@ public void storeCreatedWithFeatureFlagsWithMonitoring() { List featureList = new ArrayList<>(); FeatureFlagConfigurationSetting featureFlag = new FeatureFlagConfigurationSetting("Alpha", false); + featureFlag.setValue("{}"); featureList.add(featureFlag); when(configStoreMock.getFeatureFlags()).thenReturn(featureFlagStore); diff --git a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java index 21c930dd59f1..f725ce74d5de 100644 --- a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java +++ b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java @@ -2,10 +2,12 @@ // Licensed under the MIT License. package com.azure.spring.cloud.feature.management.implementation.models; +import java.util.HashMap; +import java.util.Map; + import com.azure.spring.cloud.feature.management.models.FeatureFilterEvaluationContext; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; -import java.util.HashMap; /** * App Configuration Feature defines the feature name and a Map of FeatureFilterEvaluationContexts. @@ -19,10 +21,11 @@ public class Feature { @JsonProperty("evaluate") private Boolean evaluate = true; - private String requirementType = "Any"; + @JsonProperty("requirement-type") + private String requirementType = "Any";; @JsonProperty("enabled-for") - private HashMap enabledFor; + private Map enabledFor; /** * @return the key @@ -55,7 +58,7 @@ public void setEvaluate(Boolean evaluate) { /** * @return the enabledFor */ - public HashMap getEnabledFor() { + public Map getEnabledFor() { return enabledFor; } From 8df5bc4aeaeb163fcd1d704e0a6d4d0a9c690de0 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 12 May 2023 12:40:40 -0700 Subject: [PATCH 3/6] Updated Test --- ...igurationFeatureManagementPropertySource.java | 1 - .../implementation/feature/entity/Feature.java | 16 +++++++++++++++- ...ppConfigurationPropertySourceLocatorTest.java | 11 +++++++---- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java index 26c0ee9ecfad..da2da63c17c2 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java @@ -34,7 +34,6 @@ import com.azure.spring.cloud.appconfiguration.config.implementation.http.policy.TracingInfo; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; -import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java index 47c720fcbf32..603172a97588 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java @@ -22,7 +22,7 @@ public final class Feature { @JsonProperty("enabled-for") private Map enabledFor; - + @JsonProperty("requirement-type") private String requirementType = "Any"; @@ -78,4 +78,18 @@ public void setEnabledFor(Map enabledFor) { this.enabledFor = enabledFor; } + /** + * @return the requirementType + */ + public String getRequirementType() { + return requirementType; + } + + /** + * @param requirementType the requirementType to set + */ + public void setRequirementType(String requirementType) { + this.requirementType = requirementType; + } + } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySourceLocatorTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySourceLocatorTest.java index b352521ef355..f36b392d699d 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySourceLocatorTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySourceLocatorTest.java @@ -45,7 +45,8 @@ import com.azure.data.appconfiguration.ConfigurationAsyncClient; import com.azure.data.appconfiguration.models.ConfigurationSetting; import com.azure.data.appconfiguration.models.FeatureFlagConfigurationSetting; -import com.azure.data.appconfiguration.models.FeatureFlagFilter; +import com.azure.spring.cloud.appconfiguration.config.implementation.feature.entity.Feature; +import com.azure.spring.cloud.appconfiguration.config.implementation.http.policy.TracingInfo; import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationKeyValueSelector; import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationProperties; import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationProviderProperties; @@ -386,12 +387,12 @@ public void storeCreatedWithFeatureFlagsRequireAll() { List featureList = new ArrayList<>(); FeatureFlagConfigurationSetting featureFlag = new FeatureFlagConfigurationSetting("Alpha", true); - featureFlag.setValue("{\"conditions\":{\"requirement_type\":\"All\"}}"); - featureFlag.addClientFilter(new FeatureFlagFilter("AlwaysOn")); + featureFlag.setValue("{\"id\":null,\"description\":null,\"display_name\":null,\"enabled\":true,\"conditions\":{\"requirement_type\":\"All\", \"client_filters\":[{\"name\":\"AlwaysOn\",\"parameters\":{}}]}}"); featureList.add(featureFlag); when(configStoreMock.getFeatureFlags()).thenReturn(featureFlagStore); when(replicaClientMock.listSettings(Mockito.any())).thenReturn(featureList); + when(replicaClientMock.getTracingInfo()).thenReturn(new TracingInfo(false, false, 0, null)); locator = new AppConfigurationPropertySourceLocator(appProperties, clientFactoryMock, keyVaultClientFactory, null, stores); @@ -411,7 +412,9 @@ public void storeCreatedWithFeatureFlagsRequireAll() { KEY_FILTER + "store1/\0" }; assertEquals(expectedSourceNames.length, sources.size()); - Object[] propertSources = sources.stream().map(c -> c.getProperty("feature-management.Alpha")).toArray(); + Object[] propertySources = sources.stream().map(c -> c.getProperty("feature-management.Alpha")).toArray(); + Feature alpha = (Feature) propertySources[0]; + assertEquals("All", alpha.getRequirementType()); assertArrayEquals((Object[]) expectedSourceNames, sources.stream().map(PropertySource::getName).toArray()); } From cad329114fa3bbd4f0578d0e585b9594b6dbf8cb Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 16 May 2023 11:05:27 -0700 Subject: [PATCH 4/6] Review Comments --- .../config/implementation/AppConfigurationConstants.java | 9 +++++++++ .../AppConfigurationFeatureManagementPropertySource.java | 9 +++++---- .../config/implementation/feature/entity/Feature.java | 3 ++- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationConstants.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationConstants.java index a8f6074fc551..00ceba58d172 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationConstants.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationConstants.java @@ -82,4 +82,13 @@ public class AppConfigurationConstants { public static final String DEFAULT_ROLLOUT_PERCENTAGE = "defaultRolloutPercentage"; public static final String DEFAULT_ROLLOUT_PERCENTAGE_CAPS = "DefaultRolloutPercentage"; + + + + public static final String DEFAULT_REQUIREMENT_TYPE = "Any"; + + public static final String REQUIREMENT_TYPE_SERVICE = "requirement_type"; + + public static final String REQUIREMENT_TYPE = "requirement-type"; + } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java index da2da63c17c2..3c8febc74c56 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java @@ -3,6 +3,7 @@ package com.azure.spring.cloud.appconfiguration.config.implementation; import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.AUDIENCE; +import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.DEFAULT_REQUIREMENT_TYPE; import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.DEFAULT_ROLLOUT_PERCENTAGE; import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.DEFAULT_ROLLOUT_PERCENTAGE_CAPS; import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.FEATURE_FLAG_CONTENT_TYPE; @@ -10,6 +11,7 @@ import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.FEATURE_MANAGEMENT_KEY; import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.GROUPS; import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.GROUPS_CAPS; +import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.REQUIREMENT_TYPE_SERVICE; import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.SELECT_ALL_FEATURE_FLAGS; import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.TARGETING_FILTER; import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.USERS; @@ -52,7 +54,6 @@ final class AppConfigurationFeatureManagementPropertySource extends AppConfigura .configure(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES, true).build(); private final List featureConfigurationSettings; - AppConfigurationFeatureManagementPropertySource(String originEndpoint, AppConfigurationReplicaClient replicaClient, String keyFilter, String[] labelFilter) { super("FM_" + originEndpoint, replicaClient, keyFilter, labelFilter); @@ -129,12 +130,12 @@ List getFeatureFlagSettings() { @SuppressWarnings("unchecked") private Object createFeature(FeatureFlagConfigurationSetting item) { String key = getFeatureSimpleName(item); - String requirementType = "Any"; + String requirementType = DEFAULT_REQUIREMENT_TYPE; try { JsonNode node = CASE_INSENSITIVE_MAPPER.readTree(item.getValue()); JsonNode conditions = node.get("conditions"); - if (conditions != null && conditions.get("requirement_type") != null) { - requirementType = conditions.get("requirement_type").asText(); + if (conditions != null && conditions.get(REQUIREMENT_TYPE_SERVICE) != null) { + requirementType = conditions.get(REQUIREMENT_TYPE_SERVICE).asText(); } } catch (JsonProcessingException e) { diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java index 603172a97588..24c4e504efeb 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java @@ -10,6 +10,7 @@ import com.azure.data.appconfiguration.models.FeatureFlagFilter; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; +import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.REQUIREMENT_TYPE; /** * Azure App Configuration Feature Flag. @@ -23,7 +24,7 @@ public final class Feature { @JsonProperty("enabled-for") private Map enabledFor; - @JsonProperty("requirement-type") + @JsonProperty(REQUIREMENT_TYPE) private String requirementType = "Any"; /** From c5075724ec75699bcb6f7ba9f692a16dceb676d8 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Wed, 17 May 2023 10:01:58 -0700 Subject: [PATCH 5/6] Clean up spacing --- .../config/implementation/AppConfigurationConstants.java | 2 -- .../AppConfigurationFeatureManagementPropertySource.java | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationConstants.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationConstants.java index 00ceba58d172..04f3ffea3af2 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationConstants.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationConstants.java @@ -83,8 +83,6 @@ public class AppConfigurationConstants { public static final String DEFAULT_ROLLOUT_PERCENTAGE_CAPS = "DefaultRolloutPercentage"; - - public static final String DEFAULT_REQUIREMENT_TYPE = "Any"; public static final String REQUIREMENT_TYPE_SERVICE = "requirement_type"; diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java index 3c8febc74c56..29c289cb961f 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java @@ -183,7 +183,7 @@ private Object createFeature(FeatureFlagConfigurationSetting item) { return feature; } - + /** * Looks at each filter used in a Feature Flag to check what types it is using. * From 2d6074d8c4958884946776a004bd32b58d595661 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 18 May 2023 11:10:17 -0700 Subject: [PATCH 6/6] Updating Constants --- .../config/implementation/feature/entity/Feature.java | 6 ++++-- .../implementation/FeatureManagementConstants.java | 9 +++++++++ .../management/implementation/models/Feature.java | 4 +++- 3 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/FeatureManagementConstants.java diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java index 24c4e504efeb..4435bb728367 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/feature/entity/Feature.java @@ -2,6 +2,9 @@ // Licensed under the MIT License. package com.azure.spring.cloud.appconfiguration.config.implementation.feature.entity; +import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.DEFAULT_REQUIREMENT_TYPE; +import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.REQUIREMENT_TYPE; + import java.util.HashMap; import java.util.List; import java.util.Map; @@ -10,7 +13,6 @@ import com.azure.data.appconfiguration.models.FeatureFlagFilter; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; -import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.REQUIREMENT_TYPE; /** * Azure App Configuration Feature Flag. @@ -25,7 +27,7 @@ public final class Feature { private Map enabledFor; @JsonProperty(REQUIREMENT_TYPE) - private String requirementType = "Any"; + private String requirementType = DEFAULT_REQUIREMENT_TYPE; /** * Feature Flag object. diff --git a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/FeatureManagementConstants.java b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/FeatureManagementConstants.java new file mode 100644 index 000000000000..d3aae1c796c7 --- /dev/null +++ b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/FeatureManagementConstants.java @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +package com.azure.spring.cloud.feature.management.implementation; + +public class FeatureManagementConstants { + + public static final String DEFAULT_REQUIREMENT_TYPE = "Any"; + +} diff --git a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java index f725ce74d5de..555505b56a68 100644 --- a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java +++ b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/models/Feature.java @@ -2,6 +2,8 @@ // Licensed under the MIT License. package com.azure.spring.cloud.feature.management.implementation.models; +import static com.azure.spring.cloud.feature.management.implementation.FeatureManagementConstants.DEFAULT_REQUIREMENT_TYPE; + import java.util.HashMap; import java.util.Map; @@ -22,7 +24,7 @@ public class Feature { private Boolean evaluate = true; @JsonProperty("requirement-type") - private String requirementType = "Any";; + private String requirementType = DEFAULT_REQUIREMENT_TYPE;; @JsonProperty("enabled-for") private Map enabledFor;