summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2018-11-05 15:37:21 -0800
committerStan Hu <stanhu@gmail.com>2018-11-07 11:18:05 -0800
commiteb67fe85b50c75f4547df0d8f7681ac8da6d7ebe (patch)
treee5974761173345882632ab5fcc26e3c66da2bf33
parent4068d46078faaa97acbfbe33cc7663db6d1c831a (diff)
downloadgitlab-ce-sh-paginate-bitbucket-server-imports.tar.gz
Paginate Bitbucket Server importer projectssh-paginate-bitbucket-server-imports
To prevent delays in loading the page and reduce memory usage, limit the number of projects shown at 25 per page. Part of https://gitlab.com/gitlab-org/gitlab-ce/issues/50021
-rw-r--r--app/controllers/import/bitbucket_server_controller.rb26
-rw-r--r--app/views/import/bitbucket_server/status.html.haml2
-rw-r--r--changelogs/unreleased/sh-paginate-bitbucket-server-imports.yml5
-rw-r--r--lib/bitbucket_server/client.rb8
-rw-r--r--lib/bitbucket_server/collection.rb24
-rw-r--r--lib/bitbucket_server/paginator.rb27
-rw-r--r--spec/controllers/import/bitbucket_server_controller_spec.rb11
-rw-r--r--spec/lib/bitbucket_server/client_spec.rb12
-rw-r--r--spec/lib/bitbucket_server/collection_spec.rb29
-rw-r--r--spec/lib/bitbucket_server/paginator_spec.rb10
10 files changed, 134 insertions, 20 deletions
diff --git a/app/controllers/import/bitbucket_server_controller.rb b/app/controllers/import/bitbucket_server_controller.rb
index fdd1078cdf7..6b7cc2d8ad7 100644
--- a/app/controllers/import/bitbucket_server_controller.rb
+++ b/app/controllers/import/bitbucket_server_controller.rb
@@ -1,6 +1,8 @@
# frozen_string_literal: true
class Import::BitbucketServerController < Import::BaseController
+ include PaginationHelper
+
before_action :verify_bitbucket_server_import_enabled
before_action :bitbucket_auth, except: [:new, :configure]
before_action :validate_import_params, only: [:create]
@@ -54,14 +56,14 @@ class Import::BitbucketServerController < Import::BaseController
# rubocop: disable CodeReuse/ActiveRecord
def status
- repos = bitbucket_client.repos
-
- @repos, @incompatible_repos = repos.partition { |repo| repo.valid? }
+ @collection = bitbucket_client.repos(page_offset: page_offset, limit: limit_per_page)
+ @repos, @incompatible_repos = @collection.partition { |repo| repo.valid? }
- @already_added_projects = find_already_added_projects('bitbucket_server')
+ # Use the import URL to filter beyond what BaseService#find_already_added_projects
+ @already_added_projects = filter_added_projects('bitbucket_server', @repos.map(&:browse_url))
already_added_projects_names = @already_added_projects.pluck(:import_source)
- @repos.to_a.reject! { |repo| already_added_projects_names.include?(repo.browse_url) }
+ @repos.reject! { |repo| already_added_projects_names.include?(repo.browse_url) }
rescue BitbucketServer::Connection::ConnectionError, BitbucketServer::Client::ServerError => e
flash[:alert] = "Unable to connect to server: #{e}"
clear_session_data
@@ -75,6 +77,12 @@ class Import::BitbucketServerController < Import::BaseController
private
+ # rubocop: disable CodeReuse/ActiveRecord
+ def filter_added_projects(import_type, import_sources)
+ current_user.created_projects.where(import_type: import_type, import_source: import_sources).includes(:import_state)
+ end
+ # rubocop: enable CodeReuse/ActiveRecord
+
def bitbucket_client
@bitbucket_client ||= BitbucketServer::Client.new(credentials)
end
@@ -130,4 +138,12 @@ class Import::BitbucketServerController < Import::BaseController
password: session[personal_access_token_key]
}
end
+
+ def page_offset
+ [0, params[:page].to_i].max
+ end
+
+ def limit_per_page
+ BitbucketServer::Paginator::PAGE_LENGTH
+ end
end
diff --git a/app/views/import/bitbucket_server/status.html.haml b/app/views/import/bitbucket_server/status.html.haml
index ae09e0dfa18..56d4f2ba881 100644
--- a/app/views/import/bitbucket_server/status.html.haml
+++ b/app/views/import/bitbucket_server/status.html.haml
@@ -84,4 +84,6 @@
= link_to 'import flow', status_import_bitbucket_server_path
again.
+= paginate_without_count(@collection)
+
.js-importer-status{ data: { jobs_import_path: "#{jobs_import_bitbucket_server_path}", import_path: "#{import_bitbucket_server_path}" } }
diff --git a/changelogs/unreleased/sh-paginate-bitbucket-server-imports.yml b/changelogs/unreleased/sh-paginate-bitbucket-server-imports.yml
new file mode 100644
index 00000000000..b5743e71cf9
--- /dev/null
+++ b/changelogs/unreleased/sh-paginate-bitbucket-server-imports.yml
@@ -0,0 +1,5 @@
+---
+title: Paginate Bitbucket Server importer projects
+merge_request: 22825
+author:
+type: changed
diff --git a/lib/bitbucket_server/client.rb b/lib/bitbucket_server/client.rb
index 15e59f93141..83e8808db07 100644
--- a/lib/bitbucket_server/client.rb
+++ b/lib/bitbucket_server/client.rb
@@ -35,9 +35,9 @@ module BitbucketServer
BitbucketServer::Representation::Repo.new(parsed_response)
end
- def repos
+ def repos(page_offset: 0, limit: nil)
path = "/repos"
- get_collection(path, :repo)
+ get_collection(path, :repo, page_offset: page_offset, limit: limit)
end
def create_branch(project_key, repo, branch_name, sha)
@@ -61,8 +61,8 @@ module BitbucketServer
private
- def get_collection(path, type)
- paginator = BitbucketServer::Paginator.new(connection, Addressable::URI.escape(path), type)
+ def get_collection(path, type, page_offset: 0, limit: nil)
+ paginator = BitbucketServer::Paginator.new(connection, Addressable::URI.escape(path), type, page_offset: page_offset, limit: limit)
BitbucketServer::Collection.new(paginator)
rescue *SERVER_ERRORS => e
raise ServerError, e
diff --git a/lib/bitbucket_server/collection.rb b/lib/bitbucket_server/collection.rb
index b50c5dde352..7e4b2277bbe 100644
--- a/lib/bitbucket_server/collection.rb
+++ b/lib/bitbucket_server/collection.rb
@@ -2,7 +2,13 @@
module BitbucketServer
class Collection < Enumerator
+ attr_reader :paginator
+
+ delegate :page_offset, :has_next_page?, to: :paginator
+
def initialize(paginator)
+ @paginator = paginator
+
super() do |yielder|
loop do
paginator.items.each { |item| yielder << item }
@@ -12,6 +18,24 @@ module BitbucketServer
lazy
end
+ def current_page
+ return 1 if page_offset <= 1
+
+ [1, page_offset].max
+ end
+
+ def prev_page
+ return nil unless current_page > 1
+
+ current_page - 1
+ end
+
+ def next_page
+ return nil unless has_next_page?
+
+ current_page + 1
+ end
+
def method_missing(method, *args)
return super unless self.respond_to?(method)
diff --git a/lib/bitbucket_server/paginator.rb b/lib/bitbucket_server/paginator.rb
index c351fb2f11f..aa5f84f44b3 100644
--- a/lib/bitbucket_server/paginator.rb
+++ b/lib/bitbucket_server/paginator.rb
@@ -4,34 +4,49 @@ module BitbucketServer
class Paginator
PAGE_LENGTH = 25
- def initialize(connection, url, type)
+ attr_reader :page_offset
+
+ def initialize(connection, url, type, page_offset: 0, limit: nil)
@connection = connection
@type = type
@url = url
@page = nil
+ @page_offset = page_offset
+ @limit = limit || PAGE_LENGTH
+ @total = 0
end
def items
raise StopIteration unless has_next_page?
+ raise StopIteration if over_limit?
@page = fetch_next_page
+ @total += @page.items.count
@page.items
end
+ def has_next_page?
+ page.nil? || page.next?
+ end
+
private
- attr_reader :connection, :page, :url, :type
+ attr_reader :connection, :page, :url, :type, :limit
- def has_next_page?
- page.nil? || page.next?
+ def over_limit?
+ @limit.positive? && @total >= @limit
end
def next_offset
- page.nil? ? 0 : page.next
+ page.nil? ? starting_offset : page.next
+ end
+
+ def starting_offset
+ [0, page_offset - 1].max * limit
end
def fetch_next_page
- parsed_response = connection.get(@url, start: next_offset, limit: PAGE_LENGTH)
+ parsed_response = connection.get(@url, start: next_offset, limit: @limit)
Page.new(parsed_response, type)
end
end
diff --git a/spec/controllers/import/bitbucket_server_controller_spec.rb b/spec/controllers/import/bitbucket_server_controller_spec.rb
index 5024ef71771..77060fdc3be 100644
--- a/spec/controllers/import/bitbucket_server_controller_spec.rb
+++ b/spec/controllers/import/bitbucket_server_controller_spec.rb
@@ -121,12 +121,19 @@ describe Import::BitbucketServerController do
@repo = double(slug: 'vim', project_key: 'asd', full_name: 'asd/vim', "valid?" => true, project_name: 'asd', browse_url: 'http://test', name: 'vim')
@invalid_repo = double(slug: 'invalid', project_key: 'foobar', full_name: 'asd/foobar', "valid?" => false, browse_url: 'http://bad-repo')
+ @created_repo = double(slug: 'created', project_key: 'existing', full_name: 'group/created', "valid?" => true, browse_url: 'http://existing')
assign_session_tokens
end
it 'assigns repository categories' do
- created_project = create(:project, import_type: 'bitbucket_server', creator_id: user.id, import_source: 'foo/bar', import_status: 'finished')
- expect(client).to receive(:repos).and_return([@repo, @invalid_repo])
+ created_project = create(:project, import_type: 'bitbucket_server', creator_id: user.id, import_status: 'finished', import_source: @created_repo.browse_url)
+ repos = instance_double(BitbucketServer::Collection)
+
+ expect(repos).to receive(:partition).and_return([[@repo, @created_repo], [@invalid_repo]])
+ expect(repos).to receive(:current_page).and_return(1)
+ expect(repos).to receive(:next_page).and_return(2)
+ expect(repos).to receive(:prev_page).and_return(nil)
+ expect(client).to receive(:repos).and_return(repos)
get :status
diff --git a/spec/lib/bitbucket_server/client_spec.rb b/spec/lib/bitbucket_server/client_spec.rb
index f926ae963a4..5de0a9a65b5 100644
--- a/spec/lib/bitbucket_server/client_spec.rb
+++ b/spec/lib/bitbucket_server/client_spec.rb
@@ -13,7 +13,7 @@ describe BitbucketServer::Client do
let(:path) { "/projects/#{project}/repos/#{repo_slug}/pull-requests?state=ALL" }
it 'requests a collection' do
- expect(BitbucketServer::Paginator).to receive(:new).with(anything, path, :pull_request)
+ expect(BitbucketServer::Paginator).to receive(:new).with(anything, path, :pull_request, page_offset: 0, limit: nil)
subject.pull_requests(project, repo_slug)
end
@@ -29,7 +29,7 @@ describe BitbucketServer::Client do
let(:path) { "/projects/#{project}/repos/#{repo_slug}/pull-requests/1/activities" }
it 'requests a collection' do
- expect(BitbucketServer::Paginator).to receive(:new).with(anything, path, :activity)
+ expect(BitbucketServer::Paginator).to receive(:new).with(anything, path, :activity, page_offset: 0, limit: nil)
subject.activities(project, repo_slug, 1)
end
@@ -52,10 +52,16 @@ describe BitbucketServer::Client do
let(:path) { "/repos" }
it 'requests a collection' do
- expect(BitbucketServer::Paginator).to receive(:new).with(anything, path, :repo)
+ expect(BitbucketServer::Paginator).to receive(:new).with(anything, path, :repo, page_offset: 0, limit: nil)
subject.repos
end
+
+ it 'requests a collection with an offset and limit' do
+ expect(BitbucketServer::Paginator).to receive(:new).with(anything, path, :repo, page_offset: 10, limit: 25)
+
+ subject.repos(page_offset: 10, limit: 25)
+ end
end
describe '#create_branch' do
diff --git a/spec/lib/bitbucket_server/collection_spec.rb b/spec/lib/bitbucket_server/collection_spec.rb
new file mode 100644
index 00000000000..ddd02bac88a
--- /dev/null
+++ b/spec/lib/bitbucket_server/collection_spec.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe BitbucketServer::Collection do
+ let(:connection) { instance_double(BitbucketServer::Connection) }
+ let(:page) { 1 }
+ let(:paginator) { BitbucketServer::Paginator.new(connection, 'http://more-data', :pull_request, page_offset: page) }
+
+ subject { described_class.new(paginator) }
+
+ describe '#current_page' do
+ it 'returns 1' do
+ expect(subject.current_page).to eq(1)
+ end
+ end
+
+ describe '#prev_page' do
+ it 'returns nil' do
+ expect(subject.prev_page).to be_nil
+ end
+ end
+
+ describe '#next_page' do
+ it 'returns 2' do
+ expect(subject.next_page).to eq(2)
+ end
+ end
+end
diff --git a/spec/lib/bitbucket_server/paginator_spec.rb b/spec/lib/bitbucket_server/paginator_spec.rb
index 2de50eba3c4..d268d4f23cf 100644
--- a/spec/lib/bitbucket_server/paginator_spec.rb
+++ b/spec/lib/bitbucket_server/paginator_spec.rb
@@ -20,6 +20,16 @@ describe BitbucketServer::Paginator do
expect { paginator.items }.to raise_error(StopIteration)
end
+ it 'obeys limits' do
+ limited = described_class.new(connection, 'http://more-data', :pull_request, page_offset: 0, limit: 1)
+ allow(limited).to receive(:fetch_next_page).and_return(first_page)
+
+ expect(limited.has_next_page?).to be_truthy
+ expect(limited.items).to match(['item_1'])
+ expect(limited.has_next_page?).to be_truthy
+ expect { limited.items }.to raise_error(StopIteration)
+ end
+
it 'calls the connection with different offsets' do
expect(connection).to receive(:get).with('http://more-data', start: 0, limit: BitbucketServer::Paginator::PAGE_LENGTH).and_return(page_attrs)