Merge pull request #1305 from tvdeyen/handle-picture-errors

Handle custom errors in Alchemy::Picture#url

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Change Log
 
+## 3.6.2 (unreleased)
+
+* Handle custom errors in `Alchemy::Picture#url` [#1305](https://github.com/AlchemyCMS/alchemy_cms/pull/1305) by [tvdeyen](https://github.com/tvdeyen)
+
 ## 3.6.1 (2017-08-16)
 
 * Do not ask `systempage?` everytime we load the page definition [#1239](https://github.com/AlchemyCMS/alchemy_cms/pull/1283) by [tvdeyen](https://github.com/tvdeyen)

app/models/alchemy/picture/url.rb

@@ -1,5 +1,7 @@
 module Alchemy
   module Picture::Url
+    include Alchemy::Logger
+
     TRANSFORMATION_OPTIONS = [
       :crop,
       :crop_from,
@@ -28,6 +30,9 @@ def url(options = {})
       image = encoded_image(image, options)
 
       image.url(options.except(*TRANSFORMATION_OPTIONS).merge(name: name))
+    rescue MissingImageFileError, WrongImageFormatError => e
+      log_warning e.message
+      nil
     end
 
     private
@@ -53,7 +58,10 @@ def processed_image(image, options = {})
     #
     def encoded_image(image, options = {})
       target_format = options[:format] || default_render_format
-      raise WrongImageFormatError if !target_format.in?(Alchemy::Picture.allowed_filetypes)
+
+      unless target_format.in?(Alchemy::Picture.allowed_filetypes)
+        raise WrongImageFormatError.new(self, target_format)
+      end
 
       options = {
         flatten: target_format != 'gif' && image_file_format == 'gif'

lib/alchemy/errors.rb

@@ -55,9 +55,14 @@ class MissingImageFileError < StandardError; end
 
   # Raised if calling +image_file+ on a Picture object returns nil.
   class WrongImageFormatError < StandardError
+    def initialize(image, requested_format)
+      @image = image
+      @requested_format = requested_format
+    end
+
     def message
       allowed_filetypes = Alchemy::Picture.allowed_filetypes.map(&:upcase).to_sentence
-      "Requested image format is not one of allowed filetypes (#{allowed_filetypes})."
+      "Requested image format (#{@requested_format.inspect}) for #{@image.inspect} is not one of allowed filetypes (#{allowed_filetypes})."
     end
   end
 

lib/alchemy/logger.rb

@@ -7,7 +7,7 @@ def self.warn(message, caller_string)
     end
 
     def log_warning(message)
-      Alchemy::Logger.warn(message, caller(0..0))
+      Alchemy::Logger.warn(message, caller(1..1))
     end
   end
 end

spec/libraries/logger_spec.rb

@@ -0,0 +1,35 @@
+require 'spec_helper'
+
+RSpec.describe Alchemy::Logger do
+  let(:message) { "Something bad happened" }
+
+  describe '.warn' do
+    let(:caller_string) { "file.rb:14" }
+
+    subject { Alchemy::Logger.warn(message, caller_string) }
+
+    it { is_expected.to be_nil }
+
+    it 'uses Rails debug logger' do
+      expect(Rails.logger).to receive(:debug) { message }
+      subject
+    end
+  end
+
+  describe '#log_warning' do
+    class Something
+      include Alchemy::Logger
+    end
+
+    subject { Something.new.log_warning(message) }
+
+    before do
+      expect_any_instance_of(Something).to receive(:caller).with(1..1) { ["second"] }
+    end
+
+    it 'delegates to Alchemy::Logger.warn class method with second line of callstack' do
+      expect(Alchemy::Logger).to receive(:warn).with(message, ["second"])
+      subject
+    end
+  end
+end

spec/models/alchemy/picture_url_spec.rb

@@ -35,8 +35,13 @@ def decode_dragon_fly_job(url)
           expect(picture).to receive(:image_file) { nil }
         end
 
-        it 'raises error' do
-          expect { url }.to raise_error(Alchemy::MissingImageFileError)
+        it 'returns nil' do
+          expect(url).to be_nil
+        end
+
+        it "logs warning" do
+          expect(Alchemy::Logger).to receive(:warn)
+          url
         end
       end
 
@@ -171,8 +176,13 @@ def decode_dragon_fly_job(url)
           {format: 'zip'}
         end
 
-        it "raises error" do
-          expect { url }.to raise_error(Alchemy::WrongImageFormatError)
+        it "returns nil" do
+          expect(url).to be_nil
+        end
+
+        it "logs warning" do
+          expect(Alchemy::Logger).to receive(:warn)
+          url
         end
       end