Ticket #17021: val-index-poc.patch

File val-index-poc.patch, 23.3 KB (added by GerdP, 7 years ago)

  • test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java

     
    9393        assertEquals("{natural=}", ((ChangePropertyCommand) check.fixPrimitive(n1).getChildren().iterator().next()).getTags().toString());
    9494        assertFalse(check.test(n2));
    9595        assertEquals("The key is natural and the value is marsh",
    96                 TagCheck.insertArguments(check.rule.selectors.get(0), "The key is {0.key} and the value is {0.value}", null));
     96                TagCheck.insertArguments(check.rule.selector, "The key is {0.key} and the value is {0.value}", null));
    9797    }
    9898
    9999    /**

  • src/org/openstreetmap/josm/data/validation/Test.java

     
    188188        if (startTime > 0) {
    189189            // fix #11567 where elapsedTime is < 0
    190190            long elapsedTime = Math.max(0, System.currentTimeMillis() - startTime);
    191             Logging.debug(tr("Test ''{0}'' completed in {1}", getName(), Utils.getDurationString(elapsedTime)));
     191            Logging.warn(tr("Test ''{0}'' completed in {1}", getName(), Utils.getDurationString(elapsedTime)));
    192192        }
    193193    }
    194194

  • src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java

     
    1616import java.util.Collections;
    1717import java.util.HashMap;
    1818import java.util.HashSet;
    19 import java.util.LinkedHashMap;
     19import java.util.Iterator;
    2020import java.util.LinkedHashSet;
    2121import java.util.LinkedList;
    2222import java.util.List;
    … …  
    3535import org.openstreetmap.josm.command.DeleteCommand;
    3636import org.openstreetmap.josm.command.SequenceCommand;
    3737import org.openstreetmap.josm.data.osm.DataSet;
     38import org.openstreetmap.josm.data.osm.INode;
     39import org.openstreetmap.josm.data.osm.IRelation;
     40import org.openstreetmap.josm.data.osm.IWay;
    3841import org.openstreetmap.josm.data.osm.OsmPrimitive;
    3942import org.openstreetmap.josm.data.osm.OsmUtils;
    4043import org.openstreetmap.josm.data.osm.Tag;
    … …  
    5255import org.openstreetmap.josm.gui.mappaint.mapcss.Expression;
    5356import org.openstreetmap.josm.gui.mappaint.mapcss.Instruction;
    5457import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSRule;
    55 import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSRule.Declaration;
    5658import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSStyleSource;
     59import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSStyleSource.MapCSSRuleIndex;
    5760import org.openstreetmap.josm.gui.mappaint.mapcss.Selector;
    5861import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.AbstractSelector;
     62import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.ChildOrParentSelector;
    5963import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.GeneralSelector;
    6064import org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.MapCSSParser;
    6165import org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.ParseException;
    … …  
    6771import org.openstreetmap.josm.spi.preferences.Config;
    6872import org.openstreetmap.josm.tools.CheckParameterUtil;
    6973import org.openstreetmap.josm.tools.I18n;
     74import org.openstreetmap.josm.tools.JosmRuntimeException;
    7075import org.openstreetmap.josm.tools.Logging;
    7176import org.openstreetmap.josm.tools.MultiMap;
    7277import org.openstreetmap.josm.tools.Utils;
    7378
     79
    7480/**
    7581 * MapCSS-based tag checker/fixer.
    7682 * @since 6506
    … …  
    7884public class MapCSSTagChecker extends Test.TagTest {
    7985
    8086    /**
    81      * A grouped MapCSSRule with multiple selectors for a single declaration.
    82      * @see MapCSSRule
     87     * The rules from one url.
     88     * @author GerdP
     89     *
    8390     */
    84     public static class GroupedMapCSSRule {
    85         /** MapCSS selectors **/
    86         public final List<Selector> selectors;
    87         /** MapCSS declaration **/
    88         public final Declaration declaration;
     91    static class RulesGroup {
     92        /**
     93         * Name of the group.
     94         */
     95        private String url;
    8996
     97        List<TestError> errors;
    9098        /**
    91          * Constructs a new {@code GroupedMapCSSRule}.
    92          * @param selectors MapCSS selectors
    93          * @param declaration MapCSS declaration
     99         * all checks in this file
    94100         */
    95         public GroupedMapCSSRule(List<Selector> selectors, Declaration declaration) {
    96             this.selectors = selectors;
    97             this.declaration = declaration;
    98         }
     101        final List<TagCheck> checks = new ArrayList<>();
     102        /**
     103         * Rules for nodes
     104         */
     105        final MapCSSRuleIndex nodeRules = new MapCSSRuleIndex();
     106        /**
     107         * Rules for ways without tag area=no
     108         */
     109        final MapCSSRuleIndex wayRules = new MapCSSRuleIndex();
     110        /**
     111         * Rules for ways with tag area=no
     112         */
     113        final MapCSSRuleIndex wayNoAreaRules = new MapCSSRuleIndex();
     114        /**
     115         * Rules for relations that are not multipolygon relations
     116         */
     117        final MapCSSRuleIndex relationRules = new MapCSSRuleIndex();
     118        /**
     119         * Rules for multipolygon relations
     120         */
     121        final MapCSSRuleIndex multipolygonRules = new MapCSSRuleIndex();
     122        /**
     123         * rules to apply canvas properties
     124         */
     125        final MapCSSRuleIndex canvasRules = new MapCSSRuleIndex();
    99126
    100         @Override
    101         public int hashCode() {
    102             return Objects.hash(selectors, declaration);
     127        final MultiMap<MapCSSRule, TagCheck> map = new MultiMap<>();
     128
     129        public RulesGroup(String url, List<TagCheck> parseChecks) {
     130            this.url = url;
     131            this.checks.addAll(parseChecks);
     132            buildIndex();
    103133        }
    104134
    105         @Override
    106         public boolean equals(Object obj) {
    107             if (this == obj) return true;
    108             if (obj == null || getClass() != obj.getClass()) return false;
    109             GroupedMapCSSRule that = (GroupedMapCSSRule) obj;
    110             return Objects.equals(selectors, that.selectors) &&
    111                     Objects.equals(declaration, that.declaration);
     135        private void buildIndex() {
     136            map.clear();
     137            nodeRules.clear();
     138            wayRules.clear();
     139            wayNoAreaRules.clear();
     140            relationRules.clear();
     141            multipolygonRules.clear();
     142            canvasRules.clear();
     143            // optimization: filter rules for different primitive types
     144
     145            for (TagCheck c : checks) {
     146                MapCSSRule r  = c.rule;
     147                // find the rightmost selector, this must be a GeneralSelector
     148                Selector selRightmost = r.selector;
     149                while (selRightmost instanceof ChildOrParentSelector) {
     150                    selRightmost = ((ChildOrParentSelector) selRightmost).right;
     151                }
     152                MapCSSRule optRule = new MapCSSRule(r.selector.optimizedBaseCheck(), r.declaration);
     153                map.put(optRule, c);
     154                final String base = ((GeneralSelector) selRightmost).getBase();
     155                switch (base) {
     156                    case "node":
     157                        nodeRules.add(optRule);
     158                        break;
     159                    case "way":
     160                        wayNoAreaRules.add(optRule);
     161                        wayRules.add(optRule);
     162                        break;
     163                    case "area":
     164                        wayRules.add(optRule);
     165                        multipolygonRules.add(optRule);
     166                        break;
     167                    case "relation":
     168                        relationRules.add(optRule);
     169                        multipolygonRules.add(optRule);
     170                        break;
     171                    case "*":
     172                        nodeRules.add(optRule);
     173                        wayRules.add(optRule);
     174                        wayNoAreaRules.add(optRule);
     175                        relationRules.add(optRule);
     176                        multipolygonRules.add(optRule);
     177                        break;
     178                    case "canvas":
     179                        canvasRules.add(r);
     180                        break;
     181                    case "meta":
     182                    case "setting":
     183                        break;
     184                    default:
     185                        final RuntimeException e = new JosmRuntimeException(MessageFormat.format("Unknown MapCSS base selector {0}", base));
     186                        Logging.warn(tr("Failed to parse Mappaint styles from ''{0}''. Error was: {1}", url, e.getMessage()));
     187                        Logging.error(e);
     188                }
     189            }
     190            nodeRules.initIndex();
     191            wayRules.initIndex();
     192            wayNoAreaRules.initIndex();
     193            relationRules.initIndex();
     194            multipolygonRules.initIndex();
     195            canvasRules.initIndex();
    112196        }
    113197
    114         @Override
    115         public String toString() {
    116             return "GroupedMapCSSRule [selectors=" + selectors + ", declaration=" + declaration + ']';
     198        public List<TestError> apply(OsmPrimitive p, boolean includeOtherSeverity) {
     199            final List<TestError> res = new ArrayList<>();
     200            MapCSSRuleIndex matchingRuleIndex;
     201            if (p instanceof INode) {
     202                matchingRuleIndex = nodeRules;
     203            } else if (p instanceof IWay) {
     204                if (OsmUtils.isFalse(p.get("area"))) {
     205                    matchingRuleIndex = wayNoAreaRules;
     206                } else {
     207                    matchingRuleIndex = wayRules;
     208                }
     209            } else if (p instanceof IRelation) {
     210                if (((IRelation<?>) p).isMultipolygon()) {
     211                    matchingRuleIndex = multipolygonRules;
     212                } else if (p.hasKey("#canvas")) {
     213                    matchingRuleIndex = canvasRules;
     214                } else {
     215                    matchingRuleIndex = relationRules;
     216                }
     217            } else {
     218                throw new IllegalArgumentException("Unsupported type: " + p);
     219            }
     220
     221            Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null);
     222            // the declaration indices are sorted, so it suffices to save the last used index
     223            int lastDeclUsed = -1;
     224
     225            Iterator<MapCSSRule> candidates = matchingRuleIndex.getRuleCandidates(p);
     226            while (candidates.hasNext()) {
     227                MapCSSRule r = candidates.next();
     228                env.clearSelectorMatchingInformation();
     229                if (r.selector.matches(env)) { // as side effect env.parent will be set (if s is a child selector)
     230                    Set<TagCheck> tests = map.get(r);
     231                    for (TagCheck check: tests) {
     232                        boolean ignoreError = Severity.OTHER == check.getSeverity() && !includeOtherSeverity;
     233                        // Do not run "information" level checks if not wanted, unless they also set a MapCSS class
     234                        if (ignoreError && check.setClassExpressions.isEmpty()) {
     235                            continue;
     236                        }
     237                        if (r.declaration.idx == lastDeclUsed)
     238                            continue; // don't apply one declaration more than once
     239                        lastDeclUsed = r.declaration.idx;
     240
     241                        r.declaration.execute(env);
     242                        if (!ignoreError && !check.errors.isEmpty()) {
     243                            final TestError error = check.getErrorForPrimitive(p, check.rule.selector, env, new MapCSSTagCheckerAndRule(check.rule));
     244                            if (error != null) {
     245                                res.add(error);
     246                            }
     247                        }
     248
     249                    }
     250                }
     251            }
     252            return res;
    117253        }
    118254    }
    119255
    … …  
    239375    }
    240376
    241377    final MultiMap<String, TagCheck> checks = new MultiMap<>();
     378    final LinkedHashSet<RulesGroup> checks2 = new LinkedHashSet<>();
    242379
    243380    /**
    244381     * Result of {@link TagCheck#readMapCSS}
    … …  
    266403     */
    267404    public static class TagCheck implements Predicate<OsmPrimitive> {
    268405        /** The selector of this {@code TagCheck} */
    269         protected final GroupedMapCSSRule rule;
     406        protected final MapCSSRule rule;
    270407        /** Commands to apply in order to fix a matching primitive */
    271408        protected final List<FixCommand> fixCommands = new ArrayList<>();
    272409        /** Tags (or arbitraty strings) of alternatives to be presented to the user */
    … …  
    283420        /** A string used to group similar tests */
    284421        protected String group;
    285422
    286         TagCheck(GroupedMapCSSRule rule) {
     423        TagCheck(MapCSSRule rule) {
    287424            this.rule = rule;
    288425        }
    289426
    … …  
    302439            return sb.toString();
    303440        }
    304441
    305         static TagCheck ofMapCSSRule(final GroupedMapCSSRule rule) throws IllegalDataException {
     442        static TagCheck ofMapCSSRule(final MapCSSRule rule) throws IllegalDataException {
    306443            final TagCheck check = new TagCheck(rule);
    307444            for (Instruction i : rule.declaration.instructions) {
    308445                if (i instanceof Instruction.AssignmentInstruction) {
    … …  
    357494            }
    358495            if (check.errors.isEmpty() && check.setClassExpressions.isEmpty()) {
    359496                throw new IllegalDataException(
    360                         "No "+POSSIBLE_THROWS+" given! You should specify a validation error message for " + rule.selectors);
     497                        "No "+POSSIBLE_THROWS+" given! You should specify a validation error message for " + rule.selector);
    361498            } else if (check.errors.size() > 1) {
    362499                throw new IllegalDataException(
    363500                        "More than one "+POSSIBLE_THROWS+" given! You should specify a single validation error message for "
    364                                 + rule.selectors);
     501                                + rule.selector);
    365502            }
    366503            return check;
    367504        }
    … …  
    376513            parser.sheet(source);
    377514            // Ignore "meta" rule(s) from external rules of JOSM wiki
    378515            source.removeMetaRules();
    379             // group rules with common declaration block
    380             Map<Declaration, List<Selector>> g = new LinkedHashMap<>();
     516            List<TagCheck> parseChecks = new ArrayList<>();
    381517            for (MapCSSRule rule : source.rules) {
    382                 if (!g.containsKey(rule.declaration)) {
    383                     List<Selector> sels = new ArrayList<>();
    384                     sels.add(rule.selector);
    385                     g.put(rule.declaration, sels);
    386                 } else {
    387                     g.get(rule.declaration).add(rule.selector);
    388                 }
    389             }
    390             List<TagCheck> parseChecks = new ArrayList<>();
    391             for (Map.Entry<Declaration, List<Selector>> map : g.entrySet()) {
    392518                try {
    393                     parseChecks.add(TagCheck.ofMapCSSRule(
    394                             new GroupedMapCSSRule(map.getValue(), map.getKey())));
     519                    parseChecks.add(TagCheck.ofMapCSSRule(rule));
    395520                } catch (IllegalDataException e) {
    396521                    Logging.error("Cannot add MapCss rule: "+e.getMessage());
    397522                    source.logError(e);
    … …  
    402527
    403528        @Override
    404529        public boolean test(OsmPrimitive primitive) {
    405             // Tests whether the primitive contains a deprecated tag which is represented by this MapCSSTagChecker.
    406             return whichSelectorMatchesPrimitive(primitive) != null;
     530            return rule.selector.matches(new Environment(primitive));
    407531        }
    408532
    409         Selector whichSelectorMatchesPrimitive(OsmPrimitive primitive) {
    410             return whichSelectorMatchesEnvironment(new Environment(primitive));
    411         }
    412 
    413         Selector whichSelectorMatchesEnvironment(Environment env) {
    414             for (Selector i : rule.selectors) {
    415                 env.clearSelectorMatchingInformation();
    416                 if (i.matches(env)) {
    417                     return i;
    418                 }
    419             }
    420             return null;
    421         }
    422 
    423533        /**
    424534         * Determines the {@code index}-th key/value/tag (depending on {@code type}) of the
    425535         * {@link org.openstreetmap.josm.gui.mappaint.mapcss.Selector.GeneralSelector}.
    … …  
    492602                return null;
    493603            }
    494604            try {
    495                 final Selector matchingSelector = whichSelectorMatchesPrimitive(p);
     605                final Selector matchingSelector = rule.selector;
    496606                Collection<Command> cmds = new LinkedList<>();
    497607                for (FixCommand fixCommand : fixCommands) {
    498608                    cmds.add(fixCommand.createCommand(p, matchingSelector));
    … …  
    563673        public String toString() {
    564674            return getDescription(null);
    565675        }
    566 
    567         /**
    568          * Constructs a {@link TestError} for the given primitive, or returns null if the primitive does not give rise to an error.
    569          *
    570          * @param p the primitive to construct the error for
    571          * @return an instance of {@link TestError}, or returns null if the primitive does not give rise to an error.
    572          */
    573676        TestError getErrorForPrimitive(OsmPrimitive p) {
    574677            final Environment env = new Environment(p);
    575             return getErrorForPrimitive(p, whichSelectorMatchesEnvironment(env), env, null);
     678            return getErrorForPrimitive(p, rule.selector, env, null);
    576679        }
    577680
    578681        TestError getErrorForPrimitive(OsmPrimitive p, Selector matchingSelector, Environment env, Test tester) {
    … …  
    632735         */
    633736        public Set<String> getClassesIds() {
    634737            Set<String> result = new HashSet<>();
    635             for (Selector s : rule.selectors) {
    636                 if (s instanceof AbstractSelector) {
    637                     for (Condition c : ((AbstractSelector) s).getConditions()) {
    638                         if (c instanceof ClassCondition) {
    639                             result.add(((ClassCondition) c).id);
    640                         }
     738            if (rule.selector instanceof AbstractSelector) {
     739                for (Condition c : ((AbstractSelector) rule.selector).getConditions()) {
     740                    if (c instanceof ClassCondition) {
     741                        result.add(((ClassCondition) c).id);
    641742                    }
    642743                }
    643744            }
    … …  
    646747    }
    647748
    648749    static class MapCSSTagCheckerAndRule extends MapCSSTagChecker {
    649         public final GroupedMapCSSRule rule;
     750        public final MapCSSRule rule;
    650751
    651         MapCSSTagCheckerAndRule(GroupedMapCSSRule rule) {
     752        MapCSSTagCheckerAndRule(MapCSSRule rule) {
    652753            this.rule = rule;
    653754        }
    654755
    … …  
    656757        public synchronized boolean equals(Object obj) {
    657758            return super.equals(obj)
    658759                    || (obj instanceof TagCheck && rule.equals(((TagCheck) obj).rule))
    659                     || (obj instanceof GroupedMapCSSRule && rule.equals(obj));
     760                    || (obj instanceof MapCSSRule && rule.equals(obj));
    660761        }
    661762
    662763        @Override
    … …  
    677778     * @return all errors for the given primitive, with or without those of "info" severity
    678779     */
    679780    public synchronized Collection<TestError> getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity) {
    680         return getErrorsForPrimitive(p, includeOtherSeverity, checks.values());
     781        List<TestError> errors = new ArrayList<>();
     782        for (RulesGroup g : checks2) {
     783            errors.addAll(g.apply(p, includeOtherSeverity));
     784        }
     785        return errors;
     786
    681787    }
    682788
    683789    private static Collection<TestError> getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity,
    … …  
    684790            Collection<Set<TagCheck>> checksCol) {
    685791        final List<TestError> r = new ArrayList<>();
    686792        final Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null);
     793
    687794        for (Set<TagCheck> schecks : checksCol) {
    688795            for (TagCheck check : schecks) {
    689796                boolean ignoreError = Severity.OTHER == check.getSeverity() && !includeOtherSeverity;
    … …  
    691798                if (ignoreError && check.setClassExpressions.isEmpty()) {
    692799                    continue;
    693800                }
    694                 final Selector selector = check.whichSelectorMatchesEnvironment(env);
    695                 if (selector != null) {
     801                if (check.rule.selector.matches(env)) {
    696802                    check.rule.declaration.execute(env);
    697803                    if (!ignoreError && !check.errors.isEmpty()) {
    698                         final TestError error = check.getErrorForPrimitive(p, selector, env, new MapCSSTagCheckerAndRule(check.rule));
     804                        final TestError error = check.getErrorForPrimitive(p, check.rule.selector, env, new MapCSSTagCheckerAndRule(check.rule));
    699805                        if (error != null) {
    700806                            r.add(error);
    701807                        }
    702808                    }
     809
    703810                }
    704811            }
    705812        }
    … …  
    706813        return r;
    707814    }
    708815
     816
    709817    /**
    710818     * Visiting call for primitives.
    711819     *
    … …  
    713821     */
    714822    @Override
    715823    public void check(OsmPrimitive p) {
    716         errors.addAll(getErrorsForPrimitive(p, ValidatorPrefHelper.PREF_OTHER.get()));
     824        boolean includeOtherSeverity = ValidatorPrefHelper.PREF_OTHER.get();
     825
     826        for (RulesGroup g : checks2) {
     827            errors.addAll(g.apply(p, includeOtherSeverity));
     828        }
     829
    717830    }
    718831
    719832    /**
    … …  
    736849            result = TagCheck.readMapCSS(reader);
    737850            checks.remove(url);
    738851            checks.putAll(url, result.parseChecks);
     852            RulesGroup rg = new RulesGroup(url, result.parseChecks);
     853            checks2.add(rg);
    739854            // Check assertions, useful for development of local files
    740855            if (Config.getPref().getBoolean("validator.check_assert_local_rules", false) && Utils.isLocalUrl(url)) {
    741856                for (String msg : checkAsserts(result.parseChecks)) {
    … …  
    749864    @Override
    750865    public synchronized void initialize() throws Exception {
    751866        checks.clear();
     867        checks2.clear();
     868
    752869        for (SourceEntry source : new ValidatorPrefHelper().get()) {
    753870            if (!source.active) {
    754871                continue;
    … …  
    803920                final boolean isError = pErrors.stream().anyMatch(e -> e.getTester().equals(check.rule));
    804921                if (isError != i.getValue()) {
    805922                    final String error = MessageFormat.format("Expecting test ''{0}'' (i.e., {1}) to {2} {3} (i.e., {4})",
    806                             check.getMessage(p), check.rule.selectors, i.getValue() ? "match" : "not match", i.getKey(), p.getKeys());
     923                            check.getMessage(p), check.rule.selector, i.getValue() ? "match" : "not match", i.getKey(), p.getKeys());
    807924                    assertionErrors.add(error);
    808925                }
    809926                ds.removePrimitive(p);