Skip to content

Commit 7ce44e5

Browse files
mrm9084Copilot
mrm9084
and
Copilot
authored
6.0.0 bug fixes (#47225)
* Adds safety for immutable profiles * Fixing unknown exception * Update sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationKeyValueSelectorTest.java Co-authored-by: Copilot <[email protected]> * Update sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java Co-authored-by: Copilot <[email protected]> * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * Update AzureAppConfigDataLoader.java * fix copilot change * fix linting * Update AzureAppConfigDataResourceTest.java --------- Co-authored-by: Copilot <[email protected]>
1 parent 6773d8e commit 7ce44e5

File tree

6 files changed

+283
-14
lines changed

6 files changed

+283
-14
lines changed

‎sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ public ConfigData load(ConfigDataLoaderContext context, AzureAppConfigDataResour
185185
} catch (Exception e) {
186186
// Store the exception to potentially use if all replicas fail
187187
lastException = e; // Log the specific replica failure with context
188+
replicaClientFactory.backoffClient(resource.getEndpoint(), currentClient.getEndpoint());
188189
AppConfigurationReplicaClient nextClient = replicaClientFactory
189190
.getNextActiveClient(resource.getEndpoint(), false);
190191
logReplicaFailure(currentClient, "exception", nextClient != null, e);

‎sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataResource.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@ public class AzureAppConfigDataResource extends ConfigDataResource {
5252
*
5353
* @param configStore the configuration store settings containing endpoint, selectors, and other options
5454
* @param profiles the Spring Boot profiles for conditional configuration loading
55-
* @param isRefresh whether this resource supports runtime configuration refresh
55+
* @param startup true if this is a startup load operation, false if it is a refresh operation
5656
* @param refreshInterval the interval at which configuration should be refreshed
5757
*/
58-
AzureAppConfigDataResource(boolean appConfigEnabled, ConfigStore configStore, Profiles profiles, boolean isRefresh,
58+
AzureAppConfigDataResource(boolean appConfigEnabled, ConfigStore configStore, Profiles profiles, boolean startup,
5959
Duration refreshInterval) {
6060
this.configStoreEnabled = appConfigEnabled && configStore.isEnabled();
6161
this.endpoint = configStore.getEndpoint();
@@ -64,7 +64,7 @@ public class AzureAppConfigDataResource extends ConfigDataResource {
6464
this.trimKeyPrefix = configStore.getTrimKeyPrefix();
6565
this.monitoring = configStore.getMonitoring();
6666
this.profiles = profiles;
67-
this.isRefresh = isRefresh;
67+
this.isRefresh = !startup;
6868
this.refreshInterval = refreshInterval;
6969
}
7070

‎sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ void backoffClient(String endpoint) {
232232
int failedAttempt = client.getFailedAttempts();
233233
long backoffTime = BackoffTimeCalculator.calculateBackoff(failedAttempt);
234234
client.updateBackoffEndTime(Instant.now().plusNanos(backoffTime));
235+
activeClients.removeIf(removeClient -> removeClient.getEndpoint().equals(endpoint));
235236
return;
236237
}
237238
}

‎sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationKeyValueSelector.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
// Licensed under the MIT License.
33
package com.azure.spring.cloud.appconfiguration.config.implementation.properties;
44

5-
import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.EMPTY_LABEL;
6-
5+
import java.util.ArrayList;
76
import java.util.Arrays;
87
import java.util.Collections;
98
import java.util.List;
@@ -12,6 +11,8 @@
1211
import org.springframework.util.Assert;
1312
import org.springframework.util.StringUtils;
1413

14+
import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.EMPTY_LABEL;
15+
1516
import jakarta.annotation.PostConstruct;
1617
import jakarta.validation.constraints.NotNull;
1718

@@ -80,12 +81,18 @@ public AppConfigurationKeyValueSelector setKeyFilter(String keyFilter) {
8081
* latter label has higher priority
8182
*/
8283
public String[] getLabelFilter(List<String> profiles) {
83-
if (labelFilter == null && profiles.size() > 0) {
84-
Collections.reverse(profiles);
85-
return profiles.toArray(new String[profiles.size()]);
86-
} else if (StringUtils.hasText(snapshotName)) {
84+
if (StringUtils.hasText(snapshotName)) {
8785
return new String[0];
88-
} else if (!StringUtils.hasText(labelFilter)) {
86+
}
87+
if (labelFilter == null && !profiles.isEmpty()) {
88+
List<String> mutableProfiles = new ArrayList<>(profiles);
89+
// Defensive copy: profiles may be immutable when provided by certain Spring Boot contexts,
90+
// such as when obtained from Environment.getActiveProfiles(). See
91+
// https://github.com/Azure/azure-sdk-for-java/issues/32708 for details.
92+
Collections.reverse(mutableProfiles);
93+
return mutableProfiles.toArray(new String[mutableProfiles.size()]);
94+
}
95+
if (!StringUtils.hasText(labelFilter)) {
8996
return EMPTY_LABEL_ARRAY;
9097
}
9198

‎sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataResourceTest.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ void testEnabledStateWithRefreshScenarios(boolean isRefresh, String scenarioDesc
7575

7676
assertTrue(resource.isConfigStoreEnabled(),
7777
"Config store should be enabled in " + scenarioDescription + " when conditions are met");
78-
assertEquals(isRefresh, resource.isRefresh(),
78+
// You pass in startup, but it becomes is refresh
79+
assertEquals(!isRefresh, resource.isRefresh(),
7980
"Should correctly report refresh state for " + scenarioDescription);
8081
}
8182

@@ -93,14 +94,14 @@ void testAllPropertiesSetCorrectlyRegardlessOfEnabledState() {
9394
true, configStore, mockProfiles, false, TEST_REFRESH_INTERVAL);
9495

9596
assertTrue(enabledResource.isConfigStoreEnabled());
96-
assertAllPropertiesCorrect(enabledResource, trimKeyPrefixes, selects, featureFlagSelects, false);
97+
assertAllPropertiesCorrect(enabledResource, trimKeyPrefixes, selects, featureFlagSelects, true);
9798

9899
configStore.setEnabled(false);
99100
AzureAppConfigDataResource disabledResource = new AzureAppConfigDataResource(
100101
true, configStore, mockProfiles, true, TEST_REFRESH_INTERVAL);
101102

102103
assertFalse(disabledResource.isConfigStoreEnabled());
103-
assertAllPropertiesCorrect(disabledResource, trimKeyPrefixes, selects, featureFlagSelects, true);
104+
assertAllPropertiesCorrect(disabledResource, trimKeyPrefixes, selects, featureFlagSelects, false);
104105
}
105106

106107
private void assertAllPropertiesCorrect(AzureAppConfigDataResource resource,
@@ -131,7 +132,7 @@ void testNullRefreshIntervalHandling() {
131132
false, configStore, mockProfiles, true, null);
132133
assertFalse(disabledResource.isConfigStoreEnabled());
133134
assertNull(disabledResource.getRefreshInterval());
134-
assertTrue(disabledResource.isRefresh());
135+
assertFalse(disabledResource.isRefresh());
135136
}
136137

137138
}

‎sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationKeyValueSelectorTest.java‎

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
package com.azure.spring.cloud.appconfiguration.config.implementation.properties;
4+
5+
import java.util.ArrayList;
6+
import java.util.Arrays;
7+
import java.util.List;
8+
9+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
10+
import static org.junit.jupiter.api.Assertions.assertEquals;
11+
import static org.junit.jupiter.api.Assertions.assertThrows;
12+
import static org.junit.jupiter.api.Assertions.assertTrue;
13+
import org.junit.jupiter.api.BeforeEach;
14+
import org.junit.jupiter.api.Test;
15+
16+
import static com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationConstants.EMPTY_LABEL;
17+
18+
public class AppConfigurationKeyValueSelectorTest {
19+
20+
private AppConfigurationKeyValueSelector selector;
21+
22+
@BeforeEach
23+
public void setup() {
24+
selector = new AppConfigurationKeyValueSelector();
25+
}
26+
27+
@Test
28+
public void getKeyFilterDefaultTest() {
29+
// When no key filter is set, should return default
30+
String keyFilter = selector.getKeyFilter();
31+
assertEquals("/application/", keyFilter);
32+
}
33+
34+
@Test
35+
public void getKeyFilterCustomTest() {
36+
// When custom key filter is set
37+
selector.setKeyFilter("/custom/");
38+
String keyFilter = selector.getKeyFilter();
39+
assertEquals("/custom/", keyFilter);
40+
}
41+
42+
@Test
43+
public void getKeyFilterEmptyTest() {
44+
// When empty key filter is set, should return default
45+
selector.setKeyFilter("");
46+
String keyFilter = selector.getKeyFilter();
47+
assertEquals("/application/", keyFilter);
48+
}
49+
50+
@Test
51+
public void getLabelFilterWithProfilesTest() {
52+
// Test with profiles when labelFilter is null
53+
List<String> profiles = new ArrayList<>(Arrays.asList("dev", "prod", "staging"));
54+
55+
String[] result = selector.getLabelFilter(profiles);
56+
57+
// Should be reversed: staging, prod, dev
58+
assertArrayEquals(new String[]{"staging", "prod", "dev"}, result);
59+
}
60+
61+
@Test
62+
public void getLabelFilterWithEmptyProfilesTest() {
63+
// Test with empty profiles when labelFilter is null
64+
List<String> profiles = new ArrayList<>();
65+
66+
String[] result = selector.getLabelFilter(profiles);
67+
68+
// Should return empty label array
69+
assertArrayEquals(new String[]{EMPTY_LABEL}, result);
70+
}
71+
72+
@Test
73+
public void getLabelFilterWithImmutableProfilesTest() {
74+
// Test with immutable profiles list (simulates the UnsupportedOperationException scenario)
75+
List<String> profiles = List.of("dev", "prod"); // Immutable list
76+
77+
String[] result = selector.getLabelFilter(profiles);
78+
79+
// Should handle immutable list and return reversed: prod, dev
80+
assertArrayEquals(new String[]{"prod", "dev"}, result);
81+
}
82+
83+
@Test
84+
public void getLabelFilterWithSnapshotTest() {
85+
// Test when snapshot is set
86+
selector.setSnapshotName("test-snapshot");
87+
List<String> profiles = Arrays.asList("dev", "prod");
88+
89+
String[] result = selector.getLabelFilter(profiles);
90+
91+
// Should return empty array when snapshot is set
92+
assertArrayEquals(new String[0], result);
93+
}
94+
95+
@Test
96+
public void getLabelFilterWithCustomLabelFilterTest() {
97+
// Test with custom label filter
98+
selector.setLabelFilter("dev,prod,staging");
99+
List<String> profiles = Arrays.asList("ignored");
100+
101+
String[] result = selector.getLabelFilter(profiles);
102+
103+
// Should be reversed and trimmed
104+
assertArrayEquals(new String[]{"staging", "prod", "dev"}, result);
105+
}
106+
107+
@Test
108+
public void getLabelFilterWithLabelFilterTrailingCommaTest() {
109+
// Test with trailing comma in label filter
110+
selector.setLabelFilter("dev,prod,");
111+
List<String> profiles = Arrays.asList("ignored");
112+
113+
String[] result = selector.getLabelFilter(profiles);
114+
115+
// Should include empty label at the end after reversing
116+
assertArrayEquals(new String[]{EMPTY_LABEL, "prod", "dev"}, result);
117+
}
118+
119+
@Test
120+
public void getLabelFilterWithSpacesTest() {
121+
// Test label filter with spaces (should be trimmed)
122+
selector.setLabelFilter(" dev , prod , staging ");
123+
List<String> profiles = Arrays.asList("ignored");
124+
125+
String[] result = selector.getLabelFilter(profiles);
126+
127+
// Should be reversed and trimmed
128+
assertArrayEquals(new String[]{"staging", "prod", "dev"}, result);
129+
}
130+
131+
@Test
132+
public void getLabelFilterWithDuplicatesTest() {
133+
// Test label filter with duplicates (should be distinct)
134+
selector.setLabelFilter("dev,prod,dev,staging,prod");
135+
List<String> profiles = Arrays.asList("ignored");
136+
137+
String[] result = selector.getLabelFilter(profiles);
138+
139+
// Should be reversed, trimmed, and distinct
140+
assertArrayEquals(new String[]{"staging", "prod", "dev"}, result);
141+
}
142+
143+
@Test
144+
public void getLabelFilterWithEmptyLabelsTest() {
145+
// Test label filter with empty labels
146+
selector.setLabelFilter("dev,,prod,");
147+
List<String> profiles = Arrays.asList("ignored");
148+
149+
String[] result = selector.getLabelFilter(profiles);
150+
151+
// Should match the expected order and duplicates, including EMPTY_LABEL
152+
assertArrayEquals(new String[]{EMPTY_LABEL, "prod", EMPTY_LABEL, "dev"}, result);
153+
}
154+
155+
@Test
156+
public void validateAndInitValidConfigurationTest() {
157+
// Test valid configuration
158+
selector.setKeyFilter("/valid/");
159+
selector.setLabelFilter("dev,prod");
160+
161+
// Should not throw any exception
162+
selector.validateAndInit();
163+
}
164+
165+
@Test
166+
public void validateAndInitInvalidKeyFilterTest() {
167+
// Test invalid key filter with asterisk
168+
selector.setKeyFilter("/invalid*filter/");
169+
170+
assertThrows(IllegalArgumentException.class, () -> {
171+
selector.validateAndInit();
172+
});
173+
}
174+
175+
@Test
176+
public void validateAndInitInvalidLabelFilterTest() {
177+
// Test invalid label filter with asterisk
178+
selector.setLabelFilter("dev*,prod");
179+
180+
assertThrows(IllegalArgumentException.class, () -> {
181+
selector.validateAndInit();
182+
});
183+
}
184+
185+
@Test
186+
public void validateAndInitSnapshotWithKeyFilterTest() {
187+
// Test snapshot with key filter (should fail)
188+
selector.setKeyFilter("/test/");
189+
selector.setSnapshotName("test-snapshot");
190+
191+
assertThrows(IllegalArgumentException.class, () -> {
192+
selector.validateAndInit();
193+
});
194+
}
195+
196+
@Test
197+
public void validateAndInitSnapshotWithLabelFilterTest() {
198+
// Test snapshot with label filter (should fail)
199+
selector.setLabelFilter("dev");
200+
selector.setSnapshotName("test-snapshot");
201+
202+
assertThrows(IllegalArgumentException.class, () -> {
203+
selector.validateAndInit();
204+
});
205+
}
206+
207+
@Test
208+
public void validateAndInitValidSnapshotTest() {
209+
// Test valid snapshot configuration (no key or label filters)
210+
selector.setSnapshotName("test-snapshot");
211+
212+
// Should not throw any exception
213+
selector.validateAndInit();
214+
}
215+
216+
@Test
217+
public void mapLabelNullTest() {
218+
// Test the private mapLabel method indirectly through getLabelFilter
219+
selector.setLabelFilter("dev,,prod");
220+
List<String> profiles = Arrays.asList("ignored");
221+
222+
String[] result = selector.getLabelFilter(profiles);
223+
224+
// Empty label should be converted to EMPTY_LABEL constant
225+
assertTrue(Arrays.asList(result).contains(EMPTY_LABEL));
226+
}
227+
228+
@Test
229+
public void complexScenarioTest() {
230+
// Test a complex real-world scenario
231+
List<String> profiles = new ArrayList<>(Arrays.asList("test", "dev", "prod"));
232+
233+
// First call with profiles
234+
String[] profileResult = selector.getLabelFilter(profiles);
235+
assertArrayEquals(new String[]{"prod", "dev", "test"}, profileResult);
236+
237+
// Then set a custom label filter
238+
selector.setLabelFilter("custom1, custom2 ,custom3,");
239+
String[] customResult = selector.getLabelFilter(profiles);
240+
assertArrayEquals(new String[]{EMPTY_LABEL, "custom3", "custom2", "custom1"}, customResult);
241+
242+
// Finally set a snapshot (should override everything)
243+
selector.setSnapshotName("final-snapshot");
244+
String[] snapshotResult = selector.getLabelFilter(profiles);
245+
assertArrayEquals(new String[0], snapshotResult);
246+
}
247+
248+
@Test
249+
public void profilesListModificationSafetyTest() {
250+
// Test that the original profiles list is not modified
251+
List<String> originalProfiles = new ArrayList<>(Arrays.asList("a", "b", "c"));
252+
List<String> profilesCopy = new ArrayList<>(originalProfiles);
253+
254+
selector.getLabelFilter(originalProfiles);
255+
256+
// Original list should remain unchanged
257+
assertEquals(profilesCopy, originalProfiles);
258+
}
259+
}

0 commit comments

Comments
 (0)