diff --git a/app/models/concerns/consistency_validations.rb b/app/models/concerns/consistency_validations.rb index 101fd43103..2df85f2ac1 100644 --- a/app/models/concerns/consistency_validations.rb +++ b/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 ## diff --git a/app/models/node.rb b/app/models/node.rb index ad4318487b..825336d16e 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -145,7 +145,7 @@ def delete_with_history!(new_node, user) # 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 @@ def delete_with_history!(new_node, user) 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 @@ def update_from(new_node, user) end def create_with_history(user) - check_create_consistency(self, user) + check_create_element_consistency(self, user) self.version = 0 self.visible = true diff --git a/app/models/relation.rb b/app/models/relation.rb index 0a4a660a6f..f096473200 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -166,7 +166,7 @@ def delete_with_history!(new_relation, user) # 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 @@ def delete_with_history!(new_relation, user) 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 @@ def update_from(new_relation, user) 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 diff --git a/app/models/way.rb b/app/models/way.rb index b11c225ddb..203d3b7036 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -142,7 +142,7 @@ def bbox 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 @@ def update_from(new_way, user) 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 @@ def delete_with_history!(new_way, user) # 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? diff --git a/test/models/node_test.rb b/test/models/node_test.rb index ee0a77649e..94cb5ec814 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -362,4 +362,86 @@ def test_containing_relations 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 diff --git a/test/models/relation_test.rb b/test/models/relation_test.rb index 575813ad53..405dd353d3 100644 --- a/test/models/relation_test.rb +++ b/test/models/relation_test.rb @@ -250,4 +250,86 @@ def test_max_members_per_relation_limit 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 diff --git a/test/models/way_test.rb b/test/models/way_test.rb index 8674b37904..36debfac08 100644 --- a/test/models/way_test.rb +++ b/test/models/way_test.rb @@ -217,4 +217,86 @@ def test_containing_relations 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