You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by ma...@redhat.com on 2010/09/23 19:57:47 UTC
['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.
From: marios <ma...@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
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.
Posted by David Lutterkort <lu...@redhat.com>.
On Fri, 2010-09-24 at 12:51 +0200, Michal Fojtik wrote:
> 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.
Tests need to pass, absolutely. Though, our tests should do negotiation
through the Accept header, instead of an explicit format, since that is
how we expect clients to use the server.
David
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.
Posted by "marios@redhat.com" <ma...@redhat.com>.
>
> 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.
ok mate thanks, yes forgot that we already used the 'auto-negotiation'
stuff already for other collections - added to list for fixing,
marios
>
> 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
>
>>
>
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.
Posted by Michal Fojtik <mf...@redhat.com>.
On 23/09/10 18:57 +0100, mandreou@redhat.com wrote:
>From: marios <ma...@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
--------------------------------------------------------