-
Notifications
You must be signed in to change notification settings - Fork 7
Serialize properties in impression #754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,6 +125,10 @@ public void submitOnMainThread(SplitTask splitTask) { | |
| } | ||
|
|
||
| public void pause() { | ||
| long start = System.currentTimeMillis(); | ||
| while (!mScheduledTasks.isEmpty() && (System.currentTimeMillis() - start) < 500L) { | ||
| try { Thread.sleep(50); } catch (InterruptedException e) { break; } | ||
| } | ||
|
Comment on lines
+128
to
+131
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is this for?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the app goes to background we pause the task executor and sometimes tasks could end up not completing. This adds a small grace period to give them a chance to finish. |
||
| mScheduler.pause(); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ | |
|
|
||
| public class SplitTaskExecutorImpl extends SplitBaseTaskExecutor { | ||
|
|
||
| private static final int MIN_THREAD_POOL_SIZE_WHEN_IDLE = 2; | ||
| private static final int MIN_THREAD_POOL_SIZE_WHEN_IDLE = 6; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was originally 6, it was reduced to 2 in the last rc but I'm restoring it. |
||
| private static final String THREAD_NAME_FORMAT = "split-taskExecutor-%d"; | ||
|
|
||
| @NonNull | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| package io.split.android.client.validators; | ||
|
|
||
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.Nullable; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
| public interface PropertyValidator { | ||
|
|
||
| Result validate(Map<String, Object> properties); | ||
|
|
||
| Result validate(Map<String, Object> properties, int initialSizeInBytes, String validationTag); | ||
|
|
||
| class Result { | ||
|
|
||
| private final boolean mIsValid; | ||
| @Nullable | ||
| private final Map<String, Object> mValidatedProperties; | ||
| private final int mSizeInBytes; | ||
| @Nullable | ||
| private final String mErrorMessage; | ||
|
|
||
| private Result(boolean isValid, Map<String, Object> properties, int sizeInBytes, String errorMessage) { | ||
| mIsValid = isValid; | ||
| mValidatedProperties = properties; | ||
| mSizeInBytes = sizeInBytes; | ||
| mErrorMessage = errorMessage; | ||
| } | ||
|
|
||
| public boolean isValid() { | ||
| return mIsValid; | ||
| } | ||
|
|
||
| @Nullable | ||
| public Map<String, Object> getValidatedProperties() { | ||
| return mValidatedProperties; | ||
| } | ||
|
|
||
| public int getSizeInBytes() { | ||
| return mSizeInBytes; | ||
| } | ||
|
|
||
| @Nullable | ||
| public String getErrorMessage() { | ||
| return mErrorMessage; | ||
| } | ||
|
|
||
| @NonNull | ||
| public static Result valid(Map<String, Object> properties, int sizeInBytes) { | ||
| return new Result(true, properties, sizeInBytes, null); | ||
| } | ||
|
|
||
| @NonNull | ||
| public static Result invalid(String errorMessage, int sizeInBytes) { | ||
| return new Result(false, null, sizeInBytes, errorMessage); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package io.split.android.client.validators; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
| public class PropertyValidatorImpl implements PropertyValidator { | ||
|
|
||
| @Override | ||
| public Result validate(Map<String, Object> properties) { | ||
| return Result.valid(properties, 0); // TODO implement | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is missing this implementation?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented in #756 |
||
| } | ||
|
|
||
| @Override | ||
| public Result validate(Map<String, Object> properties, int initialSizeInBytes, String validationTag) { | ||
| return Result.valid(properties, initialSizeInBytes); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,24 +11,6 @@ | |
|
|
||
| public interface TreatmentManager { | ||
|
|
||
| String getTreatment(String split, Map<String, Object> attributes, boolean isClientDestroyed); | ||
|
|
||
| SplitResult getTreatmentWithConfig(String split, Map<String, Object> attributes, boolean isClientDestroyed); | ||
|
|
||
| Map<String, String> getTreatments(List<String> splits, Map<String, Object> attributes, boolean isClientDestroyed); | ||
|
|
||
| Map<String, SplitResult> getTreatmentsWithConfig(List<String> splits, Map<String, Object> attributes, boolean isClientDestroyed); | ||
|
|
||
| Map<String, String> getTreatmentsByFlagSet(@NonNull String flagSet, @Nullable Map<String, Object> attributes, boolean isClientDestroyed); | ||
|
|
||
| Map<String, String> getTreatmentsByFlagSets(@NonNull List<String> flagSets, @Nullable Map<String, Object> attributes, boolean isClientDestroyed); | ||
|
|
||
| Map<String, SplitResult> getTreatmentsWithConfigByFlagSet(@NonNull String flagSet, @Nullable Map<String, Object> attributes, boolean isClientDestroyed); | ||
|
|
||
| Map<String, SplitResult> getTreatmentsWithConfigByFlagSets(@NonNull List<String> flagSets, @Nullable Map<String, Object> attributes, boolean isClientDestroyed); | ||
|
|
||
|
|
||
| // temporary methods to reduce changes in this iteration | ||
|
Comment on lines
-14
to
-31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are you removing all this methods?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added new ones. This is internal, no changes to the API. |
||
| String getTreatment(String split, Map<String, Object> attributes, EvaluationOptions evaluationOptions, boolean isClientDestroyed); | ||
|
|
||
| SplitResult getTreatmentWithConfig(String split, Map<String, Object> attributes, EvaluationOptions evaluationOptions, boolean isClientDestroyed); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i love this change