Skip to content

Commit fc8c18f

Browse files
suvaidkhansuvaidkhan
committed
Multiprovider: fixed log messages and exception handling
Signed-off-by: suvaidkhan <[email protected]>
1 parent 969bbbf commit fc8c18f

File tree

6 files changed

+54
-52
lines changed

6 files changed

+54
-52
lines changed

‎src/main/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategy.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static dev.openfeature.sdk.ErrorCode.FLAG_NOT_FOUND;
44

5+
import dev.openfeature.sdk.ErrorCode;
56
import dev.openfeature.sdk.EvaluationContext;
67
import dev.openfeature.sdk.FeatureProvider;
78
import dev.openfeature.sdk.ProviderEvaluation;
@@ -45,9 +46,12 @@ public <T> ProviderEvaluation<T> evaluate(
4546
return res;
4647
}
4748
} catch (FlagNotFoundError e) {
48-
log.debug("flag not found {}", e.getMessage());
49+
log.debug("flag not found {}", key, e);
4950
}
5051
}
51-
throw new FlagNotFoundError("flag not found");
52+
return ProviderEvaluation.<T>builder()
53+
.errorMessage("No provider successfully responded")
54+
.errorCode(ErrorCode.GENERAL)
55+
.build();
5256
}
5357
}

‎src/main/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategy.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
package dev.openfeature.sdk.multiprovider;
22

3+
import dev.openfeature.sdk.ErrorCode;
34
import dev.openfeature.sdk.EvaluationContext;
45
import dev.openfeature.sdk.FeatureProvider;
56
import dev.openfeature.sdk.ProviderEvaluation;
6-
import dev.openfeature.sdk.exceptions.GeneralError;
77
import java.util.Map;
88
import java.util.function.Function;
99
import lombok.NoArgsConstructor;
@@ -32,10 +32,13 @@ public <T> ProviderEvaluation<T> evaluate(
3232
return res;
3333
}
3434
} catch (Exception e) {
35-
log.debug("evaluation exception {}", e.getMessage());
35+
log.debug("evaluation exception {}", key, e);
3636
}
3737
}
3838

39-
throw new GeneralError("evaluation error");
39+
return ProviderEvaluation.<T>builder()
40+
.errorMessage("No provider successfully responded")
41+
.errorCode(ErrorCode.GENERAL)
42+
.build();
4043
}
4144
}

‎src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
8181
var metadataBuilder = MultiProviderMetadata.builder();
8282
metadataBuilder.name(NAME);
8383
HashMap<String, Metadata> providersMetadata = new HashMap<>();
84-
ExecutorService initPool = Executors.newFixedThreadPool(Math.min(INIT_THREADS_COUNT, providers.size()));
84+
ExecutorService executorService = Executors.newFixedThreadPool(Math.min(INIT_THREADS_COUNT, providers.size()));
8585
Collection<Callable<Boolean>> tasks = new ArrayList<>(providers.size());
8686
for (FeatureProvider provider : providers.values()) {
8787
tasks.add(() -> {
@@ -92,14 +92,14 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
9292
providersMetadata.put(providerMetadata.getName(), providerMetadata);
9393
}
9494
metadataBuilder.originalMetadata(providersMetadata);
95-
List<Future<Boolean>> results = initPool.invokeAll(tasks);
95+
List<Future<Boolean>> results = executorService.invokeAll(tasks);
9696
for (Future<Boolean> result : results) {
9797
if (!result.get()) {
98-
initPool.shutdown();
98+
executorService.shutdown();
9999
throw new GeneralError("init failed");
100100
}
101101
}
102-
initPool.shutdown();
102+
executorService.shutdown();
103103
metadata = metadataBuilder.build();
104104
}
105105

‎src/test/java/dev/openfeature/sdk/multiProvider/FirstMatchStrategyTest.java

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,9 @@
33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertNotNull;
55
import static org.junit.jupiter.api.Assertions.assertNull;
6-
import static org.junit.jupiter.api.Assertions.assertThrows;
76

87
import dev.openfeature.sdk.ErrorCode;
98
import dev.openfeature.sdk.ProviderEvaluation;
10-
import dev.openfeature.sdk.exceptions.FlagNotFoundError;
119
import dev.openfeature.sdk.multiprovider.FirstMatchStrategy;
1210
import org.junit.jupiter.api.Test;
1311

@@ -42,8 +40,6 @@ void shouldReturnFirstNonFlagNotFoundError() {
4240
DEFAULT_STRING,
4341
null,
4442
p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null));
45-
46-
assertNotNull(result);
4743
assertEquals(ErrorCode.PARSE_ERROR, result.getErrorCode());
4844
}
4945

@@ -68,16 +64,15 @@ void shouldThrowFlagNotFoundWhenAllProvidersReturnFlagNotFound() {
6864
setupProviderFlagNotFound(mockProvider1);
6965
setupProviderFlagNotFound(mockProvider2);
7066
setupProviderFlagNotFound(mockProvider3);
71-
FlagNotFoundError exception = assertThrows(FlagNotFoundError.class, () -> {
72-
strategy.evaluate(
73-
orderedProviders,
74-
FLAG_KEY,
75-
DEFAULT_STRING,
76-
null,
77-
p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null));
78-
});
67+
ProviderEvaluation<String> providerEvaluation = strategy.evaluate(
68+
orderedProviders,
69+
FLAG_KEY,
70+
DEFAULT_STRING,
71+
null,
72+
p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null));
7973

80-
assertEquals("flag not found", exception.getMessage());
74+
assertEquals(ErrorCode.GENERAL, providerEvaluation.getErrorCode());
75+
assertEquals("No provider successfully responded", providerEvaluation.getErrorMessage());
8176
}
8277

8378
@Test

‎src/test/java/dev/openfeature/sdk/multiProvider/FirstSuccessfulStrategyTest.java

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,9 @@
33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertNotNull;
55
import static org.junit.jupiter.api.Assertions.assertNull;
6-
import static org.junit.jupiter.api.Assertions.assertThrows;
76

87
import dev.openfeature.sdk.ErrorCode;
98
import dev.openfeature.sdk.ProviderEvaluation;
10-
import dev.openfeature.sdk.exceptions.GeneralError;
119
import dev.openfeature.sdk.multiprovider.FirstSuccessfulStrategy;
1210
import org.junit.jupiter.api.Test;
1311

@@ -35,16 +33,15 @@ void shouldThrowGeneralErrorWhenAllProvidersFail() {
3533
setupProviderFlagNotFound(mockProvider1);
3634
setupProviderError(mockProvider2, ErrorCode.PARSE_ERROR);
3735
setupProviderError(mockProvider3, ErrorCode.TYPE_MISMATCH);
38-
GeneralError exception = assertThrows(GeneralError.class, () -> {
39-
strategy.evaluate(
40-
orderedProviders,
41-
FLAG_KEY,
42-
DEFAULT_STRING,
43-
null,
44-
p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null));
45-
});
36+
ProviderEvaluation<String> providerEvaluation = strategy.evaluate(
37+
orderedProviders,
38+
FLAG_KEY,
39+
DEFAULT_STRING,
40+
null,
41+
p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null));
4642

47-
assertEquals("evaluation error", exception.getMessage());
43+
assertEquals(ErrorCode.GENERAL, providerEvaluation.getErrorCode());
44+
assertEquals("No provider successfully responded", providerEvaluation.getErrorMessage());
4845
}
4946

5047
@Test
@@ -53,28 +50,30 @@ void shouldSkipProvidersThatOnlyReturnErrors() {
5350
setupProviderError(mockProvider2, ErrorCode.PROVIDER_NOT_READY);
5451
setupProviderError(mockProvider3, ErrorCode.GENERAL);
5552

56-
assertThrows(GeneralError.class, () -> {
57-
strategy.evaluate(
58-
orderedProviders,
59-
FLAG_KEY,
60-
DEFAULT_STRING,
61-
null,
62-
p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null));
63-
});
53+
ProviderEvaluation<String> providerEvaluation = strategy.evaluate(
54+
orderedProviders,
55+
FLAG_KEY,
56+
DEFAULT_STRING,
57+
null,
58+
p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null));
59+
60+
assertEquals(ErrorCode.GENERAL, providerEvaluation.getErrorCode());
61+
assertEquals("No provider successfully responded", providerEvaluation.getErrorMessage());
6462
}
6563

6664
@Test
6765
void shouldThrowGeneralErrorForNonExistentFlag() {
6866
orderedProviders.clear();
6967
orderedProviders.put("old-provider", inMemoryProvider1);
7068
orderedProviders.put("new-provider", inMemoryProvider2);
71-
assertThrows(GeneralError.class, () -> {
72-
strategy.evaluate(
73-
orderedProviders,
74-
"non-existent-flag",
75-
DEFAULT_STRING,
76-
null,
77-
p -> p.getStringEvaluation("non-existent-flag", DEFAULT_STRING, null));
78-
});
69+
ProviderEvaluation<String> providerEvaluation = strategy.evaluate(
70+
orderedProviders,
71+
FLAG_KEY,
72+
DEFAULT_STRING,
73+
null,
74+
p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null));
75+
76+
assertEquals(ErrorCode.GENERAL, providerEvaluation.getErrorCode());
77+
assertEquals("No provider successfully responded", providerEvaluation.getErrorMessage());
7978
}
8079
}

‎src/test/java/dev/openfeature/sdk/multiProvider/MultiProviderTest.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@
99
import static org.mockito.Mockito.mock;
1010
import static org.mockito.Mockito.when;
1111

12+
import dev.openfeature.sdk.ErrorCode;
1213
import dev.openfeature.sdk.EvaluationContext;
1314
import dev.openfeature.sdk.FeatureProvider;
1415
import dev.openfeature.sdk.Metadata;
1516
import dev.openfeature.sdk.MutableContext;
1617
import dev.openfeature.sdk.ProviderEvaluation;
1718
import dev.openfeature.sdk.Value;
18-
import dev.openfeature.sdk.exceptions.FlagNotFoundError;
1919
import dev.openfeature.sdk.exceptions.GeneralError;
2020
import dev.openfeature.sdk.multiprovider.FirstMatchStrategy;
2121
import dev.openfeature.sdk.multiprovider.MultiProvider;
@@ -80,7 +80,6 @@ void shouldRetrieveCorrectMetadataName() {
8080
Strategy mockStrategy = mock(Strategy.class);
8181
MultiProvider multiProvider = new MultiProvider(providers, mockStrategy);
8282
multiProvider.initialize(null);
83-
assertNotNull(multiProvider);
8483
MultiProviderMetadata metadata = (MultiProviderMetadata) multiProvider.getMetadata();
8584
Map<String, Metadata> map = metadata.getOriginalMetadata();
8685
assertEquals(mockMetaData1, map.get(mockProvider1.getMetadata().getName()));
@@ -101,7 +100,9 @@ void shouldUseDefaultFirstMatchStrategy() {
101100
assertEquals(
102101
"v1",
103102
multiProvider.getObjectEvaluation("o1", null, null).getValue().asString());
104-
assertThrows(FlagNotFoundError.class, () -> multiProvider.getStringEvaluation("non-existing", "", null));
103+
ProviderEvaluation<String> providerEvaluation = multiProvider.getStringEvaluation("non-existing", "", null);
104+
assertEquals(ErrorCode.GENERAL, providerEvaluation.getErrorCode());
105+
assertEquals("No provider successfully responded", providerEvaluation.getErrorMessage());
105106
}
106107

107108
@SneakyThrows

0 commit comments

Comments
 (0)