On Fri, 2012-08-03 at 15:15 +0200, [email protected] wrote: > Hi, > > Sorry for rev 4, but I fixed more tests to use rubygems builderplate > and consolidated helpers re-definitions.
ACK. I get one test failure: 1) Failure: test_0001_can be constructed from XML and JSON(MachineImage model) [./tests/cimi/spec/cimi/model/../../spec_helper.rb:101]: XML documents do not match entries differ[/MachineImage[0]/property[0]/name]: "locked" != "status" entries differ[/MachineImage[0]/property[0]/content]: "true" != "BUILD" entries differ[/MachineImage[0]/property[1]/name]: "status" != "locked" entries differ[/MachineImage[0]/property[1]/content]: "BUILD" != "true" That's because the machine_image fixture has two properties, and in Ruby 1.8, hashes do not preserve any sort of order; even though we tried very hard to work around that fact, those workarounds are defeated by the fact that JSON.parse maps properties to a hash. Rather than work around that, the attached patch addresses this by special-casing properties when comparing XML/JSON representations. David
>From ddb49d355f2c29b14000e9ee2357fe768e083782 Mon Sep 17 00:00:00 2001 From: David Lutterkort <[email protected]> Date: Mon, 6 Aug 2012 17:02:55 -0700 Subject: [PATCH] CIMI: map properties to an actual hash Even though we tried hard to make sure that properties were always kept in the order in which they appear in the source document, the fact that they are hashes in JSON makes that futile under Ruby 1.8 Rather than work around that, map them to a Hash in the deserialization, and change the tests to take into account that order for properties does not matter. --- server/lib/cimi/models/base.rb | 5 +---- server/lib/cimi/models/machine.rb | 6 +++--- server/lib/cimi/models/schema.rb | 21 +++++++++++---------- server/tests/cimi/spec/spec_helper.rb | 8 ++++++++ 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/server/lib/cimi/models/base.rb b/server/lib/cimi/models/base.rb index 16812ac..a41fb21 100644 --- a/server/lib/cimi/models/base.rb +++ b/server/lib/cimi/models/base.rb @@ -199,10 +199,7 @@ class CIMI::Model::Base # text :id, :name, :description, :created - # FIXME: this doesn't match with JSON - hash :property, :content => :value do - scalar :name - end + hash :property def self.act_as_root_entity(name=nil) if name diff --git a/server/lib/cimi/models/machine.rb b/server/lib/cimi/models/machine.rb index c4a37a1..415b0b5 100644 --- a/server/lib/cimi/models/machine.rb +++ b/server/lib/cimi/models/machine.rb @@ -158,10 +158,10 @@ class CIMI::Model::Machine < CIMI::Model::Base end def self.convert_instance_properties(instance, context) - properties = [] - properties << { :name => :machine_image, :value => context.machine_image_url(instance.image_id) } + properties = {} + properties["machine_image"] = context.machine_image_url(instance.image_id) if instance.respond_to? :keyname - properties << { :name => :machine_admin, :value => context.machine_admin_url(instance.keyname) } + properties["machine_admin"] = context.machine_admin_url(instance.keyname) end properties end diff --git a/server/lib/cimi/models/schema.rb b/server/lib/cimi/models/schema.rb index f301b69..fa0d11a 100644 --- a/server/lib/cimi/models/schema.rb +++ b/server/lib/cimi/models/schema.rb @@ -176,27 +176,28 @@ class CIMI::Model::Schema def initialize(name, opts = {}, &block) opts[:json_name] = name.to_s.pluralize unless opts[:json_name] super(name, opts) - @struct = Struct.new(name, opts, &block) end def from_xml(xml, model) - model[name] = (xml[xml_name] || []).map { |elt| @struct.convert_from_xml(elt) } + model[name] = (xml[xml_name] || []).inject({}) do |result, item| + result[item["name"]] = item["content"] + result + end end def from_json(json, model) - model[name] = (json[json_name] || {}).inject([]) do |result,item| - result << @struct.convert_from_json({ 'name' => item[0], 'value' => item[1] }) - end + model[name] = json[json_name] || {} end def to_xml(model, xml) - ary = (model[name] || []).map { |elt| @struct.convert_to_xml(elt) } + ary = (model[name] || {}).map { |k, v| { "name" => k, "content" => v } } xml[xml_name] = ary unless ary.empty? end def to_json(model, json) - ary = (model[name] || []).map { |elt| @struct.convert_to_json(elt) } - json[json_name] = ary.inject({}) { |result, item| result[item['name']] = item['value']; result } unless ary.empty? + if model[name] && ! model[name].empty? + json[json_name] = model[name] + end end end @@ -262,8 +263,8 @@ class CIMI::Model::Schema add_attributes!([name, opts], Struct, &block) end - def hash(name, opts={}, &block) - add_attributes!([name, opts], Hash, &block) + def hash(name) + add_attributes!([name, {}], Hash) end end diff --git a/server/tests/cimi/spec/spec_helper.rb b/server/tests/cimi/spec/spec_helper.rb index c2ea2d1..3e23fc0 100644 --- a/server/tests/cimi/spec/spec_helper.rb +++ b/server/tests/cimi/spec/spec_helper.rb @@ -58,6 +58,14 @@ class HashCmp mismatch("different array lengths", exp, act, path) end name = path.pop + if name == "property" + # Special hack for properties, since they truly are a hash + # and therefore not ordered, even though XmlSimple + # represents them as an array; since that array can be generated + # from a hash, in Ruby 1.8 the order of the array is random + exp = exp.sort! { |item1, item2| item1["name"] <=> item2["name"] } + act = act.sort! { |item1, item2| item1["name"] <=> item2["name"] } + end 0.upto(exp.size-1) do |i| compare_values(exp[i], act[i], path + [ "#{name}[#{i}]" ]) end -- 1.7.7.6
