Index: /trunk/data_nodist/filterTests.osm =================================================================== --- /trunk/data_nodist/filterTests.osm (revision 3366) +++ /trunk/data_nodist/filterTests.osm (revision 3367) @@ -37,5 +37,5 @@ - + @@ -243,4 +243,5 @@ + @@ -258,4 +259,5 @@ + @@ -272,4 +274,5 @@ + Index: /trunk/src/org/openstreetmap/josm/data/osm/FilterMatcher.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/FilterMatcher.java (revision 3366) +++ /trunk/src/org/openstreetmap/josm/data/osm/FilterMatcher.java (revision 3367) @@ -17,4 +17,5 @@ final Match match; final boolean isDelete; + final boolean isInverted; FilterInfo(Filter filter) throws ParseError { @@ -27,4 +28,5 @@ Match compiled = SearchCompiler.compile(filter.text, filter.caseSensitive, filter.regexSearch); this.match = filter.inverted?new Not(compiled):compiled; + this.isInverted = filter.inverted; } } @@ -43,33 +45,87 @@ } - List list = filter.hiding?hiddenFilters:disabledFilters; + FilterInfo fi = new FilterInfo(filter); + if (fi.isDelete) { + if (filter.hiding) { + // Remove only hide flag + hiddenFilters.add(fi); + } else { + // Remove both flags + disabledFilters.add(fi); + hiddenFilters.add(fi); + } + } else { + if (filter.mode == SearchMode.replace) { + if (filter.hiding) { + hiddenFilters.clear(); + disabledFilters.clear(); + } + } - if (filter.mode == SearchMode.replace) { - // No point in evalutaing filter when value will get replaced anyway (and yes, there is no point in using replace mode with filters) - list.clear(); + disabledFilters.add(fi); + if (filter.hiding) { + hiddenFilters.add(fi); + } } - - list.add(new FilterInfo(filter)); } } - private boolean test(List filters, OsmPrimitive primitive) { + private boolean getState(OsmPrimitive primitive, boolean hidden) { + return hidden?primitive.isDisabledAndHidden():primitive.isDisabled(); + } + + private boolean allParentWaysFiltered(OsmPrimitive primitive, boolean hidden) { + List refs = primitive.getReferrers(); + if (refs.isEmpty()) + return false; + + for (OsmPrimitive p: refs) { + if (p instanceof Way && !getState(p, hidden)) + return false; + } + + return true; + } + + private boolean oneParentWayNotFiltered(OsmPrimitive primitive, boolean hidden) { + List refs = primitive.getReferrers(); + for (OsmPrimitive p: refs) { + if (p instanceof Way && !getState(p, hidden)) + return true; + } + + return false; + } + + private boolean test(List filters, OsmPrimitive primitive, boolean hidden) { boolean selected = false; + boolean onlyInvertedFilters = true; + for (FilterInfo fi: filters) { if (fi.isDelete && selected && fi.match.match(primitive)) { selected = false; - } else if (!fi.isDelete && !selected && fi.match.match(primitive)) { + } else if (!fi.isDelete && (!selected || (onlyInvertedFilters && !fi.isInverted)) && fi.match.match(primitive)) { selected = true; + onlyInvertedFilters = onlyInvertedFilters && fi.isInverted; } } - return selected; + + if (primitive instanceof Node) { + if (!selected) + return !primitive.isTagged() && allParentWaysFiltered(primitive, hidden); + if (onlyInvertedFilters) + return selected && !oneParentWayNotFiltered(primitive, hidden); + return true; + } else + return selected; + } public boolean isHidden(OsmPrimitive primitive) { - return test(hiddenFilters, primitive); + return test(hiddenFilters, primitive, true); } public boolean isDisabled(OsmPrimitive primitive) { - return test(disabledFilters, primitive); + return test(disabledFilters, primitive, false); } Index: /trunk/src/org/openstreetmap/josm/data/osm/FilterWorker.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/FilterWorker.java (revision 3366) +++ /trunk/src/org/openstreetmap/josm/data/osm/FilterWorker.java (revision 3367) @@ -33,35 +33,27 @@ */ public static void executeFilters(Collection all, FilterMatcher filterMatcher) { + + // First relation and ways for (OsmPrimitive primitive: all) { - if (filterMatcher.isHidden(primitive)) { - primitive.setDisabledState(true); - } else if (filterMatcher.isDisabled(primitive)) { - primitive.setDisabledState(false); - } else { - primitive.unsetDisabledState(); + if (!(primitive instanceof Node)) { + if (filterMatcher.isHidden(primitive)) { + primitive.setDisabledState(true); + } else if (filterMatcher.isDisabled(primitive)) { + primitive.setDisabledState(false); + } else { + primitive.unsetDisabledState(); + } } } + // Then nodes (because they state may depend on parent ways) for (OsmPrimitive primitive: all) { - if (primitive instanceof Way && primitive.isDisabled()) { - Way w = (Way)primitive; - for (Node n: w.getNodes()) { - - if (n.isTagged()) { - continue; - } - - boolean disabled = w.isDisabled(); - boolean hidden = w.isDisabledAndHidden(); - for (OsmPrimitive ref: n.getReferrers()) { - if (ref instanceof Way) { - disabled = disabled && ref.isDisabled(); - hidden = hidden && ref.isDisabledAndHidden(); - } - } - - if (disabled) { - n.setDisabledState(hidden); - } + if (primitive instanceof Node) { + if (filterMatcher.isHidden(primitive)) { + primitive.setDisabledState(true); + } else if (filterMatcher.isDisabled(primitive)) { + primitive.setDisabledState(false); + } else { + primitive.unsetDisabledState(); } } Index: /trunk/test/unit/org/openstreetmap/josm/data/osm/FilterTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/osm/FilterTest.java (revision 3366) +++ /trunk/test/unit/org/openstreetmap/josm/data/osm/FilterTest.java (revision 3367) @@ -2,7 +2,9 @@ package org.openstreetmap.josm.data.osm; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + import java.io.FileInputStream; import java.io.FileNotFoundException; - import java.util.Arrays; import java.util.Collection; @@ -11,10 +13,6 @@ import java.util.List; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; import org.junit.BeforeClass; import org.junit.Test; - import org.openstreetmap.josm.Main; import org.openstreetmap.josm.actions.search.SearchAction.SearchMode; @@ -31,5 +29,5 @@ Main.proj = new Mercator(); } - + @Test public void basic_test() throws ParseError { @@ -40,4 +38,5 @@ n2.put("fixme", "continue"); ds.addPrimitive(n1); + ds.addPrimitive(n2); OsmPrimitive p = ds.getPrimitiveById(1,OsmPrimitiveType.NODE); assertNotNull(p); @@ -63,83 +62,95 @@ @Test public void filter_test() throws ParseError, IllegalDataException, FileNotFoundException { - for (int i : new int [] {1,2,3, 11,12,13,14}) { + for (int i : new int [] {1,2,3, 11,12,13,14, 15}) { DataSet ds = OsmReader.parseDataSet(new FileInputStream("data_nodist/filterTests.osm"), NullProgressMonitor.INSTANCE); List filters = new LinkedList(); switch (i) { - case 1: { - Filter f1 = new Filter(); - f1.text = "power"; - f1.hiding = true; - filters.add(f1); - break; - } - case 2: { - Filter f1 = new Filter(); - f1.text = "highway"; - f1.inverted = true; - filters.add(f1); - break; - } - case 3: { - Filter f1 = new Filter(); - f1.text = "power"; - f1.inverted = true; - f1.hiding = true; - Filter f2 = new Filter(); - f2.text = "highway"; - filters.addAll(Arrays.asList(new Filter[] {f1, f2})); - break; - } - case 11: { - Filter f1 = new Filter(); - f1.text = "highway"; - f1.inverted = true; - f1.hiding = true; - filters.add(f1); - break; - } - case 12: { - Filter f1 = new Filter(); - f1.text = "highway"; - f1.inverted = true; - f1.hiding = true; - Filter f2 = new Filter(); - f2.text = "water"; - f2.mode = SearchMode.remove; - filters.addAll(Arrays.asList(new Filter[] {f1, f2})); - break; - } - case 13: { - Filter f1 = new Filter(); - f1.text = "highway"; - f1.inverted = true; - f1.hiding = true; - Filter f2 = new Filter(); - f2.text = "water"; - f2.mode = SearchMode.remove; - Filter f3 = new Filter(); - f3.text = "natural"; - filters.addAll(Arrays.asList(new Filter[] {f1, f2, f3})); - break; - } - case 14: { - /* show all highways and all water features, but not lakes - * except those that have a name */ - Filter f1 = new Filter(); - f1.text = "highway"; - f1.inverted = true; - f1.hiding = true; - Filter f2 = new Filter(); - f2.text = "water"; - f2.mode = SearchMode.remove; - Filter f3 = new Filter(); - f3.text = "natural"; - Filter f4 = new Filter(); - f4.text = "name"; - f4.mode = SearchMode.remove; - filters.addAll(Arrays.asList(new Filter[] {f1, f2, f3, f4})); - break; - } + case 1: { + Filter f1 = new Filter(); + f1.text = "power"; + f1.hiding = true; + filters.add(f1); + break; + } + case 2: { + Filter f1 = new Filter(); + f1.text = "highway"; + f1.inverted = true; + filters.add(f1); + break; + } + case 3: { + Filter f1 = new Filter(); + f1.text = "power"; + f1.inverted = true; + f1.hiding = true; + Filter f2 = new Filter(); + f2.text = "highway"; + filters.addAll(Arrays.asList(new Filter[] {f1, f2})); + break; + } + case 11: { + Filter f1 = new Filter(); + f1.text = "highway"; + f1.inverted = true; + f1.hiding = true; + filters.add(f1); + break; + } + case 12: { + Filter f1 = new Filter(); + f1.text = "highway"; + f1.inverted = true; + f1.hiding = true; + Filter f2 = new Filter(); + f2.text = "water"; + f2.mode = SearchMode.remove; + filters.addAll(Arrays.asList(new Filter[] {f1, f2})); + break; + } + case 13: { + Filter f1 = new Filter(); + f1.text = "highway"; + f1.inverted = true; + f1.hiding = true; + Filter f2 = new Filter(); + f2.text = "water"; + f2.mode = SearchMode.remove; + Filter f3 = new Filter(); + f3.text = "natural"; + filters.addAll(Arrays.asList(new Filter[] {f1, f2, f3})); + break; + } + case 14: { + /* show all highways and all water features, but not lakes + * except those that have a name */ + Filter f1 = new Filter(); + f1.text = "highway"; + f1.inverted = true; + f1.hiding = true; + Filter f2 = new Filter(); + f2.text = "water"; + f2.mode = SearchMode.remove; + Filter f3 = new Filter(); + f3.text = "natural"; + Filter f4 = new Filter(); + f4.text = "name"; + f4.mode = SearchMode.remove; + filters.addAll(Arrays.asList(new Filter[] {f1, f2, f3, f4})); + break; + } + case 15: { + Filter f1 = new Filter(); + f1.text = "highway"; + f1.inverted = true; + f1.hiding = true; + Filter f2 = new Filter(); + f2.text = "water"; + f2.mode = SearchMode.remove; + f2.hiding = true; // Remove only hide flag so water should stay disabled + filters.addAll(Arrays.asList(new Filter[] {f1, f2})); + break; + } } @@ -151,13 +162,17 @@ boolean foundAtLeastOne = false; System.err.println("Run #"+i); + StringBuilder failedPrimitives = new StringBuilder(); for (OsmPrimitive osm : ds.allPrimitives()) { String key = "source:RESULT"+i; // use key that counts as untagged if (osm.hasKey(key)) { foundAtLeastOne = true; -// System.err.println("osm "+osm.getId()+" "+filterCode(osm)+" "+osm.get(key)); - assertEquals(String.format("Run #%d Object %s", i,osm.toString()), filterCode(osm), osm.get(key)); + if (!osm.get(key).equals(filterCode(osm))) { + failedPrimitives.append(String.format("Object %s. Expected [%s] but was [%s]\n", osm.toString(), osm.get(key), filterCode(osm))); + } } } assertTrue(foundAtLeastOne); + if (failedPrimitives.length() != 0) + throw new AssertionError(String.format("Run #%d\n%s", i, failedPrimitives.toString())); } }