X-Git-Url: https://git.openstreetmap.org/rails.git/blobdiff_plain/413baecf6b27056cb73e6bb4bedb33de6410c65e..34e3e51456774127d43408b7ab65c24f41373f62:/app/models/relation.rb

diff --git a/app/models/relation.rb b/app/models/relation.rb
index b76213c7a..464766802 100644
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -1,6 +1,6 @@
 class Relation < ActiveRecord::Base
   require 'xml/libxml'
-  
+
   include ConsistencyValidations
   include NotRedactable
   include ObjectMetadata
@@ -15,16 +15,16 @@ class Relation < ActiveRecord::Base
   has_many :relation_tags
 
   has_many :containing_relation_members, :class_name => "RelationMember", :as => :member
-  has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation, :extend => ObjectFinder
+  has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation
 
   validates_presence_of :id, :on => :update
-  validates_presence_of :timestamp,:version,  :changeset_id 
+  validates_presence_of :timestamp,:version,  :changeset_id
   validates_uniqueness_of :id
   validates_inclusion_of :visible, :in => [ true, false ]
   validates_numericality_of :id, :on => :update, :integer_only => true
   validates_numericality_of :changeset_id, :version, :integer_only => true
   validates_associated :changeset
-  
+
   scope :visible, -> { where(:visible => true) }
   scope :invisible, -> { where(:visible => false) }
   scope :nodes, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Node", :member_id => ids.flatten }) }
@@ -54,17 +54,17 @@ class Relation < ActiveRecord::Base
     relation.version = pt['version']
     raise OSM::APIBadXMLError.new("relation", pt, "Changeset id is missing") if pt['changeset'].nil?
     relation.changeset_id = pt['changeset']
-    
+
     unless create
       raise OSM::APIBadXMLError.new("relation", pt, "ID is required when updating") if pt['id'].nil?
       relation.id = pt['id'].to_i
-      # .to_i will return 0 if there is no number that can be parsed. 
+      # .to_i will return 0 if there is no number that can be parsed.
       # We want to make sure that there is no id with zero anyway
       raise OSM::APIBadUserInput.new("ID of relation cannot be zero when updating.") if relation.id == 0
     end
-    
+
     # We don't care about the timestamp nor the visibility as these are either
-    # set explicitly or implicit in the action. The visibility is set to true, 
+    # set explicitly or implicit in the action. The visibility is set to true,
     # and manually set to false before the actual delete.
     relation.visible = true
 
@@ -79,13 +79,13 @@ class Relation < ActiveRecord::Base
     end
 
     # need to initialise the relation members array explicitly, as if this
-    # isn't done for a new relation then @members attribute will be nil, 
-    # and the members will be loaded from the database instead of being 
+    # isn't done for a new relation then @members attribute will be nil,
+    # and the members will be loaded from the database instead of being
     # empty, as intended.
     relation.members = Array.new
 
     pt.find('member').each do |member|
-      #member_type = 
+      #member_type =
       logger.debug "each member"
       raise OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member['type']
       logger.debug "after raise"
@@ -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,22 +126,18 @@ 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
-  end 
+    add_tags_to_xml_node(el, self.relation_tags)
+
+    return el
+  end
 
   # FIXME is this really needed?
   def members
@@ -150,13 +147,7 @@ class Relation < ActiveRecord::Base
   end
 
   def tags
-    unless @tags
-      @tags = Hash.new
-      self.relation_tags.each do |tag|
-        @tags[tag.k] = tag.v
-      end
-    end
-    @tags
+    @tags ||= Hash[self.relation_tags.collect { |t| [t.k, t.v] }]
   end
 
   def members=(m)
@@ -183,21 +174,21 @@ class Relation < ActiveRecord::Base
   end
 
   ##
-  # updates the changeset bounding box to contain the bounding box of 
+  # updates the changeset bounding box to contain the bounding box of
   # the element with given +type+ and +id+. this only works with nodes
   # and ways at the moment, as they're the only elements to respond to
   # the :bbox call.
   def update_changeset_element(type, id)
     element = Kernel.const_get(type.capitalize).find(id)
     changeset.update_bbox! element.bbox
-  end    
+  end
 
   def delete_with_history!(new_relation, user)
     unless self.visible
       raise OSM::APIAlreadyDeletedError.new("relation", new_relation.id)
     end
 
-    # need to start the transaction here, so that the database can 
+    # need to start the transaction here, so that the database can
     # provide repeatable reads for the used-by checks. this means it
     # shouldn't be possible to get race conditions.
     Relation.transaction do
@@ -230,7 +221,7 @@ class Relation < ActiveRecord::Base
       save_with_history!
     end
   end
-  
+
   def create_with_history(user)
     check_create_consistency(self, user)
     unless self.preconditions_ok?
@@ -242,9 +233,9 @@ class Relation < ActiveRecord::Base
   end
 
   def preconditions_ok?(good_members = [])
-    # These are hastables that store an id in the index of all 
+    # These are hastables that store an id in the index of all
     # the nodes/way/relations that have already been added.
-    # If the member is valid and visible then we add it to the 
+    # If the member is valid and visible then we add it to the
     # relevant hash table, with the value true as a cache.
     # Thus if you have nodes with the ids of 50 and 1 already in the
     # relation, then the hash table nodes would contain:
@@ -282,8 +273,8 @@ class Relation < ActiveRecord::Base
 
   ##
   # if any members are referenced by placeholder IDs (i.e: negative) then
-  # this calling this method will fix them using the map from placeholders 
-  # to IDs +id_map+. 
+  # this calling this method will fix them using the map from placeholders
+  # to IDs +id_map+.
   def fix_placeholders!(id_map, placeholder_id = nil)
     self.members.map! do |type, id, role|
       old_id = id.to_i
@@ -298,7 +289,7 @@ class Relation < ActiveRecord::Base
   end
 
   private
-  
+
   def save_with_history!
     Relation.transaction do
       # have to be a little bit clever here - to detect if any tags
@@ -315,9 +306,9 @@ class Relation < ActiveRecord::Base
         key = old_tag.k
         # if we can match the tags we currently have to the list
         # of old tags, then we never set the tags_changed flag. but
-        # if any are different then set the flag and do the DB 
+        # if any are different then set the flag and do the DB
         # update.
-        if tags.has_key? key 
+        if tags.has_key? key
           tags_changed |= (old_tag.v != tags[key])
 
           # remove from the map, so that we can expect an empty map
@@ -340,7 +331,7 @@ class Relation < ActiveRecord::Base
         tag.v = v
         tag.save!
       end
-      
+
       # same pattern as before, but this time we're collecting the
       # changed members in an array, as the bounding box updates for
       # elements are per-element, not blanked on/off like for tags.
@@ -387,7 +378,7 @@ class Relation < ActiveRecord::Base
       # bounding box. this is similar to how the map call does things and is
       # reasonable on the assumption that adding or removing members doesn't
       # materially change the rest of the relation.
-      any_relations = 
+      any_relations =
         changed_members.collect { |id,type| type == "relation" }.
         inject(false) { |b,s| b or s }
 
@@ -396,7 +387,7 @@ class Relation < ActiveRecord::Base
                          # FIXME: check for tag changes along with element deletions and
                          # make sure that the deleted element's bounding box is hit.
                          self.members
-                       else 
+                       else
                          changed_members
                        end
       update_members.each do |type, id, role|