From 1ba288e5e68a1918baffda2cec45fe6a4b310333 Mon Sep 17 00:00:00 2001
From: Chris Narkiewicz <hello@ezaquarii.com>
Date: Wed, 4 Mar 2020 06:07:42 +0000
Subject: [PATCH] Properly signal error when user id migration fails

Previous implementation was returning success status
when *any* account was migrated, despite possible
failures.

Return error when any account fails to be migrated.

The migration remines idempotent and can be re-run
again, until if finally succeeds.

Signed-off-by: Chris Narkiewicz <hello@ezaquarii.com>
---
 .../com/nextcloud/client/account/UserAccountManager.java | 6 ++++--
 .../nextcloud/client/account/UserAccountManagerImpl.java | 9 ++++-----
 .../java/com/nextcloud/client/migrations/Migrations.kt   | 6 +++++-
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/main/java/com/nextcloud/client/account/UserAccountManager.java b/src/main/java/com/nextcloud/client/account/UserAccountManager.java
index 1281ec50f5..2c3de3779a 100644
--- a/src/main/java/com/nextcloud/client/account/UserAccountManager.java
+++ b/src/main/java/com/nextcloud/client/account/UserAccountManager.java
@@ -89,9 +89,11 @@ public interface UserAccountManager extends CurrentAccountProvider {
     boolean exists(Account account);
 
     /**
-     * Verifies that every account has userId set
+     * Verifies that every account has userId set and sets the user id if not.
+     * This migration is idempotent and can be run multiple times until
+     * all accounts are migrated.
      *
-     * @return true if any account was migrated, false if no account was migrated
+     * @return true if migration was successful, false if any account failed to be migrated
      */
     boolean migrateUserId();
 
diff --git a/src/main/java/com/nextcloud/client/account/UserAccountManagerImpl.java b/src/main/java/com/nextcloud/client/account/UserAccountManagerImpl.java
index 49d695a2f7..4784b94412 100644
--- a/src/main/java/com/nextcloud/client/account/UserAccountManagerImpl.java
+++ b/src/main/java/com/nextcloud/client/account/UserAccountManagerImpl.java
@@ -342,12 +342,11 @@ public class UserAccountManagerImpl implements UserAccountManager {
     }
 
     public boolean migrateUserId() {
-        boolean success = false;
         Account[] ocAccounts = accountManager.getAccountsByType(MainApp.getAccountType(context));
         String userId;
         String displayName;
         GetUserInfoRemoteOperation remoteUserNameOperation = new GetUserInfoRemoteOperation();
-
+        int failed = 0;
         for (Account account : ocAccounts) {
             String storedUserId = accountManager.getUserData(account, com.owncloud.android.lib.common.accounts.AccountUtils.Constants.KEY_USER_ID);
 
@@ -370,10 +369,12 @@ public class UserAccountManagerImpl implements UserAccountManager {
                 } else {
                     // skip account, try it next time
                     Log_OC.e(TAG, "Error while getting username for account: " + account.name);
+                    failed++;
                     continue;
                 }
             } catch (Exception e) {
                 Log_OC.e(TAG, "Error while getting username: " + e.getMessage());
+                failed++;
                 continue;
             }
 
@@ -383,11 +384,9 @@ public class UserAccountManagerImpl implements UserAccountManager {
             accountManager.setUserData(account,
                                        com.owncloud.android.lib.common.accounts.AccountUtils.Constants.KEY_USER_ID,
                                        userId);
-
-            success = true;
         }
 
-        return success;
+        return failed == 0;
     }
 
     private String getAccountType() {
diff --git a/src/main/java/com/nextcloud/client/migrations/Migrations.kt b/src/main/java/com/nextcloud/client/migrations/Migrations.kt
index dce6eac4fc..366e09fcb3 100644
--- a/src/main/java/com/nextcloud/client/migrations/Migrations.kt
+++ b/src/main/java/com/nextcloud/client/migrations/Migrations.kt
@@ -21,6 +21,7 @@ package com.nextcloud.client.migrations
 
 import com.nextcloud.client.account.UserAccountManager
 import javax.inject.Inject
+import kotlin.IllegalStateException
 
 /**
  * This class collects all migration steps and provides API to supply those
@@ -50,6 +51,9 @@ class Migrations @Inject constructor(
     ).sortedBy { it.id }
 
     fun migrateUserId() {
-        userAccountManager.migrateUserId()
+        val allAccountsHaveUserId = userAccountManager.migrateUserId()
+        if (!allAccountsHaveUserId) {
+            throw IllegalStateException("Failed to set user id for all accounts")
+        }
     }
 }