Ticket #23527: 23527-2.patch

src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java

@@ -55 +55 @@
 import org.openstreetmap.josm.data.UndoRedoHandler;
 import org.openstreetmap.josm.data.UndoRedoHandler.CommandQueueListener;
 import org.openstreetmap.josm.data.osm.DefaultNameFormatter;
-import org.openstreetmap.josm.data.osm.IRelation;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
@@ -405 +404 @@
     /**
      * builds the panel with the OK and the Cancel button
      * @param okAction OK action
+     * @param deleteAction  Delete action
      * @param cancelAction Cancel action
      *
      * @return the panel with the OK and the Cancel button
@@ -421 +421 @@
         pnl.add(new JButton(new ContextSensitiveHelpAction(ht("/Dialog/RelationEditor"))));
         // Keep users from saving invalid relations -- a relation MUST have at least a tag with the key "type"
         // AND must contain at least one other OSM object.
-        final TableModelListener listener = l -> updateOkPanel(this.actionAccess.getChangedRelation(), okButton, deleteButton);
+        final TableModelListener listener = l -> updateOkPanel(okButton, deleteButton);
         listener.tableChanged(null);
         this.memberTableModel.addTableModelListener(listener);
         this.tagEditorPanel.getModel().addTableModelListener(listener);
@@ -429 +429 @@
     }
 
     /**
-     * Update the OK panel area
-     * @param newRelation What the new relation would "look" like if it were to be saved now
+     * Update the OK panel area with a temporary relation that looks if it were to be saved now.
      * @param okButton The OK button
      * @param deleteButton The delete button
      */
-    private void updateOkPanel(IRelation<?> newRelation, JButton okButton, JButton deleteButton) {
-        okButton.setVisible(newRelation.isUseful() || this.getRelationSnapshot() == null);
-        deleteButton.setVisible(!newRelation.isUseful() && this.getRelationSnapshot() != null);
-        if (this.getRelationSnapshot() == null && !newRelation.isUseful()) {
+    private void updateOkPanel(JButton okButton, JButton deleteButton) {
+        boolean useful = this.actionAccess.wouldRelationBeUseful();
+        okButton.setVisible(useful || this.getRelationSnapshot() == null);
+        deleteButton.setVisible(!useful && this.getRelationSnapshot() != null);
+        if (this.getRelationSnapshot() == null && !useful) {
             okButton.setText(tr("Delete"));
         } else {
             okButton.setText(tr("OK"));

src/org/openstreetmap/josm/gui/dialogs/relation/actions/ApplyAction.java

@@ -35 +35 @@
 
     @Override
     public void updateEnabledState() {
-        setEnabled(this.editorAccess.getChangedRelation().isUseful() && isEditorDirty());
+        setEnabled(this.editorAccess.wouldRelationBeUseful() && isEditorDirty());
     }
 }

src/org/openstreetmap/josm/gui/dialogs/relation/actions/CancelAction.java

@@ -8 +8 @@
 import javax.swing.JOptionPane;
 import javax.swing.RootPaneContainer;
 
-import org.openstreetmap.josm.data.osm.IRelation;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.gui.HelpAwareOptionPane;
 import org.openstreetmap.josm.gui.HelpAwareOptionPane.ButtonSpec;
@@ -48 +47 @@
         if ((!getMemberTableModel().hasSameMembersAs(snapshot) || getTagModel().isDirty())
          && !(snapshot == null && getTagModel().getTags().isEmpty())) {
             //give the user a chance to save the changes
-            int ret = confirmClosingByCancel(this.editorAccess.getChangedRelation());
+            int ret = confirmClosingByCancel(this.editorAccess.wouldRelationBeUseful());
             if (ret == 0) { //Yes, save the changes
                 //copied from OKAction.run()
                 Config.getPref().put("relation.editor.generic.lastrole", Utils.strip(tfRole.getText()));
@@ -61 +60 @@
         hideEditor();
     }
 
-    protected int confirmClosingByCancel(final IRelation<?> newRelation) {
+    protected int confirmClosingByCancel(boolean isUseful) {
         ButtonSpec[] options = {
                 new ButtonSpec(
                         tr("Yes, save the changes and close"),
@@ -85 +84 @@
 
         // Keep users from saving invalid relations -- a relation MUST have at least a tag with the key "type"
         // AND must contain at least one other OSM object.
-        options[0].setEnabled(newRelation.isUseful());
+        options[0].setEnabled(isUseful);
 
         return HelpAwareOptionPane.showOptionDialog(
                 MainApplication.getMainFrame(),

src/org/openstreetmap/josm/gui/dialogs/relation/actions/IRelationEditorActionAccess.java

@@ -71 +71 @@
 
     /**
      * Get the changed relation
-     * @return The changed relation (note: will not be part of a dataset). This should never be {@code null}.
+     * @return The changed relation (note: will not be part of a dataset) or the
+     * value returned by {@code getEditor().getRelation()}. This should never be {@code null}.
+     * If called for a temporary use of the relation instance, make sure to cleanup a copy to avoid memory leaks, see #23527
      * @since 18413
      */
     default IRelation<?> getChangedRelation() {
@@ -97 +99 @@
     }
 
     /**
+     * Check if the changed relation would be useful.
+     * @return true if the saved relation has at least one tag and one member
+     * @since xxx
+     */
+    default boolean wouldRelationBeUseful() {
+        return (getTagModel().getRowCount() > 0 && getMemberTableModel().getRowCount() > 0);
+    }
+
+    /**
      * Get the text field that is used to edit the role.
      * @return The role text field.
      */

test/unit/org/openstreetmap/josm/gui/dialogs/relation/actions/RelationEditorActionsTest.java

@@ -16 +16 @@
 import org.openstreetmap.josm.TestUtils;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.osm.IRelation;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
@@ -168 +169 @@
         relationEditorAccess.getMemberTableModel().populate(relation);
         relationEditorAccess.getTagModel().initFromPrimitive(relation);
         relationEditorAccess.getEditor().reloadDataFromRelation();
-        assertDoesNotThrow(relationEditorAccess::getChangedRelation);
+
+        assertDoesNotThrow(() -> {
+            IRelation<?> tempRelation = relationEditorAccess.getChangedRelation();
+            if (tempRelation.getDataSet() == null)
+                tempRelation.setMembers(null); // see #19885
+        });
     }
 }