Map 'full' to api way show action

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
index c74a4d0996adedcebd66ec59d47d696b01ddd28c..7ce3aa599ba0d8265bb18e649e9801e410833fe8 100644 (file)
--- a/app/abilities/api_ability.rb
+++ b/app/abilities/api_ability.rb
@@ -16,7 +16,7 @@ class ApiAbility
       can :read, Tracepoint
       can :read, User
       can :read, Node
-      can [:read, :full, :ways_for_node], Way
+      can [:read, :ways_for_node], Way
       can [:read, :full, :relations_for_node, :relations_for_way, :relations_for_relation], Relation
       can [:history, :read], [OldNode, OldWay, OldRelation]
       can :read, UserBlock

diff --git a/app/controllers/api/ways_controller.rb b/app/controllers/api/ways_controller.rb
index 285ed46046041af296b1dc223e9320293fcd14d5..b1bc8d799b0747c3f35dc3b36bdadcadd21f26e7 100644 (file)
--- a/app/controllers/api/ways_controller.rb
+++ b/app/controllers/api/ways_controller.rb
@@ -26,12 +26,21 @@ module Api
     end
 
     def show
-      @way = Way.find(params[:id])
+      @way = Way
+      @way = @way.includes(:nodes => :node_tags) if params[:full]
+      @way = @way.find(params[:id])
 
-      response.last_modified = @way.timestamp
+      response.last_modified = @way.timestamp unless params[:full]
 
       if @way.visible
-        # Render the result
+        if params[:full]
+          @nodes = []
+
+          @way.nodes.uniq.each do |node|
+            @nodes << node if node.visible
+          end
+        end
+
         respond_to do |format|
           format.xml
           format.json
@@ -72,26 +81,6 @@ module Api
       end
     end
 
-    def full
-      @way = Way.includes(:nodes => :node_tags).find(params[:id])
-
-      if @way.visible
-        @nodes = []
-
-        @way.nodes.uniq.each do |node|
-          @nodes << node if node.visible
-        end
-
-        # Render the result
-        respond_to do |format|
-          format.xml
-          format.json
-        end
-      else
-        head :gone
-      end
-    end
-
     ##
     # returns all the ways which are currently using the node given in the
     # :id parameter. note that this used to return deleted ways as well, but

diff --git a/app/views/api/ways/full.json.jbuilder b/app/views/api/ways/full.json.jbuilder
deleted file mode 100644 (file)
index 1bd127d..0000000
--- a/app/views/api/ways/full.json.jbuilder
+++ /dev/null
@@ -1,6 +0,0 @@
-json.partial! "api/root_attributes"
-
-json.elements do
-  json.array! @nodes, :partial => "/api/nodes/node", :as => :node
-  json.array! [@way], :partial => "/api/ways/way", :as => :way
-end

diff --git a/app/views/api/ways/full.xml.builder b/app/views/api/ways/full.xml.builder
deleted file mode 100644 (file)
index 0252916..0000000
--- a/app/views/api/ways/full.xml.builder
+++ /dev/null
@@ -1,6 +0,0 @@
-xml.instruct!
-
-xml.osm(OSM::API.new.xml_root_attributes) do |osm|
-  osm << (render(@nodes) || "")
-  osm << (render(@way) || "")
-end

diff --git a/app/views/api/ways/show.json.jbuilder b/app/views/api/ways/show.json.jbuilder
index 92e570d869f300204bb8d92eb9a20190b242f4de..adf8beab2d3d1b2196a6281a6b68f0ed9a4a2ab5 100644 (file)
--- a/app/views/api/ways/show.json.jbuilder
+++ b/app/views/api/ways/show.json.jbuilder
@@ -1,5 +1,6 @@
 json.partial! "api/root_attributes"
 
 json.elements do
+  json.array! @nodes, :partial => "/api/nodes/node", :as => :node if @nodes
   json.array! [@way], :partial => "way", :as => :way
 end

diff --git a/app/views/api/ways/show.xml.builder b/app/views/api/ways/show.xml.builder
index d520a08444376f96f25a5eb93099fb43e70bd526..72b22e7374c97a6b56e06a7228085e5ec95c29ff 100644 (file)
--- a/app/views/api/ways/show.xml.builder
+++ b/app/views/api/ways/show.xml.builder
@@ -1,5 +1,6 @@
 xml.instruct!
 
 xml.osm(OSM::API.new.xml_root_attributes) do |osm|
+  osm << (render(@nodes) || "") if @nodes
   osm << (render(@way) || "")
 end

diff --git a/config/routes.rb b/config/routes.rb
index b562ca9f4a0c60b6d1984b5db21acf80aec0153a..a7631849cca6d8d58c4aab296767b6dfd8ffedd9 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -37,7 +37,6 @@ OpenStreetMap::Application.routes.draw do
     get "node/:id/:version" => "old_nodes#show", :as => :api_old_node, :id => /\d+/, :version => /\d+/
 
     get "way/:id/history" => "old_ways#history", :as => :api_way_history, :id => /\d+/
-    get "way/:id/full" => "ways#full", :as => :way_full, :id => /\d+/
     get "way/:id/relations" => "relations#relations_for_way", :as => :way_relations, :id => /\d+/
     post "way/:id/:version/redact" => "old_ways#redact", :as => :way_version_redact, :version => /\d+/, :id => /\d+/
     get "way/:id/:version" => "old_ways#show", :as => :api_old_way, :id => /\d+/, :version => /\d+/
@@ -55,7 +54,11 @@ OpenStreetMap::Application.routes.draw do
     put "node/create" => "nodes#create", :as => nil
 
     resources :ways, :only => [:index, :create]
-    resources :ways, :path => "way", :id => /\d+/, :only => [:show, :update, :destroy]
+    resources :ways, :path => "way", :id => /\d+/, :only => [:show, :update, :destroy] do
+      member do
+        get :full, :action => :show, :full => true, :as => nil
+      end
+    end
     put "way/create" => "ways#create", :as => nil
 
     resources :relations, :only => [:index, :create]

diff --git a/test/controllers/api/ways_controller_test.rb b/test/controllers/api/ways_controller_test.rb
index 2ff5e6f29f786a94b3c7080535fcc371ce9150d3..191f9a82037b14c8f2437c0e1bb5f9661953dd81 100644 (file)
--- a/test/controllers/api/ways_controller_test.rb
+++ b/test/controllers/api/ways_controller_test.rb
@@ -27,11 +27,11 @@ module Api
       )
       assert_routing(
         { :path => "/api/0.6/way/1/full", :method => :get },
-        { :controller => "api/ways", :action => "full", :id => "1" }
+        { :controller => "api/ways", :action => "show", :full => true, :id => "1" }
       )
       assert_routing(
         { :path => "/api/0.6/way/1/full.json", :method => :get },
-        { :controller => "api/ways", :action => "full", :id => "1", :format => "json" }
+        { :controller => "api/ways", :action => "show", :full => true, :id => "1", :format => "json" }
       )
       assert_routing(
         { :path => "/api/0.6/way/1", :method => :put },
@@ -136,10 +136,10 @@ module Api
 
     ##
     # check the "full" mode
-    def test_full
+    def test_show_full
       way = create(:way_with_nodes, :nodes_count => 3)
 
-      get way_full_path(way)
+      get api_way_path(way, :full => true)
 
       assert_response :success
 
@@ -154,10 +154,42 @@ module Api
       end
     end
 
-    def test_full_deleted
+    def test_show_full_json
+      way = create(:way_with_nodes, :nodes_count => 3)
+
+      get api_way_path(way, :full => true, :format => "json")
+
+      assert_response :success
+
+      # Check the way is correctly returned
+      js = ActiveSupport::JSON.decode(@response.body)
+      assert_not_nil js
+      assert_equal 4, js["elements"].count
+      js_ways = js["elements"].filter { |e| e["type"] == "way" }
+      assert_equal 1, js_ways.count
+      assert_equal way.id, js_ways[0]["id"]
+      assert_equal 1, js_ways[0]["version"]
+
+      # check that each node in the way appears once in the output as a
+      # reference and as the node element.
+      js_nodes = js["elements"].filter { |e| e["type"] == "node" }
+      assert_equal 3, js_nodes.count
+
+      way.nodes.each_with_index do |n, i|
+        assert_equal n.id, js_ways[0]["nodes"][i]
+        js_nodes_with_id = js_nodes.filter { |e| e["id"] == n.id }
+        assert_equal 1, js_nodes_with_id.count
+        assert_equal n.id, js_nodes_with_id[0]["id"]
+        assert_equal 1, js_nodes_with_id[0]["version"]
+        assert_equal n.lat, js_nodes_with_id[0]["lat"]
+        assert_equal n.lon, js_nodes_with_id[0]["lon"]
+      end
+    end
+
+    def test_show_full_deleted
       way = create(:way, :deleted)
 
-      get way_full_path(way)
+      get api_way_path(way, :full => true)
 
       assert_response :gone
     end