Validation in controller or service?

user_8604720 0
Viewed 5475

I have a method of downloading messages in the controller

@GetMapping(value = "/sent/{id}")
public
HttpEntity<MessageSent> getMessageSent(
        @ApiParam(value = "The message ID", required = true) @PathVariable Long id
) {
    return ResponseEntity.ok().body(messageSearchService.getMessageSent(id, authorizationService.getUserId()));
}

However, I have forgotten to verify if the message about the given ID belongs to the user. It does not do this either in the service.

@Override
public MessageSent getMessageSent(
        @Min(1) Long messageId,
        @Min(1) Long userId
) throws ResourceNotFoundException {
    Optional<UserEntity> user = this.userRepository.findByIdAndEnabledTrue(userId);
    user.orElseThrow(() -> new ResourceNotFoundException("No user found with id " + userId));

    return this.messageRepository.findByIdAndSenderAndIsVisibleForSenderTrue(messageId, user.get())
            .map(MessageEntity::getSentDTO)
            .orElseThrow(() -> new ResourceNotFoundException("No message found with id " + messageId));
}

And now my question is whether it should be done in the controller or service? I would prefer to do this in the service, but I don't know if it is appropriate.

spring-boot spring-mvc spring java
2 Answers

As a general rule of thumb, I would say that business logic of this sort should be in the service. Controllers should be light-weight and pass on requests. Further, there may be other clients of your service, not just controllers, so this allows you to keep validation in one place.

user_1353510

  1. Don't put this validation in the controller - controllers part is only be entry points for coming requests and exposing API's.

  2. I would suggest you to create additional service which responsible of doing the validation and injecting this service in your messageSearchService service. This way you can use the validation service in other services as well which required same validation. In Addition , this why you follow the principle of each class has only one its own responsibility.

I hope this helps.

user_35777650
Related