Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API element resources - full paths #5620

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/abilities/api_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ def initialize(token)
can :read, Tracepoint
can :read, User
can :read, Node
can [:read, :full, :ways_for_node], Way
can [:read, :full, :relations_for_node, :relations_for_way, :relations_for_relation], Relation
can [:read, :ways_for_node], Way
can [:read, :relations_for_node, :relations_for_way, :relations_for_relation], Relation
can [:history, :read], [OldNode, OldWay, OldRelation]
can :read, UserBlock

Expand Down
129 changes: 58 additions & 71 deletions app/controllers/api/relations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,64 @@ def index
end

def show
@relation = Relation.find(params[:id])
response.last_modified = @relation.timestamp
if @relation.visible
relation = Relation.find(params[:id])

response.last_modified = relation.timestamp unless params[:full]

@nodes = []
@ways = []
@relations = []

if relation.visible
if params[:full]
# with parameter :full
# returns representation of one relation object plus all its
# members, plus all nodes part of member ways

# first find the ids of nodes, ways and relations referenced by this
# relation - note that we exclude this relation just in case.

node_ids = relation.members.select { |m| m[0] == "Node" }.pluck(1)
way_ids = relation.members.select { |m| m[0] == "Way" }.pluck(1)
relation_ids = relation.members.select { |m| m[0] == "Relation" && m[1] != relation.id }.pluck(1)

# next load the relations and the ways.

relations = Relation.where(:id => relation_ids).includes(:relation_tags)
ways = Way.where(:id => way_ids).includes(:way_nodes, :way_tags)

# now additionally collect nodes referenced by ways. Note how we
# recursively evaluate ways but NOT relations.

way_node_ids = ways.collect do |way|
way.way_nodes.collect(&:node_id)
end
node_ids += way_node_ids.flatten
nodes = Node.where(:id => node_ids.uniq).includes(:node_tags)

@nodes = []
nodes.each do |node|
next unless node.visible? # should be unnecessary if data is consistent.

@nodes << node
end

ways.each do |way|
next unless way.visible? # should be unnecessary if data is consistent.

@ways << way
end

relations.each do |rel|
next unless rel.visible? # should be unnecessary if data is consistent.

@relations << rel
end
end

# finally add self
@relations << relation

# Render the result
respond_to do |format|
format.xml
Expand Down Expand Up @@ -68,74 +123,6 @@ def destroy
end
end

# -----------------------------------------------------------------
# full
#
# input parameters: id
#
# returns XML representation of one relation object plus all its
# members, plus all nodes part of member ways
# -----------------------------------------------------------------
def full
relation = Relation.find(params[:id])

if relation.visible

# first find the ids of nodes, ways and relations referenced by this
# relation - note that we exclude this relation just in case.

node_ids = relation.members.select { |m| m[0] == "Node" }.pluck(1)
way_ids = relation.members.select { |m| m[0] == "Way" }.pluck(1)
relation_ids = relation.members.select { |m| m[0] == "Relation" && m[1] != relation.id }.pluck(1)

# next load the relations and the ways.

relations = Relation.where(:id => relation_ids).includes(:relation_tags)
ways = Way.where(:id => way_ids).includes(:way_nodes, :way_tags)

# now additionally collect nodes referenced by ways. Note how we
# recursively evaluate ways but NOT relations.

way_node_ids = ways.collect do |way|
way.way_nodes.collect(&:node_id)
end
node_ids += way_node_ids.flatten
nodes = Node.where(:id => node_ids.uniq).includes(:node_tags)

@nodes = []
nodes.each do |node|
next unless node.visible? # should be unnecessary if data is consistent.

@nodes << node
end

@ways = []
ways.each do |way|
next unless way.visible? # should be unnecessary if data is consistent.

@ways << way
end

@relations = []
relations.each do |rel|
next unless rel.visible? # should be unnecessary if data is consistent.

@relations << rel
end

# finally add self
@relations << relation

# Render the result
respond_to do |format|
format.xml
format.json
end
else
head :gone
end
end

def relations_for_way
relations_for_object("Way")
end
Expand Down
35 changes: 12 additions & 23 deletions app/controllers/api/ways_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,21 @@ def index
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
Expand Down Expand Up @@ -72,26 +81,6 @@ def destroy
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
Expand Down
7 changes: 0 additions & 7 deletions app/views/api/relations/full.json.jbuilder

This file was deleted.

7 changes: 0 additions & 7 deletions app/views/api/relations/full.xml.builder

This file was deleted.

4 changes: 3 additions & 1 deletion app/views/api/relations/show.json.jbuilder
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
json.partial! "api/root_attributes"

json.elements do
json.array! [@relation], :partial => "relation", :as => :relation
json.array! @nodes, :partial => "/api/nodes/node", :as => :node
json.array! @ways, :partial => "/api/ways/way", :as => :way
json.array! @relations, :partial => "/api/relations/relation", :as => :relation
end
4 changes: 3 additions & 1 deletion app/views/api/relations/show.xml.builder
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
xml.instruct!

xml.osm(OSM::API.new.xml_root_attributes) do |osm|
osm << (render(@relation) || "")
osm << (render(@nodes) || "")
osm << (render(@ways) || "")
osm << (render(@relations) || "")
end
6 changes: 0 additions & 6 deletions app/views/api/ways/full.json.jbuilder

This file was deleted.

6 changes: 0 additions & 6 deletions app/views/api/ways/full.xml.builder

This file was deleted.

1 change: 1 addition & 0 deletions app/views/api/ways/show.json.jbuilder
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions app/views/api/ways/show.xml.builder
Original file line number Diff line number Diff line change
@@ -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
14 changes: 10 additions & 4 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,12 @@
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+/

get "relation/:id/relations" => "relations#relations_for_relation", :as => :relation_relations, :id => /\d+/
get "relation/:id/history" => "old_relations#history", :as => :api_relation_history, :id => /\d+/
get "relation/:id/full" => "relations#full", :as => :relation_full, :id => /\d+/
post "relation/:id/:version/redact" => "old_relations#redact", :as => :relation_version_redact, :version => /\d+/, :id => /\d+/
get "relation/:id/:version" => "old_relations#show", :as => :api_old_relation, :id => /\d+/, :version => /\d+/
end
Expand All @@ -55,11 +53,19 @@
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]
resources :relations, :path => "relation", :id => /\d+/, :only => [:show, :update, :destroy]
resources :relations, :path => "relation", :id => /\d+/, :only => [:show, :update, :destroy] do
member do
get :full, :action => :show, :full => true, :as => nil
end
end
put "relation/create" => "relations#create", :as => nil

resource :map, :only => :show
Expand Down
38 changes: 31 additions & 7 deletions test/controllers/api/relations_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ def test_routes
)
assert_routing(
{ :path => "/api/0.6/relation/1/full", :method => :get },
{ :controller => "api/relations", :action => "full", :id => "1" }
{ :controller => "api/relations", :action => "show", :full => true, :id => "1" }
)
assert_routing(
{ :path => "/api/0.6/relation/1/full.json", :method => :get },
{ :controller => "api/relations", :action => "full", :id => "1", :format => "json" }
{ :controller => "api/relations", :action => "show", :full => true, :id => "1", :format => "json" }
)
assert_routing(
{ :path => "/api/0.6/relation/1", :method => :put },
Expand Down Expand Up @@ -149,19 +149,19 @@ def test_show
end

def test_full_not_found
get relation_full_path(999999)
get api_relation_path(999999, :full => true)
assert_response :not_found
end

def test_full_deleted
get relation_full_path(create(:relation, :deleted))
get api_relation_path(create(:relation, :deleted), :full => true)
assert_response :gone
end

def test_full_empty
relation = create(:relation)

get relation_full_path(relation)
get api_relation_path(relation, :full => true)

assert_response :success
assert_dom "relation", :count => 1 do
Expand All @@ -174,7 +174,7 @@ def test_full_with_node_member
node = create(:node)
create(:relation_member, :relation => relation, :member => node)

get relation_full_path(relation)
get api_relation_path(relation, :full => true)

assert_response :success
assert_dom "node", :count => 1 do
Expand All @@ -190,7 +190,7 @@ def test_full_with_way_member
way = create(:way_with_nodes)
create(:relation_member, :relation => relation, :member => way)

get relation_full_path(relation)
get api_relation_path(relation, :full => true)

assert_response :success
assert_dom "node", :count => 1 do
Expand All @@ -204,6 +204,30 @@ def test_full_with_way_member
end
end

def test_full_with_node_member_json
relation = create(:relation)
node = create(:node)
create(:relation_member, :relation => relation, :member => node)

get api_relation_path(relation, :full => true, :format => "json")

assert_response :success
js = ActiveSupport::JSON.decode(@response.body)
assert_not_nil js
assert_equal 2, js["elements"].count

js_relations = js["elements"].filter { |e| e["type"] == "relation" }
assert_equal 1, js_relations.count
assert_equal relation.id, js_relations[0]["id"]
assert_equal 1, js_relations[0]["members"].count
assert_equal "node", js_relations[0]["members"][0]["type"]
assert_equal node.id, js_relations[0]["members"][0]["ref"]

js_nodes = js["elements"].filter { |e| e["type"] == "node" }
assert_equal 1, js_nodes.count
assert_equal node.id, js_nodes[0]["id"]
end

##
# check that all relations containing a particular node, and no extra
# relations, are returned from the relations_for_node call.
Expand Down
Loading