Index: /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java (revision 18803) +++ /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java (revision 18804) @@ -189,4 +189,11 @@ @Override public Collection getSelection() { + // Creating a new relation will open the window. The relation, in that case, will be null. + if (getRelation() == null) { + Relation relation = new Relation(); + tagEditorPanel.getModel().applyToPrimitive(relation); + memberTableModel.applyToRelation(relation); + return Collections.singletonList(relation); + } return Collections.singletonList(getRelation()); } Index: /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java (revision 18803) +++ /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java (revision 18804) @@ -2,17 +2,29 @@ package org.openstreetmap.josm.gui.dialogs.relation; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; - +import static org.openstreetmap.josm.tools.I18n.tr; + +import java.awt.Container; +import java.awt.event.WindowEvent; import java.util.Collection; import java.util.Collections; - +import java.util.concurrent.atomic.AtomicReference; + +import javax.swing.Action; +import javax.swing.JButton; import javax.swing.JOptionPane; import javax.swing.JPanel; +import mockit.Invocation; import mockit.Mock; import mockit.MockUp; + +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.platform.commons.support.ReflectionSupport; @@ -22,5 +34,10 @@ 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; +import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.SideButton; +import org.openstreetmap.josm.gui.dialogs.RelationListDialog; +import org.openstreetmap.josm.gui.dialogs.relation.actions.AddSelectedAtStartAction; import org.openstreetmap.josm.gui.dialogs.relation.actions.IRelationEditorActionAccess; import org.openstreetmap.josm.gui.dialogs.relation.actions.PasteMembersAction; @@ -31,4 +48,5 @@ import org.openstreetmap.josm.testutils.annotations.BasicPreferences; import org.openstreetmap.josm.testutils.annotations.Main; +import org.openstreetmap.josm.testutils.annotations.Projection; import org.openstreetmap.josm.testutils.mockers.JOptionPaneSimpleMocker; import org.openstreetmap.josm.testutils.mockers.WindowMocker; @@ -39,4 +57,5 @@ @BasicPreferences @Main +@Projection public class GenericRelationEditorTest { /** @@ -80,4 +99,10 @@ } }; + } + + @BeforeEach + void setup() { + new PasteMembersActionMock(); + new WindowMocker(); } @@ -137,12 +162,4 @@ @Test void testNonRegression23091() throws Exception { - new MockUp() { - @Mock - protected void updateEnabledState() { - // Do nothing - } - }; - new WindowMocker(); - DataSet ds = new DataSet(); Relation relation = new Relation(1); @@ -161,3 +178,74 @@ assertSame(relation, selection.iterator().next(), "The selection should be the same"); } + + /** + * Ensure that users can create new relations and modify them. + */ + @Test + void testNonRegression23116() { + // Setup the mocks + final AtomicReference editorReference = new AtomicReference<>(); + new MockUp() { + @Mock public RelationEditor getEditor(Invocation invocation, OsmDataLayer layer, Relation r, + Collection selectedMembers) { + editorReference.set(invocation.proceed(layer, r, selectedMembers)); + return editorReference.get(); + } + }; + // We want to go through the `setVisible` code, just in case. So we have to mock the window location + new MockUp() { + @Mock public void setVisible(boolean visible) { + // Do nothing. Ideally, we would just mock the awt methods called, but that would take a lot of mocking. + } + }; + // Set up the data + final DataSet dataSet = new DataSet(); + MainApplication.getLayerManager().addLayer(new OsmDataLayer(dataSet, "GenericRelationEditorTest.testNonRegression23116", null)); + dataSet.addPrimitive(TestUtils.newNode("")); + dataSet.setSelected(dataSet.allPrimitives()); + final RelationListDialog relationListDialog = new RelationListDialog(); + try { + final Action newAction = ((SideButton) getComponent(relationListDialog, 2, 0, 0)).getAction(); + assertEquals("class org.openstreetmap.josm.gui.dialogs.RelationListDialog$NewAction", + newAction.getClass().toString()); + // Now get the buttons we want to push + newAction.actionPerformed(null); + final GenericRelationEditor editor = assertInstanceOf(GenericRelationEditor.class, editorReference.get()); + final JButton okAction = getComponent(editor, 0, 1, 0, 2, 0); + assertEquals(tr("Delete"), okAction.getText(), "OK is Delete until the relation actually has data"); + assertNotNull(editor); + final TagEditorPanel tagEditorPanel = getComponent(editor, 0, 1, 0, 1, 0, 0, 1, 1); + // We need at least one tag for the action to not be "Delete". + tagEditorPanel.getModel().add("type", "someUnknownTypeHere"); + final Action addAtStartAction = assertInstanceOf(AddSelectedAtStartAction.class, + ((JButton) getComponent(editor, 0, 1, 0, 1, 0, 0, 2, 0, 2, 1, 2, 0, 0)).getAction()); + // Perform the actual test. + assertDoesNotThrow(() -> addAtStartAction.actionPerformed(null)); + assertDoesNotThrow(() -> okAction.getAction().actionPerformed(null)); + assertFalse(dataSet.getRelations().isEmpty()); + assertSame(dataSet.getNodes().iterator().next(), + dataSet.getRelations().iterator().next().getMember(0).getNode()); + } finally { + // This avoids an issue with the cleanup code and the mocks for this test + if (editorReference.get() != null) { + RelationDialogManager.getRelationDialogManager().windowClosed(new WindowEvent(editorReference.get(), 0)); + } + } + } + + @SuppressWarnings("unchecked") + private static T getComponent(Container parent, int... tree) { + Container current = parent; + for (int i : tree) { + current = (Container) current.getComponent(i); + } + return (T) current; + } + + private static class PasteMembersActionMock extends MockUp { + @Mock + protected void updateEnabledState() { + // Do nothing + } + } }