6
\$\begingroup\$

In my project I am using the ADR architecture, a branch of MVC.

In my actions (Controllers) I handle only data that comes from the request.

When there is a business rule that defines which status code I return, I do this treatment in my service.

Imagine this scenario:

When editing a user, I check if the user's email already exists, if it exists, I return an error informed to your message. This treatment is done in the service, and in these cases I get the error in it.

I would like to hear from you about my approach to, in some cases, the service layer returning the payload.

Action:

class CreateUserAction extends Action
{
    private $userService;


    /**
     * @param UserService $userService
     */
    public function __construct(UserService $userService)
    {
        $this->userService = $userService;
    }

    public function handle(Request $request) : JsonResponse
    {
        $data = $request->getData();

        $payload = $this->userService->create($data, $user);

        return response()->json($payload->body, $payload->statusCode);
    }
}

Service:

class UserService extends BaseService
{
    private $userRepository;

    public function __construct(UserRepository $userRepository)
    {
        $this->userRepository = $userRepository;
    }

    public function create(array $data) : object
    {
        if ($this->userRepository->findByEmail($data['email'])) {
            $this->setPayloadError(
                __('messages.user.contributor_email_already_exists'),
                Response::HTTP_BAD_REQUEST
            );
            return $this->getPayload();
        }
        
        $userCreated = $this->userRepository->create($data);

        $this->setPayload($userCreated, Response::HTTP_CREATED);
        return $this->getPayload();

    }
}
```
\$\endgroup\$
4
  • \$\begingroup\$ I believe HTTP status code should be handled by the controller. In your case, an Action. The service layer should be independent from the HTTP part of the application, that's what controllers are for. So the service layer should just throw an exception if something goes up, and the controller/action should catch that and handle it by returning a proper status code in the request. \$\endgroup\$
    – Phiter
    Jul 17, 2020 at 17:02
  • \$\begingroup\$ I thought about this scenario, but found 2 problems: 1 - As it is something predictable in the business rule, I don't know if it would be interesting to return an exception in this case, in addition to being more costly, I understand that I would treat the exception as flow control. 2 - When I need to return different types of status code, I would have to keep this in my trycacth, checking the type of that exception to set the status code. \$\endgroup\$
    – Tiago
    Jul 20, 2020 at 13:00
  • \$\begingroup\$ I have seen in some designs, about having a layer where you pass an enum to it, it is responsible for forming the response.I decided to return the http code to avoid this other layer, in my view returning only the code does not leave it attached to the entire http layer. Since it is not formed by its codes. \$\endgroup\$
    – Tiago
    Jul 20, 2020 at 13:01
  • \$\begingroup\$ Yeah using exceptions as flow control is kinda bad practice, so if your service layer could just return a object with a certain structure, containing an error like "ACCESS_DENIED" for example, the error can be understood by the controller without handling exceptions, and it would be decoupled from the HTTP status part. \$\endgroup\$
    – Phiter
    Jul 20, 2020 at 13:12

1 Answer 1

1
\$\begingroup\$

In my actions (Controllers) I handle only data that comes from the request.

If above is the case you might be better off handling validation before calling service class. In other words, prepare data into a format that business service classes can understand. Same applies about a decision on which HTTP Code to return.

I am not sure why you have single responsibility controllers, a number of files would bloat very quickly in a larger application.

Controller:

class CreateUserAction extends Action
{ 
    public function handle(UserCreateRequest $request): JsonResponse
    {
        return (new UserService)->create($request->validated(), $user);

        // OR
        $user = (new UserService)->create($request->validated(), $user);

        return response($user, Response::HTTP_CREATED);
    }
}

I am not sure why you are using repository unless you are doing complex DB operation. Service should have all the business logic. This way service layer/business logic can be easier extracted and reused in other applications thanks to less dependencies.

use App/Models/User;

class UserService extends BaseService
{ 
    public function create(array $data): User
    { 
        return (new userRepository)->create($data); 

    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.