FriendsOfSymfony / FOSRestBundle

This Bundle provides various tools to rapidly develop RESTful API's with Symfony
http://symfony.com/doc/master/bundles/FOSRestBundle/index.html
MIT License
2.79k stars 703 forks source link

Issue with View handler #909

Open ama3ing opened 9 years ago

ama3ing commented 9 years ago

Hi guys, I've noticed a bit strange issue. Assume that I have form:

<?php
     public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder
            ->add('email', 'email')
            ->add('password', 'password', array('property_path'=>'plainPassword'))
            ->add('firstName', 'text')
            ->add('lastName', 'text')
        ;

    }
// ...
    public function getName()
    {
        return 'user';
    }

And controller:

<?php
    /*
     * @Rest\Post("/user")
     * @Rest\View(statusCode=201)
     */
    public function createUserAction(Request $request)
    {
                    $user = new User();

                    $form = $this->createForm(new RegistrationType(), $user);
                    $form->handleRequest($request);
                    if($form->isValid()) {
//Nevermind code
                            return null;
                    }
                    return $form;
    }

So when I'm sending request like:

{
      user: {
             //incorrect user data
      }
}

Everything works as expected. But when I am sending something like

{
//Incorrect user data
}

Response status code is 201, but there is content indicating that form is invalid, what am I doing wrong :laughing:?

lsmith77 commented 9 years ago

uhm .. not sure. what does the form layer do? I mean this should be caught by your validation rules ..?

ama3ing commented 9 years ago

Yeah, it should. Since $form->isValid() returns false. And actually response content returns form errors.

lsmith77 commented 9 years ago

so you end up returning $form ?

what I suspect is that the form is not considered to be bound when there is no data and therefore the handler doesn't look at the form validation failure: https://github.com/FriendsOfSymfony/FOSRestBundle/blob/master/View/ViewHandler.php#L200

ama3ing commented 9 years ago

@lsmith77, but still this is a bug IMHO

lsmith77 commented 9 years ago

yeah .. I am not sure why we had the isBound() check .. seems a bit unlikely that someone would pass a form instance for which they do not expect something to be bound, ie. where nothing bound would be considered an error.

anyway, can you try if the issue would be fixed by removing the isBound() check?

ama3ing commented 9 years ago

@lsmith77 checked, that works. Created a PR for that :wink:

lsmith77 commented 9 years ago

as discussed in the PR .. the isBound() call needs to stay. lets see if we can find an elegant way to handle this .. but I am not sure I like the idea to add a flag to the View to be able to tell the ViewHandler if to do the isBound() check or not.