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 liton...@us.ibm.com From: mfoj...@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