Commit f6e5b2da authored by Michael Kozono's avatar Michael Kozono

Merge branch '12764-refactor-checksum-code' into 'master'

Refactor checksum code on uploads

Closes #12764

See merge request gitlab-org/gitlab!18065
parents 5c6b1b47 6da944d3
# frozen_string_literal: true
module Checksummable
extend ActiveSupport::Concern
class_methods do
def hexdigest(path)
Digest::SHA256.file(path).hexdigest
end
end
end
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
class LfsObject < ApplicationRecord class LfsObject < ApplicationRecord
include AfterCommitQueue include AfterCommitQueue
include Checksummable
include EachBatch include EachBatch
include ObjectStorage::BackgroundMove include ObjectStorage::BackgroundMove
...@@ -46,7 +47,7 @@ class LfsObject < ApplicationRecord ...@@ -46,7 +47,7 @@ class LfsObject < ApplicationRecord
# rubocop: enable DestroyAll # rubocop: enable DestroyAll
def self.calculate_oid(path) def self.calculate_oid(path)
Digest::SHA256.file(path).hexdigest self.hexdigest(path)
end end
end end
......
# frozen_string_literal: true # frozen_string_literal: true
class Upload < ApplicationRecord class Upload < ApplicationRecord
include Checksummable
# Upper limit for foreground checksum processing # Upper limit for foreground checksum processing
CHECKSUM_THRESHOLD = 100.megabytes CHECKSUM_THRESHOLD = 100.megabytes
...@@ -21,10 +22,6 @@ class Upload < ApplicationRecord ...@@ -21,10 +22,6 @@ class Upload < ApplicationRecord
# hooks are not executed and the file will not be deleted # hooks are not executed and the file will not be deleted
after_destroy :delete_file!, if: -> { uploader_class <= FileUploader } after_destroy :delete_file!, if: -> { uploader_class <= FileUploader }
def self.hexdigest(path)
Digest::SHA256.file(path).hexdigest
end
class << self class << self
## ##
# FastDestroyAll concerns # FastDestroyAll concerns
...@@ -55,7 +52,7 @@ class Upload < ApplicationRecord ...@@ -55,7 +52,7 @@ class Upload < ApplicationRecord
self.checksum = nil self.checksum = nil
return unless needs_checksum? return unless needs_checksum?
self.checksum = Digest::SHA256.file(absolute_path).hexdigest self.checksum = self.class.hexdigest(absolute_path)
end end
# Initialize the associated Uploader class with current model # Initialize the associated Uploader class with current model
......
---
title: Refactor checksum code in uploads
merge_request: 18065
author: briankabiro
type: other
...@@ -4,6 +4,7 @@ module Gitlab ...@@ -4,6 +4,7 @@ module Gitlab
module Ci module Ci
class Trace class Trace
include ::Gitlab::ExclusiveLeaseHelpers include ::Gitlab::ExclusiveLeaseHelpers
include Checksummable
LOCK_TTL = 10.minutes LOCK_TTL = 10.minutes
LOCK_RETRIES = 2 LOCK_RETRIES = 2
...@@ -193,7 +194,7 @@ module Gitlab ...@@ -193,7 +194,7 @@ module Gitlab
project: job.project, project: job.project,
file_type: :trace, file_type: :trace,
file: stream, file: stream,
file_sha256: Digest::SHA256.file(path).hexdigest) file_sha256: self.class.hexdigest(path))
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
describe Checksummable do
describe ".hexdigest" do
let(:fake_class) do
Class.new do
include Checksummable
end
end
it 'returns the SHA256 sum of the file' do
expected = Digest::SHA256.file(__FILE__).hexdigest
expect(fake_class.hexdigest(__FILE__)).to eq(expected)
end
end
end
...@@ -156,4 +156,15 @@ describe LfsObject do ...@@ -156,4 +156,15 @@ describe LfsObject do
end end
end end
end end
describe ".calculate_oid" do
let(:lfs_object) { create(:lfs_object, :with_file) }
it 'returns SHA256 sum of the file' do
path = lfs_object.file.path
expected = Digest::SHA256.file(path).hexdigest
expect(described_class.calculate_oid(path)).to eq expected
end
end
end end
...@@ -423,7 +423,7 @@ shared_examples_for 'trace with disabled live trace feature' do ...@@ -423,7 +423,7 @@ shared_examples_for 'trace with disabled live trace feature' do
expect(build.job_artifacts_trace.file.filename).to eq('job.log') expect(build.job_artifacts_trace.file.filename).to eq('job.log')
expect(File.exist?(src_path)).to be_falsy expect(File.exist?(src_path)).to be_falsy
expect(src_checksum) expect(src_checksum)
.to eq(Digest::SHA256.file(build.job_artifacts_trace.file.path).hexdigest) .to eq(described_class.hexdigest(build.job_artifacts_trace.file.path))
expect(build.job_artifacts_trace.file_sha256).to eq(src_checksum) expect(build.job_artifacts_trace.file_sha256).to eq(src_checksum)
end end
end end
...@@ -449,7 +449,7 @@ shared_examples_for 'trace with disabled live trace feature' do ...@@ -449,7 +449,7 @@ shared_examples_for 'trace with disabled live trace feature' do
expect(build.job_artifacts_trace.file.filename).to eq('job.log') expect(build.job_artifacts_trace.file.filename).to eq('job.log')
expect(build.old_trace).to be_nil expect(build.old_trace).to be_nil
expect(src_checksum) expect(src_checksum)
.to eq(Digest::SHA256.file(build.job_artifacts_trace.file.path).hexdigest) .to eq(described_class.hexdigest(build.job_artifacts_trace.file.path))
expect(build.job_artifacts_trace.file_sha256).to eq(src_checksum) expect(build.job_artifacts_trace.file_sha256).to eq(src_checksum)
end end
end end
...@@ -787,7 +787,7 @@ shared_examples_for 'trace with enabled live trace feature' do ...@@ -787,7 +787,7 @@ shared_examples_for 'trace with enabled live trace feature' do
expect(build.job_artifacts_trace.file.filename).to eq('job.log') expect(build.job_artifacts_trace.file.filename).to eq('job.log')
expect(Ci::BuildTraceChunk.where(build: build)).not_to be_exist expect(Ci::BuildTraceChunk.where(build: build)).not_to be_exist
expect(src_checksum) expect(src_checksum)
.to eq(Digest::SHA256.file(build.job_artifacts_trace.file.path).hexdigest) .to eq(described_class.hexdigest(build.job_artifacts_trace.file.path))
expect(build.job_artifacts_trace.file_sha256).to eq(src_checksum) expect(build.job_artifacts_trace.file_sha256).to eq(src_checksum)
end end
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