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

[RFC] Subresource metadata #2706

Open
soyuka opened this Issue Apr 7, 2019 · 7 comments

Comments

Projects
None yet
7 participants
@soyuka
Copy link
Member

commented Apr 7, 2019

This issue merges multiple subresources issues because the root problem is the same.

While talking with core members at EU-FOSSA Hackathon we came to a solution. Best would be to refactor the ApiSubresource annotation so that it supports the same attributes as does the ApiResource. For example:

  • routing options
  • access_control
  • normalization context

Thanks to this it'll be easier to declare these and we solve the issue (mentioned in #1617 (comment)). The subresource metadata will not be in ApiResource anymore but directly inside the ApiSubresource annotation.

Todo list :

  • Fix swagger and don't show the collection on the side that doesn't own the Subresource (#1612)
  • Fix the serialization context declaration (solves #1617 and #1616 and improves DX for #1649)
  • Simplified the code for the documentation (ses #1736)
  • Subresource operations may override collection operations which should not happend (see api-platform/api-platform#1022)

Maybe that this is a prerequisite for write support on subresources (see #2598 #2428)

ping @torreytsui @vincentchalamon please lmk if I forgot things
@lyrixx I'm closing related PRs we will definitely take back the work you did if we can, thanks for pushing this issues

@lyrixx

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2019

Very good move! Can't wait to have it land in APIP 👍

@Devristo

This comment has been minimized.

Copy link

commented Apr 10, 2019

Would this RFC also help creating custom subresources as described in #1654?

@teohhanhui

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

Fix swagger and don't show the collection on the side that doesn't own the Subresource

I disagree. We could show subresource operations in a separate subsection, maybe? (Not sure if possible with Swagger UI.)

@dunglas

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

Not sure if possible with Swagger UI.

AFAIR we tried and it’s not.

@teohhanhui

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

To answer my own question:

OAI/OpenAPI-Specification#1367
swagger-api/swagger-ui#2129

So, nope.

@teohhanhui

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

An alternative is to tag them as e.g. Question (subresource)?

EDIT: Never mind, I think the suggestion in the original issue is better indeed. Just tag it from the root resource it could be accessed from. But perhaps the operation's description could be improved?

@torreytsui

This comment has been minimized.

Copy link

commented Apr 11, 2019

I think tagging on the root resource seems the best option we have so far. My thought is that subresource no matter how deep is it (/foos/1/bars/1/bazs/1/quxs/...), it starts with the root and the user will have to be aware of it and also consider it.

Fix the serialization context declaration

Should all levels configure in root (i.e., foo below) or spread into each level (i.e., foo configure bar, and bar configure baz)?

  • /foos/1/bars/1/bazs
  • /foos/1/bars/1/bazs/1
  • /foos/1/bars/1

Each of above are different operations and it makes sense to have their own independent configurations. How do we explicitly declare them?

How to enable / disable certain operations? At the moment, there are only item GET and collection GET, but when we implement write operations, it will get complicated and escalate as the depth grows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.