From: Andy Allan <git@gravitystorm.co.uk>
Date: Sun, 24 Feb 2019 09:42:01 +0000 (+0100)
Subject: Move map method to its own controller
X-Git-Tag: live~2694^2
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/f4e29905263efbb2eafdedad6a5c050ba559efd8?hp=d887252eeb49f905a979c21f2559c85d9a0455bd

Move map method to its own controller
---

diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb
index 2ba714990..5e8ef997f 100644
--- a/app/abilities/ability.rb
+++ b/app/abilities/ability.rb
@@ -4,7 +4,6 @@ class Ability
   include CanCan::Ability
 
   def initialize(user)
-    can [:map, :changes], :api
     can [:relation, :relation_history, :way, :way_history, :node, :node_history,
          :changeset, :note, :new_note, :query], :browse
     can :show, :capability
@@ -17,6 +16,7 @@ class Ability
     can [:finish, :embed], :export
     can [:search, :search_latlon, :search_ca_postcode, :search_osm_nominatim,
          :search_geonames, :search_osm_nominatim_reverse, :search_geonames_reverse], :geocoder
+    can :index, :map
     can [:index, :create, :comment, :feed, :show, :search, :mine], Note
     can [:token, :request_token, :access_token, :test_request], :oauth
     can :show, :permission
diff --git a/app/controllers/api/map_controller.rb b/app/controllers/api/map_controller.rb
new file mode 100644
index 000000000..47c0aeb36
--- /dev/null
+++ b/app/controllers/api/map_controller.rb
@@ -0,0 +1,107 @@
+module Api
+  class MapController < ApplicationController
+    skip_before_action :verify_authenticity_token
+    before_action :api_deny_access_handler
+
+    authorize_resource :class => false
+
+    before_action :check_api_readable
+    around_action :api_call_handle_error, :api_call_timeout
+
+    # This is probably the most common call of all. It is used for getting the
+    # OSM data for a specified bounding box, usually for editing. First the
+    # bounding box (bbox) is checked to make sure that it is sane. All nodes
+    # are searched, then all the ways that reference those nodes are found.
+    # All Nodes that are referenced by those ways are fetched and added to the list
+    # of nodes.
+    # Then all the relations that reference the already found nodes and ways are
+    # fetched. All the nodes and ways that are referenced by those ways are then
+    # fetched. Finally all the xml is returned.
+    def index
+      # Figure out the bbox
+      # check boundary is sane and area within defined
+      # see /config/application.yml
+      begin
+        bbox = BoundingBox.from_bbox_params(params)
+        bbox.check_boundaries
+        bbox.check_size
+      rescue StandardError => err
+        report_error(err.message)
+        return
+      end
+
+      nodes = Node.bbox(bbox).where(:visible => true).includes(:node_tags).limit(MAX_NUMBER_OF_NODES + 1)
+
+      node_ids = nodes.collect(&:id)
+      if node_ids.length > MAX_NUMBER_OF_NODES
+        report_error("You requested too many nodes (limit is #{MAX_NUMBER_OF_NODES}). Either request a smaller area, or use planet.osm")
+        return
+      end
+
+      doc = OSM::API.new.get_xml_doc
+
+      # add bounds
+      doc.root << bbox.add_bounds_to(XML::Node.new("bounds"))
+
+      # get ways
+      # find which ways are needed
+      ways = []
+      if node_ids.empty?
+        list_of_way_nodes = []
+      else
+        way_nodes = WayNode.where(:node_id => node_ids)
+        way_ids = way_nodes.collect { |way_node| way_node.id[0] }
+        ways = Way.preload(:way_nodes, :way_tags).find(way_ids)
+
+        list_of_way_nodes = ways.collect do |way|
+          way.way_nodes.collect(&:node_id)
+        end
+        list_of_way_nodes.flatten!
+      end
+
+      # - [0] in case some thing links to node 0 which doesn't exist. Shouldn't actually ever happen but it does. FIXME: file a ticket for this
+      nodes_to_fetch = (list_of_way_nodes.uniq - node_ids) - [0]
+
+      nodes += Node.includes(:node_tags).find(nodes_to_fetch) unless nodes_to_fetch.empty?
+
+      visible_nodes = {}
+      changeset_cache = {}
+      user_display_name_cache = {}
+
+      nodes.each do |node|
+        if node.visible?
+          doc.root << node.to_xml_node(changeset_cache, user_display_name_cache)
+          visible_nodes[node.id] = node
+        end
+      end
+
+      way_ids = []
+      ways.each do |way|
+        if way.visible?
+          doc.root << way.to_xml_node(visible_nodes, changeset_cache, user_display_name_cache)
+          way_ids << way.id
+        end
+      end
+
+      relations = Relation.nodes(visible_nodes.keys).visible +
+                  Relation.ways(way_ids).visible
+
+      # we do not normally return the "other" partners referenced by an relation,
+      # e.g. if we return a way A that is referenced by relation X, and there's
+      # another way B also referenced, that is not returned. But we do make
+      # an exception for cases where an relation references another *relation*;
+      # in that case we return that as well (but we don't go recursive here)
+      relations += Relation.relations(relations.collect(&:id)).visible
+
+      # this "uniq" may be slightly inefficient; it may be better to first collect and output
+      # all node-related relations, then find the *not yet covered* way-related ones etc.
+      relations.uniq.each do |relation|
+        doc.root << relation.to_xml_node(changeset_cache, user_display_name_cache)
+      end
+
+      response.headers["Content-Disposition"] = "attachment; filename=\"map.osm\""
+
+      render :xml => doc.to_s
+    end
+  end
+end
diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
deleted file mode 100644
index 735e1bd59..000000000
--- a/app/controllers/api_controller.rb
+++ /dev/null
@@ -1,105 +0,0 @@
-class ApiController < ApplicationController
-  skip_before_action :verify_authenticity_token
-  before_action :api_deny_access_handler
-
-  authorize_resource :class => false
-
-  before_action :check_api_readable
-  around_action :api_call_handle_error, :api_call_timeout
-
-  # This is probably the most common call of all. It is used for getting the
-  # OSM data for a specified bounding box, usually for editing. First the
-  # bounding box (bbox) is checked to make sure that it is sane. All nodes
-  # are searched, then all the ways that reference those nodes are found.
-  # All Nodes that are referenced by those ways are fetched and added to the list
-  # of nodes.
-  # Then all the relations that reference the already found nodes and ways are
-  # fetched. All the nodes and ways that are referenced by those ways are then
-  # fetched. Finally all the xml is returned.
-  def map
-    # Figure out the bbox
-    # check boundary is sane and area within defined
-    # see /config/application.yml
-    begin
-      bbox = BoundingBox.from_bbox_params(params)
-      bbox.check_boundaries
-      bbox.check_size
-    rescue StandardError => err
-      report_error(err.message)
-      return
-    end
-
-    nodes = Node.bbox(bbox).where(:visible => true).includes(:node_tags).limit(MAX_NUMBER_OF_NODES + 1)
-
-    node_ids = nodes.collect(&:id)
-    if node_ids.length > MAX_NUMBER_OF_NODES
-      report_error("You requested too many nodes (limit is #{MAX_NUMBER_OF_NODES}). Either request a smaller area, or use planet.osm")
-      return
-    end
-
-    doc = OSM::API.new.get_xml_doc
-
-    # add bounds
-    doc.root << bbox.add_bounds_to(XML::Node.new("bounds"))
-
-    # get ways
-    # find which ways are needed
-    ways = []
-    if node_ids.empty?
-      list_of_way_nodes = []
-    else
-      way_nodes = WayNode.where(:node_id => node_ids)
-      way_ids = way_nodes.collect { |way_node| way_node.id[0] }
-      ways = Way.preload(:way_nodes, :way_tags).find(way_ids)
-
-      list_of_way_nodes = ways.collect do |way|
-        way.way_nodes.collect(&:node_id)
-      end
-      list_of_way_nodes.flatten!
-    end
-
-    # - [0] in case some thing links to node 0 which doesn't exist. Shouldn't actually ever happen but it does. FIXME: file a ticket for this
-    nodes_to_fetch = (list_of_way_nodes.uniq - node_ids) - [0]
-
-    nodes += Node.includes(:node_tags).find(nodes_to_fetch) unless nodes_to_fetch.empty?
-
-    visible_nodes = {}
-    changeset_cache = {}
-    user_display_name_cache = {}
-
-    nodes.each do |node|
-      if node.visible?
-        doc.root << node.to_xml_node(changeset_cache, user_display_name_cache)
-        visible_nodes[node.id] = node
-      end
-    end
-
-    way_ids = []
-    ways.each do |way|
-      if way.visible?
-        doc.root << way.to_xml_node(visible_nodes, changeset_cache, user_display_name_cache)
-        way_ids << way.id
-      end
-    end
-
-    relations = Relation.nodes(visible_nodes.keys).visible +
-                Relation.ways(way_ids).visible
-
-    # we do not normally return the "other" partners referenced by an relation,
-    # e.g. if we return a way A that is referenced by relation X, and there's
-    # another way B also referenced, that is not returned. But we do make
-    # an exception for cases where an relation references another *relation*;
-    # in that case we return that as well (but we don't go recursive here)
-    relations += Relation.relations(relations.collect(&:id)).visible
-
-    # this "uniq" may be slightly inefficient; it may be better to first collect and output
-    # all node-related relations, then find the *not yet covered* way-related ones etc.
-    relations.uniq.each do |relation|
-      doc.root << relation.to_xml_node(changeset_cache, user_display_name_cache)
-    end
-
-    response.headers["Content-Disposition"] = "attachment; filename=\"map.osm\""
-
-    render :xml => doc.to_s
-  end
-end
diff --git a/app/controllers/export_controller.rb b/app/controllers/export_controller.rb
index 18ac15c10..fc5b0ec80 100644
--- a/app/controllers/export_controller.rb
+++ b/app/controllers/export_controller.rb
@@ -13,7 +13,7 @@ class ExportController < ApplicationController
 
     if format == "osm"
       # redirect to API map get
-      redirect_to :controller => "api", :action => "map", :bbox => bbox
+      redirect_to :controller => "api/map", :action => "index", :bbox => bbox
 
     elsif format == "mapnik"
       # redirect to a special 'export' cgi script
diff --git a/config/routes.rb b/config/routes.rb
index 1f0f73c7e..3c0cf61f1 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -55,7 +55,7 @@ OpenStreetMap::Application.routes.draw do
     delete "relation/:id" => "relations#delete", :id => /\d+/
     get "relations" => "relations#index"
 
-    get "map" => "api#map"
+    get "map" => "api/map#index"
 
     get "trackpoints" => "api/tracepoints#index"
 
diff --git a/test/controllers/api/map_controller_test.rb b/test/controllers/api/map_controller_test.rb
new file mode 100644
index 000000000..5ee5a9e6c
--- /dev/null
+++ b/test/controllers/api/map_controller_test.rb
@@ -0,0 +1,181 @@
+require "test_helper"
+
+module Api
+  class MapControllerTest < ActionController::TestCase
+    def setup
+      super
+      @badbigbbox = %w[-0.1,-0.1,1.1,1.1 10,10,11,11]
+      @badmalformedbbox = %w[-0.1 hello
+                             10N2W10.1N2.1W]
+      @badlatmixedbbox = %w[0,0.1,0.1,0 -0.1,80,0.1,70 0.24,54.34,0.25,54.33]
+      @badlonmixedbbox = %w[80,-0.1,70,0.1 54.34,0.24,54.33,0.25]
+      # @badlatlonoutboundsbbox = %w{ 191,-0.1,193,0.1  -190.1,89.9,-190,90 }
+      @goodbbox = %w[-0.1,-0.1,0.1,0.1 51.1,-0.1,51.2,0
+                     -0.1,%20-0.1,%200.1,%200.1 -0.1edcd,-0.1d,0.1,0.1 -0.1E,-0.1E,0.1S,0.1N S0.1,W0.1,N0.1,E0.1]
+      # That last item in the goodbbox really shouldn't be there, as the API should
+      # reall reject it, however this is to test to see if the api changes.
+    end
+
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/api/0.6/map", :method => :get },
+        { :controller => "api/map", :action => "index" }
+      )
+    end
+
+    # -------------------------------------
+    # Test reading a bounding box.
+    # -------------------------------------
+
+    def test_map
+      node = create(:node, :lat => 7, :lon => 7)
+      tag = create(:node_tag, :node => node)
+      way1 = create(:way_node, :node => node).way
+      way2 = create(:way_node, :node => node).way
+      relation = create(:relation_member, :member => node).relation
+
+      # 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 :index, :params => { :bbox => bbox }
+      if $VERBOSE
+        print @request.to_yaml
+        print @response.body
+      end
+      assert_response :success, "Expected scucess with the map call"
+      assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
+        assert_select "bounds[minlon='#{format('%.7f', minlon)}'][minlat='#{format('%.7f', minlat)}'][maxlon='#{format('%.7f', maxlon)}'][maxlat='#{format('%.7f', maxlat)}']", :count => 1
+        assert_select "node[id='#{node.id}'][lat='#{format('%.7f', node.lat)}'][lon='#{format('%.7f', 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='#{tag.k}'][v='#{tag.v}']"
+        end
+        assert_select "way", :count => 2
+        assert_select "way[id='#{way1.id}']", :count => 1
+        assert_select "way[id='#{way2.id}']", :count => 1
+        assert_select "relation", :count => 1
+        assert_select "relation[id='#{relation.id}']", :count => 1
+      end
+    end
+
+    # This differs from the above test in that we are making the bbox exactly
+    # the same as the node we are looking at
+    def test_map_inclusive
+      node = create(:node, :lat => 7, :lon => 7)
+      tag = create(:node_tag, :node => node)
+      way1 = create(:way_node, :node => node).way
+      way2 = create(:way_node, :node => node).way
+      relation = create(:relation_member, :member => node).relation
+
+      bbox = "#{node.lon},#{node.lat},#{node.lon},#{node.lat}"
+      get :index, :params => { :bbox => bbox }
+      assert_response :success, "The map call should have succeeded"
+      assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
+        assert_select "bounds[minlon='#{node.lon}'][minlat='#{node.lat}'][maxlon='#{node.lon}'][maxlat='#{node.lat}']", :count => 1
+        assert_select "node[id='#{node.id}'][lat='#{format('%.7f', node.lat)}'][lon='#{format('%.7f', 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='#{tag.k}'][v='#{tag.v}']"
+        end
+        assert_select "way", :count => 2
+        assert_select "way[id='#{way1.id}']", :count => 1
+        assert_select "way[id='#{way2.id}']", :count => 1
+        assert_select "relation", :count => 1
+        assert_select "relation[id='#{relation.id}']", :count => 1
+      end
+    end
+
+    def test_map_complete_way
+      node = create(:node, :lat => 7, :lon => 7)
+      # create a couple of nodes well outside of the bbox
+      node2 = create(:node, :lat => 45, :lon => 45)
+      node3 = create(:node, :lat => 10, :lon => 10)
+      way1 = create(:way_node, :node => node).way
+      create(:way_node, :way => way1, :node => node2, :sequence_id => 2)
+      way2 = create(:way_node, :node => node).way
+      create(:way_node, :way => way2, :node => node3, :sequence_id => 2)
+      relation = create(:relation_member, :member => way1).relation
+
+      bbox = "#{node.lon},#{node.lat},#{node.lon},#{node.lat}"
+      get :index, :params => { :bbox => bbox }
+      assert_response :success, "The map call should have succeeded"
+      assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
+        assert_select "bounds[minlon='#{node.lon}'][minlat='#{node.lat}'][maxlon='#{node.lon}'][maxlat='#{node.lat}']", :count => 1
+        assert_select "node", :count => 3
+        assert_select "node[id='#{node.id}']", :count => 1
+        assert_select "node[id='#{node2.id}']", :count => 1
+        assert_select "node[id='#{node3.id}']", :count => 1
+        assert_select "way", :count => 2
+        assert_select "way[id='#{way1.id}']", :count => 1
+        assert_select "way[id='#{way2.id}']", :count => 1
+        assert_select "relation", :count => 1
+        assert_select "relation[id='#{relation.id}']", :count => 1
+      end
+    end
+
+    def test_map_empty
+      get :index, :params => { :bbox => "179.998,89.998,179.999.1,89.999" }
+      assert_response :success, "The map call should have succeeded"
+      assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
+        assert_select "bounds[minlon='179.9980000'][minlat='89.9980000'][maxlon='179.9990000'][maxlat='89.9990000']", :count => 1
+        assert_select "node", :count => 0
+        assert_select "way", :count => 0
+        assert_select "relation", :count => 0
+      end
+    end
+
+    def test_map_without_bbox
+      get :index
+      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, "A bbox param was expected"
+    end
+
+    def test_bbox_too_big
+      @badbigbbox.each do |bbox|
+        get :index, :params => { :bbox => bbox }
+        assert_response :bad_request, "The bbox:#{bbox} was expected to be too big"
+        assert_equal "The maximum bbox size is #{MAX_REQUEST_AREA}, and your request was too large. Either request a smaller area, or use planet.osm", @response.body, "bbox: #{bbox}"
+      end
+    end
+
+    def test_bbox_malformed
+      @badmalformedbbox.each do |bbox|
+        get :index, :params => { :bbox => bbox }
+        assert_response :bad_request, "The bbox:#{bbox} was expected to be malformed"
+        assert_equal "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat", @response.body, "bbox: #{bbox}"
+      end
+    end
+
+    def test_bbox_lon_mixedup
+      @badlonmixedbbox.each do |bbox|
+        get :index, :params => { :bbox => bbox }
+        assert_response :bad_request, "The bbox:#{bbox} was expected to have the longitude mixed up"
+        assert_equal "The minimum longitude must be less than the maximum longitude, but it wasn't", @response.body, "bbox: #{bbox}"
+      end
+    end
+
+    def test_bbox_lat_mixedup
+      @badlatmixedbbox.each do |bbox|
+        get :index, :params => { :bbox => bbox }
+        assert_response :bad_request, "The bbox:#{bbox} was expected to have the latitude mixed up"
+        assert_equal "The minimum latitude must be less than the maximum latitude, but it wasn't", @response.body, "bbox: #{bbox}"
+      end
+    end
+
+    # We can't actually get an out of bounds error, as the bbox is sanitised.
+    # def test_latlon_outofbounds
+    #  @badlatlonoutboundsbbox.each do |bbox|
+    #    [ "trackpoints", "map" ].each do |tq|
+    #      get tq, :bbox => bbox
+    #      #print @request.to_yaml
+    #      assert_response :bad_request, "The bbox #{bbox} was expected to be out of range"
+    #      assert_equal "The latitudes must be between -90 an 90, and longitudes between -180 and 180", @response.body, "bbox: #{bbox}"
+    #    end
+    #  end
+    # end
+  end
+end
diff --git a/test/controllers/api_controller_test.rb b/test/controllers/api_controller_test.rb
deleted file mode 100644
index a9f8323b3..000000000
--- a/test/controllers/api_controller_test.rb
+++ /dev/null
@@ -1,179 +0,0 @@
-require "test_helper"
-
-class ApiControllerTest < ActionController::TestCase
-  def setup
-    super
-    @badbigbbox = %w[-0.1,-0.1,1.1,1.1 10,10,11,11]
-    @badmalformedbbox = %w[-0.1 hello
-                           10N2W10.1N2.1W]
-    @badlatmixedbbox = %w[0,0.1,0.1,0 -0.1,80,0.1,70 0.24,54.34,0.25,54.33]
-    @badlonmixedbbox = %w[80,-0.1,70,0.1 54.34,0.24,54.33,0.25]
-    # @badlatlonoutboundsbbox = %w{ 191,-0.1,193,0.1  -190.1,89.9,-190,90 }
-    @goodbbox = %w[-0.1,-0.1,0.1,0.1 51.1,-0.1,51.2,0
-                   -0.1,%20-0.1,%200.1,%200.1 -0.1edcd,-0.1d,0.1,0.1 -0.1E,-0.1E,0.1S,0.1N S0.1,W0.1,N0.1,E0.1]
-    # That last item in the goodbbox really shouldn't be there, as the API should
-    # reall reject it, however this is to test to see if the api changes.
-  end
-
-  ##
-  # test all routes which lead to this controller
-  def test_routes
-    assert_routing(
-      { :path => "/api/0.6/map", :method => :get },
-      { :controller => "api", :action => "map" }
-    )
-  end
-
-  # -------------------------------------
-  # Test reading a bounding box.
-  # -------------------------------------
-
-  def test_map
-    node = create(:node, :lat => 7, :lon => 7)
-    tag = create(:node_tag, :node => node)
-    way1 = create(:way_node, :node => node).way
-    way2 = create(:way_node, :node => node).way
-    relation = create(:relation_member, :member => node).relation
-
-    # 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, :params => { :bbox => bbox }
-    if $VERBOSE
-      print @request.to_yaml
-      print @response.body
-    end
-    assert_response :success, "Expected scucess with the map call"
-    assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
-      assert_select "bounds[minlon='#{format('%.7f', minlon)}'][minlat='#{format('%.7f', minlat)}'][maxlon='#{format('%.7f', maxlon)}'][maxlat='#{format('%.7f', maxlat)}']", :count => 1
-      assert_select "node[id='#{node.id}'][lat='#{format('%.7f', node.lat)}'][lon='#{format('%.7f', 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='#{tag.k}'][v='#{tag.v}']"
-      end
-      assert_select "way", :count => 2
-      assert_select "way[id='#{way1.id}']", :count => 1
-      assert_select "way[id='#{way2.id}']", :count => 1
-      assert_select "relation", :count => 1
-      assert_select "relation[id='#{relation.id}']", :count => 1
-    end
-  end
-
-  # This differs from the above test in that we are making the bbox exactly
-  # the same as the node we are looking at
-  def test_map_inclusive
-    node = create(:node, :lat => 7, :lon => 7)
-    tag = create(:node_tag, :node => node)
-    way1 = create(:way_node, :node => node).way
-    way2 = create(:way_node, :node => node).way
-    relation = create(:relation_member, :member => node).relation
-
-    bbox = "#{node.lon},#{node.lat},#{node.lon},#{node.lat}"
-    get :map, :params => { :bbox => bbox }
-    assert_response :success, "The map call should have succeeded"
-    assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
-      assert_select "bounds[minlon='#{node.lon}'][minlat='#{node.lat}'][maxlon='#{node.lon}'][maxlat='#{node.lat}']", :count => 1
-      assert_select "node[id='#{node.id}'][lat='#{format('%.7f', node.lat)}'][lon='#{format('%.7f', 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='#{tag.k}'][v='#{tag.v}']"
-      end
-      assert_select "way", :count => 2
-      assert_select "way[id='#{way1.id}']", :count => 1
-      assert_select "way[id='#{way2.id}']", :count => 1
-      assert_select "relation", :count => 1
-      assert_select "relation[id='#{relation.id}']", :count => 1
-    end
-  end
-
-  def test_map_complete_way
-    node = create(:node, :lat => 7, :lon => 7)
-    # create a couple of nodes well outside of the bbox
-    node2 = create(:node, :lat => 45, :lon => 45)
-    node3 = create(:node, :lat => 10, :lon => 10)
-    way1 = create(:way_node, :node => node).way
-    create(:way_node, :way => way1, :node => node2, :sequence_id => 2)
-    way2 = create(:way_node, :node => node).way
-    create(:way_node, :way => way2, :node => node3, :sequence_id => 2)
-    relation = create(:relation_member, :member => way1).relation
-
-    bbox = "#{node.lon},#{node.lat},#{node.lon},#{node.lat}"
-    get :map, :params => { :bbox => bbox }
-    assert_response :success, "The map call should have succeeded"
-    assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
-      assert_select "bounds[minlon='#{node.lon}'][minlat='#{node.lat}'][maxlon='#{node.lon}'][maxlat='#{node.lat}']", :count => 1
-      assert_select "node", :count => 3
-      assert_select "node[id='#{node.id}']", :count => 1
-      assert_select "node[id='#{node2.id}']", :count => 1
-      assert_select "node[id='#{node3.id}']", :count => 1
-      assert_select "way", :count => 2
-      assert_select "way[id='#{way1.id}']", :count => 1
-      assert_select "way[id='#{way2.id}']", :count => 1
-      assert_select "relation", :count => 1
-      assert_select "relation[id='#{relation.id}']", :count => 1
-    end
-  end
-
-  def test_map_empty
-    get :map, :params => { :bbox => "179.998,89.998,179.999.1,89.999" }
-    assert_response :success, "The map call should have succeeded"
-    assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
-      assert_select "bounds[minlon='179.9980000'][minlat='89.9980000'][maxlon='179.9990000'][maxlat='89.9990000']", :count => 1
-      assert_select "node", :count => 0
-      assert_select "way", :count => 0
-      assert_select "relation", :count => 0
-    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, "A bbox param was expected"
-  end
-
-  def test_bbox_too_big
-    @badbigbbox.each do |bbox|
-      get :map, :params => { :bbox => bbox }
-      assert_response :bad_request, "The bbox:#{bbox} was expected to be too big"
-      assert_equal "The maximum bbox size is #{MAX_REQUEST_AREA}, and your request was too large. Either request a smaller area, or use planet.osm", @response.body, "bbox: #{bbox}"
-    end
-  end
-
-  def test_bbox_malformed
-    @badmalformedbbox.each do |bbox|
-      get :map, :params => { :bbox => bbox }
-      assert_response :bad_request, "The bbox:#{bbox} was expected to be malformed"
-      assert_equal "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat", @response.body, "bbox: #{bbox}"
-    end
-  end
-
-  def test_bbox_lon_mixedup
-    @badlonmixedbbox.each do |bbox|
-      get :map, :params => { :bbox => bbox }
-      assert_response :bad_request, "The bbox:#{bbox} was expected to have the longitude mixed up"
-      assert_equal "The minimum longitude must be less than the maximum longitude, but it wasn't", @response.body, "bbox: #{bbox}"
-    end
-  end
-
-  def test_bbox_lat_mixedup
-    @badlatmixedbbox.each do |bbox|
-      get :map, :params => { :bbox => bbox }
-      assert_response :bad_request, "The bbox:#{bbox} was expected to have the latitude mixed up"
-      assert_equal "The minimum latitude must be less than the maximum latitude, but it wasn't", @response.body, "bbox: #{bbox}"
-    end
-  end
-
-  # We can't actually get an out of bounds error, as the bbox is sanitised.
-  # def test_latlon_outofbounds
-  #  @badlatlonoutboundsbbox.each do |bbox|
-  #    [ "trackpoints", "map" ].each do |tq|
-  #      get tq, :bbox => bbox
-  #      #print @request.to_yaml
-  #      assert_response :bad_request, "The bbox #{bbox} was expected to be out of range"
-  #      assert_equal "The latitudes must be between -90 an 90, and longitudes between -180 and 180", @response.body, "bbox: #{bbox}"
-  #    end
-  #  end
-  # end
-end
diff --git a/test/controllers/export_controller_test.rb b/test/controllers/export_controller_test.rb
index 94cf6cf52..daaf193c8 100644
--- a/test/controllers/export_controller_test.rb
+++ b/test/controllers/export_controller_test.rb
@@ -19,7 +19,7 @@ class ExportControllerTest < ActionController::TestCase
   def test_finish_osm
     get :finish, :params => { :minlon => 0, :minlat => 50, :maxlon => 1, :maxlat => 51, :format => "osm" }
     assert_response :redirect
-    assert_redirected_to "controller" => "api", "action" => "map", "bbox" => "0.0,50.0,1.0,51.0"
+    assert_redirected_to "controller" => "api/map", "action" => "index", "bbox" => "0.0,50.0,1.0,51.0"
   end
 
   ###