fix: return 400 for invalid relationship resource linkage

lib/jsonapi/error_view.ex

@@ -117,23 +117,36 @@ defmodule JSONAPI.ErrorView do
     )
   end
 
-  @spec missing_relationship_data_id_param_error_attrs(binary()) :: error_attrs()
-  def missing_relationship_data_id_param_error_attrs(relationship_name) do
+  @spec missing_relationship_data_id_param_error_attrs(binary(), non_neg_integer() | nil) ::
+          error_attrs()
+  def missing_relationship_data_id_param_error_attrs(relationship_name, index \\ nil) do
     build_error(
       "Missing id in relationship data parameter",
       400,
       @relationship_resource_linkage_message,
-      "/data/relationships/#{relationship_name}/data/id"
+      relationship_data_pointer(relationship_name, index) <> "/id"
     )
   end
 
-  @spec missing_relationship_data_type_param_error_attrs(binary()) :: error_attrs()
-  def missing_relationship_data_type_param_error_attrs(relationship_name) do
+  @spec missing_relationship_data_type_param_error_attrs(binary(), non_neg_integer() | nil) ::
+          error_attrs()
+  def missing_relationship_data_type_param_error_attrs(relationship_name, index \\ nil) do
     build_error(
       "Missing type in relationship data parameter",
       400,
       @relationship_resource_linkage_message,
-      "/data/relationships/#{relationship_name}/data/type"
+      relationship_data_pointer(relationship_name, index) <> "/type"
+    )
+  end
+
+  @spec invalid_relationship_linkage_error_attrs(binary(), non_neg_integer() | nil) ::
+          error_attrs()
+  def invalid_relationship_linkage_error_attrs(relationship_name, index \\ nil) do
+    build_error(
+      "Invalid resource linkage in relationship",
+      400,
+      @relationship_resource_linkage_message,
+      relationship_data_pointer(relationship_name, index)
     )
   end
 
@@ -184,6 +197,12 @@ defmodule JSONAPI.ErrorView do
     Map.take(error, [:detail, :id, :links, :meta, :source, :status, :title])
   end
 
+  defp relationship_data_pointer(relationship_name, nil),
+    do: "/data/relationships/#{relationship_name}/data"
+
+  defp relationship_data_pointer(relationship_name, index),
+    do: "/data/relationships/#{relationship_name}/data/#{index}"
+
   defp append_field(error, _field, nil), do: error
   defp append_field(error, :meta, value), do: Map.put(error, :meta, %{meta: value})
   defp append_field(error, :source, value), do: Map.put(error, :source, %{pointer: value})

lib/jsonapi/plugs/format_required.ex

@@ -31,28 +31,22 @@ defmodule JSONAPI.FormatRequired do
       when method in ~w[POST PATCH] and is_map(relationships) do
     errors =
       Enum.reduce(relationships, [], fn
-        {_relationship_name, %{"data" => %{"type" => _type, "id" => _}}}, acc ->
+        # Resource linkage per https://jsonapi.org/format/#document-resource-object-linkage
+        # - null for empty to-one relationships.
+        {_relationship_name, %{"data" => nil}}, acc ->
           acc
 
-        {relationship_name, %{"data" => %{"type" => _type}}}, acc ->
-          error = missing_relationship_data_id_param_error_attrs(relationship_name)
-          [error | acc]
-
-        {relationship_name, %{"data" => %{"id" => _type}}}, acc ->
-          error = missing_relationship_data_type_param_error_attrs(relationship_name)
-          [error | acc]
+        # - an array of resource identifier objects for to-many relationships, possibly empty.
+        {relationship_name, %{"data" => linkages}}, acc when is_list(linkages) ->
+          linkages
+          |> Enum.with_index()
+          |> Enum.reduce(acc, fn {linkage, index}, inner_acc ->
+            resource_identifier_errors(linkage, relationship_name, index) ++ inner_acc
+          end)
 
-        {relationship_name, %{"data" => %{}}}, acc ->
-          id_error = missing_relationship_data_id_param_error_attrs(relationship_name)
-          type_error = missing_relationship_data_type_param_error_attrs(relationship_name)
-          [id_error | [type_error | acc]]
-
-        {_relationship_name, %{"data" => _}}, acc ->
-          # Allow things other than resource identifier objects per https://jsonapi.org/format/#document-resource-object-linkage
-          # - null for empty to-one relationships.
-          # - an empty array ([]) for empty to-many relationships.
-          # - an array of resource identifier objects for non-empty to-many relationships.
-          acc
+        # - a single resource identifier object for non-empty to-one relationships.
+        {relationship_name, %{"data" => linkage}}, acc ->
+          resource_identifier_errors(linkage, relationship_name, nil) ++ acc
 
         {relationship_name, _}, acc ->
           error = missing_relationship_data_param_error_attrs(relationship_name)
@@ -94,4 +88,22 @@ defmodule JSONAPI.FormatRequired do
     do: send_error(conn, missing_data_attributes_param())
 
   def call(conn, _), do: send_error(conn, missing_data_param())
+
+  defp resource_identifier_errors(%{"type" => _, "id" => _}, _relationship_name, _index), do: []
+
+  defp resource_identifier_errors(%{"type" => _}, relationship_name, index),
+    do: [missing_relationship_data_id_param_error_attrs(relationship_name, index)]
+
+  defp resource_identifier_errors(%{"id" => _}, relationship_name, index),
+    do: [missing_relationship_data_type_param_error_attrs(relationship_name, index)]
+
+  defp resource_identifier_errors(%{}, relationship_name, index) do
+    [
+      missing_relationship_data_id_param_error_attrs(relationship_name, index),
+      missing_relationship_data_type_param_error_attrs(relationship_name, index)
+    ]
+  end
+
+  defp resource_identifier_errors(_linkage, relationship_name, index),
+    do: [invalid_relationship_linkage_error_attrs(relationship_name, index)]
 end

test/jsonapi/plugs/format_required_test.exs

@@ -444,6 +444,113 @@ defmodule JSONAPI.FormatRequiredTest do
     refute conn.halted
   end
 
+  for {method, path, data} <- [
+        {:post, "/example", %{}},
+        {:patch, "/example/some-id", %{id: "some-id"}}
+      ],
+      linkage <- ["oops", 123, true, ""] do
+    test "halts and returns an error for a scalar resource linkage #{inspect(linkage)} on #{method |> to_string() |> String.upcase()}" do
+      conn =
+        unquote(method)
+        |> conn(
+          unquote(path),
+          Jason.encode!(%{
+            data:
+              Map.merge(unquote(Macro.escape(data)), %{
+                type: "example",
+                attributes: %{},
+                relationships: %{comment: %{data: unquote(linkage)}}
+              })
+          })
+        )
+        |> call_plug
+
+      assert conn.halted
+      assert 400 == conn.status
+
+      %{"errors" => [error]} = Jason.decode!(conn.resp_body)
+
+      assert %{
+               "source" => %{"pointer" => "/data/relationships/comment/data"},
+               "title" => "Invalid resource linkage in relationship"
+             } = error
+    end
+  end
+
+  test "halts and returns an error for a to-many resource linkage containing a non-object" do
+    conn =
+      :post
+      |> conn(
+        "/example",
+        Jason.encode!(%{
+          data: %{
+            type: "example",
+            attributes: %{},
+            relationships: %{reviews: %{data: [%{type: "review", id: "1"}, "oops"]}}
+          }
+        })
+      )
+      |> call_plug
+
+    assert conn.halted
+    assert 400 == conn.status
+
+    %{"errors" => [error]} = Jason.decode!(conn.resp_body)
+
+    assert %{
+             "source" => %{"pointer" => "/data/relationships/reviews/data/1"},
+             "title" => "Invalid resource linkage in relationship"
+           } = error
+  end
+
+  test "halts and returns an error for a to-many resource linkage missing type and id members" do
+    conn =
+      :post
+      |> conn(
+        "/example",
+        Jason.encode!(%{
+          data: %{
+            type: "example",
+            attributes: %{},
+            relationships: %{reviews: %{data: [%{type: "review"}, %{id: "2"}]}}
+          }
+        })
+      )
+      |> call_plug
+
+    assert conn.halted
+    assert 400 == conn.status
+
+    %{"errors" => errors} = Jason.decode!(conn.resp_body)
+
+    error_pointers = Enum.map(errors, fn error -> get_in(error, ["source", "pointer"]) end)
+
+    assert Enum.sort(error_pointers) == [
+             "/data/relationships/reviews/data/0/id",
+             "/data/relationships/reviews/data/1/type"
+           ]
+  end
+
+  test "accepts a to-many resource linkage of resource identifier objects" do
+    conn =
+      :post
+      |> conn(
+        "/example",
+        Jason.encode!(%{
+          data: %{
+            type: "example",
+            attributes: %{},
+            relationships: %{
+              reviews: %{data: [%{type: "review", id: "1"}, %{type: "review", id: "2"}]}
+            }
+          }
+        })
+      )
+      |> call_plug
+
+    refute conn.halted
+  end
+
   test "passes request through" do
     conn =
       :post