Laravel-json-api: Laravel 5.3 support

[egeriis]

It would be nice to get an update with support for Laravel 5.3. Looks like the service provider needs to have it's dependency injection removed from the method signature.

https://laravel.com/docs/5.3/upgrade

We're currently using v0.2, so if an update could be issued to this version, that would be great.

[lindyhopchris]

@egeriis only just looking at the 5.3 upgrade notes now. Does it definitely not work with the method signature as is? I wasn't sure whether the upgrade guide saying "you may remove the arguments..." meant it was optional?

If that's the only breaking thing then I'm happy to release a new version of v0.2 as it's a fairly straightforward change. I probably won't be able to test it though because all the apps I have are currently on the v0.4 (latest) version. If I make the change would you be able to try it out for me? I probably wouldn't tag it though until Laravel 5.3 is officially released - though I believe that's going to be very soon.

[lindyhopchris]

ah, just seen that 5.3 was released yesterday :smile:

[lindyhopchris]

@egeriis the 5.3 docs are still saying that dependency injection can be used for the boot method:
https://laravel.com/docs/5.3/providers#the-boot-method

So not sure this change is required?

[egeriis]

There must be something else broken then. I did a bunch of debugging yesterday, my conclusion so far is that something is breaking the json-api middleware. I don't know if it works for v0.4 though, as we're in the middle of upgrading our API to use this version.

Let me get back when that upgrade is done—should be by the end of this week. Then I'll attempt a L5.3 upgrade from this branch.

[lindyhopchris]

I can upgrade one of the apps that I've got v0.4 in sometime this week as well, so hopefully between the two of us we can work out if any changes are required!

[egeriis]

Sounds great! I'll see if I can get some work done on this one of the coming days.

[lindyhopchris]

@egeriis yeah, the package is not working in 5.3, but not to do with the service providers. Basically, a controller's constructor is now being invoked _before_ middleware is run, which means that the request that is injected into the controller is being validated before the json-api middleware is run. This pretty much fundamental breaks the architecture for this package, and lots of people's apps according to this:
https://github.com/laravel/framework/issues/15072

Basically if we want to keep a single request class per resource type, I'm going to have to remove the ValidatesWhenResolved contract from it and trigger the validation at a later point. There's no way in a controller at the moment to trigger something for _every_ single action method on it, though they're planning a patch that will allow closure middleware to be registered in a controller - which would mean I could use that to trigger the validation:
https://github.com/laravel/framework/pull/15080

In the meantime I'm going to tie v0.4 down to Laravel 5.1.*|5.2.* as it only works with those versions at the moment.

[lindyhopchris]

This is now working with Laravel 5.3 - I've implemented on the Laravel JSON API demo app here:
https://github.com/cloudcreativity/demo-laravel-json-api/commit/0fbbdd89790babfa5271ea0f91f6cdf66346e6ff

I still need to test it on a few other apps, so there might still be kinks to iron out!

To use it in the meantime, add the following to your composer.json:

"cloudcreativity/json-api": "0.6.x-dev",
"cloudcreativity/laravel-json-api": "0.5.x-dev"

Then:

composer up cloudcreativity/* neomerx/*

[egeriis]

Sounds great. We are almost done with our refactoring, so we can attempt an upgrade to L5.3 :)

[lindyhopchris]

@egeriis just a quick heads up - I push a change to the dev branches that broke it. It's a minor thing but I won't be able to fix it until tonight (UK time). Just so you don't waste your time trying to get it working! Will post here again when I've push the fix.

[lindyhopchris]

Ok, the demo app is working again. Let me know if you have any problems

[nokios]

Trying to follow your demo example but it seems to require some things I don't see in laravel-json-api. For example, AbstractRequestHandler doesn't exist in 0.5.x-dev's latest commit. (Demo has the composer.lock @ 9a7cb63).

[lindyhopchris]

@nokios try switching the demo app to the laravel-5.3 branch - I updated that branch the other day and it was working for me. Let me know if that fixes it - if not then I might not have committed the latest composer.lock

[egeriis]

@lindyhopchris An update from here, we're finalizing our update to 0.4. As we're done, I'll find an off-hours time to check this out :)

[egeriis]

Appears that CloudCreativity\JsonApi\Exceptions\StoreException no longer exists i v0.6, but is still referred in EloquentAdapter in v0.5 :)

[lindyhopchris]

@egeriis good spot - thanks!

Any other problems or all ok so far?

[egeriis]

Everything seems fine. We looked through the changelog, and it appears that everything we needed was mentioned there.

[egeriis]

Looks like there's an error in UPGRADE.md, shouldn't this be "pagination", not "paging"? https://github.com/cloudcreativity/laravel-json-api/blame/develop/UPGRADE.md#L31

[egeriis]

It would also appear that getRequestHandler is not required by _laravel-json-api_ to be provided in controllers. But is it a required method to implement?

[lindyhopchris]

Thanks, have fixed the upgrade guide.

At the moment JsonApiController::getRequestHandler() is abstract, so always has to be implemented, but null is a valid return value. Wasn't clear on what you're proposing - are you proposing it should be non-abstract, with the default implementation returning null and child controllers needing to override it if they want to use a request handler?

[egeriis]

I was just looking at EloquentController, and I didn't see it as an abstract method. Didn't consider that it was extending from another abstract class.

[lindyhopchris]

makes sense. I'm planning at some point to separate EloquentController so that it isn't extended from JsonApiController - as I don't feel there's any benefit to the extension. But will do that in the future.

I'm likely to be tagging this today or Monday, unless there's any last minute objections?! Obviously it's still a development release so there's still plenty of chances to improve things, though I'd like to keep the breaking changes a lot less between each development release in the future (as this one has been bigger than expected due to the Laravel 5.3 changes).

[egeriis]

I am not sure I understand this section: https://github.com/cloudcreativity/laravel-json-api/blob/develop/UPGRADE.md#search-all

My controllers already have Model, Hydrator and SearchInterface as arguments. Where does SearchAll go?

[egeriis]

I am upgrading our big API _right now_, so I'll let you know today if there are any major breaking things for us :)

[lindyhopchris]

Re: search all... it means if you're not using a custom SearchInterface class for a specific model, then you can use the generic SearchAll class instead.

Cool, well I'll probably wait until Monday to tag then as I don't _need_ to do it today. That gives you a chance to raise any problems - your input is really helpful!

[egeriis]

Wrong namespace for RequestHandler: https://github.com/cloudcreativity/laravel-json-api/blob/develop/UPGRADE.md#requests

[lindyhopchris]

Thanks, have fixed that

[egeriis]

Upgrade guide seems to me missing what to do about the removed getResource from JsonApiController?

[egeriis]

More specifically, how do I replace $this->getResource() in a created event callback?

[egeriis]

Hold your horses, I reckon this code I have is due to a previous caveat.

[lindyhopchris]

Good point, at the moment it isn't passed down through the hooks. It is possible to do so because the resource is passed to the commit method which is the one that triggers the hooks.
https://github.com/cloudcreativity/laravel-json-api/blob/develop/src/Http/Controllers/EloquentController.php#L246

I'm thinking the hooks probably do need the resource as their second argument - what do you think?

[lindyhopchris]

@egeriis I'm going to add the passing of the incoming resource down the model hooks, then tag. Sound ok or did you have anything else to raise at this point?

[egeriis]

Don't seem to have any other findings. Makes sense to have the resource passed into the model hooks.

[egeriis]

@lindyhopchris Oh, I have this question :)

I used to do this, in the Request class for my User model:

    public function getResourceId()
    {
        $id = $this
            ->getHttpRequest()
            ->route(ResourceRegistrar::PARAM_RESOURCE_ID);

        if ($id === 'me') {
            return Auth::user()->getKey();
        }

        return $id;
    }

The purpose being that a request to /users/me will return the currently auth'ed user. I can't figure out how to do this with 0.5. Can you assist?

[lindyhopchris]

The resource identifier (type/id) is now checked a lot further up the stack. This is done via the store, which delegates to adapters, of which the default one provided by the package is the Eloquent adapter.

To use a "custom" id for a user, you'll need to attach an adapter for your user model, rather than using the Eloquent adapter to resolve the id. Something along these lines:

<?php

namespace App\JsonApi\User;

use CloudCreativity\JsonApi\Contracts\Store\AdapterInterface;
use CloudCreativity\JsonApi\Contracts\Object\ResourceIdentifierInterface;
use Illuminate\Contracts\Auth\Guard;

class Adapter implements AdapterInterface
{

    private $auth;

    public function __construct(Guard $auth)
    {
        $this->auth = $auth;
    }

    public function recognises($resourceType)
    {
        return 'users' === $resourceType;
    }

    public function exists(ResourceIdentifierInterface $identifier)
    {
        $id = $identifier->getId();

        if ('me' === $id) {
            return $this->auth->check();
        }

        return User::exists('id', $id);
    }

    public function find(ResourceIdentifierInterface $identifier)
    {
        $id = $identifier->getId();

        if ('me' === $id) {
            return $this->auth->user();
        }

        return User::find($id);
    }

}

Remove the user model from your Eloquent adapter config, and then attach your custom adapter in your json-api.php config file:
https://github.com/cloudcreativity/laravel-json-api/blob/develop/config/json-api.php#L130-L144

[lindyhopchris]

Closing - v0.5.0 released

[egeriis]

So rather than pointing 'user' to User::class I would point it to UserAdapter::class? :)

Edit: Never mind, I get it now. It's a new feature :)