diff --git a/CHANGELOG.md b/CHANGELOG.md index 0218a0772..963d3a53e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ _Breaking developer changes, which may affect downstream projects or sites that #### :white_check_mark: Validation * Add warning if aeroways cross each other, buildings or highways ([#9315], thanks [@k-yle]) * Warn when a way with more than the maximum allowed number of nodes is to be uploaded and provide a way to fix it ([#7381]) +* Revalidate ways that are connected to the currently edited way to also properly update/catch _disconnected way_s and _impossible oneway_ errors ([#8911], thanks [@andrewpmk]) #### :bug: Bugfixes * Prevent degenerate ways caused by deleting a corner of a triangle ([#10003], thanks [@k-yle]) * Fix briefly disappearing data layer during background layer tile layer switching transition ([#10748]) @@ -66,6 +67,7 @@ _Breaking developer changes, which may affect downstream projects or sites that #### :hammer: Development [#7381]: https://github.com/openstreetmap/iD/issues/7381 +[#8911]: https://github.com/openstreetmap/iD/pull/8911 [#9635]: https://github.com/openstreetmap/iD/pull/9635 [#10003]: https://github.com/openstreetmap/iD/pull/10003 [#10648]: https://github.com/openstreetmap/iD/pull/10648 @@ -83,6 +85,7 @@ _Breaking developer changes, which may affect downstream projects or sites that [@Deeptanshu-sankhwar]: https://github.com/Deeptanshu-sankhwar [@draunger]: https://github.com/draunger [@burrscurr]: https://github.com/burrscurr +[@andrewpmk]: https://github.com/andrewpmk # 2.31.1 diff --git a/modules/core/validator.js b/modules/core/validator.js index 58a8b7700..24cb4cf53 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -495,14 +495,25 @@ export function coreValidator(context) { _headCache.graph = currGraph; // take snapshot _completeDiff = context.history().difference().complete(); const incrementalDiff = coreDifference(prevGraph, currGraph); - let entityIDs = Object.keys(incrementalDiff.complete()); - entityIDs = _headCache.withAllRelatedEntities(entityIDs); // expand set + const diff = Object.keys(incrementalDiff.complete()); + const entityIDs = _headCache.withAllRelatedEntities(diff); // expand set if (!entityIDs.size) { dispatch.call('validated'); return Promise.resolve(); } + // Re-validate also connected (or previously connected) entities to the current way + // fix #8758 + const addConnectedWays = graph => diff + .filter(entityID => graph.hasEntity(entityID)) + .map(entityID => graph.entity(entityID)) + .flatMap(entity => graph.childNodes(entity)) + .flatMap(vertex => graph.parentWays(vertex)) + .forEach(way => entityIDs.add(way.id)); + addConnectedWays(currGraph); + addConnectedWays(prevGraph); + _headPromise = validateEntitiesAsync(entityIDs, _headCache) .then(() => updateResolvedIssues(entityIDs)) .then(() => dispatch.call('validated')) diff --git a/test/spec/core/validator.js b/test/spec/core/validator.js index e9f2759c3..ce10bd80b 100644 --- a/test/spec/core/validator.js +++ b/test/spec/core/validator.js @@ -1,4 +1,4 @@ -describe('iD.coreValidator', function () { +describe('iD.coreValidator', function() { var context; beforeEach(function() { @@ -6,9 +6,9 @@ describe('iD.coreValidator', function () { }); function createInvalidWay() { - var n1 = iD.osmNode({id: 'n-1', loc: [4,4]}); - var n2 = iD.osmNode({id: 'n-2', loc: [4,5]}); - var w = iD.osmWay({id: 'w-1', nodes: ['n-1', 'n-2']}); + var n1 = iD.osmNode({ id: 'n-1', loc: [4, 4] }); + var n2 = iD.osmNode({ id: 'n-2', loc: [4, 5] }); + var w = iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'] }); context.perform( iD.actionAddEntity(n1), @@ -47,4 +47,80 @@ describe('iD.coreValidator', function () { }); }); + it('removes validation issue when highway is no longer disconnected', function(done) { + // Add a way which is disconnected from the rest of the map + var n1 = iD.osmNode({ id: 'n-1', loc: [4, 4] }); + var n2 = iD.osmNode({ id: 'n-2', loc: [4, 5] }); + var w = iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: { 'highway': 'unclassified' } }); + context.perform( + iD.actionAddEntity(n1), + iD.actionAddEntity(n2), + iD.actionAddEntity(w) + ); + var validator = new iD.coreValidator(context); + validator.init(); + validator.validate().then(function() { + // Should produce disconnected way error + let issues = validator.getIssues(); + expect(issues).to.have.lengthOf(1); + + // Add new node with entrance node to simulate connection with rest of map + var n3 = iD.osmNode({ id: 'n-3', loc: [4, 6], tags: { 'entrance': 'yes' } }); + var w2 = iD.osmWay({ id: 'w-2', nodes: ['n-2', 'n-3'], tags: { 'highway': 'unclassified' } }); + context.perform( + iD.actionAddEntity(n3), + iD.actionAddEntity(w2) + ); + validator.validate().then(function() { + // Should be no errors + issues = validator.getIssues(); + expect(issues).to.have.lengthOf(0); + done(); + }).catch(function(err) { + done(err); + }); + }).catch(function(err) { + done(err); + }); + }); + + it('add validation issue when highway becomes disconnected', function(done) { + // Add a way which is connected to another way with an entrance node to simulate connection with rest of map + var n1 = iD.osmNode({ id: 'n-1', loc: [4, 4] }); + var n2 = iD.osmNode({ id: 'n-2', loc: [4, 5] }); + var w = iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: { 'highway': 'unclassified' } }); + var n3 = iD.osmNode({ id: 'n-3', loc: [4, 6], tags: { 'entrance': 'yes' } }); + var w2 = iD.osmWay({ id: 'w-2', nodes: ['n-2', 'n-3'], tags: { 'highway': 'unclassified' } }); + context.perform( + iD.actionAddEntity(n1), + iD.actionAddEntity(n2), + iD.actionAddEntity(w), + iD.actionAddEntity(n3), + iD.actionAddEntity(w2) + ); + var validator = new iD.coreValidator(context); + validator.init(); + validator.validate().then(function() { + // Should be no errors + let issues = validator.getIssues(); + expect(issues).to.have.lengthOf(0); + + // delete second way -> first way becomes disconnected form the rest of the network + context.perform( + iD.actionDeleteWay(w2.id) + ); + + validator.validate().then(function() { + // Should produce disconnected way error + issues = validator.getIssues(); + expect(issues).to.have.lengthOf(1); + done(); + }).catch(function(err) { + done(err); + }); + }).catch(function(err) { + done(err); + }); + }); + }); diff --git a/test/spec/validations/disconnected_way.js b/test/spec/validations/disconnected_way.js index 859b822c8..4e24b8860 100644 --- a/test/spec/validations/disconnected_way.js +++ b/test/spec/validations/disconnected_way.js @@ -1,4 +1,4 @@ -describe('iD.validations.disconnected_way', function () { +describe('iD.validations.disconnected_way', function() { var context; beforeEach(function() { @@ -6,9 +6,9 @@ describe('iD.validations.disconnected_way', function () { }); function createWay(tags) { - var n1 = iD.osmNode({id: 'n-1', loc: [4,4]}); - var n2 = iD.osmNode({id: 'n-2', loc: [4,5]}); - var w = iD.osmWay({id: 'w-1', nodes: ['n-1', 'n-2'], tags: tags}); + var n1 = iD.osmNode({ id: 'n-1', loc: [4, 4] }); + var n2 = iD.osmNode({ id: 'n-2', loc: [4, 5] }); + var w = iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: tags }); context.perform( iD.actionAddEntity(n1), @@ -18,11 +18,11 @@ describe('iD.validations.disconnected_way', function () { } function createConnectingWays(tags1, tags2) { - var n1 = iD.osmNode({id: 'n-1', loc: [4,4]}); - var n2 = iD.osmNode({id: 'n-2', loc: [4,5]}); - var n3 = iD.osmNode({id: 'n-3', loc: [5,5]}); - var w = iD.osmWay({id: 'w-1', nodes: ['n-1', 'n-2'], tags: tags1}); - var w2 = iD.osmWay({id: 'w-2', nodes: ['n-1', 'n-3'], tags: tags2}); + var n1 = iD.osmNode({ id: 'n-1', loc: [4, 4] }); + var n2 = iD.osmNode({ id: 'n-2', loc: [4, 5] }); + var n3 = iD.osmNode({ id: 'n-3', loc: [5, 5] }); + var w = iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: tags1 }); + var w2 = iD.osmWay({ id: 'w-2', nodes: ['n-1', 'n-3'], tags: tags2 }); context.perform( iD.actionAddEntity(n1), @@ -50,7 +50,7 @@ describe('iD.validations.disconnected_way', function () { }); it('flags disconnected highway', function() { - createWay({'highway': 'unclassified'}); + createWay({ 'highway': 'unclassified' }); var issues = validate(); expect(issues).to.have.lengthOf(1); var issue = issues[0]; @@ -62,7 +62,7 @@ describe('iD.validations.disconnected_way', function () { }); it('flags highway connected only to service area', function() { - createConnectingWays({'highway': 'unclassified'}, {'highway': 'services'}); + createConnectingWays({ 'highway': 'unclassified' }, { 'highway': 'services' }); var issues = validate(); expect(issues).to.have.lengthOf(1); var issue = issues[0]; @@ -75,11 +75,11 @@ describe('iD.validations.disconnected_way', function () { it('ignores highway with connected entrance vertex', function() { - var n1 = iD.osmNode({id: 'n-1', loc: [4,4], tags: {'entrance': 'yes'}}); - var n2 = iD.osmNode({id: 'n-2', loc: [4,5]}); - var n3 = iD.osmNode({id: 'n-3', loc: [5,5]}); - var w = iD.osmWay({id: 'w-1', nodes: ['n-1', 'n-2'], tags: {'highway': 'unclassified'}}); - var w2 = iD.osmWay({id: 'w-2', nodes: ['n-1', 'n-3']}); + var n1 = iD.osmNode({ id: 'n-1', loc: [4, 4], tags: { 'entrance': 'yes' } }); + var n2 = iD.osmNode({ id: 'n-2', loc: [4, 5] }); + var n3 = iD.osmNode({ id: 'n-3', loc: [5, 5] }); + var w = iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: { 'highway': 'unclassified' } }); + var w2 = iD.osmWay({ id: 'w-2', nodes: ['n-1', 'n-3'] }); context.perform( iD.actionAddEntity(n1), @@ -92,5 +92,4 @@ describe('iD.validations.disconnected_way', function () { var issues = validate(); expect(issues).to.have.lengthOf(0); }); - });