Merge pull request #13074 from nextcloud/bugfix/fix-npe-FileDataStora…

…geManager

Bugfix Fix NPE FileDataStorageManager

app/src/main/java/com/nextcloud/client/account/CurrentAccountProvider.java

@@ -24,7 +24,7 @@ public interface CurrentAccountProvider {
      * @return Currently selected {@link Account} or first valid {@link Account} registered in OS or null, if not available at all.
      */
     @Deprecated
-    @Nullable
+    @NonNull
     Account getCurrentAccount();
 
     /**

app/src/main/java/com/nextcloud/client/account/UserAccountManagerImpl.java

@@ -18,9 +18,9 @@
 import android.content.SharedPreferences;
 import android.preference.PreferenceManager;
 import android.text.TextUtils;
-import android.util.Log;
 
 import com.nextcloud.common.NextcloudClient;
+import com.nextcloud.utils.extensions.AccountExtensionsKt;
 import com.owncloud.android.MainApp;
 import com.owncloud.android.R;
 import com.owncloud.android.authentication.AuthenticatorActivity;
@@ -39,6 +39,7 @@
 import java.io.IOException;
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
 
@@ -132,42 +133,42 @@ public boolean exists(Account account) {
     }
 
     @Override
-    @Nullable
+    @NonNull
     public Account getCurrentAccount() {
         Account[] ocAccounts = getAccounts();
-        Account defaultAccount = null;
 
         ArbitraryDataProvider arbitraryDataProvider = new ArbitraryDataProviderImpl(context);
-
         SharedPreferences appPreferences = PreferenceManager.getDefaultSharedPreferences(context);
         String accountName = appPreferences.getString(PREF_SELECT_OC_ACCOUNT, null);
 
-        // account validation: the saved account MUST be in the list of ownCloud Accounts known by the AccountManager
-        if (accountName != null) {
-            for (Account account : ocAccounts) {
-                if (account.name.equals(accountName)) {
-                    defaultAccount = account;
-                    break;
-                }
-            }
+        Account defaultAccount = Arrays.stream(ocAccounts)
+            .filter(account -> account.name.equals(accountName))
+            .findFirst()
+            .orElse(null);
+
+        // take first which is not pending for removal account as fallback
+        if (defaultAccount == null) {
+            defaultAccount = Arrays.stream(ocAccounts)
+                .filter(account -> !arbitraryDataProvider.getBooleanValue(account.name, PENDING_FOR_REMOVAL))
+                .findFirst()
+                .orElse(null);
         }
 
-        if (defaultAccount == null && ocAccounts.length > 0) {
-            // take first which is not pending for removal account as fallback
-            for (Account account: ocAccounts) {
-                boolean pendingForRemoval = arbitraryDataProvider.getBooleanValue(account.name,
-                                                                                  PENDING_FOR_REMOVAL);
-
-                if (!pendingForRemoval) {
-                    defaultAccount = account;
-                    break;
-                }
+        if (defaultAccount == null) {
+            if (ocAccounts.length > 0) {
+                defaultAccount = ocAccounts[0];
+            } else {
+                defaultAccount = getAnonymousAccount();
             }
         }
 
         return defaultAccount;
     }
 
+    private Account getAnonymousAccount() {
+        return new Account("Anonymous", context.getString(R.string.anonymous_account_type));
+    }
+
     /**
      * Temporary solution to convert platform account to user instance.
      * It takes null and returns null on error to ease error handling
@@ -177,8 +178,8 @@ public Account getCurrentAccount() {
      * @return User instance or null, if conversion failed
      */
     @Nullable
-    private User createUserFromAccount(@Nullable Account account) {
-        if (account == null) {
+    private User createUserFromAccount(@NonNull Account account) {
+        if (AccountExtensionsKt.isAnonymous(account, context)) {
             return null;
         }
 
@@ -260,15 +261,15 @@ public OwnCloudAccount getCurrentOwnCloudAccount() {
     }
 
     @Override
-    @Nullable
+    @NonNull
     public Account getAccountByName(String name) {
         for (Account account : getAccounts()) {
             if (account.name.equals(name)) {
                 return account;
             }
         }
 
-        return null;
+        return getAnonymousAccount();
     }
 
     @Override

app/src/main/java/com/nextcloud/client/di/AppModule.java

@@ -100,8 +100,7 @@ Resources resources(Application application) {
     @Provides
     UserAccountManager userAccountManager(
         Context context,
-        AccountManager accountManager
-                                         ) {
+        AccountManager accountManager) {
         return new UserAccountManagerImpl(context, accountManager);
     }
 

app/src/main/java/com/nextcloud/client/mixins/SessionMixin.kt

@@ -9,14 +9,12 @@ package com.nextcloud.client.mixins
 
 import android.accounts.Account
 import android.app.Activity
-import android.content.ContentResolver
 import android.content.Intent
 import android.os.Bundle
 import com.nextcloud.client.account.User
 import com.nextcloud.client.account.UserAccountManager
-import com.owncloud.android.datamodel.FileDataStorageManager
+import com.nextcloud.utils.extensions.isAnonymous
 import com.owncloud.android.lib.resources.status.OCCapability
-import com.owncloud.android.ui.activity.BaseActivity
 import com.owncloud.android.utils.theme.CapabilityUtils
 import java.util.Optional
 
@@ -27,46 +25,38 @@ import java.util.Optional
  * It is an intermediary step facilitating comprehensive rework of
  * account handling logic.
  */
-class SessionMixin constructor(
+class SessionMixin(
     private val activity: Activity,
-    private val contentResolver: ContentResolver,
     private val accountManager: UserAccountManager
 ) : ActivityMixin {
-
-    private companion object {
-        private val TAG = BaseActivity::class.java.simpleName
-    }
-
-    var currentAccount: Account? = null
-        private set
-    var storageManager: FileDataStorageManager? = null
+    lateinit var currentAccount: Account
         private set
+
     val capabilities: OCCapability?
         get() = getUser()
             .map { CapabilityUtils.getCapability(it, activity) }
             .orElse(null)
 
-    fun setAccount(account: Account?) {
-        val validAccount = account != null && accountManager.setCurrentOwnCloudAccount(account.name)
-        if (validAccount) {
-            currentAccount = account
-        } else {
-            swapToDefaultAccount()
-        }
+    fun setAccount(account: Account) {
+        val validAccount = (accountManager.setCurrentOwnCloudAccount(account.name))
 
-        currentAccount?.let {
-            val storageManager = FileDataStorageManager(getUser().get(), contentResolver)
-            this.storageManager = storageManager
+        currentAccount = if (validAccount) {
+            account
+        } else {
+            getDefaultAccount()
         }
     }
 
     fun setUser(user: User) {
         setAccount(user.toPlatformAccount())
     }
 
-    fun getUser(): Optional<User> = when (val it = this.currentAccount) {
-        null -> Optional.empty()
-        else -> accountManager.getUser(it.name)
+    fun getUser(): Optional<User> {
+        return if (currentAccount.isAnonymous(activity)) {
+            Optional.empty()
+        } else {
+            accountManager.getUser(currentAccount.name)
+        }
     }
 
     /**
@@ -75,16 +65,14 @@ class SessionMixin constructor(
      * If no valid ownCloud [Account] exists, then the user is requested
      * to create a new ownCloud [Account].
      */
-    private fun swapToDefaultAccount() {
-        // default to the most recently used account
-        val newAccount = accountManager.currentAccount
-        if (newAccount == null) {
-            // no account available: force account creation
-            currentAccount = null
+    private fun getDefaultAccount(): Account {
+        val defaultAccount = accountManager.currentAccount
+
+        if (defaultAccount.isAnonymous(activity)) {
             startAccountCreation()
-        } else {
-            currentAccount = newAccount
         }
+
+        return defaultAccount
     }
 
     /**
@@ -98,7 +86,7 @@ class SessionMixin constructor(
         super.onNewIntent(intent)
         val current = accountManager.currentAccount
         val currentAccount = this.currentAccount
-        if (current != null && currentAccount != null && !currentAccount.name.equals(current.name)) {
+        if (!currentAccount.name.equals(current.name)) {
             this.currentAccount = current
         }
     }
@@ -109,9 +97,9 @@ class SessionMixin constructor(
      */
     override fun onRestart() {
         super.onRestart()
-        val validAccount = currentAccount != null && accountManager.exists(currentAccount)
+        val validAccount = accountManager.exists(currentAccount)
         if (!validAccount) {
-            swapToDefaultAccount()
+            getDefaultAccount()
         }
     }
 
@@ -120,11 +108,4 @@ class SessionMixin constructor(
         val account = accountManager.currentAccount
         setAccount(account)
     }
-
-    override fun onResume() {
-        super.onResume()
-        if (currentAccount == null) {
-            swapToDefaultAccount()
-        }
-    }
 }

app/src/main/java/com/nextcloud/utils/extensions/AccountExtensions.kt

@@ -0,0 +1,14 @@
+/*
+ * Nextcloud - Android Client
+ *
+ * SPDX-FileCopyrightText: 2024 Your Name <[email protected]>
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+package com.nextcloud.utils.extensions
+
+import android.accounts.Account
+import android.content.Context
+import com.owncloud.android.R
+
+fun Account.isAnonymous(context: Context): Boolean = type.equals(context.getString(R.string.anonymous_account_type))

app/src/main/java/com/owncloud/android/ui/activity/BaseActivity.java

@@ -48,6 +48,7 @@ public abstract class BaseActivity extends AppCompatActivity implements Injectab
 
     @Inject UserAccountManager accountManager;
     @Inject AppPreferences preferences;
+    @Inject FileDataStorageManager fileDataStorageManager;
 
     private AppPreferences.Listener onPreferencesChanged = new AppPreferences.Listener() {
         @Override
@@ -63,9 +64,7 @@ public UserAccountManager getUserAccountManager() {
     @Override
     protected void onCreate(@Nullable Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
-        sessionMixin = new SessionMixin(this,
-                                        getContentResolver(),
-                                        accountManager);
+        sessionMixin = new SessionMixin(this, accountManager);
         mixinRegistry.add(sessionMixin);
 
         if (enableAccountHandling) {
@@ -174,6 +173,6 @@ public Optional<User> getUser() {
     }
 
     public FileDataStorageManager getStorageManager() {
-        return sessionMixin.getStorageManager();
+        return fileDataStorageManager;
     }
 }

app/src/main/res/values/strings.xml

@@ -30,6 +30,8 @@
     <string name="app_config_proxy_host_title">Proxy Host Name</string>
     <string name="app_config_proxy_port_title">Proxy Port</string>
 
+    <string name="anonymous_account_type" translatable="false">AnonymousAccountType</string>
+
     <string name="assistant_screen_task_types_error_state_message">Unable to fetch task types, please check your internet connection.</string>
     <string name="assistant_screen_task_list_error_state_message">Unable to fetch task list, please check your internet connection.</string>
 

app/src/test/java/com/nextcloud/client/mixins/SessionMixinTest.kt

@@ -8,9 +8,7 @@
 package com.nextcloud.client.mixins
 
 import android.app.Activity
-import android.content.ContentResolver
 import com.nextcloud.client.account.UserAccountManager
-import junit.framework.Assert.assertNull
 import org.junit.Before
 import org.junit.Test
 import org.mockito.Mock
@@ -24,9 +22,6 @@ class SessionMixinTest {
     @Mock
     private lateinit var activity: Activity
 
-    @Mock
-    private lateinit var contentResolver: ContentResolver
-
     @Mock
     private lateinit var userAccountManager: UserAccountManager
 
@@ -38,7 +33,6 @@ class SessionMixinTest {
         session = spy(
             SessionMixin(
                 activity,
-                contentResolver,
                 userAccountManager
             )
         )
@@ -55,15 +49,4 @@ class SessionMixinTest {
         //      account manager receives parent activity
         verify(userAccountManager).startAccountCreation(same(activity))
     }
-
-    @Test
-    fun `trigger accountCreation on resume when currentAccount is null`() {
-        // WHEN
-        //      start onResume and currentAccount is null
-        assertNull(session.currentAccount)
-        session.onResume()
-        // THEN
-        //      accountCreation flow is started
-        verify(session).startAccountCreation()
-    }
 }