From f5fe254c0929a0e225ad3806082e0e1a8cfb8803 Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Mon, 9 Feb 2015 15:27:44 +0100 Subject: [PATCH 1/3] Load of credentials from AccountManager is refactored out from constructor --- .../android/lib/common/OwnCloudAccount.java | 53 ++++++++++++++++--- .../lib/common/OwnCloudClientManager.java | 3 +- .../lib/common/SimpleFactoryManager.java | 28 ++++++---- .../lib/common/SingleSessionManager.java | 42 +++++++-------- 4 files changed, 88 insertions(+), 38 deletions(-) diff --git a/src/com/owncloud/android/lib/common/OwnCloudAccount.java b/src/com/owncloud/android/lib/common/OwnCloudAccount.java index 339eb128..1b339af8 100644 --- a/src/com/owncloud/android/lib/common/OwnCloudAccount.java +++ b/src/com/owncloud/android/lib/common/OwnCloudAccount.java @@ -41,9 +41,43 @@ public class OwnCloudAccount { private OwnCloudCredentials mCredentials; private String mSavedAccountName; - - - public OwnCloudAccount(Account savedAccount, Context context) + + private Account mSavedAccount; + + + /** + * Constructor for already saved OC accounts. + * + * Do not use for anonymous credentials. + */ + public OwnCloudAccount(Account savedAccount, Context context) throws AccountNotFoundException { + if (savedAccount == null) { + throw new IllegalArgumentException("Parameter 'savedAccount' cannot be null"); + } + mSavedAccount = savedAccount; + mSavedAccountName = savedAccount.name; + mBaseUri = Uri.parse(AccountUtils.getBaseUrlForAccount(context, mSavedAccount)); + mCredentials = null; + } + + /** + * Method for deferred load of account attributes from AccountManager + * + * @param context + * @throws AccountNotFoundException + * @throws AuthenticatorException + * @throws IOException + * @throws OperationCanceledException + */ + public void loadCredentials(Context context) + throws AccountNotFoundException, AuthenticatorException, + IOException, OperationCanceledException { + + mCredentials = AccountUtils.getCredentialsForAccount(context, mSavedAccount); + } + + /* + public OwnCloudAccount(Account savedAccount, Context context) throws AccountNotFoundException, AuthenticatorException, IOException, OperationCanceledException { @@ -61,12 +95,19 @@ public class OwnCloudAccount { mCredentials = OwnCloudCredentialsFactory.getAnonymousCredentials(); } } - - + */ + + /** + * Constructor for non yet saved OC accounts. + * + * @param baseUri URI to the OC server to get access to. + * @param credentials Credentials to authenticate in the server. NULL is valid for anonymous credentials. + */ public OwnCloudAccount(Uri baseUri, OwnCloudCredentials credentials) { if (baseUri == null) { throw new IllegalArgumentException("Parameter 'baseUri' cannot be null"); } + mSavedAccount = null; mSavedAccountName = null; mBaseUri = baseUri; mCredentials = credentials != null ? @@ -80,7 +121,7 @@ public class OwnCloudAccount { public boolean isAnonymous() { return (mCredentials == null); - } + } // TODO no more public Uri getBaseUri() { return mBaseUri; diff --git a/src/com/owncloud/android/lib/common/OwnCloudClientManager.java b/src/com/owncloud/android/lib/common/OwnCloudClientManager.java index e54ae191..b5df543f 100644 --- a/src/com/owncloud/android/lib/common/OwnCloudClientManager.java +++ b/src/com/owncloud/android/lib/common/OwnCloudClientManager.java @@ -42,7 +42,8 @@ import com.owncloud.android.lib.common.accounts.AccountUtils.AccountNotFoundExce public interface OwnCloudClientManager { - public OwnCloudClient getClientFor(OwnCloudAccount account, Context context); + public OwnCloudClient getClientFor(OwnCloudAccount account, Context context) + throws AccountNotFoundException, OperationCanceledException, AuthenticatorException, IOException; public OwnCloudClient removeClientFor(OwnCloudAccount account); diff --git a/src/com/owncloud/android/lib/common/SimpleFactoryManager.java b/src/com/owncloud/android/lib/common/SimpleFactoryManager.java index 1cd89f8a..88e27feb 100644 --- a/src/com/owncloud/android/lib/common/SimpleFactoryManager.java +++ b/src/com/owncloud/android/lib/common/SimpleFactoryManager.java @@ -25,32 +25,42 @@ package com.owncloud.android.lib.common; +import android.accounts.AuthenticatorException; +import android.accounts.OperationCanceledException; import android.content.Context; import com.owncloud.android.lib.common.accounts.AccountUtils; +import com.owncloud.android.lib.common.accounts.AccountUtils.AccountNotFoundException; import com.owncloud.android.lib.common.utils.Log_OC; +import java.io.IOException; + public class SimpleFactoryManager implements OwnCloudClientManager { private static final String TAG = SimpleFactoryManager.class.getSimpleName(); @Override - public OwnCloudClient getClientFor(OwnCloudAccount account, Context context) { + public OwnCloudClient getClientFor(OwnCloudAccount account, Context context) + throws AccountNotFoundException, OperationCanceledException, AuthenticatorException, IOException { + Log_OC.d(TAG, "getClientFor(OwnCloudAccount ... : "); + OwnCloudClient client = OwnCloudClientFactory.createOwnCloudClient( account.getBaseUri(), context.getApplicationContext(), true); - Log_OC.d(TAG, " new client {" + - (account.getName() != null ? + Log_OC.v(TAG, " new client {" + + (account.getName() != null ? account.getName() : - AccountUtils.buildAccountName( - account.getBaseUri(), - account.getCredentials().getAuthToken())) + - ", " + client.hashCode() + "}"); - - client.setCredentials(account.getCredentials()); + AccountUtils.buildAccountName(account.getBaseUri(), "") + + ) + ", " + client.hashCode() + "}"); + + if (account.getCredentials() == null) { + account.loadCredentials(context); + } + client.setCredentials(account.getCredentials()); return client; } diff --git a/src/com/owncloud/android/lib/common/SingleSessionManager.java b/src/com/owncloud/android/lib/common/SingleSessionManager.java index 46e20dfb..dd94db03 100644 --- a/src/com/owncloud/android/lib/common/SingleSessionManager.java +++ b/src/com/owncloud/android/lib/common/SingleSessionManager.java @@ -62,7 +62,9 @@ public class SingleSessionManager implements OwnCloudClientManager { @Override - public synchronized OwnCloudClient getClientFor(OwnCloudAccount account, Context context) { + public synchronized OwnCloudClient getClientFor(OwnCloudAccount account, Context context) + throws AccountNotFoundException, OperationCanceledException, AuthenticatorException, IOException { + Log_OC.d(TAG, "getClientFor(OwnCloudAccount ... : "); if (account == null) { throw new IllegalArgumentException("Cannot get an OwnCloudClient for a null account"); @@ -70,10 +72,13 @@ public class SingleSessionManager implements OwnCloudClientManager { OwnCloudClient client = null; String accountName = account.getName(); - String sessionName = AccountUtils.buildAccountName( - account.getBaseUri(), - account.getCredentials().getAuthToken()); - + String sessionName = account.getCredentials() == null ? "" : + AccountUtils.buildAccountName ( + account.getBaseUri(), + account.getCredentials().getAuthToken() + ) + ; + if (accountName != null) { client = mClientsWithKnownUsername.get(accountName); } @@ -82,10 +87,11 @@ public class SingleSessionManager implements OwnCloudClientManager { if (accountName != null) { client = mClientsWithUnknownUsername.remove(sessionName); if (client != null) { + // TODO REMOVE THIS LOG Log_OC.d(TAG, " reusing client {" + sessionName + ", " + client.hashCode() + "}"); mClientsWithKnownUsername.put(accountName, client); - Log_OC.d(TAG, " moved client to {" + accountName + ", " + + Log_OC.d(TAG, " moved client to {" + accountName + ", " + client.hashCode() + "}"); } } else { @@ -105,10 +111,9 @@ public class SingleSessionManager implements OwnCloudClientManager { client.getParams().setCookiePolicy(CookiePolicy.BROWSER_COMPATIBILITY); // enable cookie tracking - - // Restore Cookies ?? - AccountUtils.restoreCookies(accountName, client, context); - + AccountUtils.restoreCookies(accountName, client, context); + + account.loadCredentials(context); client.setCredentials(account.getCredentials()); if (accountName != null) { mClientsWithKnownUsername.put(accountName, client); @@ -116,10 +121,12 @@ public class SingleSessionManager implements OwnCloudClientManager { } else { mClientsWithUnknownUsername.put(sessionName, client); + // TODO REMOVE THIS LOG Log_OC.d(TAG, " new client {" + sessionName + ", " + client.hashCode() + "}"); } } else { if (!reusingKnown) { + // TODO REMOVE THIS LOG Log_OC.d(TAG, " reusing client {" + sessionName + ", " + client.hashCode() + "}"); } keepCredentialsUpdated(account, client); @@ -148,18 +155,9 @@ public class SingleSessionManager implements OwnCloudClientManager { Log_OC.d(TAG, "No client tracked for {" + accountName + "}"); } } - - String sessionName = AccountUtils.buildAccountName( - account.getBaseUri(), - account.getCredentials().getAuthToken()); - client = mClientsWithUnknownUsername.remove(sessionName); - if (client != null) { - Log_OC.d(TAG, "Removed client {" + sessionName + ", " + client.hashCode() + "}"); - return client; - } - Log_OC.d(TAG, "No client tracked for {" + sessionName + "}"); - - Log_OC.d(TAG, "No client removed"); + + mClientsWithUnknownUsername.clear(); + return null; } From 24110ba17870aa48a85751367922a87415a550d4 Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Tue, 10 Feb 2015 12:57:17 +0100 Subject: [PATCH 2/3] Updated unit tests --- .../android/lib/common/OwnCloudAccount.java | 15 +++++++-- .../network/AdvancedSslSocketFactory.java | 3 ++ .../test/SimpleFactoryManagerTest.java | 28 ++++++++++------ .../test/SingleSessionManagerTest.java | 33 ++++++++++++------- 4 files changed, 55 insertions(+), 24 deletions(-) diff --git a/src/com/owncloud/android/lib/common/OwnCloudAccount.java b/src/com/owncloud/android/lib/common/OwnCloudAccount.java index 1b339af8..302678e4 100644 --- a/src/com/owncloud/android/lib/common/OwnCloudAccount.java +++ b/src/com/owncloud/android/lib/common/OwnCloudAccount.java @@ -54,6 +54,11 @@ public class OwnCloudAccount { if (savedAccount == null) { throw new IllegalArgumentException("Parameter 'savedAccount' cannot be null"); } + + if (context == null) { + throw new IllegalArgumentException("Parameter 'context' cannot be null"); + } + mSavedAccount = savedAccount; mSavedAccountName = savedAccount.name; mBaseUri = Uri.parse(AccountUtils.getBaseUrlForAccount(context, mSavedAccount)); @@ -73,8 +78,14 @@ public class OwnCloudAccount { throws AccountNotFoundException, AuthenticatorException, IOException, OperationCanceledException { - mCredentials = AccountUtils.getCredentialsForAccount(context, mSavedAccount); - } + if (context == null) { + throw new IllegalArgumentException("Parameter 'context' cannot be null"); + } + + if (mSavedAccount != null) { + mCredentials = AccountUtils.getCredentialsForAccount(context, mSavedAccount); + } + } /* public OwnCloudAccount(Account savedAccount, Context context) diff --git a/src/com/owncloud/android/lib/common/network/AdvancedSslSocketFactory.java b/src/com/owncloud/android/lib/common/network/AdvancedSslSocketFactory.java index 99c05e68..4580331b 100644 --- a/src/com/owncloud/android/lib/common/network/AdvancedSslSocketFactory.java +++ b/src/com/owncloud/android/lib/common/network/AdvancedSslSocketFactory.java @@ -92,6 +92,7 @@ public class AdvancedSslSocketFactory implements SecureProtocolSocketFactory { /** * @see ProtocolSocketFactory#createSocket(java.lang.String,int,java.net.InetAddress,int) */ + @Override public Socket createSocket(String host, int port, InetAddress clientHost, int clientPort) throws IOException, UnknownHostException { @@ -157,6 +158,7 @@ public class AdvancedSslSocketFactory implements SecureProtocolSocketFactory { * @throws UnknownHostException if the IP address of the host cannot be * determined */ + @Override public Socket createSocket(final String host, final int port, final InetAddress localAddress, final int localPort, final HttpConnectionParams params) throws IOException, @@ -187,6 +189,7 @@ public class AdvancedSslSocketFactory implements SecureProtocolSocketFactory { /** * @see ProtocolSocketFactory#createSocket(java.lang.String,int) */ + @Override public Socket createSocket(String host, int port) throws IOException, UnknownHostException { Log_OC.d(TAG, "Creating SSL Socket with remote " + host + ":" + port); diff --git a/test_client/tests/src/com/owncloud/android/lib/test_project/test/SimpleFactoryManagerTest.java b/test_client/tests/src/com/owncloud/android/lib/test_project/test/SimpleFactoryManagerTest.java index 3254c589..05a72003 100644 --- a/test_client/tests/src/com/owncloud/android/lib/test_project/test/SimpleFactoryManagerTest.java +++ b/test_client/tests/src/com/owncloud/android/lib/test_project/test/SimpleFactoryManagerTest.java @@ -94,23 +94,31 @@ public class SimpleFactoryManagerTest extends AndroidTestCase { } public void testGetClientFor() { - OwnCloudClient client = mSFMgr.getClientFor(mValidAccount, getContext()); + try { + OwnCloudClient client = mSFMgr.getClientFor(mValidAccount, getContext()); - assertNotSame("Got same client instances for same account", - client, mSFMgr.getClientFor(mValidAccount, getContext())); + assertNotSame("Got same client instances for same account", + client, mSFMgr.getClientFor(mValidAccount, getContext())); - assertNotSame("Got same client instances for different accounts", - client, mSFMgr.getClientFor(mAnonymousAccount, getContext())); + assertNotSame("Got same client instances for different accounts", + client, mSFMgr.getClientFor(mAnonymousAccount, getContext())); + } catch (Exception e) { + throw new AssertionFailedError("Exception getting client for account: " + e.getMessage()); + } // TODO harder tests } public void testRemoveClientFor() { - OwnCloudClient client = mSFMgr.getClientFor(mValidAccount, getContext()); - mSFMgr.removeClientFor(mValidAccount); - assertNotSame("Got same client instance after removing it from manager", - client, mSFMgr.getClientFor(mValidAccount, getContext())); - + try { + OwnCloudClient client = mSFMgr.getClientFor(mValidAccount, getContext()); + mSFMgr.removeClientFor(mValidAccount); + assertNotSame("Got same client instance after removing it from manager", + client, mSFMgr.getClientFor(mValidAccount, getContext())); + + } catch (Exception e) { + throw new AssertionFailedError("Exception getting client for account: " + e.getMessage()); + } // TODO harder tests } diff --git a/test_client/tests/src/com/owncloud/android/lib/test_project/test/SingleSessionManagerTest.java b/test_client/tests/src/com/owncloud/android/lib/test_project/test/SingleSessionManagerTest.java index 8e0799ae..2a084ae6 100644 --- a/test_client/tests/src/com/owncloud/android/lib/test_project/test/SingleSessionManagerTest.java +++ b/test_client/tests/src/com/owncloud/android/lib/test_project/test/SingleSessionManagerTest.java @@ -93,22 +93,31 @@ public class SingleSessionManagerTest extends AndroidTestCase { } public void testGetClientFor() { - OwnCloudClient client1 = mSSMgr.getClientFor(mValidAccount, getContext()); - OwnCloudClient client2 = mSSMgr.getClientFor(mAnonymousAccount, getContext()); - - assertNotSame("Got same client instances for different accounts", - client1, client2); - assertSame("Got different client instances for same account", - client1, mSSMgr.getClientFor(mValidAccount, getContext())); - + try { + OwnCloudClient client1 = mSSMgr.getClientFor(mValidAccount, getContext()); + OwnCloudClient client2 = mSSMgr.getClientFor(mAnonymousAccount, getContext()); + + assertNotSame("Got same client instances for different accounts", + client1, client2); + assertSame("Got different client instances for same account", + client1, mSSMgr.getClientFor(mValidAccount, getContext())); + + } catch (Exception e) { + throw new AssertionFailedError("Exception getting client for account: " + e.getMessage()); + } + // TODO harder tests } public void testRemoveClientFor() { - OwnCloudClient client1 = mSSMgr.getClientFor(mValidAccount, getContext()); - mSSMgr.removeClientFor(mValidAccount); - assertNotSame("Got same client instance after removing it from manager", - client1, mSSMgr.getClientFor(mValidAccount, getContext())); + try { + OwnCloudClient client1 = mSSMgr.getClientFor(mValidAccount, getContext()); + mSSMgr.removeClientFor(mValidAccount); + assertNotSame("Got same client instance after removing it from manager", + client1, mSSMgr.getClientFor(mValidAccount, getContext())); + } catch (Exception e) { + throw new AssertionFailedError("Exception getting client for account: " + e.getMessage()); + } // TODO harder tests } From c2b5ddd2364f53201e7df6210ce04bc6bf4a2bbd Mon Sep 17 00:00:00 2001 From: masensio Date: Wed, 18 Feb 2015 10:57:17 +0100 Subject: [PATCH 3/3] Verify null recentCredential in SingleSessionManager.keepCreadentialsUpdated --- src/com/owncloud/android/lib/common/SingleSessionManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/com/owncloud/android/lib/common/SingleSessionManager.java b/src/com/owncloud/android/lib/common/SingleSessionManager.java index dd94db03..6f5d5dac 100644 --- a/src/com/owncloud/android/lib/common/SingleSessionManager.java +++ b/src/com/owncloud/android/lib/common/SingleSessionManager.java @@ -184,7 +184,7 @@ public class SingleSessionManager implements OwnCloudClientManager { private void keepCredentialsUpdated(OwnCloudAccount account, OwnCloudClient reusedClient) { OwnCloudCredentials recentCredentials = account.getCredentials(); - if (!recentCredentials.getAuthToken().equals( + if (recentCredentials != null && !recentCredentials.getAuthToken().equals( reusedClient.getCredentials().getAuthToken())) { reusedClient.setCredentials(recentCredentials); }