Skip to content

Commit 013124a

Browse files
committed
Significantly reduce performance impact
We were previously regenerating the article list on just about ''every'' dataset change. This included node movement. We fix that by listening for events that are ''only'' related to objects gaining or losing tags. This additionally fixes a bunch of lint issues. Signed-off-by: Taylor Smock <[email protected]>
1 parent e3f000b commit 013124a

File tree

2 files changed

+173
-96
lines changed

2 files changed

+173
-96
lines changed

src/main/java/org/wikipedia/WikipediaApp.java

Lines changed: 75 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.io.BufferedReader;
55
import java.io.IOException;
66
import java.io.InputStream;
7+
import java.io.UncheckedIOException;
78
import java.net.URL;
89
import java.nio.charset.StandardCharsets;
910
import java.util.AbstractList;
@@ -29,9 +30,11 @@
2930

3031
import org.openstreetmap.josm.data.coor.LatLon;
3132
import org.openstreetmap.josm.data.osm.OsmPrimitive;
33+
import org.openstreetmap.josm.data.osm.Tagged;
3234
import org.openstreetmap.josm.gui.Notification;
3335
import org.openstreetmap.josm.tools.HttpClient;
3436
import org.openstreetmap.josm.tools.I18n;
37+
import org.openstreetmap.josm.tools.JosmRuntimeException;
3538
import org.openstreetmap.josm.tools.LanguageInfo;
3639
import org.openstreetmap.josm.tools.Logging;
3740
import org.openstreetmap.josm.tools.Pair;
@@ -55,21 +58,28 @@ public final class WikipediaApp {
5558

5659
private static final String STRING_URI_PIPE = Utils.encodeUrl("|");
5760

61+
private static final String WIKIDATA = "wikidata";
62+
private static final String WIKIPEDIA = "wikipedia";
63+
64+
private final String[] wikipediaKeys;
5865
private final String wikipediaLang;
5966
private final SitematrixResult.Sitematrix.Site site;
6067

6168
private WikipediaApp(final String wikipediaLang) throws IOException {
6269
this.wikipediaLang = wikipediaLang;
70+
this.wikipediaKeys = new String[] {WIKIDATA, WIKIPEDIA, WIKIPEDIA + ':' + wikipediaLang};
6371

6472
final SitematrixResult.Sitematrix sitematrix = ApiQueryClient.query(WikidataActionApiQuery.sitematrix());
65-
final SitematrixResult.Sitematrix.Language language = sitematrix.getLanguages().stream().filter(it -> wikipediaLang.equalsIgnoreCase(it.getCode())).findFirst().orElse(null);
66-
final SitematrixResult.Sitematrix.Site site;
73+
final SitematrixResult.Sitematrix.Language language = sitematrix.getLanguages().stream()
74+
.filter(it -> wikipediaLang.equalsIgnoreCase(it.getCode())).findFirst().orElse(null);
6775
if (language != null) {
68-
site = language.getSites().stream().filter(it -> "wiki".equals(it.getCode())).findFirst().orElseThrow(() -> new IllegalArgumentException("No Wikipedia for language " + language.getName() + " (" + language.getCode() + ") found!"));
76+
this.site = language.getSites().stream().filter(it -> "wiki".equals(it.getCode())).findFirst()
77+
.orElseThrow(() -> new IllegalArgumentException("No Wikipedia for language " + language.getName()
78+
+ " (" + language.getCode() + ") found!"));
6979
} else {
70-
site = sitematrix.getSpecialSites().stream().filter(it -> wikipediaLang.equals(it.getCode())).findFirst().orElseThrow(() -> new IllegalArgumentException("No wiki site for code '" + wikipediaLang + "' found!"));
80+
this.site = sitematrix.getSpecialSites().stream().filter(it -> wikipediaLang.equals(it.getCode())).findFirst()
81+
.orElseThrow(() -> new IllegalArgumentException("No wiki site for code '" + wikipediaLang + "' found!"));
7182
}
72-
this.site = site;
7383
}
7484

7585
public static WikipediaApp forLanguage(final String wikipediaLang) {
@@ -108,20 +118,19 @@ private static HttpClient.Response connect(String url) throws IOException {
108118
public List<WikipediaEntry> getEntriesFromCoordinates(LatLon min, LatLon max) {
109119
try {
110120
// construct url
111-
final String url = new StringBuilder(getSiteUrl()).append("/w/api.php")
112-
.append("?action=query")
113-
.append("&list=geosearch")
114-
.append("&format=xml")
115-
.append("&gslimit=500")
116-
.append("&gsbbox=")
117-
.append(max.lat()).append(STRING_URI_PIPE).append(min.lon())
118-
.append(STRING_URI_PIPE).append(min.lat()).append(STRING_URI_PIPE).append(max.lon())
119-
.toString();
121+
final String url = getSiteUrl() + "/w/api.php" +
122+
"?action=query" +
123+
"&list=geosearch" +
124+
"&format=xml" +
125+
"&gslimit=500" +
126+
"&gsbbox=" +
127+
max.lat() + STRING_URI_PIPE + min.lon() +
128+
STRING_URI_PIPE + min.lat() + STRING_URI_PIPE + max.lon();
120129
// parse XML document
121130
try (InputStream in = connect(url).getContent()) {
122131
final Document doc = newDocumentBuilder().parse(in);
123132
final String errorInfo = X_PATH.evaluateString("//error/@info", doc);
124-
if (errorInfo != null && errorInfo.length() >= 1) {
133+
if (errorInfo != null && !errorInfo.isEmpty()) {
125134
// I18n: {0} is the error message returned by the API
126135
new Notification(I18n.tr("Downloading entries with geo coordinates failed: {0}", errorInfo))
127136
.setIcon(WikipediaPlugin.NOTIFICATION_ICON)
@@ -133,24 +142,25 @@ public List<WikipediaEntry> getEntriesFromCoordinates(LatLon min, LatLon max) {
133142
final LatLon latLon = new LatLon(
134143
X_PATH.evaluateDouble("@lat", node),
135144
X_PATH.evaluateDouble("@lon", node));
136-
if ("wikidata".equals(wikipediaLang)) {
145+
if (WIKIDATA.equals(wikipediaLang)) {
137146
return new WikidataEntry(name, null, latLon, null);
138147
} else {
139148
return new WikipediaEntry(wikipediaLang, name, latLon);
140149
}
141150
}).collect(Collectors.toList());
142-
if ("wikidata".equals(wikipediaLang)) {
151+
if (WIKIDATA.equals(wikipediaLang)) {
143152
return new ArrayList<>(getLabelForWikidata(entries, Locale.getDefault()));
144153
} else {
145154
return entries;
146155
}
147156
}
148157
} catch (Exception ex) {
149-
throw new RuntimeException(ex);
158+
throw new JosmRuntimeException(ex);
150159
}
151160
}
152161

153-
public static List<WikidataEntry> getWikidataEntriesForQuery(final String languageForQuery, final String query, final Locale localeForLabels) {
162+
public static List<WikidataEntry> getWikidataEntriesForQuery(final String languageForQuery, final String query,
163+
final Locale localeForLabels) {
154164
try {
155165
final String url = "https://www.wikidata.org/w/api.php" +
156166
"?action=wbsearchentities" +
@@ -167,7 +177,7 @@ public static List<WikidataEntry> getWikidataEntriesForQuery(final String langua
167177
return getLabelForWikidata(r, localeForLabels);
168178
}
169179
} catch (Exception ex) {
170-
throw new RuntimeException(ex);
180+
throw new JosmRuntimeException(ex);
171181
}
172182
}
173183

@@ -184,12 +194,12 @@ public List<WikipediaEntry> getEntriesFromCategory(String category, int depth) {
184194
.collect(Collectors.toList());
185195
}
186196
} catch (IOException ex) {
187-
throw new RuntimeException(ex);
197+
throw new UncheckedIOException(ex);
188198
}
189199
}
190200

191201
public static List<WikipediaEntry> getEntriesFromClipboard(final String wikipediaLang, String clipboardStringContent) {
192-
if ("wikidata".equals(wikipediaLang)) {
202+
if (WIKIDATA.equals(wikipediaLang)) {
193203
List<WikidataEntry> entries = new ArrayList<>();
194204
Matcher matcher = RegexUtil.Q_ID_PATTERN.matcher(clipboardStringContent);
195205
while (matcher.find()) {
@@ -231,7 +241,7 @@ public void updateWIWOSMStatus(List<WikipediaEntry> entries) {
231241
});
232242
}
233243
} catch (Exception ex) {
234-
throw new RuntimeException(ex);
244+
throw new JosmRuntimeException(ex);
235245
}
236246
}
237247
for (WikipediaEntry i : entries) {
@@ -240,15 +250,33 @@ public void updateWIWOSMStatus(List<WikipediaEntry> entries) {
240250
}
241251

242252
public boolean hasWikipediaTag(final OsmPrimitive p) {
243-
return p.hasKey("wikidata", "wikipedia", "wikipedia:" + wikipediaLang);
253+
return p.hasKey(wikipediaKeys);
254+
}
255+
256+
/**
257+
* Check to see if a tagged object has had its wikipedia tag change
258+
* @param primitive The tagged object to check
259+
* @param originalKeys The original keys
260+
* @return {@code true} if the tagged object has had a change in wikipedia keys
261+
*/
262+
public boolean tagChangeWikipedia(Tagged primitive, Map<String, String> originalKeys) {
263+
for (String key : wikipediaKeys) {
264+
// If the key has been added or removed, it has been changed.
265+
if (primitive.hasKey(key) != originalKeys.containsKey(key) ||
266+
// If the original key doesn't equal the new key, then it has been changed
267+
(primitive.hasKey(key) && originalKeys.containsKey(key) && !originalKeys.get(key).equals(primitive.get(key)))) {
268+
return true;
269+
}
270+
}
271+
return false;
244272
}
245273

246274
public Stream<String> getWikipediaArticles(final OsmPrimitive p) {
247-
if ("wikidata".equals(wikipediaLang)) {
248-
return Stream.of(p.get("wikidata")).filter(Objects::nonNull);
275+
if (WIKIDATA.equals(wikipediaLang)) {
276+
return Stream.of(p.get(WIKIDATA)).filter(Objects::nonNull);
249277
}
250278
return Stream
251-
.of("wikipedia", "wikipedia:" + wikipediaLang)
279+
.of(WIKIPEDIA, WIKIPEDIA + ':' + wikipediaLang)
252280
.map(key -> WikipediaEntry.parseTag(key, p.get(key)))
253281
.filter(Objects::nonNull)
254282
.filter(wp -> wikipediaLang.equals(wp.lang))
@@ -263,9 +291,7 @@ public Stream<String> getWikipediaArticles(final OsmPrimitive p) {
263291
public Map<String, String> getWikidataForArticles(Collection<String> articles) {
264292
final Map<String, String> result = new HashMap<>();
265293
// maximum of 50 titles
266-
ListUtil.processInBatches(new ArrayList<>(articles), 50, batch -> {
267-
result.putAll(resolveWikidataItems(batch));
268-
});
294+
ListUtil.processInBatches(new ArrayList<>(articles), 50, batch -> result.putAll(resolveWikidataItems(batch)));
269295
return result;
270296
}
271297

@@ -321,10 +347,10 @@ private Map<String, String> getWikidataForArticles0(Collection<String> articles)
321347
return ApiQueryClient.query(WikidataActionApiQuery.wbgetentities(site, articles))
322348
.getEntities().values()
323349
.stream()
324-
.filter(it -> RegexUtil.isValidQId(it.getId()) && it.getSitelinks().size() >= 1)
350+
.filter(it -> RegexUtil.isValidQId(it.getId()) && !it.getSitelinks().isEmpty())
325351
.collect(Collectors.toMap(it -> it.getSitelinks().iterator().next().getTitle(), WbgetentitiesResult.Entity::getId));
326352
} catch (IOException ex) {
327-
throw new RuntimeException(ex);
353+
throw new UncheckedIOException(ex);
328354
}
329355
}
330356

@@ -336,9 +362,10 @@ private Map<String, String> getWikidataForArticles0(Collection<String> articles)
336362
*/
337363
Map<String, String> resolveRedirectsForArticles(Collection<String> articles) {
338364
try {
339-
return articles.stream().collect(Collectors.toMap(it -> it, ApiQueryClient.query(WikipediaActionApiQuery.query(site, articles)).getQuery()::resolveRedirect));
365+
return articles.stream().collect(Collectors.toMap(it -> it,
366+
ApiQueryClient.query(WikipediaActionApiQuery.query(site, articles)).getQuery()::resolveRedirect));
340367
} catch (Exception ex) {
341-
throw new RuntimeException(ex);
368+
throw new JosmRuntimeException(ex);
342369
}
343370
}
344371

@@ -359,21 +386,24 @@ public List<String> getCategoriesForPrefix(final String prefix) {
359386
.collect(Collectors.toList())
360387
).orElse(new ArrayList<>());
361388
} catch (IOException ex) {
362-
throw new RuntimeException(ex);
389+
throw new UncheckedIOException(ex);
363390
}
364391
}
365392

366393
public static String getLabelForWikidata(String wikidataId, Locale locale, String... preferredLanguage) {
367394
try {
368395
final List<WikidataEntry> entry = Collections.singletonList(new WikidataEntry(wikidataId));
369396
return getLabelForWikidata(entry, locale, preferredLanguage).get(0).label;
370-
} catch (IndexOutOfBoundsException ignore) {
397+
} catch (IndexOutOfBoundsException indexOutOfBoundsException) {
398+
Logging.trace(indexOutOfBoundsException);
371399
return null;
372400
}
373401
}
374402

375-
static List<WikidataEntry> getLabelForWikidata(final List<? extends WikipediaEntry> entries, final Locale locale, final String... preferredLanguage) {
376-
final List<WikidataEntry> wdEntries = entries.stream().map(it -> it instanceof WikidataEntry ? (WikidataEntry) it : null).filter(Objects::nonNull).collect(Collectors.toList());
403+
static List<WikidataEntry> getLabelForWikidata(final List<? extends WikipediaEntry> entries, final Locale locale,
404+
final String... preferredLanguage) {
405+
final List<WikidataEntry> wdEntries = entries.stream()
406+
.map(it -> it instanceof WikidataEntry ? (WikidataEntry) it : null).filter(Objects::nonNull).collect(Collectors.toList());
377407
if (wdEntries.size() != entries.size()) {
378408
throw new IllegalArgumentException("The entries given to method `getLabelForWikidata` must all be of type WikidataEntry!");
379409
}
@@ -389,7 +419,9 @@ static List<WikidataEntry> getLabelForWikidata(final List<? extends WikipediaEnt
389419
final List<WikidataEntry> result = new ArrayList<>(wdEntries.size());
390420
ListUtil.processInBatches(wdEntries, 50, batch -> {
391421
try {
392-
final Map<String, Optional<WbgetentitiesResult.Entity>> entities = ApiQueryClient.query(WikidataActionApiQuery.wbgetentitiesLabels(batch.stream().map(it -> it.article).collect(Collectors.toList())));
422+
final Map<String, Optional<WbgetentitiesResult.Entity>> entities =
423+
ApiQueryClient.query(WikidataActionApiQuery.wbgetentitiesLabels(batch.stream().map(it -> it.article)
424+
.collect(Collectors.toList())));
393425
if (entities != null) {
394426
for (final WikidataEntry batchEntry : batch) {
395427
Optional.ofNullable(entities.get(batchEntry.article)).flatMap(it -> it).ifPresent(entity -> {
@@ -403,7 +435,7 @@ static List<WikidataEntry> getLabelForWikidata(final List<? extends WikipediaEnt
403435
}
404436
}
405437
} catch (Exception ex) {
406-
throw new RuntimeException(ex);
438+
throw new JosmRuntimeException(ex);
407439
}
408440
});
409441
return result;
@@ -435,7 +467,7 @@ public Collection<WikipediaEntry> getInterwikiArticles(String article) {
435467
}).collect(Collectors.toList());
436468
}
437469
} catch (Exception ex) {
438-
throw new RuntimeException(ex);
470+
throw new JosmRuntimeException(ex);
439471
}
440472
}
441473

@@ -456,7 +488,7 @@ public LatLon getCoordinateForArticle(String article) {
456488
}
457489
}
458490
} catch (Exception ex) {
459-
throw new RuntimeException(ex);
491+
throw new JosmRuntimeException(ex);
460492
}
461493
}
462494

@@ -482,7 +514,7 @@ private static DocumentBuilder newDocumentBuilder() {
482514
} catch (ParserConfigurationException e) {
483515
Logging.warn("Cannot create DocumentBuilder");
484516
Logging.warn(e);
485-
throw new RuntimeException(e);
517+
throw new JosmRuntimeException(e);
486518
}
487519
}
488520
}

0 commit comments

Comments
 (0)