Index: trunk/src/org/openstreetmap/josm/gui/conflict/tags/CombinePrimitiveResolverDialog.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/conflict/tags/CombinePrimitiveResolverDialog.java (revision 8870) +++ trunk/src/org/openstreetmap/josm/gui/conflict/tags/CombinePrimitiveResolverDialog.java (revision 8871) @@ -18,9 +18,6 @@ import java.beans.PropertyChangeListener; import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; import java.util.LinkedList; import java.util.List; -import java.util.Map; import java.util.Set; @@ -51,6 +48,4 @@ import org.openstreetmap.josm.tools.CheckParameterUtil; import org.openstreetmap.josm.tools.ImageProvider; -import org.openstreetmap.josm.tools.MultiMap; -import org.openstreetmap.josm.tools.Predicates; import org.openstreetmap.josm.tools.Utils; import org.openstreetmap.josm.tools.Utils.Function; @@ -298,46 +293,10 @@ } - protected void prepareDefaultTagDecisions() { + /** + * Prepares the default decisions for populated tag and relation membership conflicts. + */ + public void prepareDefaultDecisions() { getTagConflictResolverModel().prepareDefaultTagDecisions(); - } - - protected void prepareDefaultRelationDecisions() { - final RelationMemberConflictResolverModel model = getRelationMemberConflictResolverModel(); - final Map numberOfKeepResolutions = new HashMap<>(); - final MultiMap resolvedRelationsPerPrimitive = new MultiMap<>(); - - for (int i = 0; i < model.getNumDecisions(); i++) { - final RelationMemberConflictDecision decision = model.getDecision(i); - final Relation r = decision.getRelation(); - final OsmPrimitive p = decision.getOriginalPrimitive(); - if (!numberOfKeepResolutions.containsKey(r)) { - decision.decide(RelationMemberConflictDecisionType.KEEP); - numberOfKeepResolutions.put(r, 1); - resolvedRelationsPerPrimitive.put(p, r); - continue; - } - - final Integer keepResolutions = numberOfKeepResolutions.get(r); - final Collection resolvedRelations = Utils.firstNonNull( - resolvedRelationsPerPrimitive.get(p), Collections.emptyList()); - if (keepResolutions <= Utils.filter(resolvedRelations, Predicates.equalTo(r)).size()) { - // old relation contains one primitive more often than the current resolution => keep the current member - decision.decide(RelationMemberConflictDecisionType.KEEP); - numberOfKeepResolutions.put(r, keepResolutions + 1); - resolvedRelationsPerPrimitive.put(p, r); - } else { - decision.decide(RelationMemberConflictDecisionType.REMOVE); - resolvedRelationsPerPrimitive.put(p, r); - } - } - model.refresh(); - } - - /** - * Prepares the default decisions for populated tag and relation membership conflicts. - */ - public void prepareDefaultDecisions() { - prepareDefaultTagDecisions(); - prepareDefaultRelationDecisions(); + getRelationMemberConflictResolverModel().prepareDefaultRelationDecisions(); } Index: trunk/src/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictDecision.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictDecision.java (revision 8870) +++ trunk/src/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictDecision.java (revision 8871) @@ -115,3 +115,8 @@ return true; } + + @Override + public String toString() { + return originalPrimitive.getPrimitiveId() + " at index " + pos + " with role " + role + " in " + relation.getUniqueId() + " => " + decision; + } } Index: trunk/src/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictResolverModel.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictResolverModel.java (revision 8870) +++ trunk/src/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictResolverModel.java (revision 8871) @@ -6,8 +6,13 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.TreeSet; import javax.swing.table.DefaultTableModel; @@ -20,4 +25,6 @@ import org.openstreetmap.josm.data.osm.RelationToChildReference; import org.openstreetmap.josm.gui.util.GuiHelper; +import org.openstreetmap.josm.tools.Predicate; +import org.openstreetmap.josm.tools.Utils; /** @@ -34,4 +41,6 @@ /** the collection of relations for which we manage conflicts */ protected transient Collection relations; + /** the collection of primitives for which we manage conflicts */ + protected transient Collection primitives; /** the number of conflicts */ private int numConflicts; @@ -151,5 +160,5 @@ public void populate(Collection relations, Collection memberPrimitives) { decisions.clear(); - relations = relations == null ? new LinkedList() : relations; + relations = relations == null ? Collections.emptyList() : relations; memberPrimitives = memberPrimitives == null ? new LinkedList() : memberPrimitives; for (Relation r : relations) { @@ -159,4 +168,5 @@ } this.relations = relations; + this.primitives = memberPrimitives; refresh(); } @@ -172,9 +182,89 @@ decisions.clear(); this.relations = new HashSet<>(references.size()); + final Collection primitives = new HashSet<>(); for (RelationToChildReference reference: references) { decisions.add(new RelationMemberConflictDecision(reference.getParent(), reference.getPosition())); relations.add(reference.getParent()); - } + primitives.add(reference.getChild()); + } + this.primitives = primitives; refresh(); + } + + /** + * Prepare the default decisions for the current model. + * + * Keep/delete decisions are made if every member has the same role and the members are in consecutive order within the relation. + * For multiple occurrences those conditions are tested stepwise for each occurrence. + */ + public void prepareDefaultRelationDecisions() { + + for (final Relation relation : relations) { + final Map> decisionsByPrimitive = new LinkedHashMap<>(primitives.size(), 1); + for (final RelationMemberConflictDecision decision : decisions) { + if (decision.getRelation() == relation) { + final OsmPrimitive primitive = decision.getOriginalPrimitive(); + if (!decisionsByPrimitive.containsKey(primitive)) { + decisionsByPrimitive.put(primitive, new ArrayList()); + } + decisionsByPrimitive.get(primitive).add(decision); + } + } + + //noinspection StatementWithEmptyBody + if (!decisionsByPrimitive.keySet().containsAll(primitives)) { + // some primitives are not part of the relation, leave undecided + } else { + final Collection> iterators = new ArrayList<>(primitives.size()); + for (final Collection i : decisionsByPrimitive.values()) { + iterators.add(i.iterator()); + } + while (Utils.forAll(iterators, new Predicate>() { + @Override + public boolean evaluate(Iterator it) { + return it.hasNext(); + } + })) { + final List decisions = new ArrayList<>(); + final Collection roles = new HashSet<>(); + final Collection indices = new TreeSet<>(); + for (Iterator it : iterators) { + final RelationMemberConflictDecision decision = it.next(); + decisions.add(decision); + roles.add(decision.getRole()); + indices.add(decision.getPos()); + } + if (roles.size() != 1) { + // roles to not patch, leave undecided + continue; + } else if (!isCollectionOfConsecutiveNumbers(indices)) { + // not consecutive members in relation, leave undecided + continue; + } + decisions.get(0).decide(RelationMemberConflictDecisionType.KEEP); + for (RelationMemberConflictDecision decision : decisions.subList(1, decisions.size())) { + decision.decide(RelationMemberConflictDecisionType.REMOVE); + } + } + } + } + + refresh(); + } + + static boolean isCollectionOfConsecutiveNumbers(Collection numbers) { + if (numbers.isEmpty()) { + return true; + } + final Iterator it = numbers.iterator(); + Integer previousValue = it.next(); + while (it.hasNext()) { + final Integer i = it.next(); + if (previousValue + 1 != i) { + return false; + } + previousValue = i; + } + return true; } @@ -195,5 +285,5 @@ */ public int getNumDecisions() { - return decisions == null ? 0 : decisions.size(); + return decisions == null /* accessed via super constructor */ ? 0 : decisions.size(); } Index: trunk/test/unit/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictResolverModelTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictResolverModelTest.java (revision 8871) +++ trunk/test/unit/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictResolverModelTest.java (revision 8871) @@ -0,0 +1,120 @@ +package org.openstreetmap.josm.gui.conflict.tags; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.junit.BeforeClass; +import org.junit.Test; +import org.openstreetmap.josm.JOSMFixture; +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.OsmPrimitive; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.RelationMember; +import org.openstreetmap.josm.data.osm.Way; + +public class RelationMemberConflictResolverModelTest { + + @BeforeClass + public static void init() { + JOSMFixture.createUnitTestFixture().init(); + } + + List buildTestDataSet() { + final DataSet ds = new DataSet(); + final Node n1 = new Node(new LatLon(1, 1)); + final Node n2 = new Node(new LatLon(2, 2)); + final Node n3 = new Node(new LatLon(3, 3)); + final Way w1 = new Way(); + final Way w2 = new Way(); + final Way w3 = new Way(); + ds.addPrimitive(n1); + ds.addPrimitive(n2); + ds.addPrimitive(n3); + ds.addPrimitive(w1); + ds.addPrimitive(w2); + ds.addPrimitive(w3); + w1.addNode(n1); + w1.addNode(n2); + w2.addNode(n2); + w2.addNode(n3); + w3.addNode(n3); + w3.addNode(n1); + return Arrays.asList(w1, w2, w3); + } + + @Test + public void testSameRoles() throws Exception { + final List ways = buildTestDataSet(); + final Relation r = new Relation(); + r.addMember(new RelationMember("foo", ways.get(0))); + r.addMember(new RelationMember("foo", ways.get(1))); + + final RelationMemberConflictResolverModel model = new RelationMemberConflictResolverModel(); + model.populate(Collections.singleton(r), ways.subList(0, 2)); + model.prepareDefaultRelationDecisions(); + assertTrue(model.isResolvedCompletely()); + assertThat(model.getDecision(0).getDecision(), is(RelationMemberConflictDecisionType.KEEP)); + assertThat(model.getDecision(0).getOriginalPrimitive(), is((OsmPrimitive) ways.get(0))); + assertThat(model.getDecision(0).getRole(), is("foo")); + assertThat(model.getDecision(1).getDecision(), is(RelationMemberConflictDecisionType.REMOVE)); + assertThat(model.getDecision(1).getOriginalPrimitive(), is((OsmPrimitive) ways.get(1))); + } + + @Test + public void testDifferentRoles() throws Exception { + final List ways = buildTestDataSet(); + final Relation r = new Relation(); + r.addMember(new RelationMember("foo", ways.get(0))); + r.addMember(new RelationMember("bar", ways.get(1))); + + final RelationMemberConflictResolverModel model = new RelationMemberConflictResolverModel(); + model.populate(Collections.singleton(r), ways.subList(0, 2)); + model.prepareDefaultRelationDecisions(); + assertFalse(model.isResolvedCompletely()); + } + + @Test + public void testDifferentPresence() throws Exception { + final List ways = buildTestDataSet(); + final Relation r = new Relation(); + r.addMember(new RelationMember("foo", ways.get(0))); + + final RelationMemberConflictResolverModel model = new RelationMemberConflictResolverModel(); + model.populate(Collections.singleton(r), ways.subList(0, 2)); + model.prepareDefaultRelationDecisions(); + assertFalse(model.isResolvedCompletely()); + } + + @Test + public void testEveryMemberIsPresentTwice() throws Exception { + final List ways = buildTestDataSet(); + final Relation r = new Relation(); + r.addMember(new RelationMember("foo", ways.get(0))); + r.addMember(new RelationMember("foo", ways.get(1))); + r.addMember(new RelationMember("xoo", ways.get(2))); + r.addMember(new RelationMember("bar", ways.get(0))); + r.addMember(new RelationMember("bar", ways.get(1))); + r.addMember(new RelationMember("xoo", ways.get(2))); + + final RelationMemberConflictResolverModel model = new RelationMemberConflictResolverModel(); + model.populate(Collections.singleton(r), ways.subList(0, 2)); + model.prepareDefaultRelationDecisions(); + assertTrue(model.isResolvedCompletely()); + assertThat(model.getDecision(0).getDecision(), is(RelationMemberConflictDecisionType.KEEP)); + assertThat(model.getDecision(0).getOriginalPrimitive(), is((OsmPrimitive) ways.get(0))); + assertThat(model.getDecision(0).getRole(), is("foo")); + assertThat(model.getDecision(1).getDecision(), is(RelationMemberConflictDecisionType.KEEP)); + assertThat(model.getDecision(1).getOriginalPrimitive(), is((OsmPrimitive) ways.get(0))); + assertThat(model.getDecision(1).getRole(), is("bar")); + assertThat(model.getDecision(2).getDecision(), is(RelationMemberConflictDecisionType.REMOVE)); + assertThat(model.getDecision(3).getDecision(), is(RelationMemberConflictDecisionType.REMOVE)); + } +}