rails-api / active_model_serializers

ActiveModel::Serializer implementation and Rails hooks
MIT License
5.33k stars 1.39k forks source link

Self-reference relationship without association naming on include #2204

Closed rafaelcgo closed 7 years ago

rafaelcgo commented 7 years ago

Expected behavior vs actual behavior

Shouldn't included section include friends array with all users inside them?

If I have more than one association that refers to users (ex: invited_users), will all associations be written on the included section mixed up, as type: 'users'? How will I be able to differentiate the collections of the same type?

Steps to reproduce

class User < ApplicationRecord
  has_many :friendships, dependent: :destroy
  has_many :friends, through: :friendships
end
class Friendship < ApplicationRecord
  belongs_to :user
  belongs_to :friend, class_name: 'User'
end
class UserSerializer < ActiveModel::Serializer
  has_many :friends
end
class UsersController < ApplicationController
  def show
    render json: @user, include: ['friends']
  end
end

email1@email.com is friend of email2@email.com and email3@email.com

users#show - aa67c98c-d81f-5a9c-b0bc-26caa0051aea

{
  "data": {
    "id": "aa67c98c-d81f-5a9c-b0bc-26caa0051aea",
    "type": "users",
    "attributes": {
      "email": "email1@email.com"
    },
    "links": {
      "self": "http://localhost:5001/users/aa67c98c-d81f-5a9c-b0bc-26caa0051aea.json"
    }
  },
  "included": [
    {
      "id": "37f7eeff-831b-5c41-984a-254965f58c0f",
      "type": "users",
      "attributes": {
        "email": "email2@email.com"
      },
      "links": {
        "self": "http://localhost:5001/users/37f7eeff-831b-5c41-984a-254965f58c0f.json"
      }
    },
    {
      "id": "3c3a8751-f8dd-4d80-93b2-4df8725a6490",
      "type": "users",
      "attributes": {
        "email": "email3@email.com"
      },
      "links": {
        "self": "http://localhost:5001/users/3c3a8751-f8dd-4d80-93b2-4df8725a6490.json"
      }
    }
  ]
}

Environment

ActiveModelSerializers Version (commit ref if not on tag): 0.10.6

Output of ruby -e "puts RUBY_DESCRIPTION": ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-darwin15]

OS Type & Version: MacOS 10.11.6

Integrated application and version (e.g., Rails, Grape, etc): Rails 5.1.4

Additonal helpful information

(e.g., Gemfile.lock, configurations, PR containing a failing test, git bisect results)

bf4 commented 7 years ago

If I have more than one association that refers to users (ex: invited_users), will all associations be written on the included section mixed up, as type: 'users'? How will I be able to differentiate the collections of the same type?

@rafaelcgo I think perhaps that's a question for the json:api spec. I'm surprised the relationships are missing. What's I'd expect would be something like:

{
  "data": {
    "id": "aa67c98c-d81f-5a9c-b0bc-26caa0051aea",
    "type": "users",
      "attributes": {
        "email": "email1@email.com"
      },
    "relationships": { "friends": { 
       "data": [{ "id": "37f7eeff-831b-5c41-984a-254965f58c0f", "type": "users" }, { "id": "3c3a8751-f8dd-4d80-93b2-4df8725a6490", "type": "users" }],
    "links": {
      "self": "http://localhost:5001/users/aa67c98c-d81f-5a9c-b0bc-26caa0051aea.json"
    }
  },
  "included": [
    {
      "id": "37f7eeff-831b-5c41-984a-254965f58c0f",
      "type": "users",
      "attributes": {
        "email": "email2@email.com"
      },
      "links": {
        "self": "http://localhost:5001/users/37f7eeff-831b-5c41-984a-254965f58c0f.json"
      }
    },
    {
      "id": "3c3a8751-f8dd-4d80-93b2-4df8725a6490",
      "type": "users",
      "attributes": {
        "email": "email3@email.com"
      },
      "links": {
        "self": "http://localhost:5001/users/3c3a8751-f8dd-4d80-93b2-4df8725a6490.json"
      }
    }
  ]
}
rafaelcgo commented 7 years ago

I think I got it. I will investigate further.

@bf4 The way you expected, in order to a client identify which users are friends or invited_users it would need to filter the included: users by friend_ids or invited_user_ids. Is it right?

bf4 commented 7 years ago

@rafaelcgo I'm not sure what you mean by filter.

relationships and included a way to flatten the response, and avoid nesting duplicate objects

primary data 
  user 1 with attributes x has friends who are users 2 and 3
  user 4 with attributes y has friends who are users 3
included 
   user 2 with attributes z
   user 3 with attributes q

so the client can build the graph of [1 -> [2,3], 4 -> [3] ] and use the included records to populate details for 2 and 3. that's called sideloading

alternatively, instead of including 2 and 3, you could have each user's friends relationship have a link to an endpoint for getting the full resource objects, to be requested on demand

bf4 commented 7 years ago

How will I be able to differentiate the collections of the same type?

I think that's what the purpose of the type: users attribute is in json:api.

You have a record/entity you call users that can have different meanings.

For example, if you had a resource /users/1/friends you'd expect it to return a collection of resource objects of type: users even though the HTTP resource is 'friends'

Similarly, when a user's relationships are { friends: {...}, partners: {...}, posts: {...} those are the names of the relationships. It's the role of the type attributes of the resource linkage (object identifier) to tell you the what the relationship object is.. e,g, type: users, type: users, type: posts, etc.

rafaelcgo commented 7 years ago

@bf4 got it, sorry for taking your time on my json:api misunderstanding.

@rafaelcgo I think perhaps that's a question for the json:api spec. I'm surprised the relationships are missing. What's I'd expect would be something like:

When I try to set sparse fields for the 'main' resource, relationships is not rendered.

class UsersController < ApplicationController
  def show
    render json: @user, include: ['profile'], fields: { user: ['email'] }
  end
end
{
  "data": {
    "id": "3c3a8751-f8dd-4d80-93b2-4df8725a6490",
    "type": "users",
    "attributes": {
      "email": "email1@email.com"
    },
    "links": {
      "self": "http://localhost:5001/users/3c3a8751-f8dd-4d80-93b2-4df8725a6490.json"
    }
  },
  "included": [
    {
      "id": "c4293b72-20a8-46da-8b72-7364d2d49864",
      "type": "profiles",
      "attributes": {
        "first-name": "first",
        "last-name": null,
        "created-at": "2017-09-26T22:52:10.096Z"
      }
    }
  ]
}

When I try to set sparse fields for the associated resource (or when I don't set sparse fields), relationships is rendered.

class UsersController < ApplicationController
  def show
    render json: @user, include: ['profile'], fields: { profile: ['first_name'] }
  end
end
{
  "data": {
    "id": "3c3a8751-f8dd-4d80-93b2-4df8725a6490",
    "type": "users",
    "attributes": {
      "email": "email1@email.com",
      "created-at": "2017-09-26T19:42:38.948Z"
    },
    "relationships": {
      "profile": {
        "data": {
          "id": "c4293b72-20a8-46da-8b72-7364d2d49864",
          "type": "profiles"
        }
      }
    },
    "links": {
      "self": "http://localhost:5001/users/3c3a8751-f8dd-4d80-93b2-4df8725a6490.json"
    }
  },
  "included": [
    {
      "id": "c4293b72-20a8-46da-8b72-7364d2d49864",
      "type": "profiles",
      "attributes": {
        "first-name": "first"
      }
    }
  ]
}

I've tried to do add 'relationships' to the fields, but it doesn't work (and it shouldn't because 'relationships' is not an 'attribute').

render json: @user, include: ['profile'], fields: { user: ['relationships', 'email'] }

Is this a bug or an expected behavior and I'm missing something again?

bf4 commented 7 years ago

@rafaelcgo On first glance, it appears to be a misunderstanding of the JSON:API spec. Fields include attributes and relationships. So, email and profile are both fields. And since fields is a whitelist, it would make sense that fields: { user: ['email'] } would not include the profile relationship. However, since you are include: ['profile'], and it ,may be a bug for the profile relationship not to be linked to the user.

http://jsonapi.org/format/#document-compound-documents

The only exception to the full linkage requirement is when relationship fields that would otherwise contain linkage data are excluded via sparse fieldsets.

http://jsonapi.org/examples/

Pay attention to the fact that you have to add a relationship name both in include and fields (since relationships are fields too), otherwise you’ll get:

rafaelcgo commented 7 years ago

From http://jsonapi.org/examples/:

Request with fields parameter: GET /articles?include=author&fields[articles]=title,body,author&fields[people]=name HTTP/1.1

One should be able to specify sparse fields for the 'main' resource and 'relationships' should be rendered as well.

bf4 commented 7 years ago

From http://jsonapi.org/examples/:

Request with fields parameter: GET /articles?include=author&fields[articles]=title,body,author&fields[people]=name HTTP/1.1

One should be able to specify sparse fields for the 'main' resource and 'relationships' should be rendered as well.

@rafaelcgo I don't follow your previous comment

That example includes author and specifies the field articles[author]

The example you reference is followed by my quote above, then the same example without the field articles[author] and there are no relationships

screen grab below:

screen shot 2017-10-18 at 10 45 10 pm screen shot 2017-10-18 at 10 45 19 pm
bf4 commented 7 years ago

At this point it looks like this issue is invalid, as the compound document with spare fields behaves according to the spec.

I'm going to close this issue, but please follow up with your thoughts.

rafaelcgo commented 7 years ago

@bf4 Thank you, I was missing the part that I needed to include the relationship name inside the fields as well. You were right from the beginning.

I see that http://jsonapi-rb.org/ is now referenced in the README and the owner of the project used to be a core contributor of AMS.

Do you guys recommend migrating to jsonapi-rb for those who are using AMS with jsonapi adapter?

bf4 commented 7 years ago

Do you guys recommend migrating to jsonapi-rb for those who are using AMS with jsonapi adapter?

Really up to you. If you're doing JSON:API, it's a better implementation, I think. The 0-10 got kind of messed up

beauby commented 7 years ago

@rafaelcgo I'm obviously partial, but I believe that at the moment jsonapi-rb is faster, more customizable, and has less bugs. This is due to the facts that 1. it supports JSON API only, so it is not burdened with unrelated constraints, and 2. it was started from scratch using the insights gathered working on AMS, so it is less code and no legacy burden.

rafaelcgo commented 7 years ago

@beauby Thanks for your comment. Is it production ready already?

bf4 commented 7 years ago

@rafaelcgo believe me, we wouldn't recommend it if it weren't.

rafaelcgo commented 7 years ago

Gonna give it a try! Thanks guys.