Skip to content

Commit

Permalink
Merge 04666d1 into 1adc83e
Browse files Browse the repository at this point in the history
  • Loading branch information
ndushay committed Dec 3, 2019
2 parents 1adc83e + 04666d1 commit 0dcf9b1
Show file tree
Hide file tree
Showing 9 changed files with 300 additions and 48 deletions.
6 changes: 6 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ Bundler/OrderedGems:
Layout/EmptyLineAfterGuardClause:
Enabled: false

Layout/SpaceAroundEqualsInParameterDefault:
Enabled: false

Metrics/BlockLength:
Exclude:
- 'app/services/preserved_object_handler.rb' # FIXME check_existence shameless green
Expand All @@ -45,6 +48,8 @@ Metrics/LineLength:

Metrics/MethodLength:
Max: 25
Exclude:
- 'app/services/preserved_object_handler.rb'

Naming/FileName:
Exclude:
Expand Down Expand Up @@ -78,6 +83,7 @@ RSpec/NestedGroups:
Exclude:
- 'spec/lib/audit/catalog_to_moab_spec.rb'
- 'spec/lib/audit/moab_to_catalog_spec.rb'
- 'spec/requests/objects_controller_file_spec.rb'
- 'spec/services/checksum_validator_spec.rb'
- 'spec/services/preserved_object_handler_*.rb'

Expand Down
42 changes: 11 additions & 31 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# This configuration was edited manually on 2018-08-13 while using RuboCop version 0.58.2.
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2019-11-21 20:21:47 -0800 using RuboCop version 0.73.0.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
Expand Down Expand Up @@ -31,16 +33,6 @@ Layout/MultilineMethodCallIndentation:
Exclude:
- 'spec/lib/preservation_catalog/s3/audit_spec.rb'

# Offense count: 9
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: space, no_space
Layout/SpaceAroundEqualsInParameterDefault:
Exclude:
- 'app/lib/audit/checksum.rb'
- 'app/services/audit_results.rb'
- 'app/services/preserved_object_handler.rb'

# Offense count: 31
Lint/AmbiguousBlockAssociation:
Exclude:
Expand All @@ -49,9 +41,9 @@ Lint/AmbiguousBlockAssociation:
- 'spec/services/preserved_object_handler_update_version_spec.rb'
- 'spec/services/shared_examples_preserved_object_handler.rb'

# Offense count: 20
# Offense count: 21
Metrics/AbcSize:
Max: 71
Max: 69

# Offense count: 2
# Configuration parameters: CountBlocks.
Expand All @@ -67,18 +59,10 @@ Metrics/ClassLength:
Metrics/CyclomaticComplexity:
Max: 11

# Offense count: 5
# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
# URISchemes: http, https
Metrics/LineLength:
Max: 205

# Offense count: 1
# Configuration parameters: CountComments.
# Offense count: 2
# Configuration parameters: CountComments, ExcludedMethods.
Metrics/MethodLength:
Exclude:
- 'app/services/preserved_object_handler.rb'
- 'app/lib/audit/catalog_to_archive.rb'
Max: 38

# Offense count: 3
Metrics/PerceivedComplexity:
Expand All @@ -89,13 +73,9 @@ Naming/AccessorMethodName:
Exclude:
- 'app/services/moab_validation_handler.rb'

# Offense count: 1
RSpec/BeforeAfterAll:
Exclude:
- 'spec/spec_helper.rb'
- 'spec/rails_helper.rb'
- 'spec/support/**/*.rb'
- 'spec/models/preservation_policy_spec.rb'
# Offense count: 12
RSpec/NestedGroups:
Max: 7

# Offense count: 1
# Cop supports --auto-correct.
Expand Down
33 changes: 27 additions & 6 deletions app/controllers/objects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,34 @@ def show
render json: PreservedObject.find_by!(druid: druid).to_json
end

# return a specific file from the Moab
# GET /objects/:druid/file?category=manifest&filepath=signatureCatalog.xml
def file
if (params[:version] && !params[:version].match?(/^[1-9]\d*$/))
render(plain: "400 Bad Request: version parameter must be positive integer", status: :bad_request)
return
end

obj_version = params[:version].to_i if params[:version]&.match?(/^[1-9]\d*$/)
location = MoabStorageService.filepath(druid, params[:category], params[:filepath], obj_version)
if location
send_file location
else
render(plain: "404 File Not Found: #{druid}, #{params[:category]}, #{params[:filepath]}, #{params[:version]}", status: :not_found)
end
rescue ArgumentError => e
render(plain: "400 Bad Request: #{e}", status: :bad_request)
rescue Moab::MoabRuntimeError => e
render(plain: "404 Not Found: #{e}", status: :not_found)
end

# return the checksums and filesize for a single druid (supplied with druid: prefix)
# GET /objects/:druid/checksum
def checksum
render json: checksum_for_object(druid).to_json
render json: content_files_checksums(druid).to_json
end

# return the checksums and filesize for a list of druid (supplied with druid: prefix)
# return the checksums and filesize for a list of druids (supplied with druid: prefix)
# note: this is deliberately allowed to be a POST to allow for a large number of druids to be passed in
# GET OR POST /objects/checksums?druids[]=druid1&druids[]=druid2&druids[]=druid3
def checksums
Expand All @@ -34,7 +55,7 @@ def checksums
format.csv do
render plain: csv_checksum_list
end
format.any { render status: :not_acceptable, plain: 'Format not acceptable' }
format.any { render status: :not_acceptable, plain: 'Format not acceptable' }
end
end

Expand All @@ -58,20 +79,20 @@ def returned_druid(druid)
end

def json_checksum_list
druids.map { |druid| { returned_druid(druid) => checksum_for_object(druid) } }.to_json
druids.map { |druid| { returned_druid(druid) => content_files_checksums(druid) } }.to_json
end

def csv_checksum_list
CSV.generate do |csv|
druids.each do |druid|
checksum_for_object(druid).each do |checksum|
content_files_checksums(druid).each do |checksum|
csv << [returned_druid(druid), checksum[:filename], checksum[:md5], checksum[:sha1], checksum[:sha256], checksum[:filesize]]
end
end
end
end

def checksum_for_object(druid)
def content_files_checksums(druid)
content_group = MoabStorageService.retrieve_content_file_group(druid)
content_group.path_hash.map do |file, signature|
{ filename: file, md5: signature.md5, sha1: signature.sha1, sha256: signature.sha256, filesize: signature.size }
Expand Down
13 changes: 13 additions & 0 deletions app/services/moab_storage_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@

# Responsibility: interactions with Moab storage to support read-only access for ReST API calls
class MoabStorageService
# @return [Pathname] Pathname object containing the full path for the specified file
# raises ArgumentError or Moab::MoabRuntimeError as appropriate
# @param [String] druid
# @param [String] category category of desired file ('content', 'metadata', or 'manifest')
# @param [String] filename name of the file (path relative to base directory)
# @param [Integer, nil] version of the file (nil = latest)
def self.filepath(druid, category, filename, version=nil)
raise(ArgumentError, "No filename provided to MoabStorageService.filepath for druid #{druid}") if filename.blank?
err_msg = "category arg must be 'content', 'metadata', or 'manifest' (MoabStorageService.filepath for druid #{druid})"
raise(ArgumentError, err_msg) unless ['content', 'metadata', 'manifest'].include?(category)
Stanford::StorageServices.retrieve_file(category, filename, druid, version)
end

def self.retrieve_content_file_group(druid)
Moab::StorageServices.retrieve_file_group('content', druid)
end
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
resources :objects, only: %i[show] do
member do
get 'checksum'
get 'file', format: false
end
collection do
match 'checksums', via: %i[get post]
Expand Down
19 changes: 8 additions & 11 deletions spec/requests/objects_controller_checksums_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,16 +168,16 @@
end
end

context 'when no druids param provided' do
context 'when druids param is empty' do
context 'when no druids param' do
context 'when param is empty' do
it 'body has additional information from the exception if available' do
post checksums_objects_url, params: { druids: [], format: :json }
expect(response).to have_http_status(:bad_request)
expect(response.body).to eq '400 bad request: Identifier has invalid suri syntax: nil or empty'
end
end

context "when no druids param" do
context "when no param" do
it 'body has additional information from the exception if available' do
post checksums_objects_url, params: { format: :json }
expect(response).to have_http_status(:bad_request)
Expand All @@ -186,22 +186,19 @@
end
end

context 'when bad parameter passed in' do
it 'returns a 400 response code with a bad druid passed in' do
context 'when druid invalid' do
it 'returns 400 response code' do
post checksums_objects_url, params: { druids: [prefixed_druid, 'foobar'], format: :json }
expect(response).to have_http_status(:bad_request)
expect(response.body).to eq '400 bad request: Identifier has invalid suri syntax: foobar'
end
end

it 'returns a 406 response code when an unsupported response format is provided' do
context 'when unsupported response format' do
it 'returns 406 response code' do
post checksums_objects_url, params: { druids: [prefixed_druid], format: :xml }
expect(response).to have_http_status(:not_acceptable)
end

it 'returns a 400 response code with no druid parameters passed in' do
post checksums_objects_url, params: { format: :json }
expect(response).to have_http_status(:bad_request)
end
end
end
end
111 changes: 111 additions & 0 deletions spec/requests/objects_controller_file_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe ObjectsController, type: :request do
let(:prefixed_druid) { 'druid:bj102hs9687' }
let(:bare_druid) { 'bj102hs9687' }
let(:pres_obj) { create(:preserved_object) }

describe 'GET #file' do
context 'when object exists' do
context 'when druid is prefixed' do
it 'returns the requested file' do
get file_object_url(id: prefixed_druid), params: { category: 'manifest', filepath: 'manifestInventory.xml' }
expect(response).to have_http_status(:ok)
expect(response.body).to include 'md5'
end
end

context 'when druid is bare' do
it 'returns the requested file' do
get file_object_url(id: bare_druid), params: { category: 'manifest', filepath: 'manifestInventory.xml' }
expect(response).to have_http_status(:ok)
expect(response.body).to include 'md5'
end
end

context 'when file is not present in Moab' do
it 'returns 404 response code; body has additional information' do
get file_object_url(id: prefixed_druid), params: { category: 'manifest', filepath: 'foobar' }
expect(response).to have_http_status(:not_found)
expect(response.body).to eq '404 Not Found: manifest file foobar not found for bj102hs9687 - 3'
end
end

context 'when version specified' do
it 'retrieves the correct version of the file' do
get file_object_url(id: prefixed_druid), params: { category: 'manifest', filepath: 'manifestInventory.xml', version: '3' }
expect(response).to have_http_status(:ok)
expect(response.body).to include 'size="7133"'
end

context 'when version is too high' do
it 'returns 404 response code with details' do
get file_object_url(id: prefixed_druid), params: { category: 'manifest', filepath: 'ignored', version: '666' }
expect(response).to have_http_status(:not_found)
expect(response.body).to eq '404 Not Found: Version ID 666 does not exist'
end
end

context 'when version param is not digits only' do
it 'returns 400 response code with details' do
get file_object_url(id: prefixed_druid), params: { category: 'manifest', filepath: 'manifestInventory.xml', version: 'v3' }
expect(response).to have_http_status(:bad_request)
expect(response.body).to eq '400 Bad Request: version parameter must be positive integer'
end
end
end

context 'when ArgumentError from MoabStorageService' do
it 'returns 400 response code with details' do
get file_object_url(id: prefixed_druid), params: { category: 'metadata' }
expect(response).to have_http_status(:bad_request)
expect(response.body).to eq '400 Bad Request: No filename provided to MoabStorageService.filepath for druid bj102hs9687'
end
end

context 'when MoabRuntimeError from MoabStorageService' do
it 'returns 404 response code; body has additional information' do
get file_object_url(id: prefixed_druid), params: { category: 'manifest', filepath: 'foobar' }
expect(response).to have_http_status(:not_found)
expect(response.body).to eq '404 Not Found: manifest file foobar not found for bj102hs9687 - 3'
end
end
end

context 'when object does not exist' do
it 'returns 404 response code with "No storage object found"' do
get file_object_url(id: 'druid:xx123yy9999'), params: { category: 'manifest', filepath: 'manifestInventory.xml' }
expect(response).to have_http_status(:not_found)
expect(response.body).to eq '404 Not Found: No storage object found for xx123yy9999'
end
end

context 'when no id param' do
context 'when id param is empty' do
it "returns 404 with 'Couldn't find PreservedObject'" do
get file_object_url(id: ''), params: { category: 'manifest', filepath: 'manifestInventory.xml' }
expect(response).to have_http_status(:not_found)
expect(response.body).to eq "404 Not Found: Couldn't find PreservedObject"
end
end

context "when id param missing" do
it 'Rails will raise error and do the right thing' do
expect do
get file_object_url({}), params: { category: 'manifest', filepath: 'manifestInventory.xml' }
end.to raise_error(ActionController::UrlGenerationError)
end
end
end

context 'when druid invalid' do
it 'returns 404 response code with "Identifier has invalid suri syntax"' do
get file_object_url(id: 'foobar'), params: { category: 'manifest', filepath: 'manifestInventory.xml' }
expect(response).to have_http_status(:not_found)
expect(response.body).to eq '404 Not Found: Identifier has invalid suri syntax: foobar'
end
end
end
end
19 changes: 19 additions & 0 deletions spec/routing/routing_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe 'routes', type: :routing do
describe 'objects/:id/file' do
it 'file_object_path' do
druid = 'druid:ab123cd4567'
expect(get: file_object_path(id: druid)).to route_to(controller: 'objects', action: 'file', id: druid)
end
end

describe 'objects/:id/checksum' do
it 'checksum_object_path' do
druid = 'druid:ab123cd4567'
expect(get: checksum_object_path(id: druid)).to route_to(controller: 'objects', action: 'checksum', id: druid)
end
end
end

0 comments on commit 0dcf9b1

Please sign in to comment.