[PATCH] Add the driver to URLs returned.

[Chris Lalancette <cl...@redhat.com>]

Currently when using a driver URL like:

http://localhost:3001/api;driver=ec2

The links that get returned for the collections look like:

<link rel='instances' href='/api/instances' />

However, that is not exactly right; the HREF should take
the driver into account, so you end up with:

<link rel='instances' href='/api;driver=ec2/instances' />

So that clients can do the right thing.

This patch fixes the problem by substituting in the REQUEST_URI
(which contains the driver, if any) in the url_for method.  This
seems to make things work for me.

Signed-off-by: Chris Lalancette <cl...@redhat.com>
---
 server/lib/sinatra/url_for.rb |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/server/lib/sinatra/url_for.rb b/server/lib/sinatra/url_for.rb
index 60da0ef..990c49c 100644
--- a/server/lib/sinatra/url_for.rb
+++ b/server/lib/sinatra/url_for.rb
@@ -63,8 +63,14 @@ module Sinatra
       # Don't add the base fragment if url_for gets called more than once
       # per url or the url_fragment passed in is an absolute url
       if url_escape.match(/^#{base}/) or url_escape.match(/^http/)
+        # here, we assume that if any driver additions are needed, they have
+        # been done prior to entering this method.  Hence we just return the
+        # URL as-is
         url_escape
       else
+        # one last thing we need to do here is to make sure we properly add
+        # the driver type to the URL (if necessary)
+        url_escape.sub!(/^\/api/, request.env['REQUEST_URI'])
         "#{base}#{url_escape}"
       end
     end
-- 
1.7.4.4

Re: [PATCH] Add the driver to URLs returned.

[Michal Fojtik <mf...@redhat.com>]

On Jun 6, 2011, at 11:49 AM, David Lutterkort wrote:

> On Fri, 2011-06-03 at 09:37 -0400, Chris Lalancette wrote:
>> This patch fixes the problem by substituting in the REQUEST_URI
>> (which contains the driver, if any) in the url_for method.  This
>> seems to make things work for me.
> 
> I am having trouble with this: when I request
> http://localhost:3001/api;driver=ec2?format=xml the server tries to load
> the driver 'ec2?format=xml' which of course crashes and burns. I also
> see env["HTTP_X_DELTACLOUD_DRIVER"]=ec2?format=xml hinting that the
> matrix param processing code is where this issue originates

I made a new patch, fixing params handling in matrix params and improving
URI generation in url_for.

I also added some unit tests to be sure that it works as expected.

Re: [PATCH] Add the driver to URLs returned.

[David Lutterkort <lu...@redhat.com>]

On Fri, 2011-06-03 at 09:37 -0400, Chris Lalancette wrote:
> This patch fixes the problem by substituting in the REQUEST_URI
> (which contains the driver, if any) in the url_for method.  This
> seems to make things work for me.

I am having trouble with this: when I request
http://localhost:3001/api;driver=ec2?format=xml the server tries to load
the driver 'ec2?format=xml' which of course crashes and burns. I also
see env["HTTP_X_DELTACLOUD_DRIVER"]=ec2?format=xml hinting that the
matrix param processing code is where this issue originates

> Signed-off-by: Chris Lalancette <cl...@redhat.com>
> ---
>  server/lib/sinatra/url_for.rb |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/server/lib/sinatra/url_for.rb b/server/lib/sinatra/url_for.rb
> index 60da0ef..990c49c 100644
> --- a/server/lib/sinatra/url_for.rb
> +++ b/server/lib/sinatra/url_for.rb
> @@ -63,8 +63,14 @@ module Sinatra
>        # Don't add the base fragment if url_for gets called more than once
>        # per url or the url_fragment passed in is an absolute url
>        if url_escape.match(/^#{base}/) or url_escape.match(/^http/)
> +        # here, we assume that if any driver additions are needed, they have
> +        # been done prior to entering this method.  Hence we just return the
> +        # URL as-is
>          url_escape
>        else
> +        # one last thing we need to do here is to make sure we properly add
> +        # the driver type to the URL (if necessary)
> +        url_escape.sub!(/^\/api/, request.env['REQUEST_URI'])

You need to erase the query params from the REQUEST_URI, i.e.
request.env['REQUEST_URI'].sub(/\?.*$/, "")

David