commit fcd8204971c64c1d217ae7a946e254d50b1b5332 Author: Simon Legner Date: 2020-02-05 22:06:43 +0100 fix #18679 diff --git a/src/org/openstreetmap/josm/gui/autofilter/AutoFilterManager.java b/src/org/openstreetmap/josm/gui/autofilter/AutoFilterManager.java index c5e1fd9f6..2c16604eb 100644 --- a/src/org/openstreetmap/josm/gui/autofilter/AutoFilterManager.java +++ b/src/org/openstreetmap/josm/gui/autofilter/AutoFilterManager.java @@ -7,20 +7,17 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Comparator; -import java.util.Iterator; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.NavigableSet; import java.util.Objects; -import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.IntStream; -import java.util.stream.Stream; import org.openstreetmap.josm.actions.mapmode.MapMode; import org.openstreetmap.josm.data.osm.BBox; @@ -93,7 +90,7 @@ /** * The buttons currently displayed in map view. */ - private final Map buttons = new TreeMap<>(); + private final Map buttons = new TreeMap<>(); /** * The list of registered auto filter rules. @@ -145,7 +142,7 @@ private synchronized void updateButtons() { if (enabledRule != null && map != null && enabledRule.getMinZoomLevel() <= Selector.GeneralSelector.scale2level(map.mapView.getDist100Pixel())) { // Retrieve the values from current rule visible on screen - NavigableSet values = getNumericValues(enabledRule.getKey(), enabledRule.getValueComparator()); + NavigableSet values = getNumericValues(enabledRule.getKey()); // Make sure current auto filter button remains visible even if no data is found, to allow user to disable it if (currentAutoFilter != null) { values.add(currentAutoFilter.getFilter().value); @@ -157,11 +154,11 @@ private synchronized void updateButtons() { } } - static class CompiledFilter extends Filter implements MatchSupplier { + class CompiledFilter extends Filter implements MatchSupplier { final String key; - final String value; + final int value; - CompiledFilter(String key, String value) { + CompiledFilter(String key, int value) { this.key = key; this.value = value; this.enable = true; @@ -174,21 +171,21 @@ private synchronized void updateButtons() { return new SearchCompiler.Match() { @Override public boolean match(OsmPrimitive osm) { - return getTagValuesForPrimitive(key, osm).anyMatch(value::equals); + return getTagValuesForPrimitive(key, osm).anyMatch(v -> v == value); } }; } } - private synchronized void addNewButtons(NavigableSet values) { + private synchronized void addNewButtons(NavigableSet values) { if (values.isEmpty()) { return; } int i = 0; int maxWidth = 16; final AutoFilterButton keyButton = AutoFilterButton.forOsmKey(enabledRule.getKey()); - addButton(keyButton, Integer.toString(Integer.MIN_VALUE), i++); - for (final String value : values.descendingSet()) { + addButton(keyButton, Integer.MIN_VALUE, i++); + for (final Integer value : values.descendingSet()) { CompiledFilter filter = new CompiledFilter(enabledRule.getKey(), value); String label = enabledRule.getValueFormatter().apply(value); AutoFilter autoFilter = new AutoFilter(label, filter.text, filter); @@ -205,71 +202,52 @@ private synchronized void addNewButtons(NavigableSet values) { MainApplication.getMap().mapView.validate(); } - private void addButton(AutoFilterButton button, String value, int i) { + private void addButton(AutoFilterButton button, int value, int i) { MapView mapView = MainApplication.getMap().mapView; buttons.put(value, button); mapView.add(button).setLocation(3, 60 + 22*i); } private void removeAllButtons() { - for (Iterator it = buttons.keySet().iterator(); it.hasNext();) { - MainApplication.getMap().mapView.remove(buttons.get(it.next())); - it.remove(); - } + buttons.values().forEach(MainApplication.getMap().mapView::remove); + buttons.clear(); } - private static NavigableSet getNumericValues(String key, Comparator comparator) { - NavigableSet values = new TreeSet<>(comparator); - for (String s : getTagValues(key)) { - try { - Integer.parseInt(s); - values.add(s); - } catch (NumberFormatException e) { - Logging.trace(e); - } - } - return values; - } - - private static Set getTagValues(String key) { + private NavigableSet getNumericValues(String key) { DataSet ds = MainApplication.getLayerManager().getActiveDataSet(); - Set values = new TreeSet<>(); - if (ds != null) { - BBox bbox = MainApplication.getMap().mapView.getState().getViewArea().getLatLonBoundsBox().toBBox(); - Consumer consumer = o -> getTagValuesForPrimitive(key, o).forEach(values::add); - ds.searchNodes(bbox).forEach(consumer); - ds.searchWays(bbox).forEach(consumer); - ds.searchRelations(bbox).forEach(consumer); + if (ds == null) { + return Collections.emptyNavigableSet(); } + BBox bbox = MainApplication.getMap().mapView.getState().getViewArea().getLatLonBoundsBox().toBBox(); + NavigableSet values = new TreeSet<>(); + Consumer consumer = o -> getTagValuesForPrimitive(key, o).forEach(values::add); + ds.searchNodes(bbox).forEach(consumer); + ds.searchWays(bbox).forEach(consumer); + ds.searchRelations(bbox).forEach(consumer); return values; } - static Stream getTagValuesForPrimitive(String key, OsmPrimitive osm) { + private IntStream getTagValuesForPrimitive(String key, OsmPrimitive osm) { String value = osm.get(key); if (value != null) { Pattern p = Pattern.compile("(-?[0-9]+)-(-?[0-9]+)"); - return OsmUtils.splitMultipleValues(value).flatMap(v -> { + return OsmUtils.splitMultipleValues(value).flatMapToInt(v -> { Matcher m = p.matcher(v); if (m.matches()) { int a = Integer.parseInt(m.group(1)); int b = Integer.parseInt(m.group(2)); - return IntStream.rangeClosed(Math.min(a, b), Math.max(a, b)) - .mapToObj(Integer::toString); + return IntStream.rangeClosed(Math.min(a, b), Math.max(a, b)); } else { - return Stream.of(v); + try { + return IntStream.of(enabledRule.getValueExtractor().applyAsInt(v)); + } catch (NumberFormatException e) { + Logging.trace(e); + return IntStream.empty(); + } } }); - } else if (PROP_AUTO_FILTER_DEFAULTS.get() && "layer".equals(key)) { - // assume sensible defaults, see #17496 - if (osm.hasTag("bridge") || osm.hasTag("power", "line") || osm.hasTag("location", "overhead")) { - return Stream.of("1"); - } else if (osm.isKeyTrue("tunnel") || osm.hasTag("tunnel", "culvert") || osm.hasTag("location", "underground")) { - return Stream.of("-1"); - } else if (osm.hasTag("tunnel", "building_passage") || osm.hasKey("highway", "railway", "waterway")) { - return Stream.of("0"); - } } - return Stream.empty(); + return enabledRule.getExtractDefaultValues().apply(osm); } @Override diff --git a/src/org/openstreetmap/josm/gui/autofilter/AutoFilterRule.java b/src/org/openstreetmap/josm/gui/autofilter/AutoFilterRule.java index 858e772e4..c45d3b571 100644 --- a/src/org/openstreetmap/josm/gui/autofilter/AutoFilterRule.java +++ b/src/org/openstreetmap/josm/gui/autofilter/AutoFilterRule.java @@ -1,10 +1,13 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.gui.autofilter; -import java.util.Comparator; import java.util.Objects; import java.util.function.Function; -import java.util.function.UnaryOperator; +import java.util.function.IntFunction; +import java.util.function.ToIntFunction; +import java.util.stream.IntStream; + +import org.openstreetmap.josm.data.osm.OsmPrimitive; /** * An auto filter rule determines how auto filter can be built from visible map data. @@ -19,9 +22,11 @@ private final int minZoomLevel; - private UnaryOperator valueFormatter = s -> s; + private ToIntFunction valueExtractor = Integer::parseInt; + + private IntFunction valueFormatter = Integer::toString; - private Comparator valueComparator = Comparator.comparingInt(s -> Integer.parseInt(valueFormatter.apply(s))); + private Function extractDefaultValues = p -> IntStream.empty(); /** * Constructs a new {@code AutoFilterRule}. @@ -53,7 +58,7 @@ public int getMinZoomLevel() { * Returns the OSM value formatter that defines the associated button label. * @return the OSM value formatter that defines the associated button label (identity by default) */ - public Function getValueFormatter() { + public IntFunction getValueFormatter() { return valueFormatter; } @@ -63,27 +68,26 @@ public int getMinZoomLevel() { * @return {@code this} * @throws NullPointerException if {@code valueFormatter} is null */ - public AutoFilterRule setValueFormatter(UnaryOperator valueFormatter) { + public AutoFilterRule setValueFormatter(IntFunction valueFormatter) { this.valueFormatter = Objects.requireNonNull(valueFormatter); return this; } - /** - * Returns the OSM value comparator used to order the buttons. - * @return the OSM value comparator - */ - public Comparator getValueComparator() { - return valueComparator; + public ToIntFunction getValueExtractor() { + return valueExtractor; } - /** - * Sets the OSM value comparator used to order the buttons. - * @param valueComparator the OSM value comparator - * @return {@code this} - * @throws NullPointerException if {@code valueComparator} is null - */ - public AutoFilterRule setValueComparator(Comparator valueComparator) { - this.valueComparator = valueComparator; + public AutoFilterRule setValueExtractor(ToIntFunction valueExtractor) { + this.valueExtractor = valueExtractor; + return this; + } + + public Function getExtractDefaultValues() { + return extractDefaultValues; + } + + public AutoFilterRule setExtractDefaultValues(Function extractDefaultValues) { + this.extractDefaultValues = extractDefaultValues; return this; } @@ -92,17 +96,38 @@ public AutoFilterRule setValueComparator(Comparator valueComparator) { * @return the default list of auto filter rules */ public static AutoFilterRule[] defaultRules() { - return new AutoFilterRule[] { + return new AutoFilterRule[]{ new AutoFilterRule("level", 17), - new AutoFilterRule("layer", 16), + new AutoFilterRule("layer", 16) + .setExtractDefaultValues(AutoFilterRule::defaultLayer), new AutoFilterRule("maxspeed", 16) - .setValueFormatter(s -> s.replace(" mph", "")), + .setValueExtractor(s -> Integer.parseInt(s.replace(" mph", ""))), new AutoFilterRule("voltage", 5) - .setValueFormatter(s -> s.replaceAll("000$", "k") + 'V') - .setValueComparator(Comparator.comparingInt(Integer::parseInt)) + .setValueFormatter(s -> s % 1000 == 0 ? (s / 1000) + "kV" : s + "V"), + new AutoFilterRule("building:levels", 17), + new AutoFilterRule("gauge", 5), + new AutoFilterRule("frequency", 5), + new AutoFilterRule("incline", 13) + .setValueExtractor(s -> Integer.parseInt(s.replaceAll("%$", ""))) + .setValueFormatter(v -> v + "\u2009%"), + new AutoFilterRule("lanes", 13), + new AutoFilterRule("admin_level", 11) }; } + private static IntStream defaultLayer(OsmPrimitive osm) { + // assume sensible defaults, see #17496 + if (osm.hasTag("bridge") || osm.hasTag("power", "line") || osm.hasTag("location", "overhead")) { + return IntStream.of(1); + } else if (osm.isKeyTrue("tunnel") || osm.hasTag("tunnel", "culvert") || osm.hasTag("location", "underground")) { + return IntStream.of(-1); + } else if (osm.hasTag("tunnel", "building_passage") || osm.hasKey("highway", "railway", "waterway")) { + return IntStream.of(0); + } else { + return IntStream.empty(); + } + } + @Override public String toString() { return key + '[' + minZoomLevel + ']'; diff --git a/test/unit/org/openstreetmap/josm/gui/autofilter/AutoFilterManagerTest.java b/test/unit/org/openstreetmap/josm/gui/autofilter/AutoFilterManagerTest.java index dc9fbf9aa..2b26dfe0b 100644 --- a/test/unit/org/openstreetmap/josm/gui/autofilter/AutoFilterManagerTest.java +++ b/test/unit/org/openstreetmap/josm/gui/autofilter/AutoFilterManagerTest.java @@ -1,70 +1,8 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.gui.autofilter; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; - -import java.util.Arrays; -import java.util.TreeSet; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -import org.junit.Rule; -import org.junit.Test; -import org.openstreetmap.josm.data.osm.OsmUtils; -import org.openstreetmap.josm.testutils.JOSMTestRules; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - /** * Unit tests of {@link AutoFilterManager} class. */ public class AutoFilterManagerTest { - - /** - * Setup tests - */ - @Rule - @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") - public JOSMTestRules test = new JOSMTestRules(); - - /** - * Unit test of {@link AutoFilterManager#getTagValuesForPrimitive}. - */ - @Test - public void testTagValuesForPrimitive() { - final TreeSet values = Stream.of( - OsmUtils.createPrimitive("way level=-4--5"), - OsmUtils.createPrimitive("way level=-2"), - OsmUtils.createPrimitive("node level=0"), - OsmUtils.createPrimitive("way level=1"), - OsmUtils.createPrimitive("way level=2;3"), - OsmUtils.createPrimitive("way level=6-9"), - OsmUtils.createPrimitive("way level=10;12-13")) - .flatMap(o -> AutoFilterManager.getTagValuesForPrimitive("level", o)) - .collect(Collectors.toCollection(TreeSet::new)); - assertEquals(new TreeSet<>(Arrays.asList("-5", "-4", "-2", "0", "1", "2", "3", "6", "7", "8", "9", "10", "12", "13")), values); - - } - - /** - * Unit test of {@link AutoFilterManager#getTagValuesForPrimitive} provides sensible defaults, see #17496. - */ - @Test - public void testTagValuesForPrimitivesDefaults() { - assertNull(getLayer("way foo=bar")); - assertEquals("1", getLayer("way bridge=yes")); - assertEquals("1", getLayer("way power=line")); - assertEquals("-1", getLayer("way tunnel=yes")); - assertEquals("0", getLayer("way tunnel=building_passage")); - assertEquals("0", getLayer("way highway=residential")); - assertEquals("0", getLayer("way railway=rail")); - assertEquals("0", getLayer("way waterway=canal")); - } - - private String getLayer(final String assertion) { - return AutoFilterManager.getTagValuesForPrimitive("layer", OsmUtils.createPrimitive(assertion)) - .findFirst() - .orElse(null); - } }