additional consistency checks. making the error message for the number of nodes use...
authorShaun McDonald <shaun@shaunmcdonald.me.uk>
Sun, 12 Oct 2008 22:32:51 +0000 (22:32 +0000)
committerShaun McDonald <shaun@shaunmcdonald.me.uk>
Sun, 12 Oct 2008 22:32:51 +0000 (22:32 +0000)
app/controllers/api_controller.rb
app/controllers/way_controller.rb
lib/geo_record.rb
lib/osm.rb
test/functional/way_controller_test.rb

index 2f040a9..8b876d3 100644 (file)
@@ -116,7 +116,7 @@ class ApiController < ApplicationController
 
     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
index 6f4704c..0827009 100644 (file)
@@ -12,6 +12,10 @@ class WayController < ApplicationController
       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
index 3eab72b..2734197 100644 (file)
@@ -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
-  # 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)
+    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?
index e0e8384..82fc835 100644 (file)
@@ -46,6 +46,13 @@ module OSM
       { :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
index b4f4599..558e454 100644 (file)
@@ -109,6 +109,7 @@ class WayControllerTest < Test::Unit::TestCase
   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
@@ -137,9 +138,16 @@ class WayControllerTest < Test::Unit::TestCase
     # 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
-    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