Skip to content

Commit eac440b

Browse files
Address more PR review feedback on GraphQL::Schema::Type helpers
- `source_type`: use `defined?(@source_type)` pattern to correctly memoize nil return values (||= doesn't memoize nil) - Rename `non_subtypes_in_shared_index` → `concrete_non_subtypes_in_shared_index` to make explicit that abstract types are excluded from the result - Tests: add a second sibling type so at least one test returns multiple elements, making it harder for a partial implementation to pass Generated with Claude Code
1 parent 2445473 commit eac440b

3 files changed

Lines changed: 24 additions & 12 deletions

File tree

  • elasticgraph-graphql

elasticgraph-graphql/lib/elastic_graph/graphql/schema/type.rb

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,18 +131,19 @@ def subtypes
131131
# For derived types (e.g. indexed aggregations), returns the underlying source document type.
132132
# Returns `nil` for non-derived types.
133133
def source_type
134-
@source_type ||= @object_runtime_metadata&.source_type&.then { |st| @schema.type_named(st) }
134+
return @source_type if defined?(@source_type)
135+
@source_type = @object_runtime_metadata&.source_type&.then { |st| @schema.type_named(st) }
135136
end
136137

137-
# Returns the set of concrete indexed document types that share any of this type's search
138-
# indexes but are not subtypes of this type. Used to determine whether a `__typename`
139-
# filter is needed when querying an abstract type.
138+
# Returns the set of concrete (non-abstract) indexed document types that share any of this
139+
# type's search indexes but are not subtypes of this type. Used to determine whether a
140+
# `__typename` filter is needed when querying an abstract type.
140141
#
141142
# Abstract types are excluded because documents in the datastore are always associated
142143
# with a concrete `__typename`. When filtering by `__typename`, only concrete types are
143144
# relevant.
144-
def non_subtypes_in_shared_index
145-
@non_subtypes_in_shared_index ||=
145+
def concrete_non_subtypes_in_shared_index
146+
@concrete_non_subtypes_in_shared_index ||=
146147
search_index_definitions
147148
.flat_map { |index_def| @schema.document_types_stored_in(index_def.name).to_a }
148149
.reject { |t| t == self || subtypes.include?(t) || t.abstract? }

elasticgraph-graphql/sig/elastic_graph/graphql/schema/type.rbs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ module ElasticGraph
1111
def search_index_definitions: () -> ::Array[DatastoreCore::_IndexDefinition]
1212
def source_type: () -> Type?
1313
def subtypes: () -> ::Set[Type]
14-
def non_subtypes_in_shared_index: () -> ::Set[Type]
14+
def concrete_non_subtypes_in_shared_index: () -> ::Set[Type]
1515
def unwrap_fully: () -> Type
1616
def field_named: (::String) -> Field
1717
def fields_by_name_in_index: () -> ::Hash[::String, ::Array[Field]]

elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema/type_spec.rb

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ def search_index_definitions_from(type_name: "TheType")
793793
end
794794
end
795795

796-
describe "#non_subtypes_in_shared_index" do
796+
describe "#concrete_non_subtypes_in_shared_index" do
797797
attr_reader :schema
798798

799799
before(:context) do
@@ -804,12 +804,17 @@ def search_index_definitions_from(type_name: "TheType")
804804
t.index "channels"
805805
end
806806

807-
# A sibling type — NOT under Store, but shares the Channel index.
807+
# Sibling types — NOT under Store, but share the Channel index.
808808
s.object_type "Wholesaler" do |t|
809809
t.implements "Channel"
810810
t.field "id", "ID!"
811811
end
812812

813+
s.object_type "Distributor" do |t|
814+
t.implements "Channel"
815+
t.field "id", "ID!"
816+
end
817+
813818
# A sub-interface and its concrete subtypes.
814819
s.interface_type "Store" do |t|
815820
t.implements "Channel"
@@ -831,17 +836,23 @@ def search_index_definitions_from(type_name: "TheType")
831836
end
832837

833838
it "excludes the type itself and its subtypes, returning only concrete sibling types in the shared index" do
834-
expect(schema.type_named("Store").non_subtypes_in_shared_index).to contain_exactly(schema.type_named("Wholesaler"))
839+
expect(schema.type_named("Store").concrete_non_subtypes_in_shared_index).to contain_exactly(
840+
schema.type_named("Wholesaler"),
841+
schema.type_named("Distributor")
842+
)
835843
end
836844

837845
it "excludes the type itself even when the type is a concrete type in a shared index" do
838846
# Wholesaler is a concrete type stored in the "channels" index, so it appears in
839847
# document_types_stored_in("channels"). Without the `t == self` guard it would include itself.
840-
expect(schema.type_named("Wholesaler").non_subtypes_in_shared_index).to contain_exactly(schema.type_named("OnlineStore"))
848+
expect(schema.type_named("Wholesaler").concrete_non_subtypes_in_shared_index).to contain_exactly(
849+
schema.type_named("Distributor"),
850+
schema.type_named("OnlineStore")
851+
)
841852
end
842853

843854
it "returns an empty set when all types sharing its indexes are subtypes" do
844-
expect(schema.type_named("Channel").non_subtypes_in_shared_index).to be_empty
855+
expect(schema.type_named("Channel").concrete_non_subtypes_in_shared_index).to be_empty
845856
end
846857
end
847858

0 commit comments

Comments
 (0)