Add unit tests and complete implementation

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

@@ -351,8 +351,14 @@ class OkHttpJsonApiClient @Inject constructor(
 
             is NearbyQueryParams.Radial -> {
                 FileUtils.readFromResource("/queries/radius_query_for_item_count.rq")
-                    .replace("\${LAT}", String.format(Locale.ROOT, "%.4f", queryParams.center.latitude))
+                    .replace(
-                    .replace("\${LONG}", String.format(Locale.ROOT, "%.4f", queryParams.center.longitude))
+                        "\${LAT}",
+                        String.format(Locale.ROOT, "%.4f", queryParams.center.latitude)
+                    )
+                    .replace(
+                        "\${LONG}",
+                        String.format(Locale.ROOT, "%.4f", queryParams.center.longitude)
+                    )
                     .replace("\${RAD}", String.format(Locale.ROOT, "%.2f", queryParams.radius))
             }
         }
@@ -378,34 +384,72 @@ class OkHttpJsonApiClient @Inject constructor(
 
     @Throws(Exception::class)
     fun getNearbyPlaces(
-        screenTopRight: LatLng,
+        queryParams: NearbyQueryParams, language: String,
-        screenBottomLeft: LatLng, language: String,
         shouldQueryForMonuments: Boolean, customQuery: String?
     ): List<Place>? {
         Timber.d("CUSTOM_SPARQL: %s", (customQuery != null).toString())
 
+        val locale = Locale.ROOT;
         val wikidataQuery: String = if (customQuery != null) {
-            customQuery
+                when (queryParams) {
-        } else if (!shouldQueryForMonuments) {
+                    is NearbyQueryParams.Rectangular -> {
-            FileUtils.readFromResource("/queries/rectangle_query_for_nearby.rq")
+                        val westCornerLat = queryParams.screenTopRight.latitude
-        } else {
+                        val westCornerLong = queryParams.screenTopRight.longitude
-            FileUtils.readFromResource("/queries/rectangle_query_for_nearby_monuments.rq")
+                        val eastCornerLat = queryParams.screenBottomLeft.latitude
+                        val eastCornerLong = queryParams.screenBottomLeft.longitude
+                        customQuery
+                            .replace("\${LAT_WEST}", String.format(locale, "%.4f", westCornerLat))
+                            .replace("\${LONG_WEST}", String.format(locale, "%.4f", westCornerLong))
+                            .replace("\${LAT_EAST}", String.format(locale, "%.4f", eastCornerLat))
+                            .replace("\${LONG_EAST}", String.format(locale, "%.4f", eastCornerLong))
+                            .replace("\${LANG}", language)
+                    }
+                    is NearbyQueryParams.Radial -> {
+                        Timber.e(
+                            "%s%s",
+                            "okHttpJsonApiClient.getNearbyPlaces invoked with custom query",
+                            "and radial coordinates. This is currently not supported."
+                        )
+                        ""
+                    }
+                }
+        } else when (queryParams) {
+            is NearbyQueryParams.Radial -> {
+                val placeHolderQuery: String = if (!shouldQueryForMonuments) {
+                    FileUtils.readFromResource("/queries/radius_query_for_nearby.rq")
+                } else {
+                    FileUtils.readFromResource("/queries/radius_query_for_nearby_monuments.rq")
+                }
+                placeHolderQuery.replace(
+                        "\${LAT}", String.format(locale, "%.4f", queryParams.center.latitude)
+                    ).replace(
+                        "\${LONG}", String.format(locale, "%.4f", queryParams.center.longitude)
+                    )
+                    .replace("\${RAD}", String.format(locale, "%.2f", queryParams.radius))
+            }
+
+            is NearbyQueryParams.Rectangular -> {
+                val placeHolderQuery: String = if (!shouldQueryForMonuments) {
+                    FileUtils.readFromResource("/queries/rectangle_query_for_nearby.rq")
+                } else {
+                    FileUtils.readFromResource("/queries/rectangle_query_for_nearby_monuments.rq")
+                }
+                val westCornerLat = queryParams.screenTopRight.latitude
+                val westCornerLong = queryParams.screenTopRight.longitude
+                val eastCornerLat = queryParams.screenBottomLeft.latitude
+                val eastCornerLong = queryParams.screenBottomLeft.longitude
+                placeHolderQuery
+                    .replace("\${LAT_WEST}", String.format(locale, "%.4f", westCornerLat))
+                    .replace("\${LONG_WEST}", String.format(locale, "%.4f", westCornerLong))
+                    .replace("\${LAT_EAST}", String.format(locale, "%.4f", eastCornerLat))
+                    .replace("\${LONG_EAST}", String.format(locale, "%.4f", eastCornerLong))
+                    .replace("\${LANG}", language)
+            }
         }
 
-        val westCornerLat = screenTopRight.latitude
-        val westCornerLong = screenTopRight.longitude
-        val eastCornerLat = screenBottomLeft.latitude
-        val eastCornerLong = screenBottomLeft.longitude
-
-        val query = wikidataQuery
-            .replace("\${LAT_WEST}", String.format(Locale.ROOT, "%.4f", westCornerLat))
-            .replace("\${LONG_WEST}", String.format(Locale.ROOT, "%.4f", westCornerLong))
-            .replace("\${LAT_EAST}", String.format(Locale.ROOT, "%.4f", eastCornerLat))
-            .replace("\${LONG_EAST}", String.format(Locale.ROOT, "%.4f", eastCornerLong))
-            .replace("\${LANG}", language)
         val urlBuilder: HttpUrl.Builder = sparqlQueryUrl.toHttpUrlOrNull()!!
             .newBuilder()
-            .addQueryParameter("query", query)
+            .addQueryParameter("query", wikidataQuery)
             .addQueryParameter("format", "json")
 
         val request: Request = Request.Builder()

app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java

@@ -119,51 +119,56 @@ public class NearbyPlaces {
         final fr.free.nrw.commons.location.LatLng screenBottomLeft, final String lang,
         final boolean shouldQueryForMonuments,
         @Nullable final String customQuery) throws Exception {
-        if (customQuery != null){
+        if (customQuery != null) {
             return okHttpJsonApiClient
-                .getNearbyPlaces(screenTopRight, screenBottomLeft, lang, shouldQueryForMonuments,
+                .getNearbyPlaces(
+                    new NearbyQueryParams.Rectangular(screenTopRight, screenBottomLeft), lang,
+                    shouldQueryForMonuments,
                     customQuery);
         }
 
-        final int lowerLimit = 1000, upperLimit=1500;
+        final int lowerLimit = 1000, upperLimit = 1500;
 
         final float[] results = new float[1];
         Location.distanceBetween(centerPoint.getLatitude(), screenTopRight.getLongitude(),
             centerPoint.getLatitude(), screenBottomLeft.getLongitude(), results);
-        final float longGap = results[0]/1000f;
+        final float longGap = results[0] / 1000f;
         Location.distanceBetween(screenTopRight.getLatitude(), centerPoint.getLongitude(),
             screenBottomLeft.getLatitude(), centerPoint.getLongitude(), results);
-        final float latGap = results[0]/1000f;
+        final float latGap = results[0] / 1000f;
 
-        if (Math.max(longGap,latGap)<100f){
+        if (Math.max(longGap, latGap) < 100f) {
             final int itemCount = okHttpJsonApiClient.getNearbyItemCount(
                 new NearbyQueryParams.Rectangular(screenTopRight, screenBottomLeft));
-            if(itemCount<upperLimit) {
+            if (itemCount < upperLimit) {
-                return okHttpJsonApiClient.getNearbyPlaces(screenTopRight, screenBottomLeft, lang,
+                return okHttpJsonApiClient.getNearbyPlaces(
+                    new NearbyQueryParams.Rectangular(screenTopRight, screenBottomLeft), lang,
                     shouldQueryForMonuments, null);
             }
         }
 
-        int minRadius = 0, maxRadius = Math.round(Math.min(100f, Math.min(longGap, latGap)))*100;
+        int minRadius = 0, maxRadius = Math.round(Math.min(100f, Math.min(longGap, latGap))) * 100;
-        int targetRadius = maxRadius/2;
+        int targetRadius = maxRadius / 2;
-        while (minRadius<maxRadius) {
+        while (minRadius < maxRadius) {
             targetRadius = minRadius + (maxRadius - minRadius + 1) / 2;
             final int itemCount = okHttpJsonApiClient.getNearbyItemCount(
                 new NearbyQueryParams.Radial(centerPoint, targetRadius / 100f));
-            if (itemCount >= lowerLimit && itemCount < upperLimit){
+            if (itemCount >= lowerLimit && itemCount < upperLimit) {
                 break;
             }
-            if (targetRadius>maxRadius/2 && itemCount<lowerLimit/5) {
+            if (targetRadius > maxRadius / 2 && itemCount < lowerLimit / 5) {
                 minRadius = targetRadius + (maxRadius - targetRadius + 1) / 2;
                 continue;
             }
-            if (itemCount<upperLimit) {
+            if (itemCount < upperLimit) {
                 minRadius = targetRadius;
             } else {
-                 maxRadius = targetRadius - 1;
+                maxRadius = targetRadius - 1;
             }
         }
-        return new java.util.ArrayList<Place>();
+        return okHttpJsonApiClient.getNearbyPlaces(
+            new NearbyQueryParams.Radial(centerPoint, targetRadius / 100f), lang, shouldQueryForMonuments,
+            null);
     }
 
     /**

app/src/test/kotlin/fr/free/nrw/commons/OkHttpJsonApiClientTests.kt

@@ -6,6 +6,7 @@ import com.nhaarman.mockitokotlin2.verify
 import fr.free.nrw.commons.explore.depictions.DepictsClient
 import fr.free.nrw.commons.location.LatLng
 import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient
+import fr.free.nrw.commons.nearby.model.NearbyQueryParams
 import okhttp3.Call
 import okhttp3.HttpUrl
 import okhttp3.OkHttpClient
@@ -14,6 +15,7 @@ import org.junit.Before
 import org.junit.Test
 import org.mockito.Mock
 import org.mockito.Mockito
+import org.mockito.Mockito.times
 import org.mockito.MockitoAnnotations
 import java.lang.Exception
 
@@ -64,12 +66,16 @@ class OkHttpJsonApiClientTests {
         Mockito.`when`(response.message).thenReturn("test")
         try {
             okHttpJsonApiClient.getNearbyPlaces(latLng, "test", 10.0, "test")
-            okHttpJsonApiClient.getNearbyPlaces(latLng, latLng, "test", true, "test")
         } catch (e: Exception) {
             assert(e.message.equals("test"))
         }
-        verify(okhttpClient).newCall(any())
+        try {
-        verify(call).execute()
+            okHttpJsonApiClient.getNearbyPlaces(NearbyQueryParams.Rectangular(latLng, latLng), "test", true, "test")
+        } catch (e: Exception) {
+            assert(e.message.equals("test"))
+        }
+        verify(okhttpClient, times(2)).newCall(any())
+        verify(call, times(2)).execute()
     }
 
     @Test
@@ -77,11 +83,48 @@ class OkHttpJsonApiClientTests {
         Mockito.`when`(response.message).thenReturn("test")
         try {
             okHttpJsonApiClient.getNearbyPlaces(latLng, "test", 10.0, null)
-            okHttpJsonApiClient.getNearbyPlaces(latLng, latLng, "test", true, null)
         } catch (e: Exception) {
             assert(e.message.equals("test"))
         }
-        verify(okhttpClient).newCall(any())
+        try {
-        verify(call).execute()
+            okHttpJsonApiClient.getNearbyPlaces(
+                NearbyQueryParams.Rectangular(latLng, latLng),
+                "test",
+                true,
+                null
+            )
+
+        } catch (e: Exception) {
+            assert(e.message.equals("test"))
+        }
+        try {
+            okHttpJsonApiClient.getNearbyPlaces(
+                NearbyQueryParams.Radial(latLng, 10f),
+                "test",
+                true,
+                null
+            )
+        } catch (e: Exception) {
+            assert(e.message.equals("test"))
+        }
+        verify(okhttpClient, times(3)).newCall(any())
+        verify(call, times(3)).execute()
+    }
+
+    @Test
+    fun testGetNearbyItemCount() {
+        Mockito.`when`(response.message).thenReturn("test")
+        try {
+            okHttpJsonApiClient.getNearbyItemCount(NearbyQueryParams.Radial(latLng, 10f))
+        } catch (e: Exception) {
+            assert(e.message.equals("test"))
+        }
+        try {
+            okHttpJsonApiClient.getNearbyItemCount(NearbyQueryParams.Rectangular(latLng, latLng))
+        } catch (e: Exception) {
+            assert(e.message.equals("test"))
+        }
+        verify(okhttpClient, times(2)).newCall(any())
+        verify(call, times(2)).execute()
     }
 }