Moved transaction boundary to cover used-by tests on deletion so that the database...

[rails.git] / app / models / way.rb

diff --git a/app/models/way.rb b/app/models/way.rb
index 7c29a44b379857574a7327a6b2f458099916bf9a..2071fd559c33ddd136da6608a71606f4698362b7 100644 (file)
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@ -4,9 +4,6 @@ class Way < ActiveRecord::Base
   include ConsistencyValidations
 
   set_table_name 'current_ways'
-
-  validates_presence_of :changeset_id, :timestamp
-  validates_inclusion_of :visible, :in => [ true, false ]
   
   belongs_to :changeset
 
@@ -20,6 +17,14 @@ class Way < ActiveRecord::Base
   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
 
+  validates_presence_of :id, :on => :update
+  validates_presence_of :changeset_id,:version,  :timestamp
+  validates_uniqueness_of :id
+  validates_inclusion_of :visible, :in => [ true, false ]
+  validates_numericality_of :changeset_id, :version, :integer_only => true
+  validates_numericality_of :id, :on => :update, :integer_only => true
+  validates_associated :changeset
+
   def self.from_xml(xml, create=false)
     begin
       p = XML::Parser.new
@@ -27,7 +32,7 @@ class Way < ActiveRecord::Base
       doc = p.parse
 
       doc.find('//osm/way').each do |pt|
-       return Way.from_xml_node(pt, create)
+        return Way.from_xml_node(pt, create)
       end
     rescue
       return nil
@@ -233,6 +238,10 @@ class Way < ActiveRecord::Base
       # update and commit the bounding box, now that way nodes 
       # have been updated and we're in a transaction.
       changeset.update_bbox!(bbox) unless nodes.empty?
+
+      # tell the changeset we updated one element only
+      changeset.add_changes! 1
+
       changeset.save!
     end
   end
@@ -274,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
@@ -286,8 +302,6 @@ class Way < ActiveRecord::Base
         self.visible = false
         self.save_with_history!
       end
-    else
-      raise OSM::APIAlreadyDeletedError
     end
   end