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 9db196782..d6f1e462b 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 @@ -382,15 +382,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 865f52da4..307f74737 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 272a5958c..a12862103 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb @@ -383,14 +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. - res.resolver_ref.fetch("require_path").start_with?("elastic_graph/graphql/resolvers/") + 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 c5ad83350..5bbfbe0d4 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 6dfe4cbb1..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,10 +144,24 @@ 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 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 :resolver1, + GraphQLResolverWithoutLookahead, + defined_at: "some/path", + built_in: true + end + + expect(output.string).to eq("") + 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 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