Skip to content

Commit e2beebe

Browse files
jonathannorrisjonathannorris
committed
fix: address review feedback and forward domain in MultiProvider
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
1 parent acea953 commit e2beebe

6 files changed

Lines changed: 121 additions & 150 deletions

File tree

‎.gitignore‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,3 @@ target
1515

1616
# used for spec compliance tooling
1717
java-report.json
18-
.worktrees/
19-
.worktrees/

‎src/main/java/dev/openfeature/sdk/ProviderRepository.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,8 @@ private void prepareAndInitializeProvider(
169169
if (isShuttingDown.get()) {
170170
throw new IllegalStateException("Provider cannot be set while repository is shutting down");
171171
}
172-
validateDomainScopedBinding(domain, newProvider);
173172
FeatureProviderStateManager existing = getExistingStateManagerForProvider(newProvider);
173+
validateDomainScopedBinding(domain, newProvider, existing);
174174
if (existing == null) {
175175
openFeatureAPI.registerGlobalProvider(newProvider);
176176
newStateManager = new FeatureProviderStateManager(newProvider);
@@ -196,11 +196,11 @@ private void prepareAndInitializeProvider(
196196
}
197197
}
198198

199-
private void validateDomainScopedBinding(@Nullable String domain, FeatureProvider newProvider) {
199+
private void validateDomainScopedBinding(
200+
@Nullable String domain, FeatureProvider newProvider, @Nullable FeatureProviderStateManager existing) {
200201
if (!newProvider.isDomainScoped()) {
201202
return;
202203
}
203-
FeatureProviderStateManager existing = getExistingStateManagerForProvider(newProvider);
204204
if (existing == null) {
205205
return;
206206
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public void initialize(EvaluationContext evaluationContext, @Nullable String dom
112112
Collection<Callable<Void>> tasks = new ArrayList<>(providers.size());
113113
for (FeatureProvider provider : providers.values()) {
114114
tasks.add(() -> {
115-
provider.initialize(evaluationContext, null);
115+
provider.initialize(evaluationContext, domain);
116116
return null;
117117
});
118118
Metadata providerMetadata = provider.getMetadata();

‎src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java‎

Lines changed: 73 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -22,80 +22,44 @@ class DefaultMethodDelegation {
2222
@Test
2323
@DisplayName("two-arg default delegates to a single-arg override")
2424
void twoArgDefaultDelegatesToSingleArgOverride() throws Exception {
25-
AtomicInteger singleArgInitCount = new AtomicInteger();
26-
FeatureProvider provider = new TestProvider() {
27-
@Override
28-
public void initialize(EvaluationContext evaluationContext) {
29-
singleArgInitCount.incrementAndGet();
30-
}
31-
};
25+
LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider();
3226

3327
provider.initialize(new ImmutableContext(), DOMAIN);
3428

35-
assertThat(singleArgInitCount).hasValue(1);
29+
assertThat(provider.singleArgInitCount()).isOne();
3630
}
3731

3832
@Test
3933
@DisplayName("two-arg override is used without invoking single-arg")
4034
void twoArgOverrideDoesNotInvokeSingleArg() throws Exception {
41-
AtomicInteger singleArgInitCount = new AtomicInteger();
42-
AtomicReference<String> domainReceived = new AtomicReference<>();
43-
FeatureProvider provider = new TestProvider() {
44-
@Override
45-
public void initialize(EvaluationContext evaluationContext, String domain) {
46-
domainReceived.set(domain);
47-
}
48-
49-
@Override
50-
public void initialize(EvaluationContext evaluationContext) {
51-
singleArgInitCount.incrementAndGet();
52-
}
53-
};
35+
TwoArgInitProvider provider = new TwoArgInitProvider();
5436

5537
provider.initialize(new ImmutableContext(), DOMAIN);
5638

57-
assertThat(domainReceived).hasValue(DOMAIN);
58-
assertThat(singleArgInitCount).hasValue(0);
39+
assertThat(provider.initDomain()).isEqualTo(DOMAIN);
40+
assertThat(provider.singleArgInitCount()).isZero();
5941
}
6042

6143
@Test
6244
@DisplayName("two-arg-only override receives null domain for default binding")
6345
void twoArgOnlyOverrideReceivesNullDomain() throws Exception {
64-
AtomicInteger singleArgInitCount = new AtomicInteger();
65-
AtomicReference<String> domainReceived = new AtomicReference<>("unset");
66-
FeatureProvider provider = new TestProvider() {
67-
@Override
68-
public void initialize(EvaluationContext evaluationContext, String domain) {
69-
domainReceived.set(domain);
70-
}
71-
72-
@Override
73-
public void initialize(EvaluationContext evaluationContext) {
74-
singleArgInitCount.incrementAndGet();
75-
}
76-
};
46+
TwoArgInitProvider provider = new TwoArgInitProvider();
7747

7848
provider.initialize(new ImmutableContext(), null);
7949

80-
assertThat(domainReceived.get()).isNull();
81-
assertThat(singleArgInitCount).hasValue(0);
50+
assertThat(provider.initDomain()).isNull();
51+
assertThat(provider.singleArgInitCount()).isZero();
8252
}
8353

8454
@Test
8555
@DisplayName("single-arg override is only invoked once per initialization")
8656
void singleArgOverrideIsOnlyInvokedOnce() throws Exception {
87-
AtomicInteger singleArgInitCount = new AtomicInteger();
88-
FeatureProvider provider = new TestProvider() {
89-
@Override
90-
public void initialize(EvaluationContext evaluationContext) {
91-
singleArgInitCount.incrementAndGet();
92-
}
93-
};
57+
LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider();
9458

9559
provider.initialize(new ImmutableContext(), DOMAIN);
9660
provider.initialize(new ImmutableContext(), DOMAIN);
9761

98-
assertThat(singleArgInitCount).hasValue(2);
62+
assertThat(provider.singleArgInitCount()).isEqualTo(2);
9963
}
10064
}
10165

@@ -114,84 +78,48 @@ void setupTest() {
11478
@Test
11579
@DisplayName("legacy single-arg provider is initialized when registered to a named domain")
11680
void legacySingleArgProviderInitializedForNamedDomain() {
117-
AtomicInteger singleArgInitCount = new AtomicInteger();
118-
FeatureProvider provider = new TestProvider() {
119-
@Override
120-
public void initialize(EvaluationContext evaluationContext) {
121-
singleArgInitCount.incrementAndGet();
122-
}
123-
};
81+
LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider();
12482

12583
api.setProvider(DOMAIN, provider);
12684

127-
await().atMost(Duration.ofSeconds(5))
128-
.untilAsserted(() -> assertThat(singleArgInitCount).hasValue(1));
85+
await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> assertThat(provider.singleArgInitCount())
86+
.isOne());
12987
}
13088

13189
@Test
13290
@DisplayName("legacy single-arg provider is initialized when registered as the default provider")
13391
void legacySingleArgProviderInitializedAsDefault() {
134-
AtomicInteger singleArgInitCount = new AtomicInteger();
135-
FeatureProvider provider = new TestProvider() {
136-
@Override
137-
public void initialize(EvaluationContext evaluationContext) {
138-
singleArgInitCount.incrementAndGet();
139-
}
140-
};
92+
LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider();
14193

14294
api.setProvider(provider);
14395

144-
await().atMost(Duration.ofSeconds(5))
145-
.untilAsserted(() -> assertThat(singleArgInitCount).hasValue(1));
96+
await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> assertThat(provider.singleArgInitCount())
97+
.isOne());
14698
}
14799

148100
@Test
149101
@DisplayName("two-arg-only provider receives the bound domain from the SDK")
150102
void twoArgOnlyProviderReceivesBoundDomainFromSdk() {
151-
AtomicInteger singleArgInitCount = new AtomicInteger();
152-
AtomicReference<String> domainReceived = new AtomicReference<>();
153-
FeatureProvider provider = new TestProvider() {
154-
@Override
155-
public void initialize(EvaluationContext evaluationContext, String domain) {
156-
domainReceived.set(domain);
157-
}
158-
159-
@Override
160-
public void initialize(EvaluationContext evaluationContext) {
161-
singleArgInitCount.incrementAndGet();
162-
}
163-
};
103+
TwoArgInitProvider provider = new TwoArgInitProvider();
164104

165105
api.setProvider(DOMAIN, provider);
166106

167107
await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> {
168-
assertThat(domainReceived).hasValue(DOMAIN);
169-
assertThat(singleArgInitCount).hasValue(0);
108+
assertThat(provider.initDomain()).isEqualTo(DOMAIN);
109+
assertThat(provider.singleArgInitCount()).isZero();
170110
});
171111
}
172112

173113
@Test
174114
@DisplayName("two-arg-only default provider receives null domain from the SDK")
175115
void twoArgOnlyDefaultProviderReceivesNullDomainFromSdk() {
176-
AtomicInteger singleArgInitCount = new AtomicInteger();
177-
AtomicReference<String> domainReceived = new AtomicReference<>("unset");
178-
FeatureProvider provider = new TestProvider() {
179-
@Override
180-
public void initialize(EvaluationContext evaluationContext, String domain) {
181-
domainReceived.set(domain);
182-
}
183-
184-
@Override
185-
public void initialize(EvaluationContext evaluationContext) {
186-
singleArgInitCount.incrementAndGet();
187-
}
188-
};
116+
TwoArgInitProvider provider = new TwoArgInitProvider();
189117

190118
api.setProvider(provider);
191119

192120
await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> {
193-
assertThat(domainReceived.get()).isNull();
194-
assertThat(singleArgInitCount).hasValue(0);
121+
assertThat(provider.initDomain()).isNull();
122+
assertThat(provider.singleArgInitCount()).isZero();
195123
});
196124
}
197125
}
@@ -203,40 +131,74 @@ class StateManager {
203131
@Test
204132
@DisplayName("delegates two-arg initialize to a legacy single-arg provider")
205133
void delegatesTwoArgInitializeToLegacySingleArgProvider() throws Exception {
206-
AtomicInteger singleArgInitCount = new AtomicInteger();
207-
FeatureProvider provider = new TestProvider() {
208-
@Override
209-
public void initialize(EvaluationContext evaluationContext) {
210-
singleArgInitCount.incrementAndGet();
211-
}
212-
};
134+
LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider();
213135
FeatureProviderStateManager stateManager = new FeatureProviderStateManager(provider);
214136

215137
stateManager.initialize(new ImmutableContext(), DOMAIN);
216138

217-
assertThat(singleArgInitCount).hasValue(1);
139+
assertThat(provider.singleArgInitCount()).isOne();
218140
}
219141

220142
@Test
221143
@DisplayName("only invokes two-arg initialize once for a legacy single-arg provider")
222144
void onlyInvokesLegacySingleArgProviderOncePerStateManagerInit() throws Exception {
223-
AtomicInteger singleArgInitCount = new AtomicInteger();
224-
FeatureProvider provider = new TestProvider() {
225-
@Override
226-
public void initialize(EvaluationContext evaluationContext) {
227-
singleArgInitCount.incrementAndGet();
228-
}
229-
};
145+
LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider();
230146
FeatureProviderStateManager stateManager = new FeatureProviderStateManager(provider);
231147

232148
stateManager.initialize(new ImmutableContext(), DOMAIN);
233149
stateManager.initialize(new ImmutableContext(), DOMAIN);
234150

235-
assertThat(singleArgInitCount).hasValue(1);
151+
assertThat(provider.singleArgInitCount()).isOne();
236152
}
237153
}
238154

239-
private abstract static class TestProvider extends EventProvider {
155+
/**
156+
* Legacy provider that only overrides single-arg {@link FeatureProvider#initialize(EvaluationContext)}.
157+
*/
158+
private static final class LegacySingleArgInitProvider extends StubProvider {
159+
160+
private final AtomicInteger singleArgInitCount = new AtomicInteger();
161+
162+
@Override
163+
public void initialize(EvaluationContext evaluationContext) {
164+
singleArgInitCount.incrementAndGet();
165+
}
166+
167+
int singleArgInitCount() {
168+
return singleArgInitCount.get();
169+
}
170+
}
171+
172+
/**
173+
* Domain-aware provider that only overrides two-arg
174+
* {@link FeatureProvider#initialize(EvaluationContext, String)}. Single-arg is overridden solely to detect
175+
* accidental delegation.
176+
*/
177+
private static final class TwoArgInitProvider extends StubProvider {
178+
179+
private final AtomicInteger singleArgInitCount = new AtomicInteger();
180+
private final AtomicReference<String> initDomain = new AtomicReference<>();
181+
182+
@Override
183+
public void initialize(EvaluationContext evaluationContext, String domain) {
184+
initDomain.set(domain);
185+
}
186+
187+
@Override
188+
public void initialize(EvaluationContext evaluationContext) {
189+
singleArgInitCount.incrementAndGet();
190+
}
191+
192+
int singleArgInitCount() {
193+
return singleArgInitCount.get();
194+
}
195+
196+
String initDomain() {
197+
return initDomain.get();
198+
}
199+
}
200+
201+
private abstract static class StubProvider extends EventProvider {
240202

241203
@Override
242204
public Metadata getMetadata() {

‎src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,15 @@
11
package dev.openfeature.sdk.fixtures;
22

3-
import static dev.openfeature.sdk.testutils.stubbing.ConditionStubber.doBlock;
43
import static org.mockito.ArgumentMatchers.any;
54
import static org.mockito.ArgumentMatchers.nullable;
6-
import static org.mockito.Mockito.doAnswer;
75
import static org.mockito.Mockito.doReturn;
86
import static org.mockito.Mockito.doThrow;
97
import static org.mockito.Mockito.mock;
108

119
import dev.openfeature.sdk.FeatureProvider;
12-
import dev.openfeature.sdk.ImmutableContext;
1310
import dev.openfeature.sdk.ProviderState;
1411
import java.io.FileNotFoundException;
15-
import java.util.concurrent.CountDownLatch;
1612
import lombok.experimental.UtilityClass;
17-
import org.mockito.stubbing.Answer;
1813

1914
@UtilityClass
2015
public class ProviderFixture {
@@ -37,32 +32,4 @@ public static FeatureProvider createMockedErrorProvider() throws Exception {
3732
doThrow(FileNotFoundException.class).when(provider).initialize(any(), nullable(String.class));
3833
return provider;
3934
}
40-
41-
public static FeatureProvider createBlockedProvider(CountDownLatch latch, Runnable onAnswer) throws Exception {
42-
FeatureProvider provider = createMockedProvider();
43-
doBlock(latch, createAnswerExecutingCode(onAnswer))
44-
.when(provider)
45-
.initialize(new ImmutableContext(), nullable(String.class));
46-
doReturn("blockedProvider").when(provider).toString();
47-
return provider;
48-
}
49-
50-
private static Answer<?> createAnswerExecutingCode(Runnable onAnswer) {
51-
return invocation -> {
52-
onAnswer.run();
53-
return null;
54-
};
55-
}
56-
57-
public static FeatureProvider createUnblockingProvider(CountDownLatch latch) throws Exception {
58-
FeatureProvider provider = createMockedProvider();
59-
doAnswer(invocation -> {
60-
latch.countDown();
61-
return null;
62-
})
63-
.when(provider)
64-
.initialize(new ImmutableContext(), nullable(String.class));
65-
doReturn("unblockingProvider").when(provider).toString();
66-
return provider;
67-
}
6835
}

0 commit comments

Comments
 (0)