Open lindyhopchris opened 1 year ago
@lindyhopchris Just one suggestion here. pestphp is really cool tool for writing testcase. Can we use it as a defacto standard for testing?
@lindyhopchris Regarding API documentation, I think this is the most current option which was forked from my original effort https://github.com/swisnl/openapi-spec-generator.
Would it make sense to review and perhaps bring the package into the organisation so it is more official and can be pushed forward?
@glennjacobs I've not had time to look through that yet, but yeah it's likely to be a good starting point when I get to the documentation
Good news, having started developer for v4, I've added JSON Schema as in-scope for development. Which will be a massive step towards documentation.
Does version 4 also have optimalizations over version 3? As mentioned in some other issue (or in the Slack), I had the feeling that my code ran way to long when calling the toResponse
method.
I promised you some benchmarking, so that you have an idea where it is slowing down the process. If I have time, I will make some scripts to generate some benchmark data.
@ben221199 as I mentioned in that thread, toResponse
is just way too general to know that any performance problems are in this package. That method triggers the encoding - which ends up calling loads of Laravel Eloquent code, plus any logic you've built into the serialization on the resource fields, and any logic you've put on your Eloquent models as well.
As an example, if you're serializing 100 models to resources, with 15 attributes + relations per model, this code in the Eloquent model will be hit 1,500 times: https://github.com/illuminate/database/blob/3025a3669248185f4af576759b11e461c9edf5b2/Eloquent/Concerns/HasAttributes.php#L428-L455
I'm not seeing any performance issues in the apps I've written. That's not me saying there aren't performance issues - but I'd need something to be traced to a slow execution within this package itself, rather than code in this package that triggers a huge amount of code that's outside of this package's control.
That is why I will try some benchmarking in the near future. I hope I can give some information that hopefully can solve some bottlenecks I experience. In case I don't find anything, I should look to other solutions.
@lindyhopchris Do you have any idea of timeframe for v4?
@glennjacobs I'm working on it at the moment, it's going to take a good few months to work through everything. I might also do it as several major releases, as that'll probably make upgrading a bit easier - i.e. several upgrade steps to move through major versions, rather than one huge upgrade.
@glennjacobs I'm working on it at the moment, it's going to take a good few months to work through everything. I might also do it as several major releases, as that'll probably make upgrading a bit easier - i.e. several upgrade steps to move through major versions, rather than one huge upgrade.
Sounds good. The bit I'm most interested in is the JSON schema for generating documentation. I'd be keen to help on that with any beta testing or w.e.
Hmm yeah not sure when I'll hit the JSON schema part.
The first release will definitely be the decoupling from form requests (required to unblock atomic operations plus programatic execution) along with moving validation rules to the schemas i.e. validation rules on field classes. That's a relatively large upgrade, which is why I think I'll do it as a release in its own right, with its own upgrade guide.
Then I'll probably add some non-breaking things from the list above during the v4 timeline.
Then v5 will be the remaining breaking things from the list. JSON Schema will probably drop here, as I suspect I might need a few breaking changes to do it.
I'm hoping to be done by the end of July / early August, but it does all depend on how much time I get as I'm doing this all in my free time i.e. no work time on it.
@lindyhopchris just wondered how you were getting on?
Yeah good thanks. I've refactored the internal plumbing of the package, which was necessary to decouple from Laravel form requests - paving the way for Atomic Operations and programmatic execution.
At the moment I'm working on moving validation rules to the field, filter and pagination classes. Which will give a much more Nova-style validation implementation. This is part of decoupling from the form requests, plus also is going to be much nicer DX.
I'm now planning to release all of the features at the top of this issue over a number of major releases, rather than all at once. That's to make upgrading in stages easier.
@lindyhopchris while you are busy working on refactoring, I am hoping you will consider expanding the sort abilities to allow sorting a main resource based on relation value. This is supported in the JSON API spec. The easiest example might be when using a Profile
relation on a User
and the profile has additional info contained in it and you would want sort the user list based on some profile field without loading the whole collection into memory. Please see #205 for more details.
I have dug into the code and see no easy way to implement this without considerable refactoring. Perhaps you have a better idea on how to make it work.
@freestyledork if it's possible to support (i.e. we can do it in Eloquent) then yes, it would be good to add this feature. I've asked a question on #205 - can we move the discussion about it there rather than on this issue? Thanks.
Wondering how things are going for you?
I have recently resumed work on this. Balancing the amount of work on it verses other work, but should now be able to make progress. Planning to allocate time each week for it, so I make steady progress rather than putting it totally down for other work.
Hi @lindyhopchris, first of all, thanks for this amazing package. Definitely, my favourite Laravel package.
I'm extensively using it for a new open source project I'm planning to launch around October, what means I'll be spending a lot of time on using the Laravel Json Api package. I'll be happy to contribute as much as I can.
Ah thanks for the feedback!
I'm close to an alpha release of the re-write of the package internals - which unlocks a huge amount of the new features that need to be built. What's particularly nice about the alpha is it moves validation rules to the fields, filter etc classes. I wasn't sure how that would turn out, but actually it's looking really nice and a good step forward.
It's a bit difficult to get someone to help with getting that alpha out, because so much has changed and really I just need to push it over the line. There might be scope to help beyond that - particularly with trying the alpha out.
It's a bit difficult to get someone to help with getting that alpha out, because so much has changed and really I just need to push it over the line. There might be scope to help beyond that - particularly with trying the alpha out.
Nice to hear! I'll be waiting for that alpha release to do some testing.
Update and example of schemas in the new world here: https://github.com/laravel-json-api/laravel/issues/39#issuecomment-2101260482
@ben221199 as I mentioned in that thread,
toResponse
is just way too general to know that any performance problems are in this package. That method triggers the encoding - which ends up calling loads of Laravel Eloquent code, plus any logic you've built into the serialization on the resource fields, and any logic you've put on your Eloquent models as well.
A year ago I posted something about performance here. Recently I installed Xdebug, so here is some profiling:
And even deeper:
The total request is 4 seconds. 25% of that is getting my cached models from Redis and unserializing them. The other 75% is encoding it to JSON. It seems that addResourceToIncluded
takes a while, specificly getResourceRepresentation
.
I will debug further to find out if it is only my implementation, or that it is something in the library.
May be create a separate issue if needed - that's all way too specific and not related to this issue which is about package development?
FYI getResourceRepresentation
will be triggering all the code in schemas etc to work out what values are in the JSON. Which is why I can't definitely say it's a problem with this package - because it could be hitting implementation-specific code in your schema, your Eloquent model, etc etc.
I will investigate further and will create a seperate issue if needed.
Many of you may have noticed I haven't been doing a lot of work on this project recently since v3 (Laravel 10) dropped earlier this year. To explain, open source time for me is limited at the moment - I've moved to a job that doesn't use this package plus I've had a lot going on outside of work. The combination means I haven't really been keeping on top of things!
The good news is from the middle of May 2023 I will have more time available to look at the new features that need adding to this package.
I wanted to share here what my priorities are. I.e. what is in-scope for v4. My hit-list of features are as follows:
JsonApiRoute::mount()
, customised via PHP attributes on the schema.SELECT
statements to retrieve only the fields that are actually needed in responses - #146 plus only iterating over sparse fields in resources when sparse fields are being used - #241It's worth noting that this is a big set of work, and will be breaking. To support Atomic Operations and allow the implementation to be consumed programmatically, I need to decouple the implementation from Laravel's form requests. This will be a big step forward, especially as Atomic Operations is one of the big features lots of us want. But it does mean there will be some refactoring when upgrading an application that uses Laravel JSON:API.
Finally I want to mention OpenAPI documentation. I know a lot of people want this. I do want to add it at some point. However, the above list provides a number of features I need in my production APIs. So for me, they are higher priority - and as the creator of this open source project, the reality is I have to prioritise my time on things that are priorities for my projects.
However, the good news is that if I clear the above list, OpenAPI documentation would most likely be the next highest priority to solve.