From d6c877b435161f38364b5d1bda46fd637fca6b08 Mon Sep 17 00:00:00 2001 From: SebaMutuku <36365043+SebaMutuku@users.noreply.github.com> Date: Tue, 5 Jul 2022 16:56:29 +0300 Subject: [PATCH 1/5] Checking null location --- build.gradle | 2 +- gradle.properties | 1 - .../org/smartregister/sync/helper/LocationServiceHelper.java | 3 +++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 31af54615..98f20f591 100644 --- a/build.gradle +++ b/build.gradle @@ -10,7 +10,7 @@ buildscript { dependencies { classpath "io.codearte.gradle.nexus:gradle-nexus-staging-plugin:0.11.0" classpath 'com.android.tools.build:gradle:4.0.0' - classpath 'gradle.plugin.org.kt3k.gradle.plugin:coveralls-gradle-plugin:2.10.2' + classpath 'gradle.plugin.org.kt3k.gradle.plugin:coveralls-gradle-plugin:2.12.0' classpath 'org.smartregister:gradle-jarjar-plugin:1.0.0-SNAPSHOT' classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.5.31' } diff --git a/gradle.properties b/gradle.properties index 9eae38662..3f988726f 100644 --- a/gradle.properties +++ b/gradle.properties @@ -13,5 +13,4 @@ POM_SETTING_DEVELOPER_ID=opensrp POM_SETTING_DEVELOPER_NAME=OpenSRP Onadev android.useAndroidX=true android.enableJetifier=true - org.gradle.jvmargs=-Xmx2048m diff --git a/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java b/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java index cf15111c2..7d3c4d155 100644 --- a/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java +++ b/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java @@ -472,6 +472,9 @@ public void fetchAllLocations(int recordCount) { location.setSyncStatus(BaseRepository.TYPE_Synced); locationRepository.addOrUpdate(location); + if (location.getLocationTags() == null) { + return; + } for (LocationTag tag : location.getLocationTags()) { LocationTag locationTag = new LocationTag(); From b5606ad9da9fd5b68b20be28ba0b72fcba8708ac Mon Sep 17 00:00:00 2001 From: SebaMutuku <36365043+SebaMutuku@users.noreply.github.com> Date: Tue, 2 Aug 2022 11:34:25 +0300 Subject: [PATCH 2/5] Checking null location tags --- .../sync/helper/LocationServiceHelper.java | 53 ++++++++++--------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java b/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java index 7d3c4d155..ed729cdb7 100644 --- a/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java +++ b/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java @@ -1,5 +1,27 @@ package org.smartregister.sync.helper; +import static org.smartregister.AllConstants.COUNT; +import static org.smartregister.AllConstants.JURISDICTION_IDS; +import static org.smartregister.AllConstants.LocationConstants.DISPLAY; +import static org.smartregister.AllConstants.LocationConstants.LOCATION; +import static org.smartregister.AllConstants.LocationConstants.LOCATIONS; +import static org.smartregister.AllConstants.LocationConstants.SPECIAL_TAG_FOR_OPENMRS_TEAM_MEMBERS; +import static org.smartregister.AllConstants.LocationConstants.TEAM; +import static org.smartregister.AllConstants.LocationConstants.UUID; +import static org.smartregister.AllConstants.OPERATIONAL_AREAS; +import static org.smartregister.AllConstants.PerformanceMonitoring.ACTION; +import static org.smartregister.AllConstants.PerformanceMonitoring.FETCH; +import static org.smartregister.AllConstants.PerformanceMonitoring.LOCATION_SYNC; +import static org.smartregister.AllConstants.PerformanceMonitoring.PUSH; +import static org.smartregister.AllConstants.PerformanceMonitoring.STRUCTURE; +import static org.smartregister.AllConstants.RETURN_COUNT; +import static org.smartregister.AllConstants.TYPE; +import static org.smartregister.util.PerformanceMonitoringUtils.addAttribute; +import static org.smartregister.util.PerformanceMonitoringUtils.clearTraceAttributes; +import static org.smartregister.util.PerformanceMonitoringUtils.initTrace; +import static org.smartregister.util.PerformanceMonitoringUtils.startTrace; +import static org.smartregister.util.PerformanceMonitoringUtils.stopTrace; + import android.content.Context; import android.text.TextUtils; @@ -45,28 +67,6 @@ import timber.log.Timber; -import static org.smartregister.AllConstants.COUNT; -import static org.smartregister.AllConstants.JURISDICTION_IDS; -import static org.smartregister.AllConstants.LocationConstants.DISPLAY; -import static org.smartregister.AllConstants.LocationConstants.LOCATION; -import static org.smartregister.AllConstants.LocationConstants.LOCATIONS; -import static org.smartregister.AllConstants.LocationConstants.SPECIAL_TAG_FOR_OPENMRS_TEAM_MEMBERS; -import static org.smartregister.AllConstants.LocationConstants.TEAM; -import static org.smartregister.AllConstants.LocationConstants.UUID; -import static org.smartregister.AllConstants.OPERATIONAL_AREAS; -import static org.smartregister.AllConstants.PerformanceMonitoring.ACTION; -import static org.smartregister.AllConstants.PerformanceMonitoring.FETCH; -import static org.smartregister.AllConstants.PerformanceMonitoring.LOCATION_SYNC; -import static org.smartregister.AllConstants.PerformanceMonitoring.PUSH; -import static org.smartregister.AllConstants.PerformanceMonitoring.STRUCTURE; -import static org.smartregister.AllConstants.RETURN_COUNT; -import static org.smartregister.AllConstants.TYPE; -import static org.smartregister.util.PerformanceMonitoringUtils.addAttribute; -import static org.smartregister.util.PerformanceMonitoringUtils.clearTraceAttributes; -import static org.smartregister.util.PerformanceMonitoringUtils.initTrace; -import static org.smartregister.util.PerformanceMonitoringUtils.startTrace; -import static org.smartregister.util.PerformanceMonitoringUtils.stopTrace; - public class LocationServiceHelper extends BaseHelper { public static final String LOCATION_STRUCTURE_URL = "/rest/location/sync"; @@ -466,21 +466,22 @@ public void fetchAllLocations(int recordCount) { new TypeToken>() { }.getType() ); - + if (locations.isEmpty()) { + Timber.e("Empty/Null location response payload"); + return; + } for (Location location : locations) { try { location.setSyncStatus(BaseRepository.TYPE_Synced); - locationRepository.addOrUpdate(location); if (location.getLocationTags() == null) { + Timber.e("Empty/Null location tags %s", location.getLocationTags()); return; } - for (LocationTag tag : location.getLocationTags()) { LocationTag locationTag = new LocationTag(); locationTag.setLocationId(location.getId()); locationTag.setName(tag.getName()); - locationTagRepository.addOrUpdate(locationTag); } } catch (Exception e) { From 840f10cb603572f68576d7f19f31a13187db4b27 Mon Sep 17 00:00:00 2001 From: SebaMutuku <36365043+SebaMutuku@users.noreply.github.com> Date: Tue, 2 Aug 2022 11:53:17 +0300 Subject: [PATCH 3/5] testGenerateDefaultLocationHierarchyWithRegisteredANMNotSetReturnsNull --- .../sync/helper/LocationServiceHelper.java | 3 ++- .../location/helper/LocationHelperTest.java | 20 ++++++++----------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java b/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java index ed729cdb7..47db4a3d5 100644 --- a/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java +++ b/opensrp-core/src/main/java/org/smartregister/sync/helper/LocationServiceHelper.java @@ -63,6 +63,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import timber.log.Timber; @@ -470,7 +471,7 @@ public void fetchAllLocations(int recordCount) { Timber.e("Empty/Null location response payload"); return; } - for (Location location : locations) { + for (Location location : Objects.requireNonNull(locations)) { try { location.setSyncStatus(BaseRepository.TYPE_Synced); locationRepository.addOrUpdate(location); diff --git a/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java b/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java index 243dcbdad..523e6803f 100644 --- a/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java +++ b/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java @@ -1,5 +1,10 @@ package org.smartregister.location.helper; +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertNotNull; +import static junit.framework.Assert.assertNull; +import static junit.framework.Assert.assertTrue; + import android.util.Pair; import net.sqlcipher.database.SQLiteDatabase; @@ -8,6 +13,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.mockito.ArgumentMatchers; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnit; @@ -32,11 +38,6 @@ import java.util.LinkedHashMap; import java.util.List; -import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertNotNull; -import static junit.framework.Assert.assertNull; -import static junit.framework.Assert.assertTrue; - public class LocationHelperTest extends BaseRobolectricUnitTest { private static final String anmLocation1 = "{\"locationsHierarchy\":{\"map\":{\"9c3e8715-1c59-44db-9709-2b49f440ef00\":{\"children\":{\"2e823ceb-4de6-41ac-8025-e2ae3512a331\":{\"children\":{\"620332e0-6108-4611-bac5-8b48d20051c9\":{\"children\":{\"ed7c4a07-6e02-4784-ae9a-9cd41cfef390\":{\"children\":{\"1b0ba804-54c3-40ef-820b-a8eaffa5d054\":{\"id\":\"1b0ba804-54c3-40ef-820b-a8eaffa5d054\",\"label\":\"ra_ksh_5\",\"node\":{\"locationId\":\"1b0ba804-54c3-40ef-820b-a8eaffa5d054\",\"name\":\"ra_ksh_5\",\"parentLocation\":{\"locationId\":\"ed7c4a07-6e02-4784-ae9a-9cd41cfef390\",\"name\":\"ra Kashikishi HAHC\",\"parentLocation\":{\"locationId\":\"620332e0-6108-4611-bac5-8b48d20051c9\",\"name\":\"ra Nchelenge\",\"serverVersion\":0,\"voided\":false},\"serverVersion\":0,\"voided\":false},\"tags\":[\"Operational Area\"],\"serverVersion\":0,\"voided\":false},\"parent\":\"ed7c4a07-6e02-4784-ae9a-9cd41cfef390\"}},\"id\":\"ed7c4a07-6e02-4784-ae9a-9cd41cfef390\",\"label\":\"ra Kashikishi HAHC\",\"node\":{\"locationId\":\"ed7c4a07-6e02-4784-ae9a-9cd41cfef390\",\"name\":\"ra Kashikishi HAHC\",\"parentLocation\":{\"locationId\":\"620332e0-6108-4611-bac5-8b48d20051c9\",\"name\":\"ra Nchelenge\",\"parentLocation\":{\"locationId\":\"2e823ceb-4de6-41ac-8025-e2ae3512a331\",\"name\":\"ra Luapula\",\"serverVersion\":0,\"voided\":false},\"serverVersion\":0,\"voided\":false},\"tags\":[\"Village\"],\"serverVersion\":0,\"voided\":false},\"parent\":\"620332e0-6108-4611-bac5-8b48d20051c9\"}},\"id\":\"620332e0-6108-4611-bac5-8b48d20051c9\",\"label\":\"ra Nchelenge\",\"node\":{\"locationId\":\"620332e0-6108-4611-bac5-8b48d20051c9\",\"name\":\"ra Nchelenge\",\"parentLocation\":{\"locationId\":\"2e823ceb-4de6-41ac-8025-e2ae3512a331\",\"name\":\"ra Luapula\",\"parentLocation\":{\"locationId\":\"9c3e8715-1c59-44db-9709-2b49f440ef00\",\"name\":\"ra Zambia\",\"serverVersion\":0,\"voided\":false},\"serverVersion\":0,\"voided\":false},\"tags\":[\"District\"],\"serverVersion\":0,\"voided\":false},\"parent\":\"2e823ceb-4de6-41ac-8025-e2ae3512a331\"}},\"id\":\"2e823ceb-4de6-41ac-8025-e2ae3512a331\",\"label\":\"ra Luapula\",\"node\":{\"locationId\":\"2e823ceb-4de6-41ac-8025-e2ae3512a331\",\"name\":\"ra Luapula\",\"parentLocation\":{\"locationId\":\"9c3e8715-1c59-44db-9709-2b49f440ef00\",\"name\":\"ra Zambia\",\"serverVersion\":0,\"voided\":false},\"tags\":[\"Province\"],\"serverVersion\":0,\"voided\":false},\"parent\":\"9c3e8715-1c59-44db-9709-2b49f440ef00\"}},\"id\":\"9c3e8715-1c59-44db-9709-2b49f440ef00\",\"label\":\"ra Zambia\",\"node\":{\"locationId\":\"9c3e8715-1c59-44db-9709-2b49f440ef00\",\"name\":\"ra Zambia\",\"tags\":[\"Country\"],\"serverVersion\":0,\"voided\":false}}},\"parentChildren\":{\"9c3e8715-1c59-44db-9709-2b49f440ef00\":[\"2e823ceb-4de6-41ac-8025-e2ae3512a331\"],\"ed7c4a07-6e02-4784-ae9a-9cd41cfef390\":[\"1b0ba804-54c3-40ef-820b-a8eaffa5d054\"],\"620332e0-6108-4611-bac5-8b48d20051c9\":[\"ed7c4a07-6e02-4784-ae9a-9cd41cfef390\"],\"2e823ceb-4de6-41ac-8025-e2ae3512a331\":[\"620332e0-6108-4611-bac5-8b48d20051c9\"]}}}"; @@ -232,17 +233,12 @@ public void testGenerateDefaultLocationHierarchyWithRegisteredANMNotSetReturnsNu LocationHelper spyLocationHelper = Mockito.spy(locationHelper); AllSharedPreferences spiedAllSharedPreferences = Mockito.spy((AllSharedPreferences) ReflectionHelpers.getField(spyLocationHelper, "allSharedPreferences")); ReflectionHelpers.setField(spyLocationHelper, "allSharedPreferences", spiedAllSharedPreferences); - ANMLocationController anmLocationController = Mockito.spy(CoreLibrary.getInstance().context().anmLocationController()); ReflectionHelpers.setField(CoreLibrary.getInstance().context(), "anmLocationController", anmLocationController); - - Mockito.doReturn(anmLocation1) - .when(anmLocationController).get(); - + Mockito.doReturn(anmLocation1).when(anmLocationController).get(); List allowedLevels = Arrays.asList("District", "Village"); List result = spyLocationHelper.generateDefaultLocationHierarchy(allowedLevels); - - Mockito.verify(spiedAllSharedPreferences).fetchDefaultLocalityId(Mockito.eq("")); + Mockito.verify(spiedAllSharedPreferences).fetchDefaultLocalityId(ArgumentMatchers.anyString()); Mockito.verify(spyLocationHelper).getDefaultLocationHierarchy(Mockito.isNull(), Mockito.any(), Mockito.anyList(), Mockito.eq(allowedLevels), Mockito.eq(false)); assertNull(result); } From 77884ce636776bf17e9f66a7bba43f40143a418b Mon Sep 17 00:00:00 2001 From: SebaMutuku <36365043+SebaMutuku@users.noreply.github.com> Date: Tue, 2 Aug 2022 13:12:57 +0300 Subject: [PATCH 4/5] Improve test coverage --- .../smartregister/location/helper/LocationHelperTest.java | 1 + .../sync/helper/LocationServiceHelperTest.java | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java b/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java index 523e6803f..28db2ddc2 100644 --- a/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java +++ b/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java @@ -30,6 +30,7 @@ import org.smartregister.repository.AllSharedPreferences; import org.smartregister.repository.Repository; import org.smartregister.repository.SettingsRepository; +import org.smartregister.sync.helper.LocationServiceHelper; import org.smartregister.util.AssetHandler; import org.smartregister.view.controller.ANMLocationController; diff --git a/opensrp-core/src/test/java/org/smartregister/sync/helper/LocationServiceHelperTest.java b/opensrp-core/src/test/java/org/smartregister/sync/helper/LocationServiceHelperTest.java index 4404fb211..039e723e7 100644 --- a/opensrp-core/src/test/java/org/smartregister/sync/helper/LocationServiceHelperTest.java +++ b/opensrp-core/src/test/java/org/smartregister/sync/helper/LocationServiceHelperTest.java @@ -356,4 +356,12 @@ public void testGetLocationHierarchyReturnsTreeForValidParentLocationId() { public void tearDown() { ReflectionHelpers.setStaticField(CoreLibrary.class, "instance", null); } + @Test + public void testFetchNullLocations() { + Mockito.doReturn(new Response<>(ResponseStatus.success, "[]")) .when(httpAgent).fetch(stringArgumentCaptor.capture()); + locationServiceHelper.fetchAllLocations(); + verify(locationServiceHelper).fetchAllLocations(ArgumentMatchers.anyInt()); + String syncUrl = stringArgumentCaptor.getAllValues().get(0); + assertEquals("https://sample-stage.smartregister.org/opensrp//rest/location/getAll?is_jurisdiction=true&serverVersion=0&limit=2000", syncUrl); + } } \ No newline at end of file From c9ac7394765198882c54121542b97e281e006211 Mon Sep 17 00:00:00 2001 From: SebaMutuku <36365043+SebaMutuku@users.noreply.github.com> Date: Tue, 2 Aug 2022 13:13:47 +0300 Subject: [PATCH 5/5] Improve test coverage -1 --- .../org/smartregister/location/helper/LocationHelperTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java b/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java index 28db2ddc2..523e6803f 100644 --- a/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java +++ b/opensrp-core/src/test/java/org/smartregister/location/helper/LocationHelperTest.java @@ -30,7 +30,6 @@ import org.smartregister.repository.AllSharedPreferences; import org.smartregister.repository.Repository; import org.smartregister.repository.SettingsRepository; -import org.smartregister.sync.helper.LocationServiceHelper; import org.smartregister.util.AssetHandler; import org.smartregister.view.controller.ANMLocationController;