Merge branch 'sh-paginate-bitbucket-server-imports' into 'master'

Paginate Bitbucket Server importer projects

See merge request gitlab-org/gitlab-ce!22825

app/controllers/import/bitbucket_server_controller.rb

@@ -54,14 +54,14 @@ class Import::BitbucketServerController < Import::BaseController
 
   # rubocop: disable CodeReuse/ActiveRecord
   def status
-    repos = bitbucket_client.repos
+    @collection = bitbucket_client.repos(page_offset: page_offset, limit: limit_per_page)
+    @repos, @incompatible_repos = @collection.partition { |repo| repo.valid? }
 
-    @repos, @incompatible_repos = repos.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 +75,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 +136,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

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}" } }

changelogs/unreleased/sh-paginate-bitbucket-server-imports.yml

+---
+title: Paginate Bitbucket Server importer projects
+merge_request: 22825
+author:
+type: changed

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

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)
 

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

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
 

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

spec/lib/bitbucket_server/collection_spec.rb

+# 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

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)