From 867a158f731730105a9a8f19c7e63f33253dc602 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Wed, 6 Nov 2024 16:14:46 -0500 Subject: [PATCH] Fix Visibility preloading without profiles --- lib/graphql/schema.rb | 23 ++++++++-- lib/graphql/schema/visibility.rb | 57 ++++++++++++++++++++++-- lib/graphql/schema/visibility/profile.rb | 2 +- spec/graphql/schema/visibility_spec.rb | 22 +++++++++ 4 files changed, 96 insertions(+), 8 deletions(-) diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 67f9d11e2c..80db425e4f 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -445,7 +445,12 @@ def query(new_query_object = nil, &lazy_load_block) dup_defn = new_query_object || yield raise GraphQL::Error, "Second definition of `query(...)` (#{dup_defn.inspect}) is invalid, already configured with #{@query_object.inspect}" elsif use_visibility_profile? - @query_object = block_given? ? lazy_load_block : new_query_object + if block_given? + @query_object = lazy_load_block + else + @query_object = new_query_object + self.visibility.query_configured(@query_object) + end else @query_object = new_query_object || lazy_load_block.call add_type_and_traverse(@query_object, root: true) @@ -453,6 +458,7 @@ def query(new_query_object = nil, &lazy_load_block) nil elsif @query_object.is_a?(Proc) @query_object = @query_object.call + self.visibility&.query_configured(@query_object) else @query_object || find_inherited_value(:query) end @@ -472,7 +478,12 @@ def mutation(new_mutation_object = nil, &lazy_load_block) dup_defn = new_mutation_object || yield raise GraphQL::Error, "Second definition of `mutation(...)` (#{dup_defn.inspect}) is invalid, already configured with #{@mutation_object.inspect}" elsif use_visibility_profile? - @mutation_object = block_given? ? lazy_load_block : new_mutation_object + if block_given? + @mutation_object = lazy_load_block + else + @mutation_object = new_mutation_object + self.visibility.mutation_configured(@mutation_object) + end else @mutation_object = new_mutation_object || lazy_load_block.call add_type_and_traverse(@mutation_object, root: true) @@ -480,6 +491,7 @@ def mutation(new_mutation_object = nil, &lazy_load_block) nil elsif @mutation_object.is_a?(Proc) @mutation_object = @mutation_object.call + self.visibility&.mutation_configured(@query_object) else @mutation_object || find_inherited_value(:mutation) end @@ -499,7 +511,12 @@ def subscription(new_subscription_object = nil, &lazy_load_block) dup_defn = new_subscription_object || yield raise GraphQL::Error, "Second definition of `subscription(...)` (#{dup_defn.inspect}) is invalid, already configured with #{@subscription_object.inspect}" elsif use_visibility_profile? - @subscription_object = block_given? ? lazy_load_block : new_subscription_object + if block_given? + @subscription_object = lazy_load_block + else + @subscription_object = new_mutation_object + self.visibility.mutation_configured(@subscription_object) + end add_subscription_extension_if_necessary else @subscription_object = new_subscription_object || lazy_load_block.call diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index ea170d4dc9..e624ff6a8d 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -21,19 +21,55 @@ def initialize(schema, dynamic:, preload:, profiles:, migration_errors:) if migration_errors schema.visibility_profile_class = Migration end + @preload = preload @profiles = profiles @cached_profiles = {} @dynamic = dynamic @migration_errors = migration_errors if preload - profiles.each do |profile_name, example_ctx| - example_ctx[:visibility_profile] = profile_name - prof = profile_for(example_ctx, profile_name) - prof.all_types # force loading + @preloaded_types = Set.new + if profiles.any? + profiles.each do |profile_name, example_ctx| + example_ctx[:visibility_profile] = profile_name + prof = profile_for(example_ctx, profile_name) + prof.all_types # force loading + end + else + types_to_visit = [ + @schema.query, + @schema.mutation, + @schema.subscription, + *@schema.introspection_system.types.values, + *@schema.orphan_types, + *@schema.directives.map { |name, dir| dir.all_argument_definitions.map { |defn| defn.type.unwrap } }.flatten + ] + types_to_visit.compact! + ensure_all_loaded(types_to_visit) + # Somehow load files here end end end + def query_configured(query_type) + if @preload + ensure_all_loaded([query_type]) + end + end + + def mutation_configured(mutation_type) + if @preload + ensure_all_loaded([mutation_type]) + end + end + + def subscription_configured(subscription_type) + if @preload + ensure_all_loaded([subscription_type]) + end + end + + # TODO: on_orphan_types, on_directive, on_introspection_system + # Make another Visibility for `schema` based on this one # @return [Visibility] # @api private @@ -70,6 +106,19 @@ def profile_for(context, visibility_profile) @schema.visibility_profile_class.new(context: context, schema: @schema) end end + + private + + def ensure_all_loaded(types_to_visit) + while (type = types_to_visit.shift) + if type.kind.fields? && @preloaded_types.add?(type) + type.all_field_definitions.each do |field_defn| + field_defn.ensure_loaded + types_to_visit << field_defn.type.unwrap + end + end + end + end end end end diff --git a/lib/graphql/schema/visibility/profile.rb b/lib/graphql/schema/visibility/profile.rb index 330875a994..f744bf64c9 100644 --- a/lib/graphql/schema/visibility/profile.rb +++ b/lib/graphql/schema/visibility/profile.rb @@ -404,7 +404,7 @@ def load_all_types end end - entry_point_types.compact! # TODO why is this necessary?! + entry_point_types.compact! # Root types might be nil entry_point_types.flatten! # handle multiple defns entry_point_types.each { |t| add_type(t, true) } diff --git a/spec/graphql/schema/visibility_spec.rb b/spec/graphql/schema/visibility_spec.rb index ed0ed02283..d089e856d8 100644 --- a/spec/graphql/schema/visibility_spec.rb +++ b/spec/graphql/schema/visibility_spec.rb @@ -86,5 +86,27 @@ def exec_query(...) assert_equal [:public, :admin], VisSchema.visibility.cached_profiles.keys, "preload: true" assert_equal 0, DynVisSchema.visibility.cached_profiles.size, "preload: false" end + + describe "when no profile is defined" do + class NoProfileSchema < GraphQL::Schema + class ExampleExtension < GraphQL::Schema::FieldExtension + end + + class Query < GraphQL::Schema::Object + field :str do + type(String) + extension(ExampleExtension) + end + end + + use GraphQL::Schema::Visibility, preload: true + end + + it "still preloads" do + assert_equal [], NoProfileSchema::Query.all_field_definitions.first.extensions.map(&:class) + NoProfileSchema.query(NoProfileSchema::Query) + assert_equal [NoProfileSchema::ExampleExtension], NoProfileSchema::Query.all_field_definitions.first.extensions.map(&:class) + end + end end end