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

Bugfix Fix NPE FileDataStorageManager
This commit is contained in:
Alper Öztürk 2024-06-13 15:42:56 +02:00 committed by GitHub
commit a352271896
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 73 additions and 94 deletions

View file

@ -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();
/**

View file

@ -18,9 +18,9 @@ import android.content.Intent;
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 com.owncloud.android.lib.resources.users.GetUserInfoRemoteOperation;
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 class UserAccountManagerImpl implements UserAccountManager {
}
@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);
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) {
defaultAccount = Arrays.stream(ocAccounts)
.filter(account -> !arbitraryDataProvider.getBooleanValue(account.name, PENDING_FOR_REMOVAL))
.findFirst()
.orElse(null);
}
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 class UserAccountManagerImpl implements UserAccountManager {
* @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,7 +261,7 @@ public class UserAccountManagerImpl implements UserAccountManager {
}
@Override
@Nullable
@NonNull
public Account getAccountByName(String name) {
for (Account account : getAccounts()) {
if (account.name.equals(name)) {
@ -268,7 +269,7 @@ public class UserAccountManagerImpl implements UserAccountManager {
}
}
return null;
return getAnonymousAccount();
}
@Override

View file

@ -100,8 +100,7 @@ class AppModule {
@Provides
UserAccountManager userAccountManager(
Context context,
AccountManager accountManager
) {
AccountManager accountManager) {
return new UserAccountManagerImpl(context, accountManager);
}

View file

@ -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,36 +25,25 @@ 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()
}
}
@ -64,9 +51,12 @@ class SessionMixin constructor(
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()
}
}
}

View file

@ -0,0 +1,14 @@
/*
* Nextcloud - Android Client
*
* SPDX-FileCopyrightText: 2024 Your Name <your@email.com>
* 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))

View file

@ -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 abstract class BaseActivity extends AppCompatActivity implements Injectab
@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 abstract class BaseActivity extends AppCompatActivity implements Injectab
}
public FileDataStorageManager getStorageManager() {
return sessionMixin.getStorageManager();
return fileDataStorageManager;
}
}

View file

@ -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>

View file

@ -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()
}
}