New resource-based API

[tobyzerner]

Simpler, 2-3x faster, and doesn't violate the LSP like the old serializer-based API (#115).

Still to come:

• [x] Rewrite tests
• [x] Redo error handling
• [x] Finalize meta/links handling methods
• [ ] Clean up code/tests
• [x] Finalize README
• [ ] Update CHANGELOG

[tobyzerner]

That's what I like to see 😍

[tobyzerner]

Example usage:

class PostResource extends AbstractResource
{
    protected $type = 'posts';

    protected $post;

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

    public function getId()
    {
        return $this->post->id;
    }

    public function getAttributes(array $fields = null)
    {
        return ['body' => $this->post->body];
    }

    public function author()
    {
        return new Relationship(new UserResource($this->post->author));
    }
}

class UserResource extends AbstractResource
{
    protected $type = 'users';

    protected $user;

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

    public function getId()
    {
        return $this->user->id;
    }

    public function getAttributes(array $fields = null)
    {
        return ['name' => $this->user->name];
    }
}

for ($i = 1; $i <= 100; $i++) {
    $users[] = (object) ['id' => $i, 'name' => 'Toby'];
}

for ($i = 1; $i <= 50; $i++) {
    $posts[] = (object) ['id' => $i, 'body' => 'hello', 'author' => $users[array_rand($users)]];
}

$resources = array_map(function ($post) {
    return new PostResource($post);
}, $posts);

$document = new Document($resources);
$document->setInclude(['author']);

echo $document;

[franzliedke]

Good stuff. :)

[crhayes]

I just started using this library and wanted to drop an opinion here 😄

I like this change, although I like the name Serializer more than Resource. I think it better describes what the class is doing (i.e. serializing the resource into another format). The data object that is being serialized is the resource.

For instance, api/users/me queries for a User resource, and then serializes it using the UserSerializer.

I'm guessing maybe this change is to be more in line with the wording in the JSON API spec?

[franzliedke]

What about Representation? That would fit the REST concept very well.

[crhayes]

@franzliedke I like Resource better than Representation. TBH I've come around to Resource. I also think this bit of code makes a lot of sense:

return new Relationship(new UserResource($this->post->author));

Since you have a relationship with another resource. It reads well!

@tobscure Anything I can do to help move this along? I'm excited to start using this when it's ready :)

[tobyzerner]

Yeah I'm quite happy with Resource, it makes semantic sense as @crhayes pointed out. The class isn't just serialising or representing a JSON-API resource; it is a JSON-API resource.

@crhayes Feel free to start using it now, and let me know if you uncover any issues. I don't expect any further API changes. I just want to rewrite the test suite from scratch before releasing.

[tobyzerner]

@f3ath @franzliedke @crhayes I've been working on a rewrite of the README for the new API. I've also documented a couple other big changes, which I haven't yet implemented:

• New error handling API: The old one violated the LSP. The new one is simpler and clearer. Also introduce Error objects.

• New links API: Remove addLink method and change to setSelfLink, setRelatedLink, and setPaginationLinks, since these are the only links that are allowed in link objects in the spec. Also introduce optional Link objects in accordance with the spec.

Would you please mind reading over the new README and letting me know what you think?

[crhayes]

@tobscure I haven't had any time to code over the last 4 days or so, but I'll try and spend some time tonight refactoring my code to use this new approach. That way I can reference the documentation and let you know if I have any issues :)

[f3ath]

@tobscure took a quick look, not a thorough analysis. One random thought about naming. I find the combination of setFoo and addFoo a bit confusing. Adding implies that the element did not exist before we added it, it is a non-idempotent operation if you will. But the code does not check if is already exists, so calling "add" with the same argument multiple times does not change the state, acting as an idempotent operation. How about using setLink($name, $url) to set an individual item of a set and replaceLinks(array $links) to replace the entire set? I'm not a native English speaker though, so might be getting it wrong.

[tobyzerner]

@f3ath Good point. How about this API:

trait LinksTrait {
    public function getLink($key);
    public function setLink($key, $link);
    public function removeLink($key);

    public function getLinks();
    public function replaceLinks(array $links);
}

trait SelfLinkTrait {
    public function getSelfLink();
    public function setSelfLink($link);
}

trait RelatedLinkTrait {
    public function getRelatedLink();
    public function setRelatedLink($link);
}

trait PaginationLinksTrait {
    public function setPaginationLinks($url, array $queryParams, $offset, $limit, $total = null);
}

trait MetaTrait {
    // "meta" is the plural here, so we use a suffix for the singular version
    public function getMetaItem($key);
    public function setMetaItem($key, $value);
    public function removeMetaItem($key);

    public function getMeta();
    public function replaceMeta(array $meta);
}

[tobyzerner]

More changes:

• Static Document/Relationship constructors inspired by @f3ath
• Rename meta/link methods (still a bit unsure about these...)
• Remove error handling stuff - I think it's just slightly beyond the scope of this library, it's better implemented in the user space
• Refactor internal Document logic (more concise code)
• Remove toArray methods, just use jsonSerialize

https://github.com/tobscure/json-api/blob/resource-interface/README.md

[f3ath]

I like the recent change. It narrows the focus and moves us towards single responsibility.

[tobyzerner]

Further changes:

• Add internal ResourceIdentifier and ResourceObject classes so that all serialization is done via JsonSerializable. This change means that theoretically @f3ath's new json-api library could be pulled in and used to construct the actual JSON. (Just a theoretical though as we need this library to support PHP 5.x)

• Change AbstractResource relationship method name format to getXxxRelationship (rather than just xxx)

• Further internal refactor of Document and resource merging process

• Rename links/meta methods again: setLinks, setLink, removeLink setMeta, setMetaItem, removeMetaItem Respecting the Symfony coding conventions (they make sense to me)

• Remove getters where they are not essential, make properties private

• Finalize README

[franzliedke]

From looking at the README, this looks like good stuff. :+1:

[tobyzerner]

@franzliedke @f3ath Thoughts on renaming the ResourceInterface methods as follows:

getId → getResourceId getType → getResourceType getAttributes → getResourceAttributes getLinks → getResourceLinks getMeta → getResourceMeta getRelationship → getResourceRelationship

This would allow the interface to be implemented directly on eg, an Eloquent model.

[franzliedke]

I don't like the names (so superfluous since the Resource is already in the class name), but I understand the idea. Which method name clashes with an Eloquent method?

[tobyzerner]

getAttributes at least, possibly getRelationship? Or does Eloquent use getRelation...

[f3ath]

I agree with @franzliedke. I don't see any reason to let Eloquent, or any other potential use case where naming is dictated by 3rd party APIs, damage the design of the library's API.

Cases like class Foo extends ActiveRecord implements ResourceInterface, SomeOtherInterface would smell like breaking SRP.

[tobyzerner]

Fair call. It's probably bad to encourage coupling of the Resource to the Model anyway.

My only question then is: do we really need ResourceInterface, or can we just get away with AbstractResource as the base?

[f3ath]

I'd say we do need an interface as a mean of expressing the library's public API.

[tobyzerner]

But what does the interface offer that the abstract class does not? Sorry, I'm still a bit naive on the philosophical and practical differences between interfaces/abstracts :/

[franzliedke]

Most likely that it can be implemented by classes that already extend from other classes, I suppose?

[f3ath]

Abstract class forces the object to belong to a certain hierarchy. If your domain object is already a part of another hierarchy (e.g. class DiscountShoppingCart extends ShoppingCart) you won't be able to extend from the abstract class in the same time. But it is possible for a ShoppingCart to implement ResourceInterface.

[f3ath]

It also forces your clients to prefer inheritance over composition, you don't want to do that. https://www.thoughtworks.com/insights/blog/composition-vs-inheritance-how-choose

[tobyzerner]

Right, understood. But then that leads me back to the method naming... I feel like the methods as they are will be pretty likely to conflict if implemented into an existing hierarchy. Like in your shopping cart example - a shopping cart could easily have "attributes" and "meta" which are not the same as JSON-API attributes/meta.

I agree we shouldn't rename them to avoid conflict with any specific library, but what about to avoid conflict in general?

[f3ath]

You can't solve every potential corner case. I assume the best strategy is to stay focused on ResourceInterface as a part of JSON API domain.

[tobyzerner]

By the logic you've argued, should we also be using a RelationshipInterface?

[f3ath]

The original problem was LSP and it seems getting solved as far as I can tell. How to implement the architecture properly in general - I don't know, that's what I'm trying to answer by building my library. It's tough ;)

[tobyzerner]

I think technically, yes, it should be an interface. Because there are two separate concerns:

1. Representing the relationship information in the context of a tree of resources (this is the job of an interface - we just need to be able to getData, getMeta, and getLinks, we don't care how it's constructed)

2. Serializing into a JSON-API relationship object (we just need to be able to setData, setMeta, and setLinks, and then run jsonSerialize) - these are the kinds of objects in your library

You could also argue the same for Error and Link objects.

But pragmatically, it's much easier to just combine these responsibilities into the same class, so we can skip the step of mapping a (1) object to a (2) object during the serialization process. Even though it's technically not quite correct, I think I'm happy to leave it as is - otherwise I will end up rewriting your library in PHP 5, which I'm too lazy to do right now :P