From fb981e4d1b90a86fe93c31144cfb734e4d0168ac Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 1 May 2026 14:24:55 -0700 Subject: [PATCH 1/6] Cleaning up codebase --- ...ationApplicationSettingPropertySource.java | 8 +- .../AppConfigurationRefreshUtil.java | 6 +- .../AppConfigurationReplicaClient.java | 66 ++++------- ...ppConfigurationSnapshotPropertySource.java | 2 +- .../AzureAppConfigDataLoader.java | 2 +- .../AzureAppConfigDataLocationResolver.java | 2 +- .../implementation/ConnectionManager.java | 29 ++--- .../implementation/FeatureFlagClient.java | 24 ++-- .../config/implementation/State.java | 6 +- .../config/implementation/StateHolder.java | 6 +- .../autofailover/ReplicaLookUp.java | 111 ++++++++++-------- .../autofailover/SRVRecord.java | 33 ++---- .../WatchedConfigurationSettings.java | 18 +-- .../feature/entity/Feature.java | 5 +- .../AppConfigurationKeyValueSelector.java | 6 +- .../AppConfigurationProperties.java | 12 +- .../properties/ConfigStore.java | 7 ++ .../FeatureFlagKeyValueSelector.java | 12 +- .../properties/FeatureFlagStore.java | 8 +- .../AppConfigurationSecretClientManager.java | 7 +- .../AppConfigurationReplicaClientTest.java | 24 ++-- .../implementation/FeatureFlagClientTest.java | 56 ++++----- .../autofailover/SRVRecordTest.java | 5 +- .../stores/ConfigStoreTest.java | 7 +- 24 files changed, 209 insertions(+), 253 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationApplicationSettingPropertySource.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationApplicationSettingPropertySource.java index 4fcc931cb93e..03a83d0e3913 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationApplicationSettingPropertySource.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationApplicationSettingPropertySource.java @@ -2,8 +2,6 @@ // Licensed under the MIT License. package com.azure.spring.cloud.appconfiguration.config.implementation; -import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.FEATURE_FLAG_CONTENT_TYPE; - import java.net.URI; import java.net.URISyntaxException; import java.util.ArrayList; @@ -24,6 +22,7 @@ import com.azure.data.appconfiguration.models.SecretReferenceConfigurationSetting; import com.azure.data.appconfiguration.models.SettingSelector; import com.azure.security.keyvault.secrets.models.KeyVaultSecret; +import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.FEATURE_FLAG_CONTENT_TYPE; /** * Azure App Configuration PropertySource unique per Store Label(Profile) combo. @@ -68,7 +67,7 @@ class AppConfigurationApplicationSettingPropertySource extends AppConfigurationP @Override public void initProperties(List keyPrefixTrimValues, Context context) throws InvalidConfigurationPropertyValueException { - List labels = Arrays.asList(labelFilters); + List labels = new ArrayList<>(Arrays.asList(labelFilters)); // Reverse labels so they have the right priority order. Collections.reverse(labels); @@ -114,7 +113,6 @@ protected void processConfigurationSettings(List settings, * * @param key Application Setting name * @param secretReference {"uri": "<your-vault-url>/secret/<secret>/<version>"} - * @return Key Vault Secret Value * @throws InvalidConfigurationPropertyValueException */ private void handleKeyVaultReference(String key, SecretReferenceConfigurationSetting secretReference) @@ -153,7 +151,7 @@ private String trimKey(String key, List trimStrings) { if (trimStrings != null) { for (String trim : trimStrings) { if (key.startsWith(trim)) { - return key.replaceFirst("^" + trim, "").replace('/', '.'); + return key.substring(trim.length()).replace('/', '.'); } } } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationRefreshUtil.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationRefreshUtil.java index abec63fa1ba8..02c053f5afa7 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationRefreshUtil.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationRefreshUtil.java @@ -2,8 +2,6 @@ // Licensed under the MIT License. package com.azure.spring.cloud.appconfiguration.config.implementation; -import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.PUSH_REFRESH; - import java.time.Duration; import java.time.Instant; import java.util.List; @@ -16,6 +14,7 @@ import com.azure.core.exception.HttpResponseException; import com.azure.core.util.Context; import com.azure.data.appconfiguration.models.ConfigurationSetting; +import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.PUSH_REFRESH; import com.azure.spring.cloud.appconfiguration.config.implementation.autofailover.ReplicaLookUp; import com.azure.spring.cloud.appconfiguration.config.implementation.configuration.WatchedConfigurationSettings; import com.azure.spring.cloud.appconfiguration.config.implementation.feature.FeatureFlagState; @@ -229,7 +228,7 @@ static boolean refreshStoreCheck(AppConfigurationReplicaClient client, String or * @param context the operation context * @return true if a refresh should be triggered, false otherwise */ - boolean refreshStoreFeatureFlagCheck(Boolean featureStoreEnabled, + boolean refreshStoreFeatureFlagCheck(boolean featureStoreEnabled, AppConfigurationReplicaClient client, Context context) { RefreshEventData eventData = new RefreshEventData(); String endpoint = client.getEndpoint(); @@ -382,6 +381,7 @@ private static void refreshWithoutTimeFeatureFlags(AppConfigurationReplicaClient LOGGER.info("Configuration Refresh Event triggered by {}", FEATURE_FLAG_PREFIX); eventData.setMessage(FEATURE_FLAG_PREFIX); + return; } } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java index edb761e0e737..d4cebb7ed8fb 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java @@ -18,7 +18,6 @@ import com.azure.data.appconfiguration.ConfigurationClient; import com.azure.data.appconfiguration.models.ConfigurationSetting; import com.azure.data.appconfiguration.models.ConfigurationSnapshot; -import com.azure.data.appconfiguration.models.FeatureFlagConfigurationSetting; import com.azure.data.appconfiguration.models.SettingSelector; import com.azure.data.appconfiguration.models.SnapshotComposition; import com.azure.spring.cloud.appconfiguration.config.implementation.configuration.WatchedConfigurationSettings; @@ -151,15 +150,14 @@ List listSettings(SettingSelector settingSelector, Context } /** - * Gets configuration settings using watched configuration settings. This method retrieves all settings matching the - * selector and captures ETags for collection-based refresh monitoring. - * + * Lists configuration settings page by page, capturing ETags for collection-based refresh monitoring. + * * @param settingSelector selector criteria for configuration settings * @param context Azure SDK context for request correlation - * @return WatchedConfigurationSettings containing the retrieved configuration settings and match conditions + * @return WatchedConfigurationSettings containing the retrieved settings and match conditions * @throws HttpResponseException if the request fails */ - WatchedConfigurationSettings loadWatchedSettings(SettingSelector settingSelector, Context context) { + WatchedConfigurationSettings listSettingsByPage(SettingSelector settingSelector, Context context) { List configurationSettings = new ArrayList<>(); List checks = new ArrayList<>(); try { @@ -182,39 +180,6 @@ WatchedConfigurationSettings loadWatchedSettings(SettingSelector settingSelector } } - /** - * Lists feature flags from the Azure App Configuration store. - * - * @param settingSelector selector criteria for feature flags - * @param context Azure SDK context for request correlation - * @return WatchedConfigurationSettings containing the retrieved feature flags and match conditions - * @throws HttpResponseException if the request fails - */ - WatchedConfigurationSettings listFeatureFlags(SettingSelector settingSelector, Context context) - throws HttpResponseException { - List configurationSettings = new ArrayList<>(); - List checks = new ArrayList<>(); - try { - client.listConfigurationSettings(settingSelector, context).streamByPage().forEach(pagedResponse -> { - checks.add( - new MatchConditions().setIfNoneMatch(pagedResponse.getHeaders().getValue(HttpHeaderName.ETAG))); - for (ConfigurationSetting featureFlag : pagedResponse.getValue()) { - configurationSettings - .add((FeatureFlagConfigurationSetting) NormalizeNull.normalizeNullLabel(featureFlag)); - } - }); - - // Needs to happen after or we don't know if the request succeeded or failed. - this.failedAttempts = 0; - settingSelector.setMatchConditions(checks); - return new WatchedConfigurationSettings(settingSelector, configurationSettings); - } catch (HttpResponseException e) { - throw handleHttpResponseException(e); - } catch (UncheckedIOException e) { - throw new AppConfigurationStatusException(e.getMessage(), null, null); - } - } - /** * Lists configuration settings from a specific snapshot. * @@ -245,11 +210,26 @@ List listSettingSnapshot(String snapshotName, Context cont } } + /** + * Checks if any watched configuration settings have been modified by comparing ETags. + * + * @param settingSelector selector with match conditions to check for modifications + * @param context Azure SDK context for request correlation + * @return true if any settings have been modified, false otherwise + * @throws HttpResponseException if the request fails + */ boolean checkWatchKeys(SettingSelector settingSelector, Context context) { - List> results = client - .listConfigurationSettings(settingSelector, context) - .streamByPage().filter(pagedResponse -> pagedResponse.getStatusCode() != HTTP_NOT_MODIFIED).toList(); - return !results.isEmpty(); + try { + List> results = client + .listConfigurationSettings(settingSelector, context) + .streamByPage().filter(pagedResponse -> pagedResponse.getStatusCode() != HTTP_NOT_MODIFIED).toList(); + this.failedAttempts = 0; + return !results.isEmpty(); + } catch (HttpResponseException e) { + throw handleHttpResponseException(e); + } catch (UncheckedIOException e) { + throw new AppConfigurationStatusException(e.getMessage(), null, null); + } } /** diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationSnapshotPropertySource.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationSnapshotPropertySource.java index 47c39c3fd920..ea0a4a12aa47 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationSnapshotPropertySource.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationSnapshotPropertySource.java @@ -50,7 +50,7 @@ public void initProperties(List trim, Context context) throws InvalidCon processConfigurationSettings(replicaClient.listSettingSnapshot(snapshotName, context), null, trim); WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings(null, featureFlagsList); - featureFlagClient.proccessFeatureFlags(featureFlags, replicaClient.getEndpoint()); + featureFlagClient.processFeatureFlags(featureFlags, replicaClient.getEndpoint()); } @Override diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java index eb1401d404c9..04f8442e63d7 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java @@ -431,7 +431,7 @@ private List getWatchedConfigurationSettings(AppCo settingSelector.setTagsFilter(selectedKeys.getTagsFilter()); } - WatchedConfigurationSettings watchedConfigurationSettings = client.loadWatchedSettings(settingSelector, + WatchedConfigurationSettings watchedConfigurationSettings = client.listSettingsByPage(settingSelector, requestContext); watchedConfigurationSettingsList.add(watchedConfigurationSettings); } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLocationResolver.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLocationResolver.java index da582adbf5bf..39898e0b5144 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLocationResolver.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLocationResolver.java @@ -60,7 +60,7 @@ public boolean isResolvable(ConfigDataLocationResolverContext context, ConfigDat Binder binder = context.getBinder(); // Check if Azure App Configuration is enabled - Boolean enabled = binder.bind(AppConfigurationProperties.CONFIG_PREFIX + ".enabled", Boolean.class).orElse(true); + boolean enabled = binder.bind(AppConfigurationProperties.CONFIG_PREFIX + ".enabled", Boolean.class).orElse(true); if (!enabled) { return false; } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java index 36049ed52fc8..563ccf56afab 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java @@ -156,22 +156,10 @@ public List getAvailableClients() { List availableClients = new ArrayList<>(); - if (clients.size() == 1 && !configStore.isLoadBalancingEnabled()) { - if (clients.get(0).getBackoffEndTime().isBefore(Instant.now())) { - availableClients.add(clients.get(0)); - } - } else if (!clients.isEmpty() && !configStore.isLoadBalancingEnabled()) { - for (AppConfigurationReplicaClient replicaClient : clients) { - if (replicaClient.getBackoffEndTime().isBefore(Instant.now())) { - LOGGER.debug("Using Client: " + replicaClient.getEndpoint()); - availableClients.add(replicaClient); - } - } - } else if (configStore.isLoadBalancingEnabled()) { - for (AppConfigurationReplicaClient client : clients) { - if (client.getBackoffEndTime().isBefore(Instant.now())) { - availableClients.add(client); - } + for (AppConfigurationReplicaClient client : clients) { + if (client.getBackoffEndTime().isBefore(Instant.now())) { + LOGGER.debug("Using Client: " + client.getEndpoint()); + availableClients.add(client); } } @@ -217,9 +205,12 @@ void backoffClient(String endpoint) { } } - int failedAttempt = autoFailoverClients.get(endpoint).getFailedAttempts(); - long backoffTime = BackoffTimeCalculator.calculateBackoff(failedAttempt); - autoFailoverClients.get(endpoint).updateBackoffEndTime(Instant.now().plusNanos(backoffTime)); + AppConfigurationReplicaClient failoverClient = autoFailoverClients.get(endpoint); + if (failoverClient != null) { + int failedAttempt = failoverClient.getFailedAttempts(); + long backoffTime = BackoffTimeCalculator.calculateBackoff(failedAttempt); + failoverClient.updateBackoffEndTime(Instant.now().plusNanos(backoffTime)); + } } /** diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClient.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClient.java index 18049c5200af..3b30f46bcdef 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClient.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClient.java @@ -64,7 +64,7 @@ class FeatureFlagClient { private static final ObjectMapper CASE_INSENSITIVE_MAPPER = JsonMapper.builder() .configure(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES, true).build(); - private FeatureFlagTracing tracing = new FeatureFlagTracing(); + private final FeatureFlagTracing tracing = new FeatureFlagTracing(); /** *

@@ -88,9 +88,11 @@ List loadFeatureFlags(AppConfigurationReplicaClien keyFilter = FEATURE_FLAG_PREFIX + customKeyFilter; } - List labels = Arrays.asList(labelFilter); + List labels = new ArrayList<>(Arrays.asList(labelFilter)); Collections.reverse(labels); + Context featureFlagContext = context.addData("FeatureFlagTracing", tracing); + for (String label : labels) { SettingSelector settingSelector = new SettingSelector().setKeyFilter(keyFilter).setLabelFilter(label); @@ -98,15 +100,13 @@ List loadFeatureFlags(AppConfigurationReplicaClien settingSelector.setTagsFilter(tagsFilter); } - context.addData("FeatureFlagTracing", tracing); - - WatchedConfigurationSettings features = replicaClient.listFeatureFlags(settingSelector, context); - loadedFeatureFlags.add(proccessFeatureFlags(features, replicaClient.getOriginClient())); + WatchedConfigurationSettings features = replicaClient.listSettingsByPage(settingSelector, featureFlagContext); + loadedFeatureFlags.add(processFeatureFlags(features, replicaClient.getOriginClient())); } return loadedFeatureFlags; } - WatchedConfigurationSettings proccessFeatureFlags(WatchedConfigurationSettings features, String endpoint) { + WatchedConfigurationSettings processFeatureFlags(WatchedConfigurationSettings features, String endpoint) { // Reading In Features for (ConfigurationSetting setting : features.getConfigurationSettings()) { if (setting instanceof FeatureFlagConfigurationSetting @@ -137,9 +137,7 @@ protected static Feature createFeature(FeatureFlagConfigurationSetting item, Str } JsonNode telemetryNode = node.get(TELEMETRY); if (telemetryNode != null && !telemetryNode.isEmpty()) { - ObjectMapper objectMapper = JsonMapper.builder() - .configure(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES, true).build(); - featureTelemetry = objectMapper.convertValue(telemetryNode, FeatureTelemetry.class); + featureTelemetry = CASE_INSENSITIVE_MAPPER.convertValue(telemetryNode, FeatureTelemetry.class); } feature = new Feature(item, requirementType, featureTelemetry); @@ -168,7 +166,7 @@ protected static Feature createFeature(FeatureFlagConfigurationSetting item, Str final Map originMetadata = telemetry.getMetadata(); originMetadata.put(E_TAG, item.getETag()); if (originEndpoint != null && !originEndpoint.isEmpty()) { - final String labelPart = item.getLabel().isEmpty() ? "" + final String labelPart = !StringUtils.hasText(item.getLabel()) ? "" : String.format("?label=%s", item.getLabel()); originMetadata.put(FEATURE_FLAG_REFERENCE, String.format("%s/kv/%s%s", originEndpoint, item.getKey(), labelPart)); @@ -272,9 +270,9 @@ static String generateAllocationId(JsonNode featureFlagValue) { } }); } - if (variantsValue != null && !variantsValue.isEmpty()) { + if (!variantsValue.isEmpty()) { List> sortedVariants = variantsValue.stream() - .filter(v -> allocatedVariants.contains(v.get("name"))) + .filter(v -> allocatedVariants.contains((String) v.get("name"))) .sorted(Comparator.comparing(v -> (String) v.get("name"))) .collect(Collectors.toList()); diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/State.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/State.java index 59a1b95150e9..15182065778f 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/State.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/State.java @@ -36,7 +36,7 @@ class State { private final String originEndpoint; /** Number of refresh attempts for exponential backoff calculation. */ - private int refreshAttempt; + private final int refreshAttempt; /** The refresh interval in seconds. */ private final int refreshInterval; @@ -152,8 +152,8 @@ public int getRefreshAttempt() { * @return the State instance with refreshAttempt incremented by 1 */ public State incrementRefreshAttempt() { - this.refreshAttempt += 1; - return this; + return new State(this.watchKeys, this.collectionWatchKeys, this.refreshInterval, + this.originEndpoint, this.nextRefreshCheck, this.refreshAttempt + 1); } /** diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/StateHolder.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/StateHolder.java index 6ec13ae92cd9..10cac09d9b2b 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/StateHolder.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/StateHolder.java @@ -42,7 +42,7 @@ public class StateHolder { private final Map loadState = new ConcurrentHashMap<>(); /** Number of client-level refresh attempts for backoff calculation. */ - private Integer clientRefreshAttempts = 1; + private int clientRefreshAttempts = 1; /** The next time a forced refresh should occur across all stores. */ private Instant nextForcedRefresh; @@ -155,7 +155,7 @@ public void expireState(String originEndpoint) { } long wait = (long) (new SecureRandom().nextDouble() * MAX_JITTER); - long timeLeft = (int) ((oldState.getNextRefreshCheck().toEpochMilli() - (Instant.now().toEpochMilli())) / 1000); + long timeLeft = ((oldState.getNextRefreshCheck().toEpochMilli() - (Instant.now().toEpochMilli())) / 1000); if (wait < timeLeft) { state.put(originEndpoint, new State(oldState, Instant.now().plusSeconds(wait))); } @@ -165,7 +165,7 @@ public void expireState(String originEndpoint) { * @param originEndpoint the configuration store connected to. * @param loaded true if the configuration store was loaded. */ - public void setLoadState(String originEndpoint, Boolean loaded) { + public void setLoadState(String originEndpoint, boolean loaded) { loadState.put(originEndpoint, loaded); } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/ReplicaLookUp.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/ReplicaLookUp.java index 93d91a301b85..ebfc7b97ec2e 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/ReplicaLookUp.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/ReplicaLookUp.java @@ -7,9 +7,9 @@ import java.time.Duration; import java.time.Instant; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Semaphore; import javax.naming.NameNotFoundException; @@ -39,7 +39,7 @@ public class ReplicaLookUp { private static final String REPLICA_PREFIX_TCP = "._tcp."; - private static final String SRC_RECORD = "SRV"; + private static final String SRV_RECORD = "SRV"; private static final String[] TRUSTED_DOMAIN_LABELS = { "azconfig", "appconfig" }; @@ -49,9 +49,9 @@ public class ReplicaLookUp { InitialDirContext context; - private Map> records = new HashMap>(); + private final Map> records = new ConcurrentHashMap<>(); - private Map wait = new HashMap<>(); + private final Map wait = new ConcurrentHashMap<>(); private final AppConfigurationProperties properties; @@ -66,38 +66,45 @@ public ReplicaLookUp(AppConfigurationProperties properties) throws NamingExcepti @Async public void updateAutoFailoverEndpoints() { if (semaphore.tryAcquire()) { - for (ConfigStore configStore : properties.getStores()) { - if (!configStore.isEnabled() || !configStore.isReplicaDiscoveryEnabled()) { - continue; - } - String mainEndpoint = configStore.getEndpoint(); - - List providedEndpoints = new ArrayList<>(); - if (configStore.getConnectionStrings().size() > 0) { - providedEndpoints = configStore.getConnectionStrings().stream().map(connectionString -> { - return (AppConfigurationReplicaClientsBuilder - .getEndpointFromConnectionString(connectionString)); - }).toList(); - } else if (configStore.getEndpoints().size() > 0) { - providedEndpoints = configStore.getEndpoints(); - } else { - providedEndpoints = List.of(configStore.getEndpoint()); - } - - try { - List srvRecords = findAutoFailoverEndpoints(mainEndpoint, providedEndpoints); - - srvRecords.sort((SRVRecord a, SRVRecord b) -> a.compareTo(b)); + try { + for (ConfigStore configStore : properties.getStores()) { + if (!configStore.isEnabled() || !configStore.isReplicaDiscoveryEnabled()) { + continue; + } + String mainEndpoint = configStore.getEndpoint(); + + Instant nextRefresh = wait.get(mainEndpoint); + if (nextRefresh != null && Instant.now().isBefore(nextRefresh)) { + continue; + } + + List providedEndpoints; + if (!configStore.getConnectionStrings().isEmpty()) { + providedEndpoints = configStore.getConnectionStrings().stream() + .map(AppConfigurationReplicaClientsBuilder::getEndpointFromConnectionString) + .toList(); + } else if (!configStore.getEndpoints().isEmpty()) { + providedEndpoints = configStore.getEndpoints(); + } else { + providedEndpoints = List.of(configStore.getEndpoint()); + } + + try { + List srvRecords = findAutoFailoverEndpoints(mainEndpoint, providedEndpoints); + + srvRecords.sort(SRVRecord::compareTo); + + records.put(mainEndpoint, srvRecords); + wait.put(mainEndpoint, Instant.now().plus(FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL)); + } catch (AppConfigurationReplicaException e) { + LOGGER.warn("Failed to find replicas due to: " + e.getMessage()); + wait.put(mainEndpoint, Instant.now().plus(MINIMAL_CLIENT_REFRESH_INTERVAL)); + } - records.put(mainEndpoint, srvRecords); - wait.put(mainEndpoint, Instant.now().plus(FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL)); - } catch (AppConfigurationReplicaException e) { - LOGGER.warn("Failed to finde replicas due to: " + e.getMessage()); - wait.put(mainEndpoint, Instant.now().plus(MINIMAL_CLIENT_REFRESH_INTERVAL)); } - + } finally { + semaphore.release(); } - semaphore.release(); } } @@ -111,8 +118,8 @@ public List getAutoFailoverEndpoints(String mainEndpoint) { private List findAutoFailoverEndpoints(String endpoint, List providedEndpoints) throws AppConfigurationReplicaException { - List records = new ArrayList<>(); - String host = ""; + List srvRecords = new ArrayList<>(); + String host; try { URI uri = new URI(endpoint); host = uri.getHost(); @@ -127,30 +134,34 @@ private List findAutoFailoverEndpoints(String endpoint, List String knownDomain = getKnownDomain(endpoint); if (!providedEndpoints.contains(origin.getEndpoint()) && validate(knownDomain, origin.getEndpoint())) { - records.add(origin); + srvRecords.add(origin); } - replicas.stream().forEach(replica -> { + replicas.forEach(replica -> { if (!providedEndpoints.contains(replica.getEndpoint()) && validate(knownDomain, replica.getEndpoint())) { - records.add(replica); + srvRecords.add(replica); } }); } - return records; + return srvRecords; } private SRVRecord getOriginRecord(String url) throws AppConfigurationReplicaException { Attribute attribute = requestRecord(ORIGIN_PREFIX + url); if (attribute != null) { - return parseHosts(attribute).get(0); + List hosts = parseHosts(attribute); + if (!hosts.isEmpty()) { + return hosts.get(0); + } } return null; } + private static final int MAX_REPLICA_COUNT = 20; + private List getReplicaRecords(SRVRecord origin) throws AppConfigurationReplicaException { List replicas = new ArrayList<>(); - int i = 0; - while (true) { + for (int i = 0; i < MAX_REPLICA_COUNT; i++) { Attribute attribute = requestRecord( REPLICA_PREFIX_ALT + i + REPLICA_PREFIX_TCP + origin.getTarget()); @@ -159,7 +170,6 @@ private List getReplicaRecords(SRVRecord origin) throws AppConfigurat } replicas.addAll(parseHosts(attribute)); - i++; } return replicas; } @@ -168,12 +178,18 @@ private Attribute requestRecord(String name) throws AppConfigurationReplicaExcep Instant retryTime = Instant.now().plusSeconds(30); while (retryTime.isAfter(Instant.now())) { try { - return context.getAttributes(name, new String[] { SRC_RECORD }).get(SRC_RECORD); + return context.getAttributes(name, new String[] { SRV_RECORD }).get(SRV_RECORD); } catch (NameNotFoundException e) { // Found Last Record, should be the case that no SRV Record exists. return null; } catch (NamingException e) { // Will retry for up to 30 seconds + try { + Thread.sleep(1000); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new AppConfigurationReplicaException(); + } } } throw new AppConfigurationReplicaException(); @@ -182,11 +198,12 @@ private Attribute requestRecord(String name) throws AppConfigurationReplicaExcep private List parseHosts(Attribute attribute) { List hosts = new ArrayList<>(); try { - NamingEnumeration records = attribute.getAll(); - while (records.hasMore()) { - hosts.add(new SRVRecord(((String) records.next()).toString().split(" "))); + NamingEnumeration srvRecords = attribute.getAll(); + while (srvRecords.hasMore()) { + hosts.add(new SRVRecord(((String) srvRecords.next()).split(" "))); } } catch (NamingException e) { + LOGGER.warn("Failed to parse SRV record hosts", e); } return hosts; diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/SRVRecord.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/SRVRecord.java index 7b7e9c1225f2..2266d80757cc 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/SRVRecord.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/SRVRecord.java @@ -2,7 +2,7 @@ // Licensed under the MIT License. package com.azure.spring.cloud.appconfiguration.config.implementation.autofailover; -class SRVRecord { +class SRVRecord implements Comparable { private final int priority; @@ -12,12 +12,12 @@ class SRVRecord { private final String target; - private static final String PROTOCAL = "https://"; + private static final String PROTOCOL = "https://"; SRVRecord(String[] record) { - this.priority = Integer.valueOf(record[0]); - this.weight = Integer.valueOf(record[1]); - this.port = Integer.valueOf(record[2]); + this.priority = Integer.parseInt(record[0]); + this.weight = Integer.parseInt(record[1]); + this.port = Integer.parseInt(record[2]); this.target = record[3].substring(0, record[3].length() - 1); } @@ -38,24 +38,15 @@ public String getTarget() { } public String getEndpoint() { - return PROTOCAL + target; + return PROTOCOL + target; } - int compareTo(SRVRecord record) { - if (priority > record.getPriority()) { - return 1; + @Override + public int compareTo(SRVRecord record) { + if (priority != record.getPriority()) { + return Integer.compare(priority, record.getPriority()); } - if (record.getPriority() > priority) { - return -1; - } - - if (weight > record.getWeight()) { - return 1; - } - if (record.getWeight() > weight) { - return -1; - } - - return 0; + // Higher weight should be preferred (sorted first) + return Integer.compare(record.getWeight(), weight); } } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/configuration/WatchedConfigurationSettings.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/configuration/WatchedConfigurationSettings.java index 20b09087087e..1b12e7a5e6e9 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/configuration/WatchedConfigurationSettings.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/configuration/WatchedConfigurationSettings.java @@ -9,9 +9,9 @@ public class WatchedConfigurationSettings { - private SettingSelector settingSelector; + private final SettingSelector settingSelector; - private List configurationSettings; + private final List configurationSettings; public WatchedConfigurationSettings(SettingSelector settingSelector, List configurationSettings) { @@ -26,13 +26,6 @@ public SettingSelector getSettingSelector() { return settingSelector; } - /** - * @param settingSelector the settingSelector to set - */ - public void setSettingSelector(SettingSelector settingSelector) { - this.settingSelector = settingSelector; - } - /** * @return the configurationSettings */ @@ -40,11 +33,4 @@ public List getConfigurationSettings() { return configurationSettings; } - /** - * @param configurations the configurations to set - */ - public void setConfigurationSettings(List configurations) { - this.configurationSettings = configurations; - } - } 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 462e9d503292..b4bcc64826ba 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 @@ -44,8 +44,9 @@ public Feature() { /** * Feature Flag object. * - * @param key Name of the Feature Flag - * @param featureItem Configurations of the Feature Flag. + * @param featureFlag Feature Flag configuration setting. + * @param requirementType Requirement type for the feature flag conditions. + * @param telemetry Telemetry configuration for the feature flag. */ public Feature(FeatureFlagConfigurationSetting featureFlag, String requirementType, FeatureTelemetry telemetry) { this.id = featureFlag.getFeatureId(); diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationKeyValueSelector.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationKeyValueSelector.java index a26431966344..259b0734cbb6 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationKeyValueSelector.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationKeyValueSelector.java @@ -12,7 +12,6 @@ import org.springframework.util.StringUtils; import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.EMPTY_LABEL; - import com.azure.spring.cloud.appconfiguration.config.implementation.ValidationUtil; import jakarta.annotation.PostConstruct; @@ -112,7 +111,7 @@ public String[] getLabelFilter(List profiles) { // such as when obtained from Environment.getActiveProfiles(). See // https://github.com/Azure/azure-sdk-for-java/issues/32708 for details. Collections.reverse(mutableProfiles); - return mutableProfiles.toArray(new String[mutableProfiles.size()]); + return mutableProfiles.toArray(String[]::new); } if (!StringUtils.hasText(labelFilter)) { return EMPTY_LABEL_ARRAY; @@ -127,8 +126,7 @@ public String[] getLabelFilter(List profiles) { } Collections.reverse(labels); - String[] t = new String[labels.size()]; - return labels.toArray(t); + return labels.toArray(String[]::new); } /** diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationProperties.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationProperties.java index adfe72d0627b..d57a37bf8fd2 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationProperties.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationProperties.java @@ -4,9 +4,9 @@ import java.time.Duration; import java.util.ArrayList; -import java.util.HashMap; +import java.util.HashSet; import java.util.List; -import java.util.Map; +import java.util.Set; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.annotation.Configuration; @@ -126,7 +126,7 @@ public void validateAndInit() { store.validateAndInit(); } - Map existingEndpoints = new HashMap<>(); + Set existingEndpoints = new HashSet<>(); for (ConfigStore store : this.stores) { if (!store.isEnabled()) { @@ -134,16 +134,14 @@ public void validateAndInit() { } if (!store.getEndpoints().isEmpty()) { for (String endpoint : store.getEndpoints()) { - if (existingEndpoints.containsKey(endpoint)) { + if (!existingEndpoints.add(endpoint)) { throw new IllegalArgumentException("Duplicate store name exists."); } - existingEndpoints.put(endpoint, true); } } else if (StringUtils.hasText(store.getEndpoint())) { - if (existingEndpoints.containsKey(store.getEndpoint())) { + if (!existingEndpoints.add(store.getEndpoint())) { throw new IllegalArgumentException("Duplicate store name exists."); } - existingEndpoints.put(store.getEndpoint(), true); } } if (refreshInterval != null) { diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/ConfigStore.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/ConfigStore.java index ffa0450762ad..8a488f50553c 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/ConfigStore.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/ConfigStore.java @@ -222,6 +222,13 @@ public void setFeatureFlags(FeatureFlagStore featureFlags) { this.featureFlags = featureFlags; } + /** + * Checks whether this config store contains the given endpoint. Used by the + * web library to match incoming refresh requests to a known store. + * + * @param endpoint the endpoint URL to check + * @return {@code true} if the primary endpoint or any replica endpoint starts with the given value + */ public boolean containsEndpoint(String endpoint) { if (this.endpoint.startsWith(endpoint)) { return true; diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagKeyValueSelector.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagKeyValueSelector.java index 356bf7fa3c00..92a5f4150879 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagKeyValueSelector.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagKeyValueSelector.java @@ -4,7 +4,7 @@ import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.EMPTY_LABEL; import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.LABEL_SEPARATOR; - +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -87,9 +87,10 @@ public String getLabelFilter() { * @return an array of resolved labels, ordered from lowest to highest priority */ public String[] getLabelFilter(List profiles) { - if (labelFilter == null && profiles.size() > 0) { - Collections.reverse(profiles); - return profiles.toArray(new String[profiles.size()]); + if (labelFilter == null && !profiles.isEmpty()) { + List mutableProfiles = new ArrayList<>(profiles); + Collections.reverse(mutableProfiles); + return mutableProfiles.toArray(String[]::new); } else if (!StringUtils.hasText(labelFilter)) { return EMPTY_LABEL_ARRAY; } @@ -105,8 +106,7 @@ public String[] getLabelFilter(List profiles) { } Collections.reverse(labels); - String[] t = new String[labels.size()]; - return labels.toArray(t); + return labels.toArray(String[]::new); } /** diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagStore.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagStore.java index 606fd920fd1c..5e565127ab38 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagStore.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagStore.java @@ -16,7 +16,7 @@ public final class FeatureFlagStore { /** * Enables or disables feature flag loading from the store. */ - private Boolean enabled = false; + private boolean enabled = false; private List selects = new ArrayList<>(); @@ -25,7 +25,7 @@ public final class FeatureFlagStore { * * @return {@code true} if enabled, {@code false} otherwise */ - public Boolean getEnabled() { + public boolean getEnabled() { return enabled; } @@ -34,7 +34,7 @@ public Boolean getEnabled() { * * @param enabled {@code true} to enable, {@code false} to disable */ - public void setEnabled(Boolean enabled) { + public void setEnabled(boolean enabled) { this.enabled = enabled; } @@ -62,7 +62,7 @@ public void setSelects(List selects) { */ @PostConstruct void validateAndInit() { - if (enabled && selects.size() == 0) { + if (enabled && selects.isEmpty()) { selects.add(new FeatureFlagKeyValueSelector()); } selects.forEach(FeatureFlagKeyValueSelector::validateAndInit); diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/AppConfigurationSecretClientManager.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/AppConfigurationSecretClientManager.java index 9a8437d7bfd2..b71ae05e00cd 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/AppConfigurationSecretClientManager.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/AppConfigurationSecretClientManager.java @@ -31,8 +31,8 @@ public final class AppConfigurationSecretClientManager { private final SecretClientBuilderFactory secretClientFactory; private final boolean credentialConfigured; - - private final int timeout = 30; + + private static final int TIMEOUT = 30; /** * Creates a Client for connecting to Key Vault @@ -74,7 +74,6 @@ AppConfigurationSecretClientManager build() { * Gets the specified secret using the Secret Identifier * * @param secretIdentifier The Secret Identifier to Secret - * @param timeout How long it waits for a response from Key Vault * @return Secret values that matches the secretIdentifier */ public KeyVaultSecret getSecret(URI secretIdentifier) { @@ -94,7 +93,7 @@ public KeyVaultSecret getSecret(URI secretIdentifier) { } } - return secretClient.getSecret(name, version).block(Duration.ofSeconds(timeout)); + return secretClient.getSecret(name, version).block(Duration.ofSeconds(TIMEOUT)); } } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientTest.java index 3e18eb2263ec..7b59c2865e89 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientTest.java @@ -158,7 +158,7 @@ public void listSettingsTest() { } @Test - public void listFeatureFlagsTest() { + public void listSettingsByPageTest() { AppConfigurationReplicaClient client = new AppConfigurationReplicaClient(endpoint, endpoint, clientMock); FeatureFlagConfigurationSetting featureFlag = new FeatureFlagConfigurationSetting("Alpha", false); @@ -175,24 +175,24 @@ public void listFeatureFlagsTest() { .thenReturn(new PagedIterable<>(pagedFlux)); assertEquals(configurations, - client.listFeatureFlags(new SettingSelector(), contextMock).getConfigurationSettings()); + client.listSettingsByPage(new SettingSelector(), contextMock).getConfigurationSettings()); when(clientMock.listConfigurationSettings(Mockito.any(), Mockito.any())).thenThrow(exceptionMock); when(exceptionMock.getResponse()).thenReturn(responseMock); when(responseMock.getStatusCode()).thenReturn(429); assertThrows(AppConfigurationStatusException.class, - () -> client.listFeatureFlags(new SettingSelector(), contextMock)); + () -> client.listSettingsByPage(new SettingSelector(), contextMock)); when(responseMock.getStatusCode()).thenReturn(408); assertThrows(AppConfigurationStatusException.class, - () -> client.listFeatureFlags(new SettingSelector(), contextMock)); + () -> client.listSettingsByPage(new SettingSelector(), contextMock)); when(responseMock.getStatusCode()).thenReturn(500); assertThrows(AppConfigurationStatusException.class, - () -> client.listFeatureFlags(new SettingSelector(), contextMock)); + () -> client.listSettingsByPage(new SettingSelector(), contextMock)); when(responseMock.getStatusCode()).thenReturn(499); - assertThrows(HttpResponseException.class, () -> client.listFeatureFlags(new SettingSelector(), contextMock)); + assertThrows(HttpResponseException.class, () -> client.listSettingsByPage(new SettingSelector(), contextMock)); } @Test @@ -380,7 +380,7 @@ public void watchedConfigurationSettingsTest() { .thenReturn(new PagedIterable<>(pagedFlux)); SettingSelector selector = new SettingSelector().setKeyFilter("*"); - WatchedConfigurationSettings result = client.loadWatchedSettings(selector, contextMock); + WatchedConfigurationSettings result = client.listSettingsByPage(selector, contextMock); assertEquals(2, result.getConfigurationSettings().size()); assertEquals("key1", result.getConfigurationSettings().get(0).getKey()); @@ -399,19 +399,19 @@ public void watchedConfigurationSettingsErrorTest() { when(responseMock.getStatusCode()).thenReturn(429); assertThrows(AppConfigurationStatusException.class, - () -> client.loadWatchedSettings(new SettingSelector(), contextMock)); + () -> client.listSettingsByPage(new SettingSelector(), contextMock)); when(responseMock.getStatusCode()).thenReturn(408); assertThrows(AppConfigurationStatusException.class, - () -> client.loadWatchedSettings(new SettingSelector(), contextMock)); + () -> client.listSettingsByPage(new SettingSelector(), contextMock)); when(responseMock.getStatusCode()).thenReturn(500); assertThrows(AppConfigurationStatusException.class, - () -> client.loadWatchedSettings(new SettingSelector(), contextMock)); + () -> client.listSettingsByPage(new SettingSelector(), contextMock)); when(responseMock.getStatusCode()).thenReturn(499); assertThrows(HttpResponseException.class, - () -> client.loadWatchedSettings(new SettingSelector(), contextMock)); + () -> client.listSettingsByPage(new SettingSelector(), contextMock)); } @Test @@ -422,7 +422,7 @@ public void watchedConfigurationSettingsUncheckedIOExceptionTest() { .thenThrow(new UncheckedIOException(new IOException("Network error"))); assertThrows(AppConfigurationStatusException.class, - () -> client.loadWatchedSettings(new SettingSelector(), contextMock)); + () -> client.listSettingsByPage(new SettingSelector(), contextMock)); } } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClientTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClientTest.java index ea4b036fbae3..f72af44dae08 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClientTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClientTest.java @@ -49,8 +49,7 @@ public class FeatureFlagClientTest { @Mock private AppConfigurationReplicaClient clientMock; - @Mock - private Context contextMock; + private final Context context = Context.NONE; private FeatureFlagClient featureFlagClient; @@ -84,14 +83,11 @@ public void cleanup() throws Exception { public void loadFeatureFlagsTestNoFeatureFlags() { List settings = List.of(new ConfigurationSetting().setKey("FakeKey")); WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings(null, settings); - when(clientMock.listFeatureFlags(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); + when(clientMock.listSettingsByPage(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); List featureFlagsList = featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, null, - contextMock); - assertEquals(1, featureFlagsList.size()); - assertEquals(featureFlags, featureFlagsList.get(0)); - assertEquals("FakeKey", featureFlagsList.get(0).getConfigurationSettings().get(0).getKey()); + context); assertEquals(0, featureFlagClient.getFeatureFlags().size()); } @@ -100,11 +96,11 @@ public void loadFeatureFlagsTestFeatureFlags() { List settings = List.of(new FeatureFlagConfigurationSetting("Alpha", false), new FeatureFlagConfigurationSetting("Beta", true)); WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings(null, settings); - when(clientMock.listFeatureFlags(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); + when(clientMock.listSettingsByPage(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); List featureFlagsList = featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, null, - contextMock); + context); assertEquals(1, featureFlagsList.size()); assertEquals(featureFlags, featureFlagsList.get(0)); assertEquals(".appconfig.featureflag/Alpha", @@ -118,11 +114,11 @@ public void loadFeatureFlagsTestMultipleLoads() { List settings = List.of(new FeatureFlagConfigurationSetting("Alpha", false), new FeatureFlagConfigurationSetting("Beta", true)); WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings(null, settings); - when(clientMock.listFeatureFlags(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); + when(clientMock.listSettingsByPage(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); List featureFlagsList = featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, null, - contextMock); + context); assertEquals(1, featureFlagsList.size()); assertEquals(featureFlags, featureFlagsList.get(0)); assertEquals(".appconfig.featureflag/Alpha", @@ -133,9 +129,9 @@ public void loadFeatureFlagsTestMultipleLoads() { List settings2 = List.of(new FeatureFlagConfigurationSetting("Alpha", true), new FeatureFlagConfigurationSetting("Gamma", false)); featureFlags = new WatchedConfigurationSettings(null, settings2); - when(clientMock.listFeatureFlags(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); + when(clientMock.listSettingsByPage(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); - featureFlagsList = featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, null, contextMock); + featureFlagsList = featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, null, context); assertEquals(1, featureFlagsList.size()); assertEquals(featureFlags, featureFlagsList.get(0)); assertEquals(".appconfig.featureflag/Alpha", @@ -181,11 +177,11 @@ public void loadFeatureFlagsTestTargetingFilter() { targetingFlag.addClientFilter(targetingFilter); List settings = List.of(targetingFlag); WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings(null, settings); - when(clientMock.listFeatureFlags(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); + when(clientMock.listSettingsByPage(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); List featureFlagsList = featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, null, - contextMock); + context); assertEquals(1, featureFlagsList.size()); assertEquals(featureFlags, featureFlagsList.get(0)); assertEquals(".appconfig.featureflag/TargetingTest", @@ -387,14 +383,14 @@ public void testFeatureFlagWithoutVariantsOrAllocation() { public void loadFeatureFlagsWithTagsFilterTest() { List settings = List.of(new FeatureFlagConfigurationSetting("Alpha", false)); WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings(null, settings); - when(clientMock.listFeatureFlags(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); + when(clientMock.listSettingsByPage(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); List tagsFilter = Arrays.asList("env=prod", "team=backend"); - featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, tagsFilter, contextMock); + featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, tagsFilter, context); - // Capture the SettingSelector passed to listFeatureFlags + // Capture the SettingSelector passed to listSettingsByPage ArgumentCaptor selectorCaptor = ArgumentCaptor.forClass(SettingSelector.class); - Mockito.verify(clientMock).listFeatureFlags(selectorCaptor.capture(), Mockito.any(Context.class)); + Mockito.verify(clientMock).listSettingsByPage(selectorCaptor.capture(), Mockito.any(Context.class)); SettingSelector capturedSelector = selectorCaptor.getValue(); assertEquals(2, capturedSelector.getTagsFilter().size()); @@ -406,13 +402,13 @@ public void loadFeatureFlagsWithTagsFilterTest() { public void loadFeatureFlagsWithNullTagsFilterTest() { List settings = List.of(new FeatureFlagConfigurationSetting("Alpha", false)); WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings(null, settings); - when(clientMock.listFeatureFlags(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); + when(clientMock.listSettingsByPage(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); - featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, null, contextMock); + featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, null, context); - // Capture the SettingSelector passed to listFeatureFlags + // Capture the SettingSelector passed to listSettingsByPage ArgumentCaptor selectorCaptor = ArgumentCaptor.forClass(SettingSelector.class); - Mockito.verify(clientMock).listFeatureFlags(selectorCaptor.capture(), Mockito.any(Context.class)); + Mockito.verify(clientMock).listSettingsByPage(selectorCaptor.capture(), Mockito.any(Context.class)); SettingSelector capturedSelector = selectorCaptor.getValue(); // Tags filter should not be set when null @@ -423,14 +419,14 @@ public void loadFeatureFlagsWithNullTagsFilterTest() { public void loadFeatureFlagsWithEmptyTagsFilterTest() { List settings = List.of(new FeatureFlagConfigurationSetting("Alpha", false)); WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings(null, settings); - when(clientMock.listFeatureFlags(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); + when(clientMock.listSettingsByPage(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); List emptyTags = List.of(); - featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, emptyTags, contextMock); + featureFlagClient.loadFeatureFlags(clientMock, null, emptyLabelList, emptyTags, context); - // Capture the SettingSelector passed to listFeatureFlags + // Capture the SettingSelector passed to listSettingsByPage ArgumentCaptor selectorCaptor = ArgumentCaptor.forClass(SettingSelector.class); - Mockito.verify(clientMock).listFeatureFlags(selectorCaptor.capture(), Mockito.any(Context.class)); + Mockito.verify(clientMock).listSettingsByPage(selectorCaptor.capture(), Mockito.any(Context.class)); SettingSelector capturedSelector = selectorCaptor.getValue(); // Tags filter should not be set when empty @@ -441,15 +437,15 @@ public void loadFeatureFlagsWithEmptyTagsFilterTest() { public void loadFeatureFlagsWithTagsFilterMultipleLabelsTest() { List settings = List.of(new FeatureFlagConfigurationSetting("Alpha", false)); WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings(null, settings); - when(clientMock.listFeatureFlags(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); + when(clientMock.listSettingsByPage(Mockito.any(), Mockito.any(Context.class))).thenReturn(featureFlags); String[] multiLabelList = { "dev", "prod" }; List tagsFilter = Arrays.asList("env=staging"); - featureFlagClient.loadFeatureFlags(clientMock, null, multiLabelList, tagsFilter, contextMock); + featureFlagClient.loadFeatureFlags(clientMock, null, multiLabelList, tagsFilter, context); // Capture all SettingSelector instances passed to listFeatureFlags (one per label) ArgumentCaptor selectorCaptor = ArgumentCaptor.forClass(SettingSelector.class); - Mockito.verify(clientMock, Mockito.times(2)).listFeatureFlags(selectorCaptor.capture(), + Mockito.verify(clientMock, Mockito.times(2)).listSettingsByPage(selectorCaptor.capture(), Mockito.any(Context.class)); // Both calls should have the tags filter set diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/SRVRecordTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/SRVRecordTest.java index 7c3401d6b7c9..2c11830d9017 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/SRVRecordTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/autofailover/SRVRecordTest.java @@ -3,7 +3,6 @@ package com.azure.spring.cloud.appconfiguration.config.implementation.autofailover; import static org.junit.jupiter.api.Assertions.assertEquals; - import org.junit.jupiter.api.Test; public class SRVRecordTest { @@ -30,8 +29,8 @@ public void compareTest() { String[] p1w2Record = {"1", "2", "1", "p1."}; SRVRecord p1w2 = new SRVRecord(p1w2Record); - assertEquals(-1, p1.compareTo(p1w2)); - assertEquals(1, p1w2.compareTo(p1)); + assertEquals(1, p1.compareTo(p1w2)); + assertEquals(-1, p1w2.compareTo(p1)); assertEquals(0, p1.compareTo(p1)); } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/ConfigStoreTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/ConfigStoreTest.java index f2407defceb1..5ba5cc12629c 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/ConfigStoreTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/ConfigStoreTest.java @@ -3,14 +3,11 @@ package com.azure.spring.cloud.appconfiguration.config.implementation.stores; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.util.ArrayList; import java.util.List; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import org.junit.jupiter.api.Test; import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationKeyValueSelector; From 982aef2657d1453801c092cb46da1fa235207edc Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 1 May 2026 16:01:35 -0700 Subject: [PATCH 2/6] Fixing backoff calculation --- .../config/implementation/BackoffTimeCalculator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/BackoffTimeCalculator.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/BackoffTimeCalculator.java index 39f378fe2eff..40c0fe5ffbf8 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/BackoffTimeCalculator.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/BackoffTimeCalculator.java @@ -83,7 +83,7 @@ static long calculateBackoff(Integer attempts) { maxNanoSeconds = maxBackoffNano; } - return (long) (minBackoffNano + ((RANDOM.nextDouble() * (maxNanoSeconds - minBackoffNano)) + minBackoffNano)); + return (long) (minBackoffNano + ((RANDOM.nextDouble() * (maxNanoSeconds - minBackoffNano)))); } } From 42f5d1b9b8b5d8cc646fd58e61b205977759f69c Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 1 May 2026 16:02:02 -0700 Subject: [PATCH 3/6] Update AppConfigurationReplicaClientFactory.java --- .../AppConfigurationReplicaClientFactory.java | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientFactory.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientFactory.java index c2c7e7939048..4e042087d153 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientFactory.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientFactory.java @@ -16,7 +16,7 @@ public class AppConfigurationReplicaClientFactory { /** Map of connection managers keyed by origin endpoint */ - private static final Map CONNECTIONS = new HashMap<>(); + private final Map connections = new HashMap<>(); /** List of configured stores for endpoint resolution */ private final List configStores; @@ -31,11 +31,9 @@ public class AppConfigurationReplicaClientFactory { AppConfigurationReplicaClientFactory(AppConfigurationReplicaClientsBuilder clientBuilder, List configStores, ReplicaLookUp replicaLookUp) { this.configStores = configStores; - if (CONNECTIONS.isEmpty()) { - for (ConfigStore store : configStores) { - ConnectionManager manager = new ConnectionManager(clientBuilder, store, replicaLookUp); - CONNECTIONS.put(manager.getMainEndpoint(), manager); - } + for (ConfigStore store : configStores) { + ConnectionManager manager = new ConnectionManager(clientBuilder, store, replicaLookUp); + connections.put(manager.getMainEndpoint(), manager); } } @@ -45,7 +43,7 @@ public class AppConfigurationReplicaClientFactory { * @return map of endpoint to connection manager */ public Map getConnections() { - return CONNECTIONS; + return connections; } /** @@ -56,7 +54,7 @@ public Map getConnections() { * @return the next active AppConfigurationReplicaClient */ AppConfigurationReplicaClient getNextActiveClient(String originEndpoint, boolean useLastActive) { - return CONNECTIONS.get(originEndpoint).getNextActiveClient(useLastActive); + return connections.get(originEndpoint).getNextActiveClient(useLastActive); } /** @@ -65,7 +63,7 @@ AppConfigurationReplicaClient getNextActiveClient(String originEndpoint, boolean * @param originEndpoint the origin configuration store endpoint */ void findActiveClients(String originEndpoint) { - CONNECTIONS.get(originEndpoint).findActiveClients(); + connections.get(originEndpoint).findActiveClients(); } /** @@ -75,7 +73,7 @@ void findActiveClients(String originEndpoint) { * @param endpoint the specific replica endpoint that failed */ void backoffClient(String originEndpoint, String endpoint) { - CONNECTIONS.get(originEndpoint).backoffClient(endpoint); + connections.get(originEndpoint).backoffClient(endpoint); } /** @@ -86,7 +84,7 @@ void backoffClient(String originEndpoint, String endpoint) { Map getHealth() { Map health = new HashMap<>(); - CONNECTIONS.forEach((key, value) -> health.put(key, value.getHealth())); + connections.forEach((key, value) -> health.put(key, value.getHealth())); return health; } @@ -114,7 +112,7 @@ String findOriginForEndpoint(String endpoint) { * @return duration in milliseconds until next client is available, or 0 if one is available now */ long getMillisUntilNextClientAvailable(String originEndpoint) { - return CONNECTIONS.get(originEndpoint).getMillisUntilNextClientAvailable(); + return connections.get(originEndpoint).getMillisUntilNextClientAvailable(); } /** @@ -125,7 +123,7 @@ long getMillisUntilNextClientAvailable(String originEndpoint) { * @param syncToken the new sync token to store */ void updateSyncToken(String originEndpoint, String endpoint, String syncToken) { - CONNECTIONS.get(originEndpoint).updateSyncToken(endpoint, syncToken); + connections.get(originEndpoint).updateSyncToken(endpoint, syncToken); } } From d9a56642f2111403db8d4135c02fa20c0672a2a4 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 1 May 2026 16:02:53 -0700 Subject: [PATCH 4/6] Updating Client Builder --- ...AppConfigurationReplicaClientsBuilder.java | 237 +++++++++++------- ...ConfigurationReplicaClientBuilderTest.java | 25 +- .../stores/ConfigStoreTest.java | 2 + 3 files changed, 167 insertions(+), 97 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientsBuilder.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientsBuilder.java index 8832196d5dc2..aaa6756f5fd9 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientsBuilder.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientsBuilder.java @@ -73,11 +73,11 @@ public class AppConfigurationReplicaClientsBuilder { private static final Pattern CONN_STRING_PATTERN = Pattern.compile(CONN_STRING_REGEXP); - private ConfigurationClientCustomizer clientCustomizer; + private final ConfigurationClientCustomizer clientCustomizer; private final ConfigurationClientBuilderFactory clientFactory; - private boolean isKeyVaultConfigured; + private final boolean isKeyVaultConfigured; private final boolean credentialConfigured; @@ -119,59 +119,43 @@ public static String getEndpointFromConnectionString(String connectionString) { * @throws IllegalArgumentException when more than 1 connection method is given. */ List buildClients(ConfigStore configStore) { - List clients = new ArrayList<>(); - // Single client or Multiple? - // If single call buildClient - int hasSingleConnectionString = StringUtils.hasText(configStore.getConnectionString()) ? 1 : 0; - int hasMultiEndpoints = configStore.getEndpoints().size() > 0 ? 1 : 0; - int hasMultiConnectionString = configStore.getConnectionStrings().size() > 0 ? 1 : 0; - - if (hasSingleConnectionString + hasMultiEndpoints + hasMultiConnectionString > 1) { - throw new IllegalArgumentException( - "More than 1 connection method was set for connecting to App Configuration."); - } - - boolean connectionStringIsPresent = configStore.getConnectionString() != null - || configStore.getConnectionStrings().size() > 0; - - if (credentialConfigured && connectionStringIsPresent) { + // Defensive copies — ConfigStore supports both singular (connectionString/endpoint) and + // plural (connectionStrings/endpoints) properties. We normalize into these local lists. + List connectionStrings = new ArrayList<>(configStore.getConnectionStrings()); + List endpoints = new ArrayList<>(configStore.getEndpoints()); + + boolean hasSingleConnectionString = StringUtils.hasText(configStore.getConnectionString()); + boolean hasMultiEndpoints = !endpoints.isEmpty(); + boolean hasMultiConnectionStrings = !connectionStrings.isEmpty(); + + boolean hasConnectionString = hasSingleConnectionString || hasMultiConnectionStrings; + + // Connection strings include their own auth (Id + Secret), so they cannot be combined with + // endpoints or with credentialConfigured (which uses Entra ID via Spring Cloud Azure auto-config). + // Endpoints + credentialConfigured is valid — that's the intended Entra ID auth flow. + if ((hasConnectionString && hasMultiEndpoints) + || (hasSingleConnectionString && hasMultiConnectionStrings) + || (credentialConfigured && hasConnectionString)) { throw new IllegalArgumentException( "More than 1 connection method was set for connecting to App Configuration."); } - List connectionStrings = configStore.getConnectionStrings(); - List endpoints = configStore.getEndpoints(); - - if (connectionStrings.size() == 0 && StringUtils.hasText(configStore.getConnectionString())) { + if (hasSingleConnectionString) { + // Normalize singular properties into the lists. connectionStrings.add(configStore.getConnectionString()); - } - - if (endpoints.size() == 0 && StringUtils.hasText(configStore.getEndpoint())) { + } else if (endpoints.isEmpty() && StringUtils.hasText(configStore.getEndpoint())) { + // Single endpoint is the recommended Entra ID connection method. When the plural + // endpoints list is populated, validateAndInit already sets endpoint = endpoints[0], + // so we must not add it again. endpoints.add(configStore.getEndpoint()); } - if (connectionStrings.size() > 0) { - for (String connectionString : connectionStrings) { - clientFactory.setConnectionStringProvider(new ConnectionStringConnector(connectionString)); - String endpoint = getEndpointFromConnectionString(connectionString); - LOGGER.debug("Connecting to " + endpoint + " using Connecting String."); - ConfigurationClientBuilder builder = createBuilderInstance().connectionString(connectionString); + List clients; - clients.add( - modifyAndBuildClient(builder, endpoint, configStore.getEndpoint(), connectionStrings.size() - 1)); - } + if (!connectionStrings.isEmpty()) { + clients = buildClientsFromConnectionStrings(connectionStrings, configStore.getEndpoint()); } else { - DefaultAzureCredential defautAzureCredential = new DefaultAzureCredentialBuilder().build(); - for (String endpoint : endpoints) { - ConfigurationClientBuilder builder = this.createBuilderInstance(); - if (!credentialConfigured) { - builder.credential(defautAzureCredential); - } - - builder.endpoint(endpoint); - - clients.add(modifyAndBuildClient(builder, endpoint, configStore.getEndpoint(), endpoints.size() - 1)); - } + clients = buildClientsFromEndpoints(endpoints, configStore.getEndpoint()); } if (configStore.isLoadBalancingEnabled()) { @@ -181,28 +165,84 @@ List buildClients(ConfigStore configStore) { return clients; } + /** + * Builds a single client for an auto-failover endpoint discovered via DNS SRV. + * Reuses the auth method from the original ConfigStore but targets the failover endpoint. + * + * @param failoverEndpoint the failover endpoint to connect to + * @param configStore the config store providing auth credentials + * @return a replica client targeting the failover endpoint + */ AppConfigurationReplicaClient buildClient(String failoverEndpoint, ConfigStore configStore) { - if (StringUtils.hasText(configStore.getConnectionString())) { - ConnectionString connectionString = new ConnectionString(configStore.getConnectionString()); - connectionString.setUri(failoverEndpoint); - ConfigurationClientBuilder builder = createBuilderInstance().connectionString(connectionString.toString()); - return modifyAndBuildClient(builder, failoverEndpoint, configStore.getEndpoint(), 0); - } else if (configStore.getConnectionStrings().size() > 0) { - ConnectionString connectionString = new ConnectionString(configStore.getConnectionStrings().get(0)); - connectionString.setUri(failoverEndpoint); - ConfigurationClientBuilder builder = createBuilderInstance().connectionString(connectionString.toString()); - return modifyAndBuildClient(builder, failoverEndpoint, configStore.getEndpoint(), 0); + return buildClientFromConnectionString(configStore.getConnectionString(), failoverEndpoint, + configStore.getEndpoint()); + } else if (!configStore.getConnectionStrings().isEmpty()) { + return buildClientFromConnectionString(configStore.getConnectionStrings().get(0), failoverEndpoint, + configStore.getEndpoint()); } else { - ConfigurationClientBuilder builder = createBuilderInstance(); - if (!credentialConfigured) { - builder.credential(new DefaultAzureCredentialBuilder().build()); - } - builder.endpoint(failoverEndpoint); - return modifyAndBuildClient(builder, failoverEndpoint, configStore.getEndpoint(), 0); + return buildClientFromEndpoint(failoverEndpoint, configStore.getEndpoint(), null); } } + private List buildClientsFromConnectionStrings(List connectionStrings, + String originEndpoint) { + List clients = new ArrayList<>(); + for (String connectionString : connectionStrings) { + clientFactory.setConnectionStringProvider(new ConnectionStringConnector(connectionString)); + String endpoint = getEndpointFromConnectionString(connectionString); + LOGGER.debug("Connecting to {} using Connection String.", endpoint); + ConfigurationClientBuilder builder = createBuilderInstance().connectionString(connectionString); + clients.add( + modifyAndBuildClient(builder, endpoint, originEndpoint, connectionStrings.size() - 1)); + } + return clients; + } + + private List buildClientsFromEndpoints(List endpoints, + String originEndpoint) { + List clients = new ArrayList<>(); + // When credentialConfigured is true, the credential is already set on the builder via + // ConfigurationClientBuilderFactory (Spring Cloud Azure auto-config). Otherwise, create a + // shared DefaultAzureCredential to avoid the cost of building one per endpoint. + DefaultAzureCredential defaultAzureCredential = credentialConfigured ? null + : new DefaultAzureCredentialBuilder().build(); + for (String endpoint : endpoints) { + clients.add(buildClientFromEndpoint(endpoint, originEndpoint, defaultAzureCredential)); + } + return clients; + } + + private AppConfigurationReplicaClient buildClientFromEndpoint(String endpoint, String originEndpoint, + DefaultAzureCredential sharedCredential) { + ConfigurationClientBuilder builder = createBuilderInstance(); + // When credentialConfigured is false, no credential was provided via Spring Cloud Azure + // auto-config, so we fall back to DefaultAzureCredential. Prefer the shared instance + // when available; create a new one only for single-endpoint failover calls (null passed in). + if (!credentialConfigured) { + builder.credential(sharedCredential != null ? sharedCredential + : new DefaultAzureCredentialBuilder().build()); + } + builder.endpoint(endpoint); + return modifyAndBuildClient(builder, endpoint, originEndpoint, 0); + } + + /** + * For failover: parses the original connection string, swaps the endpoint to the failover + * target, and rebuilds. This preserves the original Id/Secret credentials. + */ + private AppConfigurationReplicaClient buildClientFromConnectionString(String connectionString, + String failoverEndpoint, String originEndpoint) { + ConnectionString connStr = new ConnectionString(connectionString); + connStr.setUri(failoverEndpoint); + ConfigurationClientBuilder builder = createBuilderInstance().connectionString(connStr.toFullString()); + return modifyAndBuildClient(builder, failoverEndpoint, originEndpoint, 0); + } + + /** + * Final assembly: adds the tracing/telemetry HTTP policy, applies any user-provided customizer, + * then builds the ConfigurationClient and wraps it in a replica-aware client. + */ private AppConfigurationReplicaClient modifyAndBuildClient(ConfigurationClientBuilder builder, String endpoint, String originEndpoint, Integer replicaCount) { @@ -216,15 +256,20 @@ private AppConfigurationReplicaClient modifyAndBuildClient(ConfigurationClientBu return new AppConfigurationReplicaClient(endpoint, originEndpoint, builder.buildClient()); } + /** + * Creates a ConfigurationClientBuilder with retry policy configured from system properties. + * Properties are checked at two levels: service-specific (spring.cloud.azure.appconfiguration) + * takes precedence over global (spring.cloud.azure). Defaults to exponential backoff. + */ private ConfigurationClientBuilder createBuilderInstance() { - RetryStrategy retryStatagy = null; + RetryStrategy retryStrategy = null; String mode = System.getProperty(AzureGlobalProperties.PREFIX + "." + RETRY_MODE_PROPERTY_NAME); String modeService = System .getProperty(AzureAppConfigurationProperties.PREFIX + "." + RETRY_MODE_PROPERTY_NAME); if ("exponential".equals(mode) || "exponential".equals(modeService) || (mode == null && modeService == null)) { - Function checkPropertyInt = parameter -> (Integer.parseInt(parameter)); + Function checkPropertyInt = Integer::parseInt; Function checkPropertyDuration = parameter -> (DurationStyle.detectAndParse(parameter)); int retries = checkProperty(MAX_RETRIES_PROPERTY_NAME, defaultMaxRetries, @@ -235,18 +280,23 @@ private ConfigurationClientBuilder createBuilderInstance() { Duration maxDelay = checkProperty(MAX_DELAY_PROPERTY_NAME, DEFAULT_MAX_RETRY_POLICY, " isn't a valid Duration, using default value.", checkPropertyDuration); - retryStatagy = new ExponentialBackoff(retries, baseDelay, maxDelay); + retryStrategy = new ExponentialBackoff(retries, baseDelay, maxDelay); } ConfigurationClientBuilder builder = clientFactory.build(); - if (retryStatagy != null) { - builder.retryPolicy(new RetryPolicy(retryStatagy)); + if (retryStrategy != null) { + builder.retryPolicy(new RetryPolicy(retryStrategy)); } return builder; } + /** + * Reads a retry-related property from system properties. Service-specific properties + * ({@code spring.cloud.azure.appconfiguration.{name}}) override global ones ({@code spring.cloud.azure.{name}}). + * Falls back to defaultValue if the property is missing or fails to parse. + */ private T checkProperty(String propertyName, T defaultValue, String errMsg, Function fn) { String envValue = System.getProperty(AzureGlobalProperties.PREFIX + "." + propertyName); String envServiceValue = System.getProperty(AzureAppConfigurationProperties.PREFIX + "." + propertyName); @@ -269,6 +319,11 @@ private T checkProperty(String propertyName, T defaultValue, String errMsg, return value; } + /** + * Adapter to provide a connection string to Spring Cloud Azure's auto-configured + * client builders, enabling other Spring Cloud Azure integrations (e.g., Key Vault + * secret resolution) to authenticate to App Configuration. + */ private static class ConnectionStringConnector implements ServiceConnectionStringProvider { @@ -289,7 +344,13 @@ public AppConfiguration getServiceType() { } } - private static class ConnectionString { + /** + * Parses an App Configuration connection string into its components (Endpoint, Id, Secret). + * Used for auto-failover: {@link #setUri(String)} swaps the endpoint while preserving the original + * credentials, and {@link #toFullString()} reassembles the connection string for the SDK. + * {@link #toString()} always redacts the secret for safe logging. + */ + static class ConnectionString { private static final String ENDPOINT = "Endpoint="; private static final String ID = "Id="; @@ -310,48 +371,56 @@ private static class ConnectionString { if (args.length < 3) { throw new IllegalArgumentException("invalid connection string segment count"); } - URL baseUri = null; - String id = null; - String secret = null; + URL parsedUri = null; + String parsedId = null; + String parsedSecret = null; for (String arg : args) { String segment = arg.trim(); if (ENDPOINT.regionMatches(true, 0, segment, 0, ENDPOINT.length())) { try { - baseUri = new URI(segment.substring(ENDPOINT.length())).toURL(); - } catch (MalformedURLException ex) { - throw new IllegalArgumentException(ex); - } catch (URISyntaxException ex) { + parsedUri = new URI(segment.substring(ENDPOINT.length())).toURL(); + } catch (MalformedURLException | URISyntaxException ex) { throw new IllegalArgumentException(ex); } } else if (ID.regionMatches(true, 0, segment, 0, ID.length())) { - id = segment.substring(ID.length()); + parsedId = segment.substring(ID.length()); } else if (SECRET_PREFIX.regionMatches(true, 0, segment, 0, SECRET_PREFIX.length())) { - secret = segment.substring(SECRET_PREFIX.length()); + parsedSecret = segment.substring(SECRET_PREFIX.length()); } } - this.baseUri = baseUri; - this.id = id; - this.secret = secret; + this.baseUri = parsedUri; + this.id = parsedId; + this.secret = parsedSecret; if (this.baseUri == null || CoreUtils.isNullOrEmpty(this.id) || this.secret == null || this.secret.length() == 0) { throw new IllegalArgumentException("Could not parse 'connectionString'." - + " Expected format: 'endpoint={endpoint};id={id};secret={secret}'. Actual:" + connectionString); + + " Expected format: 'endpoint={endpoint};id={id};secret={secret}'."); } } protected ConnectionString setUri(String uri) { try { this.baseUri = new URI(uri).toURL(); - } catch (MalformedURLException ex) { - throw new IllegalArgumentException(ex); - } catch (URISyntaxException ex) { + } catch (MalformedURLException | URISyntaxException ex) { throw new IllegalArgumentException(ex); } return this; } - public String toString() { + /** + * Returns the full connection string including the secret. Use only when passing + * to the SDK's {@link ConfigurationClientBuilder#connectionString(String)}. + */ + String toFullString() { return String.format("%s%s;%s%s;%s%s", ENDPOINT, baseUri, ID, id, SECRET_PREFIX, secret); } + + /** + * Returns the connection string with the secret redacted. Safe for logging and error messages. + */ + @Override + public String toString() { + return String.format("%s%s;%s%s;%s", ENDPOINT, baseUri, ID, id, SECRET_PREFIX); + } } } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientBuilderTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientBuilderTest.java index 3b95e77724f2..e24f4df1aa94 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientBuilderTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientBuilderTest.java @@ -2,27 +2,22 @@ // Licensed under the MIT License. package com.azure.spring.cloud.appconfiguration.config.implementation; -import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_CONN_STRING; -import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_CONN_STRING_GEO; -import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_ENDPOINT; -import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_ENDPOINT_GEO; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - import java.time.Instant; import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.AfterEach; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.mockito.Mockito; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import org.mockito.MockitoAnnotations; import org.mockito.MockitoSession; import org.mockito.quality.Strictness; @@ -31,6 +26,10 @@ import com.azure.core.credential.TokenCredential; import com.azure.data.appconfiguration.ConfigurationClientBuilder; import com.azure.spring.cloud.appconfiguration.config.ConfigurationClientCustomizer; +import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_CONN_STRING; +import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_CONN_STRING_GEO; +import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_ENDPOINT; +import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_ENDPOINT_GEO; import com.azure.spring.cloud.appconfiguration.config.implementation.properties.ConfigStore; import com.azure.spring.cloud.service.implementation.appconfiguration.ConfigurationClientBuilderFactory; @@ -294,7 +293,7 @@ public void buildClientConnectionStringInvalid3Test() { () -> spy.buildClient("fake.test.config.io", configStore)); assertEquals( - "Could not parse 'connectionString'. Expected format: 'endpoint={endpoint};id={id};secret={secret}'. Actual:Not;A;Connection String", + "Could not parse 'connectionString'. Expected format: 'endpoint={endpoint};id={id};secret={secret}'.", exception.getMessage()); } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/ConfigStoreTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/ConfigStoreTest.java index 5ba5cc12629c..af2d802efa4e 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/ConfigStoreTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/ConfigStoreTest.java @@ -7,7 +7,9 @@ import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.Test; import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationKeyValueSelector; From c36e8aa55fe93567e843d656a7e8171e5fdf1ccb Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 1 May 2026 16:13:56 -0700 Subject: [PATCH 5/6] Update AppConfigurationReplicaClientBuilderTest.java --- ...ConfigurationReplicaClientBuilderTest.java | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientBuilderTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientBuilderTest.java index e24f4df1aa94..d0a255d2724f 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientBuilderTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientBuilderTest.java @@ -2,22 +2,27 @@ // Licensed under the MIT License. package com.azure.spring.cloud.appconfiguration.config.implementation; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_CONN_STRING; +import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_CONN_STRING_GEO; +import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_ENDPOINT; +import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_ENDPOINT_GEO; + import java.time.Instant; import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.AfterEach; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.mockito.Mockito; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import org.mockito.MockitoAnnotations; import org.mockito.MockitoSession; import org.mockito.quality.Strictness; @@ -26,10 +31,6 @@ import com.azure.core.credential.TokenCredential; import com.azure.data.appconfiguration.ConfigurationClientBuilder; import com.azure.spring.cloud.appconfiguration.config.ConfigurationClientCustomizer; -import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_CONN_STRING; -import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_CONN_STRING_GEO; -import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_ENDPOINT; -import static com.azure.spring.cloud.appconfiguration.config.implementation.TestConstants.TEST_ENDPOINT_GEO; import com.azure.spring.cloud.appconfiguration.config.implementation.properties.ConfigStore; import com.azure.spring.cloud.service.implementation.appconfiguration.ConfigurationClientBuilderFactory; @@ -50,7 +51,7 @@ public class AppConfigurationReplicaClientBuilderTest { @Mock private ConfigurationClientBuilderFactory clientFactoryMock; - + @Mock private Environment envMock; @@ -176,7 +177,7 @@ public void buildClientsFromMultipleConnectionStringsTest() { clientBuilder = new AppConfigurationReplicaClientsBuilder(clientFactoryMock, null, false, false); AppConfigurationReplicaClientsBuilder spy = Mockito.spy(clientBuilder); - + when(builderMock.addPolicy(Mockito.any())).thenReturn(builderMock); when(clientFactoryMock.build()).thenReturn(builderMock); when(builderMock.connectionString(Mockito.anyString())).thenReturn(builderMock); @@ -200,7 +201,7 @@ public void endpointAndConnectionString() { clientBuilder = new AppConfigurationReplicaClientsBuilder(clientFactoryMock, null, false, false); String message = assertThrows(IllegalArgumentException.class, - () -> clientBuilder.buildClients(configStore).get(0)).getMessage(); + () -> clientBuilder.buildClients(configStore).get(0)).getMessage(); assertEquals("More than 1 connection method was set for connecting to App Configuration.", message); } @@ -266,7 +267,7 @@ public void buildClientConnectionStringInvalidTest() { AppConfigurationReplicaClientsBuilder spy = Mockito.spy(clientBuilder); IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, - () -> spy.buildClient("fake.test.config.io", configStore)); + () -> spy.buildClient("fake.test.config.io", configStore)); assertEquals("URI is not absolute", exception.getMessage()); } @@ -278,7 +279,7 @@ public void buildClientConnectionStringInvalid2Test() { AppConfigurationReplicaClientsBuilder spy = Mockito.spy(clientBuilder); IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, - () -> spy.buildClient("fake.test.config.io", configStore)); + () -> spy.buildClient("fake.test.config.io", configStore)); assertEquals("invalid connection string segment count", exception.getMessage()); } @@ -290,11 +291,11 @@ public void buildClientConnectionStringInvalid3Test() { AppConfigurationReplicaClientsBuilder spy = Mockito.spy(clientBuilder); IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, - () -> spy.buildClient("fake.test.config.io", configStore)); + () -> spy.buildClient("fake.test.config.io", configStore)); assertEquals( - "Could not parse 'connectionString'. Expected format: 'endpoint={endpoint};id={id};secret={secret}'.", - exception.getMessage()); + "Could not parse 'connectionString'. Expected format: 'endpoint={endpoint};id={id};secret={secret}'.", + exception.getMessage()); } @Test From a8f2f9f9a9154be3a04432752ba84d8d4b7c3c83 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 1 May 2026 16:59:33 -0700 Subject: [PATCH 6/6] More review items --- .../implementation/AzureAppConfigBootstrapRegistrar.java | 3 --- .../config/implementation/FeatureFlagClient.java | 7 ++++++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigBootstrapRegistrar.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigBootstrapRegistrar.java index c41ea1b21511..2f17a9d8232e 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigBootstrapRegistrar.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigBootstrapRegistrar.java @@ -109,9 +109,6 @@ private static AppConfigurationReplicaClientsBuilder replicaClientBuilder(Config } return factory; }); - if (customizer != null) { - clientFactory.addBuilderCustomizer(customizer.get(context.getBootstrapContext())); - } InstanceSupplier configurationClientCustomizer = context .getBootstrapContext() diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClient.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClient.java index 3b30f46bcdef..d8f5eee54c8e 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClient.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClient.java @@ -113,7 +113,12 @@ WatchedConfigurationSettings processFeatureFlags(WatchedConfigurationSettings fe && FEATURE_FLAG_CONTENT_TYPE.equals(setting.getContentType())) { FeatureFlagConfigurationSetting featureFlag = (FeatureFlagConfigurationSetting) setting; updateTelemetry(featureFlag); - properties.put(featureFlag.getKey(), createFeature(featureFlag, endpoint)); + Feature feature = createFeature(featureFlag, endpoint); + if (feature != null) { + properties.put(featureFlag.getKey(), feature); + } else { + LOGGER.warn("Skipping invalid feature flag: {}", featureFlag.getKey()); + } } } return features;