vt-elixir / ja_serializer

JSONAPI.org Serialization in Elixir.
Other
640 stars 148 forks source link

Should links be output for an empty relationship? #338

Closed mhanberg closed 2 years ago

mhanberg commented 3 years ago

Problem

I currently have a relationship that looks like so

  has_one :foo,
    field: :foo_id,
    type: "foo",
    links: [
      related: "/foos/:foo_id"
    ]

  def foo(%MyApp.Foo{foo_id: nil}, _conn) do
    nil
  end

  def foo(%MyApp.Foo{foo_id: foo_id}, _conn) do
    %{id: foo_id}
  end

  def foo_id(struct, _conn) do
    struct.foo_id
  end

This results in the code rendering a broken links. The results resembling

%{"data" => nil, "links" => %{"related" => "/foos/"}}

I feel like there should not be any links rendered, as there is no resource to link to.

Am I missing the way to do this or is this broken?

Thanks

beerlington commented 3 years ago

Hey sorry for the delay on this. Yes you are right, this does seem broken. Looking at the spec, it seems like the correct response would be %{"data" => nil }. I'm surprised I haven't run into this before so I'll see if I can reproduce it. Stay tuned.

lucas-nelson commented 3 years ago

I'm running into a semi-related problem. The JaSerializer.Builder.ResourceIdentifier.build function is causing the data key on a %JaSerializer.Builder.Relationship{} to be set to :empty_relationship.

The JaSerializer.Builder.Relationship.build function has an empty? function that operates on that struct and seems to be missing handling for this. If I add: defp empty?(%__MODULE__{data: :empty_relationship, links: nil, meta: nil}), do: true to JaSerializer.Builder.Relationship, the relationship (correctly IMO) does not appear in the output.

Perhaps the same change would fix this issue?

beerlington commented 3 years ago

@lucas-nelson Looking into how we handle empty relationships elsewhere, I don't believe stripping out the relationship is always the right approach. In the case of a link where the parent ID is used, I believe we still want to return the links because they are valid.

For example, if we had this:

  defmodule ArticleSerializer do
    use JaSerializer

    has_one(
      :author,
      link: "/articles/:id/author",
      serializer: PersonSerializer,
    )
  end

We would still want it serialized since the link could technically still be valid:

    "relationships": {
      "author": {
        "links": {
          "related": "/articles/1/author"
        },
        "data": null
      },

Instead what I'm going to do is remove any broken links from the links array when it's serialized. This seems to satisfy the JSON API spec where it says:

If present, a related resource link MUST reference a valid URL, even if the relationship isn’t currently associated with any target resources. Additionally, a related resource link MUST NOT change because its relationship’s content changes.

So then in @mhanberg 's original example, the response would just be %{"data" => nil}