Merge pull request #646 from rationalrevolt/userservice-tests

Additional tests on UserService.login and refactor

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;
 
@@ -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
@@ -105,25 +106,7 @@ public class UserService {
 	 * should triggers after successful login
 	 */
 	private void afterLogin(User user) {
-		Date lastLogin = user.getLastLogin();
+		postLoginActivities.executeFor(user);
-		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);
-		}
 	}
 
 	public User register(String name, String password, String email, Collection<Role> roles) {

src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java

@@ -0,0 +1,47 @@
+package com.commafeed.backend.service.internal;
+
+import java.util.Date;
+
+import javax.inject.Inject;
+import javax.inject.Singleton;
+
+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 }))
+@Singleton
+public class PostLoginActivities {
+	
+	private final UserDAO userDAO;
+	private final FeedSubscriptionService feedSubscriptionService;
+	private final CommaFeedConfiguration config;
+	
+	public void executeFor(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);
+		}
+	}
+
+}

src/test/java/com/commafeed/backend/service/UserServiceTest.java

@@ -2,6 +2,7 @@ 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;
@@ -11,9 +12,9 @@ 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 
@@ -97,12 +98,114 @@ 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");
 		
 		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, encryptionService, null, null);
+		
+		// Try to login as the user
+		Optional<User> authenticatedUser = service.login("test", "password");
+		
+		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
+		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
+		Optional<User> authenticatedUser = service.login("test", "password");
+		
+		Assert.assertTrue(authenticatedUser.isPresent());
+		Assert.assertEquals(user, authenticatedUser.get());
+	}
 
 }