From 181bc3ab50e8b39768f6320247502e52b54e86ae Mon Sep 17 00:00:00 2001
From: Konrad <konradnarewski@gmail.com>
Date: Tue, 10 Dec 2024 12:52:44 +0100
Subject: [PATCH] feat: explicitly raise error when trying to use unsupported
 operation

That should prevent accidental, silent omission of a desired transformation
---
 CHANGELOG.md                        |  1 +
 lib/morandi.rb                      |  6 +++++-
 lib/morandi/vips_image_processor.rb | 12 ++++++++++++
 spec/morandi_spec.rb                | 29 +++++++++++++++++++++++++++++
 4 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fa5d57e..1c6cc28 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Vips straighten
 - Vips gamma
 - Vips stripping alpha
+- Explicit error when trying to use VipsProcessor with unsupported options
 
 ### Removed
 - [BREAKING] dropped support for a broken 'dominant' border colour
diff --git a/lib/morandi.rb b/lib/morandi.rb
index 0bebc18..62e1b42 100644
--- a/lib/morandi.rb
+++ b/lib/morandi.rb
@@ -44,10 +44,14 @@ module Morandi
   # @param target_path [String] target location for image
   # @param local_options [Hash] Hash of options other than desired transformations
   # @option local_options [String] 'path.icc' A path to store the input after converting to sRGB colour space
-  # @options local_options [String] 'processor' ('pixbuf') Name of the image processing library ('pixbuf', 'vips')
+  # @option local_options [String] 'processor' ('pixbuf') Name of the image processing library ('pixbuf', 'vips')
+  #                                                       NOTE: vips processor only handles subset of operations,
+  #                                                       see `Morandi::VipsImageProcessor.supports?` for details
   def process(source, options, target_path, local_options = {})
     case local_options['processor']
     when 'vips'
+      raise(ArgumentError, 'Requested unsupported Vips operation') unless VipsImageProcessor.supports?(source, options)
+
       # Cache saves time in expense of RAM when performing the same processing multiple times
       # Cache is also created for files based on their names, which can lead to leaking files data, so in terms
       # of security it feels prudent to disable it. Latest libvips supports "revalidate" option to prevent that risk
diff --git a/lib/morandi/vips_image_processor.rb b/lib/morandi/vips_image_processor.rb
index 706a70e..349b758 100644
--- a/lib/morandi/vips_image_processor.rb
+++ b/lib/morandi/vips_image_processor.rb
@@ -18,6 +18,18 @@ class VipsImageProcessor
     }.freeze
     SUPPORTED_FILTERS = COLOUR_FILTER_MODIFIERS.keys + ['greyscale']
 
+    def self.supports?(input, options)
+      return false unless input.is_a?(String)
+      return false if options['brighten'].to_f != 0
+      return false if options['contrast'].to_f != 0
+      return false if options['sharpen'].to_f != 0
+      return false if options['redeye']&.any?
+      return false if options['border-style']
+      return false if options['background-style']
+
+      true
+    end
+
     # Vips options are global, this method sets them for yielding, then restores to original
     def self.with_global_options(cache_max:, concurrency:)
       previous_cache_max = Vips.cache_max
diff --git a/spec/morandi_spec.rb b/spec/morandi_spec.rb
index 4eb044e..4191866 100644
--- a/spec/morandi_spec.rb
+++ b/spec/morandi_spec.rb
@@ -750,6 +750,35 @@
   end
 
   context 'vips processor' do
+    subject(:process_image) do
+      Morandi.process(file_arg, options, file_out, 'processor' => 'vips')
+    end
+
     it_behaves_like 'an image processor', 'vips'
+
+    context 'with pixbuf object given as input' do
+      let(:file_arg) { GdkPixbuf::Pixbuf.new(file: file_in) }
+
+      it 'raises ArgumentError' do
+        expect { process_image }.to raise_error(ArgumentError, 'Requested unsupported Vips operation')
+      end
+    end
+
+    {
+      'brighten' => 1.0,
+      'contrast' => 1.0,
+      'sharpen' => 1.0,
+      'redeye' => [[540, 650]],
+      'border-style' => 'square',
+      'background-style' => 'retro'
+    }.each do |option_name, option_value|
+      context "with unsupported operation `#{option_name}` requested" do
+        let(:options) { { option_name => option_value } }
+
+        it 'raises ArgumentError' do
+          expect { process_image }.to raise_error(ArgumentError, 'Requested unsupported Vips operation')
+        end
+      end
+    end
   end
 end