Ticket #21951: 21951.patch

src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java

@@ -154 +154 @@
                     .message(tr("Route scheme is unspecified. Add {0} ({1}=public_transport; {2}=legacy)", "public_transport:version", "2", "1"))
                     .primitives(n)
                     .build());
-        } else if (n.hasKey("type") && allroles.isEmpty()) {
+        } else if ((n.hasKey("type") && allroles.isEmpty()) || !n.hasKey("type")) {
             errors.add(TestError.builder(this, Severity.OTHER, RELATION_UNKNOWN)
                     .message(tr("Relation type is unknown"))
                     .primitives(n)

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

@@ -3 +3 @@
 
 import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
 import static org.openstreetmap.josm.tools.I18n.tr;
+import static org.openstreetmap.josm.tools.I18n.trn;
 
 import java.awt.BorderLayout;
 import java.awt.Component;
@@ -26 +27 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
 import javax.swing.AbstractAction;
@@ -47 +51 @@
 import javax.swing.JTable;
 import javax.swing.JToolBar;
 import javax.swing.KeyStroke;
+import javax.swing.event.TableModelListener;
+import javax.swing.table.TableModel;
 
 import org.openstreetmap.josm.actions.JosmAction;
 import org.openstreetmap.josm.command.ChangeMembersCommand;
@@ -54 +60 @@
 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;
 import org.openstreetmap.josm.data.osm.Tag;
+import org.openstreetmap.josm.data.validation.TestError;
 import org.openstreetmap.josm.data.validation.tests.RelationChecker;
 import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
 import org.openstreetmap.josm.gui.MainApplication;
@@ -105 +113 @@
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetHandler;
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetType;
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresets;
+import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.gui.util.WindowGeometry;
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.CheckParameterUtil;
+import org.openstreetmap.josm.tools.GBC;
+import org.openstreetmap.josm.tools.ImageProvider;
 import org.openstreetmap.josm.tools.InputMapUtils;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Shortcut;
@@ -132 +143 @@
     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 +274 @@
             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(l -> {
+            // Short circuit -- the tag editor will fire an update event on selection change, even if nothing actually
+            // changed.
+            if (l != null && l.getSource() instanceof TagEditorModel && !((TagEditorModel) l.getSource()).isDirty()) {
+                return;
+            }
+            applyAction.tableChanged(l);
+        });
+
         addPropertyChangeListener(deleteAction);
 
         okAction = new OKAction(actionAccess);
@@ -276 +299 @@
 
         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 +430 @@
      *
      * @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 JPanel buildOkCancelButtonPanel(OKAction okAction, DeleteCurrentRelationAction deleteAction,
+            CancelAction cancelAction) {
+        JPanel mainPanel = new JPanel(new GridBagLayout());
+        final JLabel errorLabel = new JLabel(" ");
+        final JPanel pnl = new JPanel(new FlowLayout(FlowLayout.CENTER));
+        mainPanel.add(errorLabel, GBC.eol().anchor(GridBagConstraints.CENTER));
+        mainPanel.add(pnl, GBC.eol().anchor(GridBagConstraints.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"))));
-        return pnl;
+        // 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 AtomicReference<Future<?>> testErrorsUpdate = new AtomicReference<>();
+        final TableModelListener listener = l -> {
+            final IRelation<?> newRelation = this.actionAccess.getChangedRelation();
+            updateOkPanel(newRelation, okButton, deleteButton);
+            final Future<?> oldFuture = testErrorsUpdate.getAndSet(MainApplication.worker.submit(() -> {
+                GuiHelper.runInEDTAndWait(() -> errorLabel.setText(tr("Validation in progress")));
+                final Collection<TestError> testErrors = newRelation.validate();
+                GuiHelper.runInEDTAndWait(() -> updateErrorMessage(testErrors, errorLabel));
+            }));
+            if (oldFuture != null) {
+                oldFuture.cancel(true);
+            }
+        };
+        listener.tableChanged(null);
+        this.memberTableModel.addTableModelListener(listener);
+        this.tagEditorPanel.getModel().addTableModelListener(l -> {
+            // Short circuit -- the tag editor will fire an update event on selection change, even if nothing actually
+            // changed.
+            if (l != null && l.getSource() instanceof TagEditorModel && !((TagEditorModel) l.getSource()).isDirty()) {
+                return;
+            }
+            listener.tableChanged(l);
+        });
+        return mainPanel;
+    }
+
+    /**
+     * 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"));
+        }
+    }
+
+    /**
+     * Update the error message
+     * @param newRelation What the new relation would "look" like if it were to be saved now
+     * @param errorLabel The label to update
+     */
+    private void updateErrorMessage(Collection<TestError> errors, JLabel errorLabel) {
+        if (errors.isEmpty()) {
+            // Setting " " helps keep the label in place for layout calculations
+            errorLabel.setText(" ");
+            errorLabel.setIcon(null);
+        } else {
+            final TestError error = errors.stream()
+                    .min(Comparator.comparingInt(testError -> testError.getSeverity().getLevel()))
+                    .orElse(errors.iterator().next());
+            final StringBuilder sb = new StringBuilder();
+            if (error.getDescription() != null) {
+                sb.append(error.getDescription()).append(": ");
+            }
+            sb.append(error.getMessage());
+            if (errors.size() > 1) {
+                sb.append(trn(" with {0} more problem", " with {0} more problems", errors.size() - 1, errors.size() - 1));
+            }
+            errorLabel.setIcon(ImageProvider.get("data", error.getSeverity().getIcon()));
+            errorLabel.setText(sb.toString());
+        }
     }
 
     /**