More refactoring of common code in object models
authorTom Hughes <tom@compton.nu>
Wed, 4 Dec 2013 00:29:48 +0000 (00:29 +0000)
committerTom Hughes <tom@compton.nu>
Wed, 4 Dec 2013 00:29:48 +0000 (00:29 +0000)
Tidy up code and extract generation of tag elements to the
common code in the ObjectMetadata module.

app/models/node.rb
app/models/old_node.rb
app/models/old_relation.rb
app/models/old_way.rb
app/models/relation.rb
app/models/way.rb
lib/object_metadata.rb

index af2505b..9cac983 100644 (file)
@@ -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
index 0d4fd14..3a0ecc3 100644 (file)
@@ -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
index 25f44d8..284efd0 100644 (file)
@@ -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
index 8b9bf19..408dd72 100644 (file)
@@ -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
index b76213c..5207e98 100644 (file)
@@ -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?
index 074ae3b..f9db684 100644 (file)
@@ -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
index d4d0bcf..4243c64 100644 (file)
@@ -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