Mailing-List: contact deltacloud-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: deltacloud-dev@incubator.apache.org
Received-SPF: pass (nike.apache.org: domain of mfojtik@redhat.com designates
 209.132.183.28 as permitted sender)
From: mfojtik@redhat.com
To: deltacloud-dev@incubator.apache.org
Subject: [PATCH core] Various fixes in REST and Load Balancer code.
Date: Tue,  7 Dec 2010 14:53:37 +0100
Message-Id: <1291730017-12321-1-git-send-email-mfojtik@redhat.com>

From: Michal Fojtik <mfojtik@redhat.com>

---
 .../lib/deltacloud/helpers/application_helper.rb   |   22 +++++++--
 server/server.rb                                   |   48 ++++++++++++--------
 server/views/load_balancers/new.html.haml          |    4 +-
 server/views/load_balancers/show.html.haml         |    3 +-
 4 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/server/lib/deltacloud/helpers/application_helper.rb b/server/lib/deltacloud/helpers/application_helper.rb
index f538714..b27e377 100644
--- a/server/lib/deltacloud/helpers/application_helper.rb
+++ b/server/lib/deltacloud/helpers/application_helper.rb
@@ -112,12 +112,26 @@ module ApplicationHelper
 
     @instance = driver.send(:"#{name}_instance", credentials, params["id"])
 
-    return redirect(instances_url) if name.eql?(:destroy) or @instance.class!=Instance
+    # Check if instance has image_id attribute, so for example when you execute
+    # action and there is no response from that action (eg. no Instance),  it
+    # means that this action doesn't modify a state of given Instance
+    # eg. try a 'start' action on non-ELB instance which is in STOPPED state
+    unless @instance
+      status 304
+      response['Location'] = instance_url(params[:id])
+      halt
+    end
+
+    if name.eql?(:destroy) or @instance.class!=Instance
+      status 410 # Instance is Gone and clients should not link to this resouce
+    else
+      status 202 # Instance action was accepted and it will be executed on backend
+    end
 
     respond_to do |format|
-      format.xml { haml :"instances/show" }
-      format.html { haml :"instances/show" }
-      format.json {convert_to_json(:instance, @instance) }
+      format.xml { haml(:"instances/show") }
+      format.html { haml(:"instances/show") }
+      format.json { convert_to_json(:instance, @instance) }
     end
   end
 
diff --git a/server/server.rb b/server/server.rb
index 076859d..0e36fd5 100644
--- a/server/server.rb
+++ b/server/server.rb
@@ -229,19 +229,21 @@ collection :load_balancers do
     control do
       @load_balancer = driver.create_load_balancer(credentials, params)
       respond_to do |format|
-        format.xml { haml :"load_balancers/show" }
-        format.html { haml :"load_balancers/show" }
+        format.xml { [ 201, haml(:"load_balancers/show") ] }
+        format.html { [ 201, haml(:"load_balancers/show") ] }
       end
     end
   end
 
-  operation :register, :method => :post, :member => true do
+  operation :register, :method => :put, :member => true do
     description "Add instance to loadbalancer"
     param :id,  :string,  :required
     param :instance_id, :string,  :required
     control do
-      driver.lb_register_instance(credentials, params)
-      redirect(load_balancer_url(params[:id]))
+      if driver.lb_register_instance(credentials, params)
+        status 202 # Accepted
+        show( :load_balancer )
+      end
     end
   end
 
@@ -250,8 +252,10 @@ collection :load_balancers do
     param :id,  :string,  :required
     param :instance_id, :string,  :required
     control do
-      driver.lb_unregister_instance(credentials, params)
-      redirect(load_balancer_url(params[:id]))
+      if driver.lb_unregister_instance(credentials, params)
+        status 202
+        show( :load_balancer )
+      end
     end
   end
 
@@ -259,8 +263,9 @@ collection :load_balancers do
     description "Destroy given load balancer"
     param :id,  :string,  :required
     control do
-      driver.destroy_load_balancer(credentials, params[:id])
-      redirect(load_balancers_url)
+      if driver.destroy_load_balancer(credentials, params[:id])
+        status 410
+      end
     end
   end
 
@@ -299,10 +304,10 @@ END
       instance = driver.create_instance(credentials, @image.id, params)
       respond_to do |format|
         format.xml do
-          response.status = 201  # Created
           response['Location'] = instance_url(instance.id)
           @instance = instance
-          haml :"instances/show"
+          status 210
+          haml(:"instances/show")
         end
         format.html do
           redirect instance_url(instance.id) if instance and instance.id
@@ -450,9 +455,11 @@ collection :keys do
     param :name,  :string,  :required
     control do
       @key = driver.create_key(credentials, { :key_name => params[:name] })
+      response['Location'] = key_url(@key.id)
+      status 210
       respond_to do |format|
-        format.html { haml :"keys/show" }
-        format.xml { haml :"keys/show", :ugly => true }
+        format.html { haml(:"keys/show") }
+        format.xml { haml(:"keys/show", :ugly => true) }
       end
     end
   end
@@ -462,8 +469,9 @@ collection :keys do
     with_capability :destroy_key
     param :id,  :string,  :required
     control do
-      driver.destroy_key(credentials, { :key_name => params[:id]})
-      redirect(keys_url)
+      if driver.destroy_key(credentials, { :key_name => params[:id]})
+        status 410 # Resource is 'Gone'
+      end
     end
   end
 
@@ -493,8 +501,9 @@ end
 delete '/api/buckets/:bucket/:blob' do
   bucket_id = params[:bucket]
   blob_id = params[:blob]
-  driver.delete_blob(credentials, bucket_id, blob_id)
-  redirect(bucket_url(bucket_id))
+  if driver.delete_blob(credentials, bucket_id, blob_id)
+    status 410 # Resource is 'Gone'
+  end
 end
 
 #Get a particular blob's particulars (not actual blob data)
@@ -570,8 +579,9 @@ collection :buckets do
     with_capability :delete_bucket
     param :id,    :string,    :required
     control do
-      driver.delete_bucket(credentials, params[:id], params)
-      redirect(buckets_url)
+      if driver.delete_bucket(credentials, params[:id], params)
+        status 410
+      end
     end
   end
 
diff --git a/server/views/load_balancers/new.html.haml b/server/views/load_balancers/new.html.haml
index 561caa4..58a89f7 100644
--- a/server/views/load_balancers/new.html.haml
+++ b/server/views/load_balancers/new.html.haml
@@ -28,11 +28,11 @@
   %p
     %label
       Load balancer port:
-    %input{ :name => "listener_lbr_port", :size => 30}
+    %input{ :name => "listener_balancer_port", :size => 30}
   %p
     %label
       Instances port:
-    %input{ :name => "listener_inst_port", :size => 30}
+    %input{ :name => "listener_instance_port", :size => 30}
   %p
     %input{ :type => :submit, :name => "commit", :value => "create" }/
 
diff --git a/server/views/load_balancers/show.html.haml b/server/views/load_balancers/show.html.haml
index 26e1766..6ef52b9 100644
--- a/server/views/load_balancers/show.html.haml
+++ b/server/views/load_balancers/show.html.haml
@@ -25,9 +25,10 @@
       - @load_balancer.instances.each do |inst|
         %dd
           =inst.id
-          =link_to_action, 'Delete', unregister_load_balancer_url(@load_balancer.id, :instance_id => inst.id), :post
+          =link_to_action 'Delete', unregister_load_balancer_url(@load_balancer.id, :instance_id => inst.id), :post
 
 %form{:action => url_for("/api/load_balancers/#{@load_balancer.id}/register"), :method => :post}
+  %input{ :type => :hidden, :name => "_method", :value => 'put'}
   %p
     %strong Add instances to load balancer
   %p
-- 
1.7.3.2