From 42a9c7d9bc353381de23af21f7abf1b73ff2f598 Mon Sep 17 00:00:00 2001 From: savsch Date: Tue, 24 Dec 2024 02:07:43 +0530 Subject: [PATCH] Add unit tests and complete implementation --- .../nrw/commons/mwapi/OkHttpJsonApiClient.kt | 86 ++++++++++++++----- .../free/nrw/commons/nearby/NearbyPlaces.java | 37 ++++---- .../nrw/commons/OkHttpJsonApiClientTests.kt | 55 ++++++++++-- 3 files changed, 135 insertions(+), 43 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.kt b/app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.kt index e64d8821f..e3e8e8b70 100644 --- a/app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.kt +++ b/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("\${LONG}", String.format(Locale.ROOT, "%.4f", queryParams.center.longitude)) + .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.radius)) } } @@ -378,34 +384,72 @@ class OkHttpJsonApiClient @Inject constructor( @Throws(Exception::class) fun getNearbyPlaces( - screenTopRight: LatLng, - screenBottomLeft: LatLng, language: String, + queryParams: NearbyQueryParams, language: String, shouldQueryForMonuments: Boolean, customQuery: String? ): List? { Timber.d("CUSTOM_SPARQL: %s", (customQuery != null).toString()) + val locale = Locale.ROOT; val wikidataQuery: String = if (customQuery != null) { - customQuery - } else if (!shouldQueryForMonuments) { - FileUtils.readFromResource("/queries/rectangle_query_for_nearby.rq") - } else { - FileUtils.readFromResource("/queries/rectangle_query_for_nearby_monuments.rq") + 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 + 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() diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java index 6f619cc85..51c126ad8 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java +++ b/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= lowerLimit && itemCount < upperLimit){ + if (itemCount >= lowerLimit && itemCount < upperLimit) { break; } - if (targetRadius>maxRadius/2 && itemCount maxRadius / 2 && itemCount < lowerLimit / 5) { minRadius = targetRadius + (maxRadius - targetRadius + 1) / 2; continue; } - if (itemCount(); + return okHttpJsonApiClient.getNearbyPlaces( + new NearbyQueryParams.Radial(centerPoint, targetRadius / 100f), lang, shouldQueryForMonuments, + null); } /** diff --git a/app/src/test/kotlin/fr/free/nrw/commons/OkHttpJsonApiClientTests.kt b/app/src/test/kotlin/fr/free/nrw/commons/OkHttpJsonApiClientTests.kt index 3431efd6b..9e7891560 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/OkHttpJsonApiClientTests.kt +++ b/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()) - verify(call).execute() + try { + 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()) - verify(call).execute() + try { + 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() } }