Index: src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java =================================================================== --- src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java (revision 19004) +++ src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java (working copy) @@ -55,7 +55,6 @@ 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,6 +404,7 @@ /** * 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,7 +421,7 @@ 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,15 +429,15 @@ } /** - * 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")); Index: src/org/openstreetmap/josm/gui/dialogs/relation/actions/ApplyAction.java =================================================================== --- src/org/openstreetmap/josm/gui/dialogs/relation/actions/ApplyAction.java (revision 19004) +++ src/org/openstreetmap/josm/gui/dialogs/relation/actions/ApplyAction.java (working copy) @@ -35,6 +35,6 @@ @Override public void updateEnabledState() { - setEnabled(this.editorAccess.getChangedRelation().isUseful() && isEditorDirty()); + setEnabled(this.editorAccess.wouldRelationBeUseful() && isEditorDirty()); } } Index: src/org/openstreetmap/josm/gui/dialogs/relation/actions/CancelAction.java =================================================================== --- src/org/openstreetmap/josm/gui/dialogs/relation/actions/CancelAction.java (revision 19004) +++ src/org/openstreetmap/josm/gui/dialogs/relation/actions/CancelAction.java (working copy) @@ -8,7 +8,6 @@ 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,7 +47,7 @@ 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,7 +60,7 @@ hideEditor(); } - protected int confirmClosingByCancel(final IRelation newRelation) { + protected int confirmClosingByCancel(boolean isUseful) { ButtonSpec[] options = { new ButtonSpec( tr("Yes, save the changes and close"), @@ -85,7 +84,7 @@ // 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(), Index: src/org/openstreetmap/josm/gui/dialogs/relation/actions/IRelationEditorActionAccess.java =================================================================== --- src/org/openstreetmap/josm/gui/dialogs/relation/actions/IRelationEditorActionAccess.java (revision 19004) +++ src/org/openstreetmap/josm/gui/dialogs/relation/actions/IRelationEditorActionAccess.java (working copy) @@ -74,6 +74,7 @@ * @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,6 +98,35 @@ } /** + * 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. */ Index: test/unit/org/openstreetmap/josm/gui/dialogs/relation/actions/RelationEditorActionsTest.java =================================================================== --- test/unit/org/openstreetmap/josm/gui/dialogs/relation/actions/RelationEditorActionsTest.java (revision 19004) +++ test/unit/org/openstreetmap/josm/gui/dialogs/relation/actions/RelationEditorActionsTest.java (working copy) @@ -169,5 +169,6 @@ relationEditorAccess.getTagModel().initFromPrimitive(relation); relationEditorAccess.getEditor().reloadDataFromRelation(); assertDoesNotThrow(relationEditorAccess::getChangedRelation); + assertDoesNotThrow(relationEditorAccess::wouldRelationBeUseful); } }