Merge branch '235759_backward_compatibility_on_jira_users_api_endpoint' into 'master'

Handle user mapping for Jira server

See merge request gitlab-org/gitlab!39362

app/models/project_services/jira_service.rb

@@ -8,6 +8,12 @@ class JiraService < IssueTrackerService
 
   PROJECTS_PER_PAGE = 50
 
+  # TODO: use jira_service.deployment_type enum when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37003 is merged
+  DEPLOYMENT_TYPES = {
+    server: 'SERVER',
+    cloud: 'CLOUD'
+  }.freeze
+
   validates :url, public_url: true, presence: true, if: :activated?
   validates :api_url, public_url: true, allow_blank: true
   validates :username, presence: true, if: :activated?

app/services/jira_import/cloud_users_mapper_service.rb

+# frozen_string_literal: true
+
+module JiraImport
+  class CloudUsersMapperService < UsersMapperService
+    private
+
+    def url
+      "/rest/api/2/users?maxResults=#{MAX_USERS}&startAt=#{start_at.to_i}"
+    end
+
+    def jira_user_id(jira_user)
+      jira_user['accountId']
+    end
+
+    def jira_user_name(jira_user)
+      jira_user['displayName']
+    end
+  end
+end

app/services/jira_import/server_users_mapper_service.rb

+# frozen_string_literal: true
+
+module JiraImport
+  class ServerUsersMapperService < UsersMapperService
+    private
+
+    def url
+      "/rest/api/2/user/search?username=''&maxResults=#{MAX_USERS}&startAt=#{start_at.to_i}"
+    end
+
+    def jira_user_id(jira_user)
+      jira_user['key']
+    end
+
+    def jira_user_name(jira_user)
+      jira_user['name']
+    end
+  end
+end

app/services/jira_import/users_importer.rb

@@ -2,9 +2,7 @@
 
 module JiraImport
   class UsersImporter
-    attr_reader :user, :project, :start_at, :result
-
-    MAX_USERS = 50
+    attr_reader :user, :project, :start_at
 
     def initialize(user, project, start_at)
       @project = project
@@ -15,29 +13,43 @@ module JiraImport
     def execute
       Gitlab::JiraImport.validate_project_settings!(project, user: user)
 
-      return ServiceResponse.success(payload: nil) if users.blank?
-
-      result = UsersMapper.new(project, users).execute
-      ServiceResponse.success(payload: result)
+      ServiceResponse.success(payload: mapped_users)
     rescue Timeout::Error, Errno::EINVAL, Errno::ECONNRESET, Errno::ECONNREFUSED, URI::InvalidURIError, JIRA::HTTPError, OpenSSL::SSL::SSLError => error
-      Gitlab::ErrorTracking.track_exception(error, project_id: project.id, request: url)
-      ServiceResponse.error(message: "There was an error when communicating to Jira: #{error.message}")
+      Gitlab::ErrorTracking.track_exception(error, project_id: project.id)
+      ServiceResponse.error(message: "There was an error when communicating to Jira")
     rescue Projects::ImportService::Error => error
       ServiceResponse.error(message: error.message)
     end
 
     private
 
-    def users
-      @users ||= client.get(url)
+    def mapped_users
+      users_mapper_service.execute
+    end
+
+    def users_mapper_service
+      @users_mapper_service ||= user_mapper_service_factory
     end
 
-    def url
-      "/rest/api/2/users?maxResults=#{MAX_USERS}&startAt=#{start_at.to_i}"
+    def deployment_type
+      # TODO: use project.jira_service.deployment_type value when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37003 is merged
+      @deployment_type ||= client.ServerInfo.all.deploymentType
     end
 
     def client
       @client ||= project.jira_service.client
     end
+
+    def user_mapper_service_factory
+      # TODO: use deployment_type enum from jira service when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37003 is merged
+      case deployment_type.upcase
+      when JiraService::DEPLOYMENT_TYPES[:server]
+        ServerUsersMapperService.new(project.jira_service, start_at)
+      when JiraService::DEPLOYMENT_TYPES[:cloud]
+        CloudUsersMapperService.new(project.jira_service, start_at)
+      else
+        raise ArgumentError
+      end
+    end
   end
 end

app/services/jira_import/users_mapper.rb → app/services/jira_import/users_mapper_service.rb

 # frozen_string_literal: true
 
 module JiraImport
-  class UsersMapper
-    attr_reader :project, :jira_users
+  class UsersMapperService
+    MAX_USERS = 50
 
-    def initialize(project, jira_users)
-      @project = project
-      @jira_users = jira_users
+    attr_reader :jira_service, :start_at
+
+    def initialize(jira_service, start_at)
+      @jira_service = jira_service
+      @start_at = start_at
     end
 
     def execute
-      jira_users.to_a.map do |jira_user|
+      users.to_a.map do |jira_user|
         {
-          jira_account_id: jira_user['accountId'],
-          jira_display_name: jira_user['displayName'],
+          jira_account_id: jira_user_id(jira_user),
+          jira_display_name: jira_user_name(jira_user),
           jira_email: jira_user['emailAddress']
         }.merge(match_user(jira_user))
       end
@@ -21,6 +23,26 @@ module JiraImport
 
     private
 
+    def users
+      @users ||= client.get(url)
+    end
+
+    def client
+      @client ||= jira_service.client
+    end
+
+    def url
+      raise NotImplementedError
+    end
+
+    def jira_user_id(jira_user)
+      raise NotImplementedError
+    end
+
+    def jira_user_name(jira_user)
+      raise NotImplementedError
+    end
+
     # TODO: Matching user by email and displayName will be done as the part
     # of follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/219023
     def match_user(jira_user)

changelogs/unreleased/235759_backward_compatibility_on_jira_users_api_endpoint.yml

+---
+title: Handle user mapping for Jira server instances
+merge_request: 39362
+author:
+type: fixed

spec/services/jira_import/cloud_users_mapper_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe JiraImport::CloudUsersMapperService do
+  let(:start_at) { 7 }
+  let(:url) { "/rest/api/2/users?maxResults=50&startAt=#{start_at}" }
+  let(:jira_users) do
+    [
+      { 'accountId' => 'abcd', 'displayName' => 'user1' },
+      { 'accountId' => 'efg' },
+      { 'accountId' => 'hij', 'displayName' => 'user3', 'emailAddress' => 'user3@example.com' }
+    ]
+  end
+
+  describe '#execute' do
+    it_behaves_like 'mapping jira users'
+  end
+end

spec/services/jira_import/server_users_mapper_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe JiraImport::ServerUsersMapperService do
+  let(:start_at) { 7 }
+  let(:url) { "/rest/api/2/user/search?username=''&maxResults=50&startAt=#{start_at}" }
+  let(:jira_users) do
+    [
+      { 'key' => 'abcd', 'name' => 'user1' },
+      { 'key' => 'efg' },
+      { 'key' => 'hij', 'name' => 'user3', 'emailAddress' => 'user3@example.com' }
+    ]
+  end
+
+  describe '#execute' do
+    it_behaves_like 'mapping jira users'
+  end
+end

spec/services/jira_import/users_importer_spec.rb

@@ -14,6 +14,27 @@ RSpec.describe JiraImport::UsersImporter do
   subject { importer.execute }
 
   describe '#execute' do
+    let(:mapped_users) do
+      [
+        {
+          jira_account_id: 'acc1',
+          jira_display_name: 'user1',
+          jira_email: 'sample@jira.com',
+          gitlab_id: nil,
+          gitlab_username: nil,
+          gitlab_name: nil
+        },
+        {
+          jira_account_id: 'acc2',
+          jira_display_name: 'user2',
+          jira_email: nil,
+          gitlab_id: nil,
+          gitlab_username: nil,
+          gitlab_name: nil
+        }
+      ]
+    end
+
     before do
       stub_jira_service_test
       project.add_maintainer(user)
@@ -25,53 +46,83 @@ RSpec.describe JiraImport::UsersImporter do
       end
     end
 
-    context 'when Jira import is configured correctly' do
-      let_it_be(:jira_service) { create(:jira_service, project: project, active: true) }
-      let(:client) { double }
+    RSpec.shared_examples 'maps jira users to gitlab users' do
+      context 'when Jira import is configured correctly' do
+        let_it_be(:jira_service) { create(:jira_service, project: project, active: true) }
+        let(:client) { double }
 
-      before do
-        expect(importer).to receive(:client).and_return(client)
-      end
-
-      context 'when jira client raises an error' do
-        it 'returns an error response' do
-          expect(client).to receive(:get).and_raise(Timeout::Error)
-
-          expect(subject.error?).to be_truthy
-          expect(subject.message).to include('There was an error when communicating to Jira')
-        end
-      end
-
-      context 'when jira client returns result' do
         before do
-          allow(client).to receive(:get).with('/rest/api/2/users?maxResults=50&startAt=7')
-            .and_return(jira_users)
+          expect(importer).to receive(:client).at_least(1).and_return(client)
+          allow(client).to receive_message_chain(:ServerInfo, :all, :deploymentType).and_return(deployment_type)
         end
 
-        context 'when jira client returns an empty array' do
-          let(:jira_users) { [] }
+        context 'when jira client raises an error' do
+          it 'returns an error response' do
+            expect(client).to receive(:get).and_raise(Timeout::Error)
 
-          it 'retturns nil payload' do
-            expect(subject.success?).to be_truthy
-            expect(subject.payload).to be_nil
+            expect(subject.error?).to be_truthy
+            expect(subject.message).to include('There was an error when communicating to Jira')
           end
         end
 
-        context 'when jira client returns an results' do
-          let(:jira_users)   { [{ 'name' => 'user1' }, { 'name' => 'user2' }] }
-          let(:mapped_users) { [{ jira_display_name: 'user1', gitlab_id: 5 }] }
+        context 'when jira client returns result' do
+          context 'when jira client returns an empty array' do
+            let(:jira_users) { [] }
 
-          before do
-            expect(JiraImport::UsersMapper).to receive(:new).with(project, jira_users)
-            .and_return(double(execute: mapped_users))
+            it 'retturns nil payload' do
+              expect(subject.success?).to be_truthy
+              expect(subject.payload).to be_empty
+            end
           end
 
-          it 'returns the mapped users' do
-            expect(subject.success?).to be_truthy
-            expect(subject.payload).to eq(mapped_users)
+          context 'when jira client returns an results' do
+            it 'returns the mapped users' do
+              expect(subject.success?).to be_truthy
+              expect(subject.payload).to eq(mapped_users)
+            end
           end
         end
       end
     end
+
+    context 'when Jira instance is of Server deployment type' do
+      let(:deployment_type) { 'Server' }
+      let(:url) { "/rest/api/2/user/search?username=''&maxResults=50&startAt=#{start_at}" }
+      let(:jira_users) do
+        [
+          { 'key' => 'acc1', 'name' => 'user1', 'emailAddress' => 'sample@jira.com' },
+          { 'key' => 'acc2', 'name' => 'user2' }
+        ]
+      end
+
+      before do
+        allow_next_instance_of(JiraImport::ServerUsersMapperService) do |instance|
+          allow(instance).to receive(:client).and_return(client)
+          allow(client).to receive(:get).with(url).and_return(jira_users)
+        end
+      end
+
+      it_behaves_like 'maps jira users to gitlab users'
+    end
+
+    context 'when Jira instance is of Cloud deploymet type' do
+      let(:deployment_type) { 'Cloud' }
+      let(:url) { "/rest/api/2/users?maxResults=50&startAt=#{start_at}" }
+      let(:jira_users) do
+        [
+          { 'accountId' => 'acc1', 'displayName' => 'user1', 'emailAddress' => 'sample@jira.com' },
+          { 'accountId' => 'acc2', 'displayName' => 'user2' }
+        ]
+      end
+
+      before do
+        allow_next_instance_of(JiraImport::CloudUsersMapperService) do |instance|
+          allow(instance).to receive(:client).and_return(client)
+          allow(client).to receive(:get).with(url).and_return(jira_users)
+        end
+      end
+
+      it_behaves_like 'maps jira users to gitlab users'
+    end
   end
 end

spec/services/jira_import/users_mapper_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe JiraImport::UsersMapper do
-  let_it_be(:project) { create(:project) }
-
-  subject { described_class.new(project, jira_users).execute }
-
-  describe '#execute' do
-    context 'jira_users is nil' do
-      let(:jira_users) { nil }
-
-      it 'returns an empty array' do
-        expect(subject).to be_empty
-      end
-    end
-
-    context 'when jira_users is present' do
-      let(:jira_users) do
-        [
-          { 'accountId' => 'abcd', 'displayName' => 'user1' },
-          { 'accountId' => 'efg' },
-          { 'accountId' => 'hij', 'displayName' => 'user3', 'emailAddress' => 'user3@example.com' }
-        ]
-      end
-
-      # TODO: now we only create an array in a proper format
-      # mapping is tracked in https://gitlab.com/gitlab-org/gitlab/-/issues/219023
-      let(:mapped_users) do
-        [
-          { jira_account_id: 'abcd', jira_display_name: 'user1', jira_email: nil, gitlab_id: nil, gitlab_username: nil, gitlab_name: nil },
-          { jira_account_id: 'efg', jira_display_name: nil, jira_email: nil, gitlab_id: nil, gitlab_username: nil, gitlab_name: nil },
-          { jira_account_id: 'hij', jira_display_name: 'user3', jira_email: 'user3@example.com', gitlab_id: nil, gitlab_username: nil, gitlab_name: nil }
-        ]
-      end
-
-      it 'returns users mapped to Gitlab' do
-        expect(subject).to eq(mapped_users)
-      end
-    end
-  end
-end

spec/support/shared_examples/services/jira_import/user_mapper_services_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'mapping jira users' do
+  let(:client) { double }
+
+  let_it_be(:project) { create(:project) }
+  let_it_be(:jira_service) { create(:jira_service, project: project, active: true) }
+
+  before do
+    allow(subject).to receive(:client).and_return(client)
+    allow(client).to receive(:get).with(url).and_return(jira_users)
+  end
+
+  subject { described_class.new(jira_service, start_at) }
+
+  context 'jira_users is nil' do
+    let(:jira_users) { nil }
+
+    it 'returns an empty array' do
+      expect(subject.execute).to be_empty
+    end
+  end
+
+  context 'when jira_users is present' do
+    # TODO: now we only create an array in a proper format
+    # mapping is tracked in https://gitlab.com/gitlab-org/gitlab/-/issues/219023
+    let(:mapped_users) do
+      [
+        { jira_account_id: 'abcd', jira_display_name: 'user1', jira_email: nil, gitlab_id: nil, gitlab_username: nil, gitlab_name: nil },
+        { jira_account_id: 'efg', jira_display_name: nil, jira_email: nil, gitlab_id: nil, gitlab_username: nil, gitlab_name: nil },
+        { jira_account_id: 'hij', jira_display_name: 'user3', jira_email: 'user3@example.com', gitlab_id: nil, gitlab_username: nil, gitlab_name: nil }
+      ]
+    end
+
+    it 'returns users mapped to Gitlab' do
+      expect(subject.execute).to eq(mapped_users)
+    end
+  end
+end