Skip to content

Commit

Permalink
Fix dependency injection for owncloud clients with no account creation
Browse files Browse the repository at this point in the history
  • Loading branch information
@davigonz
davigonz committed Feb 14, 2020
1 parent 2504a0f commit 1078d75
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 38 deletions.
2 changes: 1 addition & 1 deletion owncloud-android-library
Submodule owncloud-android-library updated 4 files
+12 −3 owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/OwnCloudAccount.java
+0 −2 owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/OwnCloudClientFactory.java
+1 −0 owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/accounts/AccountUtils.java
+2 −1 owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/server/FileService.kt
16 changes: 15 additions & 1 deletion owncloudApp/src/main/java/com/owncloud/android/authentication/AccountUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.owncloud.android.MainApp;
import com.owncloud.android.datamodel.FileDataStorageManager;
import com.owncloud.android.domain.capabilities.model.OCCapability;
import com.owncloud.android.lib.common.OwnCloudAccount;
import com.owncloud.android.lib.common.accounts.AccountTypeUtils;
import com.owncloud.android.lib.common.accounts.AccountUtils.Constants;
import com.owncloud.android.lib.resources.status.OwnCloudVersion;
Expand All @@ -38,12 +39,13 @@
import java.util.Locale;

import static com.owncloud.android.lib.common.OwnCloudClient.WEBDAV_PATH_4_0_AND_LATER;
import static com.owncloud.android.lib.common.accounts.AccountUtils.TEMPORAL_ACCOUNT_NAME;

public class AccountUtils {

private static final String ODAV_PATH = "/remote.php/odav";

public static final int ACCOUNT_VERSION = 1;
static final int ACCOUNT_VERSION = 1;

/**
* Can be used to get the currently selected ownCloud {@link Account} in the
Expand Down Expand Up @@ -81,6 +83,18 @@ public static Account getCurrentOwnCloudAccount(Context context) {
return defaultAccount;
}

/**
* Used for requests when there's no account yet, i.e. login process
*/
public static Account getCurrentOwnCloudAccountOrATemporalOne(Context context) {
Account currentAccount = getCurrentOwnCloudAccount(context);
if (currentAccount == null) {
return new Account(TEMPORAL_ACCOUNT_NAME, MainApp.Companion.getAccountType());
} else {
return currentAccount;
}
}

public static Account[] getAccounts(Context context) {
AccountManager accountManager = AccountManager.get(context);
return accountManager.getAccountsByType(MainApp.Companion.getAccountType());
Expand Down
5 changes: 5 additions & 0 deletions owncloudApp/src/main/java/com/owncloud/android/authentication/AuthenticatorActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,11 @@ private void checkOcServer() {
mServerStatusIcon = R.drawable.progress_small;
showServerStatus();

SharedPreferences.Editor editor = PreferenceManager
.getDefaultSharedPreferences(this).edit();
editor.putString("server_url", normalizeUrlSuffix(uri));
editor.apply();

Intent getServerInfoIntent = new Intent();
getServerInfoIntent.setAction(OperationsService.ACTION_GET_SERVER_INFO);
getServerInfoIntent.putExtra(OperationsService.EXTRA_SERVER_URL, normalizeUrlSuffix(uri));
Expand Down
4 changes: 4 additions & 0 deletions owncloudApp/src/main/java/com/owncloud/android/authentication/AuthenticatorAsyncTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,12 @@ protected RemoteOperationResult doInBackground(Object... params) {
credentials = (OwnCloudCredentials) params[1];
}

mUseCaseHelper.checkPathExistence(url);
mUseCaseHelper.getServerInfo(url);

// Client
Uri uri = Uri.parse(url);

OwnCloudClient client = OwnCloudClientFactory.createOwnCloudClient(uri, mContext, true);
client.setCredentials(credentials);

Expand Down
8 changes: 4 additions & 4 deletions owncloudApp/src/main/java/com/owncloud/android/dependecyinjection/RemoteDataSourceModule.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import com.owncloud.android.data.server.datasources.RemoteAnonymousDatasource
import com.owncloud.android.data.server.datasources.RemoteServerDataSource
import com.owncloud.android.data.server.datasources.implementation.OCRemoteAnonymousDataSource
import com.owncloud.android.data.server.datasources.implementation.OCRemoteServerDataSource
import com.owncloud.android.data.server.network.OCAnonymousServerService
import com.owncloud.android.data.server.network.OCFileService
import com.owncloud.android.data.server.network.OCServerService
import com.owncloud.android.data.sharing.shares.network.OCShareService
import com.owncloud.android.data.sharing.sharees.datasources.RemoteShareeDataSource
Expand All @@ -42,7 +42,7 @@ import com.owncloud.android.data.user.datasources.implementation.OCRemoteUserDat
import com.owncloud.android.data.user.network.OCUserService
import com.owncloud.android.lib.common.OwnCloudAccount
import com.owncloud.android.lib.common.SingleSessionManager
import com.owncloud.android.lib.resources.server.AnonymousService
import com.owncloud.android.lib.resources.server.FileService
import com.owncloud.android.lib.resources.server.ServerService
import com.owncloud.android.lib.resources.shares.ShareService
import com.owncloud.android.lib.resources.shares.ShareeService
Expand All @@ -52,7 +52,7 @@ import org.koin.android.ext.koin.androidContext
import org.koin.dsl.module

val remoteDataSourceModule = module {
single { AccountUtils.getCurrentOwnCloudAccount(androidContext()) }
single { AccountUtils.getCurrentOwnCloudAccountOrATemporalOne(androidContext()) }
single { OwnCloudAccount(get(), androidContext()) }
single { SingleSessionManager.getDefaultSingleton().getClientFor(get(), androidContext()) }

Expand All @@ -61,7 +61,7 @@ val remoteDataSourceModule = module {
single<ShareeService> { OCShareeService(get()) }
single<UserService> { OCUserService(get()) }
single<ServerService> { OCServerService(get()) }
single<AnonymousService>{ OCAnonymousServerService()}
single<FileService>{ OCFileService(get())}

factory<RemoteCapabilitiesDataSource> {
OCRemoteCapabilitiesDataSource(
Expand Down
10 changes: 5 additions & 5 deletions ...om/owncloud/android/data/server/datasources/implementation/OCRemoteAnonymousDataSource.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,20 @@ import com.owncloud.android.data.server.datasources.RemoteAnonymousDatasource
import com.owncloud.android.domain.server.model.AuthenticationMethod
import com.owncloud.android.lib.common.http.HttpConstants
import com.owncloud.android.lib.common.operations.RemoteOperationResult
import com.owncloud.android.lib.resources.server.AnonymousService
import com.owncloud.android.lib.resources.server.FileService
import com.owncloud.android.lib.resources.status.OwnCloudVersion

class OCRemoteAnonymousDataSource(
private val anonymousService: AnonymousService
private val fileService: FileService
) : RemoteAnonymousDatasource {

/* Basically, tries to access to the root folder without authorization and analyzes the response.*/
override fun getAuthenticationMethod(path: String): AuthenticationMethod {
// Step 1: check whether the root folder exists, following redirections
var checkPathExistenceResult = anonymousService.checkPathExistence(path, isUserLogged = false)
var checkPathExistenceResult = fileService.checkPathExistence(path, isUserLogged = false)
var redirectionLocation = checkPathExistenceResult.redirectedLocation
while (!redirectionLocation.isNullOrEmpty()) {
checkPathExistenceResult = anonymousService.checkPathExistence(redirectionLocation, isUserLogged = false)
checkPathExistenceResult = fileService.checkPathExistence(redirectionLocation, isUserLogged = false)
redirectionLocation = checkPathExistenceResult.redirectedLocation
}

Expand All @@ -55,7 +55,7 @@ class OCRemoteAnonymousDataSource(
}

override fun getRemoteStatus(path: String): Pair<OwnCloudVersion, Boolean> {
val remoteStatusResult = anonymousService.getRemoteStatus(path)
val remoteStatusResult = fileService.getRemoteStatus(path)

val ownCloudVersion = executeRemoteOperation {
remoteStatusResult
Expand Down
13 changes: 4 additions & 9 deletions ...erver/network/OCAnonymousServerService.kt → ...roid/data/server/network/OCFileService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,20 @@

package com.owncloud.android.data.server.network

import android.net.Uri
import com.owncloud.android.lib.common.OwnCloudClient
import com.owncloud.android.lib.common.operations.RemoteOperationResult
import com.owncloud.android.lib.resources.server.AnonymousService
import com.owncloud.android.lib.resources.server.CheckPathExistenceRemoteOperation
import com.owncloud.android.lib.resources.server.FileService
import com.owncloud.android.lib.resources.server.GetRemoteStatusOperation
import com.owncloud.android.lib.resources.status.OwnCloudVersion

class OCAnonymousServerService : AnonymousService {
class OCFileService(override val client: OwnCloudClient) : FileService {
override fun checkPathExistence(path: String, isUserLogged: Boolean): RemoteOperationResult<Boolean> =
CheckPathExistenceRemoteOperation(
remotePath = path,
isUserLogged = true
).execute(createClientFromPath(path))
).execute(client)

override fun getRemoteStatus(path: String): RemoteOperationResult<OwnCloudVersion> =
GetRemoteStatusOperation().execute(createClientFromPath(path))

private fun createClientFromPath(path: String): OwnCloudClient {
return OwnCloudClient(Uri.parse(path))
}
GetRemoteStatusOperation().execute(client)
}
36 changes: 18 additions & 18 deletions ...test/java/com/owncloud/android/data/server/datasources/OCRemoteAnonymousDatasourceTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
package com.owncloud.android.data.server.datasources

import com.owncloud.android.data.server.datasources.implementation.OCRemoteAnonymousDataSource
import com.owncloud.android.data.server.network.OCAnonymousServerService
import com.owncloud.android.data.server.network.OCFileService
import com.owncloud.android.domain.server.model.AuthenticationMethod
import com.owncloud.android.lib.common.http.HttpConstants.HTTP_UNAUTHORIZED
import com.owncloud.android.lib.common.operations.RemoteOperationResult
Expand All @@ -40,7 +40,7 @@ import org.junit.Test
class OCRemoteAnonymousDatasourceTest {
private lateinit var ocRemoteAnonymousDatasource: OCRemoteAnonymousDataSource

private val ocAnonymousService: OCAnonymousServerService = mockk()
private val ocFileService: OCFileService = mockk()

private val OC_OWNCLOUD_VERSION = OwnCloudVersion("10.3.2")
private val basicAuthHeader = "basic realm=\"owncloud\", charset=\"utf-8\""
Expand All @@ -49,7 +49,7 @@ class OCRemoteAnonymousDatasourceTest {

@Before
fun init() {
ocRemoteAnonymousDatasource = OCRemoteAnonymousDataSource(ocAnonymousService)
ocRemoteAnonymousDatasource = OCRemoteAnonymousDataSource(ocFileService)
}

@Test
Expand All @@ -70,19 +70,19 @@ class OCRemoteAnonymousDatasourceTest {
)

every {
ocAnonymousService.checkPathExistence(redirectedLocation, false)
ocFileService.checkPathExistence(redirectedLocation, false)
} returns checkPathExistenceResultFollowRedirectionMocked

every {
ocAnonymousService.checkPathExistence(OC_ServerInfo.baseUrl, false)
ocFileService.checkPathExistence(OC_ServerInfo.baseUrl, false)
} returns checkPathExistenceResultMocked

val authenticationMethod = ocRemoteAnonymousDatasource.getAuthenticationMethod(redirectedLocation)

assertNotNull(authenticationMethod)
assertEquals(AuthenticationMethod.BASIC_HTTP_AUTH, authenticationMethod)

verify { ocAnonymousService.checkPathExistence(OC_ServerInfo.baseUrl, false) }
verify { ocFileService.checkPathExistence(OC_ServerInfo.baseUrl, false) }
}

@Test
Expand All @@ -98,15 +98,15 @@ class OCRemoteAnonymousDatasourceTest {
)

every {
ocAnonymousService.checkPathExistence(OC_ServerInfo.baseUrl, false)
ocFileService.checkPathExistence(OC_ServerInfo.baseUrl, false)
} returns checkPathExistenceResultMocked

val authenticationMethod = ocRemoteAnonymousDatasource.getAuthenticationMethod(OC_ServerInfo.baseUrl)

assertNotNull(authenticationMethod)
assertEquals(AuthenticationMethod.BASIC_HTTP_AUTH, authenticationMethod)

verify { ocAnonymousService.checkPathExistence(OC_ServerInfo.baseUrl, false) }
verify { ocFileService.checkPathExistence(OC_ServerInfo.baseUrl, false) }
}

@Test
Expand All @@ -122,29 +122,29 @@ class OCRemoteAnonymousDatasourceTest {
)

every {
ocAnonymousService.checkPathExistence(OC_ServerInfo.baseUrl, false)
ocFileService.checkPathExistence(OC_ServerInfo.baseUrl, false)
} returns checkPathExistenceResultMocked

val authenticationMethod = ocRemoteAnonymousDatasource.getAuthenticationMethod(OC_ServerInfo.baseUrl)

assertNotNull(authenticationMethod)
assertEquals(AuthenticationMethod.BEARER_TOKEN, authenticationMethod)

verify { ocAnonymousService.checkPathExistence(OC_ServerInfo.baseUrl, false) }
verify { ocFileService.checkPathExistence(OC_ServerInfo.baseUrl, false) }
}

@Test(expected = Exception::class)
fun getAuthenticationMethodException() {
every {
ocAnonymousService.checkPathExistence(OC_ServerInfo.baseUrl, false)
ocFileService.checkPathExistence(OC_ServerInfo.baseUrl, false)
} throws Exception()

val authenticationMethod = ocRemoteAnonymousDatasource.getAuthenticationMethod(OC_ServerInfo.baseUrl)

assertNotNull(authenticationMethod)
assertEquals(AuthenticationMethod.BASIC_HTTP_AUTH, authenticationMethod)

verify { ocAnonymousService.checkPathExistence(OC_ServerInfo.baseUrl, false) }
verify { ocFileService.checkPathExistence(OC_ServerInfo.baseUrl, false) }
}

@Test
Expand All @@ -153,15 +153,15 @@ class OCRemoteAnonymousDatasourceTest {
createRemoteOperationResultMock(data = OC_OWNCLOUD_VERSION, isSuccess = true, resultCode = OK_SSL)

every {
ocAnonymousService.getRemoteStatus(OC_ServerInfo.baseUrl)
ocFileService.getRemoteStatus(OC_ServerInfo.baseUrl)
} returns remoteStatusResultMocked

val remoteStatus = ocRemoteAnonymousDatasource.getRemoteStatus(OC_ServerInfo.baseUrl)

assertNotNull(remoteStatus)
assertEquals(Pair(OC_OWNCLOUD_VERSION, true), remoteStatus)

verify { ocAnonymousService.getRemoteStatus(OC_ServerInfo.baseUrl) }
verify { ocFileService.getRemoteStatus(OC_ServerInfo.baseUrl) }
}

@Test
Expand All @@ -170,28 +170,28 @@ class OCRemoteAnonymousDatasourceTest {
createRemoteOperationResultMock(data = OC_OWNCLOUD_VERSION, isSuccess = true, resultCode = OK_NO_SSL)

every {
ocAnonymousService.getRemoteStatus(OC_ServerInfo.baseUrl)
ocFileService.getRemoteStatus(OC_ServerInfo.baseUrl)
} returns remoteStatusResultMocked

val remoteStatus = ocRemoteAnonymousDatasource.getRemoteStatus(OC_ServerInfo.baseUrl)

assertNotNull(remoteStatus)
assertEquals(Pair(OC_OWNCLOUD_VERSION, false), remoteStatus)

verify { ocAnonymousService.getRemoteStatus(OC_ServerInfo.baseUrl) }
verify { ocFileService.getRemoteStatus(OC_ServerInfo.baseUrl) }
}

@Test(expected = Exception::class)
fun getRemoteStatusException() {
every {
ocAnonymousService.getRemoteStatus(OC_ServerInfo.baseUrl)
ocFileService.getRemoteStatus(OC_ServerInfo.baseUrl)
} throws Exception()

val remoteStatus = ocRemoteAnonymousDatasource.getRemoteStatus(OC_ServerInfo.baseUrl)

assertNotNull(remoteStatus)
assertEquals(Pair(OC_OWNCLOUD_VERSION, true), remoteStatus)

verify { ocAnonymousService.getRemoteStatus(OC_ServerInfo.baseUrl) }
verify { ocFileService.getRemoteStatus(OC_ServerInfo.baseUrl) }
}
}

0 comments on commit 1078d75

Please sign in to comment.