relationships aren't returned according to spec

[jamesdixon]

When a model has a relationship, according to spec, a reference to the relationship should always be returned with the payload. For example, a book model that has a one-to-one relationship with an author, should be returned like so:

{
    "data": {
        "type": "book",
        "id": "1",
        "attributes": {
            "title": "Mistborn"
        },
        "relationships": {
            "author": {
                "links": {
                    "self": "http://example.com/books/1/relationships/author",
                    "related": "http://example.com/books/1/author"
                },
                "data": {
                    "type": "author",
                    "id": "1"
                }
            }
        },
        "links": {
            "self": "https://localhost:3001/books/1"
        }
    }
}

However, the response generated by the library looks like this:

{
    "data": {
        "type": "book",
        "id": "1",
        "attributes": {
            "title": "Mistborn",
                        "authorId": 1
        },
        "links": {
            "self": "https://localhost:3001/books/1"
        }
    }
}

There are two problems:

1. Relationships are not returned unless included (this is the same as/related to #13)
2. The foreign key (staffId) is being returned and it shouldn't be. According to the spec, or at least how I've interpreted it, the underlying relation semantics should be hidden and only exposed with the relationships key.

As far as I can tell, the only way to solve issue 1 is to always call withRelated on your Bookshelf models as the spec requires the relationship reference to always be returned. However, in order to construct this reference, both the type of relation and its id are needed. This information isn't accessible in Bookshelf unless the relationship is included, except in the case of a one to _x_ relationship where a relation can be derived from a foreign key on the model.

I believe there are two ways to solve issue 2:

• The short term solution would be to use some convention logic to parse out any fields referencing an id and then transforming them into a relation.
• The better solution would be to analyze the the relation (if passed), find any that are one to _x_ and then remove the corresponding keys in the model. However, this relates back to issue 1 mentioned above.

@ShadowManu @jcao02 any thoughts on this?

[ShadowManu]

There's 2 things to handle here, and I will prefer to handle them on different issues, but I will talk about both. The second being simpler:

2) There are initial tests that try to follow the spec regarding that *_id and *_type attributes should be ignored. I don't remember exactly why they were done in snake_case but at most (after reading) it should mean adding similar tests if required.

1) The mapper does a dynamic check of the response object from Bookshelf. As you said, one of the main issues with that is that if the response doesn't include related objects, it neither tells that they exist. To solve this I believe are 2 options: 1.1) Bookshelf adds this metadata on responses: if bookshelf manages to add this information for us, we just need to use it. 1.2) WE add this metadata: giving a fast look at our own API, an approachable option is using the relations option to somehow allow users to indicate existing relations. Other ways probably include modify API signatures or asking users to manually do things on Model classes.

[jamesdixon]

1) The mapper does a dynamic check of the response object from Bookshelf. As you said, one of the main issues with that is that if the response doesn't include related objects, it neither tells that they exist. To solve this I believe are 2 options: 1.1) Bookshelf adds this metadata on responses: if bookshelf manages to add this information for us, we just need to use it. 1.2) WE add this metadata: giving a fast look at our own API, an approachable option is using the relations option to somehow allow users to indicate existing relations. Other ways probably include modify API signatures or asking users to manually do things on Model classes.

I toyed around with this today. There's a Bookshelf plugin called bookshelf-relationships, which allows access to the relationship names. This works well, except for the fact that it doesn't return the ids of the actual related items. That said, I don't see a truly efficient way of doing this a query is always going to be required to load a relationship, even if it's just to get its ids. Unfortunately, Bookshelf doesn't have a method to cache, so it's always going to require a full load.

All that said, it seems to come down to whether or not you want to always include your relations. If so, then we really don't have any changes to make at this point. Alternatively, if you don't want to return the included relation as part of the payload, we could modify the relations option to just return the relationship identifiers.

[jamesdixon]

@ShadowManu taking a second look at this now, we can probably close the issue as it's out of our hands.

As I mentioned in my previous post, regardless if Bookshelf knows the names of the relationships, we still needs the ids associated with those relationships in order to return relationships properly. If a user always passes data to the mapper that has those relationships, then deciding whether to include them or not is just a matter of toggling the serializer option to include or not include the relationships.

That said, I believe the challenge is finding a way to load those relations quickly on the backend without having to do heavy lookups on every relation, every time whether it be through Bookshelf or in another matter. I'm thinking of some sort of "Model cache" that keeps a local cache of those relationships and their ids in some sort of local cache (in-memory, redis, whatever). When doing any sort of fetch, the relationships would be retrieved from that cache rather than having to do a database lookup on each relation. When doing any sort of insert or update, that cache could be invalidated and updated. I realize this is out of the scope of this topic, but just wanted to get your thoughts.

Closing for now -- let me know if you believe we should re-open.