Visibility: require all root config before preloading

lib/graphql/schema.rb

@@ -757,6 +757,7 @@ def introspection(new_introspection_namespace = nil)
           # reset this cached value:
           @introspection_system = nil
           introspection_system
+          self.visibility&.introspection_system_configured(introspection_system)
           @introspection
         else
           @introspection || find_inherited_value(:introspection)
@@ -768,7 +769,6 @@ def introspection_system
         if !@introspection_system
           @introspection_system = Schema::IntrospectionSystem.new(self)
           @introspection_system.resolve_late_bindings
-          self.visibility&.introspection_system_configured(@introspection_system)
         end
         @introspection_system
       end

lib/graphql/schema/introspection_system.rb

@@ -105,11 +105,13 @@ def resolve_late_binding(late_bound_type)
       end
 
       def load_constant(class_name)
-        const = @custom_namespace.const_get(class_name)
+        const = begin
+          @custom_namespace.const_get(class_name)
+        rescue NameError
+          # Dup the built-in so that the cached fields aren't shared
+          @built_in_namespace.const_get(class_name)
+        end
         dup_type_class(const)
-      rescue NameError
-        # Dup the built-in so that the cached fields aren't shared
-        dup_type_class(@built_in_namespace.const_get(class_name))
       end
 
       def get_fields_from_class(class_sym:)
@@ -133,23 +135,6 @@ def dup_type_class(type_class)
           end
         end
       end
-
-      class PerFieldProxyResolve
-        def initialize(object_class:, inner_resolve:)
-          @object_class = object_class
-          @inner_resolve = inner_resolve
-        end
-
-        def call(obj, args, ctx)
-          query_ctx = ctx.query.context
-          # Remove the QueryType wrapper
-          if obj.is_a?(GraphQL::Schema::Object)
-            obj = obj.object
-          end
-          wrapped_object = @object_class.wrap(obj, query_ctx)
-          @inner_resolve.call(wrapped_object, args, ctx)
-        end
-      end
     end
   end
 end

lib/graphql/schema/printer.rb

@@ -58,8 +58,8 @@ def self.visible?(ctx)
           end
         end
         schema = Class.new(GraphQL::Schema) {
-          use GraphQL::Schema::Visibility
           query(query_root)
+          use GraphQL::Schema::Visibility
           def self.visible?(member, _ctx)
             member.graphql_name != "Root"
           end

lib/graphql/schema/visibility.rb

@@ -8,19 +8,25 @@ class Schema
     # Use this plugin to make some parts of your schema hidden from some viewers.
     #
     class Visibility
+      class TypeConfigurationError < GraphQL::Error
+        def initialize(config_message, config_str)
+          message = "GraphQL::Schema::Visibility already preloaded, but #{config_message} added to the schema. Move this `#{config_str}` configuration above `use(GraphQL::Schema::Visibility)"
+          super(message)
+        end
+      end
       # @param schema [Class<GraphQL::Schema>]
       # @param profiles [Hash<Symbol => Hash>] A hash of `name => context` pairs for preloading visibility profiles
       # @param preload [Boolean] if `true`, load the default schema profile and all named profiles immediately (defaults to `true` for `Rails.env.production?` and `Rails.env.staging?`)
       # @param migration_errors [Boolean] if `true`, raise an error when `Visibility` and `Warden` return different results
-      def self.use(schema, dynamic: false, profiles: EmptyObjects::EMPTY_HASH, preload: (defined?(Rails.env) ? (Rails.env.production? || Rails.env.staging?) : nil), migration_errors: false)
+      def self.use(schema, dynamic: false, profiles: EmptyObjects::EMPTY_HASH, preload: (defined?(Rails.env) ? (Rails.env.production? || Rails.env.staging? || nil) : false), migration_errors: false)
         profiles&.each { |name, ctx|
           ctx[:visibility_profile] = name
           ctx.freeze
         }
         schema.visibility = self.new(schema, dynamic: dynamic, preload: preload, profiles: profiles, migration_errors: migration_errors)
       end
 
-      def initialize(schema, dynamic:, preload:, profiles:, migration_errors:)
+      def initialize(schema, dynamic:, preload:, profiles:, migration_errors:, configuration_inherited: false)
         @schema = schema
         schema.use_visibility_profile = true
         schema.visibility_profile_class = if migration_errors
@@ -40,6 +46,7 @@ def initialize(schema, dynamic:, preload:, profiles:, migration_errors:)
         @types = nil
         @all_references = nil
         @loaded_all = false
+        @configuration_inherited = configuration_inherited
         if preload
           self.preload
         end
@@ -94,6 +101,7 @@ def preload
         # Root types may have been nil:
         types_to_visit.compact!
         ensure_all_loaded(types_to_visit)
+        @cached_profiles.clear
         @profiles.each do |profile_name, example_ctx|
           prof = profile_for(example_ctx)
           prof.preload
@@ -102,41 +110,27 @@ def preload
 
       # @api private
       def query_configured(query_type)
-        if @preload
-          ensure_all_loaded([query_type])
-        end
+        require_if_preloaded("a query type was", "query(...)")
       end
 
       # @api private
       def mutation_configured(mutation_type)
-        if @preload
-          ensure_all_loaded([mutation_type])
-        end
+        require_if_preloaded("a mutation type was", "mutation(...)")
       end
 
       # @api private
       def subscription_configured(subscription_type)
-        if @preload
-          ensure_all_loaded([subscription_type])
-        end
+        require_if_preloaded("a mutation type was", "subscription(...)")
       end
 
       # @api private
       def orphan_types_configured(orphan_types)
-        if @preload
-          ensure_all_loaded(orphan_types)
-        end
+        require_if_preloaded("orphan types were", "orphan_types(...)")
       end
 
       # @api private
       def introspection_system_configured(introspection_system)
-        if @preload
-          introspection_types = [
-            *@schema.introspection_system.types.values,
-            *@schema.introspection_system.entry_points.map { |ep| ep.type.unwrap },
-          ]
-          ensure_all_loaded(introspection_types)
-        end
+        require_if_preloaded("custom introspection was", "introspection(...)")
       end
 
       # Make another Visibility for `schema` based on this one
@@ -148,7 +142,8 @@ def dup_for(other_schema)
           dynamic: @dynamic,
           preload: @preload,
           profiles: @profiles,
-          migration_errors: @migration_errors
+          migration_errors: @migration_errors,
+          configuration_inherited: true,
         )
       end
 
@@ -196,6 +191,19 @@ def top_level_profile(refresh: false)
 
       private
 
+      def require_if_preloaded(config_message, config_code)
+        case @preload
+        when false
+          # Rails.env wasn't defined, so this won't try to preload unless manually set to true
+        when true, nil
+          if @configuration_inherited
+            preload
+          else
+            raise TypeConfigurationError.new(config_message, config_code)
+          end
+        end
+      end
+
       def ensure_all_loaded(types_to_visit)
         while (type = types_to_visit.shift)
           if type.kind.fields? && @preloaded_types.add?(type)

lib/graphql/schema/visibility/profile.rb

@@ -290,11 +290,13 @@ def preload
             end
             # Lots more to do here
           end
-          @schema.introspection_system.entry_points.each do |f|
-            arguments(f).each do |arg|
-              argument(f, arg.graphql_name)
+          if @schema.query
+            @schema.introspection_system.entry_points.each do |f|
+              arguments(f).each do |arg|
+                argument(f, arg.graphql_name)
+              end
+              field(@schema.query, f.graphql_name)
             end
-            field(@schema.query, f.graphql_name)
           end
           @schema.introspection_system.dynamic_fields.each do |f|
             arguments(f).each do |arg|

spec/graphql/introspection/schema_type_spec.rb

@@ -180,7 +180,7 @@ def self.visible?(context)
       end
 
       Class.new(GraphQL::Schema) do
-        use GraphQL::Schema::Visibility
+        use GraphQL::Schema::Visibility, preload: false
         query query_type
         directives invisible_directive, visible_directive
       end

spec/graphql/schema/introspection_system_spec.rb

@@ -282,7 +282,7 @@ class DynamicFields < GraphQL::Introspection::DynamicFields
 
         class EntryPoints < GraphQL::Introspection::EntryPoints
           field_class(BaseField)
-          field :__type, GraphQL::Introspection::TypeType, resolve_static: true do
+          field :__type, GraphQL::Schema::LateBoundType.new("__Type"), resolve_static: true do
             argument :name, String
           end
         end
@@ -294,7 +294,6 @@ class SchemaType < GraphQL::Introspection::SchemaType
 
       class Query < GraphQL::Schema::Object
         field :int, Integer, null: false
-        def int; 1; end
       end
 
       query(Query)

spec/graphql/schema/visibility_spec.rb

@@ -85,6 +85,74 @@ def exec_query(...)
     refute_includes schema_sdl, "WidgetKind"
   end
 
+  it "raises errors when configuration comes after preload: true" do
+    assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do
+      Class.new(GraphQL::Schema) do
+        use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true
+        query(VisSchema::Query)
+      end
+    end
+
+    assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do
+      Class.new(GraphQL::Schema) do
+        use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true
+        mutation(VisSchema::Mutation)
+      end
+    end
+
+    assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do
+      Class.new(GraphQL::Schema) do
+        use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true
+        subscription(VisSchema::Subscription)
+      end
+    end
+
+    assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do
+      Class.new(GraphQL::Schema) do
+        use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true
+        orphan_types([VisSchema::Query])
+      end
+    end
+
+    assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do
+      Class.new(GraphQL::Schema) do
+        use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true
+        introspection(VisSchema::IntrospectionSystem)
+      end
+    end
+  end
+
+  it "doesn't raise when preload: false" do
+    assert(Class.new(GraphQL::Schema) do
+      use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: false
+      query(VisSchema::Query)
+    end)
+  end
+
+  it "doesn't raise on subclasses and preloads" do
+    base_schema = Class.new(GraphQL::Schema) do
+      query(VisSchema::Query)
+      use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: true
+    end
+
+    child_schema = Class.new(base_schema) do
+      query(VisSchema::Query)
+    end
+
+    assert_equal [:public, :admin], base_schema.visibility.cached_profiles.keys
+    assert_equal [:public, :admin], child_schema.visibility.cached_profiles.keys
+    assert_equal [:public, :admin], Class.new(child_schema).visibility.cached_profiles.keys
+  end
+
+  it "raises when preload: nil" do
+    assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do
+      Class.new(GraphQL::Schema) do
+        use GraphQL::Schema::Visibility, profiles: { public: {}, admin: { is_admin: true } }, preload: nil
+        query(VisSchema::Query)
+      end
+    end
+  end
+
   describe "running queries" do
     it "requires context[:visibility]" do
       err = assert_raises ArgumentError do
@@ -173,10 +241,10 @@ class OrphanType < GraphQL::Schema::Object
         end
         # This one is added before `Visibility`
         subscription(Subscription)
-        use GraphQL::Schema::Visibility, preload: true
         query { Query }
         mutation { Mutation }
         orphan_types(OrphanType)
+        use GraphQL::Schema::Visibility, preload: true
 
         module CustomIntrospection
           class DynamicFields < GraphQL::Introspection::DynamicFields
@@ -195,7 +263,9 @@ class DynamicFields < GraphQL::Introspection::DynamicFields
         assert_equal [NoProfileSchema::ExampleExtension, NoProfileSchema::OtherExampleExtension], NoProfileSchema::OrphanType.all_field_definitions.first.extensions.map(&:class)
         custom_int_field = NoProfileSchema::CustomIntrospection::DynamicFields.all_field_definitions.find { |f| f.original_name == :__hello }
         assert_equal [], custom_int_field.extensions
-        NoProfileSchema.introspection(NoProfileSchema::CustomIntrospection)
+        assert_raises GraphQL::Schema::Visibility::TypeConfigurationError do
+          NoProfileSchema.introspection(NoProfileSchema::CustomIntrospection)
+        end
         assert_equal [NoProfileSchema::OtherExampleExtension], custom_int_field.extensions.map(&:class)
       end
     end
@@ -313,8 +383,8 @@ def node
       end
 
       query(Query)
-      def self.resolve_type(...); Thing; end
       use GraphQL::Schema::Visibility
+      def self.resolve_type(...); Thing; end
     end
   end
 

spec/graphql/schema_spec.rb

@@ -530,7 +530,7 @@ class Query < GraphQL::Schema::Object
   it "defers root type blocks until those types are used" do
     calls = []
     schema = Class.new(GraphQL::Schema) do
-      use(GraphQL::Schema::Visibility)
+      use(GraphQL::Schema::Visibility, preload: false)
       query { calls << :query; Class.new(GraphQL::Schema::Object) { graphql_name("Query") } }
       mutation { calls << :mutation; Class.new(GraphQL::Schema::Object) { graphql_name("Mutation") } }
       subscription { calls << :subscription; Class.new(GraphQL::Schema::Object) { graphql_name("Subscription") } }
@@ -557,7 +557,7 @@ class Query < GraphQL::Schema::Object
     assert schema.instance_variable_get(:@subscription_extension_added)
 
     schema2 = Class.new(GraphQL::Schema) do
-      use(GraphQL::Schema::Visibility)
+      use(GraphQL::Schema::Visibility, preload: false)
       use GraphQL::Subscriptions
       subscription(Class.new(GraphQL::Schema::Object) { graphql_name("Subscription") })
     end