diff --git a/src/org/openstreetmap/josm/data/osm/IRelation.java b/src/org/openstreetmap/josm/data/osm/IRelation.java index 6d59bd427a..9a7897d36f 100644 --- a/src/org/openstreetmap/josm/data/osm/IRelation.java +++ b/src/org/openstreetmap/josm/data/osm/IRelation.java @@ -2,9 +2,11 @@ package org.openstreetmap.josm.data.osm; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.stream.Collectors; +import org.openstreetmap.josm.data.validation.TestError; import org.openstreetmap.josm.tools.Utils; /** @@ -138,4 +140,20 @@ public interface IRelation> extends IPrimitive { return getMembers().stream().filter(rmv -> role.equals(rmv.getRole())) .map(IRelationMember::getMember).collect(Collectors.toList()); } + + /** + * Check if this relation is useful + * @return {@code true} if this relation is useful + */ + default boolean isUseful() { + return !this.isEmpty() && this.hasKey("type"); + } + + /** + * Check if this relation is valid. Note: You probably don't want to run this in the EDT. + * @return A collection of errors, if any + */ + default Collection validate() { + return Collections.emptyList(); + } } diff --git a/src/org/openstreetmap/josm/data/osm/Relation.java b/src/org/openstreetmap/josm/data/osm/Relation.java index 3b12c48c80..1653b30f2e 100644 --- a/src/org/openstreetmap/josm/data/osm/Relation.java +++ b/src/org/openstreetmap/josm/data/osm/Relation.java @@ -14,6 +14,12 @@ import java.util.stream.Stream; import org.openstreetmap.josm.data.osm.visitor.OsmPrimitiveVisitor; import org.openstreetmap.josm.data.osm.visitor.PrimitiveVisitor; +import org.openstreetmap.josm.data.validation.OsmValidator; +import org.openstreetmap.josm.data.validation.TestError; +import org.openstreetmap.josm.data.validation.tests.MultipolygonTest; +import org.openstreetmap.josm.data.validation.tests.RelationChecker; +import org.openstreetmap.josm.data.validation.tests.TurnrestrictionTest; +import org.openstreetmap.josm.gui.progress.NullProgressMonitor; import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.tools.CopyList; import org.openstreetmap.josm.tools.SubclassFilteredCollection; @@ -567,4 +573,15 @@ public final class Relation extends OsmPrimitive implements IRelation validate() { + return Stream.of(MultipolygonTest.class, TurnrestrictionTest.class, RelationChecker.class) + .map(OsmValidator::getTest).filter(test -> test.enabled || test.testBeforeUpload).flatMap(test -> { + test.startTest(NullProgressMonitor.INSTANCE); + test.visit(this); + test.endTest(); + return test.getErrors().stream(); + }).collect(Collectors.toList()); + } } diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java b/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java index 743e05f4e0..f597a078c0 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java @@ -47,6 +47,7 @@ import javax.swing.JTabbedPane; import javax.swing.JTable; import javax.swing.JToolBar; import javax.swing.KeyStroke; +import javax.swing.event.TableModelListener; import org.openstreetmap.josm.actions.JosmAction; import org.openstreetmap.josm.command.ChangeMembersCommand; @@ -54,6 +55,7 @@ import org.openstreetmap.josm.command.Command; 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; @@ -132,6 +134,7 @@ public class GenericRelationEditor extends RelationEditor implements CommandQueu private final SelectionTableModel selectionTableModel; private final AutoCompletingTextField tfRole; + private final RelationEditorActionAccess actionAccess; /** * the menu item in the windows menu. Required to properly hide on dialog close. @@ -262,13 +265,17 @@ public class GenericRelationEditor extends RelationEditor implements CommandQueu selectedTabPane = sourceTabbedPane.getSelectedComponent(); }); - IRelationEditorActionAccess actionAccess = new RelationEditorActionAccess(); + actionAccess = new RelationEditorActionAccess(); refreshAction = new RefreshAction(actionAccess); applyAction = new ApplyAction(actionAccess); selectAction = new SelectAction(actionAccess); duplicateAction = new DuplicateRelationAction(actionAccess); deleteAction = new DeleteCurrentRelationAction(actionAccess); + + this.memberTableModel.addTableModelListener(applyAction); + this.tagEditorPanel.getModel().addTableModelListener(applyAction); + addPropertyChangeListener(deleteAction); okAction = new OKAction(actionAccess); @@ -276,7 +283,7 @@ public class GenericRelationEditor extends RelationEditor implements CommandQueu getContentPane().add(buildToolBar(refreshAction, applyAction, selectAction, duplicateAction, deleteAction), BorderLayout.NORTH); getContentPane().add(tabbedPane, BorderLayout.CENTER); - getContentPane().add(buildOkCancelButtonPanel(okAction, cancelAction), BorderLayout.SOUTH); + getContentPane().add(buildOkCancelButtonPanel(okAction, deleteAction, cancelAction), BorderLayout.SOUTH); setSize(findMaxDialogSize()); @@ -407,14 +414,41 @@ public class GenericRelationEditor extends RelationEditor implements CommandQueu * * @return the panel with the OK and the Cancel button */ - protected static JPanel buildOkCancelButtonPanel(OKAction okAction, CancelAction cancelAction) { - JPanel pnl = new JPanel(new FlowLayout(FlowLayout.CENTER)); - pnl.add(new JButton(okAction)); + protected final JPanel buildOkCancelButtonPanel(OKAction okAction, DeleteCurrentRelationAction deleteAction, + CancelAction cancelAction) { + final JPanel pnl = new JPanel(new FlowLayout(FlowLayout.CENTER)); + final JButton okButton = new JButton(okAction); + final JButton deleteButton = new JButton(deleteAction); + okButton.setPreferredSize(deleteButton.getPreferredSize()); + pnl.add(okButton); + pnl.add(deleteButton); pnl.add(new JButton(cancelAction)); 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); + listener.tableChanged(null); + this.memberTableModel.addTableModelListener(listener); + this.tagEditorPanel.getModel().addTableModelListener(listener); return pnl; } + /** + * Update the OK panel area + * @param newRelation What the new relation would "look" like 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()) { + okButton.setText(tr("Delete")); + } else { + okButton.setText(tr("OK")); + } + } + /** * builds the panel with the tag editor * @param tagEditorPanel tag editor panel @@ -1001,7 +1035,7 @@ public class GenericRelationEditor extends RelationEditor implements CommandQueu @Override public void mouseClicked(MouseEvent e) { if (e.getButton() == MouseEvent.BUTTON1 && e.getClickCount() == 2) { - new EditAction(new RelationEditorActionAccess()).actionPerformed(null); + new EditAction(actionAccess).actionPerformed(null); } } } diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/ApplyAction.java b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/ApplyAction.java index 7ddfae63ac..0b65d27cb5 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/ApplyAction.java +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/ApplyAction.java @@ -35,6 +35,6 @@ public class ApplyAction extends SavingAction { @Override public void updateEnabledState() { - setEnabled(isEditorDirty()); + setEnabled(this.editorAccess.getChangedRelation().isUseful() && isEditorDirty()); } } diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/CancelAction.java b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/CancelAction.java index 6efdaeabdf..a51be40834 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/CancelAction.java +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/CancelAction.java @@ -8,6 +8,7 @@ import java.awt.event.ActionEvent; 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; @@ -47,7 +48,7 @@ public class CancelAction extends SavingAction { if ((!getMemberTableModel().hasSameMembersAs(snapshot) || getTagModel().isDirty()) && !(snapshot == null && getTagModel().getTags().isEmpty())) { //give the user a chance to save the changes - int ret = confirmClosingByCancel(); + int ret = confirmClosingByCancel(this.editorAccess.getChangedRelation()); if (ret == 0) { //Yes, save the changes //copied from OKAction.run() Config.getPref().put("relation.editor.generic.lastrole", Utils.strip(tfRole.getText())); @@ -60,7 +61,7 @@ public class CancelAction extends SavingAction { hideEditor(); } - protected int confirmClosingByCancel() { + protected int confirmClosingByCancel(final IRelation newRelation) { ButtonSpec[] options = { new ButtonSpec( tr("Yes, save the changes and close"), @@ -82,6 +83,10 @@ public class CancelAction extends SavingAction { ) }; + // 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()); + return HelpAwareOptionPane.showOptionDialog( MainApplication.getMainFrame(), tr("The relation has been changed.

Do you want to save your changes?"), diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/IRelationEditorActionAccess.java b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/IRelationEditorActionAccess.java index cddedaf165..b1b79a3128 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/IRelationEditorActionAccess.java +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/IRelationEditorActionAccess.java @@ -3,6 +3,8 @@ package org.openstreetmap.josm.gui.dialogs.relation.actions; import javax.swing.Action; +import org.openstreetmap.josm.data.osm.IRelation; +import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.gui.dialogs.relation.IRelationEditor; import org.openstreetmap.josm.gui.dialogs.relation.MemberTable; import org.openstreetmap.josm.gui.dialogs.relation.MemberTableModel; @@ -65,6 +67,23 @@ public interface IRelationEditorActionAccess { */ TagEditorModel getTagModel(); + /** + * Get the changed relation + * @return The changed relation (note: will not be part of a dataset). This should never be {@code null}. + * @since xxx + */ + default IRelation getChangedRelation() { + final Relation newRelation; + if (getEditor().getRelation() != null) { + newRelation = new Relation(getEditor().getRelation()); + } else { + newRelation = new Relation(); + } + getTagModel().applyToPrimitive(newRelation); + getMemberTableModel().applyToRelation(newRelation); + return newRelation; + } + /** * Get the text field that is used to edit the role. * @return The role text field. diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/SavingAction.java b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/SavingAction.java index 26813f23c1..35ca1dcfb7 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/SavingAction.java +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/SavingAction.java @@ -54,8 +54,8 @@ abstract class SavingAction extends AbstractRelationEditorAction { tagEditorModel.applyToPrimitive(newRelation); getMemberTableModel().applyToRelation(newRelation); // If the user wanted to create a new relation, but hasn't added any members or - // tags, don't add an empty relation - if (newRelation.isEmpty() && !newRelation.hasKeys()) + // tags (specifically the "type" tag), don't add the relation + if (!newRelation.isUseful()) return; UndoRedoHandler.getInstance().add(new AddCommand(getLayer().getDataSet(), newRelation));