Nelmioapidocbundle: Entities with validations ignore JMS serializer groups

Created on 4 Oct 2014  路  24Comments  路  Source: nelmio/NelmioApiDocBundle

It seems that entity properties that have validations have their JMS serialization group ignored when filtered in the output parameter.

For example, here's a property in my entity

    /**
     * @ORM\Column(type="integer")
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     * @Serializer\Expose
     * @Serializer\Since("1.0")
     * @Serializer\Groups({"userDetails", "userList"})
     */
    protected $id;

    /**
     * @ORM\Column(type="string", length=25, unique=true)
     * @Serializer\Expose
     * @Serializer\Since("1.0")
     * @Serializer\Groups({"userDetails", "userList"})
     */
    protected $username;

    /**
     * @ORM\Column(type="string", length=60, unique=true)
     * @Serializer\Expose
     * @Serializer\Since("1.0")
     * @Serializer\Groups({"userDetails"})
     */
    private $email;

In my controller's ApiDoc annotation, I have

    * ...
     * output={
     *      "class"="Acme\UserBundle\Entity\User",
     *      "groups"={"userList"}
     *   }
     *  ...

In the docs, email still shows up because I have in my loadValidatorMetadata function

$metadata->addPropertyConstraint('email',     new Assert\Email(
            array(
                'message' => 'Email invalid,
                'groups'  => array('SecondPass')
            )
        ));

        $metadata->addConstraint(new UniqueEntity(
            array(
                'fields'           => array('email'),
                'message'          => 'Email invalid',
                'repositoryMethod' => 'checkUniqueUsernameEmail'
            )
        ));

So....any suggestions on how I can get the api doc to not include the properties that match this scenario?

Thanks,
Alan

Most helpful comment

Okey I found what is happing.

In ApiDocExtractor::extractData, several Parsers can be used.
In our case, two are called to get parameters,Nelmio\ApiDocBundle\Parser\ValidationParser and Nelmio\ApiDocBundle\Parser\JmsMetadataParser

ValidationParser is called first. As it was reported in https://github.com/nelmio/NelmioApiDocBundle/issues/125#issuecomment-148345123, ValidationParser does not consider @Groups in entities but consider all other proprieties with @Assert.

When JmsMetadataParser is called, $parameters had already parameters extracted with ValidationParser.
To don't lose information $parameters variable are erased with the result of mergeParameters function.
At the end , $parameters contains values come from ValidationParser and JmsMetadataParser.

Problem this is here ! Because ValidationParser extracts all proprieties with @Assert without checking if any of one have @Groups annotations.

Code to input extraction to get a better understanding (is the same with output).

$supportedParsers = array();
            foreach ($this->getParsers($normalizedInput) as $parser) {
                if ($parser->supports($normalizedInput)) {
                    $supportedParsers[] = $parser;
                    $parameters         = $this->mergeParameters($parameters, $parser->parse($normalizedInput));
                }
            }

I did a very simple PR. But before waiting PR merging, here one situation !

     *  output= {
     *     "class"="Freedom\DemophonieBundle\Entity\Decision",
     *     "groups"={"list"},
     *     "parsers"={"Nelmio\ApiDocBundle\Parser\JmsMetadataParser"},
     *  },

Only JmsMetadataParser will be used.
Enjoy !

All 24 comments

I have the same issue. No matter what the groups are, as long as I define constraints (via annotations in my case), the properties shows up in the documentation.

Here is how I reproduce this issue :

Class Article :

    /**
     * Title of the article
     *
     * @var string
     *
     * @ORM\Column(name="title", type="string", length=255)
     *
     * @Assert\NotBlank()
     *
     * @Serializer\Expose()
     * @Serializer\Groups({"ArticleDetails"})
     * @Serializer\SerializedName("title")
     */
    protected $title;

Class Comment :

    /**
     * The article the comment belongs to
     *
     * @var Article
     *
     * @ORM\ManyToOne(targetEntity="Leadz\DummyBundle\Entity\Article", inversedBy="comments")
     * @ORM\JoinColumn(name="article_id", referencedColumnName="id", nullable=false)
     *
     * @Assert\NotNull()
     *
     * @Serializer\Expose()
     * @Serializer\Groups({"CommentDetails"})
     */
    protected $article;

CommentController :

/**
     * Retrieves a list of comments
     *
     * @Rest\View(serializerGroups={"List", "CommentDetails"})
     *
     * @ApiDoc(
     *  resource=true,
     *  description="Retrieves a list of comments.",
     *  filters={
     *      {"name"="limit", "dataType"="integer", "description"="Limit the number of comments", "default"="10"},
     *      {"name"="offset", "dataType"="integer", "description"="Offset of the results", "default"="0"},
     *  },
     *  statusCodes={
     *      200="Returned when successful"
     *  },
     *  output={
     *      "class"="Leadz\DummyBundle\Entity\Comment",
     *      "groups"={"List", "CommentDetails"}
     *  }
     * )
     *
     * @param Request $request
     *
     * @return Entity\Comment[]
     */
    public function getCommentsAction(Request $request)

As you can see, the CommentController uses the CommentDetails group.

Result :
capture d ecran 2014-10-09 a 09 40 37

If I remove @Assert\NotBlank() in the Article class, then :

capture d ecran 2014-10-09 a 09 40 31

I also have the same issue. Seems that any entity property having validation rules enabled automatically added into "output" doc section. Moreover, I have firstName property of User entity with NotBlank assertion which described in serializer\Entity.User.yml. The result is that I have firstName at the top of the doc (which is wrong; note that it is in camelCase, not underscored) and first_name next (which is right and that I need, and that I described in serializer\Entity.User.yml).

The same problem =/
Any solutions ?

Unproper patch (see comments at end):

--- a/Parser/JmsMetadataParser.php
+++ b/Parser/JmsMetadataParser.php
@@ -85,7 +85,7 @@ class JmsMetadataParser implements ParserInterface, PostParserInterface
     public function parse(array $input)
     {
         $className = $input['class'];
-        $groups    = $input['groups'];
+        $groups    = !empty($input['groups']) ? $input['groups'] : array("Default");

         return $this->doParse($className, array(), $groups);
     }
@@ -133,6 +133,7 @@ class JmsMetadataParser implements ParserInterface, PostParserInterface
                 // apply exclusion strategies
                 foreach ($exclusionStrategies as $strategy) {
                     if (true === $strategy->shouldSkipProperty($item, SerializationContext::create())) {
+                        $params[$name] = null;
                         continue 2;
                     }
                 }
@@ -274,6 +275,11 @@ class JmsMetadataParser implements ParserInterface, PostParserInterface
     protected function doPostParse (array $parameters, array $visited = array(), array $groups = array())
     {
         foreach ($parameters as $param => $data) {
+               if (!array_key_exists('description', $data))
+                           $parameters[$param] = null;
+        }
+
+        foreach ($parameters as $param => $data) {
             if (isset($data['class']) && isset($data['children']) && !in_array($data['class'], $visited)) {
                 $visited[] = $data['class'];

First chunk sets group to Default... I guess unrelated to this PR but I find it odd that it doesn't do this.

Second chunk partially fixes the issue described here: when items should be excuded due to groups it will mark them as "please delete this" (by setting $parameters[$param] = null). I guess this only works because JmsMetadataParser is apparently called after the Validation parser (sequence).

Third chunk, this is hackish and needs to be done in a differentl way: it will strip any parameters that were not parsed by JmsMetadataParser (recognized by a key value, chose description..).

If you don't do the 3rd chunk then for example if you have any unexposed objects, such as $password, then this won't show up in foreach ($meta->propertyMetadata as $item) { (in Parser/JmsMetadataParser.php:127)... and thus the skipping (the newly added $params[$name] = null;) will never happen.

Sorry for the lack of a PR but this patch shouldn't go in as-is anyway ;)

Please solve this issue

@syzop : thanks

+1

Any way to get this merged? @sghribi can you open a PR for this? That code works perfectly for me.

Here is the following test suite output with @sghribi's patch merged into master:

There were 2 failures:

1) NelmioApiDocBundle\Tests\Parser\JmsMetadataParserTest::testParserWithGroups
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     'foo' => Array (...)
     'bar' => Array (...)
-    'baz' => Array (...)
+    'baz' => null
 )

/Users/william/projects/NelmioApiDocBundle/Tests/Parser/JmsMetadataParserTest.php:201

2) NelmioApiDocBundle\Tests\Parser\JmsMetadataParserTest::testNestedGroups
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
             'hidden' => Array (...)
+            'foo' => null
         )
     )
+    'foo' => null
+    'bar' => null
 )

Bug still here. Any updates?

Any update regarding this bug?

@sghribi's fork seems to be working for me, see this repo.

Not sure if there's a proper PR or not, but its been working great for us.

Let's merge !!!

I did a fork to try to resolve this issue. If I am able to succeed. I would do a PR.

Okey I found what is happing.

In ApiDocExtractor::extractData, several Parsers can be used.
In our case, two are called to get parameters,Nelmio\ApiDocBundle\Parser\ValidationParser and Nelmio\ApiDocBundle\Parser\JmsMetadataParser

ValidationParser is called first. As it was reported in https://github.com/nelmio/NelmioApiDocBundle/issues/125#issuecomment-148345123, ValidationParser does not consider @Groups in entities but consider all other proprieties with @Assert.

When JmsMetadataParser is called, $parameters had already parameters extracted with ValidationParser.
To don't lose information $parameters variable are erased with the result of mergeParameters function.
At the end , $parameters contains values come from ValidationParser and JmsMetadataParser.

Problem this is here ! Because ValidationParser extracts all proprieties with @Assert without checking if any of one have @Groups annotations.

Code to input extraction to get a better understanding (is the same with output).

$supportedParsers = array();
            foreach ($this->getParsers($normalizedInput) as $parser) {
                if ($parser->supports($normalizedInput)) {
                    $supportedParsers[] = $parser;
                    $parameters         = $this->mergeParameters($parameters, $parser->parse($normalizedInput));
                }
            }

I did a very simple PR. But before waiting PR merging, here one situation !

     *  output= {
     *     "class"="Freedom\DemophonieBundle\Entity\Decision",
     *     "groups"={"list"},
     *     "parsers"={"Nelmio\ApiDocBundle\Parser\JmsMetadataParser"},
     *  },

Only JmsMetadataParser will be used.
Enjoy !

Can we somehow get this merged, please?

@GuilhemN ping

@adiachenko @rcatlin it looks like it has some issues, see https://github.com/nelmio/NelmioApiDocBundle/issues/525#issuecomment-151437507. Please open a PR to discuss more easily.

BTW @willdurand should we still merge features/bug fixes in 2.x ?

Are there any news on this issue?

Also having this issue. Any working forks that can resolve this? Basically I want to be able to define doc-specific groups and have those groups determine which parameters are shown regardless of if they are JMS Serializer groups, or Symfony constraint groups. Basically if I set a group on an ApiDoc block, I don't want anything shown if it doesn't have that group explicitly.

Actually the above post will solve the problem for me if you only specify the JMS parser. It's a workaround for now since we still lose the ability to specify the 'required' aspect in the docs based on annotations, but it's a workaround for now.

Is it solved or we should use workaround?

Thanks

Same question: Is it solved or we should use workaround?

Thanks

I don't think this is solved. I am using a fork as specified in my comment above and it has been working.

if one of you could do a pull request to fix it, that would be appreciated.

Was this page helpful?
0 / 5 - 0 ratings