fix: convert jobscheduler to work manager #360

thomaszurkan-optimizely · 2021-02-02T20:06:50Z

Summary

Deprecate and remove job scheduler in favor of the androidx work manager. Unable to create/stop/unbind ScheduledJobService #350
Deprecate Intent and Intent service in favor of workers. Unable to create/stop/unbind ScheduledJobService #350
Remove deprecated API SUPPLICANT ACTION. SUPPLICANT_CONNECTION_CHANGE_ACTION is deprecated in API 28 #347
Remove AsyncTask in favor of straight executor. AsyncTask is deprecated in API 30 (Android 11) #351
DatafileClient download timeout can be set.

Test plan

Tests are all passing
Some tests can be removed with the deprecated APIs/classes

Issues

SUPPLICANT_CONNECTION_CHANGE_ACTION is deprecated in API 28 #347 Unable to create/stop/unbind ScheduledJobService #350 AsyncTask is deprecated in API 30 (Android 11) #351

… into workManager

prasannarupan

Changes looks good. Will wait for the new build to test this out.

… into workManager

mnoman09

Overall looks good. Update headers in all the edited files.

jaeopt

This fix is great. I see a lot of old messes cleaned up.
See my comments for changes suggestion.

datafile-handler/build.gradle

datafile-handler/src/main/AndroidManifest.xml

jaeopt · 2021-02-22T19:57:58Z

event-handler/src/main/java/com/optimizely/ab/android/event_handler/EventRescheduler.java

@@ -82,23 +85,14 @@ public void onReceive(Context context, Intent intent) {
    void reschedule(@NonNull Context context, @NonNull Intent broadcastIntent, @NonNull Intent eventServiceIntent, @NonNull ServiceScheduler serviceScheduler) {


remove serviceScheduler param, not used here.

jaeopt · 2021-02-22T22:34:24Z

event-handler/src/main/java/com/optimizely/ab/android/event_handler/EventRescheduler.java

-                        logger.info("Preemptively flushing events since wifi became available");
-                    }
-                }
+        } else if (broadcastIntent.getAction().equals(WifiManager.WIFI_STATE_CHANGED_ACTION)) {


Don't we need to add WIFI_STATE_CHANGED into intent-filters in AndroidManifest?

I, it is registered through register service api in app (look at splash application)

jaeopt · 2021-02-22T23:30:48Z

...le-handler/src/main/java/com/optimizely/ab/android/datafile_handler/DatafileRescheduler.java

@@ -90,15 +90,18 @@ public void onReceive(Context context, Intent intent) {
            this.logger = logger;
        }

-        void dispatch(Intent intent) {
+        void dispatch() {
            List<DatafileConfig> datafileConfigs = backgroundWatchersCache.getWatchingDatafileConfigs();

            for (DatafileConfig datafileConfig : datafileConfigs) {
                // for scheduled jobs Android O and above, we use the JobScheduler and persistent periodic jobs
                // so, we don't need to do anything.
               if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {


Do we still need extra support for old versions? WorkManager is supposed to take care of it even for old versions. We can completely remove this rescheduler.

It does handle alarm and job scheduler (old and new). No it cannot be removed, in cases of a reboot or reload the datafile rescheduler is needed because jobs are not consistent.

...handler/src/main/java/com/optimizely/ab/android/datafile_handler/DefaultDatafileHandler.java

datafile-handler/src/main/java/com/optimizely/ab/android/datafile_handler/DatafileWorker.java

jaeopt · 2021-02-23T17:29:25Z

test-app/src/main/java/com/optimizely/ab/android/test_app/SplashScreenActivity.kt

        // with the new Android O differences, you need to register the service for the intent filter you desire in code instead of
        // in the manifest.
        val eventRescheduler = EventRescheduler()
-        applicationContext.registerReceiver(eventRescheduler, IntentFilter(WifiManager.SUPPLICANT_CONNECTION_CHANGE_ACTION))
+        applicationContext.registerReceiver(eventRescheduler, IntentFilter(WifiManager.WIFI_STATE_CHANGED_ACTION))


Move this to AndroidManifest "intent-filter" of EventRescheduler like other intents.

No, that has been deprecated. You use register receiver.

jaeopt · 2021-02-23T18:01:28Z

shared/src/main/java/com/optimizely/ab/android/shared/WorkerScheduler.java

+            workRequestBuilder.setBackoffCriteria(
+                    BackoffPolicy.LINEAR,
+                    retryInterval * 1000,
+                    TimeUnit.MILLISECONDS);


Is this going to retry forever? There should be a way to set the max retries like 3?

It is up to the worker. The event worker allows for the backoff policy to continue. It should retry until sent. Otherwise, they may never get sent.

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

…ile_handler/DefaultDatafileHandler.java Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

… into workManager

jaeopt

LGTM

I updated the proper headers

checkin replacement of intents with work manager

1b45db6

thomaszurkan-optimizely requested a review from a team as a code owner February 2, 2021 20:06

optimizely-codereview-bot assigned thomaszurkan-optimizely Feb 2, 2021

thomaszurkan-optimizely added 3 commits February 2, 2021 15:34

Merge branch 'master' into workManager

27233e8

disable some tests

50dffab

Merge branch 'workManager' of https://github.com/optimizely/android-sdk…

1ed5dbb

… into workManager

prasannarupan reviewed Feb 3, 2021

View reviewed changes

twzurkan and others added 2 commits February 18, 2021 18:47

ready for review

8fa6667

Merge branch 'master' into workManager

275b937

thomaszurkan-optimizely closed this Feb 19, 2021

thomaszurkan-optimizely reopened this Feb 19, 2021

twzurkan added 2 commits February 22, 2021 10:19

cleanup and prepare for merge

ff14557

Merge branch 'workManager' of https://github.com/optimizely/android-sdk…

5500d2d

… into workManager

thomaszurkan-optimizely closed this Feb 22, 2021

thomaszurkan-optimizely reopened this Feb 22, 2021

mnoman09 previously requested changes Feb 23, 2021

View reviewed changes

jaeopt requested changes Feb 23, 2021

View reviewed changes

jaeopt reviewed Feb 23, 2021

View reviewed changes

thomaszurkan-optimizely and others added 6 commits February 23, 2021 12:46

Update datafile-handler/build.gradle

e1c6f5d

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

Update datafile-handler/src/main/java/com/optimizely/ab/android/dataf…

52dc4c1

…ile_handler/DefaultDatafileHandler.java Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

Update datafile-handler/src/main/java/com/optimizely/ab/android/dataf…

c91881a

…ile_handler/DefaultDatafileHandler.java Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

update from comments

48c7838

Merge branch 'workManager' of https://github.com/optimizely/android-sdk…

834e432

… into workManager

fix typo from auto-merge

911f0da

thomaszurkan-optimizely closed this Feb 24, 2021

thomaszurkan-optimizely reopened this Feb 24, 2021

jaeopt approved these changes Feb 24, 2021

View reviewed changes

santosh399762 approved these changes Feb 24, 2021

View reviewed changes

thomaszurkan-optimizely merged commit 790e1de into master Feb 24, 2021

thomaszurkan-optimizely deleted the workManager branch February 24, 2021 20:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: convert jobscheduler to work manager #360

fix: convert jobscheduler to work manager #360

thomaszurkan-optimizely commented Feb 2, 2021 •

edited

Loading

prasannarupan left a comment

mnoman09 left a comment

jaeopt left a comment

jaeopt Feb 22, 2021

jaeopt Feb 22, 2021

thomaszurkan-optimizely Feb 23, 2021

jaeopt Feb 22, 2021

thomaszurkan-optimizely Feb 23, 2021 •

edited

Loading

jaeopt Feb 23, 2021

thomaszurkan-optimizely Feb 23, 2021 •

edited

Loading

jaeopt Feb 23, 2021

thomaszurkan-optimizely Feb 23, 2021 •

edited

Loading

jaeopt left a comment

		@@ -82,23 +85,14 @@ public void onReceive(Context context, Intent intent) {
		void reschedule(@NonNull Context context, @NonNull Intent broadcastIntent, @NonNull Intent eventServiceIntent, @NonNull ServiceScheduler serviceScheduler) {

fix: convert jobscheduler to work manager #360

fix: convert jobscheduler to work manager #360

Conversation

thomaszurkan-optimizely commented Feb 2, 2021 • edited Loading

Summary

Test plan

Issues

prasannarupan left a comment

Choose a reason for hiding this comment

mnoman09 left a comment

Choose a reason for hiding this comment

jaeopt left a comment

Choose a reason for hiding this comment

jaeopt Feb 22, 2021

Choose a reason for hiding this comment

jaeopt Feb 22, 2021

Choose a reason for hiding this comment

thomaszurkan-optimizely Feb 23, 2021

Choose a reason for hiding this comment

jaeopt Feb 22, 2021

Choose a reason for hiding this comment

thomaszurkan-optimizely Feb 23, 2021 • edited Loading

Choose a reason for hiding this comment

jaeopt Feb 23, 2021

Choose a reason for hiding this comment

thomaszurkan-optimizely Feb 23, 2021 • edited Loading

Choose a reason for hiding this comment

jaeopt Feb 23, 2021

Choose a reason for hiding this comment

thomaszurkan-optimizely Feb 23, 2021 • edited Loading

Choose a reason for hiding this comment

jaeopt left a comment

Choose a reason for hiding this comment

thomaszurkan-optimizely commented Feb 2, 2021 •

edited

Loading

thomaszurkan-optimizely Feb 23, 2021 •

edited

Loading

thomaszurkan-optimizely Feb 23, 2021 •

edited

Loading

thomaszurkan-optimizely Feb 23, 2021 •

edited

Loading