cloudcreativity / laravel-json-api

JSON API (jsonapi.org) package for Laravel applications.
http://laravel-json-api.readthedocs.io/en/latest/
Apache License 2.0
776 stars 109 forks source link

Lumen support #61

Open jelhan opened 7 years ago

jelhan commented 7 years ago

Is there any plan to add support for lumen? I tried out with lumen 5.4 but it seem to break php artisan in similar way as described in https://github.com/laravel/lumen-framework/issues/504.

lindyhopchris commented 7 years ago

Would love to add this, but it hasn't been a priority because we don't have any Lumen apps. I haven't actually used Lumen before so don't know what the differences are in terms of wiring it all up.

Is this something you'd be willing to help with?

jelhan commented 7 years ago

I've started to dig into a little bit. Trying to write a proof of concept for Lumen 5.4. It's not working yet but also I didn't faced any show-stopper yet. What I've figured out so far:

That is how far I got yet. Hopefully I've a little spare time the next weeks to investigate it further.

I'm having a simple lumen app consuming my lumen branch of laravel-json-api to test with. Could push that one also to GitHub if you or someone else also like to work on these issue.

lindyhopchris commented 7 years ago

Hey. This sounds good - thanks so much for putting the time into it. I'll need to look into this in a bit more detail, but here's some initial thoughts...

I think ServiceProvider should be left specifically for Laravel and we should add a LumenServiceProvider. This will just keep things a lot clearer and easier to maintain between the two. The stuff that is common to both could be in an AbstractServiceProvider that both extend?

The routing is the thing that I thought would be the most difficult, considering that Lumen does it completely differently. The regular expression isn't massively important, but the defaults are being used to push the relationship name and the resource type into the route so that the package can detect these values as things happen through the middleware. I'll have to dig around in the Lumen code a bit. If we can't push these in to the routing, then one solution that springs to mind is to provide them to the jsonapi middleware and push them in at that stage.

In terms of a simple Lumen app, that sounds great. There's currently a demo-laravel-json-api repo so it would make sense for me to add a demo-lumen-json-api repo too. If I set that repo up, would you be ok to push your sample app to it? That might help because then I can look around how things work as well.

jelhan commented 7 years ago

Thanks for your fast response and thanks for pointing out what injecting defaults in a route is used for. :+1: I think I found a quick work-a-round to move forward. https://github.com/jelhan/laravel-json-api/commit/9200b176136ef235bd93d7d1414aab82dca486a4

What I've done so far was just meant as a proof of concept. Just a quick write to find possible show-stoppers and have an idea what need to be done. Just wanna prove before that it would work and that it won't be a waste of time before starting refactor laravel-json-api, adding tests etc.

Pushing my sample app in it's current stage is absolutely fine for my.

jelhan commented 7 years ago

Moved forward and faced an issue I wasn't able to resolve yet. CloudCreativity\LaravelJsonApi\Document\LinkFactory expects a Illuminate\Contracts\Routing\UrlGenerator in constructor. That one is not present in Lumen. And I didn't figured out yet at what point a new instance of LinkFactory is created. Do you have any hints?

lindyhopchris commented 7 years ago

It's used to generate paging links, so is injected into the StandardStrategy instance.

Also anything that uses the GeneratesLink trait will be resolving it out of the container. That's in the abstract schema and the controller classes as a helper. It's also in the test helpers and is definitely being used heavily in the testing.

I'm surprised there isn't one in Lumen - how in Lumen do you create a link to a named route?

jelhan commented 7 years ago

Ah yeah, of course. Guess I'm already in weekend mode. Sorry for disturbing you, it's just Laravel\Lumen\Routing\UrlGenerator. https://github.com/jelhan/laravel-json-api/commit/dd57d984323ab2d4d5a13547d13601d2c05fe946

A first, very simple request is passing now! Maybe a good point to go into the weekend. :sunglasses:

simonhamp commented 6 years ago

@jelhan is your version working well with Lumen then?

jelhan commented 6 years ago

Sorry for late update. Haven't had the time to push that one further. Faced more and more lumen limitations in my application I want to use laravel-json-api and decided therefore to migrate to Laravel.

I have pushed my latest attempts to https://github.com/jelhan/laravel-json-api/tree/lumen and https://github.com/jelhan/demo-lumen-json-api. Currently only fetching a list of posts is working and it relies on some dirty work-a-rounds.

There are two main issues I have faced and have not treated so far. There may be more which does not came to table yet since I did not tried creating or updating a model at all.

  1. nicik/FastRoute does not support setting (default) Route parameters. Actually it does not provide a route object at all. laravel-json-api uses route parameters for model binding (CloudCreativity\LaravelJsonApi\Http\Middleware\SubstituteBindings).

  2. Lumen has not followed testing changes introduced by Laravel 5.4 yet. Test helpers does not work therefore.

lindyhopchris commented 6 years ago

@jelhan thanks for the update.

For (1) I'm planning some improvements to the middleware and route parameter stuff. At the moment my current thinking is to remove the default parameters and instead push the resource type and relationship names via middleware parameters. That'll solve the issue in Lumen I think and if I make the JSON API registrar only use methods that are on both the Laravel and Lumen routers, the two should be cross compatible.

I don't know how feasible it is considering that I don't use Lumen but hopefully it'll help.

simonhamp commented 6 years ago

@lindyhopchris I'm going to be working with @jelhan's repo for now and will try to work out any kinks and share what I find here.

Hope it can all make it back into this base package as it would be nice to have something that works well in both Laravel and Lumen.

Will keep you posted.

kevyworks commented 6 years ago

Status update?

lindyhopchris commented 6 years ago

@kevmt still want to add this but haven't had time to do it yet... it's not helped by us not having any Lumen projects at the moment.

Can look at it once we get to 1.0.0 (which we're not currently far off from)

kevyworks commented 6 years ago

@lindyhopchris Thanks man.. really looking forward.

Vendin commented 5 years ago

@lindyhopchris when the release of this functionality?

lindyhopchris commented 5 years ago

@Vendin still want to put this in but unfortunately I don't have any projects at the moment that are Lumen - I'm only doing Laravel work at the mo. So that means I still haven't had time to look at Lumen support.

Still intending to put it in though, after the 1.0.0 release (which should be coming by the end of September).

kevyworks commented 5 years ago

@lindyhopchris Hi man, I hope this will help a bit, https://gist.github.com/kevyworks/9ef3e50d01584ecf6e682266c51971f2 There are lots to do "maybe" but for now it simply works, with laravel's standard Resource class.

j3rrey commented 5 years ago

I'm actually looking for a good lumen package for a project at work and will try this one (https://github.com/swisnl/json-api-client) at first, if I find some time I'll take a look at your repo compare it to the other one and eventually do a merge request

lindyhopchris commented 5 years ago

@j3rrey yeah, that one is for retrieving resources from a JSON API server... looks good, will have to give it a go at some point.

This package is the server-side, not the client side. I.e. it processes inbound JSON API requests. So the two packages might be quite different, though it would be nice to see if there's any places we could converge.

My plan with 2.0 of this package is to convert things into a style that matches Laravel's Eloquent resources - ideally extending the top-level JsonResource class, but will have to see if that's possible. If it is, it should make using this package in Lumen a lot more possible.

lindyhopchris commented 4 years ago

The design for 2.0 in theory means that using this in a Lumen app will be more than possible.

See #411