From fcfed4ce2d338c39baca3e99b0eca8d5fa7ec8f8 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 26 Aug 2024 14:56:28 -0400 Subject: [PATCH 01/17] Add cached, named Visibility profiles --- lib/graphql/query.rb | 5 +- lib/graphql/schema.rb | 3 +- lib/graphql/schema/visibility.rb | 41 +++++++++-- lib/graphql/schema/visibility/subset.rb | 10 ++- spec/graphql/schema/visibility_spec.rb | 95 +++++++++++++++++++++++++ 5 files changed, 141 insertions(+), 13 deletions(-) create mode 100644 spec/graphql/schema/visibility_spec.rb diff --git a/lib/graphql/query.rb b/lib/graphql/query.rb index d5243c83b5..06a589dfbb 100644 --- a/lib/graphql/query.rb +++ b/lib/graphql/query.rb @@ -95,7 +95,8 @@ def selected_operation_name # @param root_value [Object] the object used to resolve fields on the root type # @param max_depth [Numeric] the maximum number of nested selections allowed for this query (falls back to schema-level value) # @param max_complexity [Numeric] the maximum field complexity for this query (falls back to schema-level value) - def initialize(schema, query_string = nil, query: nil, document: nil, context: nil, variables: nil, validate: true, static_validator: nil, subscription_topic: nil, operation_name: nil, root_value: nil, max_depth: schema.max_depth, max_complexity: schema.max_complexity, warden: nil, use_schema_subset: nil) + # @param visibility_profile [Symbol] + def initialize(schema, query_string = nil, query: nil, document: nil, context: nil, variables: nil, validate: true, static_validator: nil, visibility_profile: nil, subscription_topic: nil, operation_name: nil, root_value: nil, max_depth: schema.max_depth, max_complexity: schema.max_complexity, warden: nil, use_schema_subset: nil) # Even if `variables: nil` is passed, use an empty hash for simpler logic variables ||= {} @schema = schema @@ -106,7 +107,7 @@ def initialize(schema, query_string = nil, query: nil, document: nil, context: n end if use_schema_subset - @schema_subset = @schema.subset_class.new(context: @context, schema: @schema) + @schema_subset = @schema.visibility.profile_for(@context, visibility_profile) @warden = Schema::Warden::NullWarden.new(context: @context, schema: @schema) else @schema_subset = nil diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 1a7b534b7d..39c8f09e1b 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -530,7 +530,8 @@ def subset_class end end - attr_writer :subset_class, :use_schema_visibility, :visibility + attr_writer :subset_class, :use_schema_visibility + attr_accessor :visibility def use_schema_visibility? if defined?(@use_schema_visibility) diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index e9684a06a0..827a071bc9 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -4,25 +4,52 @@ module GraphQL class Schema + # Use this plugin to make some parts of your schema hidden from some viewers. + # class Visibility - def self.use(schema, preload: nil, migration_errors: false) - schema.visibility = self.new(schema, preload: preload) + # @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?`) + # @param migration_errors [Boolean] if `true`, raise an error when `Visibility` and `Warden` return different results + def self.use(schema, dynamic: false, profiles: nil, preload: (defined?(Rails) ? Rails.env.production? : nil), migration_errors: false) + schema.visibility = self.new(schema, dynamic: dynamic, preload: preload, profiles: profiles,) schema.use_schema_visibility = true if migration_errors schema.subset_class = Migration end end - def initialize(schema, preload:) + def initialize(schema, dynamic:, preload:, profiles:) @schema = schema - @cached_subsets = {} + @profiles = profiles + @cached_profiles = {} + @dynamic = dynamic - if preload.nil? && defined?(Rails) && Rails.env.production? - preload = true + if preload + profiles.each do |profile_name, example_ctx| + prof = profile_for(example_ctx, profile_name) + prof.all_types # force loading + end end + end - if preload + attr_reader :cached_profiles + def profile_for(context, visibility_profile) + if @profiles.any? + if visibility_profile.nil? + if @dynamic + Subset.new(context: context, schema: @schema) + elsif @profiles.any? + raise ArgumentError, "#{@schema} expects a visibility profile, but `visibility_profile:` wasn't passed. Provide a `visibility_profile:` value or add `dynamic: true` to your visibility configuration." + end + elsif !@profiles.include?(visibility_profile) + raise ArgumentError, "`#{visibility_profile.inspect}` isn't allowed for `visibility_profile:` (must be one of #{@profiles.keys.map(&:inspect).join(", ")}). Or, add `#{visibility_profile.inspect}` to the list of profiles in the schema definition." + else + @cached_profiles[visibility_profile] ||= Subset.new(name: visibility_profile, context: context, schema: @schema) + end + else + Subset.new(context: context, schema: @schema) end end end diff --git a/lib/graphql/schema/visibility/subset.rb b/lib/graphql/schema/visibility/subset.rb index f653288cb5..e73ebe6611 100644 --- a/lib/graphql/schema/visibility/subset.rb +++ b/lib/graphql/schema/visibility/subset.rb @@ -13,14 +13,14 @@ class Visibility # - It checks `.visible?` on root introspection types # # In the future, {Subset} will support lazy-loading types as needed during execution and multi-request caching of subsets. + # TODO rename to Profile? class Subset # @return [Schema::Visibility::Subset] def self.from_context(ctx, schema) if ctx.respond_to?(:types) && (types = ctx.types).is_a?(self) types else - # TODO use a cached instance from the schema - self.new(context: ctx, schema: schema) + schema.visibility.profile_for(ctx, nil) end end @@ -30,7 +30,11 @@ def self.pass_thru(context:, schema:) subset end - def initialize(context:, schema:) + # @return [Symbol, nil] + attr_reader :name + + def initialize(name: nil, context:, schema:) + @name = name @context = context @schema = schema @all_types = {} diff --git a/spec/graphql/schema/visibility_spec.rb b/spec/graphql/schema/visibility_spec.rb new file mode 100644 index 0000000000..12d2e2f925 --- /dev/null +++ b/spec/graphql/schema/visibility_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true +require "spec_helper" + +describe GraphQL::Schema::Visibility do + class VisSchema < GraphQL::Schema + class BaseField < GraphQL::Schema::Field + def initialize(*args, admin_only: false, **kwargs, &block) + super(*args, **kwargs, &block) + @admin_only = admin_only + end + + def visible?(ctx) + super && if @admin_only + !!ctx[:is_admin] + else + true + end + end + end + + class BaseObject < GraphQL::Schema::Object + field_class(BaseField) + end + + class Product < BaseObject + field :name, String + field :price, Integer + field :cost_of_goods_sold, Integer, admin_only: true + field :quantity_in_stock, Integer + end + + class Query < BaseObject + field :products, [Product] + + def products + [{ name: "Pool Noodle", price: 100, cost_of_goods_sold: 5, quantity_in_stock: 100 }] + end + end + + query(Query) + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true }, edge: {} }, preload: true + end + + class DynVisSchema < VisSchema + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: {}, edge: {} }, dynamic: true, preload: false + end + + def exec_query(...) + VisSchema.execute(...) + end + describe "running queries" do + it "requires context[:visibility]" do + err = assert_raises ArgumentError do + exec_query("{ products { name } }") + end + expected_msg = "VisSchema expects a visibility profile, but `visibility_profile:` wasn't passed. Provide a `visibility_profile:` value or add `dynamic: true` to your visibility configuration." + assert_equal expected_msg, err.message + end + + it "requires a context[:visibility] which is on the list" do + err = assert_raises ArgumentError do + exec_query("{ products { name } }", visibility_profile: :nonsense ) + end + expected_msg = "`:nonsense` isn't allowed for `visibility_profile:` (must be one of :public, :admin, :edge). Or, add `:nonsense` to the list of profiles in the schema definition." + assert_equal expected_msg, err.message + end + + it "permits `nil` when nil is on the list" do + res = DynVisSchema.execute("{ products { name } }") + assert_equal 1, res["data"]["products"].size + assert_nil res.context.types.name + assert_equal [], DynVisSchema.visibility.cached_profiles.keys + end + + it "uses the named visibility" do + res = exec_query("{ products { name } }", visibility_profile: :public) + assert_equal ["Pool Noodle"], res["data"]["products"].map { |p| p["name"] } + assert_equal :public, res.context.types.name + assert res.context.types.equal?(VisSchema.visibility.cached_profiles[:public]), "It uses the cached instance" + + res = exec_query("{ products { costOfGoodsSold } }", visibility_profile: :public) + assert_equal ["Field 'costOfGoodsSold' doesn't exist on type 'Product'"], res["errors"].map { |e| e["message"] } + + res = exec_query("{ products { name costOfGoodsSold } }", visibility_profile: :admin) + assert_equal [{ "name" => "Pool Noodle", "costOfGoodsSold" => 5}], res["data"]["products"] + end + end + + describe "preloading profiles" do + it "preloads when true" do + assert_equal [:public, :admin, :edge], VisSchema.visibility.cached_profiles.keys, "preload: true" + assert_equal 0, DynVisSchema.visibility.cached_profiles.size, "preload: false" + end + end +end From 5830fb605ce9d2894f4239870bc5bedeac757e1a Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 26 Aug 2024 15:53:44 -0400 Subject: [PATCH 02/17] Start visible_in --- lib/graphql/query.rb | 5 +++ lib/graphql/schema.rb | 3 ++ lib/graphql/schema/visibility.rb | 57 ++++++++++++++++++++++++++ spec/graphql/schema/object_spec.rb | 1 + spec/graphql/schema/visibility_spec.rb | 47 ++++++++++++++++++--- 5 files changed, 107 insertions(+), 6 deletions(-) diff --git a/lib/graphql/query.rb b/lib/graphql/query.rb index 06a589dfbb..d055642cc5 100644 --- a/lib/graphql/query.rb +++ b/lib/graphql/query.rb @@ -106,6 +106,8 @@ def initialize(schema, query_string = nil, query: nil, document: nil, context: n use_schema_subset = warden ? false : schema.use_schema_visibility? end + @visibility_profile = visibility_profile + if use_schema_subset @schema_subset = @schema.visibility.profile_for(@context, visibility_profile) @warden = Schema::Warden::NullWarden.new(context: @context, schema: @schema) @@ -188,6 +190,9 @@ def query_string @query_string ||= (document ? document.to_query_string : nil) end + # @return [Symbol, nil] + attr_reader :visibility_profile + attr_accessor :multiplex # @return [GraphQL::Tracing::Trace] diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 39c8f09e1b..9789e6f317 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -316,6 +316,9 @@ def static_validator GraphQL::StaticValidation::Validator.new(schema: self) end + # Add `plugin` to this schema + # @param plugin [#use] A Schema plugin + # @return void def use(plugin, **kwargs) if kwargs.any? plugin.use(self, **kwargs) diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index 827a071bc9..8caa180816 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -27,6 +27,7 @@ def initialize(schema, dynamic:, preload:, profiles:) 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 end @@ -52,6 +53,62 @@ def profile_for(context, visibility_profile) Subset.new(context: context, schema: @schema) end end + + module TypeIntegration + def self.included(child_cls) + child_cls.extend(ClassMethods) + end + + module ClassMethods + def visible_in(profiles = NOT_CONFIGURED) + if NOT_CONFIGURED.equal?(profiles) + @visible_in + else + @visible_in = Array(profiles) + end + end + + # TODO visible? + + def inherited(child_cls) + super + if visible_in + child_cls.visible_in(visible_in) + else + child_cls.visible_in(nil) + end + end + end + end + module FieldIntegration + def self.included(child_cls) + child_cls.extend(ClassMethods) + end + + module ClassMethods + def visible_in(visible_in = NOT_CONFIGURED) + if NOT_CONFIGURED.equal?(visible_in) + @visible_in + else + @visible_in = Array(visible_in) + end + end + end + def initialize(*args, visible_in: nil, **kwargs, &block) + @visible_in = visible_in ? Array(visible_in) : nil + super(*args, **kwargs, &block) + end + + def visible?(context) + v_i = @visible_in || self.class.visible_in + if v_i + v_p = context.respond_to?(:query) ? context.query.visibility_profile : context[:visibility_profile] + super && v_i.include?(v_p) + else + super + end + end + end end end end diff --git a/spec/graphql/schema/object_spec.rb b/spec/graphql/schema/object_spec.rb index b732a68c4b..a9fbf623d7 100644 --- a/spec/graphql/schema/object_spec.rb +++ b/spec/graphql/schema/object_spec.rb @@ -375,6 +375,7 @@ def self.type_error(err, ctx) shape.delete(:@configs) shape.delete(:@future_schema) shape.delete(:@metadata) + shape.delete(:@admin_only) if type_defn_shapes.add?(shape) example_shapes_by_name[cls.graphql_name] = shape end diff --git a/spec/graphql/schema/visibility_spec.rb b/spec/graphql/schema/visibility_spec.rb index 12d2e2f925..c0c72df42b 100644 --- a/spec/graphql/schema/visibility_spec.rb +++ b/spec/graphql/schema/visibility_spec.rb @@ -26,23 +26,22 @@ class Product < BaseObject field :name, String field :price, Integer field :cost_of_goods_sold, Integer, admin_only: true - field :quantity_in_stock, Integer end class Query < BaseObject field :products, [Product] def products - [{ name: "Pool Noodle", price: 100, cost_of_goods_sold: 5, quantity_in_stock: 100 }] + [{ name: "Pool Noodle", price: 100, cost_of_goods_sold: 5 }] end end query(Query) - use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true }, edge: {} }, preload: true + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true end class DynVisSchema < VisSchema - use GraphQL::Schema::Visibility, profiles: { public: {}, admin: {}, edge: {} }, dynamic: true, preload: false + use GraphQL::Schema::Visibility, profiles: { public: {}, admin: {} }, dynamic: true, preload: false end def exec_query(...) @@ -61,7 +60,7 @@ def exec_query(...) err = assert_raises ArgumentError do exec_query("{ products { name } }", visibility_profile: :nonsense ) end - expected_msg = "`:nonsense` isn't allowed for `visibility_profile:` (must be one of :public, :admin, :edge). Or, add `:nonsense` to the list of profiles in the schema definition." + expected_msg = "`:nonsense` isn't allowed for `visibility_profile:` (must be one of :public, :admin). Or, add `:nonsense` to the list of profiles in the schema definition." assert_equal expected_msg, err.message end @@ -88,8 +87,44 @@ def exec_query(...) describe "preloading profiles" do it "preloads when true" do - assert_equal [:public, :admin, :edge], VisSchema.visibility.cached_profiles.keys, "preload: true" + assert_equal [:public, :admin], VisSchema.visibility.cached_profiles.keys, "preload: true" assert_equal 0, DynVisSchema.visibility.cached_profiles.size, "preload: false" end end + + describe "configuring named profiles" do + class NamedVisSchema < GraphQL::Schema + class BaseField < GraphQL::Schema::Field + include GraphQL::Schema::Visibility::FieldIntegration + visible_in([:public, :admin]) + end + class BaseObject < GraphQL::Schema::Object + include GraphQL::Schema::Visibility::TypeIntegration + field_class(BaseField) + visible_in([:public, :admin]) + end + class Query < BaseObject + field :i, Int, fallback_value: 1 + field :i2, Int, fallback_value: 2, visible_in: :admin + end + + query(Query) + use GraphQL::Schema::Visibility, profiles: [:public, :admin] + end + + it "runs queries by named profile" do + res = NamedVisSchema.execute("{ i }", visibility_profile: :public) + assert_equal 1, res["data"]["i"] + + res = NamedVisSchema.execute("{ i2 }", visibility_profile: :admin) + assert_equal 2, res["data"]["i2"] + + res = NamedVisSchema.execute("{ i i2 }", visibility_profile: :public) + assert_equal ["Field 'i2' doesn't exist on type 'Query'"], res["errors"].map { |e| e["message"] } + + res = NamedVisSchema.execute("{ i i2 }", visibility_profile: :admin) + assert_equal 1, res["data"]["i"] + assert_equal 2, res["data"]["i2"] + end + end end From 792bfb763bde0717bfe8ddfe2a74e3107f66ab40 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 26 Aug 2024 15:55:54 -0400 Subject: [PATCH 03/17] Fix test --- lib/graphql/schema/visibility.rb | 2 +- spec/graphql/schema/visibility/subset_spec.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index 8caa180816..8917768cfe 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -11,7 +11,7 @@ class Visibility # @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?`) # @param migration_errors [Boolean] if `true`, raise an error when `Visibility` and `Warden` return different results - def self.use(schema, dynamic: false, profiles: nil, preload: (defined?(Rails) ? Rails.env.production? : nil), migration_errors: false) + def self.use(schema, dynamic: false, profiles: EmptyObjects::EMPTY_ARRAY, preload: (defined?(Rails) ? Rails.env.production? : nil), migration_errors: false) schema.visibility = self.new(schema, dynamic: dynamic, preload: preload, profiles: profiles,) schema.use_schema_visibility = true if migration_errors diff --git a/spec/graphql/schema/visibility/subset_spec.rb b/spec/graphql/schema/visibility/subset_spec.rb index 684cbe4b3f..1262c72f07 100644 --- a/spec/graphql/schema/visibility/subset_spec.rb +++ b/spec/graphql/schema/visibility/subset_spec.rb @@ -13,6 +13,8 @@ class Query < GraphQL::Schema::Object end query(Query) + + use GraphQL::Schema::Visibility end it "only loads the types it needs" do query = GraphQL::Query.new(SubsetSchema, "{ thing { name } }", use_schema_subset: true) From a00bbbe88ec898a4b0736e3509c6b950a9c34fcd Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Wed, 4 Sep 2024 14:13:50 -0400 Subject: [PATCH 04/17] update object shape testS --- spec/graphql/schema/field_spec.rb | 3 ++- spec/graphql/schema/object_spec.rb | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/graphql/schema/field_spec.rb b/spec/graphql/schema/field_spec.rb index 129900bc34..a745467fa1 100644 --- a/spec/graphql/schema/field_spec.rb +++ b/spec/graphql/schema/field_spec.rb @@ -844,7 +844,8 @@ def resolve # end # end default_field_shape = GraphQL::Introspection::TypeType.get_field("name").instance_variables - assert_equal [default_field_shape], shapes.to_a + default_visibility_field_shape = Class.new(GraphQL::Schema::Field) { include(GraphQL::Schema::Visibility::FieldIntegration) }.instance_variables + assert_equal [default_field_shape, default_visibility_field_shape], shapes.to_a end it "works with implicit hash key and default value" do diff --git a/spec/graphql/schema/object_spec.rb b/spec/graphql/schema/object_spec.rb index a9fbf623d7..aefc870363 100644 --- a/spec/graphql/schema/object_spec.rb +++ b/spec/graphql/schema/object_spec.rb @@ -397,12 +397,14 @@ def self.type_error(err, ctx) default_edge_shape = Class.new(GraphQL::Types::Relay::BaseEdge).instance_variables default_connection_shape = Class.new(GraphQL::Types::Relay::BaseConnection).instance_variables default_mutation_payload_shape = Class.new(GraphQL::Schema::RelayClassicMutation) { graphql_name("DoSomething") }.payload_type.instance_variables + default_visibility_shape = Class.new(GraphQL::Schema::Object) { graphql_name("Thing2"); include(GraphQL::Schema::Visibility::TypeIntegration) }.instance_variables expected_default_shapes = Set.new([ default_shape, default_shape_with_connection_type, default_edge_shape, default_connection_shape, default_mutation_payload_shape + default_visibility_shape ]) assert_equal expected_default_shapes, type_defn_shapes From 3d6b1ae4d8e87ff5e9d03579ccfd7b1d876534e5 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Wed, 4 Sep 2024 14:36:16 -0400 Subject: [PATCH 05/17] update shape tests again --- spec/graphql/schema/field_spec.rb | 10 ++++++++-- spec/graphql/schema/object_spec.rb | 3 ++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/spec/graphql/schema/field_spec.rb b/spec/graphql/schema/field_spec.rb index 10a3c6f02c..003a50285b 100644 --- a/spec/graphql/schema/field_spec.rb +++ b/spec/graphql/schema/field_spec.rb @@ -834,7 +834,7 @@ def resolve shapes = Set.new # This is custom state added by some test schemas: - custom_ivars = [:@upcase, :@future_schema, :@visible, :@allow_for, :@metadata] + custom_ivars = [:@upcase, :@future_schema, :@visible, :@allow_for, :@metadata, :@admin_only] ObjectSpace.each_object(GraphQL::Schema::Field) do |field_obj| field_ivars = field_obj.instance_variables @@ -852,7 +852,13 @@ def resolve # end # end default_field_shape = GraphQL::Introspection::TypeType.get_field("name").instance_variables - default_visibility_field_shape = Class.new(GraphQL::Schema::Field) { include(GraphQL::Schema::Visibility::FieldIntegration) }.instance_variables + vis_field = Class.new(GraphQL::Schema::Field) { include(GraphQL::Schema::Visibility::FieldIntegration) } + vis_field_owner = Class.new(GraphQL::Schema::Object) do + graphql_name "Thing" + field_class(vis_field) + field :x, String, visible_in: :x + end + default_visibility_field_shape = vis_field_owner.get_field("x").instance_variables assert_equal [default_field_shape, default_visibility_field_shape], shapes.to_a end diff --git a/spec/graphql/schema/object_spec.rb b/spec/graphql/schema/object_spec.rb index dea567fb84..6b81eabbe0 100644 --- a/spec/graphql/schema/object_spec.rb +++ b/spec/graphql/schema/object_spec.rb @@ -397,7 +397,7 @@ def self.type_error(err, ctx) default_edge_shape = Class.new(GraphQL::Types::Relay::BaseEdge).instance_variables default_connection_shape = Class.new(GraphQL::Types::Relay::BaseConnection).instance_variables default_mutation_payload_shape = Class.new(GraphQL::Schema::RelayClassicMutation) { graphql_name("DoSomething") }.payload_type.instance_variables - default_visibility_shape = Class.new(GraphQL::Schema::Object) { graphql_name("Thing2"); include(GraphQL::Schema::Visibility::TypeIntegration) }.instance_variables + default_visibility_shape = Class.new(GraphQL::Schema::Object) { include(GraphQL::Schema::Visibility::TypeIntegration); visible_in(:x) }.instance_variables expected_default_shapes = [ default_shape, default_shape_with_connection_type, @@ -421,6 +421,7 @@ def self.type_error(err, ctx) name = example_shapes_by_name.key(shape) extra_shapes_by_name[name] = shape end + assert_equal({}, extra_shapes_by_name, "There aren't any extra shape profiles") end From d52198b9dca415a2b3bb70910e2b3e22c3f8685d Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 9 Sep 2024 15:11:16 -0400 Subject: [PATCH 06/17] Fix tests --- lib/graphql/schema.rb | 20 +++++++---- lib/graphql/schema/argument.rb | 1 + lib/graphql/schema/build_from_definition.rb | 1 + lib/graphql/schema/visibility.rb | 37 ++++++++++++++++----- lib/graphql/schema/visibility/migration.rb | 8 +++-- lib/graphql/schema/visibility/subset.rb | 26 +++++++++++---- lib/graphql/schema/warden.rb | 4 +-- spec/graphql/query/result_spec.rb | 8 +++-- spec/graphql/schema/timeout_spec.rb | 2 +- 9 files changed, 75 insertions(+), 32 deletions(-) diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 9789e6f317..419befd4bc 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -337,7 +337,8 @@ def plugins # @see get_type Which is more efficient for finding _one type_ by name, because it doesn't merge hashes. def types(context = GraphQL::Query::NullContext.instance) if use_schema_visibility? - return Visibility::Subset.from_context(context, self).all_types_h + types = Visibility::Subset.from_context(context, self) + return types.all_types_h end all_types = non_introspection_types.merge(introspection_system.types) visible_types = {} @@ -365,8 +366,8 @@ def types(context = GraphQL::Query::NullContext.instance) # @param type_name [String] # @return [Module, nil] A type, or nil if there's no type called `type_name` - def get_type(type_name, context = GraphQL::Query::NullContext.instance) - if use_schema_visibility? + def get_type(type_name, context = GraphQL::Query::NullContext.instance, use_schema_visibility: use_schema_visibility?) + if use_schema_visibility return Visibility::Subset.from_context(context, self).type(type_name) end local_entry = own_types[type_name] @@ -400,7 +401,7 @@ def get_type(type_name, context = GraphQL::Query::NullContext.instance) type_defn || introspection_system.types[type_name] || # todo context-specific introspection? - (superclass.respond_to?(:get_type) ? superclass.get_type(type_name, context) : nil) + (superclass.respond_to?(:get_type) ? superclass.get_type(type_name, context, use_schema_visibility: use_schema_visibility) : nil) end # @return [Boolean] Does this schema have _any_ definition for a type named `type_name`, regardless of visibility? @@ -549,8 +550,8 @@ def use_schema_visibility? # @param type [Module] The type definition whose possible types you want to see # @return [Hash] All possible types, if no `type` is given. # @return [Array] Possible types for `type`, if it's given. - def possible_types(type = nil, context = GraphQL::Query::NullContext.instance) - if use_schema_visibility? + def possible_types(type = nil, context = GraphQL::Query::NullContext.instance, use_schema_visibility: use_schema_visibility?) + if use_schema_visibility if type return Visibility::Subset.from_context(context, self).possible_types(type) else @@ -574,7 +575,7 @@ def possible_types(type = nil, context = GraphQL::Query::NullContext.instance) introspection_system.possible_types[type] || ( superclass.respond_to?(:possible_types) ? - superclass.possible_types(type, context) : + superclass.possible_types(type, context, use_schema_visibility: use_schema_visibility) : EMPTY_ARRAY ) end @@ -1072,6 +1073,11 @@ def inherited(child_class) child_class.own_trace_modes[name] = child_class.build_trace_mode(name) end child_class.singleton_class.prepend(ResolveTypeWithType) + + if use_schema_visibility? + vis = self.visibility + child_class.visibility = vis.dup_for(child_class) + end super end diff --git a/lib/graphql/schema/argument.rb b/lib/graphql/schema/argument.rb index 6f2a2d65e6..5fdd03c1ad 100644 --- a/lib/graphql/schema/argument.rb +++ b/lib/graphql/schema/argument.rb @@ -364,6 +364,7 @@ def load_and_authorize_value(load_method_owner, coerced_value, context) # @api private def validate_default_value + return unless default_value? coerced_default_value = begin # This is weird, but we should accept single-item default values for list-type arguments. # If we used `coerce_isolated_input` below, it would do this for us, but it's not really diff --git a/lib/graphql/schema/build_from_definition.rb b/lib/graphql/schema/build_from_definition.rb index 1640e372ac..7db47a30a3 100644 --- a/lib/graphql/schema/build_from_definition.rb +++ b/lib/graphql/schema/build_from_definition.rb @@ -188,6 +188,7 @@ def definition_default_resolve def self.inherited(child_class) child_class.definition_default_resolve = self.definition_default_resolve + super end end diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index 8917768cfe..806e2d4100 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -12,45 +12,64 @@ class Visibility # @param preload [Boolean] if `true`, load the default schema profile and all named profiles immediately (defaults to `true` for `Rails.env.production?`) # @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_ARRAY, preload: (defined?(Rails) ? Rails.env.production? : nil), migration_errors: false) - schema.visibility = self.new(schema, dynamic: dynamic, preload: preload, profiles: profiles,) + schema.visibility = self.new(schema, dynamic: dynamic, preload: preload, profiles: profiles, migration_errors: migration_errors) + end + + def initialize(schema, dynamic:, preload:, profiles:, migration_errors:) + @schema = schema schema.use_schema_visibility = true if migration_errors schema.subset_class = Migration end - end - - def initialize(schema, dynamic:, preload:, profiles:) - @schema = schema @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) + query_ctx = GraphQL::Query.new(@schema, "{ __typename }", context: example_ctx).context + prof = profile_for(query_ctx, profile_name) prof.all_types # force loading end end end + # Make another Visibility for `schema` based on this one + # @return [Visibility] + # @api private + def dup_for(other_schema) + self.class.new( + other_schema, + dynamic: @dynamic, + preload: @preload, + profiles: @profiles, + migration_errors: @migration_errors + ) + end + + def migration_errors? + @migration_errors + end + attr_reader :cached_profiles def profile_for(context, visibility_profile) if @profiles.any? if visibility_profile.nil? if @dynamic - Subset.new(context: context, schema: @schema) + @schema.subset_class.new(context: context, schema: @schema) elsif @profiles.any? raise ArgumentError, "#{@schema} expects a visibility profile, but `visibility_profile:` wasn't passed. Provide a `visibility_profile:` value or add `dynamic: true` to your visibility configuration." end elsif !@profiles.include?(visibility_profile) raise ArgumentError, "`#{visibility_profile.inspect}` isn't allowed for `visibility_profile:` (must be one of #{@profiles.keys.map(&:inspect).join(", ")}). Or, add `#{visibility_profile.inspect}` to the list of profiles in the schema definition." else - @cached_profiles[visibility_profile] ||= Subset.new(name: visibility_profile, context: context, schema: @schema) + @cached_profiles[visibility_profile] ||= @schema.subset_class.new(name: visibility_profile, context: context, schema: @schema) end else - Subset.new(context: context, schema: @schema) + @schema.subset_class.new(context: context, schema: @schema) end end diff --git a/lib/graphql/schema/visibility/migration.rb b/lib/graphql/schema/visibility/migration.rb index 618b42539d..4e995582ca 100644 --- a/lib/graphql/schema/visibility/migration.rb +++ b/lib/graphql/schema/visibility/migration.rb @@ -80,11 +80,11 @@ def humanize(val) end end - def initialize(context:, schema:) - @skip_error = context[:skip_visibility_migration_error] - context[:visibility_migration_running] = true + def initialize(context:, schema:, name: nil) + @skip_error = context[:skip_visibility_migration_error] || context.is_a?(Query::NullContext) || context.is_a?(Hash) @subset_types = GraphQL::Schema::Visibility::Subset.new(context: context, schema: schema) if !@skip_error + context[:visibility_migration_running] = true warden_ctx_vals = context.to_h.dup warden_ctx_vals[:visibility_migration_warden_running] = true if defined?(schema::WardenCompatSchema) @@ -95,6 +95,7 @@ def initialize(context:, schema:) # TODO public API warden_schema.send(:add_type_and_traverse, [warden_schema.query, warden_schema.mutation, warden_schema.subscription].compact, root: true) warden_schema.send(:add_type_and_traverse, warden_schema.directives.values + warden_schema.orphan_types, root: false) + schema.const_set(:WardenCompatSchema, warden_schema) end warden_ctx = GraphQL::Query::Context.new(query: context.query, values: warden_ctx_vals) example_warden = GraphQL::Schema::Warden.new(schema: warden_schema, context: warden_ctx) @@ -112,6 +113,7 @@ def loaded_types :enum_values, :interfaces, :all_types, + :all_types_h, :fields, :loadable?, :type, diff --git a/lib/graphql/schema/visibility/subset.rb b/lib/graphql/schema/visibility/subset.rb index e73ebe6611..dc94dbfab1 100644 --- a/lib/graphql/schema/visibility/subset.rb +++ b/lib/graphql/schema/visibility/subset.rb @@ -71,6 +71,7 @@ def initialize(name: nil, context:, schema:) @cached_visible_arguments = Hash.new do |h, arg| h[arg] = if @cached_visible[arg] && (arg_type = arg.type.unwrap) && @cached_visible[arg_type] add_type(arg_type, arg) + arg.validate_default_value true else false @@ -407,8 +408,9 @@ def load_all_types @unfiltered_interface_type_memberships = Hash.new { |h, k| h[k] = [] }.compare_by_identity @add_possible_types = Set.new + @late_types = [] - while @unvisited_types.any? + while @unvisited_types.any? || @late_types.any? while t = @unvisited_types.pop # These have already been checked for `.visible?` visit_type(t) @@ -422,6 +424,12 @@ def load_all_types end end @add_possible_types.clear + + while (union_tm = @late_types.shift) + late_obj_t = union_tm.object_type + obj_t = @all_types[late_obj_t.graphql_name] || raise("Failed to resolve #{late_obj_t.graphql_name.inspect} from #{union_tm.inspect}") + union_tm.abstract_type.assign_type_membership_object_type(obj_t) + end end @all_types.delete_if { |type_name, type_defn| !referenced?(type_defn) } @@ -474,12 +482,16 @@ def visit_type(type) type.type_memberships.each do |tm| if @cached_visible[tm] obj_t = tm.object_type - if obj_t.is_a?(String) - obj_t = Member::BuildType.constantize(obj_t) - tm.object_type = obj_t - end - if @cached_visible[obj_t] - add_type(obj_t, tm) + if obj_t.is_a?(GraphQL::Schema::LateBoundType) + @late_types << tm + else + if obj_t.is_a?(String) + obj_t = Member::BuildType.constantize(obj_t) + tm.object_type = obj_t + end + if @cached_visible[obj_t] + add_type(obj_t, tm) + end end end end diff --git a/lib/graphql/schema/warden.rb b/lib/graphql/schema/warden.rb index c9d5dfa8a5..be86f73961 100644 --- a/lib/graphql/schema/warden.rb +++ b/lib/graphql/schema/warden.rb @@ -218,7 +218,7 @@ def loadable?(type, _ctx) # @return [GraphQL::BaseType, nil] The type named `type_name`, if it exists (else `nil`) def get_type(type_name) @visible_types ||= read_through do |name| - type_defn = @schema.get_type(name, @context) + type_defn = @schema.get_type(name, @context, use_schema_visibility: false) if type_defn && visible_and_reachable_type?(type_defn) type_defn else @@ -265,7 +265,7 @@ def get_argument(parent_type, argument_name) # @return [Array] The types which may be member of `type_defn` def possible_types(type_defn) @visible_possible_types ||= read_through { |type_defn| - pt = @schema.possible_types(type_defn, @context) + pt = @schema.possible_types(type_defn, @context, use_schema_visibility: false) pt.select { |t| visible_and_reachable_type?(t) } } @visible_possible_types[type_defn] diff --git a/spec/graphql/query/result_spec.rb b/spec/graphql/query/result_spec.rb index 12edfe68dd..5c8126485a 100644 --- a/spec/graphql/query/result_spec.rb +++ b/spec/graphql/query/result_spec.rb @@ -24,10 +24,12 @@ it "exposes the context" do assert_instance_of GraphQL::Query::Context, result.context - if GraphQL::Schema.use_schema_visibility? - assert_equal({a: :b, visibility_migration_running: true}, result.context.to_h) + expected_ctx = if GraphQL::Schema.use_schema_visibility? && result.context.schema.visibility.migration_errors? + {a: :b, visibility_migration_running: true} else - assert_equal({a: :b}, result.context.to_h) + {a: :b} end + + assert_equal(expected_ctx, result.context.to_h) end end diff --git a/spec/graphql/schema/timeout_spec.rb b/spec/graphql/schema/timeout_spec.rb index 51949dacbf..91a6213960 100644 --- a/spec/graphql/schema/timeout_spec.rb +++ b/spec/graphql/schema/timeout_spec.rb @@ -21,7 +21,7 @@ def seconds object end - field :nested_sleep, GraphQL::Schema::LateBoundType.new(graphql_name) do + field :nested_sleep, self do argument :seconds, Float end From bb05e1571cc978ec86e858ff4cf82c418d54e45a Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 9 Sep 2024 15:20:49 -0400 Subject: [PATCH 07/17] Fix vis initialization --- lib/graphql/schema/visibility.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index 806e2d4100..30270532dc 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -25,12 +25,10 @@ def initialize(schema, dynamic:, preload:, profiles:, migration_errors:) @cached_profiles = {} @dynamic = dynamic @migration_errors = migration_errors - if preload profiles.each do |profile_name, example_ctx| - example_ctx ||= { visibility_profile: profile_name } - query_ctx = GraphQL::Query.new(@schema, "{ __typename }", context: example_ctx).context - prof = profile_for(query_ctx, profile_name) + example_ctx[:visibility_profile] = profile_name + prof = profile_for(example_ctx, profile_name) prof.all_types # force loading end end From 213253e01be21d26b0f1f2a57b88b6de33d7aa9b Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 9 Sep 2024 15:31:09 -0400 Subject: [PATCH 08/17] update test --- spec/graphql/schema_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/graphql/schema_spec.rb b/spec/graphql/schema_spec.rb index 50e5f679d5..34298251c2 100644 --- a/spec/graphql/schema_spec.rb +++ b/spec/graphql/schema_spec.rb @@ -205,7 +205,7 @@ def self.reset_calls end METHODS_TO_CACHE.each do |method_name, allowed_calls| - define_singleton_method(method_name) do |*args, &block| + define_singleton_method(method_name) do |*args, **kwargs, &block| if @calls call_count = @calls[method_name] += 1 @callers[method_name] << caller @@ -215,7 +215,7 @@ def self.reset_calls if call_count > allowed_calls raise "Called #{method_name} more than #{allowed_calls} times, previous caller: \n#{@callers[method_name].first.join("\n")}" end - super(*args, &block) + super(*args, **kwargs, &block) end end end From 37f508111b3ae6ae91a246568dbed679c7306e84 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 24 Sep 2024 11:58:41 -0400 Subject: [PATCH 09/17] Use a positional arg for Ruby 2.7 compat --- lib/graphql/schema.rb | 12 ++++++++---- lib/graphql/schema/warden.rb | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 419befd4bc..484f4cf452 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -365,8 +365,10 @@ def types(context = GraphQL::Query::NullContext.instance) end # @param type_name [String] + # @param context [GraphQL::Query::Context] Used for filtering definitions at query-time + # @param use_schema_visibility Private, for migration to {Schema::Visibility} # @return [Module, nil] A type, or nil if there's no type called `type_name` - def get_type(type_name, context = GraphQL::Query::NullContext.instance, use_schema_visibility: use_schema_visibility?) + def get_type(type_name, context = GraphQL::Query::NullContext.instance, use_schema_visibility = use_schema_visibility?) if use_schema_visibility return Visibility::Subset.from_context(context, self).type(type_name) end @@ -401,7 +403,7 @@ def get_type(type_name, context = GraphQL::Query::NullContext.instance, use_sche type_defn || introspection_system.types[type_name] || # todo context-specific introspection? - (superclass.respond_to?(:get_type) ? superclass.get_type(type_name, context, use_schema_visibility: use_schema_visibility) : nil) + (superclass.respond_to?(:get_type) ? superclass.get_type(type_name, context, use_schema_visibility) : nil) end # @return [Boolean] Does this schema have _any_ definition for a type named `type_name`, regardless of visibility? @@ -548,9 +550,11 @@ def use_schema_visibility? end # @param type [Module] The type definition whose possible types you want to see + # @param context [GraphQL::Query::Context] used for filtering visible possible types at runtime + # @param use_schema_visibility Private, for migration to {Schema::Visibility} # @return [Hash] All possible types, if no `type` is given. # @return [Array] Possible types for `type`, if it's given. - def possible_types(type = nil, context = GraphQL::Query::NullContext.instance, use_schema_visibility: use_schema_visibility?) + def possible_types(type = nil, context = GraphQL::Query::NullContext.instance, use_schema_visibility = use_schema_visibility?) if use_schema_visibility if type return Visibility::Subset.from_context(context, self).possible_types(type) @@ -575,7 +579,7 @@ def possible_types(type = nil, context = GraphQL::Query::NullContext.instance, u introspection_system.possible_types[type] || ( superclass.respond_to?(:possible_types) ? - superclass.possible_types(type, context, use_schema_visibility: use_schema_visibility) : + superclass.possible_types(type, context, use_schema_visibility) : EMPTY_ARRAY ) end diff --git a/lib/graphql/schema/warden.rb b/lib/graphql/schema/warden.rb index be86f73961..4add9e4f64 100644 --- a/lib/graphql/schema/warden.rb +++ b/lib/graphql/schema/warden.rb @@ -218,7 +218,7 @@ def loadable?(type, _ctx) # @return [GraphQL::BaseType, nil] The type named `type_name`, if it exists (else `nil`) def get_type(type_name) @visible_types ||= read_through do |name| - type_defn = @schema.get_type(name, @context, use_schema_visibility: false) + type_defn = @schema.get_type(name, @context, false) if type_defn && visible_and_reachable_type?(type_defn) type_defn else @@ -265,7 +265,7 @@ def get_argument(parent_type, argument_name) # @return [Array] The types which may be member of `type_defn` def possible_types(type_defn) @visible_possible_types ||= read_through { |type_defn| - pt = @schema.possible_types(type_defn, @context, use_schema_visibility: false) + pt = @schema.possible_types(type_defn, @context, false) pt.select { |t| visible_and_reachable_type?(t) } } @visible_possible_types[type_defn] From 122272d1b831fc6d4b11ff223e24366a54554189 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 3 Oct 2024 15:55:05 -0400 Subject: [PATCH 10/17] Fix some tests --- lib/graphql/query.rb | 25 +++++++++++++++++++++- lib/graphql/schema/always_visible.rb | 9 +++++--- lib/graphql/schema/visibility/migration.rb | 10 ++++----- lib/graphql/schema/warden.rb | 4 ++-- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/lib/graphql/query.rb b/lib/graphql/query.rb index d055642cc5..8aa1d75d90 100644 --- a/lib/graphql/query.rb +++ b/lib/graphql/query.rb @@ -349,7 +349,30 @@ def warden with_prepared_ast { @warden } end - def_delegators :warden, :get_type, :get_field, :possible_types, :root_type_for_operation + def get_type(type_name) + types.type(type_name) # rubocop:disable Development/ContextIsPassedCop + end + + def get_field(owner, field_name) + types.field(owner, field_name) # rubocop:disable Development/ContextIsPassedCop + end + + def possible_types(type) + types.possible_types(type) # rubocop:disable Development/ContextIsPassedCop + end + + def root_type_for_operation(op_type) + case op_type + when "query" + types.query_root # rubocop:disable Development/ContextIsPassedCop + when "mutation" + types.mutation_root # rubocop:disable Development/ContextIsPassedCop + when "subscription" + types.subscription_root # rubocop:disable Development/ContextIsPassedCop + else + raise ArgumentError, "unexpected root type name: #{op_type.inspect}; expected 'query', 'mutation', or 'subscription'" + end + end def types @schema_subset || warden.schema_subset diff --git a/lib/graphql/schema/always_visible.rb b/lib/graphql/schema/always_visible.rb index 6feadc123c..20be78db4a 100644 --- a/lib/graphql/schema/always_visible.rb +++ b/lib/graphql/schema/always_visible.rb @@ -1,10 +1,13 @@ # frozen_string_literal: true module GraphQL class Schema - class AlwaysVisible + module AlwaysVisible def self.use(schema, **opts) - schema.warden_class = GraphQL::Schema::Warden::NullWarden - schema.subset_class = GraphQL::Schema::Warden::NullWarden::NullSubset + schema.extend(self) + end + + def visible?(_member, _context) + true end end end diff --git a/lib/graphql/schema/visibility/migration.rb b/lib/graphql/schema/visibility/migration.rb index 4e995582ca..8285b25017 100644 --- a/lib/graphql/schema/visibility/migration.rb +++ b/lib/graphql/schema/visibility/migration.rb @@ -87,8 +87,8 @@ def initialize(context:, schema:, name: nil) context[:visibility_migration_running] = true warden_ctx_vals = context.to_h.dup warden_ctx_vals[:visibility_migration_warden_running] = true - if defined?(schema::WardenCompatSchema) - warden_schema = schema::WardenCompatSchema + if schema.const_defined?(:WardenCompatSchema, false) # don't use a defn from a superclass + warden_schema = schema.const_get(:WardenCompatSchema, false) else warden_schema = Class.new(schema) warden_schema.use_schema_visibility = false @@ -98,10 +98,8 @@ def initialize(context:, schema:, name: nil) schema.const_set(:WardenCompatSchema, warden_schema) end warden_ctx = GraphQL::Query::Context.new(query: context.query, values: warden_ctx_vals) - example_warden = GraphQL::Schema::Warden.new(schema: warden_schema, context: warden_ctx) - @warden_types = example_warden.schema_subset - warden_ctx.warden = example_warden - warden_ctx.types = @warden_types + warden_ctx.warden = GraphQL::Schema::Warden.new(schema: warden_schema, context: warden_ctx) + warden_ctx.types = @warden_types = warden_ctx.warden.schema_subset end end diff --git a/lib/graphql/schema/warden.rb b/lib/graphql/schema/warden.rb index 4add9e4f64..a229305233 100644 --- a/lib/graphql/schema/warden.rb +++ b/lib/graphql/schema/warden.rb @@ -88,7 +88,7 @@ def visible_type?(type_defn, _ctx = nil); true; end def visible_enum_value?(enum_value, _ctx = nil); true; end def visible_type_membership?(type_membership, _ctx = nil); true; end def interface_type_memberships(obj_type, _ctx = nil); obj_type.interface_type_memberships; end - def get_type(type_name); @schema.get_type(type_name); end # rubocop:disable Development/ContextIsPassedCop + def get_type(type_name); @schema.get_type(type_name, Query::NullContext.instance, false); end # rubocop:disable Development/ContextIsPassedCop def arguments(argument_owner, ctx = nil); argument_owner.all_argument_definitions; end def enum_values(enum_defn); enum_defn.enum_values; end # rubocop:disable Development/ContextIsPassedCop def get_argument(parent_type, argument_name); parent_type.get_argument(argument_name); end # rubocop:disable Development/ContextIsPassedCop @@ -100,7 +100,7 @@ def get_field(parent_type, field_name); @schema.get_field(parent_type, field_nam def reachable_type?(type_name); true; end def loadable?(type, _ctx); true; end def reachable_types; @schema.types.values; end # rubocop:disable Development/ContextIsPassedCop - def possible_types(type_defn); @schema.possible_types(type_defn); end + def possible_types(type_defn); @schema.possible_types(type_defn, Query::NullContext.instance, false); end def interfaces(obj_type); obj_type.interfaces; end end From 22bec21f1da7a5dd246e526a4ed4bf37a5015e43 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 4 Oct 2024 09:21:15 -0400 Subject: [PATCH 11/17] Fix name in migration mode --- lib/graphql/schema/visibility/migration.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/graphql/schema/visibility/migration.rb b/lib/graphql/schema/visibility/migration.rb index 8285b25017..1c055126b4 100644 --- a/lib/graphql/schema/visibility/migration.rb +++ b/lib/graphql/schema/visibility/migration.rb @@ -81,6 +81,7 @@ def humanize(val) end def initialize(context:, schema:, name: nil) + @name = name @skip_error = context[:skip_visibility_migration_error] || context.is_a?(Query::NullContext) || context.is_a?(Hash) @subset_types = GraphQL::Schema::Visibility::Subset.new(context: context, schema: schema) if !@skip_error From d05874f49d703cec09794ce3c555cdb90e5b4641 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 4 Oct 2024 09:31:56 -0400 Subject: [PATCH 12/17] Update ruby version on lint --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 8fce0469b0..0675490d06 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -9,7 +9,7 @@ jobs: - uses: actions/checkout@v4 - uses: ruby/setup-ruby@v1 with: - ruby-version: 2.7 + ruby-version: 3.3 bundler-cache: true - run: bundle exec rake rubocop system_tests: From 3c0eb04dc541beb6cc2f7e83bde839731d6d87ff Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 4 Oct 2024 09:46:54 -0400 Subject: [PATCH 13/17] Lint fixes --- .rubocop.yml | 2 +- graphql.gemspec | 2 +- spec/graphql/schema/printer_spec.rb | 4 ++-- spec/graphql/schema/visibility_spec.rb | 6 +----- spec/graphql/subscriptions_spec.rb | 2 +- 5 files changed, 6 insertions(+), 10 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 0baca45b21..3b8f4a6cfa 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -8,7 +8,7 @@ require: AllCops: DisabledByDefault: true SuggestExtensions: false - TargetRubyVersion: 2.4 + TargetRubyVersion: 2.7 Exclude: - 'lib/graphql/language/lexer.rb' - 'lib/graphql/language/parser.rb' diff --git a/graphql.gemspec b/graphql.gemspec index d04e94d72f..88754f5b97 100644 --- a/graphql.gemspec +++ b/graphql.gemspec @@ -38,7 +38,7 @@ Gem::Specification.new do |s| s.add_development_dependency "minitest-reporters" s.add_development_dependency "rake" s.add_development_dependency 'rake-compiler' - s.add_development_dependency "rubocop", "1.12" # for Ruby 2.4 enforcement + s.add_development_dependency "rubocop" # website stuff s.add_development_dependency "jekyll" s.add_development_dependency "yard" diff --git a/spec/graphql/schema/printer_spec.rb b/spec/graphql/schema/printer_spec.rb index 7906daf11f..0ab7fad553 100644 --- a/spec/graphql/schema/printer_spec.rb +++ b/spec/graphql/schema/printer_spec.rb @@ -124,7 +124,7 @@ class Subscription < GraphQL::Schema::Object describe ".print_introspection_schema" do it "returns the schema as a string for the introspection types" do # From https://github.com/graphql/graphql-js/blob/6a0e00fe46951767287f2cc62e1a10b167b2eaa6/src/utilities/__tests__/schemaPrinter-test.js#L599 - expected = < Date: Fri, 4 Oct 2024 09:57:46 -0400 Subject: [PATCH 14/17] Remove visible_in --- lib/graphql/schema/visibility.rb | 56 -------------------------- spec/graphql/schema/field_spec.rb | 9 +---- spec/graphql/schema/object_spec.rb | 2 +- spec/graphql/schema/visibility_spec.rb | 36 ----------------- 4 files changed, 2 insertions(+), 101 deletions(-) diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index 30270532dc..6d4c6c7b2c 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -70,62 +70,6 @@ def profile_for(context, visibility_profile) @schema.subset_class.new(context: context, schema: @schema) end end - - module TypeIntegration - def self.included(child_cls) - child_cls.extend(ClassMethods) - end - - module ClassMethods - def visible_in(profiles = NOT_CONFIGURED) - if NOT_CONFIGURED.equal?(profiles) - @visible_in - else - @visible_in = Array(profiles) - end - end - - # TODO visible? - - def inherited(child_cls) - super - if visible_in - child_cls.visible_in(visible_in) - else - child_cls.visible_in(nil) - end - end - end - end - module FieldIntegration - def self.included(child_cls) - child_cls.extend(ClassMethods) - end - - module ClassMethods - def visible_in(visible_in = NOT_CONFIGURED) - if NOT_CONFIGURED.equal?(visible_in) - @visible_in - else - @visible_in = Array(visible_in) - end - end - end - def initialize(*args, visible_in: nil, **kwargs, &block) - @visible_in = visible_in ? Array(visible_in) : nil - super(*args, **kwargs, &block) - end - - def visible?(context) - v_i = @visible_in || self.class.visible_in - if v_i - v_p = context.respond_to?(:query) ? context.query.visibility_profile : context[:visibility_profile] - super && v_i.include?(v_p) - else - super - end - end - end end end end diff --git a/spec/graphql/schema/field_spec.rb b/spec/graphql/schema/field_spec.rb index 003a50285b..e767c3d9dd 100644 --- a/spec/graphql/schema/field_spec.rb +++ b/spec/graphql/schema/field_spec.rb @@ -852,14 +852,7 @@ def resolve # end # end default_field_shape = GraphQL::Introspection::TypeType.get_field("name").instance_variables - vis_field = Class.new(GraphQL::Schema::Field) { include(GraphQL::Schema::Visibility::FieldIntegration) } - vis_field_owner = Class.new(GraphQL::Schema::Object) do - graphql_name "Thing" - field_class(vis_field) - field :x, String, visible_in: :x - end - default_visibility_field_shape = vis_field_owner.get_field("x").instance_variables - assert_equal [default_field_shape, default_visibility_field_shape], shapes.to_a + assert_equal [default_field_shape], shapes.to_a end it "works with implicit hash key and default value" do diff --git a/spec/graphql/schema/object_spec.rb b/spec/graphql/schema/object_spec.rb index 6b81eabbe0..6f7e24f49e 100644 --- a/spec/graphql/schema/object_spec.rb +++ b/spec/graphql/schema/object_spec.rb @@ -397,7 +397,7 @@ def self.type_error(err, ctx) default_edge_shape = Class.new(GraphQL::Types::Relay::BaseEdge).instance_variables default_connection_shape = Class.new(GraphQL::Types::Relay::BaseConnection).instance_variables default_mutation_payload_shape = Class.new(GraphQL::Schema::RelayClassicMutation) { graphql_name("DoSomething") }.payload_type.instance_variables - default_visibility_shape = Class.new(GraphQL::Schema::Object) { include(GraphQL::Schema::Visibility::TypeIntegration); visible_in(:x) }.instance_variables + default_visibility_shape = Class.new(GraphQL::Schema::Object).instance_variables expected_default_shapes = [ default_shape, default_shape_with_connection_type, diff --git a/spec/graphql/schema/visibility_spec.rb b/spec/graphql/schema/visibility_spec.rb index 85b9658409..ed0ed02283 100644 --- a/spec/graphql/schema/visibility_spec.rb +++ b/spec/graphql/schema/visibility_spec.rb @@ -87,40 +87,4 @@ def exec_query(...) assert_equal 0, DynVisSchema.visibility.cached_profiles.size, "preload: false" end end - - describe "configuring named profiles" do - class NamedVisSchema < GraphQL::Schema - class BaseField < GraphQL::Schema::Field - include GraphQL::Schema::Visibility::FieldIntegration - visible_in([:public, :admin]) - end - class BaseObject < GraphQL::Schema::Object - include GraphQL::Schema::Visibility::TypeIntegration - field_class(BaseField) - visible_in([:public, :admin]) - end - class Query < BaseObject - field :i, Int, fallback_value: 1 - field :i2, Int, fallback_value: 2, visible_in: :admin - end - - query(Query) - use GraphQL::Schema::Visibility, profiles: [:public, :admin] - end - - it "runs queries by named profile" do - res = NamedVisSchema.execute("{ i }", visibility_profile: :public) - assert_equal 1, res["data"]["i"] - - res = NamedVisSchema.execute("{ i2 }", visibility_profile: :admin) - assert_equal 2, res["data"]["i2"] - - res = NamedVisSchema.execute("{ i i2 }", visibility_profile: :public) - assert_equal ["Field 'i2' doesn't exist on type 'Query'"], res["errors"].map { |e| e["message"] } - - res = NamedVisSchema.execute("{ i i2 }", visibility_profile: :admin) - assert_equal 1, res["data"]["i"] - assert_equal 2, res["data"]["i2"] - end - end end From 23c79bb7f72d7cf0ad74b0612daac22215d8af90 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 4 Oct 2024 10:20:35 -0400 Subject: [PATCH 15/17] Rename Subset => Profile --- lib/graphql/query.rb | 14 ++-- lib/graphql/query/null_context.rb | 2 +- lib/graphql/schema.rb | 71 ++++++++++--------- lib/graphql/schema/member/has_arguments.rb | 4 +- lib/graphql/schema/member/has_fields.rb | 4 +- lib/graphql/schema/visibility.rb | 12 ++-- lib/graphql/schema/visibility/migration.rb | 44 ++++++------ .../visibility/{subset.rb => profile.rb} | 14 ++-- lib/graphql/schema/warden.rb | 20 +++--- lib/graphql/testing/helpers.rb | 2 +- spec/graphql/authorization_spec.rb | 2 +- .../graphql/execution/instrumentation_spec.rb | 2 +- spec/graphql/logger_spec.rb | 8 +-- spec/graphql/query/result_spec.rb | 2 +- spec/graphql/schema/addition_spec.rb | 2 +- spec/graphql/schema/directive/flagged_spec.rb | 2 +- spec/graphql/schema/dynamic_members_spec.rb | 18 ++--- spec/graphql/schema/visibility/subset_spec.rb | 6 +- spec/graphql/schema/warden_spec.rb | 8 +-- spec/graphql/schema_spec.rb | 4 +- spec/integration/rails/graphql/schema_spec.rb | 4 +- spec/spec_helper.rb | 2 +- 22 files changed, 124 insertions(+), 123 deletions(-) rename lib/graphql/schema/visibility/{subset.rb => profile.rb} (97%) diff --git a/lib/graphql/query.rb b/lib/graphql/query.rb index 8aa1d75d90..3f55f5f5d7 100644 --- a/lib/graphql/query.rb +++ b/lib/graphql/query.rb @@ -96,23 +96,23 @@ def selected_operation_name # @param max_depth [Numeric] the maximum number of nested selections allowed for this query (falls back to schema-level value) # @param max_complexity [Numeric] the maximum field complexity for this query (falls back to schema-level value) # @param visibility_profile [Symbol] - def initialize(schema, query_string = nil, query: nil, document: nil, context: nil, variables: nil, validate: true, static_validator: nil, visibility_profile: nil, subscription_topic: nil, operation_name: nil, root_value: nil, max_depth: schema.max_depth, max_complexity: schema.max_complexity, warden: nil, use_schema_subset: nil) + def initialize(schema, query_string = nil, query: nil, document: nil, context: nil, variables: nil, validate: true, static_validator: nil, visibility_profile: nil, subscription_topic: nil, operation_name: nil, root_value: nil, max_depth: schema.max_depth, max_complexity: schema.max_complexity, warden: nil, use_visibility_profile: nil) # Even if `variables: nil` is passed, use an empty hash for simpler logic variables ||= {} @schema = schema @context = schema.context_class.new(query: self, values: context) - if use_schema_subset.nil? - use_schema_subset = warden ? false : schema.use_schema_visibility? + if use_visibility_profile.nil? + use_visibility_profile = warden ? false : schema.use_visibility_profile? end @visibility_profile = visibility_profile - if use_schema_subset - @schema_subset = @schema.visibility.profile_for(@context, visibility_profile) + if use_visibility_profile + @visibility_profile = @schema.visibility.profile_for(@context, visibility_profile) @warden = Schema::Warden::NullWarden.new(context: @context, schema: @schema) else - @schema_subset = nil + @visibility_profile = nil @warden = warden end @@ -375,7 +375,7 @@ def root_type_for_operation(op_type) end def types - @schema_subset || warden.schema_subset + @visibility_profile || warden.visibility_profile end # @param abstract_type [GraphQL::UnionType, GraphQL::InterfaceType] diff --git a/lib/graphql/query/null_context.rb b/lib/graphql/query/null_context.rb index 814d981bd2..96d270c73b 100644 --- a/lib/graphql/query/null_context.rb +++ b/lib/graphql/query/null_context.rb @@ -28,7 +28,7 @@ def initialize end def types - @types ||= GraphQL::Schema::Warden::SchemaSubset.new(@warden) + @types ||= Schema::Warden::VisibilityProfile.new(@warden) end end end diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 484f4cf452..ed64a330af 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -336,8 +336,8 @@ def plugins # @return [Hash Class>] A dictionary of type classes by their GraphQL name # @see get_type Which is more efficient for finding _one type_ by name, because it doesn't merge hashes. def types(context = GraphQL::Query::NullContext.instance) - if use_schema_visibility? - types = Visibility::Subset.from_context(context, self) + if use_visibility_profile? + types = Visibility::Profile.from_context(context, self) return types.all_types_h end all_types = non_introspection_types.merge(introspection_system.types) @@ -366,18 +366,18 @@ def types(context = GraphQL::Query::NullContext.instance) # @param type_name [String] # @param context [GraphQL::Query::Context] Used for filtering definitions at query-time - # @param use_schema_visibility Private, for migration to {Schema::Visibility} + # @param use_visibility_profile Private, for migration to {Schema::Visibility} # @return [Module, nil] A type, or nil if there's no type called `type_name` - def get_type(type_name, context = GraphQL::Query::NullContext.instance, use_schema_visibility = use_schema_visibility?) - if use_schema_visibility - return Visibility::Subset.from_context(context, self).type(type_name) + def get_type(type_name, context = GraphQL::Query::NullContext.instance, use_visibility_profile = use_visibility_profile?) + if use_visibility_profile + return Visibility::Profile.from_context(context, self).type(type_name) end local_entry = own_types[type_name] type_defn = case local_entry when nil nil when Array - if context.respond_to?(:types) && context.types.is_a?(GraphQL::Schema::Visibility::Subset) + if context.respond_to?(:types) && context.types.is_a?(GraphQL::Schema::Visibility::Profile) local_entry else visible_t = nil @@ -403,7 +403,7 @@ def get_type(type_name, context = GraphQL::Query::NullContext.instance, use_sche type_defn || introspection_system.types[type_name] || # todo context-specific introspection? - (superclass.respond_to?(:get_type) ? superclass.get_type(type_name, context, use_schema_visibility) : nil) + (superclass.respond_to?(:get_type) ? superclass.get_type(type_name, context, use_visibility_profile) : nil) end # @return [Boolean] Does this schema have _any_ definition for a type named `type_name`, regardless of visibility? @@ -435,7 +435,7 @@ def query(new_query_object = nil, &lazy_load_block) if @query_object 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_schema_visibility? + elsif use_visibility_profile? @query_object = block_given? ? lazy_load_block : new_query_object else @query_object = new_query_object || lazy_load_block.call @@ -454,7 +454,7 @@ def mutation(new_mutation_object = nil, &lazy_load_block) if @mutation_object 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_schema_visibility? + elsif use_visibility_profile? @mutation_object = block_given? ? lazy_load_block : new_mutation_object else @mutation_object = new_mutation_object || lazy_load_block.call @@ -473,7 +473,7 @@ def subscription(new_subscription_object = nil, &lazy_load_block) if @subscription_object 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_schema_visibility? + elsif use_visibility_profile? @subscription_object = block_given? ? lazy_load_block : new_subscription_object add_subscription_extension_if_necessary else @@ -507,7 +507,7 @@ def root_type_for_operation(operation) end def root_types - if use_schema_visibility? + if use_visibility_profile? [query, mutation, subscription].compact else @root_types @@ -526,24 +526,27 @@ def warden_class attr_writer :warden_class - def subset_class - if defined?(@subset_class) - @subset_class - elsif superclass.respond_to?(:subset_class) - superclass.subset_class + # @api private + def visibility_profile_class + if defined?(@visibility_profile_class) + @visibility_profile_class + elsif superclass.respond_to?(:visibility_profile_class) + superclass.visibility_profile_class else - GraphQL::Schema::Visibility::Subset + GraphQL::Schema::Visibility::Profile end end - attr_writer :subset_class, :use_schema_visibility + # @api private + attr_writer :visibility_profile_class, :use_visibility_profile + # @api private attr_accessor :visibility - - def use_schema_visibility? - if defined?(@use_schema_visibility) - @use_schema_visibility - elsif superclass.respond_to?(:use_schema_visibility?) - superclass.use_schema_visibility? + # @api private + def use_visibility_profile? + if defined?(@use_visibility_profile) + @use_visibility_profile + elsif superclass.respond_to?(:use_visibility_profile?) + superclass.use_visibility_profile? else false end @@ -551,15 +554,15 @@ def use_schema_visibility? # @param type [Module] The type definition whose possible types you want to see # @param context [GraphQL::Query::Context] used for filtering visible possible types at runtime - # @param use_schema_visibility Private, for migration to {Schema::Visibility} + # @param use_visibility_profile Private, for migration to {Schema::Visibility} # @return [Hash] All possible types, if no `type` is given. # @return [Array] Possible types for `type`, if it's given. - def possible_types(type = nil, context = GraphQL::Query::NullContext.instance, use_schema_visibility = use_schema_visibility?) - if use_schema_visibility + def possible_types(type = nil, context = GraphQL::Query::NullContext.instance, use_visibility_profile = use_visibility_profile?) + if use_visibility_profile if type - return Visibility::Subset.from_context(context, self).possible_types(type) + return Visibility::Profile.from_context(context, self).possible_types(type) else - raise "Schema.possible_types is not implemented for `use_schema_visibility?`" + raise "Schema.possible_types is not implemented for `use_visibility_profile?`" end end if type @@ -579,7 +582,7 @@ def possible_types(type = nil, context = GraphQL::Query::NullContext.instance, u introspection_system.possible_types[type] || ( superclass.respond_to?(:possible_types) ? - superclass.possible_types(type, context, use_schema_visibility) : + superclass.possible_types(type, context, use_visibility_profile) : EMPTY_ARRAY ) end @@ -935,7 +938,7 @@ def orphan_types(*new_orphan_types) To add other types to your schema, you might want `extra_types`: https://graphql-ruby.org/schema/definition.html#extra-types ERR end - add_type_and_traverse(new_orphan_types, root: false) unless use_schema_visibility? + add_type_and_traverse(new_orphan_types, root: false) unless use_visibility_profile? own_orphan_types.concat(new_orphan_types.flatten) end @@ -1078,7 +1081,7 @@ def inherited(child_class) end child_class.singleton_class.prepend(ResolveTypeWithType) - if use_schema_visibility? + if use_visibility_profile? vis = self.visibility child_class.visibility = vis.dup_for(child_class) end @@ -1199,7 +1202,7 @@ def directives(*new_directives) # @param new_directive [Class] # @return void def directive(new_directive) - if use_schema_visibility? + if use_visibility_profile? own_directives[new_directive.graphql_name] = new_directive else add_type_and_traverse(new_directive, root: false) diff --git a/lib/graphql/schema/member/has_arguments.rb b/lib/graphql/schema/member/has_arguments.rb index 8548166fa9..126c406ab8 100644 --- a/lib/graphql/schema/member/has_arguments.rb +++ b/lib/graphql/schema/member/has_arguments.rb @@ -135,7 +135,7 @@ def all_argument_definitions def get_argument(argument_name, context = GraphQL::Query::NullContext.instance) warden = Warden.from_context(context) - skip_visible = context.respond_to?(:types) && context.types.is_a?(GraphQL::Schema::Visibility::Subset) + skip_visible = context.respond_to?(:types) && context.types.is_a?(GraphQL::Schema::Visibility::Profile) for ancestor in ancestors if ancestor.respond_to?(:own_arguments) && (a = ancestor.own_arguments[argument_name]) && @@ -210,7 +210,7 @@ def all_argument_definitions # @return [GraphQL::Schema::Argument, nil] Argument defined on this thing, fetched by name. def get_argument(argument_name, context = GraphQL::Query::NullContext.instance) warden = Warden.from_context(context) - if (arg_config = own_arguments[argument_name]) && ((context.respond_to?(:types) && context.types.is_a?(GraphQL::Schema::Visibility::Subset)) || (visible_arg = Warden.visible_entry?(:visible_argument?, arg_config, context, warden))) + if (arg_config = own_arguments[argument_name]) && ((context.respond_to?(:types) && context.types.is_a?(GraphQL::Schema::Visibility::Profile)) || (visible_arg = Warden.visible_entry?(:visible_argument?, arg_config, context, warden))) visible_arg || arg_config elsif defined?(@resolver_class) && @resolver_class @resolver_class.get_field_argument(argument_name, context) diff --git a/lib/graphql/schema/member/has_fields.rb b/lib/graphql/schema/member/has_fields.rb index da7dcd16d5..3a5065500f 100644 --- a/lib/graphql/schema/member/has_fields.rb +++ b/lib/graphql/schema/member/has_fields.rb @@ -99,7 +99,7 @@ def all_field_definitions module InterfaceMethods def get_field(field_name, context = GraphQL::Query::NullContext.instance) warden = Warden.from_context(context) - skip_visible = context.respond_to?(:types) && context.types.is_a?(GraphQL::Schema::Visibility::Subset) + skip_visible = context.respond_to?(:types) && context.types.is_a?(GraphQL::Schema::Visibility::Profile) for ancestor in ancestors if ancestor.respond_to?(:own_fields) && (f_entry = ancestor.own_fields[field_name]) && @@ -135,7 +135,7 @@ def get_field(field_name, context = GraphQL::Query::NullContext.instance) # Objects need to check that the interface implementation is visible, too warden = Warden.from_context(context) ancs = ancestors - skip_visible = context.respond_to?(:types) && context.types.is_a?(GraphQL::Schema::Visibility::Subset) + skip_visible = context.respond_to?(:types) && context.types.is_a?(GraphQL::Schema::Visibility::Profile) i = 0 while (ancestor = ancs[i]) if ancestor.respond_to?(:own_fields) && diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index 6d4c6c7b2c..972c2d0ed7 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true -require "graphql/schema/visibility/subset" +require "graphql/schema/visibility/profile" require "graphql/schema/visibility/migration" module GraphQL @@ -17,9 +17,9 @@ def self.use(schema, dynamic: false, profiles: EmptyObjects::EMPTY_ARRAY, preloa def initialize(schema, dynamic:, preload:, profiles:, migration_errors:) @schema = schema - schema.use_schema_visibility = true + schema.use_visibility_profile = true if migration_errors - schema.subset_class = Migration + schema.visibility_profile_class = Migration end @profiles = profiles @cached_profiles = {} @@ -57,17 +57,17 @@ def profile_for(context, visibility_profile) if @profiles.any? if visibility_profile.nil? if @dynamic - @schema.subset_class.new(context: context, schema: @schema) + @schema.visibility_profile_class.new(context: context, schema: @schema) elsif @profiles.any? raise ArgumentError, "#{@schema} expects a visibility profile, but `visibility_profile:` wasn't passed. Provide a `visibility_profile:` value or add `dynamic: true` to your visibility configuration." end elsif !@profiles.include?(visibility_profile) raise ArgumentError, "`#{visibility_profile.inspect}` isn't allowed for `visibility_profile:` (must be one of #{@profiles.keys.map(&:inspect).join(", ")}). Or, add `#{visibility_profile.inspect}` to the list of profiles in the schema definition." else - @cached_profiles[visibility_profile] ||= @schema.subset_class.new(name: visibility_profile, context: context, schema: @schema) + @cached_profiles[visibility_profile] ||= @schema.visibility_profile_class.new(name: visibility_profile, context: context, schema: @schema) end else - @schema.subset_class.new(context: context, schema: @schema) + @schema.visibility_profile_class.new(context: context, schema: @schema) end end end diff --git a/lib/graphql/schema/visibility/migration.rb b/lib/graphql/schema/visibility/migration.rb index 1c055126b4..7a9afe8493 100644 --- a/lib/graphql/schema/visibility/migration.rb +++ b/lib/graphql/schema/visibility/migration.rb @@ -2,7 +2,7 @@ module GraphQL class Schema class Visibility - # You can use this to see how {GraphQL::Schema::Warden} and {GraphQL::Schema::Visibility::Subset} + # You can use this to see how {GraphQL::Schema::Warden} and {GraphQL::Schema::Visibility::Profile} # handle `.visible?` differently in your schema. # # It runs the same method on both implementations and raises an error when the results diverge. @@ -15,28 +15,28 @@ class Visibility # This plugin adds two keys to `context` when running: # # - `visibility_migration_running: true` - # - For the {Warden} which it instantiates, it adds `visibility_migration_warden_running: true`. + # - For the {Schema::Warden} which it instantiates, it adds `visibility_migration_warden_running: true`. # # Use those keys to modify your `visible?` behavior as needed. # # Also, in a pinch, you can set `skip_visibility_migration_error: true` in context to turn off this behavior per-query. - # (In that case, it uses {Subset} directly.) + # (In that case, it uses {Profile} directly.) # # @example Adding this plugin # # use GraphQL::Schema::Visibility::Migration # - class Migration < GraphQL::Schema::Visibility::Subset + class Migration < GraphQL::Schema::Visibility::Profile def self.use(schema) - schema.subset_class = self + schema.visibility_profile_class = self end class RuntimeTypesMismatchError < GraphQL::Error - def initialize(method_called, warden_result, subset_result, method_args) + def initialize(method_called, warden_result, profile_result, method_args) super(<<~ERR) Mismatch in types for `##{method_called}(#{method_args.map(&:inspect).join(", ")})`: - #{compare_results(warden_result, subset_result)} + #{compare_results(warden_result, profile_result)} Update your `.visible?` implementation to make these implementations return the same value. @@ -45,9 +45,9 @@ def initialize(method_called, warden_result, subset_result, method_args) end private - def compare_results(warden_result, subset_result) - if warden_result.is_a?(Array) && subset_result.is_a?(Array) - all_results = warden_result | subset_result + def compare_results(warden_result, profile_result) + if warden_result.is_a?(Array) && profile_result.is_a?(Array) + all_results = warden_result | profile_result all_results.sort_by!(&:graphql_name) entries_text = all_results.map { |entry| "#{entry.graphql_name} (#{entry})"} @@ -55,13 +55,13 @@ def compare_results(warden_result, subset_result) yes = " ✔ " no = " " res = "".dup - res << "#{"Result".center(width)} Warden Subset \n" + res << "#{"Result".center(width)} Warden Profile \n" all_results.each_with_index do |entry, idx| - res << "#{entries_text[idx].ljust(width)}#{warden_result.include?(entry) ? yes : no}#{subset_result.include?(entry) ? yes : no}\n" + res << "#{entries_text[idx].ljust(width)}#{warden_result.include?(entry) ? yes : no}#{profile_result.include?(entry) ? yes : no}\n" end res << "\n" else - "- Warden returned: #{humanize(warden_result)}\n\n- Subset returned: #{humanize(subset_result)}" + "- Warden returned: #{humanize(warden_result)}\n\n- Visibility::Profile returned: #{humanize(profile_result)}" end end def humanize(val) @@ -83,7 +83,7 @@ def humanize(val) def initialize(context:, schema:, name: nil) @name = name @skip_error = context[:skip_visibility_migration_error] || context.is_a?(Query::NullContext) || context.is_a?(Hash) - @subset_types = GraphQL::Schema::Visibility::Subset.new(context: context, schema: schema) + @profile_types = GraphQL::Schema::Visibility::Profile.new(context: context, schema: schema) if !@skip_error context[:visibility_migration_running] = true warden_ctx_vals = context.to_h.dup @@ -92,7 +92,7 @@ def initialize(context:, schema:, name: nil) warden_schema = schema.const_get(:WardenCompatSchema, false) else warden_schema = Class.new(schema) - warden_schema.use_schema_visibility = false + warden_schema.use_visibility_profile = false # TODO public API warden_schema.send(:add_type_and_traverse, [warden_schema.query, warden_schema.mutation, warden_schema.subscription].compact, root: true) warden_schema.send(:add_type_and_traverse, warden_schema.directives.values + warden_schema.orphan_types, root: false) @@ -100,15 +100,15 @@ def initialize(context:, schema:, name: nil) end warden_ctx = GraphQL::Query::Context.new(query: context.query, values: warden_ctx_vals) warden_ctx.warden = GraphQL::Schema::Warden.new(schema: warden_schema, context: warden_ctx) - warden_ctx.types = @warden_types = warden_ctx.warden.schema_subset + warden_ctx.types = @warden_types = warden_ctx.warden.visibility_profile end end def loaded_types - @subset_types.loaded_types + @profile_types.loaded_types end - PUBLIC_SUBSET_METHODS = [ + PUBLIC_PROFILE_METHODS = [ :enum_values, :interfaces, :all_types, @@ -128,14 +128,14 @@ def loaded_types :reachable_type? ] - PUBLIC_SUBSET_METHODS.each do |subset_method| - define_method(subset_method) do |*args| - call_method_and_compare(subset_method, args) + PUBLIC_PROFILE_METHODS.each do |profile_method| + define_method(profile_method) do |*args| + call_method_and_compare(profile_method, args) end end def call_method_and_compare(method, args) - res_1 = @subset_types.public_send(method, *args) + res_1 = @profile_types.public_send(method, *args) if @skip_error return res_1 end diff --git a/lib/graphql/schema/visibility/subset.rb b/lib/graphql/schema/visibility/profile.rb similarity index 97% rename from lib/graphql/schema/visibility/subset.rb rename to lib/graphql/schema/visibility/profile.rb index dc94dbfab1..85ccba2b9e 100644 --- a/lib/graphql/schema/visibility/subset.rb +++ b/lib/graphql/schema/visibility/profile.rb @@ -11,11 +11,9 @@ class Visibility # - It doesn't use {Schema}'s top-level caches (eg {Schema.references_to}, {Schema.possible_types}, {Schema.types}) # - It doesn't hide Interface or Union types when all their possible types are hidden. (Instead, those types should implement `.visible?` to hide in that case.) # - It checks `.visible?` on root introspection types - # - # In the future, {Subset} will support lazy-loading types as needed during execution and multi-request caching of subsets. - # TODO rename to Profile? - class Subset - # @return [Schema::Visibility::Subset] + # - It can be used to cache profiles by name for re-use across queries + class Profile + # @return [Schema::Visibility::Profile] def self.from_context(ctx, schema) if ctx.respond_to?(:types) && (types = ctx.types).is_a?(self) types @@ -25,9 +23,9 @@ def self.from_context(ctx, schema) end def self.pass_thru(context:, schema:) - subset = self.new(context: context, schema: schema) - subset.instance_variable_set(:@cached_visible, Hash.new { |h,k| h[k] = true }) - subset + profile = self.new(context: context, schema: schema) + profile.instance_variable_set(:@cached_visible, Hash.new { |h,k| h[k] = true }) + profile end # @return [Symbol, nil] diff --git a/lib/graphql/schema/warden.rb b/lib/graphql/schema/warden.rb index a229305233..b905ddef55 100644 --- a/lib/graphql/schema/warden.rb +++ b/lib/graphql/schema/warden.rb @@ -61,8 +61,8 @@ def visible_type_membership?(tm, ctx); tm.visible?(ctx); end def interface_type_memberships(obj_t, ctx); obj_t.interface_type_memberships; end def arguments(owner, ctx); owner.arguments(ctx); end def loadable?(type, ctx); type.visible?(ctx); end - def schema_subset - @schema_subset ||= Warden::SchemaSubset.new(self) + def visibility_profile + @visibility_profile ||= Warden::VisibilityProfile.new(self) end end end @@ -70,17 +70,17 @@ def schema_subset class NullWarden def initialize(_filter = nil, context:, schema:) @schema = schema - @schema_subset = Warden::SchemaSubset.new(self) + @visibility_profile = Warden::VisibilityProfile.new(self) end # @api private - module NullSubset + module NullVisibilityProfile def self.new(context:, schema:) - NullWarden.new(context: context, schema: schema).schema_subset + NullWarden.new(context: context, schema: schema).visibility_profile end end - attr_reader :schema_subset + attr_reader :visibility_profile def visible_field?(field_defn, _ctx = nil, owner = nil); true; end def visible_argument?(arg_defn, _ctx = nil); true; end @@ -104,11 +104,11 @@ def possible_types(type_defn); @schema.possible_types(type_defn, Query::NullCont def interfaces(obj_type); obj_type.interfaces; end end - def schema_subset - @schema_subset ||= SchemaSubset.new(self) + def visibility_profile + @visibility_profile ||= VisibilityProfile.new(self) end - class SchemaSubset + class VisibilityProfile def initialize(warden) @warden = warden end @@ -193,7 +193,7 @@ def initialize(context:, schema:) @visible_possible_types = @visible_fields = @visible_arguments = @visible_enum_arrays = @visible_enum_values = @visible_interfaces = @type_visibility = @type_memberships = @visible_and_reachable_type = @unions = @unfiltered_interfaces = - @reachable_type_set = @schema_subset = + @reachable_type_set = @visibility_profile = nil end diff --git a/lib/graphql/testing/helpers.rb b/lib/graphql/testing/helpers.rb index 42be0279f0..f15832e0f9 100644 --- a/lib/graphql/testing/helpers.rb +++ b/lib/graphql/testing/helpers.rb @@ -92,7 +92,7 @@ def run_graphql_field(schema, field_path, object, arguments: {}, context: {}, as end graphql_result else - unfiltered_type = Schema::Visibility::Subset.pass_thru(schema: schema, context: context).type(type_name) + unfiltered_type = Schema::Visibility::Profile.pass_thru(schema: schema, context: context).type(type_name) if unfiltered_type raise TypeNotVisibleError.new(type_name: type_name) else diff --git a/spec/graphql/authorization_spec.rb b/spec/graphql/authorization_spec.rb index 0620a0f48e..3df00a305c 100644 --- a/spec/graphql/authorization_spec.rb +++ b/spec/graphql/authorization_spec.rb @@ -88,7 +88,7 @@ def resolve_type(obj, ctx) end module HiddenDefaultInterface - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? include HiddenInterface else # Warden will detect no possible types diff --git a/spec/graphql/execution/instrumentation_spec.rb b/spec/graphql/execution/instrumentation_spec.rb index 52170e6320..1044d0da47 100644 --- a/spec/graphql/execution/instrumentation_spec.rb +++ b/spec/graphql/execution/instrumentation_spec.rb @@ -166,7 +166,7 @@ def int(value:) assert multiplex_ctx[:second_instrumenter_did_begin] refute multiplex_ctx[:second_instrumenter_did_end] # No query instrumentation was run at all - expected_ctx_size = GraphQL::Schema.use_schema_visibility? ? 1 : 0 + expected_ctx_size = GraphQL::Schema.use_visibility_profile? ? 1 : 0 assert_equal expected_ctx_size, query_1_ctx.size assert_equal expected_ctx_size, query_2_ctx.size end diff --git a/spec/graphql/logger_spec.rb b/spec/graphql/logger_spec.rb index 7df5cf9729..c811067685 100644 --- a/spec/graphql/logger_spec.rb +++ b/spec/graphql/logger_spec.rb @@ -96,8 +96,8 @@ class CustomLoggerSchema < DefaultLoggerSchema it "logs about hidden interfaces with no implementations" do res = LoggerTest::CustomLoggerSchema.execute("{ node(id: \"5\") { id } }", context: { skip_visibility_migration_error: true }) - if GraphQL::Schema.use_schema_visibility? - assert_nil res["data"]["node"], "Schema::Visibility::Subset doesn't warn in this case -- it doesn't check possible types because it doesn't have to" + if GraphQL::Schema.use_visibility_profile? + assert_nil res["data"]["node"], "Schema::Visibility::Profile doesn't warn in this case -- it doesn't check possible types because it doesn't have to" else assert_equal ["Field 'node' doesn't exist on type 'Query'"], res["errors"].map { |err| err["message"] } assert_includes LoggerTest::CustomLoggerSchema::LOG_STRING.string, "Interface `Node` hidden because it has no visible implementers" @@ -110,8 +110,8 @@ class CustomLoggerSchema < DefaultLoggerSchema res = LoggerTest::DefaultLoggerSchema.execute("{ node(id: \"5\") { id } }", context: { skip_visibility_migration_error: true }) end - if GraphQL::Schema.use_schema_visibility? - assert_nil res["data"]["node"], "Schema::Visibility::Subset doesn't warn in this case -- it doesn't check possible types because it doesn't have to" + if GraphQL::Schema.use_visibility_profile? + assert_nil res["data"]["node"], "Schema::Visibility::Profile doesn't warn in this case -- it doesn't check possible types because it doesn't have to" else assert_equal ["Field 'node' doesn't exist on type 'Query'"], res["errors"].map { |err| err["message"] } end diff --git a/spec/graphql/query/result_spec.rb b/spec/graphql/query/result_spec.rb index 5c8126485a..c725c29915 100644 --- a/spec/graphql/query/result_spec.rb +++ b/spec/graphql/query/result_spec.rb @@ -24,7 +24,7 @@ it "exposes the context" do assert_instance_of GraphQL::Query::Context, result.context - expected_ctx = if GraphQL::Schema.use_schema_visibility? && result.context.schema.visibility.migration_errors? + expected_ctx = if GraphQL::Schema.use_visibility_profile? && result.context.schema.visibility.migration_errors? {a: :b, visibility_migration_running: true} else {a: :b} diff --git a/spec/graphql/schema/addition_spec.rb b/spec/graphql/schema/addition_spec.rb index 17cfbdcb99..0819509c83 100644 --- a/spec/graphql/schema/addition_spec.rb +++ b/spec/graphql/schema/addition_spec.rb @@ -4,7 +4,7 @@ describe GraphQL::Schema::Addition do it "handles duplicate types with cycles" do duplicate_types_schema = Class.new(GraphQL::Schema) - duplicate_types_schema.use_schema_visibility = false + duplicate_types_schema.use_visibility_profile = false duplicate_types = 2.times.map { Class.new(GraphQL::Schema::Object) do graphql_name "Thing" diff --git a/spec/graphql/schema/directive/flagged_spec.rb b/spec/graphql/schema/directive/flagged_spec.rb index 8bb9582009..17836aae69 100644 --- a/spec/graphql/schema/directive/flagged_spec.rb +++ b/spec/graphql/schema/directive/flagged_spec.rb @@ -5,7 +5,7 @@ class FlaggedSchema < GraphQL::Schema module Animal include GraphQL::Schema::Interface - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? # It won't check possible types, so it needs this directly directive GraphQL::Schema::Directive::Flagged, by: ["northPole", "southPole"] end diff --git a/spec/graphql/schema/dynamic_members_spec.rb b/spec/graphql/schema/dynamic_members_spec.rb index 6d30713fc9..c868935a1c 100644 --- a/spec/graphql/schema/dynamic_members_spec.rb +++ b/spec/graphql/schema/dynamic_members_spec.rb @@ -200,8 +200,8 @@ class Country < BaseObject class Locale < BaseUnion possible_types Country, future_schema: true - if GraphQL::Schema.use_schema_visibility? - # Subset won't check possible_types, this must be flagged + if GraphQL::Schema.use_visibility_profile? + # Profile won't check possible_types, this must be flagged self.future_schema = true end end @@ -595,7 +595,7 @@ def legacy_schema_sdl assert_includes MultifieldSchema::Country.interfaces({ future_schema: true }), MultifieldSchema::HasCapital assert_includes MultifieldSchema.possible_types(MultifieldSchema::HasCapital, { future_schema: true }), MultifieldSchema::Country assert_includes MultifieldSchema::Country.interfaces, MultifieldSchema::HasCapital - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? # filtered with `future_schema: nil` refute_includes MultifieldSchema.possible_types(MultifieldSchema::HasCapital), MultifieldSchema::Country else @@ -605,17 +605,17 @@ def legacy_schema_sdl it "hides hidden union memberships" do assert MultifieldSchema::Locale.visible?({ future_schema: true }) - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? refute MultifieldSchema::Locale.visible?({ future_schema: false }) else - # Warden will check possible types -- but Subset doesn't + # Warden will check possible types -- but Profile doesn't assert MultifieldSchema::Locale.visible?({ future_schema: false }) end # and the possible types relationship is sometimes hidden: refute_includes MultifieldSchema.possible_types(MultifieldSchema::Locale, { future_schema: false }), MultifieldSchema::Country assert_includes MultifieldSchema.possible_types(MultifieldSchema::Locale, { future_schema: true }), MultifieldSchema::Country - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? # This type is hidden in this case assert_equal [], MultifieldSchema.possible_types(MultifieldSchema::Locale) else @@ -631,7 +631,7 @@ def legacy_schema_sdl # and the possible types relationship is sometimes hidden: assert_equal [], MultifieldSchema.possible_types(MultifieldSchema::Region, { future_schema: false }) assert_equal [MultifieldSchema::Country, MultifieldSchema::Place], MultifieldSchema.possible_types(MultifieldSchema::Region, { future_schema: true }) - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? # Filtered like `future_schema: false` assert_equal [MultifieldSchema::Country, MultifieldSchema::LegacyPlace], MultifieldSchema.possible_types(MultifieldSchema::Region) else @@ -694,7 +694,7 @@ def legacy_schema_sdl assert_equal MultifieldSchema::MoneyScalar, MultifieldSchema.get_type("Money", { future_schema: nil }) assert_equal MultifieldSchema::MoneyScalar, MultifieldSchema.get_type("Money", { future_schema: false }) assert_equal MultifieldSchema::Money, MultifieldSchema.get_type("Money", { future_schema: true }) - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? # Filtered like `future_schema: nil` assert_equal MultifieldSchema::MoneyScalar, MultifieldSchema.get_type("Money") else @@ -874,7 +874,7 @@ class OtherObject < GraphQL::Schema::Object field :f, Int, null: false end - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? ThingInterface.orphan_types(OtherObject) end diff --git a/spec/graphql/schema/visibility/subset_spec.rb b/spec/graphql/schema/visibility/subset_spec.rb index 1262c72f07..c54b1ca4af 100644 --- a/spec/graphql/schema/visibility/subset_spec.rb +++ b/spec/graphql/schema/visibility/subset_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true require "spec_helper" -describe GraphQL::Schema::Visibility::Subset do - class SubsetSchema < GraphQL::Schema +describe GraphQL::Schema::Visibility::Profile do + class ProfileSchema < GraphQL::Schema class Thing < GraphQL::Schema::Object field :name, String, method: :to_s end @@ -17,7 +17,7 @@ class Query < GraphQL::Schema::Object use GraphQL::Schema::Visibility end it "only loads the types it needs" do - query = GraphQL::Query.new(SubsetSchema, "{ thing { name } }", use_schema_subset: true) + query = GraphQL::Query.new(ProfileSchema, "{ thing { name } }", use_visibility_profile: true) assert_equal [], query.types.loaded_types res = query.result diff --git a/spec/graphql/schema/warden_spec.rb b/spec/graphql/schema/warden_spec.rb index f0646ea487..08ae4e6405 100644 --- a/spec/graphql/schema/warden_spec.rb +++ b/spec/graphql/schema/warden_spec.rb @@ -226,7 +226,7 @@ class QueryType < BaseObject field :public_type, PublicType, null: false # Warden would exclude this when it was only referenced as a possible_type of LanguageMemberType. - # But Subset always included it. This makes them behave the + # But Profile always included it. This makes them behave the same field :example_character, Character do metadata :hidden_abstract_type, true end @@ -535,7 +535,7 @@ class C < A; end class BagOfThings < GraphQL::Schema::Union possible_types A, B, C - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? def self.visible?(ctx) ( possible_types.any? { |pt| ctx.schema.visible?(pt, ctx) } || @@ -636,7 +636,7 @@ def self.visible?(member, context) res = schema.execute(query_string, context: { skip_visibility_migration_error: true, except: ->(m, _) { ["A", "B", "C"].include?(m.graphql_name) } }) - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? # Node is still visible even though it has no possible types assert res["data"]["Node"] assert_equal [{ "name" => "node" }], res["data"]["Query"]["fields"] @@ -1079,7 +1079,7 @@ def account result = schema.execute(query_str, context: { skip_visibility_migration_error: true }) - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? assert_equal "1", result["data"]["account"]["id"] else assert_equal ["Field 'id' doesn't exist on type 'NewAccount'"], result["errors"].map { |e| e["message"] } diff --git a/spec/graphql/schema_spec.rb b/spec/graphql/schema_spec.rb index 34298251c2..2e2d9279c5 100644 --- a/spec/graphql/schema_spec.rb +++ b/spec/graphql/schema_spec.rb @@ -75,7 +75,7 @@ class CustomSubscriptions < GraphQL::Subscriptions::ActionCableSubscriptions assert_equal base_schema.multiplex_analyzers, schema.multiplex_analyzers assert_equal base_schema.disable_introspection_entry_points?, schema.disable_introspection_entry_points? expected_plugins = [ - (GraphQL::Schema.use_schema_visibility? ? GraphQL::Schema::Visibility : nil), + (GraphQL::Schema.use_visibility_profile? ? GraphQL::Schema::Visibility : nil), GraphQL::Backtrace, GraphQL::Subscriptions::ActionCableSubscriptions ].compact @@ -149,7 +149,7 @@ class CustomSubscriptions < GraphQL::Subscriptions::ActionCableSubscriptions assert_equal base_schema.query_analyzers + [query_analyzer], schema.query_analyzers assert_equal base_schema.multiplex_analyzers + [multiplex_analyzer], schema.multiplex_analyzers expected_plugins = [GraphQL::Backtrace, GraphQL::Subscriptions::ActionCableSubscriptions, CustomSubscriptions] - if GraphQL::Schema.use_schema_visibility? + if GraphQL::Schema.use_visibility_profile? expected_plugins.unshift(GraphQL::Schema::Visibility) end assert_equal expected_plugins, schema.plugins.map(&:first) diff --git a/spec/integration/rails/graphql/schema_spec.rb b/spec/integration/rails/graphql/schema_spec.rb index acaa69a49a..731d9c0ef3 100644 --- a/spec/integration/rails/graphql/schema_spec.rb +++ b/spec/integration/rails/graphql/schema_spec.rb @@ -16,7 +16,7 @@ describe "#union_memberships" do it "returns a list of unions that include the type" do - skip("Not implemented for Subset") if GraphQL::Schema.use_schema_visibility? + skip("Not implemented for Visibility::Profile") if GraphQL::Schema.use_visibility_profile? assert_equal [schema.types["Animal"], schema.types["AnimalAsCow"]], schema.union_memberships(schema.types["Cow"]) end end @@ -43,7 +43,7 @@ describe "#references_to" do it "returns a list of Field and Arguments of that type" do - skip "Not implemented when using Subset" if GraphQL::Schema.use_schema_visibility? + skip "Not implemented when using Visibility::Profile" if GraphQL::Schema.use_visibility_profile? cow_field = schema.get_field("Query", "cow") cow_t = schema.get_type("Cow") assert_equal [cow_field], schema.references_to(cow_t) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 08882b0263..6b1f9d016d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -19,7 +19,7 @@ if ENV["GRAPHQL_REJECT_NUMBERS_FOLLOWED_BY_NAMES"] puts "Opting into GraphQL.reject_numbers_followed_by_names" GraphQL.reject_numbers_followed_by_names = true - puts "Opting into GraphQL::Schema::Visibility::Subset" + puts "Opting into GraphQL::Schema::Visibility::Profile" GraphQL::Schema.use(GraphQL::Schema::Visibility, migration_errors: true) end From 466a4e6b212084aefb6900ecd401bcba0a06eaf0 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 8 Oct 2024 12:35:12 -0400 Subject: [PATCH 16/17] Start documenting Visibility::Profile --- guides/authorization/visibility.md | 59 ++++++++++++++++++++- guides/schema/dynamic_types.md | 5 +- lib/graphql/schema/visibility.rb | 2 +- lib/graphql/schema/visibility/migration.rb | 6 +-- spec/graphql/schema/dynamic_members_spec.rb | 2 +- 5 files changed, 65 insertions(+), 9 deletions(-) diff --git a/guides/authorization/visibility.md b/guides/authorization/visibility.md index 70320afa8e..e6205435e8 100644 --- a/guides/authorization/visibility.md +++ b/guides/authorization/visibility.md @@ -18,7 +18,16 @@ Here are some reasons you might want to hide parts of your schema: ## Hiding Parts of the Schema -You can customize the visibility of parts of your schema by reimplementing various `visible?` methods: +To start limiting visibility of your schema, add the plugin: + +```ruby +class MySchema < GraphQL::Schema + # ... + use GraphQL::Schema::Visibility # see below for options +end +``` + +Then, you can customize the visibility of parts of your schema by reimplementing various `visible?` methods: - Type classes have a `.visible?(context)` class method - Fields and arguments have a `#visible?(context)` instance method @@ -30,6 +39,31 @@ These methods are called with the query context, based on the hash you pass as ` - In introspection, the member will _not_ be included in the result - In normal queries, if a query references that member, it will return a validation error, since that member doesn't exist +## Visibility Profiles + +You can use named profiles to cache your schema's visibility modes. For example: + +```ruby +use GraphQL::Schema::Visibility, profiles: { + # mode_name => example_context_hash + public: { public: true }, + beta: { public: true, beta: true }, + internal_admin: { internal_admin: true } +} +``` + +Then, you can run queries with `context[:visibility_profile]` equal to one of the pre-defined profiles. When you do, GraphQL-Ruby will use a precomputed set of types and fields for that query. + +### Preloading profiles + +By default, GraphQL-Ruby will preload all named visibility profiles when `Rails.env.production?` is present and true. You can manually set this option by passing `use ... preload: true` (or `false`). Enable preloading in production to reduce latency of the first request to each visibility profile. Disable preloading in development to speed up application boot. + +### Dynamic profiles + +When you provide named visibility profiles, `context[:visibility_profile]` is required for query execution. You can also permit dynamic visibility for queries which _don't_ have that key set by passing `use ..., dynamic: true`. You could use this to support backwards compatibility or when visibility calculations are too complex to predefine. + +When no named profiles are defined, all queries use dynamic visibility. + ## Object Visibility Let's say you're working on a new feature which should remain secret for a while. You can implement `.visible?` in a type: @@ -107,3 +141,26 @@ end ``` For big schemas, this can be a worthwhile speed-up. + +## Migration Notes + +{% "GraphQL::Schema::Visibility" | api_doc %} is a _new_ implementation of visibility in GraphQL-Ruby. It has some slight differences from the previous implementation ({% "GraphQL::Schema::Warden" | api_doc %}): + +- `Visibility` speeds up Rails app boot because it doesn't require all types to be loaded during boot and only loads types as they are used by queries. +- `Visibility` supports predefined, reusable visibility profiles which speeds up queries using complicated `visible?` checks. +- `Visibility` hides types differently in a few edge cases: + - Previously, `Warden` hide interface and union types which had no possible types. `Visibility` doesn't check possible types (in order to support performance improvements), so those types must return `false` for `visible?` in the same cases where all possible types were hidden. Otherwise, that interface or union type will be visible but have no possible types. + - Some other thing, see TODO +- When `Visibility` is used, several (Ruby-level) Schema introspection methods don't work because the caches they draw on haven't been calculated (`Schema.references_to`, `Schema.union_memberships`). If you're using these, please get in touch so that we can find a way forward. + +### Migration Mode + +You can use `use GraphQL::Schema::Visibility, ... migration_errors: true` to enable migration mode. In this mode, GraphQL-Ruby will make visibility checks with _both_ `Visibility` and `Warden` and compare the result, raising a descriptive error when the two systems return different results. As you migrate to `Visibility`, enable this mode in test to find any unexpected discrepancies. + +Sometimes, there's a discrepancy that is hard to resolve but doesn't make any _real_ difference in application behavior. To address these cases, you can use these flags in `context`: + +- `context[:visibility_migration_running] = true` is set in the main query context. +- `context[:visibility_migration_warden_running] = true` is set in the _duplicate_ context which is passed to a `Warden` instance. +- If you set `context[:skip_migration_error] = true`, then no migration error will be raised for that query. + +You can use these flags to conditionally handle edge cases that should be ignored in testing. diff --git a/guides/schema/dynamic_types.md b/guides/schema/dynamic_types.md index 123cadd873..9497b5d2e8 100644 --- a/guides/schema/dynamic_types.md +++ b/guides/schema/dynamic_types.md @@ -8,7 +8,10 @@ desc: Using different schema members for each request index: 8 --- -You can use different versions of your GraphQL schema for each operation. To do this, implement `visible?(context)` on the parts of your schema that will be conditionally accessible. Additionally, many schema elements have definition methods which are called at runtime by GraphQL-Ruby. You can re-implement those to return any valid schema objects. GraphQL-Ruby caches schema elements for the duration of the operation, but if you're making external service calls to implement the methods below, consider adding a cache layer to improve the client experience and reduce load on your backend. +You can use different versions of your GraphQL schema for each operation. To do this, add `use GraphQL::Schema::Visibility` and implement `visible?(context)` on the parts of your schema that will be conditionally accessible. Additionally, many schema elements have definition methods which are called at runtime by GraphQL-Ruby. You can re-implement those to return any valid schema objects. + + +GraphQL-Ruby caches schema elements for the duration of the operation, but if you're making external service calls to implement the methods below, consider adding a cache layer to improve the client experience and reduce load on your backend. At runtime, ensure that only one object is visible per name (type name, field name, etc.). (If `.visible?(context)` returns `false`, then that part of the schema will be hidden for the current operation.) diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index 972c2d0ed7..ea170d4dc9 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -11,7 +11,7 @@ class Visibility # @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?`) # @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_ARRAY, preload: (defined?(Rails) ? Rails.env.production? : nil), migration_errors: false) + def self.use(schema, dynamic: false, profiles: EmptyObjects::EMPTY_HASH, preload: (defined?(Rails) ? Rails.env.production? : nil), migration_errors: false) schema.visibility = self.new(schema, dynamic: dynamic, preload: preload, profiles: profiles, migration_errors: migration_errors) end diff --git a/lib/graphql/schema/visibility/migration.rb b/lib/graphql/schema/visibility/migration.rb index 7a9afe8493..f3eca3422f 100644 --- a/lib/graphql/schema/visibility/migration.rb +++ b/lib/graphql/schema/visibility/migration.rb @@ -24,13 +24,9 @@ class Visibility # # @example Adding this plugin # - # use GraphQL::Schema::Visibility::Migration + # use GraphQL::Schema::Visibility, migration_errors: true # class Migration < GraphQL::Schema::Visibility::Profile - def self.use(schema) - schema.visibility_profile_class = self - end - class RuntimeTypesMismatchError < GraphQL::Error def initialize(method_called, warden_result, profile_result, method_args) super(<<~ERR) diff --git a/spec/graphql/schema/dynamic_members_spec.rb b/spec/graphql/schema/dynamic_members_spec.rb index c868935a1c..86f5d5757d 100644 --- a/spec/graphql/schema/dynamic_members_spec.rb +++ b/spec/graphql/schema/dynamic_members_spec.rb @@ -874,6 +874,7 @@ class OtherObject < GraphQL::Schema::Object field :f, Int, null: false end + # TODO why is this necessary? if GraphQL::Schema.use_visibility_profile? ThingInterface.orphan_types(OtherObject) end @@ -934,7 +935,6 @@ def thing end query(Query) - use GraphQL::Schema::Visibility::Migration end end From f591d987c7581ac67cf179edc4b4e2e8f7608916 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Wed, 9 Oct 2024 10:48:08 -0400 Subject: [PATCH 17/17] Update test --- spec/graphql/schema/dynamic_members_spec.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/spec/graphql/schema/dynamic_members_spec.rb b/spec/graphql/schema/dynamic_members_spec.rb index 86f5d5757d..89ee7469c5 100644 --- a/spec/graphql/schema/dynamic_members_spec.rb +++ b/spec/graphql/schema/dynamic_members_spec.rb @@ -873,12 +873,6 @@ class OtherObject < GraphQL::Schema::Object implements ThingInterface field :f, Int, null: false end - - # TODO why is this necessary? - if GraphQL::Schema.use_visibility_profile? - ThingInterface.orphan_types(OtherObject) - end - class ThingUnion < GraphQL::Schema::Union graphql_name "Thing" possible_types OtherObject @@ -932,6 +926,8 @@ def thing raise ArgumentError, "Unhandled type kind: #{type_kind.inspect}" end end + + field :other_object, OtherObject end query(Query)