Merge branch 'refactor-jira-requets-1' into 'master'

Extract Jira requests to separate classes

See merge request gitlab-org/gitlab!33157

app/graphql/resolvers/projects/jira_projects_resolver.rb

@@ -57,7 +57,7 @@ module Resolvers
       def jira_projects(name:, start_at:, limit:)
         args = { query: name, start_at: start_at, limit: limit }.compact
 
-        response = jira_service&.jira_projects(args)
+        response = Jira::Requests::Projects.new(project.jira_service, args).execute
         projects = response.payload[:projects]
         start_cursor = start_at == 0 ? nil : Base64.encode64((start_at - 1).to_s)
         end_cursor = Base64.encode64((start_at + projects.size - 1).to_s)

app/models/project_services/jira_service.rb

@@ -226,26 +226,8 @@ class JiraService < IssueTrackerService
     true
   end
 
-  def jira_projects(query: '', limit: PROJECTS_PER_PAGE, start_at: 0)
-    return ServiceResponse.success(payload: { projects: [], is_last: true }) if limit.to_i <= 0
-
-    response = jira_request { client.get(projects_url(query: query, limit: limit.to_i, start_at: start_at.to_i)) }
-
-    return ServiceResponse.error(message: @error.message) if @error.present?
-    return ServiceResponse.success(payload: { projects: [] }) unless response['values'].present?
-
-    projects = response['values'].map { |v| JIRA::Resource::Project.build(client, v) }
-
-    ServiceResponse.success(payload: { projects: projects, is_last: response['isLast'] })
-  end
-
   private
 
-  def projects_url(query:, limit:, start_at:)
-    '/rest/api/2/project/search?query=%{query}&maxResults=%{limit}&startAt=%{start_at}' %
-    { query: CGI.escape(query.to_s), limit: limit, start_at: start_at }
-  end
-
   def test_settings
     return unless client_url.present?
 

app/services/jira/requests/base.rb

+# frozen_string_literal: true
+
+module Jira
+  module Requests
+    class Base
+      include ProjectServicesLoggable
+
+      PER_PAGE = 50
+
+      attr_reader :jira_service, :project, :limit, :start_at, :query
+
+      def initialize(jira_service, limit: PER_PAGE, start_at: 0, query: nil)
+        @project = jira_service&.project
+        @jira_service = jira_service
+
+        @limit    = limit
+        @start_at = start_at
+        @query    = query
+      end
+
+      def execute
+        return ServiceResponse.error(message: _('Jira service not configured.')) unless jira_service&.active?
+        return ServiceResponse.success(payload: empty_payload) if limit.to_i <= 0
+
+        request
+      end
+
+      private
+
+      def client
+        @client ||= jira_service.client
+      end
+
+      def request
+        response = client.get(url)
+        build_service_response(response)
+      rescue Timeout::Error, Errno::EINVAL, Errno::ECONNRESET, Errno::ECONNREFUSED, URI::InvalidURIError, JIRA::HTTPError, OpenSSL::SSL::SSLError => error
+        error_message = error.message
+        log_error("Error sending message", client_url: client.options[:site], error: error_message)
+        ServiceResponse.error(message: error_message)
+      end
+
+      def url
+        raise NotImplementedError
+      end
+
+      def build_service_response(response)
+        raise NotImplementedError
+      end
+    end
+  end
+end

app/services/jira/requests/projects.rb

+# frozen_string_literal: true
+
+module Jira
+  module Requests
+    class Projects < Base
+      extend ::Gitlab::Utils::Override
+
+      private
+
+      override :url
+      def url
+        '/rest/api/2/project/search?query=%{query}&maxResults=%{limit}&startAt=%{start_at}' %
+        { query: CGI.escape(query.to_s), limit: limit.to_i, start_at: start_at.to_i }
+      end
+
+      override :build_service_response
+      def build_service_response(response)
+        return ServiceResponse.success(payload: empty_payload) unless response['values'].present?
+
+        ServiceResponse.success(payload: { projects: map_projects(response), is_last: response['isLast'] })
+      end
+
+      def map_projects(response)
+        response['values'].map { |v| JIRA::Resource::Project.build(client, v) }
+      end
+
+      def empty_payload
+        { projects: [], is_last: true }
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -12346,6 +12346,9 @@ msgstr ""
 msgid "Jira project: %{importProject}"
 msgstr ""
 
+msgid "Jira service not configured."
+msgstr ""
+
 msgid "JiraService| on branch %{branch_link}"
 msgstr ""
 

spec/models/project_services/jira_service_spec.rb

@@ -812,85 +812,4 @@ describe JiraService do
       end
     end
   end
-
-  describe '#jira_projects' do
-    let(:project) { create(:project) }
-    let(:jira_service) do
-      described_class.new(
-        project: project,
-        url: url,
-        username: username,
-        password: password
-      )
-    end
-
-    context 'when request to the jira server fails' do
-      it 'returns error' do
-        test_url = "#{url}/rest/api/2/project/search?maxResults=50&query=&startAt=0"
-        WebMock.stub_request(:get, test_url).with(basic_auth: [username, password])
-          .to_raise(JIRA::HTTPError.new(double(message: 'random error')))
-
-        response = jira_service.jira_projects
-
-        expect(response.error?).to be true
-        expect(response.message).to eq('random error')
-      end
-    end
-
-    context 'with invalid params' do
-      it 'escapes params' do
-        escaped_url = "#{url}/rest/api/2/project/search?query=Test%26maxResults%3D3&maxResults=10&startAt=0"
-        WebMock.stub_request(:get, escaped_url).with(basic_auth: [username, password])
-          .to_return(body: {}.to_json, headers: { "Content-Type": "application/json" })
-
-        response = jira_service.jira_projects(query: 'Test&maxResults=3', limit: 10, start_at: 'zero')
-
-        expect(response.error?).to be false
-      end
-    end
-
-    context 'when no jira_projects are returned' do
-      let(:jira_projects_json) do
-        '{
-          "self": "https://your-domain.atlassian.net/rest/api/2/project/search?startAt=0&maxResults=2",
-          "nextPage": "https://your-domain.atlassian.net/rest/api/2/project/search?startAt=2&maxResults=2",
-          "maxResults": 2,
-          "startAt": 0,
-          "total": 7,
-          "isLast": false,
-          "values": []
-        }'
-      end
-
-      it 'returns empty array of jira projects' do
-        test_url = "#{url}/rest/api/2/project/search?maxResults=50&query=&startAt=0"
-        WebMock.stub_request(:get, test_url).with(basic_auth: [username, password])
-          .to_return(body: jira_projects_json, headers: { "Content-Type": "application/json" })
-
-        response = jira_service.jira_projects
-
-        expect(response.success?).to be true
-        expect(response.payload).not_to be nil
-      end
-    end
-
-    context 'when jira_projects are returned' do
-      include_context 'jira projects request context'
-
-      it 'returns array of jira projects' do
-        response = jira_service.jira_projects
-
-        projects = response.payload[:projects]
-        project_keys = projects.map(&:key)
-        project_names = projects.map(&:name)
-        project_ids = projects.map(&:id)
-
-        expect(response.success?).to be true
-        expect(projects.size).to eq(2)
-        expect(project_keys).to eq(%w(EX ABC))
-        expect(project_names).to eq(%w(Example Alphabetical))
-        expect(project_ids).to eq(%w(10000 10001))
-      end
-    end
-  end
 end

spec/services/jira/requests/projects_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Jira::Requests::Projects do
+  let(:jira_service) { create(:jira_service) }
+  let(:params) { {} }
+
+  describe '#execute' do
+    let(:service) { described_class.new(jira_service, params) }
+
+    subject { service.execute }
+
+    context 'without jira_service' do
+      before do
+        jira_service.update!(active: false)
+      end
+
+      it 'returns an error response' do
+        expect(subject.error?).to be_truthy
+        expect(subject.message).to eq('Jira service not configured.')
+      end
+    end
+
+    context 'when jira_service is nil' do
+      let(:jira_service) { nil }
+
+      it 'returns an error response' do
+        expect(subject.error?).to be_truthy
+        expect(subject.message).to eq('Jira service not configured.')
+      end
+    end
+
+    context 'with jira_service' do
+      context 'when limit is invalid' do
+        let(:params) { { limit: 0 } }
+
+        it 'returns a paylod with no projects returned' do
+          expect(subject.payload[:projects]).to be_empty
+        end
+      end
+
+      context 'when validations and params are ok' do
+        let(:client) { double(options: { site: 'https://jira.example.com' }) }
+
+        before do
+          expect(service).to receive(:client).at_least(:once).and_return(client)
+        end
+
+        context 'when the request to Jira returns an error' do
+          before do
+            expect(client).to receive(:get).and_raise(Timeout::Error)
+          end
+
+          it 'returns an error response' do
+            expect(subject.error?).to be_truthy
+            expect(subject.message).to eq('Timeout::Error')
+          end
+        end
+
+        context 'when the request does not return any values' do
+          before do
+            expect(client).to receive(:get).and_return({ 'someKey' => 'value' })
+          end
+
+          it 'returns a paylod with no projects returned' do
+            payload = subject.payload
+
+            expect(subject.success?).to be_truthy
+            expect(payload[:projects]).to be_empty
+            expect(payload[:is_last]).to be_truthy
+          end
+        end
+
+        context 'when the request returns values' do
+          before do
+            expect(client).to receive(:get).and_return(
+              { 'values' => %w(project1 project2), 'isLast' => false }
+            )
+            expect(JIRA::Resource::Project).to receive(:build).with(client, 'project1').and_return('jira_project1')
+            expect(JIRA::Resource::Project).to receive(:build).with(client, 'project2').and_return('jira_project2')
+          end
+
+          it 'returns a paylod with jira projets' do
+            payload = subject.payload
+
+            expect(subject.success?).to be_truthy
+            expect(payload[:projects]).to eq(%w(jira_project1 jira_project2))
+            expect(payload[:is_last]).to be_falsey
+          end
+        end
+      end
+    end
+  end
+end