Changeset 35556 in osm for applications


Ignore:
Timestamp:
2020-09-25T17:23:43+02:00 (4 years ago)
Author:
GerdP
Message:

fix #josm19680: Error when reverting changeset with JOSM reverter plugin
The collection toDelete contains objects in random order, but we have to process it in a certain order to make sure that parents are processed before children.
The patched code also should work for the case that a nested relations are contained in this collection where and conflict is found for a relation which previously was found as a parent. In this rare case the iteration starts again after the conflict was produced and the relation was removed from the collection.

File:
1 edited

Legend:

Unmodified
Added
Removed
  • applications/editors/josm/plugins/reverter/src/reverter/ChangesetReverter.java

    r35382 r35556  
    1515import java.util.List;
    1616import java.util.Map;
     17import java.util.stream.Collectors;
    1718
    1819import org.openstreetmap.josm.command.Command;
     
    2829import org.openstreetmap.josm.data.osm.Node;
    2930import org.openstreetmap.josm.data.osm.OsmPrimitive;
     31import org.openstreetmap.josm.data.osm.OsmPrimitiveComparator;
    3032import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
    3133import org.openstreetmap.josm.data.osm.PrimitiveId;
     
    4749import org.openstreetmap.josm.io.OsmTransferException;
    4850import org.openstreetmap.josm.tools.Logging;
    49 import org.openstreetmap.josm.tools.Utils;
    5051
    5152/**
     
    235236                        trn("Downloading history for {0} object", "Downloading history for {0} objects", num, num)), num + 1);
    236237        try {
    237             HashMap<Long,Integer> nodeList = new HashMap<>(),
     238            HashMap<Long, Integer> nodeList = new HashMap<>(),
    238239                    wayList = new HashMap<>(),
    239240                    relationList = new HashMap<>();
     
    258259            if (progressMonitor.isCanceled()) return;
    259260            // If multi-read failed, retry with regular read
    260             for (Map.Entry<Long,Integer> entry : nodeList.entrySet()) {
     261            for (Map.Entry<Long, Integer> entry : nodeList.entrySet()) {
    261262                if (progressMonitor.isCanceled()) return;
    262                 readObjectVersion(rdr, new SimplePrimitiveId(entry.getKey(),OsmPrimitiveType.NODE), entry.getValue(), progressMonitor);
    263             }
    264             for (Map.Entry<Long,Integer> entry : wayList.entrySet()) {
     263                readObjectVersion(rdr, new SimplePrimitiveId(entry.getKey(), OsmPrimitiveType.NODE), entry.getValue(), progressMonitor);
     264            }
     265            for (Map.Entry<Long, Integer> entry : wayList.entrySet()) {
    265266                if (progressMonitor.isCanceled()) return;
    266                 readObjectVersion(rdr, new SimplePrimitiveId(entry.getKey(),OsmPrimitiveType.WAY), entry.getValue(), progressMonitor);
    267             }
    268             for (Map.Entry<Long,Integer> entry : relationList.entrySet()) {
     267                readObjectVersion(rdr, new SimplePrimitiveId(entry.getKey(), OsmPrimitiveType.WAY), entry.getValue(), progressMonitor);
     268            }
     269            for (Map.Entry<Long, Integer> entry : relationList.entrySet()) {
    269270                if (progressMonitor.isCanceled()) return;
    270                 readObjectVersion(rdr, new SimplePrimitiveId(entry.getKey(),OsmPrimitiveType.RELATION), entry.getValue(), progressMonitor);
     271                readObjectVersion(rdr, new SimplePrimitiveId(entry.getKey(), OsmPrimitiveType.RELATION), entry.getValue(), progressMonitor);
    271272            }
    272273            if (progressMonitor.isCanceled()) return;
     
    445446         * isn't going to be deleted or modified, create a conflict.
    446447         */
    447         for (Iterator<OsmPrimitive> it = toDelete.iterator(); it.hasNext();) {
    448             OsmPrimitive p = it.next();
    449             if (p.isDeleted()) {
    450                 it.remove();
    451                 continue;
    452             }
    453             for (OsmPrimitive referrer : p.getReferrers()) {
    454                 if (toDelete.contains(referrer)) continue; // object is going to be deleted
    455                 if (nds.getPrimitiveById(referrer) != null)
    456                     continue; /* object is going to be modified so it cannot refer to
    457                                * objects created in changeset to be reverted
    458                                */
    459                 if (!conflicted.contains(p)) {
    460                     cmds.add(new ConflictAddCommand(layer.data, createConflict(p, true)));
    461                     conflicted.add(p);
     448        List<OsmPrimitive> delSorted = toDelete.stream()
     449                .filter(p -> !p.isDeleted())
     450                .sorted(OsmPrimitiveComparator.orderingRelationsWaysNodes())
     451                .collect(Collectors.toList());
     452        boolean restartNeeded;
     453        do {
     454            restartNeeded = false;
     455            for (int i = 0; i < delSorted.size() && !restartNeeded; i++) {
     456                OsmPrimitive p = delSorted.get(i);
     457                for (OsmPrimitive referrer : p.getReferrers()) {
     458                    if (toDelete.contains(referrer)) continue; // object is going to be deleted
     459                    if (nds.getPrimitiveById(referrer) != null)
     460                        continue; /* object is going to be modified so it cannot refer to
     461                         * objects created in changeset to be reverted
     462                         */
     463                    if (conflicted.add(p)) {
     464                        cmds.add(new ConflictAddCommand(layer.data, createConflict(p, true)));
     465                        if (p instanceof Relation) {
     466                            // handle possible special case with nested relations
     467                            for (int j = 0; j < i; j++) {
     468                                if (delSorted.get(j).getReferrers().contains(p)) {
     469                                    // we have to create a conflict for a previously processed relation
     470                                    restartNeeded = true;
     471                                    break;
     472                                }
     473                            }
     474                        }
     475                    }
     476                    toDelete.remove(p);
     477                    break;
    462478                }
    463                 it.remove();
    464                 break;
    465             }
    466         }
    467 
     479            }
     480        } while (restartNeeded);
    468481        // Create a Command to delete all marked objects
    469         Collection<? extends OsmPrimitive> collection;
    470         collection = Utils.filteredCollection(toDelete, Relation.class);
    471         if (!collection.isEmpty()) cmds.add(new DeleteCommand(collection));
    472         collection = Utils.filteredCollection(toDelete, Way.class);
    473         if (!collection.isEmpty()) cmds.add(new DeleteCommand(collection));
    474         collection = Utils.filteredCollection(toDelete, Node.class);
    475         if (!collection.isEmpty()) cmds.add(new DeleteCommand(collection));
     482        delSorted.retainAll(toDelete);
     483        if (!delSorted.isEmpty()) {
     484            cmds.add(new DeleteCommand(delSorted));
     485        }
    476486        return cmds;
    477487    }
Note: See TracChangeset for help on using the changeset viewer.