From 425b3dd7cc9a3e010dc252dbb2048acba2e3fb80 Mon Sep 17 00:00:00 2001
From: Eric O <eric@ejdigitalmedia.com>
Date: Sun, 22 Dec 2024 18:28:19 -0500
Subject: [PATCH] Test updates and rubocop updates

---
 Gemfile                                       |   2 +
 Gemfile.lock                                  |   4 +
 .../concerns/triclops/resource/as_json.rb     |  13 +-
 app/models/resource.rb                        |  39 +-
 lib/tasks/triclops/setup.rake                 |   6 +-
 lib/triclops/utils/uri_utils.rb               |  29 ++
 spec/factories/resources.rb                   |  12 +-
 .../triclops/resource/as_json_spec.rb         |   3 -
 .../triclops/resource/iiif_info_spec.rb       |   2 +-
 spec/models/resource_spec.rb                  | 355 ++++++++++--------
 .../v1/resources/create_or_replace_spec.rb    |   2 +-
 spec/requests/iiif/images/info_spec.rb        |  10 +-
 spec/requests/iiif/images/raster_spec.rb      |  12 +-
 spec/spec_helper.rb                           |   3 +
 spec/triclops/raster_cache_spec.rb            |  16 +-
 15 files changed, 302 insertions(+), 206 deletions(-)
 create mode 100644 lib/triclops/utils/uri_utils.rb

diff --git a/Gemfile b/Gemfile
index 63c7304..7f6c1ff 100644
--- a/Gemfile
+++ b/Gemfile
@@ -1,6 +1,8 @@
 source 'https://rubygems.org'
 git_source(:github) { |repo| "https://github.com/#{repo}.git" }
 
+# URI encoding and decoding
+gem 'addressable', '~> 2.8.0'
 # Use best_type for media type detection
 gem 'best_type', '~> 0.0.10'
 # For schema validation
diff --git a/Gemfile.lock b/Gemfile.lock
index beae02f..09a040c 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -83,6 +83,8 @@ GEM
       minitest (>= 5.1)
       mutex_m
       tzinfo (~> 2.0)
+    addressable (2.8.7)
+      public_suffix (>= 2.0.2, < 7.0)
     airbrussh (1.5.0)
       sshkit (>= 1.6.1, != 1.7.0)
     ast (2.4.2)
@@ -247,6 +249,7 @@ GEM
       racc
     psych (5.1.2)
       stringio
+    public_suffix (6.0.1)
     puma (5.6.8)
       nio4r (~> 2.0)
     racc (1.7.3)
@@ -446,6 +449,7 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
+  addressable (~> 2.8.0)
   base64 (= 0.1.1)
   best_type (~> 0.0.10)
   bootsnap (>= 1.4.2)
diff --git a/app/models/concerns/triclops/resource/as_json.rb b/app/models/concerns/triclops/resource/as_json.rb
index 93e82aa..6f2ea5b 100644
--- a/app/models/concerns/triclops/resource/as_json.rb
+++ b/app/models/concerns/triclops/resource/as_json.rb
@@ -12,15 +12,10 @@ def as_json(_options = {})
           :has_view_limitation,
           :featured_region,
           :source_uri,
-          :standard_width,
-          :standard_height,
-          :limited_width,
-          :limited_height,
-          :featured_width,
-          :featured_height,
-          :created_at,
-          :updated_at,
-          :accessed_at
+          :standard_width, :standard_height,
+          :limited_width, :limited_height,
+          :featured_width, :featured_height,
+          :created_at, :updated_at, :accessed_at
         ].map { |field_name| [field_name, self.send(field_name)] }.to_h
       end
     end
diff --git a/app/models/resource.rb b/app/models/resource.rb
index 950b360..61d2bca 100644
--- a/app/models/resource.rb
+++ b/app/models/resource.rb
@@ -16,28 +16,31 @@ class Resource < ApplicationRecord
   after_destroy :delete_filesystem_cache!
 
   def wait_for_source_uri_if_local_disk_file
-    return if self.source_uri.nil?
-    protocol, path = self.source_uri.split('://')
-    return unless protocol == 'file'
+    return if self.source_uri.nil? || !self.source_uri.start_with?('file:/')
+    file_path = Triclops::Utils::UriUtils.location_uri_to_file_path(self.source_uri)
 
     # Under certain circumstances, a source_uri file that was recently writter by an external
     # process may take a few seconds to become available for reading (for example, if the file
     # was written to a network disk and the change has not been propagated yet to other servers).
     # So we'll wait and try again a few times, if it's not found right away.
     5.times do
-      break if File.exist?(path)
+      break if File.exist?(file_path)
       sleep 1
     end
   end
 
-  # Generates a placeholder resource dynamically (without any database interaction)
-  def self.placeholder_resource_for(identifier)
+  # Generates a placeholder resource dynamically (without any database interaction),
+  # based on the given placeholder_resource_identifier.  Note that this method
+  # will raise an exception if placeholder_resource_identifier is not a
+  # recognized placeholder identifier.
+  def self.placeholder_resource_for(placeholder_resource_identifier)
+    raise ArgumentError unless KNOWN_PLACEHOLDER_IDENTIFIERS.include?(placeholder_resource_identifier)
     Resource.new(
-      identifier: identifier,
+      identifier: placeholder_resource_identifier,
       has_view_limitation: false,
       status: 'ready',
       updated_at: Time.current,
-      source_uri: identifier.sub(':', '://'),
+      source_uri: placeholder_resource_identifier.sub(':', ':///'),
       standard_width: PLACEHOLDER_SIZE,
       standard_height: PLACEHOLDER_SIZE,
       limited_width: Triclops::Iiif::Constants::LIMITED_BASE_SIZE,
@@ -331,21 +334,13 @@ def switch_to_pending_state_if_core_properties_changed!
   # @yield source_image_file [File] A file holding the source image content.
   def with_source_image_file
     raise Errno::ENOENT, 'Missing source_uri' if self.source_uri.blank?
-    protocol, path = self.source_uri.split('://')
+    uri_scheme = Addressable::URI.parse(self.source_uri).scheme
 
-    case protocol
-    when 'railsroot'
-      yield File.new(Rails.root.join(path).to_s)
+    if ['file', 'railsroot', 'placeholder'].include?(uri_scheme)
+      yield File.new(Triclops::Utils::UriUtils.location_uri_to_file_path(self.source_uri))
       return
-    when 'placeholder'
-      yield File.new(File.join(PLACEHOLDER_ROOT, path + '.png').to_s)
-      return
-    when 'file'
-      if File.exist?(path)
-        yield File.new(path)
-        return
-      end
     end
+
     raise Errno::ENOENT, "Could not resolve file location: #{self.source_uri}"
   end
 
@@ -450,10 +445,10 @@ def yield_uncached_raster(raster_opts)
   # Returns true if this Resource's source_uri points to a placeholder image value.
   #
   # @api private
-  # @return [Boolean] true if source_uri starts with 'placeholder://'
+  # @return [Boolean] true if source_uri starts with 'placeholder:///'
   def source_uri_is_placeholder?
     return false if self.source_uri.nil?
-    self.source_uri.start_with?('placeholder://')
+    self.source_uri.start_with?('placeholder:///')
   end
 
   def self.generate_raster_tempfile_path(extension = '.blob')
diff --git a/lib/tasks/triclops/setup.rake b/lib/tasks/triclops/setup.rake
index 884cf37..1c3c858 100644
--- a/lib/tasks/triclops/setup.rake
+++ b/lib/tasks/triclops/setup.rake
@@ -23,21 +23,21 @@ namespace :triclops do
       [
         {
           identifier: 'sample',
-          source_uri: 'railsroot://' + File.join('spec', 'fixtures', 'files', 'sample.jpg'),
+          source_uri: 'railsroot:///' + File.join('spec', 'fixtures', 'files', 'sample.jpg'),
           width: 1920,
           height: 3125,
           featured_region: '320,616,1280,1280'
         },
         {
           identifier: 'sample-with-transparency',
-          source_uri: 'railsroot://' + File.join('spec', 'fixtures', 'files', 'sample-with-transparency.png'),
+          source_uri: 'railsroot:///' + File.join('spec', 'fixtures', 'files', 'sample-with-transparency.png'),
           width: 1920,
           height: 1920,
           featured_region: '320,320,1280,1280'
         },
         {
           identifier: 'sound-resource',
-          source_uri: 'placeholder://sound',
+          source_uri: 'placeholder:///sound',
           width: 2292,
           height: 2292,
           featured_region: '0,0,1280,1280'
diff --git a/lib/triclops/utils/uri_utils.rb b/lib/triclops/utils/uri_utils.rb
new file mode 100644
index 0000000..bbe374a
--- /dev/null
+++ b/lib/triclops/utils/uri_utils.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+# TODO: A very similar UriUtils file also exists in the Hyacinth 2 code base.
+# Probably want to move this to a shared gem at some point.
+class Triclops::Utils::UriUtils
+  # Converts a file path to a location URI value
+  def self.file_path_to_location_uri(path)
+    raise ArgumentError, "Given path must be absolute.  Must start with a slash: #{path}" unless path.start_with?('/')
+
+    "file://#{Addressable::URI.encode(path).gsub('&', '%26').gsub('#', '%23')}"
+  end
+
+  def self.location_uri_to_file_path(location_uri)
+    parsed_uri = Addressable::URI.parse(location_uri)
+    uri_scheme = parsed_uri.scheme
+    unencoded_uri_path = Addressable::URI.unencode(parsed_uri.path)
+
+    case uri_scheme
+    when 'file'
+      return unencoded_uri_path
+    when 'railsroot'
+      return Rails.root.join(unencoded_uri_path[1..]).to_s
+    when 'placeholder'
+      return File.join(PLACEHOLDER_ROOT, "#{unencoded_uri_path[1..]}.png")
+    end
+
+    raise ArgumentError, "Unhandled URI: #{location_uri}"
+  end
+end
diff --git a/spec/factories/resources.rb b/spec/factories/resources.rb
index bb74294..af674f9 100644
--- a/spec/factories/resources.rb
+++ b/spec/factories/resources.rb
@@ -6,20 +6,20 @@
 
     identifier { "test-resource-#{identifier_counter}" }
     has_view_limitation { false }
-    source_uri { "railsroot://#{File.join('spec', 'fixtures', 'files', 'sample.jpg')}" }
+    source_uri { "railsroot:///#{File.join('spec', 'fixtures', 'files', 'sample.jpg')}" }
     standard_width { 1920 }
     standard_height { 3125 }
-    limited_width { standard_width > standard_height ? 768 : ((768.to_f / standard_height) * standard_width).round }
-    limited_height { standard_height > standard_width ? 768 : ((768.to_f / standard_width) * standard_height).round }
-    featured_width { 768 }
-    featured_height { 768 }
     featured_region { '320,616,1280,1280' }
     pcdm_type { BestType::PcdmTypeLookup::IMAGE }
     status { Resource.statuses[:pending] }
-    accessed_at { DateTime.parse('2024-01-01T01:23:45-05:00') }
 
     trait :ready do
       status { Resource.statuses[:ready] }
+      limited_width { standard_width > standard_height ? 768 : ((768.to_f / standard_height) * standard_width).round }
+      limited_height { standard_height > standard_width ? 768 : ((768.to_f / standard_width) * standard_height).round }
+      featured_width { 768 }
+      featured_height { 768 }
+      accessed_at { DateTime.parse('2024-01-01T01:23:45-05:00') }
     end
 
     trait :with_view_limitation do
diff --git a/spec/models/concerns/triclops/resource/as_json_spec.rb b/spec/models/concerns/triclops/resource/as_json_spec.rb
index 8fc2879..ea619d6 100644
--- a/spec/models/concerns/triclops/resource/as_json_spec.rb
+++ b/spec/models/concerns/triclops/resource/as_json_spec.rb
@@ -7,9 +7,6 @@
 
   context "#as_json" do
     it 'returns the expected hash' do
-      created_at = resource.created_at
-      updated_at = resource.updated_at
-      accessed_at = resource.accessed_at
       expect(
         {
           identifier: resource.identifier,
diff --git a/spec/models/concerns/triclops/resource/iiif_info_spec.rb b/spec/models/concerns/triclops/resource/iiif_info_spec.rb
index 0763278..29616bf 100644
--- a/spec/models/concerns/triclops/resource/iiif_info_spec.rb
+++ b/spec/models/concerns/triclops/resource/iiif_info_spec.rb
@@ -4,7 +4,7 @@
   let(:identifier) { 'test' }
   let(:rails_root_relative_path) { File.join('spec', 'fixtures', 'files', 'sample.jpg') }
   let(:source_file_path) { Rails.root.join(rails_root_relative_path).to_s }
-  let(:source_uri) { 'railsroot://' + rails_root_relative_path }
+  let(:source_uri) { 'railsroot:///' + rails_root_relative_path }
   let(:standard_width) { 1920 }
   let(:standard_height) { 3125 }
   let(:featured_region) { '320,616,1280,1280' }
diff --git a/spec/models/resource_spec.rb b/spec/models/resource_spec.rb
index 90b8b23..5301d8d 100644
--- a/spec/models/resource_spec.rb
+++ b/spec/models/resource_spec.rb
@@ -2,23 +2,9 @@
 
 RSpec.describe Resource, type: :model do
   let(:base_type) { Triclops::Iiif::Constants::BASE_TYPE_STANDARD }
-  let(:identifier) { 'test' }
-  let(:rails_root_relative_image_path) { File.join('spec', 'fixtures', 'files', 'sample.jpg') }
-  let(:source_file_path) { Rails.root.join(rails_root_relative_image_path).to_s }
-  let(:source_uri) { 'railsroot://' + rails_root_relative_image_path }
-  let(:standard_width) { 1920 }
-  let(:standard_height) { 3125 }
-  let(:featured_region) { '320,616,1280,1280' }
-  let(:instance) do
-    described_class.new({
-      identifier: identifier,
-      source_uri: source_uri,
-      standard_width: standard_width,
-      standard_height: standard_height,
-      featured_region: featured_region,
-      pcdm_type: BestType::PcdmTypeLookup::IMAGE
-    })
-  end
+
+  let(:pending_resource) { FactoryBot.create(:resource, identifier: 'pending-resource') }
+  let(:ready_resource) { FactoryBot.create(:resource, :ready, identifier: 'ready-resource') }
   let(:raster_opts) do
     {
       region: 'full',
@@ -30,85 +16,107 @@
   end
 
   context '#initialize' do
+    let(:identifier) { 'some-identifier' }
+    let(:new_instance) do
+      described_class.new(
+        identifier: identifier,
+        has_view_limitation: false,
+        source_uri: 'railsroot:///spec/fixtures/files/sample.jpg',
+        standard_width: 1920,
+        standard_height: 3125,
+        featured_region: '320,616,1280,1280',
+        pcdm_type: BestType::PcdmTypeLookup::IMAGE
+      )
+    end
+
     it 'successfully creates a new instance and sets up appropriate fields' do
-      expect(instance).to be_a(described_class)
-      expect(instance.identifier).to eq(identifier)
-      expect(instance.standard_width).to eq(standard_width)
-      expect(instance.standard_height).to eq(standard_height)
-      expect(instance.featured_region).to eq(featured_region)
+      expect(new_instance).to be_a(described_class)
+      expect(new_instance.identifier).to eq(identifier)
+      expect(new_instance.has_view_limitation).to eq(false)
+      expect(new_instance.source_uri).to eq('railsroot:///spec/fixtures/files/sample.jpg')
+      expect(new_instance.standard_width).to eq(1920)
+      expect(new_instance.standard_height).to eq(3125)
+      expect(new_instance.featured_region).to eq('320,616,1280,1280')
+      expect(new_instance.pcdm_type).to eq(BestType::PcdmTypeLookup::IMAGE)
+      expect(new_instance.status).to eq('pending')
+    end
+  end
+
+  context 'validation' do
+    it 'calls wait_for_source_uri_if_local_disk_file before validation' do
+      expect(ready_resource).to receive(:wait_for_source_uri_if_local_disk_file)
+      ready_resource.valid?
+    end
+  end
+
+  context '#save' do
+    it 'calls switch_to_pending_state_if_core_properties_changed! before save' do
+      expect(ready_resource).to receive(:switch_to_pending_state_if_core_properties_changed!)
+      ready_resource.save
+    end
+
+    it 'calls queue_base_derivative_generation_if_pending after save' do
+      expect(ready_resource).to receive(:queue_base_derivative_generation_if_pending)
+      ready_resource.save
+    end
+  end
+
+  context '#destroy' do
+    it 'calls delete_filesystem_cache! after destroy' do
+      expect(ready_resource).to receive(:delete_filesystem_cache!).and_call_original
+      ready_resource.destroy
+    end
+  end
+
+  context '#delete_filesystem_cache!' do
+    it 'performs the expected deletion operation' do
+      expect(FileUtils).to receive(:rm_rf).with(Triclops::RasterCache.instance.cache_directory_for_identifier(ready_resource.identifier))
+      ready_resource.delete_filesystem_cache!
+    end
+  end
+
+  context '#queue_base_derivative_generation_if_pending' do
+    it 'queues generation for a pending resource' do
+      expect(CreateBaseDerivativesJob).to receive(:perform_later).with(pending_resource.identifier)
+      pending_resource.queue_base_derivative_generation_if_pending
+    end
+
+    it 'does not queue generation for a non-pending resource' do
+      expect(CreateBaseDerivativesJob).not_to receive(:perform_later)
+      ready_resource.queue_base_derivative_generation_if_pending
     end
   end
 
-  # context '#extract_width_and_height_if_missing_or_source_changed!' do
-  #   let(:width) { nil }
-  #   let(:height) { nil }
-  #   let(:image_double) do
-  #     instance_double('image').tap do |dbl|
-  #       allow(dbl).to receive(:width).and_return(1920)
-  #       allow(dbl).to receive(:height).and_return(3125)
-  #     end
-  #   end
-
-  #   before do
-  #     allow(Imogen).to receive(:with_image).and_yield(image_double)
-  #     # Skip base derivative generation for this set of tests
-  #     allow(instance).to receive(:queue_base_derivative_generation_if_pending)
-  #   end
-
-  #   context "for a new resource instance that has a source_uri, but does not currently store width or height" do
-  #     it "extracts the expected width" do
-  #       expect(instance).to receive(:width=).with(1920)
-  #       instance.extract_width_and_height_if_missing_or_source_changed!
-  #     end
-
-  #     it "extracts the expected height" do
-  #       expect(instance).to receive(:height=).with(3125)
-  #       instance.extract_width_and_height_if_missing_or_source_changed!
-  #     end
-  #   end
-
-  #   context "when the source_uri HAS NOT changed, and the method is called again" do
-  #     before do
-  #       # Extract properties and save before upcoming tests run
-  #       instance.extract_width_and_height_if_missing_or_source_changed!
-  #       instance.save
-  #       expect(instance.errors.full_messages).to be_blank
-  #     end
-
-  #     it "does not re-extract the width" do
-  #       expect(instance).not_to receive(:width=)
-  #       instance.extract_width_and_height_if_missing_or_source_changed!
-  #     end
-
-  #     it "does not re-extract the height" do
-  #       expect(instance).not_to receive(:height=)
-  #       instance.extract_width_and_height_if_missing_or_source_changed!
-  #     end
-  #   end
-
-  #   context "when the source_uri HAS changed, and the method is called again" do
-  #     before do
-  #       # Extract properties and save before upcoming tests run
-  #       instance.extract_width_and_height_if_missing_or_source_changed!
-  #       instance.save
-  #       # Then change the source uri to nil and save
-  #       instance.source_uri = nil
-  #       instance.save
-  #       # And then reassign the source_uri to the original value
-  #       instance.source_uri = source_uri
-  #     end
-
-  #     it "does not re-extract the width" do
-  #       expect(instance).to receive(:width=)
-  #       instance.extract_width_and_height_if_missing_or_source_changed!
-  #     end
-
-  #     it "does not re-extract the height" do
-  #       expect(instance).to receive(:height=)
-  #       instance.extract_width_and_height_if_missing_or_source_changed!
-  #     end
-  #   end
-  # end
+  context '#switch_to_pending_state_if_core_properties_changed!' do
+    let(:new_unsaved_resource) { FactoryBot.create(:resource) }
+    it 'does not change the status for a new record' do
+      expect(new_unsaved_resource).not_to receive(:status=)
+      new_unsaved_resource.switch_to_pending_state_if_core_properties_changed!
+    end
+
+    it 'does not change the status for a persisted resource if none of its properties have changed since it was last saved' do
+      expect(ready_resource).not_to receive(:status=)
+      ready_resource.switch_to_pending_state_if_core_properties_changed!
+    end
+
+    it 'changes the status for a persisted resource if its source_uri has changed' do
+      expect(ready_resource).to receive(:status=).with(:pending)
+      ready_resource.source_uri = ready_resource.source_uri.sub('sample.jpg', 'sample2.jpg')
+      ready_resource.switch_to_pending_state_if_core_properties_changed!
+    end
+
+    it 'changes the status for a persisted resource if its featured_region has changed' do
+      expect(ready_resource).to receive(:status=).with(:pending)
+      ready_resource.featured_region = '10,10,200,200'
+      ready_resource.switch_to_pending_state_if_core_properties_changed!
+    end
+
+    it 'changes the status for a persisted resource if its pcdm_type has changed' do
+      expect(ready_resource).to receive(:status=).with(:pending)
+      ready_resource.pcdm_type = BestType::PcdmTypeLookup::VIDEO
+      ready_resource.switch_to_pending_state_if_core_properties_changed!
+    end
+  end
 
   context '#yield_uncached_raster' do
     let(:tmp_file_path) { Rails.root.join('tmp', 'test-tmp-file.png').to_s }
@@ -118,9 +126,13 @@
     end
 
     it 'generates a raster file and automatically deletes that raster file after yielding' do
-      expect(Triclops::Raster).to receive(:generate).with(source_file_path, tmp_file_path, raster_opts).and_call_original
+      expect(Triclops::Raster).to receive(:generate).with(
+        Triclops::Utils::UriUtils.location_uri_to_file_path(ready_resource.source_uri),
+        tmp_file_path,
+        raster_opts
+      ).and_call_original
 
-      instance.yield_uncached_raster(raster_opts) do |raster_file|
+      ready_resource.yield_uncached_raster(raster_opts) do |raster_file|
         expect(tmp_file_path).to eq(raster_file.path)
         expect(File.exist?(tmp_file_path)).to eq(true)
       end
@@ -138,14 +150,18 @@
 
     after do
       # Clean up files created by the test
-      instance.delete_filesystem_cache!
+      ready_resource.delete_filesystem_cache!
     end
 
     it 'when an existing raster file does not exist, generates and caches a new raster file and yields that new raster file' do
-      expected_cache_path = Triclops::RasterCache.instance.iiif_cache_path_for_raster(base_type, instance.identifier, raster_opts)
-      expect(Triclops::Raster).to receive(:generate).with(source_file_path, expected_cache_path, raster_opts).and_call_original
-
-      instance.yield_cached_raster(base_type, raster_opts) do |raster_file|
+      expected_cache_path = Triclops::RasterCache.instance.iiif_cache_path_for_raster(base_type, ready_resource.identifier, raster_opts)
+      expect(Triclops::Raster).to receive(:generate).with(
+        Triclops::Utils::UriUtils.location_uri_to_file_path(ready_resource.source_uri),
+        expected_cache_path,
+        raster_opts
+      ).and_call_original
+
+      ready_resource.yield_cached_raster(base_type, raster_opts) do |raster_file|
         expect(raster_file.path).to eq(expected_cache_path)
       end
     end
@@ -153,23 +169,23 @@
     it 'yields the same raster file when called multiple times, and does not regenerate the file for the second yield' do
       # Generate the raster
       path_from_first_yield = nil
-      instance.yield_cached_raster(base_type, raster_opts) do |raster_file|
+      ready_resource.yield_cached_raster(base_type, raster_opts) do |raster_file|
         path_from_first_yield = raster_file.path
       end
 
       # Then call yield_cached_raster again to return the already-generated raster
       expect(Triclops::Raster).not_to receive(:generate)
-      instance.yield_cached_raster(base_type, raster_opts) do |raster_file|
+      ready_resource.yield_cached_raster(base_type, raster_opts) do |raster_file|
         expect(path_from_first_yield).to eq(raster_file.path)
       end
     end
 
     context 'when two different resources have different identifiers, but the same source_uri value' do
-      let(:resource1) do
+      let(:ready_resource1) do
         FactoryBot.create(:resource, source_uri: source_uri)
       end
-      let(:resource2) do
-        FactoryBot.create(:resource, source_uri: source_uri)
+      let(:ready_resource2) do
+        FactoryBot.create(:resource, source_uri: ready_resource1.source_uri)
       end
       let(:raster_opts_base) do
         {
@@ -183,61 +199,63 @@
 
       after do
         # Cleanup after tests
-        resource1.delete_filesystem_cache!
-        resource2.delete_filesystem_cache!
+        ready_resource1.delete_filesystem_cache!
+        ready_resource2.delete_filesystem_cache!
       end
 
-      context 'when both resources have the same NON-"placeholder://"-prefixed source_uri value' do
-        it 'results in two different raster cache paths for each resource' do
-          resource1_raster_path = nil
-          resource2_raster_path = nil
-          resource1.yield_cached_raster(base_type, raster_opts) { |raster_file| resource1_raster_path = raster_file.path }
-          resource2.yield_cached_raster(base_type, raster_opts) { |raster_file| resource2_raster_path = raster_file.path }
-          expect(resource1_raster_path).not_to eq(resource2_raster_path)
+      context 'when both resources have the same NON-"placeholder:///"-prefixed source_uri value' do
+        let(:source_uri) { "railsroot:///#{File.join('spec', 'fixtures', 'files', 'sample.jpg')}" }
+        it  'results in two different raster cache paths for each resource '\
+            '(because we want to be able to clear each cache independently)' do
+          ready_resource1_raster_path = nil
+          ready_resource2_raster_path = nil
+          ready_resource1.yield_cached_raster(base_type, raster_opts) { |raster_file| ready_resource1_raster_path = raster_file.path }
+          ready_resource2.yield_cached_raster(base_type, raster_opts) { |raster_file| ready_resource2_raster_path = raster_file.path }
+          expect(ready_resource1_raster_path).not_to eq(ready_resource2_raster_path)
         end
       end
 
-      context 'when both resources have the SAME "placeholder://"-prefixed source_uri value' do
-        let(:source_uri) { 'placeholder://sound' }
+      context 'when both resources have the SAME "placeholder:///"-prefixed source_uri value' do
+        let(:source_uri) { 'placeholder:///sound' }
         it 'uses the same raster cache path for each resource' do
-          resource1_raster_path = nil
-          resource2_raster_path = nil
-          resource1.yield_cached_raster(base_type, raster_opts) { |raster_file| resource1_raster_path = raster_file.path }
-          resource2.yield_cached_raster(base_type, raster_opts) { |raster_file| resource2_raster_path = raster_file.path }
-          expect(resource1_raster_path).to eq(resource2_raster_path)
+          ready_resource1_raster_path = nil
+          ready_resource2_raster_path = nil
+          ready_resource1.yield_cached_raster(base_type, raster_opts) { |raster_file| ready_resource1_raster_path = raster_file.path }
+          ready_resource2.yield_cached_raster(base_type, raster_opts) { |raster_file| ready_resource2_raster_path = raster_file.path }
+          expect(ready_resource1_raster_path).to eq(ready_resource2_raster_path)
         end
       end
     end
   end
 
   context '#with_source_image_file' do
-    context "with a railsroot:// path" do
+    context "with a railsroot:/// path" do
       it "returns the path to an existing file" do
-        allow(instance).to receive(:source_uri).and_return('railsroot://spec/fixtures/files/sample-with-transparency.png')
+        allow(ready_resource).to receive(:source_uri).and_return('railsroot:///spec/fixtures/files/sample-with-transparency.png')
 
-        instance.with_source_image_file do |file|
+        ready_resource.with_source_image_file do |file|
           expect(Rails.root.join('spec', 'fixtures', 'files', 'sample-with-transparency.png').to_s).to eq(file.path)
         end
       end
 
       it "raises an error for a file that doesn't exist" do
-        allow(instance).to receive(:source_uri).and_return('railsroot://nofile.png')
-        expect { instance.with_source_image_file { |_file| } }.to raise_error(Errno::ENOENT)
+        allow(ready_resource).to receive(:source_uri).and_return('railsroot:///nofile.png')
+        expect { ready_resource.with_source_image_file { |_file| } }.to raise_error(Errno::ENOENT)
       end
     end
 
-    context "with a placeholder:// path" do
+    context "with a placeholder:/// path" do
       it "returns the path to an existing file" do
-        allow(instance).to receive(:source_uri).and_return('placeholder://sound')
+        allow(ready_resource).to receive(:source_uri).and_return('placeholder:///sound')
 
-        instance.with_source_image_file do |file|
+        ready_resource.with_source_image_file do |file|
           expect(Rails.root.join('app', 'assets', 'images', 'placeholders', 'sound.png').to_s).to eq(file.path)
         end
       end
 
       it "raises an error for a file that doesn't exist" do
-        allow(instance).to receive(:source_uri).and_return('placeholder://nofile')
-        expect { instance.with_source_image_file { |_file| } }.to raise_error(Errno::ENOENT)
+        allow(ready_resource).to receive(:source_uri).and_return('placeholder:///nofile')
+        expect { ready_resource.with_source_image_file { |_file| } }.to raise_error(Errno::ENOENT)
       end
     end
 
@@ -245,9 +263,9 @@
       it "returns the path to an existing file" do
         temp_file_path = Dir.tmpdir + '/triclops-test-file.png'
         FileUtils.touch(temp_file_path)
-        allow(instance).to receive(:source_uri).and_return('file://' + temp_file_path)
+        allow(ready_resource).to receive(:source_uri).and_return('file://' + temp_file_path)
 
-        instance.with_source_image_file do |file|
+        ready_resource.with_source_image_file do |file|
           expect(temp_file_path).to eq(file.path)
         end
       ensure
@@ -255,14 +273,14 @@
       end
 
       it "raises an error for a file that doesn't exist" do
-        allow(instance).to receive(:source_uri).and_return('file:///no/file/here.png')
-        expect { instance.with_source_image_file { |_file| } }.to raise_error(Errno::ENOENT)
+        allow(ready_resource).to receive(:source_uri).and_return('file:///no/file/here.png')
+        expect { ready_resource.with_source_image_file { |_file| } }.to raise_error(Errno::ENOENT)
       end
     end
 
     it "raises an error for an unsupported protocol" do
-      allow(instance).to receive(:source_uri).and_return('abc://what/does/this/protocol/even/mean.png')
-      expect { instance.with_source_image_file { |_file| } }.to raise_error(Errno::ENOENT)
+      allow(ready_resource).to receive(:source_uri).and_return('abc://what/does/this/protocol/even/mean.png')
+      expect { ready_resource.with_source_image_file { |_file| } }.to raise_error(Errno::ENOENT)
     end
   end
 
@@ -291,36 +309,67 @@
 
   context '#iiif_cache_path_for_raster' do
     it 'works as expected for a non-placeholder source_uri' do
-      expect(instance.iiif_cache_path_for_raster(base_type, raster_opts)).to eq(
-        "#{TRICLOPS[:raster_cache][:directory]}/9f/86/d0/81/"\
-        "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08/#{base_type}/iiif/full/full/0/color.png"
+      expect(ready_resource.iiif_cache_path_for_raster(base_type, raster_opts)).to eq(
+        "#{TRICLOPS[:raster_cache][:directory]}/62/60/d5/01/"\
+        "6260d501613040c7993755c92621e0a90abc64e3a81006259e6532a785d5072c/#{base_type}/iiif/full/full/0/color.png"
       )
     end
 
     it 'works as expected for a placeholder source_uri' do
-      instance.source_uri = 'placeholder://cool'
-      expect(instance.iiif_cache_path_for_raster(base_type, raster_opts)).to eq(
-        "#{TRICLOPS[:raster_cache][:directory]}/63/0f/dc/84/"\
-        "630fdc84e37d2c114ca6afdccb24fdc534bdd5f363745fe26833607fb067a080/#{base_type}/iiif/full/full/0/color.png"
+      ready_resource.source_uri = 'placeholder:///cool'
+      expect(ready_resource.iiif_cache_path_for_raster(base_type, raster_opts)).to eq(
+        "#{TRICLOPS[:raster_cache][:directory]}/14/31/7a/33/"\
+        "14317a3348a284ab4de0eee4c049370b93f709c9661c200d569902b959224ae1/#{base_type}/iiif/full/full/0/color.png"
       )
     end
   end
 
   context '#source_uri_is_placeholder?' do
-    it 'returns false when location uri does not start with placeholder://' do
-      expect(instance.source_uri_is_placeholder?).to eq(false)
+    it 'returns false when location uri does not start with placeholder:///' do
+      expect(ready_resource.source_uri_is_placeholder?).to eq(false)
+    end
+
+    it 'returns true when location uri starts with placeholder:///' do
+      ready_resource.source_uri = 'placeholder:///cool'
+      expect(ready_resource.source_uri_is_placeholder?).to eq(true)
     end
+  end
 
-    it 'returns true when location uri starts with placeholder://' do
-      instance.source_uri = 'placeholder://cool'
-      expect(instance.source_uri_is_placeholder?).to eq(true)
+  context '#wait_for_source_uri_if_local_disk_file' do
+    it 'returns immediately (and does not sleep) if the file exists' do
+      expect(Kernel).not_to receive(:sleep)
+      ready_resource.wait_for_source_uri_if_local_disk_file
     end
   end
 
-  context 'on save' do
-    # it 'automatically extracts missing image properties' do
-    #   expect(instance).to receive(:extract_width_and_height_if_missing_or_source_changed!)
-    #   expect(instance.save).to eq(true)
-    # end
+  context '.placeholder_resource_for' do
+    let(:valid_placeholder_resource_identifier) { 'placeholder:file' }
+    let(:invalid_placeholder_resource_identifier) { 'placeholder:banana' }
+
+    it 'creates the expected resource when a valid placeholder_resource_identifier is given' do
+      res = described_class.placeholder_resource_for(valid_placeholder_resource_identifier)
+      expect(res).to be_a(Resource)
+      expect(res.as_json).to eq({
+        accessed_at: nil,
+        created_at: nil,
+        featured_height: 768,
+        featured_region: "0,0,2292,2292",
+        featured_width: 768,
+        has_view_limitation: false,
+        identifier: "placeholder:file",
+        limited_height: 768,
+        limited_width: 768,
+        source_uri: "placeholder:///file",
+        standard_height: 2292,
+        standard_width: 2292,
+        updated_at: res.updated_at
+      })
+    end
+
+    it 'raises an exception if an unsupported placeholder_resource_identifier is given' do
+      expect {
+        described_class.placeholder_resource_for(invalid_placeholder_resource_identifier)
+      }.to raise_error(ArgumentError)
+    end
   end
 end
diff --git a/spec/requests/api/v1/resources/create_or_replace_spec.rb b/spec/requests/api/v1/resources/create_or_replace_spec.rb
index 036b75b..60ece71 100644
--- a/spec/requests/api/v1/resources/create_or_replace_spec.rb
+++ b/spec/requests/api/v1/resources/create_or_replace_spec.rb
@@ -10,7 +10,7 @@
 
   let(:valid_create_attributes) do
     valid_update_attributes.merge({
-      source_uri: "railsroot://#{File.join('spec', 'fixtures', 'files', 'sample.jpg')}"
+      source_uri: "railsroot:///#{File.join('spec', 'fixtures', 'files', 'sample.jpg')}"
     })
   end
 
diff --git a/spec/requests/iiif/images/info_spec.rb b/spec/requests/iiif/images/info_spec.rb
index 4439cc4..36ceb6f 100644
--- a/spec/requests/iiif/images/info_spec.rb
+++ b/spec/requests/iiif/images/info_spec.rb
@@ -5,7 +5,10 @@
     let(:base_type) { Triclops::Iiif::Constants::BASE_TYPE_STANDARD }
     let(:ready_identifier) { 'ready-resource' }
     let(:ready_info_url) { "/iiif/2/#{base_type}/#{ready_identifier}/info.json" }
-    let!(:ready_resource) { FactoryBot.create(:resource, :ready, identifier: ready_identifier) }
+
+    before do
+      FactoryBot.create(:resource, :ready, identifier: ready_identifier)
+    end
 
     it "returns a successful response (with CORS header) for a ready resource info url" do
       get ready_info_url
@@ -16,7 +19,10 @@
     context "when a resource exists, but does not have a ready status" do
       let(:pending_identifier) { 'pending-resource' }
       let(:pending_info_url) { "/iiif/2/#{base_type}/#{pending_identifier}/info.json" }
-      let!(:pending_resource) { FactoryBot.create(:resource, identifier: pending_identifier) }
+
+      before do
+        FactoryBot.create(:resource, identifier: pending_identifier)
+      end
 
       it "returns a 302 response for info url, and redirects to a placeholder" do
         get pending_info_url
diff --git a/spec/requests/iiif/images/raster_spec.rb b/spec/requests/iiif/images/raster_spec.rb
index 68c8157..54c0cc3 100644
--- a/spec/requests/iiif/images/raster_spec.rb
+++ b/spec/requests/iiif/images/raster_spec.rb
@@ -5,9 +5,10 @@
     let(:base_type) { Triclops::Iiif::Constants::BASE_TYPE_STANDARD }
     let(:ready_identifier) { 'ready-resource' }
     let(:ready_raster_url) { "/iiif/2/#{base_type}/#{ready_identifier}/full/512,/0/default.jpg" }
-    let!(:ready_resource) {
+
+    before do
       FactoryBot.create(:resource, :ready, identifier: ready_identifier)
-    }
+    end
 
     context "successful response" do
       let(:config_with_access_stats_enabled) do
@@ -43,9 +44,11 @@
     context "when a resource exists, but does not have a ready status" do
       let(:pending_resource_identifier) { 'pending-resource' }
       let(:pending_raster_url) { "/iiif/2/#{base_type}/#{pending_resource_identifier}/full/512,/0/default.jpg" }
-      let!(:pending_resource) {
+
+      before do
         FactoryBot.create(:resource, identifier: pending_resource_identifier)
-      }
+      end
+
       it "returns a 302 response for raster url when a ready resource does not exist, and redirects to a placeholder" do
         get pending_raster_url
         expect(response).to have_http_status(:found)
@@ -56,6 +59,7 @@
     context "when a resource does not exist" do
       let(:non_existent_resource_identifier) { 'non-existent' }
       let(:non_existent_raster_url) { "/iiif/2/#{base_type}/#{non_existent_resource_identifier}/full/512,/0/default.jpg" }
+
       it "returns a 302 response for raster url when a ready resource does not exist, and redirects to a placeholder" do
         get non_existent_raster_url
         expect(response).to have_http_status(:found)
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 464b7f7..1786ea3 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -34,6 +34,9 @@
     # ...rather than:
     #     # => "be bigger than 2"
     expectations.include_chain_clauses_in_custom_matcher_descriptions = true
+
+    # Allow a much longer output length than the default (so that string comparison error messsages display the full string)
+    expectations.max_formatted_output_length = 2000
   end
 
   # rspec-mocks config goes here. You can use an alternate test double
diff --git a/spec/triclops/raster_cache_spec.rb b/spec/triclops/raster_cache_spec.rb
index 1f3a804..01ddbc7 100644
--- a/spec/triclops/raster_cache_spec.rb
+++ b/spec/triclops/raster_cache_spec.rb
@@ -43,13 +43,25 @@
 
   context '#cache_directory_for_identifier' do
     it 'returns the expected value' do
-      expect(instance.cache_directory_for_identifier(identifier)).to eq(File.join(cache_directory, 'c7/b0/1c/2c/c7b01c2c7f3383eba7c7e993ab921f5a8dc06421f67120d7dfe8735673fc2d32'))
+      expect(
+        instance.cache_directory_for_identifier(identifier)
+      ).to eq(
+        File.join(cache_directory, 'c7/b0/1c/2c/c7b01c2c7f3383eba7c7e993ab921f5a8dc06421f67120d7dfe8735673fc2d32')
+      )
     end
   end
 
   context '#iiif_cache_path_for_raster' do
     it 'returns the expected value' do
-      expect(instance.iiif_cache_path_for_raster(Triclops::Iiif::Constants::BASE_TYPE_STANDARD, identifier, raster_opts)).to eq(File.join(cache_directory, "c7/b0/1c/2c/c7b01c2c7f3383eba7c7e993ab921f5a8dc06421f67120d7dfe8735673fc2d32/#{Triclops::Iiif::Constants::BASE_TYPE_STANDARD}/iiif/full/full/0/color.png"))
+      expect(
+        instance.iiif_cache_path_for_raster(Triclops::Iiif::Constants::BASE_TYPE_STANDARD, identifier, raster_opts)
+      ).to eq(
+        File.join(
+          cache_directory,
+          'c7/b0/1c/2c/c7b01c2c7f3383eba7c7e993ab921f5a8dc06421f67120d7dfe8735673fc2d32/'\
+          "#{Triclops::Iiif::Constants::BASE_TYPE_STANDARD}/iiif/full/full/0/color.png"
+        )
+      )
     end
   end
 end