diff --git a/src/org/openstreetmap/josm/actions/ValidateAction.java b/src/org/openstreetmap/josm/actions/ValidateAction.java index 12febf6..4f2851f 100644 --- a/src/org/openstreetmap/josm/actions/ValidateAction.java +++ b/src/org/openstreetmap/josm/actions/ValidateAction.java @@ -168,7 +168,7 @@ public class ValidateAction extends JosmAction { errors.addAll(test.getErrors()); } tests = null; - if (Main.pref.getBoolean(ValidatorPreference.PREF_USE_IGNORE, true)) { + if (ValidatorPreference.PREF_USE_IGNORE.get()) { getProgressMonitor().subTask(tr("Updating ignored errors ...")); for (TestError error : errors) { if (canceled) return; diff --git a/src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java b/src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java index 2704956..6d32ccf 100644 --- a/src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java +++ b/src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java @@ -60,8 +60,7 @@ public class ValidateUploadHook implements UploadHook { test.startTest(null); test.visit(selection); test.endTest(); - if (ValidatorPreference.PREF_OTHER.get() && - Main.pref.getBoolean(ValidatorPreference.PREF_OTHER_UPLOAD, false)) { + if (ValidatorPreference.PREF_OTHER.get() && ValidatorPreference.PREF_OTHER_UPLOAD.get()) { errors.addAll(test.getErrors()); } else { for (TestError e : test.getErrors()) { @@ -82,7 +81,7 @@ public class ValidateUploadHook implements UploadHook { if (errors.isEmpty()) return true; - if (Main.pref.getBoolean(ValidatorPreference.PREF_USE_IGNORE, true)) { + if (ValidatorPreference.PREF_USE_IGNORE.get()) { int nume = 0; for (TestError error : errors) { List s = new ArrayList<>(); diff --git a/src/org/openstreetmap/josm/data/validation/OsmValidator.java b/src/org/openstreetmap/josm/data/validation/OsmValidator.java index fc4da6b..a25960d 100644 --- a/src/org/openstreetmap/josm/data/validation/OsmValidator.java +++ b/src/org/openstreetmap/josm/data/validation/OsmValidator.java @@ -182,7 +182,7 @@ public final class OsmValidator { private static void loadIgnoredErrors() { ignoredErrors.clear(); - if (Main.pref.getBoolean(ValidatorPreference.PREF_USE_IGNORE, true)) { + if (ValidatorPreference.PREF_USE_IGNORE.get()) { Path path = Paths.get(getValidatorDir()).resolve("ignorederrors"); if (Files.exists(path)) { try { @@ -216,7 +216,7 @@ public final class OsmValidator { } public static synchronized void initializeErrorLayer() { - if (!Main.pref.getBoolean(ValidatorPreference.PREF_LAYER, true)) + if (!ValidatorPreference.PREF_LAYER.get()) return; if (errorLayer == null) { errorLayer = new ValidatorLayer(); diff --git a/src/org/openstreetmap/josm/data/validation/TestError.java b/src/org/openstreetmap/josm/data/validation/TestError.java index d9d86a7..9379629 100644 --- a/src/org/openstreetmap/josm/data/validation/TestError.java +++ b/src/org/openstreetmap/josm/data/validation/TestError.java @@ -268,6 +268,10 @@ public class TestError implements Comparable, DataSetListener { return tester; } + /** + * Set the tester that raised the error. + * @param tester te tester + */ public void setTester(Test tester) { this.tester = tester; } diff --git a/src/org/openstreetmap/josm/gui/dialogs/ValidatorDialog.java b/src/org/openstreetmap/josm/gui/dialogs/ValidatorDialog.java index a3e121a..7ae55df 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/ValidatorDialog.java +++ b/src/org/openstreetmap/josm/gui/dialogs/ValidatorDialog.java @@ -150,7 +150,7 @@ public class ValidatorDialog extends ToggleDialog implements SelectionChangedLis fixButton.setEnabled(false); buttons.add(fixButton); - if (Main.pref.getBoolean(ValidatorPreference.PREF_USE_IGNORE, true)) { + if (ValidatorPreference.PREF_USE_IGNORE.get()) { ignoreButton = new SideButton(new AbstractAction() { { putValue(NAME, tr("Ignore")); @@ -192,7 +192,6 @@ public class ValidatorDialog extends ToggleDialog implements SelectionChangedLis tree.setVisible(v); } super.setVisible(v); - Main.map.repaint(); } /** diff --git a/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanel.java b/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanel.java index fd212c8..a399bdf 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanel.java +++ b/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanel.java @@ -10,13 +10,13 @@ import java.util.Collection; import java.util.Collections; import java.util.EnumMap; import java.util.Enumeration; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; import java.util.function.Predicate; +import java.util.stream.Collectors; import javax.swing.JTree; import javax.swing.ToolTipManager; @@ -35,7 +35,7 @@ import org.openstreetmap.josm.data.validation.util.MultipleNameVisitor; import org.openstreetmap.josm.gui.preferences.validator.ValidatorPreference; 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; /** * A panel that displays the error tree. The selection manager @@ -72,8 +72,7 @@ public class ValidatorTreePanel extends JTree implements Destroyable { */ private transient Set filter; - /** a counter to check if tree has been rebuild */ - private int updateCount; + private final ListenerList invalidationListeners = ListenerList.create(); /** * Constructor @@ -134,13 +133,13 @@ public class ValidatorTreePanel extends JTree implements Destroyable { valTreeModel.setRoot(new DefaultMutableTreeNode()); } super.setVisible(v); + invalidationListeners.fireEvent(Runnable::run); } /** * Builds the errors tree */ public void buildTree() { - updateCount++; final DefaultMutableTreeNode rootNode = new DefaultMutableTreeNode(); if (errors == null || errors.isEmpty()) { @@ -171,56 +170,21 @@ public class ValidatorTreePanel extends JTree implements Destroyable { } } - Map> errorTree = new EnumMap<>(Severity.class); - Map>> errorTreeDeep = new EnumMap<>(Severity.class); - for (Severity s : Severity.values()) { - errorTree.put(s, new MultiMap(20)); - errorTreeDeep.put(s, new HashMap>()); + Predicate filterToUse = e -> !e.isIgnored(); + if (!ValidatorPreference.PREF_OTHER.get()) { + filterToUse = filterToUse.and(e -> e.getSeverity() != Severity.OTHER); } - - 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 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); - } + if (filter != null) { + filterToUse = filterToUse.and(e -> e.getPrimitives().stream().anyMatch(filter::contains)); } + Map>>> 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 expandedPaths = new ArrayList<>(); - for (Severity s : Severity.values()) { - MultiMap severityErrors = errorTree.get(s); - Map> severityErrorsDeep = errorTreeDeep.get(s); - if (severityErrors.isEmpty() && severityErrorsDeep.isEmpty()) { - continue; - } - + errorTreeDeep.forEach((s, severityErrorsDeep) -> { // Severity node DefaultMutableTreeNode severityNode = new GroupTreeNode(s); rootNode.add(severityNode); @@ -229,43 +193,46 @@ public class ValidatorTreePanel extends JTree implements Destroyable { expandedPaths.add(new TreePath(new Object[] {rootNode, severityNode})); } - for (Entry> msgErrors : severityErrors.entrySet()) { - // Message node - Set errs = msgErrors.getValue(); - String msg = tr("{0} ({1})", msgErrors.getKey(), errs.size()); - DefaultMutableTreeNode messageNode = new DefaultMutableTreeNode(msg); - severityNode.add(messageNode); + Map> severityErrors = severityErrorsDeep.get(""); + if (severityErrors != null) { + for (Entry> msgErrors : severityErrors.entrySet()) { + // Message node + List 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})); - } + 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); + errs.stream().map(DefaultMutableTreeNode::new).forEach(messageNode::add); } } - for (Entry> bag : severityErrorsDeep.entrySet()) { + + severityErrorsDeep.forEach((description, errorlist) -> { + if (description.isEmpty()) { + return; + } // Group node - MultiMap 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})); } + } else { + groupNode = null; } - for (Entry> msgErrors : errorlist.entrySet()) { + errorlist.forEach((message, errs) -> { // Message node - Set 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); if (groupNode != null) { @@ -274,7 +241,7 @@ public class ValidatorTreePanel extends JTree implements Destroyable { severityNode.add(messageNode); } - if (oldSelectedRows.contains(msgErrors.getKey())) { + if (oldSelectedRows.contains(message)) { if (groupNode != null) { expandedPaths.add(new TreePath(new Object[] {rootNode, severityNode, groupNode, messageNode})); } else { @@ -282,19 +249,33 @@ public class ValidatorTreePanel extends JTree implements Destroyable { } } - 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); for (TreePath path : expandedPaths) { 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 + */ + public void removeInvalidationListener(Runnable listener) { + invalidationListeners.removeListener(listener); } /** @@ -425,14 +406,6 @@ public class ValidatorTreePanel extends JTree implements Destroyable { return (DefaultMutableTreeNode) valTreeModel.getRoot(); } - /** - * 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) { DataSet ds = Main.getLayerManager().getEditDataSet(); diff --git a/src/org/openstreetmap/josm/gui/layer/ValidatorLayer.java b/src/org/openstreetmap/josm/gui/layer/ValidatorLayer.java index 4044d99..cc52120 100644 --- a/src/org/openstreetmap/josm/gui/layer/ValidatorLayer.java +++ b/src/org/openstreetmap/josm/gui/layer/ValidatorLayer.java @@ -40,8 +40,7 @@ import org.openstreetmap.josm.tools.MultiMap; * @since 10386 (new LayerChangeListener interface) */ public class ValidatorLayer extends Layer implements LayerChangeListener { - - private int updateCount = -1; + private final Runnable invalidator = this::invalidate; /** * Constructs a new Validator layer @@ -49,6 +48,7 @@ public class ValidatorLayer extends Layer implements LayerChangeListener { public ValidatorLayer() { super(tr("Validation errors")); Main.getLayerManager().addLayerChangeListener(this); + Main.map.validatorDialog.tree.addInvalidationListener(invalidator); } /** @@ -67,7 +67,6 @@ public class ValidatorLayer extends Layer implements LayerChangeListener { @SuppressWarnings("unchecked") @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) return; @@ -123,11 +122,6 @@ public class ValidatorLayer extends Layer implements LayerChangeListener { } @Override - public boolean isChanged() { - return updateCount != Main.map.validatorDialog.tree.getUpdateCount(); - } - - @Override public void visitBoundingBox(BoundingXYVisitor v) { // Do nothing } @@ -167,7 +161,6 @@ public class ValidatorLayer extends Layer implements LayerChangeListener { if (e.getRemovedLayer() instanceof OsmDataLayer && e.getSource().getLayersOfType(OsmDataLayer.class).size() <= 1) { e.scheduleRemoval(Collections.singleton(this)); } else if (e.getRemovedLayer() == this) { - Main.getLayerManager().removeLayerChangeListener(this); OsmValidator.errorLayer = null; } } @@ -176,4 +169,11 @@ public class ValidatorLayer extends Layer implements LayerChangeListener { public LayerPositionStrategy getDefaultLayerPosition() { return LayerPositionStrategy.IN_FRONT; } + + @Override + public void destroy() { + Main.map.validatorDialog.tree.removeInvalidationListener(invalidator); + Main.getLayerManager().removeLayerChangeListener(this); + super.destroy(); + } } diff --git a/src/org/openstreetmap/josm/gui/preferences/validator/ValidatorPreference.java b/src/org/openstreetmap/josm/gui/preferences/validator/ValidatorPreference.java index f647d56..24ec975 100644 --- a/src/org/openstreetmap/josm/gui/preferences/validator/ValidatorPreference.java +++ b/src/org/openstreetmap/josm/gui/preferences/validator/ValidatorPreference.java @@ -39,19 +39,19 @@ public final class ValidatorPreference extends DefaultTabPreferenceSetting { public static final String PREFIX = "validator"; /** 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 */ public static final String PREF_SKIP_TESTS = PREFIX + ".skip"; /** 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*/ public static final String PREF_SKIP_TESTS_BEFORE_UPLOAD = PREFIX + ".skipBeforeUpload"; /** 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 */ public static final BooleanProperty PREF_OTHER = new BooleanProperty(PREFIX + ".other", false); diff --git a/src/org/openstreetmap/josm/gui/preferences/validator/ValidatorTestsPreference.java b/src/org/openstreetmap/josm/gui/preferences/validator/ValidatorTestsPreference.java index 67b1a7e..3a4128a 100644 --- a/src/org/openstreetmap/josm/gui/preferences/validator/ValidatorTestsPreference.java +++ b/src/org/openstreetmap/josm/gui/preferences/validator/ValidatorTestsPreference.java @@ -57,11 +57,11 @@ public class ValidatorTestsPreference implements SubPreferenceSetting { JPanel testPanel = new VerticallyScrollablePanel(new GridBagLayout()); 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()); @@ -70,7 +70,7 @@ public class ValidatorTestsPreference implements SubPreferenceSetting { testPanel.add(prefOther, GBC.eol()); 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()); @@ -116,10 +116,10 @@ public class ValidatorTestsPreference implements SubPreferenceSetting { 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; } diff --git a/test/unit/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanelTest.java b/test/unit/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanelTest.java index 58fc137..abf7c21 100644 --- a/test/unit/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanelTest.java +++ b/test/unit/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanelTest.java @@ -43,7 +43,6 @@ public class ValidatorTreePanelTest { new TestError(null, Severity.ERROR, "err", 0, new Node(1)), new TestError(null, Severity.WARNING, "warn", 0, new Node(2))))); assertNotNull(vtp); - assertEquals(1, vtp.getUpdateCount()); assertEquals(2, vtp.getErrors().size()); vtp.setVisible(true); vtp.setVisible(false);