Skip to content

Commit

Permalink
Merge pull request #1219 from sul-dlss/t1213-fetch-versioned-cocina
Browse files Browse the repository at this point in the history
Use version when requesting cocina from purl endpoint
  • Loading branch information
jcoyne authored Aug 5, 2024
2 parents 2a5595f + 98ccf69 commit ad5f306
Show file tree
Hide file tree
Showing 9 changed files with 166 additions and 17 deletions.
4 changes: 4 additions & 0 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ RSpec/AnyInstance:
- 'spec/features/status_spec.rb'
- 'spec/requests/file_auth_request_spec.rb'
- 'spec/requests/file_spec.rb'
- 'spec/requests/versioned_file_spec.rb'
- 'spec/requests/iiif/auth/v1/tokens_spec.rb'
- 'spec/requests/iiif/auth/v2/tokens_spec.rb'
- 'spec/requests/iiif_auth_request_spec.rb'
Expand Down Expand Up @@ -169,6 +170,7 @@ RSpec/LeadingSubject:
RSpec/MessageSpies:
Exclude:
- 'spec/controllers/file_controller_spec.rb'
- 'spec/controllers/v2/versions_controller_spec.rb'
- 'spec/controllers/media_controller_spec.rb'

# Offense count: 85
Expand All @@ -182,6 +184,7 @@ RSpec/NamedSubject:
Exclude:
- 'spec/controllers/application_controller_spec.rb'
- 'spec/controllers/file_controller_spec.rb'
- 'spec/controllers/v2/versions_controller_spec.rb'
- 'spec/controllers/webauth_controller_spec.rb'
- 'spec/models/approved_location_spec.rb'
- 'spec/models/projection_spec.rb'
Expand All @@ -208,6 +211,7 @@ RSpec/RepeatedDescription:
RSpec/StubbedMock:
Exclude:
- 'spec/controllers/file_controller_spec.rb'
- 'spec/controllers/v2/versions_controller_spec.rb'
- 'spec/controllers/media_controller_spec.rb'

# Offense count: 1
Expand Down
8 changes: 6 additions & 2 deletions app/controllers/file_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def disposition
end

def file_params
params.permit(:id, :file_name, :download)
params.permit(:id, :file_name, :download, :version_id)
end

# called when CanCan::AccessDenied error is raised, typically by authorize!
Expand Down Expand Up @@ -74,6 +74,10 @@ def current_file
end

def cocina
@cocina ||= Cocina.find(params[:id])
@cocina ||= Cocina.find(params[:id], version)
end

def version
params[:version_id] || :head
end
end
6 changes: 5 additions & 1 deletion app/controllers/object_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ObjectController < ApplicationController
# Return a zip of all the files if they have access to all the files.
# This will force a login if any of the files is not access=world
def show
cocina = Cocina.find(druid)
cocina = Cocina.find(druid, version)
files = cocina.files
raise ActionController::RoutingError, 'No downloadable files' if files.none?

Expand Down Expand Up @@ -38,6 +38,10 @@ def druid
params[:id]
end

def version
params[:version_id] || :head
end

# called when CanCan::AccessDenied error is raised by authorize!
# Should only be here if
# a) access not allowed (send to super) OR
Expand Down
6 changes: 5 additions & 1 deletion app/controllers/webauth_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ def logout
# TODO: we may want one method for all the below, with a referer param to know where to redirect

def login_file
redirect_to file_path(params.to_unsafe_h.symbolize_keys)
if params[:version_id]
redirect_to versioned_file_path(params.to_unsafe_h.symbolize_keys)
else
redirect_to file_path(params.to_unsafe_h.symbolize_keys)
end
end

# For zip files
Expand Down
18 changes: 13 additions & 5 deletions app/models/cocina.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ class Cocina

THUMBNAIL_MIME_TYPE = 'image/jp2'

def self.find(druid)
data = Rails.cache.fetch("purl/#{druid}.json", expires_in: 10.minutes) do
benchmark "Fetching public json for #{druid}" do
response = Faraday.get(public_json_url(druid))
def self.find(druid, version = :head)
data = Rails.cache.fetch(metadata_cache_key(druid, version), expires_in: 10.minutes) do
benchmark "Fetching public json for #{druid} version #{version}" do
response = Faraday.get(public_json_url(druid, version))
raise Purl::Exception, response.status unless response.success?

JSON.parse(response.body)
Expand All @@ -18,7 +18,15 @@ def self.find(druid)
new(data)
end

def self.public_json_url(druid)
def self.metadata_cache_key(druid, version)
return "purl/#{druid}.json" if version == :head

"purl/#{druid}.#{version}.json"
end

def self.public_json_url(druid, version)
return "#{Settings.purl.url}#{druid}/#{version}.json" unless version == :head

"#{Settings.purl.url}#{druid}.json"
end

Expand Down
3 changes: 3 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@
druid_regex = /([a-z]{2})(\d{3})([a-z]{2})(\d{4})/i

get '/object/:id' => 'object#show', as: :object
get '/object/:id/:version_id', to: 'object#show', constraints: { version_id: /v\d+/ }

constraints id: druid_regex do
get '/file/:id/*file_name' => 'file#show', format: false, as: :file
get '/v2/file/:id/:version_id/*file_name', to: 'file#show', format: false, as: :versioned_file, constraints: { version_id: /v\d+/ }
options '/file/:id/*file_name', to: 'file#options', format: false
options '/v2/file/:id/:version_id/*file_name', to: 'file#options', format: false, constraints: { version_id: /v\d+/ }
get '/file/app/:id/*file_name' => 'webauth#login_file', format: false
get '/file/auth/:id/*file_name' => 'webauth#login_file', format: false, as: :auth_file
get '/file/auth/:id' => 'webauth#login_object', format: false, as: :auth_object
Expand Down
14 changes: 12 additions & 2 deletions spec/controllers/webauth_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,18 @@
subject { get :login_file, params: }
let(:params) { { id: 'xf680rd3068', file_name: 'xf680rd3068_1.jp2' } }

it 'returns the user to the file api' do
expect(subject).to redirect_to file_url(params)
context 'without a version specified' do
it 'returns the user to the file api' do
expect(subject).to redirect_to file_url(params)
end
end

context 'with version specified' do
let(:params) { { id: 'xf680rd3068', file_name: 'xf680rd3068_1.jp2', version_id: 'v1' } }

it 'returns the user to the versioned file api' do
expect(subject).to redirect_to versioned_file_url(params)
end
end

it 'stores user information in the session' do
Expand Down
27 changes: 21 additions & 6 deletions spec/models/cocina_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,30 @@
}.to_json
end

before do
stub_request(:get, "https://purl.stanford.edu/abc.json")
.to_return(status: 200, body: json)
context 'when requesting the head version' do
before do
stub_request(:get, "https://purl.stanford.edu/abc.json")
.to_return(status: 200, body: json)
end

it 'gets all the files for a resource' do
actual = described_class.find('abc').files.map { |file| "#{file.id}/#{file.file_name}" }

expect(actual).to contain_exactly('abc/26855.jp2', 'abc/123.jp2')
end
end

it 'gets all the files for a resource' do
actual = described_class.find('abc').files.map { |file| "#{file.id}/#{file.file_name}" }
context 'when requesting a specific version' do
before do
stub_request(:get, "https://purl.stanford.edu/abc/v2.json")
.to_return(status: 200, body: json)
end

it 'gets all the files for a resource' do
actual = described_class.find('abc', 'v2').files.map { |file| "#{file.id}/#{file.file_name}" }

expect(actual).to contain_exactly('abc/26855.jp2', 'abc/123.jp2')
expect(actual).to contain_exactly('abc/26855.jp2', 'abc/123.jp2')
end
end
end
end
97 changes: 97 additions & 0 deletions spec/requests/versioned_file_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe "Versioned File requests" do
before do
allow(Cocina).to receive(:find).and_call_original
end

let(:druid) { 'nr349ct7889' }
let(:version_id) { 'v1' }
let(:file_name) { 'image.jp2' }
let(:public_json) do
{
'externalIdentifier' => druid,
'structural' => {
'contains' => [
{
'structural' => {
'contains' => [
{
'filename' => file_name,
'access' => {
'view' => 'world',
'download' => 'world'
}
}
]
}
}
]
}
}
end

describe 'OPTIONS options' do
it 'permits Range headers for all origins' do
options "/v2/file/#{druid}/#{version_id}/#{file_name}"
expect(response).to be_successful
expect(response.headers['Access-Control-Allow-Origin']).to eq '*'
expect(response.headers['Access-Control-Allow-Headers']).to include 'Range'
end
end

describe 'GET file with slashes in filename' do
let(:file_name) { 'path/to/image.jp2' }
let(:version_id) { 'v1' }
let(:public_json) do
{
'externalIdentifier' => druid,
'structural' => {
'contains' => [
{
'structural' => {
'contains' => [
{
'filename' => file_name,
'access' => {
'view' => 'world',
'download' => 'world'
}
}
]
}
}
]
}
}
end

before do
allow_any_instance_of(FileController).to receive(:send_file)
.with('spec/fixtures/nr/349/ct/7889/path/to/image.jp2', disposition: :inline)
stub_request(:get, "https://purl.stanford.edu/#{druid}/#{version_id}.json")
.to_return(status: 200, body: public_json.to_json)
end

it 'returns a successful HTTP response' do
get "/v2/file/#{druid}/#{version_id}/#{file_name}"
expect(response).to be_successful
expect(Cocina).to have_received(:find).with(druid, version_id)
end
end

describe 'GET missing file' do
before do
stub_request(:get, "https://purl.stanford.edu/xf680rd3068/v1.json")
.to_return(status: 200, body: public_json.to_json)
end

it 'returns a 400 HTTP response' do
get '/v2/file/xf680rd3068/v1/path/to/99999.jp2'
expect(response).to have_http_status(:not_found)
expect(Cocina).to have_received(:find).with('xf680rd3068', 'v1')
end
end
end

0 comments on commit ad5f306

Please sign in to comment.