jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/401705 )
Change subject: Properly close HC client in WikibaseRepository
......................................................................
Properly close HC client in WikibaseRepository
This could be one of the cause for leaked threads detected in our unit
tests.
Bug: T178721
Change-Id: I882d886382fe73630cfc5d4f372de809e4ddeb29
---
A testTools/src/main/java/org/wikidata/query/rdf/test/CloseableRule.java
M tools/src/main/java/org/wikidata/query/rdf/tool/Update.java
M
tools/src/main/java/org/wikidata/query/rdf/tool/wikibase/WikibaseRepository.java
M
tools/src/test/java/org/wikidata/query/rdf/tool/AbstractUpdaterIntegrationTestBase.java
M tools/src/test/java/org/wikidata/query/rdf/tool/RdfRepositoryForTesting.java
M
tools/src/test/java/org/wikidata/query/rdf/tool/wikibase/WikibaseRepositoryIntegrationTest.java
6 files changed, 111 insertions(+), 44 deletions(-)
Approvals:
Smalyshev: Looks good to me, approved
jenkins-bot: Verified
diff --git
a/testTools/src/main/java/org/wikidata/query/rdf/test/CloseableRule.java
b/testTools/src/main/java/org/wikidata/query/rdf/test/CloseableRule.java
new file mode 100644
index 0000000..6e11979
--- /dev/null
+++ b/testTools/src/main/java/org/wikidata/query/rdf/test/CloseableRule.java
@@ -0,0 +1,48 @@
+package org.wikidata.query.rdf.test;
+
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
+
+
+/**
+ * A {@link TestRule} that will automatically close an object after tests.
+ *
+ * This makes it slightly nicer than having @After methods for each resource to
+ * close. It makes most sense when a test instantiate multiple {@link
+ * AutoCloseable} resources.
+ *
+ * @param <T>
+ */
+public final class CloseableRule<T extends AutoCloseable> implements TestRule {
+
+ private final T closeable;
+
+ public static <T extends AutoCloseable> CloseableRule<T> autoClose(T
closeable) {
+ return new CloseableRule<>(closeable);
+ }
+
+ private CloseableRule(T closeable) {
+ this.closeable = closeable;
+ }
+
+ public T get() {
+ return closeable;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public Statement apply(final Statement base, Description description) {
+ return new Statement() {
+ @Override
+ public void evaluate() throws Throwable {
+ try {
+ base.evaluate();
+ } finally {
+ closeable.close();
+ }
+ }
+ };
+ }
+
+}
diff --git a/tools/src/main/java/org/wikidata/query/rdf/tool/Update.java
b/tools/src/main/java/org/wikidata/query/rdf/tool/Update.java
index 9ea247e..5d768ac 100644
--- a/tools/src/main/java/org/wikidata/query/rdf/tool/Update.java
+++ b/tools/src/main/java/org/wikidata/query/rdf/tool/Update.java
@@ -50,10 +50,11 @@
public static void main(String[] args) throws Exception {
RdfRepository rdfRepository = null;
Updater<? extends Change.Batch> updater;
+ WikibaseRepository wikibaseRepository;
try {
UpdateOptions options = handleOptions(UpdateOptions.class, args);
- WikibaseRepository wikibaseRepository =
buildWikibaseRepository(options);
+ wikibaseRepository = buildWikibaseRepository(options);
URI sparqlUri = sparqlUri(options);
WikibaseUris uris = new WikibaseUris(options.wikibaseHost());
rdfRepository = new RdfRepository(sparqlUri, uris);
@@ -67,13 +68,15 @@
}
throw e;
}
- try (RdfRepository r = rdfRepository) {
+ try (
+ WikibaseRepository w = wikibaseRepository;
+ RdfRepository r = rdfRepository;
+ Updater u = updater
+ ) {
updater.run();
} catch (Exception e) {
log.error("Error during updater run.", e);
throw e;
- } finally {
- updater.close();
}
}
diff --git
a/tools/src/main/java/org/wikidata/query/rdf/tool/wikibase/WikibaseRepository.java
b/tools/src/main/java/org/wikidata/query/rdf/tool/wikibase/WikibaseRepository.java
index eeccbc5..58c3cfb 100644
---
a/tools/src/main/java/org/wikidata/query/rdf/tool/wikibase/WikibaseRepository.java
+++
b/tools/src/main/java/org/wikidata/query/rdf/tool/wikibase/WikibaseRepository.java
@@ -1,5 +1,6 @@
package org.wikidata.query.rdf.tool.wikibase;
+import java.io.Closeable;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.InterruptedIOException;
@@ -72,7 +73,7 @@
*/
// TODO fan out complexity
@SuppressWarnings("checkstyle:classfanoutcomplexity")
-public class WikibaseRepository {
+public class WikibaseRepository implements Closeable {
private static final Logger log =
LoggerFactory.getLogger(WikibaseRepository.class);
/**
@@ -416,6 +417,11 @@
return name.matches("^[A-Za-z0-9:]+$");
}
+ @Override
+ public void close() throws IOException {
+ client.close();
+ }
+
/**
* URIs used for accessing wikibase.
*/
diff --git
a/tools/src/test/java/org/wikidata/query/rdf/tool/AbstractUpdaterIntegrationTestBase.java
b/tools/src/test/java/org/wikidata/query/rdf/tool/AbstractUpdaterIntegrationTestBase.java
index 22bf8eb..7851a48 100644
---
a/tools/src/test/java/org/wikidata/query/rdf/tool/AbstractUpdaterIntegrationTestBase.java
+++
b/tools/src/test/java/org/wikidata/query/rdf/tool/AbstractUpdaterIntegrationTestBase.java
@@ -1,5 +1,7 @@
package org.wikidata.query.rdf.tool;
+import static org.wikidata.query.rdf.test.CloseableRule.autoClose;
+
import java.util.concurrent.ExecutorService;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadPoolExecutor;
@@ -8,6 +10,7 @@
import org.junit.Rule;
import org.junit.runner.RunWith;
import org.wikidata.query.rdf.common.uri.WikibaseUris;
+import org.wikidata.query.rdf.test.CloseableRule;
import org.wikidata.query.rdf.tool.change.Change;
import org.wikidata.query.rdf.tool.change.IdRangeChangeSource;
import org.wikidata.query.rdf.tool.rdf.Munger;
@@ -24,7 +27,8 @@
/**
* Wikibase test against.
*/
- private final WikibaseRepository wikibaseRepository = new
WikibaseRepository("https", "www.wikidata.org");
+ @Rule
+ public final CloseableRule<WikibaseRepository> wikibaseRepository =
autoClose(new WikibaseRepository("https", "www.wikidata.org"));
/**
* Munger to test against.
*/
@@ -42,10 +46,9 @@
*/
public void update(int from, int to) {
Change.Source<?> source = IdRangeChangeSource.forItems(from, to, 30);
- ExecutorService executorService = new ThreadPoolExecutor(0, 10, 0,
TimeUnit.SECONDS,
- new LinkedBlockingQueue<Runnable>());
+ ExecutorService executorService = new ThreadPoolExecutor(0, 10, 0,
TimeUnit.SECONDS, new LinkedBlockingQueue<>());
WikibaseUris uris = new WikibaseUris("www.wikidata.org");
- try (Updater<?> updater = new Updater<>(source, wikibaseRepository,
rdfRepository, munger, executorService, 0, uris, false)) {
+ try (Updater<?> updater = new Updater<>(source,
wikibaseRepository.get(), rdfRepository, munger, executorService, 0, uris,
false)) {
updater.run();
}
}
diff --git
a/tools/src/test/java/org/wikidata/query/rdf/tool/RdfRepositoryForTesting.java
b/tools/src/test/java/org/wikidata/query/rdf/tool/RdfRepositoryForTesting.java
index 731942e..ab98668 100644
---
a/tools/src/test/java/org/wikidata/query/rdf/tool/RdfRepositoryForTesting.java
+++
b/tools/src/test/java/org/wikidata/query/rdf/tool/RdfRepositoryForTesting.java
@@ -84,6 +84,7 @@
}
/** {@inheritDoc} */
+ @Override
public Statement apply(final Statement base, Description description) {
return new Statement() {
@Override
diff --git
a/tools/src/test/java/org/wikidata/query/rdf/tool/wikibase/WikibaseRepositoryIntegrationTest.java
b/tools/src/test/java/org/wikidata/query/rdf/tool/wikibase/WikibaseRepositoryIntegrationTest.java
index bf4c887..85ff47e 100644
---
a/tools/src/test/java/org/wikidata/query/rdf/tool/wikibase/WikibaseRepositoryIntegrationTest.java
+++
b/tools/src/test/java/org/wikidata/query/rdf/tool/wikibase/WikibaseRepositoryIntegrationTest.java
@@ -8,8 +8,10 @@
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.isA;
import static org.hamcrest.Matchers.not;
+import static org.wikidata.query.rdf.test.CloseableRule.autoClose;
import static
org.wikidata.query.rdf.tool.wikibase.WikibaseRepository.inputDateFormat;
+import java.io.IOException;
import java.text.DateFormat;
import java.text.ParseException;
import java.util.Collection;
@@ -21,9 +23,11 @@
import org.apache.commons.lang3.time.DateUtils;
import org.json.simple.JSONArray;
import org.json.simple.JSONObject;
+import org.junit.Rule;
import org.junit.Test;
import org.openrdf.model.Statement;
import org.wikidata.query.rdf.common.uri.WikibaseUris;
+import org.wikidata.query.rdf.test.CloseableRule;
import org.wikidata.query.rdf.tool.change.Change;
import org.wikidata.query.rdf.tool.exception.ContainedException;
import org.wikidata.query.rdf.tool.exception.RetryableException;
@@ -36,8 +40,9 @@
*/
public class WikibaseRepositoryIntegrationTest extends RandomizedTest {
private static final String HOST = "test.wikidata.org";
- private final WikibaseRepository repo = new WikibaseRepository("https",
HOST);
- private final WikibaseRepository proxyRepo = new
WikibaseRepository("http", "localhost", 8812);
+ @Rule
+ public final CloseableRule<WikibaseRepository> repo = autoClose(new
WikibaseRepository("https", HOST));
+ private final CloseableRule<WikibaseRepository> proxyRepo = autoClose(new
WikibaseRepository("http", "localhost", 8812));
private final WikibaseUris uris = new WikibaseUris(HOST);
@Test
@@ -48,7 +53,7 @@
* is probably ok.
*/
int batchSize = randomIntBetween(3, 30);
- JSONObject changes = repo.fetchRecentChanges(new
Date(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(30)),
+ JSONObject changes = repo.get().fetchRecentChanges(new
Date(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(30)),
null, batchSize);
Map<String, Object> c = changes;
assertThat(c, hasKey("continue"));
@@ -65,8 +70,8 @@
assertThat(rc, hasEntry(equalTo("timestamp"),
instanceOf(String.class)));
assertThat(rc, hasEntry(equalTo("revid"), instanceOf(Long.class)));
}
- final Date nextDate = repo.getChangeFromContinue((Map<String,
Object>)changes.get("continue")).timestamp();
- changes = repo.fetchRecentChanges(nextDate, null, batchSize);
+ final Date nextDate = repo.get().getChangeFromContinue((Map<String,
Object>)changes.get("continue")).timestamp();
+ changes = repo.get().fetchRecentChanges(nextDate, null, batchSize);
assertThat(c, hasKey("query"));
assertThat((Map<String, Object>) c.get("query"),
hasKey("recentchanges"));
}
@@ -78,7 +83,7 @@
* This relies on there being very few changes in the current
* second.
*/
- JSONObject changes = repo.fetchRecentChanges(new
Date(System.currentTimeMillis()), null, 500);
+ JSONObject changes = repo.get().fetchRecentChanges(new
Date(System.currentTimeMillis()), null, 500);
Map<String, Object> c = changes;
assertThat(c, not(hasKey("continue")));
assertThat(c, hasKey("query"));
@@ -103,7 +108,7 @@
} catch (InterruptedException e) {
// nothing to do here, sorry. I know it looks bad.
}
- JSONObject result = repo.fetchRecentChanges(date, null, batchSize);
+ JSONObject result = repo.get().fetchRecentChanges(date, null,
batchSize);
return (JSONArray) ((JSONObject)
result.get("query")).get("recentchanges");
}
@@ -111,8 +116,8 @@
private void editShowsUpInRecentChangesTestCase(String label, String type)
throws RetryableException,
ContainedException {
long now = System.currentTimeMillis();
- String entityId = repo.firstEntityIdForLabelStartingWith(label, "en",
type);
- repo.setLabel(entityId, type, label + now, "en");
+ String entityId = repo.get().firstEntityIdForLabelStartingWith(label,
"en", type);
+ repo.get().setLabel(entityId, type, label + now, "en");
JSONArray changes = getRecentChanges(new Date(now - 10000), 10);
boolean found = false;
String title = entityId;
@@ -129,7 +134,7 @@
}
}
assertTrue("Didn't find new page in recent changes", found);
- Collection<Statement> statements = repo.fetchRdfForEntity(entityId);
+ Collection<Statement> statements =
repo.get().fetchRdfForEntity(entityId);
found = false;
for (Statement statement : statements) {
if (statement.getSubject().stringValue().equals(uris.entity() +
entityId)) {
@@ -141,37 +146,38 @@
}
@Test
- public void fetchIsNormalized() throws RetryableException,
ContainedException {
+ public void fetchIsNormalized() throws RetryableException,
ContainedException, IOException {
long now = System.currentTimeMillis();
- WikibaseRepository proxyRepo = new WikibaseRepository("http",
"localhost", 8812);
- String entityId =
repo.firstEntityIdForLabelStartingWith("QueryTestItem", "en", "item");
- repo.setLabel(entityId, "item", "QueryTestItem" + now, "en");
- Collection<Statement> statements =
proxyRepo.fetchRdfForEntity(entityId);
- boolean foundBad = false;
- boolean foundGood = false;
- for (Statement statement : statements) {
- if
(statement.getObject().stringValue().contains("http://www.wikidata.org/ontology-beta#";))
{
- foundBad = true;
+ try (WikibaseRepository proxyRepo = new WikibaseRepository("http",
"localhost", 8812)) {
+ String entityId =
repo.get().firstEntityIdForLabelStartingWith("QueryTestItem", "en", "item");
+ repo.get().setLabel(entityId, "item", "QueryTestItem" + now, "en");
+ Collection<Statement> statements =
proxyRepo.fetchRdfForEntity(entityId);
+ boolean foundBad = false;
+ boolean foundGood = false;
+ for (Statement statement : statements) {
+ if
(statement.getObject().stringValue().contains("http://www.wikidata.org/ontology-beta#";))
{
+ foundBad = true;
+ }
+ if
(statement.getObject().stringValue().contains("http://www.wikidata.org/ontology-0.0.1#";))
{
+ foundBad = true;
+ }
+ if
(statement.getObject().stringValue().contains("http://www.wikidata.org/ontology#";))
{
+ foundBad = true;
+ }
+ if
(statement.getObject().stringValue().contains("http://wikiba.se/ontology#";)) {
+ foundGood = true;
+ }
}
- if
(statement.getObject().stringValue().contains("http://www.wikidata.org/ontology-0.0.1#";))
{
- foundBad = true;
- }
- if
(statement.getObject().stringValue().contains("http://www.wikidata.org/ontology#";))
{
- foundBad = true;
- }
- if
(statement.getObject().stringValue().contains("http://wikiba.se/ontology#";)) {
- foundGood = true;
- }
+ assertTrue("Did not find correct ontology statements", foundGood);
+ assertFalse("Found incorrect ontology statements", foundBad);
}
- assertTrue("Did not find correct ontology statements", foundGood);
- assertFalse("Found incorrect ontology statements", foundBad);
}
@Test
public void continueWorks() throws RetryableException, ContainedException,
ParseException, InterruptedException {
long now = System.currentTimeMillis();
- String entityId =
repo.firstEntityIdForLabelStartingWith("QueryTestItem", "en", "item");
- repo.setLabel(entityId, "item", "QueryTestItem" + now, "en");
+ String entityId =
repo.get().firstEntityIdForLabelStartingWith("QueryTestItem", "en", "item");
+ repo.get().setLabel(entityId, "item", "QueryTestItem" + now, "en");
JSONArray changes = getRecentChanges(new Date(now - 10000), 10);
Change change = null;
long oldRevid = 0;
@@ -192,7 +198,7 @@
// Ensure this change is in different second
Thread.sleep(1000);
// make new edit now
- repo.setLabel(entityId, "item", "QueryTestItem" + now + "updated",
"en");
+ repo.get().setLabel(entityId, "item", "QueryTestItem" + now +
"updated", "en");
changes = getRecentChanges(DateUtils.addSeconds(change.timestamp(),
1), 10);
// check that new result does not contain old edit but contains new
edit
boolean found = false;
@@ -210,7 +216,7 @@
@Test
@SuppressWarnings({ "unchecked", "rawtypes" })
public void recentChangesWithErrors() throws RetryableException,
ContainedException {
- JSONObject changes = proxyRepo.fetchRecentChanges(new
Date(System.currentTimeMillis()), null, 500);
+ JSONObject changes = proxyRepo.get().fetchRecentChanges(new
Date(System.currentTimeMillis()), null, 500);
Map<String, Object> c = changes;
assertThat(c, not(hasKey("continue")));
assertThat(c, hasKey("query"));
--
To view, visit https://gerrit.wikimedia.org/r/401705
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I882d886382fe73630cfc5d4f372de809e4ddeb29
Gerrit-PatchSet: 4
Gerrit-Project: wikidata/query/rdf
Gerrit-Branch: master
Gerrit-Owner: Gehel <guillaume.leder...@wikimedia.org>
Gerrit-Reviewer: Lucas Werkmeister (WMDE) <lucas.werkmeis...@wikimedia.de>
Gerrit-Reviewer: Smalyshev <smalys...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
