Changeset 18847 in josm for trunk/src/org/openstreetmap


Ignore:
Timestamp:
2023-10-03T16:25:42+02:00 (14 months ago)
Author:
taylor.smock
Message:

Fix #23212: Overpass query wizard should transform key: to ["key"] instead of [~"key"~""]

This is fixed by using ExactKeyValue instead of KeyValue for key: queries.
As a happy side effect, this significantly reduces the cost of key: queries.
In a test area ("Mesa County, Colorado"), this reduces the cpu time from 2600ms
to 200ms and reduces memory allocations from 425mb to effectively 0 for a name:
query.

In addition, this simplifies many equals methods by converting the following
pattern to Objects.equals(first, second):

if (first == null) {
    if (second != null) {
        return true;
    }
} else if (!first.equals(second))
    return false;
return true;

There are some additional changes, mostly related to documentation and lint
issues.

Location:
trunk/src/org/openstreetmap/josm
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/org/openstreetmap/josm/data/osm/search/SearchCompiler.java

    r18801 r18847  
    2727import java.util.regex.PatternSyntaxException;
    2828import java.util.stream.Collectors;
     29
     30import javax.annotation.Nonnull;
     31import javax.annotation.Nullable;
    2932
    3033import org.openstreetmap.josm.data.Bounds;
    … …  
    127130    }
    128131
     132    /**
     133     * The core factory for "simple" {@link Match} objects
     134     */
    129135    public static class CoreSimpleMatchFactory implements SimpleMatchFactory {
    130136        private final Collection<String> keywords = Arrays.asList("id", "version", "type", "user", "role",
    … …  
    156162            default:
    157163                if (tokenizer != null) {
    158                     switch (keyword) {
    159                     case "id":
    160                         return new Id(tokenizer);
    161                     case "version":
    162                         return new Version(tokenizer);
    163                     case "type":
    164                         return new ExactType(tokenizer.readTextOrNumber());
    165                     case "preset":
    166                         return new Preset(tokenizer.readTextOrNumber());
    167                     case "user":
    168                         return new UserMatch(tokenizer.readTextOrNumber());
    169                     case "role":
    170                         return new RoleMatch(tokenizer.readTextOrNumber());
    171                     case "changeset":
    172                         return new ChangesetId(tokenizer);
    173                     case "nodes":
    174                         return new NodeCountRange(tokenizer);
    175                     case "ways":
    176                         return new WayCountRange(tokenizer);
    177                     case "members":
    178                         return new MemberCountRange(tokenizer);
    179                     case "tags":
    180                         return new TagCountRange(tokenizer);
    181                     case "areasize":
    182                         return new AreaSize(tokenizer);
    183                     case "waylength":
    184                         return new WayLength(tokenizer);
    185                     case "nth":
    186                         return new Nth(tokenizer, false);
    187                     case "nth%":
    188                         return new Nth(tokenizer, true);
    189                     case "hasRole":
    190                         return new HasRole(tokenizer);
    191                     case "timestamp":
    192                         // add leading/trailing space in order to get expected split (e.g. "a--" => {"a", ""})
    193                         String rangeS = ' ' + tokenizer.readTextOrNumber() + ' ';
    194                         String[] rangeA = rangeS.split("/", -1);
    195                         if (rangeA.length == 1) {
    196                             return new KeyValue(keyword, rangeS.trim(), regexSearch, caseSensitive);
    197                         } else if (rangeA.length == 2) {
    198                             return TimestampRange.create(rangeA);
    199                         } else {
    200                             throw new SearchParseError("<html>" + tr("Expecting {0} after {1}", "<i>min</i>/<i>max</i>", "<i>timestamp</i>")
    201                                 + "</html>");
    202                         }
    203                     }
     164                    return getTokenizer(keyword, caseSensitive, regexSearch, tokenizer);
    204165                } else {
    205166                    throw new SearchParseError("<html>" + tr("Expecting {0} after {1}", "<code>:</code>", "<i>" + keyword + "</i>") + "</html>");
    206167                }
    207168            }
    208             throw new IllegalStateException("Not expecting keyword " + keyword);
     169        }
     170
     171        private static Match getTokenizer(String keyword, boolean caseSensitive, boolean regexSearch, PushbackTokenizer tokenizer)
     172                throws SearchParseError {
     173            switch (keyword) {
     174                case "id":
     175                    return new Id(tokenizer);
     176                case "version":
     177                    return new Version(tokenizer);
     178                case "type":
     179                    return new ExactType(tokenizer.readTextOrNumber());
     180                case "preset":
     181                    return new Preset(tokenizer.readTextOrNumber());
     182                case "user":
     183                    return new UserMatch(tokenizer.readTextOrNumber());
     184                case "role":
     185                    return new RoleMatch(tokenizer.readTextOrNumber());
     186                case "changeset":
     187                    return new ChangesetId(tokenizer);
     188                case "nodes":
     189                    return new NodeCountRange(tokenizer);
     190                case "ways":
     191                    return new WayCountRange(tokenizer);
     192                case "members":
     193                    return new MemberCountRange(tokenizer);
     194                case "tags":
     195                    return new TagCountRange(tokenizer);
     196                case "areasize":
     197                    return new AreaSize(tokenizer);
     198                case "waylength":
     199                    return new WayLength(tokenizer);
     200                case "nth":
     201                    return new Nth(tokenizer, false);
     202                case "nth%":
     203                    return new Nth(tokenizer, true);
     204                case "hasRole":
     205                    return new HasRole(tokenizer);
     206                case "timestamp":
     207                    // add leading/trailing space in order to get expected split (e.g. "a--" => {"a", ""})
     208                    String rangeS = ' ' + tokenizer.readTextOrNumber() + ' ';
     209                    String[] rangeA = rangeS.split("/", -1);
     210                    if (rangeA.length == 1) {
     211                        return new KeyValue(keyword, rangeS.trim(), regexSearch, caseSensitive);
     212                    } else if (rangeA.length == 2) {
     213                        return TimestampRange.create(rangeA);
     214                    } else {
     215                        throw new SearchParseError("<html>" + tr("Expecting {0} after {1}", "<i>min</i>/<i>max</i>", "<i>timestamp</i>")
     216                                + "</html>");
     217                    }
     218                default:
     219                    throw new IllegalStateException("Not expecting keyword " + keyword);
     220            }
    209221        }
    210222
    … …  
    215227    }
    216228
     229    /**
     230     * The core {@link UnaryMatch} factory
     231     */
    217232    public static class CoreUnaryMatchFactory implements UnaryMatchFactory {
    218233        private static final Collection<String> keywords = Arrays.asList("parent", "child");
    … …  
    242257    }
    243258
     259    /**
     260     * A factory for getting {@link Match} objects
     261     */
    244262    public interface SimpleMatchFactory extends MatchFactory {
     263        /**
     264         * Get the {@link Match} object
     265         * @param keyword The keyword to get/create the correct {@link Match} object
     266         * @param caseSensitive {@code true} if the search is case-sensitive
     267         * @param regexSearch {@code true} if the search is regex-based
     268         * @param tokenizer May be used to construct the {@link Match} object
     269         * @return The {@link Match} object for the keyword and its arguments
     270         * @throws SearchParseError If the {@link Match} object could not be constructed.
     271         */
    245272        Match get(String keyword, boolean caseSensitive, boolean regexSearch, PushbackTokenizer tokenizer) throws SearchParseError;
    246273    }
    247274
     275    /**
     276     * A factory for getting {@link UnaryMatch} objects
     277     */
    248278    public interface UnaryMatchFactory extends MatchFactory {
     279        /**
     280         * Get the {@link UnaryMatch} object
     281         * @param keyword The keyword to get/create the correct {@link UnaryMatch} object
     282         * @param matchOperand May be used to construct the {@link UnaryMatch} object
     283         * @param tokenizer May be used to construct the {@link UnaryMatch} object
     284         * @return The {@link UnaryMatch} object for the keyword and its arguments
     285         * @throws SearchParseError If the {@link UnaryMatch} object could not be constructed.
     286         */
    249287        UnaryMatch get(String keyword, Match matchOperand, PushbackTokenizer tokenizer) throws SearchParseError;
    250288    }
    251289
     290    /**
     291     * A factor for getting {@link AbstractBinaryMatch} objects
     292     */
    252293    public interface BinaryMatchFactory extends MatchFactory {
     294        /**
     295         * Get the {@link AbstractBinaryMatch} object
     296         * @param keyword The keyword to get/create the correct {@link AbstractBinaryMatch} object
     297         * @param lhs May be used to construct the {@link AbstractBinaryMatch} object (see {@link AbstractBinaryMatch#getLhs()})
     298         * @param rhs May be used to construct the {@link AbstractBinaryMatch} object (see {@link AbstractBinaryMatch#getRhs()})
     299         * @param tokenizer May be used to construct the {@link AbstractBinaryMatch} object
     300         * @return The {@link AbstractBinaryMatch} object for the keyword and its arguments
     301         * @throws SearchParseError If the {@link AbstractBinaryMatch} object could not be constructed.
     302         */
    253303        AbstractBinaryMatch get(String keyword, Match lhs, Match rhs, PushbackTokenizer tokenizer) throws SearchParseError;
    254304    }
    … …  
    303353    }
    304354
     355    /**
     356     * A common subclass of {@link Match} for matching against tags
     357     */
    305358    public abstract static class TaggedMatch extends Match {
    306359
    … …  
    330383     */
    331384    public abstract static class UnaryMatch extends Match {
    332 
     385        @Nonnull
    333386        protected final Match match;
    334387
    335         protected UnaryMatch(Match match) {
     388        protected UnaryMatch(@Nullable Match match) {
    336389            if (match == null) {
    337390                // "operator" (null) should mean the same as "operator()"
    … …  
    349402        @Override
    350403        public int hashCode() {
    351             return 31 + ((match == null) ? 0 : match.hashCode());
     404            return 31 + match.hashCode();
    352405        }
    353406
    … …  
    359412                return false;
    360413            UnaryMatch other = (UnaryMatch) obj;
    361             if (match == null) {
    362                 if (other.match != null)
    363                     return false;
    364             } else if (!match.equals(other.match))
    365                 return false;
    366             return true;
     414            return match.equals(other.match);
    367415        }
    368416    }
    … …  
    430478                return false;
    431479            AbstractBinaryMatch other = (AbstractBinaryMatch) obj;
    432             if (lhs == null) {
    433                 if (other.lhs != null)
    434                     return false;
    435             } else if (!lhs.equals(other.lhs))
    436                 return false;
    437             if (rhs == null) {
    438                 if (other.rhs != null)
    439                     return false;
    440             } else if (!rhs.equals(other.rhs))
    441                 return false;
    442             return true;
     480            return Objects.equals(lhs, other.lhs) && Objects.equals(rhs, other.rhs);
    443481        }
    444482    }
    … …  
    536574            if (defaultValue != other.defaultValue)
    537575                return false;
    538             if (key == null) {
    539                 if (other.key != null)
    540                     return false;
    541             } else if (!key.equals(other.key))
    542                 return false;
    543             return true;
     576            return Objects.equals(key, other.key);
    544577        }
    545578    }
    … …  
    794827    }
    795828
     829    /**
     830     * Match a primitive based off of a value comparison. This currently supports:
     831     * <ul>
     832     *     <li>ISO8601 dates (YYYY-MM-DD)</li>
     833     *     <li>Numbers</li>
     834     *     <li>Alpha-numeric comparison</li>
     835     * </ul>
     836     */
    796837    public static class ValueComparison extends TaggedMatch {
    797838        private final String key;
    … …  
    801842        private static final Pattern ISO8601 = Pattern.compile("\\d+-\\d+-\\d+");
    802843
     844        /**
     845         * Create a new {@link ValueComparison} object
     846         * @param key The key to get the value from
     847         * @param referenceValue The value to compare to
     848         * @param compareMode The compare mode to use; {@code < 0} is {@code currentValue < referenceValue} and
     849         *                    {@code > 0} is {@code currentValue > referenceValue}. {@code 0} is effectively an equality check.
     850         */
    803851        public ValueComparison(String key, String referenceValue, int compareMode) {
    804852            this.key = key;
    … …  
    809857                    v = Double.valueOf(referenceValue);
    810858                }
    811             } catch (NumberFormatException ignore) {
    812                 Logging.trace(ignore);
     859            } catch (NumberFormatException numberFormatException) {
     860                Logging.trace(numberFormatException);
    813861            }
    814862            this.referenceNumber = v;
    … …  
    855903            if (compareMode != other.compareMode)
    856904                return false;
    857             if (key == null) {
    858                 if (other.key != null)
    859                     return false;
    860             } else if (!key.equals(other.key))
    861                 return false;
    862             if (referenceNumber == null) {
    863                 if (other.referenceNumber != null)
    864                     return false;
    865             } else if (!referenceNumber.equals(other.referenceNumber))
    866                 return false;
    867             if (referenceValue == null) {
    868                 if (other.referenceValue != null)
    869                     return false;
    870             } else if (!referenceValue.equals(other.referenceValue))
    871                 return false;
    872             return true;
     905            return Objects.equals(key, other.key)
     906                    && Objects.equals(referenceNumber, other.referenceNumber)
     907                    && Objects.equals(referenceValue, other.referenceValue);
    873908        }
    874909
    … …  
    895930    public static class ExactKeyValue extends TaggedMatch {
    896931
     932        /**
     933         * The mode to use for the comparison
     934         */
    897935        public enum Mode {
    898             ANY, ANY_KEY, ANY_VALUE, EXACT, NONE, MISSING_KEY,
    899             ANY_KEY_REGEXP, ANY_VALUE_REGEXP, EXACT_REGEXP, MISSING_KEY_REGEXP;
     936            /** Matches everything */
     937            ANY,
     938            /** Any key with the specified value will match */
     939            ANY_KEY,
     940            /** Any value with the specified key will match */
     941            ANY_VALUE,
     942            /** A key with the specified value will match */
     943            EXACT,
     944            /** Nothing matches */
     945            NONE,
     946            /** The key does not exist */
     947            MISSING_KEY,
     948            /** Similar to {@link #ANY_KEY}, but the value matches a regex */
     949            ANY_KEY_REGEXP,
     950            /** Similar to {@link #ANY_VALUE}, but the key matches a regex */
     951            ANY_VALUE_REGEXP,
     952            /** Both the key and the value matches their respective regex */
     953            EXACT_REGEXP,
     954            /** No key matching the regex exists */
     955            MISSING_KEY_REGEXP
    900956        }
    901957
    … …  
    9711027                return false;
    9721028            case MISSING_KEY:
    973                 return !osm.hasTag(key);
     1029                return !osm.hasKey(key);
    9741030            case ANY:
    9751031                return true;
    9761032            case ANY_VALUE:
    977                 return osm.hasTag(key);
     1033                return osm.hasKey(key);
    9781034            case ANY_KEY:
    9791035                return osm.getKeys().values().stream().anyMatch(v -> v.equals(value));
    … …  
    10211077                return false;
    10221078            ExactKeyValue other = (ExactKeyValue) obj;
    1023             if (key == null) {
    1024                 if (other.key != null)
    1025                     return false;
    1026             } else if (!key.equals(other.key))
    1027                 return false;
    1028             if (keyPattern == null) {
    1029                 if (other.keyPattern != null)
    1030                     return false;
    1031             } else if (!keyPattern.equals(other.keyPattern))
    1032                 return false;
    10331079            if (mode != other.mode)
    10341080                return false;
    1035             if (value == null) {
    1036                 if (other.value != null)
    1037                     return false;
    1038             } else if (!value.equals(other.value))
    1039                 return false;
    1040             if (valuePattern == null) {
    1041                 if (other.valuePattern != null)
    1042                     return false;
    1043             } else if (!valuePattern.equals(other.valuePattern))
    1044                 return false;
    1045             return true;
     1081            return Objects.equals(key, other.key)
     1082                    && Objects.equals(value, other.value)
     1083                    && Objects.equals(keyPattern, other.keyPattern)
     1084                    && Objects.equals(valuePattern, other.valuePattern);
    10461085        }
    10471086    }
    … …  
    11241163            if (caseSensitive != other.caseSensitive)
    11251164                return false;
    1126             if (search == null) {
    1127                 if (other.search != null)
    1128                     return false;
    1129             } else if (!search.equals(other.search))
    1130                 return false;
    1131             if (searchRegex == null) {
    1132                 if (other.searchRegex != null)
    1133                     return false;
    1134             } else if (!searchRegex.equals(other.searchRegex))
    1135                 return false;
    1136             return true;
    1137         }
    1138     }
    1139 
     1165            return Objects.equals(search, other.search)
     1166                    && Objects.equals(searchRegex, other.searchRegex);
     1167        }
     1168    }
     1169
     1170    /**
     1171     * Filter OsmPrimitives based off of the base primitive type
     1172     */
    11401173    public static class ExactType extends Match {
    11411174        private final OsmPrimitiveType type;
    … …  
    12201253                return false;
    12211254            UserMatch other = (UserMatch) obj;
    1222             if (user == null) {
    1223                 if (other.user != null)
    1224                     return false;
    1225             } else if (!user.equals(other.user))
    1226                 return false;
    1227             return true;
     1255            return Objects.equals(user, other.user);
    12281256        }
    12291257    }
    … …  
    12331261     */
    12341262    private static class RoleMatch extends Match {
     1263        @Nonnull
    12351264        private final String role;
    12361265
    … …  
    12591288        @Override
    12601289        public int hashCode() {
    1261             return 31 + ((role == null) ? 0 : role.hashCode());
     1290            return 31 + role.hashCode();
    12621291        }
    12631292
    … …  
    12691298                return false;
    12701299            RoleMatch other = (RoleMatch) obj;
    1271             if (role == null) {
    1272                 if (other.role != null)
    1273                     return false;
    1274             } else if (!role.equals(other.role))
    1275                 return false;
    1276             return true;
     1300            return role.equals(other.role);
    12771301        }
    12781302    }
    … …  
    12831307    private static class Nth extends Match {
    12841308
    1285         private final int nth;
     1309        private final int nthObject;
    12861310        private final boolean modulo;
    12871311
    … …  
    12911315
    12921316        private Nth(int nth, boolean modulo) throws SearchParseError {
    1293             this.nth = nth;
     1317            this.nthObject = nth;
    12941318            this.modulo = modulo;
    1295             if (this.modulo && this.nth == 0) {
     1319            if (this.modulo && this.nthObject == 0) {
    12961320                throw new SearchParseError(tr("Non-zero integer expected"));
    12971321            }
    … …  
    13141338                    continue;
    13151339                }
    1316                 if (nth < 0 && idx - maxIndex == nth) {
     1340                if (nthObject < 0 && idx - maxIndex == nthObject) {
    13171341                    return true;
    1318                 } else if (idx == nth || (modulo && idx % nth == 0))
     1342                } else if (idx == nthObject || (modulo && idx % nthObject == 0))
    13191343                    return true;
    13201344            }
    … …  
    13241348        @Override
    13251349        public String toString() {
    1326             return "Nth{nth=" + nth + ", modulo=" + modulo + '}';
     1350            return "Nth{nth=" + nthObject + ", modulo=" + modulo + '}';
    13271351        }
    13281352
    13291353        @Override
    13301354        public int hashCode() {
    1331             return Objects.hash(modulo, nth);
     1355            return Objects.hash(modulo, nthObject);
    13321356        }
    13331357
    … …  
    13401364            Nth other = (Nth) obj;
    13411365            return modulo == other.modulo
    1342                    && nth == other.nth;
     1366                   && nthObject == other.nthObject;
    13431367        }
    13441368    }
    … …  
    15211545            final long maxDate;
    15221546            try {
    1523                 // if min timestamp is empty: use lowest possible date
     1547                // if min timestamp is empty: use the lowest possible date
    15241548                minDate = DateUtils.parseInstant(rangeA1.isEmpty() ? "1980" : rangeA1).toEpochMilli();
    15251549            } catch (UncheckedParseException | DateTimeException ex) {
    … …  
    15731597                return false;
    15741598            HasRole other = (HasRole) obj;
    1575             if (role == null) {
    1576                 if (other.role != null)
    1577                     return false;
    1578             } else if (!role.equals(other.role))
    1579                 return false;
    1580             return true;
     1599            return Objects.equals(role, other.role);
    15811600        }
    15821601    }
    … …  
    16441663    /**
    16451664     * Match objects that are incomplete, where only id and type are known.
    1646      * Typically some members of a relation are incomplete until they are
     1665     * Typically, some members of a relation are incomplete until they are
    16471666     * fetched from the server.
    16481667     */
    … …  
    18651884    /**
    18661885     * Matches objects which are not outside the source area ("downloaded area").
    1867      * Unlike {@link InDataSourceArea} this matches also if no source area is set (e.g., for new layers).
     1886     * Unlike {@link InDataSourceArea}, this matches also if no source area is set (e.g., for new layers).
    18681887     */
    18691888    public static class NotOutsideDataSourceArea extends InDataSourceArea {
    … …  
    19551974                return false;
    19561975            Preset other = (Preset) obj;
    1957             if (presets == null) {
    1958                 if (other.presets != null)
    1959                     return false;
    1960             } else if (!presets.equals(other.presets))
    1961                 return false;
    1962             return true;
     1976            return Objects.equals(presets, other.presets);
    19631977        }
    19641978    }
    … …  
    21562170                // key:value form where value is a string (may be OSM key search)
    21572171                final String value = tokenizer.readTextOrNumber();
    2158                 return new KeyValue(key, value != null ? value : "", regexSearch, caseSensitive).validate();
     2172                if (value == null) {
     2173                    return new ExactKeyValue(regexSearch, caseSensitive, key, "*").validate();
     2174                }
     2175                return new KeyValue(key, value, regexSearch, caseSensitive).validate();
    21592176            } else if (tokenizer.readIfEqual(Token.QUESTION_MARK))
    21602177                return new BooleanMatch(key, false);

  • trunk/src/org/openstreetmap/josm/tools/SearchCompilerQueryWizard.java

    r17988 r18847  
    3838    public static String constructQuery(final String search) {
    3939        try {
    40             Matcher matcher = Pattern.compile("\\s+GLOBAL\\s*$", Pattern.CASE_INSENSITIVE).matcher(search);
     40            Matcher matcher = Pattern.compile("\\s+GLOBAL\\s*$", Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CHARACTER_CLASS)
     41                    .matcher(search);
    4142            if (matcher.find()) {
    4243                final Match match = SearchCompiler.compile(matcher.replaceFirst(""));
    … …  
    4445            }
    4546
    46             matcher = Pattern.compile("\\s+IN BBOX\\s*$", Pattern.CASE_INSENSITIVE).matcher(search);
     47            matcher = Pattern.compile("\\s+IN BBOX\\s*$", Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CHARACTER_CLASS)
     48                    .matcher(search);
    4749            if (matcher.find()) {
    4850                final Match match = SearchCompiler.compile(matcher.replaceFirst(""));
    … …  
    5052            }
    5153
    52             matcher = Pattern.compile("\\s+(?<mode>IN|AROUND)\\s+(?<area>[^\" ]+|\"[^\"]+\")\\s*$", Pattern.CASE_INSENSITIVE).matcher(search);
     54            matcher = Pattern.compile("\\s+(?<mode>IN|AROUND)\\s+(?<area>[^\" ]+|\"[^\"]+\")\\s*$",
     55                    Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CHARACTER_CLASS).matcher(search);
    5356            if (matcher.find()) {
    5457                final Match match = SearchCompiler.compile(matcher.replaceFirst(""));
    … …  
    7982            final EnumSet<OsmPrimitiveType> types = EnumSet.noneOf(OsmPrimitiveType.class);
    8083            final String query = constructQuery(conjunction, types);
    81             (types.isEmpty() || types.size() == 3
     84            queryLines.addAll((types.isEmpty() || types.size() == 3
    8285                    ? Stream.of("nwr")
    8386                    : types.stream().map(OsmPrimitiveType::getAPIName))
    84                     .forEach(type -> queryLines.add("  " + type + query + queryLineSuffix + ";"));
     87                    .map(type -> "  " + type + query + queryLineSuffix + ";")
     88                    .collect(Collectors.toList()));
    8589        }
    8690        queryLines.add(");");
Note: See TracChangeset for help on using the changeset viewer.