From 1238fe0384c317394645b16012a446cf5bb9ba5d Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Wed, 30 Oct 2019 04:17:57 +0100 Subject: [PATCH] Remove old property-name mappings and cleanup xml_name functionality Closes #29 --- .../entities/account_deletion.rb | 2 +- .../entities/conversation.rb | 4 +- lib/diaspora_federation/entities/like.rb | 2 +- lib/diaspora_federation/entities/message.rb | 2 +- .../entities/participation.rb | 4 +- lib/diaspora_federation/entities/person.rb | 2 +- lib/diaspora_federation/entities/photo.rb | 2 +- lib/diaspora_federation/entities/post.rb | 2 +- lib/diaspora_federation/entities/profile.rb | 2 +- lib/diaspora_federation/entities/relayable.rb | 2 +- lib/diaspora_federation/entities/reshare.rb | 4 +- .../entities/retraction.rb | 6 +-- .../entities/status_message.rb | 2 +- lib/diaspora_federation/entity.rb | 1 - .../parsers/json_parser.rb | 2 +- lib/diaspora_federation/parsers/xml_parser.rb | 21 +++++++--- lib/diaspora_federation/properties_dsl.rb | 37 ----------------- spec/entities.rb | 5 --- .../entities/conversation_spec.rb | 2 +- .../diaspora_federation/entities/like_spec.rb | 2 +- .../entities/relayable_spec.rb | 2 +- spec/lib/diaspora_federation/entity_spec.rb | 9 ----- .../parsers/xml_parser_spec.rb | 28 ------------- .../properties_dsl_spec.rb | 40 ------------------- 24 files changed, 37 insertions(+), 148 deletions(-) diff --git a/lib/diaspora_federation/entities/account_deletion.rb b/lib/diaspora_federation/entities/account_deletion.rb index d969b24..d17d067 100644 --- a/lib/diaspora_federation/entities/account_deletion.rb +++ b/lib/diaspora_federation/entities/account_deletion.rb @@ -14,7 +14,7 @@ module DiasporaFederation # Alias for author # @see AccountDeletion#author # @return [String] diaspora* ID - property :author, :string, alias: :diaspora_id, xml_name: :diaspora_handle + property :author, :string, alias: :diaspora_id # @return [String] string representation of this object def to_s diff --git a/lib/diaspora_federation/entities/conversation.rb b/lib/diaspora_federation/entities/conversation.rb index b3720d3..60ef992 100644 --- a/lib/diaspora_federation/entities/conversation.rb +++ b/lib/diaspora_federation/entities/conversation.rb @@ -10,7 +10,7 @@ module DiasporaFederation # The diaspora* ID of the person initiated the conversation # @see Person#author # @return [String] diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] guid # A random string of at least 16 chars @@ -29,7 +29,7 @@ module DiasporaFederation # @!attribute [r] participants # The diaspora* IDs of the persons participating the conversation separated by ";" # @return [String] participants diaspora* IDs - property :participants, :string, xml_name: :participant_handles + property :participants, :string # @!attribute [r] messages # @return [[Entities::Message]] Messages of this conversation diff --git a/lib/diaspora_federation/entities/like.rb b/lib/diaspora_federation/entities/like.rb index b589058..941bf19 100644 --- a/lib/diaspora_federation/entities/like.rb +++ b/lib/diaspora_federation/entities/like.rb @@ -13,7 +13,7 @@ module DiasporaFederation # Can be "Post" or "Comment" (Comments are currently not implemented in the # diaspora* frontend). # @return [String] parent type - property :parent_type, :string, xml_name: :target_type + property :parent_type, :string # @!attribute [r] positive # If +true+ set a like, if +false+, set a dislike (dislikes are currently not diff --git a/lib/diaspora_federation/entities/message.rb b/lib/diaspora_federation/entities/message.rb index 81c631a..e6f073b 100644 --- a/lib/diaspora_federation/entities/message.rb +++ b/lib/diaspora_federation/entities/message.rb @@ -10,7 +10,7 @@ module DiasporaFederation # The diaspora* ID of the author # @see Person#author # @return [String] diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] guid # A random string of at least 16 chars diff --git a/lib/diaspora_federation/entities/participation.rb b/lib/diaspora_federation/entities/participation.rb index 554d9bd..18c49f7 100644 --- a/lib/diaspora_federation/entities/participation.rb +++ b/lib/diaspora_federation/entities/participation.rb @@ -10,7 +10,7 @@ module DiasporaFederation # The diaspora* ID of the author # @see Person#author # @return [String] diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] guid # A random string of at least 16 chars @@ -27,7 +27,7 @@ module DiasporaFederation # A string describing a type of the target to subscribe on # Currently only "Post" is supported. # @return [String] parent type - property :parent_type, :string, xml_name: :target_type + property :parent_type, :string # @return [String] string representation of this object def to_s diff --git a/lib/diaspora_federation/entities/person.rb b/lib/diaspora_federation/entities/person.rb index e4ad225..ad35947 100644 --- a/lib/diaspora_federation/entities/person.rb +++ b/lib/diaspora_federation/entities/person.rb @@ -21,7 +21,7 @@ module DiasporaFederation # alias for author # @see Person#author # @return [String] diaspora* ID - property :author, :string, alias: :diaspora_id, xml_name: :diaspora_handle + property :author, :string, alias: :diaspora_id # @!attribute [r] url # @see Discovery::WebFinger#seed_url diff --git a/lib/diaspora_federation/entities/photo.rb b/lib/diaspora_federation/entities/photo.rb index 4f11179..fcda7aa 100644 --- a/lib/diaspora_federation/entities/photo.rb +++ b/lib/diaspora_federation/entities/photo.rb @@ -16,7 +16,7 @@ module DiasporaFederation # The diaspora* ID of the person who uploaded the photo # @see Person#author # @return [String] author diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] public # Points if the photo is visible to everyone or only to some aspects diff --git a/lib/diaspora_federation/entities/post.rb b/lib/diaspora_federation/entities/post.rb index a5c0a48..4eb9d93 100644 --- a/lib/diaspora_federation/entities/post.rb +++ b/lib/diaspora_federation/entities/post.rb @@ -32,7 +32,7 @@ module DiasporaFederation # @param [Entity] entity the entity in which it is included def self.included(entity) entity.class_eval do - property :author, :string, xml_name: :diaspora_handle + property :author, :string property :guid, :string property :created_at, :timestamp, default: -> { Time.now.utc } property :public, :boolean, default: false diff --git a/lib/diaspora_federation/entities/profile.rb b/lib/diaspora_federation/entities/profile.rb index b6ba0ff..08b53ae 100644 --- a/lib/diaspora_federation/entities/profile.rb +++ b/lib/diaspora_federation/entities/profile.rb @@ -14,7 +14,7 @@ module DiasporaFederation # Alias for author # @see Profile#author # @return [String] diaspora* ID - property :author, :string, alias: :diaspora_id, xml_name: :diaspora_handle + property :author, :string, alias: :diaspora_id # @!attribute [r] edited_at # The timestamp when the profile was edited diff --git a/lib/diaspora_federation/entities/relayable.rb b/lib/diaspora_federation/entities/relayable.rb index 3fe94c8..49b55cd 100644 --- a/lib/diaspora_federation/entities/relayable.rb +++ b/lib/diaspora_federation/entities/relayable.rb @@ -48,7 +48,7 @@ module DiasporaFederation # @param [Entity] klass the entity in which it is included def self.included(klass) klass.class_eval do - property :author, :string, xml_name: :diaspora_handle + property :author, :string property :guid, :string property :parent_guid, :string property :author_signature, :string, default: nil diff --git a/lib/diaspora_federation/entities/reshare.rb b/lib/diaspora_federation/entities/reshare.rb index aa38b29..f194771 100644 --- a/lib/diaspora_federation/entities/reshare.rb +++ b/lib/diaspora_federation/entities/reshare.rb @@ -10,7 +10,7 @@ module DiasporaFederation # The diaspora* ID of the person who reshares the post # @see Person#author # @return [String] diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] guid # A random string of at least 16 chars @@ -27,7 +27,7 @@ module DiasporaFederation # The diaspora* ID of the person who posted the original post # @see Person#author # @return [String] diaspora* ID - property :root_author, :string, optional: true, xml_name: :root_diaspora_id + property :root_author, :string, optional: true # @!attribute [r] root_guid # Guid of the original post diff --git a/lib/diaspora_federation/entities/retraction.rb b/lib/diaspora_federation/entities/retraction.rb index 5c53eda..e809b2f 100644 --- a/lib/diaspora_federation/entities/retraction.rb +++ b/lib/diaspora_federation/entities/retraction.rb @@ -10,17 +10,17 @@ module DiasporaFederation # The diaspora* ID of the person who deletes the entity # @see Person#author # @return [String] diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] target_guid # Guid of the entity to be deleted # @return [String] target guid - property :target_guid, :string, xml_name: :post_guid + property :target_guid, :string # @!attribute [r] target_type # A string describing the type of the target # @return [String] target type - property :target_type, :string, xml_name: :type + property :target_type, :string # @!attribute [r] target # Target entity diff --git a/lib/diaspora_federation/entities/status_message.rb b/lib/diaspora_federation/entities/status_message.rb index 1da0d8b..e893a23 100644 --- a/lib/diaspora_federation/entities/status_message.rb +++ b/lib/diaspora_federation/entities/status_message.rb @@ -11,7 +11,7 @@ module DiasporaFederation # @!attribute [r] text # Text of the status message composed by the user # @return [String] text of the status message - property :text, :string, xml_name: :raw_message + property :text, :string # @!attribute [r] edited_at # The timestamp when the status message was edited diff --git a/lib/diaspora_federation/entity.rb b/lib/diaspora_federation/entity.rb index 26526d1..3aa042b 100644 --- a/lib/diaspora_federation/entity.rb +++ b/lib/diaspora_federation/entity.rb @@ -17,7 +17,6 @@ module DiasporaFederation # property :prop # property :optional, default: false # property :dynamic_default, default: -> { Time.now } - # property :another_prop, xml_name: :another_name # entity :nested, NestedEntity # entity :multiple, [OtherEntity] # end diff --git a/lib/diaspora_federation/parsers/json_parser.rb b/lib/diaspora_federation/parsers/json_parser.rb index 1a8642f..b91dad7 100644 --- a/lib/diaspora_federation/parsers/json_parser.rb +++ b/lib/diaspora_federation/parsers/json_parser.rb @@ -18,7 +18,7 @@ module DiasporaFederation def parse_entity_data(entity_data) hash = entity_data.map {|key, value| - property = entity_type.find_property_for_xml_name(key) + property = entity_type.class_props.keys.find {|name| name.to_s == key } if property type = entity_type.class_props[property] [property, parse_element_from_value(type, entity_data[key])] diff --git a/lib/diaspora_federation/parsers/xml_parser.rb b/lib/diaspora_federation/parsers/xml_parser.rb index 40acfda..10baf2a 100644 --- a/lib/diaspora_federation/parsers/xml_parser.rb +++ b/lib/diaspora_federation/parsers/xml_parser.rb @@ -15,14 +15,12 @@ module DiasporaFederation from_xml_sanity_validation(root_node) hash = root_node.element_children.map {|child| - xml_name = child.name - property = entity_type.find_property_for_xml_name(xml_name) + property, type = find_property_for(child.name) if property - type = class_properties[property] - value = parse_element_from_node(xml_name, type, root_node) + value = parse_element_from_node(child.name, type, root_node) [property, value] else - [xml_name, child.text] + [child.name, child.text] end }.to_h @@ -31,6 +29,18 @@ module DiasporaFederation private + def find_property_for(xml_name) + class_properties.find {|name, type| + if type.instance_of?(Symbol) + name.to_s == xml_name + elsif type.instance_of?(Array) + type.first.entity_name == xml_name + elsif type.ancestors.include?(Entity) + type.entity_name == xml_name + end + } + end + # @param [String] name property name to parse # @param [Class, Symbol] type target type to parse # @param [Nokogiri::XML::Element] root_node XML node to parse @@ -53,7 +63,6 @@ module DiasporaFederation # @return [String] data def parse_string_from_node(name, type, root_node) node = root_node.xpath(name.to_s) - node = root_node.xpath(xml_names[name].to_s) if node.empty? parse_string(type, node.first.text) if node.any? end diff --git a/lib/diaspora_federation/properties_dsl.rb b/lib/diaspora_federation/properties_dsl.rb index b473529..b420c14 100644 --- a/lib/diaspora_federation/properties_dsl.rb +++ b/lib/diaspora_federation/properties_dsl.rb @@ -8,7 +8,6 @@ module DiasporaFederation # property :prop # property :optional, default: false # property :dynamic_default, default: -> { Time.now } - # property :another_prop, xml_name: :another_name # property :original_prop, alias: :alias_prop # entity :nested, NestedEntity # entity :multiple, [OtherEntity] @@ -24,7 +23,6 @@ module DiasporaFederation # @param [Hash] opts further options # @option opts [Object, #call] :default a default value, making the # property optional - # @option opts [Symbol] :xml_name another name used for xml generation def property(name, type, opts={}) raise InvalidType unless property_type_valid?(type) @@ -79,49 +77,14 @@ module DiasporaFederation }.to_h 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.keys.find {|name| [name.to_s, xml_names[name].to_s].include?(xml_name) } - end - private - # @deprecated - def determine_xml_name(name, type, opts={}) - if !type.instance_of?(Symbol) && opts.has_key?(:xml_name) - raise ArgumentError, "xml_name is not supported for nested entities" - end - - if type.instance_of?(Symbol) - if opts.has_key? :xml_name - raise InvalidName, "invalid xml_name" unless name_valid?(opts[:xml_name]) - - opts[:xml_name] - else - name - end - elsif type.instance_of?(Array) - type.first.entity_name.to_sym - elsif type.ancestors.include?(Entity) - type.entity_name.to_sym - end - end - def define_property(name, type, opts={}) raise InvalidName unless name_valid?(name) class_props[name] = type optional_props << name if opts[:optional] default_props[name] = opts[:default] if opts.has_key? :default - xml_names[name] = determine_xml_name(name, type, opts) instance_eval { attr_reader name } diff --git a/spec/entities.rb b/spec/entities.rb index 429ea8a..8eab15c 100644 --- a/spec/entities.rb +++ b/spec/entities.rb @@ -28,11 +28,6 @@ module DiasporaFederation entity :multi, [OtherEntity] end - class TestEntityWithXmlName < DiasporaFederation::Entity - property :test, :string - property :qwer, :string, xml_name: :asdf - end - class TestEntityWithRelatedEntity < DiasporaFederation::Entity property :test, :string entity :parent, RelatedEntity diff --git a/spec/lib/diaspora_federation/entities/conversation_spec.rb b/spec/lib/diaspora_federation/entities/conversation_spec.rb index ace9e08..dc3a72c 100644 --- a/spec/lib/diaspora_federation/entities/conversation_spec.rb +++ b/spec/lib/diaspora_federation/entities/conversation_spec.rb @@ -54,7 +54,7 @@ module DiasporaFederation #{parent.guid} #{data[:subject]} #{data[:created_at]} - #{data[:participants]} + #{data[:participants]} XML diff --git a/spec/lib/diaspora_federation/entities/like_spec.rb b/spec/lib/diaspora_federation/entities/like_spec.rb index 307f530..5b02a8b 100644 --- a/spec/lib/diaspora_federation/entities/like_spec.rb +++ b/spec/lib/diaspora_federation/entities/like_spec.rb @@ -75,7 +75,7 @@ module DiasporaFederation it "raises a ValidationError if the parent_guid is missing" do broken_xml = <<~XML - #{parent.entity_type} + #{parent.entity_type} XML diff --git a/spec/lib/diaspora_federation/entities/relayable_spec.rb b/spec/lib/diaspora_federation/entities/relayable_spec.rb index 4daa408..a03744c 100644 --- a/spec/lib/diaspora_federation/entities/relayable_spec.rb +++ b/spec/lib/diaspora_federation/entities/relayable_spec.rb @@ -310,7 +310,7 @@ module DiasporaFederation let(:new_signature_data) { "#{author};#{guid};#{parent_guid};#{new_property};#{property}" } let(:new_xml) { <<~XML } - #{author} + #{author} #{guid} #{parent_guid} #{new_property} diff --git a/spec/lib/diaspora_federation/entity_spec.rb b/spec/lib/diaspora_federation/entity_spec.rb index cd52a27..a349cc9 100644 --- a/spec/lib/diaspora_federation/entity_spec.rb +++ b/spec/lib/diaspora_federation/entity_spec.rb @@ -528,14 +528,5 @@ module DiasporaFederation expect(entity.multi).to be_empty end end - - context "xml_name" do - let(:hash) { {test: "test", qwer: "qwer"} } - - it "should not use the xml_name for the #to_h" do - entity = Entities::TestEntityWithXmlName.new(hash) - expect(entity.to_h).to eq(hash) - end - end end end diff --git a/spec/lib/diaspora_federation/parsers/xml_parser_spec.rb b/spec/lib/diaspora_federation/parsers/xml_parser_spec.rb index da3a661..5923234 100644 --- a/spec/lib/diaspora_federation/parsers/xml_parser_spec.rb +++ b/spec/lib/diaspora_federation/parsers/xml_parser_spec.rb @@ -33,34 +33,6 @@ module DiasporaFederation end end - it "uses xml_name for parsing" do - xml = <<~XML.strip - - asdf - qwer - - XML - - parsed = Parsers::XmlParser.new(Entities::TestEntityWithXmlName).parse(Nokogiri::XML(xml).root) - - expect(parsed[0][:test]).to eq("asdf") - expect(parsed[0][:qwer]).to eq("qwer") - end - - it "allows name for parsing even when property has a xml_name" do - xml = <<~XML.strip - - asdf - qwer - - XML - - parsed = Parsers::XmlParser.new(Entities::TestEntityWithXmlName).parse(Nokogiri::XML(xml).root) - - expect(parsed[0][:test]).to eq("asdf") - expect(parsed[0][:qwer]).to eq("qwer") - end - it "parses the string to the correct type" do xml = <<~XML.strip diff --git a/spec/lib/diaspora_federation/properties_dsl_spec.rb b/spec/lib/diaspora_federation/properties_dsl_spec.rb index f5ff9ee..b758ee2 100644 --- a/spec/lib/diaspora_federation/properties_dsl_spec.rb +++ b/spec/lib/diaspora_federation/properties_dsl_spec.rb @@ -10,7 +10,6 @@ module DiasporaFederation properties = dsl.class_props expect(properties).to have(1).item expect(properties[:test]).to eq(:string) - expect(dsl.xml_names[:test]).to eq(:test) end it "will not accept other types for names" do @@ -46,22 +45,6 @@ module DiasporaFederation expect(properties.keys).to include(:test, :asdf, :zzzz) properties.each_value {|type| expect(type).to eq(:string) } end - - it "can add an xml name to simple properties with a symbol" do - dsl.property :test, :string, xml_name: :xml_test - properties = dsl.class_props - expect(properties).to have(1).item - expect(properties[:test]).to eq(:string) - expect(dsl.xml_names[:test]).to eq(:xml_test) - end - - it "will not accept other types for xml names" do - ["test", 1234, true, {}].each do |val| - expect { - dsl.property :test, :string, xml_name: val - }.to raise_error PropertiesDSL::InvalidName, "invalid xml_name" - end - end end context "nested entities" do @@ -99,12 +82,6 @@ module DiasporaFederation }.to raise_error PropertiesDSL::InvalidType end end - - it "can not add an xml name to a nested entity" do - expect { - dsl.entity :other, Entities::TestEntity, xml_name: :other_name - }.to raise_error ArgumentError, "xml_name is not supported for nested entities" - end end describe ".optional_props" do @@ -174,22 +151,5 @@ module DiasporaFederation expect(data).not_to have_key(:test_alias) end end - - describe ".find_property_for_xml_name" do - it "finds property by xml_name" do - dsl.property :test, :string, xml_name: :xml_test - expect(dsl.find_property_for_xml_name("xml_test")).to eq(:test) - end - - it "finds property by name" do - dsl.property :test, :string, xml_name: :xml_test - expect(dsl.find_property_for_xml_name("test")).to eq(:test) - end - - it "returns nil if property is not defined" do - dsl.property :test, :string, xml_name: :xml_test - expect(dsl.find_property_for_xml_name("unknown")).to be_nil - end - end end end