Index: /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/AssociatedStreetFixer.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/AssociatedStreetFixer.java (revision 36102) +++ /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/AssociatedStreetFixer.java (revision 36103) @@ -12,4 +12,5 @@ import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.command.SequenceCommand; +import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; @@ -18,4 +19,5 @@ import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.tools.Utils; public class AssociatedStreetFixer extends RelationFixer { @@ -135,5 +137,6 @@ List commandList = new ArrayList<>(); if (fixed) { - commandList.add(new ChangeCommand(MainApplication.getLayerManager().getEditDataSet(), source, rel)); + final DataSet ds = Utils.firstNonNull(source.getDataSet(), MainApplication.getLayerManager().getEditDataSet()); + commandList.add(new ChangeCommand(ds, source, rel)); } Index: /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/BoundaryFixer.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/BoundaryFixer.java (revision 36102) +++ /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/BoundaryFixer.java (revision 36103) @@ -6,4 +6,5 @@ import org.openstreetmap.josm.command.ChangeCommand; import org.openstreetmap.josm.command.Command; +import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; @@ -11,4 +12,5 @@ import org.openstreetmap.josm.data.osm.RelationMember; import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.tools.Utils; /** @@ -66,5 +68,9 @@ r = rr; } - return fixed ? new ChangeCommand(MainApplication.getLayerManager().getEditDataSet(), rel, r) : null; + if (fixed) { + final DataSet ds = Utils.firstNonNull(rel.getDataSet(), MainApplication.getLayerManager().getEditDataSet()); + return new ChangeCommand(ds, rel, r); + } + return null; } Index: /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/MultipolygonFixer.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/MultipolygonFixer.java (revision 36102) +++ /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/MultipolygonFixer.java (revision 36103) @@ -11,6 +11,6 @@ import org.openstreetmap.josm.command.ChangeCommand; import org.openstreetmap.josm.command.Command; +import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.MultipolygonBuilder; -import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.Relation; @@ -18,4 +18,5 @@ import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.tools.Utils; /** @@ -47,5 +48,9 @@ public Command fixRelation(Relation rel) { Relation rr = fixMultipolygonRoles(rel); - return rr != null ? new ChangeCommand(MainApplication.getLayerManager().getEditDataSet(), rel, rr) : null; + if (rr != null) { + final DataSet ds = Utils.firstNonNull(rel.getDataSet(), MainApplication.getLayerManager().getEditDataSet()); + return new ChangeCommand(ds, rel, rr); + } + return null; } @@ -54,10 +59,5 @@ */ protected Relation fixMultipolygonRoles(Relation source) { - Collection ways = new ArrayList<>(); - for (OsmPrimitive p : source.getMemberPrimitives()) { - if (p instanceof Way) { - ways.add((Way) p); - } - } + Collection ways = new ArrayList<>(source.getMemberPrimitives(Way.class)); MultipolygonBuilder mpc = new MultipolygonBuilder(); String error = mpc.makeFromWays(ways); Index: /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/PublicTransportFixer.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/PublicTransportFixer.java (revision 36102) +++ /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/PublicTransportFixer.java (revision 36103) @@ -6,4 +6,5 @@ import org.openstreetmap.josm.command.ChangeCommand; import org.openstreetmap.josm.command.Command; +import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.Relation; @@ -11,4 +12,5 @@ import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.tools.Utils; import relcontext.actions.PublicTransportHelper; @@ -61,5 +63,9 @@ r = rr; } - return fixed ? new ChangeCommand(MainApplication.getLayerManager().getEditDataSet(), rel, r) : null; + if (fixed) { + final DataSet ds = Utils.firstNonNull(rel.getDataSet(), MainApplication.getLayerManager().getEditDataSet()); + return new ChangeCommand(ds, rel, r); + } + return null; } Index: /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/AssociatedStreetFixerTest.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/AssociatedStreetFixerTest.java (revision 36103) +++ /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/AssociatedStreetFixerTest.java (revision 36103) @@ -0,0 +1,49 @@ +// License: GPL. For details, see LICENSE file. +package relcontext.relationfix; + +import java.util.stream.Stream; + +import org.junit.jupiter.api.BeforeEach; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.RelationMember; +import relcontext.ChosenRelation; +import relcontext.actions.SortAndFixAction; + +/** + * Test class for {@link AssociatedStreetFixer} + */ +class AssociatedStreetFixerTest implements RelationFixerTest { + private AssociatedStreetFixer instance; + + @BeforeEach + void setup() { + this.instance = new AssociatedStreetFixer(); + this.instance.setFixAction(new SortAndFixAction(new ChosenRelation())); + } + + @Override + public RelationFixer getInstance() { + return this.instance; + } + + @Override + public Stream getBadRelations() { + final Relation relation = TestUtils.newRelation("type=associatedStreet name=FooBar", + new RelationMember("street_misspelled", TestUtils.newWay("highway=residential name=Baz", + TestUtils.newNode(""), TestUtils.newNode(""))), + new RelationMember("house_misspelled", TestUtils.newNode(""))); + final DataSet ds = new DataSet(); + ds.addPrimitiveRecursive(relation); + return Stream.of(relation); + } + + @Override + public Stream getGoodRelations() { + final Relation relation = TestUtils.newRelation("type=associatedStreet name=FooBar", + new RelationMember("street", TestUtils.newWay("highway=residential name=FooBar", TestUtils.newNode(""), TestUtils.newNode(""))), + new RelationMember("house", TestUtils.newNode(""))); + return Stream.of(relation); + } +} Index: /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/BoundaryFixerTest.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/BoundaryFixerTest.java (revision 36103) +++ /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/BoundaryFixerTest.java (revision 36103) @@ -0,0 +1,73 @@ +// License: GPL. For details, see LICENSE file. +package relcontext.relationfix; + +import java.util.Arrays; +import java.util.stream.Stream; + +import org.junit.jupiter.api.BeforeEach; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.RelationMember; +import org.openstreetmap.josm.data.osm.Way; +import relcontext.ChosenRelation; +import relcontext.actions.SortAndFixAction; + +/** + * Test class for {@link BoundaryFixer} + */ +class BoundaryFixerTest implements RelationFixerTest { + + private BoundaryFixer instance; + + @BeforeEach + void setup() { + this.instance = new BoundaryFixer(); + this.instance.setFixAction(new SortAndFixAction(new ChosenRelation())); + } + + @Override + public RelationFixer getInstance() { + return instance; + } + + @Override + public Stream getBadRelations() { + final Relation wayRelation = TestUtils.newRelation("type=boundary boundary=administrative", + new RelationMember("outer_misspelled", TestUtils.newWay("", TestUtils.newNode(""), TestUtils.newNode("")))); + final Relation nodeRelation = TestUtils.newRelation("type=boundary boundary=administrative", + new RelationMember("admin_centre_misspelled", TestUtils.newNode(""))); + final Relation relationRelation = TestUtils.newRelation("type=boundary boundary=administrative", + new RelationMember("subarea_misspelled", TestUtils.newRelation(""))); + Relation[] relations = {nodeRelation, wayRelation, relationRelation}; + for (Relation relation : relations) { + final DataSet ds = new DataSet(); + ds.addPrimitiveRecursive(relation); + } + // Ensure that the boundary is enclosed + final Node tNode = TestUtils.newNode(""); + wayRelation.getDataSet().addPrimitive(tNode); + final Way way2 = TestUtils.newWay("", wayRelation.getMember(0).getWay().firstNode(), + tNode, wayRelation.getMember(0).getWay().lastNode()); + wayRelation.getDataSet().addPrimitive(way2); + wayRelation.addMember(new RelationMember("outer_misspelled", way2)); + return Arrays.stream(relations); + } + + @Override + public Stream getGoodRelations() { + final Relation wayRelation = TestUtils.newRelation("type=boundary boundary=administrative", + new RelationMember("outer", TestUtils.newWay("", TestUtils.newNode(""), TestUtils.newNode("")))); + final Relation nodeRelation = TestUtils.newRelation("type=boundary boundary=administrative", + new RelationMember("admin_centre", TestUtils.newNode(""))); + final Relation relationRelation = TestUtils.newRelation("type=boundary boundary=administrative", + new RelationMember("subarea", TestUtils.newRelation(""))); + Relation[] relations = {nodeRelation, wayRelation, relationRelation}; + for (Relation relation : relations) { + final DataSet ds = new DataSet(); + ds.addPrimitiveRecursive(relation); + } + return Arrays.stream(relations); + } +} Index: /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/MultipolygonFixerTest.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/MultipolygonFixerTest.java (revision 36103) +++ /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/MultipolygonFixerTest.java (revision 36103) @@ -0,0 +1,57 @@ +// License: GPL. For details, see LICENSE file. +package relcontext.relationfix; + +import java.util.stream.Stream; + +import org.junit.jupiter.api.BeforeEach; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.RelationMember; +import org.openstreetmap.josm.data.osm.Way; +import relcontext.ChosenRelation; +import relcontext.actions.SortAndFixAction; + +/** + * Test class for {@link MultipolygonFixer} + */ +class MultipolygonFixerTest implements RelationFixerTest { + private MultipolygonFixer instance; + + @BeforeEach + void setup() { + this.instance = new MultipolygonFixer(); + this.instance.setFixAction(new SortAndFixAction(new ChosenRelation())); + } + + @Override + public RelationFixer getInstance() { + return this.instance; + } + + @Override + public Stream getBadRelations() { + final Relation wayRelation = TestUtils.newRelation("type=multipolygon", + new RelationMember("outer_misspelled", TestUtils.newWay("", TestUtils.newNode(""), TestUtils.newNode("")))); + final DataSet ds = new DataSet(); + ds.addPrimitiveRecursive(wayRelation); + // Ensure that the boundary is enclosed + final Node tNode = TestUtils.newNode(""); + wayRelation.getDataSet().addPrimitive(tNode); + final Way way2 = TestUtils.newWay("", wayRelation.getMember(0).getWay().firstNode(), + tNode, wayRelation.getMember(0).getWay().lastNode()); + wayRelation.getDataSet().addPrimitive(way2); + wayRelation.addMember(new RelationMember("outer_misspelled", way2)); + return Stream.of(wayRelation); + } + + @Override + public Stream getGoodRelations() { + final Relation wayRelation = TestUtils.newRelation("type=multipolygon", + new RelationMember("outer", TestUtils.newWay("", TestUtils.newNode(""), TestUtils.newNode("")))); + final DataSet ds = new DataSet(); + ds.addPrimitiveRecursive(wayRelation); + return Stream.of(wayRelation); + } +} Index: /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/PublicTransportFixerTest.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/PublicTransportFixerTest.java (revision 36103) +++ /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/PublicTransportFixerTest.java (revision 36103) @@ -0,0 +1,59 @@ +// License: GPL. For details, see LICENSE file. +package relcontext.relationfix; + +import java.util.Arrays; +import java.util.stream.Stream; + +import org.junit.jupiter.api.BeforeEach; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.RelationMember; +import relcontext.ChosenRelation; +import relcontext.actions.SortAndFixAction; + +/** + * Test class for {@link PublicTransportFixer} + */ +class PublicTransportFixerTest implements RelationFixerTest { + private PublicTransportFixer instance; + + @BeforeEach + void setup() { + this.instance = new PublicTransportFixer(); + this.instance.setFixAction(new SortAndFixAction(new ChosenRelation())); + } + + @Override + public RelationFixer getInstance() { + return this.instance; + } + + @Override + public Stream getBadRelations() { + final Relation badWay = TestUtils.newRelation("type=public_transport", + new RelationMember("", TestUtils.newWay("public_transport=platform"))); + final Relation badNode = TestUtils.newRelation("type=public_transport", + new RelationMember("", TestUtils.newNode("public_transport=stop_position"))); + final Relation[] relations = {badWay, badNode}; + for (Relation relation : relations) { + final DataSet ds = new DataSet(); + ds.addPrimitiveRecursive(relation); + } + return Arrays.stream(relations); + } + + @Override + public Stream getGoodRelations() { + final Relation way = TestUtils.newRelation("type=public_transport", + new RelationMember("platform", TestUtils.newWay("public_transport=platform"))); + final Relation node = TestUtils.newRelation("type=public_transport", + new RelationMember("stop", TestUtils.newNode("public_transport=stop_position"))); + final Relation[] relations = {way, node}; + for (Relation relation : relations) { + final DataSet ds = new DataSet(); + ds.addPrimitiveRecursive(relation); + } + return Arrays.stream(relations); + } +} Index: /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/RelationFixerTest.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/RelationFixerTest.java (revision 36103) +++ /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/relationfix/RelationFixerTest.java (revision 36103) @@ -0,0 +1,94 @@ +// License: GPL. For details, see LICENSE file. +package relcontext.relationfix; + +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.openstreetmap.josm.command.Command; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.testutils.annotations.Projection; + +/** + * A class for checking common issues with the {@link RelationFixer} subclasses + */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +interface RelationFixerTest { + /** + * Get the instance to use for checking + * @return The fixer instance + */ + RelationFixer getInstance(); + + /** + * A relation that should be fixed by the fixer + * @return The bad relation + */ + Stream getBadRelations(); + + /** + * A relation that should not be fixed by the fixer + * @return The good relation + */ + Stream getGoodRelations(); + + default Stream getRelations() { + return Stream.concat(getBadRelations(), getGoodRelations()); + } + + @ParameterizedTest + @MethodSource("getRelations") + default void testIsFixerApplicable(Relation relation) { + final RelationFixer fixer = getInstance(); + assertTrue(fixer.isFixerApplicable(relation)); + } + + @Test + default void testIsFixerApplicableEmptyRelation() { + assertFalse(getInstance().isFixerApplicable(new Relation())); + } + + @ParameterizedTest + @MethodSource("getGoodRelations") + default void testGoodRelationMatches(Relation goodRelation) { + final RelationFixer fixer = getInstance(); + assertTrue(fixer.isFixerApplicable(goodRelation)); + assertFalse(fixer.isFixerApplicable(new Relation())); + } + + @ParameterizedTest + @MethodSource("getGoodRelations") + default void testIsRelationGoodGoodRelation(Relation goodRelation) { + assertTrue(getInstance().isRelationGood(goodRelation)); + } + + @ParameterizedTest + @MethodSource("getBadRelations") + default void testIsRelationGoodBadRelation(Relation badRelation) { + assertFalse(getInstance().isRelationGood(badRelation)); + } + + @MethodSource("getBadRelations") + @ParameterizedTest + @Projection + default void testFixBadRelation(Relation badRelation) { + final DataSet ds = badRelation.getDataSet(); + final RelationFixer fixer = getInstance(); + final Command command = fixer.fixRelation(badRelation); + assertNotNull(command); + assertDoesNotThrow(command::executeCommand); + final Relation relation = ds.getRelations().stream().filter(fixer::isFixerApplicable).findFirst().orElseThrow(AssertionError::new); + assertAll(relation.getMemberPrimitives().stream() + .map(member -> () -> assertSame(ds, member.getDataSet(), member + " does not have the same dataset as " + relation))); + } +}