Index: /trunk/src/org/openstreetmap/josm/gui/autofilter/AutoFilterManager.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/autofilter/AutoFilterManager.java (revision 15838) +++ /trunk/src/org/openstreetmap/josm/gui/autofilter/AutoFilterManager.java (revision 15839) @@ -16,7 +16,4 @@ 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 org.openstreetmap.josm.actions.mapmode.MapMode; @@ -26,5 +23,4 @@ import org.openstreetmap.josm.data.osm.FilterModel; import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.data.osm.OsmUtils; import org.openstreetmap.josm.data.osm.event.AbstractDatasetChangedEvent; import org.openstreetmap.josm.data.osm.event.DataChangedEvent; @@ -58,5 +54,4 @@ import org.openstreetmap.josm.spi.preferences.PreferenceChangeEvent; import org.openstreetmap.josm.spi.preferences.PreferenceChangedListener; -import org.openstreetmap.josm.tools.Logging; /** @@ -79,9 +74,4 @@ /** - * Property to determine if the auto filter should assume sensible defaults for values (such as layer=1 for bridge=yes). - */ - private static final BooleanProperty PROP_AUTO_FILTER_DEFAULTS = new BooleanProperty("auto.filter.defaults", true); - - /** * The unique instance. */ @@ -111,5 +101,5 @@ * The currently enabled rule, if any. */ - private AutoFilterRule enabledRule; + AutoFilterRule enabledRule; /** @@ -143,5 +133,5 @@ && enabledRule.getMinZoomLevel() <= Selector.GeneralSelector.scale2level(map.mapView.getDist100Pixel())) { // Retrieve the values from current rule visible on screen - NavigableSet values = getNumericValues(enabledRule.getKey()); + NavigableSet values = getNumericValues(); // Make sure current auto filter button remains visible even if no data is found, to allow user to disable it if (currentAutoFilter != null) { @@ -155,14 +145,14 @@ } - class CompiledFilter extends Filter implements MatchSupplier { - final String key; + static class CompiledFilter extends Filter implements MatchSupplier { + final AutoFilterRule rule; final int value; - CompiledFilter(String key, int value) { - this.key = key; + CompiledFilter(AutoFilterRule rule, int value) { + this.rule = rule; this.value = value; this.enable = true; this.inverted = true; - this.text = key + "=" + value; + this.text = rule.getKey() + "=" + value; } @@ -172,5 +162,5 @@ @Override public boolean match(OsmPrimitive osm) { - return getTagValuesForPrimitive(key, osm).anyMatch(v -> v == value); + return rule.getTagValuesForPrimitive(osm).anyMatch(v -> v == value); } }; @@ -187,6 +177,6 @@ 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); + CompiledFilter filter = new CompiledFilter(enabledRule, value); + String label = enabledRule.formatValue(value); AutoFilter autoFilter = new AutoFilter(label, filter.text, filter); AutoFilterButton button = new AutoFilterButton(autoFilter); @@ -214,5 +204,5 @@ } - private NavigableSet getNumericValues(String key) { + private NavigableSet getNumericValues() { DataSet ds = MainApplication.getLayerManager().getActiveDataSet(); if (ds == null) { @@ -221,35 +211,9 @@ BBox bbox = MainApplication.getMap().mapView.getState().getViewArea().getLatLonBoundsBox().toBBox(); NavigableSet values = new TreeSet<>(); - Consumer consumer = o -> getTagValuesForPrimitive(key, o).forEach(values::add); + Consumer consumer = o -> enabledRule.getTagValuesForPrimitive(o).forEach(values::add); ds.searchNodes(bbox).forEach(consumer); ds.searchWays(bbox).forEach(consumer); ds.searchRelations(bbox).forEach(consumer); return values; - } - - protected IntStream getTagValuesForPrimitive(String key, OsmPrimitive osm) { - if (enabledRule == null) { - return IntStream.empty(); - } - String value = osm.get(key); - if (value != null) { - Pattern p = Pattern.compile("(-?[0-9]+)-(-?[0-9]+)"); - 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)); - } else { - try { - return IntStream.of(enabledRule.getValueExtractor().applyAsInt(v)); - } catch (NumberFormatException e) { - Logging.trace(e); - return IntStream.empty(); - } - } - }); - } - return enabledRule.getDefaultValueSupplier().apply(osm); } Index: /trunk/src/org/openstreetmap/josm/gui/autofilter/AutoFilterRule.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/autofilter/AutoFilterRule.java (revision 15838) +++ /trunk/src/org/openstreetmap/josm/gui/autofilter/AutoFilterRule.java (revision 15839) @@ -8,7 +8,12 @@ import java.util.function.IntFunction; import java.util.function.ToIntFunction; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.IntStream; import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.OsmUtils; +import org.openstreetmap.josm.data.preferences.BooleanProperty; +import org.openstreetmap.josm.tools.Logging; /** @@ -20,4 +25,9 @@ */ public class AutoFilterRule { + + /** + * Property to determine if the auto filter should assume sensible defaults for values (such as layer=1 for bridge=yes). + */ + private static final BooleanProperty PROP_AUTO_FILTER_DEFAULTS = new BooleanProperty("auto.filter.defaults", true); private final String key; @@ -58,9 +68,10 @@ /** - * 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) + * Formats the numeric value + * @param value the numeric value to format + * @return the formatted value */ - public IntFunction getValueFormatter() { - return valueFormatter; + public String formatValue(int value) { + return valueFormatter.apply(value); } @@ -74,12 +85,4 @@ this.valueFormatter = Objects.requireNonNull(valueFormatter); return this; - } - - /** - * Returns a function which yields default values for the given OSM primitive - * @return a function which yields default values for the given OSM primitive - */ - public Function getDefaultValueSupplier() { - return defaultValueSupplier; } @@ -97,12 +100,4 @@ /** - * Returns a function which extracts a numeric value from an OSM value - * @return a function which extracts a numeric value from an OSM value - */ - public ToIntFunction getValueExtractor() { - return valueExtractor; - } - - /** * Sets the function which extracts a numeric value from an OSM value * @param valueExtractor the function which extracts a numeric value from an OSM value @@ -113,4 +108,32 @@ this.valueExtractor = Objects.requireNonNull(valueExtractor); return this; + } + + /** + * Returns the numeric values for the given OSM primitive + * @param osm the primitive + * @return a stream of numeric values + */ + public IntStream getTagValuesForPrimitive(OsmPrimitive osm) { + String value = osm.get(key); + if (value != null) { + Pattern p = Pattern.compile("(-?[0-9]+)-(-?[0-9]+)"); + 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)); + } else { + try { + return IntStream.of(valueExtractor.applyAsInt(v)); + } catch (NumberFormatException e) { + Logging.trace(e); + return IntStream.empty(); + } + } + }); + } + return PROP_AUTO_FILTER_DEFAULTS.get() ? defaultValueSupplier.apply(osm) : IntStream.empty(); } Index: unk/test/unit/org/openstreetmap/josm/gui/autofilter/AutoFilterManagerTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/autofilter/AutoFilterManagerTest.java (revision 15838) +++ (revision ) @@ -1,75 +1,0 @@ -// License: GPL. For details, see LICENSE file. -package org.openstreetmap.josm.gui.autofilter; - -import static org.junit.Assert.assertEquals; - -import java.util.Arrays; -import java.util.OptionalInt; -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() { - AutoFilterManager.getInstance().setCurrentAutoFilter(null); - 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")) - .flatMapToInt(o -> AutoFilterManager.getInstance().getTagValuesForPrimitive("level", o)) - .boxed() - .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() { - AutoFilterManager.getInstance().setCurrentAutoFilter(null); - assertEquals(OptionalInt.empty(), getLayer("way foo=bar")); - AutoFilterRule.getDefaultRule("layer").ifPresent(AutoFilterManager.getInstance()::enableAutoFilterRule); - assertEquals(OptionalInt.empty(), getLayer("way foo=bar")); - assertEquals(OptionalInt.of(1), getLayer("way bridge=yes")); - assertEquals(OptionalInt.of(1), getLayer("way power=line")); - assertEquals(OptionalInt.of(-1), getLayer("way tunnel=yes")); - assertEquals(OptionalInt.of(0), getLayer("way tunnel=building_passage")); - assertEquals(OptionalInt.of(0), getLayer("way highway=residential")); - assertEquals(OptionalInt.of(0), getLayer("way railway=rail")); - assertEquals(OptionalInt.of(0), getLayer("way waterway=canal")); - } - - private OptionalInt getLayer(final String assertion) { - return AutoFilterManager.getInstance() - .getTagValuesForPrimitive("layer", OsmUtils.createPrimitive(assertion)) - .findFirst(); - } -} Index: /trunk/test/unit/org/openstreetmap/josm/gui/autofilter/AutoFilterRuleTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/autofilter/AutoFilterRuleTest.java (revision 15839) +++ /trunk/test/unit/org/openstreetmap/josm/gui/autofilter/AutoFilterRuleTest.java (revision 15839) @@ -0,0 +1,88 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.gui.autofilter; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; + +import java.util.NoSuchElementException; + +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.OsmUtils; +import org.openstreetmap.josm.testutils.JOSMTestRules; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +/** + * Unit tests of {@link AutoFilterRule} class. + */ +public class AutoFilterRuleTest { + + /** + * Setup tests + */ + @Rule + @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") + public JOSMTestRules test = new JOSMTestRules(); + + /** + * Unit test of {@link AutoFilterRule#getTagValuesForPrimitive}. + */ + @Test + public void testTagValuesForPrimitive() { + final AutoFilterRule level = AutoFilterRule.getDefaultRule("level").orElseThrow(NoSuchElementException::new); + assertTagValuesForPrimitive(level, "way level=-4--5", -5, -4); + assertTagValuesForPrimitive(level, "way level=-2", -2); + assertTagValuesForPrimitive(level, "node level=0", 0); + assertTagValuesForPrimitive(level, "way level=1", 1); + assertTagValuesForPrimitive(level, "way level=2;3", 2, 3); + assertTagValuesForPrimitive(level, "way level=6-9", 6, 7, 8, 9); + assertTagValuesForPrimitive(level, "way level=10;12-13", 10, 12, 13); + } + + /** + * Unit test of {@link AutoFilterRule#getTagValuesForPrimitive} to deal with {@code %} of key {@code incline}. + */ + @Test + public void testTagValuesForPrimitiveInclineUnit() { + final AutoFilterRule incline = AutoFilterRule.getDefaultRule("incline").orElseThrow(NoSuchElementException::new); + assertTagValuesForPrimitive(incline, "way incline=up"); + assertTagValuesForPrimitive(incline, "way incline=20", 20); + assertTagValuesForPrimitive(incline, "way incline=20%", 20); + } + + /** + * Unit test of {@link AutoFilterRule#getTagValuesForPrimitive} provides sensible defaults, see #17496. + */ + @Test + public void testTagValuesForPrimitivesDefaults() { + final AutoFilterRule layer = AutoFilterRule.getDefaultRule("layer").orElseThrow(NoSuchElementException::new); + assertTagValuesForPrimitive(layer, "way foo=bar"); + assertTagValuesForPrimitive(layer, "way bridge=yes", 1); + assertTagValuesForPrimitive(layer, "way power=line", 1); + assertTagValuesForPrimitive(layer, "way tunnel=yes", -1); + assertTagValuesForPrimitive(layer, "way tunnel=building_passage", 0); + assertTagValuesForPrimitive(layer, "way highway=residential", 0); + assertTagValuesForPrimitive(layer, "way railway=rail", 0); + assertTagValuesForPrimitive(layer, "way waterway=canal", 0); + } + + private void assertTagValuesForPrimitive(AutoFilterRule rule, String assertion, int... expected) { + final OsmPrimitive primitive = OsmUtils.createPrimitive(assertion); + final int[] actual = rule.getTagValuesForPrimitive(primitive).toArray(); + assertArrayEquals(expected, actual); + } + + /** + * Unit test of {@link AutoFilterRule#formatValue} + */ + @Test + public void testValueFormatter() { + final AutoFilterRule voltage = AutoFilterRule.getDefaultRule("voltage").orElseThrow(NoSuchElementException::new); + assertEquals("230V", voltage.formatValue(230)); + assertEquals("1kV", voltage.formatValue(1000)); + assertEquals("15kV", voltage.formatValue(15000)); + assertEquals("380kV", voltage.formatValue(380000)); + } +}