CODEWITHSUNDEEP
javaspringspring-mvcrefactoring
vtanase
vtanase

Reputation: 39

Removing duplication from Spring controllers

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

Answers (2)

vtanase
vtanase

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

Sotirios Delimanolis
Sotirios Delimanolis

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

Related Questions