summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBob Van Landuyt <bob@vanlanduyt.co>2019-07-03 19:44:05 +0200
committerBob Van Landuyt <bob@vanlanduyt.co>2019-07-10 17:15:43 +0200
commit32184839c3983babe53ea93fc16b7cde5ada95f6 (patch)
tree80b91701e08c15eafd49f7449e238847736244e2
parente787676b3752405cbe4ae75788c6cff53154783d (diff)
downloadgitlab-ce-32184839c3983babe53ea93fc16b7cde5ada95f6.tar.gz
Fetch users from Phabricator to link to issues
We fetch the users from Phabricator based on their Phabricator ID. If a user with the same username exists and is a member of the project, we set them as assignee or author. When a user is applicable, we also cache it in Redis so we don't have to perform the request again for the same phid.
-rw-r--r--doc/user/project/import/phabricator.md6
-rw-r--r--lib/gitlab/phabricator_import/cache/map.rb10
-rw-r--r--lib/gitlab/phabricator_import/conduit/user.rb31
-rw-r--r--lib/gitlab/phabricator_import/conduit/users_response.rb23
-rw-r--r--lib/gitlab/phabricator_import/issues/task_importer.rb12
-rw-r--r--lib/gitlab/phabricator_import/representation/task.rb12
-rw-r--r--lib/gitlab/phabricator_import/representation/user.rb25
-rw-r--r--lib/gitlab/phabricator_import/user_finder.rb52
-rw-r--r--spec/fixtures/phabricator_responses/user.search.json62
-rw-r--r--spec/lib/gitlab/phabricator_import/cache/map_spec.rb17
-rw-r--r--spec/lib/gitlab/phabricator_import/conduit/user_spec.rb49
-rw-r--r--spec/lib/gitlab/phabricator_import/conduit/users_response_spec.rb21
-rw-r--r--spec/lib/gitlab/phabricator_import/issues/importer_spec.rb32
-rw-r--r--spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb36
-rw-r--r--spec/lib/gitlab/phabricator_import/representation/task_spec.rb16
-rw-r--r--spec/lib/gitlab/phabricator_import/representation/user_spec.rb28
-rw-r--r--spec/lib/gitlab/phabricator_import/user_finder_spec.rb89
17 files changed, 500 insertions, 21 deletions
diff --git a/doc/user/project/import/phabricator.md b/doc/user/project/import/phabricator.md
index 5c624e3aff6..b8f89caba24 100644
--- a/doc/user/project/import/phabricator.md
+++ b/doc/user/project/import/phabricator.md
@@ -15,6 +15,12 @@ Currently, only the following basic fields are imported:
- Created at
- Closed at
+## Users
+
+The assignee and author of a user are deducted from a Task's owner and
+author: If a user with the same username has access to the namespace
+of the project being imported into, then the user will be linked.
+
## Enabling this feature
While this feature is incomplete, a feature flag is required to enable it so that
diff --git a/lib/gitlab/phabricator_import/cache/map.rb b/lib/gitlab/phabricator_import/cache/map.rb
index fa8b37b20ca..6a2841b6a8e 100644
--- a/lib/gitlab/phabricator_import/cache/map.rb
+++ b/lib/gitlab/phabricator_import/cache/map.rb
@@ -9,9 +9,15 @@ module Gitlab
def get_gitlab_model(phabricator_id)
cached_info = get(phabricator_id)
- return unless cached_info[:classname] && cached_info[:database_id]
- cached_info[:classname].constantize.find_by_id(cached_info[:database_id])
+ if cached_info[:classname] && cached_info[:database_id]
+ object = cached_info[:classname].constantize.find_by_id(cached_info[:database_id])
+ else
+ object = yield if block_given?
+ set_gitlab_model(object, phabricator_id) if object
+ end
+
+ object
end
def set_gitlab_model(object, phabricator_id)
diff --git a/lib/gitlab/phabricator_import/conduit/user.rb b/lib/gitlab/phabricator_import/conduit/user.rb
new file mode 100644
index 00000000000..fc8c3f7cde9
--- /dev/null
+++ b/lib/gitlab/phabricator_import/conduit/user.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+module Gitlab
+ module PhabricatorImport
+ module Conduit
+ class User
+ MAX_PAGE_SIZE = 100
+
+ def initialize(phabricator_url:, api_token:)
+ @client = Client.new(phabricator_url, api_token)
+ end
+
+ def users(phids)
+ phids.each_slice(MAX_PAGE_SIZE).map { |limited_phids| get_page(limited_phids) }
+ end
+
+ private
+
+ def get_page(phids)
+ UsersResponse.new(get_users(phids))
+ end
+
+ def get_users(phids)
+ client.get('user.search',
+ params: { constraints: { phids: phids } })
+ end
+
+ attr_reader :client
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/phabricator_import/conduit/users_response.rb b/lib/gitlab/phabricator_import/conduit/users_response.rb
new file mode 100644
index 00000000000..3dfb29a7be5
--- /dev/null
+++ b/lib/gitlab/phabricator_import/conduit/users_response.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module PhabricatorImport
+ module Conduit
+ class UsersResponse
+ def initialize(conduit_response)
+ @conduit_response = conduit_response
+ end
+
+ def users
+ @users ||= conduit_response.data.map do |user_json|
+ Gitlab::PhabricatorImport::Representation::User.new(user_json)
+ end
+ end
+
+ private
+
+ attr_reader :conduit_response
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/phabricator_import/issues/task_importer.rb b/lib/gitlab/phabricator_import/issues/task_importer.rb
index 40d4392cbc1..77ee11c7cdd 100644
--- a/lib/gitlab/phabricator_import/issues/task_importer.rb
+++ b/lib/gitlab/phabricator_import/issues/task_importer.rb
@@ -8,9 +8,7 @@ module Gitlab
end
def execute
- # TODO: get the user from the project namespace from the username loaded by Phab-id
- # https://gitlab.com/gitlab-org/gitlab-ce/issues/60565
- issue.author = User.ghost
+ issue.author = user_finder.find(task.author_phid) || User.ghost
# TODO: Reformat the description with attachments, escaping accidental
# links and add attachments
@@ -19,6 +17,10 @@ module Gitlab
save!
+ if owner = user_finder.find(task.owner_phid)
+ issue.assignees << owner
+ end
+
issue
end
@@ -41,6 +43,10 @@ module Gitlab
project.issues.new
end
+ def user_finder
+ @issue_finder ||= Gitlab::PhabricatorImport::UserFinder.new(project, task.phids)
+ end
+
def find_issue_by_phabricator_id(phabricator_id)
object_map.get_gitlab_model(phabricator_id)
end
diff --git a/lib/gitlab/phabricator_import/representation/task.rb b/lib/gitlab/phabricator_import/representation/task.rb
index 6aedc71b626..ba93fb37a8e 100644
--- a/lib/gitlab/phabricator_import/representation/task.rb
+++ b/lib/gitlab/phabricator_import/representation/task.rb
@@ -11,6 +11,18 @@ module Gitlab
json['phid']
end
+ def author_phid
+ json['fields']['authorPHID']
+ end
+
+ def owner_phid
+ json['fields']['ownerPHID']
+ end
+
+ def phids
+ @phids ||= [author_phid, owner_phid]
+ end
+
def issue_attributes
@issue_attributes ||= {
title: issue_title,
diff --git a/lib/gitlab/phabricator_import/representation/user.rb b/lib/gitlab/phabricator_import/representation/user.rb
new file mode 100644
index 00000000000..7fd7cecc6ae
--- /dev/null
+++ b/lib/gitlab/phabricator_import/representation/user.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module PhabricatorImport
+ module Representation
+ class User
+ def initialize(json)
+ @json = json
+ end
+
+ def phabricator_id
+ json['phid']
+ end
+
+ def username
+ json['fields']['username']
+ end
+
+ private
+
+ attr_reader :json
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/phabricator_import/user_finder.rb b/lib/gitlab/phabricator_import/user_finder.rb
new file mode 100644
index 00000000000..4b50431e0e0
--- /dev/null
+++ b/lib/gitlab/phabricator_import/user_finder.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module PhabricatorImport
+ class UserFinder
+ def initialize(project, phids)
+ @project, @phids = project, phids
+ @loaded_phids = Set.new
+ end
+
+ def find(phid)
+ found_user = object_map.get_gitlab_model(phid) do
+ find_user_for_phid(phid)
+ end
+
+ loaded_phids << phid
+
+ found_user
+ end
+
+ private
+
+ attr_reader :project, :phids, :loaded_phids
+
+ def object_map
+ @object_map ||= Gitlab::PhabricatorImport::Cache::Map.new(project)
+ end
+
+ def find_user_for_phid(phid)
+ phabricator_user = phabricator_users.find { |u| u.phabricator_id == phid }
+ return unless phabricator_user
+
+ project.authorized_users.find_by_username(phabricator_user.username)
+ end
+
+ def phabricator_users
+ @user_responses ||= client.users(users_to_request).flat_map(&:users)
+ end
+
+ def users_to_request
+ phids - loaded_phids.to_a
+ end
+
+ def client
+ @client ||=
+ Gitlab::PhabricatorImport::Conduit::User
+ .new(phabricator_url: project.import_data.data['phabricator_url'],
+ api_token: project.import_data.credentials[:api_token])
+ end
+ end
+ end
+end
diff --git a/spec/fixtures/phabricator_responses/user.search.json b/spec/fixtures/phabricator_responses/user.search.json
new file mode 100644
index 00000000000..f3ec653a23e
--- /dev/null
+++ b/spec/fixtures/phabricator_responses/user.search.json
@@ -0,0 +1,62 @@
+{
+ "result": {
+ "data": [
+ {
+ "id": 1,
+ "type": "USER",
+ "phid": "PHID-USER-hohoho",
+ "fields": {
+ "username": "jane",
+ "realName": "Jane Doe",
+ "roles": [
+ "admin",
+ "verified",
+ "approved",
+ "activated"
+ ],
+ "dateCreated": 1405970599,
+ "dateModified": 1406705963,
+ "policy": {
+ "view": "public",
+ "edit": "no-one"
+ }
+ },
+ "attachments": {}
+ },
+ {
+ "id": 2,
+ "type": "USER",
+ "phid": "PHID-USER-hihihi",
+ "fields": {
+ "username": "john",
+ "realName": "John Doe",
+ "roles": [
+ "admin",
+ "verified",
+ "approved",
+ "activated"
+ ],
+ "dateCreated": 1403609184,
+ "dateModified": 1559138722,
+ "policy": {
+ "view": "public",
+ "edit": "no-one"
+ }
+ },
+ "attachments": {}
+ }
+ ],
+ "maps": {},
+ "query": {
+ "queryKey": null
+ },
+ "cursor": {
+ "limit": "100",
+ "after": null,
+ "before": null,
+ "order": null
+ }
+ },
+ "error_code": null,
+ "error_info": null
+}
diff --git a/spec/lib/gitlab/phabricator_import/cache/map_spec.rb b/spec/lib/gitlab/phabricator_import/cache/map_spec.rb
index 52c7a02219f..b6629fad453 100644
--- a/spec/lib/gitlab/phabricator_import/cache/map_spec.rb
+++ b/spec/lib/gitlab/phabricator_import/cache/map_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
require 'spec_helper'
describe Gitlab::PhabricatorImport::Cache::Map, :clean_gitlab_redis_cache do
@@ -28,6 +30,21 @@ describe Gitlab::PhabricatorImport::Cache::Map, :clean_gitlab_redis_cache do
expect(ttl).to be > 10.seconds
end
+
+ it 'sets the object in redis once if a block was given and nothing was cached' do
+ issue = create(:issue, project: project)
+
+ expect(map.get_gitlab_model('does not exist') { issue }).to eq(issue)
+
+ expect { |b| map.get_gitlab_model('does not exist', &b) }
+ .not_to yield_control
+ end
+
+ it 'does not cache `nil` objects' do
+ expect(map).not_to receive(:set_gitlab_model)
+
+ map.get_gitlab_model('does not exist') { nil }
+ end
end
describe '#set_gitlab_model' do
diff --git a/spec/lib/gitlab/phabricator_import/conduit/user_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/user_spec.rb
new file mode 100644
index 00000000000..e88eec2c393
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/conduit/user_spec.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Conduit::User do
+ let(:user_client) do
+ described_class.new(phabricator_url: 'https://see-ya-later.phabricator', api_token: 'api-token')
+ end
+
+ describe '#users' do
+ let(:fake_client) { double('Phabricator client') }
+
+ before do
+ allow(user_client).to receive(:client).and_return(fake_client)
+ end
+
+ it 'calls the api with the correct params' do
+ expected_params = {
+ constraints: { phids: ['phid-1', 'phid-2'] }
+ }
+
+ expect(fake_client).to receive(:get).with('user.search',
+ params: expected_params)
+
+ user_client.users(['phid-1', 'phid-2'])
+ end
+
+ it 'returns an array of parsed responses' do
+ response = Gitlab::PhabricatorImport::Conduit::Response
+ .new(fixture_file('phabricator_responses/user.search.json'))
+
+ allow(fake_client).to receive(:get).and_return(response)
+
+ expect(user_client.users(%w[some phids])).to match_array([an_instance_of(Gitlab::PhabricatorImport::Conduit::UsersResponse)])
+ end
+
+ it 'performs multiple requests if more phids than the maximum page size are passed' do
+ stub_const('Gitlab::PhabricatorImport::Conduit::User::MAX_PAGE_SIZE', 1)
+ first_params = { constraints: { phids: ['phid-1'] } }
+ second_params = { constraints: { phids: ['phid-2'] } }
+
+ expect(fake_client).to receive(:get).with('user.search',
+ params: first_params).once
+ expect(fake_client).to receive(:get).with('user.search',
+ params: second_params).once
+
+ user_client.users(['phid-1', 'phid-2'])
+ end
+ end
+end
diff --git a/spec/lib/gitlab/phabricator_import/conduit/users_response_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/users_response_spec.rb
new file mode 100644
index 00000000000..00778ad90fd
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/conduit/users_response_spec.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Conduit::UsersResponse do
+ let(:conduit_response) do
+ Gitlab::PhabricatorImport::Conduit::Response
+ .new(JSON.parse(fixture_file('phabricator_responses/user.search.json')))
+ end
+
+ subject(:response) { described_class.new(conduit_response) }
+
+ describe '#users' do
+ it 'builds the correct users representation' do
+ tasks = response.users
+
+ usernames = tasks.map(&:username)
+
+ expect(usernames).to contain_exactly('jane', 'john')
+ end
+ end
+end
diff --git a/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb b/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb
index 2412cf76f79..667321409da 100644
--- a/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb
+++ b/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
describe Gitlab::PhabricatorImport::Issues::Importer do
- set(:project) { create(:project) }
+ let(:project) { create(:project) }
let(:response) do
Gitlab::PhabricatorImport::Conduit::TasksResponse.new(
@@ -15,7 +15,6 @@ describe Gitlab::PhabricatorImport::Issues::Importer do
before do
client = instance_double(Gitlab::PhabricatorImport::Conduit::Maniphest)
-
allow(client).to receive(:tasks).and_return(response)
allow(importer).to receive(:client).and_return(client)
end
@@ -34,20 +33,29 @@ describe Gitlab::PhabricatorImport::Issues::Importer do
importer.execute
end
- it 'schedules the next batch if there is one' do
- expect(Gitlab::PhabricatorImport::ImportTasksWorker)
- .to receive(:schedule).with(project.id, response.pagination.next_page)
+ context 'stubbed task import' do
+ before do
+ # Stub out the actual importing so we don't perform aditional requests
+ expect_next_instance_of(Gitlab::PhabricatorImport::Issues::TaskImporter) do |task_importer|
+ allow(task_importer).to receive(:execute)
+ end.at_least(1)
+ end
- importer.execute
- end
+ it 'schedules the next batch if there is one' do
+ expect(Gitlab::PhabricatorImport::ImportTasksWorker)
+ .to receive(:schedule).with(project.id, response.pagination.next_page)
- it 'does not reschedule when there is no next page' do
- allow(response.pagination).to receive(:has_next_page?).and_return(false)
+ importer.execute
+ end
- expect(Gitlab::PhabricatorImport::ImportTasksWorker)
- .not_to receive(:schedule)
+ it 'does not reschedule when there is no next page' do
+ allow(response.pagination).to receive(:has_next_page?).and_return(false)
- importer.execute
+ expect(Gitlab::PhabricatorImport::ImportTasksWorker)
+ .not_to receive(:schedule)
+
+ importer.execute
+ end
end
end
end
diff --git a/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb b/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb
index 1625604e754..06ed264e781 100644
--- a/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb
+++ b/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb
@@ -12,6 +12,8 @@ describe Gitlab::PhabricatorImport::Issues::TaskImporter do
'description' => {
'raw' => '# This is markdown\n it can contain more text.'
},
+ 'authorPHID' => 'PHID-USER-456',
+ 'ownerPHID' => 'PHID-USER-123',
'dateCreated' => '1518688921',
'dateClosed' => '1518789995'
}
@@ -19,9 +21,18 @@ describe Gitlab::PhabricatorImport::Issues::TaskImporter do
)
end
+ subject(:importer) { described_class.new(project, task) }
+
describe '#execute' do
+ let(:fake_user_finder) { instance_double(Gitlab::PhabricatorImport::UserFinder) }
+
+ before do
+ allow(fake_user_finder).to receive(:find)
+ allow(importer).to receive(:user_finder).and_return(fake_user_finder)
+ end
+
it 'creates the issue with the expected attributes' do
- issue = described_class.new(project, task).execute
+ issue = importer.execute
expect(issue.project).to eq(project)
expect(issue).to be_persisted
@@ -34,21 +45,38 @@ describe Gitlab::PhabricatorImport::Issues::TaskImporter do
end
it 'does not recreate the issue when called multiple times' do
- expect { described_class.new(project, task).execute }
+ expect { importer.execute }
.to change { project.issues.reload.size }.from(0).to(1)
- expect { described_class.new(project, task).execute }
+ expect { importer.execute }
.not_to change { project.issues.reload.size }
end
it 'does not trigger a save when the object did not change' do
existing_issue = create(:issue,
task.issue_attributes.merge(author: User.ghost))
- importer = described_class.new(project, task)
allow(importer).to receive(:issue).and_return(existing_issue)
expect(existing_issue).not_to receive(:save!)
importer.execute
end
+
+ it 'links the author if the author can be found' do
+ author = create(:user)
+ expect(fake_user_finder).to receive(:find).with('PHID-USER-456').and_return(author)
+
+ issue = importer.execute
+
+ expect(issue.author).to eq(author)
+ end
+
+ it 'links an assignee if the user can be found' do
+ assignee = create(:user)
+ expect(fake_user_finder).to receive(:find).with('PHID-USER-123').and_return(assignee)
+
+ issue = importer.execute
+
+ expect(issue.assignees).to include(assignee)
+ end
end
end
diff --git a/spec/lib/gitlab/phabricator_import/representation/task_spec.rb b/spec/lib/gitlab/phabricator_import/representation/task_spec.rb
index dfbd8c546eb..5603a6961d6 100644
--- a/spec/lib/gitlab/phabricator_import/representation/task_spec.rb
+++ b/spec/lib/gitlab/phabricator_import/representation/task_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
require 'spec_helper'
describe Gitlab::PhabricatorImport::Representation::Task do
@@ -7,6 +9,8 @@ describe Gitlab::PhabricatorImport::Representation::Task do
'phid' => 'the-phid',
'fields' => {
'name' => 'Title'.ljust(257, '.'), # A string padded to 257 chars
+ 'authorPHID' => 'a phid',
+ 'ownerPHID' => 'another user phid',
'description' => {
'raw' => '# This is markdown\n it can contain more text.'
},
@@ -30,4 +34,16 @@ describe Gitlab::PhabricatorImport::Representation::Task do
expect(task.issue_attributes).to eq(expected_attributes)
end
end
+
+ describe '#author_phid' do
+ it 'returns the correct field' do
+ expect(task.author_phid).to eq('a phid')
+ end
+ end
+
+ describe '#owner_phid' do
+ it 'returns the correct field' do
+ expect(task.owner_phid).to eq('another user phid')
+ end
+ end
end
diff --git a/spec/lib/gitlab/phabricator_import/representation/user_spec.rb b/spec/lib/gitlab/phabricator_import/representation/user_spec.rb
new file mode 100644
index 00000000000..f52467a0cf1
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/representation/user_spec.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Representation::User do
+ subject(:user) do
+ described_class.new(
+ {
+ 'phid' => 'the-phid',
+ 'fields' => {
+ 'username' => 'the-username'
+ }
+ }
+ )
+ end
+
+ describe '#phabricator_id' do
+ it 'returns the phabricator id' do
+ expect(user.phabricator_id).to eq('the-phid')
+ end
+ end
+
+ describe '#username' do
+ it 'returns the username' do
+ expect(user.username).to eq('the-username')
+ end
+ end
+end
diff --git a/spec/lib/gitlab/phabricator_import/user_finder_spec.rb b/spec/lib/gitlab/phabricator_import/user_finder_spec.rb
new file mode 100644
index 00000000000..096321cda5f
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/user_finder_spec.rb
@@ -0,0 +1,89 @@
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::UserFinder, :clean_gitlab_redis_cache do
+ let(:project) { create(:project, namespace: create(:group)) }
+ subject(:finder) { described_class.new(project, ['first-phid', 'second-phid']) }
+
+ before do
+ project.namespace.add_developer(existing_user)
+ end
+
+ describe '#find' do
+ let!(:existing_user) { create(:user, username: 'existing-user') }
+ let(:cache) { Gitlab::PhabricatorImport::Cache::Map.new(project) }
+
+ before do
+ allow(finder).to receive(:object_map).and_return(cache)
+ end
+
+ context 'for a cached phid' do
+ before do
+ cache.set_gitlab_model(existing_user, 'first-phid')
+ end
+
+ it 'returns the existing user' do
+ expect(finder.find('first-phid')).to eq(existing_user)
+ end
+
+ it 'does not perform a find using the API' do
+ expect(finder).not_to receive(:find_user_for_phid)
+
+ finder.find('first-phid')
+ end
+
+ it 'excludes the phid from the request if one needs to be made' do
+ client = instance_double(Gitlab::PhabricatorImport::Conduit::User)
+ allow(finder).to receive(:client).and_return(client)
+
+ expect(client).to receive(:users).with(['second-phid']).and_return([])
+
+ finder.find('first-phid')
+ finder.find('second-phid')
+ end
+ end
+
+ context 'when the phid is not cached' do
+ let(:response) do
+ [
+ instance_double(
+ Gitlab::PhabricatorImport::Conduit::UsersResponse,
+ users: [instance_double(Gitlab::PhabricatorImport::Representation::User, phabricator_id: 'second-phid', username: 'existing-user')]
+ ),
+ instance_double(
+ Gitlab::PhabricatorImport::Conduit::UsersResponse,
+ users: [instance_double(Gitlab::PhabricatorImport::Representation::User, phabricator_id: 'first-phid', username: 'other-user')]
+ )
+ ]
+ end
+ let(:client) do
+ client = instance_double(Gitlab::PhabricatorImport::Conduit::User)
+ allow(client).to receive(:users).and_return(response)
+
+ client
+ end
+
+ before do
+ allow(finder).to receive(:client).and_return(client)
+ end
+
+ it 'loads the users from the API once' do
+ expect(client).to receive(:users).and_return(response).once
+
+ expect(finder.find('second-phid')).to eq(existing_user)
+ expect(finder.find('first-phid')).to be_nil
+ end
+
+ it 'adds found users to the cache' do
+ expect { finder.find('second-phid') }
+ .to change { cache.get_gitlab_model('second-phid') }
+ .from(nil).to(existing_user)
+ end
+
+ it 'only returns users that are members of the project' do
+ create(:user, username: 'other-user')
+
+ expect(finder.find('first-phid')).to eq(nil)
+ end
+ end
+ end
+end