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 2012/03/13 17:17:14 UTC
[PATCH] CIMI - slight hack to prefer content_type of xml when CIMI frontend is being used (and client specifies this as accepted type)
From: marios <ma...@redhat.com>
HTML views for CIMI are being phased out...
Signed-off-by: marios <ma...@redhat.com>
---
server/lib/sinatra/rack_accept.rb | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/server/lib/sinatra/rack_accept.rb b/server/lib/sinatra/rack_accept.rb
index b576307..0f179ea 100644
--- a/server/lib/sinatra/rack_accept.rb
+++ b/server/lib/sinatra/rack_accept.rb
@@ -77,9 +77,13 @@ module Rack
self[type] = handler
end
yield wants
- @media_type = accepting_formats.to_a.sort { |a,b| a[1]<=>b[1] }.reverse.select do |format, priority|
- wants.keys.include?(format) == true
- end.first
+ if ENV['API_FRONTEND'] == "cimi"
+ @media_type = (accepting_formats.has_key?(:xml) ? [:xml, accepting_formats[:xml]] : nil)
+ else
+ @media_type ||= accepting_formats.to_a.sort { |a,b| a[1]<=>b[1] }.reverse.select do |format, priority|
+ wants.keys.include?(format) == true
+ end.first
+ end
if @media_type and @media_type[0]
@media_type = @media_type[0]
headers 'Content-Type' => Rack::MediaType::ACCEPTED_MEDIA_TYPES[@media_type][:return]
--
1.7.6.5
Re: [PATCH] CIMI - slight hack to prefer content_type of xml when
CIMI frontend is being used (and client specifies this as accepted type)
Posted by David Lutterkort <lu...@redhat.com>.
Hi Marios,
On Wed, 2012-03-14 at 13:22 +0200, marios@redhat.com wrote:
> diff --git a/server/lib/cimi/server.rb b/server/lib/cimi/server.rb
> index 6cd845e..653c8c8 100644
> --- a/server/lib/cimi/server.rb
> +++ b/server/lib/cimi/server.rb
> @@ -35,6 +35,7 @@ use Rack::Date
> use Rack::CIMI
>
> configure do
> + set :cimi_preferred_formats, [:xml, :json]
> set :root_url, "/cimi"
> set :views, File::join($top_srcdir, 'views', 'cimi')
> set :public_folder, File::join($top_srcdir, 'public')
>
> diff --git a/server/lib/sinatra/rack_accept.rb
> b/server/lib/sinatra/rack_accept.rb
> index 2ad52c7..7b2f200 100644
> --- a/server/lib/sinatra/rack_accept.rb
> +++ b/server/lib/sinatra/rack_accept.rb
> @@ -77,10 +77,11 @@ module Rack
> self[type] = handler
> end
> yield wants
> - if ENV['API_FRONTEND'] == "cimi"
> - @media_type = (accepting_formats.has_key?(:xml) ? [:xml,
> accepting_formats[:xml]] : nil)
> + if (ENV['API_FRONTEND'] == "cimi" &&
> accepting_formats.has_key?(:html) &&
> + (accepting_formats.keys &
> settings.cimi_preferred_formats).length > 0 )
My comment about making desirable formats a constant was meant so that
we can get rid of the ENV['API_FRONTEND'] == "cimi" test.
Come to think of it, if I write
respond_to do |fmt|
fmt.xml { .. stuff .. }
fmt.json { .. stuff .. }
end
rack_accept should only ever choose XML or JSON, without any additional
logic; in particular, it should never choose HTML. If rack_accept
behaves differently, there's a bug in how it does content negotiation.
To be clear, with the above respond_to, we should
* only consider XML or JSON for the response, regardless what else
is in the Accept header
* prefer XML over JSON if the Accept header has them at the same
priority (q value)
David
Re: [PATCH] CIMI - slight hack to prefer content_type of xml when
CIMI frontend is being used (and client specifies this as accepted type)
Posted by "marios@redhat.com" <ma...@redhat.com>.
On 13/03/12 23:11, David Lutterkort wrote:
> On Tue, 2012-03-13 at 18:17 +0200, marios@redhat.com wrote:
>> From: marios <ma...@redhat.com>
>>
>> HTML views for CIMI are being phased out...
>>
>> Signed-off-by: marios <ma...@redhat.com>
>> ---
>> server/lib/sinatra/rack_accept.rb | 10 +++++++---
>> 1 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/server/lib/sinatra/rack_accept.rb b/server/lib/sinatra/rack_accept.rb
>> index b576307..0f179ea 100644
>> --- a/server/lib/sinatra/rack_accept.rb
>> +++ b/server/lib/sinatra/rack_accept.rb
>> @@ -77,9 +77,13 @@ module Rack
>> self[type] = handler
>> end
>> yield wants
>> - @media_type = accepting_formats.to_a.sort { |a,b| a[1]<=>b[1] }.reverse.select do |format, priority|
>> - wants.keys.include?(format) == true
>> - end.first
>> + if ENV['API_FRONTEND'] == "cimi"
>> + @media_type = (accepting_formats.has_key?(:xml) ? [:xml, accepting_formats[:xml]] : nil)
>> + else
>> + @media_type ||= accepting_formats.to_a.sort { |a,b| a[1]<=>b[1] }.reverse.select do |format, priority|
>> + wants.keys.include?(format) == true
>> + end.first
>> + end
>
> Doesn't this break content negotiation ? We can only ignore other
> formats if q=1 for application/xml (implicitly or explicitly) In
> particular, wouldn't the above break with an Accept header of
> 'application/json, application/xml;q=0.9' ?
ok, good point. How about something like this (separate patch on list
but some explanation below):
diff --git a/server/lib/cimi/server.rb b/server/lib/cimi/server.rb
index 6cd845e..653c8c8 100644
--- a/server/lib/cimi/server.rb
+++ b/server/lib/cimi/server.rb
@@ -35,6 +35,7 @@ use Rack::Date
use Rack::CIMI
configure do
+ set :cimi_preferred_formats, [:xml, :json]
set :root_url, "/cimi"
set :views, File::join($top_srcdir, 'views', 'cimi')
set :public_folder, File::join($top_srcdir, 'public')
diff --git a/server/lib/sinatra/rack_accept.rb
b/server/lib/sinatra/rack_accept.rb
index 2ad52c7..7b2f200 100644
--- a/server/lib/sinatra/rack_accept.rb
+++ b/server/lib/sinatra/rack_accept.rb
@@ -77,10 +77,11 @@ module Rack
self[type] = handler
end
yield wants
- if ENV['API_FRONTEND'] == "cimi"
- @media_type = (accepting_formats.has_key?(:xml) ? [:xml,
accepting_formats[:xml]] : nil)
+ if (ENV['API_FRONTEND'] == "cimi" &&
accepting_formats.has_key?(:html) &&
+ (accepting_formats.keys &
settings.cimi_preferred_formats).length > 0 )
===explain===
... if -> serving CIMI, AND,
-> :html is specified as a format by the client, AND
-> the intersection of client.formats with [:xml, :json] contains at
least one element
... then remove the :html from the client's preferred formats...
+ request.env['rack-accept.formats'].delete(:html)
end
- @media_type ||= accepting_formats.to_a.sort { |a,b|
a[1]<=>b[1] }.reverse.select do |format, priority|
+ @media_type = accepting_formats.to_a.sort { |a,b| a[1]<=>b[1]
}.reverse.select do |format, priority|
wants.keys.include?(format) == true
end.first
if @media_type and @media_type[0]
--
there is still a slight problem here, though not related to my edits.
When using cURL, I noticed that the Accept is specified as */*. However,
the request.env['rack-accept.formats'] (i.e. the 'accepting_formats'
method returns) is given as {:html=>0}; even though
request.env["HTTP_ACCEPT"] is in fact */*. This means that if you are
using cURL and do something like:
curl -iv http://localhost:3001/cimi/cloudEntryPoint
the respond_to method above returns 406 Not Acceptable
marios
>
> Also, it would be cleaner if the format preference was encoded in each
> server.rb (e.g., as an array constant) AFAIK, self at this point is the
> Sinatra app.
>
> David
>
>
Re: [PATCH] CIMI - slight hack to prefer content_type of xml when
CIMI frontend is being used (and client specifies this as accepted type)
Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2012-03-13 at 18:17 +0200, marios@redhat.com wrote:
> From: marios <ma...@redhat.com>
>
> HTML views for CIMI are being phased out...
>
> Signed-off-by: marios <ma...@redhat.com>
> ---
> server/lib/sinatra/rack_accept.rb | 10 +++++++---
> 1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/server/lib/sinatra/rack_accept.rb b/server/lib/sinatra/rack_accept.rb
> index b576307..0f179ea 100644
> --- a/server/lib/sinatra/rack_accept.rb
> +++ b/server/lib/sinatra/rack_accept.rb
> @@ -77,9 +77,13 @@ module Rack
> self[type] = handler
> end
> yield wants
> - @media_type = accepting_formats.to_a.sort { |a,b| a[1]<=>b[1] }.reverse.select do |format, priority|
> - wants.keys.include?(format) == true
> - end.first
> + if ENV['API_FRONTEND'] == "cimi"
> + @media_type = (accepting_formats.has_key?(:xml) ? [:xml, accepting_formats[:xml]] : nil)
> + else
> + @media_type ||= accepting_formats.to_a.sort { |a,b| a[1]<=>b[1] }.reverse.select do |format, priority|
> + wants.keys.include?(format) == true
> + end.first
> + end
Doesn't this break content negotiation ? We can only ignore other
formats if q=1 for application/xml (implicitly or explicitly) In
particular, wouldn't the above break with an Accept header of
'application/json, application/xml;q=0.9' ?
Also, it would be cleaner if the format preference was encoded in each
server.rb (e.g., as an array constant) AFAIK, self at this point is the
Sinatra app.
David
Re: [PATCH] CIMI - slight hack to prefer content_type of xml when CIMI frontend is being used (and client specifies this as accepted type)
Posted by Michal Fojtik <mf...@redhat.com>.
On Mar 13, 2012, at 5:17 PM, marios@redhat.com wrote:
ACK. Did the job. Good catch Marios!
-- Michal
> From: marios <ma...@redhat.com>
>
> HTML views for CIMI are being phased out...
>
> Signed-off-by: marios <ma...@redhat.com>
> ---
> server/lib/sinatra/rack_accept.rb | 10 +++++++---
> 1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/server/lib/sinatra/rack_accept.rb b/server/lib/sinatra/rack_accept.rb
> index b576307..0f179ea 100644
> --- a/server/lib/sinatra/rack_accept.rb
> +++ b/server/lib/sinatra/rack_accept.rb
> @@ -77,9 +77,13 @@ module Rack
> self[type] = handler
> end
> yield wants
> - @media_type = accepting_formats.to_a.sort { |a,b| a[1]<=>b[1] }.reverse.select do |format, priority|
> - wants.keys.include?(format) == true
> - end.first
> + if ENV['API_FRONTEND'] == "cimi"
> + @media_type = (accepting_formats.has_key?(:xml) ? [:xml, accepting_formats[:xml]] : nil)
> + else
> + @media_type ||= accepting_formats.to_a.sort { |a,b| a[1]<=>b[1] }.reverse.select do |format, priority|
> + wants.keys.include?(format) == true
> + end.first
> + end
> if @media_type and @media_type[0]
> @media_type = @media_type[0]
> headers 'Content-Type' => Rack::MediaType::ACCEPTED_MEDIA_TYPES[@media_type][:return]
> --
> 1.7.6.5
>