Support versions in elements multi fetch

app/controllers/api/elements_controller.rb

@@ -0,0 +1,37 @@
+module Api
+  class ElementsController < ApiController
+    # Dump the details on many elements whose ids and optionally versions are given in the "nodes"/"ways"/"relations" parameter.
+    def index
+      type_plural = current_model.model_name.plural
+
+      raise OSM::APIBadUserInput, "The parameter #{type_plural} is required, and must be of the form #{type_plural}=ID[vVER][,ID[vVER][,ID[vVER]...]]" unless params[type_plural]
+
+      search_strings = params[type_plural].split(",")
+
+      raise OSM::APIBadUserInput, "No #{type_plural} were given to search for" if search_strings.empty?
+
+      id_ver_strings, id_strings = search_strings.partition { |iv| iv.include? "v" }
+      id_vers = id_ver_strings.map { |iv| iv.split("v", 2).map(&:to_i) }
+      ids = id_strings.map(&:to_i)
+
+      result = current_model.find(ids)
+      unless id_vers.empty?
+        result += old_model.find(id_vers)
+        result.uniq! do |element|
+          if element.id.is_a?(Array)
+            element.id
+          else
+            [element.id, element.version]
+          end
+        end
+      end
+      instance_variable_set :"@#{type_plural}", result
+
+      # Render the result
+      respond_to do |format|
+        format.xml
+        format.json
+      end
+    end
+  end
+end

app/controllers/api/nodes_controller.rb

@@ -1,7 +1,7 @@
 # The NodeController is the RESTful interface to Node objects
 
 module Api
-  class NodesController < ApiController
+  class NodesController < ElementsController
     before_action :check_api_writable, :only => [:create, :update, :delete]
     before_action :authorize, :only => [:create, :update, :delete]
 
@@ -11,23 +11,6 @@ class NodesController < ApiController
     before_action :set_request_formats, :except => [:create, :update, :delete]
     before_action :check_rate_limit, :only => [:create, :update, :delete]
 
-    # Dump the details on many nodes whose ids are given in the "nodes" parameter.
-    def index
-      raise OSM::APIBadUserInput, "The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]" unless params["nodes"]
-
-      ids = params["nodes"].split(",").collect(&:to_i)
-
-      raise OSM::APIBadUserInput, "No nodes were given to search for" if ids.empty?
-
-      @nodes = Node.find(ids)
-
-      # Render the result
-      respond_to do |format|
-        format.xml
-        format.json
-      end
-    end
-
     # Dump the details on a node given in params[:id]
     def show
       @node = Node.find(params[:id])
@@ -77,5 +60,15 @@ def delete
       node.delete_with_history!(new_node, current_user)
       render :plain => node.version.to_s
     end
+
+    private
+
+    def current_model
+      Node
+    end
+
+    def old_model
+      OldNode
+    end
   end
 end

app/controllers/api/relations_controller.rb

@@ -1,5 +1,5 @@
 module Api
-  class RelationsController < ApiController
+  class RelationsController < ElementsController
     before_action :check_api_writable, :only => [:create, :update, :delete]
     before_action :authorize, :only => [:create, :update, :delete]
 
@@ -9,22 +9,6 @@ class RelationsController < ApiController
     before_action :set_request_formats, :except => [:create, :update, :delete]
     before_action :check_rate_limit, :only => [:create, :update, :delete]
 
-    def index
-      raise OSM::APIBadUserInput, "The parameter relations is required, and must be of the form relations=id[,id[,id...]]" unless params["relations"]
-
-      ids = params["relations"].split(",").collect(&:to_i)
-
-      raise OSM::APIBadUserInput, "No relations were given to search for" if ids.empty?
-
-      @relations = Relation.find(ids)
-
-      # Render the result
-      respond_to do |format|
-        format.xml
-        format.json
-      end
-    end
-
     def show
       @relation = Relation.find(params[:id])
       response.last_modified = @relation.timestamp
@@ -170,5 +154,13 @@ def relations_for_object(objtype)
         format.json
       end
     end
+
+    def current_model
+      Relation
+    end
+
+    def old_model
+      OldRelation
+    end
   end
 end

app/controllers/api/ways_controller.rb

@@ -1,5 +1,5 @@
 module Api
-  class WaysController < ApiController
+  class WaysController < ElementsController
     before_action :check_api_writable, :only => [:create, :update, :delete]
     before_action :authorize, :only => [:create, :update, :delete]
 
@@ -9,22 +9,6 @@ class WaysController < ApiController
     before_action :set_request_formats, :except => [:create, :update, :delete]
     before_action :check_rate_limit, :only => [:create, :update, :delete]
 
-    def index
-      raise OSM::APIBadUserInput, "The parameter ways is required, and must be of the form ways=id[,id[,id...]]" unless params["ways"]
-
-      ids = params["ways"].split(",").collect(&:to_i)
-
-      raise OSM::APIBadUserInput, "No ways were given to search for" if ids.empty?
-
-      @ways = Way.find(ids)
-
-      # Render the result
-      respond_to do |format|
-        format.xml
-        format.json
-      end
-    end
-
     def show
       @way = Way.find(params[:id])
 
@@ -112,5 +96,15 @@ def ways_for_node
         format.json
       end
     end
+
+    private
+
+    def current_model
+      Way
+    end
+
+    def old_model
+      OldWay
+    end
   end
 end

test/controllers/api/nodes_controller_test.rb

@@ -436,10 +436,12 @@ def test_index
       # check error when no parameter provided
       get nodes_path
       assert_response :bad_request
+      assert_match "parameter nodes is required", @response.body
 
       # check error when no parameter value provided
       get nodes_path(:nodes => "")
       assert_response :bad_request
+      assert_match "No nodes were given", @response.body
 
       # test a working call
       get nodes_path(:nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id}")
@@ -455,7 +457,6 @@ def test_index
 
       # test a working call with json format
       get nodes_path(:nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id}", :format => "json")
-
       js = ActiveSupport::JSON.decode(@response.body)
       assert_not_nil js
       assert_equal 5, js["elements"].count
@@ -471,6 +472,41 @@ def test_index
       assert_response :not_found
     end
 
+    ##
+    # test fetching multiple nodes with specified versions
+    def test_index_with_versions
+      node1 = create(:node)
+      node2 = create(:node, :with_history, :version => 2)
+
+      # test a working call only with versions
+      get nodes_path(:nodes => "#{node2.id}v1,#{node2.id}v2")
+      assert_response :success
+      assert_select "osm" do
+        assert_select "node", :count => 2
+        assert_select "node[id='#{node2.id}'][version='1']", :count => 1
+        assert_select "node[id='#{node2.id}'][version='2']", :count => 1
+      end
+
+      # test a working call
+      get nodes_path(:nodes => "#{node1.id},#{node2.id}v1,#{node2.id}v2")
+      assert_response :success
+      assert_select "osm" do
+        assert_select "node", :count => 3
+        assert_select "node[id='#{node1.id}'][version='1']", :count => 1
+        assert_select "node[id='#{node2.id}'][version='1']", :count => 1
+        assert_select "node[id='#{node2.id}'][version='2']", :count => 1
+      end
+
+      # test a working call with intersecting results
+      get nodes_path(:nodes => "#{node2.id},#{node2.id}v1,#{node2.id}v2")
+      assert_response :success
+      assert_select "osm" do
+        assert_select "node", :count => 2
+        assert_select "node[id='#{node2.id}'][version='1']", :count => 1
+        assert_select "node[id='#{node2.id}'][version='2']", :count => 1
+      end
+    end
+
     ##
     # test adding tags to a node
     def test_duplicate_tags

test/controllers/api/relations_controller_test.rb

@@ -176,10 +176,12 @@ def test_index
       # check error when no parameter provided
       get relations_path
       assert_response :bad_request
+      assert_match "parameter relations is required", @response.body
 
       # check error when no parameter value provided
       get relations_path(:relations => "")
       assert_response :bad_request
+      assert_match "No relations were given", @response.body
 
       # test a working call
       get relations_path(:relations => "#{relation1.id},#{relation2.id},#{relation3.id},#{relation4.id}")
@@ -209,6 +211,23 @@ def test_index
       assert_response :not_found
     end
 
+    ##
+    # test fetching multiple relations with specified versions
+    def test_index_with_versions
+      relation1 = create(:relation)
+      relation2 = create(:relation, :with_history, :version => 2)
+
+      # test a working call with versions
+      get relations_path(:relations => "#{relation1.id},#{relation2.id}v1,#{relation2.id}v2")
+      assert_response :success
+      assert_select "osm" do
+        assert_select "relation", :count => 3
+        assert_select "relation[id='#{relation1.id}'][version='1']", :count => 1
+        assert_select "relation[id='#{relation2.id}'][version='1']", :count => 1
+        assert_select "relation[id='#{relation2.id}'][version='2']", :count => 1
+      end
+    end
+
     # -------------------------------------
     # Test simple relation creation.
     # -------------------------------------

test/controllers/api/ways_controller_test.rb

@@ -100,10 +100,12 @@ def test_index
       # check error when no parameter provided
       get ways_path
       assert_response :bad_request
+      assert_match "parameter ways is required", @response.body
 
       # check error when no parameter value provided
       get ways_path(:ways => "")
       assert_response :bad_request
+      assert_match "No ways were given", @response.body
 
       # test a working call
       get ways_path(:ways => "#{way1.id},#{way2.id},#{way3.id},#{way4.id}")
@@ -133,6 +135,23 @@ def test_index
       assert_response :not_found
     end
 
+    ##
+    # test fetching multiple ways with specified versions
+    def test_index_with_versions
+      way1 = create(:way)
+      way2 = create(:way, :with_history, :version => 2)
+
+      # test a working call with versions
+      get ways_path(:ways => "#{way1.id},#{way2.id}v1,#{way2.id}v2")
+      assert_response :success
+      assert_select "osm" do
+        assert_select "way", :count => 3
+        assert_select "way[id='#{way1.id}'][version='1']", :count => 1
+        assert_select "way[id='#{way2.id}'][version='1']", :count => 1
+        assert_select "way[id='#{way2.id}'][version='2']", :count => 1
+      end
+    end
+
     # -------------------------------------
     # Test simple way creation.
     # -------------------------------------