Changeset 580

Timestamp:

07/02/08 13:55:58 (3 months ago)

Author:

mgranger

Message:

• Image plugin:
  
  • Updated to generate and append thumbnails.
    
  • Refactored the calculation of the supported formats.
    
• MP3 plugin:
  
  • Modified to accept 'audio/mpg' and 'audio/mp3' as well as 'audio/mpeg'.
    
  • Added some diagnostic logging for album-art extraction
    
• Fixed a bug in the formupload handler's template when uploaded files don't have an associated
  title.
  
• Wrapped Rakefile tasklib loading in better diagnostics
  
• Renamed the 'rdoc' tasklib to 'docs' and moved the manual task to that library to eliminate the
  dependency on 'uv' for non-manual-related tasks.
  
• Added coverage to the spec for ThingFish::Request and made #check_body_ios check appended
  resource IOs as well as entity bodies.
  
• Added a 'bench' shortcut for 'benchmarks:all' to the benchmark tasklib
  
• Fixed a typo in ThingFish::Client API docs.
  
• Changed the way recursion works in ThingFish::Request#yield_each_resource -- no longer (needs)
  uses Enumerable::Enumerator, and eliminated newly-appended resources from sometimes being
  yielded back to the block. Thanks to Jeremiah Chase for pairing with me while tracking this
  down.
  

Files:

• thingfish/trunk/Rakefile (modified) (2 diffs)
• thingfish/trunk/lib/thingfish/client.rb (modified) (1 diff)
• thingfish/trunk/lib/thingfish/request.rb (modified) (4 diffs)
• thingfish/trunk/misc/rake/benchmark.rb (modified) (1 diff)
• thingfish/trunk/misc/rake/docs.rb (moved) (moved from thingfish/trunk/misc/rake/rdoc.rb) (3 diffs)
• thingfish/trunk/plugins/thingfish-filestore-filesystem/lib/thingfish/filestore/filesystem.rb (modified) (2 diffs)
• thingfish/trunk/plugins/thingfish-filter-image/lib/thingfish/filter/image.rb (modified) (7 diffs)
• thingfish/trunk/plugins/thingfish-filter-image/spec/thingfish/filter/image_spec.rb (modified) (1 diff)
• thingfish/trunk/plugins/thingfish-filter-mp3/lib/thingfish/filter/mp3.rb (modified) (5 diffs)
• thingfish/trunk/plugins/thingfish-handler-formupload/resources/upload.rhtml (modified) (1 diff)
• thingfish/trunk/spec/thingfish/request_spec.rb (modified) (9 diffs)

thingfish/trunk/Rakefile

@@ -89 +89 @@
-end
-
- Load task plugin
+## Load task librarie
-require RAKE_TASKDIR + 'svn.rb'
-require RAKE_TASKDIR + 'verifytask.rb'
-Pathname.glob( RAKE_TASKDIR + '*.rb' ).each do |tasklib|
-
-
+
+
+
+
+
+
+
+
+
+
+
+
-end
+
-
-### Default task
@@ -115 +126 @@
-multitask :docs => [ :manual, :coverage, :rdoc ] do
-    log "All documentation built."
-end
-
-
-### Task: manual
-require 'misc/rake/lib/manual'
-
-directory MANUALOUTPUTDIR.to_s
-directory RDOCDIR.to_s
-
-Manual::GenTask.new do |manual|
-    manual.metadata.version = PKG_VERSION
-    manual.metadata.api_dir = RDOCDIR
-    manual.output_dir = MANUALOUTPUTDIR
-    manual.base_dir = MANUALDIR
-    manual.source_dir = 'src'
-end
-
-task :clobber_manual do
-    rmtree( MANUALOUTPUTDIR, :verbose => true )
-end
-

thingfish/trunk/lib/thingfish/client.rb

@@ -141 +141 @@
-
-    ### Create a new ThingFish client object which will interact with the
-an
+nam
-    ### the IP address, or the URI of the server. Also set any options
-    ### specified as attributes on the client.

thingfish/trunk/lib/thingfish/request.rb

@@ -326 +326 @@
-    ### metadata required for the filestore and metastore themselves.
-    def each_body( include_appended=false, &block )  # :yields: body_io, metadata_hash
-
-
-
-
+
-    end
-    
@@ -348 +345 @@
-    ### Check the body IO objects to ensure they're still open.
-    def check_body_ios
-
+( true )
-            if body.closed?
-                self.log.warn "Body IO unexpectedly closed -- reopening a new handle"
-                                
-                # Create a new IO based on what the original type was
-case body
-   when StringIO
-   
-   
-   
-   
+nil
+if body.is_a?( StringIO )
+clone = 
+
+clone = 
+
-                
-
+file and appended 
-                @entity_bodies[ clone ] = @entity_bodies.delete( body ) if @entity_bodies.key?( body )
+                @metadata[ clone ] = @metadata.delete( body ) if @metadata.key?( body )
+                
+                # Splice in the cloned IO into both keys and values of the related resources
+                # tree
-                @related_resources[ clone ] = @related_resources.delete( body ) if @related_resources.key?( body )
-                @metadata[ clone ] = @metadata.delete( body ) if @metadata.key?( body )
-                @related_resources.each do |_,hash|
-                    hash[ clone ] = hash.delete( body ) if hash.key?( body )
-                end
-
-
-
-
+
+
-            else
-                body.rewind
@@ -539 +538 @@
-    ### toplevel metadata with the resource-specific metadata and pass both to the
-    ### block.
-iterator
+resources
-        immutable_metadata = self.get_immutable_metadata
+        self.log.debug "Yielding for resources: %p" % [ resources ]
-        
-        # Call the block for every resource
-
+
+
+
+
+
-            body_metadata[ :format ] ||= DEFAULT_CONTENT_TYPE
-            extracted_metadata = self.metadata[body] || {}
@@ -555 +559 @@
-            
-            # Recurse if the appended resources should be included
-
-
-
+
+
-            end
-        end

thingfish/trunk/misc/rake/benchmark.rb

@@ -91 +91 @@
-
-task :benchmarks => [ 'benchmarks:all' ]
+task :bench => :benchmarks
-
-

thingfish/trunk/misc/rake/docs.rb

@@ -5 +5 @@
-
-require 'rake/rdoctask'
+
+# Try to require Darkfish for rdoc, but don't mandate it
+begin
+    require 'rubygems'
+    gem 'darkfish-rdoc'
+rescue LoadError; end # (ignored)
+
+begin
+    require 'rdoc/generator/darkfish'
+rescue LoadError; end # (ignored)
+
+directory RDOCDIR.to_s
-
-### Task: rdoc
@@ -15 +27 @@
-        '-SHN',
-        '-i', BASEDIR.to_s,
-        '-f', 'darkfish',
-        '-m', 'README',
-        '-W', 'http://opensource.laika.com/browser/thingfish/trunk/'
-      ]
+
+    rdoc.options += [ '-f', 'darkfish' ] 
-    
-    rdoc.rdoc_files.include 'README'
@@ -28 +41 @@
-end
-
+
+### Task: manual generation
+begin
+    require 'misc/rake/lib/manual'
+
+    directory MANUALOUTPUTDIR.to_s
+
+    Manual::GenTask.new do |manual|
+        manual.metadata.version = PKG_VERSION
+        manual.metadata.api_dir = RDOCDIR
+        manual.output_dir = MANUALOUTPUTDIR
+        manual.base_dir = MANUALDIR
+        manual.source_dir = 'src'
+    end
+
+    task :clobber_manual do
+        rmtree( MANUALOUTPUTDIR, :verbose => true )
+    end
+
+rescue LoadError => err
+    task :no_manual do
+        $stderr.puts "Manual-generation tasks not defined: %s" % [ err.message ]
+    end
+
+    task :manual => :no_manual
+    task :clobber_manual => :no_manual
+end
+
+
+

thingfish/trunk/plugins/thingfish-filestore-filesystem/lib/thingfish/filestore/filesystem.rb

@@ -186 +186 @@
-        end
-        
+        self.log.info "Wrote %d bytes." % [ data.length ]
-        self.update_size( path, data.length )
-        return Digest::MD5.hexdigest( data )
@@ -214 +215 @@
-        end
-
+        self.log.info "Wrote %d bytes (buffered)." % [ uploadsize ]
-        self.update_size( path, uploadsize )
-        return digest.hexdigest

thingfish/trunk/plugins/thingfish-filter-image/lib/thingfish/filter/image.rb

@@ -51 +51 @@
-    # SVN Id
-    SVNId = %q$Id$
+    
+    # The default dimensions of thumbnails
+    DEFAULT_THUMBNAIL_DIMENSIONS = [ 100, 100 ]
-
-
@@ -117 +120 @@
-    ### Set up a new Filter object
-    def initialize( options={} ) # :notnew:
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
-        self.log.debug "Registered mimetype mapping for %d of %d support image types" %
-            [ @supported_formats.keys.length, Magick.formats.length ]
@@ -153 +136 @@
-            collect {|type, op| ThingFish::AcceptParam.parse(type) }
-        
+        @thumb_dimensions = options[:thumbnail_dimensions] || DEFAULT_THUMBNAIL_DIMENSIONS
+        raise ThingFish::ConfigError, "invalid thumbnail dimensions %p" % [ @thumb_dimensions ] unless
+            @thumb_dimensions.is_a?( Array ) && @thumb_dimensions.all? {|dim| dim.is_a?(Fixnum) }
+        self.log.debug "Thumbnail dimensions are: %p" % [ @thumb_dimensions ]
+        
+        @thumb_dimensions = options[:thumbnail_dimensions] || DEFAULT_THUMBNAIL_DIMENSIONS
+        raise ThingFish::ConfigError, "invalid thumbnail dimensions %p" % [ @thumb_dimensions ] unless
+            @thumb_dimensions.is_a?( Array ) && @thumb_dimensions.all? {|dim| dim.is_a?(Fixnum) }
+        self.log.debug "Thumbnail dimensions are: %p" % [ @thumb_dimensions ]
+        
-        super
-    end
@@ -175 +168 @@
-                      request.http_method == 'POST'
-
-
+
+
+
-            if self.accept?( metadata[:format] )
-
@@ -184 +179 @@
-                image = images.first
-                image_attributes = self.extract_metadata( image )
-            
-                request.append_metadata_for( body, image_attributes )
+                
+                thumbnail, thumb_metadata = self.create_thumbnail( image, metadata[:title] )
+                self.log.debug "Appending a thumbnail as a related resource"
+                request.append_related_resource( body, StringIO.new(thumbnail), thumb_metadata )
-            else
-                self.log.debug "Skipping unhandled file type (%s)" % [metadata[:format]]
@@ -244 +242 @@
-          }
-    end
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-    #######
-    private
@@ -309 +345 @@
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-end # class ThingFish::ImageFilter
-

thingfish/trunk/plugins/thingfish-filter-image/spec/thingfish/filter/image_spec.rb

@@ -104 +104 @@
-    
-    
-dimension metadata
+image metadata and thumbnail
-        @request.should_receive( :http_method ).at_least( :once ).and_return( 'POST' )
-        
-
+
+
-        Magick::Image.should_receive( :from_blob ).with( :imagedata ).and_return([ image ])
-
-
-
-
+
+
+
+
+
+
-        end
-        
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-
-        # Run the request filter

thingfish/trunk/plugins/thingfish-filter-mp3/lib/thingfish/filter/mp3.rb

@@ -76 +76 @@
-    
-    # The Array of types this filter is interested in
-
+
+
-    HANDLED_TYPES.freeze
-
@@ -162 +163 @@
-    ### Normalize metadata from the MP3Info object and return it as a hash.
-    def extract_id3_metadata( id3 )
+        self.log.debug "Extracting MP3 metadata"
+        
-        mp3_metadata = {
-            :mp3_frequency => id3.samplerate,
@@ -203 +206 @@
-    ### }
-    def extract_images( id3 )
+        self.log.debug "Extracting embedded images"
-        data = {}
-
+
+
+
+
+
+
+
-        
-        if id3.tag2.APIC
+            self.log.debug "...extracting APIC (id3v2.3+) image data."
+
-            images = [ id3.tag2.APIC ].flatten
-            images.each do |img|
@@ -218 +230 @@
-            
-        elsif id3.tag2.PIC
+            self.log.debug "...extracting PIC (id3v2.2) image data."
+
-            images = [ id3.tag2.PIC ].flatten
-            images.each do |img|
@@ -228 +242 @@
-                }
-            end
+            
+        else
+            self.log.debug "...no known image tag types in tags: %p" % [ id3.tag2.keys.sort ]
-        end
-        

thingfish/trunk/plugins/thingfish-handler-formupload/resources/upload.rhtml

@@ -2 +2 @@
-<h2>Uploaded <%= files.length %> file(s)</h2>
-<ul>
-
+|| '' 
-        <li><a href="<%= meta[:uuid] %>" title="<%= meta[:uuid] %>"><%= meta[:title] %></a></li>
-<% end %>

thingfish/trunk/spec/thingfish/request_spec.rb

@@ -39 +39 @@
-    TEMPFILE_PATH = '/var/folders/k7/k7DNYX+ZGSOBod3-pJ-Lhk++0+I/-Tmp-/thingfish.65069.0'
-    
-:all
+ :all 
-        setup_logging( :fatal )
-    end
@@ -118 +118 @@
-    end
-    
+    
-    it "knows when it does not have a multipart/form-data body" do
-        params = @default_params.merge({
@@ -181 +182 @@
-        request.metadata[ upload ].should have(1).member
-        request.metadata[ upload ][ 'relation' ].should == 'thumbnail'
+    end
+    
+    
+    it "requires appended metadata to be associated with a body that's part of the request" do
+        params = {
+            'HTTP_CONTENT_TYPE'   => 'image/x-bitmap',
+            'HTTP_CONTENT_LENGTH' => 18181,
+            'HTTP_USER_AGENT'     => 'GarglePants/1.0',
+            'REMOTE_ADDR'         => '127.0.0.1',
+        }
+        upload = StringIO.new( TEST_CONTENT )
+        @mongrel_request.stub!( :params ).and_return( params )
+        @mongrel_request.stub!( :body ).and_return( upload )
+        request = ThingFish::Request.new( @mongrel_request, @config )
+
+        mystery_resource = StringIO.new( "mystery content" )
+
+        lambda { 
+            request.each_body do |body, _|
+                metadata = { 'relation' => 'thumbnail' }
+                request.append_metadata_for( mystery_resource, metadata )
+            end
+        }.should raise_error( ThingFish::ResourceError, /cannot append/i )
-    end
-    
@@ -290 +314 @@
-        request = ThingFish::Request.new( @mongrel_request, @config )
-
-
-        mystery_resource = StringIO.new( "mystery content" )
-        generated_resource = StringIO.new( "generated content" )
@@ -535 +558 @@
-        request.is_ajax_request?.should == false
-    end
-
-
-
+
+
+
+
+
+
+
+
+
+
+
-
-        before( :each ) do
@@ -545 +576 @@
-                'SERVER_NAME'   => 'thingfish.laika.com',
-                'SERVER_PORT'   => '3474',
+                'REMOTE_ADDR'   => '127.0.0.1',
-                'REQUEST_PATH'  => '/',
-                'QUERY_STRING'  => '',
+                'CONTENT_TYPE'  => 'image/jpeg',
-            })
-            @request_headers = mock( "request headers", :null_object => true )
@@ -555 +588 @@
-            @etag = %{"%s"} % [ TEST_CHECKSUM ]
-            @weak_etag = %{W/"v1.2"}
+        end
+
+
+        ### Body IO checks
+        
+        it "can sanity-check the IO objects that contain entity body data for EOF state" do
+            io = mock( "Entity body IO" )
+
+            @request.instance_variable_set( :@entity_bodies, { io => {} } )
+            @request.instance_variable_set( :@form_metadata, {} )
+            
+            io.should_receive( :closed? ).and_return( false )
+            io.should_receive( :rewind )
+            
+            @request.check_body_ios
+        end
+
+
+        it "can replace a File IO object that contains entity body data if it becomes closed" do
+            io = mock( "Entity body File IO" )
+            clone_io = mock( "Cloned entity body File IO" )
+            @request.instance_variable_set( :@entity_bodies, { io => {} } )
+            @request.instance_variable_set( :@form_metadata, {} )
+            
+            io.should_receive( :closed? ).and_return( true )
+            io.should_receive( :is_a? ).with( StringIO ).and_return( false )
+            io.should_receive( :path ).and_return( :filepath )
+            File.should_receive( :open ).with( :filepath, 'r' ).and_return( clone_io )
+            
+            @request.check_body_ios
+            
+            @request.instance_variable_get( :@entity_bodies ).should_not have_key( io )
+            @request.metadata.should_not have_key( io )
+            @request.instance_variable_get( :@entity_bodies ).should have_key( clone_io )
+            @request.metadata.should have_key( clone_io )
+        end
+
+
+        it "can replace a StringIO object that contains entity body data if it becomes closed" do
+            io = mock( "Entity body StringIO" )
+            clone_io = mock( "Cloned entity body StringIO" )
+            @request.instance_variable_set( :@entity_bodies, { io => {} } )
+            @request.instance_variable_set( :@form_metadata, {} )
+            
+            io.should_receive( :closed? ).and_return( true )
+            io.should_receive( :is_a? ).with( StringIO ).and_return( true )
+            io.should_receive( :string ).and_return( :stringdata )
+            StringIO.should_receive( :new ).with( :stringdata ).and_return( clone_io )
+            
+            # clone_io.should_receive( :closed? ).and_return( false )
+            # clone_io.should_receive( :rewind )
+
+            @request.check_body_ios
-        end
-
@@ -672 +758 @@
-                and_return([ entity_bodies, form_metadata ])
-
+            thumb_metadata = {
+                :relation => 'thumbnail',
+                :format   => 'image/jpeg',
+                :title    => 'filename1_thumb.jpg',
+              }
+                
-            yielded_pairs = {}
+            @request.each_body do |res, parsed_metadata|
+                if res == io1
+                    @request.append_related_resource( res, resource1, thumb_metadata )
+                end
+            end
-            @request.each_body( true ) do |res, parsed_metadata|
-                if res == io1
-                    thumb_metadata = {
-                        :relation => 'thumbnail',
-                        :format   => 'image/jpeg',
-                        :title    => 'filename1_thumb.jpg',
-                      }
-                    @request.append_related_resource( io1, resource1, thumb_metadata )
-                end
-                    
-                yielded_pairs[ res ] = parsed_metadata
-            end
@@ -829 +917 @@
-        end
-    
+
+        ### Body IO checks
+        
+        it "can sanity-check multiple IO objects that contain entity body data for EOF state" do
+            io1 = mock( "filehandle 1" )
+            sub_io1 = mock( "extracted filehandle 1" )
+            io2 = mock( "filehandle 2" )
+        
+            parser = mock( "multipart parser", :null_object => true )
+            entity_bodies = {
+                io1 => {:title  => "filename1", :extent => 100292},
+                io2 => {:title  => "filename2", :extent => 100234}
+              }
+            related_resources = {
+                io1 => { sub_io1 => {} }
+              }
+        
+            @request.instance_variable_set( :@entity_bodies, { io1 => {}, io2 => {} } )
+            @request.instance_variable_set( :@form_metadata, {} )
+            @request.instance_variable_set( :@related_resources, related_resources )
+            
+            io1.should_receive( :closed? ).and_return( false )
+            io1.should_receive( :rewind )
+        
+            sub_io1.should_receive( :closed? ).and_return( false )
+            sub_io1.should_receive( :rewind )
+        
+            io2.should_receive( :closed? ).and_return( false )
+            io2.should_receive( :rewind )
+            
+            @request.check_body_ios
+        end
+
+
+        it "can replace an appended File IO object if it becomes closed" do
+            io1 = mock( "filehandle 1" )
+            sub_io1 = mock( "extracted filehandle 1" )
+            clone_sub_io1 = mock( "extracted filehandle 1" )
+            io2 = mock( "filehandle 2" )
+        
+            parser = mock( "multipart parser", :null_object => true )
+            entity_bodies = {
+                io1 => {:title  => "filename1", :extent => 100292},
+                io2 => {:title  => "filename2", :extent => 100234}
+              }
+            related_resources = {
+                io1 => { sub_io1 => {} }
+              }
+        
+            @request.instance_variable_set( :@entity_bodies, { io1 => {}, io2 => {} } )
+            @request.instance_variable_set( :@form_metadata, {} )
+            @request.instance_variable_set( :@related_resources, related_resources )
+            
+            io1.should_receive( :closed? ).and_return( false )
+            io1.should_receive( :rewind )
+        
+            sub_io1.should_receive( :closed? ).and_return( true )
+            sub_io1.should_receive( :is_a? ).with( StringIO ).and_return( false )
+            sub_io1.should_receive( :path ).and_return( :filepath )
+            File.should_receive( :open ).with( :filepath, 'r' ).and_return( clone_sub_io1 )
+            
+            io2.should_receive( :closed? ).and_return( false )
+            io2.should_receive( :rewind )
+            
+            @request.check_body_ios
+
+            @request.related_resources[ io1 ].keys.should == [clone_sub_io1]
+            @request.metadata.should_not have_key( sub_io1 )
+            @request.metadata.should have_key( clone_sub_io1 )
+        end
+        
+        
+        it "can replace a StringIO object that contains entity body data if it becomes closed" do
+            io1 = mock( "filehandle 1" )
+            sub_io1 = mock( "extracted filehandle 1" )
+            clone_sub_io1 = mock( "extracted filehandle 1" )
+            io2 = mock( "filehandle 2" )
+        
+            parser = mock( "multipart parser", :null_object => true )
+            entity_bodies = {
+                io1 => {:title  => "filename1", :extent => 100292},
+                io2 => {:title  => "filename2", :extent => 100234}
+              }
+            related_resources = {
+                io1 => { sub_io1 => {} }
+              }
+        
+            @request.instance_variable_set( :@entity_bodies, { io1 => {}, io2 => {} } )
+            @request.instance_variable_set( :@form_metadata, {} )
+            @request.instance_variable_set( :@related_resources, related_resources )
+            
+            io1.should_receive( :closed? ).and_return( false )
+            io1.should_receive( :rewind )
+        
+            sub_io1.should_receive( :closed? ).and_return( true )
+            sub_io1.should_receive( :is_a? ).with( StringIO ).and_return( true )
+            sub_io1.should_receive( :string ).and_return( :datastring )
+            StringIO.should_receive( :new ).with( :datastring ).and_return( clone_sub_io1 )
+            
+            io2.should_receive( :closed? ).and_return( false )
+            io2.should_receive( :rewind )
+            
+            @request.check_body_ios
+
+            @request.related_resources[ io1 ].keys.should == [clone_sub_io1]
+            @request.metadata.should_not have_key( sub_io1 )
+            @request.metadata.should have_key( clone_sub_io1 )
+        end
+
-    end
-