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.

diff --git a/app/models/node.rb b/app/models/node.rb
index af2505b9374c4fa8d568b8dd0ce7780541552ac4..9cac9839b3600b257a94a9feed25a0fcd7b54f00 100644 (file)
--- 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 0d4fd144f8168bdb6021cc681c31e51eede85519..3a0ecc367a5b98f7862bdc7fb91039102db6d431 100644 (file)
--- 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 25f44d8325ce36eaf1b6cdf5e708fea45ee97483..284efd09cf6288d27ef0b7af5d329ad119a244ae 100644 (file)
--- 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 8b9bf19e64bb5b54b9f28bcb8f0520060bd57818..408dd72c7e93dab73f0618c2b6849ad1721b5329 100644 (file)
--- 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 b76213c7a547db17c7057efcaa7f39a43d2c02c9..5207e98d8c766c1a9224f7a17b7c080ce0b60c38 100644 (file)
--- 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 074ae3ba166e0833714164f66da7eda3b07a060e..f9db684b828854729024879d6de642d0369504cc 100644 (file)
--- 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 d4d0bcfc784260c048e5443bfcd444c2efb7aadc..4243c6437de2d297e63405bbe319a198153f0a5b 100644 (file)
--- 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