Skip to content

Commit 543277c

Browse files
maskaravivekmaskaravivek
committed
Fix crashes and other reported issues
1 parent f8eec2a commit 543277c

File tree

4 files changed

+49
-42
lines changed

4 files changed

+49
-42
lines changed

‎app/src/main/java/fr/free/nrw/commons/Media.java

+1
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,7 @@ public boolean getRequestedDeletion(){
464464
* @param page response from the API
465465
* @return Media object
466466
*/
467+
@Nullable
467468
public static Media from(MwQueryPage page) {
468469
ImageInfo imageInfo = page.imageInfo();
469470
if(imageInfo == null) {

‎app/src/main/java/fr/free/nrw/commons/category/CategoryImagesListFragment.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ private void initViews() {
101101
* @param keyword
102102
*/
103103
private void resetQueryContinueValues(String keyword) {
104-
categoryKvStore.remove(keyword);
104+
categoryKvStore.remove("query_continue_" + keyword);
105105
}
106106

107107
/**

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

+15-3
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
@Singleton
4646
public class OkHttpJsonApiClient {
4747

48-
private static final Type mapType = new TypeToken<Map<String, String>>() {
48+
public static final Type mapType = new TypeToken<Map<String, String>>() {
4949
}.getType();
5050

5151
private final OkHttpClient okHttpClient;
@@ -294,10 +294,16 @@ public Single<List<Media>> searchImages(String query) {
294294
if (response.body() != null && response.isSuccessful()) {
295295
String json = response.body().string();
296296
MwQueryResponse mwQueryResponse = gson.fromJson(json, MwQueryResponse.class);
297+
if (mwQueryResponse.query() == null) {
298+
return mediaList;
299+
}
297300
List<MwQueryPage> pages = mwQueryResponse.query().pages();
298301
putContinueValues(query, mwQueryResponse.continuation());
299302
for (MwQueryPage page : pages) {
300-
mediaList.add(Media.from(page));
303+
Media media = Media.from(page);
304+
if (media != null) {
305+
mediaList.add(media);
306+
}
301307
}
302308
}
303309
return mediaList;
@@ -345,9 +351,15 @@ public Single<List<Media>> getCategoryImages(String categoryName) {
345351
String json = response.body().string();
346352
MwQueryResponse mwQueryResponse = gson.fromJson(json, MwQueryResponse.class);
347353
putContinueValues(categoryName, mwQueryResponse.continuation());
354+
if (mwQueryResponse.query() == null) {
355+
return mediaList;
356+
}
348357
List<MwQueryPage> pages = mwQueryResponse.query().pages();
349358
for (MwQueryPage page : pages) {
350-
mediaList.add(Media.from(page));
359+
Media media = Media.from(page);
360+
if (media != null) {
361+
mediaList.add(media);
362+
}
351363
}
352364
}
353365
return mediaList;

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

+32-38
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
package fr.free.nrw.commons.mwapi
22

3-
import android.os.Build
43
import com.google.gson.Gson
54
import fr.free.nrw.commons.BuildConfig
65
import fr.free.nrw.commons.TestCommonsApplication
76
import fr.free.nrw.commons.kvstore.JsonKvStore
8-
import fr.free.nrw.commons.utils.ConfigUtils
7+
import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient.mapType
98
import junit.framework.Assert.assertEquals
109
import okhttp3.HttpUrl
1110
import okhttp3.OkHttpClient
@@ -18,11 +17,10 @@ import org.junit.Before
1817
import org.junit.Test
1918
import org.junit.runner.RunWith
2019
import org.mockito.Mockito
20+
import org.mockito.Mockito.`when`
2121
import org.robolectric.RobolectricTestRunner
22-
import org.robolectric.RuntimeEnvironment
2322
import org.robolectric.annotation.Config
2423
import java.net.URLDecoder
25-
import java.util.*
2624

2725
@RunWith(RobolectricTestRunner::class)
2826
@Config(constants = BuildConfig::class, sdk = [23], application = TestCommonsApplication::class)
@@ -59,21 +57,20 @@ class OkHttpJsonApiClientTest {
5957
@Test
6058
fun getCategoryImages() {
6159
server.enqueue(getFirstPageOfImages())
62-
val categoryImages = testObject.getCategoryImages("Watercraft moored off shore")
60+
val categoryImages = testObject.getCategoryImages("Watercraft moored off shore")!!.blockingGet()
6361

6462
assertBasicRequestParameters(server, "GET").let { request ->
6563
parseQueryParams(request).let { body ->
66-
Assert.assertEquals("format", body["json"])
64+
Assert.assertEquals("json", body["format"])
6765
Assert.assertEquals("query", body["action"])
68-
Assert.assertEquals("generator", body["categorymembers"])
69-
Assert.assertEquals("gcmtype", body["file"])
70-
Assert.assertEquals("gcmtitle", body["Watercraft moored off shore"])
71-
Assert.assertEquals("gcmsort", body["timestamp"])
72-
Assert.assertEquals("gcmdir", body["desc"])
73-
Assert.assertEquals("gcmlimit", body["2"])
66+
Assert.assertEquals("categorymembers", body["generator"])
67+
Assert.assertEquals("file", body["gcmtype"])
68+
Assert.assertEquals("Watercraft moored off shore", body["gcmtitle"])
69+
Assert.assertEquals("timestamp", body["gcmsort"])
70+
Assert.assertEquals("desc", body["gcmdir"])
7471
}
7572
}
76-
assertEquals(categoryImages!!.blockingGet().size, 2)
73+
assertEquals(categoryImages.size, 2)
7774
}
7875

7976
private fun getFirstPageOfImages(): MockResponse {
@@ -94,51 +91,48 @@ class OkHttpJsonApiClientTest {
9491
fun getCategoryImagesWithContinue() {
9592
server.enqueue(getFirstPageOfImages())
9693
server.enqueue(getSecondPageOfImages())
97-
val categoryImages = testObject.getCategoryImages("Watercraft moored off shore")
94+
val categoryImages = testObject.getCategoryImages("Watercraft moored off shore")!!.blockingGet()
9895

9996
assertBasicRequestParameters(server, "GET").let { request ->
10097
parseQueryParams(request).let { body ->
101-
Assert.assertEquals("format", body["json"])
98+
Assert.assertEquals("json", body["format"])
10299
Assert.assertEquals("query", body["action"])
103-
Assert.assertEquals("generator", body["categorymembers"])
104-
Assert.assertEquals("gcmtype", body["file"])
105-
Assert.assertEquals("gcmtitle", body["Watercraft moored off shore"])
106-
Assert.assertEquals("gcmsort", body["timestamp"])
107-
Assert.assertEquals("gcmdir", body["desc"])
108-
Assert.assertEquals("gcmlimit", body["2"])
100+
Assert.assertEquals("categorymembers", body["generator"])
101+
Assert.assertEquals("file", body["gcmtype"])
102+
Assert.assertEquals("Watercraft moored off shore", body["gcmtitle"])
103+
Assert.assertEquals("timestamp", body["gcmsort"])
104+
Assert.assertEquals("desc", body["gcmdir"])
109105
}
110106
}
111107

112-
assertEquals(categoryImages!!.blockingGet().size, 2)
108+
assertEquals(categoryImages.size, 2)
113109

114-
val categoryImagesContinued = testObject.getCategoryImages("Watercraft moored off shore")
110+
`when`(sharedPreferences.getJson<HashMap<String, String>>("query_continue_Watercraft moored off shore", mapType))
111+
.thenReturn(hashMapOf(Pair("gcmcontinue", "testvalue"), Pair("continue", "gcmcontinue||")))
112+
113+
114+
val categoryImagesContinued = testObject.getCategoryImages("Watercraft moored off shore")!!.blockingGet()
115115

116116
assertBasicRequestParameters(server, "GET").let { request ->
117117
parseQueryParams(request).let { body ->
118-
Assert.assertEquals("format", body["json"])
118+
Assert.assertEquals("json", body["format"])
119119
Assert.assertEquals("query", body["action"])
120-
Assert.assertEquals("generator", body["categorymembers"])
121-
Assert.assertEquals("gcmtype", body["file"])
122-
Assert.assertEquals("gcmtitle", body["Watercraft moored off shore"])
123-
Assert.assertEquals("gcmsort", body["timestamp"])
124-
Assert.assertEquals("gcmdir", body["desc"])
125-
Assert.assertEquals("gcmlimit", body["2"])
126-
Assert.assertEquals("gcmcontinue", body["testvalue"])
127-
Assert.assertEquals("continue", body["gcmcontinue||"])
120+
Assert.assertEquals("categorymembers", body["generator"])
121+
Assert.assertEquals("file", body["gcmtype"])
122+
Assert.assertEquals("Watercraft moored off shore", body["gcmtitle"])
123+
Assert.assertEquals("timestamp", body["gcmsort"])
124+
Assert.assertEquals("desc", body["gcmdir"])
125+
Assert.assertEquals("testvalue", body["gcmcontinue"])
126+
Assert.assertEquals("gcmcontinue||", body["continue"])
128127
}
129128
}
130129

131-
assertEquals(categoryImagesContinued!!.blockingGet().size, 2)
130+
assertEquals(categoryImagesContinued.size, 2)
132131
}
133132

134133
private fun assertBasicRequestParameters(server: MockWebServer, method: String): RecordedRequest = server.takeRequest().let {
135134
Assert.assertEquals("/", it.requestUrl.encodedPath())
136135
Assert.assertEquals(method, it.method)
137-
Assert.assertEquals("Commons/${ConfigUtils.getVersionNameWithSha(RuntimeEnvironment.application)} (https://mediawiki.org/wiki/Apps/Commons) Android/${Build.VERSION.RELEASE}",
138-
it.getHeader("User-Agent"))
139-
if ("POST" == method) {
140-
Assert.assertEquals("application/x-www-form-urlencoded", it.getHeader("Content-Type"))
141-
}
142136
return it
143137
}
144138

0 commit comments

Comments
 (0)