Move the changes api to its own controller

app/abilities/ability.rb

@@ -8,6 +8,7 @@ class Ability
     can [:relation, :relation_history, :way, :way_history, :node, :node_history,
          :changeset, :note, :new_note, :query], :browse
     can :show, :capability
+    can :index, :change
     can [:index, :feed, :show, :download, :query], Changeset
     can :index, ChangesetComment
     can :search, :direction

app/controllers/api/changes_controller.rb

@@ -0,0 +1,57 @@
+module Api
+  class ChangesController < 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
+
+    # Get a list of the tiles that have changed within a specified time
+    # period
+    def index
+      zoom = (params[:zoom] || "12").to_i
+
+      if params.include?(:start) && params.include?(:end)
+        starttime = Time.parse(params[:start])
+        endtime = Time.parse(params[:end])
+      else
+        hours = (params[:hours] || "1").to_i.hours
+        endtime = Time.now.getutc
+        starttime = endtime - hours
+      end
+
+      if zoom >= 1 && zoom <= 16 &&
+         endtime > starttime && endtime - starttime <= 24.hours
+        mask = (1 << zoom) - 1
+
+        tiles = Node.where(:timestamp => starttime..endtime).group("maptile_for_point(latitude, longitude, #{zoom})").count
+
+        doc = OSM::API.new.get_xml_doc
+        changes = XML::Node.new "changes"
+        changes["starttime"] = starttime.xmlschema
+        changes["endtime"] = endtime.xmlschema
+
+        tiles.each do |tile, count|
+          x = (tile.to_i >> zoom) & mask
+          y = tile.to_i & mask
+
+          t = XML::Node.new "tile"
+          t["x"] = x.to_s
+          t["y"] = y.to_s
+          t["z"] = zoom.to_s
+          t["changes"] = count.to_s
+
+          changes << t
+        end
+
+        doc.root << changes
+
+        render :xml => doc.to_s
+      else
+        render :plain => "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours", :status => :bad_request
+      end
+    end
+  end
+end

app/controllers/api_controller.rb

@@ -102,50 +102,4 @@ class ApiController < ApplicationController
 
     render :xml => doc.to_s
   end
-
-  # Get a list of the tiles that have changed within a specified time
-  # period
-  def changes
-    zoom = (params[:zoom] || "12").to_i
-
-    if params.include?(:start) && params.include?(:end)
-      starttime = Time.parse(params[:start])
-      endtime = Time.parse(params[:end])
-    else
-      hours = (params[:hours] || "1").to_i.hours
-      endtime = Time.now.getutc
-      starttime = endtime - hours
-    end
-
-    if zoom >= 1 && zoom <= 16 &&
-       endtime > starttime && endtime - starttime <= 24.hours
-      mask = (1 << zoom) - 1
-
-      tiles = Node.where(:timestamp => starttime..endtime).group("maptile_for_point(latitude, longitude, #{zoom})").count
-
-      doc = OSM::API.new.get_xml_doc
-      changes = XML::Node.new "changes"
-      changes["starttime"] = starttime.xmlschema
-      changes["endtime"] = endtime.xmlschema
-
-      tiles.each do |tile, count|
-        x = (tile.to_i >> zoom) & mask
-        y = tile.to_i & mask
-
-        t = XML::Node.new "tile"
-        t["x"] = x.to_s
-        t["y"] = y.to_s
-        t["z"] = zoom.to_s
-        t["changes"] = count.to_s
-
-        changes << t
-      end
-
-      doc.root << changes
-
-      render :xml => doc.to_s
-    else
-      render :plain => "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours", :status => :bad_request
-    end
-  end
 end

config/routes.rb

@@ -59,7 +59,7 @@ OpenStreetMap::Application.routes.draw do
 
     get "trackpoints" => "api/tracepoints#index"
 
-    get "changes" => "api#changes"
+    get "changes" => "api/changes#index"
 
     get "search" => "search#search_all", :as => "api_search"
     get "ways/search" => "search#search_ways"

test/controllers/api/changes_controller_test.rb

@@ -0,0 +1,106 @@
+require "test_helper"
+
+module Api
+  class ChangesControllerTest < ActionController::TestCase
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/api/0.6/changes", :method => :get },
+        { :controller => "api/changes", :action => "index" }
+      )
+    end
+
+    # MySQL and Postgres require that the C based functions are installed for
+    # this test to work. More information is available from:
+    # http://wiki.openstreetmap.org/wiki/Rails#Installing_the_quadtile_functions
+    # or by looking at the readme in db/README
+    def test_changes_simple
+      # create a selection of nodes
+      (1..5).each do |n|
+        create(:node, :timestamp => Time.utc(2007, 1, 1, 0, 0, 0), :lat => n, :lon => n)
+      end
+      # deleted nodes should also be counted
+      create(:node, :deleted, :timestamp => Time.utc(2007, 1, 1, 0, 0, 0), :lat => 6, :lon => 6)
+      # nodes in the same tile won't change the total
+      create(:node, :timestamp => Time.utc(2007, 1, 1, 0, 0, 0), :lat => 6, :lon => 6)
+      # nodes with a different timestamp should be ignored
+      create(:node, :timestamp => Time.utc(2008, 1, 1, 0, 0, 0), :lat => 7, :lon => 7)
+
+      travel_to Time.utc(2010, 4, 3, 10, 55, 0) do
+        get :index
+        assert_response :success
+        now = Time.now.getutc
+        hourago = now - 1.hour
+        assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
+          assert_select "changes[starttime='#{hourago.xmlschema}'][endtime='#{now.xmlschema}']", :count => 1 do
+            assert_select "tile", :count => 0
+          end
+        end
+      end
+
+      travel_to Time.utc(2007, 1, 1, 0, 30, 0) do
+        get :index
+        assert_response :success
+        # print @response.body
+        # As we have loaded the fixtures, we can assume that there are some
+        # changes at the time we have frozen at
+        now = Time.now.getutc
+        hourago = now - 1.hour
+        assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
+          assert_select "changes[starttime='#{hourago.xmlschema}'][endtime='#{now.xmlschema}']", :count => 1 do
+            assert_select "tile", :count => 6
+          end
+        end
+      end
+    end
+
+    def test_changes_zoom_invalid
+      zoom_to_test = %w[p -1 0 17 one two]
+      zoom_to_test.each do |zoom|
+        get :index, :params => { :zoom => zoom }
+        assert_response :bad_request
+        assert_equal @response.body, "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours"
+      end
+    end
+
+    def test_changes_zoom_valid
+      1.upto(16) do |zoom|
+        get :index, :params => { :zoom => zoom }
+        assert_response :success
+        # NOTE: there was a test here for the timing, but it was too sensitive to be a good test
+        # and it was annoying.
+        assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
+          assert_select "changes", :count => 1
+        end
+      end
+    end
+
+    def test_changes_hours_invalid
+      invalid = %w[-21 335 -1 0 25 26 100 one two three ping pong :]
+      invalid.each do |hour|
+        get :index, :params => { :hours => hour }
+        assert_response :bad_request, "Problem with the hour: #{hour}"
+        assert_equal @response.body, "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours", "Problem with the hour: #{hour}."
+      end
+    end
+
+    def test_changes_hours_valid
+      1.upto(24) do |hour|
+        get :index, :params => { :hours => hour }
+        assert_response :success
+      end
+    end
+
+    def test_changes_start_end_invalid
+      get :index, :params => { :start => "2010-04-03 10:55:00", :end => "2010-04-03 09:55:00" }
+      assert_response :bad_request
+      assert_equal @response.body, "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours"
+    end
+
+    def test_changes_start_end_valid
+      get :index, :params => { :start => "2010-04-03 09:55:00", :end => "2010-04-03 10:55:00" }
+      assert_response :success
+    end
+  end
+end

test/controllers/api_controller_test.rb

@@ -22,10 +22,6 @@ class ApiControllerTest < ActionController::TestCase
       { :path => "/api/0.6/map", :method => :get },
       { :controller => "api", :action => "map" }
     )
-    assert_routing(
-      { :path => "/api/0.6/changes", :method => :get },
-      { :controller => "api", :action => "changes" }
-    )
   end
 
   # -------------------------------------
@@ -180,96 +176,4 @@ class ApiControllerTest < ActionController::TestCase
   #    end
   #  end
   # end
-
-  # MySQL and Postgres require that the C based functions are installed for
-  # this test to work. More information is available from:
-  # http://wiki.openstreetmap.org/wiki/Rails#Installing_the_quadtile_functions
-  # or by looking at the readme in db/README
-  def test_changes_simple
-    # create a selection of nodes
-    (1..5).each do |n|
-      create(:node, :timestamp => Time.utc(2007, 1, 1, 0, 0, 0), :lat => n, :lon => n)
-    end
-    # deleted nodes should also be counted
-    create(:node, :deleted, :timestamp => Time.utc(2007, 1, 1, 0, 0, 0), :lat => 6, :lon => 6)
-    # nodes in the same tile won't change the total
-    create(:node, :timestamp => Time.utc(2007, 1, 1, 0, 0, 0), :lat => 6, :lon => 6)
-    # nodes with a different timestamp should be ignored
-    create(:node, :timestamp => Time.utc(2008, 1, 1, 0, 0, 0), :lat => 7, :lon => 7)
-
-    travel_to Time.utc(2010, 4, 3, 10, 55, 0) do
-      get :changes
-      assert_response :success
-      now = Time.now.getutc
-      hourago = now - 1.hour
-      assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
-        assert_select "changes[starttime='#{hourago.xmlschema}'][endtime='#{now.xmlschema}']", :count => 1 do
-          assert_select "tile", :count => 0
-        end
-      end
-    end
-
-    travel_to Time.utc(2007, 1, 1, 0, 30, 0) do
-      get :changes
-      assert_response :success
-      # print @response.body
-      # As we have loaded the fixtures, we can assume that there are some
-      # changes at the time we have frozen at
-      now = Time.now.getutc
-      hourago = now - 1.hour
-      assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
-        assert_select "changes[starttime='#{hourago.xmlschema}'][endtime='#{now.xmlschema}']", :count => 1 do
-          assert_select "tile", :count => 6
-        end
-      end
-    end
-  end
-
-  def test_changes_zoom_invalid
-    zoom_to_test = %w[p -1 0 17 one two]
-    zoom_to_test.each do |zoom|
-      get :changes, :params => { :zoom => zoom }
-      assert_response :bad_request
-      assert_equal @response.body, "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours"
-    end
-  end
-
-  def test_changes_zoom_valid
-    1.upto(16) do |zoom|
-      get :changes, :params => { :zoom => zoom }
-      assert_response :success
-      # NOTE: there was a test here for the timing, but it was too sensitive to be a good test
-      # and it was annoying.
-      assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
-        assert_select "changes", :count => 1
-      end
-    end
-  end
-
-  def test_changes_hours_invalid
-    invalid = %w[-21 335 -1 0 25 26 100 one two three ping pong :]
-    invalid.each do |hour|
-      get :changes, :params => { :hours => hour }
-      assert_response :bad_request, "Problem with the hour: #{hour}"
-      assert_equal @response.body, "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours", "Problem with the hour: #{hour}."
-    end
-  end
-
-  def test_changes_hours_valid
-    1.upto(24) do |hour|
-      get :changes, :params => { :hours => hour }
-      assert_response :success
-    end
-  end
-
-  def test_changes_start_end_invalid
-    get :changes, :params => { :start => "2010-04-03 10:55:00", :end => "2010-04-03 09:55:00" }
-    assert_response :bad_request
-    assert_equal @response.body, "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours"
-  end
-
-  def test_changes_start_end_valid
-    get :changes, :params => { :start => "2010-04-03 09:55:00", :end => "2010-04-03 10:55:00" }
-    assert_response :success
-  end
 end