App Config Spring - Refresh Refactor

[mrm9084]

Description

• Refactors how the state is controlled to work with newer systems instead of being static code.
• Removes a few code warnings
• Now contains a feature management bug fix for the week of daylight savings time where there is a day with less than 24 hours.

All SDK Contribution checklist:

• The pull request does not introduce [breaking changes]
• [] CHANGELOG is updated for new features, bug fixes or other significant changes.
• I have read the contribution guidelines.

General Guidelines and Best Practices

• Title of the pull request is clear and informative.
• There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

• Pull request includes test coverage for the included changes.

[Copilot]

Pull request overview

This pull request refactors the state management in the Azure App Configuration Spring library, moving from a static singleton pattern to an instance-based approach managed by Spring's dependency injection framework.

Changes:

• Refactored StateHolder from a static singleton to an instance-based class registered in Spring's BootstrapContext and promoted to the ApplicationContext
• Updated AppConfigurationRefreshUtil to accept StateHolder as a constructor parameter instead of using static methods
• Modified tests to use instance-based StateHolder instead of static methods, splitting monolithic tests into focused unit tests
• Applied minor code quality improvements including replacing .size() > 0 with .isEmpty() and improving variable naming

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
StateHolder.java	Converted from package-private singleton to public instance-based class; removed static methods and singleton pattern
StateHolderTest.java	Refactored from single monolithic test to multiple focused tests using instance-based StateHolder
AppConfigurationRefreshUtil.java	Added StateHolder constructor parameter; converted static refresh methods to instance methods
AppConfigurationRefreshUtilTest.java	Updated to use mocked StateHolder instances instead of static method mocking
AppConfigurationPullRefresh.java	Added StateHolder constructor parameter and injected dependency
AppConfigurationPullRefreshTest.java	Updated constructor calls to include StateHolder mock; reorganized imports
AzureAppConfigBoostrapRegistrar.java	Added StateHolder registration in BootstrapContext with promotion to ApplicationContext
AzureAppConfigDataLoader.java	Modified to retrieve StateHolder from BootstrapContext instead of creating new instance
AppConfigurationWatchAutoConfiguration.java	Updated bean creation to retrieve StateHolder from BootstrapContext and pass to dependencies
ConnectionManager.java	Simplified client tracking by replacing currentReplica with lastActiveClient
ConfigStore.java	Improved variable naming (endpoint → validationEndpoint) and validation logic
AppConfigurationProperties.java	Replaced .size() > 0 with .isEmpty() for better code style
AppConfigurationReplicaClient.java	Replaced .size() > 0 with .isEmpty() for better code style
AppConfigurationApplicationSettingPropertySource.java	Added @OverRide annotation and removed unnecessary return statement from void method

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (2)

sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/StateHolder.java:156

• expireState dereferences oldState without a null check. If expireState is called for an endpoint that hasn't had setState(...) called yet (or state was cleared/never initialized), this will throw a NullPointerException. Add a guard (e.g., return early when oldState == null) so expiring an unknown endpoint is a no-op.

    public void expireState(String originEndpoint) {
        State oldState = state.get(originEndpoint);
        long wait = (long) (new SecureRandom().nextDouble() * MAX_JITTER);

        long timeLeft = (int) ((oldState.getNextRefreshCheck().toEpochMilli() - (Instant.now().toEpochMilli())) / 1000);
        if (wait < timeLeft) {

sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/StateHolder.java:195

• updateNextRefreshTime calls getNextRefreshCheck(nextForcedRefresh, ...) without handling nextForcedRefresh == null, which will NPE inside getNextRefreshCheck. Consider initializing nextForcedRefresh in the constructor (e.g., Instant.now()) and/or treating null as "expired" and setting a new value before calling getNextRefreshCheck.

    public void updateNextRefreshTime(Duration refreshInterval, Long defaultMinBackoff) {
        if (refreshInterval != null) {
            Instant newForcedRefresh = getNextRefreshCheck(nextForcedRefresh,
                clientRefreshAttempts, refreshInterval.getSeconds(), defaultMinBackoff);

            if (newForcedRefresh.compareTo(nextForcedRefresh) != 0) {
                clientRefreshAttempts += 1;
            }
            nextForcedRefresh = newForcedRefresh;
        }

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/StateHolder.java:197

• updateNextRefreshTime calls getNextRefreshCheck(nextForcedRefresh, ...) when refreshInterval != null, but nextForcedRefresh can be null (per getNextForcedRefresh() JavaDoc and when setNextForcedRefresh hasn’t been called). This can cause a NullPointerException inside getNextRefreshCheck (and also in newForcedRefresh.compareTo(nextForcedRefresh)). Consider initializing nextForcedRefresh when first needed (e.g., treat null as expired and set to Instant.now()/Instant.EPOCH), or guard null explicitly before calling getNextRefreshCheck.

    public void updateNextRefreshTime(Duration refreshInterval, Long defaultMinBackoff) {
        if (refreshInterval != null) {
            Instant newForcedRefresh = getNextRefreshCheck(nextForcedRefresh,
                clientRefreshAttempts, refreshInterval.getSeconds(), defaultMinBackoff);

            if (newForcedRefresh.compareTo(nextForcedRefresh) != 0) {
                clientRefreshAttempts += 1;
            }
            nextForcedRefresh = newForcedRefresh;