From 1f7d6e0cf2b6b618c3322634ab3d4fce3f804221 Mon Sep 17 00:00:00 2001
From: Connor Ferguson <68167430+cpfergus1@users.noreply.github.com>
Date: Tue, 15 Jun 2021 10:34:35 -0600
Subject: [PATCH] Implemented fix for FileNotFound errors with Active Storage

This commit adds the ability for the application to catch missing
images with Active Storage during an API call should a file become
corrupt or deleted without removing the record through appropriate channels.
This compliments PR #4026 which prevented the same error on the backend.
The change will return and image with attributes equal to nil and
noimage replacements if an image cannot be sourced.

Reasons for the changes:
- image.attachment.url bypasses rescue check found in ActiveStorageAdapter
- API calls hit height and width prior to url rescue, had to implement
rescue in Attachment#metadata.
- metadata returns hash with strings, not symbols. It appeared that
this caused nil values to be returned regardless if the image was good or not.
---
 api/app/views/spree/api/images/_image.json.jbuilder    |  2 +-
 api/spec/requests/spree/api/images_controller_spec.rb  | 10 ++++++++++
 .../spree/active_storage_adapter/attachment.rb         |  7 ++++---
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/api/app/views/spree/api/images/_image.json.jbuilder b/api/app/views/spree/api/images/_image.json.jbuilder
index c6c35b59891..7045b5bdb38 100644
--- a/api/app/views/spree/api/images/_image.json.jbuilder
+++ b/api/app/views/spree/api/images/_image.json.jbuilder
@@ -3,5 +3,5 @@
 json.(image, *image_attributes)
 json.(image, :viewable_type, :viewable_id)
 Spree::Image.attachment_definitions[:attachment][:styles].each do |key, _value|
-  json.set! "#{key}_url", image.attachment.url(key)
+  json.set! "#{key}_url", image.url(key)
 end
diff --git a/api/spec/requests/spree/api/images_controller_spec.rb b/api/spec/requests/spree/api/images_controller_spec.rb
index fe9ca029d49..74ad540ec81 100644
--- a/api/spec/requests/spree/api/images_controller_spec.rb
+++ b/api/spec/requests/spree/api/images_controller_spec.rb
@@ -74,6 +74,16 @@ module Spree
           expect(response.status).to eq(204)
           expect { product_image.reload }.to raise_error(ActiveRecord::RecordNotFound)
         end
+
+        it "It returns nil attribute values and noimage urls when the image cannot be found",
+            if: Spree::Config.image_attachment_module == Spree::Image::ActiveStorageAttachment do
+          product_image.attachment.blob.update(key: 11)
+          get spree.api_variant_images_path(product.master.id)
+          expect(response.status).to eq(200)
+          expect(json_response[:images].first[:attachment_width]).to be_nil
+          expect(json_response[:images].first[:attachment_height]).to be_nil
+          expect(json_response[:images].first[:product_url]).to include("noimage")
+        end
       end
 
       context 'when image belongs to another product' do
diff --git a/core/app/models/concerns/spree/active_storage_adapter/attachment.rb b/core/app/models/concerns/spree/active_storage_adapter/attachment.rb
index d78587cea70..b20d2110626 100644
--- a/core/app/models/concerns/spree/active_storage_adapter/attachment.rb
+++ b/core/app/models/concerns/spree/active_storage_adapter/attachment.rb
@@ -35,11 +35,11 @@ def variant(style = nil)
       end
 
       def height
-        metadata[:height]
+        metadata["height"]
       end
 
       def width
-        metadata[:width]
+        metadata["width"]
       end
 
       def destroy
@@ -53,8 +53,9 @@ def destroy
 
       def metadata
         analyze unless analyzed?
-
         @attachment.metadata
+      rescue ActiveStorage::FileNotFoundError
+        {"identified"=>nil, "width"=>nil, "height"=>nil, "analyzed"=>false}
       end
 
       def normalize_styles(styles)