Merge remote-tracking branch 'origin/make-provider-stateless' into make-provider-stateless

chrfwow · chrfwow · commit e4ea124b388a · 2024-09-19T09:13:57.000+02:00
diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml
@@ -22,7 +22,7 @@ jobs:
     steps:
     - uses: actions/checkout@6d193bf28034eafb982f37bd894289fe649468fc
     - name: Set up JDK 8
-      uses: actions/setup-java@2dfa2011c5b2a0f1489bf9e433881c92c1631f88
+      uses: actions/setup-java@bcfbca5b713b77435aded8de683c7ee5e79f63cb
       with:
         java-version: '8'
         distribution: 'temurin'
diff --git a/.github/workflows/pullrequest.yml b/.github/workflows/pullrequest.yml
@@ -13,14 +13,14 @@ jobs:
         uses: actions/checkout@6d193bf28034eafb982f37bd894289fe649468fc
 
       - name: Set up JDK 8
-        uses: actions/setup-java@2dfa2011c5b2a0f1489bf9e433881c92c1631f88
+        uses: actions/setup-java@bcfbca5b713b77435aded8de683c7ee5e79f63cb
         with:
           java-version: '8'
           distribution: 'temurin'
           cache: maven
 
       - name: Initialize CodeQL
-        uses: github/codeql-action/init@8fd294e26a0e458834582b0fe4988d79966c7c0a
+        uses: github/codeql-action/init@64431c66d0e98a26d34f9485e5f8e317e14956fc
         with:
           languages: java
 
@@ -45,4 +45,4 @@ jobs:
           verbose: true # optional (default = false)
 
       - name: Perform CodeQL Analysis
-        uses: github/codeql-action/analyze@8fd294e26a0e458834582b0fe4988d79966c7c0a
+        uses: github/codeql-action/analyze@64431c66d0e98a26d34f9485e5f8e317e14956fc
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -31,7 +31,7 @@ jobs:
         uses: actions/checkout@6d193bf28034eafb982f37bd894289fe649468fc
       - name: Set up JDK 8
         if: ${{ steps.release.outputs.release_created }}
-        uses: actions/setup-java@2dfa2011c5b2a0f1489bf9e433881c92c1631f88
+        uses: actions/setup-java@bcfbca5b713b77435aded8de683c7ee5e79f63cb
         with:
           java-version: '8'
           distribution: 'temurin'
diff --git a/.github/workflows/static-code-scanning.yaml b/.github/workflows/static-code-scanning.yaml
@@ -33,12 +33,12 @@ jobs:
 
       # Initializes the CodeQL tools for scanning.
       - name: Initialize CodeQL
-        uses: github/codeql-action/init@8fd294e26a0e458834582b0fe4988d79966c7c0a
+        uses: github/codeql-action/init@64431c66d0e98a26d34f9485e5f8e317e14956fc
         with:
           languages: java
 
       - name: Autobuild
-        uses: github/codeql-action/autobuild@8fd294e26a0e458834582b0fe4988d79966c7c0a
+        uses: github/codeql-action/autobuild@64431c66d0e98a26d34f9485e5f8e317e14956fc
 
       - name: Perform CodeQL Analysis
-        uses: github/codeql-action/analyze@8fd294e26a0e458834582b0fe4988d79966c7c0a
+        uses: github/codeql-action/analyze@64431c66d0e98a26d34f9485e5f8e317e14956fc
diff --git a/pom.xml b/pom.xml
@@ -490,7 +490,7 @@
           <plugin>
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-gpg-plugin</artifactId>
-            <version>3.2.5</version>
+            <version>3.2.6</version>
             <executions>
               <execution>
                 <id>sign-artifacts</id>
diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java
@@ -53,7 +53,7 @@ void detach() {
      * @param event   The event type
      * @param details The details of the event
      */
-    protected void emit(ProviderEvent event, ProviderEventDetails details) {
+    public void emit(ProviderEvent event, ProviderEventDetails details) {
         if (eventProviderListener != null) {
             eventProviderListener.onEmit(event, details);
         }
@@ -68,7 +68,7 @@ protected void emit(ProviderEvent event, ProviderEventDetails details) {
      *
      * @param details The details of the event
      */
-    protected void emitProviderReady(ProviderEventDetails details) {
+    public void emitProviderReady(ProviderEventDetails details) {
         emit(ProviderEvent.PROVIDER_READY, details);
     }
 
@@ -79,7 +79,7 @@ protected void emitProviderReady(ProviderEventDetails details) {
      *
      * @param details The details of the event
      */
-    protected void emitProviderConfigurationChanged(ProviderEventDetails details) {
+    public void emitProviderConfigurationChanged(ProviderEventDetails details) {
         emit(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details);
     }
 
@@ -89,7 +89,7 @@ protected void emitProviderConfigurationChanged(ProviderEventDetails details) {
      *
      * @param details The details of the event
      */
-    protected void emitProviderStale(ProviderEventDetails details) {
+    public void emitProviderStale(ProviderEventDetails details) {
         emit(ProviderEvent.PROVIDER_STALE, details);
     }
 
@@ -99,18 +99,7 @@ protected void emitProviderStale(ProviderEventDetails details) {
      *
      * @param details The details of the event
      */
-    protected void emitProviderError(ProviderEventDetails details) {
+    public void emitProviderError(ProviderEventDetails details) {
         emit(ProviderEvent.PROVIDER_ERROR, details);
     }
-
-    @Override
-    public boolean equals(Object obj) {
-        if (obj == null) {
-            return false;
-        }
-        if (obj instanceof FeatureProviderStateManager) {
-            return this == ((FeatureProviderStateManager) obj).getProvider();
-        }
-        return this == obj;
-    }
 }
diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java
@@ -146,7 +146,11 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key
 
             details = FlagEvaluationDetails.from(providerEval, key);
             if (details.getErrorCode() != null) {
-                throw ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage());
+                OpenFeatureError error = ExceptionUtils.instantiateErrorByErrorCode(
+                        details.getErrorCode(),
+                        details.getErrorMessage());
+                enrichDetailsWithErrorDefaults(defaultValue, details);
+                hookSupport.errorHooks(type, afterHookContext, error, mergedHooks, hints);
             } else {
                 hookSupport.afterHooks(type, afterHookContext, details, mergedHooks, hints);
             }
@@ -160,8 +164,7 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key
                 details.setErrorCode(ErrorCode.GENERAL);
             }
             details.setErrorMessage(e.getMessage());
-            details.setValue(defaultValue);
-            details.setReason(Reason.ERROR.toString());
+            enrichDetailsWithErrorDefaults(defaultValue, details);
             hookSupport.errorHooks(type, afterHookContext, e, mergedHooks, hints);
         } finally {
             hookSupport.afterAllHooks(type, afterHookContext, mergedHooks, hints);
@@ -170,6 +173,11 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key
         return details;
     }
 
+    private static <T> void enrichDetailsWithErrorDefaults(T defaultValue, FlagEvaluationDetails<T> details) {
+        details.setValue(defaultValue);
+        details.setReason(Reason.ERROR.toString());
+    }
+
     /**
      * Merge invocation contexts with API, transaction and client contexts.
      * Does not merge before context.
diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java
@@ -184,7 +184,9 @@ private void prepareAndInitializeProvider(String domain,
 
     private FeatureProviderStateManager getExistingStateManagerForProvider(FeatureProvider provider) {
         for (FeatureProviderStateManager stateManager : stateManagers.values()) {
-            if (stateManager.hasSameProvider(provider)) return stateManager;
+            if (stateManager.hasSameProvider(provider)) {
+                return stateManager;
+            }
         }
         FeatureProviderStateManager defaultFeatureProviderStateManager = defaultStateManger.get();
         if (defaultFeatureProviderStateManager.hasSameProvider(provider)) {
diff --git a/src/main/java/dev/openfeature/sdk/exceptions/FlagNotFoundError.java b/src/main/java/dev/openfeature/sdk/exceptions/FlagNotFoundError.java
@@ -4,14 +4,11 @@
 import lombok.Getter;
 import lombok.experimental.StandardException;
 
-@SuppressWarnings("checkstyle:MissingJavadocType")
+@SuppressWarnings({"checkstyle:MissingJavadocType", "squid:S110"})
 @StandardException
-public class FlagNotFoundError extends OpenFeatureError {
+public class FlagNotFoundError extends OpenFeatureErrorWithoutStacktrace {
     private static final long serialVersionUID = 1L;
-    @Getter private final ErrorCode errorCode = ErrorCode.FLAG_NOT_FOUND;
+    @Getter
+    private final ErrorCode errorCode = ErrorCode.FLAG_NOT_FOUND;
 
-    @Override
-    public synchronized Throwable fillInStackTrace() {
-        return this;
-    }
 }
diff --git a/src/main/java/dev/openfeature/sdk/exceptions/OpenFeatureErrorWithoutStacktrace.java b/src/main/java/dev/openfeature/sdk/exceptions/OpenFeatureErrorWithoutStacktrace.java
@@ -0,0 +1,14 @@
+package dev.openfeature.sdk.exceptions;
+
+import lombok.experimental.StandardException;
+
+@SuppressWarnings("checkstyle:MissingJavadocType")
+@StandardException
+public abstract class OpenFeatureErrorWithoutStacktrace extends OpenFeatureError {
+    private static final long serialVersionUID = 1L;
+
+    @Override
+    public synchronized Throwable fillInStackTrace() {
+        return this;
+    }
+}
diff --git a/src/main/java/dev/openfeature/sdk/exceptions/ProviderNotReadyError.java b/src/main/java/dev/openfeature/sdk/exceptions/ProviderNotReadyError.java
@@ -4,9 +4,9 @@
 import lombok.Getter;
 import lombok.experimental.StandardException;
 
-@SuppressWarnings("checkstyle:MissingJavadocType")
+@SuppressWarnings({"checkstyle:MissingJavadocType", "squid:S110"})
 @StandardException
-public class ProviderNotReadyError extends OpenFeatureError {
+public class ProviderNotReadyError extends OpenFeatureErrorWithoutStacktrace {
     private static final long serialVersionUID = 1L;
     @Getter private final ErrorCode errorCode = ErrorCode.PROVIDER_NOT_READY;
 }
diff --git a/src/main/java/dev/openfeature/sdk/exceptions/TypeMismatchError.java b/src/main/java/dev/openfeature/sdk/exceptions/TypeMismatchError.java
@@ -7,6 +7,7 @@
 /**
  * The type of the flag value does not match the expected type.
  */
+@SuppressWarnings({"checkstyle:MissingJavadocType", "squid:S110"})
 @StandardException
 public class TypeMismatchError extends OpenFeatureError {
     private static final long serialVersionUID = 1L;
diff --git a/src/test/java/dev/openfeature/sdk/AlwaysBrokenWithDetailsProvider.java b/src/test/java/dev/openfeature/sdk/AlwaysBrokenWithDetailsProvider.java
@@ -0,0 +1,53 @@
+package dev.openfeature.sdk;
+
+import dev.openfeature.sdk.exceptions.FlagNotFoundError;
+
+public class AlwaysBrokenWithDetailsProvider implements FeatureProvider {
+
+    @Override
+    public Metadata getMetadata() {
+        return () -> {
+            throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE);
+        };
+    }
+
+    @Override
+    public ProviderEvaluation<Boolean> getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) {
+        return ProviderEvaluation.<Boolean>builder()
+                .errorMessage(TestConstants.BROKEN_MESSAGE)
+                .errorCode(ErrorCode.FLAG_NOT_FOUND)
+                .build();
+    }
+
+    @Override
+    public ProviderEvaluation<String> getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) {
+        return ProviderEvaluation.<String>builder()
+                .errorMessage(TestConstants.BROKEN_MESSAGE)
+                .errorCode(ErrorCode.FLAG_NOT_FOUND)
+                .build();
+    }
+
+    @Override
+    public ProviderEvaluation<Integer> getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) {
+        return ProviderEvaluation.<Integer>builder()
+                .errorMessage(TestConstants.BROKEN_MESSAGE)
+                .errorCode(ErrorCode.FLAG_NOT_FOUND)
+                .build();
+    }
+
+    @Override
+    public ProviderEvaluation<Double> getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) {
+        return ProviderEvaluation.<Double>builder()
+                .errorMessage(TestConstants.BROKEN_MESSAGE)
+                .errorCode(ErrorCode.FLAG_NOT_FOUND)
+                .build();
+    }
+
+    @Override
+    public ProviderEvaluation<Value> getObjectEvaluation(String key, Value defaultValue, EvaluationContext invocationContext) {
+        return ProviderEvaluation.<Value>builder()
+                .errorMessage(TestConstants.BROKEN_MESSAGE)
+                .errorCode(ErrorCode.FLAG_NOT_FOUND)
+                .build();
+    }
+}
diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java
@@ -618,7 +618,7 @@ void removedEventsShouldNotRun() {
 
             // emit event
             provider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, ProviderEventDetails.builder().build());
-
+            
             // both global and client handlers should not run.
             verify(handler1, after(TIMEOUT).never()).accept(any());
             verify(handler2, never()).accept(any());
diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java
@@ -104,7 +104,9 @@ void getApiInstance() {
         OpenFeatureAPI.getInstance().setProvider(providerName, provider);
         assertThat(api.getProviderState(providerName)).isEqualTo(ProviderState.NOT_READY);
         Client client = OpenFeatureAPI.getInstance().getClient(providerName);
-        assertEquals(ErrorCode.PROVIDER_NOT_READY, client.getBooleanDetails("return_error_when_not_initialized", false).getErrorCode());
+        FlagEvaluationDetails<Boolean> details = client.getBooleanDetails("return_error_when_not_initialized", false);
+        assertEquals(ErrorCode.PROVIDER_NOT_READY, details.getErrorCode());
+        assertEquals(Reason.ERROR.toString(), details.getReason());
     }
 
     @Specification(number="1.1.5", text="The API MUST provide a function for retrieving the metadata field of the configured provider.")
@@ -251,10 +253,29 @@ void getApiInstance() {
     @Test void broken_provider() {
         FeatureProviderTestUtils.setFeatureProvider(new AlwaysBrokenProvider());
         Client c = api.getClient();
-        assertFalse(c.getBooleanValue("key", false));
-        FlagEvaluationDetails<Boolean> details = c.getBooleanDetails("key", false);
+        boolean defaultValue = false;
+        assertFalse(c.getBooleanValue("key", defaultValue));
+        FlagEvaluationDetails<Boolean> details = c.getBooleanDetails("key", defaultValue);
         assertEquals(ErrorCode.FLAG_NOT_FOUND, details.getErrorCode());
         assertEquals(TestConstants.BROKEN_MESSAGE, details.getErrorMessage());
+        assertEquals(Reason.ERROR.toString(), details.getReason());
+        assertEquals(defaultValue, details.getValue());
+    }
+
+    @Specification(number="1.4.8", text="In cases of abnormal execution, the `evaluation details` structure's `error code` field **MUST** contain an `error code`.")
+    @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.")
+    @Specification(number="1.4.10", text="Methods, functions, or operations on the client MUST NOT throw exceptions, or otherwise abnormally terminate. Flag evaluation calls must always return the `default value` in the event of abnormal execution. Exceptions include functions or methods for the purposes for configuration or setup.")
+    @Specification(number="1.4.13", text="In cases of abnormal execution, the `evaluation details` structure's `error message` field **MAY** contain a string containing additional details about the nature of the error.")
+    @Test void broken_provider_withDetails() {
+        FeatureProviderTestUtils.setFeatureProvider(new AlwaysBrokenWithDetailsProvider());
+        Client c = api.getClient();
+        boolean defaultValue = false;
+        assertFalse(c.getBooleanValue("key", defaultValue));
+        FlagEvaluationDetails<Boolean> details = c.getBooleanDetails("key", defaultValue);
+        assertEquals(ErrorCode.FLAG_NOT_FOUND, details.getErrorCode());
+        assertEquals(TestConstants.BROKEN_MESSAGE, details.getErrorMessage());
+        assertEquals(Reason.ERROR.toString(), details.getReason());
+        assertEquals(defaultValue, details.getValue());
     }
 
     @Specification(number="1.4.11", text="Methods, functions, or operations on the client SHOULD NOT write log messages.")
diff --git a/src/test/java/dev/openfeature/sdk/HookSpecTest.java b/src/test/java/dev/openfeature/sdk/HookSpecTest.java
@@ -149,7 +149,7 @@ void optional_properties() {
     @Test
     void before_runs_ahead_of_evaluation() {
         OpenFeatureAPI api = OpenFeatureAPI.getInstance();
-        api.setProvider(new AlwaysBrokenProvider());
+        api.setProviderAndWait(new AlwaysBrokenProvider());
         Client client = api.getClient();
         Hook<Boolean> evalHook = mockBooleanHook();
 

Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ void detach() {`
`53`	`53`	`* @param event The event type`
`54`	`54`	`* @param details The details of the event`
`55`	`55`	`*/`
`56`		`- protected void emit(ProviderEvent event, ProviderEventDetails details) {`
	`56`	`+ public void emit(ProviderEvent event, ProviderEventDetails details) {`
`57`	`57`	`if (eventProviderListener != null) {`
`58`	`58`	`eventProviderListener.onEmit(event, details);`
`59`	`59`	`}`
`@@ -68,7 +68,7 @@ protected void emit(ProviderEvent event, ProviderEventDetails details) {`
`68`	`68`	`*`
`69`	`69`	`* @param details The details of the event`
`70`	`70`	`*/`
`71`		`- protected void emitProviderReady(ProviderEventDetails details) {`
	`71`	`+ public void emitProviderReady(ProviderEventDetails details) {`
`72`	`72`	`emit(ProviderEvent.PROVIDER_READY, details);`
`73`	`73`	`}`
`74`	`74`
`@@ -79,7 +79,7 @@ protected void emitProviderReady(ProviderEventDetails details) {`
`79`	`79`	`*`
`80`	`80`	`* @param details The details of the event`
`81`	`81`	`*/`
`82`		`- protected void emitProviderConfigurationChanged(ProviderEventDetails details) {`
	`82`	`+ public void emitProviderConfigurationChanged(ProviderEventDetails details) {`
`83`	`83`	`emit(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details);`
`84`	`84`	`}`
`85`	`85`
`@@ -89,7 +89,7 @@ protected void emitProviderConfigurationChanged(ProviderEventDetails details) {`
`89`	`89`	`*`
`90`	`90`	`* @param details The details of the event`
`91`	`91`	`*/`
`92`		`- protected void emitProviderStale(ProviderEventDetails details) {`
	`92`	`+ public void emitProviderStale(ProviderEventDetails details) {`
`93`	`93`	`emit(ProviderEvent.PROVIDER_STALE, details);`
`94`	`94`	`}`
`95`	`95`
`@@ -99,18 +99,7 @@ protected void emitProviderStale(ProviderEventDetails details) {`
`99`	`99`	`*`
`100`	`100`	`* @param details The details of the event`
`101`	`101`	`*/`
`102`		`- protected void emitProviderError(ProviderEventDetails details) {`
	`102`	`+ public void emitProviderError(ProviderEventDetails details) {`
`103`	`103`	`emit(ProviderEvent.PROVIDER_ERROR, details);`
`104`	`104`	`}`
`105`		`-`
`106`		`- @Override`
`107`		`- public boolean equals(Object obj) {`
`108`		`- if (obj == null) {`
`109`		`- return false;`
`110`		`- }`
`111`		`- if (obj instanceof FeatureProviderStateManager) {`
`112`		`- return this == ((FeatureProviderStateManager) obj).getProvider();`
`113`		`- }`
`114`		`- return this == obj;`
`115`		`- }`
`116`	`105`	`}`