more testing of the api. Changing the generator so that it is a constant to come in line with the server url and the api version. Adding tracepoints per page to the capabilities api call. Better error message for api call.

app/controllers/api_controller.rb

@@ -11,10 +11,10 @@ class ApiController < ApplicationController
   @@count = COUNT
 
   # The maximum area you're allowed to request, in square degrees
-  MAX_REQUEST_AREA = 0.25
+  MAX_REQUEST_AREA = APP_CONFIG['max_request_area']
 
   # Number of GPS trace/trackpoints returned per-page
-  TRACEPOINTS_PER_PAGE = 5000
+  TRACEPOINTS_PER_PAGE = APP_CONFIG['tracepoints_per_page']
 
   
   def trackpoints
@@ -109,6 +109,7 @@ class ApiController < ApplicationController
       return
     end
 
+    # FIXME um why is this area using a different order for the lat/lon from above???
     @nodes = Node.find_by_area(min_lat, min_lon, max_lat, max_lon, :conditions => "visible = 1", :limit => APP_CONFIG['max_number_of_nodes']+1)
     # get all the nodes, by tag not yet working, waiting for change from NickB
     # need to be @nodes (instance var) so tests in /spec can be performed
@@ -245,7 +246,7 @@ class ApiController < ApplicationController
 
       render :text => doc.to_s, :content_type => "text/xml"
     else
-      render :nothing => true, :status => :bad_request
+      render :text => "Requested zoom is invalid", :status => :bad_request
     end
   end
 
@@ -260,6 +261,9 @@ class ApiController < ApplicationController
     area = XML::Node.new 'area'
     area['maximum'] = MAX_REQUEST_AREA.to_s;
     api << area
+    tracepoints = XML::Node.new 'tracepoints'
+    tracepoints['per_page'] = APP_CONFIG['tracepoints_per_page'].to_s
+    api << tracepoints
     
     doc.root << api
 

config/environment.rb

@@ -10,6 +10,9 @@ RAILS_GEM_VERSION = '2.0.2' unless defined? RAILS_GEM_VERSION
 # Set the server URL
 SERVER_URL = ENV['OSM_SERVER_URL'] || 'www.openstreetmap.org'
 
+# Set the generator
+GENERATOR = ENV['OSM_SERVER_GENERATOR'] || 'OpenStreetMap server'
+
 # Application constants needed for routes.rb - must go before Initializer call
 API_VERSION = ENV['OSM_API_VERSION'] || '0.6'
 

config/routes.rb

@@ -25,6 +25,8 @@ ActionController::Routing::Routes.draw do |map|
   map.connect "api/#{API_VERSION}/way/:id", :controller => 'way', :action => 'delete', :id => /\d+/, :conditions => { :method => :delete }
   map.connect "api/#{API_VERSION}/ways", :controller => 'way', :action => 'ways', :id => nil
 
+  # FIXME Wouldn't capabilites be better placed somewhere else in this file
+  # and without the #{API_VERSION}, so that clients can always find it?
   map.connect "api/#{API_VERSION}/capabilities", :controller => 'api', :action => 'capabilities'
   map.connect "api/#{API_VERSION}/relation/create", :controller => 'relation', :action => 'create'
   map.connect "api/#{API_VERSION}/relation/:id/relations", :controller => 'relation', :action => 'relations_for_relation', :id => /\d+/

lib/osm.rb

@@ -249,7 +249,7 @@ module OSM
       doc.encoding = 'UTF-8' 
       root = XML::Node.new 'osm'
       root['version'] = API_VERSION
-      root['generator'] = 'OpenStreetMap server'
+      root['generator'] = GENERATOR
       doc.root = root
       return doc
     end

test/functional/api_controller_test.rb

@@ -23,12 +23,71 @@ class ApiControllerTest < Test::Unit::TestCase
 
   def test_map
     node = current_nodes(:used_node_1)
-    bbox = "#{node.lat-0.1},#{node.lon-0.1},#{node.lat+0.1},#{node.lon+0.1}"
+    # Need to split the min/max lat/lon out into their own variables here
+    # so that we can test they are returned later.
+    minlon = node.lon-0.1
+    minlat = node.lat-0.1
+    maxlon = node.lon+0.1
+    maxlat = node.lat+0.1
+    bbox = "#{minlon},#{minlat},#{maxlon},#{maxlat}"
     get :map, :bbox => bbox
     if $VERBOSE
-        print @response.body
+      print @request.to_yaml
+      print @response.body
     end
     assert_response :success
+    assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']:root", :count => 1 do
+      assert_select "bounds[minlon=#{minlon}][minlat=#{minlat}][maxlon=#{maxlon}][maxlat=#{maxlat}]", :count => 1
+      assert_select "node[id=#{node.id}][lat=#{node.lat}][lon=#{node.lon}][version=#{node.version}][changeset=#{node.changeset_id}][visible=#{node.visible}][timestamp=#{node.timestamp.xmlschema}]", :count => 1 do
+        # This should really be more generic
+        assert_select "tag[k=test][v=1]"
+      end
+      # Should also test for the ways and relation
+    end
   end
   
+  def test_map_without_bbox
+    get :map
+    assert_response :bad_request
+    assert_equal "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat", @response.body
+  end
+  
+  def test_traces_without_bbox
+    get :trackpoints
+    assert_response :bad_request
+    assert_equal "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat", @response.body
+  end
+  
+  def test_traces_page_less_than_0
+    -10.upto(-1) do |i|
+      get :trackpoints, :page => i, :bbox => "-0.1,-0.1,0.1,0.1"
+      assert_response :bad_request
+      assert_equal "Page number must be greater than or equal to 0", @response.body
+    end
+    0.upto(10) do |i|
+      get :trackpoints, :page => i, :bbox => "-0.1,-0.1,0.1,0.1"
+      assert_response :success
+    end
+  end
+  
+  def test_traces_bbox_too_big
+    bad = %w{ -0.1,-0.1,1.1,1.1 10,10,11,11 }
+    bad.each do |bbox|
+      get :trackpoints, :bbox => bbox
+      assert_response :bad_request
+      assert_equal "The maximum bbox size is #{APP_CONFIG['max_request_area']}, and your request was too large. Either request a smaller area, or use planet.osm", @response.body
+    end
+  end
+  
+  def test_capabilities
+    get :capabilities
+    assert_response :success
+    assert_select "osm:root[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
+      assert_select "api", :count => 1 do
+        assert_select "version[minimum=#{API_VERSION}][maximum=#{API_VERSION}]", :count => 1
+        assert_select "area[maximum=#{APP_CONFIG['max_request_area']}]", :count => 1
+        assert_select "tracepoints[per_page=#{APP_CONFIG['tracepoints_per_page']}]", :count => 1
+      end
+    end
+  end
 end