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.

• David Patterson: Question: How are you generating the form?

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
 }