Extract geometry parsing into factories

lib/paperclip.rb

@@ -29,6 +29,8 @@ require 'erb'
 require 'digest'
 require 'tempfile'
 require 'paperclip/version'
+require 'paperclip/geometry_parser_factory'
+require 'paperclip/geometry_detector_factory'
 require 'paperclip/geometry'
 require 'paperclip/processor'
 require 'paperclip/tempfile'

lib/paperclip/geometry.rb

@@ -2,39 +2,41 @@ module Paperclip
 
   # Defines the geometry of an image.
   class Geometry
-    attr_accessor :height, :width, :modifier
+    attr_accessor :height, :width, :modifier, :orientation
 
     # Gives a Geometry representing the given height and width
-    def initialize width = nil, height = nil, modifier = nil
-      @height = height.to_f
-      @width  = width.to_f
-      @modifier = modifier
+    def initialize(width = nil, height = nil, modifier = nil)
+      if width.is_a?(Hash)
+        options = width
+        @height = options[:height].to_f
+        @width = options[:width].to_f
+        @modifier = options[:modifier]
+        @orientation = options[:orientation].to_i
+      else
+        @height = height.to_f
+        @width  = width.to_f
+        @modifier = modifier
+      end
     end
 
     # Uses ImageMagick to determing the dimensions of a file, passed in as either a
     # File or path.
     # NOTE: (race cond) Do not reassign the 'file' variable inside this method as it is likely to be
     # a Tempfile object, which would be eligible for file deletion when no longer referenced.
-    def self.from_file file
-      file_path = file.respond_to?(:path) ? file.path : file
-      raise(Errors::NotIdentifiedByImageMagickError.new("Cannot find the geometry of a file with a blank name")) if file_path.blank?
-      geometry = begin
-                   silence_stream(STDERR) do
-                     Paperclip.run("identify", "-format %wx%h :file", :file => "#{file_path}[0]")
-                   end
-                 rescue Cocaine::ExitStatusError
-                   ""
-                 rescue Cocaine::CommandNotFoundError => e
-                   raise Errors::CommandNotFoundError.new("Could not run the `identify` command. Please install ImageMagick.")
-                 end
-      parse(geometry) ||
-        raise(Errors::NotIdentifiedByImageMagickError.new("#{file_path} is not recognized by the 'identify' command."))
+    def self.from_file(file)
+      GeometryDetectorFactory.new(file).make
     end
 
     # Parses a "WxH" formatted string, where W is the width and H is the height.
-    def self.parse string
-      if match = (string && string.match(/\b(\d*)x?(\d*)\b([\>\<\#\@\%^!])?/i))
-        Geometry.new(*match[1,3])
+    def self.parse(string)
+      GeometryParserFactory.new(string).make
+    end
+
+    # Swaps the height and width if necessary
+    def auto_orient
+      if [5, 6, 7, 8].include?(@orientation)
+        @height, @width = @width, @height
+        @orientation -= 4
       end
     end
 

lib/paperclip/geometry_detector_factory.rb

+module Paperclip
+  class GeometryDetectorFactory
+    def initialize(file)
+      @file = file
+      raise_if_blank_file
+    end
+
+    def make
+      GeometryParserFactory.new(geometry_string.strip).make ||
+        raise(Errors::NotIdentifiedByImageMagickError.new)
+    end
+
+    private
+
+    def path
+      @file.respond_to?(:path) ? @file.path : @file
+    end
+
+    def geometry_string
+      begin
+        silence_stream(STDERR) do
+          Paperclip.run("identify", "-format '%wx%h,%[exif:orientation]' :file", :file => "#{path}[0]")
+        end
+      rescue Cocaine::ExitStatusError
+        ""
+      rescue Cocaine::CommandNotFoundError => e
+        raise_because_imagemagick_missing
+      end
+    end
+
+    def raise_if_blank_file
+      if path.blank?
+        raise Errors::NotIdentifiedByImageMagickError.new("Cannot find the geometry of a file with a blank name")
+      end
+    end
+
+    def raise_because_imagemagick_missing
+      raise Errors::CommandNotFoundError.new("Could not run the `identify` command. Please install ImageMagick.")
+    end
+  end
+end

lib/paperclip/geometry_parser_factory.rb

+module Paperclip
+  class GeometryParserFactory
+    FORMAT = /\b(\d*)x?(\d*)\b(?:,(\d?))?([\>\<\#\@\%^!])?/i
+    def initialize(string)
+      @string = string
+    end
+
+    def make
+      if match
+        Geometry.new(
+          :height => @height,
+          :width => @width,
+          :modifier => @modifier,
+          :orientation => @orientation
+        )
+      end
+    end
+
+    private
+
+    def match
+      @height = nil
+      @width = nil
+      @modifier = nil
+      @orientation = nil
+      if actual_match = @string && @string.match(FORMAT)
+        @width = actual_match[1]
+        @height = actual_match[2]
+        @orientation = actual_match[3]
+        @modifier = actual_match[4]
+      end
+      actual_match
+    end
+  end
+end

test/attachment_test.rb

@@ -784,9 +784,7 @@ class AttachmentTest < Test::Unit::TestCase
   context "Assigning an attachment" do
     setup do
       rebuild_model :styles => { :something => "100x100#" }
-      @file = StringIO.new(".")
-      @file.stubs(:original_filename).returns("5k.png\n\n")
-      @file.stubs(:content_type).returns("image/png\n\n")
+      @file = File.new(fixture_file("5k.png"), "rb")
       @dummy = Dummy.new
       @dummy.avatar = @file
     end
@@ -803,9 +801,7 @@ class AttachmentTest < Test::Unit::TestCase
   context "Assigning an attachment" do
     setup do
       rebuild_model :styles => { :something => "100x100#" }
-      @file = StringIO.new(".")
-      @file.stubs(:original_filename).returns("5k.png\n\n")
-      @file.stubs(:content_type).returns(MIME::Type.new("image/png"))
+      @file = File.new(fixture_file("5k.png"), "rb")
       @dummy = Dummy.new
       @dummy.avatar = @file
     end
@@ -818,11 +814,8 @@ class AttachmentTest < Test::Unit::TestCase
   context "Attachment with strange letters" do
     setup do
       rebuild_model
-
-      @file  = StringIO.new(".")
-      @file.stubs(:original_filename).returns("sheep_say_bæ.png\r\n")
-      @file.stubs(:content_type).returns("image/png\r\n")
-
+      @file = File.new(fixture_file("5k.png"), "rb")
+      @file.stubs(:original_filename).returns("sheep_say_bæ.png")
       @dummy = Dummy.new
       @dummy.avatar = @file
     end

test/geometry_detector_factory_test.rb

+require './test/helper'
+
+class GeometryDetectorFactoryTest < Test::Unit::TestCase
+  should 'identify an image and extract its dimensions' do
+    Paperclip::GeometryParserFactory.stubs(:new).with("434x66,").returns(stub(:make => :correct))
+    file = fixture_file("5k.png")
+    factory = Paperclip::GeometryDetectorFactory.new(file)
+
+    output = factory.make
+
+    assert_equal :correct, output
+  end
+
+  should 'identify an image and extract its dimensions and orientation' do
+    Paperclip::GeometryParserFactory.stubs(:new).with("300x200,6").returns(stub(:make => :correct))
+    file = fixture_file("rotated.jpg")
+    factory = Paperclip::GeometryDetectorFactory.new(file)
+
+    output = factory.make
+
+    assert_equal :correct, output
+  end
+end
+

test/geometry_parser_factory_test.rb

+require './test/helper'
+
+class GeometryParserFactoryTest < Test::Unit::TestCase
+  should 'identify an image and extract its dimensions with no orientation' do
+    Paperclip::Geometry.stubs(:new).with(
+      :height => '73',
+      :width => '434',
+      :modifier => nil,
+      :orientation => nil
+    ).returns(:correct)
+    factory = Paperclip::GeometryParserFactory.new("434x73")
+
+    output = factory.make
+
+    assert_equal :correct, output
+  end
+
+  should 'identify an image and extract its dimensions with an empty orientation' do
+    Paperclip::Geometry.stubs(:new).with(
+      :height => '73',
+      :width => '434',
+      :modifier => nil,
+      :orientation => ''
+    ).returns(:correct)
+    factory = Paperclip::GeometryParserFactory.new("434x73,")
+
+    output = factory.make
+
+    assert_equal :correct, output
+  end
+
+  should 'identify an image and extract its dimensions and orientation' do
+    Paperclip::Geometry.stubs(:new).with(
+      :height => '200',
+      :width => '300',
+      :modifier => nil,
+      :orientation => '6'
+    ).returns(:correct)
+    factory = Paperclip::GeometryParserFactory.new("300x200,6")
+
+    output = factory.make
+
+    assert_equal :correct, output
+  end
+
+  should 'identify an image and extract its dimensions and modifier' do
+    Paperclip::Geometry.stubs(:new).with(
+      :height => '64',
+      :width => '64',
+      :modifier => '#',
+      :orientation => nil
+    ).returns(:correct)
+    factory = Paperclip::GeometryParserFactory.new("64x64#")
+
+    output = factory.make
+
+    assert_equal :correct, output
+  end
+
+  should 'identify an image and extract its dimensions, orientation, and modifier' do
+    Paperclip::Geometry.stubs(:new).with(
+      :height => '50',
+      :width => '100',
+      :modifier => '>',
+      :orientation => '7'
+    ).returns(:correct)
+    factory = Paperclip::GeometryParserFactory.new("100x50,7>")
+
+    output = factory.make
+
+    assert_equal :correct, output
+  end
+end
+
+

test/geometry_test.rb

@@ -49,6 +49,29 @@ class GeometryTest < Test::Unit::TestCase
       assert_nil @geo.modifier
     end
 
+    should "recognize an EXIF orientation and not rotate with auto_orient if not necessary" do
+      assert @geo = Paperclip::Geometry.new(:width => 1024, :height => 768, :orientation => 1)
+      assert_equal 1024, @geo.width
+      assert_equal 768, @geo.height
+
+      @geo.auto_orient
+
+      assert_equal 1024, @geo.width
+      assert_equal 768, @geo.height
+    end
+
+    should "recognize an EXIF orientation and rotate with auto_orient if necessary" do
+      assert @geo = Paperclip::Geometry.new(:width => 1024, :height => 768, :orientation => 6)
+      assert_equal 1024, @geo.width
+      assert_equal 768, @geo.height
+
+      @geo.auto_orient
+
+      assert_equal 768, @geo.width
+      assert_equal 1024, @geo.height
+      assert_equal 2, @geo.orientation
+    end
+
     should "treat x and X the same in geometries" do
       @lower = Paperclip::Geometry.parse("123x456")
       @upper = Paperclip::Geometry.parse("123X456")
@@ -104,15 +127,15 @@ class GeometryTest < Test::Unit::TestCase
       file = fixture_file("5k.png")
       file = File.new(file, 'rb')
       assert_nothing_raised{ @geo = Paperclip::Geometry.from_file(file) }
-      assert @geo.height > 0
-      assert @geo.width > 0
+      assert_equal 73, @geo.height
+      assert_equal 434, @geo.width
     end
 
     should "be generated from a file path" do
       file = fixture_file("5k.png")
       assert_nothing_raised{ @geo = Paperclip::Geometry.from_file(file) }
-      assert @geo.height > 0
-      assert @geo.width > 0
+      assert_equal 73, @geo.height
+      assert_equal 434, @geo.width
     end
 
     should "not generate from a bad file" do

test/helper.rb

@@ -169,7 +169,7 @@ def with_exitstatus_returning(code)
 end
 
 def fixture_file(filename)
- File.join(File.dirname(__FILE__), 'fixtures', filename)
+  File.join(File.dirname(__FILE__), 'fixtures', filename)
 end
 
 def assert_success_response(url)