Add specs for simple lazy promise

This promise abstracts over a few different kinds of laziness we have,
without coupling super tightly to the GitlabSchema.

.gitignore

@@ -101,3 +101,5 @@ apollo.config.js
 /tmp/matching_tests.txt
 ee/changelogs/unreleased-ee
 /sitespeed-result
+tags.lock
+tags.temp

app/graphql/gitlab_schema.rb

@@ -30,6 +30,8 @@ class GitlabSchema < GraphQL::Schema
 
   default_max_page_size 100
 
+  lazy_resolve ::Gitlab::Graphql::Lazy, :force
+
   class << self
     def multiplex(queries, **kwargs)
       kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context])

bin/feature-flag

@@ -104,7 +104,7 @@ class FeatureFlagOptionParser
       end
 
       # Name is a first name
-      options.name = argv.first
+      options.name = argv.first.downcase.gsub(/-/, '_')
 
       options
     end

config/feature_flags/development/graphql_lazy_authorization.yml

+---
+name: graphql_lazy_authorization
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45263
+rollout_issue_url: 
+type: development
+group: group::plan
+default_enabled: false

lib/gitlab/graphql/authorize/authorize_field_service.rb

@@ -46,6 +46,8 @@ module Gitlab
 
         # Returns any authorize metadata from @field
         def field_authorizations
+          return [] if @field.metadata[:authorize] == true
+
           Array.wrap(@field.metadata[:authorize])
         end
 
@@ -54,7 +56,7 @@ module Gitlab
             # The field is a built-in/scalar type, or a list of scalars
             # authorize using the parent's object
             parent_typed_object.object
-          elsif @field.connection? || resolved_type.is_a?(Array)
+          elsif @field.connection? || @field.type.list? || resolved_type.is_a?(Array)
             # The field is a connection or a list of non-built-in types, we'll
             # authorize each element when rendering
             nil
@@ -75,16 +77,25 @@ module Gitlab
             # no need to do anything
           elsif authorizing_object
             # Authorizing fields representing scalars, or a simple field with an object
-            resolved_type if allowed_access?(current_user, authorizing_object)
+            ::Gitlab::Graphql::Lazy.with_value(authorizing_object) do |object|
+              resolved_type if allowed_access?(current_user, object)
+            end
           elsif @field.connection?
+            ::Gitlab::Graphql::Lazy.with_value(resolved_type) do |type|
               # A connection with pagination, modify the visible nodes on the
               # connection type in place
-            resolved_type.object.edge_nodes.to_a.keep_if { |node| allowed_access?(current_user, node) }
-            resolved_type
-          elsif resolved_type.is_a? Array
+              nodes = to_nodes(type)
+              nodes.keep_if { |node| allowed_access?(current_user, node) } if nodes
+              type
+            end
+          elsif @field.type.list? || resolved_type.is_a?(Array)
             # A simple list of rendered types  each object being an object to authorize
-            resolved_type.select do |single_object_type|
-              allowed_access?(current_user, realized(single_object_type).object)
+            ::Gitlab::Graphql::Lazy.with_value(resolved_type) do |items|
+              items.select do |single_object_type|
+                object_type = realized(single_object_type)
+                object = object_type.try(:object) || object_type
+                allowed_access?(current_user, object)
+              end
             end
           else
             raise "Can't authorize #{@field}"
@@ -93,18 +104,23 @@ module Gitlab
 
         # Ensure that we are dealing with realized objects, not delayed promises
         def realized(thing)
-          case thing
-          when BatchLoader::GraphQL
-            thing.sync
-          when GraphQL::Execution::Lazy
-            thing.value # part of the private api, but we need to unwrap it here.
+          ::Gitlab::Graphql::Lazy.force(thing)
+        end
+
+        # Try to get the connection
+        # can be at type.object or at type
+        def to_nodes(type)
+          if type.respond_to?(:nodes)
+            type.nodes
+          elsif type.respond_to?(:object)
+            to_nodes(type.object)
           else
-            thing
+            nil
           end
         end
 
         def allowed_access?(current_user, object)
-          object = object.sync if object.respond_to?(:sync)
+          object = realized(object)
 
           authorizations.all? do |ability|
             Ability.allowed?(current_user, ability, object)

lib/gitlab/graphql/lazy.rb

@@ -3,17 +3,45 @@
 module Gitlab
   module Graphql
     class Lazy
+      include Gitlab::Utils::StrongMemoize
+
+      def initialize(&block)
+        @proc = block
+      end
+
+      def force
+        strong_memoize(:force) { self.class.force(@proc.call) }
+      end
+
+      def then(&block)
+        self.class.new { yield force }
+      end
+
       # Force evaluation of a (possibly) lazy value
       def self.force(value)
         case value
+        when ::Gitlab::Graphql::Lazy
+          value.force
         when ::BatchLoader::GraphQL
           value.sync
+        when ::GraphQL::Execution::Lazy
+          value.value # part of the private api, but we can force this as well
         when ::Concurrent::Promise
-          value.execute.value
+          value.execute if value.state == :unscheduled
+
+          value.value # value.value(10.seconds)
         else
           value
         end
       end
+
+      def self.with_value(unforced, &block)
+        if Feature.enabled?(:graphql_lazy_authorization)
+          self.new { unforced }.then(&block)
+        else
+          block.call(unforced)
+        end
+      end
     end
   end
 end

spec/bin/feature_flag_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe 'bin/feature-flag' do
     let(:options) { FeatureFlagOptionParser.parse(argv) }
     let(:creator) { described_class.new(options) }
     let(:existing_flags) do
-      { 'existing-feature-flag' => File.join('config', 'feature_flags', 'development', 'existing-feature-flag.yml') }
+      { 'existing_feature_flag' => File.join('config', 'feature_flags', 'development', 'existing_feature_flag.yml') }
     end
 
     before do
@@ -32,12 +32,12 @@ RSpec.describe 'bin/feature-flag' do
 
     it 'properly creates a feature flag' do
       expect(File).to receive(:write).with(
-        File.join('config', 'feature_flags', 'development', 'feature-flag-name.yml'),
+        File.join('config', 'feature_flags', 'development', 'feature_flag_name.yml'),
         anything)
 
       expect do
         subject
-      end.to output(/name: feature-flag-name/).to_stdout
+      end.to output(/name: feature_flag_name/).to_stdout
     end
 
     context 'when running on master' do

spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb

@@ -27,13 +27,17 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeFieldService do
     end
   end
 
+  def resolve
+    service.authorized_resolve[type_instance, {}, context]
+  end
+
   subject(:service) { described_class.new(field) }
 
   describe '#authorized_resolve' do
     let_it_be(:current_user) { build(:user) }
     let_it_be(:presented_object) { 'presented object' }
     let_it_be(:query_type) { GraphQL::ObjectType.new }
-    let_it_be(:schema) { GraphQL::Schema.define(query: query_type, mutation: nil)}
+    let_it_be(:schema) { GitlabSchema }
     let_it_be(:query) { GraphQL::Query.new(schema, document: nil, context: {}, variables: {}) }
     let_it_be(:context) { GraphQL::Query::Context.new(query: query, values: { current_user: current_user }, object: nil) }
 
@@ -41,8 +45,69 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeFieldService do
     let(:type_instance) { type_class.authorized_new(presented_object, context) }
     let(:field) { type_class.fields['testField'].to_graphql }
 
-    subject(:resolved) { service.authorized_resolve.call(type_instance, {}, context) }
+    subject(:resolved) { resolve&.force }
+
+    context 'reading the field of a lazy value' do
+      let(:ability) { :read_field }
+      let(:presented_object) { lazy_upcase('a') }
+      let(:type_class) { type_with_field(GraphQL::STRING_TYPE, ability) }
+
+      let(:upcaser) do
+        Module.new do
+          def self.upcase(strs)
+            strs.map(&:upcase)
+          end
+        end
+      end
+
+      def lazy_upcase(str)
+        ::BatchLoader::GraphQL.for(str).batch do |strs, found|
+          strs.zip(upcaser.upcase(strs)).each { |s, us| found[s, us] }
+        end
+      end
+
+      it 'does not run authorizations until we force the resolved value' do
+        expect(Ability).not_to receive(:allowed?)
+
+        expect(resolve).to respond_to(:force)
+      end
+
+      it 'runs authorizations when we force the resolved value' do
+        spy_ability_check_for(ability, 'A')
+
+        expect(resolved).to eq('Resolved value')
+      end
+
+      it 'redacts values that fail the permissions check' do
+        spy_ability_check_for(ability, 'A', passed: false)
+
+        expect(resolved).to be_nil
+      end
+
+      context 'we batch two calls' do
+        def resolve(value)
+          instance = type_class.authorized_new(lazy_upcase(value), context)
+          service.authorized_resolve[instance, {}, context]
+        end
+
+        it 'batches resolution, but authorizes each object separately' do
+          expect(upcaser).to receive(:upcase).once.and_call_original
+          spy_ability_check_for(:read_field, 'A', passed: true)
+          spy_ability_check_for(:read_field, 'B', passed: false)
+          spy_ability_check_for(:read_field, 'C', passed: true)
 
+          a = resolve('a')
+          b = resolve('b')
+          c = resolve('c')
+
+          expect(a.force).to be_present
+          expect(b.force).to be_nil
+          expect(c.force).to be_present
+        end
+      end
+    end
+
+    shared_examples 'authorizing fields' do
       context 'scalar types' do
         shared_examples 'checking permissions on the presented object' do
           it 'checks the abilities on the object being presented and returns the value' do
@@ -97,6 +162,20 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeFieldService do
             expect(resolved).to be_nil
           end
         end
+
+        context 'when it returns values' do
+          let(:objects) { [1, 2, 3] }
+          let(:field_type) { type([:read_object]).connection_type }
+          let(:type_class) { type_with_field(field_type, [], objects) }
+
+          it 'filters out unauthorized values' do
+            spy_ability_check_for(:read_object, 1, passed: true)
+            spy_ability_check_for(:read_object, 2, passed: false)
+            spy_ability_check_for(:read_object, 3, passed: true)
+
+            expect(resolved.nodes).to eq [1, 3]
+          end
+        end
       end
 
       context 'when the field is a specific type' do
@@ -106,8 +185,6 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeFieldService do
         let(:type_class) { type_with_field(custom_type, :read_field, object_in_field) }
         let(:type_instance) { type_class.authorized_new(object_in_field, context) }
 
-      subject(:resolved) { service.authorized_resolve.call(type_instance, {}, context) }
-
         it 'checks both field & type permissions' do
           spy_ability_check_for(:read_field, object_in_field, passed: true)
           spy_ability_check_for(:read_type, object_in_field, passed: true)
@@ -156,9 +233,22 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeFieldService do
 
             spy_ability_check_for(:read_type, object_1, passed: false)
 
-          expect(resolved.map(&:object)).to contain_exactly(object_2)
+            expect(resolved).to contain_exactly(have_attributes(object: object_2))
+          end
+        end
       end
     end
+
+    it_behaves_like 'authorizing fields'
+
+    context 'the graphql_lazy_authorization feature flag is disabled' do
+      before do
+        stub_feature_flags(graphql_lazy_authorization: false)
+      end
+
+      subject(:resolved) { resolve }
+
+      it_behaves_like 'authorizing fields'
     end
   end
 

spec/lib/gitlab/graphql/lazy_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Graphql::Lazy do
+  def load(key)
+    BatchLoader.for(key).batch do |keys, loader|
+      keys.each { |x| loader.call(x, x * x) }
+    end
+  end
+
+  let(:value) { double(x: 1) }
+
+  describe '#force' do
+    subject { described_class.new { value.x } }
+
+    it 'can extract the value' do
+      expect(subject.force).to be 1
+    end
+
+    it 'can derive new lazy values' do
+      expect(subject.then { |x| x + 2 }.force).to be 3
+    end
+
+    it 'only evaluates once' do
+      expect(value).to receive(:x).once
+
+      expect(subject.force).to eq(subject.force)
+    end
+
+    it 'deals with nested laziness' do
+      expect(described_class.new { load(10) }.force).to eq(100)
+      expect(described_class.new { described_class.new { 5 } }.force).to eq 5
+    end
+  end
+
+  describe '.with_value' do
+    let(:inner) { described_class.new { value.x } }
+
+    subject { described_class.with_value(inner) { |x| x.to_s } }
+
+    it 'defers the application of a block to a value' do
+      expect(value).not_to receive(:x)
+
+      expect(subject).to be_an_instance_of(described_class)
+    end
+
+    it 'evaluates to the application of the block to the value' do
+      expect(value).to receive(:x).once
+
+      expect(subject.force).to eq(inner.force.to_s)
+    end
+  end
+
+  describe '.force' do
+    context 'when given a plain value' do
+      subject { described_class.force(1) }
+
+      it 'unwraps the value' do
+        expect(subject).to be 1
+      end
+    end
+
+    context 'when given a wrapped lazy value' do
+      subject { described_class.force(described_class.new { 2 }) }
+
+      it 'unwraps the value' do
+        expect(subject).to be 2
+      end
+    end
+
+    context 'when the value is from a batchloader' do
+      subject { described_class.force(load(3)) }
+
+      it 'syncs the value' do
+        expect(subject).to be 9
+      end
+    end
+
+    context 'when the value is a GraphQL lazy' do
+      subject { described_class.force(GitlabSchema.after_lazy(load(3)) { |x| x + 1 } ) }
+
+      it 'forces the evaluation' do
+        expect(subject).to be 10
+      end
+    end
+
+    context 'when the value is a promise' do
+      subject { described_class.force(::Concurrent::Promise.new { 4 }) }
+
+      it 'executes the promise and waits for the value' do
+        expect(subject).to be 4
+      end
+    end
+  end
+end

spec/support/helpers/graphql_helpers.rb

@@ -478,6 +478,8 @@ module GraphqlHelpers
       use Gitlab::Graphql::Authorize
       use Gitlab::Graphql::Pagination::Connections
 
+      lazy_resolve ::Gitlab::Graphql::Lazy, :force
+
       query(query_type)
     end
 

spec/support/shared_examples/graphql/label_fields.rb

@@ -106,13 +106,11 @@ RSpec.shared_examples 'querying a GraphQL type with labels' do
     end
 
     it 'batches queries for labels by title' do
-      pending('See: https://gitlab.com/gitlab-org/gitlab/-/issues/217767')
-
       multi_selection = query_for(label_b, label_c)
       single_selection = query_for(label_d)
 
       expect { run_query(multi_selection) }
-        .to issue_same_number_of_queries_as { run_query(single_selection) }
+        .to issue_same_number_of_queries_as { run_query(single_selection) }.ignoring_cached_queries
     end
   end