Merge branch 'georgekoltsov/github-importer-pagination' into 'master'

Add GitHub Importer pagination

See merge request gitlab-org/gitlab!48983

app/controllers/import/github_controller.rb

@@ -17,6 +17,8 @@ class Import::GithubController < Import::BaseController
   rescue_from Octokit::TooManyRequests, with: :provider_rate_limit
   rescue_from Gitlab::GithubImport::RateLimitError, with: :rate_limit_threshold_exceeded
 
+  PAGE_LENGTH = 25
+
   def new
     if !ci_cd_only? && github_import_configured? && logged_in_with_provider?
       go_to_provider_for_permissions
@@ -115,19 +117,16 @@ class Import::GithubController < Import::BaseController
 
   def client_repos
     @client_repos ||= if Feature.enabled?(:remove_legacy_github_client)
-                        concatenated_repos
+                        if sanitized_filter_param
+                          client.search_repos_by_name(sanitized_filter_param, pagination_options)[:items]
+                        else
+                          client.octokit.repos(nil, pagination_options)
+                        end
                       else
                         filtered(client.repos)
                       end
   end
 
-  def concatenated_repos
-    return [] unless client.respond_to?(:each_page)
-    return client.each_page(:repos).flat_map(&:objects) unless sanitized_filter_param
-
-    client.search_repos_by_name(sanitized_filter_param).flat_map(&:objects).flat_map(&:items)
-  end
-
   def sanitized_filter_param
     super
 
@@ -257,6 +256,13 @@ class Import::GithubController < Import::BaseController
   def rate_limit_threshold_exceeded
     head :too_many_requests
   end
+
+  def pagination_options
+    {
+      page: [1, params[:page].to_i].max,
+      per_page: PAGE_LENGTH
+    }
+  end
 end
 
 Import::GithubController.prepend_if_ee('EE::Import::GithubController')

app/views/import/github/status.html.haml

@@ -7,4 +7,6 @@
     = sprite_icon('github', css_class: 'gl-mr-2')
   = _('Import repositories from GitHub')
 
-= render 'import/githubish_status', provider: 'github'
+- paginatable = Feature.enabled?(:remove_legacy_github_client)
+
+= render 'import/githubish_status', provider: 'github', paginatable: paginatable

changelogs/unreleased/georgekoltsov-github-importer-pagination.yml

+---
+title: Add GitHub Importer pagination
+merge_request: 48983
+author:
+type: changed

lib/gitlab/github_import/client.rb

@@ -163,8 +163,8 @@ module Gitlab
         end
       end
 
-      def search_repos_by_name(name)
-        each_page(:search_repositories, search_query(str: name, type: :name))
+      def search_repos_by_name(name, options = {})
+        octokit.search_repositories(search_query(str: name, type: :name), options)
       end
 
       def search_query(str:, type:, include_collaborations: true, include_orgs: true)

spec/controllers/import/github_controller_spec.rb

@@ -123,26 +123,33 @@ RSpec.describe Import::GithubController do
       end
 
       it 'fetches repos using latest github client' do
-        expect_next_instance_of(Gitlab::GithubImport::Client) do |client|
-          expect(client).to receive(:each_page).with(:repos).and_return([].to_enum)
+        expect_next_instance_of(Octokit::Client) do |client|
+          expect(client).to receive(:repos).and_return([].to_enum)
         end
 
         get :status
       end
 
-      it 'concatenates list of repos from multiple pages' do
-        repo_1 = OpenStruct.new(login: 'emacs', full_name: 'asd/emacs', name: 'emacs', owner: { login: 'owner' })
-        repo_2 = OpenStruct.new(login: 'vim', full_name: 'asd/vim', name: 'vim', owner: { login: 'owner' })
-        repos = [OpenStruct.new(objects: [repo_1]), OpenStruct.new(objects: [repo_2])].to_enum
+      context 'pagination' do
+        context 'when no page is specified' do
+          it 'requests first page' do
+            expect_next_instance_of(Octokit::Client) do |client|
+              expect(client).to receive(:repos).with(nil, { page: 1, per_page: 25 }).and_return([].to_enum)
+            end
 
-        allow(stub_client).to receive(:each_page).and_return(repos)
+            get :status
+          end
+        end
 
-        get :status, format: :json
+        context 'when page is specified' do
+          it 'requests repos with specified page' do
+            expect_next_instance_of(Octokit::Client) do |client|
+              expect(client).to receive(:repos).with(nil, { page: 2, per_page: 25 }).and_return([].to_enum)
+            end
 
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response.dig('provider_repos').count).to eq(2)
-        expect(json_response.dig('provider_repos', 0, 'id')).to eq(repo_1.id)
-        expect(json_response.dig('provider_repos', 1, 'id')).to eq(repo_2.id)
+            get :status, params: { page: 2 }
+          end
+        end
       end
 
       context 'when filtering' do
@@ -150,6 +157,7 @@ RSpec.describe Import::GithubController do
         let(:user_login) { 'user' }
         let(:collaborations_subquery) { 'repo:repo1 repo:repo2' }
         let(:organizations_subquery) { 'org:org1 org:org2' }
+        let(:search_query) { "test in:name is:public,private user:#{user_login} #{collaborations_subquery} #{organizations_subquery}" }
 
         before do
           allow_next_instance_of(Octokit::Client) do |client|
@@ -158,20 +166,56 @@ RSpec.describe Import::GithubController do
         end
 
         it 'makes request to github search api' do
-          expected_query = "test in:name is:public,private user:#{user_login} #{collaborations_subquery} #{organizations_subquery}"
+          expect_next_instance_of(Octokit::Client) do |client|
+            expect(client).to receive(:user).and_return(double(login: user_login))
+            expect(client).to receive(:search_repositories).with(search_query, { page: 1, per_page: 25 }).and_return({ items: [].to_enum })
+          end
 
           expect_next_instance_of(Gitlab::GithubImport::Client) do |client|
             expect(client).to receive(:collaborations_subquery).and_return(collaborations_subquery)
             expect(client).to receive(:organizations_subquery).and_return(organizations_subquery)
-            expect(client).to receive(:each_page).with(:search_repositories, expected_query).and_return([].to_enum)
           end
 
           get :status, params: { filter: filter }, format: :json
         end
 
+        context 'pagination' do
+          context 'when no page is specified' do
+            it 'requests first page' do
+              expect_next_instance_of(Octokit::Client) do |client|
+                expect(client).to receive(:user).and_return(double(login: user_login))
+                expect(client).to receive(:search_repositories).with(search_query, { page: 1, per_page: 25 }).and_return({ items: [].to_enum })
+              end
+
+              expect_next_instance_of(Gitlab::GithubImport::Client) do |client|
+                expect(client).to receive(:collaborations_subquery).and_return(collaborations_subquery)
+                expect(client).to receive(:organizations_subquery).and_return(organizations_subquery)
+              end
+
+              get :status, params: { filter: filter }, format: :json
+            end
+          end
+
+          context 'when page is specified' do
+            it 'requests repos with specified page' do
+              expect_next_instance_of(Octokit::Client) do |client|
+                expect(client).to receive(:user).and_return(double(login: user_login))
+                expect(client).to receive(:search_repositories).with(search_query, { page: 2, per_page: 25 }).and_return({ items: [].to_enum })
+              end
+
+              expect_next_instance_of(Gitlab::GithubImport::Client) do |client|
+                expect(client).to receive(:collaborations_subquery).and_return(collaborations_subquery)
+                expect(client).to receive(:organizations_subquery).and_return(organizations_subquery)
+              end
+
+              get :status, params: { filter: filter, page: 2 }, format: :json
+            end
+          end
+        end
+
         context 'when user input contains colons and spaces' do
           before do
-            stub_client(search_repos_by_name: [])
+            allow(controller).to receive(:client_repos).and_return([])
           end
 
           it 'sanitizes user input' do

spec/lib/gitlab/github_import/client_spec.rb

@@ -500,7 +500,7 @@ RSpec.describe Gitlab::GithubImport::Client do
       it 'searches for repositories based on name' do
         expected_search_query = 'test in:name is:public,private user:user repo:repo1 repo:repo2 org:org1 org:org2'
 
-        expect(client).to receive(:each_page).with(:search_repositories, expected_search_query)
+        expect(client.octokit).to receive(:search_repositories).with(expected_search_query, {})
 
         client.search_repos_by_name('test')
       end