X-Git-Url: https://git.openstreetmap.org/rails.git/blobdiff_plain/dbe165bbb3d6eda37d9579f1ac767599f9055607..8fe18995964717ff6f88858291c6f464cc40397f:/app/models/relation.rb

diff --git a/app/models/relation.rb b/app/models/relation.rb
index d39fd2796..d244a6d6c 100644
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -33,7 +33,7 @@ class Relation < ActiveRecord::Base
   scope :ways, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Way", :member_id => ids.flatten }) }
   scope :relations, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Relation", :member_id => ids.flatten }) }
 
-  TYPES = %w(node way relation)
+  TYPES = %w(node way relation).freeze
 
   def self.from_xml(xml, create = false)
     p = XML::Parser.string(xml)
@@ -86,13 +86,10 @@ class Relation < ActiveRecord::Base
 
     pt.find("member").each do |member|
       # member_type =
-      logger.debug "each member"
       fail OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member["type"]
-      logger.debug "after raise"
       # member_ref = member['ref']
       # member_role
       member["role"] ||= "" # Allow  the upload to not include this, in which case we default to an empty string.
-      logger.debug member["role"]
       relation.add_member(member["type"].classify, member["ref"], member["role"])
     end
     fail OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil?
@@ -186,10 +183,10 @@ class Relation < ActiveRecord::Base
     # provide repeatable reads for the used-by checks. this means it
     # shouldn't be possible to get race conditions.
     Relation.transaction do
-      self.lock!
+      lock!
       check_consistency(self, new_relation, user)
       # This will check to see if this relation is used by another relation
-      rel = RelationMember.joins(:relation).where("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id).first
+      rel = RelationMember.joins(:relation).find_by("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id)
       fail OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is used in relation #{rel.relation.id}.") unless rel.nil?
 
       self.changeset_id = new_relation.changeset_id
@@ -202,7 +199,7 @@ class Relation < ActiveRecord::Base
 
   def update_from(new_relation, user)
     Relation.transaction do
-      self.lock!
+      lock!
       check_consistency(self, new_relation, user)
       unless new_relation.preconditions_ok?(members)
         fail OSM::APIPreconditionFailedError.new("Cannot update relation #{id}: data or member data is invalid.")
@@ -218,7 +215,7 @@ class Relation < ActiveRecord::Base
 
   def create_with_history(user)
     check_create_consistency(self, user)
-    unless self.preconditions_ok?
+    unless preconditions_ok?
       fail OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.")
     end
     self.version = 0
@@ -249,8 +246,10 @@ class Relation < ActiveRecord::Base
 
       # use reflection to look up the appropriate class
       model = Kernel.const_get(m[0].capitalize)
-      # get the element with that ID
-      element = model.where(:id => m[1]).first
+      # get the element with that ID. and, if found, lock the element to
+      # ensure it can't be deleted until after the current transaction
+      # commits.
+      element = model.lock("for share").find_by(:id => m[1])
 
       # and check that it is OK to use.
       unless element && element.visible? && element.preconditions_ok?
@@ -262,11 +261,6 @@ class Relation < ActiveRecord::Base
     true
   end
 
-  # Temporary method to match interface to nodes
-  def tags_as_hash
-    tags
-  end
-
   ##
   # if any members are referenced by placeholder IDs (i.e: negative) then
   # this calling this method will fix them using the map from placeholders
@@ -295,7 +289,7 @@ class Relation < ActiveRecord::Base
       t = Time.now.getutc
       self.version += 1
       self.timestamp = t
-      self.save!
+      save!
 
       tags = self.tags.clone
       relation_tags.each do |old_tag|
@@ -318,7 +312,7 @@ class Relation < ActiveRecord::Base
       end
       # if there are left-over tags then they are new and will have to
       # be added.
-      tags_changed |= (!tags.empty?)
+      tags_changed |= !tags.empty?
       RelationTag.delete_all(:relation_id => id)
       self.tags.each do |k, v|
         tag = RelationTag.new
@@ -376,7 +370,7 @@ class Relation < ActiveRecord::Base
       # materially change the rest of the relation.
       any_relations =
         changed_members.collect { |_id, type| type == "relation" }
-        .inject(false) { |a, e| a || e }
+                       .inject(false) { |a, e| a || e }
 
       update_members = if tags_changed || any_relations
                          # add all non-relation bounding boxes to the changeset