dingo / api

A RESTful API package for the Laravel and Lumen frameworks.
BSD 3-Clause "New" or "Revised" License
9.32k stars 1.25k forks source link

Including custom data in transformer #1096

Open tr33m4n opened 8 years ago

tr33m4n commented 8 years ago

Hello there,

I have a repository that queries an API for data (rather than using a model) and returns either an object or a collection. I've been trying to include this data in a transformer for another model with limited success. Obviously I cannot us includes as this requires a model relationship, so for the moment I've hackishly grabbed my interface and checking whether a property exists on the model being transformed. My only issue is that the item is not rendering... Here's my "dodgy" code:

public function transform(Domain $domain)
    {
        $output = [
            'id' => (int) $domain->id,
            'client_id' => (int) $domain->client_id,
            'url' => (string) $domain->url
        ];

        if ($id = $domain->dns_id) {
            $zone = app(ZoneRepositoryInterface::class);
            $output['dns'] = $this->item($zone->getById($id), new ZoneTransformer);
        }

        return $output;
    }

$this->item($zone->getById($id), new ZoneTransformer) appears to be the issue, it's calling the item successfully (can see object when debugging) but it's not outputting the ZoneTransformer, just a couple of {}.

Is there a better way of doing this? Any reason the item is not rendering?

Thanks

tembra commented 8 years ago

Which Serializer are you using? JsonApi / DataArray / Array / Your own ?

tr33m4n commented 8 years ago

Currently I'm using a custom one to remove the 'data' node from the output but it extends the ArraySerializer:

class NoDataArraySerialiser extends ArraySerializer
{
    /**
     * Serialise a collection
     *
     * @param  string $resource_key Resource key
     * @param  array  $data         Data
     * @return array
     */
    public function collection($resource_key, array $data)
    {
        return ($resource_key) ? [$resource_key => $data] : $data;
    }

   /**
    * Serialise an item
    *
    * @param  string $resource_key Resource key
    * @param  array  $data         Data
    * @return array
    */
    public function item($resource_key, array $data)
    {
        return ($resource_key) ? [$resource_key => $data] : $data;
    }
}
tembra commented 8 years ago

Ok. Let's go for another big answer hehehe.

Is there a better way of doing this?

First of all you CAN use include to solve your problem and let Fractal do all this stuff for you. This is a better way. Includes DOES NOT requires a model relationship. Remember this is all about Fractal that is framework agnostic and do not know a single bit of your models. Fractal just transforms data.

Look at the example code below:

protected $availableIncludes = [
    'dns'
];

public function transform(Domain $domain)
{
    $output = [
        'id' => (int) $domain->id,
        'client_id' => (int) $domain->client_id,
        'url' => (string) $domain->url
    ];

    return $output;
}

public function includeDns(Domain $domain)
{
    if ($id = $domain->dns_id) {
        $zone = app(ZoneRepositoryInterface::class);
        return $this->item($zone->getById($id), new ZoneTransformer);
    }
}

So when you request http://example.com/xxx?include=dns for example, the includeDns will be triggered and new League\Fractal\Resource\Item will be returned that will be serialized according to ArraySerializer that you extends, because you didn't implemented the includedData method on your own serializer. This will automatically create a dns property on your object that will encapsulate that answer from your ZoneTransformer.

Any reason the item is not rendering?

That said, the real problem in your "hackish" implementation is that you are confusing $this->item method. When you call this method inside a Transformer you are just creating a new League\Fractal\Resource\Item that still needs to be serialized by your Serializer.

So if you have registered a transformer for a class you can change the line that has $output['dns'] to:

$factory = app(\Dingo\Api\Transformer\Factory::class); // or the alias app('api.transformer')
$output['dns'] = $factory->transform($zone->findById($id));

So the factory will resolve the bindings and transform the object for you, like include does. Otherwise you will need to use code below to tell the adapter which transformer needs to be use.

$factory = app(\Dingo\Api\Transformer\Factory::class); // or the alias app('api.transformer')
$adapter = $factory->getAdapter();
$binding = new \Dingo\Api\Transformer\Binding(app(), ZoneTransformer::class);
$request = app(\Dingo\Api\Http\Request::class);
$output['dns'] = $adapter->transform($zone->findById($id), new ZoneTransformer, $binding, $request);

I hope I was clear and not confusing you even more. Also I hope to have helped.

Regards, Thieres Tembra

tr33m4n commented 8 years ago

Hi Thieres,

Thanks for your detailed answer, it really helped. In my case using $availableIncludes does not work as I get Call to undefined method Illuminate\\Database\\Query\\Builder::dns(), which leads me to believe that somewhere Dingo is making the assumption that the include will be a relationship of the model... Which is handy in most cases, except this one! I have gone with your second option and registered a transformer for the class which is working well!

Thanks again

tembra commented 8 years ago

Hello again,

In which file you are getting this error? Did you create includeDns method in Transformer? This is not supposed to happen. I tested here and all is good.

tr33m4n commented 8 years ago

Hi again,

Yes I created the includeDns method, I set it up exactly like your code example. That error occurs when trying to access the data by adding the ?include=dns param to the url. It's the same error if I use $defaultIncludes as well. Interestingly if I "fake" a relationship on the model, for example adding (note the model defined in the belongsTo function does exist, however is not the correct relationship):

public function dns()
{
    return $this->belongsTo(Client::class);
}

And then querying the DNS include it works. Seems like the function needs to exist in the model, but does not require anything beyond that (I added a belongsTo statement to the function as the model was throwing errors without something there)... Any ideas? What version of Dingo/Fractal are you running?

Cheers,

tembra commented 8 years ago

Hello,

Sincerely I have no idea. I'm using on my composer.json this statement "dingo/api": "1.0.x@dev".

In my composer.lock I got:

{
    "name": "dingo/api",
    "version": "dev-master",
    ...
    "require": {
        ...
        "league/fractal": ">=0.12.0",
        ...
}
...
{
    "name": "league/fractal",
    "version": "0.13.0",
    ...
}

Maybe you have something in your code that is checking for the model relationship. Can you post the stack trace of the error?

hskrasek commented 8 years ago

This is due to the package trying to eager load relationships when includes are sent into the API. Great for performance, but makes a big assumption on how your project is setup.

Not necessarily a bug, but if you call the disableEagerLoading() method on the Fractal adapter, this issue goes away without mocking a relationship. Will update the docs to make this more clear, and look into adding an easier way to disable the eagerLoading either globally, or maybe even by transformer.

andrewmclagan commented 7 years ago

@hskrasek Would be a better approach to attempt an eager load then fallback to a normal fractal include if that fails?

tr33m4n commented 7 years ago

@andrewmclagan Personally I think that'd add unneeded overhead, better to be able to turn it off rather than have it try one, fail, try another

andrewmclagan commented 7 years ago

Yeah just saw it's able to be switched off

edbentinck commented 7 years ago

@tr33m4n I think @andrewmclagan brings up a valid point. For instance, I have many use cases where I'd like to be able to include both eager-loaded relationships and also custom includes. Logically it doesn't make sense to me that you can have one but not the other. The ability to add custom includes is essentially governed by an eager loading setting, which is strange.

An alternative approach could be to be more explicit about these includes. So, instead of $availableIncludes, you could use something like: $relationshipIncludes and $customIncludes. This way you could have custom includes without affecting the eager loading of your relationships.

tr33m4n commented 7 years ago

@edbentinck Yup he does, but my point was if you're only using one type, whatever functionality is put in place to deal with both, it shouldn't query both if only one is needed