also retrigger validation for connected ways of removed/disconnected ways

example: a way can become disconnected from the rest of the road network if the only way that connects it to the rest of the network is deleted.

this also simplifies the logic in validator to be agnostic of the validation types this can affect and optimizes it slightly.

Author: tyrasd

modules/core/validator.js

@@ -495,24 +495,24 @@ 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 all issues that are disconnected_way or impossible_oneway
+    // Re-validate also connected (or previously connected) entities to the current way
     // fix #8758
-    let additionalEntityIDs = new Set();
-    for (const IssueID in _headCache.issuesByIssueID) {
-      const issue = _headCache.issuesByIssueID[IssueID];
-      if (['disconnected_way', 'impossible_oneway'].includes(issue.type)) {
-        issue.entityIds.forEach(additionalEntityIDs.add, additionalEntityIDs);
-      }
-    }
-    entityIDs = new Set([...entityIDs, ...additionalEntityIDs]);
+    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))

test/spec/core/validator.js

@@ -59,11 +59,9 @@ describe('iD.coreValidator', function() {
         );
         var validator = new iD.coreValidator(context);
         validator.init();
-        var issues = validator.getIssues();
-        expect(issues).to.have.lengthOf(0);
         validator.validate().then(function() {
             // Should produce disconnected way error
-            issues = validator.getIssues();
+            let issues = validator.getIssues();
             expect(issues).to.have.lengthOf(1);
 
             // Add new node with entrance node to simulate connection with rest of map
@@ -84,8 +82,45 @@ describe('iD.coreValidator', function() {
         }).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);
 
-        window.setTimeout(function() {}, 20); // async - to let the promise settle in phantomjs
+            // 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);
+        });
     });
 
 });

test/spec/util/util.js

@@ -95,10 +95,13 @@ describe('iD.util', function() {
         });
         it('trims leading #? if present', function() {
             expect(iD.utilStringQs('#?foo=bar')).to.eql({foo: 'bar'});
+<<<<<<< Updated upstream
         });
         it('supports both + and %20 for escaping spaces', function() {
             expect(iD.utilStringQs('#?foo=a+b%20c')).to.eql({foo: 'a b c'});
             expect(iD.utilStringQs('#?')).to.eql({});
+=======
+>>>>>>> Stashed changes
         });
     });
 

test/spec/validations/disconnected_way.js

@@ -92,31 +92,4 @@ describe('iD.validations.disconnected_way', function() {
         var issues = validate();
         expect(issues).to.have.lengthOf(0);
     });
-
-    it('removes validation issue when highway is no longer disconnected', function() {
-        // 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 issues = validate();
-        // Should produce disconnected way error
-        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)
-        );
-        issues = validate();
-        // Should be no errors
-        expect(issues).to.have.lengthOf(0);
-    });
-
 });