Improve the Runtime::API classes

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

8 files changed, 132 insertions, 126 deletions

diff --git a/qa/qa.rb b/qa/qa.rb
index 7f2da05dd63..503379823f4 100644
--- a/qa/qa.rb
+++ b/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'
diff --git a/qa/qa/runtime/api.rb b/qa/qa/runtime/api.rb
deleted file mode 100644
index e2a096b971d..00000000000
--- a/qa/qa/runtime/api.rb
+++ /dev/null
@@ -1,82 +0,0 @@
-require 'airborne'
-
-module QA
-  module Runtime
-    module API
-      class Client
-        attr_reader :address
-
-        def initialize(address = :gitlab)
-          @address = address
-        end
-
-        def personal_access_token
-          @personal_access_token ||= get_personal_access_token
-        end
-
-        def get_personal_access_token
-          # you can set the environment variable PERSONAL_ACCESS_TOKEN
-          # to use a specific access token rather than create one from the UI
-          if Runtime::Env.personal_access_token
-            Runtime::Env.personal_access_token
-          else
-            create_personal_access_token
-          end
-        end
-
-        private
-
-        def create_personal_access_token
-          Runtime::Browser.visit(@address, Page::Main::Login) do
-            Page::Main::Login.act { sign_in_using_credentials }
-            Factory::Resource::PersonalAccessToken.fabricate!.access_token
-          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
diff --git a/qa/qa/runtime/api/client.rb b/qa/qa/runtime/api/client.rb
new file mode 100644
index 00000000000..02015e23ad8
--- /dev/null
+++ b/qa/qa/runtime/api/client.rb
@@ -0,0 +1,39 @@
+require 'airborne'
+
+module QA
+  module Runtime
+    module API
+      class Client
+        attr_reader :address
+
+        def initialize(address = :gitlab, personal_access_token: nil)
+          @address = address
+          @personal_access_token = personal_access_token
+        end
+
+        def personal_access_token
+          @personal_access_token ||= get_personal_access_token
+        end
+
+        def get_personal_access_token
+          # you can set the environment variable PERSONAL_ACCESS_TOKEN
+          # to use a specific access token rather than create one from the UI
+          if Runtime::Env.personal_access_token
+            Runtime::Env.personal_access_token
+          else
+            create_personal_access_token
+          end
+        end
+
+        private
+
+        def create_personal_access_token
+          Runtime::Browser.visit(@address, Page::Main::Login) do
+            Page::Main::Login.act { sign_in_using_credentials }
+            Factory::Resource::PersonalAccessToken.fabricate!.access_token
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/qa/qa/runtime/api/request.rb b/qa/qa/runtime/api/request.rb
new file mode 100644
index 00000000000..c33ada0de7a
--- /dev/null
+++ b/qa/qa/runtime/api/request.rb
@@ -0,0 +1,43 @@
+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
diff --git a/qa/qa/specs/features/api/users_spec.rb b/qa/qa/specs/features/api/users_spec.rb
index 38f4c497183..0aecf89e1b7 100644
--- a/qa/qa/specs/features/api/users_spec.rb
+++ b/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
 
diff --git a/qa/spec/runtime/api_client_spec.rb b/qa/spec/runtime/api/client_spec.rb
index d497d8839b8..d497d8839b8 100644
--- a/qa/spec/runtime/api_client_spec.rb
+++ b/qa/spec/runtime/api/client_spec.rb
diff --git a/qa/spec/runtime/api/request_spec.rb b/qa/spec/runtime/api/request_spec.rb
new file mode 100644
index 00000000000..80e3149f32d
--- /dev/null
+++ b/qa/spec/runtime/api/request_spec.rb
@@ -0,0 +1,44 @@
+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
diff --git a/qa/spec/runtime/api_request_spec.rb b/qa/spec/runtime/api_request_spec.rb
index 8cf4b040c24..e69de29bb2d 100644
--- a/qa/spec/runtime/api_request_spec.rb
+++ b/qa/spec/runtime/api_request_spec.rb
@@ -1,42 +0,0 @@
-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