Changeset 10880 in josm for trunk/src/org/openstreetmap/josm/gui

Timestamp:

2016-08-23T22:07:06+02:00 (8 years ago)

Author:

Don-vip

Message:

fix #13429 - Clean validator tree and use listener to find changes (patch by michael2402) - gsoc-core

Location:

trunk/src/org/openstreetmap/josm/gui

Files:

• dialogs/ValidatorDialog.java (modified) (2 diffs)
• dialogs/validator/ValidatorTreePanel.java (modified) (12 diffs)
• layer/ValidatorLayer.java (modified) (6 diffs)
• preferences/validator/ValidatorPreference.java (modified) (3 diffs)
• preferences/validator/ValidatorTestsPreference.java (modified) (3 diffs)

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

@@ -151 +151 @@
         buttons.add(fixButton);
 
-        if (Main.pref.getBoolean(ValidatorPreference.PREF_USE_IGNORE, true)) {
+        if (ValidatorPreference.PREF_USE_IGNORE.get()) {
             ignoreButton = new SideButton(new AbstractAction() {
                 {
@@ -193 +193 @@
         }
         super.setVisible(v);
-        Main.map.repaint();
     }
 

trunk/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanel.java

@@ -11 +11 @@
 import java.util.EnumMap;
 import java.util.Enumeration;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -18 +17 @@
 import java.util.Set;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 import javax.swing.JTree;
@@ -36 +36 @@
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.tools.Destroyable;
-import org.openstreetmap.josm.tools.MultiMap;
+import org.openstreetmap.josm.tools.ListenerList;
 
 /**
@@ -73 +73 @@
     private transient Set<? extends OsmPrimitive> filter;
 
-    /** a counter to check if tree has been rebuild */
-    private int updateCount;
+    private final ListenerList<Runnable> invalidationListeners = ListenerList.create();
 
     /**
@@ -135 +134 @@
         }
         super.setVisible(v);
+        invalidationListeners.fireEvent(Runnable::run);
     }
 
@@ -141 +141 @@
      */
     public void buildTree() {
-        updateCount++;
         final DefaultMutableTreeNode rootNode = new DefaultMutableTreeNode();
 
@@ -172 +171 @@
         }
 
-        Map<Severity, MultiMap<String, TestError>> errorTree = new EnumMap<>(Severity.class);
-        Map<Severity, HashMap<String, MultiMap<String, TestError>>> errorTreeDeep = new EnumMap<>(Severity.class);
-        for (Severity s : Severity.values()) {
-            errorTree.put(s, new MultiMap<String, TestError>(20));
-            errorTreeDeep.put(s, new HashMap<String, MultiMap<String, TestError>>());
-        }
-
-        final Boolean other = ValidatorPreference.PREF_OTHER.get();
-        for (TestError e : errors) {
-            if (e.isIgnored()) {
-                continue;
-            }
-            Severity s = e.getSeverity();
-            if (!other && s == Severity.OTHER) {
-                continue;
-            }
-            String d = e.getDescription();
-            String m = e.getMessage();
-            if (filter != null) {
-                boolean found = false;
-                for (OsmPrimitive p : e.getPrimitives()) {
-                    if (filter.contains(p)) {
-                        found = true;
-                        break;
-                    }
-                }
-                if (!found) {
-                    continue;
-                }
-            }
-            if (d != null) {
-                MultiMap<String, TestError> b = errorTreeDeep.get(s).get(m);
-                if (b == null) {
-                    b = new MultiMap<>(20);
-                    errorTreeDeep.get(s).put(m, b);
-                }
-                b.put(d, e);
-            } else {
-                errorTree.get(s).put(m, e);
-            }
-        }
+        Predicate<TestError> filterToUse = e -> !e.isIgnored();
+        if (!ValidatorPreference.PREF_OTHER.get()) {
+            filterToUse = filterToUse.and(e -> e.getSeverity() != Severity.OTHER);
+        }
+        if (filter != null) {
+            filterToUse = filterToUse.and(e -> e.getPrimitives().stream().anyMatch(filter::contains));
+        }
+        Map<Severity, Map<String, Map<String, List<TestError>>>> errorTreeDeep
+            = errors.stream().filter(filterToUse).collect(
+                    Collectors.groupingBy(e -> e.getSeverity(), () -> new EnumMap<>(Severity.class),
+                            Collectors.groupingBy(e -> e.getDescription() == null ? "" : e.getDescription(),
+                                    Collectors.groupingBy(e -> e.getMessage()))));
 
         List<TreePath> expandedPaths = new ArrayList<>();
-        for (Severity s : Severity.values()) {
-            MultiMap<String, TestError> severityErrors = errorTree.get(s);
-            Map<String, MultiMap<String, TestError>> severityErrorsDeep = errorTreeDeep.get(s);
-            if (severityErrors.isEmpty() && severityErrorsDeep.isEmpty()) {
-                continue;
-            }
-
+        errorTreeDeep.forEach((s, severityErrorsDeep) -> {
             // Severity node
             DefaultMutableTreeNode severityNode = new GroupTreeNode(s);
@@ -230 +194 @@
             }
 
-            for (Entry<String, Set<TestError>> msgErrors : severityErrors.entrySet()) {
-                // Message node
-                Set<TestError> errs = msgErrors.getValue();
-                String msg = tr("{0} ({1})", msgErrors.getKey(), errs.size());
-                DefaultMutableTreeNode messageNode = new DefaultMutableTreeNode(msg);
-                severityNode.add(messageNode);
-
-                if (oldSelectedRows.contains(msgErrors.getKey())) {
-                    expandedPaths.add(new TreePath(new Object[] {rootNode, severityNode, messageNode}));
-                }
-
-                for (TestError error : errs) {
-                    // Error node
-                    DefaultMutableTreeNode errorNode = new DefaultMutableTreeNode(error);
-                    messageNode.add(errorNode);
-                }
-            }
-            for (Entry<String, MultiMap<String, TestError>> bag : severityErrorsDeep.entrySet()) {
+            Map<String, List<TestError>> severityErrors = severityErrorsDeep.get("");
+            if (severityErrors != null) {
+                for (Entry<String, List<TestError>> msgErrors : severityErrors.entrySet()) {
+                    // Message node
+                    List<TestError> errs = msgErrors.getValue();
+                    String msg = tr("{0} ({1})", msgErrors.getKey(), errs.size());
+                    DefaultMutableTreeNode messageNode = new DefaultMutableTreeNode(msg);
+                    severityNode.add(messageNode);
+
+                    if (oldSelectedRows.contains(msgErrors.getKey())) {
+                        expandedPaths.add(new TreePath(new Object[] {rootNode, severityNode, messageNode}));
+                    }
+
+                    errs.stream().map(DefaultMutableTreeNode::new).forEach(messageNode::add);
+                }
+            }
+
+            severityErrorsDeep.forEach((description, errorlist) -> {
+                if (description.isEmpty()) {
+                    return;
+                }
                 // Group node
-                MultiMap<String, TestError> errorlist = bag.getValue();
-                DefaultMutableTreeNode groupNode = null;
+                DefaultMutableTreeNode groupNode;
                 if (errorlist.size() > 1) {
-                    groupNode = new GroupTreeNode(bag.getKey());
+                    groupNode = new GroupTreeNode(description);
                     severityNode.add(groupNode);
-                    if (oldSelectedRows.contains(bag.getKey())) {
+                    if (oldSelectedRows.contains(description)) {
                         expandedPaths.add(new TreePath(new Object[] {rootNode, severityNode, groupNode}));
                     }
-                }
-
-                for (Entry<String, Set<TestError>> msgErrors : errorlist.entrySet()) {
+                } else {
+                    groupNode = null;
+                }
+
+                errorlist.forEach((message, errs) -> {
                     // Message node
-                    Set<TestError> errs = msgErrors.getValue();
                     String msg;
                     if (groupNode != null) {
-                        msg = tr("{0} ({1})", msgErrors.getKey(), errs.size());
+                        msg = tr("{0} ({1})", message, errs.size());
                     } else {
-                        msg = tr("{0} - {1} ({2})", msgErrors.getKey(), bag.getKey(), errs.size());
+                        msg = tr("{0} - {1} ({2})", message, description, errs.size());
                     }
                     DefaultMutableTreeNode messageNode = new DefaultMutableTreeNode(msg);
@@ -275 +242 @@
                     }
 
-                    if (oldSelectedRows.contains(msgErrors.getKey())) {
+                    if (oldSelectedRows.contains(message)) {
                         if (groupNode != null) {
                             expandedPaths.add(new TreePath(new Object[] {rootNode, severityNode, groupNode, messageNode}));
@@ -283 +250 @@
                     }
 
-                    for (TestError error : errs) {
-                        // Error node
-                        DefaultMutableTreeNode errorNode = new DefaultMutableTreeNode(error);
-                        messageNode.add(errorNode);
-                    }
-                }
-            }
-        }
+                    errs.stream().map(DefaultMutableTreeNode::new).forEach(messageNode::add);
+                });
+            });
+        });
 
         valTreeModel.setRoot(rootNode);
@@ -296 +259 @@
             this.expandPath(path);
         }
+
+        invalidationListeners.fireEvent(Runnable::run);
+    }
+
+    /**
+     * Add a new invalidation listener
+     * @param listener The listener
+     */
+    public void addInvalidationListener(Runnable listener) {
+        invalidationListeners.addListener(listener);
+    }
+
+    /**
+     * Remove an invalidation listener
+     * @param listener The listener
+     * @since 10880
+     */
+    public void removeInvalidationListener(Runnable listener) {
+        invalidationListeners.removeListener(listener);
     }
 
@@ -426 +408 @@
     }
 
-    /**
-     * Returns a value to check if tree has been rebuild
-     * @return the current counter
-     */
-    public int getUpdateCount() {
-        return updateCount;
-    }
-
     private void clearErrors() {
         if (errors != null) {

trunk/src/org/openstreetmap/josm/gui/layer/ValidatorLayer.java

@@ -41 +41 @@
  */
 public class ValidatorLayer extends Layer implements LayerChangeListener {
-
-    private int updateCount = -1;
+    private final Runnable invalidator = this::invalidate;
 
     /**
@@ -50 +49 @@
         super(tr("Validation errors"));
         Main.getLayerManager().addLayerChangeListener(this);
+        Main.map.validatorDialog.tree.addInvalidationListener(invalidator);
     }
 
@@ -68 +68 @@
     @Override
     public void paint(final Graphics2D g, final MapView mv, Bounds bounds) {
-        updateCount = Main.map.validatorDialog.tree.getUpdateCount();
         DefaultMutableTreeNode root = Main.map.validatorDialog.tree.getRoot();
         if (root == null || root.getChildCount() == 0)
@@ -124 +123 @@
 
     @Override
-    public boolean isChanged() {
-        return updateCount != Main.map.validatorDialog.tree.getUpdateCount();
-    }
-
-    @Override
     public void visitBoundingBox(BoundingXYVisitor v) {
         // Do nothing
@@ -168 +162 @@
             e.scheduleRemoval(Collections.singleton(this));
         } else if (e.getRemovedLayer() == this) {
-            Main.getLayerManager().removeLayerChangeListener(this);
             OsmValidator.errorLayer = null;
         }
@@ -177 +170 @@
         return LayerPositionStrategy.IN_FRONT;
     }
+
+    @Override
+    public void destroy() {
+        Main.map.validatorDialog.tree.removeInvalidationListener(invalidator);
+        Main.getLayerManager().removeLayerChangeListener(this);
+        super.destroy();
+    }
 }

trunk/src/org/openstreetmap/josm/gui/preferences/validator/ValidatorPreference.java

@@ -40 +40 @@
 
     /** The preferences key for error layer */
-    public static final String PREF_LAYER = PREFIX + ".layer";
+    public static final BooleanProperty PREF_LAYER = new BooleanProperty(PREFIX + ".layer", true);
 
     /** The preferences key for enabled tests */
@@ -46 +46 @@
 
     /** The preferences key for enabled tests */
-    public static final String PREF_USE_IGNORE = PREFIX + ".ignore";
+    public static final BooleanProperty PREF_USE_IGNORE = new BooleanProperty(PREFIX + ".ignore", true);
 
     /** The preferences key for enabled tests before upload*/
@@ -52 +52 @@
 
     /** The preferences key for ignored severity other on upload */
-    public static final String PREF_OTHER_UPLOAD = PREFIX + ".otherUpload";
+    public static final BooleanProperty PREF_OTHER_UPLOAD = new BooleanProperty(PREFIX + ".otherUpload", false);
 
     /** The preferences for ignored severity other */

trunk/src/org/openstreetmap/josm/gui/preferences/validator/ValidatorTestsPreference.java

@@ -58 +58 @@
         testPanel.setBorder(BorderFactory.createEmptyBorder(5, 5, 5, 5));
 
-        prefUseIgnore = new JCheckBox(tr("Use ignore list."), Main.pref.getBoolean(ValidatorPreference.PREF_USE_IGNORE, true));
+        prefUseIgnore = new JCheckBox(tr("Use ignore list."), ValidatorPreference.PREF_USE_IGNORE.get());
         prefUseIgnore.setToolTipText(tr("Use the ignore list to suppress warnings."));
         testPanel.add(prefUseIgnore, GBC.eol());
 
-        prefUseLayer = new JCheckBox(tr("Use error layer."), Main.pref.getBoolean(ValidatorPreference.PREF_LAYER, true));
+        prefUseLayer = new JCheckBox(tr("Use error layer."), ValidatorPreference.PREF_LAYER.get());
         prefUseLayer.setToolTipText(tr("Use the error layer to display problematic elements."));
         testPanel.add(prefUseLayer, GBC.eol());
@@ -71 +71 @@
 
         prefOtherUpload = new JCheckBox(tr("Show informational level on upload."),
-                Main.pref.getBoolean(ValidatorPreference.PREF_OTHER_UPLOAD, false));
+                ValidatorPreference.PREF_OTHER_UPLOAD.get());
         prefOtherUpload.setToolTipText(tr("Show the informational tests in the upload check windows."));
         testPanel.add(prefOtherUpload, GBC.eol());
@@ -117 +117 @@
         Main.pref.putCollection(ValidatorPreference.PREF_SKIP_TESTS, tests);
         Main.pref.putCollection(ValidatorPreference.PREF_SKIP_TESTS_BEFORE_UPLOAD, testsBeforeUpload);
-        Main.pref.put(ValidatorPreference.PREF_USE_IGNORE, prefUseIgnore.isSelected());
+        ValidatorPreference.PREF_USE_IGNORE.put(prefUseIgnore.isSelected());
         ValidatorPreference.PREF_OTHER.put(prefOther.isSelected());
-        Main.pref.put(ValidatorPreference.PREF_OTHER_UPLOAD, prefOtherUpload.isSelected());
-        Main.pref.put(ValidatorPreference.PREF_LAYER, prefUseLayer.isSelected());
+        ValidatorPreference.PREF_OTHER_UPLOAD.put(prefOtherUpload.isSelected());
+        ValidatorPreference.PREF_LAYER.put(prefUseLayer.isSelected());
         return false;
     }