2
\$\begingroup\$

Hey I always used to create some pretty messy endpoints and I decided that it's about time to start writing cleaner code, I'll provide an example of how would I design an endpoint and I'll be thankful for any insights why is it good or bad practice to do it like this also I'd be more than happy if you provide your own tweaks with explanation why would you do it like that.
Scenario: user post endpoint
Pathname: /api/user
Method: POST
Controller:

class UserPostEndpoint extends AbstractController
{
    private UserPostHandlerInterface $handler;
    private SerializerInterface $serializer;

    function __construct(UserPostHandlerInterface $handler, SerializerInterface $serializer)
    {
        $this->handler = $handler;
        $this->serializer = $serializer;
    }

    public function handleRequest(Request $request): Response
    {
        try {
            $userPostRequestDTO = $this->serializer->unserialize($request->getContent(), UserPostRequestDTO::class);
            $response = $this->handler->handle($userPostRequestDTO); // <- ApiSuccessResponse(statusCode = 201)
        } catch(SerializationException $e) {
            $response = new ApiErrorResponse('Invalid request content provided', 400);
        } catch(ApiRequestContentValidationException $e) {
            $response = new ApiErrorReponse($e->getPlainMessage(), 400);
        } catch(\Exception $e) {
            $response = new ApiErrorReponse('Internal server error', 500);
        } finally {
            return $response;
        }
    }
}

Handler would contain some basic things like: invoke createUser in UserFactory, persist the entity with repository. ApiErrorReponse: message, statusCode
ApiSuccessReponse: UserPostResponseDTO, statusCode(201)
Would you add / change / remove something from the given example to make it more "correct"?
Thanks for the insights
EDIT
After a few comments from @slepic I refactored the code into this

class UserPostEndpoint extends AbstractController
{
    private UserPostHandlerInterface $handler;
    private DataTransformerInterface $dataTransformer;

    function __construct(UserPostHandlerInterface $handler, DataTransformerInterface $dataTransformer)
    {
        $this->handler = $handler;
        $this->dataTransformer = $dataTransformer;
    }

    public function handleRequest(Request $request): Response
    {
        try {
            $userPostRequestDTO = $this->dataTransformer->toRequestDto($request->getContent(), $request->headers->get('Content-Type', 'application/json'));
            $userPostResponseDTO = $this->handler->handle($userPostRequestDTO);
            $response = new ApiSuccessResponse($userPostResponseDTO, Response::HTTP_CREATED);
        } catch(SerializationException $e) {
            $response = new ApiErrorResponse('Invalid request content provided', Response::HTTP_BAD_REQUEST);
        } catch(ApiRequestContentValidationException $e) {
            $response = new ApiErrorReponse($e->getPlainMessage(), Response::HTTP_BAD_REQUEST);
        } catch(\Exception $e) {
            $response = new ApiErrorReponse('Internal server error', Response::HTTP_INTERNAL_SERVER_ERROR);
        } finally {
            return $response;
        }
    }
}

I hope this is more appropriate

\$\endgroup\$
  • \$\begingroup\$ It Is a bit wierd that the controller dictates what error status codes should be returned, but status code for success is selected by handler. \$\endgroup\$ – slepic Jul 9 '20 at 3:20
  • \$\begingroup\$ @slepic after taking a second look thats true, should I move the try catch to the handler then? \$\endgroup\$ – TZiebura Jul 9 '20 at 7:29
  • \$\begingroup\$ No I think just like you have unserializer for request, you should have a serializer for response body, then, in the controller, build response object by composing 201 status code and the serialized body. \$\endgroup\$ – slepic Jul 9 '20 at 7:59
  • 1
    \$\begingroup\$ In other words, http statuses and headers and bodies live in the http domain thus handled by controllers, your application dtos live in your business domain thus handled by your domain handlers. \$\endgroup\$ – slepic Jul 9 '20 at 8:18
  • 2
    \$\begingroup\$ The current question title of your question is too generic to be helpful. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$ – BCdotWEB Jul 9 '20 at 14:50
0
\$\begingroup\$

How about:

class PostController extends AbstractController
{
    public function store(PostStoreRequest $request): Response
    {
        $dataStruct = app(PostDataStruct::class)->fill($request->validated());
    
        return app(UserPostHandlerInterface::class)->createPost($dataStruct);
    }
}
  • PostStoreRequest validates data
  • PostDataStruct stores contents and has a method that fill all the data for you.
  • Your code looks a bit like Laravel so I utilized Laravels app container. This way you will have an easier time mocking objects in phpunit.
  • Within Kernel file forException Handlers, you can define how certain exceptions are handled. i.e. if you call throw new SystemExceptionError('my message') - in the Kernel file state that SystemExceptionError should have HTTP status 500.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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