fix ticket 20629 / issue #24

src/main/java/org/openstreetmap/josm/plugins/contourmerge/ContourMergeMode.java

@@ -281,7 +281,7 @@ protected void onDrop(Point target){
                  * slice given by the drop target.
                  */
                 getMapView().setCursor(Cursor.getDefaultCursor());
-                Command cmd = model.buildContourAlignCommand();
+                final Command cmd = model.buildContourAlignCommand();
                 if (cmd != null){
                     UndoRedoHandler.getInstance().add(cmd);
                 }

src/main/java/org/openstreetmap/josm/plugins/contourmerge/ContourMergeModel.java

@@ -444,7 +444,9 @@ protected Stream<Command> buildSourceChangeCommands(
     }
 
     protected Stream<Command> buildNodeDeleteCommands(
-            final List<WaySlice> sources) {
+            final List<WaySlice> sources,
+            final WaySlice target
+            ) {
         Validate.isTrue(!sources.isEmpty(),
                 // don't translate
                 "sources must not be empty");
@@ -460,19 +462,25 @@ protected Stream<Command> buildNodeDeleteCommands(
         //   of a merge operation
 
         final WaySlice first = sources.get(0);
-        final Set<Way> ways = sources.stream().map(WaySlice::getWay)
+        final Set<OsmPrimitive> sourceWays = sources.stream().map(WaySlice::getWay)
                 .collect(Collectors.toSet());
         return first.getNodes().stream()
             .map(n -> {
+                final Set<OsmPrimitive> referrers = n.getReferrers().stream()
+                        .collect(Collectors.toSet());
+                final int numReferrersBeforeIntersection = referrers.size();
+                referrers.retainAll(sourceWays);
+
                 // true, if the node n is only referenced by source ways
                 // in the merge operation
-                final boolean hasNoParents = n.getReferrers().stream()
-                    .allMatch(ways::contains);
+                boolean hasOnlySourcesAsParents =
+                    numReferrersBeforeIntersection == referrers.size();
 
-                if (hasNoParents && !n.isTagged()) {
+                if (hasOnlySourcesAsParents && !n.isTagged() && !target.containsNode(n)) {
                     return Optional.of(new DeleteCommand(n));
+                } else {
+                    return Optional.<DeleteCommand>empty();
                 }
-                return Optional.<DeleteCommand>empty();
              })
             .filter(Optional::isPresent)
             .map(Optional::get);
@@ -496,13 +504,13 @@ public Command buildContourAlignCommand() {
             @Null final WaySlice dropTarget) {
         if (dragSource == null || dropTarget == null) return null;
 
-        final List<WaySlice> waySlices =
+        final List<WaySlice> sourceWaySlices =
                 dragSource.findAllEquivalentWaySlices()
                         .collect(Collectors.toList());
 
         final List<Command> cmds = Stream.concat(
-                buildSourceChangeCommands(waySlices, dropTarget),
-                buildNodeDeleteCommands(waySlices)
+                buildSourceChangeCommands(sourceWaySlices, dropTarget),
+                buildNodeDeleteCommands(sourceWaySlices, dropTarget)
         ).collect(Collectors.toList());
 
         return new SequenceCommand(tr("Merging Contour"), cmds);

src/main/java/org/openstreetmap/josm/plugins/contourmerge/WaySlice.java

@@ -75,8 +75,8 @@ public WaySlice(@NotNull Way w, int start, int end){
      * @param end the index of the end node. 0 <= end < w.getNodeCount().
      * start < end
      * @param inDirection true, this way slice is given by the nodes
-     * <code>[start, ..., end]</code>; false, if
-     * is  given by the nodes <code>[end,..,0,..,start]</code>
+     * <code>[start, ..., end]</code>; false, if it
+     * is given by the nodes <code>[end,..,0,..,start]</code>
      * (provided the way is closed)
      * @throws IllegalArgumentException thrown if a precondition is violated
      */
@@ -139,6 +139,15 @@ public Node getEndNode() {
         return w.getNode(end);
     }
 
+    /**
+     * Replies true if this slice contains the node <code>node</code>
+     * @param node the node
+     * @return true if this slice contains the node <code>node</code>
+     */
+    public boolean containsNode(@NotNull final Node node) {
+        return getNodes().contains(node);
+    }
+
     /**
      * Replies the lower node idx of the node from which this way slice is
      * torn off.
@@ -267,7 +276,7 @@ public int getNumSegments() {
      *
      * @return the opposite way slice
      */
-    public WaySlice getOpositeSlice(){
+    public WaySlice getOppositeSlice(){
         if (!w.isClosed()) return null;
         return new WaySlice(w, start, end, !inDirection);
     }

src/test/groovy/org/openstreetmap/josm/plugins/contourmerge/DataIntegrityProblemTest.groovy

@@ -7,12 +7,16 @@ import org.openstreetmap.josm.data.Preferences
 import org.openstreetmap.josm.data.coor.LatLon
 import org.openstreetmap.josm.data.osm.DataSet
 import org.openstreetmap.josm.data.osm.Node
+import org.openstreetmap.josm.data.osm.OsmPrimitiveType
 import org.openstreetmap.josm.data.osm.Way
 import org.openstreetmap.josm.data.projection.ProjectionRegistry
 import org.openstreetmap.josm.data.projection.Projections
 import org.openstreetmap.josm.gui.layer.OsmDataLayer
+import org.openstreetmap.josm.io.OsmReader
 import org.openstreetmap.josm.spi.preferences.Config
 
+import javax.swing.ProgressMonitor
+
 import static org.hamcrest.MatcherAssert.assertThat
 import static org.hamcrest.Matchers.equalTo
 import static org.junit.Assert.*
@@ -268,5 +272,106 @@ class DataIntegrityProblemTest02 {
     }
 }
 
+class DataIntegrityProblemTest03 {
+
+    // dataset reported in ticket https://josm.openstreetmap.de/ticket/20629
+    static DATA_SET = """
+<osm version='0.6' generator='JOSM'>
+  <bounds minlat='-23.1550787' minlon='47.0651293' maxlat='-23.1489626' maxlon='47.0745707' origin='CGImap 0.8.3 (2888343 spike-08.openstreetmap.org)' />
+  <bounds minlat='-23.1550787' minlon='47.0651293' maxlat='-23.1489626' maxlon='47.0745707' origin='OpenStreetMap server' />
+  <node id='-102544' action='modify' visible='true' lat='-23.15020552151' lon='47.0683025658' />
+  <node id='-102545' action='modify' visible='true' lat='-23.15045423564' lon='47.06711683267' />
+  <node id='-102546' action='modify' visible='true' lat='-23.1513673192' lon='47.06758000968' />
+  <node id='-102547' action='modify' visible='true' lat='-23.15138776129' lon='47.06829144955' />
+  <node id='-102548' action='modify' visible='true' lat='-23.15059733124' lon='47.06959946141' />
+  <node id='-102549' action='modify' visible='true' lat='-23.15108453652' lon='47.06960687224' />
+  <node id='-102550' action='modify' visible='true' lat='-23.15100385464' lon='47.06707902709' />
+  <node id='-102551' action='modify' visible='true' lat='-23.15019717563' lon='47.06768486771' />
+  <node id='-102552' action='modify' visible='true' lat='-23.15020538297' lon='47.06919191663' />
+  <node id='-102553' action='modify' visible='true' lat='-23.15139691206' lon='47.06905852456' />
+  <way id='-104797' action='modify' visible='true'>
+    <nd ref='-102544' />
+    <nd ref='-102551' />
+    <nd ref='-102545' />
+    <nd ref='-102550' />
+    <nd ref='-102546' />
+    <nd ref='-102547' />
+    <nd ref='-102544' />
+    <tag k='natural' v='water' />
+    <tag k='water' v='pond' />
+  </way>
+  <way id='-104798' action='modify' visible='true'>
+    <nd ref='-102547' />
+    <nd ref='-102544' />
+    <nd ref='-102552' />
+    <nd ref='-102548' />
+    <nd ref='-102549' />
+    <nd ref='-102553' />
+    <nd ref='-102547' />
+    <tag k='natural' v='wood' />
+  </way>
+</osm>
+"""
+
+    @BeforeClass
+    static void initJosmConfig() {
+        Config.setPreferencesInstance(new Preferences())
+        ProjectionRegistry.setProjection(
+                Projections.getProjectionByCode("EPSG:3857"))
+    }
+
+    def DataSet dataSet
+    def layer
+    def mergeModel
+
+    def createDataSet() {
+        dataSet = OsmReader.parseDataSet(
+            new ByteArrayInputStream(DATA_SET.getBytes()),
+            null // no progress monitor
+        )
+    }
+
+    def prepareMergeModel() {
+        layer = new OsmDataLayer(dataSet, "test 03", null /* no file */)
+        mergeModel = new ContourMergeModel(layer)
+    }
+
+
+    @Before
+    void prepareTestData() {
+        createDataSet()
+        prepareMergeModel()
+    }
+
+    @Test
+    void "test case 03"() {
+
+        def pond = dataSet.getPrimitiveById(-104797, OsmPrimitiveType.WAY)
+        def wood = dataSet.getPrimitiveById(-104798, OsmPrimitiveType.WAY)
+
+        def sourceSlice = new WaySlice(wood, 0, 1, true /* in direction */)
+        def targetSlice = new WaySlice(pond, 5, 6, true /* in direction */)
+
+        def mergeCommand = mergeModel.buildContourAlignCommand(
+                sourceSlice,
+                targetSlice
+        )
+        mergeCommand.executeCommand()
+
+        // https://josm.openstreetmap.de/ticket/20629 reports that both
+        // the pond and the wood way refer to deleted nodes after the merge
+        // operation.
+        //
+        // Make sure there are no deleted nodes after the merge operation.
+        // As a consequence, neither the wood nor the pond way still refer
+        // to deleted nodes.
+        def numDeletedNodes = dataSet.getPrimitives {obj ->
+            obj.isDeleted() && obj.getType() == OsmPrimitiveType.NODE
+        }.size()
+        assertTrue("has $numDeletedNodes nodes, expected none", numDeletedNodes == 0)
+    }
+
+}
+
 
 

src/test/groovy/org/openstreetmap/josm/plugins/contourmerge/WaySliceTests.groovy

@@ -412,12 +412,12 @@ class WaySliceTest {
 
         ws = new WaySlice(w, 0, 3)
         assert ws.getNumSegments() == 3
-        ws = ws.getOpositeSlice()
+        ws = ws.getOppositeSlice()
         assert ws.getNumSegments() == 2
 
         ws = new WaySlice(w, 1, 2)
         assert ws.getNumSegments() == 1
-        ws = ws.getOpositeSlice()
+        ws = ws.getOppositeSlice()
         assert ws.getNumSegments() == 4
     }
 

test/data/issue-24/after-merge.osm

@@ -0,0 +1,42 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<osm version='0.6' generator='JOSM'>
+  <bounds minlat='-23.1550787' minlon='47.0651293' maxlat='-23.1489626' maxlon='47.0745707' origin='CGImap 0.8.3 (2888343 spike-08.openstreetmap.org)' />
+  <bounds minlat='-23.1550787' minlon='47.0651293' maxlat='-23.1489626' maxlon='47.0745707' origin='OpenStreetMap server' />
+  <node id='-102596' action='modify' visible='true' lat='-23.15045423564' lon='47.06711683267' />
+  <node id='-102597' action='modify' visible='true' lat='-23.1513673192' lon='47.06758000968' />
+  <node id='-102599' action='modify' visible='true' lat='-23.15059733124' lon='47.06959946141' />
+  <node id='-102600' action='modify' visible='true' lat='-23.15108453652' lon='47.06960687224' />
+  <node id='-102601' action='modify' visible='true' lat='-23.15100385464' lon='47.06707902709' />
+  <node id='-102602' action='modify' visible='true' lat='-23.15019717563' lon='47.06768486771' />
+  <node id='-102603' action='modify' visible='true' lat='-23.15020538297' lon='47.06919191663' />
+  <node id='-102604' action='modify' visible='true' lat='-23.15139691206' lon='47.06905852456' />
+  <way id='-104840' action='modify' visible='true'>
+    <nd ref='-102602' />
+    <nd ref='-102596' />
+    <nd ref='-102601' />
+    <nd ref='-102597' />
+    <nd ref='-102598' />
+    <nd ref='-102597' />
+    <nd ref='-102601' />
+    <nd ref='-102596' />
+    <nd ref='-102602' />
+    <nd ref='-102595' />
+    <nd ref='-102602' />
+    <tag k='natural' v='water' />
+    <tag k='water' v='pond' />
+  </way>
+  <way id='-104841' action='modify' visible='true'>
+    <nd ref='-102604' />
+    <nd ref='-102598' />
+    <nd ref='-102597' />
+    <nd ref='-102601' />
+    <nd ref='-102596' />
+    <nd ref='-102602' />
+    <nd ref='-102595' />
+    <nd ref='-102603' />
+    <nd ref='-102599' />
+    <nd ref='-102600' />
+    <nd ref='-102604' />
+    <tag k='natural' v='wood' />
+  </way>
+</osm>

test/data/issue-24/test.osm

@@ -0,0 +1,36 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<osm version='0.6' generator='JOSM'>
+  <bounds minlat='-23.1550787' minlon='47.0651293' maxlat='-23.1489626' maxlon='47.0745707' origin='CGImap 0.8.3 (2888343 spike-08.openstreetmap.org)' />
+  <bounds minlat='-23.1550787' minlon='47.0651293' maxlat='-23.1489626' maxlon='47.0745707' origin='OpenStreetMap server' />
+  <node id='-102544' action='modify' visible='true' lat='-23.15020552151' lon='47.0683025658' />
+  <node id='-102545' action='modify' visible='true' lat='-23.15045423564' lon='47.06711683267' />
+  <node id='-102546' action='modify' visible='true' lat='-23.1513673192' lon='47.06758000968' />
+  <node id='-102547' action='modify' visible='true' lat='-23.15138776129' lon='47.06829144955' />
+  <node id='-102548' action='modify' visible='true' lat='-23.15059733124' lon='47.06959946141' />
+  <node id='-102549' action='modify' visible='true' lat='-23.15108453652' lon='47.06960687224' />
+  <node id='-102550' action='modify' visible='true' lat='-23.15100385464' lon='47.06707902709' />
+  <node id='-102551' action='modify' visible='true' lat='-23.15019717563' lon='47.06768486771' />
+  <node id='-102552' action='modify' visible='true' lat='-23.15020538297' lon='47.06919191663' />
+  <node id='-102553' action='modify' visible='true' lat='-23.15139691206' lon='47.06905852456' />
+  <way id='-104797' action='modify' visible='true'>
+    <nd ref='-102544' />
+    <nd ref='-102551' />
+    <nd ref='-102545' />
+    <nd ref='-102550' />
+    <nd ref='-102546' />
+    <nd ref='-102547' />
+    <nd ref='-102544' />
+    <tag k='natural' v='water' />
+    <tag k='water' v='pond' />
+  </way>
+  <way id='-104798' action='modify' visible='true'>
+    <nd ref='-102547' />
+    <nd ref='-102544' />
+    <nd ref='-102552' />
+    <nd ref='-102548' />
+    <nd ref='-102549' />
+    <nd ref='-102553' />
+    <nd ref='-102547' />
+    <tag k='natural' v='wood' />
+  </way>
+</osm>

test/data/issue-24/test2.osm

@@ -0,0 +1,42 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<osm version='0.6' generator='JOSM'>
+  <bounds minlat='-23.1550787' minlon='47.0651293' maxlat='-23.1489626' maxlon='47.0745707' origin='CGImap 0.8.3 (2888343 spike-08.openstreetmap.org)' />
+  <bounds minlat='-23.1550787' minlon='47.0651293' maxlat='-23.1489626' maxlon='47.0745707' origin='OpenStreetMap server' />
+  <node id='-102573' action='modify' visible='true' lat='-23.15045423564' lon='47.06711683267' />
+  <node id='-102574' action='modify' visible='true' lat='-23.1513673192' lon='47.06758000968' />
+  <node id='-102576' action='modify' visible='true' lat='-23.15059733124' lon='47.06959946141' />
+  <node id='-102577' action='modify' visible='true' lat='-23.15108453652' lon='47.06960687224' />
+  <node id='-102578' action='modify' visible='true' lat='-23.15100385464' lon='47.06707902709' />
+  <node id='-102579' action='modify' visible='true' lat='-23.15019717563' lon='47.06768486771' />
+  <node id='-102580' action='modify' visible='true' lat='-23.15020538297' lon='47.06919191663' />
+  <node id='-102581' action='modify' visible='true' lat='-23.15139691206' lon='47.06905852456' />
+  <way id='-104830' action='modify' visible='true'>
+    <nd ref='-102579' />
+    <nd ref='-102573' />
+    <nd ref='-102578' />
+    <nd ref='-102574' />
+    <nd ref='-102575' />
+    <nd ref='-102574' />
+    <nd ref='-102578' />
+    <nd ref='-102573' />
+    <nd ref='-102579' />
+    <nd ref='-102572' />
+    <nd ref='-102579' />
+    <tag k='natural' v='water' />
+    <tag k='water' v='pond' />
+  </way>
+  <way id='-104831' action='modify' visible='true'>
+    <nd ref='-102581' />
+    <nd ref='-102575' />
+    <nd ref='-102574' />
+    <nd ref='-102578' />
+    <nd ref='-102573' />
+    <nd ref='-102579' />
+    <nd ref='-102572' />
+    <nd ref='-102580' />
+    <nd ref='-102576' />
+    <nd ref='-102577' />
+    <nd ref='-102581' />
+    <tag k='natural' v='wood' />
+  </way>
+</osm>