framework
framework copied to clipboard
refactor: JSON:API
Closes #3964
Changes proposed in this pull request: This is a difficult review, I couldn't really find a way to split this since it can only be all done together, I would recommend looking at some specific resources, for example, comparing the main branch Discussion model JSON API with this one. The controllers and serializer class and command handlers vs the DiscussionResource class.
Reading the issue itself will help you a lot in getting context about the changes here. Other than that, some key things to think about:
- Do we want to add out fork of the library into the monorepo? I'm leaning towards yes.
- Any specific concerns about the new API? everything that an extension dev could do before should be able to do now as well, only differently. Once we start updating FoF extensions we will be able to see and make any necessary changes.
Some notable changes
- The
validation.yml
file has been updated. - The discussions sortmap is no longer bound to the container, instead the
SortColumn
on the DiscussionResource can receive anascendingAlias
anddescendngAlias
which can them easily be accessed in the content classes, this also means extensions can add to the sortmap by just the sorts they add themselves in the same manner.
Todo
- [x] Tag a 1.0 of the forked library: https://github.com/flarum/json-api-server
- [x] Extensively document the changes from 1.0 to extension devs. (https://github.com/flarum/docs/pull/472)
still a couple test failures to fix as well, and phpstan..
serializers have been replaced with resources, and resources contain not just endpoint instruction, but also field instructions
Yes, and columns. Filters have been disabled since we have our own driver-based filter implementation (which we can always improve in terms of boilerplate as well).
many of the controllers have been simplified or removed entirely because of this refactor, which is also nice. I do think it will need some getting used to, to easily find where what is declared when looking things up.
Keep in mind, you can still create a separate controller the old fashion way, only there are non of the previous parent serializer classes. So the result of it cannot be the serialization of a resource. You can also just use a custom endpoint (see user resource delete avatar).
Can we somehow get rid of all the Command/Handlers and move them to the respective controllers where possible or would that better be done in a follow up PR?
They are still useful reusable domain logic, for example check the delete avatar custom endpoint in the user resource class. Now that most controllers have been removed, command handlers aren't actually bad to have in terms of boilerplate. An additional improvement to them would be to make them one class rather than requiring two classes imo.
documentation added: https://github.com/flarum/docs/pull/472
There's two things I'd love to be able to do:
- allow custom paths that aren't prefixed with
/api
, eg/.well-known/webfinger
- allow a callback to be executed before payload is send to the user, so I can flatten the response or filter some values. I considered a middleware, but I'm not sure this feature could be applied for other purposes than mine (flattening the json payload by pushing
attributes
one level up). And yes this is a hack, but extensibility is about flexibility too.
Overall using this is very nice, it reminds me a lot of the filament resources logic. I have had no issue declaring the resources, their endpoints and their schema. So 👍
allow custom paths that aren't prefixed with /api, eg /.well-known/webfinger
That would mean adding a route to the forum rather than the API, it's quite messy and would require quite a lot of changes.
This is an edge case where I would rather you use a normal controller. Does this endpoint require the api resourceat all? if it returns custom json structure that isn't actually of the JSON:API spec for resources, all the more reason to use a normal controller instead.
If it does return valid json for a resource, then checkout the ShowForumController
controller and how you can call an api resource's endpoint.
allow a callback to be executed before payload is send to the user, so I can flatten the response or filter some values.
You can use the response
method on the endpoint. Though again, if you are returning non json api spec json, what you might be needing to do instead is using custom controllers.
Will merge after the DB PRs are merged.
@SychO9 this one could/should be merged asap. Let's work off of this codebase moving forward before any other branches change dramatically.