From: Tom Hughes Date: Sun, 9 Feb 2025 15:09:45 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/5633' X-Git-Tag: live~344 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/974e404a6e21e953354c38fc71338deb259f13ed?hp=8130b38368baec913df53a63f14a9e4af1a31a75 Merge remote-tracking branch 'upstream/pull/5633' --- diff --git a/.rubocop.yml b/.rubocop.yml index 68a7ca003..946dace60 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -129,13 +129,10 @@ Rails/SpecificActionNames: # This is a todo list, but is currently too long for `rubocop --auto-gen-config` - 'app/controllers/api/changeset_comments_controller.rb' - 'app/controllers/api/changesets_controller.rb' - - 'app/controllers/api/nodes_controller.rb' - 'app/controllers/api/notes_controller.rb' - 'app/controllers/api/old_elements_controller.rb' - - 'app/controllers/api/relations_controller.rb' - 'app/controllers/api/user_preferences_controller.rb' - 'app/controllers/api/users_controller.rb' - - 'app/controllers/api/ways_controller.rb' - 'app/controllers/browse_controller.rb' - 'app/controllers/changesets_controller.rb' - 'app/controllers/confirmations_controller.rb' diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 000000000..8a3d341d4 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,5 @@ +{ + "eslint.options": { + "overrideConfigFile": "config/eslint.js" + } +} diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index 97b461b1f..55476ab53 100644 --- a/app/abilities/api_ability.rb +++ b/app/abilities/api_ability.rb @@ -15,9 +15,7 @@ class ApiAbility can [:read, :download], Changeset can :read, Tracepoint can :read, User - can :read, Node - can [:read, :ways_for_node], Way - can [:read, :relations_for_node, :relations_for_way, :relations_for_relation], Relation + can :read, [Node, Way, Relation] can [:history, :read], [OldNode, OldWay, OldRelation] can :read, UserBlock diff --git a/app/assets/javascripts/index/directions-endpoint.js b/app/assets/javascripts/index/directions-endpoint.js index 3fdae7c54..11a70c62f 100644 --- a/app/assets/javascripts/index/directions-endpoint.js +++ b/app/assets/javascripts/index/directions-endpoint.js @@ -101,8 +101,8 @@ OSM.DirectionsEndpoint = function Endpoint(map, input, iconUrl, dragCallback, ch }; function getGeocode() { - var viewbox = map.getBounds().toBBoxString(); // ,,, - var geocodeUrl = OSM.NOMINATIM_URL + "search?q=" + encodeURIComponent(endpoint.value) + "&format=json&viewbox=" + viewbox; + const viewbox = map.getBounds().toBBoxString(), // ,,, + geocodeUrl = OSM.NOMINATIM_URL + "search?" + new URLSearchParams({ q: endpoint.value, format: "json", viewbox }); endpoint.geocodeRequest = $.getJSON(geocodeUrl, function (json) { delete endpoint.geocodeRequest; @@ -123,8 +123,9 @@ OSM.DirectionsEndpoint = function Endpoint(map, input, iconUrl, dragCallback, ch } function getReverseGeocode() { - var latlng = endpoint.latlng.clone(); - var reverseGeocodeUrl = OSM.NOMINATIM_URL + "reverse?lat=" + latlng.lat + "&lon=" + latlng.lng + "&format=json"; + const latlng = endpoint.latlng.clone(), + { lat, lng } = latlng, + reverseGeocodeUrl = OSM.NOMINATIM_URL + "reverse?" + new URLSearchParams({ lat, lon: lng, format: "json" }); endpoint.geocodeRequest = $.getJSON(reverseGeocodeUrl, function (json) { delete endpoint.geocodeRequest; diff --git a/app/assets/javascripts/index/directions/fossgis_osrm.js b/app/assets/javascripts/index/directions/fossgis_osrm.js index bd44be662..bb968f2da 100644 --- a/app/assets/javascripts/index/directions/fossgis_osrm.js +++ b/app/assets/javascripts/index/directions/fossgis_osrm.js @@ -3,111 +3,104 @@ (function () { function FOSSGISOSRMEngine(id, vehicleType) { - var cachedHints = []; + let cachedHints = []; + + function _processDirections(route) { + const INSTRUCTION_TEMPLATE = { + "continue": "continue", + "merge right": "merge_right", + "merge left": "merge_left", + "off ramp right": "offramp_right", + "off ramp left": "offramp_left", + "on ramp right": "onramp_right", + "on ramp left": "onramp_left", + "fork right": "fork_right", + "fork left": "fork_left", + "end of road right": "endofroad_right", + "end of road left": "endofroad_left", + "turn straight": "continue", + "turn slight right": "slight_right", + "turn right": "turn_right", + "turn sharp right": "sharp_right", + "turn uturn": "uturn", + "turn sharp left": "sharp_left", + "turn left": "turn_left", + "turn slight left": "slight_left", + "roundabout": "roundabout", + "rotary": "roundabout", + "exit roundabout": "exit_roundabout", + "exit rotary": "exit_roundabout", + "depart": "start", + "arrive": "destination" + }; + const ICON_MAP = { + "continue": 0, + "merge right": 21, + "merge left": 20, + "off ramp right": 24, + "off ramp left": 25, + "on ramp right": 2, + "on ramp left": 6, + "fork right": 18, + "fork left": 19, + "end of road right": 22, + "end of road left": 23, + "turn straight": 0, + "turn slight right": 1, + "turn right": 2, + "turn sharp right": 3, + "turn uturn": 4, + "turn slight left": 5, + "turn left": 6, + "turn sharp left": 7, + "roundabout": 10, + "rotary": 10, + "exit roundabout": 10, + "exit rotary": 10, + "depart": 8, + "arrive": 14 + }; + function numToWord(num) { + return ["first", "second", "third", "fourth", "fifth", "sixth", "seventh", "eighth", "ninth", "tenth"][num - 1]; + } + function getManeuverId(maneuver) { + // special case handling + switch (maneuver.type) { + case "on ramp": + case "off ramp": + case "merge": + case "end of road": + case "fork": + return maneuver.type + " " + (maneuver.modifier.indexOf("left") >= 0 ? "left" : "right"); + case "depart": + case "arrive": + case "roundabout": + case "rotary": + case "exit roundabout": + case "exit rotary": + return maneuver.type; + case "roundabout turn": + case "turn": + return "turn " + maneuver.modifier; + // for unknown types the fallback is turn + default: + return "turn " + maneuver.modifier; + } + } - return { - id: id, - creditline: "OSRM (FOSSGIS)", - draggable: true, + const steps = route.legs.flatMap( + leg => leg.steps.map(function (step, idx) { + const maneuver_id = getManeuverId(step.maneuver); - _transformSteps: function (input_steps, line) { - var INSTRUCTION_TEMPLATE = { - "continue": "javascripts.directions.instructions.continue", - "merge right": "javascripts.directions.instructions.merge_right", - "merge left": "javascripts.directions.instructions.merge_left", - "off ramp right": "javascripts.directions.instructions.offramp_right", - "off ramp left": "javascripts.directions.instructions.offramp_left", - "on ramp right": "javascripts.directions.instructions.onramp_right", - "on ramp left": "javascripts.directions.instructions.onramp_left", - "fork right": "javascripts.directions.instructions.fork_right", - "fork left": "javascripts.directions.instructions.fork_left", - "end of road right": "javascripts.directions.instructions.endofroad_right", - "end of road left": "javascripts.directions.instructions.endofroad_left", - "turn straight": "javascripts.directions.instructions.continue", - "turn slight right": "javascripts.directions.instructions.slight_right", - "turn right": "javascripts.directions.instructions.turn_right", - "turn sharp right": "javascripts.directions.instructions.sharp_right", - "turn uturn": "javascripts.directions.instructions.uturn", - "turn sharp left": "javascripts.directions.instructions.sharp_left", - "turn left": "javascripts.directions.instructions.turn_left", - "turn slight left": "javascripts.directions.instructions.slight_left", - "roundabout": "javascripts.directions.instructions.roundabout", - "rotary": "javascripts.directions.instructions.roundabout", - "exit roundabout": "javascripts.directions.instructions.exit_roundabout", - "exit rotary": "javascripts.directions.instructions.exit_roundabout", - "depart": "javascripts.directions.instructions.start", - "arrive": "javascripts.directions.instructions.destination" - }; - var ICON_MAP = { - "continue": 0, - "merge right": 21, - "merge left": 20, - "off ramp right": 24, - "off ramp left": 25, - "on ramp right": 2, - "on ramp left": 6, - "fork right": 18, - "fork left": 19, - "end of road right": 22, - "end of road left": 23, - "turn straight": 0, - "turn slight right": 1, - "turn right": 2, - "turn sharp right": 3, - "turn uturn": 4, - "turn slight left": 5, - "turn left": 6, - "turn sharp left": 7, - "roundabout": 10, - "rotary": 10, - "exit roundabout": 10, - "exit rotary": 10, - "depart": 8, - "arrive": 14 - }; - var numToWord = function (num) { - return ["first", "second", "third", "fourth", "fifth", "sixth", "seventh", "eighth", "ninth", "tenth"][num - 1]; - }; - var transformed_steps = input_steps.map(function (step, idx) { - var maneuver_id; - - // special case handling - switch (step.maneuver.type) { - case "on ramp": - case "off ramp": - case "merge": - case "end of road": - case "fork": - maneuver_id = step.maneuver.type + " " + (step.maneuver.modifier.indexOf("left") >= 0 ? "left" : "right"); - break; - case "depart": - case "arrive": - case "roundabout": - case "rotary": - case "exit roundabout": - case "exit rotary": - maneuver_id = step.maneuver.type; - break; - case "roundabout turn": - case "turn": - maneuver_id = "turn " + step.maneuver.modifier; - break; - // for unknown types the fallback is turn - default: - maneuver_id = "turn " + step.maneuver.modifier; - break; - } - var template = INSTRUCTION_TEMPLATE[maneuver_id]; + const instrPrefix = "javascripts.directions.instructions."; + let template = instrPrefix + INSTRUCTION_TEMPLATE[maneuver_id]; - // convert lat,lng pairs to LatLng objects - var step_geometry = L.PolylineUtil.decode(step.geometry, { precision: 5 }).map(function (a) { return L.latLng(a); }); - // append step_geometry on line - Array.prototype.push.apply(line, step_geometry); + const step_geometry = L.PolylineUtil.decode(step.geometry, { precision: 5 }).map(L.latLng); - var instText = "" + (idx + 1) + ". "; - var destinations = "" + step.destinations + ""; - var namedRoad = true; - var name; + let instText = "" + (idx + 1) + ". "; + const destinations = "" + step.destinations + ""; + let namedRoad = true; + let name; if (step.name && step.ref) { name = "" + step.name + " (" + step.ref + ")"; @@ -116,7 +109,7 @@ } else if (step.ref) { name = "" + step.ref + ""; } else { - name = I18n.t("javascripts.directions.instructions.unnamed"); + name = I18n.t(instrPrefix + "unnamed"); namedRoad = false; } @@ -125,7 +118,7 @@ } else if (step.maneuver.type.match(/^(rotary|roundabout)$/)) { if (step.maneuver.exit) { if (step.maneuver.exit <= 10) { - instText += I18n.t(template + "_with_exit_ordinal", { exit: I18n.t("javascripts.directions.instructions.exit_counts." + numToWord(step.maneuver.exit)), name: name }); + instText += I18n.t(template + "_with_exit_ordinal", { exit: I18n.t(instrPrefix + "exit_counts." + numToWord(step.maneuver.exit)), name: name }); } else { instText += I18n.t(template + "_with_exit", { exit: step.maneuver.exit, name: name }); } @@ -133,7 +126,7 @@ instText += I18n.t(template + "_without_exit", { name: name }); } } else if (step.maneuver.type.match(/^(on ramp|off ramp)$/)) { - var params = {}; + const params = {}; if (step.exits && step.maneuver.type.match(/^(off ramp)$/)) params.exit = step.exits; if (step.destinations) params.directions = destinations; if (namedRoad) params.directions = name; @@ -145,61 +138,50 @@ instText += I18n.t(template + "_without_exit", { name: name }); } return [[step.maneuver.location[1], step.maneuver.location[0]], ICON_MAP[maneuver_id], instText, step.distance, step_geometry]; - }); + }) + ); - return transformed_steps; - }, + return { + line: steps.flatMap(step => step[4]), + steps, + distance: route.distance, + time: route.duration + }; + } + + return { + id: id, + creditline: "OSRM (FOSSGIS)", + draggable: true, getRoute: function (points, callback) { - var params = [ + const data = [ { name: "overview", value: "false" }, { name: "geometries", value: "polyline" }, { name: "steps", value: true } ]; - if (cachedHints.length === points.length) { - params.push({ name: "hints", value: cachedHints.join(";") }); + data.push({ name: "hints", value: cachedHints.join(";") }); } else { - // invalidate cache + // invalidate cache cachedHints = []; } - var encoded_coords = points.map(function (p) { - return p.lng + "," + p.lat; - }).join(";"); - - var req_url = OSM.FOSSGIS_OSRM_URL + "routed-" + vehicleType + "/route/v1/driving/" + encoded_coords; - - var onResponse = function (data) { - if (data.code !== "Ok") { - return callback(true); - } - - cachedHints = data.waypoints.map(function (wp) { - return wp.hint; - }); - - var line = []; - var transformLeg = function (leg) { - return this._transformSteps(leg.steps, line); - }; - - var steps = [].concat.apply([], data.routes[0].legs.map(transformLeg.bind(this))); - - callback(false, { - line: line, - steps: steps, - distance: data.routes[0].distance, - time: data.routes[0].duration - }); - }; + const req_path = "routed-" + vehicleType + "/route/v1/driving/" + points.map(p => p.lng + "," + p.lat).join(";"); return $.ajax({ - url: req_url, - data: params, + url: OSM.FOSSGIS_OSRM_URL + req_path, + data, dataType: "json", - success: onResponse.bind(this), + success: function (response) { + if (response.code !== "Ok") { + return callback(true); + } + + cachedHints = response.waypoints.map(wp => wp.hint); + callback(false, _processDirections(response.routes[0])); + }, error: function () { callback(true); } diff --git a/app/assets/javascripts/index/directions/fossgis_valhalla.js b/app/assets/javascripts/index/directions/fossgis_valhalla.js index bc093fea4..bbccccb13 100644 --- a/app/assets/javascripts/index/directions/fossgis_valhalla.js +++ b/app/assets/javascripts/index/directions/fossgis_valhalla.js @@ -1,6 +1,6 @@ (function () { function FOSSGISValhallaEngine(id, costing) { - var INSTR_MAP = [ + const INSTR_MAP = [ 0, // kNone = 0; 8, // kStart = 1; 8, // kStartRight = 2; @@ -42,6 +42,45 @@ 20 // kMergeLeft = 38; ]; + function _processDirections(tripLegs) { + let line = []; + let steps = []; + let distance = 0; + let time = 0; + + for (const leg of tripLegs) { + const legLine = L.PolylineUtil.decode(leg.shape, { + precision: 6 + }); + + const legSteps = leg.maneuvers.map(function (manoeuvre, idx) { + const num = `${idx + 1}. `; + const lineseg = legLine + .slice(manoeuvre.begin_shape_index, manoeuvre.end_shape_index + 1) + .map(([lat, lng]) => ({ lat, lng })); + return [ + lineseg[0], + INSTR_MAP[manoeuvre.type], + num + manoeuvre.instruction, + manoeuvre.length * 1000, + lineseg + ]; + }); + + line = line.concat(legLine); + steps = steps.concat(legSteps); + distance += leg.summary.length; + time += leg.summary.time; + } + + return { + line: line, + steps: steps, + distance: distance * 1000, + time: time + }; + } + return { id: id, creditline: @@ -49,59 +88,25 @@ draggable: false, getRoute: function (points, callback) { + const data = { + json: JSON.stringify({ + locations: points.map(function (p) { + return { lat: p.lat, lon: p.lng, radius: 5 }; + }), + costing: costing, + directions_options: { + units: "km", + language: I18n.currentLocale() + } + }) + }; return $.ajax({ url: OSM.FOSSGIS_VALHALLA_URL, - data: { - json: JSON.stringify({ - locations: points.map(function (p) { - return { lat: p.lat, lon: p.lng, radius: 5 }; - }), - costing: costing, - directions_options: { - units: "km", - language: I18n.currentLocale() - } - }) - }, + data, dataType: "json", - success: function (data) { - var trip = data.trip; - + success: function ({ trip }) { if (trip.status === 0) { - var line = []; - var steps = []; - var distance = 0; - var time = 0; - - trip.legs.forEach(function (leg) { - var legLine = L.PolylineUtil.decode(leg.shape, { - precision: 6 - }); - - line = line.concat(legLine); - - leg.maneuvers.forEach(function (manoeuvre, idx) { - var point = legLine[manoeuvre.begin_shape_index]; - - steps.push([ - { lat: point[0], lng: point[1] }, - INSTR_MAP[manoeuvre.type], - "" + (idx + 1) + ". " + manoeuvre.instruction, - manoeuvre.length * 1000, - [] - ]); - }); - - distance = distance + leg.summary.length; - time = time + leg.summary.time; - }); - - callback(false, { - line: line, - steps: steps, - distance: distance * 1000, - time: time - }); + callback(false, _processDirections(trip.legs)); } else { callback(true); } diff --git a/app/assets/javascripts/index/directions/graphhopper.js b/app/assets/javascripts/index/directions/graphhopper.js index 021fc7b64..191475873 100644 --- a/app/assets/javascripts/index/directions/graphhopper.js +++ b/app/assets/javascripts/index/directions/graphhopper.js @@ -1,6 +1,6 @@ (function () { function GraphHopperEngine(id, vehicleType) { - var GH_INSTR_MAP = { + const GH_INSTR_MAP = { "-3": 7, // sharp left "-2": 6, // left "-1": 5, // slight left @@ -18,65 +18,61 @@ "8": 4 // right u-turn }; + function _processDirections(path) { + const line = L.PolylineUtil.decode(path.points); + + const steps = path.instructions.map(function (instr, i) { + const num = `${i + 1}. `; + const lineseg = line + .slice(instr.interval[0], instr.interval[1] + 1) + .map(([lat, lng]) => ({ lat, lng })); + return [ + lineseg[0], + GH_INSTR_MAP[instr.sign], + num + instr.text, + instr.distance, + lineseg + ]; // TODO does graphhopper map instructions onto line indices? + }); + steps.at(-1)[1] = 14; + + return { + line: line, + steps: steps, + distance: path.distance, + time: path.time / 1000, + ascend: path.ascend, + descend: path.descend + }; + } + return { id: id, creditline: "GraphHopper", draggable: false, getRoute: function (points, callback) { - // GraphHopper Directions API documentation - // https://graphhopper.com/api/1/docs/routing/ + // GraphHopper Directions API documentation + // https://graphhopper.com/api/1/docs/routing/ + const data = { + vehicle: vehicleType, + locale: I18n.currentLocale(), + key: "LijBPDQGfu7Iiq80w3HzwB4RUDJbMbhs6BU0dEnn", + elevation: false, + instructions: true, + turn_costs: vehicleType === "car", + point: points.map(p => p.lat + "," + p.lng) + }; return $.ajax({ url: OSM.GRAPHHOPPER_URL, - data: { - vehicle: vehicleType, - locale: I18n.currentLocale(), - key: "LijBPDQGfu7Iiq80w3HzwB4RUDJbMbhs6BU0dEnn", - elevation: false, - instructions: true, - turn_costs: vehicleType === "car", - point: points.map(function (p) { return p.lat + "," + p.lng; }) - }, + data, traditional: true, dataType: "json", - success: function (data) { - if (!data.paths || data.paths.length === 0) { + success: function ({ paths }) { + if (!paths || paths.length === 0) { return callback(true); } - - var path = data.paths[0]; - var line = L.PolylineUtil.decode(path.points); - - var steps = []; - var len = path.instructions.length; - for (var i = 0; i < len; i++) { - var instr = path.instructions[i]; - var instrCode = (i === len - 1) ? 14 : GH_INSTR_MAP[instr.sign]; - var instrText = "" + (i + 1) + ". "; - instrText += instr.text; - var latLng = line[instr.interval[0]]; - var distInMeter = instr.distance; - var lineseg = []; - for (var j = instr.interval[0]; j <= instr.interval[1]; j++) { - lineseg.push({ lat: line[j][0], lng: line[j][1] }); - } - steps.push([ - { lat: latLng[0], lng: latLng[1] }, - instrCode, - instrText, - distInMeter, - lineseg - ]); // TODO does graphhopper map instructions onto line indices? - } - - callback(false, { - line: line, - steps: steps, - distance: path.distance, - time: path.time / 1000, - ascend: path.ascend, - descend: path.descend - }); + callback(false, _processDirections(paths[0])); }, error: function () { callback(true); diff --git a/app/assets/javascripts/index/search.js b/app/assets/javascripts/index/search.js index e54c5132c..23af46145 100644 --- a/app/assets/javascripts/index/search.js +++ b/app/assets/javascripts/index/search.js @@ -10,22 +10,18 @@ OSM.Search = function (map) { $(".search_form a.btn.switch_link").on("click", function (e) { e.preventDefault(); var query = $(this).closest("form").find("input[name=query]").val(); - if (query) { - OSM.router.route("/directions?from=" + encodeURIComponent(query) + OSM.formatHash(map)); - } else { - OSM.router.route("/directions" + OSM.formatHash(map)); - } + let search = ""; + if (query) search = "?" + new URLSearchParams({ from: query }); + OSM.router.route("/directions" + search + OSM.formatHash(map)); }); $(".search_form").on("submit", function (e) { e.preventDefault(); $("header").addClass("closed"); var query = $(this).find("input[name=query]").val(); - if (query) { - OSM.router.route("/search?query=" + encodeURIComponent(query) + OSM.formatHash(map)); - } else { - OSM.router.route("/" + OSM.formatHash(map)); - } + let search = "/"; + if (query) search = "/search?" + new URLSearchParams({ query }); + OSM.router.route(search + OSM.formatHash(map)); }); $(".describe_location").on("click", function (e) { diff --git a/app/assets/javascripts/leaflet.share.js b/app/assets/javascripts/leaflet.share.js index 7d8462f46..e89608afd 100644 --- a/app/assets/javascripts/leaflet.share.js +++ b/app/assets/javascripts/leaflet.share.js @@ -349,14 +349,14 @@ L.OSM.share = function (options) { $("#short_link").attr("href", map.getShortUrl(marker)); $("#long_link").attr("href", map.getUrl(marker)); - var params = { + const params = new URLSearchParams({ bbox: bounds.toBBoxString(), layer: map.getMapBaseLayerId() - }; + }); if (map.hasLayer(marker)) { var latLng = marker.getLatLng().wrap(); - params.marker = latLng.lat + "," + latLng.lng; + params.set("marker", latLng.lat + "," + latLng.lng); } $("#embed_link") @@ -369,7 +369,7 @@ L.OSM.share = function (options) { $("#embed_html").val( "
" + "" + escapeHTML(I18n.t("javascripts.share.view_larger_map")) + ""); diff --git a/app/controllers/api/nodes/relations_controller.rb b/app/controllers/api/nodes/relations_controller.rb new file mode 100644 index 000000000..0f0409e19 --- /dev/null +++ b/app/controllers/api/nodes/relations_controller.rb @@ -0,0 +1,25 @@ +module Api + module Nodes + class RelationsController < ApiController + authorize_resource + + before_action :set_request_formats + + def index + relation_ids = RelationMember.where(:member_type => "Node", :member_id => params[:node_id]).collect(&:relation_id).uniq + + @relations = [] + + Relation.find(relation_ids).each do |relation| + @relations << relation if relation.visible + end + + # Render the result + respond_to do |format| + format.xml + format.json + end + end + end + end +end diff --git a/app/controllers/api/nodes/ways_controller.rb b/app/controllers/api/nodes/ways_controller.rb new file mode 100644 index 000000000..2a71f1903 --- /dev/null +++ b/app/controllers/api/nodes/ways_controller.rb @@ -0,0 +1,25 @@ +module Api + module Nodes + class WaysController < ApiController + authorize_resource + + before_action :set_request_formats + + ## + # returns all the ways which are currently using the node given in the + # :node_id parameter. note that this used to return deleted ways as well, but + # this seemed not to be the expected behaviour, so it was removed. + def index + way_ids = WayNode.where(:node_id => params[:node_id]).collect { |ws| ws.id[0] }.uniq + + @ways = Way.where(:id => way_ids, :visible => true) + + # Render the result + respond_to do |format| + format.xml + format.json + end + end + end + end +end diff --git a/app/controllers/api/relations/relations_controller.rb b/app/controllers/api/relations/relations_controller.rb new file mode 100644 index 000000000..1769e1396 --- /dev/null +++ b/app/controllers/api/relations/relations_controller.rb @@ -0,0 +1,25 @@ +module Api + module Relations + class RelationsController < ApiController + authorize_resource + + before_action :set_request_formats + + def index + relation_ids = RelationMember.where(:member_type => "Relation", :member_id => params[:relation_id]).collect(&:relation_id).uniq + + @relations = [] + + Relation.find(relation_ids).each do |relation| + @relations << relation if relation.visible + end + + # Render the result + respond_to do |format| + format.xml + format.json + end + end + end + end +end diff --git a/app/controllers/api/relations_controller.rb b/app/controllers/api/relations_controller.rb index 006b3e8a6..3181a5df1 100644 --- a/app/controllers/api/relations_controller.rb +++ b/app/controllers/api/relations_controller.rb @@ -122,35 +122,5 @@ module Api head :bad_request end end - - def relations_for_way - relations_for_object("Way") - end - - def relations_for_node - relations_for_object("Node") - end - - def relations_for_relation - relations_for_object("Relation") - end - - private - - def relations_for_object(objtype) - relationids = RelationMember.where(:member_type => objtype, :member_id => params[:id]).collect(&:relation_id).uniq - - @relations = [] - - Relation.find(relationids).each do |relation| - @relations << relation if relation.visible - end - - # Render the result - respond_to do |format| - format.xml - format.json - end - end end end diff --git a/app/controllers/api/ways/relations_controller.rb b/app/controllers/api/ways/relations_controller.rb new file mode 100644 index 000000000..fcd8b40dd --- /dev/null +++ b/app/controllers/api/ways/relations_controller.rb @@ -0,0 +1,25 @@ +module Api + module Ways + class RelationsController < ApiController + authorize_resource + + before_action :set_request_formats + + def index + relation_ids = RelationMember.where(:member_type => "Way", :member_id => params[:way_id]).collect(&:relation_id).uniq + + @relations = [] + + Relation.find(relation_ids).each do |relation| + @relations << relation if relation.visible + end + + # Render the result + respond_to do |format| + format.xml + format.json + end + end + end + end +end diff --git a/app/controllers/api/ways_controller.rb b/app/controllers/api/ways_controller.rb index b1bc8d799..2fd86b997 100644 --- a/app/controllers/api/ways_controller.rb +++ b/app/controllers/api/ways_controller.rb @@ -80,21 +80,5 @@ module Api head :bad_request 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 - # this seemed not to be the expected behaviour, so it was removed. - def ways_for_node - wayids = WayNode.where(:node_id => params[:id]).collect { |ws| ws.id[0] }.uniq - - @ways = Way.where(:id => wayids, :visible => true) - - # Render the result - respond_to do |format| - format.xml - format.json - end - end end end diff --git a/app/views/api/nodes/relations/index.json.jbuilder b/app/views/api/nodes/relations/index.json.jbuilder new file mode 100644 index 000000000..1d8b9f2cd --- /dev/null +++ b/app/views/api/nodes/relations/index.json.jbuilder @@ -0,0 +1,5 @@ +json.partial! "api/root_attributes" + +json.elements do + json.array! @relations, :partial => "api/relations/relation", :as => :relation +end diff --git a/app/views/api/nodes/relations/index.xml.builder b/app/views/api/nodes/relations/index.xml.builder new file mode 100644 index 000000000..efe64f159 --- /dev/null +++ b/app/views/api/nodes/relations/index.xml.builder @@ -0,0 +1,5 @@ +xml.instruct! + +xml.osm(OSM::API.new.xml_root_attributes) do |osm| + osm << (render(:partial => "api/relations/relation", :collection => @relations) || "") +end diff --git a/app/views/api/nodes/ways/index.json.jbuilder b/app/views/api/nodes/ways/index.json.jbuilder new file mode 100644 index 000000000..27119cad5 --- /dev/null +++ b/app/views/api/nodes/ways/index.json.jbuilder @@ -0,0 +1,5 @@ +json.partial! "api/root_attributes" + +json.elements do + json.array! @ways, :partial => "api/ways/way", :as => :way +end diff --git a/app/views/api/nodes/ways/index.xml.builder b/app/views/api/nodes/ways/index.xml.builder new file mode 100644 index 000000000..7872d7d52 --- /dev/null +++ b/app/views/api/nodes/ways/index.xml.builder @@ -0,0 +1,5 @@ +xml.instruct! + +xml.osm(OSM::API.new.xml_root_attributes) do |osm| + osm << (render(:partial => "api/ways/way", :collection => @ways) || "") +end diff --git a/app/views/api/relations/relations/index.json.jbuilder b/app/views/api/relations/relations/index.json.jbuilder new file mode 100644 index 000000000..1d8b9f2cd --- /dev/null +++ b/app/views/api/relations/relations/index.json.jbuilder @@ -0,0 +1,5 @@ +json.partial! "api/root_attributes" + +json.elements do + json.array! @relations, :partial => "api/relations/relation", :as => :relation +end diff --git a/app/views/api/relations/relations/index.xml.builder b/app/views/api/relations/relations/index.xml.builder new file mode 100644 index 000000000..efe64f159 --- /dev/null +++ b/app/views/api/relations/relations/index.xml.builder @@ -0,0 +1,5 @@ +xml.instruct! + +xml.osm(OSM::API.new.xml_root_attributes) do |osm| + osm << (render(:partial => "api/relations/relation", :collection => @relations) || "") +end diff --git a/app/views/api/relations/relations_for_node.json.jbuilder b/app/views/api/relations/relations_for_node.json.jbuilder deleted file mode 100644 index 9b0f65bf1..000000000 --- a/app/views/api/relations/relations_for_node.json.jbuilder +++ /dev/null @@ -1,5 +0,0 @@ -json.partial! "api/root_attributes" - -json.elements do - json.array! @relations, :partial => "relation", :as => :relation -end diff --git a/app/views/api/relations/relations_for_node.xml.builder b/app/views/api/relations/relations_for_node.xml.builder deleted file mode 100644 index f39a20bb9..000000000 --- a/app/views/api/relations/relations_for_node.xml.builder +++ /dev/null @@ -1,5 +0,0 @@ -xml.instruct! - -xml.osm(OSM::API.new.xml_root_attributes) do |osm| - osm << (render(@relations) || "") -end diff --git a/app/views/api/relations/relations_for_relation.json.jbuilder b/app/views/api/relations/relations_for_relation.json.jbuilder deleted file mode 100644 index 9b0f65bf1..000000000 --- a/app/views/api/relations/relations_for_relation.json.jbuilder +++ /dev/null @@ -1,5 +0,0 @@ -json.partial! "api/root_attributes" - -json.elements do - json.array! @relations, :partial => "relation", :as => :relation -end diff --git a/app/views/api/relations/relations_for_relation.xml.builder b/app/views/api/relations/relations_for_relation.xml.builder deleted file mode 100644 index f39a20bb9..000000000 --- a/app/views/api/relations/relations_for_relation.xml.builder +++ /dev/null @@ -1,5 +0,0 @@ -xml.instruct! - -xml.osm(OSM::API.new.xml_root_attributes) do |osm| - osm << (render(@relations) || "") -end diff --git a/app/views/api/relations/relations_for_way.json.jbuilder b/app/views/api/relations/relations_for_way.json.jbuilder deleted file mode 100644 index 9b0f65bf1..000000000 --- a/app/views/api/relations/relations_for_way.json.jbuilder +++ /dev/null @@ -1,5 +0,0 @@ -json.partial! "api/root_attributes" - -json.elements do - json.array! @relations, :partial => "relation", :as => :relation -end diff --git a/app/views/api/relations/relations_for_way.xml.builder b/app/views/api/relations/relations_for_way.xml.builder deleted file mode 100644 index f39a20bb9..000000000 --- a/app/views/api/relations/relations_for_way.xml.builder +++ /dev/null @@ -1,5 +0,0 @@ -xml.instruct! - -xml.osm(OSM::API.new.xml_root_attributes) do |osm| - osm << (render(@relations) || "") -end diff --git a/app/views/api/ways/relations/index.json.jbuilder b/app/views/api/ways/relations/index.json.jbuilder new file mode 100644 index 000000000..1d8b9f2cd --- /dev/null +++ b/app/views/api/ways/relations/index.json.jbuilder @@ -0,0 +1,5 @@ +json.partial! "api/root_attributes" + +json.elements do + json.array! @relations, :partial => "api/relations/relation", :as => :relation +end diff --git a/app/views/api/ways/relations/index.xml.builder b/app/views/api/ways/relations/index.xml.builder new file mode 100644 index 000000000..efe64f159 --- /dev/null +++ b/app/views/api/ways/relations/index.xml.builder @@ -0,0 +1,5 @@ +xml.instruct! + +xml.osm(OSM::API.new.xml_root_attributes) do |osm| + osm << (render(:partial => "api/relations/relation", :collection => @relations) || "") +end diff --git a/app/views/api/ways/ways_for_node.json.jbuilder b/app/views/api/ways/ways_for_node.json.jbuilder deleted file mode 100644 index 7b8de5f1c..000000000 --- a/app/views/api/ways/ways_for_node.json.jbuilder +++ /dev/null @@ -1,5 +0,0 @@ -json.partial! "api/root_attributes" - -json.elements do - json.array! @ways, :partial => "way", :as => :way -end diff --git a/app/views/api/ways/ways_for_node.xml.builder b/app/views/api/ways/ways_for_node.xml.builder deleted file mode 100644 index bcb89cdc6..000000000 --- a/app/views/api/ways/ways_for_node.xml.builder +++ /dev/null @@ -1,5 +0,0 @@ -xml.instruct! - -xml.osm(OSM::API.new.xml_root_attributes) do |osm| - osm << (render(@ways) || "") -end diff --git a/config/initializers/eslint.rb b/config/initializers/eslint.rb deleted file mode 100644 index 9ddf4916e..000000000 --- a/config/initializers/eslint.rb +++ /dev/null @@ -1,13 +0,0 @@ -if defined?(ESLintRails::Runner) - module OpenStreetMap - module ESLintRails - module ExcludeI18n - def assets - super.reject { |a| a.to_s.include?("/i18n/") } - end - end - end - end - - ESLintRails::Runner.prepend(OpenStreetMap::ESLintRails::ExcludeI18n) -end diff --git a/config/routes.rb b/config/routes.rb index bc06e5f38..2b8632698 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -30,18 +30,14 @@ OpenStreetMap::Application.routes.draw do post "changeset/comment/:id/hide" => "changeset_comments#destroy", :as => :changeset_comment_hide, :id => /\d+/ post "changeset/comment/:id/unhide" => "changeset_comments#restore", :as => :changeset_comment_unhide, :id => /\d+/ - get "node/:id/ways" => "ways#ways_for_node", :as => :node_ways, :id => /\d+/ - get "node/:id/relations" => "relations#relations_for_node", :as => :node_relations, :id => /\d+/ get "node/:id/history" => "old_nodes#history", :as => :api_node_history, :id => /\d+/ post "node/:id/:version/redact" => "old_nodes#redact", :as => :node_version_redact, :version => /\d+/, :id => /\d+/ 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/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+/ 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+/ @@ -49,7 +45,12 @@ OpenStreetMap::Application.routes.draw do namespace :api, :path => "api/0.6" do resources :nodes, :only => [:index, :create] - resources :nodes, :path => "node", :id => /\d+/, :only => [:show, :update, :destroy] + resources :nodes, :path => "node", :id => /\d+/, :only => [:show, :update, :destroy] do + scope :module => :nodes do + resources :ways, :only => :index + resources :relations, :only => :index + end + end put "node/create" => "nodes#create", :as => nil resources :ways, :only => [:index, :create] @@ -57,6 +58,9 @@ OpenStreetMap::Application.routes.draw do member do get :full, :action => :show, :full => true, :as => nil end + scope :module => :ways do + resources :relations, :only => :index + end end put "way/create" => "ways#create", :as => nil @@ -65,6 +69,9 @@ OpenStreetMap::Application.routes.draw do member do get :full, :action => :show, :full => true, :as => nil end + scope :module => :relations do + resources :relations, :only => :index + end end put "relation/create" => "relations#create", :as => nil diff --git a/test/controllers/api/nodes/relations_controller_test.rb b/test/controllers/api/nodes/relations_controller_test.rb new file mode 100644 index 000000000..168fd7153 --- /dev/null +++ b/test/controllers/api/nodes/relations_controller_test.rb @@ -0,0 +1,75 @@ +require "test_helper" + +module Api + module Nodes + class RelationsControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/api/0.6/node/1/relations", :method => :get }, + { :controller => "api/nodes/relations", :action => "index", :node_id => "1" } + ) + assert_routing( + { :path => "/api/0.6/node/1/relations.json", :method => :get }, + { :controller => "api/nodes/relations", :action => "index", :node_id => "1", :format => "json" } + ) + end + + ## + # check that all relations containing a particular node, and no extra + # relations, are returned. + def test_index + node = create(:node) + # should include relations with that node as a member + relation_with_node = create(:relation_member, :member => node).relation + # should ignore relations without that node as a member + _relation_without_node = create(:relation_member).relation + # should ignore relations with the node involved indirectly, via a way + way = create(:way_node, :node => node).way + _relation_with_way = create(:relation_member, :member => way).relation + # should ignore relations with the node involved indirectly, via a relation + second_relation = create(:relation_member, :member => node).relation + _super_relation = create(:relation_member, :member => second_relation).relation + # should combine multiple relation_member references into just one relation entry + create(:relation_member, :member => node, :relation => relation_with_node) + # should not include deleted relations + deleted_relation = create(:relation, :deleted) + create(:relation_member, :member => node, :relation => deleted_relation) + + get api_node_relations_path(node) + + assert_response :success + + # count one osm element + assert_select "osm[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1 + + # we should have only the expected number of relations + expected_relations = [relation_with_node, second_relation] + assert_select "osm>relation", expected_relations.size + + # and each of them should contain the element we originally searched for + expected_relations.each do |containing_relation| + # The relation should appear once, but the element could appear multiple times + assert_select "osm>relation[id='#{containing_relation.id}']", 1 + assert_select "osm>relation[id='#{containing_relation.id}']>member[type='node'][ref='#{node.id}']" + end + end + + def test_index_json + node = create(:node) + containing_relation = create(:relation_member, :member => node).relation + + get api_node_relations_path(node, :format => "json") + + assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 1, js["elements"].count + js_relations = js["elements"].filter { |e| e["type"] == "relation" } + assert_equal 1, js_relations.count + assert_equal containing_relation.id, js_relations[0]["id"] + end + end + end +end diff --git a/test/controllers/api/nodes/ways_controller_test.rb b/test/controllers/api/nodes/ways_controller_test.rb new file mode 100644 index 000000000..0a728cf9a --- /dev/null +++ b/test/controllers/api/nodes/ways_controller_test.rb @@ -0,0 +1,72 @@ +require "test_helper" + +module Api + module Nodes + class WaysControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/api/0.6/node/1/ways", :method => :get }, + { :controller => "api/nodes/ways", :action => "index", :node_id => "1" } + ) + assert_routing( + { :path => "/api/0.6/node/1/ways.json", :method => :get }, + { :controller => "api/nodes/ways", :action => "index", :node_id => "1", :format => "json" } + ) + end + + ## + # test that a call to ways_for_node returns all ways that contain the node + # and none that don't. + def test_index + node = create(:node) + way1 = create(:way) + way2 = create(:way) + create(:way_node, :way => way1, :node => node) + create(:way_node, :way => way2, :node => node) + # create an unrelated way + create(:way_with_nodes, :nodes_count => 2) + # create a way which used to use the node + way3_v1 = create(:old_way, :version => 1) + _way3_v2 = create(:old_way, :current_way => way3_v1.current_way, :version => 2) + create(:old_way_node, :old_way => way3_v1, :node => node) + + get api_node_ways_path(node) + assert_response :success + ways_xml = XML::Parser.string(@response.body).parse + assert_not_nil ways_xml, "failed to parse ways_for_node response" + + # check that the set of IDs match expectations + expected_way_ids = [way1.id, + way2.id] + found_way_ids = ways_xml.find("//osm/way").collect { |w| w["id"].to_i } + assert_equal expected_way_ids.sort, found_way_ids.sort, + "expected ways for node #{node.id} did not match found" + + # check the full ways to ensure we're not missing anything + expected_way_ids.each do |id| + way_xml = ways_xml.find("//osm/way[@id='#{id}']").first + assert_ways_are_equal(Way.find(id), + Way.from_xml_node(way_xml)) + end + end + + def test_index_json + node = create(:node) + way = create(:way) + create(:way_node, :way => way, :node => node) + + get api_node_ways_path(node, :format => "json") + + assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 1, 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"] + end + end + end +end diff --git a/test/controllers/api/relations/relations_controller_test.rb b/test/controllers/api/relations/relations_controller_test.rb new file mode 100644 index 000000000..f3009b9bf --- /dev/null +++ b/test/controllers/api/relations/relations_controller_test.rb @@ -0,0 +1,69 @@ +require "test_helper" + +module Api + module Relations + class RelationsControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/api/0.6/relation/1/relations", :method => :get }, + { :controller => "api/relations/relations", :action => "index", :relation_id => "1" } + ) + assert_routing( + { :path => "/api/0.6/relation/1/relations.json", :method => :get }, + { :controller => "api/relations/relations", :action => "index", :relation_id => "1", :format => "json" } + ) + end + + def test_index + relation = create(:relation) + # should include relations with that relation as a member + relation_with_relation = create(:relation_member, :member => relation).relation + # should ignore any relation without that relation as a member + _relation_without_relation = create(:relation_member).relation + # should ignore relations with the relation involved indirectly, via a relation + second_relation = create(:relation_member, :member => relation).relation + _super_relation = create(:relation_member, :member => second_relation).relation + # should combine multiple relation_member references into just one relation entry + create(:relation_member, :member => relation, :relation => relation_with_relation) + # should not include deleted relations + deleted_relation = create(:relation, :deleted) + create(:relation_member, :member => relation, :relation => deleted_relation) + + get api_relation_relations_path(relation) + + assert_response :success + + # count one osm element + assert_select "osm[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1 + + # we should have only the expected number of relations + expected_relations = [relation_with_relation, second_relation] + assert_select "osm>relation", expected_relations.size + + # and each of them should contain the element we originally searched for + expected_relations.each do |containing_relation| + # The relation should appear once, but the element could appear multiple times + assert_select "osm>relation[id='#{containing_relation.id}']", 1 + assert_select "osm>relation[id='#{containing_relation.id}']>member[type='relation'][ref='#{relation.id}']" + end + end + + def test_index_json + relation = create(:relation) + containing_relation = create(:relation_member, :member => relation).relation + + get api_relation_relations_path(relation, :format => "json") + + assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 1, js["elements"].count + js_relations = js["elements"].filter { |e| e["type"] == "relation" } + assert_equal 1, js_relations.count + assert_equal containing_relation.id, js_relations[0]["id"] + end + end + end +end diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb index 803a57acf..5e480b69c 100644 --- a/test/controllers/api/relations_controller_test.rb +++ b/test/controllers/api/relations_controller_test.rb @@ -42,31 +42,6 @@ module Api { :controller => "api/relations", :action => "destroy", :id => "1" } ) - assert_routing( - { :path => "/api/0.6/node/1/relations", :method => :get }, - { :controller => "api/relations", :action => "relations_for_node", :id => "1" } - ) - assert_routing( - { :path => "/api/0.6/way/1/relations", :method => :get }, - { :controller => "api/relations", :action => "relations_for_way", :id => "1" } - ) - assert_routing( - { :path => "/api/0.6/relation/1/relations", :method => :get }, - { :controller => "api/relations", :action => "relations_for_relation", :id => "1" } - ) - assert_routing( - { :path => "/api/0.6/node/1/relations.json", :method => :get }, - { :controller => "api/relations", :action => "relations_for_node", :id => "1", :format => "json" } - ) - assert_routing( - { :path => "/api/0.6/way/1/relations.json", :method => :get }, - { :controller => "api/relations", :action => "relations_for_way", :id => "1", :format => "json" } - ) - assert_routing( - { :path => "/api/0.6/relation/1/relations.json", :method => :get }, - { :controller => "api/relations", :action => "relations_for_relation", :id => "1", :format => "json" } - ) - assert_recognizes( { :controller => "api/relations", :action => "create" }, { :path => "/api/0.6/relation/create", :method => :put } @@ -228,71 +203,6 @@ module Api 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. - def test_relations_for_node - node = create(:node) - # should include relations with that node as a member - relation_with_node = create(:relation_member, :member => node).relation - # should ignore relations without that node as a member - _relation_without_node = create(:relation_member).relation - # should ignore relations with the node involved indirectly, via a way - way = create(:way_node, :node => node).way - _relation_with_way = create(:relation_member, :member => way).relation - # should ignore relations with the node involved indirectly, via a relation - second_relation = create(:relation_member, :member => node).relation - _super_relation = create(:relation_member, :member => second_relation).relation - # should combine multiple relation_member references into just one relation entry - create(:relation_member, :member => node, :relation => relation_with_node) - # should not include deleted relations - deleted_relation = create(:relation, :deleted) - create(:relation_member, :member => node, :relation => deleted_relation) - - check_relations_for_element(node_relations_path(node), "node", - node.id, - [relation_with_node, second_relation]) - end - - def test_relations_for_way - way = create(:way) - # should include relations with that way as a member - relation_with_way = create(:relation_member, :member => way).relation - # should ignore relations without that way as a member - _relation_without_way = create(:relation_member).relation - # should ignore relations with the way involved indirectly, via a relation - second_relation = create(:relation_member, :member => way).relation - _super_relation = create(:relation_member, :member => second_relation).relation - # should combine multiple relation_member references into just one relation entry - create(:relation_member, :member => way, :relation => relation_with_way) - # should not include deleted relations - deleted_relation = create(:relation, :deleted) - create(:relation_member, :member => way, :relation => deleted_relation) - - check_relations_for_element(way_relations_path(way), "way", - way.id, - [relation_with_way, second_relation]) - end - - def test_relations_for_relation - relation = create(:relation) - # should include relations with that relation as a member - relation_with_relation = create(:relation_member, :member => relation).relation - # should ignore any relation without that relation as a member - _relation_without_relation = create(:relation_member).relation - # should ignore relations with the relation involved indirectly, via a relation - second_relation = create(:relation_member, :member => relation).relation - _super_relation = create(:relation_member, :member => second_relation).relation - # should combine multiple relation_member references into just one relation entry - create(:relation_member, :member => relation, :relation => relation_with_relation) - # should not include deleted relations - deleted_relation = create(:relation, :deleted) - create(:relation_member, :member => relation, :relation => deleted_relation) - check_relations_for_element(relation_relations_path(relation), "relation", - relation.id, - [relation_with_relation, second_relation]) - end - # ------------------------------------- # Test simple relation creation. # ------------------------------------- @@ -1103,25 +1013,6 @@ module Api private - def check_relations_for_element(path, type, id, expected_relations) - # check the "relations for relation" mode - get path - assert_response :success - - # count one osm element - assert_select "osm[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1 - - # we should have only the expected number of relations - assert_select "osm>relation", expected_relations.size - - # and each of them should contain the element we originally searched for - expected_relations.each do |relation| - # The relation should appear once, but the element could appear multiple times - assert_select "osm>relation[id='#{relation.id}']", 1 - assert_select "osm>relation[id='#{relation.id}']>member[type='#{type}'][ref='#{id}']" - end - end - ## # checks that the XML document and the string arguments have # members in the same order. diff --git a/test/controllers/api/ways/relations_controller_test.rb b/test/controllers/api/ways/relations_controller_test.rb new file mode 100644 index 000000000..ded9ba3e9 --- /dev/null +++ b/test/controllers/api/ways/relations_controller_test.rb @@ -0,0 +1,69 @@ +require "test_helper" + +module Api + module Ways + class RelationsControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/api/0.6/way/1/relations", :method => :get }, + { :controller => "api/ways/relations", :action => "index", :way_id => "1" } + ) + assert_routing( + { :path => "/api/0.6/way/1/relations.json", :method => :get }, + { :controller => "api/ways/relations", :action => "index", :way_id => "1", :format => "json" } + ) + end + + def test_index + way = create(:way) + # should include relations with that way as a member + relation_with_way = create(:relation_member, :member => way).relation + # should ignore relations without that way as a member + _relation_without_way = create(:relation_member).relation + # should ignore relations with the way involved indirectly, via a relation + second_relation = create(:relation_member, :member => way).relation + _super_relation = create(:relation_member, :member => second_relation).relation + # should combine multiple relation_member references into just one relation entry + create(:relation_member, :member => way, :relation => relation_with_way) + # should not include deleted relations + deleted_relation = create(:relation, :deleted) + create(:relation_member, :member => way, :relation => deleted_relation) + + get api_way_relations_path(way) + + assert_response :success + + # count one osm element + assert_select "osm[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1 + + # we should have only the expected number of relations + expected_relations = [relation_with_way, second_relation] + assert_select "osm>relation", expected_relations.size + + # and each of them should contain the element we originally searched for + expected_relations.each do |containing_relation| + # The relation should appear once, but the element could appear multiple times + assert_select "osm>relation[id='#{containing_relation.id}']", 1 + assert_select "osm>relation[id='#{containing_relation.id}']>member[type='way'][ref='#{way.id}']" + end + end + + def test_index_json + way = create(:way) + containing_relation = create(:relation_member, :member => way).relation + + get api_way_relations_path(way, :format => "json") + + assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 1, js["elements"].count + js_relations = js["elements"].filter { |e| e["type"] == "relation" } + assert_equal 1, js_relations.count + assert_equal containing_relation.id, js_relations[0]["id"] + end + end + end +end diff --git a/test/controllers/api/ways_controller_test.rb b/test/controllers/api/ways_controller_test.rb index 191f9a820..50077f09c 100644 --- a/test/controllers/api/ways_controller_test.rb +++ b/test/controllers/api/ways_controller_test.rb @@ -778,42 +778,6 @@ module Api assert_equal "Element way/ has duplicate tags with key addr:housenumber", @response.body end - ## - # test that a call to ways_for_node returns all ways that contain the node - # and none that don't. - def test_ways_for_node - node = create(:node) - way1 = create(:way) - way2 = create(:way) - create(:way_node, :way => way1, :node => node) - create(:way_node, :way => way2, :node => node) - # create an unrelated way - create(:way_with_nodes, :nodes_count => 2) - # create a way which used to use the node - way3_v1 = create(:old_way, :version => 1) - _way3_v2 = create(:old_way, :current_way => way3_v1.current_way, :version => 2) - create(:old_way_node, :old_way => way3_v1, :node => node) - - get node_ways_path(node) - assert_response :success - ways_xml = XML::Parser.string(@response.body).parse - assert_not_nil ways_xml, "failed to parse ways_for_node response" - - # check that the set of IDs match expectations - expected_way_ids = [way1.id, - way2.id] - found_way_ids = ways_xml.find("//osm/way").collect { |w| w["id"].to_i } - assert_equal expected_way_ids.sort, found_way_ids.sort, - "expected ways for node #{node.id} did not match found" - - # check the full ways to ensure we're not missing anything - expected_way_ids.each do |id| - way_xml = ways_xml.find("//osm/way[@id='#{id}']").first - assert_ways_are_equal(Way.find(id), - Way.from_xml_node(way_xml)) - end - end - ## # test initial rate limit def test_initial_rate_limit diff --git a/yarn.lock b/yarn.lock index c15c97e29..902fdb906 100644 --- a/yarn.lock +++ b/yarn.lock @@ -35,6 +35,13 @@ dependencies: "@types/json-schema" "^7.0.15" +"@eslint/core@^0.11.0": + version "0.11.0" + resolved "https://registry.yarnpkg.com/@eslint/core/-/core-0.11.0.tgz#7a9226e850922e42cbd2ba71361eacbe74352a12" + integrity sha512-DWUB2pksgNEb6Bz2fggIy1wh6fGgZP4Xyy/Mt0QZPiloKKXerbqq9D3SBQTlCRYOrcRPu4vuz+CGjwdfqxnoWA== + dependencies: + "@types/json-schema" "^7.0.15" + "@eslint/eslintrc@^3.2.0": version "3.2.0" resolved "https://registry.yarnpkg.com/@eslint/eslintrc/-/eslintrc-3.2.0.tgz#57470ac4e2e283a6bf76044d63281196e370542c" @@ -50,10 +57,10 @@ minimatch "^3.1.2" strip-json-comments "^3.1.1" -"@eslint/js@9.19.0": - version "9.19.0" - resolved "https://registry.yarnpkg.com/@eslint/js/-/js-9.19.0.tgz#51dbb140ed6b49d05adc0b171c41e1a8713b7789" - integrity sha512-rbq9/g38qjfqFLOVPvwjIvFFdNziEC5S65jmjPw5r6A//QH+W91akh9irMwjDN8zKUTak6W9EsAv4m/7Wnw0UQ== +"@eslint/js@9.20.0": + version "9.20.0" + resolved "https://registry.yarnpkg.com/@eslint/js/-/js-9.20.0.tgz#7421bcbe74889fcd65d1be59f00130c289856eb4" + integrity sha512-iZA07H9io9Wn836aVTytRaNqh00Sad+EamwOVJT12GTLw1VGMFV/4JaME+JjLtr9fiGaoWgYnS54wrfWsSs4oQ== "@eslint/object-schema@^2.1.4": version "2.1.4" @@ -249,16 +256,16 @@ eslint-visitor-keys@^4.2.0: integrity sha512-UyLnSehNt62FFhSwjZlHmeokpRK59rcz29j+F1/aDgbkbRTk7wIc9XzdoasMUbRNKDM0qQt/+BJ4BrpFeABemw== eslint@^9.0.0: - version "9.19.0" - resolved "https://registry.yarnpkg.com/eslint/-/eslint-9.19.0.tgz#ffa1d265fc4205e0f8464330d35f09e1d548b1bf" - integrity sha512-ug92j0LepKlbbEv6hD911THhoRHmbdXt2gX+VDABAW/Ir7D3nqKdv5Pf5vtlyY6HQMTEP2skXY43ueqTCWssEA== + version "9.20.0" + resolved "https://registry.yarnpkg.com/eslint/-/eslint-9.20.0.tgz#6244c46c1640cd5e577a31ebc460fca87838c0b7" + integrity sha512-aL4F8167Hg4IvsW89ejnpTwx+B/UQRzJPGgbIOl+4XqffWsahVVsLEWoZvnrVuwpWmnRd7XeXmQI1zlKcFDteA== dependencies: "@eslint-community/eslint-utils" "^4.2.0" "@eslint-community/regexpp" "^4.12.1" "@eslint/config-array" "^0.19.0" - "@eslint/core" "^0.10.0" + "@eslint/core" "^0.11.0" "@eslint/eslintrc" "^3.2.0" - "@eslint/js" "9.19.0" + "@eslint/js" "9.20.0" "@eslint/plugin-kit" "^0.2.5" "@humanfs/node" "^0.16.6" "@humanwhocodes/module-importer" "^1.0.1"