Skip to content

Conversation

Simperfit
Copy link
Contributor

@Simperfit Simperfit commented Jun 28, 2016

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #582
License MIT
Doc PR

I think i'm not bad on this but there is a part of this I don't know how to handle :
Since we are overriding on method, how do I tell the property-info / nelmio that we want this only for the PUT part ?

/**
 * @see http://schema.org/Thing
 *
 * 
 * @ApiResource(iri="http://schema.org/Thing", attributes={
 *     "normalization_context"={"groups"={"thing_default_output"}},
 *     "denormalization_context"={"groups"={"thing_default_input"}}
 * }, itemOperations={
 *     "get"={"method"="GET", "normalization_context"={"groups"={"thing_default_out_get"}}, "denormalization_context"={"groups"={"thing_default_in_get"}}},
 *     "put"={"method"="PUT", "normalization_context"={"groups"={"thing_default_out_put"}}, "denormalization_context"={"groups"={"thing_default_in_put"}}},
 *     }
 * )
 */

Right now I only get the overrided part in the doc. It is the intended behaviour ?

Which means I lost the ones in the attributes.

@dunglas What do I miss ?

@Simperfit Simperfit force-pushed the feature/supports-attribute-de-normalization-in-doc branch from e330e79 to e7acfef Compare June 28, 2016 13:51
@teohhanhui
Copy link
Contributor

teohhanhui commented Jun 28, 2016

@Simperfit This is not the right way to fix it. Please use ResourceMetadata::getItemOperationAttribute.

And the same fix needs to be applied to the collection operations too (because normalization and denormalization also take place).

@Simperfit
Copy link
Contributor Author

@teohhanhui This does not change the behaviour, or does it, just to reuse already implemented method ?

Yes thanks I forgot about the collection.

@Simperfit Simperfit force-pushed the feature/supports-attribute-de-normalization-in-doc branch from ae2a8b6 to 2706b5e Compare June 28, 2016 19:04
@Simperfit
Copy link
Contributor Author

@teohhanhui I've changed.
$options is good and contain all what it needs :
if we take the exemple on the PR :

'serializer_groups' =>
'thing_default_output',
'thing_default_input',
'thing_default_out_get',
'thing_default_out_put',

But it does not show the parameters linked to thing_default_out_put / thing_default_out_get

@Simperfit Simperfit force-pushed the feature/supports-attribute-de-normalization-in-doc branch 2 times, most recently from ab306bd to b5eed48 Compare June 28, 2016 20:03
@Simperfit Simperfit force-pushed the feature/supports-attribute-de-normalization-in-doc branch from b5eed48 to 17beb51 Compare June 30, 2016 18:36
@Simperfit Simperfit changed the title [WIP] fix #582 itemOperation Normalization/Denormalization in doc fix #582 itemOperation Normalization/Denormalization in doc Jun 30, 2016
@Simperfit
Copy link
Contributor Author

ping @AlexandreHagen could you please test this and tell me if it's correct the issue ?

ping @teohhanhui @dunglas @theofidry this is RFR.

@Simperfit Simperfit force-pushed the feature/supports-attribute-de-normalization-in-doc branch from f6dd043 to 51c6dd6 Compare June 30, 2016 18:50
@AlexandreHagen
Copy link

AlexandreHagen commented Jul 1, 2016

Hi @Simperfit

Unfortunately, it is not seem to work as expected:

Annotations setup:

* @ApiResource(
 *     attributes={
 *          "normalization_context"={"groups"={"user","user_read"}},
 *          "denormalization_context"={"groups"={"user","user_write"}},
 *     },
 *     collectionOperations={
 *          "get"={"method"="GET"},
 *          "post"={"method"="POST"},
 *     },
 *     itemOperations={
 *          "get"={"method"="GET"},
 *          "delete"={"method"="DELETE"},
 *          "put"={
 *              "method"="PUT",
 *              "normalization_context"={"groups"={"user_read_put"}},
 *              "denormalization_context"={"groups"={"put"}},
 *          }
 *     },
 * ) 
...
 /**
     * @var string Username of the user.
     *
     * @Groups({"user_read", "user_write", "user_read_put"})
     * @Assert\NotBlank()
     */
    protected $username;

    /**
     * @var string Email of the user.
     *
     * @Groups({"user_read", "user_write"})
     * @Assert\Email(message="The email '{{ value }}' is not a valid email.", checkMX=true, checkHost=true)
     */
    protected $email;

    /**
     * @var string Firstname of the user.
     *
     * @Groups({"user_read", "user_write", "user_read_put", "user_write_put"})
     *
     * @ORM\Column(name="firstname", type="string", length=255)
     * @Assert\NotBlank
     */
    protected $firstname;
...

And i get:

For POST:

capture d ecran 2016-07-01 a 09 54 06

For PUT:
capture d ecran 2016-07-01 a 09 54 16

@Simperfit
Copy link
Contributor Author

Simperfit commented Jul 1, 2016

@teohhanhui @dunglas @vincentchalamon I need some help in here ;)

I can't get it working, either I get the itemCollection & operationCollection, either the resource groups for one entity. I think it is directly related to nelmio and this has to be checked in nelmio.

I don't think I can do things in ApiPlatform\Core\Bridge\NelmioApiDoc\Extractor\AnnotationsProvider\ApiPlatformProvider class.

Is there another class related to this ?

edit: yep the provider helped me.

@Simperfit Simperfit changed the title fix #582 itemOperation Normalization/Denormalization in doc [wip] fix #582 itemOperation Normalization/Denormalization in doc Jul 2, 2016
@Simperfit Simperfit force-pushed the feature/supports-attribute-de-normalization-in-doc branch 3 times, most recently from f19818d to 7269a33 Compare July 3, 2016 11:40
@Simperfit
Copy link
Contributor Author

ping @AlexandreHagen I think this time is the good time ;). Thank you for ur time.
It does work properly for me.

@Simperfit Simperfit changed the title [wip] fix #582 itemOperation Normalization/Denormalization in doc fix #582 itemOperation Normalization/Denormalization in doc Jul 3, 2016
@AlexandreHagen
Copy link

You are close !

But something still not working.
When we set specific definition of (de)normalisation, property name is displayed as Input and Output.

Example:

 * @ApiResource(
 *     attributes={
 *          "normalization_context"={"groups"={"user_read"}},
 *          "denormalization_context"={"groups"={"user_write"}},
 *     },
 *     collectionOperations={
 *          "get"={"method"="GET"},
 *          "post"={"method"="POST"},
 *          "confirmation"={
 *              "route_name"="user_confirmation",
 *          },
 *          "post_invitation"={
 *              "method"="POST",
 *              "path"="/users/{invitation_code}",
 *              "normalization_context"={"groups"={"user_read_invitation"}},
 *              "denormalization_context"={"groups"={"user_write_invitation"}}
 *          },
 *     },
 * )
...
  /**
     * @var string Username of the user.
     *
     * @Groups({"user_read", "user_write", "user_write_invitation"})
     * @Assert\NotBlank()
     */
    protected $username;

    /**
     * @var string Email of the user.
     *
     * @Groups({"user_read", "user_write", "user_write_invitation"})
     * @Assert\NotBlank()
     * @Assert\Email(message="The email '{{ value }}' is not a valid email.", checkMX=true, checkHost=true)
     */
    protected $email;

    /**
     * @var string Firstname of the user.
     *
     * @Groups({"user_read", "user_write", "user_write_invitation"})
     *
     * @ORM\Column(name="firstname", type="string", length=255)
     * @Assert\NotBlank()
     */
    protected $firstname;

    /**
     * @var string Lastname of the user.
     *
     * @Groups({"user_read", "user_write", "user_write_invitation"})
     * @ORM\Column(name="lastname", type="string", length=255)
     * @Assert\NotBlank()
     */
    protected $lastname;

    /**
     * @var boolean Shows that the user is enabled (email confrimation)
     *
     * @Groups({"user_read", "user_read_invitation"})
     */
    protected $enabled;

    /**
     * @var string Password of the user.
     *
     * @Groups({"user_write", "user_write_invitation"})
     * @Assert\Regex(
     *  pattern="/(?=.*[A-Z])(?=.*[a-z])(?=.*[0-9]).{7,}/",
     *  message="Le mot de passe doit contenir au moins 7 caractères alphanumériques dont une majuscule, une minuscule et un chiffre."
     * )
     */
    protected $plainPassword;
...

Doc:
capture d ecran 2016-07-03 a 22 52 31

As expected plainPassword is only in Input and enabled is only in Output .
But username, email etc... should be only in Output. But they are in both.
It seems that user_read is still considered !

I hope it's help and thanks for you work !

*
* @return array
*/
private function getGroupsForItemAndCollectionOperation(ResourceMetadata $resourceMetadata, string $resourceClass, string $io, string $operationName, array $visited = []) : array
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for? Please just use ResourceMetadata::getItemOperationAttribute and ResourceMetadata::getCollectionOperationAttribute properly. There's already a fallback mechanism in place (4th / last argument).

Copy link
Contributor Author

@Simperfit Simperfit Jul 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what i'm doing inside of it. Look at the parse method, I added the operationName because I needed to. In in order to have the parse method not full of code, I made this method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Simperfit If you just use the fallback mechanism, it'd avoid a lot of unnecessary code.

This method is not needed in the first place.

/cc @dunglas

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@teohhanhui you're right, this method should be deleted.

@dunglas
Copy link
Member

dunglas commented Jul 7, 2016

@Simperfit can you finish this PR? Can I help you in any way?

if (isset($attributes['normalization_context']['groups'])) {
$options['serializer_groups'] = $attributes['normalization_context']['groups'];
}
$options['serializer_groups'] = $resourceMetadata->getAttribute(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the problem is here, shoud should keep the if(isset())/else structure or both read and write groups are added.

@Simperfit Simperfit force-pushed the feature/supports-attribute-de-normalization-in-doc branch from 7269a33 to 82de009 Compare July 7, 2016 08:26
@Simperfit
Copy link
Contributor Author

@dunglas Thank you for the help.

@AlexandreHagen Is it ok for you ? I have been testing this and this I feel it ok. Thanks for the time & and the help

@AlexandreHagen
Copy link

AlexandreHagen commented Jul 7, 2016

I don't know why but I got that:

FatalThrowableError in ApiPlatformProvider.php line 39:
Type error: Argument 2 passed to ApiPlatform\Core\Bridge\NelmioApiDoc\Extractor\AnnotationsProvider\ApiPlatformProvider::__construct() must be an instance of ApiPlatform\Core\Documentation\ApiDocumentationBuilderInterface, instance of ApiPlatform\Core\Hydra\ApiDocumentationBuilder given, called in /var/www/opendecide_api_v2/app/cache/dev/appDevDebugProjectContainer.php on line 958

I'm trying to solve it

@AlexandreHagen
Copy link

AlexandreHagen commented Jul 7, 2016

Okey ! It's can be merged ! Thanks @Simperfit !

@dunglas dunglas merged commit 43b7cb2 into api-platform:master Jul 7, 2016
@dunglas
Copy link
Member

dunglas commented Jul 7, 2016

Thanks for fixing this Hamza.

@Simperfit Simperfit deleted the feature/supports-attribute-de-normalization-in-doc branch July 12, 2016 06:01
magarzon pushed a commit to magarzon/core that referenced this pull request Feb 12, 2017
…oc (api-platform#587)

* feature: fix api-platform#582 add itemOperation Normalization/Denormalization into the doc

* hotfix: take comments in account

* hotfix: fix test

* hotfix: refactor parser

* hotfix: refactor parser

* hotfix: fix test

* hotfix: fix cs

* feat: add tests on attributes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants