From f5dd285ddc151cc717c15023210a769b36dfe039 Mon Sep 17 00:00:00 2001 From: Myron Marston Date: Sun, 26 Jan 2025 12:06:40 -0800 Subject: [PATCH] Introduce graphql resolver runtime metadata. I plan to use this to optimize our current GrahQL resolver so that instead of doing computation to figure out which resolver to use, it is told up front which resolver to use. --- .../runtime_metadata/graphql_field.rb | 19 ++++++--- .../runtime_metadata/object_type.rb | 15 ++++--- .../runtime_metadata/graphql_field.rbs | 15 ++++--- .../runtime_metadata/object_type.rbs | 8 +++- .../runtime_metadata/graphql_field_spec.rb | 20 ++++++++- .../runtime_metadata/object_type_spec.rb | 42 +++++++++++-------- .../runtime_metadata/schema_spec.rb | 26 ++++++++---- .../schema_definition/mixins/has_indices.rb | 3 +- .../schema_elements/field.rb | 3 +- .../schema_elements/input_type.rb | 3 +- .../spec_support/runtime_metadata_support.rb | 18 ++++++-- 11 files changed, 119 insertions(+), 53 deletions(-) diff --git a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_field.rb b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_field.rb index 1f54d600..bb6108de 100644 --- a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_field.rb +++ b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/graphql_field.rb @@ -12,17 +12,19 @@ module ElasticGraph module SchemaArtifacts module RuntimeMetadata - class GraphQLField < ::Data.define(:name_in_index, :relation, :computation_detail) - EMPTY = new(nil, nil, nil) + class GraphQLField < ::Data.define(:name_in_index, :relation, :computation_detail, :resolver) + EMPTY = new(nil, nil, nil, nil) NAME_IN_INDEX = "name_in_index" RELATION = "relation" AGGREGATION_DETAIL = "computation_detail" + RESOLVER = "resolver" def self.from_hash(hash) new( name_in_index: hash[NAME_IN_INDEX], relation: hash[RELATION]&.then { |rel_hash| Relation.from_hash(rel_hash) }, - computation_detail: hash[AGGREGATION_DETAIL]&.then { |agg_hash| ComputationDetail.from_hash(agg_hash) } + computation_detail: hash[AGGREGATION_DETAIL]&.then { |agg_hash| ComputationDetail.from_hash(agg_hash) }, + resolver: hash[RESOLVER]&.to_sym ) end @@ -31,15 +33,20 @@ def to_dumpable_hash # Keys here are ordered alphabetically; please keep them that way. AGGREGATION_DETAIL => computation_detail&.to_dumpable_hash, NAME_IN_INDEX => name_in_index, - RELATION => relation&.to_dumpable_hash + RELATION => relation&.to_dumpable_hash, + RESOLVER => resolver&.to_s } end # Indicates if we need this field in our dumped runtime metadata, when it has the given # `name_in_graphql`. Fields that have not been customized in some way do not need to be # included in the dumped runtime metadata. - def needed?(name_in_graphql) - !!relation || !!computation_detail || name_in_index&.!=(name_in_graphql) || false + def needed?(name_in_graphql, resolver) + !!relation || + !!computation_detail || + name_in_index&.!=(name_in_graphql) || + resolver != self.resolver || + false end def with_computation_detail(empty_bucket_value:, function:) diff --git a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb index 27528162..84dd1c9f 100644 --- a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb +++ b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb @@ -23,7 +23,8 @@ class ObjectType < ::Data.define( # imply that this type was user-defined; we have recently introduced this metadata and are not yet setting # it for all generated GraphQL types. For now, we are only setting it for specific cases where we need it. :source_type, - :graphql_only_return_type + :graphql_only_return_type, + :default_graphql_resolver ) UPDATE_TARGETS = "update_targets" INDEX_DEFINITION_NAMES = "index_definition_names" @@ -31,9 +32,10 @@ class ObjectType < ::Data.define( ELASTICGRAPH_CATEGORY = "elasticgraph_category" SOURCE_TYPE = "source_type" GRAPHQL_ONLY_RETURN_TYPE = "graphql_only_return_type" + DEFAULT_GRAPHQL_RESOLVER = "default_graphql_resolver" - def initialize(update_targets:, index_definition_names:, graphql_fields_by_name:, elasticgraph_category:, source_type:, graphql_only_return_type:) - graphql_fields_by_name = graphql_fields_by_name.select { |name, field| field.needed?(name) } + def initialize(update_targets:, index_definition_names:, graphql_fields_by_name:, elasticgraph_category:, source_type:, graphql_only_return_type:, default_graphql_resolver:) + graphql_fields_by_name = graphql_fields_by_name.select { |name, field| field.needed?(name, default_graphql_resolver) } super( update_targets: update_targets, @@ -41,7 +43,8 @@ def initialize(update_targets:, index_definition_names:, graphql_fields_by_name: graphql_fields_by_name: graphql_fields_by_name, elasticgraph_category: elasticgraph_category, source_type: source_type, - graphql_only_return_type: graphql_only_return_type + graphql_only_return_type: graphql_only_return_type, + default_graphql_resolver: default_graphql_resolver ) end @@ -60,13 +63,15 @@ def self.from_hash(hash) graphql_fields_by_name: graphql_fields_by_name, elasticgraph_category: hash[ELASTICGRAPH_CATEGORY]&.to_sym || nil, source_type: hash[SOURCE_TYPE] || nil, - graphql_only_return_type: !!hash[GRAPHQL_ONLY_RETURN_TYPE] + graphql_only_return_type: !!hash[GRAPHQL_ONLY_RETURN_TYPE], + default_graphql_resolver: hash[DEFAULT_GRAPHQL_RESOLVER]&.to_sym ) end def to_dumpable_hash { # Keys here are ordered alphabetically; please keep them that way. + DEFAULT_GRAPHQL_RESOLVER => default_graphql_resolver&.to_s, ELASTICGRAPH_CATEGORY => elasticgraph_category&.to_s, GRAPHQL_FIELDS_BY_NAME => HashDumper.dump_hash(graphql_fields_by_name, &:to_dumpable_hash), GRAPHQL_ONLY_RETURN_TYPE => graphql_only_return_type ? true : nil, diff --git a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_field.rbs b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_field.rbs index baeba0cb..3805ad35 100644 --- a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_field.rbs +++ b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/graphql_field.rbs @@ -5,22 +5,25 @@ module ElasticGraph attr_reader name_in_index: ::String? attr_reader relation: Relation? attr_reader computation_detail: ComputationDetail? + attr_reader resolver: ::Symbol? def initialize: ( name_in_index: ::String?, relation: Relation?, - computation_detail: ComputationDetail? + computation_detail: ComputationDetail?, + resolver: ::Symbol? ) -> void def with: ( ?name_in_index: ::String?, ?relation: Relation?, - ?computation_detail: ComputationDetail? + ?computation_detail: ComputationDetail?, + ?resolver: ::Symbol? ) -> instance def self.new: - (name_in_index: ::String?, relation: Relation?, computation_detail: ComputationDetail?) -> instance - | (::String?, Relation?, ::Symbol?) -> instance + (name_in_index: ::String?, relation: Relation?, computation_detail: ComputationDetail?, resolver: ::Symbol?) -> instance + | (::String?, Relation?, ComputationDetail?, ::Symbol?) -> instance end class GraphQLField < GraphQLFieldSupertype @@ -28,9 +31,11 @@ module ElasticGraph NAME_IN_INDEX: "name_in_index" RELATION: "relation" AGGREGATION_DETAIL: "computation_detail" + RESOLVER: "resolver" + def self.from_hash: (::Hash[::String, untyped]) -> GraphQLField def to_dumpable_hash: () -> ::Hash[::String, untyped] - def needed?: (::String) -> bool + def needed?: (::String, ::Symbol?) -> bool def with_computation_detail: ( empty_bucket_value: ::Numeric?, diff --git a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/object_type.rbs b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/object_type.rbs index ba58ebeb..80963b15 100644 --- a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/object_type.rbs +++ b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/object_type.rbs @@ -10,6 +10,7 @@ module ElasticGraph attr_reader elasticgraph_category: elasticGraphCategory? attr_reader source_type: ::String? attr_reader graphql_only_return_type: bool + attr_reader default_graphql_resolver: ::Symbol? def initialize: ( update_targets: ::Array[UpdateTarget], @@ -17,7 +18,8 @@ module ElasticGraph graphql_fields_by_name: ::Hash[::String, GraphQLField], elasticgraph_category: elasticGraphCategory?, source_type: ::String?, - graphql_only_return_type: bool + graphql_only_return_type: bool, + default_graphql_resolver: ::Symbol? ) -> void def with: ( @@ -26,7 +28,8 @@ module ElasticGraph ?graphql_fields_by_name: ::Hash[::String, GraphQLField], ?elasticgraph_category: elasticGraphCategory?, ?source_type: ::String?, - ?graphql_only_return_type: bool + ?graphql_only_return_type: bool, + ?default_graphql_resolver: ::Symbol? ) -> ObjectType end @@ -37,6 +40,7 @@ module ElasticGraph ELASTICGRAPH_CATEGORY: "elasticgraph_category" SOURCE_TYPE: "source_type" GRAPHQL_ONLY_RETURN_TYPE: "graphql_only_return_type" + DEFAULT_GRAPHQL_RESOLVER: "default_graphql_resolver" def self.from_hash: (::Hash[::String, untyped]) -> ObjectType def to_dumpable_hash: () -> ::Hash[::String, untyped] end diff --git a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_field_spec.rb b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_field_spec.rb index c2561ead..13fb7eda 100644 --- a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_field_spec.rb +++ b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/graphql_field_spec.rb @@ -19,17 +19,19 @@ module RuntimeMetadata field = GraphQLField.from_hash({}) expect(field).to eq GraphQLField.new( + computation_detail: nil, name_in_index: nil, relation: nil, - computation_detail: nil + resolver: nil ) end it "offers `with_computation_detail` updating aggregation detail" do field = GraphQLField.new( + computation_detail: nil, name_in_index: nil, relation: nil, - computation_detail: nil + resolver: :self ) updated = field.with_computation_detail( @@ -42,6 +44,20 @@ module RuntimeMetadata function: :sum )) end + + it "exposes `resolver` as a symbol while keeping it as a string in dumped form" do + field = GraphQLField.from_hash({"resolver" => "self"}) + + expect(field.resolver).to eq :self + expect(field.to_dumpable_hash).to include("resolver" => "self") + end + + it "exposes `resolver` as nil when it is unset" do + field = GraphQLField.from_hash({}) + + expect(field.resolver).to eq nil + expect(field.to_dumpable_hash).to include("resolver" => nil) + end end end end diff --git a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb index e19df9ea..d4e0b8fa 100644 --- a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb +++ b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb @@ -15,22 +15,6 @@ module RuntimeMetadata RSpec.describe ObjectType do include RuntimeMetadataSupport - it "ignores fields that have no meaningful runtime metadata" do - object_type = object_type_with(graphql_fields_by_name: { - "has_relation" => graphql_field_with(name_in_index: nil, relation: relation_with), - "has_computation_detail" => graphql_field_with(computation_detail: :sum), - "has_alternate_index_name" => graphql_field_with(name_in_index: "alternate"), - "has_nil_index_name" => graphql_field_with(name_in_index: nil), - "has_same_index_name" => graphql_field_with(name_in_index: "has_same_index_name") - }) - - expect(object_type.graphql_fields_by_name.keys).to contain_exactly( - "has_relation", - "has_computation_detail", - "has_alternate_index_name" - ) - end - it "builds from a minimal hash" do type = ObjectType.from_hash({}) @@ -40,7 +24,8 @@ module RuntimeMetadata graphql_fields_by_name: {}, elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: false + graphql_only_return_type: false, + default_graphql_resolver: nil ) end @@ -51,6 +36,13 @@ module RuntimeMetadata expect(type.to_dumpable_hash).to include("elasticgraph_category" => "scalar_aggregated_values") end + it "exposes `default_graphql_resolver` as a symbol while keeping it as a string in dumped form" do + type = ObjectType.from_hash({"default_graphql_resolver" => "self"}) + + expect(type.default_graphql_resolver).to eq :self + expect(type.to_dumpable_hash).to include("default_graphql_resolver" => "self") + end + it "models `graphql_only_return_type` as `true` or `nil` so that our runtime metadata pruning can omit nils" do type = ObjectType.from_hash({}) @@ -62,6 +54,22 @@ module RuntimeMetadata expect(type.graphql_only_return_type).to eq true expect(type.to_dumpable_hash).to include("graphql_only_return_type" => true) end + + it "omits GraphQL fields that have no meaningful metadata" do + type = object_type_with( + default_graphql_resolver: :self, + graphql_fields_by_name: { + "foo1" => graphql_field_with(resolver: :self, name_in_index: "foo1"), + "foo2" => graphql_field_with(resolver: :self, name_in_index: "foo2_in_index"), + "foo3" => graphql_field_with(resolver: :self, name_in_index: "foo3", relation: relation_with), + "foo4" => graphql_field_with(resolver: :self, name_in_index: "foo4", computation_detail: computation_detail_with), + "foo5" => graphql_field_with(resolver: :other, name_in_index: "foo5"), + "foo6" => graphql_field_with(resolver: :self, name_in_index: nil) + } + ) + + expect(type.graphql_fields_by_name.keys).to contain_exactly("foo2", "foo3", "foo4", "foo5") + end end end end diff --git a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb index 4a175042..6361c0cb 100644 --- a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb +++ b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb @@ -30,6 +30,7 @@ module RuntimeMetadata object_types_by_name: { "Widget" => ObjectType.new( index_definition_names: ["widgets"], + default_graphql_resolver: nil, update_targets: [ UpdateTarget.new( type: "WidgetCurrency", @@ -54,19 +55,21 @@ module RuntimeMetadata ], graphql_fields_by_name: { "name_graphql" => GraphQLField.new( - name_in_index: "name_index", computation_detail: nil, - relation: nil + name_in_index: "name_index", + relation: nil, + resolver: :self ), "parent" => GraphQLField.new( - name_in_index: nil, computation_detail: nil, + name_in_index: nil, relation: Relation.new( foreign_key: "grandparents.parents.some_id", direction: :out, additional_filter: {"flag_field" => {"equalToAnyOf" => [true]}}, foreign_key_nested_paths: ["grandparents", "grandparents.parents"] - ) + ), + resolver: :self ), "sum" => GraphQLField.new( name_in_index: nil, @@ -74,7 +77,8 @@ module RuntimeMetadata empty_bucket_value: 0, function: :sum ), - relation: nil + relation: nil, + resolver: :self ) }, elasticgraph_category: :some_category, @@ -167,7 +171,8 @@ module RuntimeMetadata ], "graphql_fields_by_name" => { "name_graphql" => { - "name_in_index" => "name_index" + "name_in_index" => "name_index", + "resolver" => "self" }, "parent" => { "relation" => { @@ -175,13 +180,15 @@ module RuntimeMetadata "direction" => "out", "additional_filter" => {"flag_field" => {"equalToAnyOf" => [true]}}, "foreign_key_nested_paths" => ["grandparents", "grandparents.parents"] - } + }, + "resolver" => "self" }, "sum" => { "computation_detail" => { "empty_bucket_value" => 0, "function" => "sum" - } + }, + "resolver" => "self" } }, "elasticgraph_category" => "some_category", @@ -293,7 +300,8 @@ module RuntimeMetadata "name_graphql" => GraphQLField.new( name_in_index: "name_index", computation_detail: nil, - relation: nil + relation: nil, + resolver: :self ) }), "NoMetadata" => object_type_with diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb index 8648f95c..617f278e 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb @@ -158,7 +158,8 @@ def runtime_metadata(extra_update_targets) graphql_fields_by_name: runtime_metadata_graphql_fields_by_name, elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: graphql_only? + graphql_only_return_type: graphql_only?, + default_graphql_resolver: nil ).with(**runtime_metadata_overrides) end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/field.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/field.rb index 5c12688f..02948070 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/field.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/field.rb @@ -925,7 +925,8 @@ def runtime_metadata_graphql_field SchemaArtifacts::RuntimeMetadata::GraphQLField.new( name_in_index: name_in_index, computation_detail: computation_detail, - relation: relationship&.runtime_metadata + relation: relationship&.runtime_metadata, + resolver: nil ) end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb index 1d91cde6..240b3004 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb @@ -47,7 +47,8 @@ def runtime_metadata(extra_update_targets) graphql_fields_by_name: graphql_fields_by_name.transform_values(&:runtime_metadata_graphql_field), elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: false + graphql_only_return_type: false, + default_graphql_resolver: nil ) 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 6431c52d..2f00c96f 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 @@ -38,7 +38,8 @@ def object_type_with( graphql_fields_by_name: {}, elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: false + graphql_only_return_type: false, + default_graphql_resolver: nil ) ObjectType.new( index_definition_names: index_definition_names, @@ -46,7 +47,8 @@ def object_type_with( graphql_fields_by_name: graphql_fields_by_name, elasticgraph_category: elasticgraph_category, source_type: source_type, - graphql_only_return_type: graphql_only_return_type + graphql_only_return_type: graphql_only_return_type, + default_graphql_resolver: default_graphql_resolver ) end @@ -93,6 +95,13 @@ def normal_indexing_update_target_with( ) end + def computation_detail_with(empty_bucket_value: 0, function: :sum) + ComputationDetail.new( + empty_bucket_value: empty_bucket_value, + function: function + ) + end + def dynamic_param_with(source_path: "some_field", cardinality: :one) DynamicParam.new(source_path: source_path, cardinality: cardinality) end @@ -130,11 +139,12 @@ def relation_with(foreign_key: "some_id", direction: :asc, additional_filter: {} Relation.new(foreign_key: foreign_key, direction: direction, additional_filter: additional_filter, foreign_key_nested_paths: foreign_key_nested_paths) end - def graphql_field_with(name_in_index: "name_index", relation: nil, computation_detail: nil) + def graphql_field_with(name_in_index: "name_index", relation: nil, computation_detail: nil, resolver: nil) GraphQLField.new( + computation_detail: computation_detail, name_in_index: name_in_index, relation: relation, - computation_detail: computation_detail + resolver: resolver ) end