Re: Second revamp of CIMI code

[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