Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add include id relation only option and smart related link behavior based on that #23

Closed
bterkuile opened this issue Sep 10, 2015 · 8 comments

Comments

@bterkuile
Copy link

I am experimenting with this gem in situations with heavily nested structures. The include: option of the serializer handles the full include stack. In many cases I would want only to add the type and id in the relationships section, but do not need to include the objects itself (I think, but want to test the idea over here). A possible scenario would be

JSONAPI::Serializer.serialize [posts], is_collection: true, include: %w[
  comments
  comments.annotations
  comments.#tags
  comments.#user
  #author
]

Meaning I want to serialize posts, include the comments and the comments' annotations (sorry for the bad inspiration in finding good examples, it is about the idea) but only want to include the id references of the comments' tags and user and of the post's author.

The resulting data should look something like:

data: [
  type: 'posts'
  id: 'post-1'
  attributes: {}
  relationships:
    comments:
      data: [
        {type: 'comments', id: 'comment-1'}
      ]
      # no related link needed since included in the payload
    author:
      data: {type: 'people', id: 'person-1'}
      links: {related: "/people/1"}
,
  type: 'posts'
  id: 'post-2'
  attributes: {}
  relationships: {tl: 'dr'}
]
included: [
  type: 'comments'
  id: 'comment-1'
  attributes: {}
  relationships:
    annotations:
      data: [
        {type: 'annotations', id: 'annotation-1'}
      ]
    user:
      data: {type: 'users', id: 'user-1'}
      links: {related: "/users/1"}
    tags:
      data: [{type: 'tags', id: 'tag-1'}]
      links: {related: "/comments/1/tags"}
,
  type: 'annotations'
  id: 'annotation-1'
  attributes: {}
  relationships:
    author: {links: {related: "/annotations/1/author"}}
]

Would this make sense at all? And if yes would the hashtag prefix notation be the way to go in this? This would at first need the addition to specify the hashtag relations, and a non trivial id lookup relationship option:

class PostSerializer
  has_one :author, id_lookup: -> post { post.author_id }
  has_many :comments, id_lookup: -> post { post.comment_ids }
end

I am willing to help implementing this if the idea behind it makes sense. Since I am really new to JSONAPI I would like to know first if there is not some other point I am missing. This in my idea would dramatically increase performance by removing the need to fetch all the associated objects. It would also solve the case where the same record is included in a different context bug (other issue).

For people more comfortable with the full javascript object notation in stead of the coffeescript version, here the payload as javascript:

{
  data: [
    {
      type: 'posts',
      id: 'post-1',
      attributes: {},
      relationships: {
        comments: {
          data: [
            {
              type: 'comments',
              id: 'comment-1'
            }
          ]
        },
        author: {
          data: {
            type: 'people',
            id: 'person-1'
          },
          links: {
            related: "/people/1"
          }
        }
      }
    }, {
      type: 'posts',
      id: 'post-2',
      attributes: {},
      relationships: {
        tl: 'dr'
      }
    }
  ],
  included: [
    {
      type: 'comments',
      id: 'comment-1',
      attributes: {},
      relationships: {
        annotations: {
          data: [
            {
              type: 'annotations',
              id: 'annotation-1'
            }
          ]
        },
        user: {
          data: {
            type: 'users',
            id: 'user-1'
          },
          links: {
            related: "/users/1"
          }
        },
        tags: {
          data: [
            {
              type: 'tags',
              id: 'tag-1'
            }
          ],
          links: {
            related: "/comments/1/tags"
          }
        }
      }
    }, {
      type: 'annotations',
      id: 'annotation-1',
      attributes: {},
      relationships: {
        author: {
          links: {
            related: "/annotations/1/author"
          }
        }
      }
    }
  ]
}

Thanks for the effort!

@fotinakis
Copy link
Owner

Hey @bterkuile, thanks for the thoughts here. Unfortunately I don't think the hash syntax you've suggested is compatible with the JSON-API spec, and I wouldn't want to introduce a custom syntax like that into this gem.

The links: {related: section doesn't actually matter that much—the thing that's important is whether the relationship is in included in the compound document (in the included section). If it exists in the compound document, the client will not fetch it from the server, even if there is a links related section.

I think I see what you're saying though, it's basically that things are too slow because of all the internal object allocations and lookups? I have been encountering some performance problems with jsonapi-serializers recently, so I definitely want to address that and optimize the internal recursion at some point. I think you're right—a big optimization we could make would be to have a way to do ID lookups without having to load the full object. It may be as simple as adding a hint like id_attribute: :author_id and nothing else, just letting jsonapi-serializers figure out if it only needs the ID or needs to load the full object.

@bterkuile
Copy link
Author

That is where the hash syntax is for, indicating to use the child id from the object itself without loading relation itself. I think after merging my current open pull request the performance already will increase a lot 😄

@fotinakis
Copy link
Owner

You know what, I realize this is basically the same as #8 — we could do something automatic, like if you provide an id_attribute or id_lookup attribute, we always include the linkage data automatically (since it's basically a free lookup). Then, if you also include the relationship, we load the full object and it gets loaded in to the compound document.

@fotinakis
Copy link
Owner

That avoids having to have the new hash syntax, but still gets what you want (I think).

@bterkuile
Copy link
Author

I think that could be it!

@fotinakis
Copy link
Owner

I'll take a look at your PR today, I think that should get in before moving forward with this.

@bterkuile
Copy link
Author

A conceptual implementation that automatically adds the id of the parent object if it can be inferred is done in: https://github.com/bterkuile/jsonapi-serializers/tree/feature/include_has_one_id_if_present_on_object

This breaks some existing specs, but I think in a good way. aka explicitly add data: nil if we already know that there will be no data for the has_one relationship.

@fotinakis
Copy link
Owner

Consolidating this issue into #30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants