From a896f6d8658d00f302a4adc85611f95f5388560e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Thea?= Date: Wed, 23 Aug 2023 14:03:03 -0300 Subject: [PATCH] Modify flags DTO & manager object (#522) --- .github/workflows/deploy.yml | 1 + build.gradle | 4 +- .../android/client/SplitManagerImpl.java | 1 + .../split/android/client/api/SplitView.java | 1 + .../io/split/android/client/dtos/Split.java | 33 +++++ .../engine/experiments/ParsedSplit.java | 138 +++++++++--------- .../engine/experiments/SplitParser.java | 15 +- .../android/client/SplitManagerImplTest.java | 23 ++- .../engine/experiments/SplitParserTest.java | 2 + .../io/split/android/helpers/SplitHelper.java | 31 +++- 10 files changed, 174 insertions(+), 75 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index fa1639912..9ffc78efc 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -4,6 +4,7 @@ on: push: branches: - development + - SDKS-7440 jobs: build-app: diff --git a/build.gradle b/build.gradle index aab058bc6..a4a2c5adc 100644 --- a/build.gradle +++ b/build.gradle @@ -16,7 +16,7 @@ apply plugin: 'signing' apply plugin: 'kotlin-android' ext { - splitVersion = '3.3.1-alpha-1' + splitVersion = '3.3.1-alpha-2' } android { @@ -225,6 +225,7 @@ afterEvaluate { release(MavenPublication) { from components.release + artifactId = 'android-client' groupId = 'io.split.client' version = splitVersion artifact sourcesJar @@ -241,6 +242,7 @@ afterEvaluate { development(MavenPublication) { from components.release + artifactId = 'android-client' groupId = 'io.split.client' version = splitVersion artifact sourcesJar diff --git a/src/main/java/io/split/android/client/SplitManagerImpl.java b/src/main/java/io/split/android/client/SplitManagerImpl.java index 533142730..b26ab60bd 100644 --- a/src/main/java/io/split/android/client/SplitManagerImpl.java +++ b/src/main/java/io/split/android/client/SplitManagerImpl.java @@ -142,6 +142,7 @@ private SplitView toSplitView(ParsedSplit parsedSplit) { splitView.killed = parsedSplit.killed(); splitView.changeNumber = parsedSplit.changeNumber(); splitView.configs = parsedSplit.configurations(); + splitView.sets = new ArrayList<>(parsedSplit.sets() == null ? new HashSet<>() : parsedSplit.sets()); Set treatments = new HashSet<>(); for (ParsedCondition condition : parsedSplit.parsedConditions()) { diff --git a/src/main/java/io/split/android/client/api/SplitView.java b/src/main/java/io/split/android/client/api/SplitView.java index dd13b3463..abaa0e91b 100644 --- a/src/main/java/io/split/android/client/api/SplitView.java +++ b/src/main/java/io/split/android/client/api/SplitView.java @@ -16,4 +16,5 @@ public class SplitView { public List treatments; public long changeNumber; public Map configs; + public List sets; } diff --git a/src/main/java/io/split/android/client/dtos/Split.java b/src/main/java/io/split/android/client/dtos/Split.java index d74669262..395a4c3a3 100644 --- a/src/main/java/io/split/android/client/dtos/Split.java +++ b/src/main/java/io/split/android/client/dtos/Split.java @@ -1,19 +1,52 @@ package io.split.android.client.dtos; +import androidx.annotation.Nullable; + +import com.google.gson.annotations.SerializedName; + import java.util.List; import java.util.Map; +import java.util.Set; public class Split { + + @SerializedName("name") public String name; + + @SerializedName("seed") public int seed; + + @SerializedName("status") public Status status; + + @SerializedName("killed") public boolean killed; + + @SerializedName("defaultTreatment") public String defaultTreatment; + + @SerializedName("conditions") public List conditions; + + @SerializedName("trafficTypeName") public String trafficTypeName; + + @SerializedName("changeNumber") public long changeNumber; + + @SerializedName("trafficAllocation") public Integer trafficAllocation; + + @SerializedName("trafficAllocationSeed") public Integer trafficAllocationSeed; + + @SerializedName("algo") public int algo; + + @SerializedName("configurations") public Map configurations; + + @Nullable + @SerializedName("sets") + public Set sets; } diff --git a/src/main/java/io/split/android/engine/experiments/ParsedSplit.java b/src/main/java/io/split/android/engine/experiments/ParsedSplit.java index db800870a..1661d3cdf 100644 --- a/src/main/java/io/split/android/engine/experiments/ParsedSplit.java +++ b/src/main/java/io/split/android/engine/experiments/ParsedSplit.java @@ -1,30 +1,28 @@ package io.split.android.engine.experiments; +import androidx.annotation.NonNull; + import com.google.common.collect.ImmutableList; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Set; -/** - * a value class representing an io.codigo.dtos.Experiment. Why are we not using - * that class? Because it does not have the logic of matching. ParsedExperiment - * has the matchers that also encapsulate the logic of matching. We - * can easily cache this object. - */ -@SuppressWarnings("RedundantCast") public class ParsedSplit { - private final String _split; - private final int _seed; - private final boolean _killed; - private final String _defaultTreatment; - private final ImmutableList _parsedCondition; - private final String _trafficTypeName; - private final long _changeNumber; - private final int _trafficAllocation; - private final int _trafficAllocationSeed; - private final int _algo; - private final Map _configurations; + private final String mSplit; + private final int mSeed; + private final boolean mKilled; + private final String mDefaultTreatment; + private final ImmutableList mParsedCondition; + private final String mTrafficTypeName; + private final long mChangeNumber; + private final int mTrafficAllocation; + private final int mTrafficAllocationSeed; + private final int mAlgo; + private final Map mConfigurations; + private final Set mSets; public ParsedSplit( String feature, @@ -37,81 +35,87 @@ public ParsedSplit( int trafficAllocation, int trafficAllocationSeed, int algo, - Map configurations + Map configurations, + Set sets ) { - _split = feature; - _seed = seed; - _killed = killed; - _defaultTreatment = defaultTreatment; - _parsedCondition = ImmutableList.copyOf(matcherAndSplits); - _trafficTypeName = trafficTypeName; - _changeNumber = changeNumber; - _algo = algo; - _configurations = configurations; - - if (_defaultTreatment == null) { + mSplit = feature; + mSeed = seed; + mKilled = killed; + mDefaultTreatment = defaultTreatment; + mParsedCondition = ImmutableList.copyOf(matcherAndSplits); + mTrafficTypeName = trafficTypeName; + mChangeNumber = changeNumber; + mAlgo = algo; + mConfigurations = configurations; + + if (mDefaultTreatment == null) { throw new IllegalArgumentException("DefaultTreatment is null"); } - this._trafficAllocation = trafficAllocation; - this._trafficAllocationSeed = trafficAllocationSeed; + mTrafficAllocation = trafficAllocation; + mTrafficAllocationSeed = trafficAllocationSeed; + mSets = sets; } - public String feature() { - return _split; + return mSplit; } public int trafficAllocation() { - return _trafficAllocation; + return mTrafficAllocation; } public int trafficAllocationSeed() { - return _trafficAllocationSeed; + return mTrafficAllocationSeed; } public int seed() { - return _seed; + return mSeed; } public boolean killed() { - return _killed; + return mKilled; } public String defaultTreatment() { - return _defaultTreatment; + return mDefaultTreatment; } public List parsedConditions() { - return _parsedCondition; + return mParsedCondition; } public String trafficTypeName() { - return _trafficTypeName; + return mTrafficTypeName; } public long changeNumber() { - return _changeNumber; + return mChangeNumber; } public int algo() { - return _algo; + return mAlgo; } public Map configurations() { - return _configurations; + return mConfigurations; + } + + public Set sets() { + return mSets; } @Override public int hashCode() { int result = 17; - result = 31 * result + _split.hashCode(); - result = 31 * result + (int) (_seed ^ (_seed >>> 32)); - result = 31 * result + (_killed ? 1 : 0); - result = 31 * result + _defaultTreatment.hashCode(); - result = 31 * result + _parsedCondition.hashCode(); - result = 31 * result + (_trafficTypeName == null ? 0 : _trafficTypeName.hashCode()); - result = 31 * result + (int) (_changeNumber ^ (_changeNumber >>> 32)); - result = 31 * result + (_algo ^ (_algo >>> 32)); + result = 31 * result + mSplit.hashCode(); + result = 31 * result + (int) (mSeed ^ (mSeed >>> 32)); + result = 31 * result + (mKilled ? 1 : 0); + result = 31 * result + mDefaultTreatment.hashCode(); + result = 31 * result + mParsedCondition.hashCode(); + result = 31 * result + (mTrafficTypeName == null ? 0 : mTrafficTypeName.hashCode()); + result = 31 * result + (int) (mChangeNumber ^ (mChangeNumber >>> 32)); + result = 31 * result + (mAlgo ^ (mAlgo >>> 32)); + result = 31 * result + ((mSets != null) ? mSets.hashCode() : 0); return result; } @@ -122,25 +126,27 @@ public boolean equals(Object obj) { if (!(obj instanceof ParsedSplit)) return false; ParsedSplit other = (ParsedSplit) obj; - return _split.equals(other._split) - && _seed == other._seed - && _killed == other._killed - && _defaultTreatment.equals(other._defaultTreatment) - && _parsedCondition.equals(other._parsedCondition) - && (_trafficTypeName == null ? other._trafficTypeName == null : _trafficTypeName.equals(other._trafficTypeName)) - && _changeNumber == other._changeNumber - && _algo == other._algo - && (_configurations == null ? other._configurations == null : _configurations.equals(other._configurations)); + return mSplit.equals(other.mSplit) + && mSeed == other.mSeed + && mKilled == other.mKilled + && mDefaultTreatment.equals(other.mDefaultTreatment) + && mParsedCondition.equals(other.mParsedCondition) + && (Objects.equals(mTrafficTypeName, other.mTrafficTypeName)) + && mChangeNumber == other.mChangeNumber + && mAlgo == other.mAlgo + && (Objects.equals(mConfigurations, other.mConfigurations)) + && (Objects.equals(mSets, other.mSets)); } + @NonNull @Override public String toString() { - return "name:" + _split + ", seed:" + _seed + ", killed:" + _killed + - ", default treatment:" + _defaultTreatment + - ", parsedConditions:" + _parsedCondition + - ", trafficTypeName:" + _trafficTypeName + ", changeNumber:" + _changeNumber + - ", algo:" + _algo + ", config:" + _configurations; + return "name:" + mSplit + ", seed:" + mSeed + ", killed:" + mKilled + + ", default treatment:" + mDefaultTreatment + + ", parsedConditions:" + mParsedCondition + + ", trafficTypeName:" + mTrafficTypeName + ", changeNumber:" + mChangeNumber + + ", algo:" + mAlgo + ", config:" + mConfigurations + ", sets:" + mSets; } } diff --git a/src/main/java/io/split/android/engine/experiments/SplitParser.java b/src/main/java/io/split/android/engine/experiments/SplitParser.java index f07087e0f..274afbe0d 100644 --- a/src/main/java/io/split/android/engine/experiments/SplitParser.java +++ b/src/main/java/io/split/android/engine/experiments/SplitParser.java @@ -62,7 +62,7 @@ public ParsedSplit parse(@Nullable Split split, @Nullable String matchingKey) { try { return parseWithoutExceptionHandling(split, matchingKey); } catch (Throwable t) { - Logger.e(t, "Could not parse feature flag: %s", split); + Logger.e(t, "Could not parse feature flag: %s", (split != null) ? split.name : "null"); return null; } } @@ -90,7 +90,18 @@ private ParsedSplit parseWithoutExceptionHandling(Split split, String matchingKe parsedConditionList.add(new ParsedCondition(condition.conditionType, matcher, partitions, condition.label)); } - return new ParsedSplit(split.name, split.seed, split.killed, split.defaultTreatment, parsedConditionList, split.trafficTypeName, split.changeNumber, split.trafficAllocation, split.trafficAllocationSeed, split.algo, split.configurations); + return new ParsedSplit(split.name, + split.seed, + split.killed, + split.defaultTreatment, + parsedConditionList, + split.trafficTypeName, + split.changeNumber, + split.trafficAllocation, + split.trafficAllocationSeed, + split.algo, + split.configurations, + split.sets); } private CombiningMatcher toMatcher(MatcherGroup matcherGroup, String matchingKey) { diff --git a/src/test/java/io/split/android/client/SplitManagerImplTest.java b/src/test/java/io/split/android/client/SplitManagerImplTest.java index 06dee96fa..2097ba00f 100644 --- a/src/test/java/io/split/android/client/SplitManagerImplTest.java +++ b/src/test/java/io/split/android/client/SplitManagerImplTest.java @@ -23,6 +23,7 @@ import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -32,6 +33,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.IsNull.notNullValue; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.when; @@ -65,7 +67,6 @@ public void splitCallWithNonExistentSplit() { @Test public void splitCallWithExistentSplit() { String existent = "existent"; - SplitFetcher splitFetcher = Mockito.mock(SplitFetcher.class); Map configs = new HashMap<>(); configs.put("off", "{\"f\":\"v\"}"); @@ -101,7 +102,6 @@ public void splitsCallWithNoSplit() { @Test public void splitsCallWithSplit() { - SplitFetcher splitFetcher = Mockito.mock(SplitFetcher.class); Map splitsMap = new HashMap<>(); Split split = SplitHelper.createSplit("FeatureName", 123, true, "off", Lists.newArrayList(getTestCondition()), "traffic", 456L, 1, null); splitsMap.put(split.name, split); @@ -128,7 +128,6 @@ public void splitNamesCallWithNoSplit() { @Test public void splitNamesCallWithSplit() { - SplitFetcher splitFetcher = Mockito.mock(SplitFetcher.class); Map splitsMap = new HashMap<>(); Split split = SplitHelper.createSplit("FeatureName", 123, true, "off", Lists.newArrayList(getTestCondition()), @@ -142,8 +141,24 @@ public void splitNamesCallWithSplit() { assertThat(splitNames.get(0), is(equalTo(split.name))); } + @Test + public void flagSets() { + Map splitsMap = new HashMap<>(); + Split split = SplitHelper.createSplit("FeatureName", 123, true, + "off", Lists.newArrayList(getTestCondition()), + "traffic", 456L, 1, null); + splitsMap.put(split.name, split); + when(mSplitsStorage.getAll()).thenReturn(splitsMap); + + SplitManager splitManager = mSplitManager; + + List splitNames = splitManager.splits(); + assertEquals(1, splitNames.size()); + assertEquals(split.name, splitNames.get(0).name); + assertEquals(new ArrayList<>(split.sets), splitNames.get(0).sets); + } + private Condition getTestCondition() { return SplitHelper.createCondition(CombiningMatcher.of(new AllKeysMatcher()), Lists.newArrayList(ConditionsTestUtil.partition("off", 10))); } - } 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 eb4f90da2..18fdf9fe8 100644 --- a/src/test/java/io/split/android/engine/experiments/SplitParserTest.java +++ b/src/test/java/io/split/android/engine/experiments/SplitParserTest.java @@ -16,6 +16,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -351,6 +352,7 @@ private Split makeSplit(String name, List conditions, long changeNumb split.changeNumber = changeNumber; split.algo = 1; split.configurations = configurations; + split.sets = Collections.emptySet(); return split; } diff --git a/src/test/java/io/split/android/helpers/SplitHelper.java b/src/test/java/io/split/android/helpers/SplitHelper.java index f7baf468d..0aec872e8 100644 --- a/src/test/java/io/split/android/helpers/SplitHelper.java +++ b/src/test/java/io/split/android/helpers/SplitHelper.java @@ -1,9 +1,11 @@ package io.split.android.helpers; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import io.split.android.client.dtos.Condition; import io.split.android.client.dtos.ConditionType; @@ -42,6 +44,30 @@ public static Split createSplit( long changeNumber, int algo, Map configurations + ) { + return createSplit(feature, + seed, + killed, + defaultTreatment, + conditions, + trafficTypeName, + changeNumber, + algo, + configurations, + Collections.emptySet()); + } + + public static Split createSplit( + String feature, + int seed, + boolean killed, + String defaultTreatment, + List conditions, + String trafficTypeName, + long changeNumber, + int algo, + Map configurations, + Set sets ) { Split split = new Split(); split.name = feature; @@ -52,11 +78,11 @@ public static Split createSplit( split.trafficTypeName = trafficTypeName; split.changeNumber = changeNumber; split.trafficAllocation = 100; - split.seed = seed; split.trafficAllocationSeed = seed; split.algo = algo; split.status = Status.ACTIVE; split.configurations = configurations; + split.sets = sets; return split; } @@ -95,7 +121,8 @@ public static ParsedSplit createParsedSplit( 100, seed, algo, - configurations + configurations, + Collections.emptySet() ); }