Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ target

# used for spec compliance tooling
java-report.json
.worktrees/
32 changes: 32 additions & 0 deletions src/main/java/dev/openfeature/sdk/FeatureProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,38 @@ default void initialize(EvaluationContext evaluationContext) throws Exception {
// Intentionally left blank
}

/**
* This method is called before a provider is used to evaluate flags, with the
* bound domain supplied when the provider is registered to a named client.
*
* <p>
* The default provider is initialized with a {@code null} domain. Providers that
* maintain per-domain state (for example a persistent cache) should override this
* method and declare themselves {@linkplain #isDomainScoped() domain-scoped}.
* </p>
*
* @param evaluationContext the global evaluation context
* @param domain the bound domain, or {@code null} for the default provider
*/
default void initialize(EvaluationContext evaluationContext, String domain) throws Exception {
initialize(evaluationContext);
}

/**
* Returns whether this provider maintains state specific to a single domain that
* cannot be shared across domains.
*
* <p>
* Domain-scoped providers may only be bound to one domain within a single API
* instance.
* </p>
*
* @return {@code true} if this provider is domain-scoped
*/
default boolean isDomainScoped() {
return false;
}

/**
* Called when a provider is about to be replaced or the SDK is shutting down. Providers can
* override this method if they have resources to release (background threads, connections,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ public FeatureProviderStateManager(FeatureProvider delegate) {
}

public void initialize(EvaluationContext evaluationContext) throws Exception {
initialize(evaluationContext, null);
}

public void initialize(EvaluationContext evaluationContext, String domain) throws Exception {
if (isInitialized.getAndSet(true)) {
return;
}
try {
delegate.initialize(evaluationContext);
delegate.initialize(evaluationContext, domain);
setState(ProviderState.READY);
} catch (OpenFeatureError openFeatureError) {
if (ErrorCode.PROVIDER_FATAL.equals(openFeatureError.getErrorCode())) {
Expand Down
50 changes: 45 additions & 5 deletions src/main/java/dev/openfeature/sdk/ProviderRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ private void prepareAndInitializeProvider(
throw new IllegalStateException("Provider cannot be set while repository is shutting down");
}
FeatureProviderStateManager existing = getExistingStateManagerForProvider(newProvider);
validateDomainScopedBinding(domain, newProvider);
if (existing == null) {
openFeatureAPI.registerGlobalProvider(newProvider);
newStateManager = new FeatureProviderStateManager(newProvider);
Expand All @@ -185,37 +186,76 @@ private void prepareAndInitializeProvider(
}

if (waitForInit) {
initializeProvider(newStateManager, afterInit, afterShutdown, afterError, oldStateManager);
initializeProvider(domain, newStateManager, afterInit, afterShutdown, afterError, oldStateManager);
} else {
taskExecutor.submit(() -> {
// initialization happens in a different thread if we're not waiting for it
initializeProvider(newStateManager, afterInit, afterShutdown, afterError, oldStateManager);
initializeProvider(domain, newStateManager, afterInit, afterShutdown, afterError, oldStateManager);
});
}
}

private void validateDomainScopedBinding(String domain, FeatureProvider newProvider) {
if (!newProvider.isDomainScoped()) {
return;
}

// a re-set to the identical binding is always allowed (it's a no-op)
boolean alreadyBoundHere = domain == null
? isDefaultProviderInstance(newProvider)
: getBoundDomainsForProviderInstance(newProvider).contains(domain);
if (alreadyBoundHere) {
return;
}

// any other existing binding means this instance would span more than one domain
if (isDefaultProviderInstance(newProvider)
|| !getBoundDomainsForProviderInstance(newProvider).isEmpty()) {
throw new IllegalArgumentException("Domain-scoped provider cannot be bound to more than one domain");
}
}

private boolean isDefaultProviderInstance(FeatureProvider provider) {
return defaultStateManger.get().getProvider() == provider;
}

private List<String> getBoundDomainsForProviderInstance(FeatureProvider provider) {
return stateManagers.entrySet().stream()
.filter(entry -> entry.getValue().getProvider() == provider)
.map(Map.Entry::getKey)
.collect(Collectors.toList());
}

private FeatureProviderStateManager getExistingStateManagerForProvider(FeatureProvider provider) {
for (FeatureProviderStateManager stateManager : stateManagers.values()) {
if (stateManager.hasSameProvider(provider)) {
if (matchesProvider(stateManager.getProvider(), provider)) {
return stateManager;
}
}
FeatureProviderStateManager defaultFeatureProviderStateManager = defaultStateManger.get();
if (defaultFeatureProviderStateManager.hasSameProvider(provider)) {
if (matchesProvider(defaultFeatureProviderStateManager.getProvider(), provider)) {
return defaultFeatureProviderStateManager;
}
return null;
}

private boolean matchesProvider(FeatureProvider registered, FeatureProvider candidate) {
if (candidate.isDomainScoped()) {
return registered == candidate;
}
return registered.equals(candidate);
}

private void initializeProvider(
String domain,
FeatureProviderStateManager newManager,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, OpenFeatureError> afterError,
FeatureProviderStateManager oldManager) {
try {
if (ProviderState.NOT_READY.equals(newManager.getState())) {
newManager.initialize(openFeatureAPI.getEvaluationContext());
newManager.initialize(openFeatureAPI.getEvaluationContext(), domain);
afterInit.accept(newManager.getProvider());
}
shutDownOld(oldManager, afterShutdown);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,24 @@ protected static Map<String, FeatureProvider> buildProviders(List<FeatureProvide
}

/**
* Initialize the provider.
* {@inheritDoc}
*
* @param evaluationContext evaluation context
* @throws Exception on error (e.g. wrapped {@link java.util.concurrent.ExecutionException}
* from a failing provider)
*/
@Override
public void initialize(EvaluationContext evaluationContext) throws Exception {
initialize(evaluationContext, null);
}

/**
* {@inheritDoc}
*
* @throws Exception on error (e.g. wrapped {@link java.util.concurrent.ExecutionException}
* from a failing provider)
*/
@Override
public void initialize(EvaluationContext evaluationContext, String domain) throws Exception {
Comment on lines +95 to +96

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Forward the isDomainScoped() constraint from child providers.

If a child provider is domain-scoped, it relies on the SDK's ProviderRepository to eagerly reject bindings to multiple domains. However, since MultiProvider does not override isDomainScoped(), it defaults to false.

This allows a MultiProvider that wraps a domain-scoped child to be bound to multiple domains, silently bypassing the SDK's validation and subjecting the child provider to multiple initializations with different domains.

Consider overriding isDomainScoped() to dynamically reflect the strictest constraints of the child providers.

💡 Proposed fix to add constraint propagation

Apply this snippet anywhere within the MultiProvider class:

+    `@Override`
+    public boolean isDomainScoped() {
+        return providers.values().stream().anyMatch(FeatureProvider::isDomainScoped);
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Override
public void initialize(EvaluationContext evaluationContext, String domain) throws Exception {
`@Override`
public void initialize(EvaluationContext evaluationContext, String domain) throws Exception {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java` around
lines 95 - 96, Override isDomainScoped() in MultiProvider to return true
whenever any wrapped child provider is domain-scoped, and false only when all
child providers are non-domain-scoped. Use the existing child-provider
collection and provider API so ProviderRepository can enforce single-domain
binding before initialize is called.

var metadataBuilder = MultiProviderMetadata.builder().name(NAME);
HashMap<String, Metadata> providersMetadata = new HashMap<>();

Expand All @@ -98,7 +108,7 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
Collection<Callable<Void>> tasks = new ArrayList<>(providers.size());
for (FeatureProvider provider : providers.values()) {
tasks.add(() -> {
provider.initialize(evaluationContext);
provider.initialize(evaluationContext, domain);
return null;
Comment thread
jonathannorris marked this conversation as resolved.
});
Metadata providerMetadata = provider.getMetadata();
Expand Down
Loading
Loading