From 3e9ceb0c3816a1e13c82914d16abf2997b482751 Mon Sep 17 00:00:00 2001 From: Shaun McDonald Date: Sun, 19 Oct 2008 07:09:04 +0000 Subject: [PATCH] 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 | 10 ++-- config/environment.rb | 3 ++ config/routes.rb | 2 + lib/osm.rb | 2 +- test/functional/api_controller_test.rb | 65 ++++++++++++++++++++++++-- 5 files changed, 75 insertions(+), 7 deletions(-) diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 8b876d3a7..f7c9f591f 100644 --- a/app/controllers/api_controller.rb +++ b/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 diff --git a/config/environment.rb b/config/environment.rb index 08c433788..e42e87eb7 100644 --- a/config/environment.rb +++ b/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' diff --git a/config/routes.rb b/config/routes.rb index db59a28c8..5b2ca38b9 100644 --- a/config/routes.rb +++ b/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+/ diff --git a/lib/osm.rb b/lib/osm.rb index ae8b81f5b..e9d3c9464 100644 --- a/lib/osm.rb +++ b/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 diff --git a/test/functional/api_controller_test.rb b/test/functional/api_controller_test.rb index 4c4787ff7..c722bbf60 100644 --- a/test/functional/api_controller_test.rb +++ b/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 -- 2.43.2