Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Attempt to read paths from content addressed storage if available" #1216

Merged
merged 1 commit into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 0 additions & 7 deletions app/models/cocina.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,6 @@ def find_file(file_name)
.find { |file| file['filename'] == file_name } || raise(ActionController::MissingFile, "File not found '#{file_name}'")
end

def find_file_md5(file_name)
file_node = find_file(file_name)
file_node.fetch('hasMessageDigests')
.find { |digest_node| digest_node.fetch('type') == 'md5' }
.fetch('digest')
end

def thumbnail_file
data.dig('structural', 'contains')
.lazy.flat_map { |file_set| file_set.dig('structural', 'contains') }
Expand Down
4 changes: 1 addition & 3 deletions app/models/stacks_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ class StacksFile
def initialize(file_name:, cocina:)
@file_name = file_name
@cocina = cocina
validate!
end

attr_reader :file_name, :cocina
Expand All @@ -18,7 +17,7 @@ def id
cocina.druid
end

validates :file_name, presence: true
validates :id, format: { with: StorageRoot::DRUID_PARTS_PATTERN }

# Some files exist but have unreadable permissions, treat these as non-existent
def readable?
Expand All @@ -41,7 +40,6 @@ def path
@path ||= storage_root.absolute_path
end

# Used as the IIIF identifier for retrieving this file from the image server
def treeified_path
storage_root.relative_path
end
Expand Down
35 changes: 16 additions & 19 deletions app/models/storage_root.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,21 @@ def initialize(file_name:, cocina:)

delegate :druid, to: :cocina

delegate :absolute_path, to: :path_finder
def druid_parts
@druid_parts ||= druid.match(DRUID_PARTS_PATTERN)
end

def absolute_path
return unless relative_path

delegate :relative_path, to: :path_finder
path_finder.absolute_path.to_s
end

def relative_path
return unless druid_parts && file_name

path_finder.relative_path.to_s
end

def treeified_id
File.join(druid_parts[1..4])
Expand All @@ -26,41 +38,26 @@ def treeified_id
attr_reader :cocina, :file_name

def path_finder
@path_finder ||= path_finder_class.new(treeified_id:, file_name:, cocina:)
@path_finder ||= path_finder_class.new(treeified_id:, druid:, file_name:)
end

def path_finder_class
LegacyPathFinder
end

def druid_parts
@druid_parts ||= druid.match(DRUID_PARTS_PATTERN)
end

# Calculate file paths in the legacy Stacks structure
class LegacyPathFinder
def initialize(treeified_id:, file_name:, cocina:)
def initialize(treeified_id:, file_name:, druid:) # rubocop:disable Lint/UnusedMethodArgument
@treeified_id = treeified_id
@file_name = file_name
@cocina = cocina
end

# As this is used for external service URLs (Canteloupe image server), we don't want to put content addressable path here.'
def relative_path
File.join(@treeified_id, @file_name)
end

def absolute_path
return content_addressable_path if File.exist?(content_addressable_path)

File.join(Settings.stacks.storage_root, relative_path)
end

def content_addressable_path
@content_addressable_path ||= begin
md5 = @cocina.find_file_md5(@file_name)
File.join(Settings.stacks.content_addressable_storage_root, @treeified_id, @cocina.druid, 'content', md5)
end
end
end
end
1 change: 0 additions & 1 deletion config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ features:

stacks:
storage_root: /stacks
content_addressable_storage_root: /stacks/content_addressable

imageserver:
base_uri: "http://imageserver-prod.stanford.edu/iiif/2/"
Expand Down
21 changes: 20 additions & 1 deletion spec/controllers/file_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,26 @@
end

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

describe '#show' do
Expand Down
1 change: 0 additions & 1 deletion spec/controllers/legacy_image_service_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

let(:public_json) do
{
'externalIdentifier' => 'druid:nr349ct7889',
'structural' => {
'contains' => [
{
Expand Down
34 changes: 0 additions & 34 deletions spec/factories/cocina.rb

This file was deleted.

23 changes: 13 additions & 10 deletions spec/models/stacks_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,29 @@
RSpec.describe StacksFile do
let(:druid) { 'nr349ct7889' }
let(:file_name) { 'image.jp2' }
let(:cocina) { Cocina.new(public_json) }
let(:cocina) { Cocina.new({ 'externalIdentifier' => druid }) }
let(:instance) { described_class.new(file_name:, cocina:) }
let(:path) { storage_root.absolute_path }
let(:storage_root) { StorageRoot.new(cocina:, file_name:) }
let(:public_json) { Factories.cocina_with_file }

context 'with a missing file name' do
let(:file_name) { nil }

it 'raises an error' do
expect { instance }.to raise_error ActiveModel::ValidationError
end
end

describe '#path' do
subject { instance.path }

it 'is the druid tree path to the file' do
expect(subject).to eq(path)
end

context 'with a malformed druid' do
let(:druid) { 'abcdef' }

it { is_expected.to be_nil }
end

context 'with a missing file name' do
let(:file_name) { nil }

it { is_expected.to be_nil }
end
end

describe '#readable?' do
Expand Down
2 changes: 0 additions & 2 deletions spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
require 'spec_helper'
require 'rspec/rails'
require 'capybara/rails'

require_relative 'factories/cocina'
# Add additional requires below this line. Rails is not loaded until this point!

# Requires supporting ruby files with custom matchers and macros, etc, in
Expand Down
44 changes: 41 additions & 3 deletions spec/requests/file_auth_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,26 @@
# NOTE: stanford only + location rights tested under location context
context 'stanford only (no location qualifications)' do
let(:public_json) do
Factories.cocina_with_file(file_access: { 'view' => 'stanford', 'download' => 'stanford' })
{
'externalIdentifier' => druid,
'structural' => {
'contains' => [
{
'structural' => {
'contains' => [
{
'filename' => file_name,
'access' => {
'view' => 'stanford',
'download' => 'stanford'
}
}
]
}
}
]
}
}
end

context 'webauthed user' do
Expand All @@ -50,8 +69,27 @@
context 'location' do
context 'not stanford qualified in any way' do
let(:public_json) do
Factories.cocina_with_file(file_access: { 'view' => 'location-based', 'download' => 'location-based',
'location' => 'location1' })
{
'externalIdentifier' => druid,
'structural' => {
'contains' => [
{
'structural' => {
'contains' => [
{
'filename' => file_name,
'access' => {
'view' => 'location-based',
'download' => 'location-based',
'location' => 'location1'
}
}
]
}
}
]
}
}
end

it 'allows when user in location' do
Expand Down
42 changes: 40 additions & 2 deletions spec/requests/file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,26 @@
let(:druid) { 'nr349ct7889' }
let(:file_name) { 'image.jp2' }
let(:public_json) do
Factories.cocina_with_file
{
'externalIdentifier' => druid,
'structural' => {
'contains' => [
{
'structural' => {
'contains' => [
{
'filename' => file_name,
'access' => {
'view' => 'world',
'download' => 'world'
}
}
]
}
}
]
}
}
end

describe 'OPTIONS options' do
Expand All @@ -25,7 +44,26 @@
describe 'GET file with slashes in filename' do
let(:file_name) { 'path/to/image.jp2' }
let(:public_json) do
Factories.cocina_with_file(file_name:)
{
'externalIdentifier' => druid,
'structural' => {
'contains' => [
{
'structural' => {
'contains' => [
{
'filename' => file_name,
'access' => {
'view' => 'world',
'download' => 'world'
}
}
]
}
}
]
}
}
end

before do
Expand Down
Loading