From 09242e1272e91b114740defb118c1cdd8c1a6245 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 28 Aug 2024 16:33:55 -0300 Subject: [PATCH 1/5] Clean up --- .../streaming/SdkUpdateStreamingTest.java | 1 - .../client/dtos/MyLargeSegmentsResponse.java | 29 ----------- .../split/android/client/dtos/MySegment.java | 31 ------------ .../client/dtos/MySegmentsResponse.java | 48 ------------------- .../mysegments/MySegmentsResponseParser.java | 22 --------- .../mysegments/MySegmentsSyncTask.java | 28 ++++------- .../engine/experiments/SplitParserTest.java | 1 - 7 files changed, 10 insertions(+), 150 deletions(-) delete mode 100644 src/main/java/io/split/android/client/dtos/MyLargeSegmentsResponse.java delete mode 100644 src/main/java/io/split/android/client/dtos/MySegment.java delete mode 100644 src/main/java/io/split/android/client/dtos/MySegmentsResponse.java delete mode 100644 src/main/java/io/split/android/client/service/mysegments/MySegmentsResponseParser.java diff --git a/src/androidTest/java/tests/integration/streaming/SdkUpdateStreamingTest.java b/src/androidTest/java/tests/integration/streaming/SdkUpdateStreamingTest.java index 2ca6e72c6..66ada2654 100644 --- a/src/androidTest/java/tests/integration/streaming/SdkUpdateStreamingTest.java +++ b/src/androidTest/java/tests/integration/streaming/SdkUpdateStreamingTest.java @@ -32,7 +32,6 @@ import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitFactory; import io.split.android.client.api.Key; -import io.split.android.client.dtos.MySegment; import io.split.android.client.dtos.Partition; import io.split.android.client.dtos.Segment; import io.split.android.client.dtos.Split; diff --git a/src/main/java/io/split/android/client/dtos/MyLargeSegmentsResponse.java b/src/main/java/io/split/android/client/dtos/MyLargeSegmentsResponse.java deleted file mode 100644 index 06522ca06..000000000 --- a/src/main/java/io/split/android/client/dtos/MyLargeSegmentsResponse.java +++ /dev/null @@ -1,29 +0,0 @@ -package io.split.android.client.dtos; - -import androidx.annotation.NonNull; - -import com.google.gson.annotations.SerializedName; - -import java.util.Collections; -import java.util.List; - -public class MyLargeSegmentsResponse implements SegmentResponse { - - @SerializedName("myLargeSegments") - private List myLargeSegments; - - @SerializedName("till") - private Long till; - - @Override - @NonNull - public List getSegments() { - return myLargeSegments == null ? Collections.emptyList() : - myLargeSegments; - } - - @Override - public long getTill() { - return till == null ? -1 : till; - } -} diff --git a/src/main/java/io/split/android/client/dtos/MySegment.java b/src/main/java/io/split/android/client/dtos/MySegment.java deleted file mode 100644 index 16067c4de..000000000 --- a/src/main/java/io/split/android/client/dtos/MySegment.java +++ /dev/null @@ -1,31 +0,0 @@ -package io.split.android.client.dtos; - -import com.google.gson.annotations.SerializedName; - -public class MySegment { - @SerializedName("id") - public String id; - @SerializedName("name") - public String name; - - public static MySegment create(String name) { - MySegment mySegment = new MySegment(); - mySegment.name = name; - return mySegment; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - MySegment mySegment = (MySegment) o; - - return name.equals(mySegment.name); - } - - @Override - public int hashCode() { - return name.hashCode(); - } -} diff --git a/src/main/java/io/split/android/client/dtos/MySegmentsResponse.java b/src/main/java/io/split/android/client/dtos/MySegmentsResponse.java deleted file mode 100644 index b6b1f4a49..000000000 --- a/src/main/java/io/split/android/client/dtos/MySegmentsResponse.java +++ /dev/null @@ -1,48 +0,0 @@ -package io.split.android.client.dtos; - -import androidx.annotation.NonNull; -import androidx.annotation.VisibleForTesting; - -import com.google.gson.annotations.SerializedName; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -public class MySegmentsResponse implements SegmentResponse { - - @SerializedName("mySegments") - private List mySegments; - - @SerializedName("till") - private Long till; - - @NonNull - @Override - public List getSegments() { - return mySegments == null ? Collections.emptyList() : - mapToNames(mySegments); - } - - @Override - public long getTill() { - return till == null ? -1 : till; - } - - @VisibleForTesting - public static MySegmentsResponse create(List mySegments, long till) { - MySegmentsResponse mySegmentsResponse = new MySegmentsResponse(); - mySegmentsResponse.mySegments = mySegments; - mySegmentsResponse.till = till; - return mySegmentsResponse; - } - - private static List mapToNames(List mySegments) { - List names = new ArrayList<>(); - for (MySegment segment : mySegments) { - names.add(segment.name); - } - - return names; - } -} diff --git a/src/main/java/io/split/android/client/service/mysegments/MySegmentsResponseParser.java b/src/main/java/io/split/android/client/service/mysegments/MySegmentsResponseParser.java deleted file mode 100644 index 257514989..000000000 --- a/src/main/java/io/split/android/client/service/mysegments/MySegmentsResponseParser.java +++ /dev/null @@ -1,22 +0,0 @@ -package io.split.android.client.service.mysegments; - -import com.google.gson.JsonSyntaxException; - -import io.split.android.client.dtos.MySegmentsResponse; -import io.split.android.client.service.http.HttpResponseParser; -import io.split.android.client.service.http.HttpResponseParserException; -import io.split.android.client.utils.Json; - -public class MySegmentsResponseParser implements HttpResponseParser { - - @Override - public MySegmentsResponse parse(String responseData) throws HttpResponseParserException { - try { - return Json.fromJson(responseData, MySegmentsResponse.class); - } catch (JsonSyntaxException e) { - throw new HttpResponseParserException("Syntax error parsing my segments http response: " + e.getLocalizedMessage()); - } catch (Exception e) { - throw new HttpResponseParserException("Unknown error parsing my segments http response: " + e.getLocalizedMessage()); - } - } -} diff --git a/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java b/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java index 19873596f..42dbec58c 100644 --- a/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java +++ b/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java @@ -175,34 +175,26 @@ private void fetch(int initialRetries) throws HttpFetcherException, InterruptedE private Map getParams(boolean addTill) { Map params = new HashMap<>(); if (addTill) { - long segmentsTarget = Utils.getOrDefault(mTargetSegmentsChangeNumber, -1L); - long largeSegmentsTarget = Utils.getOrDefault(mTargetLargeSegmentsChangeNumber, -1L); - params.put(TILL_PARAM, Math.max(segmentsTarget, largeSegmentsTarget)); + params.put(TILL_PARAM, Math.max( + Utils.getOrDefault(mTargetSegmentsChangeNumber, -1L), + Utils.getOrDefault(mTargetLargeSegmentsChangeNumber, -1L))); } return params; } private boolean isStaleResponse(AllSegmentsChange response) { - boolean checkSegments = Utils.getOrDefault(mTargetSegmentsChangeNumber, -1L) != -1; - boolean checkLargeSegments = Utils.getOrDefault(mTargetLargeSegmentsChangeNumber, -1L) != -1; - - boolean segmentsTargetMatched = !checkSegments || - response.getSegmentsChange() != null && mTargetSegmentsChangeNumber.equals(response.getSegmentsChange().getChangeNumber()); - boolean largeSegmentsTargetMatched = !checkLargeSegments || - response.getLargeSegmentsChange() != null && mTargetLargeSegmentsChangeNumber.equals(response.getLargeSegmentsChange().getChangeNumber()); - - if (!segmentsTargetMatched) { - Logger.v("Segments target change number not matched. Expected: " + mTargetSegmentsChangeNumber + " - Actual: " + response.getSegmentsChange().getChangeNumber()); - } - - if (!largeSegmentsTargetMatched) { - Logger.v("Large segments target change number not matched. Expected: " + mTargetLargeSegmentsChangeNumber + " - Actual: " + response.getLargeSegmentsChange().getChangeNumber()); - } + boolean segmentsTargetMatched = targetMatched(mTargetSegmentsChangeNumber, response.getSegmentsChange()); + boolean largeSegmentsTargetMatched = targetMatched(mTargetLargeSegmentsChangeNumber, response.getLargeSegmentsChange()); return !segmentsTargetMatched || !largeSegmentsTargetMatched; } + private boolean targetMatched(@Nullable Long mTargetSegmentsChangeNumber, SegmentsChange response) { + return Utils.getOrDefault(mTargetSegmentsChangeNumber, -1L) == -1 || + response != null && mTargetSegmentsChangeNumber.equals(response.getChangeNumber()); + } + private void updateStorage(AllSegmentsChange response) { List oldSegments = new ArrayList<>(); List mySegments = new ArrayList<>(); diff --git a/src/test/java/io/split/android/engine/experiments/SplitParserTest.java b/src/test/java/io/split/android/engine/experiments/SplitParserTest.java index b6b2fe26e..34071bdb5 100644 --- a/src/test/java/io/split/android/engine/experiments/SplitParserTest.java +++ b/src/test/java/io/split/android/engine/experiments/SplitParserTest.java @@ -28,7 +28,6 @@ import io.split.android.client.dtos.MatcherCombiner; import io.split.android.client.dtos.MatcherGroup; import io.split.android.client.dtos.MatcherType; -import io.split.android.client.dtos.MySegment; import io.split.android.client.dtos.Partition; import io.split.android.client.dtos.Split; import io.split.android.client.dtos.Status; From a0d74317b890c9f0e056c7adea15c4eedf75666b Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 28 Aug 2024 16:36:12 -0300 Subject: [PATCH 2/5] Path correction --- src/androidTest/java/helper/IntegrationHelper.java | 2 +- .../java/io/split/android/client/network/SdkTargetPath.java | 2 +- .../android/client/service/mysegments/MySegmentsSyncTask.java | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/androidTest/java/helper/IntegrationHelper.java b/src/androidTest/java/helper/IntegrationHelper.java index 7af76d2ca..b2a0b5b34 100644 --- a/src/androidTest/java/helper/IntegrationHelper.java +++ b/src/androidTest/java/helper/IntegrationHelper.java @@ -434,6 +434,6 @@ public interface StreamingResponseClosure { } public static class ServicePath { - public static final String MEMBERSHIPS = "membership"; + public static final String MEMBERSHIPS = "memberships"; } } diff --git a/src/main/java/io/split/android/client/network/SdkTargetPath.java b/src/main/java/io/split/android/client/network/SdkTargetPath.java index b645fef71..ff98fd6b9 100644 --- a/src/main/java/io/split/android/client/network/SdkTargetPath.java +++ b/src/main/java/io/split/android/client/network/SdkTargetPath.java @@ -9,7 +9,7 @@ public class SdkTargetPath { public static final String SPLIT_CHANGES = "/splitChanges"; - public static final String MEMBERSHIPS = "/membership"; + public static final String MEMBERSHIPS = "/memberships"; public static final String EVENTS = "/events/bulk"; public static final String IMPRESSIONS = "/testImpressions/bulk"; public static final String IMPRESSIONS_COUNT = "/testImpressions/count"; diff --git a/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java b/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java index 42dbec58c..50f8633e2 100644 --- a/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java +++ b/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java @@ -155,6 +155,7 @@ private void fetch(int initialRetries) throws HttpFetcherException, InterruptedE } if (isStaleResponse(response)) { + Logger.d("Retrying memberships fetch due to change number mismatch"); long waitMillis = TimeUnit.SECONDS.toMillis(mBackoffCounter.getNextRetryTime()); Thread.sleep(waitMillis); remainingRetries--; From 1f987e414d03185caac5da92bc43a771c19a697e Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 28 Aug 2024 16:56:36 -0300 Subject: [PATCH 3/5] Additional refactor --- .../mysegments/MySegmentsSyncTask.java | 47 +++++++++---------- .../client/network/SdkTargetPathTest.java | 6 +-- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java b/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java index 50f8633e2..866b3a088 100644 --- a/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java +++ b/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java @@ -197,24 +197,21 @@ private boolean targetMatched(@Nullable Long mTargetSegmentsChangeNumber, Segmen } private void updateStorage(AllSegmentsChange response) { + UpdateSegmentsResult segmentsResult = updateSegments(response.getSegmentsChange(), mMySegmentsStorage); + UpdateSegmentsResult largeSegmentsResult = updateSegments(response.getLargeSegmentsChange(), mMyLargeSegmentsStorage); + fireMySegmentsUpdatedIfNeeded(segmentsResult, largeSegmentsResult); + } + + @NonNull + private static UpdateSegmentsResult updateSegments(SegmentsChange segmentsChange, MySegmentsStorage storage) { List oldSegments = new ArrayList<>(); List mySegments = new ArrayList<>(); - SegmentsChange segmentsChange = response.getSegmentsChange(); if (segmentsChange != null) { - oldSegments = new ArrayList<>(mMySegmentsStorage.getAll()); + oldSegments = new ArrayList<>(storage.getAll()); mySegments = segmentsChange.getNames(); - mMySegmentsStorage.set(segmentsChange); + storage.set(segmentsChange); } - - List oldLargeSegments = new ArrayList<>(); - List myLargeSegments = new ArrayList<>(); - SegmentsChange largeSegmentsChange = response.getLargeSegmentsChange(); - if (largeSegmentsChange != null) { - myLargeSegments = largeSegmentsChange.getNames(); - oldLargeSegments = new ArrayList<>(mMyLargeSegmentsStorage.getAll()); - mMyLargeSegmentsStorage.set(largeSegmentsChange); - } - fireMySegmentsUpdatedIfNeeded(oldSegments, mySegments, oldLargeSegments, myLargeSegments); + return new UpdateSegmentsResult(oldSegments, mySegments); } private void logError(String message) { @@ -228,26 +225,18 @@ private void logError(String message) { return null; } - private void fireMySegmentsUpdatedIfNeeded(List oldSegments, List newSegments, List oldLargeSegments, List newLargeSegments) { + private void fireMySegmentsUpdatedIfNeeded(UpdateSegmentsResult segmentsResult, UpdateSegmentsResult largeSegmentsResult) { if (mEventsManager == null) { return; } // MY_SEGMENTS_UPDATED event when segments have changed - boolean segmentsHaveChanged = mMySegmentsChangeChecker.mySegmentsHaveChanged(oldSegments, newSegments); - boolean largeSegmentsHaveChanged = mMySegmentsChangeChecker.mySegmentsHaveChanged(oldLargeSegments, newLargeSegments); - - if (segmentsHaveChanged) { - Logger.v("New segments fetched: " + String.join(", ", newSegments)); - } - if (largeSegmentsHaveChanged) { - Logger.v("New large segments fetched: " + String.join(", ", newLargeSegments)); - } + boolean segmentsHaveChanged = mMySegmentsChangeChecker.mySegmentsHaveChanged(segmentsResult.oldSegments, segmentsResult.newSegments); + boolean largeSegmentsHaveChanged = mMySegmentsChangeChecker.mySegmentsHaveChanged(largeSegmentsResult.oldSegments, largeSegmentsResult.newSegments); if (segmentsHaveChanged) { mEventsManager.notifyInternalEvent(mUpdateEvent); } else { - // MY_LARGE_SEGMENTS_UPDATED event when large segments have changed if (largeSegmentsHaveChanged) { mEventsManager.notifyInternalEvent(SplitInternalEvent.MY_LARGE_SEGMENTS_UPDATED); @@ -257,4 +246,14 @@ private void fireMySegmentsUpdatedIfNeeded(List oldSegments, List oldSegments; + public final List newSegments; + + private UpdateSegmentsResult(List oldSegments, List newSegments) { + this.oldSegments = oldSegments; + this.newSegments = newSegments; + } + } } diff --git a/src/test/java/io/split/android/client/network/SdkTargetPathTest.java b/src/test/java/io/split/android/client/network/SdkTargetPathTest.java index 953c1308d..8f8c34657 100644 --- a/src/test/java/io/split/android/client/network/SdkTargetPathTest.java +++ b/src/test/java/io/split/android/client/network/SdkTargetPathTest.java @@ -13,21 +13,21 @@ public class SdkTargetPathTest { public void userKeyWithSpaces() throws URISyntaxException { URI uri = SdkTargetPath.mySegments("https://split.io", "CABM, CCIB Marketing"); - assertEquals("/membership/CABM,%20CCIB%20Marketing", uri.getRawPath()); + assertEquals(SdkTargetPath.MEMBERSHIPS+"/CABM,%20CCIB%20Marketing", uri.getRawPath()); } @Test public void userKeyWithSlash() throws URISyntaxException { URI uri = SdkTargetPath.mySegments("https://split.io", "user/key"); - assertEquals("/membership/user%2Fkey", uri.getRawPath()); + assertEquals(SdkTargetPath.MEMBERSHIPS+"/user%2Fkey", uri.getRawPath()); } @Test public void userKeyWithSpecialChars() throws URISyntaxException { URI uri = SdkTargetPath.mySegments("https://split.io", "grüneStraße"); - assertEquals("/membership/gr%C3%BCneStra%C3%9Fe", uri.getRawPath()); + assertEquals(SdkTargetPath.MEMBERSHIPS+"/gr%C3%BCneStra%C3%9Fe", uri.getRawPath()); } @Test From 17fbe844525254c1da704595676e09fb0421927a Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 28 Aug 2024 17:02:31 -0300 Subject: [PATCH 4/5] Change param name --- .../client/service/mysegments/MySegmentsSyncTask.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java b/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java index 866b3a088..62c991210 100644 --- a/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java +++ b/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java @@ -191,9 +191,9 @@ private boolean isStaleResponse(AllSegmentsChange response) { return !segmentsTargetMatched || !largeSegmentsTargetMatched; } - private boolean targetMatched(@Nullable Long mTargetSegmentsChangeNumber, SegmentsChange response) { - return Utils.getOrDefault(mTargetSegmentsChangeNumber, -1L) == -1 || - response != null && mTargetSegmentsChangeNumber.equals(response.getChangeNumber()); + private boolean targetMatched(@Nullable Long targetChangeNumber, SegmentsChange response) { + return Utils.getOrDefault(targetChangeNumber, -1L) == -1 || + response != null && targetChangeNumber.equals(response.getChangeNumber()); } private void updateStorage(AllSegmentsChange response) { From a6677615a6f8227db74ac4fb7ef8fa37b7de9bcd Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 29 Aug 2024 09:41:28 -0300 Subject: [PATCH 5/5] Less or equals comparison --- .../client/service/mysegments/MySegmentsSyncTask.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java b/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java index 62c991210..6874f950e 100644 --- a/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java +++ b/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java @@ -184,16 +184,19 @@ private Map getParams(boolean addTill) { return params; } - private boolean isStaleResponse(AllSegmentsChange response) { + private boolean isStaleResponse(@NonNull AllSegmentsChange response) { boolean segmentsTargetMatched = targetMatched(mTargetSegmentsChangeNumber, response.getSegmentsChange()); boolean largeSegmentsTargetMatched = targetMatched(mTargetLargeSegmentsChangeNumber, response.getLargeSegmentsChange()); return !segmentsTargetMatched || !largeSegmentsTargetMatched; } - private boolean targetMatched(@Nullable Long targetChangeNumber, SegmentsChange response) { - return Utils.getOrDefault(targetChangeNumber, -1L) == -1 || - response != null && targetChangeNumber.equals(response.getChangeNumber()); + private boolean targetMatched(@Nullable Long targetChangeNumber, SegmentsChange change) { + Long target = Utils.getOrDefault(targetChangeNumber, -1L); + return target == -1 || + change == null || + change.getChangeNumber() == null || + change.getChangeNumber() != null && target <= change.getChangeNumber(); } private void updateStorage(AllSegmentsChange response) {