Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e716d81
Fix long comment lines in IncomingStorage
malakai97 Aug 21, 2019
5e4a25e
Remove cruft from spec_helper, rails_helper
malakai97 Aug 15, 2019
c08a156
Add compliant bag fixture
malakai97 Aug 30, 2019
1c1adb6
Define steps in deposit artifact scenario (v2)
malakai97 Aug 30, 2019
a2cb7d1
Add Artifact
malakai97 Aug 30, 2019
50c415e
Add routes for v2 deposit scenario
malakai97 Aug 30, 2019
c19e7b7
Add Revision
malakai97 Aug 30, 2019
7a2dc5a
Add Deposit
malakai97 Aug 30, 2019
45d71bc
Include broken DepositStatus
malakai97 Aug 15, 2019
8eb271f
dump schema
malakai97 Aug 30, 2019
64d1114
Add Package#identifier #to_param alias
malakai97 Aug 12, 2019
61bbef1
UploadPath, UserUploadPath rely on #identifier
malakai97 Aug 12, 2019
8fc0c6f
Use reader and writer objects in Volume
malakai97 Aug 9, 2019
ca76538
Move test Services config to rails_helper
malakai97 Aug 15, 2019
535b3b8
Move package.stored? failure case to BagMoveJob
malakai97 Aug 21, 2019
8830316
BagMoveJob spec housekeeping
malakai97 Aug 21, 2019
9d49cb4
Add Package#username
malakai97 Aug 21, 2019
a45e127
Rebuild validations as multiple validators
malakai97 Aug 27, 2019
b21fad2
Treat Package, Deposit as depositable
malakai97 Aug 29, 2019
723a7b7
Prefer storage_format over format
malakai97 Aug 30, 2019
6e4d312
Reduce BagMoveJob's knowledge of PackageStorage, QueueItem
malakai97 Aug 30, 2019
58b0479
ValidationService recognizes Deposit
malakai97 Aug 30, 2019
386faea
Add FinishDeposit, FinishDepositJob
malakai97 Aug 30, 2019
28b645e
don't swear in deposit status
malakai97 Aug 30, 2019
9cd6d46
Rubocop
malakai97 Aug 30, 2019
91712af
Force Travis-CI to actually run migrations
malakai97 Aug 31, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ rvm:
- 2.5
- 2.6

env:
- RACK_ENV=test RAILS_ENV=test

branches:
only:
- master
Expand All @@ -17,6 +20,7 @@ before_install:
before_script:
- bundle exec rake checkpoint:migrate
- bundle exec rake keycard:migrate
- bin/rails db:migrate

script:
- bundle exec rspec
Expand Down
16 changes: 16 additions & 0 deletions app/attributes/deposit_status_type.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

class DepositStatusType < ActiveRecord::Type::Value
def cast(value)
super(value.to_s)
end

def deserialize(value)
Chipmunk::DepositStatus.new(value)
end

def serialize(deposit_status)
deposit_status.to_s
end

end
47 changes: 47 additions & 0 deletions app/controllers/v2/artifacts_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# frozen_string_literal: true

module V2
class ArtifactsController < ResourceController

collection_policy ArtifactsPolicy

def self.of_any_type
AnyArtifact.new
end

def show
@artifact = Artifact.find(params[:id])
render json: @artifact, status: 200
end

def create
collection_policy.new(current_user).authorize! :new?
# We don't explicitly check for :save? permissions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unclear whether this is descriptive or declarative. Is this a problem report or a design decision?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been a while, but the save check and the new check seemed be identical sets.


if (duplicate = Artifact.find_by(id: params[:id]))
resource_policy.new(current_user, duplicate).authorize! :show?
head 303, location: v2_artifact_path(duplicate)
else
@artifact = new_artifact(params)
if @artifact.valid?
@artifact.save!
render json: @artifact, status: 201, location: v2_artifact_path(@artifact)
else
render json: @artifact.errors, status: :unprocessable_entity
end
end
end

private

def new_artifact(params)
Artifact.new(
id: params[:id],
user: current_user,
storage_format: params[:storage_format],
content_type: params[:content_type]
)
end

end
end
33 changes: 33 additions & 0 deletions app/controllers/v2/deposits_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# frozen_string_literal: true

module V2
class DepositsController < ResourceController

def create
# TODO policy check
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker

@deposit = Deposit.create!(
artifact: Artifact.find(params[:artifact_id]),
user: current_user,
status: :started
)
# TODO: don't pick a specific template
render "v2/deposits/show", status: 201, location: v2_deposit_path(@deposit)
end

def ready
# TODO policy check
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker

@deposit = Deposit.find(params[:id])
case @deposit.status
when Deposit.statuses[:started]
@deposit.update!(status: Deposit.statuses[:ingesting])
FinishDepositJob.perform_later(@deposit)
render json: @deposit, status: 200
when Deposit.statuses[:ingesting]
render json: @deposit, status: 200
else # :completed, :failed, :cancelled
head 422 # TODO think about this response
end
end

end
end
7 changes: 7 additions & 0 deletions app/controllers/v2/revisions_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

module V2
class RevisionsController < ResourceController

end
end
18 changes: 9 additions & 9 deletions app/jobs/bag_move_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,28 +30,28 @@ def ingest!
end

def validate
package.valid_for_ingest?(errors)
result = Services.validation.validate(package)
errors.concat result.errors
result.valid?
end

def move_bag
source = incoming_storage.for(package)
package_storage.write(package, source)
package_storage.write(package, source) do |volume, storage_path|
package.storage_volume = volume.name
package.storage_path = storage_path
end
end

def record_success
queue_item.transaction do
queue_item.status = :done
queue_item.save!
queue_item.success!
package.save!
end
end

def record_failure
queue_item.transaction do
queue_item.error = errors.join("\n\n")
queue_item.status = :failed
queue_item.save!
end
queue_item.fail!(errors)
end

def log_exception(exception)
Expand Down
7 changes: 7 additions & 0 deletions app/jobs/finish_deposit_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class FinishDepositJob < ApplicationJob
def perform(deposit)
Chipmunk::FinishDeposit.new(deposit).run
end
end
53 changes: 53 additions & 0 deletions app/models/artifact.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# frozen_string_literal: true

class Artifact < ApplicationRecord

class AnyArtifact
def to_resources
Artifact.content_types.map {|t| Checkpoint::Resource::AllOfType.new(t) }
end

def resource_type
"Artifact"
end

def resource_id
Checkpoint::Resource::ALL
end
end

def self.resource_types
content_types
end

def self.content_types
Rails.application.config.validation["bagger_profile"].keys +
Rails.application.config.validation["external"].keys
end

def self.of_any_type
AnyArtifact.new
end

alias_method :identifier, :id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's talk about this identifier business... remind me of the intent behind an alias?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to be able to treat the artifact the same as our v1 objects, so there was some mismatch between id, uuid.


# Each artifact belongs to a single user
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to follow convention and have all of the relationships and validations at the very top of the class.

belongs_to :user
# Deposits are collections of zero or more revisions
has_many :revisions
# Revisions are added to artifacts via deposits
has_many :deposits

validates :id, presence: true,
format: { with: Services.uuid_format,
message: "must be a valid v4 uuid." }

validates :user, presence: true
validates :storage_format, presence: true # TODO this is a controlled vocabulary
validates :content_type, presence: true # TODO this is a controlled vocabulary

def stored?
revisions.any?
end

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

class Deposit < ApplicationRecord

# Each deposit is created by a single user
belongs_to :user
# A deposit is an attempt to append a revision to a single artifact
belongs_to :artifact

# TODO Could not get the attributes api to work with a custom type
enum status: {
started: "started",
canceled: "cancelled",
ingesting: "ingesting",
failed: "failed",
completed: "completed"
}

validates :user, presence: true
validates :artifact, presence: true
validates :status, presence: true # TODO this is a controlled vocabulary

def identifier
id.to_s
end

def username
user.username
end

def content_type
artifact.content_type
end

def storage_format
artifact.storage_format
end

def complete!
update!(status: "completed")
end

def fail!(errors)
update!(status: "failed", error: errors.join("\n"))
end

def upload_link
Services.incoming_storage.upload_link(self)
end

end
47 changes: 13 additions & 34 deletions app/models/package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,29 @@ class Package < ApplicationRecord
def to_param
bag_id
end
alias_method :identifier, :to_param

validates :bag_id, presence: true, length: { minimum: 6 }
validates :user_id, presence: true
validates :external_id, presence: true
validates :format, presence: true
validates :format, presence: true, format: /bag/

# Declare the policy class to use for authz
def self.policy_class
PackagePolicy
end

# Rails overrides the format param on requests, so we rename this to storage_format.
# This alias is for backwards compatibility
# alias_method :storage_format, :format
def storage_format
format
end

def username
user.username
end

def upload_link
Services.incoming_storage.upload_link(self)
end
Expand All @@ -40,39 +52,6 @@ def storage_location
storage_path
end

def storage_location=
raise "storage_location is not writable; use storage_volume and storage_path"
end

# TODO: This is nasty... but the storage factory checks that the package is stored,
# so we have to make the storage proxy manually here. Once the ingest and preservation
# responsibilities are clarified, this will fall out. See PFDR-184.
def valid_for_ingest?(errors = [])
if stored?
errors << "Package #{bag_id} is already stored"
elsif format != Chipmunk::Bag.format
errors << "Package #{bag_id} has invalid format: #{format}"
elsif !incoming_storage.include?(self)
errors << "Bag #{bag_id} does not exist in incoming storage."
end

return false unless errors.empty?

Chipmunk::Bag::Validator.new(self, errors, incoming_storage.for(self)).valid?
end

def external_validation_cmd
ext_cmd = Rails.application.config.validation["external"][content_type.to_s]
return unless ext_cmd

path = incoming_storage.for(self).path
[ext_cmd, external_id, path].join(" ")
end

def bagger_profile
Rails.application.config.validation["bagger_profile"][content_type.to_s]
end

def resource_type
content_type
end
Expand Down
8 changes: 8 additions & 0 deletions app/models/queue_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ def user
package.user
end

def success!
update!(status: :done)
end

def fail!(errors)
update!(status: :failed, error: errors.join("\n\n"))
end

scope :for_package, ->(package_id) { where(package_id: package_id) unless package_id.blank? }
scope :for_packages, ->(package_scope) { joins(:package).merge(package_scope) }
end
6 changes: 6 additions & 0 deletions app/models/revision.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# frozen_string_literal: true

class Revision < ApplicationRecord
belongs_to :artifact
belongs_to :deposit
end
24 changes: 24 additions & 0 deletions app/policies/artifacts_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

class ArtifactsPolicy < CollectionPolicy
def index?
can?(:show, Artifact.of_any_type)
end

def new?
can?(:save, Artifact.of_any_type)
end

def base_scope
Artifact.all
end

def resolve
# Via the role map resolver, a user has access to:
# * All artifacts, if the user is an administrator
# * Artifacts of content types for which the user is a content manager
# * Artifacts of content types for which the user is authorized viewer
# * Any specific artifacts for which the user is granted access
ViewableResources.for(user, scope)
end
end
Loading