From b0e4124429b5863afa127327caa5a41fa3d68279 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 15 May 2026 10:55:04 -0400 Subject: [PATCH 1/3] Visibility: require all root config before preloading --- lib/graphql/schema/visibility.rb | 41 ++++++++-------- lib/graphql/schema/visibility/profile.rb | 10 ++-- spec/graphql/schema/visibility_spec.rb | 61 ++++++++++++++++++++++-- 3 files changed, 85 insertions(+), 27 deletions(-) diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index 9d73bdcf483..5940abf86eb 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -8,11 +8,17 @@ class Schema # Use this plugin to make some parts of your schema hidden from some viewers. # class Visibility + class TypeConfigurationError < GraphQL::Error + def initialize(config_message, config_str) + message = "GraphQL::Schema::Visibility already preloaded, but #{config_message} added to the schema. Move this `#{config_str}` configuration above `use(GraphQL::Schema::Visibility)" + super(message) + end + end # @param schema [Class] # @param profiles [Hash Hash>] A hash of `name => context` pairs for preloading visibility profiles # @param preload [Boolean] if `true`, load the default schema profile and all named profiles immediately (defaults to `true` for `Rails.env.production?` and `Rails.env.staging?`) # @param migration_errors [Boolean] if `true`, raise an error when `Visibility` and `Warden` return different results - def self.use(schema, dynamic: false, profiles: EmptyObjects::EMPTY_HASH, preload: (defined?(Rails.env) ? (Rails.env.production? || Rails.env.staging?) : nil), migration_errors: false) + def self.use(schema, dynamic: false, profiles: EmptyObjects::EMPTY_HASH, preload: (defined?(Rails.env) ? (Rails.env.production? || Rails.env.staging? || nil) : false), migration_errors: false) profiles&.each { |name, ctx| ctx[:visibility_profile] = name ctx.freeze @@ -102,41 +108,27 @@ def preload # @api private def query_configured(query_type) - if @preload - ensure_all_loaded([query_type]) - end + require_if_preloaded("a query type was", "query(...)") end # @api private def mutation_configured(mutation_type) - if @preload - ensure_all_loaded([mutation_type]) - end + require_if_preloaded("a mutation type was", "mutation(...)") end # @api private def subscription_configured(subscription_type) - if @preload - ensure_all_loaded([subscription_type]) - end + require_if_preloaded("a mutation type was", "subscription(...)") end # @api private def orphan_types_configured(orphan_types) - if @preload - ensure_all_loaded(orphan_types) - end + require_if_preloaded("orphan types were", "orphan_types(...)") end # @api private def introspection_system_configured(introspection_system) - if @preload - introspection_types = [ - *@schema.introspection_system.types.values, - *@schema.introspection_system.entry_points.map { |ep| ep.type.unwrap }, - ] - ensure_all_loaded(introspection_types) - end + require_if_preloaded("custom introspection was", "introspection(...)") end # Make another Visibility for `schema` based on this one @@ -196,6 +188,15 @@ def top_level_profile(refresh: false) private + def require_if_preloaded(config_message, config_code) + case @preload + when false + # Rails.env wasn't defined, so this won't try to preload unless manually set to true + when true, nil + raise TypeConfigurationError.new(config_message, config_code) + end + end + def ensure_all_loaded(types_to_visit) while (type = types_to_visit.shift) if type.kind.fields? && @preloaded_types.add?(type) diff --git a/lib/graphql/schema/visibility/profile.rb b/lib/graphql/schema/visibility/profile.rb index 92a4b2cc774..9fab2198ca3 100644 --- a/lib/graphql/schema/visibility/profile.rb +++ b/lib/graphql/schema/visibility/profile.rb @@ -290,11 +290,13 @@ def preload end # Lots more to do here end - @schema.introspection_system.entry_points.each do |f| - arguments(f).each do |arg| - argument(f, arg.graphql_name) + if @schema.query + @schema.introspection_system.entry_points.each do |f| + arguments(f).each do |arg| + argument(f, arg.graphql_name) + end + field(@schema.query, f.graphql_name) end - field(@schema.query, f.graphql_name) end @schema.introspection_system.dynamic_fields.each do |f| arguments(f).each do |arg| diff --git a/spec/graphql/schema/visibility_spec.rb b/spec/graphql/schema/visibility_spec.rb index 0adbb42a5fd..99d5d01ea65 100644 --- a/spec/graphql/schema/visibility_spec.rb +++ b/spec/graphql/schema/visibility_spec.rb @@ -85,6 +85,59 @@ def exec_query(...) refute_includes schema_sdl, "WidgetKind" end + it "raises errors when configuration comes after preload: true" do + assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do + Class.new(GraphQL::Schema) do + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true + query(VisSchema::Query) + end + end + + assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do + Class.new(GraphQL::Schema) do + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true + mutation(VisSchema::Mutation) + end + end + + assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do + Class.new(GraphQL::Schema) do + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true + subscription(VisSchema::Subscription) + end + end + + assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do + Class.new(GraphQL::Schema) do + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true + orphan_types([VisSchema::Query]) + end + end + + assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do + Class.new(GraphQL::Schema) do + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true + introspection(VisSchema::IntrospectionSystem) + end + end + end + + it "doesn't raise when preload: false" do + assert(Class.new(GraphQL::Schema) do + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: false + query(VisSchema::Query) + end) + end + + it "raises when preload: nil" do + assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do + Class.new(GraphQL::Schema) do + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: nil + query(VisSchema::Query) + end + end + end + describe "running queries" do it "requires context[:visibility]" do err = assert_raises ArgumentError do @@ -173,10 +226,10 @@ class OrphanType < GraphQL::Schema::Object end # This one is added before `Visibility` subscription(Subscription) - use GraphQL::Schema::Visibility, preload: true query { Query } mutation { Mutation } orphan_types(OrphanType) + use GraphQL::Schema::Visibility, preload: true module CustomIntrospection class DynamicFields < GraphQL::Introspection::DynamicFields @@ -195,7 +248,9 @@ class DynamicFields < GraphQL::Introspection::DynamicFields assert_equal [NoProfileSchema::ExampleExtension, NoProfileSchema::OtherExampleExtension], NoProfileSchema::OrphanType.all_field_definitions.first.extensions.map(&:class) custom_int_field = NoProfileSchema::CustomIntrospection::DynamicFields.all_field_definitions.find { |f| f.original_name == :__hello } assert_equal [], custom_int_field.extensions - NoProfileSchema.introspection(NoProfileSchema::CustomIntrospection) + assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do + NoProfileSchema.introspection(NoProfileSchema::CustomIntrospection) + end assert_equal [NoProfileSchema::OtherExampleExtension], custom_int_field.extensions.map(&:class) end end @@ -313,8 +368,8 @@ def node end query(Query) - def self.resolve_type(...); Thing; end use GraphQL::Schema::Visibility + def self.resolve_type(...); Thing; end end end From 7546377683db91933042931939981944aa5d244c Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 21 May 2026 13:38:08 -0400 Subject: [PATCH 2/3] Rebuild visibility cache when config is inherited --- lib/graphql/schema/visibility.rb | 13 ++++++++++--- spec/graphql/schema/visibility_spec.rb | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index 5940abf86eb..cae29642833 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -26,7 +26,7 @@ def self.use(schema, dynamic: false, profiles: EmptyObjects::EMPTY_HASH, preload schema.visibility = self.new(schema, dynamic: dynamic, preload: preload, profiles: profiles, migration_errors: migration_errors) end - def initialize(schema, dynamic:, preload:, profiles:, migration_errors:) + def initialize(schema, dynamic:, preload:, profiles:, migration_errors:, configuration_inherited: false) @schema = schema schema.use_visibility_profile = true schema.visibility_profile_class = if migration_errors @@ -46,6 +46,7 @@ def initialize(schema, dynamic:, preload:, profiles:, migration_errors:) @types = nil @all_references = nil @loaded_all = false + @configuration_inherited = configuration_inherited if preload self.preload end @@ -100,6 +101,7 @@ def preload # Root types may have been nil: types_to_visit.compact! ensure_all_loaded(types_to_visit) + @cached_profiles.clear @profiles.each do |profile_name, example_ctx| prof = profile_for(example_ctx) prof.preload @@ -140,7 +142,8 @@ def dup_for(other_schema) dynamic: @dynamic, preload: @preload, profiles: @profiles, - migration_errors: @migration_errors + migration_errors: @migration_errors, + configuration_inherited: true, ) end @@ -193,7 +196,11 @@ def require_if_preloaded(config_message, config_code) when false # Rails.env wasn't defined, so this won't try to preload unless manually set to true when true, nil - raise TypeConfigurationError.new(config_message, config_code) + if @configuration_inherited + preload + else + raise TypeConfigurationError.new(config_message, config_code) + end end end diff --git a/spec/graphql/schema/visibility_spec.rb b/spec/graphql/schema/visibility_spec.rb index 99d5d01ea65..8509abb0b69 100644 --- a/spec/graphql/schema/visibility_spec.rb +++ b/spec/graphql/schema/visibility_spec.rb @@ -129,6 +129,21 @@ def exec_query(...) end) end + it "doesn't raise on subclasses and preloads" do + base_schema = Class.new(GraphQL::Schema) do + query(VisSchema::Query) + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true + end + + child_schema = Class.new(base_schema) do + query(VisSchema::Query) + end + + assert_equal [:public, :admin], base_schema.visibility.cached_profiles.keys + assert_equal [:public, :admin], child_schema.visibility.cached_profiles.keys + assert_equal [:public, :admin], Class.new(child_schema).visibility.cached_profiles.keys + end + it "raises when preload: nil" do assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do Class.new(GraphQL::Schema) do From 074d274d9cb55cd9cd5741eeb5eda842cbd60234 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 21 May 2026 14:23:25 -0400 Subject: [PATCH 3/3] Fix for Rails tests, remove some dead code --- lib/graphql/schema.rb | 2 +- lib/graphql/schema/introspection_system.rb | 27 +++++-------------- lib/graphql/schema/printer.rb | 2 +- .../graphql/introspection/schema_type_spec.rb | 2 +- .../schema/introspection_system_spec.rb | 3 +-- spec/graphql/schema_spec.rb | 4 +-- 6 files changed, 12 insertions(+), 28 deletions(-) diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 5d6a1df98c9..e0d2749123e 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -757,6 +757,7 @@ def introspection(new_introspection_namespace = nil) # reset this cached value: @introspection_system = nil introspection_system + self.visibility&.introspection_system_configured(introspection_system) @introspection else @introspection || find_inherited_value(:introspection) @@ -768,7 +769,6 @@ def introspection_system if !@introspection_system @introspection_system = Schema::IntrospectionSystem.new(self) @introspection_system.resolve_late_bindings - self.visibility&.introspection_system_configured(@introspection_system) end @introspection_system end diff --git a/lib/graphql/schema/introspection_system.rb b/lib/graphql/schema/introspection_system.rb index 93529d81c9e..8051bda2a1c 100644 --- a/lib/graphql/schema/introspection_system.rb +++ b/lib/graphql/schema/introspection_system.rb @@ -105,11 +105,13 @@ def resolve_late_binding(late_bound_type) end def load_constant(class_name) - const = @custom_namespace.const_get(class_name) + const = begin + @custom_namespace.const_get(class_name) + rescue NameError + # Dup the built-in so that the cached fields aren't shared + @built_in_namespace.const_get(class_name) + end dup_type_class(const) - rescue NameError - # Dup the built-in so that the cached fields aren't shared - dup_type_class(@built_in_namespace.const_get(class_name)) end def get_fields_from_class(class_sym:) @@ -133,23 +135,6 @@ def dup_type_class(type_class) end end end - - class PerFieldProxyResolve - def initialize(object_class:, inner_resolve:) - @object_class = object_class - @inner_resolve = inner_resolve - end - - def call(obj, args, ctx) - query_ctx = ctx.query.context - # Remove the QueryType wrapper - if obj.is_a?(GraphQL::Schema::Object) - obj = obj.object - end - wrapped_object = @object_class.wrap(obj, query_ctx) - @inner_resolve.call(wrapped_object, args, ctx) - end - end end end end diff --git a/lib/graphql/schema/printer.rb b/lib/graphql/schema/printer.rb index bfd8c53d2cb..b058f6cbb90 100644 --- a/lib/graphql/schema/printer.rb +++ b/lib/graphql/schema/printer.rb @@ -58,8 +58,8 @@ def self.visible?(ctx) end end schema = Class.new(GraphQL::Schema) { - use GraphQL::Schema::Visibility query(query_root) + use GraphQL::Schema::Visibility def self.visible?(member, _ctx) member.graphql_name != "Root" end diff --git a/spec/graphql/introspection/schema_type_spec.rb b/spec/graphql/introspection/schema_type_spec.rb index bf6bdc2d52e..234bb056c92 100644 --- a/spec/graphql/introspection/schema_type_spec.rb +++ b/spec/graphql/introspection/schema_type_spec.rb @@ -180,7 +180,7 @@ def self.visible?(context) end Class.new(GraphQL::Schema) do - use GraphQL::Schema::Visibility + use GraphQL::Schema::Visibility, preload: false query query_type directives invisible_directive, visible_directive end diff --git a/spec/graphql/schema/introspection_system_spec.rb b/spec/graphql/schema/introspection_system_spec.rb index 09f1b0c3126..bb0231ecbce 100644 --- a/spec/graphql/schema/introspection_system_spec.rb +++ b/spec/graphql/schema/introspection_system_spec.rb @@ -282,7 +282,7 @@ class DynamicFields < GraphQL::Introspection::DynamicFields class EntryPoints < GraphQL::Introspection::EntryPoints field_class(BaseField) - field :__type, GraphQL::Introspection::TypeType, resolve_static: true do + field :__type, GraphQL::Schema::LateBoundType.new("__Type"), resolve_static: true do argument :name, String end end @@ -294,7 +294,6 @@ class SchemaType < GraphQL::Introspection::SchemaType class Query < GraphQL::Schema::Object field :int, Integer, null: false - def int; 1; end end query(Query) diff --git a/spec/graphql/schema_spec.rb b/spec/graphql/schema_spec.rb index 09d2b432c95..3e7d318d6f3 100644 --- a/spec/graphql/schema_spec.rb +++ b/spec/graphql/schema_spec.rb @@ -530,7 +530,7 @@ class Query < GraphQL::Schema::Object it "defers root type blocks until those types are used" do calls = [] schema = Class.new(GraphQL::Schema) do - use(GraphQL::Schema::Visibility) + use(GraphQL::Schema::Visibility, preload: false) query { calls << :query; Class.new(GraphQL::Schema::Object) { graphql_name("Query") } } mutation { calls << :mutation; Class.new(GraphQL::Schema::Object) { graphql_name("Mutation") } } subscription { calls << :subscription; Class.new(GraphQL::Schema::Object) { graphql_name("Subscription") } } @@ -557,7 +557,7 @@ class Query < GraphQL::Schema::Object assert schema.instance_variable_get(:@subscription_extension_added) schema2 = Class.new(GraphQL::Schema) do - use(GraphQL::Schema::Visibility) + use(GraphQL::Schema::Visibility, preload: false) use GraphQL::Subscriptions subscription(Class.new(GraphQL::Schema::Object) { graphql_name("Subscription") }) end