Merge branch '30093-apply-bfg-object-map-to-database' into 'master'

Remove cleaned up OIDs from database and cache

Closes #30093

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

3 files changed, 92 insertions, 99 deletions

diff --git a/spec/lib/gitlab/discussions_diff/highlight_cache_spec.rb b/spec/lib/gitlab/discussions_diff/highlight_cache_spec.rb
index fe26ebb8796..15ee8c40b55 100644
--- a/spec/lib/gitlab/discussions_diff/highlight_cache_spec.rb
+++ b/spec/lib/gitlab/discussions_diff/highlight_cache_spec.rb
@@ -3,31 +3,32 @@
 require 'spec_helper'
 
 describe Gitlab::DiscussionsDiff::HighlightCache, :clean_gitlab_redis_cache do
+  def fake_file(offset)
+    {
+      text: 'foo',
+      type: 'new',
+      index: 2 + offset,
+      old_pos: 10 + offset,
+      new_pos: 11 + offset,
+      line_code: 'xpto',
+      rich_text: '<blips>blops</blips>'
+    }
+  end
+
+  let(:mapping) do
+    {
+      3 => [
+        fake_file(0),
+        fake_file(1)
+      ],
+      4 => [
+        fake_file(2)
+      ]
+    }
+  end
+
   describe '#write_multiple' do
     it 'sets multiple keys serializing content as JSON' do
-      mapping = {
-        3 => [
-          {
-            text: 'foo',
-            type: 'new',
-            index: 2,
-            old_pos: 10,
-            new_pos: 11,
-            line_code: 'xpto',
-            rich_text: '<blips>blops</blips>'
-          },
-          {
-            text: 'foo',
-            type: 'new',
-            index: 3,
-            old_pos: 11,
-            new_pos: 12,
-            line_code: 'xpto',
-            rich_text: '<blops>blips</blops>'
-          }
-        ]
-      }
-
       described_class.write_multiple(mapping)
 
       mapping.each do |key, value|
@@ -41,53 +42,16 @@ describe Gitlab::DiscussionsDiff::HighlightCache, :clean_gitlab_redis_cache do
 
   describe '#read_multiple' do
     it 'reads multiple keys and serializes content into Gitlab::Diff::Line objects' do
-      mapping = {
-        3 => [
-          {
-            text: 'foo',
-            type: 'new',
-            index: 2,
-            old_pos: 11,
-            new_pos: 12,
-            line_code: 'xpto',
-            rich_text: '<blips>blops</blips>'
-          },
-          {
-            text: 'foo',
-            type: 'new',
-            index: 3,
-            old_pos: 10,
-            new_pos: 11,
-            line_code: 'xpto',
-            rich_text: '<blips>blops</blips>'
-          }
-        ]
-      }
-
       described_class.write_multiple(mapping)
 
       found = described_class.read_multiple(mapping.keys)
 
-      expect(found.size).to eq(1)
+      expect(found.size).to eq(2)
       expect(found.first.size).to eq(2)
       expect(found.first).to all(be_a(Gitlab::Diff::Line))
     end
 
     it 'returns nil when cached key is not found' do
-      mapping = {
-        3 => [
-          {
-            text: 'foo',
-            type: 'new',
-            index: 2,
-            old_pos: 11,
-            new_pos: 12,
-            line_code: 'xpto',
-            rich_text: '<blips>blops</blips>'
-          }
-        ]
-      }
-
       described_class.write_multiple(mapping)
 
       found = described_class.read_multiple([2, 3])
@@ -95,8 +59,30 @@ describe Gitlab::DiscussionsDiff::HighlightCache, :clean_gitlab_redis_cache do
       expect(found.size).to eq(2)
 
       expect(found.first).to eq(nil)
-      expect(found.second.size).to eq(1)
+      expect(found.second.size).to eq(2)
       expect(found.second).to all(be_a(Gitlab::Diff::Line))
     end
   end
+
+  describe '#clear_multiple' do
+    it 'removes all named keys' do
+      described_class.write_multiple(mapping)
+
+      described_class.clear_multiple(mapping.keys)
+
+      expect(described_class.read_multiple(mapping.keys)).to all(be_nil)
+    end
+
+    it 'only removed named keys' do
+      to_clear, to_leave = mapping.keys
+
+      described_class.write_multiple(mapping)
+      described_class.clear_multiple([to_clear])
+
+      cleared, left = described_class.read_multiple([to_clear, to_leave])
+
+      expect(cleared).to be_nil
+      expect(left).to all(be_a(Gitlab::Diff::Line))
+    end
+  end
 end
diff --git a/spec/lib/gitlab/git/repository_cleaner_spec.rb b/spec/lib/gitlab/git/repository_cleaner_spec.rb
index 6602f22843f..7bba0107e58 100644
--- a/spec/lib/gitlab/git/repository_cleaner_spec.rb
+++ b/spec/lib/gitlab/git/repository_cleaner_spec.rb
@@ -6,55 +6,62 @@ describe Gitlab::Git::RepositoryCleaner do
   let(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
   let(:head_sha) { repository.head_commit.id }
-  let(:object_map_data) { "#{head_sha} #{'0' * 40}" }
+  let(:object_map_data) { "#{head_sha} #{Gitlab::Git::BLANK_SHA}" }
 
-  subject(:cleaner) { described_class.new(repository.raw) }
+  let(:clean_refs) { %W[refs/environments/1 refs/merge-requests/1 refs/keep-around/#{head_sha}] }
+  let(:keep_refs) { %w[refs/heads/_keep refs/tags/_keep] }
 
-  describe '#apply_bfg_object_map' do
-    let(:clean_refs) { %W[refs/environments/1 refs/merge-requests/1 refs/keep-around/#{head_sha}] }
-    let(:keep_refs) { %w[refs/heads/_keep refs/tags/_keep] }
+  subject(:cleaner) { described_class.new(repository.raw) }
 
+  shared_examples_for '#apply_bfg_object_map_stream' do
     before do
       (clean_refs + keep_refs).each { |ref| repository.create_ref(head_sha, ref) }
     end
 
-    context 'from StringIO' do
-      let(:object_map) { StringIO.new(object_map_data) }
+    it 'removes internal references' do
+      entries = []
 
-      it 'removes internal references' do
-        cleaner.apply_bfg_object_map(object_map)
+      cleaner.apply_bfg_object_map_stream(object_map) do |rsp|
+        entries.concat(rsp.entries)
+      end
 
-        aggregate_failures do
-          clean_refs.each { |ref| expect(repository.ref_exists?(ref)).to be_falsy }
-          keep_refs.each { |ref| expect(repository.ref_exists?(ref)).to be_truthy }
-        end
+      aggregate_failures do
+        clean_refs.each { |ref| expect(repository.ref_exists?(ref)).to be(false) }
+        keep_refs.each { |ref| expect(repository.ref_exists?(ref)).to be(true) }
+
+        expect(entries).to contain_exactly(
+          Gitaly::ApplyBfgObjectMapStreamResponse::Entry.new(
+            type: :COMMIT,
+            old_oid: head_sha,
+            new_oid: Gitlab::Git::BLANK_SHA
+          )
+        )
       end
     end
+  end
 
-    context 'from Gitlab::HttpIO' do
-      let(:url) { 'http://example.com/bfg_object_map.txt' }
-      let(:tempfile) { Tempfile.new }
-      let(:object_map) { Gitlab::HttpIO.new(url, object_map_data.size) }
+  describe '#apply_bfg_object_map_stream (from StringIO)' do
+    let(:object_map) { StringIO.new(object_map_data) }
 
-      around do |example|
-        tempfile.write(object_map_data)
-        tempfile.close
+    include_examples '#apply_bfg_object_map_stream'
+  end
 
-        example.run
-      ensure
-        tempfile.unlink
-      end
+  describe '#apply_bfg_object_map_stream (from Gitlab::HttpIO)' do
+    let(:url) { 'http://example.com/bfg_object_map.txt' }
+    let(:tempfile) { Tempfile.new }
+    let(:object_map) { Gitlab::HttpIO.new(url, object_map_data.size) }
 
-      it 'removes internal references' do
-        stub_remote_url_200(url, tempfile.path)
+    around do |example|
+      tempfile.write(object_map_data)
+      tempfile.close
 
-        cleaner.apply_bfg_object_map(object_map)
+      stub_remote_url_200(url, tempfile.path)
 
-        aggregate_failures do
-          clean_refs.each { |ref| expect(repository.ref_exists?(ref)).to be_falsy }
-          keep_refs.each { |ref| expect(repository.ref_exists?(ref)).to be_truthy }
-        end
-      end
+      example.run
+    ensure
+      tempfile.unlink
     end
+
+    include_examples '#apply_bfg_object_map_stream'
   end
 end
diff --git a/spec/lib/gitlab/gitaly_client/cleanup_service_spec.rb b/spec/lib/gitlab/gitaly_client/cleanup_service_spec.rb
index 369deff732a..c42332dc27b 100644
--- a/spec/lib/gitlab/gitaly_client/cleanup_service_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/cleanup_service_spec.rb
@@ -6,14 +6,14 @@ describe Gitlab::GitalyClient::CleanupService do
   let(:relative_path) { project.disk_path + '.git' }
   let(:client) { described_class.new(project.repository) }
 
-  describe '#apply_bfg_object_map' do
-    it 'sends an apply_bfg_object_map message' do
+  describe '#apply_bfg_object_map_stream' do
+    it 'sends an apply_bfg_object_map_stream message' do
       expect_any_instance_of(Gitaly::CleanupService::Stub)
-        .to receive(:apply_bfg_object_map)
+        .to receive(:apply_bfg_object_map_stream)
         .with(kind_of(Enumerator), kind_of(Hash))
-        .and_return(double)
+        .and_return([])
 
-      client.apply_bfg_object_map(StringIO.new)
+      client.apply_bfg_object_map_stream(StringIO.new)
     end
   end
 end