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

[3.0] Move relation providers in compile time #268

Merged
merged 5 commits into from
Nov 23, 2018
Merged

Conversation

goetas
Copy link
Collaborator

@goetas goetas commented Nov 16, 2018

This moves relation providers at compile (cache) time.

continuation of #266 and #267

@goetas goetas added this to the 3.0 milestone Nov 16, 2018
@goetas goetas force-pushed the v3-relation-providers branch from 6528e3b to f664828 Compare November 16, 2018 15:43
@goetas
Copy link
Collaborator Author

goetas commented Nov 16, 2018

All the tests that use collections are failing as per how collections are handled, their relations are not static.

We have some options:

  1. drop support for collection/vnd representations (~20% of the issues on github are related to collections) and wait for a better solution (user-land typed classes for collections)

  2. drop support for collection/vnd representations as they are now and offer an alternative version for:

    • VndErrorRepresentation results can be achieved by playing a bit with expression language and not allowing the user to choose the rel name (imo still good compromise)
    • CollectionRepresentation could be more difficult, my idea is to drop what can not be achieved and standardize the pagination-mode by offering some presets but not customizable as they are now. (as it is now, is not possible to generate nelmio-api-doc for CollectionRepresentation, if we make it cacheable it will be possible)

The second proposal is similar to the first excerpt that we offer to the user some alternatives.

@goetas
Copy link
Collaborator Author

goetas commented Nov 17, 2018

ok, this are the results so far regarding my previous comment:

  • VndErrorRepresentation not works almost as before.

    • Dropped the support for additional "title" attribute as it is not part of the specs.
    • Now the constructor takes string as arguments instead of Relation.
  • CollectionRepresentation does not allow customization of rel names and addition of extra relation information. Prefer to implement a proper typed "CollectionRepresentation-style" class declaring the data type present inside (this will allow also a better generation of documentation when using nelmio api-doc)

IMO all the features that are in the Hateoas\Representation namespace are too connected to the user-application logic and too opinionated for a generic use, so any change will always be a compromise.

@adrienbrault
Copy link
Contributor

Looks good.

Regarding the CollectionRepresentation, we can keep it as an easy way to create collections and/or a sample

@goetas goetas changed the base branch from v3 to develop November 23, 2018 10:10
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.

2 participants