You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by lu...@redhat.com on 2011/07/30 01:15:54 UTC

[PATCH 2/2] Unify some response behavior

From: David Lutterkort <lu...@redhat.com>

This patch makes sure that all responses adhere to the following rules:

  * The HTTP status is the same across all return formats, in particular
    XML and JSON
  * When we return 201 Created, we also set a Location header for the
    created entity
  * XML is the default format, i.e., the one that is used if the client
    does not send an Accept header

Signed-off-by: David Lutterkort <lu...@redhat.com>
---
 server/server.rb                         |  172 +++++++++++++++---------------
 server/tests/drivers/mock/images_test.rb |    5 +-
 2 files changed, 91 insertions(+), 86 deletions(-)

diff --git a/server/server.rb b/server/server.rb
index 864b6b8..60c1d36 100644
--- a/server/server.rb
+++ b/server/server.rb
@@ -171,7 +171,7 @@ end
 collection :images do
   description <<END
   An image is a platonic form of a machine. Images are not directly executable,
-  but are a template for creating actual instances of machines."
+  but are a template for creating actual instances of machines.
 END
 
   operation :index do
@@ -205,11 +205,10 @@ END
         :name => params[:name],
 	:description => params[:description]
       })
+      status 201  # Created
+      response['Location'] = image_url(@instance.id)
       respond_to do |format|
-        format.xml do
-          response.status = 201 # Created
-          haml :"images/show"
-        end
+        format.xml  { haml :"images/show" }
         format.json { convert_to_json(:image, @image) }
         format.html { haml :"images/show" }
       end
@@ -222,9 +221,10 @@ END
     param :id,    :string,    :required
     control do
       driver.destroy_image(credentials, params[:id])
+      status 204
       respond_to do |format|
-        format.xml { status 204 }
-        format.json { status 204 }
+        format.xml
+        format.json
         format.html { redirect(images_url) }
       end
     end
@@ -327,11 +327,10 @@ collection :load_balancers do
     param :listener_instance_port,  :string,  :required
     control do
       @load_balancer = driver.create_load_balancer(credentials, params)
+      status 201  # Created
+      response['Location'] = load_balancer_url(@instance.id)
       respond_to do |format|
-        format.xml do
-          response.status = 201  # Created
-          haml :"load_balancers/show"
-        end
+        format.xml  { haml :"load_balancers/show" }
         format.json { convert_to_json(:load_balancer, @load_balancer) }
         format.html { haml :"load_balancers/show" }
       end
@@ -344,9 +343,10 @@ collection :load_balancers do
     param :instance_id, :string,  :required
     control do
       driver.lb_register_instance(credentials, params)
+      status 204
       respond_to do |format|
-        format.xml { 204 }
-        format.json { 204 }
+        format.xml
+        format.json
         format.html { redirect(load_balancer_url(params[:id])) }
       end
     end
@@ -358,9 +358,10 @@ collection :load_balancers do
     param :instance_id, :string,  :required
     control do
       driver.lb_unregister_instance(credentials, params)
+      status 204
       respond_to do |format|
-        format.xml { 204 }
-        format.json { 204 }
+        format.xml
+        format.json
         format.html { redirect(load_balancer_url(params[:id])) }
       end
     end
@@ -371,9 +372,10 @@ collection :load_balancers do
     param :id,  :string,  :required
     control do
       driver.destroy_load_balancer(credentials, params[:id])
+      status 204
       respond_to do |format|
-        format.xml {  204 }
-        format.json { 204 }
+        format.xml
+        format.json
         format.html { redirect(load_balancers_url) }
       end
     end
@@ -385,7 +387,7 @@ end
 collection :instances do
   description <<END
   An instance is a concrete machine realized from an image.
-  The images collection may be obtained by following the link from the primary entry-point."
+  The images collection may be obtained by following the link from the primary entry-point.
 END
 
   operation :index do
@@ -411,12 +413,10 @@ END
     param :hwp_id,       :string, :optional
     control do
       @instance = driver.create_instance(credentials, params[:image_id], params)
+      status 201  # Created
+      response['Location'] = instance_url(@instance.id)
       respond_to do |format|
-        format.xml do
-          response.status = 201  # Created
-          response['Location'] = instance_url(@instance.id)
-          haml :"instances/show"
-        end
+        format.xml  { haml :"instances/show" }
         format.json { convert_to_json(:instance, @instance) }
         format.html do
           redirect instance_url(@instance.id) if @instance and @instance.id
@@ -546,8 +546,9 @@ collection :storage_snapshots do
     with_capability :create_storage_snapshot
     param :volume_id, :string,  :required
     control do
-      response.status = 201  # Created
       @storage_snapshot = driver.create_storage_snapshot(credentials, params)
+      status 201  # Created
+      response['Location'] = storage_snapshot_url(@storage_snapshot.id)
       show(:storage_snapshot)
     end
   end
@@ -558,9 +559,10 @@ collection :storage_snapshots do
     param :id,  :string,  :required
     control do
       driver.destroy_storage_snapshot(credentials, params)
+      status 204
       respond_to do |format|
-        format.xml { 204 }
-        format.json { 204 }
+        format.xml
+        format.json
         format.html { redirect(storage_snapshots_url) }
       end
     end
@@ -605,13 +607,12 @@ collection :storage_volumes do
     param :realm_id,    :string,  :optional
     control do
       @storage_volume = driver.create_storage_volume(credentials, params)
+      status 201
+      response['Location'] = storage_volume_url(@storage_volume.id)
       respond_to do |format|
+        format.xml  { haml :"storage_volumes/show" }
         format.html { haml :"storage_volumes/show" }
         format.json { convert_to_json(:storage_volume, @storage_volume) }
-        format.xml do
-          response.status = 201  # Created
-          haml :"storage_volumes/show"
-        end
       end
     end
   end
@@ -655,9 +656,10 @@ collection :storage_volumes do
     param :id,          :string,  :optional
     control do
       driver.destroy_storage_volume(credentials, params)
+      status 204
       respond_to do |format|
-        format.xml { 204 }
-        format.json { 204 }
+        format.xml
+        format.json
         format.html { redirect(storage_volumes_url) }
       end
     end
@@ -695,12 +697,11 @@ collection :keys do
     param :name,  :string,  :required
     control do
       @key = driver.create_key(credentials, { :key_name => params[:name] })
+      status 201
+      response['Location'] = key_url(@key.id)
       respond_to do |format|
+        format.xml  { haml :"keys/show", :ugly => true }
         format.html { haml :"keys/show" }
-        format.xml do
-          response.status = 201  # Created
-          haml :"keys/show", :ugly => true
-        end
         format.json { convert_to_json(:key, @key)}
       end
     end
@@ -712,9 +713,10 @@ collection :keys do
     param :id,  :string,  :required
     control do
       driver.destroy_key(credentials, { :id => params[:id]})
+      status 204
       respond_to do |format|
-        format.xml { 204 }
-        format.json { 204 }
+        format.xml
+        format.json
         format.html { redirect(keys_url) }
       end
     end
@@ -738,8 +740,8 @@ put "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
     @blob = driver.blob(credentials, {:id => params[:blob],
                                       'bucket' => params[:bucket]})
     respond_to do |format|
-      format.html { haml :"blobs/show" }
       format.xml { haml :"blobs/show" }
+      format.html { haml :"blobs/show" }
       format.json { convert_to_json(:blob, @blob) }
     end
   elsif(env["BLOB_FAIL"])
@@ -757,8 +759,8 @@ put "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
     @blob = driver.create_blob(credentials, bucket_id, blob_id, blob_data, user_meta)
     temp_file.delete
     respond_to do |format|
-      format.html { haml :"blobs/show"}
       format.xml { haml :"blobs/show" }
+      format.html { haml :"blobs/show"}
     end
   end
 end
@@ -781,8 +783,8 @@ post "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket" do
   end
   @blob = driver.create_blob(credentials, bucket_id, blob_id, blob_data, user_meta)
   respond_to do |format|
-    format.html { haml :"blobs/show"}
     format.xml { haml :"blobs/show" }
+    format.html { haml :"blobs/show"}
   end
 end
 
@@ -791,9 +793,10 @@ delete "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
   bucket_id = params[:bucket]
   blob_id = params[:blob]
   driver.delete_blob(credentials, bucket_id, blob_id)
+  status 204
   respond_to do |format|
-    format.xml {  204 }
-    format.json {  204 }
+    format.xml
+    format.json
     format.html { redirect(bucket_url(bucket_id)) }
   end
 end
@@ -806,9 +809,10 @@ head "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
       @blob_metadata.each do |k,v|
         headers["X-Deltacloud-Blobmeta-#{k}"] = v
       end
+      status 204
       respond_to do |format|
-        format.xml {  204 }
-        format.json {  204 }
+        format.xml
+        format.json
       end
    else
     report_error(404)
@@ -823,9 +827,10 @@ post "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
     meta_hash.each do |k,v|
       headers["X-Deltacloud-Blobmeta-#{k}"] = v
     end
+    status 204
     respond_to do |format|
-      format.xml {  204 }
-      format.json {  204 }
+      format.xml
+      format.json
     end
   else
     report_error(404) #FIXME is this the right error code?
@@ -837,8 +842,8 @@ get "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
   @blob = driver.blob(credentials, { :id => params[:blob], 'bucket' => params[:bucket]})
   if @blob
     respond_to do |format|
-      format.html { haml :"blobs/show" }
       format.xml { haml :"blobs/show" }
+      format.html { haml :"blobs/show" }
       format.json { convert_to_json(:blob, @blob) }
       end
   else
@@ -891,19 +896,15 @@ collection :buckets do
     param :name,      :string,    :required
     control do
       @bucket = driver.create_bucket(credentials, params[:name], params)
+      status 201
+      response['Location'] = bucket_url(@bucket.id)
       respond_to do |format|
+        format.xml  { haml :"buckets/show" }
+        format.json { convert_to_json(:bucket, @bucket) }
         format.html do
           redirect bucket_url(@bucket.id) if @bucket and @bucket.id
           redirect buckets_url
         end
-        format.xml do
-          response.status = 201  # Created
-          response['Location'] = bucket_url(@bucket.id)
-          haml :"buckets/show"
-        end
-        format.json do
-          convert_to_json(:bucket, @bucket)
-        end
       end
     end
   end
@@ -914,9 +915,10 @@ collection :buckets do
     param :id,    :string,    :required
     control do
       driver.delete_bucket(credentials, params[:id], params)
+      status 204
       respond_to do |format|
-        format.xml { 204 }
-        format.json {  204 }
+        format.xml
+        format.json
         format.html {  redirect(buckets_url) }
       end
     end
@@ -955,14 +957,12 @@ collection :addresses do
     with_capability :create_address
     control do
       @address = driver.create_address(credentials, {})
+      status 201    # Created
+      response['Location'] = address_url(@address.id)
       respond_to do |format|
+        format.xml  { haml :"addresses/show", :ugly => true }
         format.html { haml :"addresses/show" }
         format.json { convert_to_json(:address, @address) }
-        format.xml do
-          response.status = 201  # Created
-          response['Location'] = address_url(@address.id)
-          haml :"addresses/show", :ugly => true
-        end
       end
     end
   end
@@ -973,9 +973,10 @@ collection :addresses do
     param :id,  :string,  :required
     control do
       driver.destroy_address(credentials, { :id => params[:id]})
+      status 204
       respond_to do |format|
-        format.xml { 204 }
-        format.json { 204 }
+        format.xml
+        format.json
         format.html { redirect(addresses_url) }
       end
     end
@@ -988,9 +989,10 @@ collection :addresses do
     param :instance_id, :string, :required
     control do
       driver.associate_address(credentials, { :id => params[:id], :instance_id => params[:instance_id]})
+      status 202   # Accepted
       respond_to do |format|
-        format.xml { 202 }  # Accepted
-        format.json { 202 }
+        format.xml
+        format.json
         format.html { redirect(address_url(params[:id])) }
       end
     end
@@ -1002,9 +1004,10 @@ collection :addresses do
     param :id, :string, :required
     control do
       driver.disassociate_address(credentials, { :id => params[:id] })
+      status 202   # Accepted
       respond_to do |format|
-        format.xml { 202 }  # Accepted
-        format.json { 202 }
+        format.xml
+        format.json
         format.html { redirect(address_url(params[:id])) }
       end
     end
@@ -1033,10 +1036,11 @@ delete '/api/firewalls/:firewall/:rule' do
   opts[:firewall] = params[:firewall]
   opts[:rule_id] = params[:rule]
   driver.delete_firewall_rule(credentials, opts)
+  status 204
   respond_to do |format|
+    format.xml
+    format.json
     format.html {redirect firewall_url(params[:firewall])}
-    format.xml {204}
-    format.json {204}
   end
 end
 
@@ -1063,13 +1067,12 @@ collection :firewalls do
     param :description,   :string,    :required
     control do
       @firewall = driver.create_firewall(credentials, params )
+      status 201  # Created
+      response['Location'] = firewall_url(@firewall.id)
       respond_to do |format|
-        format.xml do
-          response.status = 201  # Created
-          haml :"firewalls/show"
-        end
-        format.html {haml :"firewalls/show"}
-        format.json {convert_to_json(:firewall, @firewall)}
+        format.xml  { haml :"firewalls/show" }
+        format.html { haml :"firewalls/show" }
+        format.json { convert_to_json(:firewall, @firewall) }
       end
     end
   end
@@ -1080,9 +1083,10 @@ collection :firewalls do
     param :id,            :string,    :required
     control do
       driver.delete_firewall(credentials, params)
+      status 204
       respond_to do |format|
-        format.xml { 204 }
-        format.json {  204 }
+        format.xml
+        format.json
         format.html {  redirect(firewalls_url) }
       end
     end
@@ -1108,13 +1112,11 @@ collection :firewalls do
       params.merge!( {'addresses' => addresses} ) ; params.merge!( {'groups' => groups} )
       driver.create_firewall_rule(credentials, params)
       @firewall = driver.firewall(credentials, {:id => params[:id]})
+      status 201
       respond_to do |format|
-        format.html {haml :"firewalls/show"}
-        format.xml do
-          response.status = 201 #created
-          haml :"firewalls/show"
-        end
-        format.json {convert_to_json(:firewall, @firewall)}
+        format.xml  { haml :"firewalls/show" }
+        format.html { haml :"firewalls/show" }
+        format.json { convert_to_json(:firewall, @firewall) }
       end
     end
   end
diff --git a/server/tests/drivers/mock/images_test.rb b/server/tests/drivers/mock/images_test.rb
index bbe6ed3..47fb690 100644
--- a/server/tests/drivers/mock/images_test.rb
+++ b/server/tests/drivers/mock/images_test.rb
@@ -113,9 +113,12 @@ module DeltacloudUnitTest
       Nokogiri::HTML(last_response.body).search('html').first.name.should == 'html'
     end
 
-    def test_it_can_destroy_created_image
+    def test_it_creates_and_destroys_image_from_instance
       post_url "/api/images", { :name => "img4", :description => "Test::Unit image", :instance_id => "inst1"}
       last_response.status.should == 201
+      last_response.headers['Location'].should_not == nil
+      get_auth_url last_response.headers['Location'], {}
+      (last_xml_response/'instance/name').should_not == nil
       delete_url "/api/images/img4", {}
       last_response.status.should == 204
       get_auth_url "/api/images/img4", {}
-- 
1.7.6


Re: [PATCH 2/2] Unify some response behavior

Posted by Michal Fojtik <mf...@redhat.com>.
On Jul 30, 2011, at 1:15 AM, lutter@redhat.com wrote:

ACK.

I tested it using all our test suites, all green to go.

  -- Michal

> From: David Lutterkort <lu...@redhat.com>
> 
> This patch makes sure that all responses adhere to the following rules:
> 
>  * The HTTP status is the same across all return formats, in particular
>    XML and JSON
>  * When we return 201 Created, we also set a Location header for the
>    created entity
>  * XML is the default format, i.e., the one that is used if the client
>    does not send an Accept header
> 
> Signed-off-by: David Lutterkort <lu...@redhat.com>
> ---
> server/server.rb                         |  172 +++++++++++++++---------------
> server/tests/drivers/mock/images_test.rb |    5 +-
> 2 files changed, 91 insertions(+), 86 deletions(-)
> 
> diff --git a/server/server.rb b/server/server.rb
> index 864b6b8..60c1d36 100644
> --- a/server/server.rb
> +++ b/server/server.rb
> @@ -171,7 +171,7 @@ end
> collection :images do
>   description <<END
>   An image is a platonic form of a machine. Images are not directly executable,
> -  but are a template for creating actual instances of machines."
> +  but are a template for creating actual instances of machines.
> END
> 
>   operation :index do
> @@ -205,11 +205,10 @@ END
>         :name => params[:name],
> 	:description => params[:description]
>       })
> +      status 201  # Created
> +      response['Location'] = image_url(@instance.id)
>       respond_to do |format|
> -        format.xml do
> -          response.status = 201 # Created
> -          haml :"images/show"
> -        end
> +        format.xml  { haml :"images/show" }
>         format.json { convert_to_json(:image, @image) }
>         format.html { haml :"images/show" }
>       end
> @@ -222,9 +221,10 @@ END
>     param :id,    :string,    :required
>     control do
>       driver.destroy_image(credentials, params[:id])
> +      status 204
>       respond_to do |format|
> -        format.xml { status 204 }
> -        format.json { status 204 }
> +        format.xml
> +        format.json
>         format.html { redirect(images_url) }
>       end
>     end
> @@ -327,11 +327,10 @@ collection :load_balancers do
>     param :listener_instance_port,  :string,  :required
>     control do
>       @load_balancer = driver.create_load_balancer(credentials, params)
> +      status 201  # Created
> +      response['Location'] = load_balancer_url(@instance.id)
>       respond_to do |format|
> -        format.xml do
> -          response.status = 201  # Created
> -          haml :"load_balancers/show"
> -        end
> +        format.xml  { haml :"load_balancers/show" }
>         format.json { convert_to_json(:load_balancer, @load_balancer) }
>         format.html { haml :"load_balancers/show" }
>       end
> @@ -344,9 +343,10 @@ collection :load_balancers do
>     param :instance_id, :string,  :required
>     control do
>       driver.lb_register_instance(credentials, params)
> +      status 204
>       respond_to do |format|
> -        format.xml { 204 }
> -        format.json { 204 }
> +        format.xml
> +        format.json
>         format.html { redirect(load_balancer_url(params[:id])) }
>       end
>     end
> @@ -358,9 +358,10 @@ collection :load_balancers do
>     param :instance_id, :string,  :required
>     control do
>       driver.lb_unregister_instance(credentials, params)
> +      status 204
>       respond_to do |format|
> -        format.xml { 204 }
> -        format.json { 204 }
> +        format.xml
> +        format.json
>         format.html { redirect(load_balancer_url(params[:id])) }
>       end
>     end
> @@ -371,9 +372,10 @@ collection :load_balancers do
>     param :id,  :string,  :required
>     control do
>       driver.destroy_load_balancer(credentials, params[:id])
> +      status 204
>       respond_to do |format|
> -        format.xml {  204 }
> -        format.json { 204 }
> +        format.xml
> +        format.json
>         format.html { redirect(load_balancers_url) }
>       end
>     end
> @@ -385,7 +387,7 @@ end
> collection :instances do
>   description <<END
>   An instance is a concrete machine realized from an image.
> -  The images collection may be obtained by following the link from the primary entry-point."
> +  The images collection may be obtained by following the link from the primary entry-point.
> END
> 
>   operation :index do
> @@ -411,12 +413,10 @@ END
>     param :hwp_id,       :string, :optional
>     control do
>       @instance = driver.create_instance(credentials, params[:image_id], params)
> +      status 201  # Created
> +      response['Location'] = instance_url(@instance.id)
>       respond_to do |format|
> -        format.xml do
> -          response.status = 201  # Created
> -          response['Location'] = instance_url(@instance.id)
> -          haml :"instances/show"
> -        end
> +        format.xml  { haml :"instances/show" }
>         format.json { convert_to_json(:instance, @instance) }
>         format.html do
>           redirect instance_url(@instance.id) if @instance and @instance.id
> @@ -546,8 +546,9 @@ collection :storage_snapshots do
>     with_capability :create_storage_snapshot
>     param :volume_id, :string,  :required
>     control do
> -      response.status = 201  # Created
>       @storage_snapshot = driver.create_storage_snapshot(credentials, params)
> +      status 201  # Created
> +      response['Location'] = storage_snapshot_url(@storage_snapshot.id)
>       show(:storage_snapshot)
>     end
>   end
> @@ -558,9 +559,10 @@ collection :storage_snapshots do
>     param :id,  :string,  :required
>     control do
>       driver.destroy_storage_snapshot(credentials, params)
> +      status 204
>       respond_to do |format|
> -        format.xml { 204 }
> -        format.json { 204 }
> +        format.xml
> +        format.json
>         format.html { redirect(storage_snapshots_url) }
>       end
>     end
> @@ -605,13 +607,12 @@ collection :storage_volumes do
>     param :realm_id,    :string,  :optional
>     control do
>       @storage_volume = driver.create_storage_volume(credentials, params)
> +      status 201
> +      response['Location'] = storage_volume_url(@storage_volume.id)
>       respond_to do |format|
> +        format.xml  { haml :"storage_volumes/show" }
>         format.html { haml :"storage_volumes/show" }
>         format.json { convert_to_json(:storage_volume, @storage_volume) }
> -        format.xml do
> -          response.status = 201  # Created
> -          haml :"storage_volumes/show"
> -        end
>       end
>     end
>   end
> @@ -655,9 +656,10 @@ collection :storage_volumes do
>     param :id,          :string,  :optional
>     control do
>       driver.destroy_storage_volume(credentials, params)
> +      status 204
>       respond_to do |format|
> -        format.xml { 204 }
> -        format.json { 204 }
> +        format.xml
> +        format.json
>         format.html { redirect(storage_volumes_url) }
>       end
>     end
> @@ -695,12 +697,11 @@ collection :keys do
>     param :name,  :string,  :required
>     control do
>       @key = driver.create_key(credentials, { :key_name => params[:name] })
> +      status 201
> +      response['Location'] = key_url(@key.id)
>       respond_to do |format|
> +        format.xml  { haml :"keys/show", :ugly => true }
>         format.html { haml :"keys/show" }
> -        format.xml do
> -          response.status = 201  # Created
> -          haml :"keys/show", :ugly => true
> -        end
>         format.json { convert_to_json(:key, @key)}
>       end
>     end
> @@ -712,9 +713,10 @@ collection :keys do
>     param :id,  :string,  :required
>     control do
>       driver.destroy_key(credentials, { :id => params[:id]})
> +      status 204
>       respond_to do |format|
> -        format.xml { 204 }
> -        format.json { 204 }
> +        format.xml
> +        format.json
>         format.html { redirect(keys_url) }
>       end
>     end
> @@ -738,8 +740,8 @@ put "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
>     @blob = driver.blob(credentials, {:id => params[:blob],
>                                       'bucket' => params[:bucket]})
>     respond_to do |format|
> -      format.html { haml :"blobs/show" }
>       format.xml { haml :"blobs/show" }
> +      format.html { haml :"blobs/show" }
>       format.json { convert_to_json(:blob, @blob) }
>     end
>   elsif(env["BLOB_FAIL"])
> @@ -757,8 +759,8 @@ put "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
>     @blob = driver.create_blob(credentials, bucket_id, blob_id, blob_data, user_meta)
>     temp_file.delete
>     respond_to do |format|
> -      format.html { haml :"blobs/show"}
>       format.xml { haml :"blobs/show" }
> +      format.html { haml :"blobs/show"}
>     end
>   end
> end
> @@ -781,8 +783,8 @@ post "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket" do
>   end
>   @blob = driver.create_blob(credentials, bucket_id, blob_id, blob_data, user_meta)
>   respond_to do |format|
> -    format.html { haml :"blobs/show"}
>     format.xml { haml :"blobs/show" }
> +    format.html { haml :"blobs/show"}
>   end
> end
> 
> @@ -791,9 +793,10 @@ delete "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
>   bucket_id = params[:bucket]
>   blob_id = params[:blob]
>   driver.delete_blob(credentials, bucket_id, blob_id)
> +  status 204
>   respond_to do |format|
> -    format.xml {  204 }
> -    format.json {  204 }
> +    format.xml
> +    format.json
>     format.html { redirect(bucket_url(bucket_id)) }
>   end
> end
> @@ -806,9 +809,10 @@ head "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
>       @blob_metadata.each do |k,v|
>         headers["X-Deltacloud-Blobmeta-#{k}"] = v
>       end
> +      status 204
>       respond_to do |format|
> -        format.xml {  204 }
> -        format.json {  204 }
> +        format.xml
> +        format.json
>       end
>    else
>     report_error(404)
> @@ -823,9 +827,10 @@ post "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
>     meta_hash.each do |k,v|
>       headers["X-Deltacloud-Blobmeta-#{k}"] = v
>     end
> +    status 204
>     respond_to do |format|
> -      format.xml {  204 }
> -      format.json {  204 }
> +      format.xml
> +      format.json
>     end
>   else
>     report_error(404) #FIXME is this the right error code?
> @@ -837,8 +842,8 @@ get "#{Sinatra::UrlForHelper::DEFAULT_URI_PREFIX}/buckets/:bucket/:blob" do
>   @blob = driver.blob(credentials, { :id => params[:blob], 'bucket' => params[:bucket]})
>   if @blob
>     respond_to do |format|
> -      format.html { haml :"blobs/show" }
>       format.xml { haml :"blobs/show" }
> +      format.html { haml :"blobs/show" }
>       format.json { convert_to_json(:blob, @blob) }
>       end
>   else
> @@ -891,19 +896,15 @@ collection :buckets do
>     param :name,      :string,    :required
>     control do
>       @bucket = driver.create_bucket(credentials, params[:name], params)
> +      status 201
> +      response['Location'] = bucket_url(@bucket.id)
>       respond_to do |format|
> +        format.xml  { haml :"buckets/show" }
> +        format.json { convert_to_json(:bucket, @bucket) }
>         format.html do
>           redirect bucket_url(@bucket.id) if @bucket and @bucket.id
>           redirect buckets_url
>         end
> -        format.xml do
> -          response.status = 201  # Created
> -          response['Location'] = bucket_url(@bucket.id)
> -          haml :"buckets/show"
> -        end
> -        format.json do
> -          convert_to_json(:bucket, @bucket)
> -        end
>       end
>     end
>   end
> @@ -914,9 +915,10 @@ collection :buckets do
>     param :id,    :string,    :required
>     control do
>       driver.delete_bucket(credentials, params[:id], params)
> +      status 204
>       respond_to do |format|
> -        format.xml { 204 }
> -        format.json {  204 }
> +        format.xml
> +        format.json
>         format.html {  redirect(buckets_url) }
>       end
>     end
> @@ -955,14 +957,12 @@ collection :addresses do
>     with_capability :create_address
>     control do
>       @address = driver.create_address(credentials, {})
> +      status 201    # Created
> +      response['Location'] = address_url(@address.id)
>       respond_to do |format|
> +        format.xml  { haml :"addresses/show", :ugly => true }
>         format.html { haml :"addresses/show" }
>         format.json { convert_to_json(:address, @address) }
> -        format.xml do
> -          response.status = 201  # Created
> -          response['Location'] = address_url(@address.id)
> -          haml :"addresses/show", :ugly => true
> -        end
>       end
>     end
>   end
> @@ -973,9 +973,10 @@ collection :addresses do
>     param :id,  :string,  :required
>     control do
>       driver.destroy_address(credentials, { :id => params[:id]})
> +      status 204
>       respond_to do |format|
> -        format.xml { 204 }
> -        format.json { 204 }
> +        format.xml
> +        format.json
>         format.html { redirect(addresses_url) }
>       end
>     end
> @@ -988,9 +989,10 @@ collection :addresses do
>     param :instance_id, :string, :required
>     control do
>       driver.associate_address(credentials, { :id => params[:id], :instance_id => params[:instance_id]})
> +      status 202   # Accepted
>       respond_to do |format|
> -        format.xml { 202 }  # Accepted
> -        format.json { 202 }
> +        format.xml
> +        format.json
>         format.html { redirect(address_url(params[:id])) }
>       end
>     end
> @@ -1002,9 +1004,10 @@ collection :addresses do
>     param :id, :string, :required
>     control do
>       driver.disassociate_address(credentials, { :id => params[:id] })
> +      status 202   # Accepted
>       respond_to do |format|
> -        format.xml { 202 }  # Accepted
> -        format.json { 202 }
> +        format.xml
> +        format.json
>         format.html { redirect(address_url(params[:id])) }
>       end
>     end
> @@ -1033,10 +1036,11 @@ delete '/api/firewalls/:firewall/:rule' do
>   opts[:firewall] = params[:firewall]
>   opts[:rule_id] = params[:rule]
>   driver.delete_firewall_rule(credentials, opts)
> +  status 204
>   respond_to do |format|
> +    format.xml
> +    format.json
>     format.html {redirect firewall_url(params[:firewall])}
> -    format.xml {204}
> -    format.json {204}
>   end
> end
> 
> @@ -1063,13 +1067,12 @@ collection :firewalls do
>     param :description,   :string,    :required
>     control do
>       @firewall = driver.create_firewall(credentials, params )
> +      status 201  # Created
> +      response['Location'] = firewall_url(@firewall.id)
>       respond_to do |format|
> -        format.xml do
> -          response.status = 201  # Created
> -          haml :"firewalls/show"
> -        end
> -        format.html {haml :"firewalls/show"}
> -        format.json {convert_to_json(:firewall, @firewall)}
> +        format.xml  { haml :"firewalls/show" }
> +        format.html { haml :"firewalls/show" }
> +        format.json { convert_to_json(:firewall, @firewall) }
>       end
>     end
>   end
> @@ -1080,9 +1083,10 @@ collection :firewalls do
>     param :id,            :string,    :required
>     control do
>       driver.delete_firewall(credentials, params)
> +      status 204
>       respond_to do |format|
> -        format.xml { 204 }
> -        format.json {  204 }
> +        format.xml
> +        format.json
>         format.html {  redirect(firewalls_url) }
>       end
>     end
> @@ -1108,13 +1112,11 @@ collection :firewalls do
>       params.merge!( {'addresses' => addresses} ) ; params.merge!( {'groups' => groups} )
>       driver.create_firewall_rule(credentials, params)
>       @firewall = driver.firewall(credentials, {:id => params[:id]})
> +      status 201
>       respond_to do |format|
> -        format.html {haml :"firewalls/show"}
> -        format.xml do
> -          response.status = 201 #created
> -          haml :"firewalls/show"
> -        end
> -        format.json {convert_to_json(:firewall, @firewall)}
> +        format.xml  { haml :"firewalls/show" }
> +        format.html { haml :"firewalls/show" }
> +        format.json { convert_to_json(:firewall, @firewall) }
>       end
>     end
>   end
> diff --git a/server/tests/drivers/mock/images_test.rb b/server/tests/drivers/mock/images_test.rb
> index bbe6ed3..47fb690 100644
> --- a/server/tests/drivers/mock/images_test.rb
> +++ b/server/tests/drivers/mock/images_test.rb
> @@ -113,9 +113,12 @@ module DeltacloudUnitTest
>       Nokogiri::HTML(last_response.body).search('html').first.name.should == 'html'
>     end
> 
> -    def test_it_can_destroy_created_image
> +    def test_it_creates_and_destroys_image_from_instance
>       post_url "/api/images", { :name => "img4", :description => "Test::Unit image", :instance_id => "inst1"}
>       last_response.status.should == 201
> +      last_response.headers['Location'].should_not == nil
> +      get_auth_url last_response.headers['Location'], {}
> +      (last_xml_response/'instance/name').should_not == nil
>       delete_url "/api/images/img4", {}
>       last_response.status.should == 204
>       get_auth_url "/api/images/img4", {}
> -- 
> 1.7.6
> 

------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org