From 9544ab12b8237b4e471f03df981a189c80216c63 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Wed, 4 Dec 2013 00:29:48 +0000 Subject: [PATCH] More refactoring of common code in object models Tidy up code and extract generation of tag elements to the common code in the ObjectMetadata module. --- app/models/node.rb | 20 ++++++++------------ app/models/old_node.rb | 24 +++++++++++------------- app/models/old_relation.rb | 30 +++++++++++++----------------- app/models/old_way.rb | 26 +++++++++++--------------- app/models/relation.rb | 27 ++++++++++++--------------- app/models/way.rb | 23 ++++++++++------------- lib/object_metadata.rb | 11 +++++++++++ 7 files changed, 76 insertions(+), 85 deletions(-) diff --git a/app/models/node.rb b/app/models/node.rb index af2505b93..9cac9839b 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -176,23 +176,19 @@ class Node < ActiveRecord::Base end def to_xml_node(changeset_cache = {}, user_display_name_cache = {}) - el1 = XML::Node.new 'node' - el1['id'] = self.id.to_s - add_metadata_to_xml_node(el1, self, changeset_cache, user_display_name_cache) + el = XML::Node.new 'node' + el['id'] = self.id.to_s + + add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache) if self.visible? - el1['lat'] = self.lat.to_s - el1['lon'] = self.lon.to_s + el['lat'] = self.lat.to_s + el['lon'] = self.lon.to_s end - self.tags.each do |k,v| - el2 = XML::Node.new('tag') - el2['k'] = k.to_s - el2['v'] = v.to_s - el1 << el2 - end + add_tags_to_xml_node(el, self.node_tags) - return el1 + return el end def tags_as_hash diff --git a/app/models/old_node.rb b/app/models/old_node.rb index 0d4fd144f..3a0ecc367 100644 --- a/app/models/old_node.rb +++ b/app/models/old_node.rb @@ -20,6 +20,8 @@ class OldNode < ActiveRecord::Base belongs_to :redaction belongs_to :current_node, :class_name => "Node", :foreign_key => "node_id" + has_many :old_tags, :class_name => 'OldNodeTag', :foreign_key => [:node_id, :version] + def validate_position errors.add(:base, "Node is not in the world") unless in_world? end @@ -44,23 +46,19 @@ class OldNode < ActiveRecord::Base end def to_xml_node(changeset_cache = {}, user_display_name_cache = {}) - el1 = XML::Node.new 'node' - el1['id'] = self.node_id.to_s - add_metadata_to_xml_node(el1, self, changeset_cache, user_display_name_cache) + el = XML::Node.new 'node' + el['id'] = self.node_id.to_s + + add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache) if self.visible? - el1['lat'] = self.lat.to_s - el1['lon'] = self.lon.to_s + el['lat'] = self.lat.to_s + el['lon'] = self.lon.to_s end - self.tags.each do |k,v| - el2 = XML::Node.new('tag') - el2['k'] = k.to_s - el2['v'] = v.to_s - el1 << el2 - end + add_tags_to_xml_node(el, self.old_tags) - return el1 + return el end def save_with_dependencies! @@ -84,7 +82,7 @@ class OldNode < ActiveRecord::Base def tags unless @tags @tags = Hash.new - OldNodeTag.where(:node_id => self.node_id, :version => self.version).each do |tag| + self.old_tags.each do |tag| @tags[tag.k] = tag.v end end diff --git a/app/models/old_relation.rb b/app/models/old_relation.rb index 25f44d832..284efd09c 100644 --- a/app/models/old_relation.rb +++ b/app/models/old_relation.rb @@ -62,7 +62,7 @@ class OldRelation < ActiveRecord::Base def members unless @members @members = Array.new - OldRelationMember.where(:relation_id => self.relation_id, :version => self.version).order(:sequence_id).each do |m| + self.old_members.order(:sequence_id).each do |m| @members += [[m.type,m.id,m.role]] end end @@ -72,7 +72,7 @@ class OldRelation < ActiveRecord::Base def tags unless @tags @tags = Hash.new - OldRelationTag.where(:relation_id => self.relation_id, :version => self.version).each do |tag| + self.tags.each do |tag| @tags[tag.k] = tag.v end end @@ -95,26 +95,22 @@ class OldRelation < ActiveRecord::Base end def to_xml_node(changeset_cache = {}, user_display_name_cache = {}) - el1 = XML::Node.new 'relation' - el1['id'] = self.relation_id.to_s - add_metadata_to_xml_node(el1, self, changeset_cache, user_display_name_cache) + el = XML::Node.new 'relation' + el['id'] = self.relation_id.to_s + + add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache) self.old_members.each do |member| - e = XML::Node.new 'member' - e['type'] = member.member_type.to_s.downcase - e['ref'] = member.member_id.to_s # "id" is considered uncool here as it should be unique in XML - e['role'] = member.member_role.to_s - el1 << e + member_el = XML::Node.new 'member' + member_el['type'] = member.member_type.to_s.downcase + member_el['ref'] = member.member_id.to_s # "id" is considered uncool here as it should be unique in XML + member_el['role'] = member.member_role.to_s + el << member_el end - self.old_tags.each do |tag| - e = XML::Node.new 'tag' - e['k'] = tag.k - e['v'] = tag.v - el1 << e - end + add_tags_to_xml_node(el, self.old_tags) - return el1 + return el end # Temporary method to match interface to nodes diff --git a/app/models/old_way.rb b/app/models/old_way.rb index 8b9bf19e6..408dd72c7 100644 --- a/app/models/old_way.rb +++ b/app/models/old_way.rb @@ -67,7 +67,7 @@ class OldWay < ActiveRecord::Base def nds unless @nds @nds = Array.new - OldWayNode.where(:way_id => self.way_id, :version => self.version).order(:sequence_id).each do |nd| + self.old_nodes.order(:sequence_id).each do |nd| @nds += [nd.node_id] end end @@ -77,7 +77,7 @@ class OldWay < ActiveRecord::Base def tags unless @tags @tags = Hash.new - OldWayTag.where(:way_id => self.way_id, :version => self.version).each do |tag| + self.old_tags.each do |tag| @tags[tag.k] = tag.v end end @@ -94,24 +94,20 @@ class OldWay < ActiveRecord::Base end def to_xml_node(changeset_cache = {}, user_display_name_cache = {}) - el1 = XML::Node.new 'way' - el1['id'] = self.way_id.to_s - add_metadata_to_xml_node(el1, self, changeset_cache, user_display_name_cache) + el = XML::Node.new 'way' + el['id'] = self.way_id.to_s + + add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache) self.old_nodes.each do |nd| # FIXME need to make sure they come back in the right order - e = XML::Node.new 'nd' - e['ref'] = nd.node_id.to_s - el1 << e + node_el = XML::Node.new 'nd' + node_el['ref'] = nd.node_id.to_s + el << node_el end - self.old_tags.each do |tag| - e = XML::Node.new 'tag' - e['k'] = tag.k - e['v'] = tag.v - el1 << e - end + add_tags_to_xml_node(el, self.old_tags) - return el1 + return el end # Read full version of old way diff --git a/app/models/relation.rb b/app/models/relation.rb index b76213c7a..5207e98d8 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -107,9 +107,10 @@ class Relation < ActiveRecord::Base end def to_xml_node(visible_members = nil, changeset_cache = {}, user_display_name_cache = {}) - el1 = XML::Node.new 'relation' - el1['id'] = self.id.to_s - add_metadata_to_xml_node(el1, self, changeset_cache, user_display_name_cache) + el = XML::Node.new 'relation' + el['id'] = self.id.to_s + + add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache) self.relation_members.each do |member| p=0 @@ -125,21 +126,17 @@ class Relation < ActiveRecord::Base end end if p - e = XML::Node.new 'member' - e['type'] = member.member_type.downcase - e['ref'] = member.member_id.to_s - e['role'] = member.member_role - el1 << e + member_el = XML::Node.new 'member' + member_el['type'] = member.member_type.downcase + member_el['ref'] = member.member_id.to_s + member_el['role'] = member.member_role + el << member_el end end - self.relation_tags.each do |tag| - e = XML::Node.new 'tag' - e['k'] = tag.k - e['v'] = tag.v - el1 << e - end - return el1 + add_tags_to_xml_node(el, self.relation_tags) + + return el end # FIXME is this really needed? diff --git a/app/models/way.rb b/app/models/way.rb index 074ae3ba1..f9db684b8 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -101,9 +101,10 @@ class Way < ActiveRecord::Base end def to_xml_node(visible_nodes = nil, changeset_cache = {}, user_display_name_cache = {}) - el1 = XML::Node.new 'way' - el1['id'] = self.id.to_s - add_metadata_to_xml_node(el1, self, changeset_cache, user_display_name_cache) + el = XML::Node.new 'way' + el['id'] = self.id.to_s + + add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache) # make sure nodes are output in sequence_id order ordered_nodes = [] @@ -123,19 +124,15 @@ class Way < ActiveRecord::Base ordered_nodes.each do |nd_id| if nd_id and nd_id != '0' - e = XML::Node.new 'nd' - e['ref'] = nd_id - el1 << e + node_el = XML::Node.new 'nd' + node_el['ref'] = nd_id + el << node_el end end - self.way_tags.each do |tag| - e = XML::Node.new 'tag' - e['k'] = tag.k - e['v'] = tag.v - el1 << e - end - return el1 + add_tags_to_xml_node(el, self.way_tags) + + return el end def nds diff --git a/lib/object_metadata.rb b/lib/object_metadata.rb index d4d0bcfc7..4243c6437 100644 --- a/lib/object_metadata.rb +++ b/lib/object_metadata.rb @@ -27,4 +27,15 @@ module ObjectMetadata el['uid'] = user_id.to_s end end + + def add_tags_to_xml_node(el, tags) + tags.each do |tag| + tag_el = XML::Node.new('tag') + + tag_el['k'] = tag.k + tag_el['v'] = tag.v + + el << tag_el + end + end end -- 2.43.2