From 174be9c2d18c7ea7fbe17ad18dd9849edbaa1aed Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Wed, 8 Oct 2014 20:59:05 -0400 Subject: [PATCH 1/5] Added additional tests for UserService login --- .../backend/service/UserService.java | 4 +- .../backend/service/UserServiceTest.java | 69 +++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/commafeed/backend/service/UserService.java b/src/main/java/com/commafeed/backend/service/UserService.java index c4bffa55..64773e73 100644 --- a/src/main/java/com/commafeed/backend/service/UserService.java +++ b/src/main/java/com/commafeed/backend/service/UserService.java @@ -103,8 +103,10 @@ public class UserService { /** * should triggers after successful login + * + * Note: Visibility changed to protected to enabled spying on this method */ - private void afterLogin(User user) { + protected void afterLogin(User user) { Date lastLogin = user.getLastLogin(); Date now = new Date(); diff --git a/src/test/java/com/commafeed/backend/service/UserServiceTest.java b/src/test/java/com/commafeed/backend/service/UserServiceTest.java index bef52eba..d00561c5 100644 --- a/src/test/java/com/commafeed/backend/service/UserServiceTest.java +++ b/src/test/java/com/commafeed/backend/service/UserServiceTest.java @@ -5,6 +5,8 @@ import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.doNothing; import org.junit.Assert; import org.junit.Test; @@ -104,5 +106,72 @@ public class UserServiceTest { verify(encryptionService).authenticate("password", encryptedPassword, salt); } + + @Test public void + calling_login_should_not_return_user_object_on_unsuccessful_authentication() { + // Make a user who is not disabled + User user = new User(); + user.setDisabled(false); + + // Set the encryptedPassword on the user + byte[] encryptedPassword = new byte[]{1,2,3}; + user.setPassword(encryptedPassword); + + // Set a salt for this user + byte[] salt = new byte[]{4,5,6}; + user.setSalt(salt); + + // Mock DAO to return the user + UserDAO dao = mock(UserDAO.class); + when(dao.findByName("test")).thenReturn(user); + + // Mock PasswordEncryptionService + PasswordEncryptionService encryptionService = mock(PasswordEncryptionService.class); + when(encryptionService.authenticate(anyString(), any(byte[].class), any(byte[].class))).thenReturn(false); + + // Create service with mocks + UserService service = new UserService(null, dao, null, null, encryptionService, null); + + // Try to login as the user + Optional authenticatedUser = service.login("test", "password"); + + Assert.assertFalse(authenticatedUser.isPresent()); + } + + @Test public void + calling_login_should_return_user_object_on_successful_authentication() { + // Make a user who is not disabled + User user = new User(); + user.setDisabled(false); + + // Set the encryptedPassword on the user + byte[] encryptedPassword = new byte[]{1,2,3}; + user.setPassword(encryptedPassword); + + // Set a salt for this user + byte[] salt = new byte[]{4,5,6}; + user.setSalt(salt); + + // Mock DAO to return the user + UserDAO dao = mock(UserDAO.class); + when(dao.findByName("test")).thenReturn(user); + + // Mock PasswordEncryptionService + PasswordEncryptionService encryptionService = mock(PasswordEncryptionService.class); + when(encryptionService.authenticate(anyString(), any(byte[].class), any(byte[].class))).thenReturn(true); + + // Create service with mocks + UserService service = new UserService(null, dao, null, null, encryptionService, null); + + // Skip afterLogin activities + UserService spy = spy(service); + doNothing().when(spy).afterLogin(any(User.class)); + + // Try to login as the user + Optional authenticatedUser = spy.login("test", "password"); + + Assert.assertTrue(authenticatedUser.isPresent()); + Assert.assertEquals(user, authenticatedUser.get()); + } } From 47da4a2a1aaef3f95a158b7159e7233fac02e9aa Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Wed, 8 Oct 2014 21:03:53 -0400 Subject: [PATCH 2/5] Change visibility to package private --- src/main/java/com/commafeed/backend/service/UserService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/commafeed/backend/service/UserService.java b/src/main/java/com/commafeed/backend/service/UserService.java index 64773e73..5609a265 100644 --- a/src/main/java/com/commafeed/backend/service/UserService.java +++ b/src/main/java/com/commafeed/backend/service/UserService.java @@ -104,9 +104,9 @@ public class UserService { /** * should triggers after successful login * - * Note: Visibility changed to protected to enabled spying on this method + * Note: Visibility changed to package private to enabled spying on this method */ - protected void afterLogin(User user) { + void afterLogin(User user) { Date lastLogin = user.getLastLogin(); Date now = new Date(); From 67d7315003af165e11998569fc74e58b734deac4 Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Wed, 8 Oct 2014 21:39:39 -0400 Subject: [PATCH 3/5] Extract afterLogin into a separate class --- .../backend/service/UserService.java | 22 +-------- .../service/internal/PostLoginActivities.java | 45 +++++++++++++++++++ 2 files changed, 47 insertions(+), 20 deletions(-) create mode 100644 src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java diff --git a/src/main/java/com/commafeed/backend/service/UserService.java b/src/main/java/com/commafeed/backend/service/UserService.java index 5609a265..0a92704c 100644 --- a/src/main/java/com/commafeed/backend/service/UserService.java +++ b/src/main/java/com/commafeed/backend/service/UserService.java @@ -12,7 +12,6 @@ import lombok.RequiredArgsConstructor; import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.lang.StringUtils; -import org.apache.commons.lang.time.DateUtils; import com.commafeed.CommaFeedConfiguration; import com.commafeed.backend.dao.FeedCategoryDAO; @@ -21,6 +20,7 @@ import com.commafeed.backend.dao.UserSettingsDAO; import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserRole; import com.commafeed.backend.model.UserRole.Role; +import com.commafeed.backend.service.internal.PostLoginActivities; import com.google.common.base.Optional; import com.google.common.base.Preconditions; @@ -107,25 +107,7 @@ public class UserService { * Note: Visibility changed to package private to enabled spying on this method */ void afterLogin(User user) { - Date lastLogin = user.getLastLogin(); - Date now = new Date(); - - boolean saveUser = false; - // only update lastLogin field every hour in order to not - // invalidate the cache everytime someone logs in - if (lastLogin == null || lastLogin.before(DateUtils.addHours(now, -1))) { - user.setLastLogin(now); - saveUser = true; - } - if (config.getApplicationSettings().isHeavyLoad() - && (user.getLastFullRefresh() == null || user.getLastFullRefresh().before(DateUtils.addMinutes(now, -30)))) { - user.setLastFullRefresh(now); - saveUser = true; - feedSubscriptionService.refreshAll(user); - } - if (saveUser) { - userDAO.merge(user); - } + new PostLoginActivities(userDAO, feedSubscriptionService, config).afterLogin(user); } public User register(String name, String password, String email, Collection roles) { diff --git a/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java b/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java new file mode 100644 index 00000000..170ea7ef --- /dev/null +++ b/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java @@ -0,0 +1,45 @@ +package com.commafeed.backend.service.internal; + +import java.util.Date; + +import javax.inject.Inject; + +import lombok.RequiredArgsConstructor; + +import org.apache.commons.lang.time.DateUtils; + +import com.commafeed.CommaFeedConfiguration; +import com.commafeed.backend.dao.UserDAO; +import com.commafeed.backend.model.User; +import com.commafeed.backend.service.FeedSubscriptionService; + +@RequiredArgsConstructor(onConstructor = @__({ @Inject })) +public class PostLoginActivities { + + private final UserDAO userDAO; + private final FeedSubscriptionService feedSubscriptionService; + private final CommaFeedConfiguration config; + + public void afterLogin(User user) { + Date lastLogin = user.getLastLogin(); + Date now = new Date(); + + boolean saveUser = false; + // only update lastLogin field every hour in order to not + // invalidate the cache everytime someone logs in + if (lastLogin == null || lastLogin.before(DateUtils.addHours(now, -1))) { + user.setLastLogin(now); + saveUser = true; + } + if (config.getApplicationSettings().isHeavyLoad() + && (user.getLastFullRefresh() == null || user.getLastFullRefresh().before(DateUtils.addMinutes(now, -30)))) { + user.setLastFullRefresh(now); + saveUser = true; + feedSubscriptionService.refreshAll(user); + } + if (saveUser) { + userDAO.merge(user); + } + } + +} From 64b5d647092da6b79c68b32b5065f818178920e7 Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Wed, 8 Oct 2014 22:18:16 -0400 Subject: [PATCH 4/5] Inject PostLoginActivities and refactor --- .../backend/service/UserService.java | 9 ++++---- .../service/internal/PostLoginActivities.java | 4 +++- .../backend/service/UserServiceTest.java | 21 +++++++++---------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/commafeed/backend/service/UserService.java b/src/main/java/com/commafeed/backend/service/UserService.java index 0a92704c..93b79eff 100644 --- a/src/main/java/com/commafeed/backend/service/UserService.java +++ b/src/main/java/com/commafeed/backend/service/UserService.java @@ -34,9 +34,10 @@ public class UserService { private final UserDAO userDAO; private final UserSettingsDAO userSettingsDAO; - private final FeedSubscriptionService feedSubscriptionService; private final PasswordEncryptionService encryptionService; private final CommaFeedConfiguration config; + + private final PostLoginActivities postLoginActivities; /** * try to log in with given credentials @@ -103,11 +104,9 @@ public class UserService { /** * should triggers after successful login - * - * Note: Visibility changed to package private to enabled spying on this method */ - void afterLogin(User user) { - new PostLoginActivities(userDAO, feedSubscriptionService, config).afterLogin(user); + private void afterLogin(User user) { + postLoginActivities.executeFor(user); } public User register(String name, String password, String email, Collection roles) { diff --git a/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java b/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java index 170ea7ef..92f0d3e7 100644 --- a/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java +++ b/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java @@ -3,6 +3,7 @@ package com.commafeed.backend.service.internal; import java.util.Date; import javax.inject.Inject; +import javax.inject.Singleton; import lombok.RequiredArgsConstructor; @@ -14,13 +15,14 @@ import com.commafeed.backend.model.User; import com.commafeed.backend.service.FeedSubscriptionService; @RequiredArgsConstructor(onConstructor = @__({ @Inject })) +@Singleton public class PostLoginActivities { private final UserDAO userDAO; private final FeedSubscriptionService feedSubscriptionService; private final CommaFeedConfiguration config; - public void afterLogin(User user) { + public void executeFor(User user) { Date lastLogin = user.getLastLogin(); Date now = new Date(); diff --git a/src/test/java/com/commafeed/backend/service/UserServiceTest.java b/src/test/java/com/commafeed/backend/service/UserServiceTest.java index d00561c5..20508143 100644 --- a/src/test/java/com/commafeed/backend/service/UserServiceTest.java +++ b/src/test/java/com/commafeed/backend/service/UserServiceTest.java @@ -2,20 +2,19 @@ package com.commafeed.backend.service; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.doNothing; import org.junit.Assert; import org.junit.Test; import com.commafeed.backend.dao.UserDAO; import com.commafeed.backend.model.User; +import com.commafeed.backend.service.internal.PostLoginActivities; import com.google.common.base.Optional; - public class UserServiceTest { @Test public void @@ -99,7 +98,7 @@ public class UserServiceTest { when(encryptionService.authenticate(anyString(), any(byte[].class), any(byte[].class))).thenReturn(false); // Create service with mocks - UserService service = new UserService(null, dao, null, null, encryptionService, null); + UserService service = new UserService(null, dao, null, encryptionService, null, null); // Try to login as the user service.login("test", "password"); @@ -130,7 +129,7 @@ public class UserServiceTest { when(encryptionService.authenticate(anyString(), any(byte[].class), any(byte[].class))).thenReturn(false); // Create service with mocks - UserService service = new UserService(null, dao, null, null, encryptionService, null); + UserService service = new UserService(null, dao, null, encryptionService, null, null); // Try to login as the user Optional authenticatedUser = service.login("test", "password"); @@ -160,15 +159,15 @@ public class UserServiceTest { PasswordEncryptionService encryptionService = mock(PasswordEncryptionService.class); when(encryptionService.authenticate(anyString(), any(byte[].class), any(byte[].class))).thenReturn(true); - // Create service with mocks - UserService service = new UserService(null, dao, null, null, encryptionService, null); + // Mock PostLoginActivities to do nothing + PostLoginActivities postLoginActivities = mock(PostLoginActivities.class); + doNothing().when(postLoginActivities).executeFor(any(User.class)); - // Skip afterLogin activities - UserService spy = spy(service); - doNothing().when(spy).afterLogin(any(User.class)); + // Create service with mocks + UserService service = new UserService(null, dao, null, encryptionService, null, postLoginActivities); // Try to login as the user - Optional authenticatedUser = spy.login("test", "password"); + Optional authenticatedUser = service.login("test", "password"); Assert.assertTrue(authenticatedUser.isPresent()); Assert.assertEquals(user, authenticatedUser.get()); From 8a172170ea8ba86c839b684d3841786a9162aece Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Wed, 8 Oct 2014 22:39:32 -0400 Subject: [PATCH 5/5] Test that PostLoginActivities are executed for user after auth success --- .../backend/service/UserServiceTest.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/test/java/com/commafeed/backend/service/UserServiceTest.java b/src/test/java/com/commafeed/backend/service/UserServiceTest.java index 20508143..37954808 100644 --- a/src/test/java/com/commafeed/backend/service/UserServiceTest.java +++ b/src/test/java/com/commafeed/backend/service/UserServiceTest.java @@ -137,6 +137,41 @@ public class UserServiceTest { Assert.assertFalse(authenticatedUser.isPresent()); } + @Test public void + calling_login_should_execute_post_login_activities_for_user_on_successful_authentication() { + // Make a user who is not disabled + User user = new User(); + user.setDisabled(false); + + // Set the encryptedPassword on the user + byte[] encryptedPassword = new byte[]{1,2,3}; + user.setPassword(encryptedPassword); + + // Set a salt for this user + byte[] salt = new byte[]{4,5,6}; + user.setSalt(salt); + + // Mock DAO to return the user + UserDAO dao = mock(UserDAO.class); + when(dao.findByName("test")).thenReturn(user); + + // Mock PasswordEncryptionService + PasswordEncryptionService encryptionService = mock(PasswordEncryptionService.class); + when(encryptionService.authenticate(anyString(), any(byte[].class), any(byte[].class))).thenReturn(true); + + // Mock PostLoginActivities to do nothing + PostLoginActivities postLoginActivities = mock(PostLoginActivities.class); + doNothing().when(postLoginActivities).executeFor(any(User.class)); + + // Create service with mocks + UserService service = new UserService(null, dao, null, encryptionService, null, postLoginActivities); + + // Try to login as the user + service.login("test", "password"); + + verify(postLoginActivities).executeFor(user); + } + @Test public void calling_login_should_return_user_object_on_successful_authentication() { // Make a user who is not disabled