On Mar 5, 2011, at 1:30 AM, David Lutterkort wrote: > On Fri, 2011-03-04 at 13:15 +0100, [email protected] wrote: >> From: Michal Fojtik <[email protected]> >> >> --- >> server/lib/sinatra/rack_matrix_params.rb | 54 >> ++++++++++++++++++++++++++++++ >> server/server.rb | 2 + >> server/views/instances/show.xml.haml | 4 +- >> 3 files changed, 58 insertions(+), 2 deletions(-) >> create mode 100644 server/lib/sinatra/rack_matrix_params.rb > > ACK with a few comments: > >> diff --git a/server/lib/sinatra/rack_matrix_params.rb >> b/server/lib/sinatra/rack_matrix_params.rb >> new file mode 100644 >> index 0000000..7488c73 >> --- /dev/null >> +++ b/server/lib/sinatra/rack_matrix_params.rb >> @@ -0,0 +1,54 @@ >> +# Copyright (C) 2009, 2010 Red Hat, Inc. > > (C) 2011 since you just wrote this.
Thanks, nice catch. Will keep this in mind before hitting copy&paste again ;-) >> +module Rack >> + >> + # This will allow to use 'matrix' params in POST requests, like: >> + # >> + # POST http://127.0.0.1:9393/test/test;person_id=i-123123;test=1 >> + # >> + # => {"instance_id"=>"i-123123", "haha"=>"1", "test"=>"1"} >> + # >> + # (where 'haha' is regular POST param sent from HTML form > > There is no special connection between POST and matrix params - they can > be used for any request method. > > There's an added wrinkle, in that they can be used with any path > component of a URL, as in > > > http://example.com/library;section=nw/books;topic=money;binding=hardcover > > You should really parse the URI, take the path, split it on '/', and > split each of the resulting components on ';' (for now, it's enough to > do that for the last path component) > > There's of course the danger that different path components have matrix > params with the same name, but I'd just ignore that for now. Cool, I didn't realized that they could be used in this way. Making it work shouldn't be hard to do I just remove 'if'. I'll rework the parsing part as well. > >> diff --git a/server/views/instances/show.xml.haml >> b/server/views/instances/show.xml.haml >> index 5e1b974..7e9f4fa 100644 >> --- a/server/views/instances/show.xml.haml >> +++ b/server/views/instances/show.xml.haml >> @@ -22,11 +22,11 @@ >> - @instance.actions.compact.each do |instance_action| >> %link{:rel => instance_action, :method => >> instance_action_method(instance_action), :href => >> self.send("#{instance_action}_instance_url", @instance.id)} >> - if driver.respond_to?(:run_on_instance) >> - %link{:rel => 'run', :method => :post, :href => >> run_instance_url(@instance.id)} >> + %link{:rel => 'run', :method => :post, :href => >> "#{run_instance_url(@instance.id)};id=#{@instance.id}"} >> - generate_action_params(:instances, :run) do |param| >> = param.call(:id => @instance.id, :password => >> @instance.password) >> - if @instance.can_create_image? >> - %link{:rel => 'create_image', :method => :post, :href => >> create_image_url} >> + %link{:rel => 'create_image', :method => :post, :href => >> "#{create_image_url};instance_id=#{@instance.id}"} >> - generate_action_params(:images, :create) do |param| >> = param.call(:instance_id => @instance.id) >> - if @instance.instance_variables.include?("@launch_time") > > I'd much prefer if this patch series was such that patch 1/2 added the > new rack middleware (including using it in server.rb), and patch 2/2 did > the create_image stuff. Sure I'll split this patch set. -- Michal Michal Fojtik Software Engineer, Deltacloud API project http://www.deltacloud.org [email protected]
