Nearby: No longer keeps loading until timeout when map is zoomed out (#6070)

* Nearby: Search for actual map center

* Add query syntax and methods

* Nearby: Added binary search for loading pins

* Add NearbyQueryParams and refactor

* Add unit tests and complete implementation

* Nearby: Increase max radius from 100km to 300km

* Nearby: Centermost pins now appear on top

* getNearbyItemCount: Added javadoc

---------

Co-authored-by: Nicolas Raoul <nicolas.raoul@gmail.com>

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

@@ -2,6 +2,7 @@ package fr.free.nrw.commons.mwapi
 
 import android.text.TextUtils
 import com.google.gson.Gson
+import com.google.gson.JsonParser
 import fr.free.nrw.commons.BuildConfig
 import fr.free.nrw.commons.campaigns.CampaignResponseDTO
 import fr.free.nrw.commons.explore.depictions.DepictsClient
@@ -10,6 +11,7 @@ import fr.free.nrw.commons.fileusages.GlobalFileUsagesResponse
 import fr.free.nrw.commons.location.LatLng
 import fr.free.nrw.commons.nearby.Place
 import fr.free.nrw.commons.nearby.model.ItemsClass
+import fr.free.nrw.commons.nearby.model.NearbyQueryParams
 import fr.free.nrw.commons.nearby.model.NearbyResponse
 import fr.free.nrw.commons.nearby.model.PlaceBindings
 import fr.free.nrw.commons.profile.achievements.FeaturedImages
@@ -330,36 +332,130 @@ class OkHttpJsonApiClient @Inject constructor(
         throw Exception(response.message)
     }
 
+    /**
+     * Returns the count of items in the specified area by querying Wikidata.
+     *
+     * @param queryParams: a `NearbyQueryParam` specifying the geographical area.
+     * @return The count of items in the specified area.
+     */
+    @Throws(Exception::class)
+    fun getNearbyItemCount(
+        queryParams: NearbyQueryParams
+    ): Int {
+        val wikidataQuery: String = when (queryParams) {
+            is NearbyQueryParams.Rectangular -> {
+                val westCornerLat = queryParams.screenTopRight.latitude
+                val westCornerLong = queryParams.screenTopRight.longitude
+                val eastCornerLat = queryParams.screenBottomLeft.latitude
+                val eastCornerLong = queryParams.screenBottomLeft.longitude
+                FileUtils.readFromResource("/queries/rectangle_query_for_item_count.rq")
+                    .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))
+            }
+
+            is NearbyQueryParams.Radial -> {
+                FileUtils.readFromResource("/queries/radius_query_for_item_count.rq")
+                    .replace(
+                        "\${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.radiusInKm))
+            }
+        }
+
+        val urlBuilder: HttpUrl.Builder = sparqlQueryUrl.toHttpUrlOrNull()!!
+            .newBuilder()
+            .addQueryParameter("query", wikidataQuery)
+            .addQueryParameter("format", "json")
+
+        val request: Request = Request.Builder()
+            .url(urlBuilder.build())
+            .build()
+
+        val response = okHttpClient.newCall(request).execute()
+        if (response.body != null && response.isSuccessful) {
+            val json = response.body!!.string()
+            return JsonParser.parseString(json).getAsJsonObject().getAsJsonObject("results")
+                .getAsJsonArray("bindings").get(0).getAsJsonObject().getAsJsonObject("itemCount")
+                .get("value").asInt
+        }
+        throw Exception(response.message)
+    }
+
     @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.radiusInKm))
+            }
+
+            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/NearbyController.java

@@ -196,8 +196,9 @@ public class NearbyController extends MapController {
             return null;
         }
 
-        List<Place> places = nearbyPlaces.getFromWikidataQuery(screenTopRight, screenBottomLeft,
+        List<Place> places = nearbyPlaces.getFromWikidataQuery(currentLatLng, screenTopRight,
-            Locale.getDefault().getLanguage(), shouldQueryForMonuments, customQuery);
+            screenBottomLeft, Locale.getDefault().getLanguage(), shouldQueryForMonuments,
+            customQuery);
 
         if (null != places && places.size() > 0) {
             LatLng[] boundaryCoordinates = {

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

@@ -1,6 +1,8 @@
 package fr.free.nrw.commons.nearby;
 
+import android.location.Location;
 import androidx.annotation.Nullable;
+import fr.free.nrw.commons.nearby.model.NearbyQueryParams;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
@@ -101,6 +103,7 @@ public class NearbyPlaces {
      * Retrieves a list of places from a Wikidata query based on screen coordinates and optional
      * parameters.
      *
+     * @param centerPoint             The center of the map, used for radius queries if required.
      * @param screenTopRight          The top right corner of the screen (latitude, longitude).
      * @param screenBottomLeft        The bottom left corner of the screen (latitude, longitude).
      * @param lang                    The language for the query.
@@ -111,13 +114,70 @@ public class NearbyPlaces {
      * @throws Exception If an error occurs during the retrieval process.
      */
     public List<Place> getFromWikidataQuery(
+        final fr.free.nrw.commons.location.LatLng centerPoint,
         final fr.free.nrw.commons.location.LatLng screenTopRight,
         final fr.free.nrw.commons.location.LatLng screenBottomLeft, final String lang,
         final boolean shouldQueryForMonuments,
         @Nullable final String customQuery) throws Exception {
-        return okHttpJsonApiClient
+        if (customQuery != null) {
-            .getNearbyPlaces(screenTopRight, screenBottomLeft, lang, shouldQueryForMonuments,
+            return okHttpJsonApiClient
-                customQuery);
+                .getNearbyPlaces(
+                    new NearbyQueryParams.Rectangular(screenTopRight, screenBottomLeft), lang,
+                    shouldQueryForMonuments,
+                    customQuery);
+        }
+
+        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;
+        Location.distanceBetween(screenTopRight.getLatitude(), centerPoint.getLongitude(),
+            screenBottomLeft.getLatitude(), centerPoint.getLongitude(), results);
+        final float latGap = results[0] / 1000f;
+
+        if (Math.max(longGap, latGap) < 100f) {
+            final int itemCount = okHttpJsonApiClient.getNearbyItemCount(
+                new NearbyQueryParams.Rectangular(screenTopRight, screenBottomLeft));
+            if (itemCount < upperLimit) {
+                return okHttpJsonApiClient.getNearbyPlaces(
+                    new NearbyQueryParams.Rectangular(screenTopRight, screenBottomLeft), lang,
+                    shouldQueryForMonuments, null);
+            }
+        }
+
+        // minRadius, targetRadius and maxRadius are radii in decameters
+        // unlike other radii here, which are in kilometers, to avoid looping over
+        // floating point values
+        int minRadius = 0, maxRadius = Math.round(Math.min(300f, Math.min(longGap, latGap))) * 100;
+        int targetRadius = maxRadius / 2;
+        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) {
+                break;
+            }
+            if (targetRadius > maxRadius / 2 && itemCount < lowerLimit / 5) { // fast forward
+                minRadius = targetRadius;
+                targetRadius = minRadius + (maxRadius - minRadius + 1) / 2;
+                minRadius = targetRadius;
+                if (itemCount < lowerLimit / 10 && minRadius < maxRadius) { // fast forward again
+                    targetRadius = minRadius + (maxRadius - minRadius + 1) / 2;
+                    minRadius = targetRadius;
+                }
+                continue;
+            }
+            if (itemCount < upperLimit) {
+                minRadius = targetRadius;
+            } else {
+                maxRadius = targetRadius - 1;
+            }
+        }
+        return okHttpJsonApiClient.getNearbyPlaces(
+            new NearbyQueryParams.Radial(centerPoint, targetRadius / 100f), lang, shouldQueryForMonuments,
+            null);
     }
 
     /**

app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java

@@ -1101,19 +1101,19 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
                 eastCornerLong, 0);
             if (currentLatLng.equals(
                 getLastMapFocus())) { // Means we are checking around current location
-                populatePlacesForCurrentLocation(getLastMapFocus(), screenTopRightLatLng,
+                populatePlacesForCurrentLocation(getMapFocus(), screenTopRightLatLng,
                     screenBottomLeftLatLng, currentLatLng, null);
             } else {
-                populatePlacesForAnotherLocation(getLastMapFocus(), screenTopRightLatLng,
+                populatePlacesForAnotherLocation(getMapFocus(), screenTopRightLatLng,
                     screenBottomLeftLatLng, currentLatLng, null);
             }
         } else {
             if (currentLatLng.equals(
                 getLastMapFocus())) { // Means we are checking around current location
-                populatePlacesForCurrentLocation(getLastMapFocus(), screenTopRightLatLng,
+                populatePlacesForCurrentLocation(getMapFocus(), screenTopRightLatLng,
                     screenBottomLeftLatLng, currentLatLng, null);
             } else {
-                populatePlacesForAnotherLocation(getLastMapFocus(), screenTopRightLatLng,
+                populatePlacesForAnotherLocation(getMapFocus(), screenTopRightLatLng,
                     screenBottomLeftLatLng, currentLatLng, null);
             }
         }
@@ -1887,9 +1887,12 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
     @Override
     public void replaceMarkerOverlays(final List<MarkerPlaceGroup> markerPlaceGroups) {
         ArrayList<Marker> newMarkers = new ArrayList<>(markerPlaceGroups.size());
-        for (MarkerPlaceGroup markerPlaceGroup : markerPlaceGroups) {
+        // iterate in reverse so that the nearest pins get rendered on top
+        for (int i = markerPlaceGroups.size() - 1; i >= 0; i--) {
             newMarkers.add(
-                convertToMarker(markerPlaceGroup.getPlace(), markerPlaceGroup.getIsBookmarked()));
+                convertToMarker(markerPlaceGroups.get(i).getPlace(),
+                markerPlaceGroups.get(i).getIsBookmarked())
+            );
         }
         clearAllMarkers();
         binding.map.getOverlays().addAll(newMarkers);

app/src/main/java/fr/free/nrw/commons/nearby/model/NearbyQueryParams.kt

@@ -0,0 +1,10 @@
+package fr.free.nrw.commons.nearby.model
+
+import fr.free.nrw.commons.location.LatLng
+
+sealed class NearbyQueryParams {
+    class Rectangular(val screenTopRight: LatLng, val screenBottomLeft: LatLng) :
+        NearbyQueryParams()
+
+    class Radial(val center: LatLng, val radiusInKm: Float) : NearbyQueryParams()
+}

app/src/main/resources/queries/radius_query_for_item_count.rq

@@ -0,0 +1,9 @@
+SELECT (COUNT(?item) AS ?itemCount)
+WHERE {
+  # Around given location.
+  SERVICE wikibase:around {
+    ?item wdt:P625 ?location.
+    bd:serviceParam wikibase:center "Point(${LONG} ${LAT})"^^geo:wktLiteral.
+    bd:serviceParam wikibase:radius "${RAD}" . # Radius in kilometers.
+  }
+}

app/src/main/resources/queries/radius_query_for_nearby.rq

@@ -0,0 +1,12 @@
+SELECT
+  ?item
+  (SAMPLE(?location) as ?location)
+WHERE {
+  # Around given location.
+  SERVICE wikibase:around {
+    ?item wdt:P625 ?location.
+    bd:serviceParam wikibase:center "Point(${LONG} ${LAT})"^^geo:wktLiteral.
+    bd:serviceParam wikibase:radius "${RAD}" . # Radius in kilometers.
+  }
+}
+GROUP BY ?item

app/src/main/resources/queries/radius_query_for_nearby_monuments.rq

@@ -0,0 +1,25 @@
+SELECT
+  ?item
+  (SAMPLE(?location) as ?location)
+  (SAMPLE(?monument) AS ?monument)
+WHERE {
+  # Around given location.
+  SERVICE wikibase:around {
+    ?item wdt:P625 ?location.
+    bd:serviceParam wikibase:center "Point(${LONG} ${LAT})"^^geo:wktLiteral.
+    bd:serviceParam wikibase:radius "${RAD}" . # Radius in kilometers.
+  }
+
+  # Wiki Loves Monuments
+  OPTIONAL {?item p:P1435 ?monument}
+  OPTIONAL {?item p:P2186 ?monument}
+  OPTIONAL {?item p:P1459 ?monument}
+  OPTIONAL {?item p:P1460 ?monument}
+  OPTIONAL {?item p:P1216 ?monument}
+  OPTIONAL {?item p:P709 ?monument}
+  OPTIONAL {?item p:P718 ?monument}
+  OPTIONAL {?item p:P5694 ?monument}
+  OPTIONAL {?item p:P3426 ?monument}
+
+}
+GROUP BY ?item

app/src/main/resources/queries/rectangle_query_for_item_count.rq

@@ -0,0 +1,8 @@
+SELECT (COUNT(?item) AS ?itemCount)
+WHERE {
+  SERVICE wikibase:box {
+    ?item wdt:P625 ?location.
+    bd:serviceParam wikibase:cornerWest "Point(${LONG_WEST} ${LAT_WEST})"^^geo:wktLiteral.
+    bd:serviceParam wikibase:cornerEast "Point(${LONG_EAST} ${LAT_EAST})"^^geo:wktLiteral.
+  }
+}

app/src/main/resources/queries/rectangle_query_for_nearby.rq

@@ -8,6 +8,5 @@ WHERE {
      bd:serviceParam wikibase:cornerWest "Point(${LONG_WEST} ${LAT_WEST})"^^geo:wktLiteral.
      bd:serviceParam wikibase:cornerEast "Point(${LONG_EAST} ${LAT_EAST})"^^geo:wktLiteral.
   }
-
 }
 GROUP BY ?item

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()
     }
 }