Merge pull request #2850 from dimagi/updateFix

Fixes a bug causing Updates to get stale

Author: shubham1g5

app/src/org/commcare/heartbeat/HeartbeatWorker.kt

@@ -29,13 +29,14 @@ class HeartbeatWorker(context: Context, workerParams: WorkerParameters):
                     Result.retry()
                 }
                 else -> {
-                    Logger.log(LogTypes.TYPE_ERROR_SERVER_COMMS,
-                            "Encountered unexpected exception during heartbeat communications: "
-                                    + e.message + ". Stopping the heartbeat thread.")
+                    Logger.exception(
+                        "Encountered unexpected exception during heartbeat communications, stopping the heartbeat thread.",
+                        e
+                    )
                     Result.failure()
                 }
             }
         }
         return Result.success()
     }
-}
+}

app/src/org/commcare/update/UpdateHelper.java

@@ -18,7 +18,6 @@
 import org.commcare.resources.ResourceInstallContext;
 import org.commcare.resources.model.InstallCancelled;
 import org.commcare.resources.model.InstallCancelledException;
-import org.commcare.resources.model.InstallRequestSource;
 import org.commcare.resources.model.InvalidResourceException;
 import org.commcare.resources.model.Resource;
 import org.commcare.resources.model.ResourceTable;
@@ -84,7 +83,7 @@ public static UpdateHelper getNewInstance(boolean autoUpdate, UpdateProgressList
 
     // Main UpdateHelper function for staging updates
     public ResultAndError<AppInstallStatus> update(String profileRef, ResourceInstallContext resourceInstallContext) {
-        setupUpdate(profileRef);
+        setupUpdate(profileRef, resourceInstallContext);
 
         try {
             return new ResultAndError<>(stageUpdate(profileRef, resourceInstallContext));
@@ -123,10 +122,10 @@ public ResultAndError<AppInstallStatus> update(String profileRef, ResourceInstal
         }
     }
 
-    private void setupUpdate(String profileRef) {
+    private void setupUpdate(String profileRef, ResourceInstallContext resourceInstallContext) {
         ResourceInstallUtils.recordUpdateAttemptTime(mApp);
         Logger.log(LogTypes.TYPE_RESOURCES,
-                "Beginning install attempt for profile " + profileRef);
+                "Beginning install attempt as " + resourceInstallContext.getInstallRequestSource() + " for profile " + profileRef);
 
         if (isAutoUpdate) {
             ResourceInstallUtils.recordAutoUpdateStart(mApp);
@@ -150,7 +149,7 @@ private AppInstallStatus stageUpdate(String profileRef, ResourceInstallContext r
 
         AppInstallStatus result = mResourceManager.checkAndPrepareUpgradeResources(profileRefWithParams, mAuthority, resourceInstallContext);
 
-        if (result == AppInstallStatus.UpdateStaged) {
+        if (result == AppInstallStatus.UpdateStaged || result == AppInstallStatus.UpToDate) {
             RequestStats.markSuccess(resourceInstallContext.getInstallRequestSource());
         }
 

app/src/org/commcare/update/UpdateWorker.kt

@@ -44,31 +44,31 @@ class UpdateWorker(appContext: Context, workerParams: WorkerParameters)
                 when {
                     exception is CancellationException -> handleUpdateResult(ResultAndError(AppInstallStatus.Cancelled))
                     exception != null -> {
-                        Logger.exception("Unknown error while app update", exception);
-                        handleUpdateResult(ResultAndError(AppInstallStatus.UnknownFailure))
+                        Logger.exception("Unknown error while app update", exception)
+                        Result.failure()
                     }
                 }
             }
 
             job.await()
         }
-
     }
 
     private fun doUpdateWork(): Result {
-        val updateResult: ResultAndError<AppInstallStatus>
-
-        // skip if - An update task is already running | no app is seated | user session is not active
-        if (UpdateTask.getRunningInstance() == null &&
-                CommCareApplication.instance().currentApp != null &&
-                CommCareApplication.instance().session.isActive) {
-
-            updateHelper.startPinnedNotification(CommCareApplication.instance())
-            updateResult = updateHelper.update(ResourceInstallUtils.getDefaultProfileRef(),
-                    ResourceInstallContext(InstallRequestSource.BACKGROUND_UPDATE))
-        } else {
+        if (UpdateTask.getRunningInstance() != null) {
+            // there is already an update running, lets just skip this run
             return Result.success()
         }
+
+        if (CommCareApplication.instance().currentApp == null) {
+            // we need a seated app to update
+            return Result.failure()
+        }
+
+        updateHelper.startPinnedNotification(CommCareApplication.instance())
+        val updateResult: ResultAndError<AppInstallStatus> = updateHelper.update(
+            ResourceInstallUtils.getDefaultProfileRef(),
+            ResourceInstallContext(InstallRequestSource.BACKGROUND_UPDATE))
         return handleUpdateResult(updateResult)
     }
 
@@ -82,6 +82,7 @@ class UpdateWorker(appContext: Context, workerParams: WorkerParameters)
 
         return when {
             updateResult.data == AppInstallStatus.UpdateStaged -> Result.success()
+            updateResult.data == AppInstallStatus.UpToDate -> Result.success()
             updateResult.data.shouldRetryUpdate() -> Result.retry()
             else -> Result.failure()
         }

app/unit-tests/src/org/commcare/android/tests/application/AppUpdateTest.kt

@@ -82,7 +82,7 @@ class AppUpdateTest {
         UpdateUtils.installUpdate(profileRef,
                 AppInstallStatus.UpToDate,
                 AppInstallStatus.UnknownFailure)
-        checkUpdateComplete(6, true, false)
+        checkUpdateComplete(6, true, true)
     }
 
     @Test
@@ -191,4 +191,4 @@ class AppUpdateTest {
         private val TAG = AppUpdateTest::class.java.simpleName
         private const val REF_BASE_DIR = "jr://resource/commcare-apps/update_tests/"
     }
-}
+}

app/unit-tests/src/org/commcare/update/UpdateWorkerTest.kt

@@ -41,9 +41,8 @@ class UpdateWorkerTest {
     @Before
     fun setUp() {
         context = ApplicationProvider.getApplicationContext()
-        TestAppInstaller.installAppAndLogin(
-                UpdateUtils.buildResourceRef(REF_BASE_DIR, "base_app", "profile.ccpr"),
-                "test", "123")
+        TestAppInstaller.installApp(
+                UpdateUtils.buildResourceRef(REF_BASE_DIR, "base_app", "profile.ccpr"))
     }
 
     @Test
@@ -114,4 +113,4 @@ class UpdateWorkerTest {
                 .putString(PREFS_APP_SERVER_KEY, profileRef)
                 .apply()
     }
-}
+}