diff --git a/config/site/examples/schema_customization_rake_tasks/Rakefile b/config/site/examples/schema_customization_rake_tasks/Rakefile index 5a8baaf22..1fd03cea2 100644 --- a/config/site/examples/schema_customization_rake_tasks/Rakefile +++ b/config/site/examples/schema_customization_rake_tasks/Rakefile @@ -39,4 +39,11 @@ ElasticGraph::Local::RakeTasks.new( # Within `ElasticGraph::Local::RakeTasks.new { ... }` in your `Rakefile`: tasks.type_name_overrides = {JsonSafeLong: "BigInt"} # :snippet-end: + + # :snippet-start: cursor_type_override + # Within `ElasticGraph::Local::RakeTasks.new { ... }` in your `Rakefile`: + # Override Cursor to String for federation compatibility with subgraphs + # that use String for cursor fields per the Relay spec. + tasks.type_name_overrides = {Cursor: "String"} + # :snippet-end: end diff --git a/config/site/src/guides/customizing-the-graphql-schema.md b/config/site/src/guides/customizing-the-graphql-schema.md index 456e7adc6..60712c684 100644 --- a/config/site/src/guides/customizing-the-graphql-schema.md +++ b/config/site/src/guides/customizing-the-graphql-schema.md @@ -70,6 +70,28 @@ scalar for one with a name your team prefers—use [`type_name_overrides`]({% ap The standard GraphQL scalars (`Boolean`, `Float`, `ID`, `Int`, `String`) and the root `Query` type cannot be renamed this way. +### Federation Compatibility: Overriding `Cursor` to `String` + +When composing an ElasticGraph subgraph into a federated supergraph alongside other subgraphs that use `String` for +cursor fields (as the [Relay GraphQL Cursor Connections Specification](https://relay.dev/graphql/connections.htm) permits), +federation composition may fail with a type incompatibility error. ElasticGraph uses a dedicated `Cursor` scalar type +for cursor fields by default, which provides better type safety and documentation but can cause conflicts. + +To resolve this, override the `Cursor` type to `String`: + +{% include copyable_code_snippet.html language="ruby" data="schema_customization_rake_tasks.snippets.Rakefile.cursor_type_override" %} + +This configuration causes ElasticGraph to: +- Skip registration of the `Cursor` scalar (avoiding duplicate type definitions) +- Use `String` for all cursor-related fields (`PageInfo.startCursor`, `PageInfo.endCursor`, `Edge.cursor`) +- Use `String` for pagination arguments (`before`, `after`) + +{: .alert-note} +**Note**{: .alert-title} +The `Cursor` scalar and `String` are semantically identical on the wire—both are opaque base64-encoded strings. The +only difference is that `Cursor` provides more expressive type information in the GraphQL schema. Using `String` for +cursor fields is fully compatible with the Relay specification and is the common convention in most GraphQL implementations. + ## Customization Hooks The schema definition API exposes hooks that let you customize generated types and fields. These hooks are commonly used diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/datastore_query/paginator.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/datastore_query/paginator.rb index 2281fc13d..ca813a3ca 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/datastore_query/paginator.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/datastore_query/paginator.rb @@ -8,6 +8,7 @@ require "elastic_graph/errors" require "elastic_graph/support/memoizable_data" +require "graphql" module ElasticGraph class GraphQL @@ -65,6 +66,18 @@ class Paginator < Support::MemoizableData.define(:default_page_size, :max_page_s # These methods are provided by `Data.define`: # @dynamic default_page_size, max_page_size, first, after, last, before, schema_element_names, initialize + # @return [DecodedCursor, nil] the decoded after cursor + def decoded_after + return @decoded_after if defined?(@decoded_after) + @decoded_after = decode_cursor(after) + end + + # @return [DecodedCursor, nil] the decoded before cursor + def decoded_before + return @decoded_before if defined?(@decoded_before) + @decoded_before = decode_cursor(before) + end + def requested_page_size # `+ 1` so we can tell if there are more docs for `has_next_page`/`has_previous_page` # ...but only if we need to get anything at all. @@ -86,8 +99,9 @@ def search_in_reverse? end # The cursor values to search after (if we need to search after one at all). + # Returns the decoded cursor when available, since callers need access to the decoded structure. def search_after - search_in_reverse? ? before : after + search_in_reverse? ? decoded_before : decoded_after end # In some cases, we're forced to search in reverse; in those caes, this is used to restore @@ -109,13 +123,13 @@ def truncate_items(items) # We can't always use `before` and `after` in the datastore query (such as when both are provided!), # so here we drop items from the start that come on or before `after`, and items from the # end that come on or after `before`. - if (after_cursor = after) + if (after_cursor = decoded_after) items = items.drop_while do |doc| item_sort_values_satisfy?(yield(doc, after_cursor), :<=) end end - if (before_cursor = before) + if (before_cursor = decoded_before) items = items.take_while do |doc| item_sort_values_satisfy?(yield(doc, before_cursor), :<) end @@ -128,7 +142,7 @@ def truncate_items(items) end def paginated_from_singleton_cursor? - before == DecodedCursor::SINGLETON || after == DecodedCursor::SINGLETON + decoded_before == DecodedCursor::SINGLETON || decoded_after == DecodedCursor::SINGLETON end def desired_page_size @@ -139,6 +153,13 @@ def desired_page_size private + def decode_cursor(cursor) + return nil if cursor.nil? + DecodedCursor.decode!(cursor) + rescue Errors::InvalidCursorError => e + raise ::GraphQL::ExecutionError, e.message + end + def first_n @first_n ||= size_arg_value(:first, first) end diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/scalar_coercion_adapters/cursor.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/scalar_coercion_adapters/cursor.rb index 62a5ccaf0..df82fe78e 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/scalar_coercion_adapters/cursor.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/scalar_coercion_adapters/cursor.rb @@ -6,28 +6,20 @@ # # frozen_string_literal: true -require "elastic_graph/graphql/decoded_cursor" - module ElasticGraph class GraphQL module ScalarCoercionAdapters + # Coercion adapter for the Cursor scalar type. + # Validates that cursor values are strings. When given a non-string value, returns nil + # to trigger GraphQL-Ruby's validation error with full field context. class Cursor def self.coerce_input(value, ctx) - case value - when DecodedCursor - value - when ::String - DecodedCursor.try_decode(value) - end + return value if value.nil? || value.is_a?(::String) + nil # Returning nil causes GraphQL-Ruby to generate a validation error end def self.coerce_result(value, ctx) - case value - when DecodedCursor - value.encode - when ::String - value if DecodedCursor.try_decode(value) - end + value end end end diff --git a/elasticgraph-graphql/sig/elastic_graph/graphql/datastore_query/paginator.rbs b/elasticgraph-graphql/sig/elastic_graph/graphql/datastore_query/paginator.rbs index c42566aa9..5a3c7a0b3 100644 --- a/elasticgraph-graphql/sig/elastic_graph/graphql/datastore_query/paginator.rbs +++ b/elasticgraph-graphql/sig/elastic_graph/graphql/datastore_query/paginator.rbs @@ -10,20 +10,26 @@ module ElasticGraph attr_reader default_page_size: ::Integer attr_reader max_page_size: ::Integer attr_reader first: ::Integer? - attr_reader after: DecodedCursor? + attr_reader after: ::String? attr_reader last: ::Integer? - attr_reader before: DecodedCursor? + attr_reader before: ::String? attr_reader schema_element_names: SchemaArtifacts::RuntimeMetadata::SchemaElementNames def initialize: ( default_page_size: ::Integer, max_page_size: ::Integer, first: ::Integer?, - after: DecodedCursor?, + after: ::String?, last: ::Integer?, - before: DecodedCursor?, + before: ::String?, schema_element_names: SchemaArtifacts::RuntimeMetadata::SchemaElementNames) -> void + @decoded_after: DecodedCursor? + def decoded_after: () -> DecodedCursor? + + @decoded_before: DecodedCursor? + def decoded_before: () -> DecodedCursor? + def requested_page_size: () -> ::Integer def search_in_reverse?: () -> boolish def search_after: () -> DecodedCursor? @@ -36,6 +42,8 @@ module ElasticGraph private + def decode_cursor: (::String?) -> DecodedCursor? + @first_n: ::Integer? def first_n: () -> ::Integer? diff --git a/elasticgraph-graphql/sig/elastic_graph/graphql/scalar_coercion_adapters/cursor.rbs b/elasticgraph-graphql/sig/elastic_graph/graphql/scalar_coercion_adapters/cursor.rbs index 3b613567f..e212bd14b 100644 --- a/elasticgraph-graphql/sig/elastic_graph/graphql/scalar_coercion_adapters/cursor.rbs +++ b/elasticgraph-graphql/sig/elastic_graph/graphql/scalar_coercion_adapters/cursor.rbs @@ -2,7 +2,7 @@ module ElasticGraph class GraphQL module ScalarCoercionAdapters class Cursor - extend SchemaArtifacts::_ScalarCoercionAdapter[DecodedCursor, ::String] + extend SchemaArtifacts::_ScalarCoercionAdapter[::String, ::String] end end end diff --git a/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb b/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb index 0f7b88761..d3cf356d5 100644 --- a/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb +++ b/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb @@ -1404,16 +1404,21 @@ def forward_paginate_through_workspace_id_groupings {"count" => 2, grouped_by => {case_correctly("workspace_id") => "w2"}} ] + # Both contexts (Cursor scalar and String override) now produce consistent error messages + # because the coercion adapter returns nil for invalid values, causing GraphQL to generate + # validation errors with full field context. + array_error = "Argument 'after' on Field '#{case_correctly("widget_aggregations")}' has an invalid value ([1, 2, 3]). Expected type '#{cursor_type}'." + expect { response = list_widget_workspace_id_groupings(first: 2, after: [1, 2, 3], expect_errors: true) - expect(response["errors"]).to contain_exactly(a_hash_including("message" => "Argument 'after' on Field '#{case_correctly("widget_aggregations")}' has an invalid value ([1, 2, 3]). Expected type 'Cursor'.")) - }.to log_warning a_string_including("Argument 'after' on Field '#{case_correctly("widget_aggregations")}' has an invalid value", "[1, 2, 3]") + expect(response["errors"]).to contain_exactly(a_hash_including("message" => array_error)) + }.to log_warning a_string_including(array_error) broken_cursor = page_info.fetch(case_correctly("end_cursor")) + "-broken" expect { response = list_widget_workspace_id_groupings(first: 2, after: broken_cursor, expect_errors: true) - expect(response["errors"]).to contain_exactly(a_hash_including("message" => "Argument 'after' on Field '#{case_correctly("widget_aggregations")}' has an invalid value (#{broken_cursor.inspect}). Expected type 'Cursor'.")) - }.to log_warning a_string_including("Argument 'after' on Field '#{case_correctly("widget_aggregations")}' has an invalid value", broken_cursor) + expect(response["errors"]).to contain_exactly(a_hash_including("message" => "`#{broken_cursor}` is an invalid cursor.")) + }.to log_warning a_string_including("`#{broken_cursor}` is an invalid cursor.") page_info, workspace_nodes = list_widget_workspace_id_groupings(first: 2, after: page_info.fetch(case_correctly("end_cursor"))) diff --git a/elasticgraph-graphql/spec/acceptance/elasticgraph_graphql_acceptance_support.rb b/elasticgraph-graphql/spec/acceptance/elasticgraph_graphql_acceptance_support.rb index 9956e95e9..c384a1dd0 100644 --- a/elasticgraph-graphql/spec/acceptance/elasticgraph_graphql_acceptance_support.rb +++ b/elasticgraph-graphql/spec/acceptance/elasticgraph_graphql_acceptance_support.rb @@ -60,7 +60,7 @@ def self.with_both_casing_forms(&block) module_exec(&block) end - context "with a camelCase schema, alternate derived type naming, and enum value overrides" do + context "with a camelCase schema, `String` cursors, alternate derived type naming, and enum value overrides" do include CamelCaseGraphQLAcceptanceAdapter # Need to use a local variable instead of an instance variable for the context state, @@ -90,6 +90,7 @@ def self.with_both_casing_forms(&block) datastore_backend: datastore_backend, schema_element_name_form: :camelCase, derived_type_name_formats: derived_type_name_formats, + type_name_overrides: {Cursor: "String"}, enum_value_overrides_by_type: enum_value_overrides_by_type, schema_definition: ->(schema) do # standard:disable Security/Eval -- it's ok here in a test. @@ -164,6 +165,12 @@ def apply_derived_type_customizations(type_name) type_name end + # Returns the cursor type name used in this schema configuration. + # In snake_case schemas, we use the default Cursor scalar. + def cursor_type + "Cursor" + end + # For parity with our `camelCase` context, also roundtrip factory-built records through JSON. # Otherwise we can have subtle, surprising differences between the two casing contexts. For # example, if the factory puts a `Date` object in a record, the JSON roundtripping will convert @@ -184,6 +191,12 @@ def enum_value(value) end end + # Returns the cursor type name used in this schema configuration. + # In camelCase schemas, we override Cursor to String for testing. + def cursor_type + "String" + end + def configure_for_camel_case(config) # Provide the same index definition settings, but for the `_camel` indices. original_index_defs = config.index_definitions diff --git a/elasticgraph-graphql/spec/acceptance/hidden_types_spec.rb b/elasticgraph-graphql/spec/acceptance/hidden_types_spec.rb index e25c611df..b61dd8667 100644 --- a/elasticgraph-graphql/spec/acceptance/hidden_types_spec.rb +++ b/elasticgraph-graphql/spec/acceptance/hidden_types_spec.rb @@ -121,7 +121,7 @@ module ElasticGraph FloatAggregatedValues IntAggregatedValues JsonSafeLongAggregatedValues LongStringAggregatedValues NonNumericAggregatedValues DateAggregatedValues DateTimeAggregatedValues LocalTimeAggregatedValues Company OnlineStore DirectWholesaler BrokerWholesaler - Cursor PageInfo Query TextFilterInput GeoLocation + PageInfo Query TextFilterInput GeoLocation DateTimeGroupingOffsetInput DateTimeUnitInput DateTimeTimeOfDayFilterInput DateGroupedBy DateGroupingOffsetInput DateGroupingTruncationUnitInput DateUnitInput DateTimeGroupedBy DateTimeGroupingTruncationUnitInput TimeZone @@ -130,7 +130,10 @@ module ElasticGraph LocalTimeGroupingOffsetInput LocalTimeGroupingTruncationUnitInput LocalTimeUnitInput MatchesQueryFilterInput MatchesPhraseFilterInput MatchesQueryWithPrefixFilterInput MatchesQueryAllowedEditsPerTermInput StringContainsFilterInput StringStartsWithFilterInput SearchHighlight - ] + ] + + # Cursor is conditionally included because when it's overridden to a built-in type like String + # (as in the camelCase test context), the Cursor scalar is not registered in the schema. + (all_fields_by_type_name.key?("Cursor") ? ["Cursor"] : []) # The sub-aggregation types are quite complicated and we just add them all here. expected_types_present_on_both_schemas += %w[ diff --git a/elasticgraph-graphql/spec/acceptance/nested_relationships_spec.rb b/elasticgraph-graphql/spec/acceptance/nested_relationships_spec.rb index e158c4c95..e13ca2109 100644 --- a/elasticgraph-graphql/spec/acceptance/nested_relationships_spec.rb +++ b/elasticgraph-graphql/spec/acceptance/nested_relationships_spec.rb @@ -307,14 +307,19 @@ module ElasticGraph }.to log_warning a_string_including("`first` cannot be negative, but is -2.") # Demonstrate how broken cursors behave. + # Both contexts (Cursor scalar and String override) now produce consistent error messages + # because the coercion adapter returns nil for invalid values, causing GraphQL to generate + # validation errors with full field context. + array_error = "Argument 'after' on Field 'components' has an invalid value ([1, 2, 3]). Expected type '#{cursor_type}'." + expect { response = query_widgets_and_components_including_page_info( widget_args: {first: 1, order_by: [:amount_cents_ASC]}, component_args: {first: 1, after: [1, 2, 3], order_by: [:name_ASC]}, expect_errors: true ) - expect(response["errors"]).to contain_exactly(a_hash_including("message" => "Argument 'after' on Field 'components' has an invalid value ([1, 2, 3]). Expected type 'Cursor'.")) - }.to log_warning a_string_including("Argument 'after' on Field 'components' has an invalid value", "[1, 2, 3]") + expect(response["errors"]).to contain_exactly(a_hash_including("message" => array_error)) + }.to log_warning a_string_including(array_error) broken_cursor = results["edges"][0]["node"]["components"][case_correctly "page_info"][case_correctly "end_cursor"] + "-broken" expect { @@ -323,8 +328,8 @@ module ElasticGraph component_args: {first: 1, after: broken_cursor, order_by: [:name_ASC]}, expect_errors: true ) - expect(response["errors"]).to contain_exactly(a_hash_including("message" => "Argument 'after' on Field 'components' has an invalid value (#{broken_cursor.inspect}). Expected type 'Cursor'.")) - }.to log_warning a_string_including("Argument 'after' on Field 'components' has an invalid value", broken_cursor) + expect(response["errors"]).to contain_exactly(a_hash_including("message" => "`#{broken_cursor}` is an invalid cursor.")) + }.to log_warning a_string_including("`#{broken_cursor}` is an invalid cursor.") # get next page of components (but still on the first page of widgets) results = query_widgets_and_components_including_page_info( diff --git a/elasticgraph-graphql/spec/integration/elastic_graph/graphql/datastore_query/aggregation_pagination_spec.rb b/elasticgraph-graphql/spec/integration/elastic_graph/graphql/datastore_query/aggregation_pagination_spec.rb index 5faaa2039..dca5bb1ff 100644 --- a/elasticgraph-graphql/spec/integration/elastic_graph/graphql/datastore_query/aggregation_pagination_spec.rb +++ b/elasticgraph-graphql/spec/integration/elastic_graph/graphql/datastore_query/aggregation_pagination_spec.rb @@ -107,6 +107,10 @@ def index_doc_with_null_value end def paginated_search(first: nil, after: nil, last: nil, before: nil, groupings: [field_term_grouping_of("name")], computations: [], filter_to: nil) + # Encode cursors (all cursors in aggregation tests are DecodedCursor objects) + after = after&.encode + before = before&.encode + aggregation_query = aggregation_query_of( groupings: groupings, computations: computations, first: first, after: after, last: last, before: before, diff --git a/elasticgraph-graphql/spec/integration/elastic_graph/graphql/datastore_query/pagination_spec.rb b/elasticgraph-graphql/spec/integration/elastic_graph/graphql/datastore_query/pagination_spec.rb index e7f4f4d66..234108d1f 100644 --- a/elasticgraph-graphql/spec/integration/elastic_graph/graphql/datastore_query/pagination_spec.rb +++ b/elasticgraph-graphql/spec/integration/elastic_graph/graphql/datastore_query/pagination_spec.rb @@ -173,6 +173,10 @@ def index_doc_with_null_value end def paginated_search(first: nil, after: nil, last: nil, before: nil, document_pagination: nil, sort: sort_list, filter_to: nil) + # Encode cursors (all cursors in document pagination tests are DecodedCursor objects) + after = after&.encode + before = before&.encode + document_pagination ||= {first: first, after: after, last: last, before: before}.compact query = nil response = search_datastore( diff --git a/elasticgraph-graphql/spec/support/scalar_coercion_adapter.rb b/elasticgraph-graphql/spec/support/scalar_coercion_adapter.rb index f3a0e8c7f..1261290b5 100644 --- a/elasticgraph-graphql/spec/support/scalar_coercion_adapter.rb +++ b/elasticgraph-graphql/spec/support/scalar_coercion_adapter.rb @@ -66,20 +66,18 @@ def execute_query_returning(value) @graphql.graphql_query_executor.execute(@query).to_h end - def expect_input_value_to_be_accepted(value, as: value, only_test_variable: false) + def expect_input_value_to_be_accepted(value, as: value) response = execute_query_with_variable_value(value) expect(response).not_to include("errors") expect(response).to eq({"data" => {"echo" => nil}}) expect(@test_resolver.last_arg_value).to eq(as) - unless only_test_variable - response = execute_query_with_inline_query_value(value) + response = execute_query_with_inline_query_value(value) - expect(response).not_to include("errors") - expect(response).to eq({"data" => {"echo" => nil}}) - expect(@test_resolver.last_arg_value).to eq(as) - end + expect(response).not_to include("errors") + expect(response).to eq({"data" => {"echo" => nil}}) + expect(@test_resolver.last_arg_value).to eq(as) end # Use `define_method` instead of `def` to have access to `scalar_type_name` diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/scalar_coercion_adapters/cursor_spec.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/scalar_coercion_adapters/cursor_spec.rb index b6f9382db..d52ef7512 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/scalar_coercion_adapters/cursor_spec.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/scalar_coercion_adapters/cursor_spec.rb @@ -15,80 +15,31 @@ module ScalarCoercionAdapters include_context "scalar coercion adapter support", "Cursor" context "input coercion" do - it "accepts a properly encoded string cursor" do - cursor = DecodedCursor.new({"a" => 1, "b" => "foo"}) - expect_input_value_to_be_accepted(cursor.encode, as: cursor) + it "accepts string values" do + expect_input_value_to_be_accepted("abc123") end - it "accepts an already decoded cursor" do - cursor = DecodedCursor.new({"a" => 1, "b" => "foo"}) - expect_input_value_to_be_accepted(cursor, only_test_variable: true) - end - - it "accepts the special singleton cursor string value" do - expect_input_value_to_be_accepted(DecodedCursor::SINGLETON.encode, as: DecodedCursor::SINGLETON) - end - - it "accepts the special singleton cursor value" do - expect_input_value_to_be_accepted(DecodedCursor::SINGLETON, only_test_variable: true) - end - - it "accepts a `nil` value as-is" do + it "accepts nil" do expect_input_value_to_be_accepted(nil) end - it "rejects values that are not strings" do - expect_input_value_to_be_rejected(3) - expect_input_value_to_be_rejected(3.7) - expect_input_value_to_be_rejected(false) + it "rejects non-string values" do + expect_input_value_to_be_rejected(123) expect_input_value_to_be_rejected([1, 2, 3]) - expect_input_value_to_be_rejected(["a", "b"]) - expect_input_value_to_be_rejected({"a" => 1, "b" => "foo"}) - end - - it "rejects broken string cursors" do - cursor = DecodedCursor.new({"a" => 1, "b" => "foo"}).encode - expect_input_value_to_be_rejected(cursor + "-broken") + expect_input_value_to_be_rejected({"key" => "value"}) + expect_input_value_to_be_rejected(true) + expect_input_value_to_be_rejected(false) end end context "result coercion" do - it "returns the encoded form of a decoded string cursor" do - cursor = DecodedCursor.new({"a" => 1, "b" => "foo"}) - expect_result_to_be_returned(cursor, as: cursor.encode) + it "returns string values as-is" do + expect_result_to_be_returned("abc123", as: "abc123") end - it "returns a properly encoded cursor as-is" do - cursor = DecodedCursor.new({"a" => 1, "b" => "foo"}) - expect_result_to_be_returned(cursor.encode, as: cursor.encode) - end - - it "returns the encoded form of the special singleton cursor" do - cursor = DecodedCursor::SINGLETON - expect_result_to_be_returned(cursor, as: cursor.encode) - end - - it "returns the encoded form of the special singleton cursor as-is when given in its string form" do - cursor = DecodedCursor::SINGLETON - expect_result_to_be_returned(cursor.encode, as: cursor.encode) - end - - it "returns `nil` as is" do + it "returns nil as-is" do expect_result_to_be_returned(nil) end - - it "returns `nil` for non-string values" do - expect_result_to_be_replaced_with_nil(3) - expect_result_to_be_replaced_with_nil(3.7) - expect_result_to_be_replaced_with_nil(false) - expect_result_to_be_replaced_with_nil([1, 2, 3]) - expect_result_to_be_replaced_with_nil(["a", "b"]) - expect_result_to_be_replaced_with_nil({"a" => 1, "b" => "foo"}) - end - - it "returns `nil` for strings that are not properly encoded cursors" do - expect_result_to_be_replaced_with_nil("not a cursor") - end end end end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb index df513de53..7aee7630d 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb @@ -161,6 +161,10 @@ module SchemaElements # @!attribute [rw] names # @private class BuiltInTypes + # Non-string standard GraphQL scalars that cannot be used for cursor overrides. + # String and ID are valid, as are any custom scalar names (e.g., PaginationCursor). + INVALID_CURSOR_TYPE_OVERRIDES = (STOCK_GRAPHQL_SCALARS - %w[ID String]).freeze + attr_reader :schema_def_api, :schema_def_state, :names # @private @@ -184,6 +188,18 @@ def register_built_in_types private + def validate_cursor_type_override! + override_name = @schema_def_state.type_namer.cursor_type_name + return if override_name == "Cursor" + + if INVALID_CURSOR_TYPE_OVERRIDES.include?(override_name) + raise Errors::SchemaError, <<~ERROR + Invalid cursor type override: `Cursor` was overridden to `#{override_name}`, but cursor types must be string-compatible. + Valid options include: String, ID, or any custom scalar name (e.g., PaginationCursor). + ERROR + end + end + def register_directives # Note: The `eg` prefix is being used based on a GraphQL Spec recommendation: # http://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives @@ -753,21 +769,30 @@ def register_standard_graphql_scalars end def register_custom_elastic_graph_scalars - schema_def_api.scalar_type "Cursor" do |t| - # Technically, we don't use the mapping or json_schema on this type since it's a return-only - # type and isn't indexed. However, `scalar_type` requires them to be set (since custom scalars - # defined by users will need those set) so we set them here to what they would be if we actually - # used them. - t.mapping type: "keyword" - t.json_schema type: "string" - t.coerce_with "ElasticGraph::GraphQL::ScalarCoercionAdapters::Cursor", - defined_at: "elastic_graph/graphql/scalar_coercion_adapters/cursor" + # Validate that Cursor type override is compatible (must be string-like) + validate_cursor_type_override! + + # Register the cursor scalar unless it's overridden to a built-in type. + # When overridden to a built-in type like String or ID, we use that type directly. + # When overridden to a custom scalar (e.g., PaginationCursor), we register it automatically. + cursor_type_name = @schema_def_state.type_namer.cursor_type_name + unless STOCK_GRAPHQL_SCALARS.include?(cursor_type_name) + schema_def_api.scalar_type "Cursor" do |t| + # Technically, we don't use the mapping or json_schema on this type since it's a return-only + # type and isn't indexed. However, `scalar_type` requires them to be set (since custom scalars + # defined by users will need those set) so we set them here to what they would be if we actually + # used them. + t.mapping type: "keyword" + t.json_schema type: "string" + t.coerce_with "ElasticGraph::GraphQL::ScalarCoercionAdapters::Cursor", + defined_at: "elastic_graph/graphql/scalar_coercion_adapters/cursor" - t.documentation <<~EOS - An opaque string value representing a specific location in a paginated connection type. - Returned cursors can be passed back in the next query via the `before` or `after` - arguments to continue paginating from that point. - EOS + t.documentation <<~EOS + An opaque string value representing a specific location in a paginated connection type. + Returned cursors can be passed back in the next query via the `before` or `after` + arguments to continue paginating from that point. + EOS + end end schema_def_api.scalar_type "Date" do |t| diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/type_namer.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/type_namer.rb index 462bd378e..c2f3a3839 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/type_namer.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/type_namer.rb @@ -58,6 +58,12 @@ def revert_override_for(potentially_overriden_name) reverse_overrides.fetch(potentially_overriden_name, potentially_overriden_name) end + # Returns the type name to use for cursor fields, respecting any type_name_overrides. + # @return [String] the cursor type name (e.g., "Cursor" or "String") + def cursor_type_name + name_for("Cursor") + end + # Generates a derived type name based on the provided format name and arguments. The given arguments must match # the placeholders in the format. If the format name is unknown or the arguments are invalid, a `Errors::ConfigError` is raised. # diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/type_namer.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/type_namer.rbs index d539d8533..6c8b473e0 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/type_namer.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/type_namer.rbs @@ -25,6 +25,7 @@ module ElasticGraph def name_for: (::Symbol | ::String) -> ::String def revert_override_for: (::String) -> ::String + def cursor_type_name: () -> ::String def generate_name_for: (::Symbol, **::String) -> ::String def extract_base_from: (::String, format: ::Symbol) -> ::String? def matches_format?: (::String, ::Symbol) -> bool diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb index e25e95cee..01637cad6 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb @@ -75,6 +75,82 @@ module SchemaDefinition EOS end + %w[ID String].each do |cursor_override| + it "allows overriding `Cursor` to `#{cursor_override}` for federation compatibility" do + result = define_schema(type_name_overrides: {Cursor: cursor_override}) do |api| + api.object_type "Widget" do |t| + t.field "id", "ID!" + t.index "widgets" + end + end + + # The Cursor scalar should not be registered when overridden to a built-in type + expect(type_def_from(result, "Cursor")).to be_nil + + # PageInfo fields should use the override instead of Cursor + expect(type_def_from(result, "PageInfo")).to eq(<<~EOS.strip) + type PageInfo { + #{schema_elements.has_next_page}: Boolean! + #{schema_elements.has_previous_page}: Boolean! + #{schema_elements.start_cursor}: #{cursor_override} + #{schema_elements.end_cursor}: #{cursor_override} + } + EOS + + # Edge.cursor field should use the override + expect(type_def_from(result, "WidgetEdge")).to include("#{schema_elements.cursor}: #{cursor_override}") + + # Pagination arguments should use the override + query_type = type_def_from(result, "Query") + expect(query_type).to include("after: #{cursor_override}") + expect(query_type).to include("before: #{cursor_override}") + end + end + + %w[Boolean Float Int].each do |invalid_cursor_override| + it "rejects overriding `Cursor` to a non-string-compatible type like #{invalid_cursor_override}" do + expect { + define_schema(type_name_overrides: {Cursor: invalid_cursor_override}) + }.to raise_error(Errors::SchemaError, a_string_including("cursor types must be string-compatible")) + end + end + + it "allows overriding `Cursor` to a custom scalar like `PaginationCursor`" do + result = define_schema(type_name_overrides: {Cursor: "PaginationCursor"}) do |api| + # Custom cursor scalars are automatically registered by built_in_types.rb + # when type_name_overrides is used, so no manual definition is needed. + + api.object_type "Widget" do |t| + t.field "id", "ID!" + t.index "widgets" + end + end + + # The standard Cursor scalar should not be registered when overridden + expect(type_def_from(result, "Cursor")).to be_nil + + # PaginationCursor should be auto-registered + expect(type_def_from(result, "PaginationCursor")).to eq("scalar PaginationCursor") + + # PageInfo fields should use the custom scalar + expect(type_def_from(result, "PageInfo")).to eq(<<~EOS.strip) + type PageInfo { + #{schema_elements.has_next_page}: Boolean! + #{schema_elements.has_previous_page}: Boolean! + #{schema_elements.start_cursor}: PaginationCursor + #{schema_elements.end_cursor}: PaginationCursor + } + EOS + + # Edge.cursor field should use the custom scalar + expect(type_def_from(result, "WidgetEdge")).to include("#{schema_elements.cursor}: PaginationCursor") + + # Pagination arguments should use the custom scalar + query_type = type_def_from(result, "Query") + expect(query_type).to include("after: PaginationCursor") + expect(query_type).to include("before: PaginationCursor") + end + it "defines a `GeoLocation` object type and related filter types" do expect(type_named("GeoLocation", include_docs: true)).to eq(<<~EOS.strip) """ diff --git a/elasticgraph-warehouse/lib/elastic_graph/warehouse/schema_definition/api_extension.rb b/elasticgraph-warehouse/lib/elastic_graph/warehouse/schema_definition/api_extension.rb index 26cc0cb0e..d5db4d36a 100644 --- a/elasticgraph-warehouse/lib/elastic_graph/warehouse/schema_definition/api_extension.rb +++ b/elasticgraph-warehouse/lib/elastic_graph/warehouse/schema_definition/api_extension.rb @@ -60,7 +60,7 @@ def self.extended(api) api.on_built_in_types do |type| case type when ScalarTypeExtension - type.warehouse_column type: COLUMN_TYPES_BY_BUILT_IN_SCALAR_TYPE.fetch(type.name) + type.warehouse_column type: COLUMN_TYPES_BY_BUILT_IN_SCALAR_TYPE.fetch(type.type_ref.with_reverted_override.name) end end end diff --git a/elasticgraph-warehouse/spec/unit/elastic_graph/warehouse/schema_definition/scalar_type_extension_spec.rb b/elasticgraph-warehouse/spec/unit/elastic_graph/warehouse/schema_definition/scalar_type_extension_spec.rb index b513b4480..8839a646a 100644 --- a/elasticgraph-warehouse/spec/unit/elastic_graph/warehouse/schema_definition/scalar_type_extension_spec.rb +++ b/elasticgraph-warehouse/spec/unit/elastic_graph/warehouse/schema_definition/scalar_type_extension_spec.rb @@ -71,6 +71,32 @@ module SchemaDefinition "call `warehouse_column type:" )) end + + it "respects type name overrides for all built in types (except stock GraphQL scalars that can't be renamed)" do + overrides = APIExtension::COLUMN_TYPES_BY_BUILT_IN_SCALAR_TYPE.except(*STOCK_GRAPHQL_SCALARS).keys.to_h do |original_type_name| + [original_type_name.to_sym, "Overridden#{original_type_name}"] + end + + results = define_warehouse_schema(type_name_overrides: overrides) do |s| + # Define a custom type that includes a PaginationCursor field so we can verify + # the warehouse column type is configured correctly. + s.object_type "CursorTest" do |t| + t.field "id", "ID" + + overrides.each do |original_type, overridden_type| + t.field "overridden_#{original_type}", overridden_type + end + + t.index "override_tests" + end + end + + overrides.each do |original_type, overridden_type| + field_name = "overridden_#{original_type}" + expected_column_type = APIExtension::COLUMN_TYPES_BY_BUILT_IN_SCALAR_TYPE.fetch(original_type.to_s) + expect(warehouse_column_def_from(results, "override_tests", field_name)).to eq "#{field_name} #{expected_column_type}" + end + end end end end diff --git a/spec_support/lib/elastic_graph/spec_support/builds_datastore_core.rb b/spec_support/lib/elastic_graph/spec_support/builds_datastore_core.rb index bca599c27..df5e50e8b 100644 --- a/spec_support/lib/elastic_graph/spec_support/builds_datastore_core.rb +++ b/spec_support/lib/elastic_graph/spec_support/builds_datastore_core.rb @@ -24,6 +24,7 @@ def build_datastore_core( schema_element_name_form: :snake_case, schema_element_name_overrides: {}, derived_type_name_formats: {}, + type_name_overrides: {}, enum_value_overrides_by_type: {}, index_definitions: nil, clusters: nil, @@ -58,6 +59,7 @@ def build_datastore_core( schema_element_name_form: schema_element_name_form, schema_element_name_overrides: schema_element_name_overrides, derived_type_name_formats: derived_type_name_formats, + type_name_overrides: type_name_overrides, enum_value_overrides_by_type: enum_value_overrides_by_type, reload_schema_artifacts: reload_schema_artifacts, &schema_definition diff --git a/spec_support/spec_helper.rb b/spec_support/spec_helper.rb index 26c0b7227..e62430bd5 100644 --- a/spec_support/spec_helper.rb +++ b/spec_support/spec_helper.rb @@ -360,6 +360,7 @@ def generate_schema_artifacts( schema_element_name_form: :snake_case, schema_element_name_overrides: {}, derived_type_name_formats: {}, + type_name_overrides: {}, enum_value_overrides_by_type: {}, reload_schema_artifacts: false ) @@ -371,6 +372,7 @@ def generate_schema_artifacts( schema_element_name_form: schema_element_name_form, schema_element_name_overrides: schema_element_name_overrides, derived_type_name_formats: derived_type_name_formats, + type_name_overrides: type_name_overrides, enum_value_overrides_by_type: enum_value_overrides_by_type, reload_schema_artifacts: reload_schema_artifacts, output: output