From c5662e90bad8bb5170e30f134d54a905b822b89f Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Fri, 21 Oct 2022 07:38:33 -0400 Subject: [PATCH] improvement: validate that relay? queries use `keyset?: true` actions improvement: only add `count` to pages when one relevant query is countable --- lib/ash_graphql.ex | 5 +- lib/resource/resource.ex | 125 +++++++++--------- .../require_keyset_for_relay_queries.ex | 27 ++++ ...e_id_pkey.ex => require_pkey_delimiter.ex} | 2 +- test/support/resources/relay_tag.ex | 2 +- 5 files changed, 95 insertions(+), 66 deletions(-) create mode 100644 lib/resource/transformers/require_keyset_for_relay_queries.ex rename lib/resource/transformers/{require_id_pkey.ex => require_pkey_delimiter.ex} (92%) diff --git a/lib/ash_graphql.ex b/lib/ash_graphql.ex index 85fb2d4..8ada1e1 100644 --- a/lib/ash_graphql.ex +++ b/lib/ash_graphql.ex @@ -1,9 +1,6 @@ defmodule AshGraphql do @moduledoc """ - AshGraphql is a graphql front extension for the Ash framework. - - See the [getting started guide](/getting_started.md) for information on setting it up, and - see the `AshGraphql.Resource` documentation for docs on its DSL + AshGraphql is a GraphQL extension for the Ash framework. """ defmacro __using__(opts) do diff --git a/lib/resource/resource.ex b/lib/resource/resource.ex index 4118946..0b9c132 100644 --- a/lib/resource/resource.ex +++ b/lib/resource/resource.ex @@ -288,7 +288,8 @@ defmodule AshGraphql.Resource do } @transformers [ - AshGraphql.Resource.Transformers.RequireIdPkey, + AshGraphql.Resource.Transformers.RequirePkeyDelimiter, + AshGraphql.Resource.Transformers.RequireKeysetForRelayQueries, AshGraphql.Resource.Transformers.ValidateActions, AshGraphql.Resource.Transformers.ValidateCompatibleNames ] @@ -2318,7 +2319,7 @@ defmodule AshGraphql.Resource do } } ] - |> add_count_to_relay(schema, countable?), + |> add_count_to_page(schema, countable?), identifier: String.to_atom("#{type}_connection"), module: schema, name: Macro.camelize("#{type}_connection"), @@ -2329,7 +2330,7 @@ defmodule AshGraphql.Resource do end end - defp add_count_to_relay(fields, schema, true) do + defp add_count_to_page(fields, schema, true) do [ %Absinthe.Blueprint.Schema.FieldDefinition{ description: "Total count on all pages", @@ -2343,7 +2344,7 @@ defmodule AshGraphql.Resource do ] end - defp add_count_to_relay(fields, _, _), do: fields + defp add_count_to_page(fields, _, _), do: fields # sobelow_skip ["DOS.StringToAtom"] defp page_of(resource, schema) do @@ -2356,31 +2357,33 @@ defmodule AshGraphql.Resource do action.type == :read && action.pagination end) + countable? = + resource + |> queries() + |> Enum.any?(fn query -> + action = Ash.Resource.Info.action(resource, query.action) + action.pagination && action.pagination.offset? && action.pagination.countable + end) + if paginatable? do %Absinthe.Blueprint.Schema.ObjectTypeDefinition{ description: "A page of #{inspect(type)}", - fields: [ - %Absinthe.Blueprint.Schema.FieldDefinition{ - description: "The records contained in the page", - identifier: :results, - module: schema, - name: "results", - __reference__: ref(__ENV__), - type: %Absinthe.Blueprint.TypeReference.List{ - of_type: %Absinthe.Blueprint.TypeReference.NonNull{ - of_type: type + fields: + [ + %Absinthe.Blueprint.Schema.FieldDefinition{ + description: "The records contained in the page", + identifier: :results, + module: schema, + name: "results", + __reference__: ref(__ENV__), + type: %Absinthe.Blueprint.TypeReference.List{ + of_type: %Absinthe.Blueprint.TypeReference.NonNull{ + of_type: type + } } } - }, - %Absinthe.Blueprint.Schema.FieldDefinition{ - description: "The count of records", - identifier: :count, - module: schema, - name: "count", - type: :integer, - __reference__: ref(__ENV__) - } - ], + ] + |> add_count_to_page(schema, countable?), identifier: String.to_atom("page_of_#{type}"), module: schema, name: Macro.camelize("page_of_#{type}"), @@ -2400,47 +2403,49 @@ defmodule AshGraphql.Resource do action.type == :read && action.pagination end) + countable? = + resource + |> queries() + |> Enum.any?(fn query -> + action = Ash.Resource.Info.action(resource, query.action) + action.pagination && action.pagination.keyset? && action.pagination.countable + end) + if paginatable? do %Absinthe.Blueprint.Schema.ObjectTypeDefinition{ description: "A keyset page of #{inspect(type)}", - fields: [ - %Absinthe.Blueprint.Schema.FieldDefinition{ - description: "The records contained in the page", - identifier: :results, - module: schema, - name: "results", - __reference__: ref(__ENV__), - type: %Absinthe.Blueprint.TypeReference.List{ - of_type: %Absinthe.Blueprint.TypeReference.NonNull{ - of_type: type + fields: + [ + %Absinthe.Blueprint.Schema.FieldDefinition{ + description: "The records contained in the page", + identifier: :results, + module: schema, + name: "results", + __reference__: ref(__ENV__), + type: %Absinthe.Blueprint.TypeReference.List{ + of_type: %Absinthe.Blueprint.TypeReference.NonNull{ + of_type: type + } } + }, + %Absinthe.Blueprint.Schema.FieldDefinition{ + description: "The first keyset in the results", + identifier: :start_keyset, + module: schema, + name: "start_keyset", + type: :string, + __reference__: ref(__ENV__) + }, + %Absinthe.Blueprint.Schema.FieldDefinition{ + description: "The last keyset in the results", + identifier: :end_keyset, + module: schema, + name: "end_keyset", + type: :string, + __reference__: ref(__ENV__) } - }, - %Absinthe.Blueprint.Schema.FieldDefinition{ - description: "The count of records", - identifier: :count, - module: schema, - name: "count", - type: :integer, - __reference__: ref(__ENV__) - }, - %Absinthe.Blueprint.Schema.FieldDefinition{ - description: "The first keyset in the results", - identifier: :start_keyset, - module: schema, - name: "start_keyset", - type: :string, - __reference__: ref(__ENV__) - }, - %Absinthe.Blueprint.Schema.FieldDefinition{ - description: "The last keyset in the results", - identifier: :end_keyset, - module: schema, - name: "end_keyset", - type: :string, - __reference__: ref(__ENV__) - } - ], + ] + |> add_count_to_page(schema, countable?), identifier: String.to_atom("keyset_page_of_#{type}"), module: schema, name: Macro.camelize("keyset_page_of_#{type}"), diff --git a/lib/resource/transformers/require_keyset_for_relay_queries.ex b/lib/resource/transformers/require_keyset_for_relay_queries.ex new file mode 100644 index 0000000..4f31a6e --- /dev/null +++ b/lib/resource/transformers/require_keyset_for_relay_queries.ex @@ -0,0 +1,27 @@ +defmodule AshGraphql.Resource.Transformers.RequireKeysetForRelayQueries do + @moduledoc "Ensures that all relay queries configure keyset pagination" + use Spark.Dsl.Transformer + + alias Spark.Dsl.Transformer + + def after_compile?, do: true + + def transform(dsl) do + dsl + |> AshGraphql.Resource.Info.queries() + |> Enum.each(fn query -> + if query.relay? do + action = Ash.Resource.Info.action(dsl, query.action) + + unless action.pagination && action.pagination.keyset? do + raise Spark.Error.DslError, + module: Transformer.get_persisted(dsl, :module), + message: "Relay queries must support keyset pagination", + path: [:graphql, :queries, query.name] + end + end + end) + + {:ok, dsl} + end +end diff --git a/lib/resource/transformers/require_id_pkey.ex b/lib/resource/transformers/require_pkey_delimiter.ex similarity index 92% rename from lib/resource/transformers/require_id_pkey.ex rename to lib/resource/transformers/require_pkey_delimiter.ex index b300e60..c8b8c03 100644 --- a/lib/resource/transformers/require_id_pkey.ex +++ b/lib/resource/transformers/require_pkey_delimiter.ex @@ -1,4 +1,4 @@ -defmodule AshGraphql.Resource.Transformers.RequireIdPkey do +defmodule AshGraphql.Resource.Transformers.RequirePkeyDelimiter do @moduledoc "Ensures that the resource has a primary key called `id`" use Spark.Dsl.Transformer diff --git a/test/support/resources/relay_tag.ex b/test/support/resources/relay_tag.ex index 428bf49..767da82 100644 --- a/test/support/resources/relay_tag.ex +++ b/test/support/resources/relay_tag.ex @@ -23,7 +23,7 @@ defmodule AshGraphql.Test.RelayTag do defaults([:create, :update, :destroy, :read]) read :read_paginated do - pagination(required?: true, keyset?: true, countable: true) + pagination(required?: true, offset?: true, keyset?: true, countable: true) end end