diff --git a/lib/graphql/schema/visibility.rb b/lib/graphql/schema/visibility.rb index a28ef42d63..9de7bc11b9 100644 --- a/lib/graphql/schema/visibility.rb +++ b/lib/graphql/schema/visibility.rb @@ -37,7 +37,7 @@ def initialize(schema, dynamic:, preload:, profiles:, migration_errors:) @interface_type_memberships = nil @directives = nil @types = nil - @references = nil + @all_references = nil @loaded_all = false end @@ -53,7 +53,7 @@ def all_interface_type_memberships def all_references load_all - @references + @all_references end def get_type(type_name) @@ -202,7 +202,7 @@ def load_all(types: nil) @interface_type_memberships = Hash.new { |h, interface_type| h[interface_type] = [] }.compare_by_identity @directives = [] @types = {} # String => Module - @references = Hash.new { |h, member| h[member] = [] }.compare_by_identity + @all_references = Hash.new { |h, member| h[member] = Set.new.compare_by_identity }.compare_by_identity @unions_for_references = Set.new @visit = Visibility::Visit.new(@schema) do |member| if member.is_a?(Module) @@ -216,11 +216,12 @@ def load_all(types: nil) else @types[member.graphql_name] = member end + member.directives.each { |dir| @all_references[dir.class] << member } if member < GraphQL::Schema::Directive @directives << member elsif member.respond_to?(:interface_type_memberships) member.interface_type_memberships.each do |itm| - @references[itm.abstract_type] << member + @all_references[itm.abstract_type] << member @interface_type_memberships[itm.abstract_type] << itm end elsif member < GraphQL::Schema::Union @@ -228,20 +229,33 @@ def load_all(types: nil) end elsif member.is_a?(GraphQL::Schema::Argument) member.validate_default_value - @references[member.type.unwrap] << member + @all_references[member.type.unwrap] << member + if (dirs = member.directives).any? + dir_owner = member.owner + if dir_owner.respond_to?(:owner) + dir_owner = dir_owner.owner + end + dirs.each { |dir| @all_references[dir.class] << dir_owner } + end elsif member.is_a?(GraphQL::Schema::Field) - @references[member.type.unwrap] << member + @all_references[member.type.unwrap] << member + if (dirs = member.directives).any? + dir_owner = member.owner + dirs.each { |dir| @all_references[dir.class] << dir_owner } + end + elsif member.is_a?(GraphQL::Schema::EnumValue) + if (dirs = member.directives).any? + dir_owner = member.owner + dirs.each { |dir| @all_references[dir.class] << dir_owner } + end end true end - @schema.root_types.each do |t| - @references[t] << true - end + @schema.root_types.each { |t| @all_references[t] << true } + @schema.introspection_system.types.each_value { |t| @all_references[t] << true } + @schema.directives.each_value { |dir_class| @all_references[dir_class] << true } - @schema.introspection_system.types.each_value do |t| - @references[t] << true - end @visit.visit_each(types: []) # visit default directives end @@ -258,20 +272,20 @@ def load_all(types: nil) # TODO: somehow don't iterate over all these, # only the ones that may have been modified @interface_type_memberships.each do |int_type, type_memberships| - referers = @references[int_type].select { |r| r.is_a?(GraphQL::Schema::Field) } + referers = @all_references[int_type].select { |r| r.is_a?(GraphQL::Schema::Field) } if referers.any? type_memberships.each do |type_membership| implementor_type = type_membership.object_type # Add new items only: - @references[implementor_type] |= referers + @all_references[implementor_type] |= referers end end end @unions_for_references.each do |union_type| - refs = @references[union_type] + refs = @all_references[union_type] union_type.all_possible_types.each do |object_type| - @references[object_type] |= refs # Add new items + @all_references[object_type] |= refs # Add new items end end end diff --git a/lib/graphql/schema/visibility/profile.rb b/lib/graphql/schema/visibility/profile.rb index bfc573105d..4a093c4d21 100644 --- a/lib/graphql/schema/visibility/profile.rb +++ b/lib/graphql/schema/visibility/profile.rb @@ -239,7 +239,9 @@ def directive_exists?(dir_name) end def directives - @all_directives ||= @schema.visibility.all_directives.select { |dir| @cached_visible[dir] } + @all_directives ||= @schema.visibility.all_directives.select { |dir| + @cached_visible[dir] && @schema.visibility.all_references[dir].any? { |ref| ref == true || (@cached_visible[ref] && referenced?(ref)) } + } end def loadable?(t, _ctx) diff --git a/spec/graphql/schema/warden_spec.rb b/spec/graphql/schema/warden_spec.rb index 4e7eeadfe0..768d4c03b8 100644 --- a/spec/graphql/schema/warden_spec.rb +++ b/spec/graphql/schema/warden_spec.rb @@ -166,6 +166,10 @@ class PublicType < BaseObject field :test, String end + class CheremeDirective < GraphQL::Schema::Directive + locations(GraphQL::Schema::Directive::OBJECT) + end + class CheremeWithInterface < BaseObject implements PublicInterfaceType @@ -175,6 +179,8 @@ class CheremeWithInterface < BaseObject class Chereme < BaseObject description "A basic unit of signed communication" implements LanguageMemberType + directive CheremeDirective + field :name, String, null: false field :chereme_with_interface, CheremeWithInterface @@ -400,6 +406,23 @@ def error_messages(query_result) assert_nil res["data"]["CheremeWithInterface"] end + it "hides directives if no other fields are using it" do + query_string = %| + { + __schema { directives { name } } + } + | + + res = MaskHelpers.query_with_mask(query_string, mask) + expected_directives = ["deprecated", "include", "oneOf", "skip", "specifiedBy"] + if !GraphQL::Schema.use_visibility_profile? + # Not supported by Warden + expected_directives.unshift("cheremeDirective") + end + + assert_equal(expected_directives, res["data"]["__schema"]["directives"].map { |d| d["name"] }) + end + it "causes validation errors" do query_string = %|{ phoneme(symbol: "ϕ") { name } }| res = MaskHelpers.query_with_mask(query_string, mask) @@ -735,15 +758,15 @@ def self.visible?(member, context) } it "hides types if no other fields or arguments are using it" do - query_string = %| - { - CheremeInput: __type(name: "CheremeInput") { fields { name } } - } - | + query_string = %| + { + CheremeInput: __type(name: "CheremeInput") { fields { name } } + } + | - res = MaskHelpers.query_with_mask(query_string, mask) - assert_nil res["data"]["CheremeInput"] - end + res = MaskHelpers.query_with_mask(query_string, mask) + assert_nil res["data"]["CheremeInput"] + end it "isn't present in introspection" do query_string = %|