From 7c42f25b60ccd53c39412000db84a72390c44f2a Mon Sep 17 00:00:00 2001 From: Arek Szostak Date: Tue, 6 Aug 2024 14:28:29 +0100 Subject: [PATCH 1/3] Extended interface to return current and previous version --- src/main/java/io/harness/cf/client/api/Query.java | 4 ++++ 1 file changed, 4 insertions(+) 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); } From da797dc6d1e49e3dedd5e80d64f018484191d2a4 Mon Sep 17 00:00:00 2001 From: Arek Szostak Date: Tue, 6 Aug 2024 14:29:40 +0100 Subject: [PATCH 2/3] Added extra configuration paramtere to the StorageRepository + added logic to store and retrieve current + previous version. --- .../io/harness/cf/client/api/InnerClient.java | 2 +- .../cf/client/api/StorageRepository.java | 74 +++++++++++++++++-- .../client/api/EvaluatorIntegrationTest.java | 2 +- .../harness/cf/client/api/EvaluatorTest.java | 8 +- 4 files changed, 73 insertions(+), 13 deletions(-) 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/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<>(); From 7b657e06bfb06c0fa48c9ebba52d9f62c0563962 Mon Sep 17 00:00:00 2001 From: Arek Szostak Date: Tue, 6 Aug 2024 14:30:50 +0100 Subject: [PATCH 3/3] Added StorageRepositoryTest class and test data. --- .../cf/client/api/StorageRepositoryTest.java | 199 ++++++++++++++++++ .../io/harness/cf/client/api/TestUtils.java | 4 + .../basic_bool_string_for_repository.json | 28 +++ 3 files changed, 231 insertions(+) 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/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 + } +