Prefer the intersection of file command and MIME::Types

lib/paperclip/content_type_detector.rb

@@ -2,22 +2,21 @@ module Paperclip
   class ContentTypeDetector
     # The content-type detection strategy is as follows:
     #
-    # 1. Blank/Empty files: If there's no filename or the file is empty, provide a sensible default
-    #    (application/octet-stream or inode/x-empty)
+    # 1. Blank/Empty files: If there's no filename or the file is empty,
+    #    provide a sensible default (application/octet-stream or inode/x-empty)
     #
-    # 2. Uploaded file: Use the uploaded file's content type if it is in the list of mime-types
-    #    for the file's extension
+    # 2. Calculated match: Return the first result that is found by both the
+    #    `file` command and MIME::Types.
     #
-    # 3. Standard types: Return the first standard (without an x- prefix) entry in the list of
-    #    mime-types
+    # 3. Standard types: Return the first standard (without an x- prefix) entry
+    #    in MIME::Types
     #
-    # 4. Experimental types: If there were no standard types in the mime-types list, try to return
-    #    the first experimental one
+    # 4. Experimental types: If there were no standard types in MIME::Types
+    #    list, try to return the first experimental one
     #
-    # 5. Unrecognized extension: Use the file's content type or a sensible default if there are
-    #    no entries in mime-types for the extension
-    #
-    
+    # 5. Raw `file` command: Just use the output of the `file` command raw, or
+    #    a sensible default. This is cached from Step 2.
+
     EMPTY_TYPE = "inode/x-empty"
     SENSIBLE_DEFAULT = "application/octet-stream"
 
@@ -31,12 +30,12 @@ module Paperclip
         SENSIBLE_DEFAULT
       elsif empty_file?
         EMPTY_TYPE
-      elsif types_matching_file.any?
-        types_matching_file.first
-      elsif official_types.any?
-        official_types.first
-      elsif possible_types.any?
-        possible_types.first
+      elsif calculated_type_matches.any?
+        calculated_type_matches.first
+      elsif official_type_matches.any?
+        official_type_matches.first
+      elsif unofficial_type_matches.any?
+        unofficial_type_matches.first
       else
         type_from_file_command || SENSIBLE_DEFAULT
       end.to_s
@@ -51,7 +50,7 @@ module Paperclip
     def blank_name?
       @filename.nil? || @filename.empty?
     end
-    
+
     def empty?
       File.exists?(@filename) && File.size(@filename) == 0
     end
@@ -59,17 +58,21 @@ module Paperclip
     def possible_types
       MIME::Types.type_for(@filename).collect(&:content_type)
     end
-    
-    def official_types
+
+    def calculated_type_matches
+      possible_types.select{|content_type| content_type == type_from_file_command }
+    end
+
+    def official_type_matches
       possible_types.reject{|content_type| content_type.match(/\/x-/) }
     end
-    
-    def types_matching_file
-      possible_types.select{|content_type| content_type == type_from_file_command}
+
+    def unofficial_type_matches
+      possible_types.select{|content_type| content_type.match(/\/x-/) }
     end
 
     def type_from_file_command
-      FileCommandContentTypeDetector.new(@filename).detect
+      @type_from_file_command ||= FileCommandContentTypeDetector.new(@filename).detect
     end
   end
 end

test/content_type_detector_test.rb

 require './test/helper'
 
 class ContentTypeDetectorTest < Test::Unit::TestCase
-  context 'given a name' do
-    should 'return a content type based on that name' do
-      @filename = "/path/to/something.jpg"
-      assert_equal "image/jpeg", Paperclip::ContentTypeDetector.new(@filename).detect
-    end
-
-    should 'return a content type based on the content of the file' do
-      tempfile = Tempfile.new("something")
-      tempfile.write("This is a file.")
-      tempfile.rewind
+  should 'give a sensible default when the name is empty' do
+    assert_equal "application/octet-stream", Paperclip::ContentTypeDetector.new("").detect
+  end
 
-      assert_equal "text/plain", Paperclip::ContentTypeDetector.new(tempfile.path).detect
-    end
+  should 'return the empty content type when the file is empty' do
+    tempfile = Tempfile.new("empty")
+    assert_equal "inode/x-empty", Paperclip::ContentTypeDetector.new(tempfile.path).detect
+  end
 
-    should 'return content type of file if it is an acceptable type' do
-      MIME::Types.stubs(:type_for).returns([MIME::Type.new('application/mp4'), MIME::Type.new('video/mp4'), MIME::Type.new('audio/mp4')])
-      Paperclip.stubs(:run).returns("video/mp4")
-      
-      @filename = "my_file.mp4"
-      assert_equal "video/mp4", Paperclip::ContentTypeDetector.new(@filename).detect
-    end
+  should 'return content type of file if it is an acceptable type' do
+    MIME::Types.stubs(:type_for).returns([MIME::Type.new('application/mp4'), MIME::Type.new('video/mp4'), MIME::Type.new('audio/mp4')])
+    Paperclip.stubs(:run).returns("video/mp4")
+    @filename = "my_file.mp4"
+    assert_equal "video/mp4", Paperclip::ContentTypeDetector.new(@filename).detect
+  end
 
-    should 'return an empty content type if the file is empty' do
-      tempfile = Tempfile.new("something")
-      tempfile.rewind
+  should 'find the first result that matches from the official types' do
+    @filename = "/path/to/something.bmp"
+    assert_equal "image/bmp", Paperclip::ContentTypeDetector.new(@filename).detect
+  end
 
-      assert_equal "inode/x-empty", Paperclip::ContentTypeDetector.new(tempfile.path).detect
-    end
+  should 'find the first unofficial result for this filename if no official ones exist' do
+    @filename = "/path/to/something.aiff"
+    assert_equal "audio/x-aiff", Paperclip::ContentTypeDetector.new(@filename).detect
+  end
 
-    should 'return a sensible default if no filename is supplied' do
-      assert_equal "application/octet-stream", Paperclip::ContentTypeDetector.new('').detect
+  should 'find the right type in the list via the file command' do
+    @filename = "#{Dir.tmpdir}/something.hahalolnotreal"
+    File.open(@filename, "w+") do |file|
+      file.puts "This is a text file."
+      file.rewind
+      assert_equal "text/plain", Paperclip::ContentTypeDetector.new(file.path).detect
     end
+    FileUtils.rm @filename
+  end
 
-    should 'return a sensible default if something goes wrong' do
-      @filename = "/path/to/something"
-      assert_equal "application/octet-stream", Paperclip::ContentTypeDetector.new(@filename).detect
-    end
+  should 'return a sensible default if something is wrong, like the file is gone' do
+    @filename = "/path/to/nothing"
+    assert_equal "application/octet-stream", Paperclip::ContentTypeDetector.new(@filename).detect
+  end
 
-    should 'return a sensible default when the file command is missing' do
-      Paperclip.stubs(:run).raises(Cocaine::CommandLineError.new)
-      @filename = "/path/to/something"
-      assert_equal "application/octet-stream", Paperclip::ContentTypeDetector.new(@filename).detect
-    end
+  should 'return a sensible default when the file command is missing' do
+    Paperclip.stubs(:run).raises(Cocaine::CommandLineError.new)
+    @filename = "/path/to/something"
+    assert_equal "application/octet-stream", Paperclip::ContentTypeDetector.new(@filename).detect
   end
 end