refactoring xml generation

also refactoring `xml_name`

lib/diaspora_federation/entity.rb

@@ -75,7 +75,7 @@ module DiasporaFederation
     # Returns a Hash representing this Entity (attributes => values)
     # @return [Hash] entity data (mostly equal to the hash used for initialization).
     def to_h
-      self.class.class_prop_names.each_with_object({}) do |prop, hash|
+      self.class.class_props.keys.each_with_object({}) do |prop, hash|
         hash[prop] = public_send(prop)
       end
     end
@@ -103,26 +103,22 @@ module DiasporaFederation
     private
 
     def setable?(name, val)
-      prop_def = self.class.class_props.find {|p| p[:name] == name }
+      type = self.class.class_props[name]
-      return false if prop_def.nil? # property undefined
+      return false if type.nil? # property undefined
 
-      setable_string?(prop_def, val) || setable_nested?(prop_def, val) || setable_multi?(prop_def, val)
+      setable_string?(type, val) || setable_nested?(type, val) || setable_multi?(type, val)
     end
 
-    def setable_string?(definition, val)
+    def setable_string?(type, val)
-      (definition[:type] == String && val.respond_to?(:to_s))
+      type == String && val.respond_to?(:to_s)
     end
 
-    def setable_nested?(definition, val)
+    def setable_nested?(type, val)
-      t = definition[:type]
+      type.is_a?(Class) && type.ancestors.include?(Entity) && val.is_a?(Entity)
-      (t.is_a?(Class) && t.ancestors.include?(Entity) && val.is_a?(Entity))
     end
 
-    def setable_multi?(definition, val)
+    def setable_multi?(type, val)
-      t = definition[:type]
+      type.instance_of?(Array) && val.instance_of?(Array) && val.all? {|v| v.instance_of?(type.first) }
-      (t.instance_of?(Array) &&
-        val.instance_of?(Array) &&
-        val.all? {|v| v.instance_of?(t.first) })
     end
 
     def nilify(value)
@@ -146,35 +142,36 @@ module DiasporaFederation
       "Failed validation for properties: #{errors.join(' | ')}"
     end
 
+    def xml_elements
+      Hash[to_h.map {|name, value| [name, self.class.class_props[name] == String ? value.to_s : value] }]
+    end
+
     # Serialize the Entity into XML elements
     # @return [Nokogiri::XML::Element] root node
     def entity_xml
       doc = Nokogiri::XML::DocumentFragment.new(Nokogiri::XML::Document.new)
       Nokogiri::XML::Element.new(self.class.entity_name, doc).tap do |root_element|
-        self.class.class_props.each do |prop_def|
+        xml_elements.each do |name, value|
-          add_property_to_xml(doc, prop_def, root_element)
+          add_property_to_xml(doc, root_element, name, value)
         end
       end
     end
 
-    def add_property_to_xml(doc, prop_def, root_element)
+    def add_property_to_xml(doc, root_element, name, value)
-      property = prop_def[:name]
+      if value.is_a? String
-      type = prop_def[:type]
+        root_element << simple_node(doc, name, value)
-      if type == String
-        root_element << simple_node(doc, prop_def[:xml_name], property)
       else
         # call #to_xml for each item and append to root
-        [*public_send(property)].compact.each do |item|
+        [*value].compact.each do |item|
           root_element << item.to_xml
         end
       end
     end
 
     # create simple node, fill it with text and append to root
-    def simple_node(doc, name, property)
+    def simple_node(doc, name, value)
-      Nokogiri::XML::Element.new(name.to_s, doc).tap do |node|
+      Nokogiri::XML::Element.new(self.class.xml_names[name].to_s, doc).tap do |node|
-        data = public_send(property).to_s
+        node.content = value unless value.empty?
-        node.content = data unless data.empty?
       end
     end
 

lib/diaspora_federation/properties_dsl.rb

@@ -11,9 +11,9 @@ module DiasporaFederation
   #   entity :nested, NestedEntity
   #   entity :multiple, [OtherEntity]
   module PropertiesDSL
-    # @return [Array<Hash>] hash of declared entity properties
+    # @return [Hash] hash of declared entity properties
     def class_props
-      @class_props ||= []
+      @class_props ||= {}
     end
 
     # Define a generic (string-type) property
@@ -43,7 +43,7 @@ module DiasporaFederation
     # Return array of missing required property names
     # @return [Array<Symbol>] missing required property names
     def missing_props(args)
-      class_prop_names - default_props.keys - args.keys
+      class_props.keys - default_props.keys - args.keys
     end
 
     # Return a new hash of default values, with dynamic values
@@ -55,18 +55,6 @@ module DiasporaFederation
       }
     end
 
-    # Returns all nested Entities
-    # @return [Array<Hash>] nested properties
-    def nested_class_props
-      @nested_class_props ||= class_props.select {|p| p[:type] != String }
-    end
-
-    # Returns all property names
-    # @return [Array] property names
-    def class_prop_names
-      @class_prop_names ||= class_props.map {|p| p[:name] }
-    end
-
     # @param [Hash] data entity data
     # @return [Hash] hash with resolved aliases
     def resolv_aliases(data)
@@ -81,15 +69,22 @@ module DiasporaFederation
       }]
     end
 
+    # @return [Symbol] alias for the xml-generation/parsing
+    # @deprecated
+    def xml_names
+      @xml_names ||= {}
+    end
+
     # finds a property by +xml_name+ or +name+
     # @param [String] xml_name name of the property from the received xml
     # @return [Hash] the property data
     def find_property_for_xml_name(xml_name)
-      class_props.find {|prop| prop[:xml_name].to_s == xml_name || prop[:name].to_s == xml_name }
+      class_props.keys.find {|name| name.to_s == xml_name || xml_names[name].to_s == xml_name }
     end
 
     private
 
+    # @deprecated
     def determine_xml_name(name, type, opts={})
       raise ArgumentError, "xml_name is not supported for nested entities" if type != String && opts.has_key?(:xml_name)
 
@@ -110,8 +105,9 @@ module DiasporaFederation
     def define_property(name, type, opts={})
       raise InvalidName unless name_valid?(name)
 
-      class_props << {name: name, xml_name: determine_xml_name(name, type, opts), type: type}
+      class_props[name] = type
       default_props[name] = opts[:default] if opts.has_key? :default
+      xml_names[name] = determine_xml_name(name, type, opts)
 
       instance_eval { attr_reader name }
 

lib/diaspora_federation/salmon/xml_payload.rb

@@ -95,7 +95,7 @@ module DiasporaFederation
           property = klass.find_property_for_xml_name(xml_name)
 
           if property
-            entity_data[property[:name]] = parse_element_from_node(property[:type], xml_name, root_node)
+            entity_data[property] = parse_element_from_node(klass.class_props[property], xml_name, root_node)
           else
             additional_xml_elements[xml_name] = child.text
           end

spec/lib/diaspora_federation/properties_dsl_spec.rb

@@ -7,9 +7,8 @@ module DiasporaFederation
         dsl.property :test
         properties = dsl.class_props
         expect(properties).to have(1).item
-        expect(properties.first[:name]).to eq(:test)
+        expect(properties[:test]).to eq(String)
-        expect(properties.first[:xml_name]).to eq(:test)
+        expect(dsl.xml_names[:test]).to eq(:test)
-        expect(properties.first[:type]).to eq(String)
       end
 
       it "will not accept other types for names" do
@@ -26,18 +25,16 @@ module DiasporaFederation
         dsl.property :zzzz
         properties = dsl.class_props
         expect(properties).to have(3).items
-        expect(properties.map {|e| e[:name] }).to include(:test, :asdf, :zzzz)
+        expect(properties.keys).to include(:test, :asdf, :zzzz)
-        expect(properties.map {|e| e[:xml_name] }).to include(:test, :asdf, :zzzz)
+        properties.values.each {|type| expect(type).to eq(String) }
-        properties.each {|e| expect(e[:type]).to eq(String) }
       end
 
       it "can add an xml name to simple properties with a symbol" do
         dsl.property :test, xml_name: :xml_test
         properties = dsl.class_props
         expect(properties).to have(1).item
-        expect(properties.first[:name]).to eq(:test)
+        expect(properties[:test]).to eq(String)
-        expect(properties.first[:xml_name]).to eq(:xml_test)
+        expect(dsl.xml_names[:test]).to eq(:xml_test)
-        expect(properties.first[:type]).to eq(String)
       end
 
       it "will not accept other types for xml names" do
@@ -51,24 +48,22 @@ module DiasporaFederation
 
     context "nested entities" do
       it "gets included in the properties" do
-        expect(Entities::TestNestedEntity.class_prop_names).to include(:test, :multi)
+        expect(Entities::TestNestedEntity.class_props.keys).to include(:test, :multi)
       end
 
       it "can define nested entities" do
         dsl.entity :other, Entities::TestEntity
         properties = dsl.class_props
         expect(properties).to have(1).item
-        expect(properties.first[:name]).to eq(:other)
+        expect(properties[:other]).to eq(Entities::TestEntity)
-        expect(properties.first[:type]).to eq(Entities::TestEntity)
       end
 
       it "can define an array of a nested entity" do
         dsl.entity :other, [Entities::TestEntity]
         properties = dsl.class_props
         expect(properties).to have(1).item
-        expect(properties.first[:name]).to eq(:other)
+        expect(properties[:other]).to be_an_instance_of(Array)
-        expect(properties.first[:type]).to be_an_instance_of(Array)
+        expect(properties[:other].first).to eq(Entities::TestEntity)
-        expect(properties.first[:type].first).to eq(Entities::TestEntity)
       end
 
       it "must be an entity subclass" do
@@ -108,22 +103,6 @@ module DiasporaFederation
       end
     end
 
-    describe ".nested_class_props" do
-      it "returns the definition of nested class properties in an array" do
-        n_props = Entities::TestNestedEntity.nested_class_props
-        expect(n_props).to be_an_instance_of(Array)
-        expect(n_props.map {|p| p[:name] }).to include(:test, :multi)
-        expect(n_props.map {|p| p[:type] }).to include(Entities::TestEntity, [Entities::OtherEntity])
-      end
-    end
-
-    describe ".class_prop_names" do
-      it "returns the names of all class props in an array" do
-        expect(Entities::TestDefaultEntity.class_prop_names).to be_an_instance_of(Array)
-        expect(Entities::TestDefaultEntity.class_prop_names).to include(:test1, :test2, :test3, :test4)
-      end
-    end
-
     describe ".resolv_aliases" do
       it "resolves the defined alias" do
         dsl.property :test, alias: :test_alias
@@ -156,12 +135,12 @@ module DiasporaFederation
     describe ".find_property_for_xml_name" do
       it "finds property by xml_name" do
         dsl.property :test, xml_name: :xml_test
-        expect(dsl.find_property_for_xml_name("xml_test")).to eq(dsl.class_props.first)
+        expect(dsl.find_property_for_xml_name("xml_test")).to eq(:test)
       end
 
       it "finds property by name" do
         dsl.property :test, xml_name: :xml_test
-        expect(dsl.find_property_for_xml_name("test")).to eq(dsl.class_props.first)
+        expect(dsl.find_property_for_xml_name("test")).to eq(:test)
       end
 
       it "returns nil if property is not defined" do

spec/support/shared_entity_specs.rb

@@ -4,7 +4,7 @@ shared_examples "an Entity subclass" do
   end
 
   it "has its properties set" do
-    expect(described_class.class_prop_names).to include(*data.keys)
+    expect(described_class.class_props.keys).to include(*data.keys)
   end
 
   context "behaviour" do
@@ -51,10 +51,8 @@ shared_examples "an XML Entity" do
   end
 
   def check_entity(entity, parsed_entity)
-    entity.class.class_props.each do |prop_def|
+    entity.class.class_props.each do |name, type|
-      name = prop_def[:name]
+      validate_values(entity.send(name), parsed_entity.send(name), type)
-
-      validate_values(entity.send(name), parsed_entity.send(name), prop_def[:type])
     end
   end