Symfony controller that registers a user via HTTP-POST

Question

This is the code I have written for one of my API routes. I feel like my controller is too big and the function does too much. What can I do to make my adhere to SOLID principles and be more maintainable/readable?

/**
 * @Route("/registration", name="registration")
 * @Method({"POST"})
 * @throws UniqueConstraintViolationException
 * @param Request $request
 * @return JsonResponse
 */
public function registrationAction(Request $request, UserPasswordEncoderInterface $encoder)
{
    // Build base response
    $response = [
        "success"   => false,
        "message"   => "",
        "user_id"   => null,
        "errors"    => []
    ];

    // Put blanks for keys not present in the request
    // This is so we can validate this using Symfony's validator class
    foreach(self::REQUIRED_KEYS as $key) {
        if (!array_key_exists($key, $request->request->all())) {
            $request->request->set($key, "");
        }
    }

    // Get validator and build array of constraints
    $validator = Validation::createValidator();
    $constraint = new Assert\Collection([
        "username"  => new Assert\NotBlank(['message' => "Username is required."]),
        "email"     => [
            new Assert\Email(["message" => "Email must be a valid email address."]),
            new Assert\NotBlank(["message" => "Email is required."])
        ],
        "address1"  => new Assert\NotBlank(['message' => "Address1 is required."]),
        "city"      => new Assert\NotBlank(['message' => "City is required."]),
        "state"     => new Assert\NotBlank(['message' => "State is required."]),
        "zip"       => new Assert\NotBlank(['message' => "Zip is required."]),
    ]);
    $constraint->allowExtraFields = true;

    $errors = $validator->validate($request->request->all(), $constraint);

    // If there are errors, return the errors as a response
    // If there are no errors, register the user and return the ID
    if (count($errors) > 0) {
        foreach($errors as $e) { $response['errors'][] = $e->getMessage(); }
        $response['message'] = "Submitted user failed validation.";
    } else {
        $user = new User($request->request->all());
        $encodedPassword = $encoder->encodePassword($user, 'password');
        $user->setPassword($encodedPassword);

        $em = $this->getDoctrine()->getManager();
        try {
            $user->setEnabled(true);
            $em->persist($user);
            $em->flush();
            $response['success'] = true;
            $response['user_id'] = $user->getId();
        } catch (UniqueConstraintViolationException $e) {
            preg_match('/(?<=Duplicate entry \')[^\']*/', $e->getMessage(), $matches);
            $response['message'] = "Unique constraint violation exception";
            $response['errors'][] = sprintf('%s already exists in the database.', $matches[0]);
        }
    }

    return new JsonResponse($response);
}

My initial hunch is that I should put this into a service and break it up. But it seems like I would just be copying and pasting this code into class.

Answer 1

I feel like my controller is too big and the function does too much.

I think you're right there. Your validation rules shouldn't be in the controller, but in separate classes. That makes re-using them much easier. I

My initial hunch is that I should put this into a service and break it up. But it seems like I would just be copying and pasting this code into class.

You're right here. Just moving code from your controller to a service makes no sense.

Why aren't you using Symfony Forms? It does exactly the job you're doing here: take the Request and some rules and tell you if the request matches your validation rules.

You can create an object (a Doctrine entity or a Data Transfer Object, DTO), add the constraints to it by using annotations.

See this to examples from https://stovepipe.systems/post/avoiding-entities-in-forms:

Your DTO:

class ChangeUsernameData
{
    /** 
     * @Some\Custom\Assert\Unique(entity="App\Entity\Authentication", field="username")
     * @Assert\Length(5, 16)
     **/
    public $newUsername;
}

Your controller:

$changeUsername = new ChangeUsernameData();
    $form = $this->formFactory->create(ChangeUsernameType::class, $changeUsername);
    $form->handleRequest($request);

    if ($form->isSubmitted() && $form->isValid()) {
        $authentication->setUsername($changeUsername->newUsername);
        $this->em->flush($authentication);

        // redirect
    }