deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mfoj...@redhat.com
Subject [PATCH core 10/11] CIMI: Replaced :grab_content_type helper with :current_content_type
Date Mon, 11 Feb 2013 12:35:51 GMT
From: Michal Fojtik <mfojtik@redhat.com>

In Sinatra helpers have direct access to the 'request' method,
so there is no need to pass any parameter to this helper.

This patch will also report unsupported media type correctly
via Deltacloud exception handler with proper HTTP status code.

Signed-off-by: Michal fojtik <mfojtik@redhat.com>
---
 server/lib/cimi/collections/address_templates.rb   |  2 +-
 server/lib/cimi/collections/addresses.rb           |  2 +-
 server/lib/cimi/collections/credentials.rb         |  2 +-
 server/lib/cimi/collections/machine_templates.rb   |  7 +---
 server/lib/cimi/collections/machines.rb            |  8 ++--
 server/lib/cimi/collections/network_ports.rb       |  6 +--
 server/lib/cimi/collections/networks.rb            |  8 ++--
 .../lib/cimi/collections/volume_configurations.rb  |  2 +-
 server/lib/cimi/collections/volume_templates.rb    |  5 ++-
 server/lib/cimi/collections/volumes.rb             |  3 +-
 server/lib/cimi/helpers/cimi_helper.rb             | 45 ++++++----------------
 server/lib/cimi/models/machine_image.rb            |  2 +-
 server/lib/cimi/models/volume_image.rb             |  2 +-
 13 files changed, 35 insertions(+), 59 deletions(-)

diff --git a/server/lib/cimi/collections/address_templates.rb b/server/lib/cimi/collections/address_templates.rb
index fea3ca1..d1bbe7d 100644
--- a/server/lib/cimi/collections/address_templates.rb
+++ b/server/lib/cimi/collections/address_templates.rb
@@ -45,7 +45,7 @@ module CIMI::Collections
       operation :create do
         description "Create new AddressTemplate"
         control do
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             new_address_template = CIMI::Model::AddressTemplate.create_from_json(request.body.read,
self)
           else
             new_address_template = CIMI::Model::AddressTemplate.create_from_xml(request.body.read,
self)
diff --git a/server/lib/cimi/collections/addresses.rb b/server/lib/cimi/collections/addresses.rb
index 9ea1764..5133894 100644
--- a/server/lib/cimi/collections/addresses.rb
+++ b/server/lib/cimi/collections/addresses.rb
@@ -47,7 +47,7 @@ module CIMI::Collections
       operation :create, :with_capability => :create_address do
         description "Create a new Address"
         control do
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             address = CIMI::Model::Address.create(request.body.read, self, :json)
           else
             address = CIMI::Model::Address.create(request.body.read, self, :xml)
diff --git a/server/lib/cimi/collections/credentials.rb b/server/lib/cimi/collections/credentials.rb
index dd0fbda..fcaf458 100644
--- a/server/lib/cimi/collections/credentials.rb
+++ b/server/lib/cimi/collections/credentials.rb
@@ -46,7 +46,7 @@ module CIMI::Collections
       operation :create, :with_capability => :create_key do
         description "Show specific machine admin"
         control do
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             new_admin = Credential.create_from_json(request.body.read, self)
           else
             new_admin = Credential.create_from_xml(request.body.read, self)
diff --git a/server/lib/cimi/collections/machine_templates.rb b/server/lib/cimi/collections/machine_templates.rb
index 99ce5cf..8734319 100644
--- a/server/lib/cimi/collections/machine_templates.rb
+++ b/server/lib/cimi/collections/machine_templates.rb
@@ -45,11 +45,8 @@ module CIMI::Collections
       operation :create do
         description "Create new machine template"
         control do
-          if grab_content_type(request.content_type, request.body) == :json
-            new_machine_template = CIMI::Model::MachineTemplate.create_from_json(request.body.read,
self)
-          else
-            new_machine_template = CIMI::Model::MachineTemplate.create_from_xml(request.body.read,
self)
-          end
+          mt = MachineTemplateCreate.parse(request.body, request.content_type)
+          new_machine_template = mt.create(self)
           headers_for_create new_machine_template
           respond_to do |format|
             format.json { new_machine_template.to_json }
diff --git a/server/lib/cimi/collections/machines.rb b/server/lib/cimi/collections/machines.rb
index 88b3c99..e3bada1 100644
--- a/server/lib/cimi/collections/machines.rb
+++ b/server/lib/cimi/collections/machines.rb
@@ -69,7 +69,7 @@ module CIMI::Collections
         param :id,          :string,    :required
         control do
           machine = Machine.find(params[:id], self)
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
@@ -86,7 +86,7 @@ module CIMI::Collections
         param :id,          :string,    :required
         control do
           machine = Machine.find(params[:id], self)
-          if  grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             action = Action.from_json(request.body.read.gsub("restart", "reboot"))
           else
             action = Action.from_xml(request.body.read.gsub("restart", "reboot"))
@@ -103,7 +103,7 @@ module CIMI::Collections
         param :id,          :string,    :required
         control do
           machine = Machine.find(params[:id], self)
-          if  grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
@@ -171,7 +171,7 @@ module CIMI::Collections
         description "Attach CIMI Volume(s) to a machine."
         param :id,          :string,    :required
         control do
-          if  grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             volume_to_attach, location = MachineVolume.find_to_attach_from_json(request.body.read,
self)
           else
             volume_to_attach, location = MachineVolume.find_to_attach_from_xml(request.body.read,
self)
diff --git a/server/lib/cimi/collections/network_ports.rb b/server/lib/cimi/collections/network_ports.rb
index ee7b83a..28c2df4 100644
--- a/server/lib/cimi/collections/network_ports.rb
+++ b/server/lib/cimi/collections/network_ports.rb
@@ -47,7 +47,7 @@ module CIMI::Collections
       operation :create, :with_capability => :create_network_port do
         description "Create a new NetworkPort"
         control do
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             network_port = CIMI::Model::NetworkPort.create(request.body.read, self, :json)
           else
             network_port = CIMI::Model::NetworkPort.create(request.body.read, self, :xml)
@@ -73,7 +73,7 @@ module CIMI::Collections
         control do
           network_port = NetworkPort.find(params[:id], self)
           report_error(404) unless network_port
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
@@ -91,7 +91,7 @@ module CIMI::Collections
         control do
           network_port = NetworkPort.find(params[:id], self)
           report_error(404) unless network_port
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
diff --git a/server/lib/cimi/collections/networks.rb b/server/lib/cimi/collections/networks.rb
index 377a868..16e4c9c 100644
--- a/server/lib/cimi/collections/networks.rb
+++ b/server/lib/cimi/collections/networks.rb
@@ -46,7 +46,7 @@ module CIMI::Collections
       operation :create, :with_capability => :create_network do
         description "Create a new Network"
         control do
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             network = Network.create(request.body.read, self, :json)
           else
             network = Network.create(request.body.read, self, :xml)
@@ -72,7 +72,7 @@ module CIMI::Collections
         control do
           network = Network.find(params[:id], self)
           report_error(404) unless network
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
@@ -90,7 +90,7 @@ module CIMI::Collections
         control do
           network = Network.find(params[:id], self)
           report_error(404) unless network
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
@@ -108,7 +108,7 @@ module CIMI::Collections
         control do
           network = Network.find(params[:id], self)
           report_error(404) unless network
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
diff --git a/server/lib/cimi/collections/volume_configurations.rb b/server/lib/cimi/collections/volume_configurations.rb
index 6a03dc5..0a33794 100644
--- a/server/lib/cimi/collections/volume_configurations.rb
+++ b/server/lib/cimi/collections/volume_configurations.rb
@@ -45,7 +45,7 @@ module CIMI::Collections
       operation :create, :with_capability => :create_storage_volume do
         description "Create new VolumeConfiguration"
         control do
-          if grab_content_type(request.content_type, request.body) == :json
+          if current_content_type == :json
             new_config = CIMI::Model::VolumeConfiguration.create_from_json(request.body.read,
self)
           else
             new_config = CIMI::Model::VolumeConfiguration.create_from_xml(request.body.read,
self)
diff --git a/server/lib/cimi/collections/volume_templates.rb b/server/lib/cimi/collections/volume_templates.rb
index 4e3dfdb..720e73b 100644
--- a/server/lib/cimi/collections/volume_templates.rb
+++ b/server/lib/cimi/collections/volume_templates.rb
@@ -45,8 +45,9 @@ module CIMI::Collections
       operation :create, :with_capability => :create_storage_volume do
         description "Create new VolumeTemplate"
         control do
-          content_type = grab_content_type(request.content_type, request.body)
-          new_template = CIMI::Model::VolumeTemplate.create(request.body.read, self, content_type)
+          new_template = CIMI::Model::VolumeTemplate.create(
+            request.body.read, self, current_content_type
+          )
           headers_for_create new_template
           respond_to do |format|
             format.json { new_template.to_json }
diff --git a/server/lib/cimi/collections/volumes.rb b/server/lib/cimi/collections/volumes.rb
index bf1f5f0..74f8c69 100644
--- a/server/lib/cimi/collections/volumes.rb
+++ b/server/lib/cimi/collections/volumes.rb
@@ -49,8 +49,7 @@ module CIMI::Collections
       operation :create, :with_capability => :create_storage_volume do
         description "Create a new Volume."
         control do
-          content_type = grab_content_type(request.content_type, request.body)
-          new_volume = Volume.create(request.body.read, self, content_type)
+          new_volume = Volume.create(request.body.read, self, current_content_type)
           headers_for_create new_volume
           respond_to do |format|
             format.json { new_volume.to_json }
diff --git a/server/lib/cimi/helpers/cimi_helper.rb b/server/lib/cimi/helpers/cimi_helper.rb
index b062af7..5e74c2c 100644
--- a/server/lib/cimi/helpers/cimi_helper.rb
+++ b/server/lib/cimi/helpers/cimi_helper.rb
@@ -16,6 +16,18 @@
 module CIMI
   module Helper
 
+    def current_content_type
+      case request.content_type
+        when 'application/json' then :json
+        when 'text/xml', 'application/xml' then :xml
+        else
+          raise Deltacloud::Exceptions.exception_from_status(
+            406,
+            translate_error_code(406)[:message]
+          )
+      end
+    end
+
     def expand?(collection)
       params['$expand'] == '*' ||
         (params['$expand'] || '').split(',').include?(collection.to_s)
@@ -59,39 +71,6 @@ module CIMI
       end
     end
 
-    def grab_content_type(request_content_type, request_body)
-      case request_content_type
-        when /xml$/ then :xml
-        when /json$/ then :json
-        else raise CIMI::Model::UnsupportedMediaType.new("Unsupported content type - only
xml and json supported by CIMI")
-        #guess_content_type(request_body)
-      end
-    end
-
-    #not being used - was called from above grab_content_type
-    #decided to reject anything not xml || json
-    def guess_content_type(request_body)
-      xml = json = false
-      body = request_body.read
-      request_body.rewind
-      begin
-        XmlSimple.xml_in(body)
-        xml = true
-      rescue Exception
-        xml = false
-      end
-      begin
-        JSON.parse(body)
-        json = true
-      rescue Exception
-        json = false
-      end
-      if (json == xml) #both true or both false
-        raise CIMI::Model::BadRequest.new("Couldn't guess content type of: #{body}")
-      end
-      type = (xml)? :xml : :json
-    end
-
     def deltacloud_create_method_for(cimi_entity)
       case cimi_entity
         when "machine"                then "create_instance"
diff --git a/server/lib/cimi/models/machine_image.rb b/server/lib/cimi/models/machine_image.rb
index 247ff72..98f0625 100644
--- a/server/lib/cimi/models/machine_image.rb
+++ b/server/lib/cimi/models/machine_image.rb
@@ -53,7 +53,7 @@ class CIMI::Model::MachineImage < CIMI::Model::Base
     # there is no way how to figure out from what Machine the MachineImage was
     # created from. For that we need to store this attribute in properties.
     #
-    if context.grab_content_type(context.request.content_type, request_body) == :xml
+    if context.current_content_type == :xml
       input = XmlSimple.xml_in(request_body.read, {"ForceArray"=>false,"NormaliseSpace"=>2})
       raise 'imageLocation attribute is mandatory' unless input['imageLocation']
       input['property'] ||= {}
diff --git a/server/lib/cimi/models/volume_image.rb b/server/lib/cimi/models/volume_image.rb
index 2dc04b5..4858ad8 100644
--- a/server/lib/cimi/models/volume_image.rb
+++ b/server/lib/cimi/models/volume_image.rb
@@ -39,7 +39,7 @@ class CIMI::Model::VolumeImage < CIMI::Model::Base
   def self.all(context); find(:all, context); end
 
   def self.create(request_body, context)
-    type = context.grab_content_type(context.request.content_type, request_body)
+    type = context.current_content_type
     input = (type == :xml)? XmlSimple.xml_in(request_body.read, {"ForceArray"=>false,"NormaliseSpace"=>2})
: JSON.parse(request_body.read)
     params = {:volume_id => context.href_id(input["imageLocation"]["href"], :volumes),
:name=>input["name"], :description=>input["description"]}
     vol_image = context.driver.create_storage_snapshot(context.credentials, params)
-- 
1.8.1.2


Mime
View raw message