Bugfix/getuploadcount (#3049)

* Closes #3048
* response.body().string() was called more than once and because response body can be huge so OkHttp doesnot store it in memory, it reads it as a stream from network when we need it, which cannot be done without a new request.

* null check in response.body()

* Fixed possible NumberFormatException, made review suggested changes

* added getUploadCount test case

app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.java

@@ -1,28 +1,10 @@
 package fr.free.nrw.commons.mwapi;
 
 import android.text.TextUtils;
-
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
-
 import com.google.gson.Gson;
 import com.google.gson.reflect.TypeToken;
-
-import org.apache.commons.lang3.StringUtils;
-import org.wikipedia.dataclient.mwapi.MwQueryPage;
-import org.wikipedia.dataclient.mwapi.MwQueryResponse;
-
-import java.io.IOException;
-import java.lang.reflect.Type;
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-
-import javax.inject.Inject;
-import javax.inject.Singleton;
-
 import fr.free.nrw.commons.Media;
 import fr.free.nrw.commons.achievements.FeaturedImages;
 import fr.free.nrw.commons.achievements.FeedbackResponse;
@@ -38,10 +20,23 @@ import fr.free.nrw.commons.utils.ConfigUtils;
 import fr.free.nrw.commons.wikidata.model.GetWikidataEditCountResponse;
 import io.reactivex.Observable;
 import io.reactivex.Single;
+import java.io.IOException;
+import java.lang.reflect.Type;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.inject.Singleton;
 import okhttp3.HttpUrl;
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
 import okhttp3.Response;
+import okhttp3.ResponseBody;
+import org.apache.commons.lang3.StringUtils;
+import org.wikipedia.dataclient.mwapi.MwQueryPage;
+import org.wikipedia.dataclient.mwapi.MwQueryResponse;
 import timber.log.Timber;
 
 /**
@@ -98,8 +93,16 @@ public class OkHttpJsonApiClient {
         return Single.fromCallable(() -> {
             Response response = okHttpClient.newCall(request).execute();
             if (response != null && response.isSuccessful()) {
-                if(!TextUtils.isEmpty(response.body().string().trim())){
-                    return Integer.parseInt(response.body().string().trim());
+                ResponseBody responseBody = response.body();
+                if (null != responseBody) {
+                    String responseBodyString = responseBody.string().trim();
+                    if (!TextUtils.isEmpty(responseBodyString)) {
+                        try {
+                            return Integer.parseInt(responseBodyString);
+                        } catch (NumberFormatException e) {
+                            Timber.e(e);
+                        }
+                    }
                 }
             }
             return 0;

app/src/test/kotlin/fr/free/nrw/commons/mwapi/OkHttpJsonApiClientTest.kt

@@ -1,7 +1,6 @@
 package fr.free.nrw.commons.mwapi
 
 import com.google.gson.Gson
-import fr.free.nrw.commons.BuildConfig
 import fr.free.nrw.commons.Media
 import fr.free.nrw.commons.TestCommonsApplication
 import fr.free.nrw.commons.kvstore.JsonKvStore
@@ -344,6 +343,15 @@ class OkHttpJsonApiClientTest {
         return it
     }
 
+    /**
+     * Check request params with encoded path
+     */
+    private fun assertBasicRequestParameters(server: MockWebServer, method: String,encodedPath: String): RecordedRequest = server.takeRequest().let {
+        Assert.assertEquals(encodedPath, it.requestUrl.encodedPath())
+        Assert.assertEquals(method, it.method)
+        return it
+    }
+
 
     /**
      * Parse query params
@@ -354,4 +362,37 @@ class OkHttpJsonApiClientTest {
         }
     }
 
+
+    /**
+     * Test getUploadCount posititive and negative cases
+     */
+    @Test
+    fun testGetUploadCount(){
+        //Positive
+        assertEquals(testBaseCasesAndGetUploadCount(true), 20)
+        //Negative
+        assertEquals(testBaseCasesAndGetUploadCount(false), 0)
+    }
+
+    /**
+     * Test getUploadCount base cases
+     */
+    private fun testBaseCasesAndGetUploadCount(shouldAddResponse: Boolean): Int? {
+        val mockResponse = MockResponse()
+        mockResponse.setResponseCode(200)
+        if(shouldAddResponse) {
+            val responseBody = "20"
+            mockResponse.setBody(responseBody)
+        }
+        toolsForgeServer.enqueue(mockResponse)
+
+        val uploadCount=testObject.getUploadCount("ashishkumar294").blockingGet()
+        assertBasicRequestParameters(toolsForgeServer, "GET","/uploadsbyuser.py").let { request ->
+            parseQueryParams(request).let { body ->
+                Assert.assertEquals("ashishkumar294", body["user"])
+            }
+        }
+        return uploadCount
+    }
+
 }