Index: /applications/editors/josm/plugins/reltoolbox/src/relcontext/RelContextDialog.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/src/relcontext/RelContextDialog.java (revision 36135) +++ /applications/editors/josm/plugins/reltoolbox/src/relcontext/RelContextDialog.java (revision 36136) @@ -178,27 +178,14 @@ rcPanel.add(chosenRelationPanel, BorderLayout.NORTH); - roleBox.addPropertyChangeListener("enabled", new PropertyChangeListener() { - @Override - public void propertyChange(PropertyChangeEvent evt) { - boolean showRoleBox = roleBox.isEnabled(); - roleBox.setVisible(showRoleBox); - chosenRelationComponent.setVisible(!showRoleBox); - } + roleBox.addPropertyChangeListener("enabled", evt -> { + boolean showRoleBox = roleBox.isEnabled(); + roleBox.setVisible(showRoleBox); + chosenRelationComponent.setVisible(!showRoleBox); }); - sortAndFixAction.addPropertyChangeListener(new PropertyChangeListener() { - @Override - public void propertyChange(PropertyChangeEvent evt) { - sortAndFixButton.setVisible(sortAndFixAction.isEnabled()); - } - }); + sortAndFixAction.addPropertyChangeListener(evt -> sortAndFixButton.setVisible(sortAndFixAction.isEnabled())); sortAndFixButton.setVisible(false); - downloadChosenRelationAction.addPropertyChangeListener(new PropertyChangeListener() { - @Override - public void propertyChange(PropertyChangeEvent evt) { - downloadButton.setVisible(downloadChosenRelationAction.isEnabled()); - } - }); + downloadChosenRelationAction.addPropertyChangeListener(evt -> downloadButton.setVisible(downloadChosenRelationAction.isEnabled())); downloadButton.setVisible(false); if (Config.getPref().getBoolean(PREF_PREFIX + ".hidetopline", false)) { @@ -317,17 +304,14 @@ columns.getColumn(1).setPreferredWidth(40); columns.getColumn(0).setPreferredWidth(220); - relationsTable.getSelectionModel().addListSelectionListener(new ListSelectionListener() { - @Override - public void valueChanged(ListSelectionEvent e) { - int selectedRow = relationsTable.getSelectedRow(); - if (selectedRow >= 0) { - chosenRelation.set((Relation) relationsData.getValueAt(selectedRow, 0)); - relationsTable.clearSelection(); - } + relationsTable.getSelectionModel().addListSelectionListener(e -> { + int selectedRow = relationsTable.getSelectedRow(); + if (selectedRow >= 0) { + chosenRelation.set((Relation) relationsData.getValueAt(selectedRow, 0)); + relationsTable.clearSelection(); } }); } - private JComponent sizeButton(JComponent b, int width, int height) { + private static JComponent sizeButton(JComponent b, int width, int height) { Dimension pref = b.getPreferredSize(); b.setPreferredSize(new Dimension(width <= 0 ? pref.width : width, height <= 0 ? pref.height : height)); @@ -495,10 +479,7 @@ dlg.setModalityType(ModalityType.DOCUMENT_MODAL); - role.getEditor().addActionListener(new ActionListener() { - @Override - public void actionPerformed(ActionEvent e) { - dlg.setVisible(false); - optionPane.setValue(JOptionPane.OK_OPTION); - } + role.getEditor().addActionListener(e -> { + dlg.setVisible(false); + optionPane.setValue(JOptionPane.OK_OPTION); }); @@ -752,5 +733,5 @@ @Override public void setSelectedItem(Object anItem) { - int newIndex = anItem instanceof String ? roles.indexOf((String) anItem) : -1; + int newIndex = anItem instanceof String ? roles.indexOf(anItem) : -1; if (newIndex != selectedIndex) { selectedIndex = newIndex; Index: /applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/ReconstructPolygonAction.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/ReconstructPolygonAction.java (revision 36135) +++ /applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/ReconstructPolygonAction.java (revision 36136) @@ -64,5 +64,5 @@ boolean wont = false; for (RelationMember m : r.getMembers()) { - if (m.isWay()) { + if (m.isWay() && m.getWay().getReferrers().size() == 1) { ways.add(m.getWay()); } else { @@ -72,5 +72,6 @@ if (wont) { JOptionPane.showMessageDialog(MainApplication.getMainFrame(), - tr("Multipolygon must consist only of ways"), tr("Reconstruct polygon"), JOptionPane.ERROR_MESSAGE); + tr("Multipolygon must consist only of ways with one referring relation"), + tr("Reconstruct polygon"), JOptionPane.ERROR_MESSAGE); return; } @@ -174,8 +175,5 @@ candidateWay = tmp; } - final Command deleteCommand = DeleteCommand.delete(Collections.singleton(w)); - if (deleteCommand != null) { - commands.add(deleteCommand); - } + commands.add(new DeleteCommand(w)); } } Index: /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/actions/ReconstructPolygonActionTest.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/actions/ReconstructPolygonActionTest.java (revision 36136) +++ /applications/editors/josm/plugins/reltoolbox/test/unit/relcontext/actions/ReconstructPolygonActionTest.java (revision 36136) @@ -0,0 +1,143 @@ +// License: GPL. For details, see LICENSE file. +package relcontext.actions; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.openstreetmap.josm.tools.I18n.tr; + +import java.awt.Component; +import java.util.Arrays; +import java.util.Collection; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Predicate; +import java.util.stream.Stream; + +import javax.swing.JOptionPane; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.actions.DeleteAction; +import org.openstreetmap.josm.command.DeleteCommand; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.DatasetConsistencyTest; +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.RelationToChildReference; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.layer.OsmDataLayer; +import org.openstreetmap.josm.gui.util.GuiHelper; +import org.openstreetmap.josm.testutils.annotations.Main; +import org.openstreetmap.josm.testutils.annotations.Projection; + +import junit.framework.AssertionFailedError; +import mockit.Mock; +import mockit.MockUp; +import relcontext.ChosenRelation; + +/** + * Test class for {@link ReconstructPolygonAction} + */ +@Projection +@Main +class ReconstructPolygonActionTest { + private DataSet ds; + private ChosenRelation chosenRelation; + private Way way1; + private Way way2; + private Way way3; + private Relation relation; + private ReconstructPolygonAction action; + + @BeforeEach + void setup() { + DeleteCommand.setDeletionCallback(new DeleteCommand.DeletionCallback() { + @Override + public boolean checkAndConfirmOutlyingDelete(Collection primitives, Collection ignore) { + return true; + } + + @Override + public boolean confirmRelationDeletion(Collection relations) { + return true; + } + + @Override + public boolean confirmDeletionFromRelation(Collection references) { + return true; + } + }); + ds = new DataSet(); + MainApplication.getLayerManager().addLayer(new OsmDataLayer(ds, "ReconstructPolygonActionTest#testNonRegression23170", null)); + chosenRelation = new ChosenRelation(); + way1 = TestUtils.newWay("", TestUtils.newNode("name=1"), TestUtils.newNode("name=2")); + way2 = TestUtils.newWay("", way1.lastNode(), TestUtils.newNode("name=3")); + way3 = TestUtils.newWay("", way2.lastNode(), way1.firstNode()); + relation = TestUtils.newRelation("type=multipolygon landuse=orchard", + new RelationMember("outer", way1), + new RelationMember("outer", way2), + new RelationMember("outer", way3)); + ds.addPrimitiveRecursive(relation); + chosenRelation.set(relation); + action = new ReconstructPolygonAction(chosenRelation); + } + + @AfterEach + void tearDown() { + DeleteCommand.setDeletionCallback(DeleteAction.defaultDeletionCallback); + } + + /** + * Check that the reconstruct code works on a minimal level + */ + @Test + void testPolygonReconstructSimple() { + assertDoesNotThrow(() -> action.actionPerformed(null)); + assertTrue(relation.isDeleted()); + assertEquals(2, Stream.of(way1, way2, way3).filter(Way::isDeleted).count()); + final Way keptWay = Stream.of(way1, way2, way3).filter(w -> !w.isDeleted()).findFirst().orElseThrow(AssertionFailedError::new); + assertTrue(keptWay.isClosed()); + assertEquals(4, keptWay.getNodesCount()); + assertEquals(1, keptWay.getNodes().stream().distinct().filter(n -> "1".equals(n.get("name"))).count()); + assertEquals(1, keptWay.getNodes().stream().distinct().filter(n -> "2".equals(n.get("name"))).count()); + assertEquals(1, keptWay.getNodes().stream().distinct().filter(n -> "3".equals(n.get("name"))).count()); + assertEmpty(DatasetConsistencyTest.runTests(ds)); + } + + /** + * Ensure that we bail if a way in the relation to be simplified will be deleted from another relation. + */ + @Test + void testPolygonReconstructComplex() { + final AtomicReference shownMessage = new AtomicReference<>(); + new MockUp() { + @Mock + void showMessageDialog(Component parentComponent, + Object message, String title, int messageType) { + shownMessage.set((String) message); + } + }; + final Relation otherRelation = TestUtils.newRelation("type=multipolygon landuse=retail", + new RelationMember("outer", way1), + new RelationMember("outer", way2), + new RelationMember("outer", way3), + new RelationMember("label", TestUtils.newNode("name=4"))); + ds.addPrimitiveRecursive(otherRelation); + assertDoesNotThrow(() -> GuiHelper.runInEDTAndWait(() -> action.actionPerformed(null))); + assertEmpty(DatasetConsistencyTest.runTests(ds)); + assertEquals(tr("Multipolygon must consist only of ways with one referring relation"), shownMessage.get()); + } + + /** + * Check that a string is empty + * @param string The string to check. Will be printed to log if not empty. + */ + private static void assertEmpty(String string) { + assertTrue(string.isEmpty(), string); + } +}