Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow an input and an output for a given resource class #2235

Merged
merged 1 commit into from
Nov 2, 2018

Conversation

soyuka
Copy link
Member

@soyuka soyuka commented Oct 4, 2018

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets may fix some issues
License MIT
Doc PR api-platform/docs#703

@soyuka soyuka force-pushed the feat-input-output branch 2 times, most recently from b43fc65 to 493ff05 Compare October 5, 2018 11:32
@meyerbaptiste
Copy link
Member

You have to add these two new keys in the ApiResource annotation.

@soyuka soyuka force-pushed the feat-input-output branch from 493ff05 to 2debd03 Compare October 5, 2018 11:57
@soyuka soyuka force-pushed the feat-input-output branch from 2debd03 to 10b1920 Compare October 5, 2018 12:02
Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

Some minor comments, but awesome!

src/Bridge/Symfony/Routing/ApiLoader.php Outdated Show resolved Hide resolved
src/Serializer/AbstractItemNormalizer.php Outdated Show resolved Hide resolved
@ragboyjr
Copy link
Contributor

@soyuka @dunglas Few things:

  1. I don't see any changes to the Swagger Documentation Normalizer, so it's possible that the swagger docs could be out of sync with the actual api.
  2. I implemented a similar feature here: https://github.com/krakphp/api-platform-extra#operation-resource-classes. The difference however is that mine is set per resource, and this is set on the class level. I'm not sure which one is better, but it may be a thing where both would valuable. Bringing this up for discussion though.

In my implementation, in order to keep the documentation in sync, I added a new attribute to schema_only which is used for defining resources that don't have any operations and are used just for being in the swagger definitions.

Let me know what you think.

@ragboyjr
Copy link
Contributor

ragboyjr commented Oct 12, 2018

Oh, and most important thing... Thanks for all the hard work you too do for this project! I'm like 4 api platform projects in and I'm loving it more and more!

@soyuka soyuka force-pushed the feat-input-output branch from 2574806 to cb5421a Compare November 2, 2018 09:42
@dunglas dunglas merged commit 4f12b21 into api-platform:master Nov 2, 2018
@dunglas
Copy link
Member

dunglas commented Nov 2, 2018

Thanks Antoine!

@sroze
Copy link
Contributor

sroze commented Dec 7, 2018

Not bad 😉

@karser
Copy link
Contributor

karser commented Dec 9, 2018

This looks great, but still not clear the transition between

!> The following isn't recommended anymore, please use Input/Output instead
/**
 * @ApiResource(
 *      collectionOperations={
 *          "post"={
 *              "path"="/users/forgot-password-request",
 *          },
 *      },
 *      itemOperations={},
 * )
 */
final class ForgotPasswordRequest

And

/**
 * @ApiResource(
 *   inputClass=Input::class,
 *   outputClass=Output::class,
 * )
 */
final class Data

Is input/output only resource level or it can be applied for operation as well?

Is it still possible to use Input/Data in the subscriber on POST_VALIDATE event or it works only in the data persister?

What would be the correct yaml syntax? This didn't work, at least wasn't relfected in swagger:

App\Dto\ForgotPasswordRequest:
    inputClass: 'App\Dto\ForgotPasswordRequest'
    outputClass: 'App\Dto\ForgotPasswordResponse'
    collectionOperations:
        post:
            path: '/users/forgot-password-request'
    itemOperations: {}

@ragboyjr
Copy link
Contributor

ragboyjr commented Dec 9, 2018

@karser, from what I can tell, these can be applied at the resource level or at the operation level (which freaking rocks)

@ragboyjr
Copy link
Contributor

ragboyjr commented Dec 9, 2018

It looks like we'll need to update the swagger documentation generator to take into account these changes as well.

@soyuka
Copy link
Member Author

soyuka commented Dec 10, 2018

Is input/output only resource level or it can be applied for operation as well?

It should be available yes, if not lmk I'll fix it.

Is it still possible to use Input/Data in the subscriber on POST_VALIDATE event or it works only in the data persister?

In fact, the input class is mainly used to know how the (de)normalizer should work. As the validate event comes before the write (data persister) you should definitely be able to use the input data there.

What would be the correct yaml syntax? This didn't work, at least wasn't relfected in swagger:

I need to update the swagger documentation to reflect the changes. These are attributes first guess would be:

App\Dto\ForgotPasswordRequest:
    attributes:
        inputClass: 'App\Dto\ForgotPasswordRequest'
        outputClass: 'App\Dto\ForgotPasswordResponse'

Note that you can use either the input, the output or both.

@karser
Copy link
Contributor

karser commented Dec 10, 2018

Thanks, I'll try it within today

@karser
Copy link
Contributor

karser commented Dec 10, 2018

I tried added outputClass in attributes and it still works (at least api tests don't fail) but swagger part is still not displaying the outputClass.
I see, the swagger part is not implemented #2305
Now I realize that I can switch from setting response in subscriber to data persister and let api-platform serialize the result.

@lyrixx
Copy link
Contributor

lyrixx commented Dec 20, 2018

I totally missed this feature. And I really love this feature. I will be able to remove custom code \o/ Thanks.

Just a side note not related: You really need a blog (I'm not talking about https://api-platform.com/news/). Something like Symfony "Living on the Edge
" > "New in Symfony 4.3: Console Hyperlinks" https://symfony.com/blog/new-in-symfony-4-3-console-hyperlinks and another category about releases.

@dunglas
Copy link
Member

dunglas commented Dec 20, 2018

@lyrixx I try to write a blog post for every releases with an explanation of new features like this one: https://dunglas.fr/category/programmation/php/api-platform/

This feature hasn't been released yet, it's why there are no blog posts about it yet.

However having a proper blog, or maybe a Planet, would be nice. We'll do it at some point.

@maks-rafalko
Copy link
Contributor

I am curious, what is Planet? :)

@dunglas
Copy link
Member

dunglas commented Dec 20, 2018

Basically a RSS feeds aggregator: https://en.wikipedia.org/wiki/Planet_(software)
2006, I feel old now 😅

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.

9 participants