Merge remote-tracking branch 'upstream/pull/4640'

app/models/concerns/consistency_validations.rb

@@ -4,32 +4,21 @@ module ConsistencyValidations
   # Generic checks that are run for the updates and deletes of
   # node, ways and relations. This code is here to avoid duplication,
   # and allow the extension of the checks without having to modify the
-  # code in 6 places for all the updates and deletes. Some of these tests are
-  # needed for creates, but are currently not run :-(
+  # code in 6 places for all the updates and deletes.
   # This will throw an exception if there is an inconsistency
-  def check_consistency(old, new, user)
+  def check_update_element_consistency(old, new, user)
     if new.id != old.id || new.id.nil? || old.id.nil?
       raise OSM::APIPreconditionFailedError, "New and old IDs don't match on #{new.class}. #{new.id} != #{old.id}."
     elsif new.version != old.version
       raise OSM::APIVersionMismatchError.new(new.id, new.class.to_s, new.version, old.version)
-    elsif new.changeset.nil?
-      raise OSM::APIChangesetMissingError
-    elsif new.changeset.user_id != user.id
-      raise OSM::APIUserChangesetMismatchError
-    elsif !new.changeset.open?
-      raise OSM::APIChangesetAlreadyClosedError, new.changeset
     end
+
+    check_changeset_consistency(new.changeset, user)
   end
 
   # This is similar to above, just some validations don't apply
-  def check_create_consistency(new, user)
-    if new.changeset.nil?
-      raise OSM::APIChangesetMissingError
-    elsif new.changeset.user_id != user.id
-      raise OSM::APIUserChangesetMismatchError
-    elsif !new.changeset.open?
-      raise OSM::APIChangesetAlreadyClosedError, new.changeset
-    end
+  def check_create_element_consistency(new, user)
+    check_changeset_consistency(new.changeset, user)
   end
 
   ##

app/models/node.rb

@@ -145,7 +145,7 @@ class Node < ApplicationRecord
     # shouldn't be possible to get race conditions.
     Node.transaction do
       lock!
-      check_consistency(self, new_node, user)
+      check_update_element_consistency(self, new_node, user)
       ways = Way.joins(:way_nodes).where(:visible => true, :current_way_nodes => { :node_id => id }).order(:id)
       raise OSM::APIPreconditionFailedError, "Node #{id} is still used by ways #{ways.collect(&:id).join(',')}." unless ways.empty?
 
@@ -166,7 +166,7 @@ class Node < ApplicationRecord
   def update_from(new_node, user)
     Node.transaction do
       lock!
-      check_consistency(self, new_node, user)
+      check_update_element_consistency(self, new_node, user)
 
       # update changeset first
       self.changeset_id = new_node.changeset_id
@@ -189,7 +189,7 @@ class Node < ApplicationRecord
   end
 
   def create_with_history(user)
-    check_create_consistency(self, user)
+    check_create_element_consistency(self, user)
     self.version = 0
     self.visible = true
 

app/models/relation.rb

@@ -166,7 +166,7 @@ class Relation < ApplicationRecord
     # shouldn't be possible to get race conditions.
     Relation.transaction do
       lock!
-      check_consistency(self, new_relation, user)
+      check_update_element_consistency(self, new_relation, user)
       # This will check to see if this relation is used by another relation
       rel = RelationMember.joins(:relation).find_by("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id)
       raise OSM::APIPreconditionFailedError, "The relation #{new_relation.id} is used in relation #{rel.relation.id}." unless rel.nil?
@@ -182,7 +182,7 @@ class Relation < ApplicationRecord
   def update_from(new_relation, user)
     Relation.transaction do
       lock!
-      check_consistency(self, new_relation, user)
+      check_update_element_consistency(self, new_relation, user)
       raise OSM::APIPreconditionFailedError, "Cannot update relation #{id}: data or member data is invalid." unless new_relation.preconditions_ok?(members)
 
       self.changeset_id = new_relation.changeset_id
@@ -195,7 +195,7 @@ class Relation < ApplicationRecord
   end
 
   def create_with_history(user)
-    check_create_consistency(self, user)
+    check_create_element_consistency(self, user)
     raise OSM::APIPreconditionFailedError, "Cannot create relation: data or member data is invalid." unless preconditions_ok?
 
     self.version = 0

app/models/way.rb

@@ -142,7 +142,7 @@ class Way < ApplicationRecord
   def update_from(new_way, user)
     Way.transaction do
       lock!
-      check_consistency(self, new_way, user)
+      check_update_element_consistency(self, new_way, user)
       raise OSM::APIPreconditionFailedError, "Cannot update way #{id}: data is invalid." unless new_way.preconditions_ok?(nds)
 
       self.changeset_id = new_way.changeset_id
@@ -155,7 +155,7 @@ class Way < ApplicationRecord
   end
 
   def create_with_history(user)
-    check_create_consistency(self, user)
+    check_create_element_consistency(self, user)
     raise OSM::APIPreconditionFailedError, "Cannot create way: data is invalid." unless preconditions_ok?
 
     self.version = 0
@@ -193,7 +193,7 @@ class Way < ApplicationRecord
     # shouldn't be possible to get race conditions.
     Way.transaction do
       lock!
-      check_consistency(self, new_way, user)
+      check_update_element_consistency(self, new_way, user)
       rels = Relation.joins(:relation_members).where(:visible => true, :current_relation_members => { :member_type => "Way", :member_id => id }).order(:id)
       raise OSM::APIPreconditionFailedError, "Way #{id} is still used by relations #{rels.collect(&:id).join(',')}." unless rels.empty?
 

test/models/node_test.rb

@@ -362,4 +362,86 @@ class NodeTest < ActiveSupport::TestCase
     assert_equal relation_member2.relation.id, cr.second.id
     assert_equal relation_member3.relation.id, cr.third.id
   end
+
+  test "raises missing changeset exception when creating" do
+    user = create(:user)
+    node = Node.new
+    assert_raises OSM::APIChangesetMissingError do
+      node.create_with_history(user)
+    end
+  end
+
+  test "raises user-changeset mismatch exception when creating" do
+    user = create(:user)
+    changeset = create(:changeset)
+    node = Node.new(:changeset => changeset)
+    assert_raises OSM::APIUserChangesetMismatchError do
+      node.create_with_history(user)
+    end
+  end
+
+  test "raises already closed changeset exception when creating" do
+    user = create(:user)
+    changeset = create(:changeset, :closed, :user => user)
+    node = Node.new(:changeset => changeset)
+    assert_raises OSM::APIChangesetAlreadyClosedError do
+      node.create_with_history(user)
+    end
+  end
+
+  test "raises id precondition exception when updating" do
+    user = create(:user)
+    node = Node.new(:id => 23)
+    new_node = Node.new(:id => 42)
+    assert_raises OSM::APIPreconditionFailedError do
+      node.update_from(new_node, user)
+    end
+  end
+
+  test "raises version mismatch exception when updating" do
+    user = create(:user)
+    node = Node.new(:id => 42, :version => 7)
+    new_node = Node.new(:id => 42, :version => 12)
+    assert_raises OSM::APIVersionMismatchError do
+      node.update_from(new_node, user)
+    end
+  end
+
+  test "raises missing changeset exception when updating" do
+    user = create(:user)
+    node = Node.new(:id => 42, :version => 12)
+    new_node = Node.new(:id => 42, :version => 12)
+    assert_raises OSM::APIChangesetMissingError do
+      node.update_from(new_node, user)
+    end
+  end
+
+  test "raises user-changeset mismatch exception when updating" do
+    user = create(:user)
+    changeset = create(:changeset)
+    node = Node.new(:id => 42, :version => 12)
+    new_node = Node.new(:id => 42, :version => 12, :changeset => changeset)
+    assert_raises OSM::APIUserChangesetMismatchError do
+      node.update_from(new_node, user)
+    end
+  end
+
+  test "raises already closed changeset exception when updating" do
+    user = create(:user)
+    changeset = create(:changeset, :closed, :user => user)
+    node = Node.new(:id => 42, :version => 12)
+    new_node = Node.new(:id => 42, :version => 12, :changeset => changeset)
+    assert_raises OSM::APIChangesetAlreadyClosedError do
+      node.update_from(new_node, user)
+    end
+  end
+
+  test "raises id precondition exception when deleting" do
+    user = create(:user)
+    node = Node.new(:id => 23, :visible => true)
+    new_node = Node.new(:id => 42, :visible => false)
+    assert_raises OSM::APIPreconditionFailedError do
+      node.delete_with_history!(new_node, user)
+    end
+  end
 end

test/models/relation_test.rb

@@ -250,4 +250,86 @@ class RelationTest < ActiveSupport::TestCase
       end
     end
   end
+
+  test "raises missing changeset exception when creating" do
+    user = create(:user)
+    relation = Relation.new
+    assert_raises OSM::APIChangesetMissingError do
+      relation.create_with_history(user)
+    end
+  end
+
+  test "raises user-changeset mismatch exception when creating" do
+    user = create(:user)
+    changeset = create(:changeset)
+    relation = Relation.new(:changeset => changeset)
+    assert_raises OSM::APIUserChangesetMismatchError do
+      relation.create_with_history(user)
+    end
+  end
+
+  test "raises already closed changeset exception when creating" do
+    user = create(:user)
+    changeset = create(:changeset, :closed, :user => user)
+    relation = Relation.new(:changeset => changeset)
+    assert_raises OSM::APIChangesetAlreadyClosedError do
+      relation.create_with_history(user)
+    end
+  end
+
+  test "raises id precondition exception when updating" do
+    user = create(:user)
+    relation = Relation.new(:id => 23)
+    new_relation = Relation.new(:id => 42)
+    assert_raises OSM::APIPreconditionFailedError do
+      relation.update_from(new_relation, user)
+    end
+  end
+
+  test "raises version mismatch exception when updating" do
+    user = create(:user)
+    relation = Relation.new(:id => 42, :version => 7)
+    new_relation = Relation.new(:id => 42, :version => 12)
+    assert_raises OSM::APIVersionMismatchError do
+      relation.update_from(new_relation, user)
+    end
+  end
+
+  test "raises missing changeset exception when updating" do
+    user = create(:user)
+    relation = Relation.new(:id => 42, :version => 12)
+    new_relation = Relation.new(:id => 42, :version => 12)
+    assert_raises OSM::APIChangesetMissingError do
+      relation.update_from(new_relation, user)
+    end
+  end
+
+  test "raises user-changeset mismatch exception when updating" do
+    user = create(:user)
+    changeset = create(:changeset)
+    relation = Relation.new(:id => 42, :version => 12)
+    new_relation = Relation.new(:id => 42, :version => 12, :changeset => changeset)
+    assert_raises OSM::APIUserChangesetMismatchError do
+      relation.update_from(new_relation, user)
+    end
+  end
+
+  test "raises already closed changeset exception when updating" do
+    user = create(:user)
+    changeset = create(:changeset, :closed, :user => user)
+    relation = Relation.new(:id => 42, :version => 12)
+    new_relation = Relation.new(:id => 42, :version => 12, :changeset => changeset)
+    assert_raises OSM::APIChangesetAlreadyClosedError do
+      relation.update_from(new_relation, user)
+    end
+  end
+
+  test "raises id precondition exception when deleting" do
+    user = create(:user)
+    relation = Relation.new(:id => 23, :visible => true)
+    new_relation = Relation.new(:id => 42, :visible => false)
+    assert_raises OSM::APIPreconditionFailedError do
+      relation.delete_with_history!(new_relation, user)
+    end
+  end
 end

test/models/way_test.rb

@@ -217,4 +217,86 @@ class WayTest < ActiveSupport::TestCase
     assert_equal 1, cr.size
     assert_equal relation.id, cr.first.id
   end
+
+  test "raises missing changeset exception when creating" do
+    user = create(:user)
+    way = Way.new
+    assert_raises OSM::APIChangesetMissingError do
+      way.create_with_history(user)
+    end
+  end
+
+  test "raises user-changeset mismatch exception when creating" do
+    user = create(:user)
+    changeset = create(:changeset)
+    way = Way.new(:changeset => changeset)
+    assert_raises OSM::APIUserChangesetMismatchError do
+      way.create_with_history(user)
+    end
+  end
+
+  test "raises already closed changeset exception when creating" do
+    user = create(:user)
+    changeset = create(:changeset, :closed, :user => user)
+    way = Way.new(:changeset => changeset)
+    assert_raises OSM::APIChangesetAlreadyClosedError do
+      way.create_with_history(user)
+    end
+  end
+
+  test "raises id precondition exception when updating" do
+    user = create(:user)
+    way = Way.new(:id => 23)
+    new_way = Way.new(:id => 42)
+    assert_raises OSM::APIPreconditionFailedError do
+      way.update_from(new_way, user)
+    end
+  end
+
+  test "raises version mismatch exception when updating" do
+    user = create(:user)
+    way = Way.new(:id => 42, :version => 7)
+    new_way = Way.new(:id => 42, :version => 12)
+    assert_raises OSM::APIVersionMismatchError do
+      way.update_from(new_way, user)
+    end
+  end
+
+  test "raises missing changeset exception when updating" do
+    user = create(:user)
+    way = Way.new(:id => 42, :version => 12)
+    new_way = Way.new(:id => 42, :version => 12)
+    assert_raises OSM::APIChangesetMissingError do
+      way.update_from(new_way, user)
+    end
+  end
+
+  test "raises user-changeset mismatch exception when updating" do
+    user = create(:user)
+    changeset = create(:changeset)
+    way = Way.new(:id => 42, :version => 12)
+    new_way = Way.new(:id => 42, :version => 12, :changeset => changeset)
+    assert_raises OSM::APIUserChangesetMismatchError do
+      way.update_from(new_way, user)
+    end
+  end
+
+  test "raises already closed changeset exception when updating" do
+    user = create(:user)
+    changeset = create(:changeset, :closed, :user => user)
+    way = Way.new(:id => 42, :version => 12)
+    new_way = Way.new(:id => 42, :version => 12, :changeset => changeset)
+    assert_raises OSM::APIChangesetAlreadyClosedError do
+      way.update_from(new_way, user)
+    end
+  end
+
+  test "raises id precondition exception when deleting" do
+    user = create(:user)
+    way = Way.new(:id => 23, :visible => true)
+    new_way = Way.new(:id => 42, :visible => false)
+    assert_raises OSM::APIPreconditionFailedError do
+      way.delete_with_history!(new_way, user)
+    end
+  end
 end