additional consistency checks. making the error message for the number of nodes use...

diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
index 2f040a92bade1c64892bc6806c3d400f18469eff..8b876d3a79830e6f40cd9073aa03e3a3bb8bbad5 100644 (file)
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@ -116,7 +116,7 @@ class ApiController < ApplicationController
 
     node_ids = @nodes.collect(&:id)
     if node_ids.length > APP_CONFIG['max_number_of_nodes']
 
     node_ids = @nodes.collect(&:id)
     if node_ids.length > APP_CONFIG['max_number_of_nodes']

-      report_error("You requested too many nodes (limit is 50,000). Either request a smaller area, or use planet.osm")
+      report_error("You requested too many nodes (limit is #{APP_CONFIG['max_number_of_nodes']}). Either request a smaller area, or use planet.osm")

       return
     end
     if node_ids.length == 0
       return
     end
     if node_ids.length == 0

diff --git a/app/controllers/way_controller.rb b/app/controllers/way_controller.rb
index 6f4704c770c24fae679d72521fc8c94dac0ca930..08270094d63e87c72677bfd49c9183d0cc08217d 100644 (file)
--- a/app/controllers/way_controller.rb
+++ b/app/controllers/way_controller.rb
@@ -12,6 +12,10 @@ class WayController < ApplicationController
       way = Way.from_xml(request.raw_post, true)
 
       if way
       way = Way.from_xml(request.raw_post, true)
 
       if way

+        # FIXME move some of this to the model. The controller shouldn't need to
+        # know about the fact that the first version number is 0 on creation
+        # it will also allow use to run a variation on the check_consistency
+        # so that we don't get exceptions thrown when the changesets are not right

         unless way.preconditions_ok?
           render :text => "", :status => :precondition_failed
         else
         unless way.preconditions_ok?
           render :text => "", :status => :precondition_failed
         else

diff --git a/lib/geo_record.rb b/lib/geo_record.rb
index 3eab72b2d91c574b1be59db17b318906e2c101c7..2734197573b83ce851ccd2c19394c70f41b2f71b 100644 (file)
--- a/lib/geo_record.rb
+++ b/lib/geo_record.rb
@@ -45,10 +45,16 @@ module GeoRecord
   # 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 extention of the checks without having to modify the
   # 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 extention of the checks without having to modify the

-  # code in 6 places. This will throw an exception if there is an inconsistency
+  # code in 6 places for all the updates and deletes. Some of these tests are 
+  # needed for creates, but are currently not run :-( 
+  # This will throw an exception if there is an inconsistency

   def check_consistency(old, new, user)
     if new.version != old.version
       raise OSM::APIVersionMismatchError.new(new.version, old.version)
   def check_consistency(old, new, user)
     if new.version != old.version
       raise OSM::APIVersionMismatchError.new(new.version, old.version)

+    elsif new.changeset.nil?
+      raise OSM::APIChangesetMissingError.new
+    elsif new.changeset.empty?
+      raise OSM::APIChangesetMissingError.new

     elsif new.changeset.user_id != user.id
       raise OSM::APIUserChangesetMismatchError.new
     elsif not new.changeset.is_open?
     elsif new.changeset.user_id != user.id
       raise OSM::APIUserChangesetMismatchError.new
     elsif not new.changeset.is_open?

diff --git a/lib/osm.rb b/lib/osm.rb
index e0e83845ea4b33ece2dda45f0c0ff3368d73feee..82fc835b4ad8c17e684ae6565604f5bd16976245 100644 (file)
--- a/lib/osm.rb
+++ b/lib/osm.rb
@@ -46,6 +46,13 @@ module OSM
       { :text => "The supplied changeset has already been closed", :status => :conflict }
     end
   end
       { :text => "The supplied changeset has already been closed", :status => :conflict }
     end
   end

+  
+  # Raised when a change is expecting a changeset, but the changeset doesn't exist
+  class APIChangesetMissingError < APIError
+    def render_opts
+      { :text => "You need to supply a changeset to be able to make a change", :status => :conflict }
+    end
+  end

 
   # Raised when the provided version is not equal to the latest in the db.
   class APIVersionMismatchError < APIError
 
   # Raised when the provided version is not equal to the latest in the db.
   class APIVersionMismatchError < APIError

diff --git a/test/functional/way_controller_test.rb b/test/functional/way_controller_test.rb
index b4f4599a407e1857b144e54bee4df7c2313b8281..558e45489cfa9a0f29095b9836136b92cbaae96a 100644 (file)
--- a/test/functional/way_controller_test.rb
+++ b/test/functional/way_controller_test.rb
@@ -109,6 +109,7 @@ class WayControllerTest < Test::Unit::TestCase
   def test_create_invalid
     basic_authorization "test@openstreetmap.org", "test"
 
   def test_create_invalid
     basic_authorization "test@openstreetmap.org", "test"
 

+    # FIXME All of these will fail because they don't have a valid changeset 

     # create a way with non-existing node
     content "<osm><way><nd ref='0'/><tag k='test' v='yes' /></way></osm>"
     put :create
     # create a way with non-existing node
     content "<osm><way><nd ref='0'/><tag k='test' v='yes' /></way></osm>"
     put :create


@@ -137,9 +138,16 @@ class WayControllerTest < Test::Unit::TestCase
     # now set auth
     basic_authorization("test@openstreetmap.org", "test");  
 
     # now set auth
     basic_authorization("test@openstreetmap.org", "test");  
 

-    # this should work
+    # this shouldn't work as with the 0.6 api we need pay load to delete

     delete :delete, :id => current_ways(:visible_way).id
     delete :delete, :id => current_ways(:visible_way).id

-    assert_response :success
+    assert_response :bad_request
+    
+    # Now try without having a changeset
+    content "<osm><way id='#{current_ways(:visible_way).id}'</osm>"
+    delete :delete, :id => current_ways(:visible_way).id
+    assert_response :bad_request
+    
+    # Now try and get a changeset

 
     # this won't work since the way is already deleted
     delete :delete, :id => current_ways(:invisible_way).id
 
     # this won't work since the way is already deleted
     delete :delete, :id => current_ways(:invisible_way).id