From a3aa5f6b96f213523aa09764aafc2bb28d49656d Mon Sep 17 00:00:00 2001 From: Connor Ferguson <68167430+cpfergus1@users.noreply.github.com> Date: Thu, 25 Aug 2022 10:19:29 -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. --- api/app/views/spree/api/images/_image.json.jbuilder | 2 +- api/spec/requests/spree/api/images_spec.rb | 11 +++++++++++ .../spree/active_storage_adapter/attachment.rb | 6 +++++- 3 files changed, 17 insertions(+), 2 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_spec.rb b/api/spec/requests/spree/api/images_spec.rb index 52c6a473aa3..0a8d02d48e1 100644 --- a/api/spec/requests/spree/api/images_spec.rb +++ b/api/spec/requests/spree/api/images_spec.rb @@ -74,6 +74,17 @@ module Spree::Api expect(response.status).to eq(204) expect { product_image.reload }.to raise_error(ActiveRecord::RecordNotFound) end + + 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) + expect(Rails.logger).to receive(:error).with(/Image is corrupted or cannot be found/).twice + 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 b2a63a6bc23..3ff9ecbb6a7 100644 --- a/core/app/models/concerns/spree/active_storage_adapter/attachment.rb +++ b/core/app/models/concerns/spree/active_storage_adapter/attachment.rb @@ -57,8 +57,12 @@ def destroy def metadata analyze unless analyzed? - + @attachment.metadata + rescue ActiveStorage::FileNotFoundError => error + logger.error("#{error} - Image is corrupted or cannot be found") + + { identified: nil, width: nil, height: nil, analyzed: true } end def styles_to_transformations(styles)