Repository: Wikidata/Wikidata-Toolkit (java)

Issue:
Support for file uploads
WDTK supports creating new items, properties and lexemes, but not MediaInfo entities, and for a good reason: those cannot be created without uploading the corresponding file (to Commons at least).

Would it be appropriate to add support for file upload, via [the corresponding MediaWiki API action](https://commons.wikimedia.org/w/api.php?action=help&modules=upload)? That API action has nothing to do with Wikibase itself, so I could understand that it is considered out of scope, but at the same time it does feel like we should be able to reuse an `ApiConnection` from WDTK to also perform this action.

More generally, we could potentially think about how to make it easier to reuse an `ApiConnection` object for MediaWiki API actions which are not directly supported by WDTK. Intuitively, it can make sense to use most of the MediaWiki API actions within the context of Wikibase, and there is no chance we implement a wrapper for every single of those actions in WDTK any time soon, right? Perhaps the right way to phrase this would be to look for MediaWiki Java clients and see if there could be some sort of interoperability.

Pull request description:
Add support for file uploads to ApiConnection.
Closes #638.

This makes the ApiConnection classes reusable to perform API calls which require uploading a file. Adding an actual wrapper on top of those actions is out of scope for WDTK, but this makes it possible for users to implement that on their side, without having to hack the user credentials out of the ApiConnection object.

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index be509d04..4e03c0e5 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -17,7 +17,7 @@ jobs:
 
     steps:
       - uses: actions/checkout@v3
-      - uses: actions/setup-java@v2
+      - uses: actions/setup-java@v3
         with:
           distribution: zulu
           java-version: ${{ matrix.java }}
@@ -27,7 +27,7 @@ jobs:
           key: ${{ runner.os }}-${{ matrix.java }}-m2-${{ hashFiles('**/pom.xml') }}
           restore-keys: ${{ runner.os }}-${{ matrix.java }}-m2
       - run: mvn --batch-mode --update-snapshots -Dgpg.skip=true jacoco:prepare-agent verify jacoco:report
-      - uses: codecov/codecov-action@v2.1.0
+      - uses: codecov/codecov-action@v3
         with:
           file: ./**/target/site/jacoco/jacoco.xml
           name: codecov
diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml
index 591d893a..33150654 100644
--- a/.github/workflows/deploy.yml
+++ b/.github/workflows/deploy.yml
@@ -10,7 +10,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
-      - uses: actions/setup-java@v2
+      - uses: actions/setup-java@v3
         with:
           distribution: zulu
           java-version: 8
@@ -28,3 +28,10 @@ jobs:
         env:
           MAVEN_USERNAME: ${{ secrets.MAVEN_USERNAME }}
           MAVEN_PASSWORD: ${{ secrets.MAVEN_PASSWORD }}
+      - name: Generate javadocs
+        run: mvn compile javadoc:aggregate
+      - name: Publish javadocs
+        uses: JamesIves/github-pages-deploy-action@v4.2.5
+        with:
+          branch: gh-pages
+          folder: target/site/apidocs
diff --git a/pom.xml b/pom.xml
index 90860ce8..d39b3dbb 100644
--- a/pom.xml
+++ b/pom.xml
@@ -79,7 +79,7 @@
 		<junitVersion>5.8.2</junitVersion>
 		<hamcrestVersion>2.2</hamcrestVersion>
 		<mockitoVersion>4.4.0</mockitoVersion>
-		<rdf4jVersion>3.7.6</rdf4jVersion>
+		<rdf4jVersion>3.7.7</rdf4jVersion>
 		<slf4jVersion>1.7.36</slf4jVersion>
 		<threetenVersion>1.7.0</threetenVersion>
 		<okhttpSignpostVersion>1.1.0</okhttpSignpostVersion>
@@ -212,7 +212,7 @@
 				<!-- Call mvn test jacoco:report to generate test coverage -->
 				<groupId>org.jacoco</groupId>
 				<artifactId>jacoco-maven-plugin</artifactId>
-				<version>0.8.7</version>
+				<version>0.8.8</version>
 				<executions>
 					<execution>
 						<id>prepare-agent</id>
diff --git a/wdtk-wikibaseapi/src/main/java/org/wikidata/wdtk/wikibaseapi/ApiConnection.java b/wdtk-wikibaseapi/src/main/java/org/wikidata/wdtk/wikibaseapi/ApiConnection.java
index cf94b154..15c6c148 100644
--- a/wdtk-wikibaseapi/src/main/java/org/wikidata/wdtk/wikibaseapi/ApiConnection.java
+++ b/wdtk-wikibaseapi/src/main/java/org/wikidata/wdtk/wikibaseapi/ApiConnection.java
@@ -20,6 +20,7 @@
 
 package org.wikidata.wdtk.wikibaseapi;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
@@ -34,6 +35,7 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 
+import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.wikidata.wdtk.wikibaseapi.apierrors.AssertUserFailedException;
@@ -47,6 +49,7 @@ import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 
 import okhttp3.MediaType;
+import okhttp3.MultipartBody;
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
 import okhttp3.RequestBody;
@@ -339,6 +342,27 @@ public abstract class ApiConnection {
 		JsonNode root = this.sendJsonRequest("POST", params);
 		return root.path("query").path("tokens").path(tokenType + "token").textValue();
 	}
+	
+	/**
+	 * Sends a request to the API with the given parameters and the given
+	 * request method and returns the result JSON tree. It automatically fills the
+	 * cookie map with cookies in the result header after the request.
+	 * It logs the request warnings and adds makes sure that "format": "json"
+	 * parameter is set.
+	 *
+	 * @param requestMethod
+	 *            either POST or GET
+	 * @param parameters
+	 *            Maps parameter keys to values. Out of this map the function
+	 *            will create a query string for the request.
+	 * @return API result
+	 * @throws IOException
+	 * @throws MediaWikiApiErrorException if the API returns an error
+	 */
+	public JsonNode sendJsonRequest(String requestMethod,
+			Map<String,String> parameters) throws IOException, MediaWikiApiErrorException {
+		return sendJsonRequest(requestMethod, parameters, null);
+	}
 
 	/**
 	 * Sends a request to the API with the given parameters and the given
@@ -352,16 +376,24 @@ public abstract class ApiConnection {
 	 * @param parameters
 	 *            Maps parameter keys to values. Out of this map the function
 	 *            will create a query string for the request.
+	 * @param files
+	 *            If GET, this should be null. If POST, this can contain
+	 *            a list of files to upload, indexed by the parameter to pass them with.
+	 *            The first component of the pair is the filename exposed to the server,
+	 *            and the second component is the path to the local file to upload.
+	 *            Set to null or empty map to avoid uploading any file.
 	 * @return API result
 	 * @throws IOException
 	 * @throws MediaWikiApiErrorException if the API returns an error
 	 */
-	public JsonNode sendJsonRequest(String requestMethod, Map<String,String> parameters) throws IOException, MediaWikiApiErrorException {
+	public JsonNode sendJsonRequest(String requestMethod,
+			Map<String,String> parameters,
+			Map<String, ImmutablePair<String,File>> files) throws IOException, MediaWikiApiErrorException {
 		parameters.put(ApiConnection.PARAM_FORMAT, "json");
 		if (loggedIn) {
 			parameters.put(ApiConnection.ASSERT_PARAMETER, "user");
 		}
-		try (InputStream response = sendRequest(requestMethod, parameters)) {
+		try (InputStream response = sendRequest(requestMethod, parameters, files)) {
 			JsonNode root = this.mapper.readTree(response);
 			this.checkErrors(root);
 			this.logWarnings(root);
@@ -383,17 +415,36 @@ public abstract class ApiConnection {
 	 * @param parameters
 	 *            Maps parameter keys to values. Out of this map the function
 	 *            will create a query string for the request.
+	 * @param files
+	 *            If GET, this should be null. If POST, this can contain
+	 *            a list of files to upload, indexed by the parameter to pass them with.
+	 *            The first component of the pair is the filename exposed to the server,
+	 *            and the second component is the path to the local file to upload.
+	 *            Set to null or empty map to avoid uploading any file.
 	 * @return API result
 	 * @throws IOException
 	 */
 	public InputStream sendRequest(String requestMethod,
-			Map<String, String> parameters) throws IOException {
+			Map<String, String> parameters,
+			Map<String, ImmutablePair<String,File>> files) throws IOException {
 		Request request;
 		String queryString = getQueryString(parameters);
 		if ("GET".equalsIgnoreCase(requestMethod)) {
 			request = new Request.Builder().url(apiBaseUrl + "?" + queryString).build();
 		} else if ("POST".equalsIgnoreCase(requestMethod)) {
-			request = new Request.Builder().url(apiBaseUrl).post(RequestBody.create(queryString, URLENCODED_MEDIA_TYPE)).build();
+			RequestBody body;
+			if (files != null && !files.isEmpty()) {
+				MultipartBody.Builder builder = new MultipartBody.Builder();
+				parameters.entrySet().stream()
+					.forEach(entry -> builder.addFormDataPart(entry.getKey(), entry.getValue()));
+				files.entrySet().stream()
+					.forEach(entry -> builder.addFormDataPart(entry.getKey(), entry.getValue().getLeft(),
+							RequestBody.create(MediaType.parse("application/octet-stream"),entry.getValue().getRight())));
+				body = builder.build();
+			} else {
+				body = RequestBody.create(queryString, URLENCODED_MEDIA_TYPE);
+			}
+			request = new Request.Builder().url(apiBaseUrl).post(body).build();
 		} else {
 			throw new IllegalArgumentException("Expected the requestMethod to be either GET or POST, but got " + requestMethod);
 		}


Test patch:
diff --git a/wdtk-wikibaseapi/src/test/java/org/wikidata/wdtk/wikibaseapi/BasicApiConnectionTest.java b/wdtk-wikibaseapi/src/test/java/org/wikidata/wdtk/wikibaseapi/BasicApiConnectionTest.java
index d90b6a99..04b8f469 100644
--- a/wdtk-wikibaseapi/src/test/java/org/wikidata/wdtk/wikibaseapi/BasicApiConnectionTest.java
+++ b/wdtk-wikibaseapi/src/test/java/org/wikidata/wdtk/wikibaseapi/BasicApiConnectionTest.java
@@ -28,6 +28,8 @@ import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.io.File;
+import java.io.FileWriter;
 import java.io.IOException;
 import java.net.HttpCookie;
 import java.net.URL;
@@ -40,6 +42,7 @@ import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.TreeSet;
 
+import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
@@ -97,7 +100,16 @@ public class BasicApiConnectionTest {
 							.setBody("{\"entities\":{\"Q8\":{\"pageid\":134,\"ns\":0,\"title\":\"Q8\",\"lastrevid\":1174289176,\"modified\":\"2020-05-05T12:39:07Z\",\"type\":\"item\",\"id\":\"Q8\"}},\"success\":1}\n");
 				}
 				try {
-					switch (request.getBody().readUtf8()) {
+					String requestBody = request.getBody().readUtf8();
+					// in the case of file uploads, the string representation of the request body is not stable
+					// so we only check that some file was uploaded (for testPostFile)
+					if (requestBody.contains("Content-Disposition: form-data; name=\"file\"; filename=\"hello.txt\"")) {
+						return new MockResponse()
+								.setHeader("Content-Type", "application/json; charset=utf-8")
+								.setBody("{\"success\":\"true\"}");
+					}
+					// otherwise, check for equality on the request body
+					switch (requestBody) {
 						case "meta=tokens&format=json&action=query&type=login":
 							return makeJsonResponseFrom("/query-login-token.json");
 						case "lgtoken=b5780b6e2f27e20b450921d9461010b4&lgpassword=password&format=json&action=login&lgname=username":
@@ -111,6 +123,7 @@ public class BasicApiConnectionTest {
 						case "assert=user&format=json&action=query":
 							return makeJsonResponseFrom("/assert-user-failed.json");
 					}
+					
 				}catch (Exception e) {
 					return new MockResponse().setResponseCode(404);
 				}
@@ -223,6 +236,27 @@ public class BasicApiConnectionTest {
 						+ "&action=login&lgname=username&format=json", '&'),
 				split(connection.getQueryString(params), '&'));
 	}
+	
+	@Test
+	public void testPostFile() throws IOException, MediaWikiApiErrorException {
+		Map<String, String> formParams = new HashMap<>();
+		formParams.put("foo", "bar");
+
+		File file = File.createTempFile("upload_test", ".txt");
+		try {
+			FileWriter writer = new FileWriter(file);
+			writer.write("contents");
+			writer.close();
+			Map<String, ImmutablePair<String, File>> fileParams = new HashMap<>();
+			fileParams.put("file", new ImmutablePair<String,File>("hello.txt", file));
+			
+			JsonNode node = connection.sendJsonRequest("POST", formParams, fileParams);
+			assertEquals(node.get("success").asText(), "true");
+		} finally {
+			file.delete();
+		}
+		
+	}
 
 	@Test
 	public void testWarnings() throws IOException {
diff --git a/wdtk-wikibaseapi/src/test/java/org/wikidata/wdtk/wikibaseapi/MockBasicApiConnection.java b/wdtk-wikibaseapi/src/test/java/org/wikidata/wdtk/wikibaseapi/MockBasicApiConnection.java
index 82a7e216..7a97e556 100644
--- a/wdtk-wikibaseapi/src/test/java/org/wikidata/wdtk/wikibaseapi/MockBasicApiConnection.java
+++ b/wdtk-wikibaseapi/src/test/java/org/wikidata/wdtk/wikibaseapi/MockBasicApiConnection.java
@@ -21,12 +21,14 @@ package org.wikidata.wdtk.wikibaseapi;
  */
 
 import java.io.ByteArrayInputStream;
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.wikidata.wdtk.testing.MockStringContentFactory;
@@ -100,7 +102,9 @@ public class MockBasicApiConnection extends BasicApiConnection {
 
 	@Override
 	public InputStream sendRequest(String requestMethod,
-			Map<String, String> parameters) throws IOException {
+			Map<String, String> parameters,
+			Map<String, ImmutablePair<String, File>> files) throws IOException {
+		// files parameter purposely ignored because we do not support mocking that yet
 		if (this.webResources.containsKey(parameters.hashCode())) {
 			return new ByteArrayInputStream(this.webResources.get(parameters
 					.hashCode()));