Add async support to gitlab-http

When using the `gitlab-http` gem, you can pass the `async: true` option
and get a `Concurrent::Promise` object. Since there is no business
logic and external connection inside the async calls, it is thread-safe.

Gemfile.lock

@@ -28,6 +28,7 @@ PATH
   specs:
     gitlab-http (0.1.0)
       activesupport (~> 7)
+      concurrent-ruby (~> 1.2)
       httparty (~> 0.21.0)
       ipaddress (~> 0.8.3)
       nokogiri (~> 1.15.4)

gems/gitlab-http/Gemfile.lock

@@ -20,6 +20,7 @@ PATH
   specs:
     gitlab-http (0.1.0)
       activesupport (~> 7)
+      concurrent-ruby (~> 1.2)
       httparty (~> 0.21.0)
       ipaddress (~> 0.8.3)
       nokogiri (~> 1.15.4)

gems/gitlab-http/README.md

@@ -24,16 +24,27 @@ end
 
 ### Actions
 
-Basic examples;
+Basic examples:
 
 ```ruby
 Gitlab::HTTP_V2.post(uri, body: body)
 
 Gitlab::HTTP_V2.try_get(uri, params)
 
-response = Gitlab::HTTP_V2.head(project_url, verify: true)
+response = Gitlab::HTTP_V2.head(project_url, verify: true) # returns an HTTParty::Response object
 
-Gitlab::HTTP_V2.post(path, base_uri: base_uri, **params)
+Gitlab::HTTP_V2.post(path, base_uri: base_uri, **params) # returns an HTTParty::Response object
+```
+
+Async usage examples:
+
+```ruby
+lazy_response = Gitlab::HTTP_V2.get(location, async: true)
+
+lazy_response.execute # starts the request and returns the same LazyResponse object
+lazy_response.wait # waits for the request to finish and returns the same LazyResponse object
+
+response = lazy_response.value # returns an HTTParty::Response object
 ```
 
 ## Development

gems/gitlab-http/gitlab-http.gemspec

@@ -20,6 +20,7 @@ Gem::Specification.new do |spec|
   spec.require_paths = ["lib"]
 
   spec.add_runtime_dependency 'activesupport', '~> 7'
+  spec.add_runtime_dependency 'concurrent-ruby', '~> 1.2'
   spec.add_runtime_dependency 'httparty', '~> 0.21.0'
   spec.add_runtime_dependency 'ipaddress', '~> 0.8.3'
   spec.add_runtime_dependency 'nokogiri', '~> 1.15.4'

gems/gitlab-http/lib/gitlab/http_v2/client.rb

@@ -4,7 +4,8 @@
 require 'net/http'
 require 'active_support/all'
 require_relative 'new_connection_adapter'
-require_relative "exceptions"
+require_relative 'exceptions'
+require_relative 'lazy_response'
 
 module Gitlab
   module HTTP_V2
@@ -45,9 +46,12 @@ def configuration
         # TODO: This overwrites a method implemented by `HTTPParty`
         # The calls to `get/...` will call this method instead of `httparty_perform_request`
         def perform_request(http_method, path, options, &block)
+          raise_if_options_are_invalid(options)
           raise_if_blocked_by_silent_mode(http_method) if options.delete(:silent_mode_enabled)
 
           log_info = options.delete(:extra_log_info)
+          async = options.delete(:async)
+
           options_with_timeouts =
             if !options.has_key?(:timeout)
               options.with_defaults(DEFAULT_TIMEOUT_OPTIONS)
@@ -57,29 +61,57 @@ def perform_request(http_method, path, options, &block)
 
           if options[:stream_body]
             httparty_perform_request(http_method, path, options_with_timeouts, &block)
+          elsif async
+            async_perform_request(http_method, path, options, options_with_timeouts, log_info, &block)
           else
-            begin
-              start_time = nil
-              read_total_timeout = options.fetch(:timeout, DEFAULT_READ_TOTAL_TIMEOUT)
-
-              httparty_perform_request(http_method, path, options_with_timeouts) do |fragment|
-                start_time ||= system_monotonic_time
-                elapsed = system_monotonic_time - start_time
-
-                raise ReadTotalTimeout, "Request timed out after #{elapsed} seconds" if elapsed > read_total_timeout
-
-                yield fragment if block
-              end
-            rescue HTTParty::RedirectionTooDeep
-              raise RedirectionTooDeep
-            rescue *HTTP_ERRORS => e
-              extra_info = log_info || {}
-              extra_info = log_info.call(e, path, options) if log_info.respond_to?(:call)
-              configuration.log_exception(e, extra_info)
-
-              raise e
+            sync_perform_request(http_method, path, options, options_with_timeouts, log_info, &block)
+          end
+        end
+
+        def async_perform_request(http_method, path, options, options_with_timeouts, log_info, &block)
+          start_time = nil
+          read_total_timeout = options.fetch(:timeout, DEFAULT_READ_TOTAL_TIMEOUT)
+
+          promise = Concurrent::Promise.new do
+            httparty_perform_request(http_method, path, options_with_timeouts) do |fragment|
+              start_time ||= system_monotonic_time
+              elapsed = system_monotonic_time - start_time
+
+              raise ReadTotalTimeout, "Request timed out after #{elapsed} seconds" if elapsed > read_total_timeout
+
+              yield fragment if block
             end
           end
+
+          LazyResponse.new(promise, path, options, log_info)
+        end
+
+        def sync_perform_request(http_method, path, options, options_with_timeouts, log_info, &block)
+          start_time = nil
+          read_total_timeout = options.fetch(:timeout, DEFAULT_READ_TOTAL_TIMEOUT)
+
+          httparty_perform_request(http_method, path, options_with_timeouts) do |fragment|
+            start_time ||= system_monotonic_time
+            elapsed = system_monotonic_time - start_time
+
+            raise ReadTotalTimeout, "Request timed out after #{elapsed} seconds" if elapsed > read_total_timeout
+
+            yield fragment if block
+          end
+        rescue HTTParty::RedirectionTooDeep
+          raise RedirectionTooDeep
+        rescue *HTTP_ERRORS => e
+          extra_info = log_info || {}
+          extra_info = log_info.call(e, path, options) if log_info.respond_to?(:call)
+          configuration.log_exception(e, extra_info)
+
+          raise e
+        end
+
+        def raise_if_options_are_invalid(options)
+          return unless options[:async] && (options[:stream_body] || options[:silent_mode_enabled])
+
+          raise ArgumentError, '`async` cannot be used with `stream_body` or `silent_mode_enabled`'
         end
 
         def raise_if_blocked_by_silent_mode(http_method)

gems/gitlab-http/lib/gitlab/http_v2/lazy_response.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module HTTP_V2
+    class LazyResponse
+      NotExecutedError = Class.new(StandardError)
+
+      attr_reader :promise
+
+      delegate :state, to: :promise
+
+      def initialize(promise, path, options, log_info)
+        @promise = promise
+        @path = path
+        @options = options
+        @log_info = log_info
+      end
+
+      def execute
+        @promise.execute
+        self
+      end
+
+      def wait
+        @promise.wait
+        self
+      end
+
+      def value
+        raise NotExecutedError, '`execute` must be called before `value`' if @promise.unscheduled?
+
+        wait # wait for the promise to be completed
+
+        raise @promise.reason if @promise.rejected?
+
+        @promise.value
+      rescue HTTParty::RedirectionTooDeep
+        raise HTTP_V2::RedirectionTooDeep
+      rescue *HTTP_V2::HTTP_ERRORS => e
+        extra_info = @log_info || {}
+        extra_info = @log_info.call(e, @path, @options) if @log_info.respond_to?(:call)
+        Gitlab::HTTP_V2.configuration.log_exception(e, extra_info)
+
+        raise e
+      end
+    end
+  end
+end

gems/gitlab-http/spec/gitlab/http_v2_spec.rb

@@ -450,4 +450,101 @@ def read_body(*)
       end
     end
   end
+
+  context 'when options[:async] is true' do
+    context 'when it is a valid request' do
+      before do
+        stub_full_request('http://example.org', method: :any).to_return(status: 200, body: 'hello world')
+      end
+
+      it 'returns a LazyResponse' do
+        result = described_class.get('http://example.org', async: true)
+
+        expect(result).to be_a(Gitlab::HTTP_V2::LazyResponse)
+        expect(result.state).to eq(:unscheduled)
+
+        expect(result.execute).to be_a(Gitlab::HTTP_V2::LazyResponse)
+        expect(result.wait).to be_a(Gitlab::HTTP_V2::LazyResponse)
+
+        expect(result.value).to be_a(HTTParty::Response)
+        expect(result.value.body).to eq('hello world')
+      end
+    end
+
+    context 'when the URL is denied' do
+      let(:url) { 'http://localhost:3003' }
+      let(:error_class) { Gitlab::HTTP_V2::BlockedUrlError }
+      let(:opts) { {} }
+
+      let(:result) do
+        described_class.get(url, allow_local_requests: false, async: true, **opts)
+      end
+
+      it 'returns a LazyResponse with error value' do
+        expect(result).to be_a(Gitlab::HTTP_V2::LazyResponse)
+
+        expect { result.execute.value }.to raise_error(error_class)
+      end
+
+      it 'logs the exception' do
+        expect(described_class.configuration)
+          .to receive(:log_exception)
+          .with(instance_of(error_class), {})
+
+        expect { result.execute.value }.to raise_error(error_class)
+      end
+
+      context 'with extra_log_info as hash' do
+        let(:opts) { { extra_log_info: { a: :b } } }
+
+        it 'handles the request' do
+          expect(described_class.configuration)
+            .to receive(:log_exception)
+            .with(instance_of(error_class), { a: :b })
+
+          expect { result.execute.value }.to raise_error(error_class)
+        end
+      end
+
+      context 'with extra_log_info as proc' do
+        let(:extra_log_info) do
+          proc do |error, url, options|
+            { klass: error.class, url: url, options: options }
+          end
+        end
+
+        let(:opts) { { extra_log_info: extra_log_info } }
+
+        it 'handles the request' do
+          expect(described_class.configuration)
+            .to receive(:log_exception)
+            .with(instance_of(error_class), { url: url, klass: error_class, options: { allow_local_requests: false } })
+
+          expect { result.execute.value }.to raise_error(error_class)
+        end
+      end
+    end
+  end
+
+  context 'when options[:async] and options[:stream_body] are true' do
+    before do
+      stub_full_request('http://example.org', method: :any)
+    end
+
+    it 'raises an ArgumentError' do
+      expect { described_class.get('http://example.org', async: true, stream_body: true) }
+        .to raise_error(ArgumentError, '`async` cannot be used with `stream_body` or `silent_mode_enabled`')
+    end
+  end
+
+  context 'when options[:async] and options[:silent_mode_enabled] are true' do
+    before do
+      stub_full_request('http://example.org', method: :any)
+    end
+
+    it 'raises an ArgumentError' do
+      expect { described_class.get('http://example.org', async: true, silent_mode_enabled: true) }
+        .to raise_error(ArgumentError, '`async` cannot be used with `stream_body` or `silent_mode_enabled`')
+    end
+  end
 end