diff --git a/lib/jsonapi/error_view.ex b/lib/jsonapi/error_view.ex index 5f8cf4b7..72055592 100644 --- a/lib/jsonapi/error_view.ex +++ b/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}) diff --git a/lib/jsonapi/plugs/format_required.ex b/lib/jsonapi/plugs/format_required.ex index ad80aa1b..37e4292d 100644 --- a/lib/jsonapi/plugs/format_required.ex +++ b/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 diff --git a/test/jsonapi/plugs/format_required_test.exs b/test/jsonapi/plugs/format_required_test.exs index 8096bda5..cac9b831 100644 --- a/test/jsonapi/plugs/format_required_test.exs +++ b/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