diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 908cacdfd..c2ee017b0 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -6,6 +6,10 @@ on: - 'development' - '*_baseline' +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: build-app: name: Build App @@ -15,16 +19,16 @@ jobs: ARTIFACTORY_TOKEN: ${{ secrets.ARTIFACTORY_TOKEN }} steps: - name: checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Gradle cache - uses: gradle/gradle-build-action@v2.4.2 + uses: gradle/gradle-build-action@v3 - - name: Set up JDK 11 + - name: Set up JDK 17 uses: actions/setup-java@v3 with: distribution: 'temurin' - java-version: 11 + java-version: 17 cache: 'gradle' - name: Publish diff --git a/.github/workflows/instrumented.yml b/.github/workflows/instrumented.yml index 1c84e88fc..339636f72 100644 --- a/.github/workflows/instrumented.yml +++ b/.github/workflows/instrumented.yml @@ -5,7 +5,10 @@ on: branches: - 'development' - 'master' - - '*_baseline' + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true jobs: test: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 99bd6aa16..05f003a70 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -4,22 +4,26 @@ on: branches: - '*' +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: build-app: name: Build App runs-on: ubuntu-latest steps: - name: checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Gradle cache uses: gradle/gradle-build-action@v3 - - name: Set up JDK 11 + - name: Set up JDK 17 uses: actions/setup-java@v4 with: distribution: 'temurin' - java-version: 11 + java-version: 17 cache: 'gradle' - name: Test with Gradle diff --git a/src/main/java/io/split/android/client/EventPropertiesProcessor.java b/src/main/java/io/split/android/client/EventPropertiesProcessor.java deleted file mode 100644 index fd7bf359d..000000000 --- a/src/main/java/io/split/android/client/EventPropertiesProcessor.java +++ /dev/null @@ -1,7 +0,0 @@ -package io.split.android.client; - -import java.util.Map; - -public interface EventPropertiesProcessor { - ProcessedEventProperties process(Map properties); -} diff --git a/src/main/java/io/split/android/client/EventsTrackerImpl.java b/src/main/java/io/split/android/client/EventsTrackerImpl.java index 8fd040672..0b8d18982 100644 --- a/src/main/java/io/split/android/client/EventsTrackerImpl.java +++ b/src/main/java/io/split/android/client/EventsTrackerImpl.java @@ -13,6 +13,7 @@ import io.split.android.client.telemetry.storage.TelemetryStorageProducer; import io.split.android.client.utils.logger.Logger; import io.split.android.client.validators.EventValidator; +import io.split.android.client.validators.PropertyValidator; import io.split.android.client.validators.ValidationErrorInfo; import io.split.android.client.validators.ValidationMessageLogger; @@ -23,20 +24,20 @@ public class EventsTrackerImpl implements EventsTracker { private final EventValidator mEventValidator; private final ValidationMessageLogger mValidationLogger; private final TelemetryStorageProducer mTelemetryStorageProducer; - private final EventPropertiesProcessor mEventPropertiesProcessor; + private final PropertyValidator mPropertyValidator; private final SyncManager mSyncManager; private final AtomicBoolean isTrackingEnabled = new AtomicBoolean(true); public EventsTrackerImpl(@NonNull EventValidator eventValidator, @NonNull ValidationMessageLogger validationLogger, @NonNull TelemetryStorageProducer telemetryStorageProducer, - @NonNull EventPropertiesProcessor eventPropertiesProcessor, + @NonNull PropertyValidator eventPropertiesProcessor, @NonNull SyncManager syncManager) { mEventValidator = checkNotNull(eventValidator); mValidationLogger = checkNotNull(validationLogger); mTelemetryStorageProducer = checkNotNull(telemetryStorageProducer); - mEventPropertiesProcessor = checkNotNull(eventPropertiesProcessor); + mPropertyValidator = checkNotNull(eventPropertiesProcessor); mSyncManager = checkNotNull(syncManager); } @@ -74,8 +75,8 @@ public boolean track(String key, String trafficType, String eventType, event.trafficTypeName = event.trafficTypeName.toLowerCase(); } - ProcessedEventProperties processedProperties = - mEventPropertiesProcessor.process(event.properties); + PropertyValidator.Result processedProperties = + mPropertyValidator.validate(event.properties, validationTag); if (!processedProperties.isValid()) { return false; } diff --git a/src/main/java/io/split/android/client/EventPropertiesProcessorImpl.java b/src/main/java/io/split/android/client/PropertyValidatorImpl.java similarity index 66% rename from src/main/java/io/split/android/client/EventPropertiesProcessorImpl.java rename to src/main/java/io/split/android/client/PropertyValidatorImpl.java index a6fd3430b..01cc06ef6 100644 --- a/src/main/java/io/split/android/client/EventPropertiesProcessorImpl.java +++ b/src/main/java/io/split/android/client/PropertyValidatorImpl.java @@ -4,32 +4,32 @@ import java.util.Map; import io.split.android.client.utils.logger.Logger; +import io.split.android.client.validators.PropertyValidator; import io.split.android.client.validators.ValidationConfig; -public class EventPropertiesProcessorImpl implements EventPropertiesProcessor { +public class PropertyValidatorImpl implements PropertyValidator { - private static final String VALIDATION_TAG = "track"; private final static int MAX_PROPS_COUNT = 300; private final static int MAXIMUM_EVENT_PROPERTY_BYTES = ValidationConfig.getInstance().getMaximumEventPropertyBytes(); @Override - public ProcessedEventProperties process(Map properties) { + public Result validate(Map properties, String validationTag) { if (properties == null) { - return new ProcessedEventProperties(true, null, 0); + return Result.valid(null, 0); } if (properties.size() > MAX_PROPS_COUNT) { - Logger.w(VALIDATION_TAG + "Event has more than " + MAX_PROPS_COUNT + + Logger.w(validationTag + "Event has more than " + MAX_PROPS_COUNT + " properties. Some of them will be trimmed when processed"); } int sizeInBytes = 0; Map finalProperties = new HashMap<>(properties); - for (Map.Entry entry : properties.entrySet()) { + for (Map.Entry entry : properties.entrySet()) { Object value = entry.getValue(); - String key = entry.getKey().toString(); + String key = entry.getKey(); if (value != null && isInvalidValueType(value)) { finalProperties.put(key, null); @@ -37,29 +37,27 @@ public ProcessedEventProperties process(Map properties) { sizeInBytes += calculateEventSizeInBytes(key, value); if (sizeInBytes > MAXIMUM_EVENT_PROPERTY_BYTES) { - Logger.w(VALIDATION_TAG + + Logger.w(validationTag + "The maximum size allowed for the " + " properties is 32kb. Current is " + key + ". Event not queued"); - return ProcessedEventProperties.InvalidProperties(); + return Result.invalid("Event properties size is too large", sizeInBytes); } } - return new ProcessedEventProperties(true, finalProperties, sizeInBytes); + return Result.valid(finalProperties, sizeInBytes); } - private boolean isInvalidValueType(Object value) { + private static boolean isInvalidValueType(Object value) { return !(value instanceof Number) && !(value instanceof Boolean) && !(value instanceof String); } - private int calculateEventSizeInBytes(String key, Object value) { + private static int calculateEventSizeInBytes(String key, Object value) { int valueSize = 0; if(value != null && value.getClass() == String.class) { valueSize = value.toString().getBytes().length; } return valueSize + key.getBytes().length; } - - } diff --git a/src/main/java/io/split/android/client/SplitFactoryImpl.java b/src/main/java/io/split/android/client/SplitFactoryImpl.java index 9fb574d7f..d2630edbc 100644 --- a/src/main/java/io/split/android/client/SplitFactoryImpl.java +++ b/src/main/java/io/split/android/client/SplitFactoryImpl.java @@ -501,7 +501,7 @@ public EventsTracker getEventsTracker() { if (mEventsTracker == null) { EventValidator eventsValidator = new EventValidatorImpl(new KeyValidatorImpl(), mSplitsStorage); mEventsTracker = new EventsTrackerImpl(eventsValidator, new ValidationMessageLoggerImpl(), mTelemetryStorage, - new EventPropertiesProcessorImpl(), mSyncManager); + new PropertyValidatorImpl(), mSyncManager); } } } diff --git a/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java b/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java index 81e0ec11e..e30233ab6 100644 --- a/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java +++ b/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java @@ -15,6 +15,7 @@ import io.split.android.client.EvaluationOptions; import io.split.android.client.EvaluatorImpl; import io.split.android.client.FlagSetsFilter; +import io.split.android.client.PropertyValidatorImpl; import io.split.android.client.SplitClient; import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitFactory; diff --git a/src/main/java/io/split/android/client/validators/PropertyValidator.java b/src/main/java/io/split/android/client/validators/PropertyValidator.java index 6b86d3b9e..093e7b895 100644 --- a/src/main/java/io/split/android/client/validators/PropertyValidator.java +++ b/src/main/java/io/split/android/client/validators/PropertyValidator.java @@ -7,9 +7,7 @@ public interface PropertyValidator { - Result validate(Map properties); - - Result validate(Map properties, int initialSizeInBytes, String validationTag); + Result validate(Map properties, String validationTag); class Result { @@ -20,7 +18,7 @@ class Result { @Nullable private final String mErrorMessage; - private Result(boolean isValid, Map properties, int sizeInBytes, String errorMessage) { + private Result(boolean isValid, @Nullable Map properties, int sizeInBytes, @Nullable String errorMessage) { mIsValid = isValid; mValidatedProperties = properties; mSizeInBytes = sizeInBytes; @@ -32,7 +30,7 @@ public boolean isValid() { } @Nullable - public Map getValidatedProperties() { + public Map getProperties() { return mValidatedProperties; } diff --git a/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java b/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java index 3cb5335ed..e9a24631a 100644 --- a/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java +++ b/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java @@ -8,6 +8,7 @@ import io.split.android.client.Evaluator; import io.split.android.client.EvaluatorImpl; import io.split.android.client.FlagSetsFilter; +import io.split.android.client.PropertyValidatorImpl; import io.split.android.client.api.Key; import io.split.android.client.attributes.AttributesManager; import io.split.android.client.attributes.AttributesMerger; diff --git a/src/main/java/io/split/android/client/validators/TreatmentManagerImpl.java b/src/main/java/io/split/android/client/validators/TreatmentManagerImpl.java index 3df6c6cbf..7493e2a7c 100644 --- a/src/main/java/io/split/android/client/validators/TreatmentManagerImpl.java +++ b/src/main/java/io/split/android/client/validators/TreatmentManagerImpl.java @@ -354,19 +354,19 @@ private String serializeProperties(@Nullable EvaluationOptions evaluationOptions } // validate using property validator - PropertyValidator.Result result = mPropertyValidator.validate(evaluationOptions.getProperties()); + PropertyValidator.Result result = mPropertyValidator.validate(evaluationOptions.getProperties(), validationTag); if (!result.isValid()) { mValidationLogger.e("Properties validation failed: " + (result.getErrorMessage() != null ? result.getErrorMessage() : "Unknown error"), validationTag); return null; } - if (result.getValidatedProperties() == null || result.getValidatedProperties().isEmpty()) { + if (result.getProperties() == null || result.getProperties().isEmpty()) { return null; } try { - return Json.toJson(result.getValidatedProperties()); + return Json.toJson(result.getProperties()); } catch (Exception e) { mValidationLogger.e("Failed to serialize properties to JSON: " + e.getLocalizedMessage(), validationTag); return null; diff --git a/src/test/java/io/split/android/client/TreatmentManagerEvaluationOptionsTest.java b/src/test/java/io/split/android/client/TreatmentManagerEvaluationOptionsTest.java index 519db3678..91fb30c09 100644 --- a/src/test/java/io/split/android/client/TreatmentManagerEvaluationOptionsTest.java +++ b/src/test/java/io/split/android/client/TreatmentManagerEvaluationOptionsTest.java @@ -75,7 +75,7 @@ public void setUp() { public void evaluationWithValidPropertiesAddsThemToImpressionAsJsonString() { when(mEvaluator.getTreatment(anyString(), anyString(), anyString(), anyMap())).thenReturn(new EvaluationResult("test", "label")); EvaluationOptions evaluationOptions = getEvaluationOptions(); - when(mPropertyValidator.validate(any())).thenReturn(PropertyValidator.Result.valid(evaluationOptions.getProperties(), 0)); + when(mPropertyValidator.validate(any(), any())).thenReturn(PropertyValidator.Result.valid(evaluationOptions.getProperties(), 0)); mTreatmentManager.getTreatmentWithConfig("test", null, evaluationOptions, false); @@ -92,7 +92,7 @@ public boolean matches(Impression argument) { @Test public void evaluationWithEmptyPropertiesAddsNullPropertiesToImpression() { when(mEvaluator.getTreatment(anyString(), anyString(), anyString(), anyMap())).thenReturn(new EvaluationResult("test", "label")); - when(mPropertyValidator.validate(any())).thenReturn(PropertyValidator.Result.valid(null, 0)); + when(mPropertyValidator.validate(any(), any())).thenReturn(PropertyValidator.Result.valid(null, 0)); mTreatmentManager.getTreatmentWithConfig("test", null, new EvaluationOptions(new HashMap<>()), false); @@ -108,7 +108,7 @@ public boolean matches(Impression argument) { public void invalidPropertiesAreNotAddedToImpression() { when(mEvaluator.getTreatment(anyString(), anyString(), anyString(), anyMap())).thenReturn(new EvaluationResult("test", "label")); EvaluationOptions evaluationOptions = getEvaluationOptions(); - when(mPropertyValidator.validate(any())).thenReturn(PropertyValidator.Result.invalid("Invalid properties", 0)); + when(mPropertyValidator.validate(any(), any())).thenReturn(PropertyValidator.Result.invalid("Invalid properties", 0)); mTreatmentManager.getTreatmentWithConfig("test", null, evaluationOptions, false); @@ -124,7 +124,7 @@ public boolean matches(Impression argument) { public void invalidPropertiesLogsMessageInValidationMessageLogger() { when(mEvaluator.getTreatment(anyString(), anyString(), anyString(), anyMap())).thenReturn(new EvaluationResult("test", "label")); EvaluationOptions evaluationOptions = getEvaluationOptions(); - when(mPropertyValidator.validate(any())).thenReturn(PropertyValidator.Result.invalid("Invalid properties", 0)); + when(mPropertyValidator.validate(any(), any())).thenReturn(PropertyValidator.Result.invalid("Invalid properties", 0)); mTreatmentManager.getTreatmentWithConfig("test", null, evaluationOptions, false); @@ -138,7 +138,7 @@ public void propertiesAreValidatedWithPropertyValidator() { mTreatmentManager.getTreatmentWithConfig("test", null, evaluationOptions, false); - verify(mPropertyValidator).validate(evaluationOptions.getProperties()); + verify(mPropertyValidator).validate(evaluationOptions.getProperties(), "getTreatmentWithConfig"); } @NonNull diff --git a/src/test/java/io/split/android/client/events/EventPropertiesProcessorTest.java b/src/test/java/io/split/android/client/events/PropertyValidatorTest.java similarity index 75% rename from src/test/java/io/split/android/client/events/EventPropertiesProcessorTest.java rename to src/test/java/io/split/android/client/events/PropertyValidatorTest.java index 46606774a..0841e2d0d 100644 --- a/src/test/java/io/split/android/client/events/EventPropertiesProcessorTest.java +++ b/src/test/java/io/split/android/client/events/PropertyValidatorTest.java @@ -7,18 +7,16 @@ import java.util.HashMap; import java.util.Map; -import io.split.android.client.EventPropertiesProcessor; -import io.split.android.client.EventPropertiesProcessorImpl; -import io.split.android.client.ProcessedEventProperties; +import io.split.android.client.PropertyValidatorImpl; import io.split.android.client.dtos.Split; import io.split.android.client.utils.Utils; +import io.split.android.client.validators.PropertyValidator; import io.split.android.client.validators.ValidationConfig; -public class EventPropertiesProcessorTest { +public class PropertyValidatorTest { - private EventPropertiesProcessor processor = new EventPropertiesProcessorImpl(); + private final PropertyValidator processor = new PropertyValidatorImpl(); private final static long MAX_BYTES = ValidationConfig.getInstance().getMaximumEventPropertyBytes(); - private final static int MAX_COUNT = 300; @Before public void setup() { @@ -33,7 +31,7 @@ public void sizeInBytesValidation() { properties.put("key" + count, Utils.repeat("a", 1021)); // 1025 bytes count++; } - ProcessedEventProperties result = processor.process(properties); + PropertyValidator.Result result = validate(properties); Assert.assertFalse(result.isValid()); } @@ -47,7 +45,7 @@ public void invalidPropertyType() { for (int i = 0; i < 10; i++) { properties.put("key" + i, new Split()); } - ProcessedEventProperties result = processor.process(properties); + PropertyValidator.Result result = validate(properties); Assert.assertTrue(result.isValid()); Assert.assertEquals(10, result.getProperties().size()); @@ -62,7 +60,7 @@ public void nullValues() { for (int i = 10; i < 20; i++) { properties.put("key" + i + 10, null); } - ProcessedEventProperties result = processor.process(properties); + PropertyValidator.Result result = validate(properties); Assert.assertTrue(result.isValid()); Assert.assertEquals(20, result.getProperties().size()); @@ -74,9 +72,13 @@ public void totalBytes() { for (int i = 0; i < 10; i++) { properties.put("k" + i, "10 bytes"); } - ProcessedEventProperties result = processor.process(properties); + PropertyValidator.Result result = validate(properties); Assert.assertTrue(result.isValid()); Assert.assertEquals(100, result.getSizeInBytes()); } + + private PropertyValidator.Result validate(Map properties) { + return processor.validate(properties, "test"); + } } diff --git a/src/test/java/io/split/android/client/service/events/EventsTrackerTest.java b/src/test/java/io/split/android/client/service/events/EventsTrackerTest.java index d1711c7a0..bf0b601e6 100644 --- a/src/test/java/io/split/android/client/service/events/EventsTrackerTest.java +++ b/src/test/java/io/split/android/client/service/events/EventsTrackerTest.java @@ -3,7 +3,6 @@ import static org.junit.Assert.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.anyDouble; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -17,9 +16,6 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import java.util.HashMap; - -import io.split.android.client.EventPropertiesProcessor; import io.split.android.client.EventsTracker; import io.split.android.client.EventsTrackerImpl; import io.split.android.client.ProcessedEventProperties; @@ -28,6 +24,7 @@ import io.split.android.client.telemetry.model.Method; import io.split.android.client.telemetry.storage.TelemetryStorageProducer; import io.split.android.client.validators.EventValidator; +import io.split.android.client.validators.PropertyValidator; import io.split.android.client.validators.ValidationMessageLogger; public class EventsTrackerTest { @@ -40,7 +37,7 @@ public class EventsTrackerTest { @Mock private TelemetryStorageProducer mTelemetryStorageProducer; @Mock - private EventPropertiesProcessor mEventPropertiesProcessor; + private PropertyValidator mPropertyValidator; @Mock private SyncManager mSyncManager; @@ -51,10 +48,10 @@ public void setup() { MockitoAnnotations.openMocks(this); when(mEventValidator.validate(any(), anyBoolean())).thenReturn(null); when(mEventsManager.eventAlreadyTriggered(any())).thenReturn(true); - when(mEventPropertiesProcessor.process(any())).thenReturn(new ProcessedEventProperties(true, null, 0)); + when(mPropertyValidator.validate(any(), any())).thenReturn(PropertyValidator.Result.valid(null, 0)); mEventsTracker = new EventsTrackerImpl(mEventValidator, mValidationLogger, mTelemetryStorageProducer, - mEventPropertiesProcessor, mSyncManager); + mPropertyValidator, mSyncManager); } @Test @@ -92,7 +89,7 @@ public void trackRecordsLatencyInEvaluationProducer() { @Test public void trackRecordsExceptionInCaseThereIsOne() { - when(mEventPropertiesProcessor.process(any())).thenAnswer(invocation -> { + when(mPropertyValidator.validate(any(), any())).thenAnswer(invocation -> { throw new Exception("test exception"); });