On Thu, 2012-09-06 at 10:00 +0300, [email protected] wrote:
> On 06/09/12 02:58, David Lutterkort wrote:
> > On Wed, 2012-09-05 at 15:15 +0200, [email protected] wrote:
> >> From: Michal Fojtik <[email protected]>
> >>
> >> * 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 <[email protected]>
> >> 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


Reply via email to