From 8b258d2cbdd37dbd0ab979db1372c717f9c02527 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Mon, 8 Jun 2020 14:38:49 -0400 Subject: [PATCH] Load the maximum nodes per way value from the OSM API and add a getter to the service object Disable the Merge operation if the resultant way would have more than the maximum number of nodes (close #6030) Simplify some code in operationMerge --- data/core.yaml | 1 + dist/locales/en.json | 3 +- modules/actions/join.js | 7 ++++ modules/operations/merge.js | 64 +++++++++++++++++-------------------- modules/services/osm.js | 13 ++++++++ 5 files changed, 52 insertions(+), 36 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 786e4bb10..5df892385 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -303,6 +303,7 @@ en: incomplete_relation: These features can't be merged because at least one hasn't been fully downloaded. conflicting_tags: These features can't be merged because some of their tags have conflicting values. paths_intersect: These features can't be merged because the resulting path would intersect itself. + too_many_vertices: These features can't be merged because the resulting path would have too many points. move: title: Move description: diff --git a/dist/locales/en.json b/dist/locales/en.json index d88ed18c1..7e271dbb0 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -396,7 +396,8 @@ "relation": "These features can't be merged because they have conflicting relation roles.", "incomplete_relation": "These features can't be merged because at least one hasn't been fully downloaded.", "conflicting_tags": "These features can't be merged because some of their tags have conflicting values.", - "paths_intersect": "These features can't be merged because the resulting path would intersect itself." + "paths_intersect": "These features can't be merged because the resulting path would intersect itself.", + "too_many_vertices": "These features can't be merged because the resulting path would have too many points." }, "move": { "title": "Move", diff --git a/modules/actions/join.js b/modules/actions/join.js index 2a9ddf27c..1fff3f7c9 100644 --- a/modules/actions/join.js +++ b/modules/actions/join.js @@ -110,6 +110,13 @@ export function actionJoin(ids) { return graph; }; + // Returns the number of nodes the resultant way is expected to have + action.resultingWayNodesLength = function(graph) { + return ids.reduce(function(count, id) { + return count + graph.entity(id).nodes.length; + }, 0) - ids.length - 1; + }; + action.disabled = function(graph) { var geometries = groupEntitiesByGeometry(graph); diff --git a/modules/operations/merge.js b/modules/operations/merge.js index de4ba276f..ea7a2bbc1 100644 --- a/modules/operations/merge.js +++ b/modules/operations/merge.js @@ -11,28 +11,30 @@ import { presetManager } from '../presets'; export function operationMerge(context, selectedIDs) { - var join = actionJoin(selectedIDs); - var merge = actionMerge(selectedIDs); - var mergePolygon = actionMergePolygon(selectedIDs); - var mergeNodes = actionMergeNodes(selectedIDs); + var _action = getAction(); function getAction() { - if (!join.disabled(context.graph())) { + var join = actionJoin(selectedIDs); + if (join.disabled(context.graph()) !== 'not_eligible') { return join; - - } else if (!merge.disabled(context.graph())) { + } + var merge = actionMerge(selectedIDs); + if (merge.disabled(context.graph()) !== 'not_eligible') { return merge; - - } else if (!mergePolygon.disabled(context.graph())) { + } + var mergePolygon = actionMergePolygon(selectedIDs); + if (mergePolygon.disabled(context.graph()) !== 'not_eligible') { return mergePolygon; } + var mergeNodes = actionMergeNodes(selectedIDs); return mergeNodes; } var operation = function() { - var action = getAction(); - context.perform(action, operation.annotation()); + if (operation.disabled()) return; + + context.perform(_action, operation.annotation()); context.validator().validate(); @@ -51,37 +53,29 @@ export function operationMerge(context, selectedIDs) { }; operation.disabled = function() { - return join.disabled(context.graph()) && - merge.disabled(context.graph()) && - mergePolygon.disabled(context.graph()) && - mergeNodes.disabled(context.graph()); + var actionDisabled = _action.disabled(context.graph()); + if (actionDisabled) return actionDisabled; + + var osm = context.connection(); + if (osm && + _action.resultingWayNodesLength && + _action.resultingWayNodesLength(context.graph()) > osm.maxWayNodes()) { + return 'too_many_vertices'; + } + + return false; }; operation.tooltip = function() { - var j = join.disabled(context.graph()); // 'not_eligible', 'not_adjacent', 'restriction', 'conflicting_tags' - var m = merge.disabled(context.graph()); // 'not_eligible' - var p = mergePolygon.disabled(context.graph()); // 'not_eligible', 'incomplete_relation' - var n = mergeNodes.disabled(context.graph()); // 'not_eligible', 'relation', 'restriction' - - // disabled for one of various reasons - if (j && m && p && n) { - if (j === 'restriction' || n === 'restriction') { + var disabled = operation.disabled(); + if (disabled) { + if (disabled === 'restriction') { return t('operations.merge.restriction', { relation: presetManager.item('type/restriction').name() }); - - } else if (p === 'incomplete_relation') { - return t('operations.merge.incomplete_relation'); - - } else if (n === 'relation') { - return t('operations.merge.relation'); - - } else { - return t('operations.merge.' + j); } - - } else { - return t('operations.merge.description'); + return t('operations.merge.' + disabled); } + return t('operations.merge.description'); }; operation.annotation = function() { diff --git a/modules/services/osm.js b/modules/services/osm.js index c5c81461d..ff292c63e 100644 --- a/modules/services/osm.js +++ b/modules/services/osm.js @@ -40,6 +40,9 @@ var _userChangesets; var _userDetails; var _off; +// set a default but also load this from the API status +var _maxWayNodes = 2000; + function authLoading() { dispatch.call('authLoading'); @@ -932,6 +935,10 @@ export default { if (_rateLimitError) { return callback(_rateLimitError, 'rateLimited'); } else { + var waynodes = xml.getElementsByTagName('waynodes'); + var maxWayNodes = waynodes.length && parseInt(waynodes[0].getAttribute('maximum'), 10); + if (maxWayNodes && isFinite(maxWayNodes)) _maxWayNodes = maxWayNodes; + var apiStatus = xml.getElementsByTagName('status'); var val = apiStatus[0].getAttribute('api'); return callback(undefined, val); @@ -958,6 +965,12 @@ export default { }, + // Returns the maximum number of nodes a single way can have + maxWayNodes: function() { + return _maxWayNodes; + }, + + // Load data (entities) from the API in tiles // GET /api/0.6/map?bbox= loadTiles: function(projection, callback) {