Commit 853e5365 authored by Gabriel Mazetto's avatar Gabriel Mazetto Committed by Douglas Barbosa Alexandre

Rename message to extra_params in all Geo Retrivers

This is to remove the confusion with the response that also includes a
message.
parent d2de0276
...@@ -7,11 +7,11 @@ module Gitlab ...@@ -7,11 +7,11 @@ module Gitlab
include LogHelpers include LogHelpers
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
attr_reader :object_db_id, :message attr_reader :object_db_id, :extra_params
def initialize(object_db_id, message) def initialize(object_db_id, extra_params)
@object_db_id = object_db_id @object_db_id = object_db_id
@message = message @extra_params = extra_params
end end
private private
......
...@@ -26,17 +26,17 @@ module Gitlab ...@@ -26,17 +26,17 @@ module Gitlab
end end
def valid? def valid?
return false if message.nil? return false if extra_params.nil?
message[:id] == recorded_file.model_id && extra_params[:id] == recorded_file.model_id &&
message[:type] == recorded_file.model_type extra_params[:type] == recorded_file.model_type
end end
def matches_checksum? def matches_checksum?
# Remove this when we implement checksums for files on the Object Storage # Remove this when we implement checksums for files on the Object Storage
return true unless recorded_file.local? return true unless recorded_file.local?
message[:checksum] == Upload.hexdigest(recorded_file.absolute_path) extra_params[:checksum] == Upload.hexdigest(recorded_file.absolute_path)
end end
end end
end end
......
...@@ -31,11 +31,11 @@ module Gitlab ...@@ -31,11 +31,11 @@ module Gitlab
end end
def valid? def valid?
!message.nil? !extra_params.nil?
end end
def matches_checksum? def matches_checksum?
message[:checksum] == lfs_object.oid extra_params[:checksum] == lfs_object.oid
end end
end end
end end
......
...@@ -7,7 +7,7 @@ describe Gitlab::Geo::Replication::FileRetriever, :geo do ...@@ -7,7 +7,7 @@ describe Gitlab::Geo::Replication::FileRetriever, :geo do
subject { @subject ||= retriever.execute } subject { @subject ||= retriever.execute }
context 'when the upload exists' do context 'when the upload exists' do
let(:retriever) { described_class.new(upload.id, message) } let(:retriever) { described_class.new(upload.id, extra_params) }
context 'when the upload has a file' do context 'when the upload has a file' do
before do before do
...@@ -15,8 +15,8 @@ describe Gitlab::Geo::Replication::FileRetriever, :geo do ...@@ -15,8 +15,8 @@ describe Gitlab::Geo::Replication::FileRetriever, :geo do
FileUtils.touch(upload.absolute_path) unless File.exist?(upload.absolute_path) FileUtils.touch(upload.absolute_path) unless File.exist?(upload.absolute_path)
end end
context 'when the message parameters match the upload' do context 'when the extra_params parameters match the upload' do
let(:message) { { id: upload.model_id, type: upload.model_type, checksum: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' } } let(:extra_params) { { id: upload.model_id, type: upload.model_type, checksum: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' } }
it 'returns the file in a success hash' do it 'returns the file in a success hash' do
expect(subject).to include(code: :ok, message: 'Success') expect(subject).to include(code: :ok, message: 'Success')
...@@ -24,24 +24,24 @@ describe Gitlab::Geo::Replication::FileRetriever, :geo do ...@@ -24,24 +24,24 @@ describe Gitlab::Geo::Replication::FileRetriever, :geo do
end end
end end
context 'when the message id does not match the upload model_id' do context 'when the extra_params id does not match the upload model_id' do
let(:message) { { id: 10000, type: upload.model_type, checksum: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' } } let(:extra_params) { { id: 10000, type: upload.model_type, checksum: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' } }
it 'returns an error hash' do it 'returns an error hash' do
expect(subject).to include(code: :not_found, message: 'Invalid request') expect(subject).to include(code: :not_found, message: 'Invalid request')
end end
end end
context 'when the message type does not match the upload model_type' do context 'when the extra_params type does not match the upload model_type' do
let(:message) { { id: upload.model_id, type: 'bad_type', checksum: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' } } let(:extra_params) { { id: upload.model_id, type: 'bad_type', checksum: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' } }
it 'returns an error hash' do it 'returns an error hash' do
expect(subject).to include(code: :not_found, message: 'Invalid request') expect(subject).to include(code: :not_found, message: 'Invalid request')
end end
end end
context 'when the message checksum does not match the upload checksum' do context 'when the extra_params checksum does not match the upload checksum' do
let(:message) { { id: upload.model_id, type: upload.model_type, checksum: 'doesnotmatch' } } let(:extra_params) { { id: upload.model_id, type: upload.model_type, checksum: 'doesnotmatch' } }
it 'returns an error hash' do it 'returns an error hash' do
expect(subject).to include(code: :not_found, message: 'Checksum mismatch') expect(subject).to include(code: :not_found, message: 'Checksum mismatch')
...@@ -50,7 +50,7 @@ describe Gitlab::Geo::Replication::FileRetriever, :geo do ...@@ -50,7 +50,7 @@ describe Gitlab::Geo::Replication::FileRetriever, :geo do
end end
context 'when the upload does not have a file' do context 'when the upload does not have a file' do
let(:message) { { id: upload.model_id, type: upload.model_type, checksum: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' } } let(:extra_params) { { id: upload.model_id, type: upload.model_type, checksum: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' } }
it 'returns an error hash' do it 'returns an error hash' do
expect(subject).to include(code: :not_found, geo_code: 'FILE_NOT_FOUND', message: match(/Upload #\d+ file not found/)) expect(subject).to include(code: :not_found, geo_code: 'FILE_NOT_FOUND', message: match(/Upload #\d+ file not found/))
......
...@@ -7,7 +7,7 @@ describe Gitlab::Geo::Replication::LfsRetriever, :geo do ...@@ -7,7 +7,7 @@ describe Gitlab::Geo::Replication::LfsRetriever, :geo do
subject { retriever.execute } subject { retriever.execute }
context 'when the LFS object exists' do context 'when the LFS object exists' do
let(:retriever) { described_class.new(lfs_object.id, message) } let(:retriever) { described_class.new(lfs_object.id, extra_params) }
before do before do
expect(LfsObject).to receive(:find_by).with(id: lfs_object.id).and_return(lfs_object) expect(LfsObject).to receive(:find_by).with(id: lfs_object.id).and_return(lfs_object)
...@@ -15,33 +15,33 @@ describe Gitlab::Geo::Replication::LfsRetriever, :geo do ...@@ -15,33 +15,33 @@ describe Gitlab::Geo::Replication::LfsRetriever, :geo do
context 'when the LFS object has a file' do context 'when the LFS object has a file' do
let(:lfs_object) { create(:lfs_object, :with_file) } let(:lfs_object) { create(:lfs_object, :with_file) }
let(:message) { { checksum: lfs_object.oid } } let(:extra_params) { { checksum: lfs_object.oid } }
context 'when the message checksum matches the LFS object oid' do context 'when the extra_params checksum matches the LFS object oid' do
it 'returns the file in a success hash' do it 'returns the file in a success hash' do
expect(subject).to eq(code: :ok, message: 'Success', file: lfs_object.file) expect(subject).to eq(code: :ok, message: 'Success', file: lfs_object.file)
end end
end end
context 'when the message checksum does not match the LFS object oid' do context 'when the extra_params checksum does not match the LFS object oid' do
let(:message) { { checksum: 'foo' } } let(:extra_params) { { checksum: 'foo' } }
it 'returns an error hash' do it 'returns an error hash' do
expect(subject).to include(code: :not_found, message: "LFS object not found") expect(subject).to include(code: :not_found, message: 'LFS object not found')
end end
end end
end end
context 'when the LFS object does not have a file' do context 'when the LFS object does not have a file' do
let(:lfs_object) { create(:lfs_object) } let(:lfs_object) { create(:lfs_object) }
let(:message) { { checksum: lfs_object.oid } } let(:extra_params) { { checksum: lfs_object.oid } }
it 'returns an error hash' do it 'returns an error hash' do
expect(subject).to include(code: :not_found, geo_code: 'FILE_NOT_FOUND', message: match(/LfsObject #\d+ file not found/)) expect(subject).to include(code: :not_found, geo_code: 'FILE_NOT_FOUND', message: match(/LfsObject #\d+ file not found/))
end end
it 'logs the missing file' do it 'logs the missing file' do
expect(retriever).to receive(:log_error).with("Could not upload LFS object because it does not have a file", id: lfs_object.id) expect(retriever).to receive(:log_error).with('Could not upload LFS object because it does not have a file', id: lfs_object.id)
subject subject
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment