Improve the Runtime::API classes

Signed-off-by: Rémy Coutable <remy@rymai.me>

qa/qa.rb

@@ -12,7 +12,11 @@ module QA
     autoload :Browser, 'qa/runtime/browser'
     autoload :Env, 'qa/runtime/env'
     autoload :Address, 'qa/runtime/address'
-    autoload :API, 'qa/runtime/api'
+
+    module API
+      autoload :Client, 'qa/runtime/api/client'
+      autoload :Request, 'qa/runtime/api/request'
+    end
 
     module Key
       autoload :Base, 'qa/runtime/key/base'

qa/qa/runtime/api.rb → qa/qa/runtime/api/client.rb

@@ -6,8 +6,9 @@ module QA
       class Client
         attr_reader :address
 
-        def initialize(address = :gitlab)
+        def initialize(address = :gitlab, personal_access_token: nil)
           @address = address
+          @personal_access_token = personal_access_token
         end
 
         def personal_access_token
@@ -33,50 +34,6 @@ module QA
           end
         end
       end
-
-      class Request
-        API_VERSION = 'v4'.freeze
-
-        def initialize(api_client, path, personal_access_token: nil)
-          personal_access_token ||= api_client.personal_access_token
-          request_path = request_path(path, personal_access_token: personal_access_token)
-          @session_address = Runtime::Address.new(api_client.address, request_path)
-        end
-
-        def url
-          @session_address.address
-        end
-
-        # Prepend a request path with the path to the API
-        #
-        # path - Path to append
-        #
-        # Examples
-        #
-        #   >> request_path('/issues')
-        #   => "/api/v4/issues"
-        #
-        #   >> request_path('/issues', personal_access_token: 'sometoken)
-        #   => "/api/v4/issues?private_token=..."
-        #
-        # Returns the relative path to the requested API resource
-        def request_path(path, version: API_VERSION, personal_access_token: nil, oauth_access_token: nil)
-          full_path = File.join('/api', version, path)
-
-          if oauth_access_token
-            query_string = "access_token=#{oauth_access_token}"
-          elsif personal_access_token
-            query_string = "private_token=#{personal_access_token}"
-          end
-
-          if query_string
-            full_path << (path.include?('?') ? '&' : '?')
-            full_path << query_string
-          end
-
-          full_path
-        end
-      end
     end
   end
 end

qa/qa/runtime/api/request.rb

+module QA
+  module Runtime
+    module API
+      class Request
+        API_VERSION = 'v4'.freeze
+
+        def initialize(api_client, path, **query_string)
+          query_string[:private_token] ||= api_client.personal_access_token unless query_string[:oauth_access_token]
+          request_path = request_path(path, **query_string)
+          @session_address = Runtime::Address.new(api_client.address, request_path)
+        end
+
+        def url
+          @session_address.address
+        end
+
+        # Prepend a request path with the path to the API
+        #
+        # path - Path to append
+        #
+        # Examples
+        #
+        #   >> request_path('/issues')
+        #   => "/api/v4/issues"
+        #
+        #   >> request_path('/issues', private_token: 'sometoken)
+        #   => "/api/v4/issues?private_token=..."
+        #
+        # Returns the relative path to the requested API resource
+        def request_path(path, version: API_VERSION, **query_string)
+          full_path = File.join('/api', version, path)
+
+          if query_string.any?
+            full_path << (path.include?('?') ? '&' : '?')
+            full_path << query_string.map { |k, v| "#{k}=#{CGI.escape(v)}" }.join('&')
+          end
+
+          full_path
+        end
+      end
+    end
+  end
+end

qa/qa/specs/features/api/users_spec.rb

@@ -31,7 +31,7 @@ module QA
     end
 
     scenario 'submit request with an invalid token' do
-      request = Runtime::API::Request.new(@api_client, '/users', personal_access_token: 'invalid')
+      request = Runtime::API::Request.new(@api_client, '/users', private_token: 'invalid')
 
       get request.url
 

qa/spec/runtime/api/request_spec.rb

+describe QA::Runtime::API::Request do
+  include Support::StubENV
+
+  before do
+    stub_env('PERSONAL_ACCESS_TOKEN', 'a_token')
+  end
+
+  let(:client)  { QA::Runtime::API::Client.new('http://example.com') }
+  let(:request) { described_class.new(client, '/users') }
+
+  describe '#url' do
+    it 'returns the full api request url' do
+      expect(request.url).to eq 'http://example.com/api/v4/users?private_token=a_token'
+    end
+  end
+
+  describe '#request_path' do
+    it 'prepends the api path' do
+      expect(request.request_path('/users')).to eq '/api/v4/users'
+    end
+
+    it 'adds the personal access token' do
+      expect(request.request_path('/users', private_token: 'token'))
+        .to eq '/api/v4/users?private_token=token'
+    end
+
+    it 'adds the oauth access token' do
+      expect(request.request_path('/users', access_token: 'otoken'))
+        .to eq '/api/v4/users?access_token=otoken'
+    end
+
+    it 'respects query parameters' do
+      expect(request.request_path('/users?page=1')).to eq '/api/v4/users?page=1'
+      expect(request.request_path('/users', private_token: 'token', foo: 'bar/baz'))
+        .to eq '/api/v4/users?private_token=token&foo=bar%2Fbaz'
+      expect(request.request_path('/users?page=1', private_token: 'token', foo: 'bar/baz'))
+        .to eq '/api/v4/users?page=1&private_token=token&foo=bar%2Fbaz'
+    end
+
+    it 'uses a different api version' do
+      expect(request.request_path('/users', version: 'other_version')).to eq '/api/other_version/users'
+    end
+  end
+end

qa/spec/runtime/api_request_spec.rb

-describe QA::Runtime::API::Request do
-  include Support::StubENV
-
-  before do
-    stub_env('PERSONAL_ACCESS_TOKEN', 'a_token')
-  end
-
-  let(:client)  { QA::Runtime::API::Client.new('http://example.com') }
-  let(:request) { described_class.new(client, '/users') }
-
-  describe '#url' do
-    it 'returns the full api request url' do
-      expect(request.url).to eq 'http://example.com/api/v4/users?private_token=a_token'
-    end
-  end
-
-  describe '#request_path' do
-    it 'prepends the api path' do
-      expect(request.request_path('/users')).to eq '/api/v4/users'
-    end
-
-    it 'adds the personal access token' do
-      expect(request.request_path('/users', personal_access_token: 'token'))
-        .to eq '/api/v4/users?private_token=token'
-    end
-
-    it 'adds the oauth access token' do
-      expect(request.request_path('/users', oauth_access_token: 'otoken'))
-        .to eq '/api/v4/users?access_token=otoken'
-    end
-
-    it 'respects query parameters' do
-      expect(request.request_path('/users?page=1')).to eq '/api/v4/users?page=1'
-      expect(request.request_path('/users?page=1', personal_access_token: 'token'))
-        .to eq '/api/v4/users?page=1&private_token=token'
-    end
-
-    it 'uses a different api version' do
-      expect(request.request_path('/users', version: 'other_version')).to eq '/api/other_version/users'
-    end
-  end
-end