Merge pull request #80 from Flagsmith/release/5.0.4

Release 5.0.4

Author: matthewelwell

pom.xml

@@ -6,7 +6,7 @@
 
   <groupId>com.flagsmith</groupId>
   <artifactId>flagsmith-java-client</artifactId>
-  <version>5.0.3</version>
+  <version>5.0.4</version>
   <packaging>jar</packaging>
 
   <name>Flagsmith Java Client</name>

src/main/java/com/flagsmith/FlagsmithClient.java

@@ -43,6 +43,8 @@ public class FlagsmithClient {
   private FlagsmithSdk flagsmithSdk;
   private EnvironmentModel environment;
   private PollingManager pollingManager;
+  private static final String UNABLE_TO_UPDATE_ENVIRONMENT_MESSAGE =
+          "Unable to update environment from API. Continuing to use previous copy.";
 
   private FlagsmithClient() { }
 
@@ -54,7 +56,29 @@ public static FlagsmithClient.Builder newBuilder() {
    * Load the environment flags in the environment variable from the API.
    */
   public void updateEnvironment() {
-    this.environment = flagsmithSdk.getEnvironment();
+    try {
+      EnvironmentModel updatedEnvironment = flagsmithSdk.getEnvironment();
+
+      // if we didn't get an environment from the API,
+      // then don't overwrite the copy we already have.
+      if (updatedEnvironment != null) {
+        this.environment = updatedEnvironment;
+      } else {
+        logger.error(UNABLE_TO_UPDATE_ENVIRONMENT_MESSAGE);
+      }
+    } catch (RuntimeException e) {
+      // if we already have a copy of the environment, don't throw an error,
+      // just continue using that one and log an error as it's likely just a
+      // temporary network issue.
+      if (this.environment != null) {
+        logger.error(UNABLE_TO_UPDATE_ENVIRONMENT_MESSAGE);
+      } else {
+        // if we don't already have an environment, then we should still throw
+        // the error since it's likely on client start up and there might be
+        // something more sinister going on.
+        throw e;
+      }
+    }
   }
 
   /**
@@ -248,6 +272,7 @@ public static class Builder {
     private String apiKey;
     private FlagsmithCacheConfig cacheConfig;
     private PollingManager pollingManager;
+    private FlagsmithApiWrapper flagsmithApiWrapper;
 
     private Builder() {
       client = new FlagsmithClient();
@@ -374,6 +399,17 @@ public Builder withPollingManager(PollingManager manager) {
       return this;
     }
 
+    /**
+     * Set the api wrapper.
+     *
+     * @param flagsmithApiWrapper FlagsmithAPIWrapper object
+     * @return the Builder
+     */
+    public Builder withFlagsmithApiWrapper(FlagsmithApiWrapper flagsmithApiWrapper) {
+      this.flagsmithApiWrapper = flagsmithApiWrapper;
+      return this;
+    }
+
     /**
      * Builds a FlagsmithClient.
      *
@@ -382,20 +418,22 @@ public Builder withPollingManager(PollingManager manager) {
     public FlagsmithClient build() {
       final FlagsmithApiWrapper flagsmithApiWrapper;
 
-      if (cacheConfig != null) {
+      if (this.flagsmithApiWrapper != null) {
+        flagsmithApiWrapper = this.flagsmithApiWrapper;
+      } else if (cacheConfig != null) {
         flagsmithApiWrapper = new FlagsmithApiWrapper(
-            cacheConfig.getCache(),
-            this.configuration,
-            this.customHeaders,
-            client.logger,
-            apiKey
+                cacheConfig.getCache(),
+                this.configuration,
+                this.customHeaders,
+                client.logger,
+                apiKey
         );
       } else {
         flagsmithApiWrapper = new FlagsmithApiWrapper(
-            this.configuration,
-            this.customHeaders,
-            client.logger,
-            apiKey
+                this.configuration,
+                this.customHeaders,
+                client.logger,
+                apiKey
         );
       }
 

src/test/java/com/flagsmith/FlagsmithClientTest.java

@@ -462,4 +462,57 @@ public void testGetIdentitySegmentsWithValidTrait() throws JsonProcessingExcepti
     Assert.assertEquals(segments.size(), 1);
     Assert.assertEquals(segments.get(0).getName(), "Test segment");
   }
+
+  @Test(groups = "unit")
+  public void testUpdateEnvironment_DoesNothing_WhenGetEnvironmentThrowsException() {
+    // Given
+    EnvironmentModel environmentModel = FlagsmithTestHelper.environmentModel();
+
+    FlagsmithApiWrapper mockApiWrapper = mock(FlagsmithApiWrapper.class);
+    when(mockApiWrapper.getEnvironment())
+            .thenReturn(environmentModel)
+            .thenThrow(RuntimeException.class);
+
+    FlagsmithClient client = FlagsmithClient.newBuilder()
+            .withFlagsmithApiWrapper(mockApiWrapper)
+            .withConfiguration(FlagsmithConfig.newBuilder().withLocalEvaluation(true).build())
+            .setApiKey("ser.dummy-key")
+            .build();
+
+    // When
+    // we call the update environment method twice (1st should be successful, 2nd will do nothing because of error)
+    client.updateEnvironment();
+    client.updateEnvironment();
+
+    // Then
+    // No exception is thrown and the client environment remains what was first retrieved from the ApiWrapper
+    Assert.assertEquals(client.getEnvironment(), environmentModel);
+  }
+
+  @Test(groups = "unit")
+  public void testUpdateEnvironment_DoesNothing_WhenGetEnvironmentReturnsNull() {
+    // Given
+    EnvironmentModel environmentModel = FlagsmithTestHelper.environmentModel();
+
+    FlagsmithApiWrapper mockApiWrapper = mock(FlagsmithApiWrapper.class);
+    when(mockApiWrapper.getEnvironment())
+            .thenReturn(environmentModel)
+            .thenReturn(null);
+
+    FlagsmithClient client = FlagsmithClient.newBuilder()
+            .withFlagsmithApiWrapper(mockApiWrapper)
+            .withConfiguration(FlagsmithConfig.newBuilder().withLocalEvaluation(true).build())
+            .setApiKey("ser.dummy-key")
+            .build();
+
+    // When
+    // we call the update environment method twice
+    // (1st should be successful, 2nd will do nothing because of null return)
+    client.updateEnvironment();
+    client.updateEnvironment();
+
+    // Then
+    // The client environment is not overwritten with null
+    Assert.assertEquals(client.getEnvironment(), environmentModel);
+  }
 }