Reputation: 39
I have been looking for a way to somehow reduce the amount of code that is duplicated with subtle variance in my Spring MVC controllers, but searching through the SO questions so far has only yielded some questions without any satisfactory answers.
One example of duplication that I want to remove is this, where the user creation page and the role creation page share similarities:
@RequestMapping(value = "user/create", method = RequestMethod.GET)
public String create(@ModelAttribute("user") User user, BindingResult errors) {
LOG.debug("Displaying user creation page.");
return "user/create";
}
@RequestMapping(value = "role/create", method = RequestMethod.GET)
public String create(@ModelAttribute("role") Role role, BindingResult errors) {
LOG.debug("Displaying role creation page.");
return "role/create";
}
A slightly more involved variant of duplication that I would like to remove is the one for posting the create form:
@RequestMapping(value = "user/create", method = RequestMethod.POST)
public String save(@ModelAttribute("user") User user, BindingResult errors) {
LOG.debug("Entering save ({})", user);
validator.validate(user, errors);
validator.validatePassword(user, errors);
validator.validateUsernameAvailable(user, errors);
String encodedPassword = encoder.encode(user.getPassword());
user.setPassword(encodedPassword);
if (errors.hasErrors()) {
return create(user, errors);
} else {
service.save(user);
}
return "redirect:/user/index/1";
}
@RequestMapping(value = "role/create", method = RequestMethod.POST)
public String save(@ModelAttribute("role") Role role, BindingResult errors) {
LOG.debug("Entering save({})", role);
validator.validate(role, errors);
if (errors.hasErrors()) {
return create(role, errors);
} else {
service.save(role);
}
return "redirect:/index";
}
This example includes a validate then save if correct and a redirect to the error page if things don't go as planned.
How to remove this duplication?
Upvotes: 3
Views: 2898
Reputation: 39
Thought I would provide the solution that I settled on in the hope that it might help someone. My gf suggested that I use the name of the entity as a path variable for the controller, and this has proved to provide a very nice solution for the problem at hand.
The two methods now look like this:
@RequestMapping(value = "{entityName}/create", method = RequestMethod.GET)
public String create(@PathVariable("entityName") String entityName, @ModelAttribute("entity") BaseEntity entity, BindingResult errors) {
LOG.debug("Displaying create page for entity named: [{}]", entityName);
return handlerFactory.getHandler(entityName).getCreateView();
}
@RequestMapping(value = "{entityName}/create", method = RequestMethod.POST)
public String save(@PathVariable("entityName") String entityName, @ModelAttribute("entity") BaseEntity entity, BindingResult errors) {
LOG.debug("Saving entity of type {}", entityName);
CrudHandler handler = handlerFactory.getHandler(entityName);
handler.getCreateValidator().validate(entity, errors);
if (errors.hasErrors()) {
return create(entityName, entity, errors);
}
handler.preSave(entity);
handler.getService().save(entity);
return "redirect:" + DASHBOARD_URL;
}
The CrudHandler
interface has implementations for each entity, and provides the controller with the entity specific classes that it needs, such as service and validator. A sample CrudHandler
implementation looks like this for me:
@Component
public class RoleCrudHandler implements CrudHandler {
private static final String ENTITY_NAME = "role";
public static final String CREATE_VIEW = "role/create";
public static final String EDIT_VIEW = "role/edit";
@Resource
private RoleService roleService;
@Resource
private RoleValidator validator;
@Resource
private CrudHandlerFactory handlerFactory;
@PostConstruct
public void init() {
handlerFactory.register(ENTITY_NAME, this);
}
@Override
public GenericService getService() {
return roleService;
}
@Override
public Validator getCreateValidator() {
return validator;
}
@Override
public Validator getUpdateValidator() {
return validator;
}
@Override
public BaseEntity createEntity() {
return new Role();
}
@Override
public void preSave(BaseEntity entity) {
}
@Override
public String getCreateView() {
return CREATE_VIEW;
}
@Override
public String getUpdateView() {
return EDIT_VIEW;
}
}
If someone sees some ways to improve this, feel free to share. Hope this will be of use for someone.
Upvotes: 0
Reputation: 280171
Spring uses your handler method parameter types to create class instances from the request parameters or body. As such, there is no way to create a handler (@RequestMapping
) method that could take an Object
and check if it is either a Role
or a User
. (Technically you could have both parameters and just check which one isn't null
, but that is terrible design).
Consequently, you need a handler method for each. This makes sense since, even through the logic is similar, it is still specific to the exact type of model object you are trying to create. You perform different validation, call a different service method, and return a different view name.
I say your code is fine.
Upvotes: 1