From 299b6715221fe2e6d31af5f5711656c8f6e40da0 Mon Sep 17 00:00:00 2001
From: Matt Amos <zerebubuth@gmail.com>
Date: Wed, 26 Nov 2008 14:49:56 +0000
Subject: [PATCH] Moved transaction boundary to cover used-by tests on deletion
 so that the database can help prevent race conditions.

---
 app/models/node.rb     | 15 ++++++++++-----
 app/models/relation.rb | 11 ++++++++---
 app/models/way.rb      | 13 +++++++++----
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/app/models/node.rb b/app/models/node.rb
index f2fe341ae..e1ad818dd 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -155,7 +155,14 @@ class Node < ActiveRecord::Base
 
   # Should probably be renamed delete_from to come in line with update
   def delete_with_history!(new_node, user)
-    if self.visible
+    unless self.visible
+      raise OSM::APIAlreadyDeletedError.new
+    end
+
+    # 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.
+    Node.transaction do
       check_consistency(self, new_node, user)
       if WayNode.find(:first, :joins => "INNER JOIN current_ways ON current_ways.id = current_way_nodes.id", :conditions => [ "current_ways.visible = ? AND current_way_nodes.node_id = ?", true, self.id ])
         raise OSM::APIPreconditionFailedError.new
@@ -164,14 +171,12 @@ class Node < ActiveRecord::Base
       else
         self.changeset_id = new_node.changeset_id
         self.visible = false
-
+        
         # update the changeset with the deleted position
         changeset.update_bbox!(bbox)
-
+        
         save_with_history!
       end
-    else
-      raise OSM::APIAlreadyDeletedError.new
     end
   end
 
diff --git a/app/models/relation.rb b/app/models/relation.rb
index 0159c60b8..0bdacb5ec 100644
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -347,7 +347,14 @@ class Relation < ActiveRecord::Base
   end    
 
   def delete_with_history!(new_relation, user)
-    if self.visible
+    unless self.visible
+      raise OSM::APIAlreadyDeletedError.new
+    end
+
+    # 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
       check_consistency(self, new_relation, user)
       # This will check to see if this relation is used by another relation
       if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = ? AND member_type='relation' and member_id=? ", true, self.id ])
@@ -358,8 +365,6 @@ class Relation < ActiveRecord::Base
       self.members = []
       self.visible = false
       save_with_history!
-    else
-      raise OSM::APIAlreadyDeletedError.new
     end
   end
 
diff --git a/app/models/way.rb b/app/models/way.rb
index e2e1ae302..2071fd559 100644
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@ -283,8 +283,15 @@ class Way < ActiveRecord::Base
   end
 
   def delete_with_history!(new_way, user)
-    check_consistency(self, new_way, user)
-    if self.visible
+    unless self.visible
+      raise OSM::APIAlreadyDeletedError
+    end
+    
+    # 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.
+    Way.transaction do
+      check_consistency(self, new_way, user)
       if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id",
                              :conditions => [ "visible = ? AND member_type='way' and member_id=? ", true, self.id])
         raise OSM::APIPreconditionFailedError
@@ -295,8 +302,6 @@ class Way < ActiveRecord::Base
         self.visible = false
         self.save_with_history!
       end
-    else
-      raise OSM::APIAlreadyDeletedError
     end
   end
 
-- 
2.43.2