From 8f9a46b75a8ac9a9030a9e11cbd4be589867374f Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 17 Jan 2018 22:59:55 -0500 Subject: [PATCH] Change actionAddMember to rearrange indexed members in place This allows it to work around issues where a relation may not be completely downloaded --- modules/actions/add_member.js | 159 ++++++++++++++++++++++++++-------- 1 file changed, 122 insertions(+), 37 deletions(-) diff --git a/modules/actions/add_member.js b/modules/actions/add_member.js index ab15f343c..67f1d8b04 100644 --- a/modules/actions/add_member.js +++ b/modules/actions/add_member.js @@ -1,26 +1,13 @@ +import _clone from 'lodash-es/clone'; import _groupBy from 'lodash-es/groupBy'; +import _omit from 'lodash-es/omit'; import { osmJoinWays, osmWay } from '../osm'; export function actionAddMember(relationId, member, memberIndex, insertPair) { - // Relation.replaceMember() removes duplicates, and we don't want that.. #4696 - function replaceMemberAll(relation, needleID, replacement) { - var members = []; - for (var i = 0; i < relation.members.length; i++) { - var member = relation.members[i]; - if (member.id !== needleID) { - members.push(member); - } else { - members.push({id: replacement.id, type: replacement.type, role: member.role}); - } - } - return relation.update({members: members}); - } - - - var action = function(graph) { + return function action(graph) { var relation = graph.entity(relationId); if ((isNaN(memberIndex) || insertPair) && member.type === 'way') { @@ -37,9 +24,7 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) { // Add a way member into the relation "wherever it makes sense". // In this situation we were not supplied a memberIndex. function addWayMember(relation, graph) { - var groups; - var tempWay; - var i, j; + var groups, tempWay, item, i, j, k; if (insertPair) { // We're adding a member that must stay paired with an existing member. @@ -59,32 +44,56 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) { groups.way = groups.way || []; } else { - // Add the member anywhere.. Just push and let `osmJoinWays` decide where to put it. + // Add the member anywhere, one time. Just push and let `osmJoinWays` decide where to put it. groups = _groupBy(relation.members, function(m) { return m.type; }); groups.way = groups.way || []; groups.way.push(member); } - var joined = osmJoinWays(groups.way, graph); + var members = withIndex(groups.way); + var joined = osmJoinWays(members, graph); - var newWayMembers = []; + // `joined` might not contain all of the way members, + // But will contain only the completed (downloaded) members for (i = 0; i < joined.length; i++) { var segment = joined[i]; var nodes = segment.nodes.slice(); + var startIndex = segment[0].index; - for (j = 0; j < segment.length; j++) { - var way = graph.entity(segment[j].id); - if (tempWay && segment[j].id === tempWay.id) { - if (nodes[0].id === insertPair.nodes[0]) { - newWayMembers.push({ id: insertPair.originalID, type: 'way', role: segment[j].role }); - newWayMembers.push({ id: insertPair.insertedID, type: 'way', role: segment[j].role }); - } else { - newWayMembers.push({ id: insertPair.insertedID, type: 'way', role: segment[j].role }); - newWayMembers.push({ id: insertPair.originalID, type: 'way', role: segment[j].role }); - } - } else { - newWayMembers.push(segment[j]); + // j = array index in `members` where this segment starts + for (j = 0; j < members.length; j++) { + if (members[j].index === startIndex) { + break; } + } + + // k = each member in segment + for (k = 0; k < segment.length; k++) { + item = segment[k]; + var way = graph.entity(item.id); + + // If this is a paired item, generate members in correct order and role + if (tempWay && item.id === tempWay.id) { + if (nodes[0].id === insertPair.nodes[0]) { + item.pair = [ + { id: insertPair.originalID, type: 'way', role: item.role }, + { id: insertPair.insertedID, type: 'way', role: item.role } + ]; + } else { + item.pair = [ + { id: insertPair.insertedID, type: 'way', role: item.role }, + { id: insertPair.originalID, type: 'way', role: item.role } + ]; + } + } + + // reorder `members` if necessary + if (k > 0) { + if (j+k >= members.length || item.index !== members[j+k].index) { + moveMember(members, item.index, j+k); + } + } + nodes.splice(0, way.nodes.length - 1); } } @@ -93,13 +102,89 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) { graph = graph.remove(tempWay); } + // Final pass: skip dead items, split pairs, remove index properties + var wayMembers = []; + for (i = 0; i < members.length; i++) { + item = members[i]; + if (item.index === -1) continue; + + if (item.pair) { + wayMembers.push(item.pair[0]); + wayMembers.push(item.pair[1]); + } else { + wayMembers.push(_omit(item, 'index')); + } + } + // Write members in the order: nodes, ways, relations // This is reccomended for Public Transport routes: // see https://wiki.openstreetmap.org/wiki/Public_transport#Service_routes - var newMembers = (groups.node || []).concat(newWayMembers, (groups.relation || [])); + var newMembers = (groups.node || []).concat(wayMembers, (groups.relation || [])); + return graph.replace(relation.update({members: newMembers})); + + + // `moveMember()` changes the `members` array in place by splicing + // the item with `.index = findIndex` to where it belongs, + // and marking the old position as "dead" with `.index = -1` + // + // j=5, k=0 jk + // segment 5 4 7 6 + // members 0 1 2 3 4 5 6 7 8 9 keep 5 in j+k + // + // j=5, k=1 j k + // segment 5 4 7 6 + // members 0 1 2 3 4 5 6 7 8 9 move 4 to j+k + // members 0 1 2 3 x 5 4 6 7 8 9 moved + // + // j=5, k=2 j k + // segment 5 4 7 6 + // members 0 1 2 3 x 5 4 6 7 8 9 move 7 to j+k + // members 0 1 2 3 x 5 4 7 6 x 8 9 moved + // + // j=5, k=3 j k + // segment 5 4 7 6 + // members 0 1 2 3 x 5 4 7 6 x 8 9 keep 6 in j+k + // + function moveMember(arr, findIndex, toIndex) { + for (var i = 0; i < arr.length; i++) { + if (arr[i].index === findIndex) { + break; + } + } + + var item = _clone(arr[i]); + arr[i].index = -1; // mark as dead + item.index = toIndex; + arr.splice(toIndex, 0, item); + } + + + // Relation.replaceMember() removes duplicates, and we don't want that.. #4696 + function replaceMemberAll(relation, needleID, replacement) { + var members = []; + for (var i = 0; i < relation.members.length; i++) { + var member = relation.members[i]; + if (member.id !== needleID) { + members.push(member); + } else { + members.push({id: replacement.id, type: replacement.type, role: member.role}); + } + } + return relation.update({members: members}); + } + + + // This is the same as `Relation.indexedMembers`, + // Except we don't want to index all the members, only the ways + function withIndex(arr) { + var result = new Array(arr.length); + for (var i = 0; i < arr.length; i++) { + result[i] = arr[i]; + result[i].index = i; + } + return result; + } } - - return action; }