Skip to content

POULPE-529 FIX BUG: incorrect error while registration with empty username #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
raimdev wants to merge 1 commit into
base: master
Choose a base branch
from
Open

POULPE-529 FIX BUG: incorrect error while registration with empty username #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions poulpe-model/src/main/resources/org/jtalks/poulpe/model/entity/applicationContext-dao.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
pointcut="execution(* org.jtalks.common.model.dao.Crud+.* (*))"/>
</aop:config>

<bean id="validator" class="org.springframework.validation.beanvalidation.LocalValidatorFactoryBean"/>

<!-- Declaration of DAO beans -->
<bean id="branchDao" class="org.jtalks.poulpe.model.dao.hibernate.BranchHibernateDao" parent="genericDao"/>

Expand Down
25 changes: 19 additions & 6 deletions ...rvice/src/main/java/org/jtalks/poulpe/service/transactional/TransactionalUserService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@
import org.jtalks.poulpe.service.UserService;
import org.jtalks.poulpe.service.exceptions.ValidationException;
import org.springframework.transaction.interceptor.TransactionInterceptor;
import org.springframework.validation.BeanPropertyBindingResult;
import org.springframework.validation.BindingResult;
import org.springframework.validation.FieldError;
import org.springframework.validation.Validator;

import javax.validation.ConstraintViolation;
import javax.validation.ConstraintViolationException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.*;

/**
* User service class, contains methods needed to manipulate with {@code User} persistent entity.
Expand All @@ -56,6 +56,7 @@ public class TransactionalUserService implements UserService {
private final UserBanner userBanner;
private final AclManager aclManager;
private final ComponentDao componentDao;
private Validator validator;


/**
Expand All @@ -65,11 +66,12 @@ public class TransactionalUserService implements UserService {
* entities
*/
public TransactionalUserService(UserDao userDao, UserBanner userBanner,
AclManager aclManager, ComponentDao componentDao) {
AclManager aclManager, ComponentDao componentDao, Validator validator) {
this.userDao = userDao;
this.userBanner = userBanner;
this.aclManager = aclManager;
this.componentDao = componentDao;
this.validator = validator;
}

/**
Expand Down Expand Up @@ -298,6 +300,17 @@ private PoulpeUser getPoulpeUser(String username) throws NotFoundException {
@Override
public void registration(PoulpeUser user) throws ValidationException {
List<String> errors = new ArrayList<String>();

BindingResult validateErrors = new BeanPropertyBindingResult(user, "user");
Copy link
Copy Markdown
Member

@ctapobep ctapobep Feb 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of validation usually happens in the REST services. Often frameworks have integration with BeanValidation so that you don't need to invoke it yourself.

Copy link
Copy Markdown
Author

@raimdev raimdev Feb 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I understand correctly, it is sufficient use @Valid annotation and delete extra codes?

     @Override
     public void registration(@Valid PoulpeUser user) throws ValidationException {

I checked, it works, but still want to clarify

Copy link
Copy Markdown
Member

@ctapobep ctapobep Feb 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, @Valid should be enough if REST/MVC framework supports it.

Copy link
Copy Markdown
Member

@ctapobep ctapobep Feb 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be awesome if you could write a test using Jersey Test Framework that checks if the validation is actually happening. You can use an in-memory container for that, read here for more details.

Copy link
Copy Markdown
Author

@raimdev raimdev Feb 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, now I try to write the test.

validator.validate(user, validateErrors);
for (FieldError error : validateErrors.getFieldErrors()) {
if (error.getCodes() != null && error.getCodes().length > 0) {
errors.add(error.getCodes()[0]);
} else {
errors.add(error.getDefaultMessage());
}
}

if (userDao.getByUsername(user.getUsername()) != null) {
errors.add(User.USER_ALREADY_EXISTS);
}
Expand Down
1 change: 1 addition & 0 deletions poulpe-service/src/main/resources/org/jtalks/poulpe/service/applicationContext-service.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
<constructor-arg index="1" ref="userBanner"/>
<constructor-arg index="2" ref="aclManager"/>
<constructor-arg index="3" ref="componentDao"/>
<constructor-arg index="4" ref="validator"/>
</bean>
<bean id="groupService" class="org.jtalks.poulpe.service.transactional.TransactionalGroupService">
<constructor-arg index="0" ref="groupDao"/>
Expand Down
9 changes: 5 additions & 4 deletions ...e-service/src/test/java/org/jtalks/poulpe/service/transactional/BeginTransactionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@
import org.springframework.test.context.testng.AbstractTransactionalTestNGSpringContextTests;
import org.springframework.test.context.transaction.TransactionConfiguration;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.validation.Validator;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import static org.mockito.Matchers.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.*;

@ContextConfiguration(locations = {"classpath:/org/jtalks/poulpe/model/entity/applicationContext-dao.xml"})
@TransactionConfiguration(transactionManager = "transactionManager", defaultRollback = true)
Expand All @@ -44,14 +43,16 @@ public class BeginTransactionTest extends AbstractTransactionalTestNGSpringConte
private UserDao userDao;
private ComponentDao componentDaoMock;
private AclManager aclManagerMock;
private Validator validator;

@BeforeMethod
public void setUp() {
MockitoAnnotations.initMocks(this);
userDao = mock(UserDao.class);
componentDaoMock = mock(ComponentDao.class);
aclManagerMock = mock(AclManager.class);
userService = new TransactionalUserService(userDao, mock(UserBanner.class), aclManagerMock, componentDaoMock);
validator = mock(Validator.class);
userService = new TransactionalUserService(userDao, mock(UserBanner.class), aclManagerMock, componentDaoMock, validator);
}

@Test(enabled = false)
Expand Down
11 changes: 7 additions & 4 deletions ...e/src/test/java/org/jtalks/poulpe/service/transactional/TransactionalUserServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.jtalks.poulpe.model.sorting.UserSearchRequest;
import org.jtalks.poulpe.service.exceptions.ValidationException;
import org.mockito.MockitoAnnotations;
import org.springframework.validation.Validator;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

Expand All @@ -54,7 +55,7 @@
* @author maxim reshetov
*/

public class TransactionalUserServiceTest{
public class TransactionalUserServiceTest {
private static final String USERNAME = "username";
private static final String HASED_PASSWORD = "password";
private static final PoulpeUser POULPE_USER = new PoulpeUser(USERNAME, "email", HASED_PASSWORD, "salt");
Expand All @@ -71,14 +72,16 @@ public class TransactionalUserServiceTest{
final String searchString = "searchString";
private ComponentDao componentDaoMock;
private AclManager aclManagerMock;
private Validator validator;

@BeforeMethod
public void setUp() {
MockitoAnnotations.initMocks(this);
userDao = mock(UserDao.class);
componentDaoMock = mock(ComponentDao.class);
aclManagerMock = mock(AclManager.class);
userService = new TransactionalUserService(userDao, mock(UserBanner.class), aclManagerMock, componentDaoMock);
validator = mock(Validator.class);
userService = new TransactionalUserService(userDao, mock(UserBanner.class), aclManagerMock, componentDaoMock, validator);
}

@Test
Expand Down Expand Up @@ -126,8 +129,8 @@ public void testUpdateUser() {
public void TestFindUsersNotInList(){
List<PoulpeUser> users = new ArrayList<PoulpeUser>();
users.add(user());
userService.findUsersNotInList(searchString,users);
verify(userDao).findUsersNotInList(searchString,users, Pages.NONE);
userService.findUsersNotInList(searchString, users);
verify(userDao).findUsersNotInList(searchString, users, Pages.NONE);
}

@Test
Expand Down