From 3e59f07adc25b39f88811a01f1493af5083eddc8 Mon Sep 17 00:00:00 2001 From: jarebudev <23311805+jarebudev@users.noreply.github.com> Date: Thu, 9 May 2024 23:46:10 +0100 Subject: [PATCH 1/5] initial changes for implementing domain scoping Signed-off-by: jarebudev <23311805+jarebudev@users.noreply.github.com> --- README.md | 24 ++++++++-------- .../java/dev/openfeature/sdk/Metadata.java | 2 ++ .../dev/openfeature/sdk/NoOpProvider.java | 12 +++++++- .../dev/openfeature/sdk/OpenFeatureAPI.java | 24 ++++++++-------- .../openfeature/sdk/OpenFeatureClient.java | 21 +++++++++++--- .../openfeature/sdk/ProviderRepository.java | 18 ++++++------ .../providers/memory/InMemoryProvider.java | 12 +++++++- .../openfeature/sdk/AlwaysBrokenProvider.java | 5 ++++ .../openfeature/sdk/DoSomethingProvider.java | 12 +++++++- .../openfeature/sdk/EventProviderTest.java | 12 ++++++-- .../sdk/FlagEvaluationSpecTest.java | 20 ++++++++----- .../dev/openfeature/sdk/HookSupportTest.java | 28 +++++++++++++++++-- .../openfeature/sdk/OpenFeatureAPITest.java | 10 +++---- .../testutils/FeatureProviderTestUtils.java | 6 ++-- .../sdk/testutils/TestEventsProvider.java | 5 ++++ 15 files changed, 152 insertions(+), 59 deletions(-) diff --git a/README.md b/README.md index 882f83f9..63eb72e2 100644 --- a/README.md +++ b/README.md @@ -126,7 +126,7 @@ See [here](https://javadoc.io/doc/dev.openfeature/sdk/latest/) for the Javadocs. | ✅ | [Targeting](#targeting) | Contextually-aware flag evaluation using [evaluation context](https://openfeature.dev/docs/reference/concepts/evaluation-context). | | ✅ | [Hooks](#hooks) | Add functionality to various stages of the flag evaluation life-cycle. | | ✅ | [Logging](#logging) | Integrate with popular logging packages. | -| ✅ | [Named clients](#named-clients) | Utilize multiple providers in a single application. | +| ✅ | [Domains](#domains) | Logically bind clients with providers. | | ✅ | [Eventing](#eventing) | React to state changes in the provider or flag management system. | | ✅ | [Shutdown](#shutdown) | Gracefully clean up a provider during application shutdown. | | ✅ | [Transaction Context Propagation](#transaction-context-propagation) | Set a specific [evaluation context](https://openfeature.dev/docs/reference/concepts/evaluation-context) for a transaction (e.g. an HTTP request or a thread). | @@ -160,7 +160,7 @@ To register a provider in a non-blocking manner, you can use the `setProvider` m ``` In some situations, it may be beneficial to register multiple providers in the same application. -This is possible using [named clients](#named-clients), which is covered in more details below. +This is possible using [domains](#domains), which is covered in more detail below. ### Targeting @@ -219,27 +219,27 @@ Once you've added a hook as a dependency, it can be registered at the global, cl The Java SDK uses SLF4J. See the [SLF4J manual](https://slf4j.org/manual.html) for complete documentation. -### Named clients +### Domains -Clients can be given a name. -A name is a logical identifier which can be used to associate clients with a particular provider. -If a name has no associated provider, the global provider is used. +Clients can be assigned to a domain. +A domain is a logical identifier which can be used to associate clients with a particular provider. +If a domain has no associated provider, the global provider is used. ```java FeatureProvider scopedProvider = new MyProvider(); // registering the default provider OpenFeatureAPI.getInstance().setProvider(LocalProvider()); -// registering a named provider -OpenFeatureAPI.getInstance().setProvider("clientForCache", new CachedProvider()); +// registering a provider to a domain +OpenFeatureAPI.getInstance().setProvider("my-domain", new CachedProvider()); -// a client backed by default provider +// A client bound to the default provider Client clientDefault = OpenFeatureAPI.getInstance().getClient(); -// a client backed by CachedProvider -Client clientNamed = OpenFeatureAPI.getInstance().getClient("clientForCache"); +// A client bound to the CachedProvider provider +Client domainScopedClient = OpenFeatureAPI.getInstance().getClient("my-domain"); ``` -Named providers can be set in a blocking or non-blocking way. +Providers for domains can be set in a blocking or non-blocking way. For more details, please refer to the [providers](#providers) section. ### Eventing diff --git a/src/main/java/dev/openfeature/sdk/Metadata.java b/src/main/java/dev/openfeature/sdk/Metadata.java index 7e614c27..2f1d5ffa 100644 --- a/src/main/java/dev/openfeature/sdk/Metadata.java +++ b/src/main/java/dev/openfeature/sdk/Metadata.java @@ -5,4 +5,6 @@ */ public interface Metadata { String getName(); + + String getDomain(); } diff --git a/src/main/java/dev/openfeature/sdk/NoOpProvider.java b/src/main/java/dev/openfeature/sdk/NoOpProvider.java index ef8cf1f8..4304b2cf 100644 --- a/src/main/java/dev/openfeature/sdk/NoOpProvider.java +++ b/src/main/java/dev/openfeature/sdk/NoOpProvider.java @@ -18,7 +18,17 @@ public ProviderState getState() { @Override public Metadata getMetadata() { - return () -> name; + return new Metadata() { + @Override + public String getName() { + return name; + } + + @Override + public String getDomain() { + return name; + } + }; } @Override diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index eb9d3a71..f89bb46a 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -67,16 +67,16 @@ public Client getClient() { /** * {@inheritDoc} */ - public Client getClient(@Nullable String name) { - return getClient(name, null); + public Client getClient(@Nullable String domain) { + return getClient(domain, null); } /** * {@inheritDoc} */ - public Client getClient(@Nullable String name, @Nullable String version) { + public Client getClient(@Nullable String domain, @Nullable String version) { return new OpenFeatureClient(this, - name, + domain, version); } @@ -154,14 +154,14 @@ public void setProvider(FeatureProvider provider) { } /** - * Add a provider for a named client. + * Add a provider for a domain. * - * @param clientName The name of the client. + * @param domain The domain to bind the provider to. * @param provider The provider to set. */ - public void setProvider(String clientName, FeatureProvider provider) { + public void setProvider(String domain, FeatureProvider provider) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { - providerRepository.setProvider(clientName, + providerRepository.setProvider(domain, provider, this::attachEventProvider, this::emitReady, @@ -187,14 +187,14 @@ public void setProviderAndWait(FeatureProvider provider) throws OpenFeatureError } /** - * Add a provider for a named client and wait for initialization to finish. + * Add a provider for a domain and wait for initialization to finish. * - * @param clientName The name of the client. + * @param domain The domain to bind the provider to. * @param provider The provider to set. */ - public void setProviderAndWait(String clientName, FeatureProvider provider) throws OpenFeatureError { + public void setProviderAndWait(String domain, FeatureProvider provider) throws OpenFeatureError { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { - providerRepository.setProvider(clientName, + providerRepository.setProvider(domain, provider, this::attachEventProvider, this::emitReady, diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 3455c0c0..826e2fdb 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -26,6 +26,8 @@ public class OpenFeatureClient implements Client { @Getter private final String name; @Getter + private final String domain; + @Getter private final String version; private final List clientHooks; private final HookSupport hookSupport; @@ -37,16 +39,17 @@ public class OpenFeatureClient implements Client { * Deprecated public constructor. Use OpenFeature.API.getClient() instead. * * @param openFeatureAPI Backing global singleton - * @param name Name of the client (used by observability tools). + * @param domain An identifier which logically binds clients with providers (used by observability tools). * @param version Version of the client (used by observability tools). * @deprecated Do not use this constructor. It's for internal use only. * Clients created using it will not run event handlers. * Use the OpenFeatureAPI's getClient factory method instead. */ @Deprecated() // TODO: eventually we will make this non-public. See issue #872 - public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String name, String version) { + public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String domain, String version) { this.openfeatureApi = openFeatureAPI; - this.name = name; + this.name = domain; + this.domain = domain; this.version = version; this.clientHooks = new ArrayList<>(); this.hookSupport = new HookSupport(); @@ -352,7 +355,17 @@ public FlagEvaluationDetails getObjectDetails(String key, Value defaultVa @Override public Metadata getMetadata() { - return () -> name; + return new Metadata() { + @Override + public String getName() { + return name; + } + + @Override + public String getDomain() { + return domain; + } + }; } /** diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 8dee0a6f..68863878 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -77,14 +77,14 @@ public void setProvider(FeatureProvider provider, } /** - * Add a provider for a named client. + * Add a provider for a domain. * - * @param clientName The name of the client. + * @param domain The domain to bind the provider to. * @param provider The provider to set. * @param waitForInit When true, wait for initialization to finish, then returns. * Otherwise, initialization happens in the background. */ - public void setProvider(String clientName, + public void setProvider(String domain, FeatureProvider provider, Consumer afterSet, Consumer afterInit, @@ -94,13 +94,13 @@ public void setProvider(String clientName, if (provider == null) { throw new IllegalArgumentException("Provider cannot be null"); } - if (clientName == null) { - throw new IllegalArgumentException("clientName cannot be null"); + if (domain == null) { + throw new IllegalArgumentException("domain cannot be null"); } - prepareAndInitializeProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit); + prepareAndInitializeProvider(domain, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit); } - private void prepareAndInitializeProvider(@Nullable String clientName, + private void prepareAndInitializeProvider(@Nullable String domain, FeatureProvider newProvider, Consumer afterSet, Consumer afterInit, @@ -114,8 +114,8 @@ private void prepareAndInitializeProvider(@Nullable String clientName, } // provider is set immediately, on this thread - FeatureProvider oldProvider = clientName != null - ? this.providers.put(clientName, newProvider) + FeatureProvider oldProvider = domain != null + ? this.providers.put(domain, newProvider) : this.defaultProvider.getAndSet(newProvider); if (waitForInit) { diff --git a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java index 8cd9fc8d..61923642 100644 --- a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java +++ b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java @@ -40,7 +40,17 @@ public class InMemoryProvider extends EventProvider { @Override public Metadata getMetadata() { - return () -> NAME; + return new Metadata() { + @Override + public String getName() { + return NAME; + } + + @Override + public String getDomain() { + return NAME; + } + }; } public InMemoryProvider(Map> flags) { diff --git a/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java b/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java index 4beb28c3..9f10f579 100644 --- a/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java +++ b/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java @@ -11,6 +11,11 @@ public Metadata getMetadata() { public String getName() { throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE); } + + @Override + public String getDomain() { + throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE); + } }; } diff --git a/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java b/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java index 4fdc433b..74efabb7 100644 --- a/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java +++ b/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java @@ -23,7 +23,17 @@ EvaluationContext getMergedContext() { @Override public Metadata getMetadata() { - return () -> name; + return new Metadata() { + @Override + public String getName() { + return name; + } + + @Override + public String getDomain() { + return name; + } + }; } @Override diff --git a/src/test/java/dev/openfeature/sdk/EventProviderTest.java b/src/test/java/dev/openfeature/sdk/EventProviderTest.java index 3744682b..8f41ef7c 100644 --- a/src/test/java/dev/openfeature/sdk/EventProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/EventProviderTest.java @@ -7,6 +7,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import org.checkerframework.checker.units.qual.N; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -77,14 +78,21 @@ void doesNotThrowWhenOnEmitSame() { class TestEventProvider extends EventProvider { + private static final String NAME = "TestEventProvider"; + @Override public Metadata getMetadata() { return new Metadata() { @Override public String getName() { - return "TestEventProvider"; + return NAME; + } + + @Override + public String getDomain() { + return NAME; } - }; + }; } @Override diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index 60b6ee13..16159e25 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -135,10 +135,13 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { assertEquals(h2, api.getHooks().get(1)); } - @Specification(number="1.1.6", text="The API MUST provide a function for creating a client which accepts the following options: - name (optional): A logical string identifier for the client.") - @Test void namedClient() { - assertThatCode(() -> api.getClient("Sir Calls-a-lot")).doesNotThrowAnyException(); - // TODO: Doesn't say that you can *get* the client name.. which seems useful? + @Specification(number="1.1.6", text="The API MUST provide a function for creating a client which accepts the following options: - domain (optional): A logical string identifier for binding clients to provider.") + @Test void domainName() { + assertNull(api.getClient().getMetadata().getDomain()); + + String domain = "Sir Calls-a-lot"; + Client clientForDomain = api.getClient(domain); + assertEquals(domain, clientForDomain.getMetadata().getDomain()); } @Specification(number="1.2.1", text="The client MUST provide a method to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.") @@ -274,14 +277,17 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { ArgumentMatchers.isA(FlagNotFoundError.class)); } - @Specification(number="1.2.2", text="The client interface MUST define a metadata member or accessor, containing an immutable name field or accessor of type string, which corresponds to the name value supplied during client creation.") + @Specification(number="1.2.2", text="The client interface MUST define a metadata member or accessor, containing an immutable domain field or accessor of type string, which corresponds to the domain value supplied during client creation. In previous drafts, this property was called name. For backwards compatibility, implementations should consider name an alias to domain.") @Test void clientMetadata() { Client c = _client(); + assertNull(c.getMetadata().getDomain()); assertNull(c.getMetadata().getName()); + String domainName = "test domain"; FeatureProviderTestUtils.setFeatureProvider(new AlwaysBrokenProvider()); - Client c2 = api.getClient("test"); - assertEquals("test", c2.getMetadata().getName()); + Client c2 = api.getClient(domainName); + assertEquals(domainName, c2.getMetadata().getDomain()); + assertEquals(domainName, c2.getMetadata().getName()); } @Specification(number="1.4.9", text="In cases of abnormal execution (network failure, unhandled error, etc) the reason field in the evaluation details SHOULD indicate an error.") diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index d8883780..da1f11e1 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -20,13 +20,37 @@ class HookSupportTest implements HookFixtures { + private Metadata clientMetadata = new Metadata() { + @Override + public String getName() { + return "client"; + } + + @Override + public String getDomain() { + return "domain"; + } + }; + + private Metadata providerMetadata = new Metadata() { + @Override + public String getName() { + return "provider"; + } + + @Override + public String getDomain() { + return "domain"; + } + }; + @Test @DisplayName("should merge EvaluationContexts on before hooks correctly") void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { Map attributes = new HashMap<>(); attributes.put("baseKey", new Value("baseValue")); EvaluationContext baseContext = new ImmutableContext(attributes); - HookContext hookContext = new HookContext<>("flagKey", FlagValueType.STRING, "defaultValue", baseContext, () -> "client", () -> "provider"); + HookContext hookContext = new HookContext<>("flagKey", FlagValueType.STRING, "defaultValue", baseContext, clientMetadata, providerMetadata); Hook hook1 = mockStringHook(); Hook hook2 = mockStringHook(); when(hook1.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("bla", "blubber"))); @@ -48,7 +72,7 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { HookSupport hookSupport = new HookSupport(); EvaluationContext baseContext = new ImmutableContext(); IllegalStateException expectedException = new IllegalStateException("All fine, just a test"); - HookContext hookContext = new HookContext<>("flagKey", flagValueType, createDefaultValue(flagValueType), baseContext, () -> "client", () -> "provider"); + HookContext hookContext = new HookContext<>("flagKey", flagValueType, createDefaultValue(flagValueType), baseContext, clientMetadata, providerMetadata); hookSupport.beforeHooks(flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap()); hookSupport.afterHooks(flagValueType, hookContext, FlagEvaluationDetails.builder().build(), Collections.singletonList(genericHook), Collections.emptyMap()); diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java index 63a1dadd..cc515f29 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java @@ -32,16 +32,16 @@ void namedProviderTest() { .isEqualTo(api.getProviderMetadata("namedProviderTest").getName()); } - @Specification(number="1.1.3", text="The API MUST provide a function to bind a given provider to one or more client names. If the client-name already has a bound provider, it is overwritten with the new mapping.") + @Specification(number="1.1.3", text="The API MUST provide a function to bind a given provider to one or more clients using a domain. If the domain already has a bound provider, it is overwritten with the new mapping.") @Test void namedProviderOverwrittenTest() { - String name = "namedProviderOverwrittenTest"; + String domain = "namedProviderOverwrittenTest"; FeatureProvider provider1 = new NoOpProvider(); FeatureProvider provider2 = new DoSomethingProvider(); - FeatureProviderTestUtils.setFeatureProvider(name, provider1); - FeatureProviderTestUtils.setFeatureProvider(name, provider2); + FeatureProviderTestUtils.setFeatureProvider(domain, provider1); + FeatureProviderTestUtils.setFeatureProvider(domain, provider2); - assertThat(OpenFeatureAPI.getInstance().getProvider(name).getMetadata().getName()) + assertThat(OpenFeatureAPI.getInstance().getProvider(domain).getMetadata().getDomain()) .isEqualTo(DoSomethingProvider.name); } diff --git a/src/test/java/dev/openfeature/sdk/testutils/FeatureProviderTestUtils.java b/src/test/java/dev/openfeature/sdk/testutils/FeatureProviderTestUtils.java index b3fc898e..41ffbe18 100644 --- a/src/test/java/dev/openfeature/sdk/testutils/FeatureProviderTestUtils.java +++ b/src/test/java/dev/openfeature/sdk/testutils/FeatureProviderTestUtils.java @@ -24,8 +24,8 @@ private static void waitForProviderInitializationComplete(Function extractor.apply(OpenFeatureAPI.getInstance()) == provider); } - public static void setFeatureProvider(String namedProvider, FeatureProvider provider) { - OpenFeatureAPI.getInstance().setProvider(namedProvider, provider); - waitForProviderInitializationComplete(api -> api.getProvider(namedProvider), provider); + public static void setFeatureProvider(String domain, FeatureProvider provider) { + OpenFeatureAPI.getInstance().setProvider(domain, provider); + waitForProviderInitializationComplete(api -> api.getProvider(domain), provider); } } diff --git a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java index b0e5eb78..fbe98a78 100644 --- a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java +++ b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java @@ -23,6 +23,11 @@ public class TestEventsProvider extends EventProvider { public String getName() { return name; } + + @Override + public String getDomain() { + return name; + } }; @Override From 203e717e55dacac3a88f277cd5e0e57e6049041c Mon Sep 17 00:00:00 2001 From: jarebudev <23311805+jarebudev@users.noreply.github.com> Date: Mon, 13 May 2024 22:35:28 +0100 Subject: [PATCH 2/5] completed the rest of the renaming and tidying up Signed-off-by: jarebudev <23311805+jarebudev@users.noreply.github.com> --- src/main/java/dev/openfeature/sdk/Client.java | 2 +- .../dev/openfeature/sdk/ClientMetadata.java | 10 ++++ .../dev/openfeature/sdk/EventDetails.java | 6 ++- .../dev/openfeature/sdk/EventSupport.java | 35 +++++++------- .../java/dev/openfeature/sdk/HookContext.java | 4 +- .../java/dev/openfeature/sdk/Metadata.java | 2 - .../dev/openfeature/sdk/NoOpProvider.java | 12 +---- .../dev/openfeature/sdk/OpenFeatureAPI.java | 48 +++++++++---------- .../openfeature/sdk/OpenFeatureClient.java | 10 ++-- .../openfeature/sdk/ProviderRepository.java | 12 ++--- .../providers/memory/InMemoryProvider.java | 12 +---- .../openfeature/sdk/AlwaysBrokenProvider.java | 12 +---- .../openfeature/sdk/DoSomethingProvider.java | 12 +---- .../openfeature/sdk/EventProviderTest.java | 13 +---- .../java/dev/openfeature/sdk/EventsTest.java | 46 +++++++++--------- .../dev/openfeature/sdk/HookContextTest.java | 5 +- .../dev/openfeature/sdk/HookSupportTest.java | 14 +----- .../sdk/InitializeBehaviorSpecTest.java | 6 ++- .../openfeature/sdk/OpenFeatureAPITest.java | 8 ++-- .../sdk/ProviderRepositoryTest.java | 34 ++++++------- .../sdk/ShutdownBehaviorSpecTest.java | 14 +++--- .../sdk/testutils/TestEventsProvider.java | 12 +---- 22 files changed, 136 insertions(+), 193 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/ClientMetadata.java diff --git a/src/main/java/dev/openfeature/sdk/Client.java b/src/main/java/dev/openfeature/sdk/Client.java index ebca0b13..eb21ef54 100644 --- a/src/main/java/dev/openfeature/sdk/Client.java +++ b/src/main/java/dev/openfeature/sdk/Client.java @@ -6,7 +6,7 @@ * Interface used to resolve flags of varying types. */ public interface Client extends Features, EventBus { - Metadata getMetadata(); + ClientMetadata getMetadata(); /** * Return an optional client-level evaluation context. diff --git a/src/main/java/dev/openfeature/sdk/ClientMetadata.java b/src/main/java/dev/openfeature/sdk/ClientMetadata.java new file mode 100644 index 00000000..a860b474 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/ClientMetadata.java @@ -0,0 +1,10 @@ +package dev.openfeature.sdk; + +/** + * Holds identifying information about a Client. + */ +public interface ClientMetadata { + String getName(); + + String getDomain(); +} diff --git a/src/main/java/dev/openfeature/sdk/EventDetails.java b/src/main/java/dev/openfeature/sdk/EventDetails.java index d4ecac93..8f5529d2 100644 --- a/src/main/java/dev/openfeature/sdk/EventDetails.java +++ b/src/main/java/dev/openfeature/sdk/EventDetails.java @@ -10,7 +10,9 @@ @Data @SuperBuilder(toBuilder = true) public class EventDetails extends ProviderEventDetails { + @Deprecated private String clientName; + private String domain; private String providerName; static EventDetails fromProviderEventDetails(ProviderEventDetails providerEventDetails, String providerName) { @@ -20,9 +22,9 @@ static EventDetails fromProviderEventDetails(ProviderEventDetails providerEventD static EventDetails fromProviderEventDetails( ProviderEventDetails providerEventDetails, @Nullable String providerName, - @Nullable String clientName) { + @Nullable String domain) { return EventDetails.builder() - .clientName(clientName) + .domain(domain) .providerName(providerName) .flagsChanged(providerEventDetails.getFlagsChanged()) .eventMetadata(providerEventDetails.getEventMetadata()) diff --git a/src/main/java/dev/openfeature/sdk/EventSupport.java b/src/main/java/dev/openfeature/sdk/EventSupport.java index 49156b1a..4cf36bb0 100644 --- a/src/main/java/dev/openfeature/sdk/EventSupport.java +++ b/src/main/java/dev/openfeature/sdk/EventSupport.java @@ -34,19 +34,19 @@ class EventSupport { }); /** - * Run all the event handlers associated with this client name. - * If the client name is null, handlers attached to unnamed clients will run. + * Run all the event handlers associated with this domain. + * If the domain is null, handlers attached to unnamed clients will run. * - * @param clientName the client name to run event handlers for, or null + * @param domain the domain to run event handlers for, or null * @param event the event type * @param eventDetails the event details */ - public void runClientHandlers(@Nullable String clientName, ProviderEvent event, EventDetails eventDetails) { - clientName = Optional.ofNullable(clientName) + public void runClientHandlers(@Nullable String domain, ProviderEvent event, EventDetails eventDetails) { + domain = Optional.ofNullable(domain) .orElse(defaultClientUuid); // run handlers if they exist - Optional.ofNullable(handlerStores.get(clientName)) + Optional.ofNullable(handlerStores.get(domain)) .filter(store -> Optional.of(store).isPresent()) .map(store -> store.handlerMap.get(event)) .ifPresent(handlers -> handlers @@ -67,15 +67,14 @@ public void runGlobalHandlers(ProviderEvent event, EventDetails eventDetails) { } /** - * Add a handler for the specified client name, or all unnamed clients. + * Add a handler for the specified domain, or all unnamed clients. * - * @param clientName the client name to add handlers for, or else the unnamed - * client + * @param domain the domain to add handlers for, or else unnamed * @param event the event type * @param handler the handler function to run */ - public void addClientHandler(@Nullable String clientName, ProviderEvent event, Consumer handler) { - final String name = Optional.ofNullable(clientName) + public void addClientHandler(@Nullable String domain, ProviderEvent event, Consumer handler) { + final String name = Optional.ofNullable(domain) .orElse(defaultClientUuid); // lazily create and cache a HandlerStore if it doesn't exist @@ -91,15 +90,15 @@ public void addClientHandler(@Nullable String clientName, ProviderEvent event, C /** * Remove a client event handler for the specified event type. * - * @param clientName the name of the client handler to remove, or null to remove + * @param domain the domain of the client handler to remove, or null to remove * from unnamed clients * @param event the event type * @param handler the handler ref to be removed */ - public void removeClientHandler(String clientName, ProviderEvent event, Consumer handler) { - clientName = Optional.ofNullable(clientName) + public void removeClientHandler(String domain, ProviderEvent event, Consumer handler) { + domain = Optional.ofNullable(domain) .orElse(defaultClientUuid); - this.handlerStores.get(clientName).removeHandler(event, handler); + this.handlerStores.get(domain).removeHandler(event, handler); } /** @@ -123,11 +122,11 @@ public void removeGlobalHandler(ProviderEvent event, Consumer hand } /** - * Get all client names for which we have event handlers registered. + * Get all domain names for which we have event handlers registered. * - * @return set of client names + * @return set of domain names */ - public Set getAllClientNames() { + public Set getAllDomainNames() { return this.handlerStores.keySet(); } diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index 26b14f79..5c9091b8 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -16,7 +16,7 @@ public class HookContext { @NonNull FlagValueType type; @NonNull T defaultValue; @NonNull EvaluationContext ctx; - Metadata clientMetadata; + ClientMetadata clientMetadata; Metadata providerMetadata; /** @@ -30,7 +30,7 @@ public class HookContext { * @param type that the flag is evaluating against * @return resulting context for hook */ - public static HookContext from(String key, FlagValueType type, Metadata clientMetadata, + public static HookContext from(String key, FlagValueType type, ClientMetadata clientMetadata, Metadata providerMetadata, EvaluationContext ctx, T defaultValue) { return HookContext.builder() .flagKey(key) diff --git a/src/main/java/dev/openfeature/sdk/Metadata.java b/src/main/java/dev/openfeature/sdk/Metadata.java index 2f1d5ffa..7e614c27 100644 --- a/src/main/java/dev/openfeature/sdk/Metadata.java +++ b/src/main/java/dev/openfeature/sdk/Metadata.java @@ -5,6 +5,4 @@ */ public interface Metadata { String getName(); - - String getDomain(); } diff --git a/src/main/java/dev/openfeature/sdk/NoOpProvider.java b/src/main/java/dev/openfeature/sdk/NoOpProvider.java index 4304b2cf..ef8cf1f8 100644 --- a/src/main/java/dev/openfeature/sdk/NoOpProvider.java +++ b/src/main/java/dev/openfeature/sdk/NoOpProvider.java @@ -18,17 +18,7 @@ public ProviderState getState() { @Override public Metadata getMetadata() { - return new Metadata() { - @Override - public String getName() { - return name; - } - - @Override - public String getDomain() { - return name; - } - }; + return () -> name; } @Override diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index f89bb46a..d1e475b8 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -53,8 +53,8 @@ public Metadata getProviderMetadata() { return getProvider().getMetadata(); } - public Metadata getProviderMetadata(String clientName) { - return getProvider(clientName).getMetadata(); + public Metadata getProviderMetadata(String domain) { + return getProvider(domain).getMetadata(); } /** @@ -240,13 +240,13 @@ public FeatureProvider getProvider() { } /** - * Fetch a provider for a named client. If not found, return the default. + * Fetch a provider for a domain. If not found, return the default. * - * @param name The client name to look for. + * @param domain The domain to look for. * @return A named {@link FeatureProvider} */ - public FeatureProvider getProvider(String name) { - return providerRepository.getProvider(name); + public FeatureProvider getProvider(String domain) { + return providerRepository.getProvider(domain); } /** @@ -344,20 +344,20 @@ public OpenFeatureAPI removeHandler(ProviderEvent event, Consumer return this; } - void removeHandler(String clientName, ProviderEvent event, Consumer handler) { + void removeHandler(String domain, ProviderEvent event, Consumer handler) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { - eventSupport.removeClientHandler(clientName, event, handler); + eventSupport.removeClientHandler(domain, event, handler); } } - void addHandler(String clientName, ProviderEvent event, Consumer handler) { + void addHandler(String domain, ProviderEvent event, Consumer handler) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { // if the provider is in the state associated with event, run immediately - if (Optional.ofNullable(this.providerRepository.getProvider(clientName).getState()) + if (Optional.ofNullable(this.providerRepository.getProvider(domain).getState()) .orElse(ProviderState.READY).matchesEvent(event)) { - eventSupport.runHandler(handler, EventDetails.builder().clientName(clientName).build()); + eventSupport.runHandler(handler, EventDetails.builder().domain(domain).build()); } - eventSupport.addClientHandler(clientName, event, handler); + eventSupport.addClientHandler(domain, event, handler); } } @@ -371,8 +371,8 @@ void addHandler(String clientName, ProviderEvent event, Consumer h private void runHandlersForProvider(FeatureProvider provider, ProviderEvent event, ProviderEventDetails details) { try (AutoCloseableLock __ = lock.readLockAutoCloseable()) { - List clientNamesForProvider = providerRepository - .getClientNamesForProvider(provider); + List domainsForProvider = providerRepository + .getDomainsForProvider(provider); final String providerName = Optional.ofNullable(provider.getMetadata()) .map(metadata -> metadata.getName()) @@ -381,20 +381,20 @@ private void runHandlersForProvider(FeatureProvider provider, ProviderEvent even // run the global handlers eventSupport.runGlobalHandlers(event, EventDetails.fromProviderEventDetails(details, providerName)); - // run the handlers associated with named clients for this provider - clientNamesForProvider.forEach(name -> { - eventSupport.runClientHandlers(name, event, - EventDetails.fromProviderEventDetails(details, providerName, name)); + // run the handlers associated with domains for this provider + domainsForProvider.forEach(domain -> { + eventSupport.runClientHandlers(domain, event, + EventDetails.fromProviderEventDetails(details, providerName, domain)); }); if (providerRepository.isDefaultProvider(provider)) { // run handlers for clients that have no bound providers (since this is the default) - Set allClientNames = eventSupport.getAllClientNames(); - Set boundClientNames = providerRepository.getAllBoundClientNames(); - allClientNames.removeAll(boundClientNames); - allClientNames.forEach(name -> { - eventSupport.runClientHandlers(name, event, - EventDetails.fromProviderEventDetails(details, providerName, name)); + Set allDomainNames = eventSupport.getAllDomainNames(); + Set boundDomains = providerRepository.getAllBoundDomains(); + allDomainNames.removeAll(boundDomains); + allDomainNames.forEach(domain -> { + eventSupport.runClientHandlers(domain, event, + EventDetails.fromProviderEventDetails(details, providerName, domain)); }); } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 826e2fdb..6f78d8d7 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -109,7 +109,7 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key try { // openfeatureApi.getProvider() must be called once to maintain a consistent reference - provider = openfeatureApi.getProvider(this.name); + provider = openfeatureApi.getProvider(this.domain); mergedHooks = ObjectUtils.merge(provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, openfeatureApi.getHooks()); @@ -354,8 +354,8 @@ public FlagEvaluationDetails getObjectDetails(String key, Value defaultVa } @Override - public Metadata getMetadata() { - return new Metadata() { + public ClientMetadata getMetadata() { + return new ClientMetadata() { @Override public String getName() { return name; @@ -405,7 +405,7 @@ public Client onProviderStale(Consumer handler) { */ @Override public Client on(ProviderEvent event, Consumer handler) { - OpenFeatureAPI.getInstance().addHandler(name, event, handler); + OpenFeatureAPI.getInstance().addHandler(domain, event, handler); return this; } @@ -414,7 +414,7 @@ public Client on(ProviderEvent event, Consumer handler) { */ @Override public Client removeHandler(ProviderEvent event, Consumer handler) { - OpenFeatureAPI.getInstance().removeHandler(name, event, handler); + OpenFeatureAPI.getInstance().removeHandler(domain, event, handler); return this; } } diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 68863878..d2cae8f1 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -38,22 +38,22 @@ public FeatureProvider getProvider() { } /** - * Fetch a provider for a named client. If not found, return the default. + * Fetch a provider for a domain. If not found, return the default. * - * @param name The client name to look for. + * @param domain The domain to look for. * @return A named {@link FeatureProvider} */ - public FeatureProvider getProvider(String name) { - return Optional.ofNullable(name).map(this.providers::get).orElse(this.defaultProvider.get()); + public FeatureProvider getProvider(String domain) { + return Optional.ofNullable(domain).map(this.providers::get).orElse(this.defaultProvider.get()); } - public List getClientNamesForProvider(FeatureProvider provider) { + public List getDomainsForProvider(FeatureProvider provider) { return providers.entrySet().stream() .filter(entry -> entry.getValue().equals(provider)) .map(entry -> entry.getKey()).collect(Collectors.toList()); } - public Set getAllBoundClientNames() { + public Set getAllBoundDomains() { return providers.keySet(); } diff --git a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java index 61923642..8cd9fc8d 100644 --- a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java +++ b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java @@ -40,17 +40,7 @@ public class InMemoryProvider extends EventProvider { @Override public Metadata getMetadata() { - return new Metadata() { - @Override - public String getName() { - return NAME; - } - - @Override - public String getDomain() { - return NAME; - } - }; + return () -> NAME; } public InMemoryProvider(Map> flags) { diff --git a/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java b/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java index 9f10f579..c1097623 100644 --- a/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java +++ b/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java @@ -6,16 +6,8 @@ public class AlwaysBrokenProvider implements FeatureProvider { @Override public Metadata getMetadata() { - return new Metadata() { - @Override - public String getName() { - throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE); - } - - @Override - public String getDomain() { - throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE); - } + return () -> { + throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE); }; } diff --git a/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java b/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java index 74efabb7..4fdc433b 100644 --- a/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java +++ b/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java @@ -23,17 +23,7 @@ EvaluationContext getMergedContext() { @Override public Metadata getMetadata() { - return new Metadata() { - @Override - public String getName() { - return name; - } - - @Override - public String getDomain() { - return name; - } - }; + return () -> name; } @Override diff --git a/src/test/java/dev/openfeature/sdk/EventProviderTest.java b/src/test/java/dev/openfeature/sdk/EventProviderTest.java index 8f41ef7c..a5be8589 100644 --- a/src/test/java/dev/openfeature/sdk/EventProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/EventProviderTest.java @@ -7,7 +7,6 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import org.checkerframework.checker.units.qual.N; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -82,17 +81,7 @@ class TestEventProvider extends EventProvider { @Override public Metadata getMetadata() { - return new Metadata() { - @Override - public String getName() { - return NAME; - } - - @Override - public String getDomain() { - return NAME; - } - }; + return () -> NAME; } @Override diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index f9f8e4b8..6b70617f 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -48,7 +48,7 @@ class Initialization { @Specification(number = "5.3.1", text = "If the provider's initialize function terminates normally," + " PROVIDER_READY handlers MUST run.") void apiInitReady() { - final Consumer handler = (Consumer)mockHandler(); + final Consumer handler = mockHandler(); final String name = "apiInitReady"; TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); @@ -105,7 +105,7 @@ void apiShouldPropagateEvents() { @Specification(number = "5.2.2", text = "The API MUST provide a function for associating handler functions" + " with a particular provider event type.") - void apiShouldSupportAllEventTypes() throws Exception { + void apiShouldSupportAllEventTypes() { final String name = "apiShouldSupportAllEventTypes"; final Consumer handler1 = mockHandler(); final Consumer handler2 = mockHandler(); @@ -191,7 +191,7 @@ class Initialization { @Test @DisplayName("should fire initial READY event when provider init succeeds after client retrieved") @Specification(number = "5.3.1", text = "If the provider's initialize function terminates normally, PROVIDER_READY handlers MUST run.") - void initReadyProviderBefore() throws InterruptedException { + void initReadyProviderBefore() { final Consumer handler = mockHandler(); final String name = "initReadyProviderBefore"; @@ -201,7 +201,7 @@ void initReadyProviderBefore() throws InterruptedException { // set provider after getting a client OpenFeatureAPI.getInstance().setProvider(name, provider); verify(handler, timeout(TIMEOUT).atLeastOnce()) - .accept(argThat(details -> details.getClientName().equals(name))); + .accept(argThat(details -> details.getDomain().equals(name))); } @Test @@ -217,7 +217,7 @@ void initReadyProviderAfter() { Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderReady(handler); verify(handler, timeout(TIMEOUT).atLeastOnce()) - .accept(argThat(details -> details.getClientName().equals(name))); + .accept(argThat(details -> details.getDomain().equals(name))); } @Test @@ -234,7 +234,7 @@ void initErrorProviderAfter() { // set provider after getting a client OpenFeatureAPI.getInstance().setProvider(name, provider); verify(handler, timeout(TIMEOUT)).accept(argThat(details -> { - return name.equals(details.getClientName()) + return name.equals(details.getDomain()) && errMessage.equals(details.getMessage()); })); } @@ -253,7 +253,7 @@ void initErrorProviderBefore() { Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderError(handler); verify(handler, timeout(TIMEOUT)).accept(argThat(details -> { - return name.equals(details.getClientName()) + return name.equals(details.getDomain()) && errMessage.equals(details.getMessage()); })); } @@ -277,7 +277,7 @@ void shouldPropagateBefore() { client.onProviderConfigurationChanged(handler); provider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, EventDetails.builder().build()); - verify(handler, timeout(TIMEOUT)).accept(argThat(details -> details.getClientName().equals(name))); + verify(handler, timeout(TIMEOUT)).accept(argThat(details -> details.getDomain().equals(name))); } @Test @@ -295,7 +295,7 @@ void shouldPropagateAfter() { OpenFeatureAPI.getInstance().setProvider(name, provider); provider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, EventDetails.builder().build()); - verify(handler, timeout(TIMEOUT)).accept(argThat(details -> details.getClientName().equals(name))); + verify(handler, timeout(TIMEOUT)).accept(argThat(details -> details.getDomain().equals(name))); } @Test @@ -306,7 +306,7 @@ void shouldPropagateAfter() { @Specification(number = "5.2.1", text = "The client MUST provide a function for associating handler functions" + " with a particular provider event type.") - void shouldSupportAllEventTypes() throws Exception { + void shouldSupportAllEventTypes() { final String name = "shouldSupportAllEventTypes"; final Consumer handler1 = mockHandler(); final Consumer handler2 = mockHandler(); @@ -325,7 +325,7 @@ void shouldSupportAllEventTypes() throws Exception { Arrays.asList(ProviderEvent.values()).stream().forEach(eventType -> { provider.mockEvent(eventType, ProviderEventDetails.builder().build()); }); - ArgumentMatcher nameMatches = (EventDetails details) -> details.getClientName() + ArgumentMatcher nameMatches = (EventDetails details) -> details.getDomain() .equals(name); verify(handler1, timeout(TIMEOUT).atLeastOnce()).accept(argThat(nameMatches)); verify(handler2, timeout(TIMEOUT).atLeastOnce()).accept(argThat(nameMatches)); @@ -337,7 +337,7 @@ void shouldSupportAllEventTypes() throws Exception { @Test @DisplayName("shutdown provider should not run handlers") - void shouldNotRunHandlers() throws Exception { + void shouldNotRunHandlers() { final Consumer handler1 = mockHandler(); final Consumer handler2 = mockHandler(); final String name = "shouldNotRunHandlers"; @@ -369,7 +369,7 @@ void shouldNotRunHandlers() throws Exception { @DisplayName("other client handlers should not run") @Specification(number = "5.1.3", text = "When a provider signals the occurrence of a particular event, " + "event handlers on clients which are not associated with that provider MUST NOT run.") - void otherClientHandlersShouldNotRun() throws Exception { + void otherClientHandlersShouldNotRun() { final String name1 = "otherClientHandlersShouldNotRun1"; final String name2 = "otherClientHandlersShouldNotRun2"; final Consumer handlerToRun = mockHandler(); @@ -396,7 +396,7 @@ void otherClientHandlersShouldNotRun() throws Exception { @DisplayName("bound named client handlers should not run with default") @Specification(number = "5.1.3", text = "When a provider signals the occurrence of a particular event, " + "event handlers on clients which are not associated with that provider MUST NOT run.") - void boundShouldNotRunWithDefault() throws Exception { + void boundShouldNotRunWithDefault() { final String name = "boundShouldNotRunWithDefault"; final Consumer handlerNotToRun = mockHandler(); @@ -422,7 +422,7 @@ void boundShouldNotRunWithDefault() throws Exception { @DisplayName("unbound named client handlers should run with default") @Specification(number = "5.1.3", text = "When a provider signals the occurrence of a particular event, " + "event handlers on clients which are not associated with that provider MUST NOT run.") - void unboundShouldRunWithDefault() throws Exception { + void unboundShouldRunWithDefault() { final String name = "unboundShouldRunWithDefault"; final Consumer handlerToRun = mockHandler(); @@ -445,7 +445,7 @@ void unboundShouldRunWithDefault() throws Exception { @Test @DisplayName("subsequent handlers run if earlier throws") @Specification(number = "5.2.5", text = "If a handler function terminates abnormally, other handler functions MUST run.") - void handlersRunIfOneThrows() throws Exception { + void handlersRunIfOneThrows() { final String name = "handlersRunIfOneThrows"; final Consumer errorHandler = mockHandler(); doThrow(new NullPointerException()).when(errorHandler).accept(any()); @@ -471,7 +471,7 @@ void handlersRunIfOneThrows() throws Exception { @DisplayName("should have all properties") @Specification(number = "5.2.4", text = "The handler function MUST accept a event details parameter.") @Specification(number = "5.2.3", text = "The `event details` MUST contain the `provider name` associated with the event.") - void shouldHaveAllProperties() throws Exception { + void shouldHaveAllProperties() { final Consumer handler1 = mockHandler(); final Consumer handler2 = mockHandler(); final String name = "shouldHaveAllProperties"; @@ -508,14 +508,14 @@ void shouldHaveAllProperties() throws Exception { return metadata.equals(eventDetails.getEventMetadata()) && flagsChanged.equals(eventDetails.getFlagsChanged()) && message.equals(eventDetails.getMessage()) - && name.equals(eventDetails.getClientName()); + && name.equals(eventDetails.getDomain()); })); } @Test @DisplayName("if the provider is ready handlers must run immediately") @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") - void matchingReadyEventsMustRunImmediately() throws Exception { + void matchingReadyEventsMustRunImmediately() { final String name = "matchingEventsMustRunImmediately"; final Consumer handler = mockHandler(); @@ -532,7 +532,7 @@ void matchingReadyEventsMustRunImmediately() throws Exception { @Test @DisplayName("if the provider is ready handlers must run immediately") @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") - void matchingStaleEventsMustRunImmediately() throws Exception { + void matchingStaleEventsMustRunImmediately() { final String name = "matchingEventsMustRunImmediately"; final Consumer handler = mockHandler(); @@ -549,7 +549,7 @@ void matchingStaleEventsMustRunImmediately() throws Exception { @Test @DisplayName("if the provider is ready handlers must run immediately") @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") - void matchingErrorEventsMustRunImmediately() throws Exception { + void matchingErrorEventsMustRunImmediately() { final String name = "matchingEventsMustRunImmediately"; final Consumer handler = mockHandler(); @@ -566,7 +566,7 @@ void matchingErrorEventsMustRunImmediately() throws Exception { @Test @DisplayName("must persist across changes") @Specification(number = "5.2.6", text = "Event handlers MUST persist across provider changes.") - void mustPersistAcrossChanges() throws Exception { + void mustPersistAcrossChanges() { final String name = "mustPersistAcrossChanges"; final Consumer handler = mockHandler(); @@ -578,7 +578,7 @@ void mustPersistAcrossChanges() throws Exception { client.onProviderConfigurationChanged(handler); provider1.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, ProviderEventDetails.builder().build()); - ArgumentMatcher nameMatches = (EventDetails details) -> details.getClientName().equals(name); + ArgumentMatcher nameMatches = (EventDetails details) -> details.getDomain().equals(name); verify(handler, timeout(TIMEOUT).times(1)).accept(argThat(nameMatches)); diff --git a/src/test/java/dev/openfeature/sdk/HookContextTest.java b/src/test/java/dev/openfeature/sdk/HookContextTest.java index 763069fd..50cc6617 100644 --- a/src/test/java/dev/openfeature/sdk/HookContextTest.java +++ b/src/test/java/dev/openfeature/sdk/HookContextTest.java @@ -9,17 +9,18 @@ class HookContextTest { @Specification(number="4.2.2.2", text="Condition: The client metadata field in the hook context MUST be immutable.") @Specification(number="4.2.2.3", text="Condition: The provider metadata field in the hook context MUST be immutable.") @Test void metadata_field_is_type_metadata() { + ClientMetadata clientMetadata = mock(ClientMetadata.class); Metadata meta = mock(Metadata.class); HookContext hc = HookContext.from( "key", FlagValueType.BOOLEAN, - meta, + clientMetadata, meta, new ImmutableContext(), false ); - assertTrue(Metadata.class.isAssignableFrom(hc.getClientMetadata().getClass())); + assertTrue(ClientMetadata.class.isAssignableFrom(hc.getClientMetadata().getClass())); assertTrue(Metadata.class.isAssignableFrom(hc.getProviderMetadata().getClass())); } diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index da1f11e1..d95d9e64 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -20,7 +20,7 @@ class HookSupportTest implements HookFixtures { - private Metadata clientMetadata = new Metadata() { + private ClientMetadata clientMetadata = new ClientMetadata() { @Override public String getName() { return "client"; @@ -32,17 +32,7 @@ public String getDomain() { } }; - private Metadata providerMetadata = new Metadata() { - @Override - public String getName() { - return "provider"; - } - - @Override - public String getDomain() { - return "domain"; - } - }; + private Metadata providerMetadata = () -> "provider"; @Test @DisplayName("should merge EvaluationContexts on before hooks correctly") diff --git a/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java b/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java index 270ac77e..4d0599a7 100644 --- a/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java @@ -8,6 +8,8 @@ class InitializeBehaviorSpecTest { + private static final String DOMAIN_NAME = "mydomain"; + @BeforeEach void setupTest() { OpenFeatureAPI.getInstance().setProvider(new NoOpProvider()); @@ -60,7 +62,7 @@ void mustCallInitializeFunctionOfTheNewlyRegisteredNamedProviderBeforeUsingItFor FeatureProvider featureProvider = mock(FeatureProvider.class); doReturn(ProviderState.NOT_READY).when(featureProvider).getState(); - OpenFeatureAPI.getInstance().setProvider("clientName", featureProvider); + OpenFeatureAPI.getInstance().setProvider(DOMAIN_NAME, featureProvider); verify(featureProvider, timeout(1000)).initialize(any()); } @@ -76,7 +78,7 @@ void shouldCatchExceptionThrownByTheNamedClientProviderOnInitialization() throws doReturn(ProviderState.NOT_READY).when(featureProvider).getState(); doThrow(TestException.class).when(featureProvider).initialize(any()); - assertThatCode(() -> OpenFeatureAPI.getInstance().setProvider("clientName", featureProvider)) + assertThatCode(() -> OpenFeatureAPI.getInstance().setProvider(DOMAIN_NAME, featureProvider)) .doesNotThrowAnyException(); verify(featureProvider, timeout(1000)).initialize(any()); diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java index cc515f29..4ad64d4f 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java @@ -14,7 +14,7 @@ class OpenFeatureAPITest { - private static final String CLIENT_NAME = "client name"; + private static final String DOMAIN_NAME = "my domain"; private OpenFeatureAPI api; @@ -41,7 +41,7 @@ void namedProviderOverwrittenTest() { FeatureProviderTestUtils.setFeatureProvider(domain, provider1); FeatureProviderTestUtils.setFeatureProvider(domain, provider2); - assertThat(OpenFeatureAPI.getInstance().getProvider(domain).getMetadata().getDomain()) + assertThat(OpenFeatureAPI.getInstance().getProvider(domain).getMetadata().getName()) .isEqualTo(DoSomethingProvider.name); } @@ -70,8 +70,8 @@ void settingDefaultProviderToNullErrors() { } @Test - void settingNamedClientProviderToNullErrors() { - assertThatCode(() -> api.setProvider(CLIENT_NAME, null)).isInstanceOf(IllegalArgumentException.class); + void settingDomainProviderToNullErrors() { + assertThatCode(() -> api.setProvider(DOMAIN_NAME, null)).isInstanceOf(IllegalArgumentException.class); } @Test diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index 20da47ed..c827c91f 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -33,8 +33,8 @@ class ProviderRepositoryTest { - private static final String CLIENT_NAME = "client name"; - private static final String ANOTHER_CLIENT_NAME = "another client name"; + private static final String DOMAIN_NAME = "domain name"; + private static final String ANOTHER_DOMAIN_NAME = "another domain name"; private static final int TIMEOUT = 5000; private final ExecutorService executorService = Executors.newCachedThreadPool(); @@ -101,13 +101,13 @@ class NamedProvider { @Test @DisplayName("should reject null as named provider") void shouldRejectNullAsNamedProvider() { - assertThatCode(() -> providerRepository.setProvider(CLIENT_NAME, null, mockAfterSet(), mockAfterInit(), + assertThatCode(() -> providerRepository.setProvider(DOMAIN_NAME, null, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), mockAfterError(), false)) .isInstanceOf(IllegalArgumentException.class); } @Test - @DisplayName("should reject null as client name") + @DisplayName("should reject null as domain name") void shouldRejectNullAsDefaultProvider() { NoOpProvider provider = new NoOpProvider(); assertThatCode(() -> providerRepository.setProvider(null, provider, mockAfterSet(), mockAfterInit(), @@ -116,8 +116,8 @@ void shouldRejectNullAsDefaultProvider() { } @Test - @DisplayName("should immediately return when calling the named client provider mutator") - void shouldImmediatelyReturnWhenCallingTheNamedClientProviderMutator() throws Exception { + @DisplayName("should immediately return when calling the domain provider mutator") + void shouldImmediatelyReturnWhenCallingTheDomainProviderMutator() throws Exception { FeatureProvider featureProvider = createMockedProvider(); doDelayResponse(Duration.ofSeconds(10)).when(featureProvider).initialize(any()); @@ -126,7 +126,7 @@ void shouldImmediatelyReturnWhenCallingTheNamedClientProviderMutator() throws Ex .pollDelay(Duration.ofMillis(1)) .atMost(Duration.ofSeconds(1)) .until(() -> { - providerRepository.setProvider("named client", featureProvider, mockAfterSet(), + providerRepository.setProvider("a domain", featureProvider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), mockAfterError(), false); verify(featureProvider, timeout(TIMEOUT)).initialize(any()); return true; @@ -137,7 +137,7 @@ void shouldImmediatelyReturnWhenCallingTheNamedClientProviderMutator() throws Ex @DisplayName("should avoid additional initialization call if provider has been initialized already") void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() throws Exception { FeatureProvider provider = createMockedReadyProvider(); - setFeatureProvider(CLIENT_NAME, provider); + setFeatureProvider(DOMAIN_NAME, provider); verify(provider, never()).initialize(any()); } @@ -176,7 +176,7 @@ void shouldNotCallShutdownIfReplacedDefaultProviderIsBoundAsNamedProvider() { FeatureProvider oldProvider = createMockedProvider(); FeatureProvider newProvider = createMockedProvider(); setFeatureProvider(oldProvider); - setFeatureProvider(CLIENT_NAME, oldProvider); + setFeatureProvider(DOMAIN_NAME, oldProvider); setFeatureProvider(newProvider); @@ -194,7 +194,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { doDelayResponse(Duration.ofSeconds(10)).when(newProvider).initialize(any()); Future providerMutation = executorService - .submit(() -> providerRepository.setProvider(CLIENT_NAME, newProvider, mockAfterSet(), + .submit(() -> providerRepository.setProvider(DOMAIN_NAME, newProvider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), mockAfterError(), false)); await() @@ -209,11 +209,11 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { void shouldNotCallShutdownIfReplacedProviderIsBoundToMultipleNames() throws InterruptedException { FeatureProvider oldProvider = createMockedProvider(); FeatureProvider newProvider = createMockedProvider(); - setFeatureProvider(CLIENT_NAME, oldProvider); + setFeatureProvider(DOMAIN_NAME, oldProvider); - setFeatureProvider(ANOTHER_CLIENT_NAME, oldProvider); + setFeatureProvider(ANOTHER_DOMAIN_NAME, oldProvider); - setFeatureProvider(CLIENT_NAME, newProvider); + setFeatureProvider(DOMAIN_NAME, newProvider); verify(oldProvider, never()).shutdown(); } @@ -224,9 +224,9 @@ void shouldNotCallShutdownIfReplacedProviderIsBoundAsDefaultProvider() { FeatureProvider oldProvider = createMockedProvider(); FeatureProvider newProvider = createMockedProvider(); setFeatureProvider(oldProvider); - setFeatureProvider(CLIENT_NAME, oldProvider); + setFeatureProvider(DOMAIN_NAME, oldProvider); - setFeatureProvider(CLIENT_NAME, newProvider); + setFeatureProvider(DOMAIN_NAME, newProvider); verify(oldProvider, never()).shutdown(); } @@ -293,8 +293,8 @@ void shouldShutdownAllFeatureProvidersOnShutdown() { FeatureProvider featureProvider2 = createMockedProvider(); setFeatureProvider(featureProvider1); - setFeatureProvider(CLIENT_NAME, featureProvider1); - setFeatureProvider(ANOTHER_CLIENT_NAME, featureProvider2); + setFeatureProvider(DOMAIN_NAME, featureProvider1); + setFeatureProvider(ANOTHER_DOMAIN_NAME, featureProvider2); providerRepository.shutdown(); verify(featureProvider1, timeout(TIMEOUT)).shutdown(); diff --git a/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java b/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java index b0a0a9d1..bc2dc0ea 100644 --- a/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java @@ -15,6 +15,8 @@ class ShutdownBehaviorSpecTest { + private String DOMAIN = "myDomain"; + @BeforeEach void resetFeatureProvider() { setFeatureProvider(new NoOpProvider()); @@ -59,11 +61,10 @@ class NamedProvider { @Test @DisplayName("must invoke shutdown method on previously registered provider once it should not be used for flag evaluation anymore") void mustInvokeShutdownMethodOnPreviouslyRegisteredProviderOnceItShouldNotBeUsedForFlagEvaluationAnymore() { - String clientName = "clientName"; FeatureProvider featureProvider = ProviderFixture.createMockedProvider(); - setFeatureProvider(clientName, featureProvider); - setFeatureProvider(clientName, new NoOpProvider()); + setFeatureProvider(DOMAIN, featureProvider); + setFeatureProvider(DOMAIN, new NoOpProvider()); verify(featureProvider, timeout(1000)).shutdown(); } @@ -75,12 +76,11 @@ void mustInvokeShutdownMethodOnPreviouslyRegisteredProviderOnceItShouldNotBeUsed @Test @DisplayName("should catch exception thrown by the named client provider on shutdown") void shouldCatchExceptionThrownByTheNamedClientProviderOnShutdown() { - String clientName = "clientName"; FeatureProvider featureProvider = ProviderFixture.createMockedProvider(); doThrow(TestException.class).when(featureProvider).shutdown(); - setFeatureProvider(clientName, featureProvider); - setFeatureProvider(clientName, new NoOpProvider()); + setFeatureProvider(DOMAIN, featureProvider); + setFeatureProvider(DOMAIN, new NoOpProvider()); verify(featureProvider, timeout(1000)).shutdown(); } @@ -96,7 +96,7 @@ void mustShutdownAllProvidersOnShuttingDownApi() { FeatureProvider defaultProvider = ProviderFixture.createMockedProvider(); FeatureProvider namedProvider = ProviderFixture.createMockedProvider(); setFeatureProvider(defaultProvider); - setFeatureProvider("clientName", namedProvider); + setFeatureProvider(DOMAIN, namedProvider); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); synchronized (OpenFeatureAPI.class) { diff --git a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java index fbe98a78..0b19f82a 100644 --- a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java +++ b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java @@ -18,17 +18,7 @@ public class TestEventsProvider extends EventProvider { private boolean shutDown = false; private int initTimeoutMs = 0; private String name = "test"; - private Metadata metadata = new Metadata() { - @Override - public String getName() { - return name; - } - - @Override - public String getDomain() { - return name; - } - }; + private Metadata metadata = () -> name; @Override public ProviderState getState() { From 25b0911d654ace5588c20595aff68d409264a6c6 Mon Sep 17 00:00:00 2001 From: jarebudev <23311805+jarebudev@users.noreply.github.com> Date: Mon, 20 May 2024 22:44:20 +0100 Subject: [PATCH 3/5] applied code review suggested changes Signed-off-by: jarebudev <23311805+jarebudev@users.noreply.github.com> --- src/main/java/dev/openfeature/sdk/Client.java | 2 +- src/main/java/dev/openfeature/sdk/ClientMetadata.java | 10 ---------- src/main/java/dev/openfeature/sdk/EventDetails.java | 1 + src/main/java/dev/openfeature/sdk/HookContext.java | 4 ++-- src/main/java/dev/openfeature/sdk/Metadata.java | 4 ++++ .../java/dev/openfeature/sdk/OpenFeatureClient.java | 4 ++-- src/test/java/dev/openfeature/sdk/HookContextTest.java | 4 ++-- src/test/java/dev/openfeature/sdk/HookSupportTest.java | 2 +- 8 files changed, 13 insertions(+), 18 deletions(-) delete mode 100644 src/main/java/dev/openfeature/sdk/ClientMetadata.java diff --git a/src/main/java/dev/openfeature/sdk/Client.java b/src/main/java/dev/openfeature/sdk/Client.java index 2184cdcb..4494180a 100644 --- a/src/main/java/dev/openfeature/sdk/Client.java +++ b/src/main/java/dev/openfeature/sdk/Client.java @@ -6,7 +6,7 @@ * Interface used to resolve flags of varying types. */ public interface Client extends Features, EventBus { - ClientMetadata getMetadata(); + Metadata getMetadata(); /** * Return an optional client-level evaluation context. diff --git a/src/main/java/dev/openfeature/sdk/ClientMetadata.java b/src/main/java/dev/openfeature/sdk/ClientMetadata.java deleted file mode 100644 index a860b474..00000000 --- a/src/main/java/dev/openfeature/sdk/ClientMetadata.java +++ /dev/null @@ -1,10 +0,0 @@ -package dev.openfeature.sdk; - -/** - * Holds identifying information about a Client. - */ -public interface ClientMetadata { - String getName(); - - String getDomain(); -} diff --git a/src/main/java/dev/openfeature/sdk/EventDetails.java b/src/main/java/dev/openfeature/sdk/EventDetails.java index a1a8f84b..0e051675 100644 --- a/src/main/java/dev/openfeature/sdk/EventDetails.java +++ b/src/main/java/dev/openfeature/sdk/EventDetails.java @@ -23,6 +23,7 @@ static EventDetails fromProviderEventDetails( String providerName, String domain) { return EventDetails.builder() + .clientName(domain) .domain(domain) .providerName(providerName) .flagsChanged(providerEventDetails.getFlagsChanged()) diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index 5c9091b8..26b14f79 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -16,7 +16,7 @@ public class HookContext { @NonNull FlagValueType type; @NonNull T defaultValue; @NonNull EvaluationContext ctx; - ClientMetadata clientMetadata; + Metadata clientMetadata; Metadata providerMetadata; /** @@ -30,7 +30,7 @@ public class HookContext { * @param type that the flag is evaluating against * @return resulting context for hook */ - public static HookContext from(String key, FlagValueType type, ClientMetadata clientMetadata, + public static HookContext from(String key, FlagValueType type, Metadata clientMetadata, Metadata providerMetadata, EvaluationContext ctx, T defaultValue) { return HookContext.builder() .flagKey(key) diff --git a/src/main/java/dev/openfeature/sdk/Metadata.java b/src/main/java/dev/openfeature/sdk/Metadata.java index 7e614c27..77db8179 100644 --- a/src/main/java/dev/openfeature/sdk/Metadata.java +++ b/src/main/java/dev/openfeature/sdk/Metadata.java @@ -5,4 +5,8 @@ */ public interface Metadata { String getName(); + + default String getDomain() { + return ""; + } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 9ba9836c..159e9b9f 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -356,8 +356,8 @@ public FlagEvaluationDetails getObjectDetails(String key, Value defaultVa } @Override - public ClientMetadata getMetadata() { - return new ClientMetadata() { + public Metadata getMetadata() { + return new Metadata() { @Override public String getName() { return name; diff --git a/src/test/java/dev/openfeature/sdk/HookContextTest.java b/src/test/java/dev/openfeature/sdk/HookContextTest.java index 50cc6617..c65f6ce4 100644 --- a/src/test/java/dev/openfeature/sdk/HookContextTest.java +++ b/src/test/java/dev/openfeature/sdk/HookContextTest.java @@ -9,7 +9,7 @@ class HookContextTest { @Specification(number="4.2.2.2", text="Condition: The client metadata field in the hook context MUST be immutable.") @Specification(number="4.2.2.3", text="Condition: The provider metadata field in the hook context MUST be immutable.") @Test void metadata_field_is_type_metadata() { - ClientMetadata clientMetadata = mock(ClientMetadata.class); + Metadata clientMetadata = mock(Metadata.class); Metadata meta = mock(Metadata.class); HookContext hc = HookContext.from( "key", @@ -20,7 +20,7 @@ class HookContextTest { false ); - assertTrue(ClientMetadata.class.isAssignableFrom(hc.getClientMetadata().getClass())); + assertTrue(Metadata.class.isAssignableFrom(hc.getClientMetadata().getClass())); assertTrue(Metadata.class.isAssignableFrom(hc.getProviderMetadata().getClass())); } diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index d95d9e64..2650236c 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -20,7 +20,7 @@ class HookSupportTest implements HookFixtures { - private ClientMetadata clientMetadata = new ClientMetadata() { + private Metadata clientMetadata = new Metadata() { @Override public String getName() { return "client"; From 2d2c5cfe708cb0899b4b8b2d39735239a4292325 Mon Sep 17 00:00:00 2001 From: Kavindu Dodanduwa Date: Thu, 30 May 2024 12:31:44 -0700 Subject: [PATCH 4/5] introdcue a dedicated interface for client metadata Signed-off-by: Kavindu Dodanduwa --- src/main/java/dev/openfeature/sdk/Client.java | 2 +- .../dev/openfeature/sdk/ClientMetadata.java | 8 ++++++++ .../dev/openfeature/sdk/EventDetails.java | 3 --- .../java/dev/openfeature/sdk/HookContext.java | 4 ++-- .../java/dev/openfeature/sdk/Metadata.java | 4 ---- .../dev/openfeature/sdk/OpenFeatureAPI.java | 10 +++++----- .../openfeature/sdk/OpenFeatureClient.java | 17 ++--------------- .../sdk/FlagEvaluationSpecTest.java | 2 -- .../dev/openfeature/sdk/HookContextTest.java | 4 ++-- .../dev/openfeature/sdk/HookSupportTest.java | 19 ++----------------- 10 files changed, 22 insertions(+), 51 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/ClientMetadata.java diff --git a/src/main/java/dev/openfeature/sdk/Client.java b/src/main/java/dev/openfeature/sdk/Client.java index 4494180a..2184cdcb 100644 --- a/src/main/java/dev/openfeature/sdk/Client.java +++ b/src/main/java/dev/openfeature/sdk/Client.java @@ -6,7 +6,7 @@ * Interface used to resolve flags of varying types. */ public interface Client extends Features, EventBus { - Metadata getMetadata(); + ClientMetadata getMetadata(); /** * Return an optional client-level evaluation context. diff --git a/src/main/java/dev/openfeature/sdk/ClientMetadata.java b/src/main/java/dev/openfeature/sdk/ClientMetadata.java new file mode 100644 index 00000000..cd8b56be --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/ClientMetadata.java @@ -0,0 +1,8 @@ +package dev.openfeature.sdk; + +/** + * Metadata specific to an OpenFeature {@code Client}. + */ +public interface ClientMetadata { + String getDomain(); +} diff --git a/src/main/java/dev/openfeature/sdk/EventDetails.java b/src/main/java/dev/openfeature/sdk/EventDetails.java index 4563bcbd..f2113eea 100644 --- a/src/main/java/dev/openfeature/sdk/EventDetails.java +++ b/src/main/java/dev/openfeature/sdk/EventDetails.java @@ -9,8 +9,6 @@ @Data @SuperBuilder(toBuilder = true) public class EventDetails extends ProviderEventDetails { - @Deprecated - private String clientName; private String domain; private String providerName; @@ -23,7 +21,6 @@ static EventDetails fromProviderEventDetails( String providerName, String domain) { return builder() - .clientName(domain) .domain(domain) .providerName(providerName) .flagsChanged(providerEventDetails.getFlagsChanged()) diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index 26b14f79..5c9091b8 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -16,7 +16,7 @@ public class HookContext { @NonNull FlagValueType type; @NonNull T defaultValue; @NonNull EvaluationContext ctx; - Metadata clientMetadata; + ClientMetadata clientMetadata; Metadata providerMetadata; /** @@ -30,7 +30,7 @@ public class HookContext { * @param type that the flag is evaluating against * @return resulting context for hook */ - public static HookContext from(String key, FlagValueType type, Metadata clientMetadata, + public static HookContext from(String key, FlagValueType type, ClientMetadata clientMetadata, Metadata providerMetadata, EvaluationContext ctx, T defaultValue) { return HookContext.builder() .flagKey(key) diff --git a/src/main/java/dev/openfeature/sdk/Metadata.java b/src/main/java/dev/openfeature/sdk/Metadata.java index 77db8179..7e614c27 100644 --- a/src/main/java/dev/openfeature/sdk/Metadata.java +++ b/src/main/java/dev/openfeature/sdk/Metadata.java @@ -5,8 +5,4 @@ */ public interface Metadata { String getName(); - - default String getDomain() { - return ""; - } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index a7fba591..bf7f24c5 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -1,5 +1,10 @@ package dev.openfeature.sdk; +import dev.openfeature.sdk.exceptions.OpenFeatureError; +import dev.openfeature.sdk.internal.AutoCloseableLock; +import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; +import lombok.extern.slf4j.Slf4j; + import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -7,11 +12,6 @@ import java.util.Set; import java.util.function.Consumer; -import dev.openfeature.sdk.exceptions.OpenFeatureError; -import dev.openfeature.sdk.internal.AutoCloseableLock; -import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; -import lombok.extern.slf4j.Slf4j; - /** * A global singleton which holds base configuration for the OpenFeature * library. diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index d4dc46ac..6b1d2c0d 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -25,8 +25,6 @@ public class OpenFeatureClient implements Client { private final OpenFeatureAPI openfeatureApi; @Getter - private final String name; - @Getter private final String domain; @Getter private final String version; @@ -49,7 +47,6 @@ public class OpenFeatureClient implements Client { @Deprecated() // TODO: eventually we will make this non-public. See issue #872 public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String domain, String version) { this.openfeatureApi = openFeatureAPI; - this.name = domain; this.domain = domain; this.version = version; this.clientHooks = new ArrayList<>(); @@ -357,18 +354,8 @@ public FlagEvaluationDetails getObjectDetails(String key, Value defaultVa } @Override - public Metadata getMetadata() { - return new Metadata() { - @Override - public String getName() { - return name; - } - - @Override - public String getDomain() { - return domain; - } - }; + public ClientMetadata getMetadata() { + return () -> domain; } /** diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index 16159e25..e697d36f 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -281,13 +281,11 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { @Test void clientMetadata() { Client c = _client(); assertNull(c.getMetadata().getDomain()); - assertNull(c.getMetadata().getName()); String domainName = "test domain"; FeatureProviderTestUtils.setFeatureProvider(new AlwaysBrokenProvider()); Client c2 = api.getClient(domainName); assertEquals(domainName, c2.getMetadata().getDomain()); - assertEquals(domainName, c2.getMetadata().getName()); } @Specification(number="1.4.9", text="In cases of abnormal execution (network failure, unhandled error, etc) the reason field in the evaluation details SHOULD indicate an error.") diff --git a/src/test/java/dev/openfeature/sdk/HookContextTest.java b/src/test/java/dev/openfeature/sdk/HookContextTest.java index c65f6ce4..50cc6617 100644 --- a/src/test/java/dev/openfeature/sdk/HookContextTest.java +++ b/src/test/java/dev/openfeature/sdk/HookContextTest.java @@ -9,7 +9,7 @@ class HookContextTest { @Specification(number="4.2.2.2", text="Condition: The client metadata field in the hook context MUST be immutable.") @Specification(number="4.2.2.3", text="Condition: The provider metadata field in the hook context MUST be immutable.") @Test void metadata_field_is_type_metadata() { - Metadata clientMetadata = mock(Metadata.class); + ClientMetadata clientMetadata = mock(ClientMetadata.class); Metadata meta = mock(Metadata.class); HookContext hc = HookContext.from( "key", @@ -20,7 +20,7 @@ class HookContextTest { false ); - assertTrue(Metadata.class.isAssignableFrom(hc.getClientMetadata().getClass())); + assertTrue(ClientMetadata.class.isAssignableFrom(hc.getClientMetadata().getClass())); assertTrue(Metadata.class.isAssignableFrom(hc.getProviderMetadata().getClass())); } diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index 2650236c..bf6501dd 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -19,28 +19,13 @@ import dev.openfeature.sdk.fixtures.HookFixtures; class HookSupportTest implements HookFixtures { - - private Metadata clientMetadata = new Metadata() { - @Override - public String getName() { - return "client"; - } - - @Override - public String getDomain() { - return "domain"; - } - }; - - private Metadata providerMetadata = () -> "provider"; - @Test @DisplayName("should merge EvaluationContexts on before hooks correctly") void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { Map attributes = new HashMap<>(); attributes.put("baseKey", new Value("baseValue")); EvaluationContext baseContext = new ImmutableContext(attributes); - HookContext hookContext = new HookContext<>("flagKey", FlagValueType.STRING, "defaultValue", baseContext, clientMetadata, providerMetadata); + HookContext hookContext = new HookContext<>("flagKey", FlagValueType.STRING, "defaultValue", baseContext, () -> "client", () -> "provider"); Hook hook1 = mockStringHook(); Hook hook2 = mockStringHook(); when(hook1.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("bla", "blubber"))); @@ -62,7 +47,7 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { HookSupport hookSupport = new HookSupport(); EvaluationContext baseContext = new ImmutableContext(); IllegalStateException expectedException = new IllegalStateException("All fine, just a test"); - HookContext hookContext = new HookContext<>("flagKey", flagValueType, createDefaultValue(flagValueType), baseContext, clientMetadata, providerMetadata); + HookContext hookContext = new HookContext<>("flagKey", flagValueType, createDefaultValue(flagValueType), baseContext, () -> "client", () -> "provider"); hookSupport.beforeHooks(flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap()); hookSupport.afterHooks(flagValueType, hookContext, FlagEvaluationDetails.builder().build(), Collections.singletonList(genericHook), Collections.emptyMap()); From 12f7f4dad04c5c95e6cab22d910eedf947b089d9 Mon Sep 17 00:00:00 2001 From: Kavindu Dodanduwa Date: Fri, 31 May 2024 10:30:56 -0700 Subject: [PATCH 5/5] add deprecated default getName to be backward compatible as much as possible Signed-off-by: Kavindu Dodanduwa --- src/main/java/dev/openfeature/sdk/ClientMetadata.java | 6 ++++++ .../java/dev/openfeature/sdk/FlagEvaluationSpecTest.java | 3 +++ 2 files changed, 9 insertions(+) diff --git a/src/main/java/dev/openfeature/sdk/ClientMetadata.java b/src/main/java/dev/openfeature/sdk/ClientMetadata.java index cd8b56be..fa0ed402 100644 --- a/src/main/java/dev/openfeature/sdk/ClientMetadata.java +++ b/src/main/java/dev/openfeature/sdk/ClientMetadata.java @@ -5,4 +5,10 @@ */ public interface ClientMetadata { String getDomain(); + + @Deprecated + // this is here for compatibility with getName() exposed from {@link Metadata} + default String getName() { + return getDomain(); + } } diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index e697d36f..d85d8825 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -280,11 +280,14 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { @Specification(number="1.2.2", text="The client interface MUST define a metadata member or accessor, containing an immutable domain field or accessor of type string, which corresponds to the domain value supplied during client creation. In previous drafts, this property was called name. For backwards compatibility, implementations should consider name an alias to domain.") @Test void clientMetadata() { Client c = _client(); + assertNull(c.getMetadata().getName()); assertNull(c.getMetadata().getDomain()); String domainName = "test domain"; FeatureProviderTestUtils.setFeatureProvider(new AlwaysBrokenProvider()); Client c2 = api.getClient(domainName); + + assertEquals(domainName, c2.getMetadata().getName()); assertEquals(domainName, c2.getMetadata().getDomain()); }