Changeset 17240 in josm


Ignore:
Timestamp:
2020-10-19T10:56:31+02:00 (6 weeks ago)
Author:
GerdP
Message:

fix #14620: DataIntegrityProblemException: Deleted node referenced while using conflation
The plugin shows an error in the core class AddPrimitivesCommand.

  • AddPrimitivesCommand undo crashes with existing data with status "deleted" Patch by Tyndare, modified
  • add stability for primitives with the same unique id but different types, like a way and a node with the same id.
  • add unit tests for both
Location:
trunk
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/command/AddPrimitivesCommand.java

    r16436 r17240  
    100100            // a subsequent command (e.g. MoveCommand) cannot be redone.
    101101            for (OsmPrimitive osm : createdPrimitives) {
    102                 if (preExistingData.stream().anyMatch(pd -> pd.getUniqueId() == osm.getUniqueId())) {
    103                     Optional<PrimitiveData> o = data.stream().filter(pd -> pd.getUniqueId() == osm.getUniqueId()).findAny();
     102                if (preExistingData.stream().anyMatch(pd -> pd.getPrimitiveId().equals(osm.getPrimitiveId()))) {
     103                    Optional<PrimitiveData> o = data.stream()
     104                            .filter(pd -> pd.getPrimitiveId().equals(osm.getPrimitiveId())).findAny();
    104105                    if (o.isPresent()) {
    105106                        osm.load(o.get());
     
    126127            createdPrimitives = PurgeCommand.topoSort(createdPrimitives);
    127128        }
    128         for (OsmPrimitive osm : createdPrimitives) {
    129             Optional<PrimitiveData> previous = preExistingData.stream().filter(pd -> pd.getUniqueId() == osm.getUniqueId()).findAny();
     129        // reversed order, see #14620
     130        for (int i = createdPrimitives.size() - 1; i >= 0; i--) {
     131            OsmPrimitive osm = createdPrimitives.get(i);
     132            Optional<PrimitiveData> previous = preExistingData.stream()
     133                    .filter(pd -> pd.getPrimitiveId().equals(osm.getPrimitiveId())).findAny();
    130134            if (previous.isPresent()) {
    131135                osm.load(previous.get());
  • trunk/test/unit/org/openstreetmap/josm/command/AddPrimitivesCommandTest.java

    r13907 r17240  
    187187        assertTrue(command.executeCommand());
    188188
    189         assertEquals(3, ds.getNodes().size());
    190         assertEquals(1, ds.getWays().size());
    191 
    192189        for (int i = 0; i < 2; i++) {
    193190            // Needs to work multiple times.
     191            assertEquals(3, ds.getNodes().size());
     192            assertEquals(1, ds.getWays().size());
     193
    194194            command.undoCommand();
    195195
     
    199199            // redo
    200200            assertTrue(command.executeCommand());
    201 
    202             assertEquals(3, ds.getNodes().size());
    203             assertEquals(1, ds.getWays().size());
     201        }
     202    }
     203
     204    /**
     205     * Tests if the undo command does not remove
     206     * data ignored by by the add command because they where already existing.
     207     * Simulates regression in #14620
     208     */
     209    @Test
     210    public void testUndoIgnoresExistingAsDeleted() {
     211        DataSet ds = new DataSet();
     212
     213        List<PrimitiveData> testData = createTestData();
     214
     215        assertTrue(new AddPrimitivesCommand(testData, ds).executeCommand());
     216        assertEquals(2, ds.getNodes().size());
     217        assertEquals(1, ds.getWays().size());
     218
     219        ds.getNodes().forEach(n -> n.setDeleted(true));
     220
     221        AddPrimitivesCommand command = new AddPrimitivesCommand(testData, ds);
     222
     223        assertTrue(command.executeCommand());
     224
     225        for (int i = 0; i < 2; i++) {
     226            // Needs to work multiple times.
     227            assertEquals(2, ds.getNodes().size());
     228            assertEquals(1, ds.getWays().size());
     229
     230            command.undoCommand();
     231
     232            assertEquals(2, ds.getNodes().size());
     233            assertEquals(1, ds.getWays().size());
     234
     235            // redo
     236            assertTrue(command.executeCommand());
     237        }
     238    }
     239
     240    /**
     241     * Tests if the undo command does not remove
     242     * data ignored by by the add command because they where already existing.
     243     */
     244    @Test
     245    public void testUndoIgnoresExistingSameUniqueIdDifferentType() {
     246        DataSet ds = new DataSet();
     247
     248        List<PrimitiveData> testData = new ArrayList<>(createTestData());
     249
     250        assertTrue(new AddPrimitivesCommand(testData, ds).executeCommand());
     251        assertEquals(2, ds.getNodes().size());
     252        assertEquals(1, ds.getWays().size());
     253
     254        NodeData n7Data = createTestNode(7);
     255        NodeData n8Data = createTestNode(8);
     256        Way w2 = new Way(5);
     257        Node n7 = new Node(7);
     258        n7.load(n7Data);
     259        Node n8 = new Node(8);
     260        n8.load(n8Data);
     261        w2.setNodes(Arrays.asList(n7, n8));
     262        testData.set(2, createTestNode(7));
     263        testData.add(n8.save());
     264        testData.add(w2.save());
     265
     266        AddPrimitivesCommand command = new AddPrimitivesCommand(testData, ds);
     267
     268        assertTrue(command.executeCommand());
     269
     270        for (int i = 0; i < 2; i++) {
     271            assertEquals(4, ds.getNodes().size());
     272            assertEquals(2, ds.getWays().size());
     273
     274            // Needs to work multiple times.
     275            command.undoCommand();
     276
     277            assertEquals(2, ds.getNodes().size());
     278            assertEquals(1, ds.getWays().size());
     279
     280            // redo
     281            assertTrue(command.executeCommand());
     282
    204283        }
    205284    }
     
    266345        NodeData node1 = createTestNode(5);
    267346        NodeData node2 = createTestNode(6);
    268         WayData way = new WayData();
     347        WayData way = new WayData(2);
    269348        way.put("test", "test");
    270349        way.setNodeIds(Arrays.asList(node1.getId(), node2.getId()));
Note: See TracChangeset for help on using the changeset viewer.