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.

Sign up for GitHub

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

Jump to bottom

Tests and fixes of potential duplicate consecutive node issues in way.js #3676

Closed
wants to merge 3 commits into from
Closed

Tests and fixes of potential duplicate consecutive node issues in way.js #3676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fa70d79
Additional tests added for addNode, updateNode, and replaceNode.
slhh Dec 16, 2016
7817d6d
Prevent duplicate consecutive nodes in addNode, updateNode, and
slhh Dec 16, 2016
4a9ee68
Limit index range and add exceptions
slhh Jan 5, 2017
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
60 changes: 48 additions & 12 deletions modules/osm/way.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,34 +187,70 @@ _.extend(osmWay.prototype, {
});
},


// Adds a node (id) in front of the node which is currently at position index.
// If index equals length, the node (id) will be added at the end of the way.
// If index is negative or > length, it will throw an error.
// Generating consecutive duplicates is silently prevented

addNode: function(id, index) {
var nodes = this.nodes.slice();
nodes.splice(index === undefined ? nodes.length : index, 0, id);
var nodes = this.nodes.slice(),
spliceIndex = index === undefined ? nodes.length : index;
if (spliceIndex > nodes.length || spliceIndex < 0) {
throw new Error('addNode: index ' + spliceIndex + ' is invalid');
}
if (nodes[spliceIndex] !== id&& nodes[spliceIndex-1] !== id) {
nodes.splice(spliceIndex, 0, id);
}
return this.update({nodes: nodes});
},

// Replaces the node which is currently at position index with the given node (id).
// If index is negative or >= length, it will throw an error.
// Consecutive duplicates are eliminated including existing ones.

updateNode: function(id, index) {
var nodes = this.nodes.slice();
nodes.splice(index, 1, id);
var nodes = [];

if (index === undefined || index >= this.nodes.length || index < 0) {
throw new Error('updateNode: index ' + index + ' is invalid');
}

for (var i = 0; i < this.nodes.length; i++) {
var node = this.nodes[i];
if (i === index) {
if (nodes[nodes.length - 1] !== id)
nodes.push(id);
} else {
if (nodes[nodes.length - 1] !== node)
nodes.push(node);
}
}

return this.update({nodes: nodes});
},

// Replaces each occurrence of node id needle with replacement.
// Consecutive duplicates are eliminated including existing ones.

replaceNode: function(needle, replacement) {
if (this.nodes.indexOf(needle) < 0)
return this;
var nodes = [];

var nodes = this.nodes.slice();
for (var i = 0; i < nodes.length; i++) {
if (nodes[i] === needle) {
nodes[i] = replacement;
}
for (var i = 0; i < this.nodes.length; i++) {
var node = this.nodes[i];
if (node === needle) {
if (nodes[nodes.length - 1] !== replacement)
nodes.push(replacement);
} else {
if (nodes[nodes.length - 1] !== node)
nodes.push(node);
}
}

return this.update({nodes: nodes});
},

// Removes each occurrence of node id needle with replacement.
// Consecutive duplicates are eliminated. Circularity is preserved.

removeNode: function(id) {
var nodes = [];
Expand Down
122 changes: 119 additions & 3 deletions test/spec/osm/way.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,11 +410,21 @@ describe('iD.osmWay', function() {
});

describe('#addNode', function () {
it('adds a node to the end of a way', function () {
it('adds a node to the end of a way when index is undefined', function () {
var w = iD.Way({nodes: ['a', 'b']});
expect(w.addNode('c').nodes).to.eql(['a', 'b', 'c']);
});

it('adds a node to an empty way', function () {
var w = iD.Way();
expect(w.addNode('a').nodes).to.eql(['a']);
});

it('throws when using a index greater than length', function () {
var w = iD.Way({nodes: ['a', 'b']});
expect(function() { w.addNode('c',3); }).to.throw;
});

it('adds a node to a way at index 0', function () {
var w = iD.Way({nodes: ['a', 'b']});
expect(w.addNode('c', 0).nodes).to.eql(['c', 'a', 'b']);
Expand All @@ -425,9 +435,34 @@ describe('iD.osmWay', function() {
expect(w.addNode('c', 1).nodes).to.eql(['a', 'c', 'b']);
});

it('adds a node to a way at a negative index', function () {
it('throws when using a negative index', function () {
var w = iD.Way({nodes: ['a', 'b']});
expect(function() { w.addNode('c', -1); }).to.throw;
});

it('prevents duplicate consecutive nodes when adding in front of', function () {
var w = iD.Way({nodes: ['a', 'b']});
expect(w.addNode('b', 1).nodes).to.eql(['a', 'b']);
});

it('prevents duplicate consecutive nodes when adding behind', function () {
var w = iD.Way({nodes: ['a', 'b']});
expect(w.addNode('a', 1).nodes).to.eql(['a', 'b']);
});

it('prevents duplicate consecutive nodes at index 0', function () {
var w = iD.Way({nodes: ['a', 'b']});
expect(w.addNode('a', 0).nodes).to.eql(['a', 'b']);
});

it('prevents duplicate consecutive nodes at a index equal to length', function () {
var w = iD.Way({nodes: ['a', 'b']});
expect(w.addNode('b', 2).nodes).to.eql(['a', 'b']);
});

it('prevents duplicate consecutive nodes when index is undefined', function () {
var w = iD.Way({nodes: ['a', 'b']});
expect(w.addNode('c', -1).nodes).to.eql(['a', 'c', 'b']);
expect(w.addNode('b').nodes).to.eql(['a', 'b']);
});
});

Expand All @@ -436,8 +471,81 @@ describe('iD.osmWay', function() {
var w = iD.Way({nodes: ['a', 'b', 'c']});
expect(w.updateNode('d', 1).nodes).to.eql(['a', 'd', 'c']);
});
it('throws at an invalid index', function () {
var w = iD.Way({nodes: ['a', 'b', 'c']});
expect(function() { w.updateNode('d', -1); }).to.throw;
expect(function() { w.updateNode('d'); }).to.throw;
expect(function() { w.updateNode('d', 5); }).to.throw;
expect(function() { w.updateNode('d', 3); }).to.throw;
});

it('prevents duplicate consecutive nodes', function () {
var w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']});
expect(w.updateNode('b',2).nodes).to.eql(['a', 'b', 'd','e']);
w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']});
expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd','e']);
w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']});
expect(w.updateNode('b',2).nodes).to.eql(['a', 'b','e']);
});

it('preserves duplicate non-consecutive nodes', function () {
var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']});
expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd', 'b','e']);
});

it('replaces a single one of duplicate nodes', function () {
var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']});
expect(w.updateNode('d',1).nodes).to.eql(['a', 'd', 'c', 'b','e']);
w = iD.Way({nodes: ['a', 'b', 'b', 'c','e']});
expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd', 'c','e']);
});

it('removes existing duplicate consecutive nodes', function () {
var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']});
expect(w.updateNode('c',5).nodes).to.eql(['a', 'b', 'd', 'b','c']);
w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']});
expect(w.updateNode('c',3).nodes).to.eql(['a', 'b', 'c', 'b', 'e']);
});
});

describe('#replaceNode', function () {
it('replaces the node', function () {
var w = iD.Way({nodes: ['a']});
expect(w.replaceNode('a','b').nodes).to.eql(['b']);
w = iD.Way({nodes: ['a', 'b', 'c']});
expect(w.replaceNode('b', 'd').nodes).to.eql(['a', 'd', 'c']);
});

it('prevents duplicate consecutive nodes', function () {
var w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']});
expect(w.replaceNode('c','b').nodes).to.eql(['a', 'b', 'd','e']);
w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']});
expect(w.replaceNode('c','d').nodes).to.eql(['a', 'b', 'd','e']);
w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']});
expect(w.replaceNode('c','b').nodes).to.eql(['a', 'b','e']);
});

it('preserves duplicate non-consecutive nodes', function () {
var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']});
expect(w.replaceNode('c','d').nodes).to.eql(['a', 'b', 'd', 'b','e']);
});

it('replaces duplicate non-consecutive nodes', function () {
var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']});
expect(w.replaceNode('b','d').nodes).to.eql(['a', 'd', 'c', 'd','e']);
});

it('removes existing duplicate consecutive nodes', function () {
var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']});
expect(w.replaceNode('e','c').nodes).to.eql(['a', 'b', 'd', 'b','c']);
w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']});
expect(w.replaceNode('d','c').nodes).to.eql(['a', 'b', 'c', 'b', 'e']);
w = iD.Way({nodes: ['a', 'b', 'b', 'c', 'b', 'e']});
expect(w.replaceNode('b','d').nodes).to.eql(['a', 'd', 'c', 'd', 'e']);
});
});


describe('#removeNode', function () {
it('removes the node', function () {
var w = iD.Way({nodes: ['a']});
Expand All @@ -457,6 +565,14 @@ describe('iD.osmWay', function() {
it('prevents duplicate consecutive nodes when preserving circularity', function () {
var w = iD.Way({nodes: ['a', 'b', 'c', 'd', 'b', 'a']});
expect(w.removeNode('a').nodes).to.eql(['b', 'c', 'd', 'b']);
w = iD.Way({nodes: ['a', 'b', 'a']});
expect(w.removeNode('b').nodes).to.eql(['a']);
});
it('removes existing duplicate consecutive nodes', function () {
var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']});
expect(w.removeNode('e').nodes).to.eql(['a', 'b', 'd', 'b']);
w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']});
expect(w.removeNode('b').nodes).to.eql(['a', 'd', 'e']);
});
});

Expand Down