incubator-deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michal Fojtik <mfoj...@redhat.com>
Subject Re: ['PATCH'] Tidy up /server/lib/sinatra/respond_to.rb - 'stock' sinatra code was stripping off extension in requests (myblob.txt) and trying to guess content type based on that. This caused problems with blob names. First check for '?format=___' in request, or use http_accept header, or set to default content.
Date Fri, 24 Sep 2010 10:51:43 GMT
On 23/09/10 18:57 +0100, mandreou@redhat.com wrote:
>From: marios <marios@redhat.com>
>
>---
> server/lib/sinatra/respond_to.rb |   39 +++++++------------------------------
> 1 files changed, 8 insertions(+), 31 deletions(-)
>
>diff --git a/server/lib/sinatra/respond_to.rb b/server/lib/sinatra/respond_to.rb
>index f88c025..926b10d 100644
>--- a/server/lib/sinatra/respond_to.rb
>+++ b/server/lib/sinatra/respond_to.rb
>@@ -30,38 +30,15 @@ module Sinatra
>       app.set :default_content, :html
>       app.set :assume_xhr_is_js, true
> 
>-      # We remove the trailing extension so routes
>-      # don't have to be of the style
>-      #
>-      #   get '/resouce.:format'
>-      #
>-      # They can instead be of the style
>-      #
>-      #   get '/resource'
>-      #
>-      # and the format will automatically be available in <tt>format</tt>
>+    #deltacloud: removed the code that tried to 'guess' content based on extension
>+    #as this broke blobstore api (stripping blob names). Use ?format if present, otherwise

>+    #use http accept, otherwise set to default
>       app.before do
>-        # Let through sinatra image urls in development
>-        next if self.class.development? && request.path_info =~ %r{/__sinatra__/.*?.png}
>-
>-        unless options.static? && options.public? && (request.get? ||
request.head?) && static_file?(request.path_info)
>-          rpi = request.path_info.sub(%r{\.([^\./]+)$}, '')
>-
>-          if (not $1) or ($1 and TEXT_MIME_TYPES.include?($1.to_sym))
>-            request.path_info, ext = rpi, nil
>-            if ! $1.nil?
>-              ext = $1
>-            elsif env['HTTP_ACCEPT'].nil? || env['HTTP_ACCEPT'].empty?
>-              ext = options.default_content
>-            end
>-            if ext
>-              @mime_types = [ Helpers::mime_type(ext) ]
>-              format ext
>-            elsif (params[:format])
>-              @mime_types = [Helpers::mime_type(params[:format])]
>-              format params[:format]
>-            end
>-          end
>+        if (params[:format])
>+          @mime_types = [Helpers::mime_type(params[:format])]
>+          format params[:format]
>+        elsif env['HTTP_ACCEPT'].nil? || env['HTTP_ACCEPT'].empty?
>+          ext = options.default_content
>         end
>       end
> 
>-- 
>1.7.2.3

Actually, I need to NACK this patch because lot of our specs  are failing
with this code. See attached output of 'rake test':
Btw. after quick look on failed tests I think we will need update our
tests to omit usage of '.format' and switch to '?format' instead.

Also please run:

cd core/tests
rake cucumber
API_DRIVER="ec2" rake cucumber

before this patch will be pushed. There are also some failures.

 -- Michal

>

-- 
--------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org
--------------------------------------------------------

Mime
View raw message