Ticket #23527: 23527-beta.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

@@ -74 +74 @@
      * @return The changed relation (note: will not be part of a dataset). This should never be {@code null}.
      * @since 18413
      */
+    @Deprecated
     default IRelation<?> getChangedRelation() {
         final Relation newRelation;
         final Relation oldRelation = getEditor().getRelation();
@@ -97 +98 @@
     }
 
     /**
+     * Check if the changed relation would be useful.
+     * @return true if the saved relation would be useful
+     * @since xxx
+     */
+    default boolean wouldRelationBeUseful() {
+        final Relation newRelation;
+        final Relation oldRelation = getEditor().getRelation();
+        boolean isUploadInProgress = MainApplication.getLayerManager().getLayersOfType(OsmDataLayer.class)
+                .stream().anyMatch(OsmDataLayer::isUploadInProgress);
+        if (isUploadInProgress || (oldRelation != null && oldRelation.getDataSet() != null && oldRelation.getDataSet().isLocked())) {
+            // If the dataset is locked, then we cannot change the relation. See JOSM #22024.
+            // We should also avoid changing the relation if there is an upload in progress. See JOSM #22268/#22398.
+            // There appears to be a race condition where a dataset might not be locked in the check, then is locked while using the
+            // copy relation constructor.
+            // This is due to the `setMembers` -> `addReferrer` call chain requires that the dataset is not read only.
+            return oldRelation != null && oldRelation.isUseful();
+        } else if (oldRelation != null) {
+            newRelation = new Relation(oldRelation);
+        } else {
+            newRelation = new Relation();
+        }
+        getTagModel().applyToPrimitive(newRelation);
+        getMemberTableModel().applyToRelation(newRelation);
+        boolean res = newRelation.isUseful();
+        newRelation.setMembers(null);
+        return res;
+    }
+
+    /**
      * 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

@@ -169 +169 @@
         relationEditorAccess.getTagModel().initFromPrimitive(relation);
         relationEditorAccess.getEditor().reloadDataFromRelation();
         assertDoesNotThrow(relationEditorAccess::getChangedRelation);
+        assertDoesNotThrow(relationEditorAccess::wouldRelationBeUseful);
     }
 }