Skip to content

feat: supply bound domain to provider initialization#1982

Open
jonathannorris wants to merge 11 commits into
mainfrom
feat/domain-scoped-provider-init
Open

feat: supply bound domain to provider initialization#1982
jonathannorris wants to merge 11 commits into
mainfrom
feat/domain-scoped-provider-init

Conversation

@jonathannorris

@jonathannorris jonathannorris commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Add optional domain parameter to provider initialize and pass the bound domain from the provider mutator (spec 1.1.2.2, 2.4.1)
  • Add isDomainScoped() provider declaration and reject binding a domain-scoped instance to more than one domain (spec 2.4.3, 1.1.8.1)
  • Forward domain through MultiProvider initialize to each child provider
  • Add spec conformance tests, including backward compatibility for providers that only override single-arg initialize

Motivation

Unblocks OFREP static-context providers that need the bound domain at init time to scope persisted cache keys per open-feature/spec#393 and protocol ADR 0009.

Parallel implementation in js-sdk: open-feature/js-sdk#1433.

Notes

  • Non-breaking: domain is an optional second parameter via a default method on FeatureProvider
  • Non-domain-scoped providers retain existing behavior (single instance can back multiple domains; initialize runs once)
  • Legacy providers that only implement initialize(context) remain compatible; the extra domain argument is ignored if unused
  • Default provider is initialized with a null domain; named clients receive the bound domain string

Related Issues

Fixes #1981

Relates to: open-feature/spec#393, open-feature/js-sdk#1433

Test plan

  • InitializeBehaviorSpecTest verifies domain is passed at init (2.4.1)
  • DomainScopedProviderSpecTest covers 1.1.8.1 and 2.4.3
  • mvn test passes after rebase onto main

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 189274ef-95d5-4ca0-8dee-bc8fb5c30fa4

📥 Commits

Reviewing files that changed from the base of the PR and between 1a2e5be and c0ed5b7.

📒 Files selected for processing (6)
  • src/main/java/dev/openfeature/sdk/FeatureProvider.java
  • src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java
  • src/main/java/dev/openfeature/sdk/ProviderRepository.java
  • src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java
  • src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java
  • src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java
  • src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java
  • src/main/java/dev/openfeature/sdk/FeatureProvider.java
  • src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java
  • src/main/java/dev/openfeature/sdk/ProviderRepository.java
  • src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java

📝 Walkthrough

Walkthrough

Adds domain-aware provider initialization, exposes domain-scoped provider metadata, propagates domains through provider setup and multi-provider initialization, enforces scoped binding rules, and expands compatibility and integration tests.

Changes

Domain-scoped initialization support

Layer / File(s) Summary
Provider initialization contract
src/main/java/dev/openfeature/sdk/FeatureProvider.java
Adds nullable domain initialization and the isDomainScoped() declaration.
Initialization propagation and binding enforcement
src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java, src/main/java/dev/openfeature/sdk/ProviderRepository.java
Passes domains through initialization and validates domain-scoped provider bindings.
Multi-provider forwarding
src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java
Forwards context and domain values to child providers.
Domain and compatibility tests
src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java, src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java, src/test/java/dev/openfeature/sdk/FeatureProviderStateManagerTest.java
Covers scoped binding rules, initialization domains, default-method delegation, and legacy providers.
Existing test and fixture updates
src/test/java/dev/openfeature/sdk/*Test.java, src/test/java/dev/openfeature/sdk/e2e/steps/ProviderSteps.java, src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java, src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java, .gitignore
Updates initialization matchers, concurrency coverage, fixture helpers, and ignored worktree paths.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related issues

  • open-feature/spec#403: Defines the domain-aware initialization and scoped binding behavior implemented here.
  • open-feature/python-sdk#615: Covers equivalent domain propagation and scoped binding enforcement.
  • open-feature/dotnet-sdk#772: Covers equivalent provider initialization and single-domain binding behavior.
  • open-feature/js-sdk#1434: Covers equivalent provider initialization and scoped enforcement.

Suggested reviewers: nicklasl, liran2000

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The .gitignore update to ignore .worktrees/ is unrelated to #1981 and appears out of scope. Remove the .gitignore change or move it to a separate PR if it is needed for a different purpose.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly states the main change: passing the bound domain into provider initialization.
Description check ✅ Passed The description matches the PR changes and mentions the optional domain, domain-scoped providers, and MultiProvider forwarding.
Linked Issues check ✅ Passed The changes implement #1981's requirements: domain passed to initialize, domain-scoped enforcement, and backward-compatible overloads.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.07%. Comparing base (1f46ab3) to head (c0ed5b7).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1982      +/-   ##
============================================
+ Coverage     92.31%   93.07%   +0.75%     
- Complexity      706      725      +19     
============================================
  Files            60       60              
  Lines          1705     1732      +27     
  Branches        195      200       +5     
============================================
+ Hits           1574     1612      +38     
+ Misses           82       72      -10     
+ Partials         49       48       -1     
Flag Coverage Δ
unittests 93.07% <100.00%> (+0.75%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jonathannorris jonathannorris force-pushed the feat/domain-scoped-provider-init branch 2 times, most recently from e7ad946 to 4a23ee9 Compare July 3, 2026 15:07
@jonathannorris jonathannorris marked this pull request as ready for review July 3, 2026 15:15
@jonathannorris jonathannorris requested review from a team as code owners July 3, 2026 15:15
@jonathannorris jonathannorris requested a review from Copilot July 3, 2026 15:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds domain-awareness to provider initialization and introduces an opt-in “domain-scoped” provider declaration that the SDK enforces to prevent a single provider instance from being bound to multiple domains, aligning the Java SDK with updated OpenFeature spec requirements.

Changes:

  • Add FeatureProvider.initialize(EvaluationContext, @Nullable String domain) default method and isDomainScoped() declaration.
  • Pass the bound domain through provider initialization paths (repository/state manager, MultiProvider) and enforce domain-scoped single-domain binding.
  • Add/adjust conformance and backward-compatibility tests for legacy single-arg providers and domain-scoped binding rules.

Reviewed changes

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

Show a summary per file
File Description
src/main/java/dev/openfeature/sdk/FeatureProvider.java Adds optional domain initialize overload + isDomainScoped() declaration.
src/main/java/dev/openfeature/sdk/ProviderRepository.java Passes domain into initialization and enforces domain-scoped binding constraints.
src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java Forwards domain into provider initialization while preserving one-time init behavior.
src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java Adds two-arg initialize and forwards domain during child initialization.
src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java Updates spec tests to assert domain (or null) is passed on init.
src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java Adds spec tests for domain-scoped binding restrictions and domain receipt.
src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java Adds explicit backward-compat coverage for legacy single-arg providers.
src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java Updates repository tests to expect two-arg initialize with proper domain/null.
src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java Updates concurrency tests to stub/verify two-arg initialize.
src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java Updates API tests to verify two-arg initialize is called with null domain.
src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java Adds tests for MultiProvider domain forwarding and legacy child providers.
src/test/java/dev/openfeature/sdk/FeatureProviderStateManagerTest.java Adds test ensuring domain is forwarded into delegate init.
src/test/java/dev/openfeature/sdk/e2e/steps/ProviderSteps.java Updates e2e mocking to stub two-arg initialize for named clients.
src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java Updates fixture to throw from two-arg initialize (nullable domain).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/main/java/dev/openfeature/sdk/ProviderRepository.java Outdated
Comment thread src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (5)
src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java (2)

93-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate test body flagged by SonarCloud (named-provider case).

mustPassBoundDomainWhenInitializingANamedProvider duplicates mustCallInitializeFunctionOfTheNewlyRegisteredNamedProviderBeforeUsingItForFlagEvaluation verbatim. Same consolidation suggestion as the default-provider pair above applies here.

Also applies to: 109-115

🤖 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/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java` around
lines 93 - 101, The named-provider test body is duplicated and should be
consolidated to match the shared pattern used for the default-provider case.
Update mustPassBoundDomainWhenInitializingANamedProvider and
mustCallInitializeFunctionOfTheNewlyRegisteredNamedProviderBeforeUsingItForFlagEvaluation
in InitializeBehaviorSpecTest so only one test covers the initialize-on-set
behavior, or extract the common setup/assertion into a shared helper and keep
distinct assertions only where behavior differs.

Source: Linters/SAST tools


40-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate test body flagged by SonarCloud.

mustPassNullDomainWhenInitializingTheDefaultProvider is byte-identical to mustCallInitializeFunctionOfTheNewlyRegisteredProviderBeforeUsingItForFlagEvaluation above it. Consider extracting the shared assertion into a private helper so both @Specification-tagged tests stay for traceability without duplicated code.

♻️ Suggested consolidation
+    private void assertInitializeCalledWithDomain(FeatureProvider featureProvider, Object domainMatcher) {
+        // shared verification helper
+    }

Or simply merge both @Specification annotations onto a single test method.

Also applies to: 55-61

🤖 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/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java` around
lines 40 - 47, The two `@Specification` tests in InitializeBehaviorSpecTest are
duplicate bodies, so consolidate the shared setup/assertion into a private
helper or merge the annotations onto one test method. Keep the existing test
names or metadata only as needed for traceability, and update both
mustCallInitializeFunctionOfTheNewlyRegisteredProviderBeforeUsingItForFlagEvaluation
and mustPassNullDomainWhenInitializingTheDefaultProvider to delegate to the
shared logic instead of repeating the same mock/verify sequence.

Source: Linters/SAST tools

src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java (2)

95-110: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Test doesn't verify DOMAIN_B initialization was actually invoked.

allowsBindingNonDomainScopedProviderToMultipleDomains only verifies initialize(any(), eq(DOMAIN_A)). To actually confirm the provider was initialized for both domains (the point of the test), also assert the DOMAIN_B call occurred.

♻️ Suggested fix
         assertThat(api.getProvider(DOMAIN_A)).isSameAs(provider);
         assertThat(api.getProvider(DOMAIN_B)).isSameAs(provider);
         verify(provider, times(1)).initialize(any(), eq(DOMAIN_A));
+        verify(provider, times(1)).initialize(any(), eq(DOMAIN_B));
🤖 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/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java` around
lines 95 - 110, The test allows a non-domain-scoped provider to be bound to two
domains, but it only verifies initialization for DOMAIN_A. Update
allowsBindingNonDomainScopedProviderToMultipleDomains in
DomainScopedProviderSpecTest to also assert that the provider’s initialize path
was invoked for DOMAIN_B, using the existing provider mock and initialization
verification so the test confirms both domain bindings.

27-31: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Missing teardown to shut down OpenFeatureAPI instances.

Each test constructs a new OpenFeatureAPI() instance in @BeforeEach, and setProvider dispatches initialization work to an internal executor owned by that instance. Without an @AfterEach calling api.shutdown(), each test leaves behind an unshut-down executor/thread pool for the lifetime of the JVM running the suite.

♻️ Suggested fix
     `@BeforeEach`
     void setupTest() {
         api = new OpenFeatureAPI();
         api.setProvider(new NoOpProvider());
     }
+
+    `@AfterEach`
+    void tearDown() {
+        api.shutdown();
+    }
🤖 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/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java` around
lines 27 - 31, The test setup creates a new OpenFeatureAPI and assigns a
provider, but the instance-owned executor is never shut down, leaving background
threads running after each test. Add an `@AfterEach` teardown in
DomainScopedProviderSpecTest that calls api.shutdown() on the same api
initialized in setupTest, and ensure this cleanup runs even when setup or
assertions fail. Use the existing OpenFeatureAPI and setupTest symbols to place
the teardown alongside the current lifecycle methods.
src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java (1)

54-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Misleading test name: asserts count == 2, not "once".

singleArgOverrideIsOnlyInvokedOnce calls provider.initialize(ctx, DOMAIN) twice directly (bypassing the FeatureProviderStateManager idempotency guard) and asserts the count is 2. The name suggests the opposite of what's asserted, and it's easy to conflate with the similarly-named onlyInvokesLegacySingleArgProviderOncePerStateManagerInit test (lines 142-152), which asserts count == 1 via the state-manager guard. Consider renaming to clarify that each direct call to the default two-arg method triggers exactly one delegated single-arg call (e.g., eachCallDelegatesExactlyOnceToSingleArgOverride).

✏️ Suggested rename
-        `@DisplayName`("single-arg override is only invoked once per initialization")
-        void singleArgOverrideIsOnlyInvokedOnce() throws Exception {
+        `@DisplayName`("each call to the two-arg default delegates exactly once to the single-arg override")
+        void eachCallDelegatesExactlyOnceToSingleArgOverride() 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/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java`
around lines 54 - 63, Rename the misleading test in
ProviderInitializeBackwardCompatibilityTest so its name matches the asserted
behavior: the current singleArgOverrideIsOnlyInvokedOnce method calls
initialize(new ImmutableContext(), DOMAIN) twice and expects
singleArgInitCount() to be 2, so update the test name to reflect that each
direct two-arg initialize call delegates exactly once to the single-arg
override. Make sure it stays clearly distinct from the state-manager-backed
onlyInvokesLegacySingleArgProviderOncePerStateManagerInit test.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@src/main/java/dev/openfeature/sdk/FeatureProvider.java`:
- Line 5: The production code in FeatureProvider imports
javax.annotation.Nullable, but the project is missing a compile-scope dependency
that provides it, which can break the main module build. Update pom.xml to add
jsr305 or an equivalent compile-time dependency so the Nullable import in
FeatureProvider resolves during production compilation and is available without
relying on test-only or transitive scope.

---

Nitpick comments:
In `@src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java`:
- Around line 95-110: The test allows a non-domain-scoped provider to be bound
to two domains, but it only verifies initialization for DOMAIN_A. Update
allowsBindingNonDomainScopedProviderToMultipleDomains in
DomainScopedProviderSpecTest to also assert that the provider’s initialize path
was invoked for DOMAIN_B, using the existing provider mock and initialization
verification so the test confirms both domain bindings.
- Around line 27-31: The test setup creates a new OpenFeatureAPI and assigns a
provider, but the instance-owned executor is never shut down, leaving background
threads running after each test. Add an `@AfterEach` teardown in
DomainScopedProviderSpecTest that calls api.shutdown() on the same api
initialized in setupTest, and ensure this cleanup runs even when setup or
assertions fail. Use the existing OpenFeatureAPI and setupTest symbols to place
the teardown alongside the current lifecycle methods.

In `@src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java`:
- Around line 93-101: The named-provider test body is duplicated and should be
consolidated to match the shared pattern used for the default-provider case.
Update mustPassBoundDomainWhenInitializingANamedProvider and
mustCallInitializeFunctionOfTheNewlyRegisteredNamedProviderBeforeUsingItForFlagEvaluation
in InitializeBehaviorSpecTest so only one test covers the initialize-on-set
behavior, or extract the common setup/assertion into a shared helper and keep
distinct assertions only where behavior differs.
- Around line 40-47: The two `@Specification` tests in InitializeBehaviorSpecTest
are duplicate bodies, so consolidate the shared setup/assertion into a private
helper or merge the annotations onto one test method. Keep the existing test
names or metadata only as needed for traceability, and update both
mustCallInitializeFunctionOfTheNewlyRegisteredProviderBeforeUsingItForFlagEvaluation
and mustPassNullDomainWhenInitializingTheDefaultProvider to delegate to the
shared logic instead of repeating the same mock/verify sequence.

In
`@src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java`:
- Around line 54-63: Rename the misleading test in
ProviderInitializeBackwardCompatibilityTest so its name matches the asserted
behavior: the current singleArgOverrideIsOnlyInvokedOnce method calls
initialize(new ImmutableContext(), DOMAIN) twice and expects
singleArgInitCount() to be 2, so update the test name to reflect that each
direct two-arg initialize call delegates exactly once to the single-arg
override. Make sure it stays clearly distinct from the state-manager-backed
onlyInvokesLegacySingleArgProviderOncePerStateManagerInit test.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0adbf73e-e6cd-4ba9-bd5d-9c956a5fc975

📥 Commits

Reviewing files that changed from the base of the PR and between a7d4e8a and 4a23ee9.

📒 Files selected for processing (14)
  • src/main/java/dev/openfeature/sdk/FeatureProvider.java
  • src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java
  • src/main/java/dev/openfeature/sdk/ProviderRepository.java
  • src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java
  • src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java
  • src/test/java/dev/openfeature/sdk/FeatureProviderStateManagerTest.java
  • src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java
  • src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java
  • src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java
  • src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java
  • src/test/java/dev/openfeature/sdk/e2e/steps/ProviderSteps.java
  • src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java
  • src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java
  • src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java

Comment thread src/main/java/dev/openfeature/sdk/FeatureProvider.java Outdated
@jonathannorris

Copy link
Copy Markdown
Member Author

Addressing the remaining CodeRabbit nitpicks from the review body:

  • @AfterEach teardown (DomainScopedProviderSpecTest): fixed in 68fa5da.
  • DOMAIN_B initialize assertion: skipping. A shared non-domain-scoped provider reuses the same state manager on the second bind; initialize runs once on first bind only (same behavior as js-sdk).
  • singleArgOverrideIsOnlyInvokedOnce rename: skipping. The @DisplayName already clarifies intent; the method name is slightly misleading but low risk.
  • InitializeBehaviorSpecTest dedup: skipping. The duplicate bodies are intentional so each @Specification requirement has its own test for traceability.

All three inline review threads (Copilot identity check, MultiProvider PR description, javax.annotation.Nullable) are resolved.

Comment thread src/main/java/dev/openfeature/sdk/FeatureProvider.java Outdated
Comment thread src/main/java/dev/openfeature/sdk/ProviderRepository.java Outdated
Comment thread src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java

import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;

@toddbaert toddbaert Jul 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We removed javax.annotation.* in main sources a while back (see #921), and there are no such imports anywhere in src/main/java/ on main today (there's one in /test I think we should even remove). The runtime generally ignores the annotation if it's not available on classpath, but there are still ways to get ClassNotFounds if no jsr305 impl is found.

jsr305 is abandoned, so I'd rather not have FeatureProvider reference it at all. The Javadoc at line 56 already documents that domain can be null, which is what we do elsewhere in src/main.

I'd suggest dropping the annotation here (and the internal ones in ProviderRepository / FeatureProviderStateManager / MultiProvider as well).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I removed these annotations.

}

/**
* Initialize the provider with the bound domain, if any.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These docs belong in the interface

* @param evaluationContext the global evaluation context
* @param domain the bound domain, or {@code null} for the default provider
*/
default void initialize(EvaluationContext evaluationContext, @Nullable String domain) throws Exception {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we sure that the addition of this default method is not a breaking change on a byte code level?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good point.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rebased against our recent additions for byecode/sourcecode change detection, and passes, so I guess no problem with this.

}
}

private void validateDomainScopedBinding(@Nullable String domain, FeatureProvider newProvider) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the functionality of this method already covered by a VmLens test? If not, we should add one to guard against registering two providers to the same domain concurrently

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added concurrentRegistrationToSameDomain_exactlyOneBoundAndLoserShutDown - please take a look - I think it covers this.

Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
@toddbaert toddbaert force-pushed the feat/domain-scoped-provider-init branch from 68fa5da to 362bcce Compare July 14, 2026 16:02

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java (1)

36-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider updating the stub to match all domains.

The mock is stubbed to specifically expect a null domain, which matches the default provider tests but will not match the new concurrentRegistrationToSameDomain_exactlyOneBoundAndLoserShutDown test that passes "domain-1".

Since Mockito defaults to returning null (doing nothing) for un-stubbed void methods, the test still passes. However, to ensure the stub explicitly matches all test cases regardless of the domain provided, consider using any() for the domain argument.

💡 Proposed refactor
-        doAnswer(invocation -> null).when(provider).initialize(any(), isNull());
+        doAnswer(invocation -> null).when(provider).initialize(any(), any());
🤖 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/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java` at line
36, Update the provider.initialize stub in ProviderRepositoryCT to match any
domain argument instead of only a null domain, while preserving the existing
behavior of returning without action for all initialization calls.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java`:
- Around line 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.

---

Nitpick comments:
In `@src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java`:
- Line 36: Update the provider.initialize stub in ProviderRepositoryCT to match
any domain argument instead of only a null domain, while preserving the existing
behavior of returning without action for all initialization calls.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1cc246fc-6456-4557-97b3-e971995c3f75

📥 Commits

Reviewing files that changed from the base of the PR and between 362bcce and 1a2e5be.

📒 Files selected for processing (6)
  • src/main/java/dev/openfeature/sdk/FeatureProvider.java
  • src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java
  • src/main/java/dev/openfeature/sdk/ProviderRepository.java
  • src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java
  • src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java
  • src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java
  • src/main/java/dev/openfeature/sdk/ProviderRepository.java
  • src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java

Comment on lines +95 to +96
@Override
public void initialize(EvaluationContext evaluationContext, String domain) throws Exception {

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.

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert force-pushed the feat/domain-scoped-provider-init branch from 1a2e5be to c0ed5b7 Compare July 14, 2026 19:48
@sonarqubecloud

Copy link
Copy Markdown

@toddbaert toddbaert self-requested a review July 15, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Supply the bound domain to provider initialization

4 participants