You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by mf...@redhat.com on 2011/10/27 13:17:20 UTC
Second revamp of CIMI code
Hi,
This patchset should improve the code we have currently for CIMI.
I will briefly explain what each patch did and why:
1/8:
This is just cosmetic change. It's better to have consistent DSL
rather than using 'global!' method inside all collections.
If some collection want be 'global' lets call it 'global_collection'.
This change was tested on Ruby 1.8 and 1.9 and should not break anything.
2/8:
Using EOS is a bit weird to me. We used EOL just because our Deltacloud
descriptions was too long to fit into one line ;-)
However the CIMI descriptions are short one liners so why not put them
into regular string.
3-6/8:
The code in cmwgapp_helper was too much complex to me to undestand
(and read) so I decided to rewrite it. You can now see the difference.
I dunno how about you, but I had very hard time to undestand what
'fixup_content' or 'respond_to_collection' methods internally do.
We can discuss it, but my personal opinion is that hacking XML using
XmlSimple and then modifying resulting Hash is too complex.
7/8:
The code previosly used to return collection was using XmlSimple
and not HAML. This throw an Exception in Ruby 1.8:
NoMethodError - undefined method `downcase' for 77:Fixnum:
./bin/../lib/cimi/helpers/cmwgapp_helper.rb:68:in `respond_to_collection'
This is one of the examples of darkside XmlSimple. The code is too
complex and thus there is higher propability of exceptions like this one.
I spend 2 hours debugging the code until I finally discover from where
this problem original came.
The 'index.xml'haml' files are easy readable, we can update them according
to CIMI everytime and everyone can undestand what should be returned on output
just by looking on HAML file. Later on, we can re-use this index.xml.haml
as we did in Deltacloud and use 'partial' rendering to render items in
collection.
8/8:
I had long conversation about this with Tong Li and as far I undestand
his point of using this variable is to make a bridge between Deltacloud API
object model and CIMI. However I think we can easely change our object
to support CIMI specific properties very easely without breaking anything.
Since every class is open we can simple 're-open' the model class (like
Instance) and add more methods here.
My personal opinion is that as this implementation will grow and we will
need to add more data and properties, this bridge will become our nightmare.
-----
IMPORTANT: This patch is not addressing the UI. The HAML views will need to
be updated to support new local variables and stuff.
Especially 'new' operation views, will need to be updated together
with our models. I'll take this task once we will have basic
Test::Unit ready (to be sure I'll not break anything).
PS: This patchset was tested under Ruby 1.8 and 1.9 (and Rubinius for fun ;-).
-- Michal
[PATCH core 7/8] CIMI: Added HAML for listing collection (index.xml.haml)
Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
.../cimi/machine_configurations/index.xml.haml | 8 ++++++++
server/views/cimi/machine_images/index.xml.haml | 8 ++++++++
server/views/cimi/machines/index.xml.haml | 8 ++++++++
server/views/cimi/volumes/index.xml.haml | 8 ++++++++
4 files changed, 32 insertions(+), 0 deletions(-)
create mode 100644 server/views/cimi/machine_configurations/index.xml.haml
create mode 100644 server/views/cimi/machine_images/index.xml.haml
create mode 100644 server/views/cimi/machines/index.xml.haml
create mode 100644 server/views/cimi/volumes/index.xml.haml
diff --git a/server/views/cimi/machine_configurations/index.xml.haml b/server/views/cimi/machine_configurations/index.xml.haml
new file mode 100644
index 0000000..f3b9b04
--- /dev/null
+++ b/server/views/cimi/machine_configurations/index.xml.haml
@@ -0,0 +1,8 @@
+%MachineConfigurationCollection{ :xmlns => CMWG_NAMESPACE }
+ %uri machineConfiguration
+ - @resources.each do |resource|
+ %machineConfiguration{ :href => resource[:href] }
+ %name Machine Configuration collection
+ %description That is just a test
+ %created 2011-09-12 11:37:28 UTC
+ %operation{ :rel => :add, :href => "/machine_configurations"}
diff --git a/server/views/cimi/machine_images/index.xml.haml b/server/views/cimi/machine_images/index.xml.haml
new file mode 100644
index 0000000..37d0e1e
--- /dev/null
+++ b/server/views/cimi/machine_images/index.xml.haml
@@ -0,0 +1,8 @@
+%MachineImageCollection{ :xmlns => CMWG_NAMESPACE }
+ %uri machineImage
+ - @resources.each do |resource|
+ %machineImage{ :href => resource[:href] }
+ %name Machine Config collection
+ %description The machine config collection
+ %created 2011-09-12 11:37:28 UTC
+ %operation{ :rel => :add, :href => "/machine_images"}
diff --git a/server/views/cimi/machines/index.xml.haml b/server/views/cimi/machines/index.xml.haml
new file mode 100644
index 0000000..d314a7d
--- /dev/null
+++ b/server/views/cimi/machines/index.xml.haml
@@ -0,0 +1,8 @@
+%MachineCollection{ :xmlns => CMWG_NAMESPACE }
+ %uri machine
+ - @resources.each do |resource|
+ %machine{ :href => resource[:href] }/
+ %name Machine collection
+ %description The machine collection
+ %created 2011-09-12 11:37:28 UTC
+ %operation{ :rel => :add, :href => "/machine_images"}/
diff --git a/server/views/cimi/volumes/index.xml.haml b/server/views/cimi/volumes/index.xml.haml
new file mode 100644
index 0000000..3706e36
--- /dev/null
+++ b/server/views/cimi/volumes/index.xml.haml
@@ -0,0 +1,8 @@
+%VolumeCollection{ :xmlns => CMWG_NAMESPACE }
+ %uri volume
+ - @resources.each do |resource|
+ %volume{ :href => resource[:href] }
+ %name Volume collection
+ %description The volume collection
+ %created 2011-09-12 11:37:28 UTC
+ %operation{ :rel => :add, :href => "/volumes"}
--
1.7.4.4
Re: [PATCH core 2/8] CIMI: Replaced EOS with regular string in collection description
Posted by Michal Fojtik <mf...@redhat.com>.
On Oct 27, 2011, at 2:37 PM, Tong Li wrote:
> ACK!
Thanks! Pushed to SVN. (r1189732)
>
> Tong Li
> Emerging Technologies & Standards
> B062/K317
> litong01@us.ibm.com
>
> mfojtik@redhat.com wrote on 10/27/2011 07:17:22 AM:
>
>> From: mfojtik@redhat.com
>> To: deltacloud-dev@incubator.apache.org
>> Date: 10/27/2011 07:18 AM
>> Subject: [PATCH core 2/8] CIMI: Replaced EOS with regular string in
>> collection description
>>
>> From: Michal Fojtik <mf...@redhat.com>
>>
>>
>> Signed-off-by: Michal fojtik <mf...@redhat.com>
>> ---
>> server/lib/cimi/server.rb | 27 +++++----------------------
>> 1 files changed, 5 insertions(+), 22 deletions(-)
>>
>> diff --git a/server/lib/cimi/server.rb b/server/lib/cimi/server.rb
>> index a76fe61..d995f34 100644
>> --- a/server/lib/cimi/server.rb
>> +++ b/server/lib/cimi/server.rb
>> @@ -77,9 +77,7 @@ get "#{settings.root_url}\/?" do
>> end
>>
>> global_collection :cloudEntryPoint do
>> - description <<EOS
>> - cloud entry point
>> -EOS
>> + description 'Cloud entry point'
>>
>> operation :index do
>> description "list all resources of the cloud"
>> @@ -91,11 +89,7 @@ EOS
>> end
>>
>> global_collection :machine_configurations do
>> -
>> -
>> - description <<EOS
>> -List all machine configurations
>> -EOS
>> + description 'List all machine configurations'
>>
>> operation :index do
>> description "List all machine configurations"
>> @@ -137,11 +131,7 @@ EOS
>> end
>>
>> global_collection :machine_images do
>> -
>> -
>> - description <<EOS
>> -List all machine images
>> -EOS
>> + description 'List all machine images'
>>
>> operation :index do
>> description "List all machine configurations"
>> @@ -185,11 +175,7 @@ EOS
>> end
>>
>> global_collection :machines do
>> -
>> -
>> - description <<EOS
>> -List all machine
>> -EOS
>> + description 'List all machine'
>>
>> operation :index do
>> description "List all machines"
>> @@ -233,10 +219,7 @@ EOS
>> end
>>
>> global_collection :volumes do
>> -
>> - description <<EOS
>> -List all volumes
>> -EOS
>> + description 'List all volumes'
>>
>> operation :index do
>> description "List all volumes"
>> --
>> 1.7.4.4
------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org
Re: [PATCH core 2/8] CIMI: Replaced EOS with regular string in collection
description
Posted by Tong Li <li...@us.ibm.com>.
ACK!
Tong Li
Emerging Technologies & Standards
B062/K317
litong01@us.ibm.com
mfojtik@redhat.com wrote on 10/27/2011 07:17:22 AM:
> From: mfojtik@redhat.com
> To: deltacloud-dev@incubator.apache.org
> Date: 10/27/2011 07:18 AM
> Subject: [PATCH core 2/8] CIMI: Replaced EOS with regular string in
> collection description
>
> From: Michal Fojtik <mf...@redhat.com>
>
>
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
> server/lib/cimi/server.rb | 27 +++++----------------------
> 1 files changed, 5 insertions(+), 22 deletions(-)
>
> diff --git a/server/lib/cimi/server.rb b/server/lib/cimi/server.rb
> index a76fe61..d995f34 100644
> --- a/server/lib/cimi/server.rb
> +++ b/server/lib/cimi/server.rb
> @@ -77,9 +77,7 @@ get "#{settings.root_url}\/?" do
> end
>
> global_collection :cloudEntryPoint do
> - description <<EOS
> - cloud entry point
> -EOS
> + description 'Cloud entry point'
>
> operation :index do
> description "list all resources of the cloud"
> @@ -91,11 +89,7 @@ EOS
> end
>
> global_collection :machine_configurations do
> -
> -
> - description <<EOS
> -List all machine configurations
> -EOS
> + description 'List all machine configurations'
>
> operation :index do
> description "List all machine configurations"
> @@ -137,11 +131,7 @@ EOS
> end
>
> global_collection :machine_images do
> -
> -
> - description <<EOS
> -List all machine images
> -EOS
> + description 'List all machine images'
>
> operation :index do
> description "List all machine configurations"
> @@ -185,11 +175,7 @@ EOS
> end
>
> global_collection :machines do
> -
> -
> - description <<EOS
> -List all machine
> -EOS
> + description 'List all machine'
>
> operation :index do
> description "List all machines"
> @@ -233,10 +219,7 @@ EOS
> end
>
> global_collection :volumes do
> -
> - description <<EOS
> -List all volumes
> -EOS
> + description 'List all volumes'
>
> operation :index do
> description "List all volumes"
> --
> 1.7.4.4
>
[PATCH core 2/8] CIMI: Replaced EOS with regular string in collection description
Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
server/lib/cimi/server.rb | 27 +++++----------------------
1 files changed, 5 insertions(+), 22 deletions(-)
diff --git a/server/lib/cimi/server.rb b/server/lib/cimi/server.rb
index a76fe61..d995f34 100644
--- a/server/lib/cimi/server.rb
+++ b/server/lib/cimi/server.rb
@@ -77,9 +77,7 @@ get "#{settings.root_url}\/?" do
end
global_collection :cloudEntryPoint do
- description <<EOS
- cloud entry point
-EOS
+ description 'Cloud entry point'
operation :index do
description "list all resources of the cloud"
@@ -91,11 +89,7 @@ EOS
end
global_collection :machine_configurations do
-
-
- description <<EOS
-List all machine configurations
-EOS
+ description 'List all machine configurations'
operation :index do
description "List all machine configurations"
@@ -137,11 +131,7 @@ EOS
end
global_collection :machine_images do
-
-
- description <<EOS
-List all machine images
-EOS
+ description 'List all machine images'
operation :index do
description "List all machine configurations"
@@ -185,11 +175,7 @@ EOS
end
global_collection :machines do
-
-
- description <<EOS
-List all machine
-EOS
+ description 'List all machine'
operation :index do
description "List all machines"
@@ -233,10 +219,7 @@ EOS
end
global_collection :volumes do
-
- description <<EOS
-List all volumes
-EOS
+ description 'List all volumes'
operation :index do
description "List all volumes"
--
1.7.4.4
Re: [PATCH core 1/8] CIMI: Replaced global! with global_collection
Posted by Michal Fojtik <mf...@redhat.com>.
On Oct 27, 2011, at 2:36 PM, Tong Li wrote:
> ACK.
Thanks, pushed to to SVN (r1189732)
-- Michal
>
> Tong Li
> Emerging Technologies & Standards
> B062/K317
> litong01@us.ibm.com
>
> mfojtik@redhat.com wrote on 10/27/2011 07:17:21 AM:
>
>> From: mfojtik@redhat.com
>> To: deltacloud-dev@incubator.apache.org
>> Date: 10/27/2011 07:17 AM
>> Subject: [PATCH core 1/8] CIMI: Replaced global! with global_collection
>>
>> From: Michal Fojtik <mf...@redhat.com>
>>
>>
>> Signed-off-by: Michal fojtik <mf...@redhat.com>
>> ---
>> server/lib/cimi/server.rb | 33 +++++++++++++--------------------
>> server/lib/sinatra/rabbit.rb | 6 ++++++
>> 2 files changed, 19 insertions(+), 20 deletions(-)
>>
>> diff --git a/server/lib/cimi/server.rb b/server/lib/cimi/server.rb
>> index 17285bd..a76fe61 100644
>> --- a/server/lib/cimi/server.rb
>> +++ b/server/lib/cimi/server.rb
>> @@ -23,9 +23,6 @@ include Deltacloud::Drivers
>> set :drivers, Proc.new { driver_config }
>>
>> STOREROOT = File.join($top_srcdir, 'lib', 'cimi', 'data')
>> -#We would like to know the storage root.
>> -puts "store root is " + STOREROOT
>> -
>> Sinatra::Application.register Rack::RespondTo
>>
>> use Rack::ETag
>> @@ -70,19 +67,16 @@ error do
>> report_error
>> end
>>
>> -get "#{settings.root_url}\/?" do
>> - if params[:force_auth]
>> - return [401, 'Authentication failed'] unless
>> driver.valid_credentials?(credentials)
>> - end
>> +get "/" do
>> + redirect settings.root_url
>> +end
>>
>> +get "#{settings.root_url}\/?" do
>> + halt 401 if params[:force_auth] and not driver.valid_credentials?
>> (credentials)
>> redirect "#{settings.root_url}/cloudEntryPoint", 301
>> end
>>
>> -collection :cloudEntryPoint do
>> - # Make sure this collection can be accessed, regardless of whether the
>> - # driver supports it or not
>> - global!
>> -
>> +global_collection :cloudEntryPoint do
>> description <<EOS
>> cloud entry point
>> EOS
>> @@ -96,8 +90,8 @@ EOS
>> end
>> end
>>
>> -collection :machine_configurations do
>> - global!
>> +global_collection :machine_configurations do
>> +
>>
>> description <<EOS
>> List all machine configurations
>> @@ -142,8 +136,8 @@ EOS
>> end
>> end
>>
>> -collection :machine_images do
>> - global!
>> +global_collection :machine_images do
>> +
>>
>> description <<EOS
>> List all machine images
>> @@ -190,8 +184,8 @@ EOS
>>
>> end
>>
>> -collection :machines do
>> - global!
>> +global_collection :machines do
>> +
>>
>> description <<EOS
>> List all machine
>> @@ -238,8 +232,7 @@ EOS
>>
>> end
>>
>> -collection :volumes do
>> - global!
>> +global_collection :volumes do
>>
>> description <<EOS
>> List all volumes
>> diff --git a/server/lib/sinatra/rabbit.rb b/server/lib/sinatra/rabbit.rb
>> index 8f34485..6e8b789 100644
>> --- a/server/lib/sinatra/rabbit.rb
>> +++ b/server/lib/sinatra/rabbit.rb
>> @@ -393,6 +393,12 @@ module Sinatra
>> collections[name].generate
>> end
>>
>> + def global_collection(name, &block)
>> + raise DuplicateCollectionException if collections[name]
>> + collections[name] = Collection.new(name, { :global => true },
> &block)
>> + collections[name].generate
>> + end
>> +
>> # Make sure this collection can be accessed, regardless of whether
> the
>> # driver supports it or not
>> def global_collection(name, &block)
>> --
>> 1.7.4.4
------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org
Re: [PATCH core 1/8] CIMI: Replaced global! with global_collection
Posted by Tong Li <li...@us.ibm.com>.
ACK.
Tong Li
Emerging Technologies & Standards
B062/K317
litong01@us.ibm.com
mfojtik@redhat.com wrote on 10/27/2011 07:17:21 AM:
> From: mfojtik@redhat.com
> To: deltacloud-dev@incubator.apache.org
> Date: 10/27/2011 07:17 AM
> Subject: [PATCH core 1/8] CIMI: Replaced global! with global_collection
>
> From: Michal Fojtik <mf...@redhat.com>
>
>
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
> server/lib/cimi/server.rb | 33 +++++++++++++--------------------
> server/lib/sinatra/rabbit.rb | 6 ++++++
> 2 files changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/server/lib/cimi/server.rb b/server/lib/cimi/server.rb
> index 17285bd..a76fe61 100644
> --- a/server/lib/cimi/server.rb
> +++ b/server/lib/cimi/server.rb
> @@ -23,9 +23,6 @@ include Deltacloud::Drivers
> set :drivers, Proc.new { driver_config }
>
> STOREROOT = File.join($top_srcdir, 'lib', 'cimi', 'data')
> -#We would like to know the storage root.
> -puts "store root is " + STOREROOT
> -
> Sinatra::Application.register Rack::RespondTo
>
> use Rack::ETag
> @@ -70,19 +67,16 @@ error do
> report_error
> end
>
> -get "#{settings.root_url}\/?" do
> - if params[:force_auth]
> - return [401, 'Authentication failed'] unless
> driver.valid_credentials?(credentials)
> - end
> +get "/" do
> + redirect settings.root_url
> +end
>
> +get "#{settings.root_url}\/?" do
> + halt 401 if params[:force_auth] and not driver.valid_credentials?
> (credentials)
> redirect "#{settings.root_url}/cloudEntryPoint", 301
> end
>
> -collection :cloudEntryPoint do
> - # Make sure this collection can be accessed, regardless of whether the
> - # driver supports it or not
> - global!
> -
> +global_collection :cloudEntryPoint do
> description <<EOS
> cloud entry point
> EOS
> @@ -96,8 +90,8 @@ EOS
> end
> end
>
> -collection :machine_configurations do
> - global!
> +global_collection :machine_configurations do
> +
>
> description <<EOS
> List all machine configurations
> @@ -142,8 +136,8 @@ EOS
> end
> end
>
> -collection :machine_images do
> - global!
> +global_collection :machine_images do
> +
>
> description <<EOS
> List all machine images
> @@ -190,8 +184,8 @@ EOS
>
> end
>
> -collection :machines do
> - global!
> +global_collection :machines do
> +
>
> description <<EOS
> List all machine
> @@ -238,8 +232,7 @@ EOS
>
> end
>
> -collection :volumes do
> - global!
> +global_collection :volumes do
>
> description <<EOS
> List all volumes
> diff --git a/server/lib/sinatra/rabbit.rb b/server/lib/sinatra/rabbit.rb
> index 8f34485..6e8b789 100644
> --- a/server/lib/sinatra/rabbit.rb
> +++ b/server/lib/sinatra/rabbit.rb
> @@ -393,6 +393,12 @@ module Sinatra
> collections[name].generate
> end
>
> + def global_collection(name, &block)
> + raise DuplicateCollectionException if collections[name]
> + collections[name] = Collection.new(name, { :global => true },
&block)
> + collections[name].generate
> + end
> +
> # Make sure this collection can be accessed, regardless of whether
the
> # driver supports it or not
> def global_collection(name, &block)
> --
> 1.7.4.4
>
[PATCH core 1/8] CIMI: Replaced global! with global_collection
Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
server/lib/cimi/server.rb | 33 +++++++++++++--------------------
server/lib/sinatra/rabbit.rb | 6 ++++++
2 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/server/lib/cimi/server.rb b/server/lib/cimi/server.rb
index 17285bd..a76fe61 100644
--- a/server/lib/cimi/server.rb
+++ b/server/lib/cimi/server.rb
@@ -23,9 +23,6 @@ include Deltacloud::Drivers
set :drivers, Proc.new { driver_config }
STOREROOT = File.join($top_srcdir, 'lib', 'cimi', 'data')
-#We would like to know the storage root.
-puts "store root is " + STOREROOT
-
Sinatra::Application.register Rack::RespondTo
use Rack::ETag
@@ -70,19 +67,16 @@ error do
report_error
end
-get "#{settings.root_url}\/?" do
- if params[:force_auth]
- return [401, 'Authentication failed'] unless driver.valid_credentials?(credentials)
- end
+get "/" do
+ redirect settings.root_url
+end
+get "#{settings.root_url}\/?" do
+ halt 401 if params[:force_auth] and not driver.valid_credentials?(credentials)
redirect "#{settings.root_url}/cloudEntryPoint", 301
end
-collection :cloudEntryPoint do
- # Make sure this collection can be accessed, regardless of whether the
- # driver supports it or not
- global!
-
+global_collection :cloudEntryPoint do
description <<EOS
cloud entry point
EOS
@@ -96,8 +90,8 @@ EOS
end
end
-collection :machine_configurations do
- global!
+global_collection :machine_configurations do
+
description <<EOS
List all machine configurations
@@ -142,8 +136,8 @@ EOS
end
end
-collection :machine_images do
- global!
+global_collection :machine_images do
+
description <<EOS
List all machine images
@@ -190,8 +184,8 @@ EOS
end
-collection :machines do
- global!
+global_collection :machines do
+
description <<EOS
List all machine
@@ -238,8 +232,7 @@ EOS
end
-collection :volumes do
- global!
+global_collection :volumes do
description <<EOS
List all volumes
diff --git a/server/lib/sinatra/rabbit.rb b/server/lib/sinatra/rabbit.rb
index 8f34485..6e8b789 100644
--- a/server/lib/sinatra/rabbit.rb
+++ b/server/lib/sinatra/rabbit.rb
@@ -393,6 +393,12 @@ module Sinatra
collections[name].generate
end
+ def global_collection(name, &block)
+ raise DuplicateCollectionException if collections[name]
+ collections[name] = Collection.new(name, { :global => true }, &block)
+ collections[name].generate
+ end
+
# Make sure this collection can be accessed, regardless of whether the
# driver supports it or not
def global_collection(name, &block)
--
1.7.4.4
[PATCH core 3/8] CIMI: Revamped cmwgapp_helper.rb to cmwg_helper with less code and without XmlSimple
Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
server/lib/cimi/helpers/cmwg_helper.rb | 102 ++++++++++++++++++++++++++++++++
1 files changed, 102 insertions(+), 0 deletions(-)
create mode 100644 server/lib/cimi/helpers/cmwg_helper.rb
diff --git a/server/lib/cimi/helpers/cmwg_helper.rb b/server/lib/cimi/helpers/cmwg_helper.rb
new file mode 100644
index 0000000..9dae5a1
--- /dev/null
+++ b/server/lib/cimi/helpers/cmwg_helper.rb
@@ -0,0 +1,102 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+module ApplicationHelper
+ include Deltacloud
+
+ def render_resource(object_name, data)
+ respond_to do |format|
+ format.xml do
+ report_error 404 unless data
+ @object = data
+ content_type cimi_content_type_for(object_name.to_s.camelize, :xml)
+ haml :"#{object_name.to_s.pluralize}/show", :layout => false
+ end
+ format.json do
+ report_error 404 unless data
+ content_type cimi_content_type_for(object_name.to_s.camelize, :json)
+ resource_to_json(object_name, data)
+ end
+ end
+ end
+
+ def render_collection(collection_name)
+ name = (collection_name == :cloud_entry_point) ? collection_name : collection_name.to_s.pluralize
+ respond_to do |format|
+ format.html do
+ haml :"#{name}/index"
+ end
+ format.xml do
+ content_type cimi_content_type_for([collection_name.to_s.camelize, 'Collection'].join, :xml)
+ haml :"#{name}/index", :layout => false
+ end
+ format.json do
+ content_type cimi_content_type_for([collection_name.to_s.camelize, 'Collection'].join, :json)
+ collection_to_json collection_name, @resources
+ end
+ end
+ end
+
+ def cimi_content_type_for(coll_type, format="html")
+ case format
+ when :xml
+ "application/CIMI-%s+xml" % coll_type
+ when :json
+ "application/CIMI-%s+json" % coll_type
+ end
+ end
+
+ def collection_to_json(collection_name, resources=[])
+ name = collection_name == :cloud_entry_point ? collection_name.to_s : collection_name.to_s.pluralize
+ engine = Tilt::HamlTemplate.new(File.join(settings.views, name, 'index.xml.haml'))
+ hash_response = XmlSimple.xml_in(engine.render(self, :'@resources' => resources))
+ hash_response.delete('xmlns')
+ hash_response.to_json
+ end
+
+ def resource_to_json(resource_name, object)
+ engine = Tilt::HamlTemplate.new(File.join(settings.views, resource_name.to_s.pluralize, 'show.xml.haml'))
+ hash_response = XmlSimple.xml_in(engine.render(self, :'@object' => object))
+ hash_response.delete('xmlns')
+ rename_key_fields!(hash_response, replacement_keys_for(resource_name))
+ hash_response.to_json
+ end
+
+ private
+
+ def replacement_keys_for(resource)
+ case resource
+ when :machine_image then {"property" => "properties", "operation" => "operations"}
+ when :machine then {"property" => "properties", "disk" => "disks", "operation" => "operations"}
+ when :colume then {"property" => "properties", "operation" => "operations"}
+ end
+ end
+
+ def rename_key_fields!(an_object, key_maps = nil)
+ if an_object.kind_of?(Hash)
+ key_maps.each do |key, value|
+ an_object[value] = an_object.delete(key) if an_object.key?(key)
+ end
+ an_object.each do |key, value|
+ rename_key_fields!(value, key_maps)
+ end
+ elsif an_object.kind_of?(Array)
+ an_object.each do |value|
+ rename_key_fields!(value, key_maps)
+ end
+ end
+ end
+
+end
--
1.7.4.4
[PATCH core 8/8] CIMI: Getting rid of @dtfm_item Hash in all 'show' operations in prior to access Deltacloud model properties directly
Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
.../cimi/machine_configurations/show.html.haml | 10 +++---
.../cimi/machine_configurations/show.xml.haml | 36 ++++++------------
server/views/cimi/machine_images/show.xml.haml | 21 ++++-------
server/views/cimi/machines/show.xml.haml | 39 +++++++-------------
server/views/cimi/volumes/show.xml.haml | 21 ++++-------
5 files changed, 45 insertions(+), 82 deletions(-)
diff --git a/server/views/cimi/machine_configurations/show.html.haml b/server/views/cimi/machine_configurations/show.html.haml
index 38dc867..80c1b76 100644
--- a/server/views/cimi/machine_configurations/show.html.haml
+++ b/server/views/cimi/machine_configurations/show.html.haml
@@ -1,21 +1,21 @@
%h1 View/Edit machine configuration
%form{ :action => machine_configurations_url }
- %input{ :name => :id, :type => :hidden, :value => @dmtfitem["uri"] }/
+ %input{ :name => :id, :type => :hidden, :value => url_for("/cimi/machine_configurations/#{@object.name}") }/
%input{ :name => :xmlRootNode, :type => :hidden, :value => @xml_root_node }/
%input{ :name => :refreshURI, :type => :hidden, :value => machine_configurations_url }/
%p
%label
Name:
%p
- %input{ :name => :name, :size => 50, :value => @dmtfitem["name"], :style => "width:50%;" }
- %input{ :name => :created, :type => :hidden, :size => 50, :value => @dmtfitem["created"] }
+ %input{ :name => :name, :size => 50, :value => @object.name, :style => "width:50%;" }
+ %input{ :name => :created, :type => :hidden, :size => 50, :value => @object.launch_time || Time.new.getutc.to_s }
%p
%br
%label
Description:
%p
- %textarea{ :name => :description, :cols => 50, :rows => 4, :style => "width:50%;" } #{@dmtfitem['description']}
+ %textarea{ :name => :description, :cols => 50, :rows => 4, :style => "width:50%;" }
%p
%br
%label
@@ -156,4 +156,4 @@
})
cellRightBut.appendChild(er);
- }
\ No newline at end of file
+ }
diff --git a/server/views/cimi/machine_configurations/show.xml.haml b/server/views/cimi/machine_configurations/show.xml.haml
index c337fe1..095bebd 100644
--- a/server/views/cimi/machine_configurations/show.xml.haml
+++ b/server/views/cimi/machine_configurations/show.xml.haml
@@ -1,27 +1,15 @@
- unless defined?(partial)
!!! XML
%MachineConfiguration{ :xmlns => CMWG_NAMESPACE }
- %uri=machine_configurations_url + "/" + @dmtfitem["uri"]
- %name=@dmtfitem["name"]
- %description=@dmtfitem["description"]
- %created=@dmtfitem["created"]
- - if @dmtfitem["property"]
- - if @dmtfitem["property"]["name"] && @dmtfitem["property"]["content"]
- - property_object = {"#{@dmtfitem['property']['name']}" => {"content" => @dmtfitem["property"]["content"]}}
- - else
- - property_object = @dmtfitem["property"]
- - property_object.each_pair do |key, value|
- %property{ :name => key}=value["content"]
- %cpu=@dmtfitem["cpu"]
- %memory{ :quantity => @dmtfitem["memory"]["quantity"], :units => @dmtfitem["memory"]["units"] }
- - if @dmtfitem["disk"]
- - if @dmtfitem["disk"].kind_of?(Array)
- - property_object = @dmtfitem["disk"]
- - else
- - property_object = [@dmtfitem["disk"]]
- - property_object.each do |disk|
- %disk
- %capacity{ :quantity => disk["capacity"]["quantity"], :units => disk["capacity"]["units"] }
- %guestInterface= disk["guestInterface"]
- %operation{ :rel => "edit", :href => machine_configurations_url + "/" + @dmtfitem["uri"] }
- %operation{ :rel => "delete", :href => machine_configurations_url + "/" + @dmtfitem["uri"] }
\ No newline at end of file
+ %uri=machine_configurations_url + "/" + @object.name
+ %uri=url_for("/cimi/machine_configurations/#{@object.name}")
+ %name=@object.name
+ %description="#{@object.name} machine configuration"
+ %created=Time.new.getutc.to_s
+ %cpu=@object.architecture.value
+ %memory{ :quantity => @object.memory.value, :units => @object.memory.unit }/
+ %disk
+ %capacity{ :quantity => @object.storage.value, :units => @object.storage.unit }/
+ %guestInterface SATA
+ %operation{ :rel => "edit", :href => machine_configurations_url + "/" + @object.name }/
+ %operation{ :rel => "delete", :href => machine_configurations_url + "/" + @object.name }/
diff --git a/server/views/cimi/machine_images/show.xml.haml b/server/views/cimi/machine_images/show.xml.haml
index a00713c..9ff2f3b 100644
--- a/server/views/cimi/machine_images/show.xml.haml
+++ b/server/views/cimi/machine_images/show.xml.haml
@@ -1,17 +1,10 @@
- unless defined?(partial)
!!! XML
%MachineImage{ :xmlns => CMWG_NAMESPACE }
- %uri=machine_images_url + "/" + @dmtfitem["uri"]
- %name=@dmtfitem["name"]
- %description=@dmtfitem["description"]
- %created=@dmtfitem["created"]
- - if @dmtfitem["property"]
- - if @dmtfitem["property"]["name"] && @dmtfitem["property"]["content"]
- - property_object = {"#{@dmtfitem['property']['name']}" => {"content" => @dmtfitem["property"]["content"]}}
- - else
- - property_object = @dmtfitem["property"]
- - property_object.each_pair do |key, value|
- %property{ :name => key}=value["content"]
- %imageLocation{ :href => @dmtfitem["imageLocation"]}
- %operation{ :rel => "edit", :href => machine_images_url + "/" + @dmtfitem["uri"] }
- %operation{ :rel => "delete", :href => machine_images_url + "/" + @dmtfitem["uri"] }
\ No newline at end of file
+ %uri=machine_images_url + "/" + @object.id
+ %name=@object.name
+ %description="#{@object.name} machine image description"
+ %created=Time.new.getutc.to_s
+ %imageLocation{ :href => '/'}/
+ %operation{ :rel => "edit", :href => machine_images_url + "/" + @object.id }/
+ %operation{ :rel => "delete", :href => machine_images_url + "/" + @object.id }/
diff --git a/server/views/cimi/machines/show.xml.haml b/server/views/cimi/machines/show.xml.haml
index c699b19..2b6da55 100644
--- a/server/views/cimi/machines/show.xml.haml
+++ b/server/views/cimi/machines/show.xml.haml
@@ -1,28 +1,17 @@
- unless defined?(partial)
!!! XML
%Machine{ :xmlns => CMWG_NAMESPACE }
- %uri=machines_url + "/" + @dmtfitem["uri"]
- %name=@dmtfitem["name"]
- %description=@dmtfitem["description"]
- %created=@dmtfitem["created"]
- - if @dmtfitem["property"]
- - if @dmtfitem["property"]["name"] && @dmtfitem["property"]["content"]
- - property_object = {"#{@dmtfitem['property']['name']}" => {"content" => @dmtfitem["property"]["content"]}}
- - else
- - property_object = @dmtfitem["property"]
- - property_object.each_pair do |key, value|
- %property{ :name => key}=value["content"]
- %status=@dmtfitem["status"]
- %cpu=@dmtfitem["cpu"]
- %memory{ :quantity => @dmtfitem["memory"]["quantity"], :units => @dmtfitem["memory"]["units"] }
- - if @dmtfitem["disk"]
- - if @dmtfitem["disk"].kind_of?(Array)
- - property_object = @dmtfitem["disk"]
- - else
- - property_object = [@dmtfitem["disk"]]
- - property_object.each do |disk|
- %disk
- %capacity{ :quantity => disk["capacity"]["quantity"], :units => disk["capacity"]["units"] }
- %guestInterface= disk["guestInterface"]
- %operation{ :rel => "edit", :href => machines_url + "/" + @dmtfitem["uri"] }
- %operation{ :rel => "delete", :href => machines_url + "/" + @dmtfitem["uri"] }
\ No newline at end of file
+ %uri=machines_url + "/" + @object.id
+ %name=@object.name
+ %description="#{@object.name} machine description"
+ %created=@object.launch_time || Time.new.getutc.to_s
+ %status=@object.state
+ %cpu=@object.instance_profile.overrides.select { |p, v| p == :architecture }.first || @profile.architecture.value
+ - memory_label, memory_value = @object.instance_profile.overrides.select { |p, v| p == :memory }
+ %memory{ :quantity => memory_value || @profile.memory.value, :units => Deltacloud::HardwareProfile::unit(:memory) }/
+ %disk
+ - storage_label, storage_value = @object.instance_profile.overrides.select { |p, v| p == :storage }
+ %capacity{ :quantity => storage_value || @profile.storage.value, :units => Deltacloud::HardwareProfile::unit(:storage)}/
+ %guestInterface SATA
+ %operation{ :rel => "edit", :href => machines_url + "/" + @object.id }/
+ %operation{ :rel => "delete", :href => machines_url + "/" + @object.id }/
diff --git a/server/views/cimi/volumes/show.xml.haml b/server/views/cimi/volumes/show.xml.haml
index 30ef2c2..5f34534 100644
--- a/server/views/cimi/volumes/show.xml.haml
+++ b/server/views/cimi/volumes/show.xml.haml
@@ -1,17 +1,10 @@
- unless defined?(partial)
!!! XML
%Volume{ :xmlns => CMWG_NAMESPACE }
- %uri=volumes_url + "/" + @dmtfitem["uri"]
- %name=@dmtfitem["name"]
- %description=@dmtfitem["description"]
- %created=@dmtfitem["created"]
- - if @dmtfitem["property"]
- - if @dmtfitem["property"]["name"] && @dmtfitem["property"]["content"]
- - property_object = {"#{@dmtfitem['property']['name']}" => {"content" => @dmtfitem["property"]["content"]}}
- - else
- - property_object = @dmtfitem["property"]
- - property_object.each_pair do |key, value|
- %property{ :name => key}=value["content"]
- %capacity{ :quantity => @dmtfitem["capacity"]["quantity"], :units => @dmtfitem["capacity"]["units"]}
- %operation{ :rel => "edit", :href => volumes_url + "/" + @dmtfitem["uri"] }
- %operation{ :rel => "delete", :href => volumes_url + "/" + @dmtfitem["uri"] }
\ No newline at end of file
+ %uri=volumes_url + "/" + @object.id
+ %name=@object.name
+ %description="#{@object.name} volume description"
+ %created=@object.created || Time.new.getutc.to_s
+ %capacity{ :quantity => @object.capacity, :units => 'GB'}
+ %operation{ :rel => "edit", :href => volumes_url + "/" + @object.id }
+ %operation{ :rel => "delete", :href => volumes_url + "/" + @object.id }
--
1.7.4.4
[PATCH core 4/8] CIMI: Revamped server.rb to use helpers from the new CMWG helper. Removed @dtmf_item variable and XML hacking.
Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
server/lib/cimi/server.rb | 136 ++++++--------------
server/views/cimi/cloudEntryPoint/index.html.haml | 5 -
server/views/cimi/cloudEntryPoint/index.xml.haml | 9 --
.../views/cimi/cloud_entry_point/index.html.haml | 5 +
server/views/cimi/cloud_entry_point/index.xml.haml | 9 ++
5 files changed, 53 insertions(+), 111 deletions(-)
delete mode 100644 server/views/cimi/cloudEntryPoint/index.html.haml
delete mode 100644 server/views/cimi/cloudEntryPoint/index.xml.haml
create mode 100644 server/views/cimi/cloud_entry_point/index.html.haml
create mode 100644 server/views/cimi/cloud_entry_point/index.xml.haml
diff --git a/server/lib/cimi/server.rb b/server/lib/cimi/server.rb
index d995f34..90d2b67 100644
--- a/server/lib/cimi/server.rb
+++ b/server/lib/cimi/server.rb
@@ -15,7 +15,7 @@
require 'cimi/helpers/dmtfdep'
-require 'cimi/helpers/cmwgapp_helper'
+require 'cimi/helpers/cmwg_helper'
set :version, '0.1.0'
@@ -82,8 +82,8 @@ global_collection :cloudEntryPoint do
operation :index do
description "list all resources of the cloud"
control do
- @collections = entry_points.reject { |p| p[0] == :cloudEntryPoint }
- show_resource "cloudEntryPoint/index", "CloudEntryPoint"
+ @resources = entry_points.reject { |p| p[0] == :cloudEntryPoint }
+ render_collection :cloud_entry_point
end
end
end
@@ -94,16 +94,15 @@ global_collection :machine_configurations do
operation :index do
description "List all machine configurations"
control do
- profiles = driver.hardware_profiles(credentials, nil)
- @dmtf_col_items = []
- if profiles
- profiles.map do |profile|
- new_item = { "name" => profile.name,
- "href" => machine_configuration_url(profile.name) }
- @dmtf_col_items.insert 0, new_item
- end
+ profiles = driver.hardware_profiles(credentials)
+ @resources = []
+ profiles.each do |profile|
+ @resources << {
+ :name => profile.name,
+ :href => machine_configuration_url(profile.name)
+ }
end
- respond_to_collection "machine_configuration.col.xml"
+ render_collection :machine_configuration
end
end
@@ -112,20 +111,7 @@ global_collection :machine_configurations do
with_capability :hardware_profile
param :id, :string, :required
control do
- @profile = driver.hardware_profile(credentials, params[:id])
- if @profile
- #setup the default values for a machine configuration
- resource_default = get_resource_default "machine_configuration"
- #get the actual values from profile
- resource_value = { "name" => @profile.name,"uri" => @profile.name,
- "href" => machine_configuration_url(@profile.name) }
- #mixin actual values get from profile
- @dmtfitem = resource_default["dmtfitem"].merge resource_value
- show_resource "machine_configurations/show", "MachineConfiguration",
- {"property" => "properties", "disk" => "disks", "operation" => "operations"}
- else
- report_error(404)
- end
+ render_resource :machine_configuration, driver.hardware_profile(credentials, params[:id])
end
end
end
@@ -137,15 +123,14 @@ global_collection :machine_images do
description "List all machine configurations"
control do
images = driver.send(:images, credentials, {})
- @dmtf_col_items = []
- if images
- images.map do |image|
- new_item = { "name" => image.name,
- "href" => machine_image_url(image.id) }
- @dmtf_col_items.insert 0, new_item
- end
+ @resources = []
+ images.each do |image|
+ @resources << {
+ :name => image.name,
+ :href => machine_image_url(image.id)
+ }
end
- respond_to_collection "machine_image.col.xml"
+ render_collection :machine_image
end
end
@@ -154,21 +139,7 @@ global_collection :machine_images do
with_capability :image
param :id, :string, :required
control do
- @image = driver.send(:image, credentials, { :id => params[:id]} )
- if @image
- #setup the default values for a machine imageion
- resource_default = get_resource_default "machine_image"
- #get the actual values from image
- resource_value = { "name" => @image.name,
- "description" => @image.description,
- "uri" => @image.id,"href" => machine_image_url(@image.id) }
- #mixin actual values get from the specific image
- @dmtfitem = resource_default["dmtfitem"].merge resource_value
- show_resource "machine_images/show", "MachineImage",
- {"property" => "properties", "operation" => "operations"}
- else
- report_error(404)
- end
+ render_resource :machine_image, driver.image(credentials, { :id => params[:id]} )
end
end
@@ -181,15 +152,14 @@ global_collection :machines do
description "List all machines"
control do
instances = driver.send(:instances, credentials, {})
- @dmtf_col_items = []
- if instances
- instances.map do |instance|
- new_item = { "name" => instance.name,
- "href" => machine_url(instance.id) }
- @dmtf_col_items.insert 0, new_item
- end
+ @resources = []
+ instances.each do |instance|
+ @resources << {
+ :name => instance.name,
+ :href => machine_url(instance.id)
+ }
end
- respond_to_collection "machine.col.xml"
+ render_collection :machine
end
end
@@ -198,21 +168,9 @@ global_collection :machines do
with_capability :instance
param :id, :string, :required
control do
- @machine = driver.send(:instance, credentials, { :id => params[:id]} )
- if @machine
- #setup the default values for a machine imageion
- resource_default = get_resource_default "machine"
- #get the actual values from image
- resource_value = { "name" => @machine.name,
- "status" => @machine.state, "uri" => @machine.id,
- "href" => machine_url(@machine.id) }
- #mixin actual values get from the specific image
- @dmtfitem = resource_default["dmtfitem"].merge resource_value
- show_resource "machines/show", "Machine",
- {"property" => "properties", "disk" => "disks", "operation" => "operations"}
- else
- report_error(404)
- end
+ instance = driver.send(:instance, credentials, { :id => params[:id]} )
+ profile = driver.send(:hardware_profile, credentials, instance.instance_profile.id)
+ render_resource :machine, instance, { :profile => profile }
end
end
@@ -224,16 +182,15 @@ global_collection :volumes do
operation :index do
description "List all volumes"
control do
- instances = driver.send(:storage_volumes, credentials, {})
- @dmtf_col_items = []
- if instances
- instances.map do |instance|
- new_item = { "name" => instance.id,
- "href" => volume_url(instance.id) }
- @dmtf_col_items.insert 0, new_item
- end
+ volumes = driver.send(:storage_volumes, credentials, {})
+ @resources = []
+ volumes.each do |volume|
+ @resources << {
+ :name => volume.name,
+ :href => machine_url(volume.id)
+ }
end
- respond_to_collection "volume.col.xml"
+ render_collection :volume
end
end
@@ -242,22 +199,7 @@ global_collection :volumes do
with_capability :storage_volume
param :id, :string, :required
control do
- @volume = driver.send(:storage_volume, credentials, { :id => params[:id]} )
- if @volume
- #setup the default values for a machine imageion
- resource_default = get_resource_default "volume"
- #get the actual values from image
- resource_value = { "name" => @volume.id,
- "status" => @volume.state, "uri" => @volume.id,
- "href" => volume_url(@volume.id),
- "capacity" => { "quantity" => @volume.capacity, "units" => "gigabyte"} }
- #mixin actual values get from the specific image
- @dmtfitem = resource_default["dmtfitem"].merge resource_value
- show_resource "volumes/show", "Volume",
- {"property" => "properties", "operation" => "operations"}
- else
- report_error(404)
- end
+ render_resource :volume, driver.send(:storage_volume, credentials, { :id => params[:id]} )
end
end
diff --git a/server/views/cimi/cloudEntryPoint/index.html.haml b/server/views/cimi/cloudEntryPoint/index.html.haml
deleted file mode 100644
index 76f147b..0000000
--- a/server/views/cimi/cloudEntryPoint/index.html.haml
+++ /dev/null
@@ -1,5 +0,0 @@
-
-- @collections.each do |p|
- - name, path = p
- = link_to name.to_s.capitalize, path
- %br/
diff --git a/server/views/cimi/cloudEntryPoint/index.xml.haml b/server/views/cimi/cloudEntryPoint/index.xml.haml
deleted file mode 100644
index 784b931..0000000
--- a/server/views/cimi/cloudEntryPoint/index.xml.haml
+++ /dev/null
@@ -1,9 +0,0 @@
-!!!XML
-%CloudEntryPoint{ :xmlns => CMWG_NAMESPACE }
- %uri= api_url_for("/cloudEntryPoint")
- %name cloud entry point
- %description cloud entry point
- %created= Time.new.getutc.to_s
- - @collections.each do |api|
- - res_name = api[0].to_s.camelize(:lower_case_first_letter)
- = "<#{res_name} href=\"#{api[1]}\"/>"
diff --git a/server/views/cimi/cloud_entry_point/index.html.haml b/server/views/cimi/cloud_entry_point/index.html.haml
new file mode 100644
index 0000000..5375bd0
--- /dev/null
+++ b/server/views/cimi/cloud_entry_point/index.html.haml
@@ -0,0 +1,5 @@
+
+- @resources.each do |p|
+ - name, path = p
+ = link_to name.to_s.capitalize, path
+ %br/
diff --git a/server/views/cimi/cloud_entry_point/index.xml.haml b/server/views/cimi/cloud_entry_point/index.xml.haml
new file mode 100644
index 0000000..8555a4b
--- /dev/null
+++ b/server/views/cimi/cloud_entry_point/index.xml.haml
@@ -0,0 +1,9 @@
+!!!XML
+%CloudEntryPoint{ :xmlns => CMWG_NAMESPACE }
+ %uri= api_url_for("/cloudEntryPoint")
+ %name cloud entry point
+ %description cloud entry point
+ %created= Time.new.getutc.to_s
+ - @resources.each do |api|
+ - res_name = api[0].to_s.camelize(:lower_case_first_letter)
+ = "<#{res_name} href=\"#{api[1]}\"/>"
--
1.7.4.4
Re: Second revamp of CIMI code
Posted by David Lutterkort <lu...@redhat.com>.
On Thu, 2011-10-27 at 11:05 -0400, Tong Li wrote:
> fixup_content is to simply make sure that the serialized xml will
> confirm to what DMTF spec requires. and it also makes sure that the format
> is good to serialize to json which also needs to be confirmed to the spec.
> fixup_content method does not operate on xml, it operates on the hash
> object as the comments indicated.
I have to agree with Michal here - the transformation that fixup_content
does is not exactly straightforward.
In general, it is much easier to follow code if the code follows
standard MVC patterns; here, that means that the rendering of an entity
should be based on a proper model object of that entity (say a Machine)
that is formatted as XML through a template.
I generally also mistrust output schemes that are too generic/automated,
since they make it too easy to change the output without noticing. Early
on, Deltacloud used XML builder, which renders an object as XML through
introspection; we moved from that to XML templates to make it more
likely that a change in the model would cause a noisy failure rather
than a silent change in output format.
As a side-benefit, the business logic (mostly drivers) can then
manipulate these model objects, and becomes much clearer and easier to
follow than the manipulation of plain hashes.
> When you drill deeper into dmtf spec, you will find most of the
> operations will require xml or json document, that is, most of the requests
> post xml or json documents, in current DC implementation, post is rarely
> used , that means, majority of the request only deal with request
> parameters, but when you start dealing with DMTF requests, most likely the
> request contains xml or json document, the very first thing that you have
> to do is to parse the xml or json. What the current CIMI code does is to
> treat the default data and the request data exactly the same, there won't
> be any extra code needed when handling more complex request. You can remove
> them now but you will eventually have to add xml/json handling to parse the
> data posted by requests, that will end up having two set of the code. I
> will be very hesitate to approve what patches 3 to 8 do.
Agreed, that's a very important point; we will need some mechanism to
deserialize XML/JSON into internal objects. I am not convinced that
XmlSimple's XML -> Hash conversion is the answer here, but it might be
part of it.
We will definitely need some kind of XML/JSON -> object converter. I
want to be able to write machine.disks[1].capacity.units (but also
machine.disks[1].capacity.in_bytes !)
Part of this discussion might just be to find a tasteful way to hide the
deserialization ugliness, probably in a manner similar to ActiveRecord.
Something that lets me write
class Machine < CIMI::Model::Base
def total_disk_size
disks.inject(0) { |sum, d| sum + d.capacity.in_bytes }
end
end
m1 = Machine.from_xml(machine_xml)
puts "Machine #{m1.name} has #{m1.total_disk_size} bytes in its disks"
Whether we then should render a machine with a template or some
m1.to_xml method is a somewhat different debate, which will largely
hinge on how much metadata we'd need to produce XML output (note that
XML is ordered, whereas hashes in Ruby 1.8 are unordered) vs. the effort
to write explicit templates.
> XmlSimple seems to me a very good xml document handling library. I do
> not feel either it being complex or hard to understand.
This isn't so much a discussion about XmlSimple, as it is about the best
strategies for (a) serializing internal objects to CIMI XML/JSON, (b)
deserializing CIMI XML/JSON to internal objects, and (c) the best
representation of internal objects so that lower layers (drivers) can be
written easily and clearly.
As I said above, I think XmlSimple will be part of the equation, but the
hashes coming out of XmlSimple aren't expressive/safe enough as the
internal representation of CIMI values/objects.
David
Re: Second revamp of CIMI code
Posted by Tong Li <li...@us.ibm.com>.
Michael,
fixup_content is to simply make sure that the serialized xml will
confirm to what DMTF spec requires. and it also makes sure that the format
is good to serialize to json which also needs to be confirmed to the spec.
fixup_content method does not operate on xml, it operates on the hash
object as the comments indicated.
When you drill deeper into dmtf spec, you will find most of the
operations will require xml or json document, that is, most of the requests
post xml or json documents, in current DC implementation, post is rarely
used , that means, majority of the request only deal with request
parameters, but when you start dealing with DMTF requests, most likely the
request contains xml or json document, the very first thing that you have
to do is to parse the xml or json. What the current CIMI code does is to
treat the default data and the request data exactly the same, there won't
be any extra code needed when handling more complex request. You can remove
them now but you will eventually have to add xml/json handling to parse the
data posted by requests, that will end up having two set of the code. I
will be very hesitate to approve what patches 3 to 8 do.
XmlSimple seems to me a very good xml document handling library. I do
not feel either it being complex or hard to understand. I feel that it is
miles miles easier to be understood than the DC code itself. If I had not
had David hand holding me on DC code and its framework, I won't be able to
understand how DC works even today. The XmlSiple library has been ported
to many different programming languages and very well accepted in Ruby
community as far as I can see. As I said, you will have to eventually deal
with XML/JSON input. Of course, this is just my 2 cents. If there is other
even better and easier alternatives for handling xml input, I'll be very
glad to try it out.
Thanks.
Tong Li
Emerging Technologies & Standards
B062/K317
litong01@us.ibm.com
From: mfojtik@redhat.com
To: deltacloud-dev@incubator.apache.org
Date: 10/27/2011 07:17 AM
Subject: Second revamp of CIMI code
Hi,
This patchset should improve the code we have currently for CIMI.
I will briefly explain what each patch did and why:
1/8:
This is just cosmetic change. It's better to have consistent DSL
rather than using 'global!' method inside all collections.
If some collection want be 'global' lets call it 'global_collection'.
This change was tested on Ruby 1.8 and 1.9 and should not break anything.
2/8:
Using EOS is a bit weird to me. We used EOL just because our Deltacloud
descriptions was too long to fit into one line ;-)
However the CIMI descriptions are short one liners so why not put them
into regular string.
3-6/8:
The code in cmwgapp_helper was too much complex to me to undestand
(and read) so I decided to rewrite it. You can now see the difference.
I dunno how about you, but I had very hard time to undestand what
'fixup_content' or 'respond_to_collection' methods internally do.
We can discuss it, but my personal opinion is that hacking XML using
XmlSimple and then modifying resulting Hash is too complex.
7/8:
The code previosly used to return collection was using XmlSimple
and not HAML. This throw an Exception in Ruby 1.8:
NoMethodError - undefined method `downcase' for 77:Fixnum:
./bin/../lib/cimi/helpers/cmwgapp_helper.rb:68:in `respond_to_collection'
This is one of the examples of darkside XmlSimple. The code is too
complex and thus there is higher propability of exceptions like this one.
I spend 2 hours debugging the code until I finally discover from where
this problem original came.
The 'index.xml'haml' files are easy readable, we can update them according
to CIMI everytime and everyone can undestand what should be returned on
output
just by looking on HAML file. Later on, we can re-use this index.xml.haml
as we did in Deltacloud and use 'partial' rendering to render items in
collection.
8/8:
I had long conversation about this with Tong Li and as far I undestand
his point of using this variable is to make a bridge between Deltacloud API
object model and CIMI. However I think we can easely change our object
to support CIMI specific properties very easely without breaking anything.
Since every class is open we can simple 're-open' the model class (like
Instance) and add more methods here.
My personal opinion is that as this implementation will grow and we will
need to add more data and properties, this bridge will become our
nightmare.
-----
IMPORTANT: This patch is not addressing the UI. The HAML views will need to
be updated to support new local variables and stuff.
Especially 'new' operation views, will need to be updated
together
with our models. I'll take this task once we will have basic
Test::Unit ready (to be sure I'll not break anything).
PS: This patchset was tested under Ruby 1.8 and 1.9 (and Rubinius for
fun ;-).
-- Michal
[PATCH core 5/8] CIMI: Removed static XML files
Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
server/lib/cimi/data/collections/machine.col.xml | 8 ----
.../data/collections/machine_configuration.col.xml | 8 ----
.../cimi/data/collections/machine_image.col.xml | 8 ----
server/lib/cimi/data/collections/volume.col.xml | 8 ----
server/lib/cimi/data/default_res/machine.col.xml | 35 --------------------
.../data/default_res/machine_configuration.col.xml | 21 ------------
.../cimi/data/default_res/machine_image.col.xml | 11 ------
server/lib/cimi/data/default_res/volume.col.xml | 14 --------
8 files changed, 0 insertions(+), 113 deletions(-)
delete mode 100644 server/lib/cimi/data/collections/machine.col.xml
delete mode 100644 server/lib/cimi/data/collections/machine_configuration.col.xml
delete mode 100644 server/lib/cimi/data/collections/machine_image.col.xml
delete mode 100644 server/lib/cimi/data/collections/volume.col.xml
delete mode 100644 server/lib/cimi/data/default_res/machine.col.xml
delete mode 100644 server/lib/cimi/data/default_res/machine_configuration.col.xml
delete mode 100644 server/lib/cimi/data/default_res/machine_image.col.xml
delete mode 100644 server/lib/cimi/data/default_res/volume.col.xml
diff --git a/server/lib/cimi/data/collections/machine.col.xml b/server/lib/cimi/data/collections/machine.col.xml
deleted file mode 100644
index 622a6fe..0000000
--- a/server/lib/cimi/data/collections/machine.col.xml
+++ /dev/null
@@ -1,8 +0,0 @@
-<?xml version='1.0' encoding='utf-8' ?>
-<MachineCollection xmlns="http://www.dmtf.org/cimi">
- <uri>machine</uri>
- <name>Machine collection </name>
- <description>The machine collection</description>
- <created>2011-09-12 11:37:28 UTC</created>
- <operation rel="add" href="/machines" />
-</MachineCollection>
diff --git a/server/lib/cimi/data/collections/machine_configuration.col.xml b/server/lib/cimi/data/collections/machine_configuration.col.xml
deleted file mode 100644
index 99af7fb..0000000
--- a/server/lib/cimi/data/collections/machine_configuration.col.xml
+++ /dev/null
@@ -1,8 +0,0 @@
-<?xml version='1.0' encoding='utf-8' ?>
-<MachineConfigurationCollection xmlns="http://www.dmtf.org/cimi">
- <uri>machineConfiguration</uri>
- <name>Machine Configuration collection </name>
- <description>That is jsut a test</description>
- <created>2011-09-12 11:37:28 UTC</created>
- <operation rel="add" href="/machine_configurations" />
-</MachineConfigurationCollection>
diff --git a/server/lib/cimi/data/collections/machine_image.col.xml b/server/lib/cimi/data/collections/machine_image.col.xml
deleted file mode 100644
index c4be389..0000000
--- a/server/lib/cimi/data/collections/machine_image.col.xml
+++ /dev/null
@@ -1,8 +0,0 @@
-<?xml version='1.0' encoding='utf-8' ?>
-<MachineImageCollection xmlns="http://www.dmtf.org/cimi">
- <uri>machineImage</uri>
- <name>Machine Image collection </name>
- <description>The machine image collection</description>
- <created>2011-09-12 11:37:28 UTC</created>
- <operation rel="add" href="/machine_images" />
-</MachineImageCollection>
diff --git a/server/lib/cimi/data/collections/volume.col.xml b/server/lib/cimi/data/collections/volume.col.xml
deleted file mode 100644
index e2d2b4a..0000000
--- a/server/lib/cimi/data/collections/volume.col.xml
+++ /dev/null
@@ -1,8 +0,0 @@
-<?xml version='1.0' encoding='utf-8' ?>
-<VolumeCollection xmlns="http://www.dmtf.org/cimi">
- <uri>volume</uri>
- <name>Volume collection </name>
- <description>The volume collection</description>
- <created>2011-09-12 11:37:28 UTC</created>
- <operation rel="add" href="/volumes" />
-</VolumeCollection>
diff --git a/server/lib/cimi/data/default_res/machine.col.xml b/server/lib/cimi/data/default_res/machine.col.xml
deleted file mode 100644
index 7c2bab1..0000000
--- a/server/lib/cimi/data/default_res/machine.col.xml
+++ /dev/null
@@ -1,35 +0,0 @@
-<?xml version='1.0' encoding='utf-8' ?>
-<Machine xmlns="http://www.dmtf.org/cimi">
- <uri>machine</uri>
- <name>Default machine name </name>
- <description>Default machine description</description>
- <created>2011-01-01 00:00:01 UTC</created>
- <property name="prop1" content="prop1 value" />
- <status>Started</status>
- <cpu>Intel</cpu>
- <memory quantity="4" units="gigabyte" />
- <disk>
- <capacity quantity="200" units="gigabyte" />
- <format>ntgs</format>
- <attachmentPoint>/dev/sdb0</attachmentPoint>
- <guestInterface>SATA</guestInterface>
- </disk>
- <volume href="http://host:port/volume/vol01" attachmentPoint="/dev/sdc01" protocol="NFS" />
- <networkInterface>
- <vsp href="http://host:port/vsp/vsp01" />
- <hostname>example host name</hostname>
- <macAddress>00:00:00:00:00:00</macAddress>
- <state>Active</state>
- <protocol>IPv6</protocol>
- <allocation>Dynamic</allocation>
- <address>10.10.10.1</address>
- <defaultGateway>10.10.0.1</defaultGateway>
- <dns>10.10.0.1</dns>
- <maxTransmissionUnit>512</maxTransmissionUnit>
- </networkInterface>
- <meter href="/meter" />
- <eventLog href="eventLog" />
- <operation rel="edit" href="/machine"/>
- <operation rel="delete" href="/machine"/>
- <operation rel="http://www.dmtf.org/cimi/stop" href="/machine/stop"/>
-</Machine>
\ No newline at end of file
diff --git a/server/lib/cimi/data/default_res/machine_configuration.col.xml b/server/lib/cimi/data/default_res/machine_configuration.col.xml
deleted file mode 100644
index db91861..0000000
--- a/server/lib/cimi/data/default_res/machine_configuration.col.xml
+++ /dev/null
@@ -1,21 +0,0 @@
-<?xml version='1.0' encoding='utf-8' ?>
-<MachineConfiguration xmlns="http://www.dmtf.org/cimi">
- <uri>machineConfiguration</uri>
- <name>Default machine configuration name </name>
- <description>Default description</description>
- <created>2011-01-01 00:00:01 UTC</created>
- <cpu>i386</cpu>
- <memory quantity="4" units="gigabyte" />
- <disk>
- <capacity quantity="200" units="gigabyte"/>
- <guestInterface>SATA</guestInterface>
- </disk>
- <disk>
- <capacity quantity="100" units="gigabyte"/>
- <guestInterface>SATA</guestInterface>
- </disk>
- <supportsSnapshots>false</supportsSnapshots>
- <guestInterface>http://www.ibm.com</guestInterface>
- <operation rel="edit" href="/machine_configuration"/>
- <operation rel="delete" href="/machinie_configuration"/>
-</MachineConfiguration>
\ No newline at end of file
diff --git a/server/lib/cimi/data/default_res/machine_image.col.xml b/server/lib/cimi/data/default_res/machine_image.col.xml
deleted file mode 100644
index e7de5e7..0000000
--- a/server/lib/cimi/data/default_res/machine_image.col.xml
+++ /dev/null
@@ -1,11 +0,0 @@
-<?xml version='1.0' encoding='utf-8' ?>
-<MachineImage xmlns="http://www.dmtf.org/cimi">
- <uri>machineImage</uri>
- <name>Default machine image name </name>
- <description>Default machine image description</description>
- <created>2011-01-01 00:00:01 UTC</created>
- <imageLocation>http://</imageLocation>
- <imageData></imageData>
- <operation rel="edit" href="/machine_image"/>
- <operation rel="delete" href="/machinie_image"/>
-</MachineImage>
\ No newline at end of file
diff --git a/server/lib/cimi/data/default_res/volume.col.xml b/server/lib/cimi/data/default_res/volume.col.xml
deleted file mode 100644
index 21b9d65..0000000
--- a/server/lib/cimi/data/default_res/volume.col.xml
+++ /dev/null
@@ -1,14 +0,0 @@
-<?xml version='1.0' encoding='utf-8' ?>
-<Volume xmlns="http://www.dmtf.org/cimi">
- <uri>volume</uri>
- <name>Default volume name </name>
- <description>Default volume description</description>
- <created>2011-01-01 00:00:01 UTC</created>
- <capacity quantity="200" units="gigabyte"/>
- <bootable>true</bootable>
- <format>ext4</format>
- <supportsSnapshots>true</supportsSnapshots>
- <guestInterface>SATA</guestInterface>
- <operation rel="edit" href="/volume"/>
- <operation rel="delete" href="/volume"/>
-</Volume>
\ No newline at end of file
--
1.7.4.4
[PATCH core 6/8] CIMI: Backported bread_crumb_ext helper. Added support for additional local variables for CIMI view
Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
server/lib/cimi/helpers/cmwg_helper.rb | 16 +++++++++++++++-
1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/server/lib/cimi/helpers/cmwg_helper.rb b/server/lib/cimi/helpers/cmwg_helper.rb
index 9dae5a1..e30ea17 100644
--- a/server/lib/cimi/helpers/cmwg_helper.rb
+++ b/server/lib/cimi/helpers/cmwg_helper.rb
@@ -16,14 +16,28 @@
module ApplicationHelper
include Deltacloud
- def render_resource(object_name, data)
+ def bread_crumb_ext
+ s = "<ul class='breadcrumb'><li class='first'><a href='#{settings.root_url}'>δ home</a></li>"
+ s+="<li class='docs'>#{link_to_documentation}</li>"
+ s+="</ul>"
+ end
+
+ def render_resource(object_name, data, locals={})
respond_to do |format|
format.xml do
report_error 404 unless data
+ locals.each { |key, value| eval "@#{key}=value" }
@object = data
content_type cimi_content_type_for(object_name.to_s.camelize, :xml)
haml :"#{object_name.to_s.pluralize}/show", :layout => false
end
+ format.html do
+ report_error 404 unless data
+ locals.each { |key, value| eval "@#{key}=value" }
+ @object = data
+ content_type 'application/xhtml+xml'
+ haml :"#{object_name.to_s.pluralize}/show", :layout => false
+ end
format.json do
report_error 404 unless data
content_type cimi_content_type_for(object_name.to_s.camelize, :json)
--
1.7.4.4