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. > +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. > 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. David
