Conflicting behavior for Relationships when using `fields` and/or `include`

[Diasporism]

I've noticed there is a non-standard behavior (feature?) that's breaking the json api spec when including different combinations of fields and include parameters.

For example, given three models Author, Book, and Article where an author has many books as well as many articles and both books and articles belong to an author:

if you request /authors/1 you will get a payload that looks like:

{
  "data": {
    "id": 1,
    "type": "authors",
    "attributes": {
      "name": "Neal Stephenson"
    },
    "relationships": {
      "books": {
        "meta": {
          "included": false
        }
      },
      "articles": {
        "meta": {
          "included": false
        }
    }
  }
}

The fact that "relationships" is present when I didn't request anything in the include param is a little weird, but I guess it's helpful? More on this in a bit...

Next hitting /authors/1?include=books outputs something like:

{
  "data": {
    "id": 1,
    "type": "authors",
    "attributes": {
      "name": "Neal Stephenson"
    },
    "relationships": {
      "books": {
        "data": [
          {
            "type": "books",
            "id": 1
          },
          {
            "type": "books",
            "id": 2
          }
        ]
      }
    }
  },
  "included": [
    {
      "id": 1,
      "type": "books",
      "attributes": {
        "name": "Snow Crash"
      }
    },
    {
      "id": 2,
      "type": "books",
      "attributes": {
        "name": "Cryptonomicon"
      }
    }
  ]
}

At first glance that seems pretty much as expected, and actually in my opinion this is correct. But what's weird is that the relationships node no longer includes:

"articles": {
  "meta": {
    "included": false
  }
}

meta is a non-standard, anything goes kinda field regardless, and relationships may or may not exist at any given time according to the spec so I guess this might be technically correct, but it's not what I would consider to be good consistency.

But it gets worse:

Go ahead and request /authors/1?include=books&fields[books]=name and the linkage between relationships and included is straight up broken:

{
  "data": {
    "id": 1,
    "type": "authors",
    "attributes": {
      "name": "Neal Stephenson"
    }
  },
  "included": [
    {
      "id": 1,
      "type": "books",
      "attributes": {
        "name": "Snow Crash"
      }
    },
    {
      "id": 2,
      "type": "books",
      "attributes": {
        "name": "Cryptonomicon"
      }
    }
  ]
}

You might say "Oh but you can just infer the books belong to the main resource," except the same thing happens on the index action as well (/authors?include=books&fields[books]=name):

{
  "data": [
    {
      "id": 1,
      "type": "authors",
      "attributes": {
        "name": "Neal Stephenson"
      }
    },
    {
      "id": 2,
      "type": "authors",
      "attributes": {
        "name": "R.A. Salvatore"
      }
    }
  ],
  "included": [
    {
      "id": 1,
      "type": "books",
      "attributes": {
        "name": "Snow Crash"
      }
    },
    {
      "id": 2,
      "type": "books",
      "attributes": {
        "name": "Cryptonomicon"
      }
    },
    {
      "id": 3,
      "type": "books",
      "attributes": {
        "name": "The Legend of Drizzt"
      }
    }
  ]
}

That ain't gunna work for anybody.

Anyway, I've been able to break this using various combinations of include and fields params and it all seems to boil down to one culprit (which is a combination of two conflicting ideas): 1) The attempt to be extra helpful with non-standard meta information in the relationships section when no relationships were asked for 2) This line passing fields instead of include into the requested_relationships method.

A naive fix is to modify two methods in lib/jsonapi/serializable/resource.rb

rels = requested_relationships(fields).each_with_object({}) do |(k, v), h|
  h[k] = v.as_jsonapi(include.include?(k))
end

to

rels = requested_relationships(include).each_with_object({}) do |(k, v), h|
  h[k] = v.as_jsonapi(include.include?(k))
end

and

def requested_relationships(fields)
  @_relationships.select { |k, _| fields.nil? || fields.include?(k) }
end

to

def requested_relationships(includes)
  return {} if includes.empty?
  @_relationships.select { |k, _| includes.include?(k) }
end

This has one caveat: relationships is no longer returned unless a user requested one or more via the include param (which in turn breaks a bunch of specs). However, this is still technically correct according to spec.

So what do you say? If I put in a PR to fix it can we live without

"relationships": {
    "books": {
      "meta": {
        "included": false
      }
    },
    "articles": {
      "meta": {
        "included": false
      }
  }
}

which really wasn't helping anyone anyway?

[Diasporism]

My naive fix suggested in the original post was close, but didn't account for a couple things like:

has_one :foo do
  linkage always: true
end

The PR address that and updates relevant specs while deleting specs no longer necessary (the ones that test for the relationships existing even though the include param is omitted).

[noefroidevaux]

Hi @Diasporism , great PR! I think it would help a lot of people to merge it. For example, iOS JSONAPI doesn't work well with the meta "included": false when it's not included. I wonder if someone of the jsonapi-rb org can take a look to this PR? /cc @remear @dawidof @beauby

[Diasporism]

Please excuse the bump, but, any word? I'm currently running a forked version of this repo and I'd rather, not if possible.

[remear]

I'm on-board with the change. I haven't seen any counterarguments presented.

Historically I don't think semver has been followed much. In my opinion, this would be a breaking change. I don't know that it's worth wrapping in an option with a deprecation. We might not even have much of a nice means of passing the options down into that area.

I'll do my best to find some time within the next week or so to review that PR. Meanwhile, I'd welcome opinions on how to handle releasing this. It's already sat for nearly half a year, so I'm not keen on holding up releasing this just to build up a larger "major" release.

[beauby]

Just offering my two cents and some context here:

---

The fact that "relationships" is present when I didn't request anything in the include param is a little weird, but I guess it's helpful? More on this in a bit...

This comes from the spec:

If a client requests a restricted set of fields for a given resource type, an endpoint MUST NOT include additional fields in resource objects of that type in its response.

Note that fields are defined as:

A resource object’s attributes and its relationships are collectively called its “fields”.

The resource linkage is indeed "broken" in that case, which is expected by the spec (emphasis mine):

Note: Because compound documents require full linkage (except when relationship linkage is excluded by sparse fieldsets), intermediate resources in a multi-part path must be returned along with the leaf nodes. For example, a response to a request for comments.author should include comments as well as the author of each of those comments.

---

At first glance that seems pretty much as expected, and actually in my opinion this is correct. But what's weird is that the relationships node no longer includes: ["meta": {"included": false}]

The reason behind the "meta included false" thing is that without a sparse field set (fields query parameter), all fields (i.e. attributes and relationships) are present in the response, but, as a default, the linkage data is not included (which is valid but not forced by the spec). The intention is that if resource links (not linkage) are specified, then only the resource links are present in the relationship (when the linkage data is not explicitly requested through ?include=), and barring that, "meta included false" is used as a placeholder to prevent the relationship object to be empty (which is not permitted by the spec). Note that t he default behavior can be changed by specifying linkage always: true as someone mentioned above.

---

You might say "Oh but you can just infer the books belong to the main resource," except the same thing happens on the index action as well (/authors?include=books&fields[books]=name):

That's true, but the assumption is that a client wouldn't explicitly discard the books relationship from the sparse field set if they actually needed the resource linkage data.

---

Anyway, I've been able to break this using various combinations of include and fields params

IMO, in ways expected by the spec, which result from the client explicitly requesting only partial information, unless I am missing something.

---

This has one caveat: relationships is no longer returned unless a user requested one or more via the include param (which in turn breaks a bunch of specs). However, this is still technically correct according to spec.

This is indeed allowed by the spec:

If a client does not specify the set of fields for a given resource type, the server MAY send all fields, a subset of fields, or no fields for that resource type.

but then so is the current behavior, and I am afraid changing the default would break quite some stuff for current users (especially those relying on resource links, which again, was the intended way).

---

In the end, I think it really boils down to whether people use resource links or not. If they do, the current defaults make sense. If they don't I agree that the "meta included false" thing is not very useful. I would personally be in favor of offering a config option to exclude relationships with "empty resource linkage" (i.e. those resulting in "meta included false") from the default field set, rather than breaking things for people using resource links.

[unicornzero]

@beauby I'm spiking using jsonapi-rb in my current codebase and found this issue when trying to understand the {"meta"=>{"included"=>false}} relationships objects in a response. Have you had any further thought/discussion/effort around the idea you mentioned in your last paragraph?:

I would personally be in favor of offering a config option to exclude relationships with "empty resource linkage" (i.e. those resulting in "meta included false") from the default field set