Skip to content

fix: setProviderAndWait must throw #794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 8, 2024
Merged

fix: setProviderAndWait must throw #794

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f453ba
fix: setProviderAndWait must throw
toddbaert Feb 7, 2024
78aec5b
Merge branch 'main' into set-and-wait-throw
toddbaert Feb 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import javax.annotation.Nullable;

import dev.openfeature.sdk.exceptions.OpenFeatureError;
import dev.openfeature.sdk.internal.AutoCloseableLock;
import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -131,14 +132,14 @@ public void setProvider(String clientName, FeatureProvider provider) {
/**
* Set the default provider and wait for initialization to finish.
*/
public void setProviderAndWait(FeatureProvider provider) {
public void setProviderAndWait(FeatureProvider provider) throws OpenFeatureError {
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
providerRepository.setProvider(
provider,
this::attachEventProvider,
this::emitReady,
this::detachEventProvider,
this::emitError,
this::emitErrorAndThrow,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the main change - I pass a new delegate which not only emits the events, but throws.

true);
}
}
Expand All @@ -149,14 +150,14 @@ public void setProviderAndWait(FeatureProvider provider) {
* @param clientName The name of the client.
* @param provider The provider to set.
*/
public void setProviderAndWait(String clientName, FeatureProvider provider) {
public void setProviderAndWait(String clientName, FeatureProvider provider) throws OpenFeatureError {
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
providerRepository.setProvider(clientName,
provider,
this::attachEventProvider,
this::emitReady,
this::detachEventProvider,
this::emitError,
this::emitErrorAndThrow,
true);
}
}
Expand All @@ -179,9 +180,14 @@ private void detachEventProvider(FeatureProvider provider) {
}
}

private void emitError(FeatureProvider provider, String message) {
private void emitError(FeatureProvider provider, OpenFeatureError exception) {
runHandlersForProvider(provider, ProviderEvent.PROVIDER_ERROR,
ProviderEventDetails.builder().message(message).build());
ProviderEventDetails.builder().message(exception.getMessage()).build());
}

private void emitErrorAndThrow(FeatureProvider provider, OpenFeatureError exception) throws OpenFeatureError {
Copy link
Member Author

@toddbaert toddbaert Feb 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

New delegate which emits errors but also throws.

this.emitError(provider, exception);
throw exception;
}

/**
Expand Down
25 changes: 15 additions & 10 deletions src/main/java/dev/openfeature/sdk/ProviderRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

import javax.annotation.Nullable;

import dev.openfeature.sdk.exceptions.GeneralError;
import dev.openfeature.sdk.exceptions.OpenFeatureError;
import lombok.extern.slf4j.Slf4j;

@Slf4j
Expand Down Expand Up @@ -66,7 +68,7 @@ public void setProvider(FeatureProvider provider,
Consumer<FeatureProvider> afterSet,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError,
BiConsumer<FeatureProvider, OpenFeatureError> afterError,
boolean waitForInit) {
if (provider == null) {
throw new IllegalArgumentException("Provider cannot be null");
Expand All @@ -83,12 +85,12 @@ public void setProvider(FeatureProvider provider,
* Otherwise, initialization happens in the background.
*/
public void setProvider(String clientName,
FeatureProvider provider,
Consumer<FeatureProvider> afterSet,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError,
boolean waitForInit) {
FeatureProvider provider,
Consumer<FeatureProvider> afterSet,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, OpenFeatureError> afterError,
boolean waitForInit) {
if (provider == null) {
throw new IllegalArgumentException("Provider cannot be null");
}
Expand All @@ -103,7 +105,7 @@ private void prepareAndInitializeProvider(@Nullable String clientName,
Consumer<FeatureProvider> afterSet,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError,
BiConsumer<FeatureProvider, OpenFeatureError> afterError,
boolean waitForInit) {

if (!isProviderRegistered(newProvider)) {
Expand All @@ -129,17 +131,20 @@ private void prepareAndInitializeProvider(@Nullable String clientName,
private void initializeProvider(FeatureProvider newProvider,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError,
BiConsumer<FeatureProvider, OpenFeatureError> afterError,
FeatureProvider oldProvider) {
try {
if (ProviderState.NOT_READY.equals(newProvider.getState())) {
newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext());
afterInit.accept(newProvider);
}
shutDownOld(oldProvider, afterShutdown);
} catch (OpenFeatureError e) {
log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e);
afterError.accept(newProvider, e);
} catch (Exception e) {
log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e);
afterError.accept(newProvider, e.getMessage());
afterError.accept(newProvider, new GeneralError(e));
}
}

Expand Down
19 changes: 16 additions & 3 deletions src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
Expand All @@ -18,9 +19,6 @@
import java.util.List;
import java.util.Map;

import dev.openfeature.sdk.providers.memory.InMemoryProvider;
import dev.openfeature.sdk.testutils.TestEventsProvider;
import lombok.SneakyThrows;
import org.awaitility.Awaitility;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -31,8 +29,12 @@
import org.slf4j.Logger;

import dev.openfeature.sdk.exceptions.FlagNotFoundError;
import dev.openfeature.sdk.exceptions.GeneralError;
import dev.openfeature.sdk.fixtures.HookFixtures;
import dev.openfeature.sdk.providers.memory.InMemoryProvider;
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
import dev.openfeature.sdk.testutils.TestEventsProvider;
import lombok.SneakyThrows;

class FlagEvaluationSpecTest implements HookFixtures {

Expand Down Expand Up @@ -87,6 +89,17 @@ void getApiInstance() {
assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.READY);
}

@SneakyThrows
@Specification(number="1.1.8", text="The API SHOULD provide functions to set a provider and wait for the initialize function to return or throw.")
@Test void providerAndWaitError() {
FeatureProvider provider1 = new TestEventsProvider(500, true, "fake error");
assertThrows(GeneralError.class, () -> api.setProviderAndWait(provider1));

FeatureProvider provider2 = new TestEventsProvider(500, true, "fake error");
String providerName = "providerAndWaitError";
assertThrows(GeneralError.class, () -> api.setProviderAndWait(providerName, provider2));
}

@Specification(number="2.4.5", text="The provider SHOULD indicate an error if flag resolution is attempted before the provider is ready.")
@Test void shouldReturnNotReadyIfNotInitialized() {
FeatureProvider provider = new InMemoryProvider(new HashMap<>()) {
Expand Down
17 changes: 9 additions & 8 deletions src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
package dev.openfeature.sdk;

import dev.openfeature.sdk.providers.memory.InMemoryProvider;
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.util.Collections;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.Collections;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import dev.openfeature.sdk.providers.memory.InMemoryProvider;
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;

class OpenFeatureAPITest {

private static final String CLIENT_NAME = "client name";
Expand Down Expand Up @@ -45,7 +46,7 @@ void namedProviderOverwrittenTest() {
}

@Test
void providerToMultipleNames() {
void providerToMultipleNames() throws Exception {
FeatureProvider inMemAsEventingProvider = new InMemoryProvider(Collections.EMPTY_MAP);
FeatureProvider noOpAsNonEventingProvider = new NoOpProvider();

Expand Down
12 changes: 6 additions & 6 deletions src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import static org.awaitility.Awaitility.await;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.atMostOnce;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
Expand All @@ -29,6 +28,7 @@
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

import dev.openfeature.sdk.exceptions.OpenFeatureError;
import dev.openfeature.sdk.testutils.exception.TestException;

class ProviderRepositoryTest {
Expand Down Expand Up @@ -253,7 +253,7 @@ void shouldRunLambdasOnSuccessful() {
Consumer<FeatureProvider> afterSet = mock(Consumer.class);
Consumer<FeatureProvider> afterInit = mock(Consumer.class);
Consumer<FeatureProvider> afterShutdown = mock(Consumer.class);
BiConsumer<FeatureProvider, String> afterError = mock(BiConsumer.class);
BiConsumer<FeatureProvider, OpenFeatureError> afterError = mock(BiConsumer.class);

FeatureProvider oldProvider = providerRepository.getProvider();
FeatureProvider featureProvider1 = createMockedProvider();
Expand All @@ -274,7 +274,7 @@ void shouldRunLambdasOnError() throws Exception {
Consumer<FeatureProvider> afterSet = mock(Consumer.class);
Consumer<FeatureProvider> afterInit = mock(Consumer.class);
Consumer<FeatureProvider> afterShutdown = mock(Consumer.class);
BiConsumer<FeatureProvider, String> afterError = mock(BiConsumer.class);
BiConsumer<FeatureProvider, OpenFeatureError> afterError = mock(BiConsumer.class);

FeatureProvider errorFeatureProvider = createMockedErrorProvider();

Expand Down Expand Up @@ -310,7 +310,7 @@ private void setFeatureProvider(FeatureProvider provider) {

private void setFeatureProvider(FeatureProvider provider, Consumer<FeatureProvider> afterSet,
Consumer<FeatureProvider> afterInit, Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError) {
BiConsumer<FeatureProvider, OpenFeatureError> afterError) {
providerRepository.setProvider(provider, afterSet, afterInit, afterShutdown,
afterError, false);
waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider);
Expand Down Expand Up @@ -348,8 +348,8 @@ private Consumer<FeatureProvider> mockAfterShutdown() {
};
}

private BiConsumer<FeatureProvider, String> mockAfterError() {
return (fp, message) -> {
private BiConsumer<FeatureProvider, OpenFeatureError> mockAfterError() {
return (fp, ex) -> {
};
}

Expand Down
3 changes: 2 additions & 1 deletion src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import dev.openfeature.sdk.ProviderEventDetails;
import dev.openfeature.sdk.ProviderState;
import dev.openfeature.sdk.Value;
import dev.openfeature.sdk.exceptions.GeneralError;

public class TestEventsProvider extends EventProvider {

Expand Down Expand Up @@ -63,7 +64,7 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
Thread.sleep(initTimeoutMs);
if (this.initError) {
this.state = ProviderState.ERROR;
throw new Exception(initErrorMessage);
throw new GeneralError(initErrorMessage);
}
this.state = ProviderState.READY;
}
Expand Down