fix: setProviderAndWait must throw (#794)

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
This commit is contained in:
Todd Baert 2024-02-08 15:19:20 -05:00 committed by GitHub
parent d5a0620f59
commit da47b7f9c0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 60 additions and 34 deletions

View File

@ -9,6 +9,7 @@ import java.util.function.Consumer;
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;
@ -131,14 +132,14 @@ public class OpenFeatureAPI implements EventBus<OpenFeatureAPI> {
/**
* 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,
true);
}
}
@ -149,14 +150,14 @@ public class OpenFeatureAPI implements EventBus<OpenFeatureAPI> {
* @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);
}
}
@ -179,9 +180,14 @@ public class OpenFeatureAPI implements EventBus<OpenFeatureAPI> {
}
}
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 {
this.emitError(provider, exception);
throw exception;
}
/**

View File

@ -15,6 +15,8 @@ import java.util.stream.Stream;
import javax.annotation.Nullable;
import dev.openfeature.sdk.exceptions.GeneralError;
import dev.openfeature.sdk.exceptions.OpenFeatureError;
import lombok.extern.slf4j.Slf4j;
@Slf4j
@ -66,7 +68,7 @@ class ProviderRepository {
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");
@ -83,12 +85,12 @@ class ProviderRepository {
* 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");
}
@ -103,7 +105,7 @@ class ProviderRepository {
Consumer<FeatureProvider> afterSet,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError,
BiConsumer<FeatureProvider, OpenFeatureError> afterError,
boolean waitForInit) {
if (!isProviderRegistered(newProvider)) {
@ -129,7 +131,7 @@ class ProviderRepository {
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())) {
@ -137,9 +139,12 @@ class ProviderRepository {
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));
}
}

View File

@ -8,6 +8,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
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;
@ -18,9 +19,6 @@ import java.util.HashMap;
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;
@ -31,8 +29,12 @@ import org.simplify4u.slf4jmock.LoggerMock;
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 {
@ -87,6 +89,17 @@ class FlagEvaluationSpecTest implements HookFixtures {
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<>()) {

View File

@ -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";
@ -45,7 +46,7 @@ class OpenFeatureAPITest {
}
@Test
void providerToMultipleNames() {
void providerToMultipleNames() throws Exception {
FeatureProvider inMemAsEventingProvider = new InMemoryProvider(Collections.EMPTY_MAP);
FeatureProvider noOpAsNonEventingProvider = new NoOpProvider();

View File

@ -9,7 +9,6 @@ import static org.assertj.core.api.Assertions.assertThatCode;
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;
@ -29,6 +28,7 @@ import org.junit.jupiter.api.DisplayName;
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 {
@ -253,7 +253,7 @@ class ProviderRepositoryTest {
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();
@ -274,7 +274,7 @@ class ProviderRepositoryTest {
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();
@ -310,7 +310,7 @@ class ProviderRepositoryTest {
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);
@ -348,8 +348,8 @@ class ProviderRepositoryTest {
};
}
private BiConsumer<FeatureProvider, String> mockAfterError() {
return (fp, message) -> {
private BiConsumer<FeatureProvider, OpenFeatureError> mockAfterError() {
return (fp, ex) -> {
};
}

View File

@ -8,6 +8,7 @@ import dev.openfeature.sdk.ProviderEvent;
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 {
@ -63,7 +64,7 @@ public class TestEventsProvider extends EventProvider {
Thread.sleep(initTimeoutMs);
if (this.initError) {
this.state = ProviderState.ERROR;
throw new Exception(initErrorMessage);
throw new GeneralError(initErrorMessage);
}
this.state = ProviderState.READY;
}