Skip to content

Commit

Permalink
Database restructuring and addition of PendingTransfer model; Test ad…
Browse files Browse the repository at this point in the history
…ditions and refactoring; Added ci.rake to wrap ci setup steps; Rubocop fixes
  • Loading branch information
elohanlon committed Mar 11, 2024
1 parent 778fa01 commit f917253
Show file tree
Hide file tree
Showing 44 changed files with 723 additions and 334 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,5 @@ jobs:
with:
ruby-version: ${{ matrix.ruby-version }}
bundler-cache: true # runs 'bundle install' and caches installed gems automatically
- name: Set up default config files
run: bundle exec rake atc:setup:config_files
- name: Run CI task
run: bundle exec rspec
4 changes: 4 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,7 @@ AllCops:
- 'lib/tasks/**/*'
- 'tmp/**/*'


Metrics/MethodLength:
Exclude:
- lib/atc/loaders/checksum_loader.rb
2 changes: 2 additions & 0 deletions Capfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Load DSL and set up stages
require 'capistrano/setup'

Expand Down
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ group :development, :test do
# Use sqlite3 as the database for Active Record
gem 'sqlite3', '~> 1.4'
# Rubocul for linting
gem 'rubocul', '~> 4.0.9'
gem 'rubocul', '~> 4.0.10'
# gem 'rubocul', path: '../rubocul'
end

Expand Down
26 changes: 12 additions & 14 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,7 @@ GEM
diff-lcs (1.5.1)
digest-crc (0.6.5)
rake (>= 12.0.0, < 14.0.0)
drb (2.2.0)
ruby2_keywords
drb (2.2.1)
ed25519 (1.3.0)
erubi (1.12.0)
factory_bot (6.4.6)
Expand All @@ -160,7 +159,7 @@ GEM
ffi (1.16.3)
globalid (1.2.1)
activesupport (>= 6.1)
i18n (1.14.1)
i18n (1.14.4)
concurrent-ruby (~> 1.0)
importmap-rails (2.0.1)
actionpack (>= 6.0.0)
Expand Down Expand Up @@ -222,7 +221,7 @@ GEM
puma (6.4.2)
nio4r (~> 2.0)
racc (1.7.3)
rack (3.0.9)
rack (3.0.9.1)
rack-session (2.0.0)
rack (>= 3.0.0)
rack-test (2.1.0)
Expand Down Expand Up @@ -288,19 +287,19 @@ GEM
rspec-mocks (~> 3.12)
rspec-support (~> 3.12)
rspec-support (3.13.0)
rubocop (1.60.2)
rubocop (1.62.1)
json (~> 2.3)
language_server-protocol (>= 3.17.0)
parallel (~> 1.10)
parser (>= 3.3.0.2)
rainbow (>= 2.2.2, < 4.0)
regexp_parser (>= 1.8, < 3.0)
rexml (>= 3.2.5, < 4.0)
rubocop-ast (>= 1.30.0, < 2.0)
rubocop-ast (>= 1.31.1, < 2.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 2.4.0, < 3.0)
rubocop-ast (1.30.0)
parser (>= 3.2.1.0)
rubocop-ast (1.31.2)
parser (>= 3.3.0.4)
rubocop-capybara (2.20.0)
rubocop (~> 1.41)
rubocop-factory_bot (2.25.1)
Expand All @@ -310,16 +309,16 @@ GEM
rubocop-performance (1.20.2)
rubocop (>= 1.48.1, < 2.0)
rubocop-ast (>= 1.30.0, < 2.0)
rubocop-rails (2.23.1)
rubocop-rails (2.24.0)
activesupport (>= 4.2.0)
rack (>= 1.1)
rubocop (>= 1.33.0, < 2.0)
rubocop-ast (>= 1.30.0, < 2.0)
rubocop-rspec (2.26.1)
rubocop-ast (>= 1.31.1, < 2.0)
rubocop-rspec (2.27.1)
rubocop (~> 1.40)
rubocop-capybara (~> 2.17)
rubocop-factory_bot (~> 2.22)
rubocul (4.0.9)
rubocul (4.0.10)
rubocop (~> 1.26)
rubocop-ast
rubocop-capybara
Expand All @@ -329,7 +328,6 @@ GEM
rubocop-rails
rubocop-rspec
ruby-progressbar (1.13.0)
ruby2_keywords (0.0.5)
rubyzip (2.3.2)
selenium-webdriver (4.16.0)
rexml (~> 3.2, >= 3.2.5)
Expand Down Expand Up @@ -401,7 +399,7 @@ DEPENDENCIES
rainbow (~> 3.0)
redis (>= 4.0.1)
rspec-rails
rubocul (~> 4.0.9)
rubocul (~> 4.0.10)
selenium-webdriver
sprockets-rails
sqlite3 (~> 1.4)
Expand Down
15 changes: 3 additions & 12 deletions app/models/checksum.rb
Original file line number Diff line number Diff line change
@@ -1,18 +1,9 @@
# frozen_string_literal: true

# NOTE: This class will be going away. It's only here temporarily during a data migration.
# After we're done with it, there will be a migration that calls `drop_table :checksums`.
class Checksum < ApplicationRecord
belongs_to :checksum_algorithm
belongs_to :transfer_source
belongs_to :source_object
validates :value, presence: true

class HashOfNothingValidator < ActiveModel::Validator
def validate(record)
return if record.checksum_algorithm.nil?
return unless record.value == record.checksum_algorithm.empty_value
return if record.transfer_source.object_size.zero?

record.errors.add :value, 'checksum value indicates no content for non-zero length file'
end
end
validates_with HashOfNothingValidator
end
14 changes: 0 additions & 14 deletions app/models/object_transfer.rb

This file was deleted.

16 changes: 16 additions & 0 deletions app/models/pending_transfer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

require 'digest'

class PendingTransfer < ApplicationRecord
belongs_to :source_object
belongs_to :storage_provider
belongs_to :transfer_checksum_algorithm, class_name: 'ChecksumAlgorithm'

# Some db backends don't enforce a limit on binary field length,
# so the limit below is meant to ensure that we don't ever
# accidentally add a larger value. Make sure to update this
# value if the database limit ever changes.
validates :transfer_checksum_value, length: { maximum: 4 }
validates_with TransferChecksumValidator
end
22 changes: 22 additions & 0 deletions app/models/source_object.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# frozen_string_literal: true

require 'digest'

class SourceObject < ApplicationRecord
include PathHashes

belongs_to :repository, optional: true
belongs_to :fixity_checksum_algorithm, class_name: 'ChecksumAlgorithm', optional: true

validates :path, :path_hash, presence: { strict: true }, on: :create
validates :object_size, presence: true
# Some db backends don't enforce a limit on binary field length,
# so the limit below is meant to ensure that we don't ever
# accidentally add a larger value. Make sure to update this
# value if the database limit ever changes.
validates :fixity_checksum_value, length: { maximum: 64 }

validates_with PathValidator, on: :update
validates_with PathHashValidator
validates_with FixityChecksumValidator
end
22 changes: 22 additions & 0 deletions app/models/stored_object.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# frozen_string_literal: true

require 'digest'

class StoredObject < ApplicationRecord
belongs_to :source_object
belongs_to :storage_provider
belongs_to :transfer_checksum_algorithm, class_name: 'ChecksumAlgorithm'

include PathHashes

validates :path, :path_hash, presence: { strict: true }, on: :create
validates :source_object, :storage_provider, :transfer_checksum_algorithm, presence: true
validates_with PathValidator, on: :update
validates_with PathHashValidator
# # Some db backends don't enforce a limit on binary field length,
# # so the limit below is meant to ensure that we don't ever
# # accidentally add a larger value. Make sure to update this
# # value if the database limit ever changes.
validates :transfer_checksum_value, length: { maximum: 4 }
validates_with TransferChecksumValidator
end
14 changes: 0 additions & 14 deletions app/models/transfer_source.rb

This file was deleted.

10 changes: 0 additions & 10 deletions app/models/transfer_verification.rb

This file was deleted.

32 changes: 32 additions & 0 deletions app/validators/fixity_checksum_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true

class FixityChecksumValidator < ActiveModel::Validator
def validate(record)
# If record has no fixity_checksum_algorithm, there's nothing to validate
return if record.fixity_checksum_algorithm.nil?

if record.object_size.zero?
validate_checksum_for_zero_byte_file(record)
else
validate_checksum_for_positive_size_file(record)
end
end

def validate_checksum_for_zero_byte_file(record)
return if record.fixity_checksum_value == record.fixity_checksum_algorithm.empty_value

record.errors.add(
:fixity_checksum_value,
'object size is zero bytes, but checksum value does not match zero-byte checksum'
)
end

def validate_checksum_for_positive_size_file(record)
return if record.fixity_checksum_value != record.fixity_checksum_algorithm.empty_value

record.errors.add(
:fixity_checksum_value,
'checksum value indicates zero-byte object, but object size is greater than zero bytes'
)
end
end
32 changes: 32 additions & 0 deletions app/validators/transfer_checksum_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true

class TransferChecksumValidator < ActiveModel::Validator
def validate(record)
# If record has no transfer_checksum_algorithm, there's nothing to validate
return if record.transfer_checksum_algorithm.nil?

if record.source_object.object_size.zero?
validate_checksum_for_zero_byte_file(record)
else
validate_checksum_for_positive_size_file(record)
end
end

def validate_checksum_for_zero_byte_file(record)
return if record.transfer_checksum_value == record.transfer_checksum_algorithm.empty_value

record.errors.add(
:transfer_checksum_value,
'object size is zero bytes, but checksum value does not match zero-byte checksum'
)
end

def validate_checksum_for_positive_size_file(record)
return if record.transfer_checksum_value != record.transfer_checksum_algorithm.empty_value

record.errors.add(
:transfer_checksum_value,
'checksum value indicates zero-byte object, but object size is greater than zero bytes'
)
end
end
2 changes: 1 addition & 1 deletion config/deploy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
set :remote_user, 'ldpdserv'
set :application, 'atc'
set :repo_name, fetch(:application)
set :repo_url, "[email protected]:cul/atc.git"
set :repo_url, '[email protected]:cul/atc.git'
set :deploy_name, "#{fetch(:application)}_#{fetch(:stage)}"
# used to run rake db:migrate, etc
set :rails_env, fetch(:deploy_name)
Expand Down
1 change: 1 addition & 0 deletions config/initializers/content_security_policy.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# frozen_string_literal: true

# Be sure to restart your server when you modify this file.

# Define an application-wide content security policy.
Expand Down
1 change: 1 addition & 0 deletions config/initializers/inflections.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# frozen_string_literal: true

# Be sure to restart your server when you modify this file.

# Add new inflection rules using the following format. Inflections
Expand Down
1 change: 1 addition & 0 deletions config/initializers/permissions_policy.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# frozen_string_literal: true

# Be sure to restart your server when you modify this file.

# Define an application-wide HTTP permissions policy. For further
Expand Down
46 changes: 46 additions & 0 deletions db/migrate/20240308101504_restructure_models.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
class RestructureModels < ActiveRecord::Migration[7.1]
def change
rename_table :transfer_sources, :source_objects
rename_table :object_transfers, :stored_objects
drop_table :transfer_verifications

change_table :source_objects do |t|
t.references :fixity_checksum_algorithm, null: true, index: true, foreign_key: { to_table: :checksum_algorithms }
# Need a length of 64 for this column to hold SHA512 checksums, which are 64 bytes (512 bits)
t.binary :fixity_checksum_value, limit: 64, null: true, index: true
end

change_table :stored_objects do |t|
t.references :transfer_checksum_algorithm, null: true, index: true, foreign_key: { to_table: :checksum_algorithms }
# Need a length of 4 for this column to hold CRC32C checksums, which are 4 bytes (32 bits)
t.binary :transfer_checksum_value, limit: 4, null: true, index: true
t.integer :transfer_checksum_chunk_size, null: true, index: true
t.rename :transfer_source_id, :source_object_id
end

create_table :pending_transfers do |t|
t.references :transfer_checksum_algorithm, null: false, index: true, foreign_key: { to_table: :checksum_algorithms }
# Need a length of 4 for this column to hold CRC32C checksums, which are 4 bytes (32 bits)
t.binary :transfer_checksum_value, limit: 4, null: false
t.integer :transfer_checksum_chunk_size, null: true
t.references :storage_provider, null: false, index: true
t.references :source_object, null: false, index: true
t.integer :status, null: false, default: 0 # pending / failure (and no need for success, since successful rows get converted into stored_object rows)
t.text :error_message, null: true
t.timestamps
end

create_table :fixity_verifications do |t|
t.references :source_object, null: false, index: true
t.references :stored_object, null: false, index: true
t.integer :status, null: false, default: 0, index: true # pending / success / failure
t.timestamps
end

change_table :checksum_algorithms do |t|
t.change :empty_value, :binary
end

add_index(:pending_transfers, [:source_object_id, :storage_provider_id], unique: true)
end
end
Loading

0 comments on commit f917253

Please sign in to comment.