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

Decorating all item normalizers on master branch #2316

Closed
silverbackdan opened this issue Nov 6, 2018 · 8 comments
Closed

Decorating all item normalizers on master branch #2316

silverbackdan opened this issue Nov 6, 2018 · 8 comments

Comments

@silverbackdan
Copy link
Contributor

Would anyone be able to give a quick example of how to create a custom item normalizer using the current master branch?

It can be done as documented, but there are issues.

  1. If you document the item normalizer as documented, you need to add a priority at 8 or higher. This is to make sure it is called over the other item normalizers (jsonld, graphql etc.)
  2. Once that is done, the new normalizer will be called and the additional array keys (e.g. @id) are not added.

The solution to this is to instead decorate the jsonld item normalizer, but then this normalizer wouldn't work if I wanted the response in another format (e.g. xml). I'm in a position with my project that json alone is fine, but it seems like a new bug would be introduced if the current master branch were to be released.

Would it be possible and appropriate to update the service definition for the item normalizer that is documented to be used to be whichever item normalizer is going to be used on the current request? E.g. a jsonld request will make the api_platform.serializer.normalizer.item service definition an alias for api_platform.jsonld.normalizer.item and therefore decorating api_platform.serializer.normalizer.item would still work no matter what format the response should be in. I'm not sure how to go about implementing this or if that is required. It could be that custom normalizers just need to be implemented slightly differently to how it is currently documented for the changes in the master branch.

@soyuka
Copy link
Member

soyuka commented Nov 7, 2018

I think that you'd need to have multiple decorations if you want to support multiple formats. May I ask what your actual need is?

Another solution would be to override the symfony ChainNormalizer thing and do your stuff before calling the correct normalizer.

@silverbackdan
Copy link
Contributor Author

silverbackdan commented Nov 7, 2018

I see, perhaps the chain normalizer is what I'll be after.

In my case I just want to add some data to the output for entities with a specific interface. Specifically, a database entity can be converted to a paragraph of text. There are some values I do not persist to the database

  • the initial template (twig template string)
  • the placeholders if a value doesn't exist
  • the values that should be included if they are not null (sometimes the values from the entity are converted depending on values of other properties so it can get a little complicated).

Currently this is an internal API for a web app, but in future it may be released as a public API for others to get use out of so I'd like to code it correctly for different formats to be able to include this generated string.

Would you be kind enough to get me started with overriding the chain normalizer with a link?

Thanks for your help!

@soyuka
Copy link
Member

soyuka commented Nov 7, 2018

Check https://github.com/soyuka/api-platform-2316

I'm really not sure that this is the best solution but it looks to work pretty well.

What I did:

  • register a normalizer that has the highest priority so that it gets called first
  • inject the normalizers inside that normalizer
  • do your stuff, then call the other normalizers just as they would have been called if this one hasn't been

It's a bit of a hack though, the cleanest thing would be to decorate the normalizers one by one.

@silverbackdan
Copy link
Contributor Author

Thank you @soyuka - that's really kind of you to show this. Does this mean you are gearing up for a major release of API Platform from the master branch? It does seem like it'll break anyone's custom normalizer implementations as they will have followed the docs.

I see it's a bit of a hack but it feels to me like it'd be a little more future proof should other standards be introduced that API Platform looks to support. I suppose that's all theoretical though so... I'll mull it over before jumping into either one. I'm happy enough to implement the normalizer in one of these 2 ways. Maybe a docs update will be needed for the upcoming release?

@soyuka
Copy link
Member

soyuka commented Nov 7, 2018

that's really kind of you to show this. Does this mean you are gearing up for a major release of API Platform from the master branch? It does seem like it'll break anyone's custom normalizer implementations as they will have followed the docs.

I'm really not sure to follow, how will the master branch break what's documented and where?
Btw my example uses 2.3 not master.

Maybe a docs update will be needed for the upcoming release?

You asked for a normalizer that can "add some data to the output for entities with a specific interface" without being format-specific. The correct way here would be to decorate the JsonLd item normalizer but this would be format-specific and is a no-go for you. I'd not recommend my hack (unless you can't do otherwise) and definitely not document it haha.

@silverbackdan
Copy link
Contributor Author

silverbackdan commented Nov 7, 2018

I'm really not sure to follow, how will the master branch break what's documented and where?
I was referencing these

https://api-platform.com/docs/core/content-negotiation/#writing-a-custom-normalizer
https://api-platform.com/docs/core/serialization/#decorating-a-serializer-and-adding-extra-data

I now see that the docs for adding extra data does indeed decorate the jsonld item normalizer. I thought it was the same as the other link which just registers a custom normalizer.

I'd not recommend my hack (unless you can't do otherwise) and definitely not document it haha.
I will take your advice and decorate each normalizer as support for the output format is required

Thanks again for your time on this issue - and sorry for my mistakes in reading the docs. I'm wondering if I should be implementing this functionality some other way. Perhaps creating a custom data provider for entities with this interface which extends the doctrine orm provider and generate the paragraph in there.

As there is no issue here I'll close this, but if you do have an opinion for me and can spare a moment I'd really appreciate it.

@soyuka
Copy link
Member

soyuka commented Nov 7, 2018

I now see that the docs for adding extra data does indeed decorate the jsonld item normalizer. I thought it was the same as the other link which just registers a custom normalizer.

Got it now, indeed :).

Thanks again for your time on this issue - and sorry for my mistakes in reading the docs. I'm wondering if I should be implementing this functionality some other way. Perhaps creating a custom data provider for entities with this interface which extends the doctrine orm provider and generate the paragraph in there.

I had a third way in mind. You could wrap your data in a "wrapper" that acts as DTO. You could do that in an event listener (POST_READ - see docs).

If you want to use master you could even benefit of a brand new feature that I'm in the process of documenting (api-platform/docs#646 - #2235).

@silverbackdan
Copy link
Contributor Author

If you want to use master you could even benefit of a brand new feature that I'm in the process of documenting (link docs here in ~1h - #2235).

That's fantastic, I personally think both of these approaches would be more beneficial for adding data to a resource for a response when it is required for every output format. Perhaps with the docs update a note could be added to the normalizer example that adds data, saying 'if the data should be added to all response formats to do it with this new feature' :)

Thanks again and the new feature looks great!

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

No branches or pull requests

2 participants