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 2012/09/05 15:15:11 UTC

[PATCH core 1/2] CIMI: Prevent recycling old Structs

From: Michal Fojtik <mf...@redhat.com>

* This should fix DTACLOUD-311

Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/lib/cimi/models/schema.rb |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/server/lib/cimi/models/schema.rb b/server/lib/cimi/models/schema.rb
index 088e111..a7dc0f2 100644
--- a/server/lib/cimi/models/schema.rb
+++ b/server/lib/cimi/models/schema.rb
@@ -140,11 +140,9 @@ class CIMI::Model::Schema
     def struct
       cname = "CIMI_#{json_name.upcase_first}"
       if ::Struct.const_defined?(cname)
-        ::Struct.const_get(cname)
-      else
-        ::Struct.new("CIMI_#{json_name.upcase_first}",
-                     *@schema.attribute_names)
+        ::Struct.send(:remove_const, "CIMI_#{json_name.upcase_first}")
       end
+      ::Struct.new("CIMI_#{json_name.upcase_first}", *@schema.attribute_names)
     end
   end
 
-- 
1.7.10.2


Re: [PATCH core 2/2] CIMI: Added more syntax sugar for add_collection_member

Posted by "marios@redhat.com" <ma...@redhat.com>.
On 05/09/12 16:15, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> * This patch also moves this to CIMI::Model::Base instead
>   of schema to make sure the updated schema is the model
>   schema.
> 
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
>  server/lib/cimi/models/base.rb                           |   14 ++++++++++++++
>  server/lib/cimi/models/disk_collection.rb                |    3 +--
>  server/lib/cimi/models/machine_collection.rb             |    2 +-
>  .../lib/cimi/models/machine_configuration_collection.rb  |    4 +---
>  server/lib/cimi/models/machine_image_collection.rb       |    2 +-
>  server/lib/cimi/models/machine_template_collection.rb    |    4 +---
>  server/lib/cimi/models/machine_volume_collection.rb      |    2 +-
>  server/lib/cimi/models/network_collection.rb             |    4 ++--
>  server/lib/cimi/models/volume_collection.rb              |    2 +-
>  .../lib/cimi/models/volume_configuration_collection.rb   |    2 +-
>  server/lib/cimi/models/volume_image_collection.rb        |    4 ++--
>  server/lib/cimi/models/volume_template_collection.rb     |    2 +-
>  12 files changed, 27 insertions(+), 18 deletions(-)
> 
> diff --git a/server/lib/cimi/models/base.rb b/server/lib/cimi/models/base.rb
> index b375ce0..b627b82 100644
> --- a/server/lib/cimi/models/base.rb
> +++ b/server/lib/cimi/models/base.rb
> @@ -94,6 +94,20 @@ class CIMI::Model::Base
>    # attribute, we also define a getter and a setter to access/change the
>    # value for that attribute
>    class << self
> +
> +    def <<(model)
> +      clone_base_schema unless base_schema_cloned?
> +      member_name = model.name.split("::").last
> +      if ::Struct.const_defined?("CIMI_#{member_name}")
> +        puts "Removing struct"
> +        ::Struct.send(:remove_const, "CIMI_#{member_name}")
> +      end
> +      member_symbol = member_name.underscore.pluralize.to_sym
> +      members = CIMI::Model::Schema::Array.new(member_symbol)
> +      members.struct.schema.attributes = model.schema.attributes
> +      base_schema.attributes << members
> +    end
> +

ACK - probably want to remove the 'puts' and also get rid of the old
242   def add_collection_member_array(model) in schema.rb (perhaps also
add some comment here to explain what << this is doing).


>      def base_schema
>        @schema ||= CIMI::Model::Schema.new
>      end
> diff --git a/server/lib/cimi/models/disk_collection.rb b/server/lib/cimi/models/disk_collection.rb
> index b86a3c9..7dbff39 100644
> --- a/server/lib/cimi/models/disk_collection.rb
> +++ b/server/lib/cimi/models/disk_collection.rb
> @@ -16,7 +16,7 @@ class CIMI::Model::DiskCollection < CIMI::Model::Base
>    text :count
>  
>    #add disks array:
> -  self.schema.add_collection_member_array(CIMI::Model::Disk)
> +  self << CIMI::Model::Disk
>  
>    array :operations do
>      scalar :rel, :href
> @@ -26,7 +26,6 @@ class CIMI::Model::DiskCollection < CIMI::Model::Base
>      instance = context.driver.instance(context.credentials, :id=>instance_id)
>      machine_conf = CIMI::Model::MachineConfiguration.find(instance.instance_profile.name, context)
>      disks = CIMI::Model::Disk.find(instance, machine_conf, context, :all)
> -    storage_override = instance.instance_profile.overrides.find { |p, v| p == :storage }
>      self.new(
>        :id => context.machine_url(instance_id)+"/disks",
>        :description => "DiskCollection for Machine #{instance_id}",
> diff --git a/server/lib/cimi/models/machine_collection.rb b/server/lib/cimi/models/machine_collection.rb
> index 27c721d..9c57301 100644
> --- a/server/lib/cimi/models/machine_collection.rb
> +++ b/server/lib/cimi/models/machine_collection.rb
> @@ -20,7 +20,7 @@ class CIMI::Model::MachineCollection < CIMI::Model::Base
>    text :count
>  
>    #add machines array:
> -  self.schema.add_collection_member_array(CIMI::Model::Machine)
> +  self << CIMI::Model::Machine
>  
>    def self.default(context)
>      machines = CIMI::Model::Machine.all(context)
> diff --git a/server/lib/cimi/models/machine_configuration_collection.rb b/server/lib/cimi/models/machine_configuration_collection.rb
> index e75bbb4..051e1b9 100644
> --- a/server/lib/cimi/models/machine_configuration_collection.rb
> +++ b/server/lib/cimi/models/machine_configuration_collection.rb
> @@ -15,11 +15,9 @@
>  
>  class CIMI::Model::MachineConfigurationCollection < CIMI::Model::Base
>  
> -  act_as_root_entity :machine_configuration
> -
>    text :count
>  
> -  self.schema.add_collection_member_array(CIMI::Model::MachineConfiguration)
> +  self << CIMI::Model::MachineConfiguration
>  
>    def self.default(context)
>      machine_configurations = CIMI::Model::MachineConfiguration.all(context)
> diff --git a/server/lib/cimi/models/machine_image_collection.rb b/server/lib/cimi/models/machine_image_collection.rb
> index 703e515..cd4b5b4 100644
> --- a/server/lib/cimi/models/machine_image_collection.rb
> +++ b/server/lib/cimi/models/machine_image_collection.rb
> @@ -19,7 +19,7 @@ class CIMI::Model::MachineImageCollection < CIMI::Model::Base
>  
>    text :count
>  
> -  self.schema.add_collection_member_array(CIMI::Model::MachineImage)
> +  self << CIMI::Model::MachineImage
>  
>    def self.default(context)
>      machine_images = CIMI::Model::MachineImage.all(context)
> diff --git a/server/lib/cimi/models/machine_template_collection.rb b/server/lib/cimi/models/machine_template_collection.rb
> index bef9aaf..289c984 100644
> --- a/server/lib/cimi/models/machine_template_collection.rb
> +++ b/server/lib/cimi/models/machine_template_collection.rb
> @@ -15,11 +15,9 @@
>  
>  class CIMI::Model::MachineTemplateCollection < CIMI::Model::Base
>  
> -  act_as_root_entity :machine_template
> -
>    text :count
>  
> -  self.schema.add_collection_member_array(CIMI::Model::MachineTemplate)
> +  self << CIMI::Model::MachineTemplate
>  
>    def self.default(context)
>      machine_templates = CIMI::Model::MachineTemplate.all(context)
> diff --git a/server/lib/cimi/models/machine_volume_collection.rb b/server/lib/cimi/models/machine_volume_collection.rb
> index 85483ff..b9dfcad 100644
> --- a/server/lib/cimi/models/machine_volume_collection.rb
> +++ b/server/lib/cimi/models/machine_volume_collection.rb
> @@ -16,7 +16,7 @@ class CIMI::Model::MachineVolumeCollection < CIMI::Model::Base
>  
>    text :count
>  
> -  self.schema.add_collection_member_array(CIMI::Model::MachineVolume)
> +  self << CIMI::Model::MachineVolume
>  
>    array :operations do
>      scalar :rel, :href
> diff --git a/server/lib/cimi/models/network_collection.rb b/server/lib/cimi/models/network_collection.rb
> index c17c3f7..7c746b4 100644
> --- a/server/lib/cimi/models/network_collection.rb
> +++ b/server/lib/cimi/models/network_collection.rb
> @@ -17,8 +17,8 @@ class CIMI::Model::NetworkCollection < CIMI::Model::Base
>    act_as_root_entity :network
>    text :count
>  
> -#add networks Array:
> -  self.schema.add_collection_member_array(CIMI::Model::Network)
> +  #add networks Array:
> +  self << CIMI::Model::Network
>  
>    def self.default(context)
>      networks = CIMI::Model::Network.all(context)
> diff --git a/server/lib/cimi/models/volume_collection.rb b/server/lib/cimi/models/volume_collection.rb
> index 6bec78f..e780845 100644
> --- a/server/lib/cimi/models/volume_collection.rb
> +++ b/server/lib/cimi/models/volume_collection.rb
> @@ -19,7 +19,7 @@ class CIMI::Model::VolumeCollection < CIMI::Model::Base
>  
>    text :count
>  
> -  self.schema.add_collection_member_array(CIMI::Model::Volume)
> +  self << CIMI::Model::Volume
>  
>    def self.default(context)
>      volumes = CIMI::Model::Volume.all(context)
> diff --git a/server/lib/cimi/models/volume_configuration_collection.rb b/server/lib/cimi/models/volume_configuration_collection.rb
> index 8800e35..d8ac229 100644
> --- a/server/lib/cimi/models/volume_configuration_collection.rb
> +++ b/server/lib/cimi/models/volume_configuration_collection.rb
> @@ -19,7 +19,7 @@ class CIMI::Model::VolumeConfigurationCollection < CIMI::Model::Base
>  
>    text :count
>  
> -  self.schema.add_collection_member_array(CIMI::Model::VolumeConfiguration)
> +  self << CIMI::Model::VolumeConfiguration
>  
>    def self.default(context)
>      volume_configurations = CIMI::Model::VolumeConfiguration.all(context)
> diff --git a/server/lib/cimi/models/volume_image_collection.rb b/server/lib/cimi/models/volume_image_collection.rb
> index 78881d5..d411431 100644
> --- a/server/lib/cimi/models/volume_image_collection.rb
> +++ b/server/lib/cimi/models/volume_image_collection.rb
> @@ -17,9 +17,9 @@ class CIMI::Model::VolumeImageCollection < CIMI::Model::Base
>  
>    act_as_root_entity :volume_image
>  
> -  text :count 
> +  text :count
>  
> -  self.schema.add_collection_member_array(CIMI::Model::VolumeImage)
> +  self << CIMI::Model::VolumeImage
>  
>    def self.default(context)
>        volume_images = CIMI::Model::VolumeImage.all(context)
> diff --git a/server/lib/cimi/models/volume_template_collection.rb b/server/lib/cimi/models/volume_template_collection.rb
> index 3f2ae02..90462a0 100644
> --- a/server/lib/cimi/models/volume_template_collection.rb
> +++ b/server/lib/cimi/models/volume_template_collection.rb
> @@ -19,7 +19,7 @@ class CIMI::Model::VolumeTemplateCollection < CIMI::Model::Base
>  
>    text :count
>  
> -  self.schema.add_collection_member_array(CIMI::Model::VolumeTemplate)
> +  self << CIMI::Model::VolumeTemplate
>  
>    def self.default(context)
>      volume_templates = CIMI::Model::VolumeTemplate.all(context)
> 


Re: [PATCH core 2/2] CIMI: Added more syntax sugar for add_collection_member

Posted by David Lutterkort <lu...@redhat.com>.
On Thu, 2012-09-06 at 10:00 +0300, marios@redhat.com wrote:
> On 06/09/12 02:58, David Lutterkort wrote:
> > On Wed, 2012-09-05 at 15:15 +0200, mfojtik@redhat.com wrote:
> >> From: Michal Fojtik <mf...@redhat.com>
> >>
> >> * This patch also moves this to CIMI::Model::Base instead
> >>   of schema to make sure the updated schema is the model
> >>   schema.
> >>
> >> Signed-off-by: Michal fojtik <mf...@redhat.com>
> >> diff --git a/server/lib/cimi/models/base.rb b/server/lib/cimi/models/base.rb
> >> index b375ce0..b627b82 100644
> >> --- a/server/lib/cimi/models/base.rb
> >> +++ b/server/lib/cimi/models/base.rb
> >> @@ -94,6 +94,20 @@ class CIMI::Model::Base
> >>    # attribute, we also define a getter and a setter to access/change the
> >>    # value for that attribute
> >>    class << self
> >> +
> >> +    def <<(model)
> >> +      clone_base_schema unless base_schema_cloned?
> >> +      member_name = model.name.split("::").last
> >> +      if ::Struct.const_defined?("CIMI_#{member_name}")
> >> +        puts "Removing struct"
> >> +        ::Struct.send(:remove_const, "CIMI_#{member_name}")
> >> +      end
> >> +      member_symbol = member_name.underscore.pluralize.to_sym
> >> +      members = CIMI::Model::Schema::Array.new(member_symbol)
> >> +      members.struct.schema.attributes = model.schema.attributes
> >> +      base_schema.attributes << members
> >> +    end
> > 
> > I have to admit that I don't understand this code at all; how does the
> > assignment members.struct.schema.attributes = model.schema.attributes
> > make things safer ? Also, why can't the same be done in the Schema
> > class, since we only manipulate attributes of the Schema class in the
> > code above ?
> 
> some clarification.

Sorry, yes, I was a little obtuse in my comment; what I wasn't clear on
(and still am not) is how having this code in Base is safer than having
it in Schema.

> > 
> > In general, we should be able to get rid of all the *Collection classes
> > - that was part of the whole collection exercise in CIMI, to make it
> > possible to use the same code for all collections.
> > 
> > In terms of the schema DSL, I would strive towards something that lets
> > us describe machines like so:
> > 
> > class CIMI::Model::Machine < CIMI::Model::Base
> >   
> >         # Set up plumbing for the machine collection, i.e., code
> >         # to serialize/deserialize a collection of machines
> >         acts_as_root_collection
> > 
> >         text :state
> >         text :cpu
> >         ...
> >         
> >         # Expand to a collection with count, operations, etc.
> >         collection :disks, :class => CIMI::Model::Disk
> > end
> > 
> > I think we will need to have collections as a subclass of
> > Schema::Attribute at some point, so that we can control expansion of
> > collections via $expand during the serialization phase. Basically,
> > collections have two serializations: the unexpanded one with just a
> > href, and the expanded one with all the details. Which means that at
> > some point the to_xml/to_json methods will have to take parameters to
> > control expansion.
> > 
> 
> This looks neat and will make serialization/deserialization of
> *sub*-collections much easier. However I'm not clear on how you think we
> should handle the collections themselves...

I should clarify things with some code of my own ;) What I am thinking
is that a collection is a collection, but there are some collections
that are reachable from the CEP. Inclusion in the CEP can be triggered
by acts_as_root_collection. Though as I said, I should write some code
to make sure this will actually work; at least the theory was that there
shouldn't be a difference for (de)serialization between subcollections
and toplevel collections.

> Is what we're doing now (i.e. the
> add_collection_member_array in schema.rb which Michal has moved to '<<'
> in base.rb) ok?

Yes, it's ok to commit, definitely an improvement over what we have
right now.

David



Re: [PATCH core 2/2] CIMI: Added more syntax sugar for add_collection_member

Posted by "marios@redhat.com" <ma...@redhat.com>.
On 06/09/12 02:58, David Lutterkort wrote:
> On Wed, 2012-09-05 at 15:15 +0200, mfojtik@redhat.com wrote:
>> From: Michal Fojtik <mf...@redhat.com>
>>
>> * This patch also moves this to CIMI::Model::Base instead
>>   of schema to make sure the updated schema is the model
>>   schema.
>>
>> Signed-off-by: Michal fojtik <mf...@redhat.com>
>> diff --git a/server/lib/cimi/models/base.rb b/server/lib/cimi/models/base.rb
>> index b375ce0..b627b82 100644
>> --- a/server/lib/cimi/models/base.rb
>> +++ b/server/lib/cimi/models/base.rb
>> @@ -94,6 +94,20 @@ class CIMI::Model::Base
>>    # attribute, we also define a getter and a setter to access/change the
>>    # value for that attribute
>>    class << self
>> +
>> +    def <<(model)
>> +      clone_base_schema unless base_schema_cloned?
>> +      member_name = model.name.split("::").last
>> +      if ::Struct.const_defined?("CIMI_#{member_name}")
>> +        puts "Removing struct"
>> +        ::Struct.send(:remove_const, "CIMI_#{member_name}")
>> +      end
>> +      member_symbol = member_name.underscore.pluralize.to_sym
>> +      members = CIMI::Model::Schema::Array.new(member_symbol)
>> +      members.struct.schema.attributes = model.schema.attributes
>> +      base_schema.attributes << members
>> +    end
> 
> I have to admit that I don't understand this code at all; how does the
> assignment members.struct.schema.attributes = model.schema.attributes
> make things safer ? Also, why can't the same be done in the Schema
> class, since we only manipulate attributes of the Schema class in the
> code above ?

some clarification. This:

>> +      member_symbol = member_name.underscore.pluralize.to_sym
>> +      members = CIMI::Model::Schema::Array.new(member_symbol)
>> +      members.struct.schema.attributes = model.schema.attributes
>> +      base_schema.attributes << members

is the code I wrote to add a 'collection member array' for each of the
collections. It's not part of 'making things safer' (the four lines
above that are to do with preventing overriding of the struct) and it
*was* is Schema originally (as I'm writing this its still in schema.rb
line 242).

The idea was to 'implement collections' as quickly as possible with what
we have... this is the quickest way I could think of. So for example:

 16 class CIMI::Model::MachineCollection < CIMI::Model::Base$
 17 $
 18   act_as_root_entity :machine$
 19 $
 20   text :count$
 21 $
 22   #add machines array:$
 23   self.schema.add_collection_member_array(CIMI::Model::Machine)

^^^line 23 here calls those four lines above. Make a new schema array
for 'machines', add the attributes of the CIMI::Model::Machine schema to
this and then add this array to the CIMI::Model::MachineCollection
schema... so that:

 25   def self.default(context)$
 26     machines = CIMI::Model::Machine.all(context)$
 27     self.new($
 28       :id => context.machines_url,$
 29       :name => 'default',$
 30       :created => Time.now,$
 31       :description => "#{context.driver.name.capitalize}
MachineCollection",$
 32       :count => machines.size,$
 33       :machines => machines$
 34     )$
 35   end$

when we add the list of machines on line 33 they get serialized
correctly and according to the CIMI::Model::Machine schema.

> 
> In general, we should be able to get rid of all the *Collection classes
> - that was part of the whole collection exercise in CIMI, to make it
> possible to use the same code for all collections.
> 
> In terms of the schema DSL, I would strive towards something that lets
> us describe machines like so:
> 
> class CIMI::Model::Machine < CIMI::Model::Base
>   
>         # Set up plumbing for the machine collection, i.e., code
>         # to serialize/deserialize a collection of machines
>         acts_as_root_collection
> 
>         text :state
>         text :cpu
>         ...
>         
>         # Expand to a collection with count, operations, etc.
>         collection :disks, :class => CIMI::Model::Disk
> end
> 
> I think we will need to have collections as a subclass of
> Schema::Attribute at some point, so that we can control expansion of
> collections via $expand during the serialization phase. Basically,
> collections have two serializations: the unexpanded one with just a
> href, and the expanded one with all the details. Which means that at
> some point the to_xml/to_json methods will have to take parameters to
> control expansion.
> 

This looks neat and will make serialization/deserialization of
*sub*-collections much easier. However I'm not clear on how you think we
should handle the collections themselves... I mean like
MachineCollection vs MachineVolumeCollection (perhaps we're overloading
'collection' - I think of them in terms of collections and
sub-collections). Is what we're doing now (i.e. the
add_collection_member_array in schema.rb which Michal has moved to '<<'
in base.rb) ok?

marios

> I don't think we should address $expand complications with these
> patches, but that's where this code is headed.
> 
> David
> 
> 


Re: [PATCH core 2/2] CIMI: Added more syntax sugar for add_collection_member

Posted by David Lutterkort <lu...@redhat.com>.
On Wed, 2012-09-05 at 15:15 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> * This patch also moves this to CIMI::Model::Base instead
>   of schema to make sure the updated schema is the model
>   schema.
> 
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> diff --git a/server/lib/cimi/models/base.rb b/server/lib/cimi/models/base.rb
> index b375ce0..b627b82 100644
> --- a/server/lib/cimi/models/base.rb
> +++ b/server/lib/cimi/models/base.rb
> @@ -94,6 +94,20 @@ class CIMI::Model::Base
>    # attribute, we also define a getter and a setter to access/change the
>    # value for that attribute
>    class << self
> +
> +    def <<(model)
> +      clone_base_schema unless base_schema_cloned?
> +      member_name = model.name.split("::").last
> +      if ::Struct.const_defined?("CIMI_#{member_name}")
> +        puts "Removing struct"
> +        ::Struct.send(:remove_const, "CIMI_#{member_name}")
> +      end
> +      member_symbol = member_name.underscore.pluralize.to_sym
> +      members = CIMI::Model::Schema::Array.new(member_symbol)
> +      members.struct.schema.attributes = model.schema.attributes
> +      base_schema.attributes << members
> +    end

I have to admit that I don't understand this code at all; how does the
assignment members.struct.schema.attributes = model.schema.attributes
make things safer ? Also, why can't the same be done in the Schema
class, since we only manipulate attributes of the Schema class in the
code above ?

In general, we should be able to get rid of all the *Collection classes
- that was part of the whole collection exercise in CIMI, to make it
possible to use the same code for all collections.

In terms of the schema DSL, I would strive towards something that lets
us describe machines like so:

class CIMI::Model::Machine < CIMI::Model::Base
  
        # Set up plumbing for the machine collection, i.e., code
        # to serialize/deserialize a collection of machines
        acts_as_root_collection

        text :state
        text :cpu
        ...
        
        # Expand to a collection with count, operations, etc.
        collection :disks, :class => CIMI::Model::Disk
end

I think we will need to have collections as a subclass of
Schema::Attribute at some point, so that we can control expansion of
collections via $expand during the serialization phase. Basically,
collections have two serializations: the unexpanded one with just a
href, and the expanded one with all the details. Which means that at
some point the to_xml/to_json methods will have to take parameters to
control expansion.

I don't think we should address $expand complications with these
patches, but that's where this code is headed.

David



[PATCH core 2/2] CIMI: Added more syntax sugar for add_collection_member

Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>

* This patch also moves this to CIMI::Model::Base instead
  of schema to make sure the updated schema is the model
  schema.

Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/lib/cimi/models/base.rb                           |   14 ++++++++++++++
 server/lib/cimi/models/disk_collection.rb                |    3 +--
 server/lib/cimi/models/machine_collection.rb             |    2 +-
 .../lib/cimi/models/machine_configuration_collection.rb  |    4 +---
 server/lib/cimi/models/machine_image_collection.rb       |    2 +-
 server/lib/cimi/models/machine_template_collection.rb    |    4 +---
 server/lib/cimi/models/machine_volume_collection.rb      |    2 +-
 server/lib/cimi/models/network_collection.rb             |    4 ++--
 server/lib/cimi/models/volume_collection.rb              |    2 +-
 .../lib/cimi/models/volume_configuration_collection.rb   |    2 +-
 server/lib/cimi/models/volume_image_collection.rb        |    4 ++--
 server/lib/cimi/models/volume_template_collection.rb     |    2 +-
 12 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/server/lib/cimi/models/base.rb b/server/lib/cimi/models/base.rb
index b375ce0..b627b82 100644
--- a/server/lib/cimi/models/base.rb
+++ b/server/lib/cimi/models/base.rb
@@ -94,6 +94,20 @@ class CIMI::Model::Base
   # attribute, we also define a getter and a setter to access/change the
   # value for that attribute
   class << self
+
+    def <<(model)
+      clone_base_schema unless base_schema_cloned?
+      member_name = model.name.split("::").last
+      if ::Struct.const_defined?("CIMI_#{member_name}")
+        puts "Removing struct"
+        ::Struct.send(:remove_const, "CIMI_#{member_name}")
+      end
+      member_symbol = member_name.underscore.pluralize.to_sym
+      members = CIMI::Model::Schema::Array.new(member_symbol)
+      members.struct.schema.attributes = model.schema.attributes
+      base_schema.attributes << members
+    end
+
     def base_schema
       @schema ||= CIMI::Model::Schema.new
     end
diff --git a/server/lib/cimi/models/disk_collection.rb b/server/lib/cimi/models/disk_collection.rb
index b86a3c9..7dbff39 100644
--- a/server/lib/cimi/models/disk_collection.rb
+++ b/server/lib/cimi/models/disk_collection.rb
@@ -16,7 +16,7 @@ class CIMI::Model::DiskCollection < CIMI::Model::Base
   text :count
 
   #add disks array:
-  self.schema.add_collection_member_array(CIMI::Model::Disk)
+  self << CIMI::Model::Disk
 
   array :operations do
     scalar :rel, :href
@@ -26,7 +26,6 @@ class CIMI::Model::DiskCollection < CIMI::Model::Base
     instance = context.driver.instance(context.credentials, :id=>instance_id)
     machine_conf = CIMI::Model::MachineConfiguration.find(instance.instance_profile.name, context)
     disks = CIMI::Model::Disk.find(instance, machine_conf, context, :all)
-    storage_override = instance.instance_profile.overrides.find { |p, v| p == :storage }
     self.new(
       :id => context.machine_url(instance_id)+"/disks",
       :description => "DiskCollection for Machine #{instance_id}",
diff --git a/server/lib/cimi/models/machine_collection.rb b/server/lib/cimi/models/machine_collection.rb
index 27c721d..9c57301 100644
--- a/server/lib/cimi/models/machine_collection.rb
+++ b/server/lib/cimi/models/machine_collection.rb
@@ -20,7 +20,7 @@ class CIMI::Model::MachineCollection < CIMI::Model::Base
   text :count
 
   #add machines array:
-  self.schema.add_collection_member_array(CIMI::Model::Machine)
+  self << CIMI::Model::Machine
 
   def self.default(context)
     machines = CIMI::Model::Machine.all(context)
diff --git a/server/lib/cimi/models/machine_configuration_collection.rb b/server/lib/cimi/models/machine_configuration_collection.rb
index e75bbb4..051e1b9 100644
--- a/server/lib/cimi/models/machine_configuration_collection.rb
+++ b/server/lib/cimi/models/machine_configuration_collection.rb
@@ -15,11 +15,9 @@
 
 class CIMI::Model::MachineConfigurationCollection < CIMI::Model::Base
 
-  act_as_root_entity :machine_configuration
-
   text :count
 
-  self.schema.add_collection_member_array(CIMI::Model::MachineConfiguration)
+  self << CIMI::Model::MachineConfiguration
 
   def self.default(context)
     machine_configurations = CIMI::Model::MachineConfiguration.all(context)
diff --git a/server/lib/cimi/models/machine_image_collection.rb b/server/lib/cimi/models/machine_image_collection.rb
index 703e515..cd4b5b4 100644
--- a/server/lib/cimi/models/machine_image_collection.rb
+++ b/server/lib/cimi/models/machine_image_collection.rb
@@ -19,7 +19,7 @@ class CIMI::Model::MachineImageCollection < CIMI::Model::Base
 
   text :count
 
-  self.schema.add_collection_member_array(CIMI::Model::MachineImage)
+  self << CIMI::Model::MachineImage
 
   def self.default(context)
     machine_images = CIMI::Model::MachineImage.all(context)
diff --git a/server/lib/cimi/models/machine_template_collection.rb b/server/lib/cimi/models/machine_template_collection.rb
index bef9aaf..289c984 100644
--- a/server/lib/cimi/models/machine_template_collection.rb
+++ b/server/lib/cimi/models/machine_template_collection.rb
@@ -15,11 +15,9 @@
 
 class CIMI::Model::MachineTemplateCollection < CIMI::Model::Base
 
-  act_as_root_entity :machine_template
-
   text :count
 
-  self.schema.add_collection_member_array(CIMI::Model::MachineTemplate)
+  self << CIMI::Model::MachineTemplate
 
   def self.default(context)
     machine_templates = CIMI::Model::MachineTemplate.all(context)
diff --git a/server/lib/cimi/models/machine_volume_collection.rb b/server/lib/cimi/models/machine_volume_collection.rb
index 85483ff..b9dfcad 100644
--- a/server/lib/cimi/models/machine_volume_collection.rb
+++ b/server/lib/cimi/models/machine_volume_collection.rb
@@ -16,7 +16,7 @@ class CIMI::Model::MachineVolumeCollection < CIMI::Model::Base
 
   text :count
 
-  self.schema.add_collection_member_array(CIMI::Model::MachineVolume)
+  self << CIMI::Model::MachineVolume
 
   array :operations do
     scalar :rel, :href
diff --git a/server/lib/cimi/models/network_collection.rb b/server/lib/cimi/models/network_collection.rb
index c17c3f7..7c746b4 100644
--- a/server/lib/cimi/models/network_collection.rb
+++ b/server/lib/cimi/models/network_collection.rb
@@ -17,8 +17,8 @@ class CIMI::Model::NetworkCollection < CIMI::Model::Base
   act_as_root_entity :network
   text :count
 
-#add networks Array:
-  self.schema.add_collection_member_array(CIMI::Model::Network)
+  #add networks Array:
+  self << CIMI::Model::Network
 
   def self.default(context)
     networks = CIMI::Model::Network.all(context)
diff --git a/server/lib/cimi/models/volume_collection.rb b/server/lib/cimi/models/volume_collection.rb
index 6bec78f..e780845 100644
--- a/server/lib/cimi/models/volume_collection.rb
+++ b/server/lib/cimi/models/volume_collection.rb
@@ -19,7 +19,7 @@ class CIMI::Model::VolumeCollection < CIMI::Model::Base
 
   text :count
 
-  self.schema.add_collection_member_array(CIMI::Model::Volume)
+  self << CIMI::Model::Volume
 
   def self.default(context)
     volumes = CIMI::Model::Volume.all(context)
diff --git a/server/lib/cimi/models/volume_configuration_collection.rb b/server/lib/cimi/models/volume_configuration_collection.rb
index 8800e35..d8ac229 100644
--- a/server/lib/cimi/models/volume_configuration_collection.rb
+++ b/server/lib/cimi/models/volume_configuration_collection.rb
@@ -19,7 +19,7 @@ class CIMI::Model::VolumeConfigurationCollection < CIMI::Model::Base
 
   text :count
 
-  self.schema.add_collection_member_array(CIMI::Model::VolumeConfiguration)
+  self << CIMI::Model::VolumeConfiguration
 
   def self.default(context)
     volume_configurations = CIMI::Model::VolumeConfiguration.all(context)
diff --git a/server/lib/cimi/models/volume_image_collection.rb b/server/lib/cimi/models/volume_image_collection.rb
index 78881d5..d411431 100644
--- a/server/lib/cimi/models/volume_image_collection.rb
+++ b/server/lib/cimi/models/volume_image_collection.rb
@@ -17,9 +17,9 @@ class CIMI::Model::VolumeImageCollection < CIMI::Model::Base
 
   act_as_root_entity :volume_image
 
-  text :count 
+  text :count
 
-  self.schema.add_collection_member_array(CIMI::Model::VolumeImage)
+  self << CIMI::Model::VolumeImage
 
   def self.default(context)
       volume_images = CIMI::Model::VolumeImage.all(context)
diff --git a/server/lib/cimi/models/volume_template_collection.rb b/server/lib/cimi/models/volume_template_collection.rb
index 3f2ae02..90462a0 100644
--- a/server/lib/cimi/models/volume_template_collection.rb
+++ b/server/lib/cimi/models/volume_template_collection.rb
@@ -19,7 +19,7 @@ class CIMI::Model::VolumeTemplateCollection < CIMI::Model::Base
 
   text :count
 
-  self.schema.add_collection_member_array(CIMI::Model::VolumeTemplate)
+  self << CIMI::Model::VolumeTemplate
 
   def self.default(context)
     volume_templates = CIMI::Model::VolumeTemplate.all(context)
-- 
1.7.10.2


Re: [PATCH core 1/2] CIMI: Prevent recycling old Structs

Posted by Michal Fojtik <mf...@redhat.com>.
On Sep 6, 2012, at 1:30 AM, David Lutterkort <lu...@redhat.com> wrote:

> On Wed, 2012-09-05 at 15:15 +0200, mfojtik@redhat.com wrote:
>> From: Michal Fojtik <mf...@redhat.com>
>> 
>> * This should fix DTACLOUD-311
>> 
>> Signed-off-by: Michal fojtik <mf...@redhat.com>
>> ---
>> server/lib/cimi/models/schema.rb |    6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>> 
>> diff --git a/server/lib/cimi/models/schema.rb b/server/lib/cimi/models/schema.rb
>> index 088e111..a7dc0f2 100644
>> --- a/server/lib/cimi/models/schema.rb
>> +++ b/server/lib/cimi/models/schema.rb
>> @@ -140,11 +140,9 @@ class CIMI::Model::Schema
>>     def struct
>>       cname = "CIMI_#{json_name.upcase_first}"
>>       if ::Struct.const_defined?(cname)
>> -        ::Struct.const_get(cname)
>> -      else
>> -        ::Struct.new("CIMI_#{json_name.upcase_first}",
>> -                     *@schema.attribute_names)
>> +        ::Struct.send(:remove_const, "CIMI_#{json_name.upcase_first}")
>>       end
>> +      ::Struct.new("CIMI_#{json_name.upcase_first}", *@schema.attribute_names)
> 
> Nice catch - instead of defining a constant Struct::Foo and then
> undefining it, and continually defining new classes, we should just save
> it in an instance variable, i.e. do the following
> 
>        def struct
>          @struct_class ||= Struct.new(nil, *@schema.attribute_names)
>        end

Cool, that can work too. Also we can use OpenStruct class if we don't want
to use name for the struct.

Do you want me to send a new rev with instance variable?

  -- Michal

Michal Fojtik
http://deltacloud.org
mfojtik@redhat.com




Re: [PATCH core 1/2] CIMI: Prevent recycling old Structs

Posted by David Lutterkort <lu...@redhat.com>.
On Thu, 2012-09-06 at 14:36 +0200, Michal Fojtik wrote:
> On Sep 6, 2012, at 1:30 AM, David Lutterkort <lu...@redhat.com> wrote:
> 
> > On Wed, 2012-09-05 at 15:15 +0200, mfojtik@redhat.com wrote:
> >> From: Michal Fojtik <mf...@redhat.com>
> >> 
> >> * This should fix DTACLOUD-311
> >> 
> >> Signed-off-by: Michal fojtik <mf...@redhat.com>
> >> ---
> >> server/lib/cimi/models/schema.rb |    6 ++----
> >> 1 file changed, 2 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/server/lib/cimi/models/schema.rb b/server/lib/cimi/models/schema.rb
> >> index 088e111..a7dc0f2 100644
> >> --- a/server/lib/cimi/models/schema.rb
> >> +++ b/server/lib/cimi/models/schema.rb
> >> @@ -140,11 +140,9 @@ class CIMI::Model::Schema
> >>     def struct
> >>       cname = "CIMI_#{json_name.upcase_first}"
> >>       if ::Struct.const_defined?(cname)
> >> -        ::Struct.const_get(cname)
> >> -      else
> >> -        ::Struct.new("CIMI_#{json_name.upcase_first}",
> >> -                     *@schema.attribute_names)
> >> +        ::Struct.send(:remove_const, "CIMI_#{json_name.upcase_first}")
> >>       end
> >> +      ::Struct.new("CIMI_#{json_name.upcase_first}", *@schema.attribute_names)
> > 
> > Nice catch - instead of defining a constant Struct::Foo and then
> > undefining it, and continually defining new classes, we should just save
> > it in an instance variable, i.e. do the following
> > 
> >        def struct
> >          @struct_class ||= Struct.new(nil, *@schema.attribute_names)
> >        end
> > 
> 
> I tried this but got a lot of:
> 
> ................F/Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Operations
> ./Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Disks
> /Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Operations
> F/Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Operations
> ./Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Volumes
> /Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_NetworkInterfaces
> /Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Operations
> ...../Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Structs
> ../Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Meter
> ...../Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Struct
> 
> 
> But no errors. I think we can combine both to:
> 
>     def struct
>       cname = "CIMI_#{json_name.upcase_first}"
>       ::Struct.send(:remove_const, cname) if ::Struct.const_defined?(cname)
>       @struct_class ||= ::Struct.new(cname, *@schema.attribute_names)
>     end
> 
> This will remove the warnings and does not define new class every time :)

Don't give the class a name at all (by passing in nil as the name)

David





Re: [PATCH core 1/2] CIMI: Prevent recycling old Structs

Posted by Michal Fojtik <mf...@redhat.com>.
On Sep 6, 2012, at 1:30 AM, David Lutterkort <lu...@redhat.com> wrote:

> On Wed, 2012-09-05 at 15:15 +0200, mfojtik@redhat.com wrote:
>> From: Michal Fojtik <mf...@redhat.com>
>> 
>> * This should fix DTACLOUD-311
>> 
>> Signed-off-by: Michal fojtik <mf...@redhat.com>
>> ---
>> server/lib/cimi/models/schema.rb |    6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>> 
>> diff --git a/server/lib/cimi/models/schema.rb b/server/lib/cimi/models/schema.rb
>> index 088e111..a7dc0f2 100644
>> --- a/server/lib/cimi/models/schema.rb
>> +++ b/server/lib/cimi/models/schema.rb
>> @@ -140,11 +140,9 @@ class CIMI::Model::Schema
>>     def struct
>>       cname = "CIMI_#{json_name.upcase_first}"
>>       if ::Struct.const_defined?(cname)
>> -        ::Struct.const_get(cname)
>> -      else
>> -        ::Struct.new("CIMI_#{json_name.upcase_first}",
>> -                     *@schema.attribute_names)
>> +        ::Struct.send(:remove_const, "CIMI_#{json_name.upcase_first}")
>>       end
>> +      ::Struct.new("CIMI_#{json_name.upcase_first}", *@schema.attribute_names)
> 
> Nice catch - instead of defining a constant Struct::Foo and then
> undefining it, and continually defining new classes, we should just save
> it in an instance variable, i.e. do the following
> 
>        def struct
>          @struct_class ||= Struct.new(nil, *@schema.attribute_names)
>        end
> 

I tried this but got a lot of:

................F/Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Operations
./Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Disks
/Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Operations
F/Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Operations
./Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Volumes
/Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_NetworkInterfaces
/Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Operations
...../Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Structs
../Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Meter
...../Users/mfojtik/code/core/server/lib/cimi/models/schema.rb:146: warning: redefining constant Struct::CIMI_Struct


But no errors. I think we can combine both to:

    def struct
      cname = "CIMI_#{json_name.upcase_first}"
      ::Struct.send(:remove_const, cname) if ::Struct.const_defined?(cname)
      @struct_class ||= ::Struct.new(cname, *@schema.attribute_names)
    end

This will remove the warnings and does not define new class every time :)

  -- Michal


Michal Fojtik
http://deltacloud.org
mfojtik@redhat.com




Re: [PATCH core 1/2] CIMI: Prevent recycling old Structs

Posted by David Lutterkort <lu...@redhat.com>.
On Wed, 2012-09-05 at 15:15 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> * This should fix DTACLOUD-311
> 
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
>  server/lib/cimi/models/schema.rb |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/server/lib/cimi/models/schema.rb b/server/lib/cimi/models/schema.rb
> index 088e111..a7dc0f2 100644
> --- a/server/lib/cimi/models/schema.rb
> +++ b/server/lib/cimi/models/schema.rb
> @@ -140,11 +140,9 @@ class CIMI::Model::Schema
>      def struct
>        cname = "CIMI_#{json_name.upcase_first}"
>        if ::Struct.const_defined?(cname)
> -        ::Struct.const_get(cname)
> -      else
> -        ::Struct.new("CIMI_#{json_name.upcase_first}",
> -                     *@schema.attribute_names)
> +        ::Struct.send(:remove_const, "CIMI_#{json_name.upcase_first}")
>        end
> +      ::Struct.new("CIMI_#{json_name.upcase_first}", *@schema.attribute_names)

Nice catch - instead of defining a constant Struct::Foo and then
undefining it, and continually defining new classes, we should just save
it in an instance variable, i.e. do the following

        def struct
          @struct_class ||= Struct.new(nil, *@schema.attribute_names)
        end

David