Break Geo::RepositorySyncService into Repository/Wiki sync services

app/services/geo/base_sync_service.rb

+module Geo
+  class BaseSyncService
+    class << self
+      attr_accessor :type
+    end
+
+    attr_reader :project
+
+    LEASE_TIMEOUT    = 8.hours.freeze
+    LEASE_KEY_PREFIX = 'geo_sync_service'.freeze
+
+    def initialize(project)
+      @project = project
+    end
+
+    def execute
+      try_obtain_lease do
+        log("Started #{type} sync")
+        sync_repository
+        log("Finished #{type} sync")
+      end
+    end
+
+    private
+
+    def registry
+      @registry ||= Geo::ProjectRegistry.find_or_initialize_by(project_id: project.id)
+    end
+
+    def try_obtain_lease
+      log("Trying to obtain lease to sync #{type}")
+      repository_lease = Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT).try_obtain
+
+      unless repository_lease
+        log("Could not obtain lease to sync #{type}")
+        return
+      end
+
+      yield
+
+      # We should release the lease for a repository, only if we have obtained
+      # it. If something went wrong when syncing the repository, we should wait
+      # for the lease timeout to try again.
+      log("Releasing leases to sync #{type}")
+      Gitlab::ExclusiveLease.cancel(lease_key, repository_lease)
+    end
+
+    def update_registry(type, started_at: nil, finished_at: nil)
+      return unless started_at || finished_at
+
+      log("Updating #{type} sync information")
+
+      if started_at
+        registry.public_send("last_#{type}_synced_at=", started_at)
+      end
+
+      if finished_at
+        registry.public_send("last_#{type}_successful_sync_at=", finished_at)
+        registry.public_send("resync_#{type}=", false)
+      end
+
+      registry.save
+    end
+
+    def lease_key
+      @lease_key ||= "#{LEASE_KEY_PREFIX}:#{type}:#{project.id}"
+    end
+
+    def type
+      self.class.type
+    end
+
+    def primary_ssh_path_prefix
+      Gitlab::Geo.primary_node.clone_url_prefix
+    end
+
+    def log(message)
+      Rails.logger.info("#{self.class.name}: #{message} for project #{project.path_with_namespace} (#{project.id})")
+    end
+  end
+end

app/services/geo/repository_sync_service.rb

 module Geo
-  class RepositorySyncService
-    attr_reader :project_id
-
-    LEASE_TIMEOUT    = 8.hours.freeze
-    LEASE_KEY_PREFIX = 'repository_sync_service'.freeze
-
-    def initialize(project_id)
-      @project_id = project_id
-    end
-
-    def execute
-      try_obtain_lease do
-        log('Started repository sync')
-        sync_project_repository
-        sync_wiki_repository
-        log('Finished repository sync')
-      end
-    rescue ActiveRecord::RecordNotFound
-      Rails.logger.error("#{self.class.name}: Couldn't find project with ID=#{project_id}, skipping syncing")
-    end
+  class RepositorySyncService < BaseSyncService
+    self.type = :repository
 
     private
 
-    def project
-      @project ||= Project.find(project_id)
-    end
-
-    def registry
-      @registry ||= Geo::ProjectRegistry.find_or_initialize_by(project_id: project_id)
-    end
-
-    def sync_project_repository
-      return unless sync_repository?
-
+    def sync_repository
       fetch_project_repository
       expire_repository_caches
     end
 
-    def sync_repository?
-      registry.resync_repository? ||
-        registry.last_repository_successful_sync_at.nil? ||
-        registry.last_repository_synced_at.nil?
-    end
-
-    def sync_wiki_repository
-      return unless sync_wiki?
-
-      fetch_wiki_repository
-    end
-
-    def sync_wiki?
-      registry.resync_wiki? ||
-        registry.last_wiki_successful_sync_at.nil? ||
-        registry.last_wiki_synced_at.nil?
-    end
-
     def fetch_project_repository
       log('Fetching project repository')
       update_registry(:repository, started_at: DateTime.now)
@@ -73,78 +27,13 @@ module Geo
       end
     end
 
-    def fetch_wiki_repository
-      log('Fetching wiki repository')
-      update_registry(:wiki, started_at: DateTime.now)
-
-      begin
-        project.wiki.ensure_repository
-        project.wiki.repository.fetch_geo_mirror(ssh_url_to_wiki)
-
-        update_registry(:wiki, finished_at: DateTime.now)
-      rescue Gitlab::Git::Repository::NoRepository, Gitlab::Shell::Error, ProjectWiki::CouldNotCreateWikiError => e
-        Rails.logger.error("#{self.class.name}: Error syncing wiki repository for project #{project.path_with_namespace}: #{e}")
-      end
-    end
-
     def expire_repository_caches
       log('Expiring caches')
       project.repository.after_sync
     end
 
-    def try_obtain_lease
-      log('Trying to obtain lease to sync repository')
-      repository_lease = Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT).try_obtain
-
-      unless repository_lease
-        log('Could not obtain lease to sync repository')
-        return
-      end
-
-      yield
-
-      # We should release the lease for a repository, only if we have obtained
-      # it. If something went wrong when syncing the repository, we should wait
-      # for the lease timeout to try again.
-      log('Releasing leases to sync repository')
-      Gitlab::ExclusiveLease.cancel(lease_key, repository_lease)
-    end
-
-    def update_registry(type, started_at: nil, finished_at: nil)
-      return unless started_at || finished_at
-
-      log("Updating #{type} sync information")
-
-      if started_at
-        registry.public_send("last_#{type}_synced_at=", started_at)
-      end
-
-      if finished_at
-        registry.public_send("last_#{type}_successful_sync_at=", finished_at)
-        registry.public_send("resync_#{type}=", false)
-      end
-
-      registry.save
-    end
-
-    def lease_key
-      @lease_key ||= "#{LEASE_KEY_PREFIX}:#{project.id}"
-    end
-
-    def primary_ssh_path_prefix
-      Gitlab::Geo.primary_node.clone_url_prefix
-    end
-
     def ssh_url_to_repo
       "#{primary_ssh_path_prefix}#{project.path_with_namespace}.git"
     end
-
-    def ssh_url_to_wiki
-      "#{primary_ssh_path_prefix}#{project.path_with_namespace}.wiki.git"
-    end
-
-    def log(message)
-      Rails.logger.info("#{self.class.name}: #{message} for project #{project.path_with_namespace} (#{project.id})")
-    end
   end
 end

app/services/geo/wiki_sync_service.rb

+module Geo
+  class WikiSyncService < BaseSyncService
+    self.type = :wiki
+
+    private
+
+    def sync_repository
+      fetch_wiki_repository
+    end
+
+    def fetch_wiki_repository
+      log('Fetching wiki repository')
+      update_registry(:wiki, started_at: DateTime.now)
+
+      begin
+        project.wiki.ensure_repository
+        project.wiki.repository.fetch_geo_mirror(ssh_url_to_wiki)
+
+        update_registry(:wiki, finished_at: DateTime.now)
+      rescue Gitlab::Git::Repository::NoRepository, Gitlab::Shell::Error, ProjectWiki::CouldNotCreateWikiError => e
+        Rails.logger.error("#{self.class.name}: Error syncing wiki repository for project #{project.path_with_namespace}: #{e}")
+      end
+    end
+
+    def ssh_url_to_wiki
+      "#{primary_ssh_path_prefix}#{project.path_with_namespace}.wiki.git"
+    end
+  end
+end

app/workers/geo/project_sync_worker.rb

@@ -11,9 +11,27 @@ module Geo
     end
 
     def perform(project_id, scheduled_time)
-      Geo::RepositorySyncService.new(project_id).execute
+      project  = Project.find(project_id)
+      registry = Geo::ProjectRegistry.find_or_initialize_by(project_id: project_id)
+
+      Geo::RepositorySyncService.new(project).execute if sync_repository?(registry)
+      Geo::WikiSyncService.new(project).execute if sync_wiki?(registry)
     rescue ActiveRecord::RecordNotFound
       logger.error("Couldn't find project with ID=#{project_id}, skipping syncing")
     end
+
+    private
+
+    def sync_repository?(registry)
+      registry.resync_repository? ||
+        registry.last_repository_successful_sync_at.nil? ||
+        registry.last_repository_synced_at.nil?
+    end
+
+    def sync_wiki?(registry)
+      registry.resync_wiki? ||
+        registry.last_wiki_successful_sync_at.nil? ||
+        registry.last_wiki_synced_at.nil?
+    end
   end
 end

spec/services/geo/wiki_sync_service_spec.rb

+require 'spec_helper'
+
+RSpec.describe Geo::WikiSyncService, services: true do
+  let!(:primary) { create(:geo_node, :primary, host: 'primary-geo-node') }
+  let(:lease) { double(try_obtain: true) }
+
+  subject { described_class.new(project) }
+
+  before do
+    allow(Gitlab::ExclusiveLease).to receive(:new)
+      .with(subject.__send__(:lease_key), anything)
+      .and_return(lease)
+
+    allow_any_instance_of(Repository).to receive(:fetch_geo_mirror)
+      .and_return(true)
+  end
+
+  describe '#execute' do
+    let(:project) { create(:project_empty_repo) }
+    let(:repository) { project.wiki.repository }
+    let(:url_to_repo) { "#{primary.clone_url_prefix}#{project.path_with_namespace}.wiki.git" }
+
+    it 'fetches wiki repository' do
+      expect(repository).to receive(:fetch_geo_mirror).with(url_to_repo).once
+
+      subject.execute
+    end
+
+    it 'releases lease' do
+      expect(Gitlab::ExclusiveLease).to receive(:cancel).once.with(
+        subject.__send__(:lease_key), anything).and_call_original
+
+      subject.execute
+    end
+
+    it 'does not fetch wiki repository if cannot obtain a lease' do
+      allow(lease).to receive(:try_obtain) { false }
+
+      expect(repository).not_to receive(:fetch_geo_mirror)
+
+      subject.execute
+    end
+
+    it 'rescues exception when Gitlab::Shell::Error is raised' do
+      allow(repository).to receive(:fetch_geo_mirror).with(url_to_repo) { raise Gitlab::Shell::Error }
+
+      expect { subject.execute }.not_to raise_error
+    end
+
+    it 'rescues exception when Gitlab::Git::Repository::NoRepository is raised' do
+      allow(repository).to receive(:fetch_geo_mirror).with(url_to_repo) { raise Gitlab::Git::Repository::NoRepository }
+
+      expect { subject.execute }.not_to raise_error
+    end
+
+    context 'tracking database' do
+      it 'creates a new registry if does not exists' do
+        expect { subject.execute }.to change(Geo::ProjectRegistry, :count).by(1)
+      end
+
+      it 'does not create a new registry if one exist' do
+        create(:geo_project_registry, project: project)
+
+        expect { subject.execute }.not_to change(Geo::ProjectRegistry, :count)
+      end
+
+      context 'when repository sync succeed' do
+        let(:registry) { Geo::ProjectRegistry.find_by(project_id: project.id) }
+
+        before do
+          subject.execute
+        end
+
+        it 'sets last_wiki_synced_at' do
+          expect(registry.last_wiki_synced_at).not_to be_nil
+        end
+
+        it 'sets last_wiki_successful_sync_at' do
+          expect(registry.last_wiki_successful_sync_at).not_to be_nil
+        end
+      end
+
+      context 'when wiki sync fail' do
+        let(:registry) { Geo::ProjectRegistry.find_by(project_id: project.id) }
+
+        before do
+          allow(repository).to receive(:fetch_geo_mirror).with(url_to_repo) { raise Gitlab::Shell::Error }
+
+          subject.execute
+        end
+
+        it 'sets last_wiki_synced_at' do
+          expect(registry.last_wiki_synced_at).not_to be_nil
+        end
+
+        it 'resets last_wiki_successful_sync_at' do
+          expect(registry.last_wiki_successful_sync_at).to be_nil
+        end
+      end
+    end
+  end
+end

spec/workers/geo/project_sync_worker_spec.rb

@@ -6,18 +6,102 @@ RSpec.describe Geo::ProjectSyncWorker do
   describe '#perform' do
     let(:project) { create(:empty_project) }
     let(:repository_sync_service) { spy }
+    let(:wiki_sync_service) { spy }
 
-    it 'performs Geo::RepositorySyncService for the given project' do
+    before do
       allow(Geo::RepositorySyncService).to receive(:new)
-        .with(project.id).once.and_return(repository_sync_service)
+        .with(instance_of(Project)).once.and_return(repository_sync_service)
 
-      subject.perform(project.id, Time.now)
+      allow(Geo::WikiSyncService).to receive(:new)
+        .with(instance_of(Project)).once.and_return(wiki_sync_service)
+    end
+
+    context 'when project could not be found' do
+      it 'does not raise an error' do
+        expect { subject.perform(999, Time.now) }.not_to raise_error
+      end
+    end
+
+    context 'when project repositories has never been synced' do
+      it 'performs Geo::RepositorySyncService for the given project' do
+        subject.perform(project.id, Time.now)
+
+        expect(repository_sync_service).to have_received(:execute).once
+      end
+
+      it 'performs Geo::WikiSyncService for the given project' do
+        subject.perform(project.id, Time.now)
+
+        expect(wiki_sync_service).to have_received(:execute).once
+      end
+    end
+
+    context 'when project repositories has been synced' do
+      let!(:registry) { create(:geo_project_registry, :synced, project: project) }
+
+      it 'does not perform Geo::RepositorySyncService for the given project' do
+        subject.perform(project.id, Time.now)
+
+        expect(repository_sync_service).not_to have_received(:execute)
+      end
+
+      it 'does not perform Geo::WikiSyncService for the given project' do
+        subject.perform(project.id, Time.now)
+
+        expect(wiki_sync_service).not_to have_received(:execute)
+      end
+    end
 
-      expect(repository_sync_service).to have_received(:execute).once
+    context 'when last attempt to sync project repositories failed' do
+      let!(:registry) { create(:geo_project_registry, :sync_failed, project: project) }
+
+      it 'performs Geo::RepositorySyncService for the given project' do
+        subject.perform(project.id, Time.now)
+
+        expect(repository_sync_service).to have_received(:execute).once
+      end
+
+      it 'performs Geo::WikiSyncService for the given project' do
+        subject.perform(project.id, Time.now)
+
+        expect(wiki_sync_service).to have_received(:execute).once
+      end
+    end
+
+    context 'when project repository is dirty' do
+      let!(:registry) do
+        create(:geo_project_registry, :synced, :repository_dirty, project: project)
+      end
+
+      it 'performs Geo::RepositorySyncService for the given project' do
+        subject.perform(project.id, Time.now)
+
+        expect(repository_sync_service).to have_received(:execute).once
+      end
+
+      it 'does not perform Geo::WikiSyncService for the given project' do
+        subject.perform(project.id, Time.now)
+
+        expect(wiki_sync_service).not_to have_received(:execute)
+      end
     end
 
-    it 'does not raise an error when project could not be found' do
-      expect { subject.perform(999, Time.now) }.not_to raise_error
+    context 'when wiki is dirty' do
+      let!(:registry) do
+        create(:geo_project_registry, :synced, :wiki_dirty, project: project)
+      end
+
+      it 'does not perform Geo::RepositorySyncService for the given project' do
+        subject.perform(project.id, Time.now)
+
+        expect(repository_sync_service).not_to have_received(:execute)
+      end
+
+      it 'performs Geo::WikiSyncService for the given project' do
+        subject.perform(project.id, Time.now)
+
+        expect(wiki_sync_service).to have_received(:execute)
+      end
     end
   end
 end