From 2729178f223e58d00425a97599d997fbdcc9a80d Mon Sep 17 00:00:00 2001 From: rdsubhas Date: Wed, 2 Jul 2014 00:27:57 +0300 Subject: [PATCH] @ctumwebaze Updated error messages for #30 --- RapidFTR-Android/build.xml | 2 +- RapidFTR-Android/pom.xml | 2 +- RapidFTR-Android/res/values/strings.xml | 14 +- .../java/com/rapidftr/bean/LoginTask.java | 132 ++++++++----- .../com/rapidftr/task/LoginAsyncTask.java | 155 ---------------- .../java/com/rapidftr/bean/LoginTaskTest.java | 173 +++++++++++++----- 6 files changed, 224 insertions(+), 254 deletions(-) delete mode 100644 RapidFTR-Android/src/main/java/com/rapidftr/task/LoginAsyncTask.java diff --git a/RapidFTR-Android/build.xml b/RapidFTR-Android/build.xml index 1cba9dfe..b7be2a0e 100644 --- a/RapidFTR-Android/build.xml +++ b/RapidFTR-Android/build.xml @@ -50,7 +50,7 @@ diff --git a/RapidFTR-Android/pom.xml b/RapidFTR-Android/pom.xml index bb946a63..37d78251 100644 --- a/RapidFTR-Android/pom.xml +++ b/RapidFTR-Android/pom.xml @@ -127,7 +127,7 @@ org.mockito - mockito-core + mockito-all 1.9.5 test diff --git a/RapidFTR-Android/res/values/strings.xml b/RapidFTR-Android/res/values/strings.xml index 49683fc3..1670b98a 100644 --- a/RapidFTR-Android/res/values/strings.xml +++ b/RapidFTR-Android/res/values/strings.xml @@ -4,16 +4,22 @@ Log In - Logging in + Trying to login online + Online login successful Online login failed, will try to login offline + No connectivity, will try to login offline + You are now a verified user, migrating your data Failed to migrate data, logging in offline, please sync data - Failed to save your details, please contact the system administrator + Failed to complete the login, please contact your system administrator + Loading latest form sections Failed to load latest form sections, you can still keep using older forms - Online login successful + + Trying to login offline Offline login successful - Login failed, Incorrect username or password + Login failed, first time users please login online to enable your account for offline mode + Login failed, Incorrect username or password Log Out diff --git a/RapidFTR-Android/src/main/java/com/rapidftr/bean/LoginTask.java b/RapidFTR-Android/src/main/java/com/rapidftr/bean/LoginTask.java index 1f7713bb..8aa8309d 100644 --- a/RapidFTR-Android/src/main/java/com/rapidftr/bean/LoginTask.java +++ b/RapidFTR-Android/src/main/java/com/rapidftr/bean/LoginTask.java @@ -4,6 +4,7 @@ import android.content.Context; import android.util.Log; import android.widget.Toast; +import com.google.common.base.Throwables; import com.google.common.io.CharStreams; import com.rapidftr.RapidFtrApplication; import com.rapidftr.model.User; @@ -11,6 +12,8 @@ import com.rapidftr.service.LoginService; import com.rapidftr.task.MigrateUnverifiedDataToVerified; import com.rapidftr.utils.http.FluentResponse; +import lombok.Getter; +import lombok.NonNull; import org.androidannotations.annotations.*; import org.json.JSONObject; @@ -22,6 +25,19 @@ @EBean public class LoginTask { + protected static class LoginException extends RuntimeException { + @Getter protected Integer messageId = null; + + public LoginException(String message, Throwable cause) { + super(message, cause); + } + + public LoginException(int messageId, Throwable cause) { + super(cause); + this.messageId = messageId; + } + } + @Bean protected ConnectivityBean connectivityBean; @@ -33,19 +49,47 @@ public class LoginTask { protected ProgressDialog progressDialog; - public User login(String userName, String password, String url) { + public void login(String userName, String password, String url) { createProgressDialog(); - User onlineUser = loadOnline(userName, password, url); - User offlineUser = loadOffline(userName, password); - User finalUser = finalizeLogin(onlineUser, offlineUser); + if (!loginOnline(userName, password, url)) { + loginOffline(userName, password); + } dismissProgressDialog(); - return finalUser; + } + + protected boolean loginOnline(String userName, String password, String url) { + try { + notifyProgress(login_online_progress); + User user = loadOnline(userName, password, url); + migrateIfVerified(user); + cacheForOffline(user); + loadFormSections(); + notifyToast(login_online_success); + return true; + } catch (LoginException e) { + Log.e(APP_IDENTIFIER, "Failed to login online", e); + notifyToast(e); + return false; + } + } + + protected boolean loginOffline(String userName, String password) { + try { + notifyProgress(login_offline_progress); + User user = loadOffline(userName, password); + cacheForOffline(user); + notifyToast(login_offline_success); + return true; + } catch (LoginException e) { + Log.e(APP_IDENTIFIER, "Failed to login offline", e); + notifyToast(e); + return false; + } } protected User loadOnline(String userName, String password, String url) { if (!connectivityBean.isOnline()) { - notifyProgress(login_online_failed); - return null; + throw new LoginException(login_online_no_connectivity, null); } try { @@ -53,77 +97,62 @@ protected User loadOnline(String userName, String password, String url) { String responseAsString = CharStreams.toString(new InputStreamReader(response.getEntity().getContent())); if (!response.isSuccess()) { - notifyProgress(login_online_failed); - notifyToast(responseAsString); - return null; + throw new LoginException(responseAsString, null); } return new User(userName, password, true, url).read(responseAsString); } catch (Exception e) { - Log.d(APP_IDENTIFIER, "Online login failed", e); - notifyToast(login_online_failed); - return null; + Throwables.propagateIfInstanceOf(e, LoginException.class); + throw new LoginException(login_online_failed, null); } } protected User loadOffline(String userName, String password) { + User user = new User(userName, password); + if (!user.exists()) { + throw new LoginException(login_offline_no_user, null); + } + try { - return new User(userName, password).load(); - } catch (Exception e) { - return null; + return user.load(); + } catch(Exception e) { + throw new LoginException(login_offline_failed, e); } } - protected User finalizeLogin(User onlineUser, User offlineUser) { - boolean isMigrated = migrateIfVerified(onlineUser, offlineUser); - boolean isOnlineLogin = onlineUser != null && isMigrated; - User finalUser = isOnlineLogin ? onlineUser : offlineUser; - boolean isLoggedIn = cacheForOffline(finalUser); - - if (isOnlineLogin) - loadFormSections(); - - notifyToast(isLoggedIn ? (isOnlineLogin ? login_online_success : login_offline_success) : login_invalid); - return isLoggedIn ? finalUser : null; - } + protected void migrateIfVerified(@NonNull User onlineUser) { + User offlineUser = null; + try { + offlineUser = loadOffline(onlineUser.getUserName(), onlineUser.getPassword()); + } catch (LoginException e) { + return; + } - protected boolean migrateIfVerified(User onlineUser, User offlineUser) { - if (onlineUser != null && offlineUser != null && onlineUser.isVerified() && !offlineUser.isVerified()) { + if (offlineUser != null && onlineUser.isVerified() && !offlineUser.isVerified()) { try { notifyProgress(login_migrate_progress); new MigrateUnverifiedDataToVerified(new JSONObject(onlineUser.asJSON()), offlineUser).execute(); } catch (Exception e) { - Log.e(APP_IDENTIFIER, "Migrate failed", e); - notifyToast(login_migrate_failed); - return false; + throw new LoginException(login_migrate_failed, null); } } - - return true; } - protected boolean cacheForOffline(User user) { - if (user == null) return false; + protected void cacheForOffline(@NonNull User user) { try { user.save(); application.setCurrentUser(user); - return true; } catch (Exception e) { - Log.e(APP_IDENTIFIER, "Failed to save user details", e); - notifyToast(login_save_failed); - return false; + throw new LoginException(login_save_failed, e); } } - protected boolean loadFormSections() { + protected void loadFormSections() { try { notifyProgress(login_form_progress); new FormService(application).getPublishedFormSections(); - return true; } catch (Exception e) { - Log.e(APP_IDENTIFIER, "Failed to get form sections", e); - notifyToast(login_form_failed); - return false; + throw new LoginException(login_form_failed, e); } } @@ -132,7 +161,6 @@ protected void createProgressDialog() { progressDialog = new ProgressDialog(activity); progressDialog.setIndeterminate(true); progressDialog.setCancelable(false); - progressDialog.setMessage(application.getString(login_progress)); progressDialog.show(); } @@ -147,12 +175,16 @@ public void notifyProgress(Integer message) { } @UiThread - public void notifyToast(Integer resId) { - Toast.makeText(application, resId, Toast.LENGTH_SHORT).show(); + public void notifyToast(LoginException e) { + if (e.getMessageId() != null) { + notifyToast(e.getMessageId()); + } else { + Toast.makeText(application, e.getMessage(), Toast.LENGTH_SHORT).show(); + } } @UiThread - public void notifyToast(String message) { + public void notifyToast(int message) { Toast.makeText(application, message, Toast.LENGTH_SHORT).show(); } diff --git a/RapidFTR-Android/src/main/java/com/rapidftr/task/LoginAsyncTask.java b/RapidFTR-Android/src/main/java/com/rapidftr/task/LoginAsyncTask.java deleted file mode 100644 index 0e79e913..00000000 --- a/RapidFTR-Android/src/main/java/com/rapidftr/task/LoginAsyncTask.java +++ /dev/null @@ -1,155 +0,0 @@ -package com.rapidftr.task; - -import android.app.ProgressDialog; -import android.content.Intent; -import android.os.AsyncTask; -import android.util.Log; -import android.widget.Toast; -import com.google.common.io.CharStreams; -import com.google.inject.Inject; -import com.rapidftr.R; -import com.rapidftr.RapidFtrApplication; -import com.rapidftr.activity.RapidFtrActivity; -import com.rapidftr.activity.RegisterChildActivity; -import com.rapidftr.model.User; -import com.rapidftr.service.FormService; -import com.rapidftr.service.LoginService; -import org.apache.http.HttpResponse; -import org.json.JSONException; -import org.json.JSONObject; - -import java.io.IOException; -import java.io.InputStreamReader; -import java.security.GeneralSecurityException; - -import static com.rapidftr.RapidFtrApplication.APP_IDENTIFIER; -import static org.apache.http.HttpStatus.SC_CREATED; - -public class LoginAsyncTask extends AsyncTask { - - protected RapidFtrActivity activity; - protected ProgressDialog mProgressDialog; - protected RapidFtrApplication application; - protected FormService formService; - - protected String userName; - protected String password; - protected String url; - - @Inject - public LoginAsyncTask(RapidFtrApplication application, FormService formService) { - this.application = application; - this.formService = formService; - } - - public void setActivity(RapidFtrActivity activity) { - this.activity = activity; - } - - @Override - protected void onPreExecute() { - mProgressDialog = new ProgressDialog(activity); - mProgressDialog.setMessage(activity.getString(R.string.loading_message)); - mProgressDialog.setCancelable(false); - mProgressDialog.show(); - } - - @Override - protected User doInBackground(String... params) { - try { - this.userName = params[0]; - this.password = params[1]; - this.url = params[2]; - - return doLogin(); - } catch (Exception error) { - Log.e(APP_IDENTIFIER, "Failed to login", error); - return null; - } - } - - protected User doLogin() throws IOException, JSONException, GeneralSecurityException { - return application.isOnline() ? doOnlineLogin() : doOfflineLogin(); - } - - protected User doOnlineLogin() throws JSONException, GeneralSecurityException, IOException { - HttpResponse response; - try{ - response = getLoginResponse(); - } catch (Exception e){ - Log.e(APP_IDENTIFIER, "Failed to login", e); - return doOfflineLogin(); - } - if (response == null || response.getStatusLine() == null || response.getStatusLine().getStatusCode() != SC_CREATED) - return doOfflineLogin(); - - String responseAsString = CharStreams.toString(new InputStreamReader(response.getEntity().getContent())); - User user = new User(this.userName, this.password, true, this.url); - user.read(responseAsString); - User userFromSharedPreference = getUserFromPreference(); - if(userFromSharedPreference !=null && (!userFromSharedPreference.isVerified() && user.isVerified())) - migrateUnverifiedData(responseAsString, userFromSharedPreference); - return user; - } - - protected User getUserFromPreference() { - try { - return new User(this.userName, this.password).load(); - } catch (Exception e) { - Log.e(APP_IDENTIFIER, "Not able to get User from preference"); - } - return null; - } - - protected void migrateUnverifiedData(String responseAsString, User user) throws JSONException { - new MigrateUnverifiedDataToVerified(new JSONObject(responseAsString), user).execute(); - } - - protected HttpResponse getLoginResponse() throws IOException { - return new LoginService().login(application, userName, password, url); - } - - protected User doOfflineLogin() throws GeneralSecurityException, IOException { - User user = new User(this.userName, this.password); - user.load(); - return user; - } - - @Override - protected void onPostExecute(User user) { - if (mProgressDialog != null) - mProgressDialog.dismiss(); - - try { - if (user == null) - throw new GeneralSecurityException(); - - user.save(); - application.setCurrentUser(user); - getFormSections(user); - - Toast.makeText(application, R.string.login_successful, Toast.LENGTH_LONG).show(); - goToHomeScreen(); - } catch (Exception e) { - Toast.makeText(application, R.string.unauthorized, Toast.LENGTH_LONG).show(); - } - } - - protected void getFormSections(User user) { - if (application.isOnline() && user.isVerified()) { - try { - formService.getPublishedFormSections(); - } catch (Exception e) { - Log.e(APP_IDENTIFIER, "Failed to download form sections", e); - Toast.makeText(application, R.string.fetch_form_sections_error, Toast.LENGTH_LONG).show(); - } - } - } - - protected void goToHomeScreen() { - activity.finish(); - activity.startActivity(new Intent(activity, RegisterChildActivity.class)); - } - -} - diff --git a/RapidFTR-Android/src/test/java/com/rapidftr/bean/LoginTaskTest.java b/RapidFTR-Android/src/test/java/com/rapidftr/bean/LoginTaskTest.java index 12bb4757..a34b9423 100644 --- a/RapidFTR-Android/src/test/java/com/rapidftr/bean/LoginTaskTest.java +++ b/RapidFTR-Android/src/test/java/com/rapidftr/bean/LoginTaskTest.java @@ -2,8 +2,10 @@ import android.app.ProgressDialog; import android.content.Context; +import com.rapidftr.R; import com.rapidftr.RapidFtrApplication; import com.rapidftr.model.User; +import com.rapidftr.service.LoginService; import com.rapidftr.task.MigrateUnverifiedDataToVerified; import org.junit.Before; import org.junit.Ignore; @@ -12,20 +14,23 @@ import org.mockito.InjectMocks; import org.mockito.Spy; import org.powermock.core.classloader.annotations.MockPolicy; +import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; -import java.io.IOException; -import java.security.GeneralSecurityException; - -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.anyInt; -import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.*; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.powermock.api.mockito.PowerMockito.mock; +import static org.powermock.api.mockito.PowerMockito.verifyNew; import static org.powermock.api.mockito.PowerMockito.whenNew; @RunWith(PowerMockRunner.class) @MockPolicy(AndroidMockPolicy.class) +@PrepareForTest({ LoginService.class, MigrateUnverifiedDataToVerified.class }) public class LoginTaskTest { ConnectivityBean connectivityBean = mock(ConnectivityBean.class, RETURNS_DEEP_STUBS); @@ -40,64 +45,146 @@ public class LoginTaskTest { public void setUp() { doNothing().when(loginTask).createProgressDialog(); doNothing().when(loginTask).dismissProgressDialog(); + doNothing().when(loginTask).notifyToast(any(LoginTask.LoginException.class)); doNothing().when(loginTask).notifyToast(anyInt()); - doNothing().when(loginTask).notifyToast(anyString()); doReturn(true).when(connectivityBean).isOnline(); } @Test public void testOnlineLogin() { - User expected = mock(User.class); - doReturn(expected).when(loginTask).loadOnline("foo", "bar", "baz"); - User actual = loginTask.login("foo", "bar", "baz"); - assertEquals(expected, actual); - verify(loginTask).cacheForOffline(expected); - verify(loginTask).loadFormSections(); + doReturn(true).when(loginTask).loginOnline("test1", "test2", "test3"); + loginTask.login("test1", "test2", "test3"); + verify(loginTask, never()).loginOffline(anyString(), anyString()); } @Test public void testOfflineLogin() { - User expected = mock(User.class); - doReturn(expected).when(loginTask).loadOffline("foo", "bar"); - User actual = loginTask.login("foo", "bar", "baz"); - assertEquals(expected, actual); - verify(loginTask).cacheForOffline(expected); + doReturn(false).when(loginTask).loginOnline("test1", "test2", "test3"); + doReturn(false).when(loginTask).loginOffline("test1", "test2"); + loginTask.login("test1", "test2", "test3"); + verify(loginTask).loginOffline("test1", "test2"); + } + + @Test + public void testLoginOnlineShouldFailIfLoadFails() { + LoginTask.LoginException loginException = new LoginTask.LoginException(1, null); + doThrow(loginException).when(loginTask).loadOnline("test1", "test2", "test3"); + + boolean result = loginTask.loginOnline("test1", "test2", "test3"); + assertFalse(result); + verify(loginTask, never()).migrateIfVerified(any(User.class)); + verify(loginTask, never()).cacheForOffline(any(User.class)); verify(loginTask, never()).loadFormSections(); + verify(loginTask).notifyToast(loginException); } @Test - public void testOnlineUserWhenOfflineProvided() { - User onlineUser = mock(User.class), offlineUser = mock(User.class); - doReturn(onlineUser).when(loginTask).loadOnline("foo", "bar", "baz"); - doReturn(offlineUser).when(loginTask).loadOffline("foo", "bar"); - User actual = loginTask.login("foo", "bar", "baz"); - assertEquals(onlineUser, actual); - verify(loginTask).cacheForOffline(onlineUser); - verify(loginTask).loadFormSections(); + public void testLoginOnlineShouldFailIfMigrationFails() { + LoginTask.LoginException loginException = new LoginTask.LoginException(1, null); + User user = mock(User.class); + doReturn(user).when(loginTask).loadOnline("test1", "test2", "test3"); + doThrow(loginException).when(loginTask).migrateIfVerified(user); + + boolean result = loginTask.loginOnline("test1", "test2", "test3"); + assertFalse(result); + verify(loginTask, never()).cacheForOffline(any(User.class)); + verify(loginTask).notifyToast(loginException); } - @Test @Ignore - public void testMigration() throws Exception { - MigrateUnverifiedDataToVerified migrateTask = mock(MigrateUnverifiedDataToVerified.class, RETURNS_DEEP_STUBS); - whenNew(MigrateUnverifiedDataToVerified.class).withAnyArguments().thenReturn(migrateTask); - - User onlineUser = mock(User.class), offlineUser = mock(User.class); - doReturn(onlineUser).when(loginTask).loadOnline("foo", "bar", "baz"); - doReturn(offlineUser).when(loginTask).loadOffline("foo", "bar"); - doReturn(true).when(onlineUser).isVerified(); - doReturn(false).when(offlineUser).isVerified(); - - loginTask.login("foo", "bar", "baz"); - verify(migrateTask).execute(); + @Test + public void testLoginOnlineShouldFailIfCachingFails() { + LoginTask.LoginException loginException = new LoginTask.LoginException(1, null); + User user = mock(User.class); + doReturn(user).when(loginTask).loadOnline("test1", "test2", "test3"); + doNothing().when(loginTask).migrateIfVerified(user); + doThrow(loginException).when(loginTask).cacheForOffline(user); + + boolean result = loginTask.loginOnline("test1", "test2", "test3"); + assertFalse(result); + verify(loginTask).notifyToast(loginException); + } + + @Test + public void testLoginOnlineShouldFailIfFormSectionsFail() { + LoginTask.LoginException loginException = new LoginTask.LoginException(1, null); + User user = mock(User.class); + doReturn(user).when(loginTask).loadOnline("test1", "test2", "test3"); + doNothing().when(loginTask).migrateIfVerified(user); + doNothing().when(loginTask).cacheForOffline(user); + doThrow(loginException).when(loginTask).loadFormSections(); + + boolean result = loginTask.loginOnline("test1", "test2", "test3"); + assertFalse(result); + verify(loginTask).notifyToast(loginException); + } + + @Test + public void testLoginOnlineShouldBeSuccess() { + User user = mock(User.class); + doReturn(user).when(loginTask).loadOnline("test1", "test2", "test3"); + doNothing().when(loginTask).migrateIfVerified(user); + doNothing().when(loginTask).cacheForOffline(user); + doNothing().when(loginTask).loadFormSections(); + + boolean result = loginTask.loginOnline("test1", "test2", "test3"); + assertTrue(result); + verify(loginTask).notifyToast(R.string.login_online_success); + } + + @Test + public void testLoginOfflineShouldFailIfLoadingFromOfflineFails() { + LoginTask.LoginException loginException = new LoginTask.LoginException(1, null); + doThrow(loginException).when(loginTask).loadOffline("test1", "test2"); + + boolean result = loginTask.loginOffline("test1", "test2"); + assertFalse(result); + verify(loginTask, never()).cacheForOffline(any(User.class)); + verify(loginTask).notifyToast(loginException); } @Test - public void testSaveDetailsOffline() throws IOException, GeneralSecurityException { - User user = mock(User.class, RETURNS_DEEP_STUBS); - loginTask.cacheForOffline(user); + public void testLoginOfflineShouldFailIfCachingForOfflineFails() { + LoginTask.LoginException loginException = new LoginTask.LoginException(1, null); + User user = mock(User.class); + doReturn(user).when(loginTask).loadOffline("test1", "test2"); + doThrow(loginException).when(loginTask).cacheForOffline(user); + + boolean result = loginTask.loginOffline("test1", "test2"); + assertFalse(result); + verify(loginTask).notifyToast(loginException); + } + + @Test + public void testLoginOfflineShouldSucceed() { + User user = mock(User.class); + doReturn(user).when(loginTask).loadOffline("test1", "test2"); + doNothing().when(loginTask).cacheForOffline(user); + + boolean result = loginTask.loginOffline("test1", "test2"); + assertTrue(result); + verify(loginTask).notifyToast(R.string.login_offline_success); + } + + @Test + public void testMigrateShouldNotMigrate() throws Exception { + whenNew(MigrateUnverifiedDataToVerified.class).withAnyArguments().thenReturn(null); + User user = new User("test1", "test2", false); + doThrow(LoginTask.LoginException.class).when(loginTask).loadOffline("test1", "test2"); + + loginTask.migrateIfVerified(user); + verifyNew(MigrateUnverifiedDataToVerified.class, never()); + } + + @Test @Ignore + public void testMigrateShouldMigrate() throws Exception { + User onlineUser = new User("test1", "test2", false); + User offlineUser = mock(User.class); + + MigrateUnverifiedDataToVerified task = mock(MigrateUnverifiedDataToVerified.class, RETURNS_DEEP_STUBS); + whenNew(MigrateUnverifiedDataToVerified.class).withArguments(onlineUser.asJSON(), offlineUser).thenReturn(task); - verify(user).save(); - verify(rapidFtrApplication).setCurrentUser(user); + loginTask.migrateIfVerified(onlineUser); + verify(task).execute(); } }