Index: /trunk/src/org/openstreetmap/josm/data/osm/Node.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/Node.java (revision 2416) +++ /trunk/src/org/openstreetmap/josm/data/osm/Node.java (revision 2417) @@ -106,4 +106,23 @@ } + /** + * Merges the technical and semantical attributes from other onto this. + * + * Both this and other must be new, or both must be assigned an OSM ID. If both this and other + * have an assigend OSM id, the IDs have to be the same. + * + * @param other the other primitive. Must not be null. + * @throws IllegalArgumentException thrown if other is null. + * @throws DataIntegrityProblemException thrown if either this is new and other is not, or other is new and this is not + * @throws DataIntegrityProblemException thrown if other is new and other.getId() != this.getId() + */ + @Override + public void mergeFrom(OsmPrimitive other) { + super.mergeFrom(other); + if (!other.incomplete) { + setCoor(new LatLon(((Node)other).coor)); + } + } + @Override public void load(PrimitiveData data) { super.load(data); Index: /trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 2416) +++ /trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 2417) @@ -789,4 +789,30 @@ clearCached(); clearErrors(); + } + + /** + * Merges the technical and semantical attributes from other onto this. + * + * Both this and other must be new, or both must be assigned an OSM ID. If both this and other + * have an assigend OSM id, the IDs have to be the same. + * + * @param other the other primitive. Must not be null. + * @throws IllegalArgumentException thrown if other is null. + * @throws DataIntegrityProblemException thrown if either this is new and other is not, or other is new and this is not + * @throws DataIntegrityProblemException thrown if other is new and other.getId() != this.getId() + */ + public void mergeFrom(OsmPrimitive other) { + if (other == null) + throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "other")); + if (other.isNew() ^ isNew()) + throw new DataIntegrityProblemException(tr("Can''t merge because either of the participating primitives is new and the other is not")); + if (! other.isNew() && other.getId() != id) + throw new DataIntegrityProblemException(tr("Can''t merge primitives with different ids. This id is {0}, the other is {1}", id, other.getId())); + keys = other.keys == null ? null : new HashMap(other.keys); + timestamp = other.timestamp; + version = other.version; + incomplete = other.incomplete; + flags = other.flags; + user= other.user; } Index: /trunk/src/org/openstreetmap/josm/data/osm/Way.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/Way.java (revision 2416) +++ /trunk/src/org/openstreetmap/josm/data/osm/Way.java (revision 2417) @@ -229,5 +229,10 @@ return false; Way w = (Way)other; - return Arrays.equals(nodes, w.nodes); + if (getNodesCount() != w.getNodesCount()) return false; + for (int i=0;i nodeshash = new HashMap(); - private final HashMap wayshash = new HashMap(); - private final HashMap relshash = new HashMap(); - - /** - * A list of all primitives that got replaced with other primitives. - * Key is the primitives in the other's dataset and the value is the one that is now - * in ds.nodes instead. - */ - private Map merged; + /** + * A map of all primitives that got replaced with other primitives. + * Key is the primitive id in their dataset, the value is the id in my dataset + */ + private Map merged; /** @@ -58,22 +51,6 @@ this.myDataSet = myDataSet; this.theirDataSet = theirDataSet; - - for (Node n : myDataSet.getNodes()) { - if (!n.isNew()) { - nodeshash.put(n.getId(), n); - } - } - for (Way w : myDataSet.getWays()) { - if (!w.isNew()) { - wayshash.put(w.getId(), w); - } - } - for (Relation r : myDataSet.getRelations()) { - if (!r.isNew()) { - relshash.put(r.getId(), r); - } - } conflicts = new ConflictCollection(); - merged = new HashMap(); + merged = new HashMap(); } @@ -91,18 +68,11 @@ * @param

the type of the other primitive * @param other the other primitive - * @param myPrimitives the collection of my relevant primitives (i.e. only my - * primitives of the same type) - * @param otherPrimitives the collection of the other primitives - * @param primitivesWithDefinedIds the collection of my primitives with an - * assigned id (i.e. id != 0) - */ - protected

void mergePrimitive(P other, - DataSet myDataset, Collection

myPrimitives, HashMap primitivesWithDefinedIds) { - + */ + protected

void mergePrimitive(P other) { if (!other.isNew() ) { // try to merge onto a matching primitive with the same // defined id // - if (mergeById(primitivesWithDefinedIds, other)) + if (mergeById(other)) return; } else { @@ -110,5 +80,11 @@ // yet but which is equal in its semantic attributes // - for (P my : myPrimitives) { + Collection candidates = null; + switch(other.getType()) { + case NODE: candidates = myDataSet.getNodes(); break; + case WAY: candidates =myDataSet.getWays(); break; + case RELATION: candidates = myDataSet.getRelations(); break; + } + for (OsmPrimitive my : candidates) { if (!my.isNew()) { continue; @@ -119,4 +95,5 @@ // conflicts.add(my, other); + merged.put(other.getUniqueId(), my.getUniqueId()); } else { // copy the technical attributes from other @@ -126,5 +103,5 @@ my.setTimestamp(other.getTimestamp()); my.setModified(other.isModified()); - merged.put(other, my); + merged.put(other.getUniqueId(), my.getUniqueId()); } return; @@ -133,34 +110,46 @@ } // If we get here we didn't find a suitable primitive in - // my dataset. Just add other to my dataset. + // my dataset. Create a clone and add it to my dataset. // - - //TODO primitive must be only in one dataset at one time, so remove it from theirDataset. This obviously - // breaks theirDataset and we can only hope that nobody will try to use theirDataset after merging is finished - theirDataSet.removePrimitive(other); - myDataset.addPrimitive(other); + OsmPrimitive my = null; + switch(other.getType()) { + case NODE: my = other.isNew() ? new Node() : new Node(other.getId()); break; + case WAY: my = other.isNew() ? new Way() : new Way(other.getId()); break; + case RELATION: my = other.isNew() ? new Relation() : new Relation(other.getId()); break; + } + my.mergeFrom(other); + myDataSet.addPrimitive(my); + merged.put(other.getUniqueId(), my.getUniqueId()); } public void visit(Node other) { - mergePrimitive(other, myDataSet, myDataSet.getNodes(), nodeshash); + mergePrimitive(other); } public void visit(Way other) { - fixWay(other); - mergePrimitive(other, myDataSet, myDataSet.getWays(), wayshash); + mergePrimitive(other); } public void visit(Relation other) { - fixRelation(other); - mergePrimitive(other, myDataSet, myDataSet.getRelations(), relshash); - } - - protected void fixIncomplete(Way w) { - if (!w.incomplete)return; - if (w.incomplete && w.getNodesCount() == 0) return; - for (Node n: w.getNodes()) { + mergePrimitive(other); + } + + protected OsmPrimitive getMergeTarget(OsmPrimitive mergeSource) { + Long targetId = merged.get(mergeSource.getUniqueId()); + if (targetId == null) + throw new RuntimeException("no merge target for merge primitive " + mergeSource.getUniqueId() + " of type " + mergeSource.getType()); + return myDataSet.getPrimitiveById(targetId, mergeSource.getType()); + } + + protected void fixIncomplete(Way other) { + Way myWay = (Way)getMergeTarget(other); + if (myWay == null) + throw new RuntimeException(tr("Missing merge target for way with id {0}", other.getUniqueId())); + if (!myWay.incomplete)return; + if (myWay.incomplete && other.getNodesCount() == 0) return; + for (Node n: myWay.getNodes()) { if (n.incomplete) return; } - w.incomplete = false; + myWay.incomplete = false; } @@ -170,62 +159,95 @@ */ public void fixReferences() { - for (Way w : myDataSet.getWays()) { - fixWay(w); - fixIncomplete(w); - } - for (Relation r : myDataSet.getRelations()) { - fixRelation(r); - } - for (OsmPrimitive osm : conflicts.getMyConflictParties()) - if (osm instanceof Way) { - fixWay((Way) osm); - } else if (osm instanceof Relation) { - fixRelation((Relation) osm); - } - } - - - private void fixWay(Way w) { - boolean replacedSomething = false; - List newNodes = new LinkedList(); - for (Node myNode : w.getNodes()) { - Node mergedNode = (Node) merged.get(myNode); - if (mergedNode != null) { - if (!mergedNode.isDeleted()) { - newNodes.add(mergedNode); - } else { - // we've removed a node from a way during merging. - // Flag the way as being modified. - // - w.setModified(true); - } - replacedSomething = true; - } else { - newNodes.add(myNode); - } - } - if (replacedSomething) { - w.setNodes(newNodes); - } - } - - private void fixRelation(Relation r) { - boolean replacedSomething = false; + for (Way w : theirDataSet.getWays()) { + if (!conflicts.hasConflictForTheir(w)) { + mergeNodeList(w); + fixIncomplete(w); + } + } + for (Relation r : theirDataSet.getRelations()) { + if (!conflicts.hasConflictForTheir(r)) { + mergeRelationMembers(r); + } + } + } + + private void mergeNodeList(Way other) { + Way myWay = (Way)getMergeTarget(other); + if (myWay == null) + throw new RuntimeException(tr("Missing merge target for way with id {0}", other.getUniqueId())); + + List myNodes = new LinkedList(); + for (Node otherNode : other.getNodes()) { + Node myNode = (Node)getMergeTarget(otherNode); + if (myNode != null) { + if (!myNode.isDeleted()) { + myNodes.add(myNode); + } + } else + throw new RuntimeException(tr("Missing merge target for node with id {0}", otherNode.getUniqueId())); + } + + // check whether the node list has changed. If so, set the modified flag on the way + // + if (myWay.getNodes().size() != myNodes.size()) { + myWay.setModified(true); + } else { + for (int i=0; i< myWay.getNodesCount();i++) { + Node n1 = myWay.getNode(i); + Node n2 = myNodes.get(i); + if (n1.isNew() ^ n2.isNew()) { + myWay.setModified(true); + break; + } else if (n1.isNew() && n1 != n2) { + myWay.setModified(true); + break; + } else if (! n1.isNew() && n1.getId() != n2.getId()) { + myWay.setModified(true); + break; + } + } + } + myWay.setNodes(myNodes); + } + + private void mergeRelationMembers(Relation other) { + Relation myRelation = (Relation) getMergeTarget(other); + if (myRelation == null) + throw new RuntimeException(tr("Missing merge target for relation with id {0}", other.getUniqueId())); LinkedList newMembers = new LinkedList(); - for (RelationMember myMember : r.getMembers()) { - OsmPrimitive mergedMember = merged.get(myMember.getMember()); - if (mergedMember == null) { - newMembers.add(myMember); - } else { - if (! mergedMember.isDeleted()) { - RelationMember newMember = new RelationMember(myMember.getRole(), mergedMember); - newMembers.add(newMember); - } - replacedSomething = true; - } - } - if (replacedSomething) { - r.setMembers(newMembers); - } + for (RelationMember otherMember : other.getMembers()) { + OsmPrimitive mergedMember = getMergeTarget(otherMember.getMember()); + if (mergedMember == null) + throw new RuntimeException(tr("Missing merge target of type {0} with id {1}", mergedMember.getType(), mergedMember.getUniqueId())); + if (! mergedMember.isDeleted()) { + RelationMember newMember = new RelationMember(otherMember.getRole(), mergedMember); + newMembers.add(newMember); + } + } + + // check whether the list of relation members has changed + // + if (other.getMembersCount() != newMembers.size()) { + myRelation.setModified(true); + } else { + for (int i=0; iother into an existing primitive with the same id. * - * @param myPrimitivesWithDefinedIds the map of primitives (potential merge targets) with an id <> 0, for faster lookup - * by id. Key is the id, value the primitive with the given value. myPrimitives.valueSet() is a - * subset of primitives. * @param other the other primitive which is to be merged onto a primitive in my primitives * @return true, if this method was able to merge other with an existing node; false, otherwise */ - private

boolean mergeById(HashMap myPrimitivesWithDefinedIds, P other) { - + private

boolean mergeById(P other) { + OsmPrimitive my = myDataSet.getPrimitiveById(other.getId(), other.getType()); // merge other into an existing primitive with the same id, if possible // - if (myPrimitivesWithDefinedIds.containsKey(other.getId())) { - P my = myPrimitivesWithDefinedIds.get(other.getId()); + if (my != null) { if (my.getVersion() <= other.getVersion()) { if (! my.isVisible() && other.isVisible()) { @@ -254,5 +272,5 @@ Long.toString(my.getId()),Long.toString(my.getVersion()), Long.toString(other.getVersion()) )); - merged.put(other, my); + merged.put(other.getUniqueId(), my.getUniqueId()); } else if (my.isVisible() && ! other.isVisible()) { // this is always a conflict because the user has to decide whether @@ -262,29 +280,30 @@ // conflicts.add(my,other); + merged.put(other.getUniqueId(), my.getUniqueId()); } else if (my.incomplete && !other.incomplete) { // my is incomplete, other completes it // => merge other onto my // - my.incomplete = false; - my.cloneFrom(other); - merged.put(other, my); + my.mergeFrom(other); + merged.put(other.getUniqueId(), my.getUniqueId()); } else if (!my.incomplete && other.incomplete) { // my is complete and the other is incomplete // => keep mine, we have more information already // - merged.put(other, my); + merged.put(other.getUniqueId(), my.getUniqueId()); } else if (my.incomplete && other.incomplete) { // my and other are incomplete. Doesn't matter which one to // take. We take mine. // - merged.put(other, my); + merged.put(other.getUniqueId(), my.getUniqueId()); } else if (my.isDeleted() && ! other.isDeleted() && my.getVersion() == other.getVersion()) { // same version, but my is deleted. Assume mine takes precedence // otherwise too many conflicts when refreshing from the server - merged.put(other, my); + merged.put(other.getUniqueId(), my.getUniqueId()); } else if (my.isDeleted() != other.isDeleted()) { // differences in deleted state have to be resolved manually // conflicts.add(my,other); + merged.put(other.getUniqueId(), my.getUniqueId()); } else if (! my.isModified() && other.isModified()) { // my not modified. We can assume that other is the most recent version. @@ -295,19 +314,19 @@ myDataSet.unlinkReferencesToPrimitive(my); } - my.cloneFrom(other); - merged.put(other, my); + my.mergeFrom(other); + merged.put(other.getUniqueId(), my.getUniqueId()); } else if (! my.isModified() && !other.isModified() && my.getVersion() == other.getVersion()) { // both not modified. Keep mine // - merged.put(other,my); + merged.put(other.getUniqueId(),my.getUniqueId()); } else if (! my.isModified() && !other.isModified() && my.getVersion() < other.getVersion()) { // my not modified but other is newer. clone other onto mine. // - my.cloneFrom(other); - merged.put(other,my); + my.mergeFrom(other); + merged.put(other.getUniqueId(),my.getUniqueId()); } else if (my.isModified() && ! other.isModified() && my.getVersion() == other.getVersion()) { // my is same as other but mine is modified // => keep mine - merged.put(other, my); + merged.put(other.getUniqueId(), my.getUniqueId()); } else if (! my.hasEqualSemanticAttributes(other)) { // my is modified and is not semantically equal with other. Can't automatically @@ -315,4 +334,5 @@ // => create a conflict conflicts.add(my,other); + merged.put(other.getUniqueId(), my.getUniqueId()); } else { // clone from other, but keep the modified flag. Clone will mainly copy @@ -320,11 +340,11 @@ // attributes should already be equal if we get here. // - my.cloneFrom(other); + my.mergeFrom(other); my.setModified(true); - merged.put(other, my); + merged.put(other.getUniqueId(), my.getUniqueId()); } } else { // my.version > other.version => keep my version - merged.put(other, my); + merged.put(other.getUniqueId(), my.getUniqueId()); } return true; @@ -341,6 +361,12 @@ */ public void merge() { - for (final OsmPrimitive primitive : theirDataSet.allPrimitives()) { - primitive.visit(this); + for (Node node: theirDataSet.getNodes()) { + node.visit(this); + } + for (Way way: theirDataSet.getWays()) { + way.visit(this); + } + for (Relation relation: theirDataSet.getRelations()) { + relation.visit(this); } fixReferences(); @@ -356,5 +382,4 @@ } - /** * replies the map of conflicts Index: /trunk/test/unit/org/openstreetmap/josm/data/osm/visitor/MergeVisitorTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/osm/visitor/MergeVisitorTest.java (revision 2416) +++ /trunk/test/unit/org/openstreetmap/josm/data/osm/visitor/MergeVisitorTest.java (revision 2417) @@ -553,6 +553,4 @@ their.addPrimitive(n5); - their.addPrimitive(n5); - Node n4 = new Node(new LatLon(2,2)); n4.setOsmId(2,2); @@ -586,5 +584,5 @@ * their way has a higher version and different nodes. My way is modified. * - * => merge it onto my way not possbile, conflict + * => merge onto my way not possible, create a conflict * */ @@ -621,6 +619,4 @@ Node n5 = new Node(new LatLon(1,1)); n5.setOsmId(4,1); - their.addPrimitive(n5); - their.addPrimitive(n5); @@ -788,5 +784,6 @@ theirWay.addNode(n3); theirWay.addNode(n4); - theirWay.setUser(User.createOsmUser(1111, "their")); + User user = User.createOsmUser(1111, "their"); + theirWay.setUser(user); theirWay.setTimestamp(new Date()); their.addPrimitive(theirWay); @@ -797,5 +794,4 @@ assertEquals(0,visitor.getConflicts().size()); assertEquals("their", myWay.getUser().getName()); - assertEquals(1111, myWay.getUser().getId()); assertEquals(1111, myWay.getUser().getId()); assertEquals(theirWay.getTimestamp(), myWay.getTimestamp());