fixup: vmlens test, remove nullable, tighten provider-repo logic

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

Author: toddbaert

src/main/java/dev/openfeature/sdk/FeatureProvider.java

@@ -2,7 +2,6 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import javax.annotation.Nullable;
 
 /**
  * The interface implemented by upstream flag providers to resolve flags for
@@ -199,7 +198,7 @@ default void initialize(EvaluationContext evaluationContext) throws Exception {
      * @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 {
+    default void initialize(EvaluationContext evaluationContext, String domain) throws Exception {
         initialize(evaluationContext);
     }
 

src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java

@@ -3,7 +3,6 @@
 import dev.openfeature.sdk.exceptions.OpenFeatureError;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
-import javax.annotation.Nullable;
 import lombok.extern.slf4j.Slf4j;
 
 @Slf4j
@@ -23,7 +22,7 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
         initialize(evaluationContext, null);
     }
 
-    public void initialize(EvaluationContext evaluationContext, @Nullable String domain) throws Exception {
+    public void initialize(EvaluationContext evaluationContext, String domain) throws Exception {
         if (isInitialized.getAndSet(true)) {
             return;
         }

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

@@ -17,7 +17,6 @@
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
-import javax.annotation.Nullable;
 import lombok.extern.slf4j.Slf4j;
 
 @Slf4j
@@ -155,7 +154,7 @@ public void setProvider(
     }
 
     private void prepareAndInitializeProvider(
-            @Nullable String domain,
+            String domain,
             FeatureProvider newProvider,
             Consumer<FeatureProvider> afterSet,
             Consumer<FeatureProvider> afterInit,
@@ -196,28 +195,22 @@ private void prepareAndInitializeProvider(
         }
     }
 
-    private void validateDomainScopedBinding(@Nullable String domain, FeatureProvider newProvider) {
+    private void validateDomainScopedBinding(String domain, FeatureProvider newProvider) {
         if (!newProvider.isDomainScoped()) {
             return;
         }
 
-        boolean currentlyDefault = isDefaultProviderInstance(newProvider);
-        List<String> boundNamedDomains = getBoundDomainsForProviderInstance(newProvider);
-
-        if (!currentlyDefault && boundNamedDomains.isEmpty()) {
+        // 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;
         }
 
-        if (domain == null) {
-            if (!currentlyDefault) {
-                throw new IllegalArgumentException("Domain-scoped provider cannot be bound to more than one domain");
-            }
-            return;
-        }
-        if (boundNamedDomains.contains(domain)) {
-            return;
-        }
-        if (!boundNamedDomains.isEmpty() || currentlyDefault) {
+        // 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");
         }
     }
@@ -254,7 +247,7 @@ private boolean matchesProvider(FeatureProvider registered, FeatureProvider cand
     }
 
     private void initializeProvider(
-            @Nullable String domain,
+            String domain,
             FeatureProviderStateManager newManager,
             Consumer<FeatureProvider> afterInit,
             Consumer<FeatureProvider> afterShutdown,

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

@@ -19,7 +19,6 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
-import javax.annotation.Nullable;
 import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 
@@ -77,9 +76,8 @@ 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)
      */
@@ -89,15 +87,13 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
     }
 
     /**
-     * Initialize the provider with the bound domain, if any.
+     * {@inheritDoc}
      *
-     * @param evaluationContext evaluation context
-     * @param domain            the bound domain, or {@code null} for the default provider
      * @throws Exception on error (e.g. wrapped {@link java.util.concurrent.ExecutionException}
      *                   from a failing provider)
      */
     @Override
-    public void initialize(EvaluationContext evaluationContext, @Nullable String domain) throws Exception {
+    public void initialize(EvaluationContext evaluationContext, String domain) throws Exception {
         var metadataBuilder = MultiProviderMetadata.builder().name(NAME);
         HashMap<String, Metadata> providersMetadata = new HashMap<>();
 

src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java

@@ -43,14 +43,15 @@ void tearDown() {
                     + "`domain`, rejecting any attempt to bind an already-bound instance to an additional `domain`.")
     @Test
     @DisplayName("rejects binding a domain-scoped provider to a second named domain")
-    void rejectsBindingDomainScopedProviderToSecondNamedDomain() {
+    void rejectsBindingDomainScopedProviderToSecondNamedDomain() throws Exception {
         DomainScopedTestProvider provider = new DomainScopedTestProvider();
 
-        api.setProvider(DOMAIN_A, provider);
+        api.setProviderAndWait(DOMAIN_A, provider);
 
         assertThatThrownBy(() -> api.setProvider(DOMAIN_B, provider))
                 .isInstanceOf(IllegalArgumentException.class)
                 .hasMessageContaining("Domain-scoped provider cannot be bound to more than one domain");
+        assertThat(provider.initCount()).isOne();
     }
 
     @Specification(
@@ -59,14 +60,15 @@ void rejectsBindingDomainScopedProviderToSecondNamedDomain() {
                     + "`domain`, rejecting any attempt to bind an already-bound instance to an additional `domain`.")
     @Test
     @DisplayName("rejects binding a domain-scoped named provider as the default provider")
-    void rejectsBindingDomainScopedNamedProviderAsDefault() {
+    void rejectsBindingDomainScopedNamedProviderAsDefault() throws Exception {
         DomainScopedTestProvider provider = new DomainScopedTestProvider();
 
-        api.setProvider(DOMAIN_A, provider);
+        api.setProviderAndWait(DOMAIN_A, provider);
 
         assertThatThrownBy(() -> api.setProvider(provider))
                 .isInstanceOf(IllegalArgumentException.class)
                 .hasMessageContaining("Domain-scoped provider cannot be bound to more than one domain");
+        assertThat(provider.initCount()).isOne();
     }
 
     @Specification(
@@ -75,14 +77,15 @@ void rejectsBindingDomainScopedNamedProviderAsDefault() {
                     + "`domain`, rejecting any attempt to bind an already-bound instance to an additional `domain`.")
     @Test
     @DisplayName("rejects binding a domain-scoped default provider to a named domain")
-    void rejectsBindingDomainScopedDefaultProviderToNamedDomain() {
+    void rejectsBindingDomainScopedDefaultProviderToNamedDomain() throws Exception {
         DomainScopedTestProvider provider = new DomainScopedTestProvider();
 
-        api.setProvider(provider);
+        api.setProviderAndWait(provider);
 
         assertThatThrownBy(() -> api.setProvider(DOMAIN_A, provider))
                 .isInstanceOf(IllegalArgumentException.class)
                 .hasMessageContaining("Domain-scoped provider cannot be bound to more than one domain");
+        assertThat(provider.initCount()).isOne();
     }
 
     @Test

src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java

@@ -156,4 +156,35 @@ void concurrentShutdown_multipleProvidersShutdownExactlyOnce() throws Exception
             }
         }
     }
+
+    @Test
+    void concurrentRegistrationToSameDomain_exactlyOneBoundAndLoserShutDown() throws Exception {
+        final String domain = "domain-1";
+        try (AllInterleavings allInterleavings = new AllInterleavings("Concurrent registration to the same domain")) {
+            while (allInterleavings.hasNext()) {
+                AtomicInteger provider1ShutdownCount = new AtomicInteger(0);
+                AtomicInteger provider2ShutdownCount = new AtomicInteger(0);
+                FeatureProvider provider1 = createMockedProvider("provider-1", provider1ShutdownCount);
+                FeatureProvider provider2 = createMockedProvider("provider-2", provider2ShutdownCount);
+                OpenFeatureAPI api = OpenFeatureAPITestUtil.createAPI();
+
+                // two different providers race to bind to the same domain
+                Runner.runParallel(
+                        () -> api.setProviderAndWait(domain, provider1),
+                        () -> api.setProviderAndWait(domain, provider2));
+
+                // the domain resolves to exactly one of the two providers
+                FeatureProvider bound = api.getProvider(domain);
+                assertThat(bound).isIn(provider1, provider2);
+
+                // the provider that lost the race is shut down exactly once; the winner is left running
+                AtomicInteger winner = bound == provider1 ? provider1ShutdownCount : provider2ShutdownCount;
+                AtomicInteger loser = bound == provider1 ? provider2ShutdownCount : provider1ShutdownCount;
+                assertThat(loser.get())
+                        .as("Losing provider shut down exactly once")
+                        .isEqualTo(1);
+                assertThat(winner.get()).as("Winning provider not shut down").isZero();
+            }
+        }
+    }
 }