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 2011/08/19 14:42:30 UTC

[PATCH 2/3] Restores explicit route for getting the 'new_blob' form. A naming clash with GET /api/buckets/:bucket/:blob meant that the 'new_blob' operation under buckets collection was never triggered.

From: marios <ma...@redhat.com>


Signed-off-by: marios <ma...@redhat.com>
---
 server/server.rb |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/server/server.rb b/server/server.rb
index 231a89e..e302331 100644
--- a/server/server.rb
+++ b/server/server.rb
@@ -786,6 +786,14 @@ put "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
   end
 end
 
+#get html form for creating a new blob
+get "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/new_blob" do
+  @bucket_id = params[:bucket]
+  respond_to do |format|
+    format.html {haml :"blobs/new"}
+  end
+end
+
 #create a new blob using html interface - NON STREAMING (i.e. browser POST http form data)
 post "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket" do
   bucket_id = params[:bucket]
@@ -897,16 +905,6 @@ collection :buckets do
     end
   end
 
-  operation :new_bucket, :form => true, :method => :get, :member => true do
-    param :bucket,  :string
-    control do
-      @bucket_id = params[:bucket]
-      respond_to do |format|
-        format.html {haml :"blobs/new"}
-      end
-    end
-  end
-
   operation :index do
     description "List buckets associated with this account"
     with_capability :buckets
-- 
1.7.3.4


Re: [PATCH 2/3] Restores explicit route for getting the 'new_blob' form. A naming clash with GET /api/buckets/:bucket/:blob meant that the 'new_blob' operation under buckets collection was never triggered.

Posted by David Lutterkort <lu...@redhat.com>.
On Mon, 2011-08-22 at 18:16 +0300, marios@redhat.com wrote:
> On 20/08/11 03:58, David Lutterkort wrote:
> > On Fri, 2011-08-19 at 15:42 +0300, marios@redhat.com wrote:
> >> +#get html form for creating a new blob
> >> +get "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/new_blob" do
> >> +  @bucket_id = params[:bucket]
> >> +  respond_to do |format|
> >> +    format.html {haml :"blobs/new"}
> >> +  end
> >> +end
> >
> > Doesn't this mean that if somebody has a blob called 'new_blob' and they
> > try to get the XML representation, they'll get nothing ?
> >
> 
> yes, it does.
> 
> > It would be better to do the following in the /buckets/:bucket/:blob
> > route:
> >
> >          respond_to do
> >            ...
> >            format.html do
> >              templ = params[:blob] == "new_blob" ? "blobs/new" : "blobs/show"
> >              haml templ.to_sym
> >            end
> >          end
> >
> > Even better, if we could make the special name to get at the new blob
> > form something less likely to collide, like
> > 'deltacloud_get_new_html_blob_form' or some such.
> >
> 
> I think this approach is the safest - though no matter what we call it 
> there will always be the (theoretical) possibility that someone may want 
> to call their blob that. If you are in agreement with the rest of this 
> series, I can push it and then make a patch on top of that to rename the 
> 'get new blob form' route to something like 
> 'deltacloud_api_get_new_blob_form'. Luckily this is not documented in 
> the REST API so shouldn't need to much explaining (and a user will only 
> get to it by clicking on the 'new blob' button if using the html interface),

Yes, other than the naming clash, ACK.

David



Re: [PATCH 2/3] Restores explicit route for getting the 'new_blob' form. A naming clash with GET /api/buckets/:bucket/:blob meant that the 'new_blob' operation under buckets collection was never triggered.

Posted by "marios@redhat.com" <ma...@redhat.com>.
On 20/08/11 03:58, David Lutterkort wrote:
> On Fri, 2011-08-19 at 15:42 +0300, marios@redhat.com wrote:
>> +#get html form for creating a new blob
>> +get "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/new_blob" do
>> +  @bucket_id = params[:bucket]
>> +  respond_to do |format|
>> +    format.html {haml :"blobs/new"}
>> +  end
>> +end
>
> Doesn't this mean that if somebody has a blob called 'new_blob' and they
> try to get the XML representation, they'll get nothing ?
>

yes, it does.

> It would be better to do the following in the /buckets/:bucket/:blob
> route:
>
>          respond_to do
>            ...
>            format.html do
>              templ = params[:blob] == "new_blob" ? "blobs/new" : "blobs/show"
>              haml templ.to_sym
>            end
>          end
>
> Even better, if we could make the special name to get at the new blob
> form something less likely to collide, like
> 'deltacloud_get_new_html_blob_form' or some such.
>

I think this approach is the safest - though no matter what we call it 
there will always be the (theoretical) possibility that someone may want 
to call their blob that. If you are in agreement with the rest of this 
series, I can push it and then make a patch on top of that to rename the 
'get new blob form' route to something like 
'deltacloud_api_get_new_blob_form'. Luckily this is not documented in 
the REST API so shouldn't need to much explaining (and a user will only 
get to it by clicking on the 'new blob' button if using the html interface),

marios



> David
>
>


Re: [PATCH 2/3] Restores explicit route for getting the 'new_blob' form. A naming clash with GET /api/buckets/:bucket/:blob meant that the 'new_blob' operation under buckets collection was never triggered.

Posted by David Lutterkort <lu...@redhat.com>.
On Fri, 2011-08-19 at 15:42 +0300, marios@redhat.com wrote:
> +#get html form for creating a new blob
> +get "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/new_blob" do
> +  @bucket_id = params[:bucket]
> +  respond_to do |format|
> +    format.html {haml :"blobs/new"}
> +  end
> +end

Doesn't this mean that if somebody has a blob called 'new_blob' and they
try to get the XML representation, they'll get nothing ?

It would be better to do the following in the /buckets/:bucket/:blob
route:

        respond_to do
          ...
          format.html do
            templ = params[:blob] == "new_blob" ? "blobs/new" : "blobs/show"
            haml templ.to_sym
          end
        end
        
Even better, if we could make the special name to get at the new blob
form something less likely to collide, like
'deltacloud_get_new_html_blob_form' or some such.

David