Subject: [PATCH] Fix #23091: Opening preset from Relation Editor window causes crashes --- Index: src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java b/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java --- a/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java (revision 18789) +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java (date 1690834745238) @@ -123,7 +123,6 @@ /** the tag table and its model */ private final TagEditorPanel tagEditorPanel; private final ReferringRelationsBrowser referrerBrowser; - private final ReferringRelationsBrowserModel referrerModel; /** the member table and its model */ private final MemberTable memberTable; @@ -153,18 +152,6 @@ */ private final SelectAction selectAction; /** - * Action for performing the {@link DuplicateRelationAction} - */ - private final DuplicateRelationAction duplicateAction; - /** - * Action for performing the {@link DeleteCurrentRelationAction} - */ - private final DeleteCurrentRelationAction deleteAction; - /** - * Action for performing the {@link OKAction} - */ - private final OKAction okAction; - /** * Action for performing the {@link CancelAction} */ private final CancelAction cancelAction; @@ -174,12 +161,12 @@ private final ArrayList clipboardListeners = new ArrayList<>(); private Component selectedTabPane; - private JTabbedPane tabbedPane; + private final JTabbedPane tabbedPane; /** * Creates a new relation editor for the given relation. The relation will be saved if the user * selects "ok" in the editor. - * + *

* If no relation is given, will create an editor for a new relation. * * @param layer the {@link OsmDataLayer} the new or edited relation belongs to @@ -201,9 +188,7 @@ @Override public Collection getSelection() { - Relation relation = new Relation(); - tagEditorPanel.getModel().applyToPrimitive(relation); - return Collections.singletonList(relation); + return Collections.singletonList(getRelation()); } }; @@ -213,7 +198,7 @@ memberTableModel.register(); selectionTableModel = new SelectionTableModel(getLayer()); selectionTableModel.register(); - referrerModel = new ReferringRelationsBrowserModel(relation); + ReferringRelationsBrowserModel referrerModel = new ReferringRelationsBrowserModel(relation); tagEditorPanel = new TagEditorPanel(relation, presetHandler); populateModels(relation); @@ -270,15 +255,18 @@ refreshAction = new RefreshAction(actionAccess); applyAction = new ApplyAction(actionAccess); selectAction = new SelectAction(actionAccess); - duplicateAction = new DuplicateRelationAction(actionAccess); - deleteAction = new DeleteCurrentRelationAction(actionAccess); + // Action for performing the {@link DuplicateRelationAction} + final DuplicateRelationAction duplicateAction = new DuplicateRelationAction(actionAccess); + // Action for performing the {@link DeleteCurrentRelationAction} + final DeleteCurrentRelationAction deleteAction = new DeleteCurrentRelationAction(actionAccess); this.memberTableModel.addTableModelListener(applyAction); this.tagEditorPanel.getModel().addTableModelListener(applyAction); addPropertyChangeListener(deleteAction); - okAction = new OKAction(actionAccess); + // Action for performing the {@link OKAction} + final OKAction okAction = new OKAction(actionAccess); cancelAction = new CancelAction(actionAccess); getContentPane().add(buildToolBar(refreshAction, applyAction, selectAction, duplicateAction, deleteAction), BorderLayout.NORTH); @@ -304,7 +292,8 @@ InputMapUtils.addCtrlEnterAction(getRootPane(), okAction); // CHECKSTYLE.OFF: LineLength registerCopyPasteAction(tagEditorPanel.getPasteAction(), "PASTE_TAGS", - Shortcut.registerShortcut("system:pastestyle", tr("Edit: {0}", tr("Paste Tags")), KeyEvent.VK_V, Shortcut.CTRL_SHIFT).getKeyStroke(), + Shortcut.registerShortcut("system:pastestyle", tr("Edit: {0}", tr("Paste Tags")), KeyEvent.VK_V, Shortcut.CTRL_SHIFT) + .getKeyStroke(), getRootPane(), memberTable, selectionTable); // CHECKSTYLE.ON: LineLength @@ -517,7 +506,7 @@ * * @return the panel for the relation member editor */ - protected static JPanel buildMemberEditorPanel( + static JPanel buildMemberEditorPanel( LeftButtonToolbar leftButtonToolbar, IRelationEditorActionAccess editorAccess) { final JPanel pnl = new JPanel(new GridBagLayout()); final JScrollPane scrollPane = new JScrollPane(editorAccess.getMemberTable()); Index: test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java b/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java --- a/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java (revision 18789) +++ b/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java (date 1690835067819) @@ -4,40 +4,40 @@ import static org.junit.jupiter.api.Assertions.assertEquals; 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 java.util.Collection; import java.util.Collections; import javax.swing.JOptionPane; import javax.swing.JPanel; +import mockit.Mock; +import mockit.MockUp; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.platform.commons.support.ReflectionSupport; 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.OsmPrimitive; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.gui.dialogs.relation.actions.IRelationEditorActionAccess; +import org.openstreetmap.josm.gui.dialogs.relation.actions.PasteMembersAction; import org.openstreetmap.josm.gui.layer.OsmDataLayer; import org.openstreetmap.josm.gui.tagging.TagEditorPanel; import org.openstreetmap.josm.gui.tagging.ac.AutoCompletingTextField; -import org.openstreetmap.josm.testutils.JOSMTestRules; +import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetHandler; +import org.openstreetmap.josm.testutils.annotations.BasicPreferences; +import org.openstreetmap.josm.testutils.annotations.Main; import org.openstreetmap.josm.testutils.mockers.JOptionPaneSimpleMocker; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - /** * Unit tests of {@link GenericRelationEditor} class. */ +@BasicPreferences +@Main public class GenericRelationEditorTest { - - /** - * Setup test. - */ - @RegisterExtension - @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") - public JOSMTestRules test = new JOSMTestRules().preferences().main(); - /** * Returns a new relation editor for unit tests. * @param orig relation @@ -132,4 +132,30 @@ assertNotNull(top); assertNotNull(tagEditorPanel.getModel()); } + + @Test + void testNonRegression23091() throws Exception { + new MockUp() { + @Mock + protected void updateEnabledState() { + // Do nothing + } + }; + + DataSet ds = new DataSet(); + Relation relation = new Relation(1); + ds.addPrimitive(relation); + OsmDataLayer layer = new OsmDataLayer(ds, "test", null); + + final GenericRelationEditor gr = new GenericRelationEditor(layer, relation, Collections.emptyList()); + final IRelationEditorActionAccess iAccess = (IRelationEditorActionAccess) + ReflectionSupport.tryToReadFieldValue(GenericRelationEditor.class.getDeclaredField("actionAccess"), gr) + .get(); + final TaggingPresetHandler handler = (TaggingPresetHandler) + ReflectionSupport.tryToReadFieldValue(MemberTableModel.class.getDeclaredField("presetHandler"), iAccess.getMemberTableModel()) + .get(); + final Collection selection = handler.getSelection(); + assertEquals(1, selection.size()); + assertSame(relation, selection.iterator().next(), "The selection should be the same"); + } }