Changeset 15981 in josm


Ignore:
Timestamp:
2020-03-01T23:35:51+01:00 (3 months ago)
Author:
simon04
Message:

see #18802 - MapCSSTagChecker: no not persist assertions

Check assertions immediately, when needed, but do not keep them.

Location:
trunk
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java

    r15980 r15981  
    2222import java.util.Optional;
    2323import java.util.Set;
     24import java.util.function.Consumer;
    2425import java.util.function.Predicate;
    2526import java.util.regex.Matcher;
     
    288289         * Is evaluated on the matching primitive to give the error message. Map is checked to contain exactly one element. */
    289290        protected final Map<Instruction.AssignmentInstruction, Severity> errors = new HashMap<>();
    290         /** Unit tests */
    291         protected final Map<String, Boolean> assertions = new HashMap<>();
    292291        /** MapCSS Classes to set on matching primitives */
    293292        protected final Set<String> setClassExpressions = new HashSet<>();
     
    303302        private static final String POSSIBLE_THROWS = "throwError/throwWarning/throwOther";
    304303
    305         static TagCheck ofMapCSSRule(final GroupedMapCSSRule rule) throws IllegalDataException {
     304        static TagCheck ofMapCSSRule(final GroupedMapCSSRule rule, AssertionConsumer assertionConsumer) throws IllegalDataException {
    306305            final TagCheck check = new TagCheck(rule);
     306            final Map<String, Boolean> assertions = new HashMap<>();
    307307            for (Instruction i : rule.declaration.instructions) {
    308308                if (i instanceof Instruction.AssignmentInstruction) {
     
    346346                            check.alternatives.add(val);
    347347                        } else if (val != null && "assertMatch".equals(ai.key)) {
    348                             check.assertions.put(val, Boolean.TRUE);
     348                            assertions.put(val, Boolean.TRUE);
    349349                        } else if (val != null && "assertNoMatch".equals(ai.key)) {
    350                             check.assertions.put(val, Boolean.FALSE);
     350                            assertions.put(val, Boolean.FALSE);
    351351                        } else if (val != null && "group".equals(ai.key)) {
    352352                            check.group = val;
     
    369369                                + rule.selectors);
    370370            }
     371            if (assertionConsumer != null) {
     372                MapCSSTagCheckerAsserts.checkAsserts(check, assertions, assertionConsumer);
     373            }
    371374            return check;
    372375        }
    373376
    374377        static ParseResult readMapCSS(Reader css) throws ParseException {
    375             return readMapCSS(css, "");
    376         }
    377 
    378         static ParseResult readMapCSS(Reader css, String url) throws ParseException {
     378            return readMapCSS(css, "", null);
     379        }
     380
     381        static ParseResult readMapCSS(Reader css, String url, AssertionConsumer assertionConsumer) throws ParseException {
    379382            CheckParameterUtil.ensureParameterNotNull(css, "css");
    380383
     
    401404                try {
    402405                    parseChecks.add(TagCheck.ofMapCSSRule(
    403                             new GroupedMapCSSRule(map.getValue(), map.getKey(), url)));
     406                            new GroupedMapCSSRule(map.getValue(), map.getKey(), url), assertionConsumer));
    404407                } catch (IllegalDataException e) {
    405408                    Logging.error("Cannot add MapCss rule: "+e.getMessage());
     
    781784
    782785    /**
     786     * A handler for assertion error messages (for not fulfilled "assertMatch", "assertNoMatch").
     787     */
     788    @FunctionalInterface
     789    interface AssertionConsumer extends Consumer<String> {
     790    }
     791
     792    /**
    783793     * Adds a new MapCSS config file from the given URL.
    784794     * @param url The unique URL of the MapCSS config file
     
    789799     */
    790800    public synchronized ParseResult addMapCSS(String url) throws ParseException, IOException {
     801        // Check assertions, useful for development of local files
     802        final boolean checkAssertions = Config.getPref().getBoolean("validator.check_assert_local_rules", false) && Utils.isLocalUrl(url);
     803        return addMapCSS(url, checkAssertions ? Logging::warn : null);
     804    }
     805
     806    synchronized ParseResult addMapCSS(String url, AssertionConsumer assertionConsumer) throws ParseException, IOException {
    791807        CheckParameterUtil.ensureParameterNotNull(url, "url");
    792808        ParseResult result;
     
    797813            if (zip != null)
    798814                I18n.addTexts(cache.getFile());
    799             result = TagCheck.readMapCSS(reader, url);
     815            result = TagCheck.readMapCSS(reader, url, assertionConsumer);
    800816            checks.remove(url);
    801817            checks.putAll(url, result.parseChecks);
    802818            indexData = null;
    803             // Check assertions, useful for development of local files
    804             if (Config.getPref().getBoolean("validator.check_assert_local_rules", false) && Utils.isLocalUrl(url)) {
    805                 for (String msg : MapCSSTagCheckerAsserts.checkAsserts(result.parseChecks)) {
    806                     Logging.warn(msg);
    807                 }
    808             }
    809819        }
    810820        return result;
     
    838848            }
    839849        }
     850        MapCSSTagCheckerAsserts.clear();
    840851    }
    841852
  • trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerAsserts.java

    r15979 r15981  
    88import java.util.Collections;
    99import java.util.HashSet;
    10 import java.util.LinkedHashSet;
    1110import java.util.List;
    1211import java.util.Map;
     
    3635
    3736/**
    38  * Utility class for checking rule {@linkplain MapCSSTagChecker.TagCheck#assertions assertions} of {@link MapCSSTagChecker.TagCheck}.
     37 * Utility class for checking rule assertions of {@link MapCSSTagChecker.TagCheck}.
    3938 */
    40 class MapCSSTagCheckerAsserts {
     39final class MapCSSTagCheckerAsserts {
     40
    4141    private MapCSSTagCheckerAsserts() {
    4242        // private constructor
    4343    }
    4444
     45    private static final ArrayList<MapCSSTagChecker.TagCheck> previousChecks = new ArrayList<>();
     46
    4547    /**
    4648     * Checks that rule assertions are met for the given set of TagChecks.
    47      * @param schecks The TagChecks for which assertions have to be checked
    48      * @return A set of error messages, empty if all assertions are met
    49      * @since 7356
     49     * @param check The TagCheck for which assertions have to be checked
     50     * @param assertionConsumer The handler for assertion error messages
    5051     */
    51     public static Set<String> checkAsserts(final Collection<MapCSSTagChecker.TagCheck> schecks) {
    52         Set<String> assertionErrors = new LinkedHashSet<>();
     52    static void checkAsserts(final MapCSSTagChecker.TagCheck check, final Map<String, Boolean> assertions,
     53                                    final MapCSSTagChecker.AssertionConsumer assertionConsumer) {
    5354        final Method insideMethod = getFunctionMethod("inside");
    5455        final DataSet ds = new DataSet();
    55         for (final MapCSSTagChecker.TagCheck check : schecks) {
    56             Logging.debug("Check: {0}", check);
    57             for (final Map.Entry<String, Boolean> i : check.assertions.entrySet()) {
    58                 Logging.debug("- Assertion: {0}", i);
    59                 final OsmPrimitive p = OsmUtils.createPrimitive(i.getKey(), getLocation(check, insideMethod), true);
    60                 // Build minimal ordered list of checks to run to test the assertion
    61                 List<Set<MapCSSTagChecker.TagCheck>> checksToRun = new ArrayList<>();
    62                 Set<MapCSSTagChecker.TagCheck> checkDependencies = getTagCheckDependencies(check, schecks);
    63                 if (!checkDependencies.isEmpty()) {
    64                     checksToRun.add(checkDependencies);
     56        Logging.debug("Check: {0}", check);
     57        for (final Map.Entry<String, Boolean> i : assertions.entrySet()) {
     58            Logging.debug("- Assertion: {0}", i);
     59            final OsmPrimitive p = OsmUtils.createPrimitive(i.getKey(), getLocation(check, insideMethod), true);
     60            // Build minimal ordered list of checks to run to test the assertion
     61            List<Set<MapCSSTagChecker.TagCheck>> checksToRun = new ArrayList<>();
     62            Set<MapCSSTagChecker.TagCheck> checkDependencies = getTagCheckDependencies(check, previousChecks);
     63            if (!checkDependencies.isEmpty()) {
     64                checksToRun.add(checkDependencies);
     65            }
     66            checksToRun.add(Collections.singleton(check));
     67            // Add primitive to dataset to avoid DataIntegrityProblemException when evaluating selectors
     68            addPrimitive(ds, p);
     69            final Collection<TestError> pErrors = MapCSSTagChecker.getErrorsForPrimitive(p, true, checksToRun);
     70            Logging.debug("- Errors: {0}", pErrors);
     71            final boolean isError = pErrors.stream().anyMatch(e -> e.getTester() instanceof MapCSSTagChecker.MapCSSTagCheckerAndRule
     72                    && ((MapCSSTagChecker.MapCSSTagCheckerAndRule) e.getTester()).rule.equals(check.rule));
     73            if (isError != i.getValue()) {
     74                assertionConsumer.accept(MessageFormat.format("Expecting test ''{0}'' (i.e., {1}) to {2} {3} (i.e., {4})",
     75                        check.getMessage(p), check.rule.selectors, i.getValue() ? "match" : "not match", i.getKey(), p.getKeys()));
     76            }
     77            if (isError) {
     78                // Check that autofix works as expected
     79                Command fix = check.fixPrimitive(p);
     80                if (fix != null && fix.executeCommand() && !MapCSSTagChecker.getErrorsForPrimitive(p, true, checksToRun).isEmpty()) {
     81                    assertionConsumer.accept(MessageFormat.format("Autofix does not work for test ''{0}'' (i.e., {1})",
     82                            check.getMessage(p), check.rule.selectors));
    6583                }
    66                 checksToRun.add(Collections.singleton(check));
    67                 // Add primitive to dataset to avoid DataIntegrityProblemException when evaluating selectors
    68                 addPrimitive(ds, p);
    69                 final Collection<TestError> pErrors = MapCSSTagChecker.getErrorsForPrimitive(p, true, checksToRun);
    70                 Logging.debug("- Errors: {0}", pErrors);
    71                 final boolean isError = pErrors.stream().anyMatch(e -> e.getTester() instanceof MapCSSTagChecker.MapCSSTagCheckerAndRule
    72                         && ((MapCSSTagChecker.MapCSSTagCheckerAndRule) e.getTester()).rule.equals(check.rule));
    73                 if (isError != i.getValue()) {
    74                     assertionErrors.add(MessageFormat.format("Expecting test ''{0}'' (i.e., {1}) to {2} {3} (i.e., {4})",
    75                             check.getMessage(p), check.rule.selectors, i.getValue() ? "match" : "not match", i.getKey(), p.getKeys()));
    76                 }
    77                 if (isError) {
    78                     // Check that autofix works as expected
    79                     Command fix = check.fixPrimitive(p);
    80                     if (fix != null && fix.executeCommand() && !MapCSSTagChecker.getErrorsForPrimitive(p, true, checksToRun).isEmpty()) {
    81                         assertionErrors.add(MessageFormat.format("Autofix does not work for test ''{0}'' (i.e., {1})",
    82                                 check.getMessage(p), check.rule.selectors));
    83                     }
    84                 }
    85                 ds.removePrimitive(p);
    8684            }
     85            ds.removePrimitive(p);
    8786        }
    88         return assertionErrors;
     87        previousChecks.add(check);
     88    }
     89
     90    public static void clear() {
     91        previousChecks.clear();
     92        previousChecks.trimToSize();
    8993    }
    9094
  • trunk/test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java

    r15979 r15981  
    1616import java.util.Set;
    1717
     18import org.junit.Before;
    1819import org.junit.Rule;
    1920import org.junit.Test;
     
    2829import org.openstreetmap.josm.data.osm.OsmPrimitive;
    2930import org.openstreetmap.josm.data.osm.OsmUtils;
     31import org.openstreetmap.josm.data.preferences.sources.ExtendedSourceEntry;
     32import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper;
    3033import org.openstreetmap.josm.data.validation.Severity;
    3134import org.openstreetmap.josm.data.validation.TestError;
     
    5356    public JOSMTestRules test = new JOSMTestRules().projection().territories().preferences();
    5457
     58    /**
     59     * Setup test.
     60     */
     61    @Before
     62    public void setUp() {
     63        MapCSSTagCheckerAsserts.clear();
     64    }
     65
    5566    static MapCSSTagChecker buildTagChecker(String css) throws ParseException {
    5667        final MapCSSTagChecker test = new MapCSSTagChecker();
    57         test.checks.putAll("test", TagCheck.readMapCSS(new StringReader(css)).parseChecks);
     68        Set<String> errors = new HashSet<>();
     69        test.checks.putAll("test", TagCheck.readMapCSS(new StringReader(css), "", errors::add).parseChecks);
     70        assertTrue(errors.toString(), errors.isEmpty());
    5871        return test;
    5972    }
     
    182195        MapCSSTagChecker c = new MapCSSTagChecker();
    183196        c.initialize();
    184 
     197    }
     198
     199    /**
     200     * Unit test for all {@link MapCSSTagChecker.TagTest} assertions.
     201     * @throws Exception if an error occurs
     202     */
     203    @Test
     204    public void testAssertions() throws Exception {
     205        MapCSSTagChecker c = new MapCSSTagChecker();
    185206        Set<String> assertionErrors = new LinkedHashSet<>();
    186         for (Set<TagCheck> schecks : c.checks.values()) {
    187             assertionErrors.addAll(MapCSSTagCheckerAsserts.checkAsserts(schecks));
    188         }
     207
     208        // initialize
     209        for (ExtendedSourceEntry entry : ValidatorPrefHelper.INSTANCE.getDefault()) {
     210            c.addMapCSS(entry.url, assertionErrors::add);
     211        }
     212
    189213        for (String msg : assertionErrors) {
    190214            Logging.error(msg);
     
    205229                "  assertNoMatch: \"node amenity=restaurant\";\n" +
    206230                "}");
    207         Set<String> errors = MapCSSTagCheckerAsserts.checkAsserts(test.checks.get("test"));
    208         assertTrue(errors.toString(), errors.isEmpty());
     231        assertNotNull(test);
    209232    }
    210233
     
    221244                "  assertNoMatch: \"way name=Hauptstrasse\";\n" +
    222245                "}");
    223         Set<String> errors = MapCSSTagCheckerAsserts.checkAsserts(test.checks.get("test"));
    224         assertTrue(errors.toString(), errors.isEmpty());
     246        assertNotNull(test);
    225247    }
    226248
Note: See TracChangeset for help on using the changeset viewer.