From ec9ade0aa7fdb86967a699b21cc7247944cedd5f Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Wed, 19 Jun 2024 21:42:16 +0700
Subject: [PATCH 01/15] Adjust Bitbucket server importer to be resumable

Changelog: performance
---
 .../wip/bitbucket_import_resumable_worker.yml |  9 ++++
 lib/bitbucket/client.rb                       | 41 +++++++++++++++-
 .../importers/pull_requests_importer.rb       | 45 +++++++++++++++--
 .../bitbucket_import/parallel_scheduling.rb   | 48 ++++++++++++++++++-
 lib/gitlab/import/page_keyset.rb              | 35 ++++++++++++++
 5 files changed, 171 insertions(+), 7 deletions(-)
 create mode 100644 config/feature_flags/wip/bitbucket_import_resumable_worker.yml
 create mode 100644 lib/gitlab/import/page_keyset.rb

diff --git a/config/feature_flags/wip/bitbucket_import_resumable_worker.yml b/config/feature_flags/wip/bitbucket_import_resumable_worker.yml
new file mode 100644
index 00000000000000..013f6044a9bfde
--- /dev/null
+++ b/config/feature_flags/wip/bitbucket_import_resumable_worker.yml
@@ -0,0 +1,9 @@
+---
+name: bitbucket_import_resumable_worker
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/466231
+introduced_by_url:
+rollout_issue_url:
+milestone: '17.2'
+group: group::import and integrate
+type: wip
+default_enabled: false
diff --git a/lib/bitbucket/client.rb b/lib/bitbucket/client.rb
index 1dd8ce590cf8a2..45b7ac9b05bff5 100644
--- a/lib/bitbucket/client.rb
+++ b/lib/bitbucket/client.rb
@@ -8,6 +8,37 @@ def initialize(options = {})
       @connection = Connection.new(options)
     end
 
+    # Fetches data from the Bitbucket API and yields a Page object for every page
+    # of data, without loading all of them into memory.
+    #
+    # method - The method name used for getting the data.
+    # args - Arguments to pass to the method.
+    def each_page(method, *args)
+      options =
+        if args.last.is_a?(Hash)
+          args.last
+        else
+          {}
+        end
+
+      loop do
+        parsed_response = public_send(method, *args) # rubocop: disable GitlabSecurity/PublicSend -- generic method for pagination
+        object = Page.new(parsed_response, options[:representation_type])
+
+        yield object
+
+        break unless object.next?
+
+        options[:next_url] = object.next
+
+        if args.last.is_a?(Hash)
+          args[-1] = options
+        else
+          args.push(options)
+        end
+      end
+    end
+
     def last_issue(repo)
       parsed_response = connection.get("/repositories/#{repo}/issues?pagelen=1&sort=-created_on&state=ALL")
       Bitbucket::Representation::Issue.new(parsed_response['values'].first)
@@ -23,9 +54,15 @@ def issue_comments(repo, issue_id)
       get_collection(path, :comment)
     end
 
-    def pull_requests(repo)
+    def pull_requests(repo, options = {})
       path = "/repositories/#{repo}/pullrequests?state=ALL&sort=created_on"
-      get_collection(path, :pull_request)
+
+      if options[:raw]
+        path = options[:next_url] if options[:next_url]
+        connection.get(path)
+      else
+        get_collection(path, :pull_request)
+      end
     end
 
     def pull_request_comments(repo, pull_request)
diff --git a/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb b/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
index 99bf434a396ac5..5adf6532ad29d2 100644
--- a/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
+++ b/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
@@ -7,6 +7,33 @@ class PullRequestsImporter
         include ParallelScheduling
 
         def execute
+          if Feature.enabled?(:bitbucket_import_resumable_worker, project)
+            resumable_execute
+          else
+            non_resumable_execute
+          end
+        end
+
+        private
+
+        def resumable_execute
+          log_info(import_stage: 'import_pull_requests', message: 'importing pull requests')
+
+          each_object_to_import do |object|
+            job_waiter.jobs_remaining = Gitlab::Cache::Import::Caching.increment(job_waiter_remaining_cache_key)
+
+            job_delay = calculate_job_delay(job_waiter.jobs_remaining)
+
+            sidekiq_worker_class.perform_in(job_delay, project.id, object.to_hash, job_waiter.key)
+          end
+
+          job_waiter
+        rescue StandardError => e
+          track_import_failure!(project, exception: e)
+          job_waiter
+        end
+
+        def non_resumable_execute
           log_info(import_stage: 'import_pull_requests', message: 'importing pull requests')
 
           pull_requests = client.pull_requests(project.import_source)
@@ -29,8 +56,6 @@ def execute
           job_waiter
         end
 
-        private
-
         def sidekiq_worker_class
           ImportPullRequestWorker
         end
@@ -39,8 +64,22 @@ def collection_method
           :pull_requests
         end
 
+        def collection_options
+          { raw: true }
+        end
+
+        def representation_type
+          :pull_request
+        end
+
         def id_for_already_enqueued_cache(object)
-          object.iid
+          if object.is_a?(Hash)
+            # used for `resumable_execute`
+            object[:iid]
+          else
+            # used for `non_resumable_execute`
+            object.iid
+          end
         end
 
         # To avoid overloading Gitaly, we use a smaller limit for pull requests than the one defined in the
diff --git a/lib/gitlab/bitbucket_import/parallel_scheduling.rb b/lib/gitlab/bitbucket_import/parallel_scheduling.rb
index 1cc5855be76a3f..c09a7de156b8c9 100644
--- a/lib/gitlab/bitbucket_import/parallel_scheduling.rb
+++ b/lib/gitlab/bitbucket_import/parallel_scheduling.rb
@@ -6,7 +6,8 @@ module ParallelScheduling
       include Loggable
       include ErrorTracking
 
-      attr_reader :project, :already_enqueued_cache_key, :job_waiter_cache_key
+      attr_reader :project, :already_enqueued_cache_key, :job_waiter_cache_key, :job_waiter_remaining_cache_key,
+        :page_keyset
 
       # The base cache key to use for tracking already enqueued objects.
       ALREADY_ENQUEUED_CACHE_KEY =
@@ -16,6 +17,10 @@ module ParallelScheduling
       JOB_WAITER_CACHE_KEY =
         'bitbucket-importer/job-waiter/%{project}/%{collection}'
 
+      # The base cache key to use for storing job waiter remaining jobs
+      JOB_WAITER_REMAINING_CACHE_KEY =
+        'bitbucket-importer/job-waiter-remaining/%{project}/%{collection}'
+
       # project - An instance of `Project`.
       def initialize(project)
         @project = project
@@ -24,10 +29,43 @@ def initialize(project)
           format(ALREADY_ENQUEUED_CACHE_KEY, project: project.id, collection: collection_method)
         @job_waiter_cache_key =
           format(JOB_WAITER_CACHE_KEY, project: project.id, collection: collection_method)
+        @job_waiter_remaining_cache_key = format(JOB_WAITER_REMAINING_CACHE_KEY, project: project.id,
+          collection: collection_method)
+        @page_keyset = Gitlab::Import::PageKeyset.new(project, collection_method, 'bitbucket-importer')
       end
 
       private
 
+      # The method that will be called for traversing through all the objects to
+      # import, yielding them to the supplied block.
+      def each_object_to_import
+        repo = project.import_source
+
+        options = collection_options.merge(representation_type: representation_type, next_url: page_keyset.current)
+
+        client.each_page(collection_method, repo, options) do |page|
+          page.items.each do |object|
+            object = object.to_hash
+
+            next if already_enqueued?(object)
+
+            yield object
+
+            # We mark the object as imported immediately so we don't end up
+            # scheduling it multiple times.
+            mark_as_enqueued(object)
+          end
+
+          page_keyset.set(page.next) if page.next?
+        end
+      end
+
+      # Any options to be passed to the method used for retrieving the data to
+      # import.
+      def collection_options
+        {}
+      end
+
       def client
         @client ||= Bitbucket::Client.new(project.import_data.credentials)
       end
@@ -51,12 +89,18 @@ def collection_method
         raise NotImplementedError
       end
 
+      # The name of the method to call to retrieve the representation object
+      def representation_type
+        raise NotImplementedError
+      end
+
       def job_waiter
         @job_waiter ||= begin
           key = Gitlab::Cache::Import::Caching.read(job_waiter_cache_key)
           key ||= Gitlab::Cache::Import::Caching.write(job_waiter_cache_key, JobWaiter.generate_key)
+          jobs_remaining = Gitlab::Cache::Import::Caching.read(job_waiter_remaining_cache_key).to_i || 0
 
-          JobWaiter.new(0, key)
+          JobWaiter.new(jobs_remaining, key)
         end
       end
 
diff --git a/lib/gitlab/import/page_keyset.rb b/lib/gitlab/import/page_keyset.rb
new file mode 100644
index 00000000000000..a6a149981c26fd
--- /dev/null
+++ b/lib/gitlab/import/page_keyset.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Import
+    # PageKeyset can be used to keep track of the last imported page of a
+    # collection, allowing workers to resume where they left off in the event of
+    # an error.
+    class PageKeyset
+      attr_reader :cache_key
+
+      # The base cache key to use for storing the last key.
+      CACHE_KEY = '%{import_type}/page-keyset/%{object}/%{collection}'
+
+      def initialize(object, collection, import_type)
+        @cache_key = format(CACHE_KEY, import_type: import_type, object: object.id, collection: collection)
+      end
+
+      # Sets the key to the given value.
+      #
+      # Returns true if the key was overwritten, false otherwise.
+      def set(key)
+        Gitlab::Cache::Import::Caching.write(cache_key, key)
+      end
+
+      # Returns the current value from the cache.
+      def current
+        Gitlab::Cache::Import::Caching.read(cache_key)
+      end
+
+      def expire!
+        Gitlab::Cache::Import::Caching.expire(cache_key, 0)
+      end
+    end
+  end
+end
-- 
GitLab


From ce700fec96eb1ff1d3ee4b5f3680ee413a5328ce Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Fri, 21 Jun 2024 19:53:38 +0700
Subject: [PATCH 02/15] move flag

---
 .../importers/pull_requests_importer.rb                |  5 ++++-
 lib/gitlab/bitbucket_import/project_creator.rb         | 10 +++++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb b/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
index 5adf6532ad29d2..12274ba6cded8a 100644
--- a/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
+++ b/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
@@ -7,7 +7,10 @@ class PullRequestsImporter
         include ParallelScheduling
 
         def execute
-          if Feature.enabled?(:bitbucket_import_resumable_worker, project)
+          bitbucket_import_resumable_worker =
+            project.import_data&.data&.dig('bitbucket_import_resumable_worker')
+
+          if bitbucket_import_resumable_worker
             resumable_execute
           else
             non_resumable_execute
diff --git a/lib/gitlab/bitbucket_import/project_creator.rb b/lib/gitlab/bitbucket_import/project_creator.rb
index 2a5c6951073468..8c7caa70bf06ff 100644
--- a/lib/gitlab/bitbucket_import/project_creator.rb
+++ b/lib/gitlab/bitbucket_import/project_creator.rb
@@ -14,6 +14,9 @@ def initialize(repo, name, namespace, current_user, credentials)
       end
 
       def execute
+        bitbucket_import_resumable_worker =
+          Feature.enabled?(:bitbucket_import_resumable_worker, current_user)
+
         ::Projects::CreateService.new(
           current_user,
           name: name,
@@ -25,7 +28,12 @@ def execute
           import_type: 'bitbucket',
           import_source: repo.full_name,
           import_url: clone_url,
-          import_data: { credentials: credentials },
+          import_data: {
+            credentials: credentials,
+            data: {
+              bitbucket_import_resumable_worker: bitbucket_import_resumable_worker
+            }
+          },
           skip_wiki: skip_wiki
         ).execute
       end
-- 
GitLab


From 052f8f8259c2dddb4e8255b4f4be0056f911ab6a Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Fri, 21 Jun 2024 20:22:28 +0700
Subject: [PATCH 03/15] Add issue url

---
 config/feature_flags/wip/bitbucket_import_resumable_worker.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/feature_flags/wip/bitbucket_import_resumable_worker.yml b/config/feature_flags/wip/bitbucket_import_resumable_worker.yml
index 013f6044a9bfde..ae411b7ae0b99d 100644
--- a/config/feature_flags/wip/bitbucket_import_resumable_worker.yml
+++ b/config/feature_flags/wip/bitbucket_import_resumable_worker.yml
@@ -1,7 +1,7 @@
 ---
 name: bitbucket_import_resumable_worker
 feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/466231
-introduced_by_url:
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156797
 rollout_issue_url:
 milestone: '17.2'
 group: group::import and integrate
-- 
GitLab


From 772a19b26f10e592babdcd7c1ad9bac7bef01902 Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Sun, 23 Jun 2024 20:47:34 +0700
Subject: [PATCH 04/15] Add spec pull request importer

---
 .../importers/pull_requests_importer.rb       |  2 -
 .../bitbucket_import/parallel_scheduling.rb   |  2 +
 .../importers/pull_requests_importer_spec.rb  | 68 +++++++++++++------
 3 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb b/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
index 12274ba6cded8a..1c959468b25ab3 100644
--- a/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
+++ b/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
@@ -23,8 +23,6 @@ def resumable_execute
           log_info(import_stage: 'import_pull_requests', message: 'importing pull requests')
 
           each_object_to_import do |object|
-            job_waiter.jobs_remaining = Gitlab::Cache::Import::Caching.increment(job_waiter_remaining_cache_key)
-
             job_delay = calculate_job_delay(job_waiter.jobs_remaining)
 
             sidekiq_worker_class.perform_in(job_delay, project.id, object.to_hash, job_waiter.key)
diff --git a/lib/gitlab/bitbucket_import/parallel_scheduling.rb b/lib/gitlab/bitbucket_import/parallel_scheduling.rb
index c09a7de156b8c9..98244cb2852b54 100644
--- a/lib/gitlab/bitbucket_import/parallel_scheduling.rb
+++ b/lib/gitlab/bitbucket_import/parallel_scheduling.rb
@@ -45,6 +45,8 @@ def each_object_to_import
 
         client.each_page(collection_method, repo, options) do |page|
           page.items.each do |object|
+            job_waiter.jobs_remaining = Gitlab::Cache::Import::Caching.increment(job_waiter_remaining_cache_key)
+
             object = object.to_hash
 
             next if already_enqueued?(object)
diff --git a/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb
index 32e3c3e3d7d1a3..92342eafef91bb 100644
--- a/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb
@@ -3,29 +3,20 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestsImporter, :clean_gitlab_redis_shared_state, feature_category: :importers do
-  let_it_be(:project) do
-    create(:project, :import_started,
-      import_data_attributes: {
-        data: { 'project_key' => 'key', 'repo_slug' => 'slug' },
-        credentials: { 'base_uri' => 'http://bitbucket.org/', 'user' => 'bitbucket', 'password' => 'password' }
-      }
-    )
-  end
-
   subject(:importer) { described_class.new(project) }
 
-  describe '#execute' do
-    before do
-      allow_next_instance_of(Bitbucket::Client) do |client|
-        allow(client).to receive(:pull_requests).and_return(
-          [
-            Bitbucket::Representation::PullRequest.new({ 'id' => 1, 'state' => 'OPENED' }),
-            Bitbucket::Representation::PullRequest.new({ 'id' => 2, 'state' => 'DECLINED' }),
-            Bitbucket::Representation::PullRequest.new({ 'id' => 3, 'state' => 'MERGED' })
-          ],
-          []
-        )
-      end
+  shared_examples 'import bitbucket PullRequestsImporter' do |bitbucket_import_resumable_worker|
+    let_it_be(:project) do
+      create(:project, :import_started,
+        import_data_attributes: {
+          data: {
+            'project_key' => 'key',
+            'repo_slug' => 'slug',
+            'bitbucket_import_resumable_worker' => bitbucket_import_resumable_worker
+          },
+          credentials: { 'base_uri' => 'http://bitbucket.org/', 'user' => 'bitbucket', 'password' => 'password' }
+        }
+      )
     end
 
     it 'imports each pull request in parallel' do
@@ -68,4 +59,39 @@
       end
     end
   end
+
+  describe '#resumable_execute' do
+    before do
+      allow_next_instance_of(Bitbucket::Client) do |client|
+        page = instance_double('Bitbucket::Page', attrs: [], items: [
+          Bitbucket::Representation::PullRequest.new({ 'id' => 1, 'state' => 'OPENED' }),
+          Bitbucket::Representation::PullRequest.new({ 'id' => 2, 'state' => 'DECLINED' }),
+          Bitbucket::Representation::PullRequest.new({ 'id' => 3, 'state' => 'MERGED' })
+        ])
+
+        allow(client).to receive(:each_page).and_yield(page)
+        allow(page).to receive(:next?).and_return(true)
+        allow(page).to receive(:next).and_return('https://example.com/next')
+      end
+    end
+
+    it_behaves_like 'import bitbucket PullRequestsImporter', true
+  end
+
+  describe '#non_resumable_execute' do
+    before do
+      allow_next_instance_of(Bitbucket::Client) do |client|
+        allow(client).to receive(:pull_requests).and_return(
+          [
+            Bitbucket::Representation::PullRequest.new({ 'id' => 1, 'state' => 'OPENED' }),
+            Bitbucket::Representation::PullRequest.new({ 'id' => 2, 'state' => 'DECLINED' }),
+            Bitbucket::Representation::PullRequest.new({ 'id' => 3, 'state' => 'MERGED' })
+          ],
+          []
+        )
+      end
+    end
+
+    it_behaves_like 'import bitbucket PullRequestsImporter', false
+  end
 end
-- 
GitLab


From fe944d6ca71dd2c4544ed0692ec6a36c486cb783 Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Sun, 23 Jun 2024 22:29:46 +0700
Subject: [PATCH 05/15] Add more spec

---
 .../bitbucket_import/parallel_scheduling.rb   |   4 +-
 .../parallel_scheduling_spec.rb               | 140 +++++++++++++++++-
 2 files changed, 134 insertions(+), 10 deletions(-)

diff --git a/lib/gitlab/bitbucket_import/parallel_scheduling.rb b/lib/gitlab/bitbucket_import/parallel_scheduling.rb
index 98244cb2852b54..8f03bf1db1cd74 100644
--- a/lib/gitlab/bitbucket_import/parallel_scheduling.rb
+++ b/lib/gitlab/bitbucket_import/parallel_scheduling.rb
@@ -34,8 +34,6 @@ def initialize(project)
         @page_keyset = Gitlab::Import::PageKeyset.new(project, collection_method, 'bitbucket-importer')
       end
 
-      private
-
       # The method that will be called for traversing through all the objects to
       # import, yielding them to the supplied block.
       def each_object_to_import
@@ -62,6 +60,8 @@ def each_object_to_import
         end
       end
 
+      private
+
       # Any options to be passed to the method used for retrieving the data to
       # import.
       def collection_options
diff --git a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
index 269ff6631e636b..554085eabe5a56 100644
--- a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
@@ -3,19 +3,29 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::BitbucketImport::ParallelScheduling, feature_category: :importers do
-  let_it_be(:project) { build(:project) }
+  let_it_be(:project) do
+    create(:project, :import_started, import_source: 'foo/bar',
+      import_data_attributes: {
+        data: {
+          'project_key' => 'key',
+          'repo_slug' => 'slug'
+        },
+        credentials: { 'base_uri' => 'http://bitbucket.org/', 'user' => 'bitbucket', 'password' => 'password' }
+      }
+    )
+  end
 
-  describe '#calculate_job_delay' do
-    let(:importer_class) do
-      Class.new do
-        include Gitlab::BitbucketImport::ParallelScheduling
+  let(:importer_class) do
+    Class.new do
+      include Gitlab::BitbucketImport::ParallelScheduling
 
-        def collection_method
-          :issues
-        end
+      def collection_method
+        :issues
       end
     end
+  end
 
+  describe '#calculate_job_delay' do
     let(:importer) { importer_class.new(project) }
 
     before do
@@ -34,4 +44,118 @@ def collection_method
       expect(importer.send(:calculate_job_delay, 100)).to eq(50.minutes)
     end
   end
+
+  describe '#each_object_to_import' do
+    let_it_be(:opened_issue) { Bitbucket::Representation::Issue.new({ 'id' => 1, 'state' => 'OPENED' }) }
+    let_it_be(:object) { opened_issue.to_hash }
+
+    let(:importer) { importer_class.new(project) }
+
+    before do
+      allow(importer)
+        .to receive(:collection_options)
+        .and_return({ raw: true })
+
+      allow(importer)
+        .to receive(:representation_type)
+        .and_return(:pull_request)
+    end
+
+    it 'yields every object to import' do
+      page = instance_double('Bitbucket::Page', attrs: [], items: [opened_issue])
+      allow(page).to receive(:next?).and_return(true)
+      allow(page).to receive(:next).and_return('https://example.com/next')
+
+      allow_next_instance_of(Bitbucket::Client) do |client|
+        expect(client)
+          .to receive(:each_page)
+          .with(:issues, 'foo/bar', { raw: true, representation_type: :pull_request, next_url: nil })
+          .and_yield(page)
+      end
+
+      expect(importer.page_keyset)
+        .to receive(:set)
+        .with('https://example.com/next')
+        .and_return(true)
+
+      expect(importer)
+        .to receive(:already_enqueued?)
+        .with(object)
+        .and_return(false)
+
+      expect(importer)
+        .to receive(:mark_as_enqueued)
+        .with(object)
+
+      expect { |b| importer.each_object_to_import(&b) }
+        .to yield_with_args(object)
+    end
+
+    it 'resumes from the last page' do
+      page = instance_double('Bitbucket::Page', attrs: [], items: [opened_issue])
+      allow(page).to receive(:next?).and_return(true)
+      allow(page).to receive(:next).and_return('https://example.com/next2')
+
+      expect(importer.page_keyset)
+        .to receive(:current)
+        .and_return('https://example.com/next')
+
+      allow_next_instance_of(Bitbucket::Client) do |client|
+        expect(client)
+          .to receive(:each_page)
+          .with(:issues, 'foo/bar', {
+            raw: true,
+            representation_type: :pull_request,
+            next_url: 'https://example.com/next'
+          })
+          .and_yield(page)
+      end
+
+      expect(importer.page_keyset)
+        .to receive(:set)
+        .with('https://example.com/next2')
+        .and_return(true)
+
+      expect(importer)
+        .to receive(:already_enqueued?)
+        .with(object)
+        .and_return(false)
+
+      expect(importer)
+        .to receive(:mark_as_enqueued)
+        .with(object)
+
+      expect { |b| importer.each_object_to_import(&b) }
+        .to yield_with_args(object)
+    end
+
+    it 'does not yield the object if it was already imported' do
+      page = instance_double('Bitbucket::Page', attrs: [], items: [opened_issue])
+      allow(page).to receive(:next?).and_return(true)
+      allow(page).to receive(:next).and_return('https://example.com/next')
+
+      allow_next_instance_of(Bitbucket::Client) do |client|
+        expect(client)
+          .to receive(:each_page)
+          .with(:issues, 'foo/bar', { raw: true, representation_type: :pull_request, next_url: nil })
+          .and_yield(page)
+      end
+
+      expect(importer.page_keyset)
+        .to receive(:set)
+        .with('https://example.com/next')
+        .and_return(true)
+
+      expect(importer)
+        .to receive(:already_enqueued?)
+        .with(object)
+        .and_return(true)
+
+      expect(importer)
+        .not_to receive(:mark_as_enqueued)
+
+      expect { |b| importer.each_object_to_import(&b) }
+        .not_to yield_control
+    end
+  end
 end
-- 
GitLab


From 327ab95fa10ae0ed5dfb18bdb04276eeec690b73 Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Sun, 23 Jun 2024 22:41:41 +0700
Subject: [PATCH 06/15] add spec for keyset

---
 spec/lib/gitlab/import/page_keyset_spec.rb | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 spec/lib/gitlab/import/page_keyset_spec.rb

diff --git a/spec/lib/gitlab/import/page_keyset_spec.rb b/spec/lib/gitlab/import/page_keyset_spec.rb
new file mode 100644
index 00000000000000..73144bda88d0b2
--- /dev/null
+++ b/spec/lib/gitlab/import/page_keyset_spec.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Import::PageKeyset, :clean_gitlab_redis_shared_state, feature_category: :importers do
+  let(:project) { instance_double(Project, id: 1) }
+  let(:keyset) { described_class.new(project, :issues, 'bitbucket-import') }
+
+  describe '#initialize' do
+    it 'sets the initial next url to be nil when no value is cached' do
+      expect(keyset.current).to eq(nil)
+    end
+
+    it 'sets the initial next url to the cached value when one is present' do
+      Gitlab::Cache::Import::Caching.write(keyset.cache_key, 'https://example.com/nextpresent')
+
+      expect(described_class.new(project, :issues, 'bitbucket-import').current).to eq('https://example.com/nextpresent')
+    end
+  end
+
+  describe '#set' do
+    it 'sets the next url' do
+      keyset.set('https://example.com/next')
+      expect(keyset.current).to eq('https://example.com/next')
+    end
+  end
+
+  describe '#expire!' do
+    it 'expires the current next url' do
+      keyset.set('https://example.com/next')
+
+      keyset.expire!
+
+      expect(Gitlab::Cache::Import::Caching.read(keyset.cache_key)).to be_nil
+      expect(keyset.current).to eq(nil)
+    end
+  end
+end
-- 
GitLab


From a518672001f5445e26658c67b07abc9fde060b40 Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Mon, 24 Jun 2024 21:41:30 +0700
Subject: [PATCH 07/15] Fix next url and error handling

---
 lib/bitbucket/client.rb                       |  1 +
 .../importers/pull_requests_importer.rb       |  3 --
 .../importers/pull_requests_importer_spec.rb  | 44 ++++++++++++-------
 3 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/lib/bitbucket/client.rb b/lib/bitbucket/client.rb
index c9f64b5248127d..a1d1f682b8e29b 100644
--- a/lib/bitbucket/client.rb
+++ b/lib/bitbucket/client.rb
@@ -24,6 +24,7 @@ class Client
       values.links
       values.summary
       values.reviewers
+      next
     ].freeze
 
     def initialize(options = {})
diff --git a/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb b/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
index 1c959468b25ab3..c9de0d3384153f 100644
--- a/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
+++ b/lib/gitlab/bitbucket_import/importers/pull_requests_importer.rb
@@ -28,9 +28,6 @@ def resumable_execute
             sidekiq_worker_class.perform_in(job_delay, project.id, object.to_hash, job_waiter.key)
           end
 
-          job_waiter
-        rescue StandardError => e
-          track_import_failure!(project, exception: e)
           job_waiter
         end
 
diff --git a/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb
index 92342eafef91bb..83197df5139c84 100644
--- a/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb
@@ -30,20 +30,6 @@
         .to match_array(%w[1 2 3])
     end
 
-    context 'when the client raises an error' do
-      before do
-        allow_next_instance_of(Bitbucket::Client) do |client|
-          allow(client).to receive(:pull_requests).and_raise(StandardError)
-        end
-      end
-
-      it 'tracks the failure and does not fail' do
-        expect(Gitlab::Import::ImportFailureService).to receive(:track).once
-
-        expect(importer.execute).to be_a(Gitlab::JobWaiter)
-      end
-    end
-
     context 'when pull request was already enqueued' do
       before do
         Gitlab::Cache::Import::Caching.set_add(importer.already_enqueued_cache_key, 1)
@@ -75,7 +61,19 @@
       end
     end
 
-    it_behaves_like 'import bitbucket PullRequestsImporter', true
+    it_behaves_like 'import bitbucket PullRequestsImporter', true do
+      context 'when the client raises an error' do
+        before do
+          allow_next_instance_of(Bitbucket::Client) do |client|
+            allow(client).to receive(:pull_requests).and_raise(StandardError)
+          end
+        end
+
+        it 'raises the error' do
+          expect { importer.execute }.to raise_error(StandardError)
+        end
+      end
+    end
   end
 
   describe '#non_resumable_execute' do
@@ -92,6 +90,20 @@
       end
     end
 
-    it_behaves_like 'import bitbucket PullRequestsImporter', false
+    it_behaves_like 'import bitbucket PullRequestsImporter', false do
+      context 'when the client raises an error' do
+        before do
+          allow_next_instance_of(Bitbucket::Client) do |client|
+            allow(client).to receive(:pull_requests).and_raise(StandardError)
+          end
+        end
+
+        it 'tracks the failure and does not fail' do
+          expect(Gitlab::Import::ImportFailureService).to receive(:track).once
+
+          expect(importer.execute).to be_a(Gitlab::JobWaiter)
+        end
+      end
+    end
   end
 end
-- 
GitLab


From fb24fe3569d235180ad5ef1b2ae1e48e8cd092f8 Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Tue, 25 Jun 2024 22:48:53 +0700
Subject: [PATCH 08/15] Add spec for client

---
 spec/lib/bitbucket/client_spec.rb | 58 +++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/spec/lib/bitbucket/client_spec.rb b/spec/lib/bitbucket/client_spec.rb
index 28724bd53ae1c2..a8c00479c43397 100644
--- a/spec/lib/bitbucket/client_spec.rb
+++ b/spec/lib/bitbucket/client_spec.rb
@@ -14,6 +14,52 @@
 
   subject(:client) { described_class.new(options) }
 
+  describe '#each_page' do
+    let_it_be(:item1) do
+      { 'username' => 'Ben' }
+    end
+
+    let_it_be(:item2) do
+      { 'username' => 'Affleck' }
+    end
+
+    let_it_be(:response1) do
+      { 'values' => [item1], 'next' => 'https://example.com/next' }
+    end
+
+    let_it_be(:response2) do
+      { 'values' => [item2], 'next' => nil }
+    end
+
+    before do
+      allow(client)
+        .to receive(:public_send)
+        .with(:foo, { representation_type: :pull_request })
+        .and_return(response1)
+
+      allow(client)
+        .to receive(:public_send)
+        .with(:foo, { representation_type: :pull_request, next_url: 'https://example.com/next' })
+        .and_return(response2)
+    end
+
+    it 'yields every retrieved page to the supplied block' do
+      pages = []
+
+      client.each_page(:foo, representation_type: :pull_request) { |page| pages << page }
+
+      expect(pages[0]).to be_an_instance_of(Bitbucket::Page)
+
+      expect(pages[0].items.count).to eq(1)
+      expect(pages[0].items.first.raw).to eq(item1)
+      expect(pages[0].attrs[:next]).to eq('https://example.com/next')
+
+      expect(pages[1].items.count).to eq(1)
+      expect(pages[1].items.first.raw).to eq(item2)
+      expect(pages[1].attrs[:next]).to eq(nil)
+    end
+  end
+
   describe '#last_issue' do
     let(:url) { "#{root_url}/repositories/#{repo}/issues?pagelen=1&sort=-created_on&state=ALL" }
 
@@ -59,6 +105,18 @@
 
       client.pull_requests(repo)
     end
+
+    context 'with options raw' do
+      let(:url) { "#{root_url}#{path}" }
+
+      it 'returns raw result' do
+        stub_request(:get, url).to_return(status: 200, headers: headers, body: '{}')
+
+        client.pull_requests(repo, raw: true)
+
+        expect(WebMock).to have_requested(:get, url)
+      end
+    end
   end
 
   describe '#pull_request_comments' do
-- 
GitLab


From 3b3ea8cf298b16414b70197a1122b43ea47c10f9 Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivantedja@gmail.com>
Date: Tue, 25 Jun 2024 19:51:47 +0000
Subject: [PATCH 09/15] Apply 1 suggestion(s) to 1 file(s)

Co-authored-by: Rodrigo Tomonari <rtomonari@gitlab.com>
---
 spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
index 554085eabe5a56..7fe8fc7b0a53ab 100644
--- a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
@@ -58,7 +58,7 @@ def collection_method
 
       allow(importer)
         .to receive(:representation_type)
-        .and_return(:pull_request)
+        .and_return(:issue)
     end
 
     it 'yields every object to import' do
-- 
GitLab


From b1d95b9c71639007c5c90921b4f49a6591d7558e Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Wed, 26 Jun 2024 08:58:32 +0700
Subject: [PATCH 10/15] Adjust representation_type argument

---
 lib/bitbucket/client.rb                                    | 5 +++--
 lib/gitlab/bitbucket_import/parallel_scheduling.rb         | 4 ++--
 spec/lib/bitbucket/client_spec.rb                          | 6 +++---
 .../gitlab/bitbucket_import/parallel_scheduling_spec.rb    | 7 +++----
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/bitbucket/client.rb b/lib/bitbucket/client.rb
index d5adb205387675..74a74a4f24d64c 100644
--- a/lib/bitbucket/client.rb
+++ b/lib/bitbucket/client.rb
@@ -40,8 +40,9 @@ def initialize(options = {})
     # of data, without loading all of them into memory.
     #
     # method - The method name used for getting the data.
+    # representation_type - The representation type name used to wrap the result
     # args - Arguments to pass to the method.
-    def each_page(method, *args)
+    def each_page(method, representation_type, *args)
       options =
         if args.last.is_a?(Hash)
           args.last
@@ -51,7 +52,7 @@ def each_page(method, *args)
 
       loop do
         parsed_response = public_send(method, *args) # rubocop: disable GitlabSecurity/PublicSend -- generic method for pagination
-        object = Page.new(parsed_response, options[:representation_type])
+        object = Page.new(parsed_response, representation_type)
 
         yield object
 
diff --git a/lib/gitlab/bitbucket_import/parallel_scheduling.rb b/lib/gitlab/bitbucket_import/parallel_scheduling.rb
index 8f03bf1db1cd74..c4703c71da9a0c 100644
--- a/lib/gitlab/bitbucket_import/parallel_scheduling.rb
+++ b/lib/gitlab/bitbucket_import/parallel_scheduling.rb
@@ -39,9 +39,9 @@ def initialize(project)
       def each_object_to_import
         repo = project.import_source
 
-        options = collection_options.merge(representation_type: representation_type, next_url: page_keyset.current)
+        options = collection_options.merge(next_url: page_keyset.current)
 
-        client.each_page(collection_method, repo, options) do |page|
+        client.each_page(collection_method, representation_type, repo, options) do |page|
           page.items.each do |object|
             job_waiter.jobs_remaining = Gitlab::Cache::Import::Caching.increment(job_waiter_remaining_cache_key)
 
diff --git a/spec/lib/bitbucket/client_spec.rb b/spec/lib/bitbucket/client_spec.rb
index a8c00479c43397..b11e9d5516b332 100644
--- a/spec/lib/bitbucket/client_spec.rb
+++ b/spec/lib/bitbucket/client_spec.rb
@@ -34,19 +34,19 @@
     before do
       allow(client)
         .to receive(:public_send)
-        .with(:foo, { representation_type: :pull_request })
+        .with(:foo)
         .and_return(response1)
 
       allow(client)
         .to receive(:public_send)
-        .with(:foo, { representation_type: :pull_request, next_url: 'https://example.com/next' })
+        .with(:foo, { next_url: 'https://example.com/next' })
         .and_return(response2)
     end
 
     it 'yields every retrieved page to the supplied block' do
       pages = []
 
-      client.each_page(:foo, representation_type: :pull_request) { |page| pages << page }
+      client.each_page(:foo, :issue) { |page| pages << page }
 
       expect(pages[0]).to be_an_instance_of(Bitbucket::Page)
 
diff --git a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
index 7fe8fc7b0a53ab..e1e0ba494a360d 100644
--- a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
@@ -69,7 +69,7 @@ def collection_method
       allow_next_instance_of(Bitbucket::Client) do |client|
         expect(client)
           .to receive(:each_page)
-          .with(:issues, 'foo/bar', { raw: true, representation_type: :pull_request, next_url: nil })
+          .with(:issues, :issue, 'foo/bar', { raw: true, next_url: nil })
           .and_yield(page)
       end
 
@@ -103,9 +103,8 @@ def collection_method
       allow_next_instance_of(Bitbucket::Client) do |client|
         expect(client)
           .to receive(:each_page)
-          .with(:issues, 'foo/bar', {
+          .with(:issues, :issue, 'foo/bar', {
             raw: true,
-            representation_type: :pull_request,
             next_url: 'https://example.com/next'
           })
           .and_yield(page)
@@ -137,7 +136,7 @@ def collection_method
       allow_next_instance_of(Bitbucket::Client) do |client|
         expect(client)
           .to receive(:each_page)
-          .with(:issues, 'foo/bar', { raw: true, representation_type: :pull_request, next_url: nil })
+          .with(:issues, :issue, 'foo/bar', { raw: true, next_url: nil })
           .and_yield(page)
       end
 
-- 
GitLab


From 62cb3290faede1c8dcab40d0a72a61355d05a332 Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Wed, 26 Jun 2024 11:16:24 +0700
Subject: [PATCH 11/15] Add spec coverage

---
 spec/lib/bitbucket/client_spec.rb             | 21 +++++++++++++++++--
 .../parallel_scheduling_spec.rb               |  9 ++------
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/spec/lib/bitbucket/client_spec.rb b/spec/lib/bitbucket/client_spec.rb
index b11e9d5516b332..00ed35ab65a86f 100644
--- a/spec/lib/bitbucket/client_spec.rb
+++ b/spec/lib/bitbucket/client_spec.rb
@@ -23,12 +23,20 @@
       { 'username' => 'Affleck' }
     end
 
+    let_it_be(:item3) do
+      { 'username' => 'Jane' }
+    end
+
     let_it_be(:response1) do
       { 'values' => [item1], 'next' => 'https://example.com/next' }
     end
 
     let_it_be(:response2) do
-      { 'values' => [item2], 'next' => nil }
+      { 'values' => [item2], 'next' => 'https://example.com/next2' }
+    end
+
+    let_it_be(:response3) do
+      { 'values' => [item3], 'next' => nil }
     end
 
     before do
@@ -41,6 +49,11 @@
         .to receive(:public_send)
         .with(:foo, { next_url: 'https://example.com/next' })
         .and_return(response2)
+
+      allow(client)
+        .to receive(:public_send)
+        .with(:foo, { next_url: 'https://example.com/next2' })
+        .and_return(response3)
     end
 
     it 'yields every retrieved page to the supplied block' do
@@ -56,7 +69,11 @@
 
       expect(pages[1].items.count).to eq(1)
       expect(pages[1].items.first.raw).to eq(item2)
-      expect(pages[1].attrs[:next]).to eq(nil)
+      expect(pages[1].attrs[:next]).to eq('https://example.com/next2')
+
+      expect(pages[2].items.count).to eq(1)
+      expect(pages[2].items.first.raw).to eq(item3)
+      expect(pages[2].attrs[:next]).to eq(nil)
     end
   end
 
diff --git a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
index e1e0ba494a360d..57cd76899f3387 100644
--- a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
@@ -52,10 +52,6 @@ def collection_method
     let(:importer) { importer_class.new(project) }
 
     before do
-      allow(importer)
-        .to receive(:collection_options)
-        .and_return({ raw: true })
-
       allow(importer)
         .to receive(:representation_type)
         .and_return(:issue)
@@ -69,7 +65,7 @@ def collection_method
       allow_next_instance_of(Bitbucket::Client) do |client|
         expect(client)
           .to receive(:each_page)
-          .with(:issues, :issue, 'foo/bar', { raw: true, next_url: nil })
+          .with(:issues, :issue, 'foo/bar', { next_url: nil })
           .and_yield(page)
       end
 
@@ -104,7 +100,6 @@ def collection_method
         expect(client)
           .to receive(:each_page)
           .with(:issues, :issue, 'foo/bar', {
-            raw: true,
             next_url: 'https://example.com/next'
           })
           .and_yield(page)
@@ -136,7 +131,7 @@ def collection_method
       allow_next_instance_of(Bitbucket::Client) do |client|
         expect(client)
           .to receive(:each_page)
-          .with(:issues, :issue, 'foo/bar', { raw: true, next_url: nil })
+          .with(:issues, :issue, 'foo/bar', { next_url: nil })
           .and_yield(page)
       end
 
-- 
GitLab


From b4b3594689444e3ed319a0ba6ec87cb9f435b78b Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Tue, 2 Jul 2024 20:32:18 +0700
Subject: [PATCH 12/15] Adjust review

---
 lib/gitlab/import/page_keyset.rb                 | 16 +++++++++++-----
 .../importers/pull_requests_importer_spec.rb     | 12 ++++++++----
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/lib/gitlab/import/page_keyset.rb b/lib/gitlab/import/page_keyset.rb
index a6a149981c26fd..3ef193a2020ac3 100644
--- a/lib/gitlab/import/page_keyset.rb
+++ b/lib/gitlab/import/page_keyset.rb
@@ -15,18 +15,24 @@ def initialize(object, collection, import_type)
         @cache_key = format(CACHE_KEY, import_type: import_type, object: object.id, collection: collection)
       end
 
-      # Sets the key to the given value.
+      # Set the key to the given value.
       #
-      # Returns true if the key was overwritten, false otherwise.
-      def set(key)
-        Gitlab::Cache::Import::Caching.write(cache_key, key)
+      # @param value [String]
+      # @return [String]
+      def set(value)
+        Gitlab::Cache::Import::Caching.write(cache_key, value)
       end
 
-      # Returns the current value from the cache.
+      # Get the current value from the cache
+      #
+      # @return [String]
       def current
         Gitlab::Cache::Import::Caching.read(cache_key)
       end
 
+      # Expire the key
+      #
+      # @return [Boolean]
       def expire!
         Gitlab::Cache::Import::Caching.expire(cache_key, 0)
       end
diff --git a/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb
index 83197df5139c84..cb292a7d6f3671 100644
--- a/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/importers/pull_requests_importer_spec.rb
@@ -65,12 +65,12 @@
       context 'when the client raises an error' do
         before do
           allow_next_instance_of(Bitbucket::Client) do |client|
-            allow(client).to receive(:pull_requests).and_raise(StandardError)
+            allow(client).to receive(:pull_requests).and_raise(StandardError.new('error fetching PRs'))
           end
         end
 
         it 'raises the error' do
-          expect { importer.execute }.to raise_error(StandardError)
+          expect { importer.execute }.to raise_error(StandardError, 'error fetching PRs')
         end
       end
     end
@@ -92,14 +92,18 @@
 
     it_behaves_like 'import bitbucket PullRequestsImporter', false do
       context 'when the client raises an error' do
+        let(:exception) { StandardError.new('error fetching PRs') }
+
         before do
           allow_next_instance_of(Bitbucket::Client) do |client|
-            allow(client).to receive(:pull_requests).and_raise(StandardError)
+            allow(client).to receive(:pull_requests).and_raise(exception)
           end
         end
 
         it 'tracks the failure and does not fail' do
-          expect(Gitlab::Import::ImportFailureService).to receive(:track).once
+          expect(Gitlab::Import::ImportFailureService).to receive(:track)
+            .once
+            .with(a_hash_including(exception: exception))
 
           expect(importer.execute).to be_a(Gitlab::JobWaiter)
         end
-- 
GitLab


From 878fe191b70b53a269f6c3c89a88b36a1f5e892e Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Wed, 3 Jul 2024 02:38:26 +0700
Subject: [PATCH 13/15] fix undercoverage

---
 .../parallel_scheduling_spec.rb                 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
index 57cd76899f3387..c548094800b23a 100644
--- a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
@@ -152,4 +152,21 @@ def collection_method
         .not_to yield_control
     end
   end
+
+  describe '#representation_type' do
+    let(:importer_class_representation) do
+      Class.new do
+        include Gitlab::BitbucketImport::ParallelScheduling
+
+        def retrieve_representation_type
+          representation_type
+        end
+      end
+    end
+
+    it 'raises NotImplementedError' do
+      expect { importer_class_representation.new(project).retrieve_representation_type }
+        .to raise_error(NotImplementedError)
+    end
+  end
 end
-- 
GitLab


From b66b13df8cd46fddc67f067fe19b35e736877ec1 Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Wed, 3 Jul 2024 05:57:34 +0700
Subject: [PATCH 14/15] fix undercoverage

---
 .../parallel_scheduling_spec.rb               | 179 +++++++++---------
 1 file changed, 85 insertions(+), 94 deletions(-)

diff --git a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
index c548094800b23a..b3eb591de48b41 100644
--- a/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/parallel_scheduling_spec.rb
@@ -51,122 +51,113 @@ def collection_method
 
     let(:importer) { importer_class.new(project) }
 
-    before do
-      allow(importer)
-        .to receive(:representation_type)
-        .and_return(:issue)
+    context 'without representation_type' do
+      it 'raises NotImplementedError' do
+        expect { importer_class.new(project).each_object_to_import }.to raise_error(NotImplementedError)
+      end
     end
 
-    it 'yields every object to import' do
-      page = instance_double('Bitbucket::Page', attrs: [], items: [opened_issue])
-      allow(page).to receive(:next?).and_return(true)
-      allow(page).to receive(:next).and_return('https://example.com/next')
-
-      allow_next_instance_of(Bitbucket::Client) do |client|
-        expect(client)
-          .to receive(:each_page)
-          .with(:issues, :issue, 'foo/bar', { next_url: nil })
-          .and_yield(page)
+    context 'with representation_type' do
+      before do
+        allow(importer)
+          .to receive(:representation_type)
+          .and_return(:issue)
       end
 
-      expect(importer.page_keyset)
-        .to receive(:set)
-        .with('https://example.com/next')
-        .and_return(true)
+      it 'yields every object to import' do
+        page = instance_double('Bitbucket::Page', attrs: [], items: [opened_issue])
+        allow(page).to receive(:next?).and_return(true)
+        allow(page).to receive(:next).and_return('https://example.com/next')
+
+        allow_next_instance_of(Bitbucket::Client) do |client|
+          expect(client)
+            .to receive(:each_page)
+            .with(:issues, :issue, 'foo/bar', { next_url: nil })
+            .and_yield(page)
+        end
 
-      expect(importer)
-        .to receive(:already_enqueued?)
-        .with(object)
-        .and_return(false)
+        expect(importer.page_keyset)
+          .to receive(:set)
+          .with('https://example.com/next')
+          .and_return(true)
 
-      expect(importer)
-        .to receive(:mark_as_enqueued)
-        .with(object)
+        expect(importer)
+          .to receive(:already_enqueued?)
+          .with(object)
+          .and_return(false)
 
-      expect { |b| importer.each_object_to_import(&b) }
-        .to yield_with_args(object)
-    end
+        expect(importer)
+          .to receive(:mark_as_enqueued)
+          .with(object)
 
-    it 'resumes from the last page' do
-      page = instance_double('Bitbucket::Page', attrs: [], items: [opened_issue])
-      allow(page).to receive(:next?).and_return(true)
-      allow(page).to receive(:next).and_return('https://example.com/next2')
-
-      expect(importer.page_keyset)
-        .to receive(:current)
-        .and_return('https://example.com/next')
-
-      allow_next_instance_of(Bitbucket::Client) do |client|
-        expect(client)
-          .to receive(:each_page)
-          .with(:issues, :issue, 'foo/bar', {
-            next_url: 'https://example.com/next'
-          })
-          .and_yield(page)
+        expect { |b| importer.each_object_to_import(&b) }
+          .to yield_with_args(object)
       end
 
-      expect(importer.page_keyset)
-        .to receive(:set)
-        .with('https://example.com/next2')
-        .and_return(true)
+      it 'resumes from the last page' do
+        page = instance_double('Bitbucket::Page', attrs: [], items: [opened_issue])
+        allow(page).to receive(:next?).and_return(true)
+        allow(page).to receive(:next).and_return('https://example.com/next2')
+
+        expect(importer.page_keyset)
+          .to receive(:current)
+          .and_return('https://example.com/next')
+
+        allow_next_instance_of(Bitbucket::Client) do |client|
+          expect(client)
+            .to receive(:each_page)
+            .with(:issues, :issue, 'foo/bar', {
+              next_url: 'https://example.com/next'
+            })
+            .and_yield(page)
+        end
 
-      expect(importer)
-        .to receive(:already_enqueued?)
-        .with(object)
-        .and_return(false)
+        expect(importer.page_keyset)
+          .to receive(:set)
+          .with('https://example.com/next2')
+          .and_return(true)
 
-      expect(importer)
-        .to receive(:mark_as_enqueued)
-        .with(object)
+        expect(importer)
+          .to receive(:already_enqueued?)
+          .with(object)
+          .and_return(false)
 
-      expect { |b| importer.each_object_to_import(&b) }
-        .to yield_with_args(object)
-    end
+        expect(importer)
+          .to receive(:mark_as_enqueued)
+          .with(object)
 
-    it 'does not yield the object if it was already imported' do
-      page = instance_double('Bitbucket::Page', attrs: [], items: [opened_issue])
-      allow(page).to receive(:next?).and_return(true)
-      allow(page).to receive(:next).and_return('https://example.com/next')
-
-      allow_next_instance_of(Bitbucket::Client) do |client|
-        expect(client)
-          .to receive(:each_page)
-          .with(:issues, :issue, 'foo/bar', { next_url: nil })
-          .and_yield(page)
+        expect { |b| importer.each_object_to_import(&b) }
+          .to yield_with_args(object)
       end
 
-      expect(importer.page_keyset)
-        .to receive(:set)
-        .with('https://example.com/next')
-        .and_return(true)
+      it 'does not yield the object if it was already imported' do
+        page = instance_double('Bitbucket::Page', attrs: [], items: [opened_issue])
+        allow(page).to receive(:next?).and_return(true)
+        allow(page).to receive(:next).and_return('https://example.com/next')
 
-      expect(importer)
-        .to receive(:already_enqueued?)
-        .with(object)
-        .and_return(true)
+        allow_next_instance_of(Bitbucket::Client) do |client|
+          expect(client)
+            .to receive(:each_page)
+            .with(:issues, :issue, 'foo/bar', { next_url: nil })
+            .and_yield(page)
+        end
 
-      expect(importer)
-        .not_to receive(:mark_as_enqueued)
+        expect(importer.page_keyset)
+          .to receive(:set)
+          .with('https://example.com/next')
+          .and_return(true)
 
-      expect { |b| importer.each_object_to_import(&b) }
-        .not_to yield_control
-    end
-  end
+        expect(importer)
+          .to receive(:already_enqueued?)
+          .with(object)
+          .and_return(true)
 
-  describe '#representation_type' do
-    let(:importer_class_representation) do
-      Class.new do
-        include Gitlab::BitbucketImport::ParallelScheduling
+        expect(importer)
+          .not_to receive(:mark_as_enqueued)
 
-        def retrieve_representation_type
-          representation_type
-        end
+        expect { |b| importer.each_object_to_import(&b) }
+          .not_to yield_control
       end
     end
-
-    it 'raises NotImplementedError' do
-      expect { importer_class_representation.new(project).retrieve_representation_type }
-        .to raise_error(NotImplementedError)
-    end
   end
 end
-- 
GitLab


From 11377b80b143d62d7d7406158314a5e8ee01f929 Mon Sep 17 00:00:00 2001
From: Ivan Sebastian <ivan.sebastian@wartek.belajar.id>
Date: Wed, 3 Jul 2024 19:31:18 +0700
Subject: [PATCH 15/15] remove public send

---
 lib/bitbucket/client.rb           | 10 +++++++++-
 spec/lib/bitbucket/client_spec.rb | 21 ++++++++++++++-------
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/lib/bitbucket/client.rb b/lib/bitbucket/client.rb
index 74a74a4f24d64c..c031851af3d7be 100644
--- a/lib/bitbucket/client.rb
+++ b/lib/bitbucket/client.rb
@@ -51,7 +51,7 @@ def each_page(method, representation_type, *args)
         end
 
       loop do
-        parsed_response = public_send(method, *args) # rubocop: disable GitlabSecurity/PublicSend -- generic method for pagination
+        parsed_response = fetch_data(method, *args)
         object = Page.new(parsed_response, representation_type)
 
         yield object
@@ -130,6 +130,14 @@ def users(workspace_key, page_number: nil, limit: nil)
 
     private
 
+    def fetch_data(method, *args)
+      case method
+      when :pull_requests then pull_requests(*args)
+      else
+        raise ArgumentError, "Unknown data method #{method}"
+      end
+    end
+
     def get_collection(path, type, page_number: nil, limit: nil)
       paginator = Paginator.new(connection, path, type, page_number: page_number, limit: limit)
       Collection.new(paginator)
diff --git a/spec/lib/bitbucket/client_spec.rb b/spec/lib/bitbucket/client_spec.rb
index 00ed35ab65a86f..a485ca5455827f 100644
--- a/spec/lib/bitbucket/client_spec.rb
+++ b/spec/lib/bitbucket/client_spec.rb
@@ -41,25 +41,25 @@
 
     before do
       allow(client)
-        .to receive(:public_send)
-        .with(:foo)
+        .to receive(:pull_requests)
+        .with('repo')
         .and_return(response1)
 
       allow(client)
-        .to receive(:public_send)
-        .with(:foo, { next_url: 'https://example.com/next' })
+        .to receive(:pull_requests)
+        .with('repo', { next_url: 'https://example.com/next' })
         .and_return(response2)
 
       allow(client)
-        .to receive(:public_send)
-        .with(:foo, { next_url: 'https://example.com/next2' })
+        .to receive(:pull_requests)
+        .with('repo', { next_url: 'https://example.com/next2' })
         .and_return(response3)
     end
 
     it 'yields every retrieved page to the supplied block' do
       pages = []
 
-      client.each_page(:foo, :issue) { |page| pages << page }
+      client.each_page(:pull_requests, :pull_request, 'repo') { |page| pages << page }
 
       expect(pages[0]).to be_an_instance_of(Bitbucket::Page)
 
@@ -75,6 +75,13 @@
       expect(pages[2].items.first.raw).to eq(item3)
       expect(pages[2].attrs[:next]).to eq(nil)
     end
+
+    context 'when fetch_data not defined' do
+      it 'raises argument error' do
+        expect { client.each_page(:foo, :pull_request, 'repo') }
+          .to raise_error(ArgumentError, 'Unknown data method foo')
+      end
+    end
   end
 
   describe '#last_issue' do
-- 
GitLab