Changeset 3034 in josm for trunk/src

Timestamp:

2010-02-23T08:58:12+01:00 (10 years ago)

Author:

jttt

Message:

Fix #4467 Don't silently drop locally deleted member primitives from downloaded ways and relation (fix the issue when deleted primitive is referenced)

Location:

trunk/src/org/openstreetmap/josm

Files:

• command/Command.java (modified) (3 diffs)
• command/CoordinateConflictResolveCommand.java (modified) (3 diffs)
• command/DeletedStateConflictResolveCommand.java (modified) (4 diffs)
• command/ModifiedConflictResolveCommand.java (modified) (2 diffs)
• command/TagConflictResolveCommand.java (modified) (2 diffs)
• command/VersionConflictResolveCommand.java (modified) (3 diffs)
• command/WayNodesConflictResolverCommand.java (modified) (1 diff)
• data/conflict/Conflict.java (modified) (2 diffs)
• data/conflict/ConflictCollection.java (modified) (1 diff)
• data/osm/DataSetMerger.java (modified) (5 diffs)
• data/osm/User.java (modified) (1 diff)
• gui/conflict/pair/ConflictResolver.java (modified) (8 diffs)
• gui/conflict/pair/IConflictResolver.java (modified) (2 diffs)
• gui/conflict/pair/nodes/NodeListMergeModel.java (modified) (2 diffs)
• gui/conflict/pair/nodes/NodeListMerger.java (modified) (2 diffs)
• gui/conflict/pair/properties/PropertiesMergeModel.java (modified) (5 diffs)
• gui/conflict/pair/properties/PropertiesMerger.java (modified) (2 diffs)
• gui/conflict/pair/relation/RelationMemberMerger.java (modified) (2 diffs)
• gui/conflict/pair/tags/TagMergeModel.java (modified) (2 diffs)
• gui/conflict/pair/tags/TagMerger.java (modified) (2 diffs)
• gui/dialogs/ConflictDialog.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/command/Command.java

@@ -2 +2 @@
 package org.openstreetmap.josm.command;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -32 +33 @@
 
     private static final class CloneVisitor extends AbstractVisitor {
-        public Map<OsmPrimitive, PrimitiveData> orig = new HashMap<OsmPrimitive, PrimitiveData>();
+        public final Map<OsmPrimitive, PrimitiveData> orig = new LinkedHashMap<OsmPrimitive, PrimitiveData>();
 
         public void visit(Node n) {
@@ -72 +73 @@
     public boolean executeCommand() {
         CloneVisitor visitor = new CloneVisitor();
-        Collection<OsmPrimitive> all = new HashSet<OsmPrimitive>();
+        Collection<OsmPrimitive> all = new ArrayList<OsmPrimitive>();
         fillModifiedData(all, all, all);
         for (OsmPrimitive osm : all) {

trunk/src/org/openstreetmap/josm/command/CoordinateConflictResolveCommand.java

@@ -23 +23 @@
 
     /** the conflict to resolve */
-    private Conflict<Node> conflict;
+    private Conflict<? extends OsmPrimitive> conflict;
 
     /** the merge decision */
@@ -35 +35 @@
      * @param decision the merge decision
      */
-    public CoordinateConflictResolveCommand(Node my, Node their, MergeDecisionType decision) {
-        this.conflict = new Conflict<Node>(my,their);
+    public CoordinateConflictResolveCommand(Conflict<? extends OsmPrimitive> conflict, MergeDecisionType decision) {
+        this.conflict = conflict;
         this.decision = decision;
     }
@@ -61 +61 @@
             // do nothing
         } else if (decision.equals(MergeDecisionType.KEEP_THEIR)) {
-            Node my = conflict.getMy();
-            Node their = conflict.getTheir();
+            Node my = (Node)conflict.getMy();
+            Node their = (Node)conflict.getTheir();
             my.setCoor(their.getCoor());
         } else

trunk/src/org/openstreetmap/josm/command/DeletedStateConflictResolveCommand.java

@@ -23 +23 @@
 
     /** the conflict to resolve */
-    private Conflict<OsmPrimitive> conflict;
+    private Conflict<? extends OsmPrimitive> conflict;
 
     /** the merge decision */
@@ -35 +35 @@
      * @param decision the merge decision
      */
-    public DeletedStateConflictResolveCommand(OsmPrimitive my, OsmPrimitive their, MergeDecisionType decision) {
-        this.conflict = new Conflict<OsmPrimitive>(my, their);
+    public DeletedStateConflictResolveCommand(Conflict<? extends OsmPrimitive> conflict, MergeDecisionType decision) {
+        this.conflict = conflict;
         this.decision = decision;
     }
@@ -61 +61 @@
 
         if (decision.equals(MergeDecisionType.KEEP_MINE)) {
-            if (conflict.getMy().isDeleted()) {
+            if (conflict.getMy().isDeleted() || conflict.isMyDeleted()) {
                 // because my was involved in a conflict it my still be referred
                 // to from a way or a relation. Fix this now.
                 //
                 layer.data.unlinkReferencesToPrimitive(conflict.getMy());
+                conflict.getMy().setDeleted(true);
             }
         } else if (decision.equals(MergeDecisionType.KEEP_THEIR)) {
@@ -86 +87 @@
             Collection<OsmPrimitive> added) {
         modified.add(conflict.getMy());
+        modified.addAll(conflict.getMy().getReferrers());
     }
 }

trunk/src/org/openstreetmap/josm/command/ModifiedConflictResolveCommand.java

@@ -24 +24 @@
 
     /** the conflict to resolve */
-    private Conflict<OsmPrimitive> conflict;
+    private Conflict<? extends OsmPrimitive> conflict;
 
     /**
@@ -31 +31 @@
      * @param their their primitive (i.e. the primitive from the server)
      */
-    public ModifiedConflictResolveCommand(OsmPrimitive my, OsmPrimitive their) {
-        conflict = new Conflict<OsmPrimitive>(my, their);
+    public ModifiedConflictResolveCommand(Conflict<? extends OsmPrimitive> conflict) {
+        this.conflict = conflict;
     }
 

trunk/src/org/openstreetmap/josm/command/TagConflictResolveCommand.java

@@ -29 +29 @@
 
     /** the conflict to resolve */
-    private Conflict<OsmPrimitive> conflict;
+    private Conflict<? extends OsmPrimitive> conflict;
 
     /** the list of merge decisions, represented as {@see TagMergeItem}s */
@@ -56 +56 @@
      * @param mergeItems the list of merge decisions, represented as {@see TagMergeItem}s
      */
-    public TagConflictResolveCommand(OsmPrimitive my, OsmPrimitive their, List<TagMergeItem> mergeItems) {
-        this.conflict = new Conflict<OsmPrimitive>(my,their);
+    public TagConflictResolveCommand(Conflict<? extends OsmPrimitive> conflict, List<TagMergeItem> mergeItems) {
+        this.conflict = conflict;
         this.mergeItems = mergeItems;
     }

trunk/src/org/openstreetmap/josm/command/VersionConflictResolveCommand.java

@@ -24 +24 @@
 
     /** the conflict to resolve */
-    private Conflict<OsmPrimitive> conflict;
+    private Conflict<? extends OsmPrimitive> conflict;
 
     /**
@@ -31 +31 @@
      * @param their their primitive (i.e. the primitive from the server)
      */
-    public VersionConflictResolveCommand(OsmPrimitive my, OsmPrimitive their) {
-        conflict = new Conflict<OsmPrimitive>(my, their);
+    public VersionConflictResolveCommand(Conflict<? extends OsmPrimitive> conflict) {
+        this.conflict = conflict;
     }
 
@@ -39 +39 @@
         String msg = "";
         switch(OsmPrimitiveType.from(conflict.getMy())) {
-            case NODE: msg = marktr("Resolve version conflict for node {0}"); break;
-            case WAY: msg = marktr("Resolve version conflict for way {0}"); break;
-            case RELATION: msg = marktr("Resolve version conflict for relation {0}"); break;
+        case NODE: msg = marktr("Resolve version conflict for node {0}"); break;
+        case WAY: msg = marktr("Resolve version conflict for way {0}"); break;
+        case RELATION: msg = marktr("Resolve version conflict for relation {0}"); break;
         }
         return new DefaultMutableTreeNode(

trunk/src/org/openstreetmap/josm/command/WayNodesConflictResolverCommand.java

@@ -41 +41 @@
      * @param mergedNodeList  the list of merged nodes
      */
-    public WayNodesConflictResolverCommand(Way my, Way their, List<Node> mergedNodeList) {
-        conflict = new Conflict<Way>(my,their);
+    @SuppressWarnings("unchecked")
+    public WayNodesConflictResolverCommand(Conflict<? extends OsmPrimitive> conflict, List<Node> mergedNodeList) {
+        this.conflict = (Conflict<Way>) conflict;
         this.mergedNodeList = mergedNodeList;
     }

trunk/src/org/openstreetmap/josm/data/conflict/Conflict.java

@@ -21 +21 @@
     private final T my;
     private final T their;
+    private final boolean isMyDeleted;
 
     public Conflict(T my, T their) {
+        this(my, their, false);
+    }
+
+    public Conflict(T my, T their, boolean isMyDeleted) {
         this.my = my;
         this.their = their;
+        this.isMyDeleted = isMyDeleted;
     }
 
@@ -96 +102 @@
         return true;
     }
+
+    /**
+     * 
+     * @return True if my primitive was deleted but it has set non deleted status because it's referred by another
+     * primitive and references to deleted primitives are not allowed.
+     */
+    public boolean isMyDeleted() {
+        return isMyDeleted;
+    }
 }

trunk/src/org/openstreetmap/josm/data/conflict/ConflictCollection.java

@@ -33 +33 @@
  * </ul>
  */
-public class ConflictCollection implements Iterable<Conflict<?>>{
-    private final List<Conflict<?>> conflicts;
+public class ConflictCollection implements Iterable<Conflict<? extends OsmPrimitive>>{
+    private final List<Conflict<? extends OsmPrimitive>> conflicts;
     private CopyOnWriteArrayList<IConflictListener> listeners;
 

trunk/src/org/openstreetmap/josm/data/osm/DataSetMerger.java

@@ -13 +13 @@
 import java.util.logging.Logger;
 
+import org.openstreetmap.josm.data.conflict.Conflict;
 import org.openstreetmap.josm.data.conflict.ConflictCollection;
 import org.openstreetmap.josm.tools.CheckParameterUtil;
@@ -160 +161 @@
      * @param other
      */
+    //TODO This method is probably useless
     protected void fixIncompleteParentWays(Node other) {
         Node myNode = (Node)getMergeTarget(other);
@@ -224 +226 @@
                     newNodes.add(targetNode);
                     if (targetNode.isDeleted() && !conflicts.hasConflictForMy(targetNode)) {
-                        conflicts.add(targetNode, sourceNode);
+                        conflicts.add(new Conflict<OsmPrimitive>(targetNode, sourceNode, true));
+                        targetNode.setDeleted(false);
                     }
                 } else {
@@ -255 +258 @@
                 newMembers.add(newMember);
                 if (targetMember.isDeleted() && !conflicts.hasConflictForMy(targetMember)) {
-                    conflicts.add(targetMember, sourceMember.getMember());
+                    conflicts.add(new Conflict<OsmPrimitive>(targetMember, sourceMember.getMember(), true));
+                    targetMember.setDeleted(false);
                 }
             } else {
@@ -319 +323 @@
             for (OsmPrimitive referrer: source.getReferrers()) {
                 if (targetDataSet.getPrimitiveById(referrer.getPrimitiveId()) == null) {
-                    conflicts.add(target, source);
+                    conflicts.add(new Conflict<OsmPrimitive>(target, source, true));
+                    target.setDeleted(false);
                     break;
                 }

trunk/src/org/openstreetmap/josm/data/osm/User.java

@@ -27 +27 @@
 
     private static long getNextLocalUid() {
-        synchronized(User.class) {
-            return uidCounter.decrementAndGet();
-        }
+        return uidCounter.decrementAndGet();
     }
 

trunk/src/org/openstreetmap/josm/gui/conflict/pair/ConflictResolver.java

@@ -19 +19 @@
 import org.openstreetmap.josm.command.SequenceCommand;
 import org.openstreetmap.josm.command.VersionConflictResolveCommand;
+import org.openstreetmap.josm.data.conflict.Conflict;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -77 +78 @@
     private OsmPrimitive my;
     private OsmPrimitive their;
+    private Conflict<? extends OsmPrimitive> conflict;
 
     private ImageIcon mergeComplete;
@@ -228 +230 @@
      *
      */
-    public void populate(OsmPrimitive my, OsmPrimitive their) {
-        setMy(my);
-        setTheir(their);
-        propertiesMerger.populate(my, their);
+    public void populate(Conflict<? extends OsmPrimitive> conflict) {
+        setMy(conflict.getMy());
+        setTheir(conflict.getTheir());
+        this.conflict = conflict;
+        propertiesMerger.populate(conflict);
         if (propertiesMerger.getModel().hasVisibleStateConflict()) {
             tabbedPane.setEnabledAt(1, false);
@@ -239 +242 @@
         }
         tabbedPane.setEnabledAt(0, true);
-        tagMerger.populate(my, their);
+        tagMerger.populate(conflict);
         tabbedPane.setEnabledAt(1, true);
 
@@ -246 +249 @@
             tabbedPane.setEnabledAt(3,false);
         } else if (my instanceof Way) {
-            nodeListMerger.populate(my, their);
+            nodeListMerger.populate(conflict);
             tabbedPane.setEnabledAt(2, true);
             tabbedPane.setEnabledAt(3, false);
@@ -252 +255 @@
             tabbedPane.setIconAt(3, null);
         } else if (my instanceof Relation) {
-            relationMemberMerger.populate(my, their);
+            relationMemberMerger.populate(conflict);
             tabbedPane.setEnabledAt(2, false);
             tabbedPane.setTitleAt(2,tr("Nodes"));
@@ -271 +274 @@
         if (propertiesMerger.getModel().hasVisibleStateConflict()) {
             if (propertiesMerger.getModel().isDecidedVisibleState()) {
-                commands.addAll(propertiesMerger.getModel().buildResolveCommand(my, their));
+                commands.addAll(propertiesMerger.getModel().buildResolveCommand(conflict));
             }
         } else {
             if (tagMerger.getModel().getNumResolvedConflicts() > 0) {
-                commands.add(tagMerger.getModel().buildResolveCommand(my, their));
-            }
-            commands.addAll(propertiesMerger.getModel().buildResolveCommand(my, their));
+                commands.add(tagMerger.getModel().buildResolveCommand(conflict));
+            }
+            commands.addAll(propertiesMerger.getModel().buildResolveCommand(conflict));
             if (my instanceof Way && nodeListMerger.getModel().isFrozen()) {
                 NodeListMergeModel model  =(NodeListMergeModel)nodeListMerger.getModel();
-                commands.add(model.buildResolveCommand((Way)my, (Way)their));
+                commands.add(model.buildResolveCommand(conflict));
             } else if (my instanceof Relation && relationMemberMerger.getModel().isFrozen()) {
                 RelationMemberListMergeModel model  =(RelationMemberListMergeModel)relationMemberMerger.getModel();
@@ -286 +289 @@
             }
             if (isResolvedCompletely()) {
-                commands.add(new VersionConflictResolveCommand(my, their));
-                commands.add(new ModifiedConflictResolveCommand(my, their));
+                commands.add(new VersionConflictResolveCommand(conflict));
+                commands.add(new ModifiedConflictResolveCommand(conflict));
             }
         }

trunk/src/org/openstreetmap/josm/gui/conflict/pair/IConflictResolver.java

@@ -2 +2 @@
 package org.openstreetmap.josm.gui.conflict.pair;
 
+import org.openstreetmap.josm.data.conflict.Conflict;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 
@@ -7 +8 @@
 
     void deletePrimitive(boolean deleted);
-    void populate(OsmPrimitive my, OsmPrimitive their);
+    void populate(Conflict<? extends OsmPrimitive> conflict);
 
 }

trunk/src/org/openstreetmap/josm/gui/conflict/pair/nodes/NodeListMergeModel.java

@@ -9 +9 @@
 
 import org.openstreetmap.josm.command.WayNodesConflictResolverCommand;
+import org.openstreetmap.josm.data.conflict.Conflict;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.Node;
@@ -61 +62 @@
      * @param their  their way. Must not be null
      * @return the command
-     * @exception IllegalArgumentException thrown, if my is null or not a {@see Way}
-     * @exception IllegalArgumentException thrown, if their is null or not a {@see Way}
      * @exception IllegalStateException thrown, if the merge is not yet frozen
      */
-    public WayNodesConflictResolverCommand buildResolveCommand(Way my, Way their) {
-        CheckParameterUtil.ensureParameterNotNull(my, "my");
-        CheckParameterUtil.ensureParameterNotNull(their, "their");
+    public WayNodesConflictResolverCommand buildResolveCommand(Conflict<? extends OsmPrimitive> conflict) {
         if (! isFrozen())
             throw new IllegalArgumentException(tr("Merged nodes not frozen yet. Cannot build resolution command."));
-        return new WayNodesConflictResolverCommand(my, their, getMergedEntries());
+        return new WayNodesConflictResolverCommand(conflict, getMergedEntries());
     }
 

trunk/src/org/openstreetmap/josm/gui/conflict/pair/nodes/NodeListMerger.java

@@ -4 +4 @@
 import javax.swing.JTable;
 
+import org.openstreetmap.josm.data.conflict.Conflict;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -63 +64 @@
     }
 
-    public void populate(OsmPrimitive my, OsmPrimitive their) {
-        ((NodeListMergeModel)model).populate((Way)my, (Way)their);
+    public void populate(Conflict<? extends OsmPrimitive> conflict) {
+        ((NodeListMergeModel)model).populate((Way)conflict.getMy(), (Way)conflict.getTheir());
     }
 

trunk/src/org/openstreetmap/josm/gui/conflict/pair/properties/PropertiesMergeModel.java

@@ -21 +21 @@
 import org.openstreetmap.josm.command.PurgePrimitivesCommand;
 import org.openstreetmap.josm.command.UndeletePrimitivesCommand;
+import org.openstreetmap.josm.data.conflict.Conflict;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.DataSet;
@@ -161 +162 @@
      * @param their their version of the primitive
      */
-    public void populate(OsmPrimitive my, OsmPrimitive their) {
-        this.my = my;
+    public void populate(Conflict<? extends OsmPrimitive> conflict) {
+        this.my = conflict.getMy();
+        OsmPrimitive their = conflict.getTheir();
         if (my instanceof Node) {
             myCoords = ((Node)my).getCoor();
@@ -171 +173 @@
         }
 
-        myDeletedState = my.isDeleted();
+        myDeletedState =  conflict.isMyDeleted() || my.isDeleted();
         theirDeletedState = their.isDeleted();
 
@@ -418 +420 @@
      * @return the list of commands
      */
-    public List<Command> buildResolveCommand(OsmPrimitive my, OsmPrimitive their) throws OperationCancelledException{
-        ArrayList<Command> cmds = new ArrayList<Command>();
+    public List<Command> buildResolveCommand(Conflict<? extends OsmPrimitive> conflict) throws OperationCancelledException{
+        OsmPrimitive my = conflict.getMy();
+        List<Command> cmds = new ArrayList<Command>();
         if (hasVisibleStateConflict() && isDecidedVisibleState()) {
             if (isVisibleStateDecision(MergeDecisionType.KEEP_MINE)) {
@@ -436 +439 @@
         }
         if (hasCoordConflict() && isDecidedCoord()) {
-            cmds.add(new CoordinateConflictResolveCommand((Node)my, (Node)their, coordMergeDecision));
+            cmds.add(new CoordinateConflictResolveCommand(conflict, coordMergeDecision));
         }
         if (hasDeletedStateConflict() && isDecidedDeletedState()) {
-            cmds.add(new DeletedStateConflictResolveCommand(my, their, deletedMergeDecision));
+            cmds.add(new DeletedStateConflictResolveCommand(conflict, deletedMergeDecision));
         }
         return cmds;

trunk/src/org/openstreetmap/josm/gui/conflict/pair/properties/PropertiesMerger.java

@@ -22 +22 @@
 import javax.swing.JPanel;
 
+import org.openstreetmap.josm.data.conflict.Conflict;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -718 +719 @@
     }
 
-    public void populate(OsmPrimitive my, OsmPrimitive their) {
-        model.populate(my, their);
+    public void populate(Conflict<? extends OsmPrimitive> conflict) {
+        model.populate(conflict);
     }
 }

trunk/src/org/openstreetmap/josm/gui/conflict/pair/relation/RelationMemberMerger.java

@@ -5 +5 @@
 import javax.swing.JTable;
 
+import org.openstreetmap.josm.data.conflict.Conflict;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Relation;
@@ -54 +55 @@
     }
 
-    public void populate(OsmPrimitive my, OsmPrimitive their) {
+    public void populate(Conflict<? extends OsmPrimitive> conflict) {
         RelationMemberListMergeModel model = (RelationMemberListMergeModel)getModel();
-        model.populate((Relation)my, (Relation)their);
+        model.populate((Relation)conflict.getMy(), (Relation)conflict.getTheir());
     }
 

trunk/src/org/openstreetmap/josm/gui/conflict/pair/tags/TagMergeModel.java

@@ -12 +12 @@
 
 import org.openstreetmap.josm.command.TagConflictResolveCommand;
+import org.openstreetmap.josm.data.conflict.Conflict;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.gui.conflict.pair.MergeDecisionType;
@@ -189 +190 @@
     }
 
-    public TagConflictResolveCommand buildResolveCommand(OsmPrimitive my, OsmPrimitive their) {
-        return new TagConflictResolveCommand(my,  their, tagMergeItems);
+    public TagConflictResolveCommand buildResolveCommand(Conflict<? extends OsmPrimitive> conflict) {
+        return new TagConflictResolveCommand(conflict, tagMergeItems);
     }
 

trunk/src/org/openstreetmap/josm/gui/conflict/pair/tags/TagMerger.java

@@ -26 +26 @@
 import javax.swing.event.ListSelectionListener;
 
+import org.openstreetmap.josm.data.conflict.Conflict;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.gui.conflict.pair.IConflictResolver;
@@ -421 +422 @@
     }
 
-    public void populate(OsmPrimitive my, OsmPrimitive their) {
-        model.populate(my, their);
+    public void populate(Conflict<? extends OsmPrimitive> conflict) {
+        model.populate(conflict.getMy(), conflict.getTheir());
         mineTable.getSelectionModel().setSelectionInterval(0, 0);
         theirTable.getSelectionModel().setSelectionInterval(0, 0);

trunk/src/org/openstreetmap/josm/gui/dialogs/ConflictDialog.java

@@ -144 +144 @@
         }
 
-        Conflict<?> c = conflicts.get(index);
-        OsmPrimitive my = c.getMy();
-        OsmPrimitive their = c.getTheir();
+        Conflict<? extends OsmPrimitive> c = conflicts.get(index);
         ConflictResolutionDialog dialog = new ConflictResolutionDialog(Main.parent);
-        dialog.getConflictResolver().populate(my, their);
+        dialog.getConflictResolver().populate(c);
         dialog.setVisible(true);