tobyzerner / json-api-server

A JSON:API server implementation in PHP.
https://tobyzerner.github.io/json-api-server/
MIT License
63 stars 21 forks source link

Included resources have empty attribute values #71

Closed bertramakers closed 1 year ago

bertramakers commented 1 year ago

Hi!

I implemented my first resource with relationships today, and I'm running into an issue when I try to include them.

Using the example "PostResource" from the documentation and previous questions as an example, I've added a ToOne relationship like so:

public function fields(): array
{
    return [
        // ...
        ToOne::make('author')
            ->type('authors')
            ->required()
            ->writable()
            ->includable()
            ->withLinkage() // Not sure if this is needed to make it includable or not?
    ];
}

I've also created and registered an "AuthorResource" with a type authors, which implements Findable and has a Show endpoint. It has one field for now:

public function fields(): array
    {
        return [
            Str::make('name')
                ->required(),
        ];
    }

This field is included in the object returned by AuthorResource::find(). For example when I fetch it:

{
    "data": {
        "type": "authors",
        "id": "351",
        "links": {
            "self": "/api/v2/authors/351"
        },
        "attributes": {
            "name": "Example name of an author"
        }
    },
    "jsonapi": {
        "version": "1.1"
    }
}

However, when I fetch a Post resource and add include=author, I get the following:

"included": [
    {
        "type": "authors",
        "id": "351",
        "links": {
            "self": "/api/v2/authors/351"
        },
        "attributes": {
            "name": ""
        }
    }
],

Do I need to implement something extra to make the data includable? Do I need to include the data somehow in the find() method of the Post? When I put a breakpoint or var_dump() inside the find() method of Author, the app just ignores it so I'm under the impression that it is not called by the library.

tobyzerner commented 1 year ago

Yeah, the included author model will be retrieved from the post model via PostResource::getValue() - the default implementation will be looking for a full author model on $post->author.

If you need to change that logic, you can use a getter on the ToOne field, or you can override getValue to implement different behaviour for all ToOne fields. (And if you want to implement eager loading, take a look at Deferred Values.)

bertramakers commented 1 year ago

Thanks @tobyzerner , I managed to automatically include the data for the ToOne relationships by overriding getValue() like this in my own Resource base class:

public function getValue(object $model, Field $field, Context $context): mixed
{
    $value = parent::getValue($model, $field, $context);

    if ($field instanceof ToOne && isset($value->type, $value->id)) {
        $resource = $context->resource($value->type);

        if ($resource instanceof Findable) {
            $model = $resource->find((string) $value->id, $context);
            if ($model !== null) {
                return $model;
            }
        }
    }

    return $value;
}

I know it's not optimal because the extra find() calls will result in one or more extra DB queries, but for the time being this is fine for us and easier than implementing a lot of joins in our repositories' queries and including related models in other models.

However, I'm wondering if there is a way to see in getValue() what relationships should be included, so we can skip the find() calls for models that should not be included? I tried using $context->include but it's always null in our case. I could check the include query parameter directly via $context->request, but then I'd need to parse it myself (explode it on comma's etc). For now, that could work though if there is no other way.

tobyzerner commented 1 year ago

Ah this is something missing from the documentation - and I haven't actually tested this myself yet - but the way it's meant to work is:

Let me know how this works in practice. I'm planning to implement the EloquentResource soon which should serve as a good example / reference implementation. Thanks for being an early adopter!

bertramakers commented 1 year ago

Hey Toby,

I have a ToOne relationship that currently looks like this:

ToOne::make('author')
  ->type('authors')
  ->required()
  ->writable()
  ->includable(),

In this case, getValue() is called when I add ?include=author to the GET request, but $context->include is an empty array.

When I add ->withLinkage() on the ToOne relationship, the $context->include is also an empty array in getValue().

So this looks like a minor bug. But I can work around it by parsing the include query parameter myself in getValue() for now.

Thanks for being an early adopter!

Thank you for creating this library! It's very helpful to easily implement JSON:API in our existing application.

tobyzerner commented 1 year ago

I could be misunderstanding but I think this actually sounds like the correct behaviour to me. $context->include is a list of the child relationships that are included underneath the one that is currently being processed – so in your case, when getValue() is called for the author relationship, $context->include === [] because no child relationships are included. If the author relationship wasn't being included itself, then $context->include === null.

bertramakers commented 1 year ago

Thanks for clarifying. I'll need to plan some time to investigate this again in more detail later.

bertramakers commented 1 year ago

Hello @tobyzerner , sorry for the delay in testing this. This works as expected now with your clarification. Thanks!