Symfony4, how to properly handle missing form field ? (without exception in $this->propertyAccessor->setValue)

Question

I have a Task entity, with two mandatory, non-nullable, fields:

title
dueDatetime

and Form to create task. The form is called by external scripts through POST with application/x-www-form-urlencoded (so no json or anything fancy), so I use standard symfony to handle this.

Problem is I don't control the scripts, and if the script forgot one of the argument, symfony4 will directly throw an exception at the handleRequest step, before I have the time to check if the form is valid or not. Which result in an ugly response 500.

My question: How to avoid that ? The best for me would be to just continue to use "form->isValid()" as before , but if there's an other standard way to handle that, it's okay too.

Note: it would be best if I don't have to put my entity's setter as accepting null values

The exception I got:

Expected argument of type "DateTimeInterface", "NULL" given.
in vendor/symfony/property-acces /PropertyAccessor.php::throwInvalidArgumentException (line 153)
in vendor/symfony/form/Extension/Core/DataMapper/PropertyPathMapper.php->setValue (line 85)
in vendor/symfony/form/Form.php->mapFormsToData (line 622)
in vendor/symfony/form/Extension/HttpFoundation/HttpFoundationRequestHandler.php->submit (line 108)
in vendor/symfony/form/Form.php->handleRequest (line 492)

A curl that reproduce the error :

curl -d 'title=foo' http://127.0.0.1:8080/users/api/tasks

The code :

Entity:

class Task
{


    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="bigint")
     */
    private $id;

    /**
     * @Assert\NotNull()
     * @Assert\NotBlank()
     * @ORM\Column(type="string", length=500)
     */
    private $title;
    /**
     *
     * @ORM\Column(type="datetimetz")
     */
    private $dueDatetime;

    public function getDueDatetime(): ?\DateTimeInterface
    {
        return $this->dueDatetime;
    }

    public function setDueDatetime(\DateTimeInterface $dueDatetime): self
    {
        $this->dueDatetime = $dueDatetime;
        return $this;
    }

    public function setTitle($title)
    {
        $this->title = $title;
        return $this;
    }

    public function getTitle()
    {
        return $this->title;
    }

}

Form

class TaskType extends AbstractType                                                                                     
{                                                                                                                       
    public function buildForm(FormBuilderInterface $builder, array $options)                                            
    {                                                                                                                   
        $builder                                                  
            ->add('title')                                                                                                                                                   
            ->add('dueDatetime')
        ;
    }
    public function configureOptions(OptionsResolver $resolver)
    {
        $resolver->setDefaults(['data_class' => Task::class]);
    }
}

Controller:

class TaskController extends AbstractController
{
   /**
     * @Route(
     *   "/users/api/tasks",
     *   methods={"POST"},
     *   name="user_api_create_task"
     * )
     */
    public function apiCreateTask(Request $request)
    {
        $task = new Task();;

        // the use of createNamed with an empty string is just so that
        // the external scripts don't have to know about symfony's convention
        $formFactory = $this->container->get('form.factory');
        $form = $formFactory->createNamed(
            '',
            TaskType::class,
            $task
        );
        $form->handleRequest($request); // <-- this throw exception

        // but this code should handle this no ? 
        if (!$form->isSubmitted() || !$form->isValid()) {
            return new JsonResponse([], 422);
        }
        $entityManager = $this->getDoctrine()->getManager();
        $entityManager->persist($task);
        $entityManager->flush();


        return new JsonResponse();
    }
}

`// but this code should handle this no ?` => no, you get an entity created out of the data from your request already at the handleRequest step (as a matter of fact, that's actually the full goal of this function). Because your request does not contain some data, then they got set as null. You can try/catch the handleRequest against the `InvalidArgumentException` though if this is really what you want to achieve. — β.εηοιτ.βε, Feb 21 '19 at 22:19
ok I see, so catching InvalidArgumentException would permit to quickly have what I want, but without being able to fully know what's missing, and if I want to know more I would have no other choice than doing something like this https://blog.martinhujer.cz/symfony-forms-with-request-objects/ ? — allan.simon, Feb 21 '19 at 22:34
mhm, no, you don't need a DTO (data transfer object) in here. Your `@Assert\NotNull()` is valid. You are just asserting a case that your own code cannot handle because your setter do not allow null. So most likely, the best you could do is to do what you explicitly said you didn't wanted, allow null in your setter: `public function setDueDatetime(?\DateTimeInterface $dueDatetime): self` — β.εηοιτ.βε, Feb 21 '19 at 22:40
yes but I don't want to remove this safety in my entity code just because i got one api at one point in time that needed it, seems like a break of separation of concern to me. — allan.simon, Feb 24 '19 at 19:04

Constantin · Answer 1 · 2019-02-21T23:05:49.663

0

There are at least 2 ways to handle this. In the two ways you will have to add @Assert\NotNull() to the dueDatetime attribute.

~~1 - You can try/catch the exception of the handleRequest call.~~
[edit] this one breaks the flow, not good.

2 - You can make nullable the setter setDueDatetime(\DateTimeInterface $dueDatetime = null). If you choose this one, please be sure to always validate your entity before an Insert/Update in DB else you will get an SQL error.

In the two cases it will be handled by the validator isValid() and you will have a nice error in your front end.

edited Feb 21 '19 at 23:05

answered Feb 21 '19 at 22:36

Constantin

1,258
10
9

Issue with 1 is that an exception breaks the flow of the code, so possibly the other setter of the entity are not going to be called with the data of the request, ending in odd form violations. The issue with 2 is that OP explicitly said he did not wanted that – β.εηοιτ.βε Feb 21 '19 at 22:44
Ok for the 1st one, completely true. For the second one, it isn't a problem because it will never be inserted/updated in DB, the structure of the DB will not let such thing happen. Entities used like that are anemic and can then be inconsistent. This is why I wrote that entity should always be validated before insert/update in DB (flush)... – Constantin Feb 21 '19 at 22:57
To be fair, what he should do is try/catch the exception and in the catch return a bad request. He shouldn't be responsible of the FE sending missing attributes... – Constantin Feb 21 '19 at 23:38
1

Really? Have you ever used a REST or SOAP API that just answer "Invalid Argument"? If you add to it a layer of badly documented API, you have the nightmare of most of the third-party integrators... – β.εηοιτ.βε Feb 21 '19 at 23:41
Then, first check if It's a valid request by hand, construct an array of errors, if not empty return a Bad request with all the details. And then let the form do it's stuff and checks if the entity is valid... – Constantin Feb 21 '19 at 23:50
> Then, first check if It's a valid request by hand, construct an array of errors hmm ok but then i'm duplicating the "validator" component of symfony no ? shouldn't then I use something like this https://stackoverflow.com/questions/9124047/validating-form-without-doctrine-entity ? – allan.simon Feb 24 '19 at 19:03
If you go that way then it's much better to use DTO... Then you use really only the validator... I suggested to only do a pre-validation to check if the fields are there, not the entire validation. No real validation logic... – Constantin Feb 25 '19 at 08:43

score 0 · Answer 2 · answered May 06 '20 at 10:20

0

You need to allow nullable parameter (with "?") in method setDueDatetime

public function setDueDatetime(?\DateTimeInterface $dueDatetime): self
{
    $this->dueDatetime = $dueDatetime;
    return $this;
}

answered May 06 '20 at 10:20

Wikub

18
5

Symfony4, how to properly handle missing form field ? (without exception in $this->propertyAccessor->setValue)

2 Answers2