Tests for PreconditionFailed error messages.

Change to relation.rb to bring element selection in-line with that of way, so to return nil to be caught later, rather than raise an error.
Change to way.rb to clarify which way on diff upload.

diff --git a/app/models/relation.rb b/app/models/relation.rb
index b66d957b90ae3bdb5827accab6a9ed28f2642f40..7be027559883c1e0f42a8f2b9f7c5a0ee5d49225 100644 (file)
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -298,7 +298,7 @@ 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
         # use reflection to look up the appropriate class
         model = Kernel.const_get(m[0].capitalize)
         # get the element with that ID

-        element = model.find(m[1])
+        element = model.find(:first, :conditions =>["id = ?", m[1]])

 
         # and check that it is OK to use.
         unless element and element.visible? and element.preconditions_ok?
 
         # and check that it is OK to use.
         unless element and element.visible? and element.preconditions_ok?

diff --git a/app/models/way.rb b/app/models/way.rb
index 16c93f608b5ddc3d93b77cdbe419354244194539..40a024b8bb2cd87853dee1ab2133c59328b40976 100644 (file)
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@ -235,7 +235,7 @@ class Way < ActiveRecord::Base
     self.nds.each do |n|
       node = Node.find(:first, :conditions => ["id = ?", n])
       unless node and node.visible
     self.nds.each do |n|
       node = Node.find(:first, :conditions => ["id = ?", n])
       unless node and node.visible

-        raise OSM::APIPreconditionFailedError.new("The node with id #{n} either does not exist, or is not visible")
+        raise OSM::APIPreconditionFailedError.new("Way #{self.id} requires the node with id #{n}, which either does not exist, or is not visible.")

       end
     end
     return true
       end
     end
     return true

diff --git a/test/functional/changeset_controller_test.rb b/test/functional/changeset_controller_test.rb
index a6eeaeeebd7911f02ae1c383a2e355af91bbc671..ce739b96c5cf5c217d8d3d46c9c6c11b2865d507 100644 (file)
--- a/test/functional/changeset_controller_test.rb
+++ b/test/functional/changeset_controller_test.rb
@@ -397,6 +397,7 @@ EOF
     post :upload, :id => 2
     assert_response :precondition_failed, 
       "shouldn't be able to upload a invalid deletion diff: #{@response.body}"
     post :upload, :id => 2
     assert_response :precondition_failed, 
       "shouldn't be able to upload a invalid deletion diff: #{@response.body}"

+    assert_equal "Precondition failed: Way 3 still used by relation 1.", @response.body

 
     # check that nothing was, in fact, deleted
     assert_equal true, Node.find(current_nodes(:node_used_by_relationship).id).visible
 
     # check that nothing was, in fact, deleted
     assert_equal true, Node.find(current_nodes(:node_used_by_relationship).id).visible

diff --git a/test/functional/node_controller_test.rb b/test/functional/node_controller_test.rb
index 6877fc96795ad44ff4b8ee5264801b564792ef46..396a667107ab2ce661792302b761fbf37aec5c95 100644 (file)
--- a/test/functional/node_controller_test.rb
+++ b/test/functional/node_controller_test.rb
@@ -197,12 +197,14 @@ class NodeControllerTest < ActionController::TestCase
     delete :delete, :id => current_nodes(:used_node_1).id
     assert_response :precondition_failed,
        "shouldn't be able to delete a node used in a way (#{@response.body})"
     delete :delete, :id => current_nodes(:used_node_1).id
     assert_response :precondition_failed,
        "shouldn't be able to delete a node used in a way (#{@response.body})"

+    assert_equal "Precondition failed: Node 3 is still used by way 1.", @response.body

 
     # in a relation...
     content(nodes(:node_used_by_relationship).to_xml)
     delete :delete, :id => current_nodes(:node_used_by_relationship).id
     assert_response :precondition_failed,
        "shouldn't be able to delete a node used in a relation (#{@response.body})"
 
     # in a relation...
     content(nodes(:node_used_by_relationship).to_xml)
     delete :delete, :id => current_nodes(:node_used_by_relationship).id
     assert_response :precondition_failed,
        "shouldn't be able to delete a node used in a relation (#{@response.body})"

+    assert_equal "Precondition failed: Node 5 is still used by relation 3.", @response.body

   end
 
   ##
   end
 
   ##

diff --git a/test/functional/relation_controller_test.rb b/test/functional/relation_controller_test.rb
index dc7c64de243480da525acdf6c0c025f42d485406..e51db0b756b23e30f64a4ad611496e28b43316a9 100644 (file)
--- a/test/functional/relation_controller_test.rb
+++ b/test/functional/relation_controller_test.rb
@@ -341,6 +341,7 @@ class RelationControllerTest < ActionController::TestCase
     # expect failure
     assert_response :precondition_failed, 
         "relation upload with invalid node did not return 'precondition failed'"
     # expect failure
     assert_response :precondition_failed, 
         "relation upload with invalid node did not return 'precondition failed'"

+    assert_equal "Precondition failed: Cannot create relation: data or member data is invalid.", @response.body

   end
 
   # -------------------------------------
   end
 
   # -------------------------------------


@@ -465,6 +466,7 @@ class RelationControllerTest < ActionController::TestCase
     delete :delete, :id => current_relations(:public_used_relation).id
     assert_response :precondition_failed, 
        "shouldn't be able to delete a relation used in a relation (#{@response.body})"
     delete :delete, :id => current_relations(:public_used_relation).id
     assert_response :precondition_failed, 
        "shouldn't be able to delete a relation used in a relation (#{@response.body})"

+    assert_equal "Precondition failed: The relation 5 is used in relation 6.", @response.body

 
     # this should work when we provide the appropriate payload...
     content(relations(:multi_tag_relation).to_xml)
 
     # this should work when we provide the appropriate payload...
     content(relations(:multi_tag_relation).to_xml)

diff --git a/test/functional/way_controller_test.rb b/test/functional/way_controller_test.rb
index 9c32ae90400508d7d3654fec1021833fd4ae6627..de23545f7d60b922e96a4ebff0986f3c00e6ed3a 100644 (file)
--- a/test/functional/way_controller_test.rb
+++ b/test/functional/way_controller_test.rb
@@ -164,6 +164,7 @@ class WayControllerTest < ActionController::TestCase
     # expect failure
     assert_response :precondition_failed, 
         "way upload with invalid node did not return 'precondition failed'"
     # expect failure
     assert_response :precondition_failed, 
         "way upload with invalid node did not return 'precondition failed'"

+    assert_equal "Precondition failed: Way  requires the node with id 0, which either does not exist, or is not visible.", @response.body

 
     # create a way with no nodes
     content "<osm><way changeset='#{open_changeset_id}'>" +
 
     # create a way with no nodes
     content "<osm><way changeset='#{open_changeset_id}'>" +


@@ -172,6 +173,7 @@ class WayControllerTest < ActionController::TestCase
     # expect failure
     assert_response :precondition_failed, 
         "way upload with no node did not return 'precondition failed'"
     # expect failure
     assert_response :precondition_failed, 
         "way upload with no node did not return 'precondition failed'"

+    assert_equal "Precondition failed: Cannot create way: data is invalid.", @response.body

 
     # create a way inside a closed changeset
     content "<osm><way changeset='#{closed_changeset_id}'>" +
 
     # create a way inside a closed changeset
     content "<osm><way changeset='#{closed_changeset_id}'>" +


@@ -286,6 +288,7 @@ class WayControllerTest < ActionController::TestCase
     delete :delete, :id => current_ways(:used_way).id
     assert_response :precondition_failed, 
        "shouldn't be able to delete a way used in a relation (#{@response.body})"
     delete :delete, :id => current_ways(:used_way).id
     assert_response :precondition_failed, 
        "shouldn't be able to delete a way used in a relation (#{@response.body})"

+    assert_equal "Precondition failed: Way 3 still used by relation 1.", @response.body

 
     # this won't work since the way never existed
     delete :delete, :id => 0
 
     # this won't work since the way never existed
     delete :delete, :id => 0