Changeset 18395 in josm

Timestamp:

2022-03-13T08:07:10+01:00 (3 years ago)

Author:

GerdP

Message:

fix #21825: Delete relations by default when all members are deleted
Patch from taylor.smock:

• Add table where users can deselect/select relations to delete, when a delete command will remove all members of the relation

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/DeleteAction.java (modified) (3 diffs)
• command/DeleteCommand.java (modified) (4 diffs)
• gui/dialogs/DeleteFromRelationConfirmationDialog.java (modified) (11 diffs)

trunk/src/org/openstreetmap/josm/actions/DeleteAction.java

@@ -10 +10 @@
 import java.awt.event.KeyEvent;
 import java.util.Collection;
+import java.util.Collections;
 
 import javax.swing.JOptionPane;
@@ -24 +25 @@
 import org.openstreetmap.josm.gui.dialogs.DeleteFromRelationConfirmationDialog;
 import org.openstreetmap.josm.gui.widgets.JMultilineLabel;
+import org.openstreetmap.josm.tools.Pair;
 import org.openstreetmap.josm.tools.Shortcut;
 
@@ -50 +52 @@
         @Override
         public boolean confirmDeletionFromRelation(Collection<RelationToChildReference> references) {
+            return this.confirmDeletionFromRelation(references, Collections.emptyList());
+        }
+
+        @Override
+        public boolean confirmDeletionFromRelation(Collection<RelationToChildReference> references,
+                Collection<Pair<Relation, Boolean>> parentsToDelete) {
             DeleteFromRelationConfirmationDialog dialog = DeleteFromRelationConfirmationDialog.getInstance();
             dialog.getModel().populate(references);
+            dialog.getDeletedRelationsModel().populate(parentsToDelete);
             dialog.setVisible(true);
             return !dialog.isCanceled();

trunk/src/org/openstreetmap/josm/command/DeleteCommand.java

@@ -36 +36 @@
 import org.openstreetmap.josm.tools.CheckParameterUtil;
 import org.openstreetmap.josm.tools.ImageProvider;
+import org.openstreetmap.josm.tools.Pair;
 import org.openstreetmap.josm.tools.Utils;
 
@@ -102 +103 @@
          */
         boolean confirmDeletionFromRelation(Collection<RelationToChildReference> references);
+
+        /**
+         * Confirm before removing a collection of primitives from their parent relations, with the probability of
+         * deleting the parents as well.
+         * @param references the list of relation-to-child references
+         * @param parentsToDelete the list of parents to delete (the boolean part will be {@code true} if the user wants
+         *                        to delete the relation).
+         * @return {@code true} if the user confirms the deletion
+         * @since 18395
+         */
+        default boolean confirmDeletionFromRelation(Collection<RelationToChildReference> references,
+                Collection<Pair<Relation, Boolean>> parentsToDelete) {
+            // This is a default method. Ensure that all the booleans are false.
+            parentsToDelete.forEach(pair -> pair.b = false);
+            return confirmDeletionFromRelation(references);
+        }
     }
 
@@ -438 +455 @@
         }
 
+        // remove the objects from their parent relations
+        //
+        final Set<Relation> relationsToBeChanged = primitivesToDelete.stream()
+                .flatMap(p -> p.referrers(Relation.class))
+                .collect(Collectors.toSet());
+        final Set<Relation> additionalRelationsToDelete = new HashSet<>();
+        for (Relation cur : relationsToBeChanged) {
+            List<RelationMember> newMembers = cur.getMembers();
+            cur.getMembersFor(primitivesToDelete).forEach(newMembers::remove);
+            cmds.add(new ChangeMembersCommand(cur, newMembers));
+            // If we don't have any members, we probably ought to delete the relation as well.
+            if (newMembers.isEmpty()) {
+                additionalRelationsToDelete.add(cur);
+            }
+        }
+
+
         // get a confirmation that the objects to delete can be removed from their parent relations
         //
@@ -443 +477 @@
             Set<RelationToChildReference> references = RelationToChildReference.getRelationToChildReferences(primitivesToDelete);
             references.removeIf(ref -> ref.getParent().isDeleted());
-            if (!references.isEmpty() && !callback.confirmDeletionFromRelation(references)) {
+            final Collection<Pair<Relation, Boolean>> pairedRelations = additionalRelationsToDelete.stream()
+                    /*
+                     * Default to true, so that users have to make a choice to not delete.
+                     * Default implementation converts it to false, so this should be safe.
+                     */
+                    .map(relation -> new Pair<>(relation, true)).collect(Collectors.toSet());
+            if (!references.isEmpty() && !callback.confirmDeletionFromRelation(references, pairedRelations)) {
                 return null;
             }
-        }
-
-        // remove the objects from their parent relations
-        //
-        final Set<Relation> relationsToBeChanged = primitivesToDelete.stream()
-                .flatMap(p -> p.referrers(Relation.class))
-                .collect(Collectors.toSet());
-        for (Relation cur : relationsToBeChanged) {
-            List<RelationMember> newMembers = cur.getMembers();
-            cur.getMembersFor(primitivesToDelete).forEach(newMembers::remove);
-            cmds.add(new ChangeMembersCommand(cur, newMembers));
+            pairedRelations.stream().filter(pair -> Boolean.TRUE.equals(pair.b)).map(pair -> pair.a)
+                    .forEach(primitivesToDelete::add);
         }
 

trunk/src/org/openstreetmap/josm/gui/dialogs/DeleteFromRelationConfirmationDialog.java

@@ -9 +9 @@
 import java.awt.Dimension;
 import java.awt.FlowLayout;
+import java.awt.GridBagLayout;
 import java.awt.event.ActionEvent;
 import java.awt.event.WindowAdapter;
@@ -35 +36 @@
 import org.openstreetmap.josm.data.osm.NameFormatter;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
+import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationToChildReference;
 import org.openstreetmap.josm.gui.MainApplication;
@@ -43 +45 @@
 import org.openstreetmap.josm.gui.util.WindowGeometry;
 import org.openstreetmap.josm.gui.widgets.HtmlPanel;
+import org.openstreetmap.josm.tools.GBC;
 import org.openstreetmap.josm.tools.I18n;
 import org.openstreetmap.josm.tools.ImageProvider;
+import org.openstreetmap.josm.tools.Pair;
+import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -69 +74 @@
     /** the data model */
     private RelationMemberTableModel model;
+    /** The data model for deleting relations */
+    private RelationDeleteModel deletedRelationsModel;
+    /** The table to hide/show if the relations to delete are not empty*/
     private final HtmlPanel htmlPanel = new HtmlPanel();
     private boolean canceled;
@@ -75 +83 @@
     protected JPanel buildRelationMemberTablePanel() {
         JTable table = new JTable(model, new RelationMemberTableColumnModel());
-        JPanel pnl = new JPanel(new BorderLayout());
-        pnl.add(new JScrollPane(table));
+        JPanel pnl = new JPanel(new GridBagLayout());
+        pnl.add(new JScrollPane(table), GBC.eol().fill());
+        JTable deletedRelationsTable = new JTable(this.deletedRelationsModel, new RelationDeleteTableColumnModel());
+        JScrollPane deletedRelationsModelTableScrollPane = new JScrollPane(deletedRelationsTable);
+        this.deletedRelationsModel.addTableModelListener(
+                e -> deletedRelationsModelTableScrollPane.setVisible(this.deletedRelationsModel.getRowCount() > 0));
+        // Default to not visible
+        deletedRelationsModelTableScrollPane.setVisible(false);
+        pnl.add(deletedRelationsModelTableScrollPane, GBC.eol().fill());
         return pnl;
     }
@@ -92 +107 @@
         model = new RelationMemberTableModel();
         model.addTableModelListener(this);
+        this.deletedRelationsModel = new RelationDeleteModel();
+        this.deletedRelationsModel.addTableModelListener(this);
         getContentPane().setLayout(new BorderLayout());
         getContentPane().add(htmlPanel, BorderLayout.NORTH);
@@ -103 +120 @@
 
     protected void updateMessage() {
-        int numObjectsToDelete = model.getNumObjectsToDelete();
-        int numParentRelations = model.getNumParentRelations();
+        int numObjectsToDelete = this.model.getNumObjectsToDelete() + this.deletedRelationsModel.getNumObjectsToDelete();
+        int numParentRelations = this.model.getNumParentRelations() + this.deletedRelationsModel.getNumParentRelations();
         final String msg1 = trn(
                 "Please confirm to remove <strong>{0} object</strong>.",
@@ -120 +137 @@
 
     protected void updateTitle() {
-        int numObjectsToDelete = model.getNumObjectsToDelete();
+        int numObjectsToDelete = this.model.getNumObjectsToDelete() + this.deletedRelationsModel.getNumObjectsToDelete();
         if (numObjectsToDelete > 0) {
             setTitle(trn("Deleting {0} object", "Deleting {0} objects", numObjectsToDelete, numObjectsToDelete));
@@ -143 +160 @@
     public RelationMemberTableModel getModel() {
         return model;
+    }
+
+    /**
+     * Replies the data model used for relations that should probably be deleted.
+     * @return the data model
+     * @since 18395
+     */
+    public RelationDeleteModel getDeletedRelationsModel() {
+        return this.deletedRelationsModel;
     }
 
@@ -174 +200 @@
             }
             model.data.clear();
+            this.deletedRelationsModel.data.clear();
         }
         super.setVisible(visible);
@@ -326 +353 @@
     }
 
+    /**
+     * The table model which manages relations that will be deleted, if their children are deleted.
+     * @since 18395
+     */
+    public static class RelationDeleteModel extends DefaultTableModel {
+        private final transient List<Pair<Relation, Boolean>> data = new ArrayList<>();
+
+        @Override
+        public int getRowCount() {
+            // This is called in the super constructor. Before we have instantiated the list. Removing the null check
+            // WILL LEAD TO A SILENT NPE!
+            if (this.data == null) {
+                return 0;
+            }
+            return this.data.size();
+        }
+
+        /**
+         * Sets the data that should be displayed in the list.
+         * @param references A list of references to display
+         */
+        public void populate(Collection<Pair<Relation, Boolean>> references) {
+            this.data.clear();
+            if (references != null) {
+                this.data.addAll(references);
+            }
+            this.data.sort(Comparator.comparing(pair -> pair.a));
+            fireTableDataChanged();
+        }
+
+        /**
+         * Gets the list of children that are currently displayed.
+         * @return The children.
+         */
+        public Set<Relation> getObjectsToDelete() {
+            return this.data.stream().filter(relation -> relation.b).map(relation -> relation.a).collect(Collectors.toSet());
+        }
+
+        /**
+         * Gets the number of elements {@link #getObjectsToDelete()} would return.
+         * @return That number.
+         */
+        public int getNumObjectsToDelete() {
+            return getObjectsToDelete().size();
+        }
+
+        /**
+         * Gets the set of parent relations
+         * @return All parent relations of the references
+         */
+        public Set<OsmPrimitive> getParentRelations() {
+            return this.data.stream()
+                    .flatMap(pair -> Utils.filteredCollection(pair.a.getReferrers(), Relation.class).stream())
+                    .collect(Collectors.toSet());
+        }
+
+        /**
+         * Gets the number of elements {@link #getParentRelations()} would return.
+         * @return That number.
+         */
+        public int getNumParentRelations() {
+            return getParentRelations().size();
+        }
+
+        @Override
+        public Object getValueAt(int rowIndex, int columnIndex) {
+            if (this.data.isEmpty()) {
+                return null;
+            }
+            Pair<Relation, Boolean> ref = this.data.get(rowIndex);
+            switch(columnIndex) {
+            case 0: return ref.a;
+            case 1: return ref.b;
+            default:
+                assert false : "Illegal column index";
+            }
+            return null;
+        }
+
+        @Override
+        public boolean isCellEditable(int row, int column) {
+            return !this.data.isEmpty() && column == 1;
+        }
+
+        @Override
+        public void setValueAt(Object aValue, int row, int column) {
+            if (this.data.size() > row && column == 1 && aValue instanceof Boolean) {
+                this.data.get(row).b = ((Boolean) aValue);
+            }
+        }
+
+        @Override
+        public Class<?> getColumnClass(int columnIndex) {
+            switch (columnIndex) {
+            case 0:
+                return Relation.class;
+            case 1:
+                return Boolean.class;
+            default:
+                return super.getColumnClass(columnIndex);
+            }
+        }
+    }
+
+    private static class RelationDeleteTableColumnModel extends DefaultTableColumnModel {
+        protected final void createColumns() {
+            // column 0 - To Delete
+            TableColumn col = new TableColumn(0);
+            col.setHeaderValue(tr("Relation"));
+            col.setResizable(true);
+            col.setWidth(100);
+            col.setPreferredWidth(100);
+            col.setCellRenderer(new PrimitiveRenderer());
+            addColumn(col);
+
+            // column 0 - From Relation
+            col = new TableColumn(1);
+            final String toDelete = tr("To delete");
+            col.setHeaderValue(toDelete);
+            col.setResizable(true);
+            col.setPreferredWidth(toDelete.length());
+            addColumn(col);
+        }
+
+        RelationDeleteTableColumnModel() {
+            createColumns();
+        }
+    }
+
     class OKAction extends AbstractAction {
         OKAction() {