From 433886ed2201bfc784d7bcdad33087e9b3277bb1 Mon Sep 17 00:00:00 2001 From: Arek Szostak <50179409+akiraqb@users.noreply.github.com> Date: Wed, 7 Aug 2024 14:55:39 +0100 Subject: [PATCH 1/6] [FFM-11846] : Store and delete previous version of the FeatureConfig along with the new one in cache (#194) * Extended interface to return current and previous version * Added extra configuration paramtere to the StorageRepository + added logic to store and retrieve current + previous version. * Added StorageRepositoryTest class and test data. --- .../io/harness/cf/client/api/InnerClient.java | 2 +- .../java/io/harness/cf/client/api/Query.java | 4 + .../cf/client/api/StorageRepository.java | 74 ++++++- .../client/api/EvaluatorIntegrationTest.java | 2 +- .../harness/cf/client/api/EvaluatorTest.java | 8 +- .../cf/client/api/StorageRepositoryTest.java | 199 ++++++++++++++++++ .../io/harness/cf/client/api/TestUtils.java | 4 + .../basic_bool_string_for_repository.json | 28 +++ 8 files changed, 308 insertions(+), 13 deletions(-) create mode 100644 src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java create mode 100644 src/test/resources/local-test-cases/basic_bool_string_for_repository.json diff --git a/src/main/java/io/harness/cf/client/api/InnerClient.java b/src/main/java/io/harness/cf/client/api/InnerClient.java index 50be3524..f29a3bc1 100644 --- a/src/main/java/io/harness/cf/client/api/InnerClient.java +++ b/src/main/java/io/harness/cf/client/api/InnerClient.java @@ -86,7 +86,7 @@ protected void setUp(@NonNull final Connector connector, @NonNull final BaseConf this.connector.setOnUnauthorized(this::onUnauthorized); // initialization - repository = new StorageRepository(options.getCache(), options.getStore(), this); + repository = new StorageRepository(options.getCache(), options.getStore(), this, false); evaluator = new Evaluator(repository, options); authService = new AuthService(this.connector, options.getPollIntervalInSeconds(), this); pollProcessor = diff --git a/src/main/java/io/harness/cf/client/api/Query.java b/src/main/java/io/harness/cf/client/api/Query.java index 3df37e97..ca59d369 100644 --- a/src/main/java/io/harness/cf/client/api/Query.java +++ b/src/main/java/io/harness/cf/client/api/Query.java @@ -13,4 +13,8 @@ public interface Query { Optional getSegment(@NonNull String identifier); List findFlagsBySegment(@NonNull String identifier); + + Optional getCurrentAndPreviousFeatureConfig(@NonNull String identifier); + + List getAllFeatureIdentifiers(String prefix); } diff --git a/src/main/java/io/harness/cf/client/api/StorageRepository.java b/src/main/java/io/harness/cf/client/api/StorageRepository.java index 840c3e45..e0c3403d 100644 --- a/src/main/java/io/harness/cf/client/api/StorageRepository.java +++ b/src/main/java/io/harness/cf/client/api/StorageRepository.java @@ -4,10 +4,7 @@ import io.harness.cf.client.common.Storage; import io.harness.cf.client.common.Utils; import io.harness.cf.model.*; -import java.util.ArrayList; -import java.util.Comparator; -import java.util.List; -import java.util.Optional; +import java.util.*; import lombok.NonNull; import lombok.extern.slf4j.Slf4j; @@ -18,13 +15,23 @@ class StorageRepository implements Repository { private Storage store; private final RepositoryCallback callback; - public StorageRepository(@NonNull Cache cache, RepositoryCallback callback) { + private final boolean cachePreviousFeatureConfigVersion; + + public StorageRepository( + @NonNull Cache cache, + RepositoryCallback callback, + boolean cachePreviousFeatureConfigVersion) { this.cache = cache; this.callback = callback; + this.cachePreviousFeatureConfigVersion = cachePreviousFeatureConfigVersion; } - public StorageRepository(@NonNull Cache cache, Storage store, RepositoryCallback callback) { - this(cache, callback); + public StorageRepository( + @NonNull Cache cache, + Storage store, + RepositoryCallback callback, + boolean cachePreviousFeatureConfigVersion) { + this(cache, callback, cachePreviousFeatureConfigVersion); this.store = store; } @@ -49,6 +56,37 @@ public Optional getFlag(@NonNull String identifier) { return getFlag(identifier, true); } + public List getAllFeatureIdentifiers(String prefix) { + List identifiers = new LinkedList<>(); + List keys = cache.keys(); + String flagPrefix = "flags/"; + for (String key : keys) { + if (key.startsWith(flagPrefix)) { + // Strip the flag prefix + String strippedKey = key.substring(flagPrefix.length()); + // If prefix is empty, add all stripped keys, otherwise check for prefix match + if (prefix.isEmpty() || strippedKey.startsWith(prefix)) { + identifiers.add(strippedKey); + } + } + } + return identifiers; + } + + public Optional getCurrentAndPreviousFeatureConfig(@NonNull String identifier) { + final String flagKey = formatFlagKey(identifier); + final String pFlagKey = formatPrevFlagKey(identifier); + + FeatureConfig pFlag = (FeatureConfig) cache.get(pFlagKey); + FeatureConfig cFlag = (FeatureConfig) cache.get(flagKey); + + // we should have at least current flag there to return. + if (cFlag != null) { + return Optional.of(new FeatureConfig[] {pFlag, cFlag}); + } + return Optional.empty(); + } + public Optional getSegment(@NonNull String identifier, boolean cacheable) { final String segmentKey = formatSegmentKey(identifier); Segment segment = (Segment) cache.get(segmentKey); @@ -115,6 +153,17 @@ public void setFlag(@NonNull String identifier, @NonNull FeatureConfig featureCo cache.delete(flagKey); log.debug("Flag {} successfully stored and cache invalidated", identifier); } else { + // extract and set the current featureConfig to the previous + if (cachePreviousFeatureConfigVersion) { + Object pFeatureConfig = cache.get(flagKey); + if (pFeatureConfig != null) { + // set the old version of the config to the cache + final String pFlagKey = formatPrevFlagKey(identifier); + cache.set(pFlagKey, pFeatureConfig); + log.debug("Flag {} successfully cached", pFlagKey); + } + } + // save a new config to the cache cache.set(flagKey, featureConfig); log.debug("Flag {} successfully cached", identifier); } @@ -150,10 +199,17 @@ public void setSegment(@NonNull String identifier, @NonNull Segment segment) { @Override public void deleteFlag(@NonNull String identifier) { final String flagKey = this.formatFlagKey(identifier); + final String pflgKey = this.formatPrevFlagKey(identifier); if (store != null) { + if (cachePreviousFeatureConfigVersion) { + store.delete(pflgKey); + } store.delete(flagKey); log.debug("Flag {} successfully deleted from store", identifier); } + if (cachePreviousFeatureConfigVersion) { + this.cache.delete(pflgKey); + } this.cache.delete(flagKey); log.debug("Flag {} successfully deleted from cache", identifier); if (callback != null) { @@ -209,6 +265,10 @@ protected String formatFlagKey(@NonNull String identifier) { return String.format("flags/%s", identifier); } + protected String formatPrevFlagKey(@NonNull String identifier) { + return String.format("previous/%s", identifier); + } + @NonNull protected String formatSegmentKey(@NonNull String identifier) { return String.format("segments/%s", identifier); diff --git a/src/test/java/io/harness/cf/client/api/EvaluatorIntegrationTest.java b/src/test/java/io/harness/cf/client/api/EvaluatorIntegrationTest.java index dea8de92..3c1b52ca 100644 --- a/src/test/java/io/harness/cf/client/api/EvaluatorIntegrationTest.java +++ b/src/test/java/io/harness/cf/client/api/EvaluatorIntegrationTest.java @@ -82,7 +82,7 @@ public List getTestCases() throws Exception { removeExtension(file.getName())); final Repository repository = - new StorageRepository(new CaffeineCache(10000), null); + new StorageRepository(new CaffeineCache(10000), null, false); final Evaluator evaluator = new Evaluator(repository, Mockito.mock(BaseConfig.class)); diff --git a/src/test/java/io/harness/cf/client/api/EvaluatorTest.java b/src/test/java/io/harness/cf/client/api/EvaluatorTest.java index c4b3e01f..3e094515 100644 --- a/src/test/java/io/harness/cf/client/api/EvaluatorTest.java +++ b/src/test/java/io/harness/cf/client/api/EvaluatorTest.java @@ -34,7 +34,7 @@ public class EvaluatorTest { @BeforeAll public void setupUp() throws IOException, URISyntaxException { - final StorageRepository repository = new StorageRepository(new CaffeineCache(100), null, null); + final StorageRepository repository = new StorageRepository(new CaffeineCache(100), null, false); evaluator = new Evaluator(repository, Mockito.mock(BaseConfig.class)); loadSegments(repository, "local-test-cases/segments.json"); @@ -98,7 +98,7 @@ public void testTargetV2OrOperator(ORTest test) throws Exception { private void testTargetV2Operator(String email, String role, String flagName, String expected) throws Exception { - final StorageRepository repository = new StorageRepository(new CaffeineCache(100), null, null); + final StorageRepository repository = new StorageRepository(new CaffeineCache(100), null, false); final Evaluator evaluator = new Evaluator(repository, Mockito.mock(BaseConfig.class)); loadFlags(repository, "local-test-cases/v2-andor-flags.json"); @@ -259,7 +259,7 @@ public void shouldReturnCorrectAttrForGetAttrValue() { @Test public void shouldCorrectlyEvaluatePrereqsIfIdAndValueDiffer() throws Exception { - final StorageRepository repo = new StorageRepository(new CaffeineCache(100), null, null); + final StorageRepository repo = new StorageRepository(new CaffeineCache(100), null, false); final Evaluator eval = new Evaluator(repo, Mockito.mock(BaseConfig.class)); loadSegments(repo, "local-test-cases/segments.json"); @@ -298,7 +298,7 @@ public void testEvaluateRules() throws InterruptedException { final CountDownLatch latch = new CountDownLatch(threadCount); final ExecutorService executor = Executors.newFixedThreadPool(threadCount); final CopyOnWriteArrayList failures = new CopyOnWriteArrayList<>(); - final Query repository = new StorageRepository(new CaffeineCache(100), null, null); + final Query repository = new StorageRepository(new CaffeineCache(100), null, false); final Evaluator evaluator = new Evaluator(repository, Mockito.mock(BaseConfig.class)); final List values = new ArrayList<>(); diff --git a/src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java b/src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java new file mode 100644 index 00000000..57d2888b --- /dev/null +++ b/src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java @@ -0,0 +1,199 @@ +package io.harness.cf.client.api; + +import static org.junit.jupiter.api.Assertions.*; + +import com.google.gson.Gson; +import io.harness.cf.model.FeatureConfig; +import java.awt.*; +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.LinkedList; +import java.util.List; +import java.util.Optional; +import java.util.stream.Stream; +import lombok.NonNull; +import org.junit.jupiter.api.Test; + +class StorageRepositoryTest { + private final Gson gson = new Gson(); + + // initialize the config and load the flags. + // load the test + + @Test + void shouldInitialiseRepo() { + final Repository repository = new StorageRepository(new CaffeineCache(10000), null, false); + assertInstanceOf(StorageRepository.class, repository); + } + + @Test + void shouldStoreCurrentConfig() throws Exception { + final Repository repository = new StorageRepository(new CaffeineCache(10000), null, false); + assertInstanceOf(StorageRepository.class, repository); + + FeatureConfig featureConfig = GetFeatureConfigFromFile(); + FeatureConfig featureConfigUpdated = GetUpdatedFeatureConfigFromFile(); + + assertNotNull(featureConfig); + assertNotNull(featureConfigUpdated); + + loadFlags(repository, makeFeatureList(featureConfig)); + loadFlags(repository, makeFeatureList(featureConfigUpdated)); + + Optional res = + repository.getCurrentAndPreviousFeatureConfig(featureConfigUpdated.getFeature()); + FeatureConfig[] resFc = res.get(); + + FeatureConfig previous = resFc[0]; + FeatureConfig current = resFc[1]; + + // check if previous version is null + assertNull(previous); + assertNotNull(current); + + // check if the current version is correct + assertEquals(current.getVersion(), new Long(2)); + } + + @Test + void shouldStorePreviousAndCurrentConfig() throws Exception { + final Repository repository = new StorageRepository(new CaffeineCache(10000), null, true); + assertInstanceOf(StorageRepository.class, repository); + + FeatureConfig featureConfig = GetFeatureConfigFromFile(); + FeatureConfig featureConfigUpdated = GetUpdatedFeatureConfigFromFile(); + + assertNotNull(featureConfig); + assertNotNull(featureConfigUpdated); + + loadFlags(repository, makeFeatureList(featureConfig)); + loadFlags(repository, makeFeatureList(featureConfigUpdated)); + + Optional res = + repository.getCurrentAndPreviousFeatureConfig(featureConfigUpdated.getFeature()); + FeatureConfig[] resFc = res.get(); + + FeatureConfig previous = resFc[0]; + FeatureConfig current = resFc[1]; + + // check if previous version is null + assertNotNull(previous); + assertNotNull(current); + + // check if the current version is correct + assertEquals(previous.getVersion(), new Long(1)); + assertEquals(current.getVersion(), new Long(2)); + } + + @Test + void shouldDeletePreviousAndCurrentConfig() throws Exception { + final Repository repository = new StorageRepository(new CaffeineCache(10000), null, true); + assertInstanceOf(StorageRepository.class, repository); + + FeatureConfig featureConfig = GetFeatureConfigFromFile(); + FeatureConfig featureConfigUpdated = GetUpdatedFeatureConfigFromFile(); + + assertNotNull(featureConfig); + assertNotNull(featureConfigUpdated); + + loadFlags(repository, makeFeatureList(featureConfig)); + loadFlags(repository, makeFeatureList(featureConfigUpdated)); + + String featureIdentifier = featureConfig.getFeature(); + + Optional res = + repository.getCurrentAndPreviousFeatureConfig(featureIdentifier); + FeatureConfig[] resFc = res.get(); + + FeatureConfig previous = resFc[0]; + FeatureConfig current = resFc[1]; + + // check if previous version is null + assertNotNull(previous); + assertNotNull(current); + + // check if the current version is correct + assertEquals(previous.getVersion(), new Long(1)); + assertEquals(current.getVersion(), new Long(2)); + + // delete config + repository.deleteFlag(featureIdentifier); + Optional result = + repository.getCurrentAndPreviousFeatureConfig(featureIdentifier); + + assertFalse(result.isPresent(), "The Optional should be empty"); + } + + @Test + void shouldListAllKeysTest() throws Exception { + final Repository repository = new StorageRepository(new CaffeineCache(10000), null, true); + assertInstanceOf(StorageRepository.class, repository); + + FeatureConfig featureConfig = GetFeatureConfigFromFile(); + FeatureConfig featureConfigUpdated = GetUpdatedFeatureConfigFromFile(); + + assertNotNull(featureConfig); + assertNotNull(featureConfigUpdated); + + loadFlags(repository, makeFeatureList(featureConfig)); + loadFlags(repository, makeFeatureList(featureConfigUpdated)); + + List keys = repository.getAllFeatureIdentifiers(""); + assertEquals(keys.size(), 1); + assertEquals(keys.get(0), featureConfigUpdated.getFeature()); + } + + private void loadFlags(Repository repository, List flags) { + if (flags != null) { + for (FeatureConfig nextFlag : flags) { + repository.setFlag(nextFlag.getFeature(), nextFlag); + } + } + } + + @NonNull + private String read(@NonNull final String path) { + + final StringBuilder builder = new StringBuilder(); + try (final Stream stream = Files.lines(Paths.get(path), StandardCharsets.UTF_8)) { + stream.forEach(s -> builder.append(s).append("\n")); + } catch (IOException e) { + fail(e.getMessage()); + } + return builder.toString(); + } + + private List makeFeatureList(FeatureConfig fc) { + List fg = new LinkedList<>(); + fg.add(fc); + return fg; + } + + private FeatureConfig GetUpdatedFeatureConfigFromFile() throws Exception { + FeatureConfig fc = GetFeatureConfigFromFile(); + fc.setVersion(new Long(2)); + return fc; + } + + // get the flags and populate it. + private FeatureConfig GetFeatureConfigFromFile() throws Exception { + try { + String relativePath = + "./src/test/resources/local-test-cases/basic_bool_string_for_repository.json"; + // Resolve the absolute path + String filePath = new File(relativePath).getCanonicalPath(); + // Read the content of the file into a String + String jsonString = new String(Files.readAllBytes(Paths.get(filePath))); + final FeatureConfig featureConfig = gson.fromJson(jsonString, FeatureConfig.class); + return featureConfig; + + } catch (IOException e) { + // Handle exceptions like file not found or read errors + e.printStackTrace(); + } + return null; + } +} diff --git a/src/test/java/io/harness/cf/client/api/TestUtils.java b/src/test/java/io/harness/cf/client/api/TestUtils.java index 61ce86ce..98894632 100644 --- a/src/test/java/io/harness/cf/client/api/TestUtils.java +++ b/src/test/java/io/harness/cf/client/api/TestUtils.java @@ -20,6 +20,10 @@ public static String makeBasicFeatureJson() throws IOException, URISyntaxExcepti return getJsonResource("local-test-cases/basic_bool_string_number_json_variations.json"); } + public static String makeBasicFeatureJsonForRepoTest() throws IOException, URISyntaxException { + return getJsonResource("local-test-cases/basic_bool_string_for_repository.json"); + } + public static String getJsonResource(String location) throws IOException, URISyntaxException { final Path path = Paths.get(EvaluatorTest.class.getClassLoader().getResource(location).toURI()); return new String(Files.readAllBytes(path)); diff --git a/src/test/resources/local-test-cases/basic_bool_string_for_repository.json b/src/test/resources/local-test-cases/basic_bool_string_for_repository.json new file mode 100644 index 00000000..04830182 --- /dev/null +++ b/src/test/resources/local-test-cases/basic_bool_string_for_repository.json @@ -0,0 +1,28 @@ +{ + "defaultServe": { + "variation": "true" + }, + "environment": "TEST", + "feature": "simplebool", + "kind": "boolean", + "offVariation": "false", + "prerequisites": [], + "project": "test", + "rules": [], + "state": "on", + "variationToTargetMap": null, + "variations": [ + { + "identifier": "true", + "name": "True", + "value": "true" + }, + { + "identifier": "false", + "name": "False", + "value": "false" + } + ], + "version": 1 + } + From 80307b1ee0e2199ae711a50d0622a2a509fad449 Mon Sep 17 00:00:00 2001 From: Arek Szostak <50179409+akiraqb@users.noreply.github.com> Date: Thu, 8 Aug 2024 09:06:00 +0100 Subject: [PATCH 2/6] [FFM-11856]: Store and retrieve previous and current version of the feature config from the fileStore. (#195) * Store previous and current version of the flag in the FileStore * Updating logs * Fixed typo --- .../cf/client/api/StorageRepository.java | 44 +++++++---- .../cf/client/api/StorageRepositoryTest.java | 73 +++++++++++++++++++ 2 files changed, 103 insertions(+), 14 deletions(-) diff --git a/src/main/java/io/harness/cf/client/api/StorageRepository.java b/src/main/java/io/harness/cf/client/api/StorageRepository.java index e0c3403d..cf1a1ece 100644 --- a/src/main/java/io/harness/cf/client/api/StorageRepository.java +++ b/src/main/java/io/harness/cf/client/api/StorageRepository.java @@ -80,10 +80,21 @@ public Optional getCurrentAndPreviousFeatureConfig(@NonNull Str FeatureConfig pFlag = (FeatureConfig) cache.get(pFlagKey); FeatureConfig cFlag = (FeatureConfig) cache.get(flagKey); - // we should have at least current flag there to return. if (cFlag != null) { return Optional.of(new FeatureConfig[] {pFlag, cFlag}); } + // if we don't have it in cache we check the file + if (this.store != null) { + pFlag = (FeatureConfig) store.get(pFlagKey); + cFlag = (FeatureConfig) store.get(flagKey); + if (pFlag != null) { + cache.set(pFlagKey, pFlag); + } + if (cFlag != null) { + cache.set(flagKey, cFlag); + } + return Optional.of(new FeatureConfig[] {pFlag, cFlag}); + } return Optional.empty(); } @@ -147,26 +158,31 @@ public void setFlag(@NonNull String identifier, @NonNull FeatureConfig featureCo log.debug("Flag {} already exists", identifier); return; } + final String flagKey = formatFlagKey(identifier); + final Object previousFeatureConfig = store != null ? store.get(flagKey) : cache.get(flagKey); + + if (cachePreviousFeatureConfigVersion && previousFeatureConfig != null) { + final String previousFlagKey = formatPrevFlagKey(identifier); + if (store != null) { + store.set(previousFlagKey, previousFeatureConfig); + cache.delete(previousFlagKey); + log.debug("Flag {} successfully stored and cache invalidated", previousFlagKey); + } else { + cache.set(previousFlagKey, previousFeatureConfig); + } + log.debug("Flag {} successfully stored", previousFlagKey); + } + if (store != null) { store.set(flagKey, featureConfig); cache.delete(flagKey); - log.debug("Flag {} successfully stored and cache invalidated", identifier); } else { - // extract and set the current featureConfig to the previous - if (cachePreviousFeatureConfigVersion) { - Object pFeatureConfig = cache.get(flagKey); - if (pFeatureConfig != null) { - // set the old version of the config to the cache - final String pFlagKey = formatPrevFlagKey(identifier); - cache.set(pFlagKey, pFeatureConfig); - log.debug("Flag {} successfully cached", pFlagKey); - } - } - // save a new config to the cache cache.set(flagKey, featureConfig); - log.debug("Flag {} successfully cached", identifier); } + + log.debug("Flag {} successfully stored", identifier); + if (callback != null) { callback.onFlagStored(identifier); } diff --git a/src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java b/src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java index 57d2888b..0cbd3ed2 100644 --- a/src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java +++ b/src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java @@ -58,6 +58,79 @@ void shouldStoreCurrentConfig() throws Exception { assertEquals(current.getVersion(), new Long(2)); } + @Test + void shouldStoreCurrentConfigWithFileStore() throws Exception { + + File file = File.createTempFile(FileMapStoreTest.class.getSimpleName(), ".tmp"); + file.deleteOnExit(); + + XmlFileMapStore store = new XmlFileMapStore(file.getAbsolutePath()); + + final Repository repository = + new StorageRepository(new CaffeineCache(10000), store, null, false); + assertInstanceOf(StorageRepository.class, repository); + + FeatureConfig featureConfig = GetFeatureConfigFromFile(); + FeatureConfig featureConfigUpdated = GetUpdatedFeatureConfigFromFile(); + + assertNotNull(featureConfig); + assertNotNull(featureConfigUpdated); + + loadFlags(repository, makeFeatureList(featureConfig)); + loadFlags(repository, makeFeatureList(featureConfigUpdated)); + + Optional res = + repository.getCurrentAndPreviousFeatureConfig(featureConfigUpdated.getFeature()); + FeatureConfig[] resFc = res.get(); + + FeatureConfig previous = resFc[0]; + FeatureConfig current = resFc[1]; + + // check if previous version is null + assertNull(previous); + assertNotNull(current); + + // check if the current version is correct + assertEquals(current.getVersion(), new Long(2)); + } + + @Test + void shouldStorePreviousAndCurrentConfigWithFileStore() throws Exception { + + File file = File.createTempFile(FileMapStoreTest.class.getSimpleName(), ".tmp"); + file.deleteOnExit(); + + XmlFileMapStore store = new XmlFileMapStore(file.getAbsolutePath()); + + final Repository repository = + new StorageRepository(new CaffeineCache(10000), store, null, true); + assertInstanceOf(StorageRepository.class, repository); + + FeatureConfig featureConfig = GetFeatureConfigFromFile(); + FeatureConfig featureConfigUpdated = GetUpdatedFeatureConfigFromFile(); + + assertNotNull(featureConfig); + assertNotNull(featureConfigUpdated); + + loadFlags(repository, makeFeatureList(featureConfig)); + loadFlags(repository, makeFeatureList(featureConfigUpdated)); + + Optional res = + repository.getCurrentAndPreviousFeatureConfig(featureConfigUpdated.getFeature()); + FeatureConfig[] resFc = res.get(); + + FeatureConfig previous = resFc[0]; + FeatureConfig current = resFc[1]; + + // check if previous version is null + assertNotNull(previous); + assertNotNull(current); + + // check if the current version is correct + assertEquals(previous.getVersion(), new Long(1)); + assertEquals(current.getVersion(), new Long(2)); + } + @Test void shouldStorePreviousAndCurrentConfig() throws Exception { final Repository repository = new StorageRepository(new CaffeineCache(10000), null, true); From d76ef2189f353360f494964cdda93b9f23699650 Mon Sep 17 00:00:00 2001 From: Arek Szostak <50179409+akiraqb@users.noreply.github.com> Date: Thu, 8 Aug 2024 14:01:20 +0100 Subject: [PATCH 3/6] [FFM-11847]: Extend SDK client to expose FeatureSnapshot (#196) * Added snapshot structure + and expose fucntions to fetch the snapshot. * Updated TODO --- .../io/harness/cf/client/api/InnerClient.java | 39 +++++++++++++++++++ src/main/resources/client-v1.yaml | 7 ++++ 2 files changed, 46 insertions(+) diff --git a/src/main/java/io/harness/cf/client/api/InnerClient.java b/src/main/java/io/harness/cf/client/api/InnerClient.java index f29a3bc1..8ad56fbd 100644 --- a/src/main/java/io/harness/cf/client/api/InnerClient.java +++ b/src/main/java/io/harness/cf/client/api/InnerClient.java @@ -1,12 +1,17 @@ package io.harness.cf.client.api; +import com.google.gson.Gson; import com.google.gson.JsonObject; import io.harness.cf.client.common.SdkCodes; import io.harness.cf.client.connector.*; import io.harness.cf.client.dto.Message; import io.harness.cf.client.dto.Target; import io.harness.cf.model.FeatureConfig; +import io.harness.cf.model.FeatureSnapshot; import io.harness.cf.model.Variation; +import java.util.LinkedList; +import java.util.List; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Consumer; @@ -307,6 +312,40 @@ public synchronized void waitForInitialization() } } + public List getFeatureSnapshots() { + // TODO return null/empty list if snapshot is not enabled in config. + return getFeatureSnapshots(""); + } + + public List getFeatureSnapshots(String prefix) { + // TODO return null/empty list if snapshot is not enabled in config. + List identifiers = repository.getAllFeatureIdentifiers(prefix); + List snapshots = new LinkedList<>(); + + for (String identifier : identifiers) { + FeatureSnapshot snapshot = getFeatureSnapshot(identifier); + snapshots.add(snapshot); + } + + return snapshots; + } + + public FeatureSnapshot getFeatureSnapshot(@NonNull String identifier) { + // TODO return null/empty list if snapshot is not enabled in config. + Optional ofc = repository.getCurrentAndPreviousFeatureConfig(identifier); + FeatureSnapshot result = new FeatureSnapshot(); + if (ofc.isPresent()) { + FeatureConfig[] fc = ofc.get(); + result.setPrevious(fc[0]); + result.setCurrent(fc[1]); + } + // this is here to create a deep copy of the object before its returned. + // this way we protect the cache. + Gson gson = new Gson(); + FeatureSnapshot deepCopySnapshot = gson.fromJson(gson.toJson(result), FeatureSnapshot.class); + return deepCopySnapshot; + } + public void on(@NonNull final Event event, @NonNull final Consumer consumer) { final CopyOnWriteArrayList> consumers = events.getOrDefault(event, new CopyOnWriteArrayList<>()); diff --git a/src/main/resources/client-v1.yaml b/src/main/resources/client-v1.yaml index f4643fe3..23c7661c 100644 --- a/src/main/resources/client-v1.yaml +++ b/src/main/resources/client-v1.yaml @@ -561,6 +561,13 @@ components: type: string required: - variation + FeatureSnapshot: + type: object + properties: + current: + $ref: '#/components/schemas/FeatureConfig' + previous: + $ref: '#/components/schemas/FeatureConfig' FeatureConfig: type: object properties: From babe4e4f9fec9f3f2c8fbfa2d59f9fa8744ed9b1 Mon Sep 17 00:00:00 2001 From: Arek Szostak <50179409+akiraqb@users.noreply.github.com> Date: Fri, 9 Aug 2024 15:32:40 +0100 Subject: [PATCH 4/6] Added base config option and public functions to get snapshot (#197) --- examples/build.gradle | 10 ++ .../EventExampleWithFeatureSnapshot.java | 106 ++++++++++++++++++ .../io/harness/cf/client/api/BaseConfig.java | 3 + .../io/harness/cf/client/api/CfClient.java | 14 +++ .../io/harness/cf/client/api/InnerClient.java | 13 ++- 5 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 examples/src/main/java/io/harness/ff/examples/EventExampleWithFeatureSnapshot.java diff --git a/examples/build.gradle b/examples/build.gradle index 0d0ec470..d06ef1e3 100644 --- a/examples/build.gradle +++ b/examples/build.gradle @@ -18,3 +18,13 @@ tasks.register('GettingStarted', JavaExec) { classpath = sourceSets.main.runtimeClasspath mainClass = "io.harness.ff.examples.GettingStarted" } + +tasks.register('EventExample', JavaExec) { + classpath = sourceSets.main.runtimeClasspath + mainClass = "io.harness.ff.examples.EventExample" +} + +tasks.register('EventExampleWithFeatureSnapshot', JavaExec) { + classpath = sourceSets.main.runtimeClasspath + mainClass = "io.harness.ff.examples.EventExampleWithFeatureSnapshot" +} diff --git a/examples/src/main/java/io/harness/ff/examples/EventExampleWithFeatureSnapshot.java b/examples/src/main/java/io/harness/ff/examples/EventExampleWithFeatureSnapshot.java new file mode 100644 index 00000000..261b1704 --- /dev/null +++ b/examples/src/main/java/io/harness/ff/examples/EventExampleWithFeatureSnapshot.java @@ -0,0 +1,106 @@ +package io.harness.ff.examples; + +import io.harness.cf.client.api.BaseConfig; +import io.harness.cf.client.api.CfClient; +import io.harness.cf.client.api.Event; +import io.harness.cf.client.api.XmlFileMapStore; +import io.harness.cf.client.connector.HarnessConfig; +import io.harness.cf.client.connector.HarnessConnector; +import io.harness.cf.client.dto.Target; +import io.harness.cf.model.FeatureSnapshot; +import lombok.extern.slf4j.Slf4j; + +import java.util.List; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +@Slf4j +public class EventExamplePoC { + private static final String SDK_KEY = ""; + private static final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); + + private static CfClient client; + + public static void main(String... args) { + + Runtime.getRuntime().addShutdownHook(new Thread(() -> { + scheduler.shutdown(); + client.close(); + })); + + final XmlFileMapStore fileStore = new XmlFileMapStore("Non-Freemium"); + // this is one way of initialising config. + final HarnessConnector hc = new HarnessConnector(SDK_KEY, HarnessConfig.builder().build()); + + BaseConfig bc = BaseConfig.builder(). + enableFeatureSnapshot(true).build(); + + // initialise the client. + client = new CfClient(hc, bc); + + client.on(Event.READY, result -> log.info("READY")); + + // example: specified prefix we can filter on. + final String FLAG_PREFIX = "FFM_"; + // example: given flag change event - get both previous and current feature if prefix is matched. + client.on(Event.CHANGED, identifier -> { + if (identifier.startsWith(FLAG_PREFIX)) { + getSnapshot(identifier); + } else { + log.info("We had an event change but flag did not have required prefix"); + } + }); + + // example : given flag change event - get all snapshots. + client.on(Event.CHANGED, identifier -> { + getAllSnapshots(); + }); + + // example : given flag change event - get all snapshots with given prefix. + client.on(Event.CHANGED, identifier -> { + getAllSnapshotsWithPrefix(); + }); + + final Target target = Target.builder().identifier("target1").attribute("testKey", "TestValue").name("target1").build(); + + scheduler.scheduleAtFixedRate(() -> { + log.info("ticking..."); + }, 0, 10, TimeUnit.SECONDS); + + } + + + // example method to extract a single snapshot. + private static void getSnapshot(String identifier) { + log.info("We had a chang event and prefix matched, lets inspect the diff"); + // fetch current and previous version of the feature + FeatureSnapshot snapshot = client.getFeatureSnapshot(identifier); + log.info("Previous flag config: {}, {}", identifier, snapshot.getPrevious()); + log.info("Current flag config: {}, {}", identifier, snapshot.getCurrent()); + } + + + // example method to extract and print all the snapshots. + private static void getAllSnapshots() { + // get all snapshots + List snapshots = client.getAllFeatureSnapshots(); + int counter = 0; + for (FeatureSnapshot snapshot : snapshots) { + log.info("snapshots {} {}", ++counter, snapshot); + } + } + + // example method to extract and print all the snapshots. + private static void getAllSnapshotsWithPrefix() { + // get all snapshots + String prefix = "FFM_"; + List snapshots = client.getAllFeatureSnapshots(prefix); + int counter = 0; + for (FeatureSnapshot snapshot : snapshots) { + log.info("snapshots {} {}", ++counter, snapshot); + } + } +} + + diff --git a/src/main/java/io/harness/cf/client/api/BaseConfig.java b/src/main/java/io/harness/cf/client/api/BaseConfig.java index cde8dee6..7986a6ef 100644 --- a/src/main/java/io/harness/cf/client/api/BaseConfig.java +++ b/src/main/java/io/harness/cf/client/api/BaseConfig.java @@ -42,6 +42,9 @@ public class BaseConfig { /** If metrics service POST call is taking > this time, we need to know about it */ @Builder.Default private final long metricsServiceAcceptableDuration = 10000; + /** store previous and current version of the FeatureConfig */ + @Builder.Default private final boolean enableFeatureSnapshot = false; + /** Get metrics post frequency in seconds */ public int getFrequency() { return Math.max(frequency, Config.MIN_FREQUENCY); diff --git a/src/main/java/io/harness/cf/client/api/CfClient.java b/src/main/java/io/harness/cf/client/api/CfClient.java index 07f580fb..111a5d00 100644 --- a/src/main/java/io/harness/cf/client/api/CfClient.java +++ b/src/main/java/io/harness/cf/client/api/CfClient.java @@ -5,6 +5,8 @@ import io.harness.cf.client.dto.Message; import io.harness.cf.client.dto.Target; import io.harness.cf.client.logger.LogUtil; +import io.harness.cf.model.FeatureSnapshot; +import java.util.List; import java.util.function.Consumer; import lombok.NonNull; @@ -83,6 +85,18 @@ public void on(@NonNull final Event event, @NonNull final Consumer consu client.on(event, consumer); } + public List getAllFeatureSnapshots(String prefix) { + return client.getFeatureSnapshots(prefix); + } + + public List getAllFeatureSnapshots() { + return client.getFeatureSnapshots(); + } + + public FeatureSnapshot getFeatureSnapshot(@NonNull String identifier) { + return client.getFeatureSnapshot(identifier); + } + public void off() { client.off(); } diff --git a/src/main/java/io/harness/cf/client/api/InnerClient.java b/src/main/java/io/harness/cf/client/api/InnerClient.java index 8ad56fbd..55192fdb 100644 --- a/src/main/java/io/harness/cf/client/api/InnerClient.java +++ b/src/main/java/io/harness/cf/client/api/InnerClient.java @@ -91,7 +91,9 @@ protected void setUp(@NonNull final Connector connector, @NonNull final BaseConf this.connector.setOnUnauthorized(this::onUnauthorized); // initialization - repository = new StorageRepository(options.getCache(), options.getStore(), this, false); + repository = + new StorageRepository( + options.getCache(), options.getStore(), this, options.isEnableFeatureSnapshot()); evaluator = new Evaluator(repository, options); authService = new AuthService(this.connector, options.getPollIntervalInSeconds(), this); pollProcessor = @@ -313,12 +315,13 @@ public synchronized void waitForInitialization() } public List getFeatureSnapshots() { - // TODO return null/empty list if snapshot is not enabled in config. return getFeatureSnapshots(""); } public List getFeatureSnapshots(String prefix) { - // TODO return null/empty list if snapshot is not enabled in config. + if (!options.isEnableFeatureSnapshot()){ + log.debug("FeatureSnapshot disabled, snapshot will contain only current version."); + } List identifiers = repository.getAllFeatureIdentifiers(prefix); List snapshots = new LinkedList<>(); @@ -331,7 +334,9 @@ public List getFeatureSnapshots(String prefix) { } public FeatureSnapshot getFeatureSnapshot(@NonNull String identifier) { - // TODO return null/empty list if snapshot is not enabled in config. + if (!options.isEnableFeatureSnapshot()){ + log.debug("FeatureSnapshot disabled, snapshot will contain only current version."); + } Optional ofc = repository.getCurrentAndPreviousFeatureConfig(identifier); FeatureSnapshot result = new FeatureSnapshot(); if (ofc.isPresent()) { From fbd3ce1d632cdd8d446633ba2aff86c0c41a1a6a Mon Sep 17 00:00:00 2001 From: Arek Szostak <50179409+akiraqb@users.noreply.github.com> Date: Wed, 14 Aug 2024 11:57:00 +0100 Subject: [PATCH 5/6] [FFM-1848]: Functional and performance testing (#198) * Updated class name * Attempted to add benchmark test wip * Further twicking * Added example results in comments. * Code and test refactoring. * Added results --- build.gradle | 5 + .../EventExampleWithFeatureSnapshot.java | 2 +- .../io/harness/cf/client/api/InnerClient.java | 19 +- .../java/io/harness/cf/client/api/Query.java | 3 +- .../cf/client/api/StorageRepository.java | 32 ++++ .../cf/client/api/StorageRepositoryTest.java | 63 +++---- .../client/api/StoreRepositoryBenchmark.java | 178 ++++++++++++++++++ .../io/harness/cf/client/api/TestUtils.java | 41 ++++ 8 files changed, 293 insertions(+), 50 deletions(-) create mode 100644 src/test/java/io/harness/cf/client/api/StoreRepositoryBenchmark.java diff --git a/build.gradle b/build.gradle index 8f6296a8..87aa10de 100644 --- a/build.gradle +++ b/build.gradle @@ -18,6 +18,7 @@ plugins { id 'jacoco-report-aggregation' id "com.github.spotbugs" version libs.versions.spotbugs id "org.owasp.dependencycheck" version libs.versions.depcheck + id 'me.champeau.jmh' version '0.6.8' // Added JMH plugin } @@ -55,6 +56,10 @@ dependencies { compileOnly libs.lombok annotationProcessor libs.lombok + + // JMH dependencies + implementation 'org.openjdk.jmh:jmh-core:1.37' + annotationProcessor 'org.openjdk.jmh:jmh-generator-annprocess:1.37' } group = 'io.harness' diff --git a/examples/src/main/java/io/harness/ff/examples/EventExampleWithFeatureSnapshot.java b/examples/src/main/java/io/harness/ff/examples/EventExampleWithFeatureSnapshot.java index 261b1704..dcf25ed3 100644 --- a/examples/src/main/java/io/harness/ff/examples/EventExampleWithFeatureSnapshot.java +++ b/examples/src/main/java/io/harness/ff/examples/EventExampleWithFeatureSnapshot.java @@ -16,7 +16,7 @@ import java.util.concurrent.TimeUnit; @Slf4j -public class EventExamplePoC { +public class EventExampleWithFeatureSnapshot { private static final String SDK_KEY = ""; private static final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); diff --git a/src/main/java/io/harness/cf/client/api/InnerClient.java b/src/main/java/io/harness/cf/client/api/InnerClient.java index 55192fdb..bec1ed88 100644 --- a/src/main/java/io/harness/cf/client/api/InnerClient.java +++ b/src/main/java/io/harness/cf/client/api/InnerClient.java @@ -1,6 +1,5 @@ package io.harness.cf.client.api; -import com.google.gson.Gson; import com.google.gson.JsonObject; import io.harness.cf.client.common.SdkCodes; import io.harness.cf.client.connector.*; @@ -11,7 +10,6 @@ import io.harness.cf.model.Variation; import java.util.LinkedList; import java.util.List; -import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Consumer; @@ -319,7 +317,7 @@ public List getFeatureSnapshots() { } public List getFeatureSnapshots(String prefix) { - if (!options.isEnableFeatureSnapshot()){ + if (!options.isEnableFeatureSnapshot()) { log.debug("FeatureSnapshot disabled, snapshot will contain only current version."); } List identifiers = repository.getAllFeatureIdentifiers(prefix); @@ -334,21 +332,10 @@ public List getFeatureSnapshots(String prefix) { } public FeatureSnapshot getFeatureSnapshot(@NonNull String identifier) { - if (!options.isEnableFeatureSnapshot()){ + if (!options.isEnableFeatureSnapshot()) { log.debug("FeatureSnapshot disabled, snapshot will contain only current version."); } - Optional ofc = repository.getCurrentAndPreviousFeatureConfig(identifier); - FeatureSnapshot result = new FeatureSnapshot(); - if (ofc.isPresent()) { - FeatureConfig[] fc = ofc.get(); - result.setPrevious(fc[0]); - result.setCurrent(fc[1]); - } - // this is here to create a deep copy of the object before its returned. - // this way we protect the cache. - Gson gson = new Gson(); - FeatureSnapshot deepCopySnapshot = gson.fromJson(gson.toJson(result), FeatureSnapshot.class); - return deepCopySnapshot; + return repository.getFeatureSnapshot(identifier); } public void on(@NonNull final Event event, @NonNull final Consumer consumer) { diff --git a/src/main/java/io/harness/cf/client/api/Query.java b/src/main/java/io/harness/cf/client/api/Query.java index ca59d369..b8a09f4d 100644 --- a/src/main/java/io/harness/cf/client/api/Query.java +++ b/src/main/java/io/harness/cf/client/api/Query.java @@ -1,6 +1,7 @@ package io.harness.cf.client.api; import io.harness.cf.model.FeatureConfig; +import io.harness.cf.model.FeatureSnapshot; import io.harness.cf.model.Segment; import java.util.List; import java.util.Optional; @@ -14,7 +15,7 @@ public interface Query { List findFlagsBySegment(@NonNull String identifier); - Optional getCurrentAndPreviousFeatureConfig(@NonNull String identifier); + FeatureSnapshot getFeatureSnapshot(@NonNull String identifier); List getAllFeatureIdentifiers(String prefix); } diff --git a/src/main/java/io/harness/cf/client/api/StorageRepository.java b/src/main/java/io/harness/cf/client/api/StorageRepository.java index cf1a1ece..0b819b1e 100644 --- a/src/main/java/io/harness/cf/client/api/StorageRepository.java +++ b/src/main/java/io/harness/cf/client/api/StorageRepository.java @@ -1,5 +1,6 @@ package io.harness.cf.client.api; +import com.google.gson.Gson; import io.harness.cf.client.common.Cache; import io.harness.cf.client.common.Storage; import io.harness.cf.client.common.Utils; @@ -98,6 +99,37 @@ public Optional getCurrentAndPreviousFeatureConfig(@NonNull Str return Optional.empty(); } + public FeatureSnapshot getFeatureSnapshot(@NonNull String identifier) { + Gson gson = new Gson(); + final String flagKey = formatFlagKey(identifier); + final String pFlagKey = formatPrevFlagKey(identifier); + + FeatureConfig pFlag = (FeatureConfig) cache.get(pFlagKey); + FeatureConfig cFlag = (FeatureConfig) cache.get(flagKey); + + if (cFlag != null) { + FeatureSnapshot deepCopySnapshot = + gson.fromJson(gson.toJson(new FeatureSnapshot(cFlag, pFlag)), FeatureSnapshot.class); + return deepCopySnapshot; + } + // if we don't have it in cache we check the file + if (this.store != null) { + pFlag = (FeatureConfig) store.get(pFlagKey); + cFlag = (FeatureConfig) store.get(flagKey); + if (pFlag != null) { + cache.set(pFlagKey, pFlag); + } + if (cFlag != null) { + cache.set(flagKey, cFlag); + } + + FeatureSnapshot deepCopySnapshot = + gson.fromJson(gson.toJson(new FeatureSnapshot(cFlag, pFlag)), FeatureSnapshot.class); + return deepCopySnapshot; + } + return null; + } + public Optional getSegment(@NonNull String identifier, boolean cacheable) { final String segmentKey = formatSegmentKey(identifier); Segment segment = (Segment) cache.get(segmentKey); diff --git a/src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java b/src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java index 0cbd3ed2..27c77aa3 100644 --- a/src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java +++ b/src/test/java/io/harness/cf/client/api/StorageRepositoryTest.java @@ -4,6 +4,7 @@ import com.google.gson.Gson; import io.harness.cf.model.FeatureConfig; +import io.harness.cf.model.FeatureSnapshot; import java.awt.*; import java.io.File; import java.io.IOException; @@ -12,7 +13,6 @@ import java.nio.file.Paths; import java.util.LinkedList; import java.util.List; -import java.util.Optional; import java.util.stream.Stream; import lombok.NonNull; import org.junit.jupiter.api.Test; @@ -20,9 +20,6 @@ class StorageRepositoryTest { private final Gson gson = new Gson(); - // initialize the config and load the flags. - // load the test - @Test void shouldInitialiseRepo() { final Repository repository = new StorageRepository(new CaffeineCache(10000), null, false); @@ -43,12 +40,10 @@ void shouldStoreCurrentConfig() throws Exception { loadFlags(repository, makeFeatureList(featureConfig)); loadFlags(repository, makeFeatureList(featureConfigUpdated)); - Optional res = - repository.getCurrentAndPreviousFeatureConfig(featureConfigUpdated.getFeature()); - FeatureConfig[] resFc = res.get(); + FeatureSnapshot res = repository.getFeatureSnapshot(featureConfigUpdated.getFeature()); - FeatureConfig previous = resFc[0]; - FeatureConfig current = resFc[1]; + FeatureConfig previous = res.getPrevious(); + FeatureConfig current = res.getCurrent(); // check if previous version is null assertNull(previous); @@ -79,12 +74,10 @@ void shouldStoreCurrentConfigWithFileStore() throws Exception { loadFlags(repository, makeFeatureList(featureConfig)); loadFlags(repository, makeFeatureList(featureConfigUpdated)); - Optional res = - repository.getCurrentAndPreviousFeatureConfig(featureConfigUpdated.getFeature()); - FeatureConfig[] resFc = res.get(); + FeatureSnapshot res = repository.getFeatureSnapshot(featureConfigUpdated.getFeature()); - FeatureConfig previous = resFc[0]; - FeatureConfig current = resFc[1]; + FeatureConfig previous = res.getPrevious(); + FeatureConfig current = res.getCurrent(); // check if previous version is null assertNull(previous); @@ -115,12 +108,10 @@ void shouldStorePreviousAndCurrentConfigWithFileStore() throws Exception { loadFlags(repository, makeFeatureList(featureConfig)); loadFlags(repository, makeFeatureList(featureConfigUpdated)); - Optional res = - repository.getCurrentAndPreviousFeatureConfig(featureConfigUpdated.getFeature()); - FeatureConfig[] resFc = res.get(); + FeatureSnapshot res = repository.getFeatureSnapshot(featureConfigUpdated.getFeature()); - FeatureConfig previous = resFc[0]; - FeatureConfig current = resFc[1]; + FeatureConfig previous = res.getPrevious(); + FeatureConfig current = res.getCurrent(); // check if previous version is null assertNotNull(previous); @@ -145,12 +136,10 @@ void shouldStorePreviousAndCurrentConfig() throws Exception { loadFlags(repository, makeFeatureList(featureConfig)); loadFlags(repository, makeFeatureList(featureConfigUpdated)); - Optional res = - repository.getCurrentAndPreviousFeatureConfig(featureConfigUpdated.getFeature()); - FeatureConfig[] resFc = res.get(); + FeatureSnapshot res = repository.getFeatureSnapshot(featureConfigUpdated.getFeature()); - FeatureConfig previous = resFc[0]; - FeatureConfig current = resFc[1]; + FeatureConfig previous = res.getPrevious(); + FeatureConfig current = res.getCurrent(); // check if previous version is null assertNotNull(previous); @@ -177,12 +166,10 @@ void shouldDeletePreviousAndCurrentConfig() throws Exception { String featureIdentifier = featureConfig.getFeature(); - Optional res = - repository.getCurrentAndPreviousFeatureConfig(featureIdentifier); - FeatureConfig[] resFc = res.get(); + FeatureSnapshot res = repository.getFeatureSnapshot(featureConfigUpdated.getFeature()); - FeatureConfig previous = resFc[0]; - FeatureConfig current = resFc[1]; + FeatureConfig previous = res.getPrevious(); + FeatureConfig current = res.getCurrent(); // check if previous version is null assertNotNull(previous); @@ -194,10 +181,9 @@ void shouldDeletePreviousAndCurrentConfig() throws Exception { // delete config repository.deleteFlag(featureIdentifier); - Optional result = - repository.getCurrentAndPreviousFeatureConfig(featureIdentifier); + FeatureSnapshot result = repository.getFeatureSnapshot(featureConfigUpdated.getFeature()); - assertFalse(result.isPresent(), "The Optional should be empty"); + assertNull(result, "The Optional should be empty"); } @Test @@ -269,4 +255,17 @@ private FeatureConfig GetFeatureConfigFromFile() throws Exception { } return null; } + + private List createBenchmarkData(int flagNumber, int version) throws Exception { + FeatureConfig fg = GetFeatureConfigFromFile(); + List list = new LinkedList(); + for (int i = 1; i <= flagNumber; i++) { + FeatureConfig f = fg; + f.setFeature("simpleBool" + i); + f.setVersion(new Long(version)); + list.add(f); + } + // System.out.println(list); + return list; + } } diff --git a/src/test/java/io/harness/cf/client/api/StoreRepositoryBenchmark.java b/src/test/java/io/harness/cf/client/api/StoreRepositoryBenchmark.java new file mode 100644 index 00000000..f34ccc63 --- /dev/null +++ b/src/test/java/io/harness/cf/client/api/StoreRepositoryBenchmark.java @@ -0,0 +1,178 @@ +package io.harness.cf.client.api; + +import com.google.gson.Gson; +import io.harness.cf.model.FeatureConfig; +import io.harness.cf.model.FeatureSnapshot; +import java.util.LinkedList; +import java.util.List; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.*; + +/* + How to run it. + ./gradlew clean build + ./gradlew jmh + + some results: + Benchmark Mode Cnt Score Error Units + StoreRepositoryBenchmark.BenchmarkGetFeatureSnapshotsCurrentOnly avgt 5 805.511 ± 105.154 ms/op + StoreRepositoryBenchmark.BenchmarkGetFeatureSnapshotsPreviousAndCurrent avgt 5 860.745 ± 61.876 ms/op + StoreRepositoryBenchmark.BenchmarkLoadFeatureConfigCurrentOnly avgt 5 1.502 ± 0.198 ms/op + StoreRepositoryBenchmark.BenchmarkLoadFeatureConfigPreviousAndCurrent avgt 5 2.071 ± 0.650 ms/op + +*/ + +@State(Scope.Thread) +public class StoreRepositoryBenchmark { + + private Repository setCurrentOnlyRepository; + private Repository setCurrentAndPreviousRepository; + + private Repository getSnapshotCurrentOnlyRepository; + private Repository getSnapshotCurrentAndPreviousRepository; + private List featureConfigs; + private List updatedFeatureConfigs; + private final Gson gson = new Gson(); + private final int FeatureConfigSize = 10000; + private final int CacheSize = FeatureConfigSize * 2; + + @Setup + public void setup() throws Exception { + TestUtils tu = new TestUtils(); + featureConfigs = tu.CreateBenchmarkData(FeatureConfigSize, 1); + updatedFeatureConfigs = tu.CreateBenchmarkData(FeatureConfigSize, 2); + + setupRepoWithCurrentOnlyRepository(); + setupRepoWithCurrentAndPreviousRepository(); + setupRepoWithCurrentOnlyRepositoryForGetSnapshot(); + setupRepoWithCurrentAndPreviousRepositoryForGetSnapshot(); + } + + @Setup + public void setupRepoWithCurrentOnlyRepository() throws Exception { + + TestUtils tu = new TestUtils(); + setCurrentOnlyRepository = new StorageRepository(new CaffeineCache(CacheSize), null, false); + // Initial loading of feature configurations + loadFlags(setCurrentOnlyRepository, featureConfigs); + } + + @Setup + public void setupRepoWithCurrentAndPreviousRepository() throws Exception { + + TestUtils tu = new TestUtils(); + setCurrentAndPreviousRepository = + new StorageRepository(new CaffeineCache(CacheSize), null, true); + // Initial loading of feature configurations + loadFlags(setCurrentAndPreviousRepository, featureConfigs); + } + + @Setup + public void setupRepoWithCurrentOnlyRepositoryForGetSnapshot() throws Exception { + + TestUtils tu = new TestUtils(); + getSnapshotCurrentOnlyRepository = + new StorageRepository(new CaffeineCache(CacheSize), null, false); + // Initial loading of feature configurations + loadFlags(getSnapshotCurrentOnlyRepository, featureConfigs); + loadFlags(getSnapshotCurrentOnlyRepository, updatedFeatureConfigs); + } + + @Setup + public void setupRepoWithCurrentAndPreviousRepositoryForGetSnapshot() throws Exception { + TestUtils tu = new TestUtils(); + getSnapshotCurrentAndPreviousRepository = + new StorageRepository(new CaffeineCache(CacheSize), null, true); + // Initial loading of feature configurations + loadFlags(getSnapshotCurrentAndPreviousRepository, featureConfigs); + loadFlags(getSnapshotCurrentAndPreviousRepository, updatedFeatureConfigs); + } + + @Fork(value = 1, warmups = 1) + @OutputTimeUnit(TimeUnit.MILLISECONDS) + @Benchmark + @BenchmarkMode(Mode.AverageTime) + public void BenchmarkLoadFeatureConfigCurrentOnly() { + // Measure average time taken to load the updated feature configurations while storing only + // current snapshot + loadFlags(setCurrentOnlyRepository, updatedFeatureConfigs); + } + + @Fork(value = 1, warmups = 1) + @OutputTimeUnit(TimeUnit.MILLISECONDS) + @Benchmark + @BenchmarkMode(Mode.AverageTime) + public void BenchmarkLoadFeatureConfigPreviousAndCurrent() { + // Measure average time taken to load the updated feature configurations while storing current + // config as well as keeping previous one. + loadFlags(setCurrentAndPreviousRepository, updatedFeatureConfigs); + } + + @Fork(value = 1, warmups = 1) + @OutputTimeUnit(TimeUnit.MILLISECONDS) + @Benchmark + @BenchmarkMode(Mode.AverageTime) + public void BenchmarkGetFeatureSnapshotsCurrentOnly() { + // Measures time taken to get snapshots + List snapshots = getFeatureSnapshots(getSnapshotCurrentOnlyRepository); + if (snapshots == null) { + throw new IllegalStateException("Snapshots are null"); + } + if (snapshots.size() != FeatureConfigSize) { + throw new IllegalStateException("Snapshots not equal"); + } + + for (int i = 0; i < FeatureConfigSize; i++) { + FeatureSnapshot fs = snapshots.get(i); + if (fs.getPrevious() != null) { + throw new IllegalStateException("Snapshots contains previous"); + } + if (fs.getCurrent() == null) { + throw new IllegalStateException("Snapshots does not contain current"); + } + } + } + + @Fork(value = 1, warmups = 1) + @OutputTimeUnit(TimeUnit.MILLISECONDS) + @Benchmark + @BenchmarkMode(Mode.AverageTime) + public void BenchmarkGetFeatureSnapshotsPreviousAndCurrent() { + // Measures time taken to get snapshots + List snapshots = getFeatureSnapshots(getSnapshotCurrentAndPreviousRepository); + if (snapshots == null) { + throw new IllegalStateException("Snapshots are null"); + } + if (snapshots.size() != FeatureConfigSize) { + throw new IllegalStateException("Snapshots not equal"); + } + + for (int i = 0; i < FeatureConfigSize; i++) { + FeatureSnapshot fs = snapshots.get(i); + if (fs.getPrevious() == null) { + throw new IllegalStateException("Snapshots does not contain previous"); + } + if (fs.getCurrent() == null) { + throw new IllegalStateException("Snapshots does not contain current"); + } + } + } + + private List getFeatureSnapshots(Repository repository) { + List identifiers = repository.getAllFeatureIdentifiers(""); + List snapshots = new LinkedList<>(); + for (String identifier : identifiers) { + FeatureSnapshot snapshot = repository.getFeatureSnapshot(identifier); + snapshots.add(snapshot); + } + return snapshots; + } + + private void loadFlags(Repository repository, List flags) { + if (flags != null) { + for (FeatureConfig nextFlag : flags) { + repository.setFlag(nextFlag.getFeature(), nextFlag); + } + } + } +} diff --git a/src/test/java/io/harness/cf/client/api/TestUtils.java b/src/test/java/io/harness/cf/client/api/TestUtils.java index 98894632..8f406e8c 100644 --- a/src/test/java/io/harness/cf/client/api/TestUtils.java +++ b/src/test/java/io/harness/cf/client/api/TestUtils.java @@ -1,10 +1,15 @@ package io.harness.cf.client.api; +import com.google.gson.Gson; +import io.harness.cf.model.FeatureConfig; +import java.io.File; import java.io.IOException; import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.LinkedList; +import java.util.List; public class TestUtils { @@ -28,4 +33,40 @@ public static String getJsonResource(String location) throws IOException, URISyn final Path path = Paths.get(EvaluatorTest.class.getClassLoader().getResource(location).toURI()); return new String(Files.readAllBytes(path)); } + + private final Gson gson = new Gson(); + + public List CreateBenchmarkData(int size, int version) throws Exception { + FeatureConfig fg = GetFeatureConfigFromFile(); + List list = new LinkedList(); + for (int i = 1; i <= size; i++) { + FeatureConfig f = fg; + f.setFeature("simpleBool" + i); + f.setVersion(new Long(version)); + // we are copying objects + FeatureConfig df = gson.fromJson(gson.toJson(f), FeatureConfig.class); + list.add(df); + } + // System.out.println(list); + return list; + } + + public FeatureConfig GetFeatureConfigFromFile() throws Exception { + try { + String relativePath = + "./src/test/resources/local-test-cases/basic_bool_string_for_repository.json"; + // Resolve the absolute path + String filePath = new File(relativePath).getCanonicalPath(); + // Read the content of the file into a String + String jsonString = new String(Files.readAllBytes(Paths.get(filePath))); + + final FeatureConfig featureConfig = gson.fromJson(jsonString, FeatureConfig.class); + return featureConfig; + + } catch (IOException e) { + // Handle exceptions like file not found or read errors + e.printStackTrace(); + } + return null; + } } From 58a5816e4901325f95a328db50ef567ba726acee Mon Sep 17 00:00:00 2001 From: Erdi Rowlands Date: Wed, 14 Aug 2024 13:15:20 +0100 Subject: [PATCH 6/6] FFM-11896 Bump minor version --- README.md | 4 ++-- settings.gradle | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index e65b4c06..4c6fcc51 100644 --- a/README.md +++ b/README.md @@ -78,14 +78,14 @@ Add the following Maven dependency in your project's pom.xml file: io.harness ff-java-server-sdk - 1.6.1 + 1.7.0 ``` #### Gradle ``` -implementation 'io.harness:ff-java-server-sdk:1.6.1' +implementation 'io.harness:ff-java-server-sdk:1.7.0' ``` ### Code Sample diff --git a/settings.gradle b/settings.gradle index 091df4c0..80326964 100644 --- a/settings.gradle +++ b/settings.gradle @@ -4,7 +4,7 @@ dependencyResolutionManagement { versionCatalogs { libs { // main sdk version - version('sdk', '1.6.1'); + version('sdk', '1.7.0'); // sdk deps version('okhttp3', '4.12.0')