InactiveProjects / limoncello-collins

Quick start JSON API application (Laravel based)
http://jsonapi.org
71 stars 10 forks source link

Response with encoded relationships #17

Closed philbates35 closed 8 years ago

philbates35 commented 8 years ago

Hello again (sorry for creating another issue!),

When fetching a resource relationship, the spec says:

The primary data in the response document MUST match the appropriate value for resource linkage, as described above for relationship objects.

For example, a GET request to a URL from a to-one relationship link could return:

HTTP/1.1 200 OK
Content-Type: application/vnd.api+json

{
  "links": {
    "self": "/articles/1/relationships/author",
    "related": "/articles/1/author"
  },
  "data": {
    "type": "people",
    "id": "12"
  }
}

In other words, only the type and id should be returned in data, no attributes etc, but it seems there's no way to be able to do this currently using your package. Here's what I'm doing currently:

In routes.php:

Route::get('api/articles/{id}/relationship/author', [
    'uses' => 'AuthorController@showRelationshipAuthor'
]);

In AuthorController:

/**
  * Display the children of the node with the specified ID as primary data.
  *
  * @param int $id
  * @return \Illuminate\Http\Response
  */
 public function showRelationshipAuthor($id)
 {
     $this->checkParametersEmpty();
     $article = Article::findOrFail($id);

     return $this->getResponse($article->author);
 }

Which gives a response that looks like:

{
  "data": {
    "type": "authors",
    "id": "2",
    "attributes": {
        "name": "Joe Bloggs"
    },
    "links": {
      "self": "http://app.dev/api/authors/2"
    }
  }
}

However in order to comply with the specification I want it to look like this:

{
  "data": {
    "type": "authors",
    "id": "2"
    "links": {
      "self": "http://app.dev/articles/2/relationship/author",
      "related": "http://app.dev/articles/2/author"
    }
  }
}

I looked through some of the source to see if I could work out any way I could accomplish this, but I couldn't figure it out. Do you have any ideas? Some sort of getRelationshipResponse() (or similar) method added to ApiTrait would make this easy.

neomerx commented 8 years ago

JSON-API package supports such encoding and here is an example

limoncello doesn't have a wrapper for this method however it should take you just a few lines of code to add a method similar to getResponse (getIdentitiesResponse could be a good name choice)

philbates35 commented 8 years ago

Thanks the reply, I'll try and get it working tomorrow. :)

philbates35 commented 8 years ago

Sorry, more questions - I was able to the data in the response to be correct by using Encoder::encodeIdentifiers() in a custom helper method as you suggested, so that's great. There's still two issues I'm having:

Can't include links automatically

I can't work out how to get the relationship self (e.g. /articles/1/relationships/author) and related (e.g. /articles/1/author) links to show in the top level links in the document automatically, in the same way that they are automatically shown when including a relationship using ?include in the URL. I could manually calculate them in each controller and pass them through as a $links parameter to the response helper, but given that the package is capable of working them out automatically when being included via the include GET parameter, I don't really want to do that.

I think a helper method such as getRelationshipResponse($primaryData, $relationship, $showSelf = false, $showRelated = false, $meta = null) that could be called from any controller would be the way to solve this. For example, the route /articles/1/relationships/author would trigger a method on my ArticleController that would look something like:

public function showRelationshipAuthor($id)
{
    $this->checkParameters();
    $article = Article::findOrFail($id);
    return $this->getRelationshipResponse($article, 'author', true, true);
}

I tried to create this helper method myself in my base JsonApiController, but I just couldn't work out what I needed to do in order for it to use the schema of the provided primary resource in the first argument, load the relationship provided in the second argument and include data from that relationship, and finally include the self and related links automatically if specified in the method arguments. Maybe this isn't possible in the current way your packages are structured, but I'd love to be proven wrong. Any advice on this would be greatly appreciated.

Can't include additional data in the URL

The spec says that I should be able to include additional relationships when viewing a relationship, the example they give is GET /articles/1/relationships/comments?include=comments.author. However, using my current helper method (below), additional includes aren't being includedat all in the response.

Here's my custom helper method in JsonApiController:

protected function getResourceLinkageResponse(
    $data,
    $statusCode = Response::HTTP_OK,
    $links = null,
    $meta = null
) {
    $parameters = $this->getParameters();
    $encoder = $this->codecMatcher->getEncoder();
    $outputMediaType = $this->codecMatcher->getEncoderRegisteredMatchedType();
    $links === null ?: $encoder->withLinks($links);
    $meta === null ?: $encoder->withMeta($meta);

    /** @var ResponsesInterface $responses */
    $responses = $this->getIntegration()->getFromContainer(ResponsesInterface::class);
    $data = $data instanceof Collection ? $data->all() : $data;
    $content = $encoder->encodeIdentifiers($data, $parameters);
    return $responses->getResponse($statusCode, $outputMediaType, $content, $this->supportedExtensions);
}

And my controller method linked to /articles/{id}/relationships/author:

protected $allowedIncludePaths = ['author'];

public function showRelationshipAuthor($id)
{
    $this->checkParameters();

    $article = Article::findOrFail($id);

    return $this->getResourceLinkageResponse($article->author);
}

But when I go to /articles/{id}/relationships/author?include=author, I just get the following response body:

{
  "data": {
    "type": "authors",
    "id": "1"
  }
}

Any thoughts, ideas, or suggestions? Thanks again for your help, and your excellent packages.

neomerx commented 8 years ago

I'm not sure I've got you but as for automatic links you can describe in Schema what links should be included https://github.com/neomerx/json-api/wiki/Schemas#show-links-related-to-resource

neomerx commented 8 years ago

As for the second question you have to understand basics how all this works. There is a JSON-API encoder. It doesn't know about Laravel, Controllers, HTTP and etc but if you give it something it can convert it to JSON-API formatted text (there are some obvious limitations and conditions to be met). Limoncello-collins gives some nice wrappers for converting the something to HTTP responses plus cool exception handling. Now back to your question. When you do the following

protected $allowedIncludePaths = ['author'];

public function showRelationshipAuthor($id)
{
    ...
    return $this->getResourceLinkageResponse($article->author);
}

You actually say 'Hey, convert author and include his relation to author'. As the wrapper internally looks for $allowedIncludePaths and if 'author' was supplied in HTTP include query parameter it overrides include paths from schema of given resource (Author's schema in your case).

What you actually want to do is just convert $article->author. Do you want to include author's relations such as articles? Well if so you have to pass them in query and allow in $allowedIncludePaths.

So why you get the resource without attributes and relationships? Most likely you convert it to identities only (as you asked earlier).

philbates35 commented 8 years ago

Okay, let me try again to explain the links thing. :)

If I load an article resource and include it's author

GET /articles/{1}?include=author

then I get this response:

{
  "data": {
    "type": "articles",
    "id": "1",
    "attributes": {
      "name": "My Article"
    },
    "relationships": {
      "author": {
        "data": {
          "type": "authors",
          "id": "2"
        },
        "links": {
          "self": "http://app.dev/api/articles/2/relationships/author",
          "related": "http://app.dev/api/articles/2/author"
        }
      }
    },
    "links": {
      "self": "http://app.dev/api/authors/1"
    }
  }
}

When I load the author relationship of the article directly:

GET /articles/{1}/relationships/author

then the spec states that the response should be the following (and importantly, notice that the entire response, including links, is identical to the data.relationships.author section of the response above where I included the relationship using ?include=author):

{
  "data": {
    "type": "authors",
    "id": "2"
  },
  "links": {
    "self": "http://app.dev/api/articles/2/relationships/author",
    "related": "http://app.dev/api/articles/2/author"
  }
}

In my first example (GET /articles/{1}?include=author), data.relationships.author.links are calculated by your package correctly without me having to do anything (aside from adding static::SHOW_SELF => true to the corresponding relationship in the ArticleSchema). Which is perfect.

The issue I have is being able to return the response in the second example (GET /articles/{1}/relationships/author). In my controller ideally I want to be able to say, here's an article - now return the response for the author relationship that I defined in my ArticleSchema, and make it identical to what would be displayed in data.relationships.author had I included the relationship from a GET /articles/{1}?include=author request.

Right now in my controller method for GET /articles/{1}/relationships/author, I can load the article, and of course I can get the author. But then I hit a wall:

In summary, your package creates the perfect response body for the author relationship behind the scenes for GET /articles/{1}?include=author, but I can't tell it to create that exact same response manually from my controller.

Does that make any more sense?

neomerx commented 8 years ago

Have a look at withRelationshipSelfLink and withRelationshipRelatedLink methods. I guess that's what you're looking for.

philbates35 commented 8 years ago

Thanks for getting back to me, that was helpful. I've been able to put a helper method together that allows me to return a relationship, which is good enough for now. I'll post my implementation for you to look at, then I'll post my concerns.

Here's how everything looks currently:

// App\Schemas\ArticleSchema

use Neomerx\JsonApi\Schema\SchemaProvider;

class ArticleSchema extends SchemaProvider
{
    /**
     * @var string
     */
    protected $resourceType = 'articles';

    /**
     * @var string Must end with '/'
     */
    protected $selfSubUrl = '/api/articles/';

    /**
     * Get resource identity.
     *
     * @param object $resource
     *
     * @return string
     */
    public function getId($resource)
    {
        return $resource->getId();
    }

    /**
     * Get resource attributes.
     *
     * @param object $resource
     *
     * @return array
     */
    public function getAttributes($resource)
    {
        return [
            'name' => $resource->getName()->toIso8601String(),
            'created_at' => $resource->getCreatedAt()->toIso8601String(),
            'updated_at' => $resource->getUpdatedAt()->toIso8601String()
        ];
    }

    /**
     * Get resource links.
     *
     * @param object $resource
     * @param array  $includeRelationships A list of relationships that will be included as full resources.
     *
     * @return array
     */
    public function getRelationships($resource, array $includeRelationships = [])
    {
        $relationships = [];

        if (isset($includeRelationships['author']) === true) {
            $relationships['author'] = [
                static::DATA => $resource->getAuthor(),
                static::SHOW_SELF => true,
                static::SHOW_RELATED => true
            ];
        }

        return $relationships;
    }
}
// App\Http\JsonApiController relationship helper method

protected function getRelationshipResponse(
    $primaryResource,
    $relatedResource,
    $relationshipName,
    $meta = null
) {
    $relatedResource = $relatedResource instanceof Collection ? $data->all() : $data;

    $integration = $this->getIntegration();
    $encoder = $this->codecMatcher->getEncoder();
    $outputMediaType = $this->codecMatcher->getEncoderRegisteredMatchedType();

    $meta  === null ?: $encoder->withMeta($meta);

    $content = $encoder->withRelationshipSelfLink($primaryResource, $relationshipName)
        ->withRelationshipRelatedLink($primaryResource, $relationshipName)
        ->encodeIdentifiers($relatedResource);

    $responses = $integration->getFromContainer(ResponsesInterface::class);

    return $responses->getResponse(Response::HTTP_OK, $outputMediaType, $content, $this->supportedExtensions);
}
// App\Http\Controllers\ArticleController calling my custom helper method

/**
  * Assigned to route /api/articles/{id}/relationships/author
  */
public function showRelationshipAuthor($id)
{
    $this->checkParameters();

    $article = Article::findOrFail($id);

    return $this->getRelationshipResponse($article, $article->getAuthor(), 'author');
}

Although this works, I don't think this is the ideal solution because the author relationship logic (i.e. $article->getAuthor(), and whether or not to show self and related relationship links has to be done twice - I define it once in the ArticleSchema which is used when the relationship is included using the URI include GET parameter, and once in ArticleController::showRelationshipAuthor(). If the way in which you retrieve an article's author changes, I'd like to only have to change it in the shema - but currently, I'd also have to potentially change individual controller methods. This issue isn't specific to the resource/{id}/relationships/relationship URIs either, it applies also to resource/{id}/relationship URIs - I have to duplicate my relationship logic in the schemas and in the controller methods. Although I appreciate this isn't a huge issue, I just think that it doesn't feel right that I define the relationship in one place (the schema), but then can't use that relationship definition elsewhere in the application.

In other words, it would be nice if my schema was the one source of truth i.e. the single place where all JSON API resources and the resource's relationships are managed, and the controllers don't have to know anything about how the relationship is calculated, and whether or not self and related links should be displayed.

More generally, as I mentioned in a previous message - the correct response for any relationship is generated by your package correctly (using the schema) whenever it is included using ?include=relationship in the URI. If there was a way to hook into the mechanism that calculates the JSON API response body for the included relationship, and call it manually from my controller (something like $this->getRelationshipResponse($article, 'author'), then that would be perfect, but unfortunately I can't work out how to do that.

Thanks again for your help.

neomerx commented 8 years ago

Hint: you'd better to set $selfSubUrl as '/articles/' and add prefix in config file as

C::JSON => [
    C::JSON_OPTIONS    => JSON_PRETTY_PRINT,
    C::JSON_DEPTH      => C::JSON_DEPTH_DEFAULT,
    C::JSON_URL_PREFIX => \Request::getSchemeAndHttpHost() . '/api',
],

also I can see an error in getRelationships it should have else with a model with id and any attributes (they won't be used).

philbates35 commented 8 years ago

Okay, I'll update the $selfSubUrl, thanks.

I'm afraid I don't understand the bug you refer to though. Are you saying that if $includeRelationships doesn't contain author then instead of returning an empty array I should return a new Author model class? If so, can you explain why that is the case? Everything seems to be loading correctly with no errors currently when I don't include author in the URL and I'm just returning an empty array as above.

neomerx commented 8 years ago
    public function getRelationships($resource, array $includeRelationships = [])
    {
        $relationships = [];

        if (isset($includeRelationships['author']) === true) {
            $relationships['author'] = [
                static::DATA => $resource->getAuthor(),
                static::SHOW_SELF => true,
                static::SHOW_RELATED => true
            ];
        } else {
            // author with filled in Id and any attributes should be here
            $relationships['author'] = ...
        }

        return $relationships;
    }
philbates35 commented 8 years ago

Thanks, I understand now :)

I was just having a play around with how I can reference my relationship definitions in my schema, and reference them elsewhere in my application without duplicating any logic, and I've come up with something I'm fairly happy with, although I'm sure there's room for improvement. Can you have a look at the following and see what you think? I think there could be value in having something like the following out of the json-api/limoncello box.

class ArticleSchema extends SchemaProvider
{
    /**
     * A new property where you define the names of the available relationships for
     * this schema.
     * This should be added to the parent SchemaProvider class with a default value
     * of an empty array and overriden in each schema.
     *
     * @var array
     */
    protected $relationships = ['author', 'tags'];

    /**
     * This is the method that the user currently overrides. I propose an implementation
     * similar to this that should be added to the parent SchemaProvider class that
     * loops through the relationships defined in the $relationships property above.
     * There would be no need for a user to override this method in their schema any more.
     *
     * @param object $resource
     * @param array  $includeRelationships A list of relationships that will be included as
     *     full resources.
     *
     * @return array
     */
    public function getRelationships($resource, array $includeRelationships = [])
    {
        $relationships = [];
        foreach ($this->relationships as $relationship) {
            $relationshipDetails = $this->getRelationshipDetails(
                $resource,
                $relationship,
                isset($includeRelationships[$relationship])
            );

           $relationships[$relationship] = $relationshipDetails;
        }

        return $relationships;
    }

    /**
     * A new method that the user would have to override in each schema that returns
     * the relationship details from a provided relationship name, that should correspond
     * to a relationship defined in the $relationships property above.
     *
     * I've implemented this using a switch statement which isn't exactly elegant - maybe
     * internally this method could delegate the retrieval of relationship details to a
     * dynamic method name instead.
     * E.g. calling getRelationshipDetails($resource, 'author', $includeRelationship)
     * would result in getAuthorRelationship($resource, $includeRelationship) being
     * called on the schema, and it would be those methods that the user would define in
     * their custom schemas.
     *
     * @param object $resource
     * @param string $relationship
     * @param bool $includeRelationship Whether the relationship should be included
     *    as a full resource.
     * @return array
     * @throws \InvalidArgumentException
     */
    public function getRelationshipDetails(
        $resource,
        $relationship,
        $includeRelationship = true
    ) {
        switch ($relationship) {
            case 'author':
                if ($includeRelationship) {
                    $author = resource->author;
                } else {
                     $author = new Author();
                     $author->id = $resource->author_id;
                }
                $relationshipDetails = [
                    static::DATA => $author,
                    static::SHOW_SELF => true,
                    static::SHOW_RELATED => false
                ];
                break;

            case 'tags':
                $relationshipDetails = [
                    static::DATA => $resource->tags,
                    static::SHOW_SELF => false,
                    static::SHOW_RELATED => true
                ];
                break;

            default:
                throw new InvalidArgumentException("Unknown relationship [{$relationship}] requested");
        }

        return $relationshipDetails;
    }
}

By implementing the above, in my JsonApiContoller (but it's more probably suited to ApiTrait if this idea was to be integrated into your limoncello package) I'm able to implement a helper method that returns a relationship response just by providing a relationship name (but importantly, not the logic that determines the relationship, or whether self of related links should be displayed as it's all pulled from the schema):

abstract class JsonApiController extends Controller
{
    /**
     * Return a response containing a relationship of a resource.
     *
     * @param object|array $resource
     * @param object|array $relationship
     * @param string $relationshipName
     * @param mixed $meta
     * @return \Symfony\Component\HttpFoundation\Response
     */
    protected function getRelationshipResponse(
        $resource,
        $relationship,
        $meta = null
    ) {
        $integration = $this->getIntegration();
        $encoder = $this->codecMatcher->getEncoder();
        $outputMediaType = $this->codecMatcher->getEncoderRegisteredMatchedType();

        $meta  === null ?: $encoder->withMeta($meta);

        $schemaContainer = $integration->getFromContainer(ContainerInterface::class);
        $schema = $schemaContainer->getSchema($resource);

        // Note: This is where we call my newly defined method on the schema
        $relationshipDetails = $schema->getRelationshipDetails($resource, $relationship);

        if (array_key_exists(SchemaProvider::SHOW_SELF, $relationshipDetails) &&
            true === $relationshipDetails[SchemaProvider::SHOW_SELF]
        ) {
            $encoder->withRelationshipSelfLink($resource, $relationship);
        }

        if (array_key_exists(SchemaProvider::SHOW_RELATED, $relationshipDetails) &&
            true === $relationshipDetails[SchemaProvider::SHOW_RELATED]
        ) {
            $encoder->withRelationshipRelatedLink($resource, $relationship);
        }

        if (isset($relationshipDetails[SchemaProvider::DATA])) {
            $relationshipData = $relationshipDetails[SchemaProvider::DATA];
        } else {
            $relationshipData = null;
        }
        $content = $encoder->encodeIdentifiers($relationshipData);

        $responses = $integration->getFromContainer(ResponsesInterface::class);

        return $responses->getResponse(
            Response::HTTP_OK,
            $outputMediaType,
            $content,
            $this->supportedExtensions
        );
    }
}

Once all that is implemented, returning any relationship of a resource is incredibly simple, with no duplication of logic:

class ArticleController extends JsonApiController
{
    /**
     * Display the article's author relationship.
     *
     * Linked to route: /articles/{id}/relationships/author
     * 
     * @param int $id
     * @return \Illuminate\Http\Response
     */
    public function showRelationshipAuthor($id)
    {
        $this->checkParameters();

        $article = Article::findOrFail($id);

        return $this->getRelationshipResponse($article, 'author');
    }
}

What are your thoughts on this? Do you think this is something that could be worth implementing properly in your packages?

neomerx commented 8 years ago

I think if more people need encoding relationships it could be added to the package.

Daveawb commented 8 years ago

I would definitely +1 this. The tighter the implementation is the the spec the better imo and wish I'd seen this post before implementing this functionality myself.

mallardduck commented 8 years ago

Just wanted to give this my :+1: as well! Having the package encode relationships would be excellent.

neomerx commented 8 years ago

New version can return relationships as

The demo app includes implementation for paginated relationships (by default no more than 10 per relationship if more + pagination links). Handlers for requesting paginated relationships are also added.