From 1870c6d8a28dc37f8cf7d069051c2d920479325c Mon Sep 17 00:00:00 2001 From: Ephraim Kigamba Date: Tue, 25 Oct 2022 12:42:58 +0300 Subject: [PATCH 1/5] Run DuplicateCleanerWorker only until all IDs are cleaned --- .../java/org/smartregister/AllConstants.java | 1 + .../job/DuplicateCleanerWorker.java | 36 +++++++++++++++++-- .../repository/AllSharedPreferences.java | 4 +++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/opensrp-core/src/main/java/org/smartregister/AllConstants.java b/opensrp-core/src/main/java/org/smartregister/AllConstants.java index 2679d9631..d7ca3bc34 100644 --- a/opensrp-core/src/main/java/org/smartregister/AllConstants.java +++ b/opensrp-core/src/main/java/org/smartregister/AllConstants.java @@ -449,6 +449,7 @@ public static class BARCODE { public static class PREF_KEY { public static final String SETTINGS = "settings"; + public static final String DUPLICATE_IDS_FIXED = "duplicate-ids-fixed"; } public static class PeerToPeer { diff --git a/opensrp-core/src/main/java/org/smartregister/job/DuplicateCleanerWorker.java b/opensrp-core/src/main/java/org/smartregister/job/DuplicateCleanerWorker.java index 19676b74e..fb878525a 100644 --- a/opensrp-core/src/main/java/org/smartregister/job/DuplicateCleanerWorker.java +++ b/opensrp-core/src/main/java/org/smartregister/job/DuplicateCleanerWorker.java @@ -3,30 +3,60 @@ import android.content.Context; import androidx.annotation.NonNull; +import androidx.work.ExistingPeriodicWorkPolicy; +import androidx.work.PeriodicWorkRequest; import androidx.work.WorkManager; import androidx.work.Worker; import androidx.work.WorkerParameters; +import org.smartregister.AllConstants; +import org.smartregister.CoreLibrary; import org.smartregister.domain.DuplicateZeirIdStatus; +import org.smartregister.repository.AllSharedPreferences; import org.smartregister.util.AppHealthUtils; +import java.util.concurrent.TimeUnit; + import timber.log.Timber; public class DuplicateCleanerWorker extends Worker { private Context mContext; + public static final String TAG = "DuplicateCleanerWorker"; + public DuplicateCleanerWorker(@NonNull Context context, @NonNull WorkerParameters workerParams) { super(context, workerParams); mContext = context; } + public static boolean shouldSchedule() { + AllSharedPreferences allSharedPreferences = CoreLibrary.getInstance().context().allSharedPreferences(); + return !allSharedPreferences.getBooleanPreference(AllConstants.PREF_KEY.DUPLICATE_IDS_FIXED); + } + + public static void schedulePeriodically(Context context, int mins) { + PeriodicWorkRequest periodicWorkRequest = new PeriodicWorkRequest.Builder(DuplicateCleanerWorker.class, mins, TimeUnit.MINUTES) + .build(); + + WorkManager.getInstance(context) + .enqueueUniquePeriodicWork(TAG, ExistingPeriodicWorkPolicy.KEEP, periodicWorkRequest); + } + @NonNull @Override public Result doWork() { - DuplicateZeirIdStatus duplicateZeirIdStatus = AppHealthUtils.cleanUniqueZeirIds(); - Timber.i("Doing some cleaning work"); - if (duplicateZeirIdStatus != null && duplicateZeirIdStatus.equals(DuplicateZeirIdStatus.CLEANED)) + AllSharedPreferences allSharedPreferences = CoreLibrary.getInstance().context().allSharedPreferences(); + + if (!allSharedPreferences.getBooleanPreference(AllConstants.PREF_KEY.DUPLICATE_IDS_FIXED)) { + DuplicateZeirIdStatus duplicateZeirIdStatus = AppHealthUtils.cleanUniqueZeirIds(); + Timber.i("Doing some cleaning work"); + if (duplicateZeirIdStatus != null && duplicateZeirIdStatus.equals(DuplicateZeirIdStatus.CLEANED)) { + allSharedPreferences.saveBooleanPreference(AllConstants.PREF_KEY.DUPLICATE_IDS_FIXED, true); + WorkManager.getInstance(mContext).cancelWorkById(this.getId()); + } + } else { WorkManager.getInstance(mContext).cancelWorkById(this.getId()); + } return Result.success(); } diff --git a/opensrp-core/src/main/java/org/smartregister/repository/AllSharedPreferences.java b/opensrp-core/src/main/java/org/smartregister/repository/AllSharedPreferences.java index dc6372a31..92197cf28 100644 --- a/opensrp-core/src/main/java/org/smartregister/repository/AllSharedPreferences.java +++ b/opensrp-core/src/main/java/org/smartregister/repository/AllSharedPreferences.java @@ -233,6 +233,10 @@ public boolean getBooleanPreference(String key) { return preferences.getBoolean(key, false); } + public boolean saveBooleanPreference(String key, boolean value) { + return preferences.edit().putBoolean(key, value).commit(); + } + public void updateUrl(String baseUrl) { try { From 6b6f0f4b8f19af1aa903c6533b8d0819dd17e748 Mon Sep 17 00:00:00 2001 From: Ephraim Kigamba Date: Tue, 25 Oct 2022 14:41:07 +0300 Subject: [PATCH 2/5] Add zeir_id and ANC_ID to duplicate IDs fix - This fixes a bug where the two are considered to be null since they are not labelled as ZEIR_ID and the cleanup job will keep regenerating them --- .../smartregister/repository/EventClientRepository.java | 8 +++++++- .../smartregister/repository/ZeirIdCleanupRepository.java | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/opensrp-core/src/main/java/org/smartregister/repository/EventClientRepository.java b/opensrp-core/src/main/java/org/smartregister/repository/EventClientRepository.java index 987e387d5..1216dff22 100644 --- a/opensrp-core/src/main/java/org/smartregister/repository/EventClientRepository.java +++ b/opensrp-core/src/main/java/org/smartregister/repository/EventClientRepository.java @@ -55,6 +55,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; @@ -2404,7 +2405,12 @@ public DuplicateZeirIdStatus cleanDuplicateMotherIds() throws Exception { String clientType = clientJson.getString(AllConstants.CLIENT_TYPE); if (AllConstants.CHILD_TYPE.equals(clientType)) { - identifiers.put(ZEIR_ID, newZeirId.replaceAll("-", "")); + String identifierLabel = ZEIR_ID; + if (!identifiers.has(ZEIR_ID)) { + identifierLabel = ZEIR_ID.toLowerCase(Locale.ROOT); + } + + identifiers.put(identifierLabel, newZeirId.replaceAll("-", "")); } else if (AllConstants.Entity.MOTHER.equals(clientType)) { identifiers.put(M_ZEIR_ID, newZeirId); eventType = AllConstants.EventType.NEW_WOMAN_REGISTRATION; diff --git a/opensrp-core/src/main/java/org/smartregister/repository/ZeirIdCleanupRepository.java b/opensrp-core/src/main/java/org/smartregister/repository/ZeirIdCleanupRepository.java index 01418d143..f049faf5f 100644 --- a/opensrp-core/src/main/java/org/smartregister/repository/ZeirIdCleanupRepository.java +++ b/opensrp-core/src/main/java/org/smartregister/repository/ZeirIdCleanupRepository.java @@ -16,7 +16,7 @@ public class ZeirIdCleanupRepository extends BaseRepository { private static final String DUPLICATES_SQL = "WITH duplicates AS ( " + " WITH clients AS ( " + - " SELECT baseEntityId, COALESCE(json_extract(json, '$.identifiers.ZEIR_ID'), json_extract(json, '$.identifiers.M_ZEIR_ID')) zeir_id " + + " SELECT baseEntityId, COALESCE(json_extract(json, '$.identifiers.ZEIR_ID'), json_extract(json, '$.identifiers.M_ZEIR_ID'), json_extract(json, '$.identifiers.zeir_id'), json_extract(json, '$.identifiers.ANC_ID')) zeir_id" + " FROM client " + " ) " + " SELECT b.* FROM (SELECT baseEntityId, zeir_id FROM clients GROUP BY zeir_id HAVING count(zeir_id) > 1) a " + From aea2c60934d7af031b7cf92d70fe692293a3d544 Mon Sep 17 00:00:00 2001 From: Ephraim Kigamba Date: Tue, 1 Nov 2022 17:35:45 +0300 Subject: [PATCH 3/5] Fix failing tests on EventClientRepository - Add more method documentation --- .../org/smartregister/job/DuplicateCleanerWorker.java | 8 +++++++- .../smartregister/repository/ZeirIdCleanupRepository.java | 2 +- .../repository/EventClientRepositoryTest.java | 3 ++- sample/build.gradle | 5 +++++ 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/opensrp-core/src/main/java/org/smartregister/job/DuplicateCleanerWorker.java b/opensrp-core/src/main/java/org/smartregister/job/DuplicateCleanerWorker.java index fb878525a..bb0966cb3 100644 --- a/opensrp-core/src/main/java/org/smartregister/job/DuplicateCleanerWorker.java +++ b/opensrp-core/src/main/java/org/smartregister/job/DuplicateCleanerWorker.java @@ -34,6 +34,12 @@ public static boolean shouldSchedule() { return !allSharedPreferences.getBooleanPreference(AllConstants.PREF_KEY.DUPLICATE_IDS_FIXED); } + /** + * Schedule this job to run periodically + * + * @param context + * @param mins - Duration after which the job repeatedly runs. This should be at least 15 mins + */ public static void schedulePeriodically(Context context, int mins) { PeriodicWorkRequest periodicWorkRequest = new PeriodicWorkRequest.Builder(DuplicateCleanerWorker.class, mins, TimeUnit.MINUTES) .build(); @@ -49,7 +55,7 @@ public Result doWork() { if (!allSharedPreferences.getBooleanPreference(AllConstants.PREF_KEY.DUPLICATE_IDS_FIXED)) { DuplicateZeirIdStatus duplicateZeirIdStatus = AppHealthUtils.cleanUniqueZeirIds(); - Timber.i("Doing some cleaning work"); + Timber.i("Started doing duplicate client-identifier cleanup"); if (duplicateZeirIdStatus != null && duplicateZeirIdStatus.equals(DuplicateZeirIdStatus.CLEANED)) { allSharedPreferences.saveBooleanPreference(AllConstants.PREF_KEY.DUPLICATE_IDS_FIXED, true); WorkManager.getInstance(mContext).cancelWorkById(this.getId()); diff --git a/opensrp-core/src/main/java/org/smartregister/repository/ZeirIdCleanupRepository.java b/opensrp-core/src/main/java/org/smartregister/repository/ZeirIdCleanupRepository.java index f049faf5f..0d22e9a05 100644 --- a/opensrp-core/src/main/java/org/smartregister/repository/ZeirIdCleanupRepository.java +++ b/opensrp-core/src/main/java/org/smartregister/repository/ZeirIdCleanupRepository.java @@ -16,7 +16,7 @@ public class ZeirIdCleanupRepository extends BaseRepository { private static final String DUPLICATES_SQL = "WITH duplicates AS ( " + " WITH clients AS ( " + - " SELECT baseEntityId, COALESCE(json_extract(json, '$.identifiers.ZEIR_ID'), json_extract(json, '$.identifiers.M_ZEIR_ID'), json_extract(json, '$.identifiers.zeir_id'), json_extract(json, '$.identifiers.ANC_ID')) zeir_id" + + " SELECT baseEntityId, COALESCE(json_extract(json, '$.identifiers.ZEIR_ID'), json_extract(json, '$.identifiers.M_ZEIR_ID'), json_extract(json, '$.identifiers.zeir_id'), json_extract(json, '$.identifiers.ANC_ID')) zeir_id " + " FROM client " + " ) " + " SELECT b.* FROM (SELECT baseEntityId, zeir_id FROM clients GROUP BY zeir_id HAVING count(zeir_id) > 1) a " + diff --git a/opensrp-core/src/test/java/org/smartregister/repository/EventClientRepositoryTest.java b/opensrp-core/src/test/java/org/smartregister/repository/EventClientRepositoryTest.java index 222fb8875..91480df5a 100644 --- a/opensrp-core/src/test/java/org/smartregister/repository/EventClientRepositoryTest.java +++ b/opensrp-core/src/test/java/org/smartregister/repository/EventClientRepositoryTest.java @@ -748,7 +748,7 @@ public MatrixCursor getCursorMaxRowId() { public void testCleanDuplicateMotherIdsShouldFixAndMarkDuplicateClientsUnSynced() throws Exception { String DUPLICATES_SQL = "WITH duplicates AS ( " + " WITH clients AS ( " + - " SELECT baseEntityId, COALESCE(json_extract(json, '$.identifiers.ZEIR_ID'), json_extract(json, '$.identifiers.M_ZEIR_ID')) zeir_id " + + " SELECT baseEntityId, COALESCE(json_extract(json, '$.identifiers.ZEIR_ID'), json_extract(json, '$.identifiers.M_ZEIR_ID'), json_extract(json, '$.identifiers.zeir_id'), json_extract(json, '$.identifiers.ANC_ID')) zeir_id " + " FROM client " + " ) " + " SELECT b.* FROM (SELECT baseEntityId, zeir_id FROM clients GROUP BY zeir_id HAVING count(zeir_id) > 1) a " + @@ -766,6 +766,7 @@ public void testCleanDuplicateMotherIdsShouldFixAndMarkDuplicateClientsUnSynced( DuplicateZeirIdStatus duplicateZeirIdStatus = eventClientRepository.cleanDuplicateMotherIds(); Assert.assertEquals(DuplicateZeirIdStatus.CLEANED, duplicateZeirIdStatus); verify(sqliteDatabase, times(1)).rawQuery(eq(DUPLICATES_SQL), any()); + verify(sqliteDatabase, times(2)).rawQuery(eq("SELECT COUNT (*) FROM unique_ids WHERE status=?"), any()); verify(sqliteDatabase, times(1)).insert(eq("client"), eq(null), any()); } diff --git a/sample/build.gradle b/sample/build.gradle index ffdcc3d12..ce7a49852 100644 --- a/sample/build.gradle +++ b/sample/build.gradle @@ -23,6 +23,11 @@ repositories { apply plugin: 'com.android.application' apply plugin: 'org.smartregister.gradle.jarjar' + +configurations.all { + all*.exclude group: 'com.google.guava', module: 'listenablefuture' +} + android { compileSdkVersion androidCompileSdkVersion From 53e21fbad7ec36c032f91eeb20ca2ae39e46a2e3 Mon Sep 17 00:00:00 2001 From: Ephraim Kigamba Date: Thu, 3 Nov 2022 11:50:34 +0300 Subject: [PATCH 4/5] Add DuplicateCleanerWorkerTest --- opensrp-core/build.gradle | 1 + .../job/DuplicateCleanerWorkerTest.java | 122 ++++++++++++++++++ .../repository/AllSharedPreferencesTest.java | 13 ++ 3 files changed, 136 insertions(+) create mode 100644 opensrp-core/src/test/java/org/smartregister/job/DuplicateCleanerWorkerTest.java diff --git a/opensrp-core/build.gradle b/opensrp-core/build.gradle index 8c95d3aea..0f8dac8ce 100644 --- a/opensrp-core/build.gradle +++ b/opensrp-core/build.gradle @@ -258,6 +258,7 @@ dependencies { def work_version = "2.7.1" implementation "androidx.work:work-runtime:$work_version" + testImplementation "androidx.work:work-testing:$work_version" // Add the dependency for the Performance Monitoring library implementation 'com.google.firebase:firebase-perf:19.0.7' diff --git a/opensrp-core/src/test/java/org/smartregister/job/DuplicateCleanerWorkerTest.java b/opensrp-core/src/test/java/org/smartregister/job/DuplicateCleanerWorkerTest.java new file mode 100644 index 000000000..23b3c688b --- /dev/null +++ b/opensrp-core/src/test/java/org/smartregister/job/DuplicateCleanerWorkerTest.java @@ -0,0 +1,122 @@ +package org.smartregister.job; + +import android.content.Context; +import android.util.Log; + +import androidx.work.Configuration; +import androidx.work.ListenableWorker; +import androidx.work.WorkInfo; +import androidx.work.WorkManager; +import androidx.work.testing.SynchronousExecutor; +import androidx.work.testing.TestWorkerBuilder; +import androidx.work.testing.WorkManagerTestInitHelper; + +import com.google.common.util.concurrent.ListenableFuture; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.util.ReflectionHelpers; +import org.smartregister.BaseRobolectricUnitTest; +import org.smartregister.CoreLibrary; +import org.smartregister.domain.DuplicateZeirIdStatus; +import org.smartregister.repository.AllSharedPreferences; +import org.smartregister.repository.EventClientRepository; + +import java.util.List; +import java.util.concurrent.ExecutionException; + +/** + * Created by Ephraim Kigamba - nek.eam@gmail.com on 02-11-2022. + */ + +public class DuplicateCleanerWorkerTest extends BaseRobolectricUnitTest { + + @Before + public void setUp() throws Exception { + //super.setUp(); + initCoreLibrary(); + initializeWorkManager(); + } + + @Test + public void shouldScheduleShouldReturnFalseAndCallDuplicateIdsFixed() { + AllSharedPreferences allSharedPreferences = Mockito.spy(CoreLibrary.getInstance().context().userService().getAllSharedPreferences()); + ReflectionHelpers.setField(CoreLibrary.getInstance().context(), "allSharedPreferences", allSharedPreferences); + + Mockito.doReturn(true).when(allSharedPreferences).getBooleanPreference("duplicate-ids-fixed"); + + Assert.assertFalse(DuplicateCleanerWorker.shouldSchedule()); + + Mockito.verify(allSharedPreferences).getBooleanPreference("duplicate-ids-fixed"); + } + + @Test + public void schedulePeriodicallyShouldScheduleJobInWorkManager() throws ExecutionException, InterruptedException { + Context context = RuntimeEnvironment.application; + DuplicateCleanerWorker.schedulePeriodically(context, 15); + + ListenableFuture> listenableFuture = WorkManager.getInstance(context) + .getWorkInfosForUniqueWork(DuplicateCleanerWorker.TAG); + + Assert.assertEquals(1, listenableFuture.get().size()); + } + + @Test + public void doWorkShouldReturnSuccess() { + DuplicateCleanerWorker duplicateCleanerWorker = TestWorkerBuilder.from(RuntimeEnvironment.application, DuplicateCleanerWorker.class) + .build(); + Assert.assertEquals(ListenableWorker.Result.success(), duplicateCleanerWorker.doWork()); + } + + @Test + public void doWorkShouldCallCleanDuplicateUniqueZeirIdsWhenDuplicateIdsFixedPreferenceIsFalse() throws Exception { + AllSharedPreferences allSharedPreferences = Mockito.spy(CoreLibrary.getInstance().context().userService().getAllSharedPreferences()); + ReflectionHelpers.setField(CoreLibrary.getInstance().context(), "allSharedPreferences", allSharedPreferences); + Mockito.doReturn(false).when(allSharedPreferences).getBooleanPreference("duplicate-ids-fixed"); + + EventClientRepository eventClientRepository = Mockito.spy(CoreLibrary.getInstance().context().getEventClientRepository()); + ReflectionHelpers.setField(CoreLibrary.getInstance().context(), "eventClientRepository", eventClientRepository); + Mockito.doReturn(DuplicateZeirIdStatus.PENDING).when(eventClientRepository).cleanDuplicateMotherIds(); + + DuplicateCleanerWorker duplicateCleanerWorker = TestWorkerBuilder.from(RuntimeEnvironment.application, DuplicateCleanerWorker.class) + .build(); + Assert.assertEquals(ListenableWorker.Result.success(), duplicateCleanerWorker.doWork()); + + Mockito.verify(eventClientRepository).cleanDuplicateMotherIds(); + Mockito.verify(allSharedPreferences, Mockito.times(0)).saveBooleanPreference("duplicate-ids-fixed", true); + } + + private void initializeWorkManager() { + Configuration config = new Configuration.Builder() + .setMinimumLoggingLevel(Log.DEBUG) + .setExecutor(new SynchronousExecutor()) + .build(); + + // Initialize WorkManager for instrumentation tests. + WorkManagerTestInitHelper.initializeTestWorkManager( + RuntimeEnvironment.application, config); + } + + @Test + public void doWorkShouldSetDuplicateIdsFixedPreferenceTrueWhenCleanUniqueZeirIdsReturnsCleaned() throws Exception { + AllSharedPreferences allSharedPreferences = Mockito.spy(CoreLibrary.getInstance().context().userService().getAllSharedPreferences()); + ReflectionHelpers.setField(CoreLibrary.getInstance().context(), "allSharedPreferences", allSharedPreferences); + Mockito.doReturn(false).when(allSharedPreferences).getBooleanPreference("duplicate-ids-fixed"); + + EventClientRepository eventClientRepository = Mockito.spy(CoreLibrary.getInstance().context().getEventClientRepository()); + ReflectionHelpers.setField(CoreLibrary.getInstance().context(), "eventClientRepository", eventClientRepository); + Mockito.doReturn(DuplicateZeirIdStatus.CLEANED).when(eventClientRepository).cleanDuplicateMotherIds(); + + //WorkManagerInitializer + DuplicateCleanerWorker duplicateCleanerWorker = TestWorkerBuilder.from(RuntimeEnvironment.application, DuplicateCleanerWorker.class) + .build(); + Assert.assertEquals(ListenableWorker.Result.success(), duplicateCleanerWorker.doWork()); + + Mockito.verify(eventClientRepository).cleanDuplicateMotherIds(); + Mockito.verify(allSharedPreferences).saveBooleanPreference("duplicate-ids-fixed", true); + } + +} \ No newline at end of file diff --git a/opensrp-core/src/test/java/org/smartregister/repository/AllSharedPreferencesTest.java b/opensrp-core/src/test/java/org/smartregister/repository/AllSharedPreferencesTest.java index ae1898fcf..ecc8dbd39 100644 --- a/opensrp-core/src/test/java/org/smartregister/repository/AllSharedPreferencesTest.java +++ b/opensrp-core/src/test/java/org/smartregister/repository/AllSharedPreferencesTest.java @@ -457,4 +457,17 @@ public void testFetchTransactionsKilledFlag() { assertTrue(allSharedPreferences.fetchTransactionsKilledFlag()); } + + @Test + public void saveBooleanPreferenceShouldCallSharedPreferencesEdit() { + SharedPreferences.Editor editor = Mockito.mock(SharedPreferences.Editor.class); + Mockito.doReturn(editor).when(preferences).edit(); + Mockito.doReturn(true).when(editor).commit(); + Mockito.doReturn(editor).when(editor).putBoolean(Mockito.anyString(), Mockito.anyBoolean()); + + allSharedPreferences.saveBooleanPreference("is-logged-in", true); + + Mockito.verify(preferences, Mockito.times(1)).edit(); + Mockito.verify(editor).putBoolean("is-logged-in", true); + } } From b3595e949318853b21c86d4e53ceebd04bce3fde Mon Sep 17 00:00:00 2001 From: Ephraim Kigamba Date: Thu, 3 Nov 2022 12:07:55 +0300 Subject: [PATCH 5/5] :arrow-up: Bump to 5.0.8-SNAPSHOT --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 8fa0c3819..662d7758f 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -VERSION_NAME=5.0.7-SNAPSHOT +VERSION_NAME=5.0.8-SNAPSHOT VERSION_CODE=1 GROUP=org.smartregister POM_SETTING_DESCRIPTION=OpenSRP Client Core Application