initialized empty list in the constuctor instead of passing null and removed check before if the list is empty

utkarsh · toddbaert · commit 9519a9e0ef77 · 2024-09-16T11:23:18.000-04:00
diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java
@@ -174,9 +174,7 @@ private void handleStateTransition(ProviderState oldState, ProviderState newStat
         if (ProviderState.READY.equals(oldState) && ProviderState.READY.equals(newState)) {
             log.debug("Configuration changed");
             ProviderEventDetails details = ProviderEventDetails.builder().message("configuration changed").build();
-            if (!changedFlagKeys.isEmpty()) {
-                details.setFlagsChanged(changedFlagKeys);
-            }
+            details.setFlagsChanged(changedFlagKeys);
             this.emitProviderConfigurationChanged(details);
             return;
         }
diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/GrpcConnector.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/GrpcConnector.java
@@ -1,5 +1,6 @@
 package dev.openfeature.contrib.providers.flagd.resolver.grpc;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.Random;
 import java.util.concurrent.TimeUnit;
@@ -100,7 +101,7 @@ public void shutdown() throws Exception {
                 this.channel.awaitTermination(this.deadline, TimeUnit.MILLISECONDS);
                 log.warn(String.format("Unable to shut down channel by %d deadline", this.deadline));
             }
-            this.stateConsumer.accept(ProviderState.NOT_READY, null);
+            this.stateConsumer.accept(ProviderState.NOT_READY, Collections.emptyList());
         }
     }
 
@@ -162,6 +163,6 @@ private void grpcStateConsumer(final ProviderState state) {
         }
 
         // chain to initiator
-        this.stateConsumer.accept(state, null);
+        this.stateConsumer.accept(state, Collections.emptyList());
     }
 }
diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolver.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolver.java
@@ -70,8 +70,7 @@ public void init() throws Exception {
                     final StorageStateDTO storageStateDTO = flagStore.getStateQueue().take();
                     switch (storageStateDTO.getStorageState()) {
                         case OK:
-                            stateConsumer.accept(ProviderState.READY,
-                                    storageStateDTO.getChangedFlags().keySet().stream().collect(Collectors.toList()));
+                            stateConsumer.accept(ProviderState.READY, storageStateDTO.getChangedFlagsKeys());
                             this.connected.set(true);
                             break;
                         case ERROR:
diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/model/FeatureFlag.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/model/FeatureFlag.java
@@ -5,6 +5,7 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import lombok.EqualsAndHashCode;
 import lombok.Getter;
 
 import java.util.Map;
@@ -16,6 +17,7 @@
 @SuppressFBWarnings(value = {"EI_EXPOSE_REP"},
         justification = "Feature flag comes as a Json configuration, hence they must be parsed and exposed")
 @JsonIgnoreProperties(ignoreUnknown = true)
+@EqualsAndHashCode
 public class FeatureFlag {
     public static final String EMPTY_TARGETING_STRING = "{}";
 
diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/FlagStore.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/FlagStore.java
@@ -7,13 +7,15 @@
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import lombok.extern.slf4j.Slf4j;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
+import java.util.stream.Collectors;
 
 /**
  * Feature flag storage.
@@ -99,29 +101,29 @@ private void streamerListener(final Connector connector) throws InterruptedExcep
             switch (take.getType()) {
                 case DATA:
                     try {
-                        Map<String,FeatureFlag> changedFlags;
+                        List<String> changedFlagsKeys;
                         Map<String, FeatureFlag> flagMap = FlagParser.parseString(take.getData(), throwIfInvalid);
                         writeLock.lock();
                         try {
-                            changedFlags = getChangedFlags(flagMap);
+                            changedFlagsKeys = getChangedFlagsKeys(flagMap);
                             flags.clear();
                             flags.putAll(flagMap);
                         } finally {
                             writeLock.unlock();
                         }
-                        if (!stateBlockingQueue.offer(new StorageStateDTO(StorageState.OK, changedFlags))) {
+                        if (!stateBlockingQueue.offer(new StorageStateDTO(StorageState.OK, changedFlagsKeys))) {
                             log.warn("Failed to convey OK satus, queue is full");
                         }
                     } catch (Throwable e) {
                         // catch all exceptions and avoid stream listener interruptions
                         log.warn("Invalid flag sync payload from connector", e);
-                        if (!stateBlockingQueue.offer(new StorageStateDTO(StorageState.STALE,null))) {
+                        if (!stateBlockingQueue.offer(new StorageStateDTO(StorageState.STALE))) {
                             log.warn("Failed to convey STALE satus, queue is full");
                         }
                     }
                     break;
                 case ERROR:
-                    if (!stateBlockingQueue.offer(new StorageStateDTO(StorageState.ERROR,null))) {
+                    if (!stateBlockingQueue.offer(new StorageStateDTO(StorageState.ERROR))) {
                         log.warn("Failed to convey ERROR satus, queue is full");
                     }
                     break;
@@ -133,7 +135,7 @@ private void streamerListener(final Connector connector) throws InterruptedExcep
         log.info("Shutting down store stream listener");
     }
 
-    private Map<String, FeatureFlag> getChangedFlags(Map<String, FeatureFlag> newFlags) {
+    private List<String> getChangedFlagsKeys(Map<String, FeatureFlag> newFlags) {
         Map<String, FeatureFlag> changedFlags = new HashMap<>();
         Map<String, FeatureFlag> addedFeatureFlags = new HashMap<>();
         Map<String, FeatureFlag> removedFeatureFlags = new HashMap<>();
@@ -153,7 +155,7 @@ private Map<String, FeatureFlag> getChangedFlags(Map<String, FeatureFlag> newFla
         changedFlags.putAll(addedFeatureFlags);
         changedFlags.putAll(removedFeatureFlags);
         changedFlags.putAll(updatedFeatureFlags);
-        return changedFlags;
+        return changedFlags.keySet().stream().collect(Collectors.toList());
     }
 
 }
diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/StorageStateDTO.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/StorageStateDTO.java
@@ -1,22 +1,27 @@
 package dev.openfeature.contrib.providers.flagd.resolver.process.storage;
 
 
-import dev.openfeature.contrib.providers.flagd.resolver.process.model.FeatureFlag;
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.ToString;
 
-import java.util.Map;
+import java.util.Collections;
+import java.util.List;
 
 @Getter
 @ToString
 @EqualsAndHashCode
 public class StorageStateDTO {
     private final StorageState storageState;
-    private final Map<String, FeatureFlag> changedFlags;
+    private final List<String> changedFlagsKeys;
 
-    public StorageStateDTO(StorageState storageState, Map<String, FeatureFlag>  changedFlags) {
+    public StorageStateDTO(StorageState storageState, List<String>  changedFlagsKeys) {
         this.storageState = storageState;
-        this.changedFlags = changedFlags;
+        this.changedFlagsKeys = changedFlagsKeys;
+    }
+
+    public StorageStateDTO(StorageState storageState) {
+        this.storageState = storageState;
+        this.changedFlagsKeys = Collections.emptyList();
     }
 }
diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java
@@ -920,7 +920,7 @@ private FlagdProvider createInProcessProvider() {
         final FlagdProvider provider = new FlagdProvider(flagdOptions);
         final MockStorage mockStorage = new MockStorage(new HashMap<String, FeatureFlag>(),
                 new LinkedBlockingQueue<StorageStateDTO>(Arrays.asList(new
-                        StorageStateDTO(StorageState.OK, Collections.emptyMap()))));
+                        StorageStateDTO(StorageState.OK))));
         
         try {
             final Field flagResolver = FlagdProvider.class.getDeclaredField("flagResolver");
diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolverTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolverTest.java
@@ -84,10 +84,10 @@ public void eventHandling() throws Throwable {
             }
         });
         initThread.start();
-        if (!sender.offer(new StorageStateDTO(StorageState.OK, Collections.emptyMap()), 100, TimeUnit.MILLISECONDS)) {
+        if (!sender.offer(new StorageStateDTO(StorageState.OK, Collections.EMPTY_LIST), 100, TimeUnit.MILLISECONDS)) {
             Assertions.fail("failed to send the event");
         }
-        if (!sender.offer(new StorageStateDTO(StorageState.ERROR,null), 100, TimeUnit.MILLISECONDS)) {
+        if (!sender.offer(new StorageStateDTO(StorageState.ERROR), 100, TimeUnit.MILLISECONDS)) {
             Assertions.fail("failed to send the event");
         }
 
diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/FlagStoreTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/FlagStoreTest.java
@@ -11,6 +11,7 @@
 import java.util.Map;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
+import java.util.stream.Collectors;
 
 import static dev.openfeature.contrib.providers.flagd.resolver.process.TestUtils.INVALID_FLAG;
 import static dev.openfeature.contrib.providers.flagd.resolver.process.TestUtils.VALID_LONG;
@@ -87,8 +88,9 @@ public void changedFlags() throws Exception {
             payload.offer(new StreamPayload(StreamPayloadType.DATA, getFlagsFromResource(VALID_SIMPLE)));
         });
         // flags changed for first time
-        assertEquals(FlagParser.parseString(getFlagsFromResource(VALID_SIMPLE), true).keySet(),
-                storageStateDTOS.take().getChangedFlags().keySet());
+        assertEquals(FlagParser.parseString(
+                getFlagsFromResource(VALID_SIMPLE), true).keySet().stream().collect(Collectors.toList()),
+                storageStateDTOS.take().getChangedFlagsKeys());
 
         assertTimeoutPreemptively(Duration.ofMillis(maxDelay), ()-> {
             payload.offer(new StreamPayload(StreamPayloadType.DATA, getFlagsFromResource(VALID_LONG)));
@@ -97,7 +99,8 @@ public void changedFlags() throws Exception {
                 FlagParser.parseString(getFlagsFromResource(VALID_LONG),true);
         expectedChangedFlags.remove("myBoolFlag");
         // flags changed from initial VALID_SIMPLE flag
-        Assert.assertEquals(expectedChangedFlags.keySet(),storageStateDTOS.take().getChangedFlags().keySet());
+        Assert.assertEquals(expectedChangedFlags.keySet().stream().collect(Collectors.toList()),
+                storageStateDTOS.take().getChangedFlagsKeys());
     }
 
 }

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`	`1`	`package dev.openfeature.contrib.providers.flagd.resolver.grpc;`
`2`	`2`
	`3`	`+import java.util.Collections;`
`3`	`4`	`import java.util.List;`
`4`	`5`	`import java.util.Random;`
`5`	`6`	`import java.util.concurrent.TimeUnit;`
`@@ -100,7 +101,7 @@ public void shutdown() throws Exception {`
`100`	`101`	`this.channel.awaitTermination(this.deadline, TimeUnit.MILLISECONDS);`
`101`	`102`	`log.warn(String.format("Unable to shut down channel by %d deadline", this.deadline));`
`102`	`103`	`}`
`103`		`- this.stateConsumer.accept(ProviderState.NOT_READY, null);`
	`104`	`+ this.stateConsumer.accept(ProviderState.NOT_READY, Collections.emptyList());`
`104`	`105`	`}`
`105`	`106`	`}`
`106`	`107`
`@@ -162,6 +163,6 @@ private void grpcStateConsumer(final ProviderState state) {`
`162`	`163`	`}`
`163`	`164`
`164`	`165`	`// chain to initiator`
`165`		`- this.stateConsumer.accept(state, null);`
	`166`	`+ this.stateConsumer.accept(state, Collections.emptyList());`
`166`	`167`	`}`
`167`	`168`	`}`
Original file line number	Diff line number	Diff line change
`@@ -84,10 +84,10 @@ public void eventHandling() throws Throwable {`
`84`	`84`	`}`
`85`	`85`	`});`
`86`	`86`	`initThread.start();`
`87`		`- if (!sender.offer(new StorageStateDTO(StorageState.OK, Collections.emptyMap()), 100, TimeUnit.MILLISECONDS)) {`
	`87`	`+ if (!sender.offer(new StorageStateDTO(StorageState.OK, Collections.EMPTY_LIST), 100, TimeUnit.MILLISECONDS)) {`
`88`	`88`	`Assertions.fail("failed to send the event");`
`89`	`89`	`}`
`90`		`- if (!sender.offer(new StorageStateDTO(StorageState.ERROR,null), 100, TimeUnit.MILLISECONDS)) {`
	`90`	`+ if (!sender.offer(new StorageStateDTO(StorageState.ERROR), 100, TimeUnit.MILLISECONDS)) {`
`91`	`91`	`Assertions.fail("failed to send the event");`
`92`	`92`	`}`
`93`	`93`