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