From 993dd7cb00c79268576e06a24fb68a18e559b84e Mon Sep 17 00:00:00 2001 From: Abdullah Yousufi Date: Sat, 28 Feb 2026 17:37:46 -0800 Subject: [PATCH 1/3] Resolve elasticgraph-apollo confusing rake warnings --- .../elastic_graph/schema_definition/results.rb | 3 ++- .../graphql_resolvers_by_name_spec.rb | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb index 0bd9ad604..bedfdd211 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb @@ -383,7 +383,8 @@ def verify_runtime_metadata(runtime_metadata) unused_resolvers = registered_resolvers.except(*fields_by_resolvers.keys).reject do |name, res| # Ignore our built-in resolvers. - res.resolver_ref.fetch("require_path").start_with?("elastic_graph/graphql/resolvers/") + require_path = res.resolver_ref.fetch("require_path") + require_path.start_with?("elastic_graph/graphql/resolvers/", "elastic_graph/apollo/graphql/") end.keys unless unused_resolvers.empty? diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb index 6dfe4cbb1..522a7ac51 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb @@ -148,6 +148,22 @@ module SchemaDefinition EOS end + it "does not warn when Apollo's optional resolvers are registered but never used" do + output = StringIO.new + + graphql_resolvers_by_name(output: output) do |schema| + schema.register_graphql_resolver :apollo_entity_ref, + GraphQLResolverWithoutLookahead, + defined_at: "elastic_graph/apollo/graphql/apollo_entity_ref_resolver" + + schema.register_graphql_resolver :apollo_entities, + GraphQLResolverWithoutLookahead, + defined_at: "elastic_graph/apollo/graphql/entities_field_resolver" + end + + expect(output.string).to eq("") + end + def graphql_resolvers_by_name(...) define_schema(...).runtime_metadata.graphql_resolvers_by_name end From f7337d51f254cb0916c6ffb3f858e4f8b8e30ed1 Mon Sep 17 00:00:00 2001 From: Abdullah Yousufi Date: Sun, 1 Mar 2026 23:45:18 -0800 Subject: [PATCH 2/3] Support built_in resolver param --- .../apollo/schema_definition/api_extension.rb | 10 +++++----- .../apollo/schema_definition_spec.rb | 4 ++++ .../runtime_metadata/graphql_resolver.rb | 7 +++++-- .../runtime_metadata/graphql_resolver.rbs | 12 ++++++++---- .../runtime_metadata/graphql_resolver_spec.rb | 12 ++++++++---- .../lib/elastic_graph/schema_definition/api.rb | 7 +++++-- .../elastic_graph/schema_definition/results.rb | 6 +++--- .../schema_elements/built_in_types.rb | 15 ++++++++++----- .../sig/elastic_graph/schema_definition/api.rbs | 2 +- .../graphql_resolvers_by_name_spec.rb | 14 ++++++-------- .../spec_support/runtime_metadata_support.rb | 5 +++-- 11 files changed, 58 insertions(+), 36 deletions(-) diff --git a/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/api_extension.rb b/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/api_extension.rb index 77f7b969d..e3b9ad687 100644 --- a/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/api_extension.rb +++ b/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/api_extension.rb @@ -389,15 +389,15 @@ def define_apollo_schema_elements register_graphql_extension GraphQL::EngineExtension, defined_at: require_path require(require_path = "elastic_graph/apollo/graphql/entities_field_resolver") - register_graphql_resolver :apollo_entities, GraphQL::EntitiesFieldResolver, defined_at: require_path + register_graphql_resolver :apollo_entities, GraphQL::EntitiesFieldResolver, defined_at: require_path, built_in: true require(require_path = "elastic_graph/apollo/graphql/service_field_resolver") - register_graphql_resolver :apollo_service, GraphQL::ServiceFieldResolver, defined_at: require_path + register_graphql_resolver :apollo_service, GraphQL::ServiceFieldResolver, defined_at: require_path, built_in: true require(require_path = "elastic_graph/apollo/graphql/apollo_entity_ref_resolver") - register_graphql_resolver :apollo_entity_ref, GraphQL::ApolloEntityRefResolver::ForSingleId, defined_at: require_path - register_graphql_resolver :apollo_entity_ref_list, GraphQL::ApolloEntityRefResolver::ForIdList, defined_at: require_path - register_graphql_resolver :apollo_entity_ref_paginated, GraphQL::ApolloEntityRefResolver::ForPaginatedList, defined_at: require_path + register_graphql_resolver :apollo_entity_ref, GraphQL::ApolloEntityRefResolver::ForSingleId, defined_at: require_path, built_in: true + register_graphql_resolver :apollo_entity_ref_list, GraphQL::ApolloEntityRefResolver::ForIdList, defined_at: require_path, built_in: true + register_graphql_resolver :apollo_entity_ref_paginated, GraphQL::ApolloEntityRefResolver::ForPaginatedList, defined_at: require_path, built_in: true end def apollo_object_type(name, &block) diff --git a/elasticgraph-apollo/spec/unit/elastic_graph/apollo/schema_definition_spec.rb b/elasticgraph-apollo/spec/unit/elastic_graph/apollo/schema_definition_spec.rb index dbd4f1ee6..b80a51b24 100644 --- a/elasticgraph-apollo/spec/unit/elastic_graph/apollo/schema_definition_spec.rb +++ b/elasticgraph-apollo/spec/unit/elastic_graph/apollo/schema_definition_spec.rb @@ -18,6 +18,10 @@ module Apollo include_context "SchemaDefinitionHelpers" + after do + expect(logged_output).to exclude(/warn/i) + end + def self.with_both_casing_forms(&block) context "with schema elements configured to use camelCase" do let(:schema_element_name_form) { :camelCase } diff --git a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rb b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rb index 559c84172..cd1d184d1 100644 --- a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rb +++ b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rb @@ -12,7 +12,7 @@ module ElasticGraph module SchemaArtifacts module RuntimeMetadata # @private - class GraphQLResolver < ::Data.define(:needs_lookahead, :resolver_ref) + class GraphQLResolver < ::Data.define(:needs_lookahead, :resolver_ref, :built_in) def self.with_lookahead_loader @with_lookahead_loader ||= ExtensionLoader.new(InterfaceWithLookahead) end @@ -21,6 +21,7 @@ def self.without_lookahead_loader @without_lookahead_loader ||= ExtensionLoader.new(InterfaceWithoutLookahead) end + BUILT_IN = "built_in" NEEDS_LOOKAHEAD = "needs_lookahead" RESOLVER_REF = "resolver_ref" @@ -32,13 +33,15 @@ def load_resolver def self.from_hash(hash) new( needs_lookahead: hash.fetch(NEEDS_LOOKAHEAD), - resolver_ref: hash.fetch(RESOLVER_REF) + resolver_ref: hash.fetch(RESOLVER_REF), + built_in: hash.fetch(BUILT_IN) { false } ) end def to_dumpable_hash { # Keys here are ordered alphabetically; please keep them that way. + BUILT_IN => built_in, NEEDS_LOOKAHEAD => needs_lookahead, RESOLVER_REF => resolver_ref } diff --git a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rbs b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rbs index 1447cd50b..c140c1303 100644 --- a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rbs +++ b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rbs @@ -4,20 +4,23 @@ module ElasticGraph class GraphQLResolverSupertype attr_reader needs_lookahead: bool attr_reader resolver_ref: ::Hash[::String, ::String] + attr_reader built_in: bool def initialize: ( needs_lookahead: bool, - resolver_ref: ::Hash[::String, ::String] + resolver_ref: ::Hash[::String, ::String], + built_in: bool ) -> void def with: ( ?needs_lookahead: bool, - ?resolver_ref: ::Hash[::String, ::String] + ?resolver_ref: ::Hash[::String, ::String], + ?built_in: bool ) -> instance def self.new: - (needs_lookahead: bool, resolver_ref: ::Hash[::String, ::String]) -> instance - | (bool, ::Hash[::String, ::String]) -> instance + (needs_lookahead: bool, resolver_ref: ::Hash[::String, ::String], built_in: bool) -> instance + | (bool, ::Hash[::String, ::String], bool) -> instance end class GraphQLResolver < GraphQLResolverSupertype @@ -27,6 +30,7 @@ module ElasticGraph self.@without_lookahead_loader: ExtensionLoader? def self.without_lookahead_loader: () -> ExtensionLoader + BUILT_IN: "built_in" NEEDS_LOOKAHEAD: "needs_lookahead" RESOLVER_REF: "resolver_ref" diff --git a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver_spec.rb b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver_spec.rb index 81096a857..c4780de0b 100644 --- a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver_spec.rb +++ b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver_spec.rb @@ -18,7 +18,8 @@ module RuntimeMetadata resolver_ref: { "name" => "ElasticGraph::GraphQLResolverWithLookahead", "require_path" => "elastic_graph/spec_support/example_extensions/graphql_resolvers" - } + }, + built_in: false ) expect(resolver.load_resolver.extension_class).to be GraphQLResolverWithLookahead @@ -30,7 +31,8 @@ module RuntimeMetadata resolver_ref: { "name" => "ElasticGraph::GraphQLResolverWithoutLookahead", "require_path" => "elastic_graph/spec_support/example_extensions/graphql_resolvers" - } + }, + built_in: false ) expect(resolver.load_resolver.extension_class).to be GraphQLResolverWithoutLookahead @@ -42,7 +44,8 @@ module RuntimeMetadata resolver_ref: { "name" => "ElasticGraph::GraphQLResolverWithoutLookahead", "require_path" => "elastic_graph/spec_support/example_extensions/graphql_resolvers" - } + }, + built_in: false ) expect { @@ -56,7 +59,8 @@ module RuntimeMetadata resolver_ref: { "name" => "ElasticGraph::GraphQLResolverWithLookahead", "require_path" => "elastic_graph/spec_support/example_extensions/graphql_resolvers" - } + }, + built_in: false ) expect { diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb index 9be4d045d..53e1fa53c 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb @@ -369,7 +369,9 @@ def register_graphql_extension(extension_module, defined_at:, **config) # end # end # end - def register_graphql_resolver(name, klass, defined_at:, **resolver_config) + # @param built_in [bool] Whether this resolver is built-in to ElasticGraph or one of its extensions. + # Built-in resolvers that are unused in a schema will not trigger a warning. + def register_graphql_resolver(name, klass, defined_at:, built_in: false, **resolver_config) extension = SchemaArtifacts::RuntimeMetadata::Extension.new(klass, defined_at, resolver_config) needs_lookahead = @@ -382,7 +384,8 @@ def register_graphql_resolver(name, klass, defined_at:, **resolver_config) resolver = SchemaArtifacts::RuntimeMetadata::GraphQLResolver.new( needs_lookahead: needs_lookahead, - resolver_ref: extension.to_dumpable_hash + resolver_ref: extension.to_dumpable_hash, + built_in: built_in ) @state.graphql_resolvers_by_name[name] = resolver diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb index bedfdd211..bb3cddde6 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb @@ -383,15 +383,15 @@ def verify_runtime_metadata(runtime_metadata) unused_resolvers = registered_resolvers.except(*fields_by_resolvers.keys).reject do |name, res| # Ignore our built-in resolvers. - require_path = res.resolver_ref.fetch("require_path") - require_path.start_with?("elastic_graph/graphql/resolvers/", "elastic_graph/apollo/graphql/") + res.built_in end.keys unless unused_resolvers.empty? state.output.puts <<~EOS WARNING: #{unused_resolvers.size} GraphQL resolver(s) have been registered but are unused: - #{unused_resolvers.sort.join("\n - ")} - These resolvers can be removed. + These resolvers can be removed. If you intended for them to be available as built-in/internal + resolvers, pass `built_in: true` when registering them. EOS 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 b040b37fc..99c6e3fb8 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 @@ -1425,25 +1425,30 @@ def register_standard_graphql_resolvers require(require_path = "elastic_graph/graphql/resolvers/get_record_field_value") schema_def_api.register_graphql_resolver :get_record_field_value, GraphQL::Resolvers::GetRecordFieldValue, - defined_at: require_path + defined_at: require_path, + built_in: true require(require_path = "elastic_graph/graphql/resolvers/list_records") schema_def_api.register_graphql_resolver :list_records, GraphQL::Resolvers::ListRecords, - defined_at: require_path + defined_at: require_path, + built_in: true require(require_path = "elastic_graph/graphql/resolvers/nested_relationships") schema_def_api.register_graphql_resolver :nested_relationships, GraphQL::Resolvers::NestedRelationships, - defined_at: require_path + defined_at: require_path, + built_in: true require(require_path = "elastic_graph/graphql/resolvers/object") schema_def_api.register_graphql_resolver :object_with_lookahead, GraphQL::Resolvers::Object::WithLookahead, - defined_at: require_path + defined_at: require_path, + built_in: true schema_def_api.register_graphql_resolver :object_without_lookahead, GraphQL::Resolvers::Object::WithoutLookahead, - defined_at: require_path + defined_at: require_path, + built_in: true end def define_date_grouping_arguments(grouping_field, omit_timezone: false) diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/api.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/api.rbs index 310ba70f9..604ed7842 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/api.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/api.rbs @@ -74,7 +74,7 @@ module ElasticGraph def results: () -> Results def json_schema_version: (::Integer) -> void def register_graphql_extension: (::Module, defined_at: ::String, **untyped) -> void - def register_graphql_resolver: (::Symbol, ::Class, defined_at: ::String, **untyped) -> void + def register_graphql_resolver: (::Symbol, ::Class, defined_at: ::String, ?built_in: bool, **untyped) -> void def on_built_in_types: () { (SchemaElements::graphQLType) -> void } -> void def on_root_query_type: () { (SchemaElements::ObjectType) -> void } -> void end diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb index 522a7ac51..9940d698b 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb @@ -144,21 +144,19 @@ module SchemaDefinition WARNING: 2 GraphQL resolver(s) have been registered but are unused: - resolver1 - resolver2 - These resolvers can be removed. + These resolvers can be removed. If you intended for them to be available as built-in/internal + resolvers, pass `built_in: true` when registering them. EOS end - it "does not warn when Apollo's optional resolvers are registered but never used" do + it "does not warn when a resolver is registered with `built_in: true` but never used" do output = StringIO.new graphql_resolvers_by_name(output: output) do |schema| - schema.register_graphql_resolver :apollo_entity_ref, - GraphQLResolverWithoutLookahead, - defined_at: "elastic_graph/apollo/graphql/apollo_entity_ref_resolver" - - schema.register_graphql_resolver :apollo_entities, + schema.register_graphql_resolver :resolver1, GraphQLResolverWithoutLookahead, - defined_at: "elastic_graph/apollo/graphql/entities_field_resolver" + defined_at: "some/path", + built_in: true end expect(output.string).to eq("") diff --git a/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb b/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb index 29316874b..9eac34f35 100644 --- a/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb +++ b/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb @@ -174,10 +174,11 @@ def configured_graphql_resolver(name, **config) "require_path" => "elastic_graph/graphql/resolvers/get_record_field_value" } - def graphql_resolver_with(needs_lookahead: false, resolver_ref: DEFAULT_RESOLVER_REF) + def graphql_resolver_with(needs_lookahead: false, resolver_ref: DEFAULT_RESOLVER_REF, built_in: false) GraphQLResolver.new( needs_lookahead: needs_lookahead, - resolver_ref: resolver_ref + resolver_ref: resolver_ref, + built_in: built_in ) end From ce8fd39e091a4fe73ceb83da35432df692bcbe65 Mon Sep 17 00:00:00 2001 From: Abdullah Yousufi Date: Thu, 19 Mar 2026 00:57:48 -0700 Subject: [PATCH 3/3] use built_in_graphql_resolvers instead of built_in in runtime metadata --- .../runtime_metadata/graphql_resolver.rb | 7 ++---- .../runtime_metadata/graphql_resolver.rbs | 12 ++++------ .../runtime_metadata/graphql_resolver_spec.rb | 12 ++++------ .../elastic_graph/schema_definition/api.rb | 12 ++++++---- .../schema_definition/results.rb | 5 +---- .../elastic_graph/schema_definition/state.rb | 3 ++- .../elastic_graph/schema_definition/state.rbs | 6 +++-- .../graphql_resolvers_by_name_spec.rb | 22 +++++++++++++++++++ .../spec_support/runtime_metadata_support.rb | 5 ++--- 9 files changed, 49 insertions(+), 35 deletions(-) diff --git a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rb b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rb index cd1d184d1..559c84172 100644 --- a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rb +++ b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rb @@ -12,7 +12,7 @@ module ElasticGraph module SchemaArtifacts module RuntimeMetadata # @private - class GraphQLResolver < ::Data.define(:needs_lookahead, :resolver_ref, :built_in) + class GraphQLResolver < ::Data.define(:needs_lookahead, :resolver_ref) def self.with_lookahead_loader @with_lookahead_loader ||= ExtensionLoader.new(InterfaceWithLookahead) end @@ -21,7 +21,6 @@ def self.without_lookahead_loader @without_lookahead_loader ||= ExtensionLoader.new(InterfaceWithoutLookahead) end - BUILT_IN = "built_in" NEEDS_LOOKAHEAD = "needs_lookahead" RESOLVER_REF = "resolver_ref" @@ -33,15 +32,13 @@ def load_resolver def self.from_hash(hash) new( needs_lookahead: hash.fetch(NEEDS_LOOKAHEAD), - resolver_ref: hash.fetch(RESOLVER_REF), - built_in: hash.fetch(BUILT_IN) { false } + resolver_ref: hash.fetch(RESOLVER_REF) ) end def to_dumpable_hash { # Keys here are ordered alphabetically; please keep them that way. - BUILT_IN => built_in, NEEDS_LOOKAHEAD => needs_lookahead, RESOLVER_REF => resolver_ref } diff --git a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rbs b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rbs index c140c1303..1447cd50b 100644 --- a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rbs +++ b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver.rbs @@ -4,23 +4,20 @@ module ElasticGraph class GraphQLResolverSupertype attr_reader needs_lookahead: bool attr_reader resolver_ref: ::Hash[::String, ::String] - attr_reader built_in: bool def initialize: ( needs_lookahead: bool, - resolver_ref: ::Hash[::String, ::String], - built_in: bool + resolver_ref: ::Hash[::String, ::String] ) -> void def with: ( ?needs_lookahead: bool, - ?resolver_ref: ::Hash[::String, ::String], - ?built_in: bool + ?resolver_ref: ::Hash[::String, ::String] ) -> instance def self.new: - (needs_lookahead: bool, resolver_ref: ::Hash[::String, ::String], built_in: bool) -> instance - | (bool, ::Hash[::String, ::String], bool) -> instance + (needs_lookahead: bool, resolver_ref: ::Hash[::String, ::String]) -> instance + | (bool, ::Hash[::String, ::String]) -> instance end class GraphQLResolver < GraphQLResolverSupertype @@ -30,7 +27,6 @@ module ElasticGraph self.@without_lookahead_loader: ExtensionLoader? def self.without_lookahead_loader: () -> ExtensionLoader - BUILT_IN: "built_in" NEEDS_LOOKAHEAD: "needs_lookahead" RESOLVER_REF: "resolver_ref" diff --git a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver_spec.rb b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver_spec.rb index c4780de0b..81096a857 100644 --- a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver_spec.rb +++ b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_resolver_spec.rb @@ -18,8 +18,7 @@ module RuntimeMetadata resolver_ref: { "name" => "ElasticGraph::GraphQLResolverWithLookahead", "require_path" => "elastic_graph/spec_support/example_extensions/graphql_resolvers" - }, - built_in: false + } ) expect(resolver.load_resolver.extension_class).to be GraphQLResolverWithLookahead @@ -31,8 +30,7 @@ module RuntimeMetadata resolver_ref: { "name" => "ElasticGraph::GraphQLResolverWithoutLookahead", "require_path" => "elastic_graph/spec_support/example_extensions/graphql_resolvers" - }, - built_in: false + } ) expect(resolver.load_resolver.extension_class).to be GraphQLResolverWithoutLookahead @@ -44,8 +42,7 @@ module RuntimeMetadata resolver_ref: { "name" => "ElasticGraph::GraphQLResolverWithoutLookahead", "require_path" => "elastic_graph/spec_support/example_extensions/graphql_resolvers" - }, - built_in: false + } ) expect { @@ -59,8 +56,7 @@ module RuntimeMetadata resolver_ref: { "name" => "ElasticGraph::GraphQLResolverWithLookahead", "require_path" => "elastic_graph/spec_support/example_extensions/graphql_resolvers" - }, - built_in: false + } ) expect { diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb index 53e1fa53c..4b61944c2 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb @@ -302,6 +302,8 @@ def register_graphql_extension(extension_module, defined_at:, **config) # @param name [Symbol] unique name of the resolver # @param klass [Class] resolver class # @param defined_at [String] the `require` path of the resolver + # @param built_in [bool] Whether this resolver is built-in to ElasticGraph or one of its extensions. + # Built-in resolvers that are unused in a schema will not trigger a warning. # @param resolver_config [Hash] configuration options for the resolver, to support parameterized resolvers # @return [void] # @see Mixins::HasIndices#resolve_fields_with @@ -369,8 +371,6 @@ def register_graphql_extension(extension_module, defined_at:, **config) # end # end # end - # @param built_in [bool] Whether this resolver is built-in to ElasticGraph or one of its extensions. - # Built-in resolvers that are unused in a schema will not trigger a warning. def register_graphql_resolver(name, klass, defined_at:, built_in: false, **resolver_config) extension = SchemaArtifacts::RuntimeMetadata::Extension.new(klass, defined_at, resolver_config) @@ -384,11 +384,15 @@ def register_graphql_resolver(name, klass, defined_at:, built_in: false, **resol resolver = SchemaArtifacts::RuntimeMetadata::GraphQLResolver.new( needs_lookahead: needs_lookahead, - resolver_ref: extension.to_dumpable_hash, - built_in: built_in + resolver_ref: extension.to_dumpable_hash ) @state.graphql_resolvers_by_name[name] = resolver + if built_in + @state.built_in_graphql_resolvers << name + else + @state.built_in_graphql_resolvers.delete(name) + end nil end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb index bb3cddde6..40afc05e5 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb @@ -381,10 +381,7 @@ def verify_runtime_metadata(runtime_metadata) end end - unused_resolvers = registered_resolvers.except(*fields_by_resolvers.keys).reject do |name, res| - # Ignore our built-in resolvers. - res.built_in - end.keys + unused_resolvers = registered_resolvers.except(*fields_by_resolvers.keys, *state.built_in_graphql_resolvers).keys unless unused_resolvers.empty? state.output.puts <<~EOS diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/state.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/state.rb index b8c73eedc..800d15cbf 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/state.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/state.rb @@ -44,7 +44,7 @@ class State < Struct.new( :json_schema_version_setter_location, :graphql_extension_modules, :graphql_resolvers_by_name, - :resolvers_by_name, + :built_in_graphql_resolvers, :initially_registered_built_in_types, :built_in_types_customization_blocks, :user_definition_complete, @@ -92,6 +92,7 @@ def self.with( json_schema_version: nil, graphql_extension_modules: [], graphql_resolvers_by_name: {}, + built_in_graphql_resolvers: ::Set.new, initially_registered_built_in_types: ::Set.new, built_in_types_customization_blocks: [], user_definition_complete: false, diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/state.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/state.rbs index ee8a043ae..7ea53f98d 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/state.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/state.rbs @@ -20,6 +20,7 @@ module ElasticGraph attr_accessor json_schema_version_setter_location: ::Thread::Backtrace::Location? attr_reader graphql_extension_modules: ::Array[SchemaArtifacts::RuntimeMetadata::GraphQLExtension] attr_reader graphql_resolvers_by_name: ::Hash[::Symbol, SchemaArtifacts::RuntimeMetadata::GraphQLResolver] + attr_reader built_in_graphql_resolvers: ::Set[::Symbol] attr_accessor initially_registered_built_in_types: ::Set[::String] attr_accessor built_in_types_customization_blocks: ::Array[^(SchemaElements::graphQLType) -> void] attr_accessor user_definition_complete: bool @@ -50,8 +51,9 @@ module ElasticGraph deleted_fields_by_type_name_and_old_field_name: ::Hash[::String, ::Hash[::String, SchemaElements::DeprecatedElement]], json_schema_version: Integer?, json_schema_version_setter_location: ::Thread::Backtrace::Location?, - graphql_extension_modules: ::Array[SchemaArtifacts::RuntimeMetadata::Extension], - graphql_resolvers_by_name: ::Hash[::Symbol, SchemaArtifacts::RuntimeMetadata::Extension], + graphql_extension_modules: ::Array[SchemaArtifacts::RuntimeMetadata::GraphQLExtension], + graphql_resolvers_by_name: ::Hash[::Symbol, SchemaArtifacts::RuntimeMetadata::GraphQLResolver], + built_in_graphql_resolvers: ::Set[::Symbol], initially_registered_built_in_types: ::Set[::String], built_in_types_customization_blocks: ::Array[^(SchemaElements::graphQLType) -> void], user_definition_complete: bool, diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb index 9940d698b..69cc710be 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/graphql_resolvers_by_name_spec.rb @@ -162,6 +162,28 @@ module SchemaDefinition expect(output.string).to eq("") end + it "warns when a built-in resolver name is re-registered as non-built-in and remains unused" do + output = StringIO.new + + graphql_resolvers_by_name(output: output) do |schema| + schema.register_graphql_resolver :resolver1, + GraphQLResolverWithoutLookahead, + defined_at: "some/path", + built_in: true + + schema.register_graphql_resolver :resolver1, + GraphQLResolverWithoutLookahead, + defined_at: "some/path" + end + + expect(output.string).to eq(<<~EOS) + WARNING: 1 GraphQL resolver(s) have been registered but are unused: + - resolver1 + These resolvers can be removed. If you intended for them to be available as built-in/internal + resolvers, pass `built_in: true` when registering them. + EOS + end + def graphql_resolvers_by_name(...) define_schema(...).runtime_metadata.graphql_resolvers_by_name end diff --git a/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb b/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb index 9eac34f35..29316874b 100644 --- a/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb +++ b/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb @@ -174,11 +174,10 @@ def configured_graphql_resolver(name, **config) "require_path" => "elastic_graph/graphql/resolvers/get_record_field_value" } - def graphql_resolver_with(needs_lookahead: false, resolver_ref: DEFAULT_RESOLVER_REF, built_in: false) + def graphql_resolver_with(needs_lookahead: false, resolver_ref: DEFAULT_RESOLVER_REF) GraphQLResolver.new( needs_lookahead: needs_lookahead, - resolver_ref: resolver_ref, - built_in: built_in + resolver_ref: resolver_ref ) end