diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/AppConfigurationWatchAutoConfiguration.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/AppConfigurationWatchAutoConfiguration.java index c4340ad2884c..69afe5a98bed 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/AppConfigurationWatchAutoConfiguration.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/AppConfigurationWatchAutoConfiguration.java @@ -2,11 +2,11 @@ // Licensed under the MIT License. package com.azure.spring.cloud.appconfiguration.config; -import org.springframework.boot.bootstrap.BootstrapContext; import org.springframework.boot.autoconfigure.AutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.bootstrap.BootstrapContext; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.endpoint.RefreshEndpoint; import org.springframework.context.annotation.Bean; @@ -15,6 +15,7 @@ import com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationPullRefresh; import com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationRefreshUtil; import com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationReplicaClientFactory; +import com.azure.spring.cloud.appconfiguration.config.implementation.StateHolder; import com.azure.spring.cloud.appconfiguration.config.implementation.autofailover.ReplicaLookUp; import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationProperties; @@ -37,15 +38,16 @@ public AppConfigurationWatchAutoConfiguration() { @Bean @ConditionalOnMissingBean AppConfigurationRefresh appConfigurationRefresh(AppConfigurationProperties properties, BootstrapContext context) { - AppConfigurationReplicaClientFactory clientFactory = context - .getOrElse(AppConfigurationReplicaClientFactory.class, null); + AppConfigurationReplicaClientFactory clientFactory = context.getOrElse(AppConfigurationReplicaClientFactory.class, null); ReplicaLookUp replicaLookUp = context.getOrElse(ReplicaLookUp.class, null); + StateHolder stateHolder = context.get(StateHolder.class); + if (clientFactory == null || replicaLookUp == null) { return null; } return new AppConfigurationPullRefresh(clientFactory, properties.getRefreshInterval(), replicaLookUp, - new AppConfigurationRefreshUtil()); + stateHolder, new AppConfigurationRefreshUtil(stateHolder)); } } 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 dd0c70cf3c8e..4fcc931cb93e 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 @@ -65,6 +65,7 @@ class AppConfigurationApplicationSettingPropertySource extends AppConfigurationP * @param keyPrefixTrimValues prefixs to trim from key values * @throws InvalidConfigurationPropertyValueException thrown if fails to parse Json content type */ + @Override public void initProperties(List keyPrefixTrimValues, Context context) throws InvalidConfigurationPropertyValueException { List labels = Arrays.asList(labelFilters); @@ -136,7 +137,6 @@ private void handleKeyVaultReference(String key, SecretReferenceConfigurationSet void handleFeatureFlag(String key, FeatureFlagConfigurationSetting setting, List trimStrings) throws InvalidConfigurationPropertyValueException { // Feature Flags aren't loaded as configuration, but are loaded as feature flags when loading a snapshot. - return; } private void handleJson(ConfigurationSetting setting, List keyPrefixTrimValues) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPullRefresh.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPullRefresh.java index cb5d5735d39b..c068f851a0b8 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPullRefresh.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPullRefresh.java @@ -4,6 +4,7 @@ import java.time.Duration; import java.util.Map; +import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import org.slf4j.Logger; @@ -36,7 +37,6 @@ public class AppConfigurationPullRefresh implements AppConfigurationRefresh { * Publisher for Spring refresh events. */ private ApplicationEventPublisher publisher; - private final Long defaultMinBackoff = (long) 30; /** * Default minimum backoff duration in seconds when refresh operations fail. @@ -63,19 +63,30 @@ public class AppConfigurationPullRefresh implements AppConfigurationRefresh { */ private final AppConfigurationRefreshUtil refreshUtils; + /** + * Holds configuration state between refreshes. + */ + private final StateHolder stateHolder; + /** * Creates a new AppConfigurationPullRefresh component. * * @param clientFactory factory for creating App Configuration clients to connect to stores * @param refreshInterval time duration between refresh interval checks * @param replicaLookUp component for handling replica lookup and failover + * @param stateHolder holds configuration state between refreshes * @param refreshUtils utility component for refresh operations */ public AppConfigurationPullRefresh(AppConfigurationReplicaClientFactory clientFactory, Duration refreshInterval, - ReplicaLookUp replicaLookUp, AppConfigurationRefreshUtil refreshUtils) { + ReplicaLookUp replicaLookUp, StateHolder stateHolder, AppConfigurationRefreshUtil refreshUtils) { this.refreshInterval = refreshInterval; this.clientFactory = clientFactory; this.replicaLookUp = replicaLookUp; + if (Objects.isNull(stateHolder)) { + // StateHolder is null if all stores are disabled. + stateHolder = new StateHolder(); + } + this.stateHolder = stateHolder; this.refreshUtils = refreshUtils; } @@ -96,6 +107,7 @@ public void setApplicationEventPublisher(ApplicationEventPublisher applicationEv * @return a Mono containing a boolean indicating if a RefreshEvent was published. Returns {@code false} if * refreshConfigurations is currently being executed elsewhere. */ + @Override public Mono refreshConfigurations() { return Mono.just(refreshStores()); } @@ -107,6 +119,7 @@ public Mono refreshConfigurations() { * @param endpoint the Config Store endpoint to expire refresh interval on * @param syncToken the syncToken to verify the latest changes are available on pull */ + @Override public void expireRefreshInterval(String endpoint, String syncToken) { LOGGER.debug("Expiring refresh interval for " + endpoint); @@ -114,7 +127,7 @@ public void expireRefreshInterval(String endpoint, String syncToken) { clientFactory.updateSyncToken(originEndpoint, endpoint, syncToken); - StateHolder.getCurrentState().expireState(originEndpoint); + stateHolder.expireState(originEndpoint); } /** @@ -135,7 +148,7 @@ private boolean refreshStores() { } catch (Exception e) { LOGGER.warn("Error occurred during configuration refresh, will retry at next interval", e); // The next refresh will happen sooner if refresh interval is expired. - StateHolder.getCurrentState().updateNextRefreshTime(refreshInterval, DEFAULT_MIN_BACKOFF_SECONDS); + stateHolder.updateNextRefreshTime(refreshInterval, DEFAULT_MIN_BACKOFF_SECONDS); throw e; } finally { running.set(false); 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 e17572efa6a9..abec63fa1ba8 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 @@ -32,6 +32,21 @@ public class AppConfigurationRefreshUtil { private static final String FEATURE_FLAG_PREFIX = ".appconfig.featureflag/*"; + private final StateHolder stateHolder; + + /** + * Creates a new AppConfigurationRefreshUtil with the specified state holder. + * + * @param stateHolder the state holder for managing configuration and feature flag states + */ + public AppConfigurationRefreshUtil(StateHolder stateHolder) { + if (stateHolder == null) { + // This is a fallback if all stores are disabled. + stateHolder = new StateHolder(); + } + this.stateHolder = stateHolder; + } + /** * Functional interface for refresh operations that can throw AppConfigurationStatusException. */ @@ -56,8 +71,8 @@ RefreshEventData refreshStoresCheck(AppConfigurationReplicaClientFactory clientF RefreshEventData eventData = new RefreshEventData(); try { - if (refreshInterval != null && StateHolder.getNextForcedRefresh() != null - && Instant.now().isAfter(StateHolder.getNextForcedRefresh())) { + if (refreshInterval != null && stateHolder.getNextForcedRefresh() != null + && Instant.now().isAfter(stateHolder.getNextForcedRefresh())) { String eventDataInfo = "Minimum refresh period reached. Refreshing configurations."; LOGGER.info(eventDataInfo); @@ -84,12 +99,20 @@ RefreshEventData refreshStoresCheck(AppConfigurationReplicaClientFactory clientF clientFactory.findActiveClients(originEndpoint); - if (monitor.isEnabled() && StateHolder.getLoadState(originEndpoint)) { + if (monitor.isEnabled() && stateHolder.getLoadState(originEndpoint)) { RefreshEventData result = executeRefreshWithRetry( clientFactory, originEndpoint, - (client, data, ctx) -> refreshWithTime(client, StateHolder.getState(originEndpoint), - monitor.getRefreshInterval(), data, replicaLookUp, ctx), + (client, data, ctx) -> { + if (stateHolder.getState(originEndpoint) == null) { + LOGGER.debug( + "Skipping configuration refresh check for {} because monitoring state is not initialized.", + originEndpoint); + return; + } + refreshWithTime(client, stateHolder.getState(originEndpoint), + monitor.getRefreshInterval(), data, replicaLookUp, ctx); + }, eventData, context, "configuration refresh check", @@ -103,12 +126,12 @@ RefreshEventData refreshStoresCheck(AppConfigurationReplicaClientFactory clientF FeatureFlagStore featureStore = connection.getFeatureFlagStore(); - if (featureStore.getEnabled() && StateHolder.getStateFeatureFlag(originEndpoint) != null) { + if (featureStore.getEnabled() && stateHolder.getStateFeatureFlag(originEndpoint) != null) { RefreshEventData result = executeRefreshWithRetry( clientFactory, originEndpoint, (client, data, ctx) -> refreshWithTimeFeatureFlags(client, - StateHolder.getStateFeatureFlag(originEndpoint), + stateHolder.getStateFeatureFlag(originEndpoint), monitor.getFeatureFlagRefreshInterval(), data, replicaLookUp, ctx), eventData, context, @@ -124,7 +147,7 @@ RefreshEventData refreshStoresCheck(AppConfigurationReplicaClientFactory clientF } } catch (Exception e) { // The next refresh will happen sooner if refresh interval is expired. - StateHolder.getCurrentState().updateNextRefreshTime(refreshInterval, defaultMinBackoff); + stateHolder.updateNextRefreshTime(refreshInterval, defaultMinBackoff); throw e; } return eventData; @@ -179,12 +202,20 @@ private RefreshEventData executeRefreshWithRetry( * @param client the client for checking refresh status * @param originEndpoint the original config store endpoint * @param context the operation context + * @param stateHolder the state holder instance * @return true if a refresh should be triggered, false otherwise */ - static boolean refreshStoreCheck(AppConfigurationReplicaClient client, String originEndpoint, Context context) { + static boolean refreshStoreCheck(AppConfigurationReplicaClient client, String originEndpoint, Context context, + StateHolder stateHolder) { RefreshEventData eventData = new RefreshEventData(); - if (StateHolder.getLoadState(originEndpoint)) { - refreshWithoutTime(client, StateHolder.getState(originEndpoint).getWatchKeys(), eventData, context); + if (stateHolder.getLoadState(originEndpoint)) { + State state = stateHolder.getState(originEndpoint); + if (state != null) { + refreshWithoutTime(client, state.getWatchKeys(), eventData, context); + } else { + LOGGER.debug("Skipping configuration refresh check for {} as no watched state is available", + originEndpoint); + } } return eventData.getDoRefresh(); } @@ -198,13 +229,13 @@ static boolean refreshStoreCheck(AppConfigurationReplicaClient client, String or * @param context the operation context * @return true if a refresh should be triggered, false otherwise */ - static boolean refreshStoreFeatureFlagCheck(Boolean featureStoreEnabled, + boolean refreshStoreFeatureFlagCheck(Boolean featureStoreEnabled, AppConfigurationReplicaClient client, Context context) { RefreshEventData eventData = new RefreshEventData(); String endpoint = client.getEndpoint(); - if (featureStoreEnabled && StateHolder.getStateFeatureFlag(endpoint) != null) { - refreshWithoutTimeFeatureFlags(client, StateHolder.getStateFeatureFlag(endpoint), eventData, context); + if (featureStoreEnabled && stateHolder.getStateFeatureFlag(endpoint) != null) { + refreshWithoutTimeFeatureFlags(client, stateHolder.getStateFeatureFlag(endpoint), eventData, context); } else { LOGGER.debug("Skipping feature flag refresh check for {}", endpoint); } @@ -223,7 +254,7 @@ static boolean refreshStoreFeatureFlagCheck(Boolean featureStoreEnabled, * @param context the operation context * @throws AppConfigurationStatusException if there's an error during the refresh check */ - private static void refreshWithTime(AppConfigurationReplicaClient client, State state, Duration refreshInterval, + private void refreshWithTime(AppConfigurationReplicaClient client, State state, Duration refreshInterval, RefreshEventData eventData, ReplicaLookUp replicaLookUp, Context context) throws AppConfigurationStatusException { if (Instant.now().isAfter(state.getNextRefreshCheck())) { @@ -238,7 +269,7 @@ private static void refreshWithTime(AppConfigurationReplicaClient client, State refreshWithoutTime(client, state.getWatchKeys(), eventData, context); } - StateHolder.getCurrentState().updateStateRefresh(state, refreshInterval); + stateHolder.updateStateRefresh(state, refreshInterval); } } @@ -308,7 +339,7 @@ private static void refreshWithoutTimeWatchedConfigurationSettings(AppConfigurat * @param context the operation context * @throws AppConfigurationStatusException if there's an error during the refresh check */ - private static void refreshWithTimeFeatureFlags(AppConfigurationReplicaClient client, FeatureFlagState state, + private void refreshWithTimeFeatureFlags(AppConfigurationReplicaClient client, FeatureFlagState state, Duration refreshInterval, RefreshEventData eventData, ReplicaLookUp replicaLookUp, Context context) throws AppConfigurationStatusException { Instant date = Instant.now(); @@ -327,7 +358,7 @@ private static void refreshWithTimeFeatureFlags(AppConfigurationReplicaClient cl } - StateHolder.getCurrentState().updateFeatureFlagStateRefresh(state, refreshInterval); + stateHolder.updateFeatureFlagStateRefresh(state, refreshInterval); } } 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 049e0ed65546..edb761e0e737 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 @@ -249,7 +249,7 @@ boolean checkWatchKeys(SettingSelector settingSelector, Context context) { List> results = client .listConfigurationSettings(settingSelector, context) .streamByPage().filter(pagedResponse -> pagedResponse.getStatusCode() != HTTP_NOT_MODIFIED).toList(); - return results.size() > 0; + return !results.isEmpty(); } /** diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigBoostrapRegistrar.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigBootstrapRegistrar.java similarity index 91% rename from sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigBoostrapRegistrar.java rename to sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigBootstrapRegistrar.java index 9f414c0ca37f..c41ea1b21511 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigBoostrapRegistrar.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigBootstrapRegistrar.java @@ -6,6 +6,7 @@ import org.springframework.boot.context.config.ConfigDataLocationResolverContext; import org.springframework.boot.context.properties.bind.Bindable; import org.springframework.boot.context.properties.bind.Binder; +import org.springframework.context.ConfigurableApplicationContext; import org.springframework.util.StringUtils; import com.azure.data.appconfiguration.ConfigurationClientBuilder; @@ -51,6 +52,17 @@ static void register(ConfigDataLocationResolverContext context, Binder binder, InstanceSupplier.from(() -> keyVaultClientFactory)); context.getBootstrapContext().registerIfAbsent(AppConfigurationReplicaClientFactory.class, InstanceSupplier.from(() -> buildClientFactory(replicaClientsBuilder, properties, replicaLookup))); + + // Register StateHolder and promote it to ApplicationContext on close + context.getBootstrapContext().registerIfAbsent(StateHolder.class, + InstanceSupplier.from(StateHolder::new)); + context.getBootstrapContext().addCloseListener(event -> { + StateHolder stateHolder = event.getBootstrapContext().get(StateHolder.class); + ConfigurableApplicationContext applicationContext = event.getApplicationContext(); + if (!applicationContext.getBeanFactory().containsBean("appConfigurationStateHolder")) { + applicationContext.getBeanFactory().registerSingleton("appConfigurationStateHolder", stateHolder); + } + }); } private static AppConfigurationKeyVaultClientFactory appConfigurationKeyVaultClientFactory( 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 566d7a59f4d2..109321c06ac8 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 @@ -11,8 +11,8 @@ import java.util.List; import org.apache.commons.logging.Log; -import org.springframework.boot.context.config.ConfigData; import org.springframework.boot.bootstrap.BootstrapRegistry.InstanceSupplier; +import org.springframework.boot.context.config.ConfigData; import org.springframework.boot.context.config.ConfigDataLoader; import org.springframework.boot.context.config.ConfigDataLoaderContext; import org.springframework.boot.context.config.ConfigDataResourceNotFoundException; @@ -61,7 +61,7 @@ public class AzureAppConfigDataLoader implements ConfigDataLoader watchedConfigurationSettingsList = getWatchedConfigurationSettings( currentClient); @@ -216,8 +220,7 @@ public ConfigData load(ConfigDataLoaderContext context, AzureAppConfigDataResour } } - StateHolder.updateState(storeState); - if (featureFlagClient.getFeatureFlags().size() > 0) { + if (!featureFlagClient.getFeatureFlags().isEmpty()) { // Don't add feature flags if there are none, otherwise the local file can't load them. sourceList.add(new AppConfigurationFeatureManagementPropertySource(featureFlagClient)); } @@ -251,7 +254,7 @@ private List createSettings(AppConfigurationRepl List profiles = resource.getProfiles().getActive(); for (AppConfigurationKeyValueSelector selectedKeys : selects) { - AppConfigurationPropertySource propertySource = null; + AppConfigurationPropertySource propertySource; if (StringUtils.hasText(selectedKeys.getSnapshotName())) { propertySource = new AppConfigurationSnapshotPropertySource( 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 4b7f6f2c1ca1..8f71b0e9e853 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 @@ -169,7 +169,7 @@ public List getAvailableClients() { if (clients.get(0).getBackoffEndTime().isBefore(Instant.now())) { availableClients.add(clients.get(0)); } - } else if (clients.size() > 0 && !configStore.isLoadBalancingEnabled()) { + } else if (!clients.isEmpty() && !configStore.isLoadBalancingEnabled()) { for (AppConfigurationReplicaClient replicaClient : clients) { if (replicaClient.getBackoffEndTime().isBefore(Instant.now())) { LOGGER.debug("Using Client: " + replicaClient.getEndpoint()); @@ -184,10 +184,10 @@ public List getAvailableClients() { } } - if (availableClients.size() == 0 || configStore.isLoadBalancingEnabled()) { + if (availableClients.isEmpty() || configStore.isLoadBalancingEnabled()) { List autoFailoverEndpoints = replicaLookUp.getAutoFailoverEndpoints(configStore.getEndpoint()); - if (autoFailoverEndpoints.size() > 0) { + if (!autoFailoverEndpoints.isEmpty()) { for (String failoverEndpoint : autoFailoverEndpoints) { AppConfigurationReplicaClient client = autoFailoverClients.get(failoverEndpoint); if (client == null) { @@ -201,9 +201,9 @@ public List getAvailableClients() { } } } - if (clients.size() > 0 && availableClients.size() == 0) { + if (!clients.isEmpty() && availableClients.isEmpty()) { this.health = AppConfigurationStoreHealth.DOWN; - } else if (clients.size() > 0) { + } else if (!clients.isEmpty()) { this.health = AppConfigurationStoreHealth.UP; } 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 e9ff389e6570..6ec13ae92cd9 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 @@ -15,21 +15,23 @@ import com.azure.spring.cloud.appconfiguration.config.implementation.feature.FeatureFlagState; /** - * Thread-safe singleton holder for managing refresh state of Azure App Configuration stores. + * Thread-safe holder for managing refresh state of Azure App Configuration stores. * *

* Maintains state for configuration settings, feature flags, and refresh intervals across multiple configuration * stores. Implements exponential backoff for failed refresh attempts and coordinates the timing of refresh operations. *

+ * + *

+ * This class is registered in the BootstrapContext during initial configuration loading and promoted to the main + * ApplicationContext for use during refresh operations. + *

*/ -final class StateHolder { +public class StateHolder { /** Maximum jitter in seconds to add when expiring state to prevent thundering herd. */ private static final int MAX_JITTER = 15; - /** The current singleton instance of StateHolder. */ - private static StateHolder currentState; - /** Map of configuration store endpoints to their refresh state. */ private final Map state = new ConcurrentHashMap<>(); @@ -45,25 +47,10 @@ final class StateHolder { /** The next time a forced refresh should occur across all stores. */ private Instant nextForcedRefresh; - StateHolder() { - } - - /** - * Gets the current singleton instance of StateHolder. - * @return the current StateHolder instance, or null if not yet initialized - */ - static StateHolder getCurrentState() { - return currentState; - } - /** - * Updates the singleton instance to a new StateHolder. - * @param newState the new StateHolder instance to set as current - * @return the updated StateHolder instance + * Creates a new StateHolder instance. */ - static StateHolder updateState(StateHolder newState) { - currentState = newState; - return currentState; + public StateHolder() { } /** @@ -71,41 +58,34 @@ static StateHolder updateState(StateHolder newState) { * @param originEndpoint the endpoint for the origin config store * @return the State for the specified store, or null if not found */ - static State getState(String originEndpoint) { - return currentState.getFullState().get(originEndpoint); + public State getState(String originEndpoint) { + return state.get(originEndpoint); } /** - * Gets the full map of configuration store states. - * @return map of endpoint to State - */ - private Map getFullState() { - return state; - } - - /** - * Gets the full map of feature flag states. - * @return map of endpoint to FeatureFlagState + * Retrieves the feature flag refresh state for a specific configuration store. + * @param originEndpoint the endpoint for the origin config store + * @return the FeatureFlagState for the specified store, or null if not found */ - private Map getFullFeatureFlagState() { - return featureFlagState; + public FeatureFlagState getStateFeatureFlag(String originEndpoint) { + return featureFlagState.get(originEndpoint); } /** - * Gets the full map of load states. - * @return map of endpoint to load status + * Checks if a configuration store has been successfully loaded. + * @param originEndpoint the endpoint of the store to check + * @return true if the store has been loaded, false otherwise */ - private Map getFullLoadState() { - return loadState; + public boolean getLoadState(String originEndpoint) { + return loadState.getOrDefault(originEndpoint, false); } /** - * Retrieves the feature flag refresh state for a specific configuration store. - * @param originEndpoint the endpoint for the origin config store - * @return the FeatureFlagState for the specified store, or null if not found + * Gets the next time a forced refresh should occur across all stores. + * @return the Instant of the next forced refresh, or null if not set */ - static FeatureFlagState getStateFeatureFlag(String originEndpoint) { - return currentState.getFullFeatureFlagState().get(originEndpoint); + public Instant getNextForcedRefresh() { + return nextForcedRefresh; } /** @@ -114,7 +94,7 @@ static FeatureFlagState getStateFeatureFlag(String originEndpoint) { * @param watchKeys list of configuration watch keys that can trigger a refresh event * @param duration refresh duration */ - void setState(String originEndpoint, List watchKeys, Duration duration) { + public void setState(String originEndpoint, List watchKeys, Duration duration) { state.put(originEndpoint, new State(watchKeys, Math.toIntExact(duration.getSeconds()), originEndpoint)); } @@ -125,7 +105,7 @@ void setState(String originEndpoint, List watchKeys, Durat * @param collectionWatchKeys list of collection monitoring configurations that can trigger a refresh event * @param duration refresh duration */ - void setState(String originEndpoint, List watchKeys, + public void setState(String originEndpoint, List watchKeys, List collectionWatchKeys, Duration duration) { state.put(originEndpoint, new State(watchKeys, collectionWatchKeys, Math.toIntExact(duration.getSeconds()), originEndpoint)); @@ -137,7 +117,7 @@ void setState(String originEndpoint, List watchKeys, * @param watchKeys list of feature flag watch keys that can trigger a refresh event * @param duration refresh duration */ - void setStateFeatureFlag(String originEndpoint, List watchKeys, + public void setStateFeatureFlag(String originEndpoint, List watchKeys, Duration duration) { featureFlagState.put(originEndpoint, new FeatureFlagState(watchKeys, Math.toIntExact(duration.getSeconds()), originEndpoint)); @@ -158,7 +138,7 @@ void updateStateRefresh(State state, Duration duration) { * @param state the current FeatureFlagState to update * @param duration the duration to add to the current time for the next refresh */ - void updateFeatureFlagStateRefresh(FeatureFlagState state, Duration duration) { + public void updateFeatureFlagStateRefresh(FeatureFlagState state, Duration duration) { this.featureFlagState.put(state.getOriginEndpoint(), new FeatureFlagState(state, Instant.now().plusSeconds(Math.toIntExact(duration.getSeconds())))); } @@ -168,8 +148,11 @@ void updateFeatureFlagStateRefresh(FeatureFlagState state, Duration duration) { * prevent thundering herd when multiple stores refresh simultaneously. * @param originEndpoint the endpoint of the store to expire */ - void expireState(String originEndpoint) { + public void expireState(String originEndpoint) { State oldState = state.get(originEndpoint); + if (oldState == null) { + return; + } long wait = (long) (new SecureRandom().nextDouble() * MAX_JITTER); long timeLeft = (int) ((oldState.getNextRefreshCheck().toEpochMilli() - (Instant.now().toEpochMilli())) / 1000); @@ -178,31 +161,14 @@ void expireState(String originEndpoint) { } } - /** - * Checks if a configuration store has been successfully loaded. - * @param originEndpoint the endpoint of the store to check - * @return true if the store has been loaded, false otherwise - */ - static boolean getLoadState(String originEndpoint) { - return currentState.getFullLoadState().getOrDefault(originEndpoint, false); - } - /** * @param originEndpoint the configuration store connected to. * @param loaded true if the configuration store was loaded. */ - void setLoadState(String originEndpoint, Boolean loaded) { + public void setLoadState(String originEndpoint, Boolean loaded) { loadState.put(originEndpoint, loaded); } - /** - * Gets the next time a forced refresh should occur across all stores. - * @return the Instant of the next forced refresh, or null if not set - */ - public static Instant getNextForcedRefresh() { - return currentState.nextForcedRefresh; - } - /** * Sets the next forced refresh time. Called after a successful load or refresh. * @param refreshPeriod the duration from now until the next forced refresh; if null, no refresh is scheduled @@ -220,7 +186,7 @@ public void setNextForcedRefresh(Duration refreshPeriod) { * @param refreshInterval period between refresh checks * @param defaultMinBackoff minimum backoff duration between checks in seconds */ - void updateNextRefreshTime(Duration refreshInterval, Long defaultMinBackoff) { + public void updateNextRefreshTime(Duration refreshInterval, Long defaultMinBackoff) { if (refreshInterval != null) { Instant newForcedRefresh = getNextRefreshCheck(nextForcedRefresh, clientRefreshAttempts, refreshInterval.getSeconds(), defaultMinBackoff); 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 6516c9c08857..8b4fc41e0829 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 @@ -106,7 +106,7 @@ public void validateAndInit() { } Assert.isTrue( StringUtils.hasText(store.getEndpoint()) || StringUtils.hasText(store.getConnectionString()) - || store.getEndpoints().size() > 0 || store.getConnectionStrings().size() > 0, + || !store.getEndpoints().isEmpty() || !store.getConnectionStrings().isEmpty(), "Either configuration store name or connection string should be configured."); store.validateAndInit(); } @@ -117,7 +117,7 @@ public void validateAndInit() { if (!store.isEnabled()) { continue; } - if (store.getEndpoints().size() > 0) { + if (!store.getEndpoints().isEmpty()) { for (String endpoint : store.getEndpoints()) { if (existingEndpoints.containsKey(endpoint)) { throw new IllegalArgumentException("Duplicate store name exists."); 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 83409f1d16f8..ffa0450762ad 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 @@ -301,29 +301,29 @@ public void validateAndInit() { } if (StringUtils.hasText(connectionString)) { - String endpoint = (AppConfigurationReplicaClientsBuilder.getEndpointFromConnectionString(connectionString)); + String validationEndpoint = (AppConfigurationReplicaClientsBuilder.getEndpointFromConnectionString(connectionString)); try { // new URI is used to validate the endpoint as a valid URI - new URI(endpoint); - this.endpoint = endpoint; - } catch (URISyntaxException e) { + new URI(validationEndpoint).toURL(); + this.endpoint = validationEndpoint; + } catch (URISyntaxException | MalformedURLException | IllegalArgumentException e) { throw new IllegalStateException("Endpoint in connection string is not a valid URI.", e); } - } else if (connectionStrings.size() > 0) { + } else if (!connectionStrings.isEmpty()) { for (String connection : connectionStrings) { - String endpoint = (AppConfigurationReplicaClientsBuilder.getEndpointFromConnectionString(connection)); + String validationEndpoint = (AppConfigurationReplicaClientsBuilder.getEndpointFromConnectionString(connection)); try { // new URI is used to validate the endpoint as a valid URI - new URI(endpoint).toURL(); + new URI(validationEndpoint).toURL(); if (!StringUtils.hasText(this.endpoint)) { - this.endpoint = endpoint; + this.endpoint = validationEndpoint; } } catch (MalformedURLException | URISyntaxException | IllegalArgumentException e) { throw new IllegalStateException("Endpoint in connection string is not a valid URI.", e); } } - } else if (endpoints.size() > 0) { + } else if (!endpoints.isEmpty()) { endpoint = endpoints.get(0); } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPullRefreshTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPullRefreshTest.java index 98d7665a184f..5e3ba0d4cf61 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPullRefreshTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPullRefreshTest.java @@ -41,6 +41,9 @@ public class AppConfigurationPullRefreshTest { @Mock private AppConfigurationRefreshUtil refreshUtilMock; + @Mock + private StateHolder stateHolderMock; + private MockitoSession session; @BeforeEach @@ -57,10 +60,11 @@ public void cleanup() throws Exception { @Test public void refreshNoChange() throws InterruptedException, ExecutionException { - when(refreshUtilMock.refreshStoresCheck(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(eventDataMock); + when(refreshUtilMock.refreshStoresCheck(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())) + .thenReturn(eventDataMock); AppConfigurationPullRefresh refresh = new AppConfigurationPullRefresh(clientFactoryMock, refreshInterval, - replicaLookUpMock, refreshUtilMock); + replicaLookUpMock, stateHolderMock, refreshUtilMock); assertFalse(refresh.refreshConfigurations().block()); } @@ -69,10 +73,11 @@ public void refreshNoChange() throws InterruptedException, ExecutionException { public void refreshUpdate() throws InterruptedException, ExecutionException { when(eventDataMock.getMessage()).thenReturn("Updated"); when(eventDataMock.getDoRefresh()).thenReturn(true); - when(refreshUtilMock.refreshStoresCheck(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(eventDataMock); + when(refreshUtilMock.refreshStoresCheck(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())) + .thenReturn(eventDataMock); AppConfigurationPullRefresh refresh = new AppConfigurationPullRefresh(clientFactoryMock, refreshInterval, - replicaLookUpMock, refreshUtilMock); + replicaLookUpMock, stateHolderMock, refreshUtilMock); refresh.setApplicationEventPublisher(publisher); assertTrue(refresh.refreshConfigurations().block()); diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationRefreshUtilTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationRefreshUtilTest.java index ebe6ea4d1fa1..02eb4b8dfc93 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationRefreshUtilTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationRefreshUtilTest.java @@ -2,6 +2,16 @@ // Licensed under the MIT License. package com.azure.spring.cloud.appconfiguration.config.implementation; +import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.EMPTY_LABEL; +import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.FEATURE_FLAG_PREFIX; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -9,19 +19,12 @@ import java.util.Map; import org.junit.jupiter.api.AfterEach; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.mockito.ArgumentCaptor; import org.mockito.Mock; -import org.mockito.MockedStatic; 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; @@ -30,8 +33,6 @@ import com.azure.data.appconfiguration.models.ConfigurationSetting; import com.azure.data.appconfiguration.models.FeatureFlagConfigurationSetting; import com.azure.data.appconfiguration.models.SettingSelector; -import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.EMPTY_LABEL; -import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.FEATURE_FLAG_PREFIX; import com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationRefreshUtil.RefreshEventData; import com.azure.spring.cloud.appconfiguration.config.implementation.autofailover.ReplicaLookUp; import com.azure.spring.cloud.appconfiguration.config.implementation.configuration.WatchedConfigurationSettings; @@ -72,6 +73,8 @@ public class AppConfigurationRefreshUtilTest { private String endpoint; + private AppConfigurationRefreshUtil refreshUtil; + private final List watchKeysFeatureFlags = generateFeatureFlagWatchKeys(); private final AppConfigurationStoreMonitoring monitoring = new AppConfigurationStoreMonitoring(); @@ -95,6 +98,8 @@ public void setup() { monitoring.setEnabled(true); featureStore.setEnabled(true); + + refreshUtil = new AppConfigurationRefreshUtil(currentStateMock); } @AfterEach @@ -107,13 +112,10 @@ public void cleanup() throws Exception { public void refreshWithoutTimeWatchKeyConfigStoreNotLoaded(TestInfo testInfo) { endpoint = testInfo.getDisplayName() + ".azconfig.io"; when(clientMock.getEndpoint()).thenReturn(endpoint); + when(currentStateMock.getLoadState(endpoint)).thenReturn(false); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(false); - - assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock)); - assertFalse(AppConfigurationRefreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); - } + assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock, currentStateMock)); + assertFalse(refreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); } @Test @@ -127,13 +129,11 @@ public void refreshWithoutTimeWatchKeyConfigStoreWatchKeyNotReturned(TestInfo te // Config Store doesn't return a watch key change. when(clientMock.getWatchKey(Mockito.eq(KEY_FILTER), Mockito.eq(EMPTY_LABEL), Mockito.any(Context.class))) .thenReturn(watchKeys.get(0)); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(true); - stateHolderMock.when(() -> StateHolder.getState(endpoint)).thenReturn(newState); + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getState(endpoint)).thenReturn(newState); - assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock)); - assertFalse(AppConfigurationRefreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); - } + assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock, currentStateMock)); + assertFalse(refreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); } @Test @@ -149,12 +149,10 @@ public void refreshWithoutTimeWatchKeyConfigStoreWatchKeyNoChange(TestInfo testI // Config Store does return a watch key change. when(clientMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) .thenReturn(false); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getStateFeatureFlag(endpoint)).thenReturn(newState); + when(currentStateMock.getStateFeatureFlag(endpoint)).thenReturn(newState); - assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock)); - assertFalse(AppConfigurationRefreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); - } + assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock, currentStateMock)); + assertFalse(refreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); } @Test @@ -164,11 +162,9 @@ public void refreshWithoutTimeFeatureFlagDisabled(TestInfo testInfo) { configStore.getFeatureFlags().setEnabled(false); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock)); - assertFalse(AppConfigurationRefreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); - stateHolderMock.verify(() -> StateHolder.getLoadState(Mockito.anyString()), times(1)); - } + assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock, currentStateMock)); + assertFalse(refreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); + verify(currentStateMock, times(1)).getLoadState(Mockito.anyString()); } @Test @@ -178,11 +174,9 @@ public void refreshWithoutTimeFeatureFlagNotLoaded(TestInfo testInfo) { configStore.getFeatureFlags().setEnabled(true); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock)); - assertFalse(AppConfigurationRefreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); - stateHolderMock.verify(() -> StateHolder.getLoadState(Mockito.anyString()), times(1)); - } + assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock, currentStateMock)); + assertFalse(refreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); + verify(currentStateMock, times(1)).getLoadState(Mockito.anyString()); } @Test @@ -198,13 +192,10 @@ public void refreshWithoutTimeFeatureFlagNoChange(TestInfo testInfo) { // Config Store doesn't return a watch key change. when(clientMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) .thenReturn(false); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getStateFeatureFlag(endpoint)).thenReturn(newState); - - assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock)); - assertFalse(AppConfigurationRefreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); - } + when(currentStateMock.getStateFeatureFlag(endpoint)).thenReturn(newState); + assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock, currentStateMock)); + assertFalse(refreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); } @Test @@ -220,12 +211,10 @@ public void refreshWithoutTimeFeatureFlagEtagChanged(TestInfo testInfo) { // Config Store does return a watch key change. when(clientMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) .thenReturn(true); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getStateFeatureFlag(endpoint)).thenReturn(newState); + when(currentStateMock.getStateFeatureFlag(endpoint)).thenReturn(newState); - assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock)); - assertTrue(AppConfigurationRefreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); - } + assertFalse(AppConfigurationRefreshUtil.refreshStoreCheck(clientMock, endpoint, contextMock, currentStateMock)); + assertTrue(refreshUtil.refreshStoreFeatureFlagCheck(true, clientMock, contextMock)); } @Test @@ -236,21 +225,12 @@ public void refreshStoresCheckSettingsTestNotEnabled(TestInfo testInfo) { when(connectionManagerMock.getFeatureFlagStore()).thenReturn(featureStore); when(clientFactoryMock.getConnections()).thenReturn(Map.of(endpoint, connectionManagerMock)); - State newState = new State(generateWatchKeys(), Math.toIntExact(Duration.ofMinutes(10).getSeconds()), endpoint); - - // Config Store doesn't return a watch key change. - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(false); - stateHolderMock.when(() -> StateHolder.getState(endpoint)).thenReturn(newState); - - // Monitor is disabled - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), - (long) 60, replicaLookUpMock); - assertFalse(eventData.getDoRefresh()); - verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - Mockito.any(Context.class)); - } + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), + (long) 60, replicaLookUpMock); + assertFalse(eventData.getDoRefresh()); + verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), + Mockito.any(Context.class)); } @Test @@ -261,65 +241,73 @@ public void refreshStoresCheckSettingsTestNotLoaded(TestInfo testInfo) { when(connectionManagerMock.getFeatureFlagStore()).thenReturn(featureStore); when(clientFactoryMock.getConnections()).thenReturn(Map.of(endpoint, connectionManagerMock)); - State newState = new State(generateWatchKeys(), Math.toIntExact(Duration.ofMinutes(10).getSeconds()), endpoint); + when(currentStateMock.getLoadState(endpoint)).thenReturn(false); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(false); - stateHolderMock.when(() -> StateHolder.getState(endpoint)).thenReturn(newState); - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), (long) 60, replicaLookUpMock); - assertFalse(eventData.getDoRefresh()); - verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - Mockito.any(Context.class)); - } + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), (long) 60, replicaLookUpMock); + assertFalse(eventData.getDoRefresh()); + verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), + Mockito.any(Context.class)); } @Test public void refreshStoresCheckSettingsTestNotRefreshTime(TestInfo testInfo) { endpoint = testInfo.getDisplayName() + ".azconfig.io"; - setupFeatureFlagLoad(); + setupFeatureFlagLoadBasic(); - State newState = new State(generateWatchKeys(), Math.toIntExact(Duration.ofMinutes(10).getSeconds()), endpoint); + // Set up state with WatchedConfigurationSettings and a future refresh time (not expired) + WatchedConfigurationSettings watchedConfigurationSettings = new WatchedConfigurationSettings( + new SettingSelector().setKeyFilter(KEY_FILTER).setLabelFilter(EMPTY_LABEL), + generateWatchKeys()); + // Use a positive duration so the refresh time is in the future + State state = new State(null, List.of(watchedConfigurationSettings), + Math.toIntExact(Duration.ofMinutes(10).getSeconds()), endpoint); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(true); - stateHolderMock.when(() -> StateHolder.getState(endpoint)).thenReturn(newState); + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getState(endpoint)).thenReturn(state); + when(clientFactoryMock.getNextActiveClient(Mockito.eq(endpoint), Mockito.booleanThat(value -> true))) + .thenReturn(clientOriginMock); - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), - (long) 60, replicaLookUpMock); - assertFalse(eventData.getDoRefresh()); - verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - Mockito.any(Context.class)); - } + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), + (long) 60, replicaLookUpMock); + assertFalse(eventData.getDoRefresh()); + // Verify that checkWatchKeys is NOT called because refresh time hasn't arrived + verify(clientOriginMock, times(0)).checkWatchKeys(Mockito.any(SettingSelector.class), + Mockito.any(Context.class)); + verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), + Mockito.any(Context.class)); } @Test - public void refreshStoresCheckSettingsTestFailedRequest(TestInfo testInfo) { + public void refreshStoresCheckSettingsTestNoChangeDetected(TestInfo testInfo) { endpoint = testInfo.getDisplayName() + ".azconfig.io"; setupFeatureFlagLoad(); - State newState = new State(generateWatchKeys(), Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); - - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(true); - stateHolderMock.when(() -> StateHolder.getState(endpoint)).thenReturn(newState); - StateHolder updatedStateHolder = new StateHolder(); - stateHolderMock.when(() -> StateHolder.getCurrentState()).thenReturn(updatedStateHolder); - - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), - (long) 60, replicaLookUpMock); - assertFalse(eventData.getDoRefresh()); - ArgumentCaptor captorParam = ArgumentCaptor.forClass(Context.class); - verify(clientOriginMock, times(1)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - captorParam.capture()); - assertEquals(newState, StateHolder.getState(endpoint)); - Context testContext = captorParam.getValue(); - assertTrue((Boolean) testContext.getData("refresh").get()); - assertFalse((Boolean) testContext.getData("PushRefresh").get()); - } + // Set up state with WatchedConfigurationSettings + WatchedConfigurationSettings watchedConfigurationSettings = new WatchedConfigurationSettings( + new SettingSelector().setKeyFilter(KEY_FILTER).setLabelFilter(EMPTY_LABEL), + generateWatchKeys()); + State state = new State(null, List.of(watchedConfigurationSettings), + Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); + + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getState(endpoint)).thenReturn(state); + + when(clientOriginMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) + .thenReturn(false); + + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), + (long) 60, replicaLookUpMock); + assertFalse(eventData.getDoRefresh()); + ArgumentCaptor captorParam = ArgumentCaptor.forClass(Context.class); + verify(clientOriginMock, times(1)).checkWatchKeys(Mockito.any(SettingSelector.class), + captorParam.capture()); + Context testContext = captorParam.getValue(); + assertTrue((Boolean) testContext.getData("refresh").get()); + assertFalse((Boolean) testContext.getData("PushRefresh").get()); } @Test @@ -327,24 +315,24 @@ public void refreshStoresCheckSettingsTestRefreshTimeNoChange(TestInfo testInfo) endpoint = testInfo.getDisplayName() + ".azconfig.io"; setupFeatureFlagLoad(); - when(clientOriginMock.getWatchKey(Mockito.anyString(), Mockito.anyString(), Mockito.any(Context.class))) - .thenReturn(generateWatchKeys().get(0)); + // Set up state with WatchedConfigurationSettings + WatchedConfigurationSettings watchedConfigurationSettings = new WatchedConfigurationSettings( + new SettingSelector().setKeyFilter(KEY_FILTER).setLabelFilter(EMPTY_LABEL), + generateWatchKeys()); + State state = new State(null, List.of(watchedConfigurationSettings), + Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); - State newState = new State(generateWatchKeys(), Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getState(endpoint)).thenReturn(state); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(true); - stateHolderMock.when(() -> StateHolder.getState(Mockito.any())).thenReturn(newState); - StateHolder updatedStateHolder = new StateHolder(); - stateHolderMock.when(() -> StateHolder.getCurrentState()).thenReturn(updatedStateHolder); + when(clientOriginMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) + .thenReturn(false); - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), (long) 60, replicaLookUpMock); - assertEquals(newState, StateHolder.getState(endpoint)); - assertFalse(eventData.getDoRefresh()); - verify(clientOriginMock, times(1)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - Mockito.any(Context.class)); - } + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), (long) 60, replicaLookUpMock); + assertFalse(eventData.getDoRefresh()); + verify(clientOriginMock, times(1)).checkWatchKeys(Mockito.any(SettingSelector.class), + Mockito.any(Context.class)); } @Test @@ -359,28 +347,28 @@ public void refreshStoresPushRefreshEnabledPrimary(TestInfo testInfo) { monitoring.setPushNotification(pushNotificaiton); when(connectionManagerMock.getMonitoring()).thenReturn(monitoring); - when(clientOriginMock.getWatchKey(Mockito.anyString(), Mockito.anyString(), Mockito.any(Context.class))) - .thenReturn(generateWatchKeys().get(0)); - - State newState = new State(generateWatchKeys(), Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); - - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(true); - stateHolderMock.when(() -> StateHolder.getState(Mockito.any())).thenReturn(newState); - StateHolder updatedStateHolder = new StateHolder(); - stateHolderMock.when(() -> StateHolder.getCurrentState()).thenReturn(updatedStateHolder); - - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), (long) 60, replicaLookUpMock); - assertEquals(newState, StateHolder.getState(endpoint)); - assertFalse(eventData.getDoRefresh()); - ArgumentCaptor captorParam = ArgumentCaptor.forClass(Context.class); - verify(clientOriginMock, times(1)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - captorParam.capture()); - Context testContext = captorParam.getValue(); - assertTrue((Boolean) testContext.getData("refresh").get()); - assertTrue((Boolean) testContext.getData("PushRefresh").get()); - } + // Set up state with WatchedConfigurationSettings + WatchedConfigurationSettings watchedConfigurationSettings = new WatchedConfigurationSettings( + new SettingSelector().setKeyFilter(KEY_FILTER).setLabelFilter(EMPTY_LABEL), + generateWatchKeys()); + State state = new State(null, List.of(watchedConfigurationSettings), + Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); + + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getState(endpoint)).thenReturn(state); + + when(clientOriginMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) + .thenReturn(false); + + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), (long) 60, replicaLookUpMock); + assertFalse(eventData.getDoRefresh()); + ArgumentCaptor captorParam = ArgumentCaptor.forClass(Context.class); + verify(clientOriginMock, times(1)).checkWatchKeys(Mockito.any(SettingSelector.class), + captorParam.capture()); + Context testContext = captorParam.getValue(); + assertTrue((Boolean) testContext.getData("refresh").get()); + assertTrue((Boolean) testContext.getData("PushRefresh").get()); } @Test @@ -391,184 +379,180 @@ public void refreshStoresPushRefreshEnabledSecondary(TestInfo testInfo) { AccessToken p2 = new AccessToken(); p2.setName("fake name"); p2.setSecret("value"); - pushNotificaiton.setPrimaryToken(p2); + pushNotificaiton.setSecondaryToken(p2); monitoring.setPushNotification(pushNotificaiton); when(connectionManagerMock.getMonitoring()).thenReturn(monitoring); - when(clientOriginMock.getWatchKey(Mockito.anyString(), Mockito.anyString(), Mockito.any(Context.class))) - .thenReturn(generateWatchKeys().get(0)); - - State newState = new State(generateWatchKeys(), Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); - - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(true); - stateHolderMock.when(() -> StateHolder.getState(Mockito.any())).thenReturn(newState); - StateHolder updatedStateHolder = new StateHolder(); - stateHolderMock.when(() -> StateHolder.getCurrentState()).thenReturn(updatedStateHolder); - - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), (long) 60, replicaLookUpMock); - assertEquals(newState, StateHolder.getState(endpoint)); - assertFalse(eventData.getDoRefresh()); - ArgumentCaptor captorParam = ArgumentCaptor.forClass(Context.class); - verify(clientOriginMock, times(1)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - captorParam.capture()); - Context testContext = captorParam.getValue(); - assertTrue((Boolean) testContext.getData("refresh").get()); - assertTrue((Boolean) testContext.getData("PushRefresh").get()); - } + // Set up state with WatchedConfigurationSettings + WatchedConfigurationSettings watchedConfigurationSettings = new WatchedConfigurationSettings( + new SettingSelector().setKeyFilter(KEY_FILTER).setLabelFilter(EMPTY_LABEL), + generateWatchKeys()); + State state = new State(null, List.of(watchedConfigurationSettings), + Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); + + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getState(endpoint)).thenReturn(state); + + when(clientOriginMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) + .thenReturn(false); + + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), (long) 60, replicaLookUpMock); + assertFalse(eventData.getDoRefresh()); + ArgumentCaptor captorParam = ArgumentCaptor.forClass(Context.class); + verify(clientOriginMock, times(1)).checkWatchKeys(Mockito.any(SettingSelector.class), + captorParam.capture()); + Context testContext = captorParam.getValue(); + assertTrue((Boolean) testContext.getData("refresh").get()); + assertTrue((Boolean) testContext.getData("PushRefresh").get()); } @Test public void refreshStoresCheckSettingsTestTriggerRefresh(TestInfo testInfo) { endpoint = testInfo.getDisplayName() + ".azconfig.io"; when(connectionManagerMock.getMonitoring()).thenReturn(monitoring); + FeatureFlagStore disabledFeatureStore = new FeatureFlagStore(); + disabledFeatureStore.setEnabled(false); + lenient().when(connectionManagerMock.getFeatureFlagStore()).thenReturn(disabledFeatureStore); when(clientFactoryMock.getConnections()).thenReturn(Map.of(endpoint, connectionManagerMock)); // Refresh Time, trigger refresh when(clientFactoryMock.getNextActiveClient(Mockito.eq(endpoint), Mockito.booleanThat(value -> true))) .thenReturn(clientOriginMock); - ConfigurationSetting refreshKey = new ConfigurationSetting().setKey(KEY_FILTER).setLabel(EMPTY_LABEL) - .setETag("new"); - - when(clientOriginMock.getWatchKey(Mockito.anyString(), Mockito.anyString(), Mockito.any(Context.class))) - .thenReturn(refreshKey); - - State newState = new State(generateWatchKeys(), Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); - - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(true); - stateHolderMock.when(() -> StateHolder.getState(endpoint)).thenReturn(newState); - stateHolderMock.when(StateHolder::getCurrentState).thenReturn(currentStateMock); - - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), - (long) 60, replicaLookUpMock); - assertTrue(eventData.getDoRefresh()); - verify(clientOriginMock, times(1)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - Mockito.any(Context.class)); - verify(currentStateMock, times(1)).updateStateRefresh(Mockito.any(), Mockito.any()); - } + + // Set up state with WatchedConfigurationSettings + WatchedConfigurationSettings watchedConfigurationSettings = new WatchedConfigurationSettings( + new SettingSelector().setKeyFilter(KEY_FILTER).setLabelFilter(EMPTY_LABEL), + generateWatchKeys()); + State state = new State(null, List.of(watchedConfigurationSettings), + Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); + + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getState(endpoint)).thenReturn(state); + + when(clientOriginMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) + .thenReturn(true); + + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), + (long) 60, replicaLookUpMock); + assertTrue(eventData.getDoRefresh()); + verify(clientOriginMock, times(1)).checkWatchKeys(Mockito.any(SettingSelector.class), + Mockito.any(Context.class)); + verify(currentStateMock, times(1)).updateStateRefresh(Mockito.any(), Mockito.any()); } @Test public void refreshStoresCheckFeatureFlagTestNotLoaded(TestInfo testInfo) { endpoint = testInfo.getDisplayName() + ".azconfig.io"; - setupFeatureFlagLoad(); - - FeatureFlagState newState = new FeatureFlagState(List.of(), - Math.toIntExact(Duration.ofMinutes(10).getSeconds()), endpoint); - - // Config Store doesn't return a watch key change. - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getStateFeatureFlag(endpoint)).thenReturn(newState); - - // Monitor is disabled - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), - (long) 60, replicaLookUpMock); - assertFalse(eventData.getDoRefresh()); - verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - Mockito.any(Context.class)); - } + setupFeatureFlagLoadBasic(); + + // Feature flag state is not loaded (null) + when(currentStateMock.getStateFeatureFlag(endpoint)).thenReturn(null); + + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), + (long) 60, replicaLookUpMock); + assertFalse(eventData.getDoRefresh()); + // Verify that checkWatchKeys is NOT called because feature flag state is not loaded + verify(clientOriginMock, times(0)).checkWatchKeys(Mockito.any(SettingSelector.class), + Mockito.any(Context.class)); + verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), + Mockito.any(Context.class)); } @Test public void refreshStoresCheckFeatureFlagTestNotRefreshTime(TestInfo testInfo) { endpoint = testInfo.getDisplayName() + ".azconfig.io"; - setupFeatureFlagLoad(); + setupFeatureFlagLoadBasic(); - FeatureFlagState newState = new FeatureFlagState(List.of(), + // Set up feature flag state with a future refresh time (not expired) + WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings( + new SettingSelector().setKeyFilter(FEATURE_FLAG_PREFIX).setLabelFilter(EMPTY_LABEL), + watchKeysFeatureFlags); + // Use a positive duration so the refresh time is in the future + FeatureFlagState ffState = new FeatureFlagState(List.of(featureFlags), Math.toIntExact(Duration.ofMinutes(10).getSeconds()), endpoint); - // Config Store doesn't return a watch key change. - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getStateFeatureFlag(endpoint)).thenReturn(newState); - - // Monitor is disabled - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), - (long) 60, replicaLookUpMock); - assertFalse(eventData.getDoRefresh()); - verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - Mockito.any(Context.class)); - } + when(currentStateMock.getStateFeatureFlag(endpoint)).thenReturn(ffState); + when(clientFactoryMock.getNextActiveClient(Mockito.eq(endpoint), Mockito.booleanThat(value -> true))) + .thenReturn(clientOriginMock); + + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), + (long) 60, replicaLookUpMock); + assertFalse(eventData.getDoRefresh()); + // Verify that checkWatchKeys is NOT called because refresh time hasn't arrived + verify(clientOriginMock, times(0)).checkWatchKeys(Mockito.any(SettingSelector.class), + Mockito.any(Context.class)); + verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), + Mockito.any(Context.class)); } @Test public void refreshStoresCheckFeatureFlagTestNoChange(TestInfo testInfo) { endpoint = testInfo.getDisplayName() + ".azconfig.io"; - configStore.setEndpoint(endpoint); - configStore.setFeatureFlags(featureStore); - configStore.setMonitoring(monitoring); - setupFeatureFlagLoad(); - when(clientOriginMock.checkWatchKeys(Mockito.any(), Mockito.any(Context.class))).thenReturn(false); - FeatureFlagState newState = new FeatureFlagState( - List.of(new WatchedConfigurationSettings( - new SettingSelector().setKeyFilter(KEY_FILTER).setLabelFilter(EMPTY_LABEL), null)), + // Set up feature flag state so it can be checked + WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings( + new SettingSelector().setKeyFilter(FEATURE_FLAG_PREFIX).setLabelFilter(EMPTY_LABEL), + watchKeysFeatureFlags); + FeatureFlagState ffState = new FeatureFlagState(List.of(featureFlags), Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); - // Config Store doesn't return a watch key change. - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getStateFeatureFlag(endpoint)).thenReturn(newState); - stateHolderMock.when(StateHolder::getCurrentState).thenReturn(currentStateMock); - - // Monitor is disabled - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), (long) 60, replicaLookUpMock); - assertFalse(eventData.getDoRefresh()); - verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - Mockito.any(Context.class)); - verify(currentStateMock, times(1)).updateFeatureFlagStateRefresh(Mockito.any(), Mockito.any()); - - } + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getStateFeatureFlag(endpoint)).thenReturn(ffState); + when(clientOriginMock.checkWatchKeys(Mockito.any(), Mockito.any(Context.class))).thenReturn(false); + + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), (long) 60, replicaLookUpMock); + assertFalse(eventData.getDoRefresh()); + verify(currentStateMock, times(1)).updateFeatureFlagStateRefresh(Mockito.any(), Mockito.any()); } @Test public void refreshStoresCheckFeatureFlagTestTriggerRefresh(TestInfo testInfo) { endpoint = testInfo.getDisplayName() + ".azconfig.io"; setupFeatureFlagLoad(); - when(clientOriginMock.checkWatchKeys(Mockito.any(), Mockito.any(Context.class))).thenReturn(true); - WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings(new SettingSelector(), + // Set up feature flag state so it can be checked + WatchedConfigurationSettings featureFlags = new WatchedConfigurationSettings( + new SettingSelector().setKeyFilter(FEATURE_FLAG_PREFIX).setLabelFilter(EMPTY_LABEL), watchKeysFeatureFlags); - - FeatureFlagState newState = new FeatureFlagState(List.of(featureFlags), + FeatureFlagState ffState = new FeatureFlagState(List.of(featureFlags), Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); - // Config Store doesn't return a watch key change. - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getStateFeatureFlag(endpoint)).thenReturn(newState); - stateHolderMock.when(StateHolder::getCurrentState).thenReturn(currentStateMock); - - // Monitor is disabled - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(10), (long) 60, replicaLookUpMock); - assertTrue(eventData.getDoRefresh()); - verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - Mockito.any(Context.class)); - } + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getStateFeatureFlag(endpoint)).thenReturn(ffState); + when(clientOriginMock.checkWatchKeys(Mockito.any(), Mockito.any(Context.class))).thenReturn(true); + + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(10), (long) 60, replicaLookUpMock); + assertTrue(eventData.getDoRefresh()); + verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), + Mockito.any(Context.class)); } @Test public void minRefreshPeriodTest() { - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getNextForcedRefresh()).thenReturn(Instant.now().minusSeconds(600)); - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck(clientFactoryMock, - Duration.ofMinutes(1), (long) 0, replicaLookUpMock); - assertTrue(eventData.getDoRefresh()); - assertEquals("Minimum refresh period reached. Refreshing configurations.", eventData.getMessage()); - } + when(currentStateMock.getNextForcedRefresh()).thenReturn(Instant.now().minusSeconds(600)); + RefreshEventData eventData = refreshUtil.refreshStoresCheck(clientFactoryMock, + Duration.ofMinutes(1), (long) 0, replicaLookUpMock); + assertTrue(eventData.getDoRefresh()); + assertEquals("Minimum refresh period reached. Refreshing configurations.", eventData.getMessage()); } private void setupFeatureFlagLoad() { + setupFeatureFlagLoadBasic(); + when(clientFactoryMock.getNextActiveClient(Mockito.eq(endpoint), Mockito.booleanThat(value -> true))) + .thenReturn(clientOriginMock); + } + + private void setupFeatureFlagLoadBasic() { when(connectionManagerMock.getMonitoring()).thenReturn(monitoring); when(connectionManagerMock.getFeatureFlagStore()).thenReturn(featureStore); when(clientFactoryMock.getConnections()).thenReturn(Map.of(endpoint, connectionManagerMock)); - when(clientFactoryMock.getNextActiveClient(Mockito.eq(endpoint), Mockito.booleanThat(value -> true))) - .thenReturn(clientOriginMock); } private List generateWatchKeys() { @@ -596,36 +580,35 @@ public void refreshAllWithWatchedConfigurationSettingsTest(TestInfo testInfo) { endpoint = testInfo.getDisplayName() + ".azconfig.io"; when(connectionManagerMock.getMonitoring()).thenReturn(monitoring); + FeatureFlagStore disabledFeatureStore = new FeatureFlagStore(); + disabledFeatureStore.setEnabled(false); + lenient().when(connectionManagerMock.getFeatureFlagStore()).thenReturn(disabledFeatureStore); when(clientFactoryMock.getConnections()).thenReturn(Map.of(endpoint, connectionManagerMock)); when(clientFactoryMock.getNextActiveClient(Mockito.eq(endpoint), Mockito.booleanThat(value -> true))) .thenReturn(clientOriginMock); - // Set up watched configuration settings state + // Set up state with WatchedConfigurationSettings WatchedConfigurationSettings watchedConfigurationSettings = new WatchedConfigurationSettings( - new SettingSelector().setKeyFilter(KEY_FILTER).setLabelFilter(EMPTY_LABEL), null); + new SettingSelector().setKeyFilter(KEY_FILTER).setLabelFilter(EMPTY_LABEL), + generateWatchKeys()); State state = new State(null, List.of(watchedConfigurationSettings), Math.toIntExact(Duration.ofMinutes(-1).getSeconds()), endpoint); - // Config Store returns a change via watched configuration settings + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getState(endpoint)).thenReturn(state); when(clientOriginMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) .thenReturn(true); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(true); - stateHolderMock.when(() -> StateHolder.getState(endpoint)).thenReturn(state); - stateHolderMock.when(StateHolder::getCurrentState).thenReturn(currentStateMock); - - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck( - clientFactoryMock, Duration.ofMinutes(10), (long) 60, replicaLookUpMock); - - assertTrue(eventData.getDoRefresh()); - // Verify checkWatchKeys is called (watched configuration settings path) - verify(clientOriginMock, times(1)).checkWatchKeys(Mockito.any(SettingSelector.class), - Mockito.any(Context.class)); - // Verify getWatchKey is NOT called (traditional watch key path) - verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), - Mockito.any(Context.class)); - } + RefreshEventData eventData = refreshUtil.refreshStoresCheck( + clientFactoryMock, Duration.ofMinutes(10), (long) 60, replicaLookUpMock); + + assertTrue(eventData.getDoRefresh()); + // Verify checkWatchKeys is called (watched configuration settings path) + verify(clientOriginMock, times(1)).checkWatchKeys(Mockito.any(SettingSelector.class), + Mockito.any(Context.class)); + // Verify getWatchKey is NOT called (traditional watch key path) + verify(clientOriginMock, times(0)).getWatchKey(Mockito.anyString(), Mockito.anyString(), + Mockito.any(Context.class)); } @Test @@ -650,19 +633,16 @@ public void refreshAllWithNullWatchKeysTest(TestInfo testInfo) { when(clientOriginMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) .thenReturn(false); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(true); - stateHolderMock.when(() -> StateHolder.getState(endpoint)).thenReturn(state); - stateHolderMock.when(StateHolder::getCurrentState).thenReturn(currentStateMock); + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getState(endpoint)).thenReturn(state); - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck( - clientFactoryMock, Duration.ofMinutes(10), (long) 60, replicaLookUpMock); + RefreshEventData eventData = refreshUtil.refreshStoresCheck( + clientFactoryMock, Duration.ofMinutes(10), (long) 60, replicaLookUpMock); - // No change detected, so should not refresh - assertFalse(eventData.getDoRefresh()); - verify(clientOriginMock, times(1)).checkWatchKeys(Mockito.any(SettingSelector.class), - Mockito.any(Context.class)); - } + // No change detected, so should not refresh + assertFalse(eventData.getDoRefresh()); + verify(clientOriginMock, times(1)).checkWatchKeys(Mockito.any(SettingSelector.class), + Mockito.any(Context.class)); } @Test @@ -688,17 +668,14 @@ public void watchedConfigurationSettingsNoChangeTest(TestInfo testInfo) { when(clientOriginMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) .thenReturn(false); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(true); - stateHolderMock.when(() -> StateHolder.getState(endpoint)).thenReturn(state); - stateHolderMock.when(StateHolder::getCurrentState).thenReturn(currentStateMock); + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getState(endpoint)).thenReturn(state); - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck( - clientFactoryMock, Duration.ofMinutes(10), (long) 60, replicaLookUpMock); + RefreshEventData eventData = refreshUtil.refreshStoresCheck( + clientFactoryMock, Duration.ofMinutes(10), (long) 60, replicaLookUpMock); - assertFalse(eventData.getDoRefresh()); - verify(currentStateMock, times(1)).updateStateRefresh(Mockito.any(), Mockito.any()); - } + assertFalse(eventData.getDoRefresh()); + verify(currentStateMock, times(1)).updateStateRefresh(Mockito.any(), Mockito.any()); } @Test @@ -707,6 +684,8 @@ public void watchedConfigurationSettingsWithChangeDetectedTest(TestInfo testInfo endpoint = testInfo.getDisplayName() + ".azconfig.io"; when(connectionManagerMock.getMonitoring()).thenReturn(monitoring); + FeatureFlagStore disabledFeatureStore = new FeatureFlagStore(); + disabledFeatureStore.setEnabled(false); when(clientFactoryMock.getConnections()).thenReturn(Map.of(endpoint, connectionManagerMock)); when(clientFactoryMock.getNextActiveClient(Mockito.eq(endpoint), Mockito.booleanThat(value -> true))) .thenReturn(clientOriginMock); @@ -721,15 +700,12 @@ public void watchedConfigurationSettingsWithChangeDetectedTest(TestInfo testInfo when(clientOriginMock.checkWatchKeys(Mockito.any(SettingSelector.class), Mockito.any(Context.class))) .thenReturn(true); - try (MockedStatic stateHolderMock = Mockito.mockStatic(StateHolder.class)) { - stateHolderMock.when(() -> StateHolder.getLoadState(endpoint)).thenReturn(true); - stateHolderMock.when(() -> StateHolder.getState(endpoint)).thenReturn(state); - stateHolderMock.when(StateHolder::getCurrentState).thenReturn(currentStateMock); + when(currentStateMock.getLoadState(endpoint)).thenReturn(true); + when(currentStateMock.getState(endpoint)).thenReturn(state); - RefreshEventData eventData = new AppConfigurationRefreshUtil().refreshStoresCheck( - clientFactoryMock, Duration.ofMinutes(10), (long) 60, replicaLookUpMock); + RefreshEventData eventData = refreshUtil.refreshStoresCheck( + clientFactoryMock, Duration.ofMinutes(10), (long) 60, replicaLookUpMock); - assertTrue(eventData.getDoRefresh()); - } + assertTrue(eventData.getDoRefresh()); } } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/StateHolderTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/StateHolderTest.java index 683de01f730b..0b288e0aabf8 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/StateHolderTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/StateHolderTest.java @@ -3,7 +3,10 @@ package com.azure.spring.cloud.appconfiguration.config.implementation; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.times; @@ -13,176 +16,145 @@ import java.util.ArrayList; import java.util.List; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.TestInfo; import org.mockito.MockedStatic; import org.mockito.Mockito; -import org.mockito.MockitoAnnotations; -import org.mockito.MockitoSession; -import org.mockito.quality.Strictness; import com.azure.data.appconfiguration.models.ConfigurationSetting; public class StateHolderTest { - private final List watchKeys = new ArrayList<>(); - - private MockitoSession session; + private List watchKeys; + private StateHolder stateHolder; + private static final String TEST_ENDPOINT = "test.azconfig.io"; @BeforeEach public void setup() { - session = Mockito.mockitoSession().initMocks(this).strictness(Strictness.STRICT_STUBS).startMocking(); - MockitoAnnotations.openMocks(this); + watchKeys = new ArrayList<>(); ConfigurationSetting watchKey = new ConfigurationSetting().setKey("sentinel").setValue("0").setETag("current"); - watchKeys.add(watchKey); - } - - @AfterEach - public void cleanup() throws Exception { - MockitoAnnotations.openMocks(this).close(); - session.finishMocking(); + stateHolder = new StateHolder(); } - /** - * Because of static code these need to run all at once. - * @param testInfo - */ @Test - public void stateHolderTest(TestInfo testInfo) { - // Expire State Tests - stateNotExpiredTest(testInfo); - stateExpiredTest(testInfo); - - // Update Next Refresh Time Tests - updateNextRefreshTimeNoRefreshTest(testInfo); - updateNextRefreshTimeRefreshTest(testInfo); - updateNextRefreshBackoffCalcTest(testInfo); - - // Load State Tests - loadStateTest(testInfo); + public void stateNotExpiredTest() { + stateHolder.setNextForcedRefresh(Duration.ofMinutes(10)); + stateHolder.setState(TEST_ENDPOINT, watchKeys, Duration.ofSeconds(30)); + + State originalState = stateHolder.getState(TEST_ENDPOINT); + assertNotNull(originalState); + Instant originalRefreshCheck = originalState.getNextRefreshCheck(); + + stateHolder.expireState(TEST_ENDPOINT); + State newState = stateHolder.getState(TEST_ENDPOINT); + + // expireState should update the refresh check time with jitter (0-15 seconds from now) + assertNotEquals(originalRefreshCheck, newState.getNextRefreshCheck()); + // The new refresh check should be sooner than the original (since jitter is added from now) + assertTrue(newState.getNextRefreshCheck().isBefore(originalRefreshCheck)); } - private void stateNotExpiredTest(TestInfo testInfo) { - // State isn't expired Test - String endpoint = testInfo.getDisplayName() + "expire" + ".azconfig.io"; - - StateHolder expireStateHolder = new StateHolder(); - expireStateHolder.setNextForcedRefresh(Duration.ofMinutes(10)); - expireStateHolder.setState(endpoint, watchKeys, Duration.ofSeconds(30)); - - StateHolder.updateState(expireStateHolder); - - State originalExpireState = StateHolder.getState(endpoint); - expireStateHolder.expireState(endpoint); - StateHolder.updateState(expireStateHolder); - assertNotEquals(originalExpireState, StateHolder.getState(endpoint)); - } - - private void stateExpiredTest(TestInfo testInfo) { - // State is expired Test - String endpoint = testInfo.getDisplayName() + "expireNegativeDuration" + ".azconfig.io"; - - StateHolder expiredNegativeDurationStateHolder = new StateHolder(); - expiredNegativeDurationStateHolder.setNextForcedRefresh(Duration.ofMinutes(10)); - expiredNegativeDurationStateHolder.setState(endpoint, watchKeys, Duration.ofHours(-30)); - - StateHolder.updateState(expiredNegativeDurationStateHolder); - - State originalExpireNegativeState = StateHolder.getState(endpoint); - expiredNegativeDurationStateHolder.expireState(endpoint); - StateHolder.updateState(expiredNegativeDurationStateHolder); - assertEquals(originalExpireNegativeState, StateHolder.getState(endpoint)); + @Test + public void stateExpiredTest() { + // State with negative duration is already expired + stateHolder.setNextForcedRefresh(Duration.ofMinutes(10)); + stateHolder.setState(TEST_ENDPOINT, watchKeys, Duration.ofHours(-30)); + + State originalState = stateHolder.getState(TEST_ENDPOINT); + Instant originalRefreshCheck = originalState.getNextRefreshCheck(); + + stateHolder.expireState(TEST_ENDPOINT); + State newState = stateHolder.getState(TEST_ENDPOINT); + + // When state is already expired, expireState won't update if jitter would make it later + // The check is: if wait < timeLeft, update. Since timeLeft is negative, wait is always >= timeLeft + assertEquals(originalRefreshCheck, newState.getNextRefreshCheck()); } - private void updateNextRefreshTimeNoRefreshTest(TestInfo testInfo) { - String endpoint = testInfo.getDisplayName() + "updateRefreshTime" + ".azconfig.io"; - - StateHolder stateHolder = new StateHolder(); - - stateHolder.setState(endpoint, watchKeys, Duration.ofMinutes((long) 10)); - - StateHolder.updateState(stateHolder); + @Test + public void updateNextRefreshTimeNoRefreshTest() { + stateHolder.setState(TEST_ENDPOINT, watchKeys, Duration.ofMinutes(10)); - State originalState = StateHolder.getState(endpoint); + State originalState = stateHolder.getState(TEST_ENDPOINT); - stateHolder.updateNextRefreshTime(null, (long) 0); - StateHolder.updateState(stateHolder); - State newState = StateHolder.getState(endpoint); + stateHolder.updateNextRefreshTime(null, 0L); + State newState = stateHolder.getState(TEST_ENDPOINT); + assertEquals(originalState.getNextRefreshCheck(), newState.getNextRefreshCheck()); } - private void updateNextRefreshTimeRefreshTest(TestInfo testInfo) { - String endpoint = testInfo.getDisplayName() + "updateRefreshTimeRefresh" + ".azconfig.io"; - - StateHolder stateHolder = new StateHolder(); - - stateHolder.setState(endpoint, watchKeys, Duration.ofMinutes((long) 0)); - - StateHolder.updateState(stateHolder); + @Test + public void updateNextRefreshTimeRefreshTest() { + // Duration 0 means refresh immediately + stateHolder.setState(TEST_ENDPOINT, watchKeys, Duration.ofMinutes(0)); - State originalState = StateHolder.getState(endpoint); + State originalState = stateHolder.getState(TEST_ENDPOINT); - // Duration is less than the minBackOff try { Thread.sleep(1000); } catch (InterruptedException e) { fail("Sleep failed"); } - stateHolder.updateNextRefreshTime(null, (long) 0); - State newState = StateHolder.getState(endpoint); + + stateHolder.updateNextRefreshTime(null, 0L); + State newState = stateHolder.getState(TEST_ENDPOINT); + assertNotEquals(originalState.getNextRefreshCheck(), newState.getNextRefreshCheck()); assertTrue(originalState.getNextRefreshCheck().isBefore(newState.getNextRefreshCheck())); } - private void updateNextRefreshBackoffCalcTest(TestInfo testInfo) { - String endpoint = testInfo.getDisplayName() + "updateRefreshTimeBackoffCalc" + ".azconfig.io"; - - StateHolder stateHolder = new StateHolder(); - stateHolder.setState(endpoint, watchKeys, Duration.ofMinutes((long) -1)); - StateHolder.updateState(stateHolder); - State originalState = StateHolder.getState(endpoint); + @Test + public void updateNextRefreshBackoffCalcTest() { + stateHolder.setState(TEST_ENDPOINT, watchKeys, Duration.ofMinutes(-1)); + State originalState = stateHolder.getState(TEST_ENDPOINT); - // Duration is less than the minBackOff try (MockedStatic backoffTimeCalculatorMock = Mockito .mockStatic(BackoffTimeCalculator.class)) { Long ns = Long.valueOf("300000000000"); backoffTimeCalculatorMock.when(() -> BackoffTimeCalculator.calculateBackoff(Mockito.anyInt())) .thenReturn(ns); - stateHolder.updateNextRefreshTime(null, (long) -120); - State newState = StateHolder.getState(endpoint); + stateHolder.updateNextRefreshTime(null, -120L); + State newState = stateHolder.getState(TEST_ENDPOINT); assertTrue(originalState.getNextRefreshCheck().isBefore(newState.getNextRefreshCheck())); backoffTimeCalculatorMock.verify(() -> BackoffTimeCalculator.calculateBackoff(Mockito.anyInt()), times(1)); } + } - stateHolder = new StateHolder(); - Duration duration = Duration.ofMinutes((long) -1); - + @Test + public void updateNextForcedRefreshTest() { + Duration duration = Duration.ofMinutes(-1); stateHolder.setNextForcedRefresh(duration); + stateHolder.setState(TEST_ENDPOINT, watchKeys, duration); - stateHolder.setState(endpoint, watchKeys, duration); + Instant originalForcedRefresh = stateHolder.getNextForcedRefresh(); - StateHolder.updateState(stateHolder); + stateHolder.updateNextRefreshTime(Duration.ofMinutes(11), 0L); - Instant originalForcedRefresh = StateHolder.getNextForcedRefresh(); - - stateHolder.updateNextRefreshTime(Duration.ofMinutes((long) 11), (long) 0); - - Instant newForcedRefresh = StateHolder.getNextForcedRefresh(); + Instant newForcedRefresh = stateHolder.getNextForcedRefresh(); assertNotEquals(originalForcedRefresh, newForcedRefresh); } - private void loadStateTest(TestInfo testInfo) { - String endpoint = testInfo.getDisplayName() + "updateRefreshTimeBackoffCalc" + ".azconfig.io"; - StateHolder testStateHolder = new StateHolder(); - testStateHolder.setLoadState(endpoint, true); - StateHolder.updateState(testStateHolder); - assertEquals(testStateHolder, StateHolder.getCurrentState()); + @Test + public void loadStateTest() { + assertFalse(stateHolder.getLoadState(TEST_ENDPOINT)); + + stateHolder.setLoadState(TEST_ENDPOINT, true); + + assertTrue(stateHolder.getLoadState(TEST_ENDPOINT)); + } + + @Test + public void getStateReturnsNullForUnknownEndpoint() { + assertNotNull(stateHolder); + assertNull(stateHolder.getState("unknown.azconfig.io")); } + @Test + public void getStateFeatureFlagReturnsNullForUnknownEndpoint() { + assertNull(stateHolder.getStateFeatureFlag("unknown.azconfig.io")); + } } diff --git a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/timewindow/recurrence/RecurrenceEvaluator.java b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/timewindow/recurrence/RecurrenceEvaluator.java index a49837a0fe46..1b6db8a5965e 100644 --- a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/timewindow/recurrence/RecurrenceEvaluator.java +++ b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/timewindow/recurrence/RecurrenceEvaluator.java @@ -98,8 +98,26 @@ private static OccurrenceInfo getWeeklyPreviousOccurrence(TimeWindowFilterSettin final long numberOfInterval = Duration.between(firstDayOfFirstWeek, now).toSeconds() / Duration.ofDays((long) interval * RecurrenceConstants.DAYS_PER_WEEK).toSeconds(); - final ZonedDateTime firstDayOfMostRecentOccurringWeek = firstDayOfFirstWeek.plusDays( + + ZonedDateTime firstDayOfMostRecentOccurringWeek = firstDayOfFirstWeek.plusDays( numberOfInterval * (interval * RecurrenceConstants.DAYS_PER_WEEK)); + + // Handle DST transitions: If the calculated week start has a fixed offset (ZoneOffset) + // and 'now' has a region zone, check if they represent the same geographic location. + // We do this by checking if the fixed offset matches what the region zone's offset + // was at the *original start time*. If it matches, they're in the same timezone, + // and we should convert to the region zone for DST-aware comparisons. + if (firstDayOfMostRecentOccurringWeek.getZone() instanceof java.time.ZoneOffset + && !(now.getZone() instanceof java.time.ZoneOffset)) { + // Check if the fixed offset matches the region zone's offset at the *start* instant + // (not at firstDayOfMostRecentOccurringWeek's instant, which might have crossed DST) + java.time.ZoneOffset offsetAtStart = now.getZone().getRules().getOffset(start.toInstant()); + if (start.getOffset().equals(offsetAtStart)) { + // Same geographic location, convert to region zone for DST-aware comparisons + firstDayOfMostRecentOccurringWeek = firstDayOfMostRecentOccurringWeek + .withZoneSameLocal(now.getZone()); + } + } final List sortedDaysOfWeek = TimeWindowUtils.sortDaysOfWeek(pattern.getDaysOfWeek(), pattern.getFirstDayOfWeek()); final int maxDayOffset = TimeWindowUtils.getPassedWeekDays(sortedDaysOfWeek.get(sortedDaysOfWeek.size() - 1), pattern.getFirstDayOfWeek()); final int minDayOffset = TimeWindowUtils.getPassedWeekDays(sortedDaysOfWeek.get(0), pattern.getFirstDayOfWeek()); diff --git a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/timewindow/recurrence/RecurrenceValidator.java b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/timewindow/recurrence/RecurrenceValidator.java index 366e36394da5..5a058527c23a 100644 --- a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/timewindow/recurrence/RecurrenceValidator.java +++ b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/implementation/timewindow/recurrence/RecurrenceValidator.java @@ -9,6 +9,7 @@ import java.time.DayOfWeek; import java.time.Duration; +import java.time.ZoneOffset; import java.time.ZonedDateTime; import java.time.temporal.ChronoUnit; import java.util.List; @@ -135,7 +136,8 @@ private static boolean isDurationCompliantWithDaysOfWeek(TimeWindowFilterSetting } // Get the date of first day of the week - final ZonedDateTime today = ZonedDateTime.now(); + // Use UTC for minGap calculation to avoid DST issues (23-hour or 25-hour days) + final ZonedDateTime today = ZonedDateTime.now(ZoneOffset.UTC); final DayOfWeek firstDayOfWeek = settings.getRecurrence().getPattern().getFirstDayOfWeek(); final int offset = TimeWindowUtils.getPassedWeekDays(today.getDayOfWeek(), firstDayOfWeek); final ZonedDateTime firstDateOfWeek = today.minusDays(offset).truncatedTo(ChronoUnit.DAYS);