This is an automated email from the ASF dual-hosted git repository. krickert pushed a commit to branch gazetteer-api in repository https://gitbox.apache.org/repos/asf/opennlp.git
commit 971703f0befc2a614d471cd4ad495eb036c0a661 Author: Kristian Rickert <[email protected]> AuthorDate: Thu Jul 16 06:16:59 2026 -0400 OPENNLP-1879: Align the byRegion contract across loaders, name format constants, fix test charsets, cite standards --- .../src/main/java/opennlp/tools/geo/Gazetteer.java | 12 +++-- .../java/opennlp/tools/geo/GazetteerEntry.java | 7 +-- .../src/main/java/opennlp/tools/geo/GeoPoint.java | 5 +- .../opennlp-geo/dev/derive-overture-divisions.py | 7 ++- .../opennlp-geo/dev/derive-populated-places.py | 32 +++++++---- opennlp-extensions/opennlp-geo/pom.xml | 6 +++ .../main/java/opennlp/geo/BundledGazetteer.java | 43 ++++++--------- .../src/main/java/opennlp/geo/GazetteerIndex.java | 51 ++++++++++++++++-- .../main/java/opennlp/geo/GeoNamesGazetteer.java | 17 ++++-- .../src/main/java/opennlp/geo/GeocoderInput.java | 62 ++++++++++++++++++++++ .../main/java/opennlp/geo/OvertureGazetteer.java | 20 ++++--- .../java/opennlp/geo/PopulationPriorGeocoder.java | 17 +----- .../java/opennlp/geo/SpatialCoherenceGeocoder.java | 25 ++++----- .../java/opennlp/geo/BundledDataAuditTest.java | 29 +++++----- .../java/opennlp/geo/BundledGazetteerTest.java | 21 +++++--- .../java/opennlp/geo/GeoNamesGazetteerTest.java | 45 ++++++++++++---- .../java/opennlp/geo/OvertureGazetteerTest.java | 45 ++++++++++++---- .../opennlp/geo/SpatialCoherenceGeocoderTest.java | 3 +- 18 files changed, 309 insertions(+), 138 deletions(-) diff --git a/opennlp-api/src/main/java/opennlp/tools/geo/Gazetteer.java b/opennlp-api/src/main/java/opennlp/tools/geo/Gazetteer.java index 15a2dd38f..2e3a8290a 100644 --- a/opennlp-api/src/main/java/opennlp/tools/geo/Gazetteer.java +++ b/opennlp-api/src/main/java/opennlp/tools/geo/Gazetteer.java @@ -61,14 +61,16 @@ public interface Gazetteer { Optional<GazetteerEntry> byId(String source, String recordId) throws IOException; /** - * Finds a representative entry for an ISO 3166-1 alpha-2 region code. + * Finds a representative entry for an + * <a href="https://www.iso.org/iso-3166-country-codes.html";>ISO 3166-1</a> alpha-2 region code. * - * @param isoCountryCode The ISO 3166-1 alpha-2 code, two ASCII capital letters. Must not be - * {@code null}. + * @param isoCountryCode The ISO 3166-1 alpha-2 code, two ASCII letters of either case. Must + * not be {@code null}. * @return A representative entry for the region (which entry represents a region is the * implementation's documented choice, for example the capital or the most populous record), - * or empty when the region is unknown to this gazetteer. - * @throws IllegalArgumentException Thrown if {@code isoCountryCode} is {@code null}. + * or empty when the code is well-formed but the region is unknown to this gazetteer. + * @throws IllegalArgumentException Thrown if {@code isoCountryCode} is {@code null} or is not + * two ASCII letters. * @throws IOException Thrown if a backing store or remote service fails. An in-memory * implementation never throws it. */ diff --git a/opennlp-api/src/main/java/opennlp/tools/geo/GazetteerEntry.java b/opennlp-api/src/main/java/opennlp/tools/geo/GazetteerEntry.java index 0e7a02119..2e882e883 100644 --- a/opennlp-api/src/main/java/opennlp/tools/geo/GazetteerEntry.java +++ b/opennlp-api/src/main/java/opennlp/tools/geo/GazetteerEntry.java @@ -27,9 +27,10 @@ import opennlp.tools.commons.ThreadSafe; * anything a dataset knows beyond the common core goes into the {@link #attributes() attributes} * map as provenance-tagged {@link AttributeValue}s. * - * <p>The {@link #countryCode() country code} is an ISO 3166-1 alpha-2 code. Attribute keys follow - * the {@code ATTRIBUTE_KEY_*} convention published as constants on this record; an entry only - * carries the keys its source actually provides, each with provenance in + * <p>The {@link #countryCode() country code} is an + * <a href="https://www.iso.org/iso-3166-country-codes.html";>ISO 3166-1</a> alpha-2 code. + * Attribute keys follow the {@code ATTRIBUTE_KEY_*} convention published as constants on this + * record; an entry only carries the keys its source actually provides, each with provenance in * {@link AttributeValue#source()}, and a key is never fabricated.</p> * * <p>Instances are immutable and thread-safe: the list and map components are defensively copied diff --git a/opennlp-api/src/main/java/opennlp/tools/geo/GeoPoint.java b/opennlp-api/src/main/java/opennlp/tools/geo/GeoPoint.java index 4333c755f..ae7557a3d 100644 --- a/opennlp-api/src/main/java/opennlp/tools/geo/GeoPoint.java +++ b/opennlp-api/src/main/java/opennlp/tools/geo/GeoPoint.java @@ -19,8 +19,9 @@ package opennlp.tools.geo; import opennlp.tools.commons.ThreadSafe; /** - * A geographic point in WGS84 decimal degrees. Both coordinates are range-validated at - * construction and neither may be {@code NaN}. + * A geographic point in <a href="https://epsg.org/crs_4326/WGS-84.html";>WGS84 (EPSG:4326)</a> + * decimal degrees. Both coordinates are range-validated at construction and neither may be + * {@code NaN}. * * <p>Instances are immutable and thread-safe.</p> * diff --git a/opennlp-extensions/opennlp-geo/dev/derive-overture-divisions.py b/opennlp-extensions/opennlp-geo/dev/derive-overture-divisions.py index 721c5ea72..98444357b 100644 --- a/opennlp-extensions/opennlp-geo/dev/derive-overture-divisions.py +++ b/opennlp-extensions/opennlp-geo/dev/derive-overture-divisions.py @@ -46,6 +46,9 @@ import duckdb KEPT_SUBTYPES = ("country", "dependency", "region", "county", "localadmin", "locality") +# The separator between the fields of one output row. +FIELD_SEPARATOR = "\t" + def main() -> None: if len(sys.argv) < 2: @@ -102,9 +105,9 @@ def main() -> None: country, subtype, str(int(population)), ) for field in fields: - if "\t" in field or "\n" in field: + if FIELD_SEPARATOR in field or "\n" in field: sys.exit(f"field contains a separator, refusing to emit: {fields}") - out.write("\t".join(fields) + "\n") + out.write(FIELD_SEPARATOR.join(fields) + "\n") kept += 1 print(f"wrote {kept} divisions to {output}") diff --git a/opennlp-extensions/opennlp-geo/dev/derive-populated-places.py b/opennlp-extensions/opennlp-geo/dev/derive-populated-places.py index 818cbeadb..000a2f565 100644 --- a/opennlp-extensions/opennlp-geo/dev/derive-populated-places.py +++ b/opennlp-extensions/opennlp-geo/dev/derive-populated-places.py @@ -40,6 +40,18 @@ import unicodedata EXTRACTION_DATE = '2026-07-06' MIRROR_COMMIT = '789c9904087846cc3361302857aa2e76b0ae71ff' +# The separator between the fields of one output row. +FIELD_SEPARATOR = ';' +# The separator between the elements of a list-valued output field. +LIST_SEPARATOR = '|' +# The number of field separators in a well-formed output row (one less than the field count). +SEPARATOR_COUNT = 10 +# The Unicode replacement character, an upstream marker of encoding damage. +# Written as an escape so this file stays pure ASCII. +REPLACEMENT_CHAR = '\uFFFD' +# The first code point beyond the ASCII range. +ASCII_LIMIT = 128 + # Letters NFD mark-stripping cannot reduce to ASCII; the standard transliterations. # Written as escapes so this file stays pure ASCII. TRANSLIT = { @@ -164,7 +176,7 @@ def fold_ascii_tagged(value): tags[-1] = True continue replacement = TRANSLIT.get(ch, ch) - source_nonascii = ord(ch) >= 128 + source_nonascii = ord(ch) >= ASCII_LIMIT for c in replacement: out.append(c) tags.append(source_nonascii) @@ -182,7 +194,7 @@ def damaged_variant_of(alt, folded_name, accent_tags): def is_ascii(value): - return all(ord(c) < 128 for c in value) + return all(ord(c) < ASCII_LIMIT for c in value) def collapse_ws(value): @@ -191,7 +203,7 @@ def collapse_ws(value): def looks_corrupted(value): """True when an ASCII value carries a mojibake artifact signature.""" - if '?' in value or '\uFFFD' in value: + if '?' in value or REPLACEMENT_CHAR in value: return True if value in LEGIT_CASE_ANOMALIES: return False @@ -200,7 +212,7 @@ def looks_corrupted(value): def clean_field(value, what, ne_id): """A single table field: no ';' (field separator), no '|' (list separator), no newline.""" - for banned in (';', '|', '\n', '\r'): + for banned in (FIELD_SEPARATOR, LIST_SEPARATOR, '\n', '\r'): if banned in value: raise SystemExit('Separator %r inside %s of NE_ID %s: %r' % (banned, what, ne_id, value)) return value @@ -267,7 +279,7 @@ def main(): for alt_field in ('NAMEALT', 'NAMEPAR', 'MEGANAME'): value = (p.get(alt_field) or '').strip() if value: - raw_alts.extend(part.strip() for part in value.split('|')) + raw_alts.extend(part.strip() for part in value.split(LIST_SEPARATOR)) for raw in raw_alts: if not raw: continue @@ -314,7 +326,7 @@ def main(): used_repairs.add(('ADM1', adm0, adm1)) adm1 = ADM1_REPAIRS[(adm0, adm1)] counts['adm1_repaired'] += 1 - elif '?' in adm1 or '\uFFFD' in adm1: + elif '?' in adm1 or REPLACEMENT_CHAR in adm1: counts['adm1_omitted_damaged'] += 1 adm1 = '' elif not is_ascii(adm1): @@ -348,22 +360,22 @@ def main(): raise SystemExit('Non-integral WOF_ID for NE_ID %s: %r' % (ne_id, wof)) attributes.append('whosonfirst=%d' % int(float(wof))) - row = ';'.join([ + row = FIELD_SEPARATOR.join([ 'naturalearth', str(ne_id), name, - '|'.join(alt_names), + LIST_SEPARATOR.join(alt_names), '%.5f' % lat, '%.5f' % lon, iso, adm1, str(population), 'CITY', - '|'.join(attributes), + LIST_SEPARATOR.join(attributes), ]) if not is_ascii(row): raise SystemExit('Non-ASCII row for NE_ID %s: %r' % (ne_id, row)) - if row.count(';') != 10: + if row.count(FIELD_SEPARATOR) != SEPARATOR_COUNT: raise SystemExit('Field-count drift for NE_ID %s: %r' % (ne_id, row)) rows.append((ne_id, row)) diff --git a/opennlp-extensions/opennlp-geo/pom.xml b/opennlp-extensions/opennlp-geo/pom.xml index 4a81a23b2..6f4c8ed8e 100644 --- a/opennlp-extensions/opennlp-geo/pom.xml +++ b/opennlp-extensions/opennlp-geo/pom.xml @@ -52,6 +52,12 @@ <artifactId>junit-jupiter-engine</artifactId> <scope>test</scope> </dependency> + + <dependency> + <groupId>org.junit.jupiter</groupId> + <artifactId>junit-jupiter-params</artifactId> + <scope>test</scope> + </dependency> </dependencies> </project> diff --git a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/BundledGazetteer.java b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/BundledGazetteer.java index b2f5c85aa..9b69578c0 100644 --- a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/BundledGazetteer.java +++ b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/BundledGazetteer.java @@ -46,9 +46,10 @@ import opennlp.tools.util.normalizer.TermAnalyzer; * thread-safe. * * <p>Indexed names and queries are folded through the same normalization chain (NFC, case fold, - * accent fold with {@link TermAnalyzer} over UAX #29 word tokens), so matching is robust to - * case, accents, and hyphenation. The bundled table is pure ASCII; native-script names are not - * matchable against it.</p> + * accent fold with {@link TermAnalyzer} over + * <a href="https://unicode.org/reports/tr29/";>UAX #29</a> word tokens), so matching is + * robust to case, accents, and hyphenation. The bundled table is pure ASCII; native-script names + * are not matchable against it.</p> * * <p>{@link #lookup(CharSequence)} returns candidates ordered by population descending, then a * feature-class prior ({@link GazetteerEntry#FEATURE_CLASS_CITY} before @@ -62,6 +63,12 @@ public final class BundledGazetteer implements Gazetteer { private static final String RESOURCE = "naturalearth-populated-places.txt"; + /** The number of semicolon separated fields in one row of the bundled table format. */ + private static final int FIELD_COUNT = 11; + + /** The number of semicolons separating {@link #FIELD_COUNT} fields. */ + private static final int SEPARATOR_COUNT = FIELD_COUNT - 1; + // The character-level matching chain; stateless and thread-safe, shared by index and queries. private static final TermAnalyzer FOLD = TermAnalyzer.builder().nfc().caseFold().accentFold().build(); @@ -174,17 +181,7 @@ public final class BundledGazetteer implements Gazetteer { /** {@inheritDoc} */ @Override public Optional<GazetteerEntry> byRegion(String isoCountryCode) { - if (isoCountryCode == null) { - throw new IllegalArgumentException("IsoCountryCode must not be null"); - } - if (isoCountryCode.length() != 2 - || !isAsciiLetter(isoCountryCode.charAt(0)) || !isAsciiLetter(isoCountryCode.charAt(1))) { - throw new IllegalArgumentException( - "IsoCountryCode must be an ISO 3166-1 alpha-2 code (two ASCII letters), got: " - + isoCountryCode); - } - final String key = new String(new char[] {upperAscii(isoCountryCode.charAt(0)), - upperAscii(isoCountryCode.charAt(1))}); + final String key = GazetteerIndex.normalizeRegionCode(isoCountryCode); return Optional.ofNullable(regionIndex.get(key)); } @@ -254,11 +251,11 @@ public final class BundledGazetteer implements Gazetteer { * {@code resourceName} and {@code lineNumber}. */ private static GazetteerEntry parseRow(String line, String resourceName, int lineNumber) { - // Scan the line into exactly 11 semicolon-separated fields. - final String[] fields = new String[11]; + // Scan the line into exactly FIELD_COUNT semicolon-separated fields. + final String[] fields = new String[FIELD_COUNT]; int fieldCount = 0; int start = 0; - while (fieldCount < 10) { + while (fieldCount < SEPARATOR_COUNT) { final int semicolon = line.indexOf(';', start); if (semicolon < 0) { throw malformed(resourceName, lineNumber, line, null); @@ -269,7 +266,7 @@ public final class BundledGazetteer implements Gazetteer { if (line.indexOf(';', start) >= 0) { throw malformed(resourceName, lineNumber, line, null); } - fields[10] = line.substring(start); + fields[SEPARATOR_COUNT] = line.substring(start); try { final double latitude = Double.parseDouble(fields[4]); final double longitude = Double.parseDouble(fields[5]); @@ -400,16 +397,6 @@ public final class BundledGazetteer implements Gazetteer { } } - /** {@return {@code true} if {@code c} is an ASCII letter}. */ - private static boolean isAsciiLetter(char c) { - return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'); - } - - /** {@return {@code c} upper-cased if it is an ASCII lowercase letter, otherwise unchanged}. */ - private static char upperAscii(char c) { - return c >= 'a' && c <= 'z' ? (char) (c - ('a' - 'A')) : c; - } - /** * Holds the shared instance, initialized on first access to {@link #getInstance()} by the * class loader without locking. diff --git a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/GazetteerIndex.java b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/GazetteerIndex.java index a5c0ea3dd..da63adbf9 100644 --- a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/GazetteerIndex.java +++ b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/GazetteerIndex.java @@ -61,7 +61,7 @@ final class GazetteerIndex { } } - /** @return {@code true} if nothing was indexed. */ + /** {@return {@code true} if nothing was indexed} */ boolean isEmpty() { return byId.isEmpty(); } @@ -77,11 +77,46 @@ final class GazetteerIndex { return Optional.ofNullable(byId.get(recordId)); } - /** Finds the most populous entry of a country. */ + /** + * Finds the most populous entry of a country. + * + * @param isoCountryCode The <a href="https://www.iso.org/iso-3166-country-codes.html";>ISO + * 3166-1</a> alpha-2 code, two ASCII letters of either case. Must + * not be {@code null}. + * @return The most populous entry, or empty when the code is well-formed but unknown. + * @throws IllegalArgumentException Thrown if {@code isoCountryCode} is {@code null} or is not + * two ASCII letters. + */ Optional<GazetteerEntry> byRegion(String isoCountryCode) { - return Optional.ofNullable(byCountry.get(isoCountryCode.toUpperCase(Locale.ROOT))); + return Optional.ofNullable(byCountry.get(normalizeRegionCode(isoCountryCode))); } + /** + * Validates an ISO 3166-1 alpha-2 region code and folds it to its canonical uppercase form. + * + * @param isoCountryCode The code to validate. Must not be {@code null}. + * @return The code with both letters upper-cased. + * @throws IllegalArgumentException Thrown if {@code isoCountryCode} is {@code null} or is not + * two ASCII letters. + */ + static String normalizeRegionCode(String isoCountryCode) { + if (isoCountryCode == null) { + throw new IllegalArgumentException("IsoCountryCode must not be null"); + } + if (isoCountryCode.length() != 2 + || !isAsciiLetter(isoCountryCode.charAt(0)) || !isAsciiLetter(isoCountryCode.charAt(1))) { + throw new IllegalArgumentException( + "IsoCountryCode must be an ISO 3166-1 alpha-2 code (two ASCII letters), got: " + + isoCountryCode); + } + return new String(new char[] {upperAscii(isoCountryCode.charAt(0)), + upperAscii(isoCountryCode.charAt(1))}); + } + + /** + * Indexes {@code entry} under the lower-cased form of {@code name}, listing it once even when + * several of its names fold to the same key. + */ private void index(String name, GazetteerEntry entry) { final List<GazetteerEntry> entries = byName.computeIfAbsent(name.toLowerCase(Locale.ROOT), key -> new ArrayList<>(2)); @@ -89,4 +124,14 @@ final class GazetteerIndex { entries.add(entry); } } + + /** {@return {@code true} if {@code c} is an ASCII letter} */ + private static boolean isAsciiLetter(char c) { + return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'); + } + + /** {@return {@code c} upper-cased if it is an ASCII lowercase letter, otherwise unchanged} */ + private static char upperAscii(char c) { + return c >= 'a' && c <= 'z' ? (char) (c - ('a' - 'A')) : c; + } } diff --git a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/GeoNamesGazetteer.java b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/GeoNamesGazetteer.java index 5deba75af..a7c0615ba 100644 --- a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/GeoNamesGazetteer.java +++ b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/GeoNamesGazetteer.java @@ -64,6 +64,12 @@ public final class GeoNamesGazetteer implements Gazetteer { private static final int COLUMNS = 19; + /** The separator between the fields of one row. */ + private static final String FIELD_SEPARATOR = "\t"; + + /** The separator between the elements of the alternate-names field. */ + private static final String LIST_SEPARATOR = ","; + private final GazetteerIndex index; private GeoNamesGazetteer(GazetteerIndex index) { @@ -121,6 +127,7 @@ public final class GeoNamesGazetteer implements Gazetteer { return new GeoNamesGazetteer(index); } + /** {@inheritDoc} */ @Override public List<GazetteerEntry> lookup(CharSequence name) { if (name == null) { @@ -129,6 +136,7 @@ public final class GeoNamesGazetteer implements Gazetteer { return index.lookup(name); } + /** {@inheritDoc} */ @Override public Optional<GazetteerEntry> byId(String source, String recordId) { if (source == null || recordId == null) { @@ -137,14 +145,13 @@ public final class GeoNamesGazetteer implements Gazetteer { return SOURCE.equals(source) ? index.byId(recordId) : Optional.empty(); } + /** {@inheritDoc} */ @Override public Optional<GazetteerEntry> byRegion(String isoCountryCode) { - if (isoCountryCode == null) { - throw new IllegalArgumentException("isoCountryCode must not be null"); - } return index.byRegion(isoCountryCode); } + /** {@inheritDoc} */ @Override public Set<String> sources() { return Set.of(SOURCE); @@ -152,7 +159,7 @@ public final class GeoNamesGazetteer implements Gazetteer { /** Parses one main-format row into an entry, failing loud with the line number. */ private static GazetteerEntry parseRow(String line, int lineNumber) { - final String[] fields = line.split("\t", -1); + final String[] fields = line.split(FIELD_SEPARATOR, -1); if (fields.length < COLUMNS) { throw new IllegalArgumentException("line " + lineNumber + " has " + fields.length + " columns, expected " + COLUMNS); @@ -165,7 +172,7 @@ public final class GeoNamesGazetteer implements Gazetteer { if (!ascii.isEmpty() && !ascii.equals(name)) { alternates.add(ascii); } - for (final String alternate : fields[3].split(",")) { + for (final String alternate : fields[3].split(LIST_SEPARATOR)) { final String trimmed = alternate.trim(); if (!trimmed.isEmpty() && !trimmed.equals(name)) { alternates.add(trimmed); diff --git a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/GeocoderInput.java b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/GeocoderInput.java new file mode 100644 index 000000000..df6a21240 --- /dev/null +++ b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/GeocoderInput.java @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package opennlp.geo; + +import java.util.List; + +import opennlp.tools.util.Span; + +/** + * The shared argument validation of this module's {@link opennlp.tools.geo.Geocoder} + * implementations. + */ +final class GeocoderInput { + + /** Prevents instantiation; this is a static utility. */ + private GeocoderInput() { + } + + /** + * Validates the arguments of + * {@link opennlp.tools.geo.Geocoder#resolve(CharSequence, List)}: the text and the + * mention list must not be {@code null}, the list must not contain a {@code null} element, + * and every mention must lie inside the text. + * + * @param text The text the mentions refer into. Must not be {@code null}. + * @param locationMentions The mention spans to validate. Must not be {@code null} or contain + * {@code null} elements, and no span may end beyond the text. + * @throws IllegalArgumentException Thrown if any of the above conditions is violated. + */ + static void validateResolveArguments(CharSequence text, List<Span> locationMentions) { + if (text == null) { + throw new IllegalArgumentException("Text must not be null"); + } + if (locationMentions == null) { + throw new IllegalArgumentException("LocationMentions must not be null"); + } + for (final Span mention : locationMentions) { + if (mention == null) { + throw new IllegalArgumentException( + "LocationMentions must not contain a null element, got: " + locationMentions); + } + if (mention.getEnd() > text.length()) { + throw new IllegalArgumentException("Mention " + mention + + " is outside the text, whose length is " + text.length()); + } + } + } +} diff --git a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/OvertureGazetteer.java b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/OvertureGazetteer.java index bf22bb224..6f80cc88d 100644 --- a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/OvertureGazetteer.java +++ b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/OvertureGazetteer.java @@ -40,7 +40,8 @@ import opennlp.tools.geo.GeoPoint; * per division with id, primary name, comma-separated alternate names, coordinates, * country code, Overture subtype, and population. * - * <p>The upstream divisions theme is published under the Open Database License, whose + * <p>The upstream divisions theme is published under the + * <a href="https://opendatacommons.org/licenses/odbl/1-0/";>Open Database License</a>, whose * attribution and database share-alike terms follow the derived table; it is * distributed as partitioned Parquet, which this module deliberately does not parse. * The derivation script flattens the division features into this plain table, and the @@ -67,6 +68,12 @@ public final class OvertureGazetteer implements Gazetteer { private static final int COLUMNS = 8; + /** The separator between the fields of one row. */ + private static final String FIELD_SEPARATOR = "\t"; + + /** The separator between the elements of the alternate-names field. */ + private static final String LIST_SEPARATOR = ","; + private static final Set<String> SUB_LOCALITY_SUBTYPES = Set.of("borough", "macrohood", "neighborhood", "microhood"); @@ -128,6 +135,7 @@ public final class OvertureGazetteer implements Gazetteer { return new OvertureGazetteer(index); } + /** {@inheritDoc} */ @Override public List<GazetteerEntry> lookup(CharSequence name) { if (name == null) { @@ -136,6 +144,7 @@ public final class OvertureGazetteer implements Gazetteer { return index.lookup(name); } + /** {@inheritDoc} */ @Override public Optional<GazetteerEntry> byId(String source, String recordId) { if (source == null || recordId == null) { @@ -144,14 +153,13 @@ public final class OvertureGazetteer implements Gazetteer { return SOURCE.equals(source) ? index.byId(recordId) : Optional.empty(); } + /** {@inheritDoc} */ @Override public Optional<GazetteerEntry> byRegion(String isoCountryCode) { - if (isoCountryCode == null) { - throw new IllegalArgumentException("isoCountryCode must not be null"); - } return index.byRegion(isoCountryCode); } + /** {@inheritDoc} */ @Override public Set<String> sources() { return Set.of(SOURCE); @@ -159,7 +167,7 @@ public final class OvertureGazetteer implements Gazetteer { /** Parses one derived row into an entry, failing loud with the line number. */ private static GazetteerEntry parseRow(String line, int lineNumber) { - final String[] fields = line.split("\t", -1); + final String[] fields = line.split(FIELD_SEPARATOR, -1); if (fields.length < COLUMNS) { throw new IllegalArgumentException("line " + lineNumber + " has " + fields.length + " columns, expected " + COLUMNS); @@ -168,7 +176,7 @@ public final class OvertureGazetteer implements Gazetteer { final String id = fields[0].trim(); final String name = fields[1].trim(); final Set<String> alternates = new LinkedHashSet<>(); - for (final String alternate : fields[2].split(",")) { + for (final String alternate : fields[2].split(LIST_SEPARATOR)) { final String trimmed = alternate.trim(); if (!trimmed.isEmpty() && !trimmed.equals(name)) { alternates.add(trimmed); diff --git a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/PopulationPriorGeocoder.java b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/PopulationPriorGeocoder.java index 5baf7f851..849187635 100644 --- a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/PopulationPriorGeocoder.java +++ b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/PopulationPriorGeocoder.java @@ -67,22 +67,7 @@ public final class PopulationPriorGeocoder implements Geocoder { @Override public List<GeoResolution> resolve(CharSequence text, List<Span> locationMentions) throws IOException { - if (text == null) { - throw new IllegalArgumentException("Text must not be null"); - } - if (locationMentions == null) { - throw new IllegalArgumentException("LocationMentions must not be null"); - } - for (final Span mention : locationMentions) { - if (mention == null) { - throw new IllegalArgumentException( - "LocationMentions must not contain a null element, got: " + locationMentions); - } - if (mention.getEnd() > text.length()) { - throw new IllegalArgumentException("Mention " + mention - + " is outside the text, whose length is " + text.length()); - } - } + GeocoderInput.validateResolveArguments(text, locationMentions); final List<GeoResolution> resolutions = new ArrayList<>(locationMentions.size()); for (final Span mention : locationMentions) { final CharSequence mentionText = text.subSequence(mention.getStart(), mention.getEnd()); diff --git a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/SpatialCoherenceGeocoder.java b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/SpatialCoherenceGeocoder.java index 604f867ac..dc51dc35e 100644 --- a/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/SpatialCoherenceGeocoder.java +++ b/opennlp-extensions/opennlp-geo/src/main/java/opennlp/geo/SpatialCoherenceGeocoder.java @@ -51,9 +51,16 @@ import opennlp.tools.util.Span; */ public final class SpatialCoherenceGeocoder implements Geocoder { + /** An unambiguous mention scores high but never certain, since gazetteers are incomplete. */ private static final double SINGLE_CANDIDATE_CONFIDENCE = 0.9; + + /** An ambiguous mention with no separating evidence scores at the middle of the scale. */ private static final double BASE_CONFIDENCE = 0.5; + + /** Separation evidence lifts the base score to at most {@link #SINGLE_CANDIDATE_CONFIDENCE}. */ private static final double SEPARATION_WEIGHT = 0.4; + + /** The mean Earth radius used by the haversine distance. */ private static final double EARTH_RADIUS_KM = 6371.0; private final Gazetteer gazetteer; @@ -75,22 +82,7 @@ public final class SpatialCoherenceGeocoder implements Geocoder { @Override public List<GeoResolution> resolve(CharSequence text, List<Span> locationMentions) throws IOException { - if (text == null) { - throw new IllegalArgumentException("Text must not be null"); - } - if (locationMentions == null) { - throw new IllegalArgumentException("LocationMentions must not be null"); - } - for (final Span mention : locationMentions) { - if (mention == null) { - throw new IllegalArgumentException( - "LocationMentions must not contain a null element, got: " + locationMentions); - } - if (mention.getEnd() > text.length()) { - throw new IllegalArgumentException("Mention " + mention - + " is outside the text, whose length is " + text.length()); - } - } + GeocoderInput.validateResolveArguments(text, locationMentions); // first pass: candidates per mention, provisional pick by the population prior final List<Span> resolvedMentions = new ArrayList<>(); @@ -160,6 +152,7 @@ public final class SpatialCoherenceGeocoder implements Geocoder { return first <= 0.0 ? 0.0 : 1.0 - (second / first); } + /** {@return the mean great-circle distance from {@code from} to the context points, in km} */ private static double meanDistanceKm(GeoPoint from, List<GeoPoint> context) { double sum = 0.0; for (final GeoPoint to : context) { diff --git a/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/BundledDataAuditTest.java b/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/BundledDataAuditTest.java index ee1f7b01e..b351c7cd3 100644 --- a/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/BundledDataAuditTest.java +++ b/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/BundledDataAuditTest.java @@ -29,6 +29,8 @@ import java.util.regex.Pattern; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import opennlp.tools.geo.GazetteerEntry; @@ -53,12 +55,16 @@ public class BundledDataAuditTest { GazetteerEntry.ATTRIBUTE_KEY_GEONAMES, GazetteerEntry.ATTRIBUTE_KEY_WHOSONFIRST); - // Upstream mojibake signature: an uppercase letter directly after a lowercase one, or a - // lowercase letter after two uppercase ones at a word start (an accented letter replaced by a - // wrong ASCII letter, e.g. 'MUdenine' for Medenine). Mirrors the generation script's detector. + // Upstream mojibake (encoding-corrupted text) signature: an uppercase letter directly after a + // lowercase one, or a lowercase letter after two uppercase ones at a word start (an accented + // letter replaced by a wrong ASCII letter, e.g. 'MUdenine' for Medenine). Mirrors the + // generation script's detector. private static final Pattern CASE_ANOMALY = Pattern.compile("[a-z][A-Z]|\\b[A-Z][A-Z][a-z]"); private static final Pattern MC_PREFIX = Pattern.compile("\\bMc(?=[A-Z])"); + /** The header's row-count claim, e.g. {@code 7343 rows.}, checked against the parsed data. */ + private static final Pattern ROW_COUNT_CLAIM = Pattern.compile("(\\d+) rows\\."); + /** Values the anomaly pattern flags that are verified correct upstream spellings. */ private static final Set<String> LEGIT_CASE_ANOMALIES = Set.of("KwaZulu-Natal", "HaMerkaz", "HaDarom", "HaZafon"); @@ -77,13 +83,12 @@ public class BundledDataAuditTest { @Test void testRowCountMatchesTheHeaderClaimExactly() throws IOException { // The header states the row count; a regeneration with a stale header fails here. - final Pattern claim = Pattern.compile("(\\d+) rows\\."); Integer claimed = null; try (BufferedReader reader = new BufferedReader(new InputStreamReader( BundledGazetteer.class.getResourceAsStream(RESOURCE), StandardCharsets.UTF_8))) { String line; while ((line = reader.readLine()) != null && line.startsWith("#")) { - final Matcher matcher = claim.matcher(line); + final Matcher matcher = ROW_COUNT_CLAIM.matcher(line); if (matcher.find()) { claimed = Integer.parseInt(matcher.group(1)); break; @@ -233,17 +238,15 @@ public class BundledDataAuditTest { gazetteer.byId("naturalearth", "1159112749").orElseThrow().containment()); } - @Test - void testByRegionServesTheRegionJoinKeyShape() { + @ParameterizedTest + @ValueSource(strings = {"US", "DE", "FR", "JP", "GB", "BR", "IN", "CN"}) + void testByRegionServesTheRegionJoinKeyShape(String code) { // ISO 3166-1 alpha-2 codes are the region join key of the seam (a flag emoji decodes to // the same code, for example). byRegion must answer for them, and the returned entry must // carry the very code it was asked for. - final BundledGazetteer gazetteer = BundledGazetteer.getInstance(); - for (final String code : new String[] {"US", "DE", "FR", "JP", "GB", "BR", "IN", "CN"}) { - final GazetteerEntry entry = gazetteer.byRegion(code).orElseThrow( - () -> new AssertionError("No entry for region " + code)); - assertEquals(code, entry.countryCode()); - } + final GazetteerEntry entry = BundledGazetteer.getInstance().byRegion(code).orElseThrow( + () -> new AssertionError("No entry for region " + code)); + assertEquals(code, entry.countryCode()); } @Test diff --git a/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/BundledGazetteerTest.java b/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/BundledGazetteerTest.java index 6f7e52f8a..aa2604dea 100644 --- a/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/BundledGazetteerTest.java +++ b/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/BundledGazetteerTest.java @@ -27,6 +27,8 @@ import java.util.Optional; import java.util.Set; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import opennlp.tools.geo.AttributeValue; import opennlp.tools.geo.GazetteerEntry; @@ -218,15 +220,18 @@ public class BundledGazetteerTest { } @Test - void testByRegionMalformedCodeFailsLoud() { + void testByRegionNullFailsLoud() { + assertThrows(IllegalArgumentException.class, () -> fixture().byRegion(null)); + } + + @ParameterizedTest + @ValueSource(strings = {"", "C", "CHE", "C1"}) + void testByRegionMalformedCodeFailsLoud(String malformed) { final BundledGazetteer gazetteer = fixture(); - assertThrows(IllegalArgumentException.class, () -> gazetteer.byRegion(null)); - for (final String malformed : new String[] {"", "C", "CHE", "C1"}) { - final IllegalArgumentException e = - assertThrows(IllegalArgumentException.class, () -> gazetteer.byRegion(malformed)); - assertTrue(e.getMessage().startsWith("IsoCountryCode must be an ISO 3166-1 alpha-2 code"), - e.getMessage()); - } + final IllegalArgumentException e = + assertThrows(IllegalArgumentException.class, () -> gazetteer.byRegion(malformed)); + assertTrue(e.getMessage().startsWith("IsoCountryCode must be an ISO 3166-1 alpha-2 code"), + e.getMessage()); } @Test diff --git a/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/GeoNamesGazetteerTest.java b/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/GeoNamesGazetteerTest.java index c3088110c..9cc8923ec 100644 --- a/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/GeoNamesGazetteerTest.java +++ b/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/GeoNamesGazetteerTest.java @@ -20,9 +20,14 @@ package opennlp.geo; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.Set; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; import opennlp.tools.geo.GazetteerEntry; @@ -51,7 +56,8 @@ public class GeoNamesGazetteerTest { "") + "\n"; private static GeoNamesGazetteer gazetteer() throws IOException { - return GeoNamesGazetteer.load(new ByteArrayInputStream(FIXTURE.getBytes())); + return GeoNamesGazetteer.load( + new ByteArrayInputStream(FIXTURE.getBytes(StandardCharsets.UTF_8))); } @Test @@ -87,19 +93,38 @@ public class GeoNamesGazetteerTest { assertTrue(gazetteer.byId("elsewhere", "2").isEmpty()); // the most populous entry represents the region assertEquals("Texas", gazetteer.byRegion("us").get().name()); - assertEquals(java.util.Set.of(GeoNamesGazetteer.SOURCE), gazetteer.sources()); + assertEquals(Set.of(GeoNamesGazetteer.SOURCE), gazetteer.sources()); } @Test - void testMalformedContentFailsLoud() { - assertThrows(IllegalArgumentException.class, - () -> GeoNamesGazetteer.load(new ByteArrayInputStream("too\tfew\tcolumns\n".getBytes()))); + void testByRegionUnknownCodeReturnsEmpty() throws IOException { + // well-formed but absent from the fixture + assertTrue(gazetteer().byRegion("XX").isEmpty()); + } + + @ParameterizedTest + @ValueSource(strings = {"", "C", "CHE", "C1"}) + void testByRegionMalformedCodeFailsLoud(String malformed) throws IOException { + final GeoNamesGazetteer gazetteer = gazetteer(); + assertThrows(IllegalArgumentException.class, () -> gazetteer.byRegion(malformed)); + } + + @ParameterizedTest + @MethodSource("malformedContent") + void testMalformedContentFailsLoud(String content) { assertThrows(IllegalArgumentException.class, () -> GeoNamesGazetteer.load( - new ByteArrayInputStream( - (row("5", "Nowhere", "Nowhere", "", "not-a-lat", "0", "P", "DE", "1") + "\n") - .getBytes()))); - assertThrows(IllegalArgumentException.class, - () -> GeoNamesGazetteer.load(new ByteArrayInputStream(new byte[0]))); + new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8)))); + } + + private static List<String> malformedContent() { + return List.of( + "too\tfew\tcolumns\n", + row("5", "Nowhere", "Nowhere", "", "not-a-lat", "0", "P", "DE", "1") + "\n", + ""); + } + + @Test + void testNullStreamFailsLoud() { assertThrows(IllegalArgumentException.class, () -> GeoNamesGazetteer.load((InputStream) null)); } diff --git a/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/OvertureGazetteerTest.java b/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/OvertureGazetteerTest.java index 7314fe843..922bae8dd 100644 --- a/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/OvertureGazetteerTest.java +++ b/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/OvertureGazetteerTest.java @@ -20,9 +20,14 @@ package opennlp.geo; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.Set; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; import opennlp.tools.geo.GazetteerEntry; @@ -51,7 +56,8 @@ public class OvertureGazetteerTest { "") + "\n"; private static OvertureGazetteer gazetteer() throws IOException { - return OvertureGazetteer.load(new ByteArrayInputStream(FIXTURE.getBytes())); + return OvertureGazetteer.load( + new ByteArrayInputStream(FIXTURE.getBytes(StandardCharsets.UTF_8))); } @Test @@ -85,19 +91,38 @@ public class OvertureGazetteerTest { assertTrue(gazetteer.byId("geonames", "d3").isEmpty()); // the most populous entry represents the region assertEquals("Australia", gazetteer.byRegion("au").get().name()); - assertEquals(java.util.Set.of(OvertureGazetteer.SOURCE), gazetteer.sources()); + assertEquals(Set.of(OvertureGazetteer.SOURCE), gazetteer.sources()); } @Test - void testMalformedContentFailsLoud() { - assertThrows(IllegalArgumentException.class, () -> OvertureGazetteer.load( - new ByteArrayInputStream("too\tfew\n".getBytes()))); - assertThrows(IllegalArgumentException.class, () -> OvertureGazetteer.load( - new ByteArrayInputStream( - (row("d9", "Nowhere", "", "not-a-lat", "0", "DE", "region", "1") + "\n") - .getBytes()))); + void testByRegionUnknownCodeReturnsEmpty() throws IOException { + // well-formed but absent from the fixture + assertTrue(gazetteer().byRegion("XX").isEmpty()); + } + + @ParameterizedTest + @ValueSource(strings = {"", "C", "CHE", "C1"}) + void testByRegionMalformedCodeFailsLoud(String malformed) throws IOException { + final OvertureGazetteer gazetteer = gazetteer(); + assertThrows(IllegalArgumentException.class, () -> gazetteer.byRegion(malformed)); + } + + @ParameterizedTest + @MethodSource("malformedContent") + void testMalformedContentFailsLoud(String content) { assertThrows(IllegalArgumentException.class, () -> OvertureGazetteer.load( - new ByteArrayInputStream("# only a header\n".getBytes()))); + new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8)))); + } + + private static List<String> malformedContent() { + return List.of( + "too\tfew\n", + row("d9", "Nowhere", "", "not-a-lat", "0", "DE", "region", "1") + "\n", + "# only a header\n"); + } + + @Test + void testNullStreamFailsLoud() { assertThrows(IllegalArgumentException.class, () -> OvertureGazetteer.load((InputStream) null)); } diff --git a/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/SpatialCoherenceGeocoderTest.java b/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/SpatialCoherenceGeocoderTest.java index 9a54bb958..dcc216fa3 100644 --- a/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/SpatialCoherenceGeocoderTest.java +++ b/opennlp-extensions/opennlp-geo/src/test/java/opennlp/geo/SpatialCoherenceGeocoderTest.java @@ -18,6 +18,7 @@ package opennlp.geo; import java.io.IOException; +import java.util.Arrays; import java.util.List; import java.util.Map; @@ -54,7 +55,7 @@ public class SpatialCoherenceGeocoderTest { private final SpatialCoherenceGeocoder geocoder = new SpatialCoherenceGeocoder(GAZETTEER); private static List<Span> mentionsOf(String text, String... names) { - return java.util.Arrays.stream(names) + return Arrays.stream(names) .map(name -> new Span(text.indexOf(name), text.indexOf(name) + name.length())) .toList(); }
