[7445] PR suggestions

splitio · Sep 1, 2023 · bd15faa · bd15faa
1 parent 6806d91
commit bd15faa
Show file tree

Hide file tree

Showing 15 changed files with 166 additions and 98 deletions.
diff --git a/client/src/main/java/io/split/client/JsonLocalhostSplitChangeFetcher.java b/client/src/main/java/io/split/client/JsonLocalhostSplitChangeFetcher.java
@@ -30,14 +30,13 @@ public JsonLocalhostSplitChangeFetcher(InputStreamProvider inputStreamProvider)
     }
 
     @Override
-    public SplitChange fetch(long since, FetchOptions options) {
+    public SplitChange fetch(long since, FetchOptions options) throws InputStreamProviderException {
         try {
             JsonReader jsonReader = new JsonReader(new BufferedReader(new InputStreamReader(_inputStreamProvider.get(), StandardCharsets.UTF_8)));
             SplitChange splitChange = Json.fromJson(jsonReader, SplitChange.class);
             return processSplitChange(splitChange, since);
         } catch (InputStreamProviderException i) {
-            throw new IllegalStateException(String.format("Problem fetching splitChanges using file named %s: %s",
-                    i.getFileName(), i.getMessage()), i);
+            throw i;
         } catch (Exception e) {
             throw new IllegalStateException("Problem fetching splitChanges: " + e.getMessage(), e);
         }

diff --git a/client/src/main/java/io/split/client/SplitFactoryImpl.java b/client/src/main/java/io/split/client/SplitFactoryImpl.java
@@ -33,7 +33,6 @@
 import io.split.client.utils.FileInputStreamProvider;
 import io.split.client.utils.FileTypeEnum;
 import io.split.client.utils.InputStreamProviderImp;
-import io.split.client.utils.LocalhostPair;
 import io.split.client.utils.SDKMetadata;
 import io.split.engine.SDKReadinessGates;
 import io.split.engine.common.ConsumerSyncManager;
@@ -371,23 +370,7 @@ protected SplitFactoryImpl(SplitClientConfig config) {
                config.getThreadFactory());
 
         // SplitFetcher
-        LocalhostPair pair = getInputStreamProviderAndFileType(config.splitFile(), config.inputStream(), config.fileType());
-        SplitChangeFetcher splitChangeFetcher;
-        if (pair == null) {
-            splitChangeFetcher = new LegacyLocalhostSplitChangeFetcher(config.splitFile());
-            _log.warn("The sdk initialize in localhost mode using Legacy file. The splitFile or inputStream doesn't add it to the config.");
-        } else {
-            switch (pair.getFileTypeEnum()) {
-                case JSON:
-                    splitChangeFetcher = new JsonLocalhostSplitChangeFetcher(pair.getInputStreamProvider());
-                    break;
-                case YAML:
-                    splitChangeFetcher = new YamlLocalhostSplitChangeFetcher(pair.getInputStreamProvider());
-                    break;
-                default:
-                    splitChangeFetcher = new LegacyLocalhostSplitChangeFetcher(config.splitFile());
-            }
-        }
+        SplitChangeFetcher splitChangeFetcher = createSplitChangeFetcher(config);
         SplitParser splitParser = new SplitParser();
 
         _splitFetcher = new SplitFetcherImp(splitChangeFetcher, splitParser, splitCache, _telemetryStorageProducer);
@@ -656,26 +639,31 @@ private UniqueKeysTracker createUniqueKeysTracker(SplitClientConfig config){
         return null;
     }
 
-    LocalhostPair getInputStreamProviderAndFileType(String splitFile, InputStream inputStream,
-                                                                                      FileTypeEnum fileType) {
+    private SplitChangeFetcher createSplitChangeFetcher(SplitClientConfig splitClientConfig) {
+        String splitFile = splitClientConfig.splitFile();
+        InputStream inputStream = splitClientConfig.inputStream();
+        FileTypeEnum fileType = splitClientConfig.fileType();
         if (splitFile != null && inputStream != null) {
             _log.warn("splitFile or inputStreamProvider should have a value, not both");
-            return null;
+            _log.warn("The sdk initialize in localhost mode using Legacy file. The splitFile or inputStream doesn't add it to the config.");
+            return new LegacyLocalhostSplitChangeFetcher(splitFile);
         }
         if (inputStream != null && fileType == null) {
             _log.warn("If inputStreamProvider is not null, then fileType must also have a non-null value");
-            return null;
+            _log.warn("The sdk initialize in localhost mode using Legacy file. The splitFile or inputStream doesn't add it to the config.");
+            return new LegacyLocalhostSplitChangeFetcher(splitFile);
         }
         if (inputStream == null && splitFile == null){
             _log.warn("splitFile or inputStreamProvider should have a value");
-            return null;
+            _log.warn("The sdk initialize in localhost mode using Legacy file. The splitFile or inputStream doesn't add it to the config.");
+            return new LegacyLocalhostSplitChangeFetcher(splitFile);
         }
         if (splitFile != null) {
             try {
                 if (splitFile.toLowerCase().endsWith(".json")) {
-                    return new LocalhostPair(new FileInputStreamProvider(splitFile), FileTypeEnum.JSON);
+                    return new JsonLocalhostSplitChangeFetcher(new FileInputStreamProvider(splitFile));
                 } else if (splitFile.endsWith(".yaml") || splitFile.endsWith(".yml")) {
-                    return new LocalhostPair(new FileInputStreamProvider(splitFile), FileTypeEnum.YAML);
+                    return new YamlLocalhostSplitChangeFetcher(new FileInputStreamProvider(splitFile));
                 }
             } catch (Exception e) {
                 _log.warn(String.format("There was no file named %s found. " +
@@ -686,8 +674,17 @@ LocalhostPair getInputStreamProviderAndFileType(String splitFile, InputStream in
                         splitFile, splitFile), e);
             }
         } else {
-            return new LocalhostPair(new InputStreamProviderImp(inputStream), fileType);
+            switch (fileType) {
+                case JSON:
+                    return new JsonLocalhostSplitChangeFetcher(new InputStreamProviderImp(inputStream));
+                case YAML:
+                    return new YamlLocalhostSplitChangeFetcher(new InputStreamProviderImp(inputStream));
+                default:
+                    _log.warn("The sdk initialize in localhost mode using Legacy file. The splitFile or inputStream doesn't add it to the config.");
+                    return new LegacyLocalhostSplitChangeFetcher(splitFile);
+            }
         }
-        return null;
+        _log.warn("The sdk initialize in localhost mode using Legacy file. The splitFile or inputStream doesn't add it to the config.");
+        return new LegacyLocalhostSplitChangeFetcher(splitFile);
     }
 }
diff --git a/client/src/main/java/io/split/client/YamlLocalhostSplitChangeFetcher.java b/client/src/main/java/io/split/client/YamlLocalhostSplitChangeFetcher.java
@@ -33,7 +33,7 @@ public YamlLocalhostSplitChangeFetcher(InputStreamProvider inputStreamProvider)
     }
 
     @Override
-    public SplitChange fetch(long since, FetchOptions options) {
+    public SplitChange fetch(long since, FetchOptions options) throws InputStreamProviderException {
         try {
             Yaml yaml = new Yaml();
             List<Map<String, Map<String, Object>>> yamlSplits = yaml.load(_inputStreamProvider.get());
@@ -76,8 +76,7 @@ public SplitChange fetch(long since, FetchOptions options) {
             splitChange.since = since;
             return splitChange;
         } catch (InputStreamProviderException i) {
-            throw new IllegalStateException(String.format("Problem fetching splitChanges using file named %s: %s",
-                    i.getFileName(), i.getMessage()), i);
+            throw i;
         } catch (Exception e) {
             throw new IllegalStateException("Problem fetching splitChanges using a yaml file: " + e.getMessage(), e);
         }

diff --git a/client/src/main/java/io/split/client/exceptions/InputStreamProviderException.java b/client/src/main/java/io/split/client/exceptions/InputStreamProviderException.java
@@ -1,14 +1,8 @@
 package io.split.client.exceptions;
 
 public class InputStreamProviderException extends Exception {
-    private final String _fileName;
 
-    public InputStreamProviderException(String fileName, String message) {
+    public InputStreamProviderException(String message) {
         super(message);
-        _fileName = fileName;
-    }
-
-    public String getFileName() {
-        return _fileName;
     }
 }
diff --git a/client/src/main/java/io/split/client/utils/FileInputStreamProvider.java b/client/src/main/java/io/split/client/utils/FileInputStreamProvider.java
@@ -19,7 +19,8 @@ public InputStream get() throws InputStreamProviderException {
         try {
             return new FileInputStream(_fileName);
         } catch (FileNotFoundException f) {
-            throw new InputStreamProviderException(_fileName, f.getMessage());
+            throw new InputStreamProviderException(String.format("Problem fetching splitChanges using file named %s: %s",
+                    _fileName, f.getMessage()));
         }
     }
 }
diff --git a/client/src/main/java/io/split/client/utils/InputStreamProviderImp.java b/client/src/main/java/io/split/client/utils/InputStreamProviderImp.java
@@ -5,7 +5,7 @@
 import java.io.InputStream;
 
 public class InputStreamProviderImp implements InputStreamProvider {
-    private InputStream _inputStream;
+    private final InputStream _inputStream;
 
     public InputStreamProviderImp(InputStream inputStream){
         _inputStream = inputStream;

diff --git a/client/src/main/java/io/split/client/utils/LocalhostPair.java b/client/src/main/java/io/split/client/utils/LocalhostPair.java
diff --git a/client/src/main/java/io/split/engine/experiments/SplitChangeFetcher.java b/client/src/main/java/io/split/engine/experiments/SplitChangeFetcher.java
@@ -1,6 +1,7 @@
 package io.split.engine.experiments;
 
 import io.split.client.dtos.SplitChange;
+import io.split.client.exceptions.InputStreamProviderException;
 import io.split.engine.common.FetchOptions;
 
 /**
@@ -32,5 +33,5 @@ public interface SplitChangeFetcher {
      * @return SegmentChange
      * @throws java.lang.RuntimeException if there was a problem computing split changes
      */
-    SplitChange fetch(long since, FetchOptions options);
+    SplitChange fetch(long since, FetchOptions options) throws InputStreamProviderException;
 }
diff --git a/client/src/main/java/io/split/engine/experiments/SplitFetcherImp.java b/client/src/main/java/io/split/engine/experiments/SplitFetcherImp.java
@@ -1,6 +1,7 @@
 package io.split.engine.experiments;
 
 import io.split.client.dtos.SplitChange;
+import io.split.client.exceptions.InputStreamProviderException;
 import io.split.client.utils.FeatureFlagsToUpdate;
 import io.split.storages.SplitCacheProducer;
 import io.split.telemetry.domain.enums.LastSynchronizationRecordsEnum;
@@ -89,7 +90,7 @@ public void run() {
         this.forceRefresh(new FetchOptions.Builder().cacheControlHeaders(false).build());
     }
 
-    private Set<String> runWithoutExceptionHandling(FetchOptions options) throws InterruptedException {
+    private Set<String> runWithoutExceptionHandling(FetchOptions options) throws InterruptedException, InputStreamProviderException {
         SplitChange change = _splitChangeFetcher.fetch(_splitCacheProducer.getChangeNumber(), options);
         Set<String> segments = new HashSet<>();
 

diff --git a/client/src/test/java/io/split/client/JsonLocalhostSplitChangeFetcherTest.java b/client/src/test/java/io/split/client/JsonLocalhostSplitChangeFetcherTest.java
@@ -4,6 +4,7 @@
 import io.split.client.dtos.Split;
 import io.split.client.dtos.SplitChange;
 import io.split.client.dtos.Status;
+import io.split.client.exceptions.InputStreamProviderException;
 import io.split.client.utils.FileInputStreamProvider;
 import io.split.client.utils.InputStreamProvider;
 import io.split.client.utils.InputStreamProviderImp;
@@ -33,7 +34,7 @@ public class JsonLocalhostSplitChangeFetcherTest {
     private String TEST_4 = "{\"splits\":[{\"trafficTypeName\":\"user\",\"name\":\"SPLIT_1\",\"trafficAllocation\":100,\"trafficAllocationSeed\":-1780071202,\"seed\":-1442762199,\"status\":\"ACTIVE\",\"killed\":false,\"defaultTreatment\":\"off\",\"changeNumber\":1675443537882,\"algo\":2,\"configurations\":{},\"conditions\":[{\"conditionType\":\"ROLLOUT\",\"matcherGroup\":{\"combiner\":\"AND\",\"matchers\":[{\"keySelector\":{\"trafficType\":\"user\",\"attribute\":null},\"matcherType\":\"ALL_KEYS\",\"negate\":false,\"userDefinedSegmentMatcherData\":null,\"whitelistMatcherData\":null,\"unaryNumericMatcherData\":null,\"betweenMatcherData\":null,\"booleanMatcherData\":null,\"dependencyMatcherData\":null,\"stringMatcherData\":null}]},\"partitions\":[{\"treatment\":\"on\",\"size\":0},{\"treatment\":\"off\",\"size\":100}],\"label\":\"default rule\"}]}],\"since\":-1,\"till\":445345}";
     private String TEST_5 = "{\"splits\":[{\"trafficTypeName\":\"user\",\"name\":\"SPLIT_1\",\"trafficAllocation\":100,\"trafficAllocationSeed\":-1780071202,\"seed\":-1442762199,\"status\":\"ACTIVE\",\"killed\":false,\"defaultTreatment\":\"off\",\"changeNumber\":1675443537882,\"algo\":2,\"configurations\":{},\"conditions\":[{\"conditionType\":\"ROLLOUT\",\"matcherGroup\":{\"combiner\":\"AND\",\"matchers\":[{\"keySelector\":{\"trafficType\":\"user\",\"attribute\":null},\"matcherType\":\"ALL_KEYS\",\"negate\":false,\"userDefinedSegmentMatcherData\":null,\"whitelistMatcherData\":null,\"unaryNumericMatcherData\":null,\"betweenMatcherData\":null,\"booleanMatcherData\":null,\"dependencyMatcherData\":null,\"stringMatcherData\":null}]},\"partitions\":[{\"treatment\":\"on\",\"size\":0},{\"treatment\":\"off\",\"size\":100}],\"label\":\"default rule\"}]},{\"trafficTypeName\":\"user\",\"name\":\"SPLIT_2\",\"trafficAllocation\":100,\"trafficAllocationSeed\":-1780071202,\"seed\":-1442762199,\"status\":\"ACTIVE\",\"killed\":false,\"defaultTreatment\":\"off\",\"changeNumber\":1675443537882,\"algo\":2,\"configurations\":{},\"conditions\":[{\"conditionType\":\"ROLLOUT\",\"matcherGroup\":{\"combiner\":\"AND\",\"matchers\":[{\"keySelector\":{\"trafficType\":\"user\",\"attribute\":null},\"matcherType\":\"ALL_KEYS\",\"negate\":false,\"userDefinedSegmentMatcherData\":null,\"whitelistMatcherData\":null,\"unaryNumericMatcherData\":null,\"betweenMatcherData\":null,\"booleanMatcherData\":null,\"dependencyMatcherData\":null,\"stringMatcherData\":null}]},\"partitions\":[{\"treatment\":\"on\",\"size\":0},{\"treatment\":\"off\",\"size\":100}],\"label\":\"default rule\"}]}],\"since\":-1,\"till\":-1}";
     @Test
-    public void testParseSplitChange() throws FileNotFoundException {
+    public void testParseSplitChange() throws FileNotFoundException, InputStreamProviderException {
         InputStream inputStream = new FileInputStream("src/test/resources/split_init.json");
         InputStreamProvider inputStreamProvider = new InputStreamProviderImp(inputStream);
         JsonLocalhostSplitChangeFetcher localhostSplitChangeFetcher = new JsonLocalhostSplitChangeFetcher(inputStreamProvider);
@@ -48,7 +49,7 @@ public void testParseSplitChange() throws FileNotFoundException {
     }
 
     @Test
-    public void testSinceAndTillSanitization() throws FileNotFoundException {
+    public void testSinceAndTillSanitization() throws FileNotFoundException, InputStreamProviderException {
         InputStream inputStream = new FileInputStream("src/test/resources/sanitizer/splitChangeTillSanitization.json");
         InputStreamProvider inputStreamProvider = new InputStreamProviderImp(inputStream);
         JsonLocalhostSplitChangeFetcher localhostSplitChangeFetcher = new JsonLocalhostSplitChangeFetcher(inputStreamProvider);
@@ -61,7 +62,7 @@ public void testSinceAndTillSanitization() throws FileNotFoundException {
     }
 
     @Test
-    public void testSplitChangeWithoutSplits() throws FileNotFoundException {
+    public void testSplitChangeWithoutSplits() throws FileNotFoundException, InputStreamProviderException {
         InputStream inputStream = new FileInputStream("src/test/resources/sanitizer/splitChangeWithoutSplits.json");
         InputStreamProvider inputStreamProvider = new InputStreamProviderImp(inputStream);
         JsonLocalhostSplitChangeFetcher localhostSplitChangeFetcher = new JsonLocalhostSplitChangeFetcher(inputStreamProvider);
@@ -73,7 +74,7 @@ public void testSplitChangeWithoutSplits() throws FileNotFoundException {
     }
 
     @Test
-    public void testSplitChangeSplitsToSanitize() throws FileNotFoundException {
+    public void testSplitChangeSplitsToSanitize() throws FileNotFoundException, InputStreamProviderException {
         InputStream inputStream = new FileInputStream("src/test/resources/sanitizer/splitChangeSplitsToSanitize.json");
         InputStreamProvider inputStreamProvider = new InputStreamProviderImp(inputStream);
         JsonLocalhostSplitChangeFetcher localhostSplitChangeFetcher = new JsonLocalhostSplitChangeFetcher(inputStreamProvider);
@@ -90,7 +91,7 @@ public void testSplitChangeSplitsToSanitize() throws FileNotFoundException {
     }
 
     @Test
-    public void testSplitChangeSplitsToSanitizeMatchersNull() throws FileNotFoundException {
+    public void testSplitChangeSplitsToSanitizeMatchersNull() throws FileNotFoundException, InputStreamProviderException {
         InputStream inputStream = new FileInputStream("src/test/resources/sanitizer/splitChangerMatchersNull.json");
         InputStreamProvider inputStreamProvider = new InputStreamProviderImp(inputStream);
         JsonLocalhostSplitChangeFetcher localhostSplitChangeFetcher = new JsonLocalhostSplitChangeFetcher(inputStreamProvider);
@@ -107,7 +108,7 @@ public void testSplitChangeSplitsToSanitizeMatchersNull() throws FileNotFoundExc
     }
 
     @Test
-    public void testSplitChangeSplitsDifferentScenarios() throws IOException {
+    public void testSplitChangeSplitsDifferentScenarios() throws IOException, InputStreamProviderException {
 
         File file = folder.newFile("test_0.json");
 
@@ -170,8 +171,8 @@ public void testSplitChangeSplitsDifferentScenarios() throws IOException {
         Assert.assertEquals(2323, splitChange.since);
     }
 
-    @Test(expected = IllegalStateException.class)
-    public void processTestForException() {
+    @Test(expected = InputStreamProviderException.class)
+    public void processTestForException() throws InputStreamProviderException {
         InputStreamProvider inputStreamProvider = new FileInputStreamProvider("src/test/resources/notExist.json");
         JsonLocalhostSplitChangeFetcher localhostSplitChangeFetcher = new JsonLocalhostSplitChangeFetcher(inputStreamProvider);
         FetchOptions fetchOptions = Mockito.mock(FetchOptions.class);