Skip to content

Commit 678bd33

Browse files
ashishkumar468ashishkumar468
authored
Make Single Query for Nearby and WLM pins (commons-app#4573)
* Merge nearby and monument queries * Bug Fix- query resource path change on shouldQueryForMonuments * Bug Fixes 1. Propagate exceptions for nearby API calls to caller 2. Fix too much work on main thread exception in NearbyParentFragment
1 parent cba99ae commit 678bd33

File tree

10 files changed

+148
-187
lines changed

10 files changed

+148
-187
lines changed

‎app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ private void displayYouWontSeeNearbyMessage() {
454454
private void updateClosestNearbyCardViewInfo() {
455455
curLatLng = locationManager.getLastLocation();
456456
compositeDisposable.add(Observable.fromCallable(() -> nearbyController
457-
.loadAttractionsFromLocation(curLatLng, curLatLng, true, false)) // thanks to boolean, it will only return closest result
457+
.loadAttractionsFromLocation(curLatLng, curLatLng, true, false, false)) // thanks to boolean, it will only return closest result
458458
.subscribeOn(Schedulers.io())
459459
.observeOn(AndroidSchedulers.mainThread())
460460
.subscribe(this::updateNearbyNotification,

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

Lines changed: 29 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import android.text.TextUtils;
77
import androidx.annotation.NonNull;
8+
import androidx.annotation.Nullable;
89
import com.google.gson.Gson;
910
import fr.free.nrw.commons.campaigns.CampaignResponseDTO;
1011
import fr.free.nrw.commons.explore.depictions.DepictsClient;
@@ -264,107 +265,54 @@ public Single<FeedbackResponse> getAchievements(String userName) {
264265
});
265266
}
266267

267-
public Observable<List<Place>> getNearbyPlaces(LatLng cur, String language, double radius)
268+
@Nullable
269+
public List<Place> getNearbyPlaces(final LatLng cur, final String language, final double radius,
270+
final boolean shouldQueryForMonuments)
268271
throws Exception {
269272

270273
Timber.d("Fetching nearby items at radius %s", radius);
271-
String wikidataQuery = FileUtils.readFromResource("/queries/nearby_query.rq");
272-
String query = wikidataQuery
274+
final String wikidataQuery;
275+
if (!shouldQueryForMonuments) {
276+
wikidataQuery = FileUtils.readFromResource("/queries/nearby_query.rq");
277+
} else {
278+
wikidataQuery = FileUtils.readFromResource("/queries/nearby_query_monuments.rq");
279+
}
280+
final String query = wikidataQuery
273281
.replace("${RAD}", String.format(Locale.ROOT, "%.2f", radius))
274282
.replace("${LAT}", String.format(Locale.ROOT, "%.4f", cur.getLatitude()))
275283
.replace("${LONG}", String.format(Locale.ROOT, "%.4f", cur.getLongitude()))
276284
.replace("${LANG}", language);
277285

278-
HttpUrl.Builder urlBuilder = HttpUrl
286+
final HttpUrl.Builder urlBuilder = HttpUrl
279287
.parse(sparqlQueryUrl)
280288
.newBuilder()
281289
.addQueryParameter("query", query)
282290
.addQueryParameter("format", "json");
283291

284-
Request request = new Request.Builder()
292+
final Request request = new Request.Builder()
285293
.url(urlBuilder.build())
286294
.build();
287295

288-
return Observable.fromCallable(() -> {
289-
Response response = okHttpClient.newCall(request).execute();
290-
if (response != null && response.body() != null && response.isSuccessful()) {
291-
String json = response.body().string();
292-
if (json == null) {
293-
return new ArrayList<>();
294-
}
295-
NearbyResponse nearbyResponse = gson.fromJson(json, NearbyResponse.class);
296-
List<NearbyResultItem> bindings = nearbyResponse.getResults().getBindings();
297-
List<Place> places = new ArrayList<>();
298-
for (NearbyResultItem item : bindings) {
299-
places.add(Place.from(item));
296+
final Response response = okHttpClient.newCall(request).execute();
297+
if (response.body() != null && response.isSuccessful()) {
298+
final String json = response.body().string();
299+
final NearbyResponse nearbyResponse = gson.fromJson(json, NearbyResponse.class);
300+
final List<NearbyResultItem> bindings = nearbyResponse.getResults().getBindings();
301+
final List<Place> places = new ArrayList<>();
302+
for (final NearbyResultItem item : bindings) {
303+
final Place placeFromNearbyItem = Place.from(item);
304+
if (shouldQueryForMonuments && item.getMonument() != null) {
305+
placeFromNearbyItem.setMonument(true);
306+
} else {
307+
placeFromNearbyItem.setMonument(false);
300308
}
301-
return places;
309+
places.add(placeFromNearbyItem);
302310
}
303-
return new ArrayList<>();
304-
});
311+
return places;
312+
}
313+
throw new Exception(response.message());
305314
}
306315

307-
/**
308-
* Wikidata query to fetch monuments
309-
*
310-
* @param cur : The current location coordinates
311-
* @param language : The language
312-
* @param radius : The radius around the current location within which we expect the results
313-
* @return
314-
* @throws IOException
315-
*/
316-
public Observable<List<Place>> getNearbyMonuments(LatLng cur, String language, final double radius){
317-
Timber.d("Fetching monuments at radius %s", radius);
318-
final String wikidataQuery;
319-
try {
320-
wikidataQuery = FileUtils.readFromResource("/queries/monuments_query.rq");
321-
if (TextUtils.isEmpty(language)) {
322-
language = "en";
323-
}
324-
String query = wikidataQuery
325-
.replace("${RAD}", String.format(Locale.ROOT, "%.2f", radius))
326-
.replace("${LAT}", String.format(Locale.ROOT, "%.4f", cur.getLatitude()))
327-
.replace("${LONG}", String.format(Locale.ROOT, "%.4f", cur.getLongitude()))
328-
.replace("${LANG}", language);
329-
330-
HttpUrl.Builder urlBuilder = HttpUrl
331-
.parse(sparqlQueryUrl)
332-
.newBuilder()
333-
.addQueryParameter("query", query)
334-
.addQueryParameter("format", "json");
335-
336-
Request request = new Request.Builder()
337-
.url(urlBuilder.build())
338-
.build();
339-
340-
Timber.d("Monuments URL: %s", request.url().toString());
341-
342-
return Observable.fromCallable(() -> {
343-
final Response response = okHttpClient.newCall(request).execute();
344-
if (response != null && response.body() != null && response.isSuccessful()) {
345-
final String json = response.body().string();
346-
if (json == null) {
347-
return new ArrayList<>();
348-
}
349-
350-
final NearbyResponse nearbyResponse = gson.fromJson(json, NearbyResponse.class);
351-
final List<NearbyResultItem> bindings = nearbyResponse.getResults().getBindings();
352-
final List<Place> places = new ArrayList<>();
353-
for (final NearbyResultItem item : bindings) {
354-
final Place place = Place.from(item);
355-
place.setMonument(true);
356-
places.add(place);
357-
}
358-
return places;
359-
}
360-
return new ArrayList<>();
361-
});
362-
} catch (final IOException e) {
363-
e.printStackTrace();
364-
return Observable.error(e);
365-
}
366-
}
367-
368316
/**
369317
* Get the QIDs of all Wikidata items that are subclasses of the given Wikidata item. Example:
370318
* bridge -> suspended bridge, aqueduct, etc

‎app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ public NearbyController(NearbyPlaces nearbyPlaces) {
5757
* @return NearbyPlacesInfo a variable holds Place list without distance information
5858
* and boundary coordinates of current Place List
5959
*/
60-
public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng searchLatLng, boolean returnClosestResult, boolean checkingAroundCurrentLocation) throws IOException {
60+
public NearbyPlacesInfo loadAttractionsFromLocation(final LatLng curLatLng, final LatLng searchLatLng,
61+
final boolean returnClosestResult, final boolean checkingAroundCurrentLocation,
62+
final boolean shouldQueryForMonuments) throws Exception {
6163

6264
Timber.d("Loading attractions near %s", searchLatLng);
6365
NearbyPlacesInfo nearbyPlacesInfo = new NearbyPlacesInfo();
@@ -66,7 +68,9 @@ public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng sea
6668
Timber.d("Loading attractions nearby, but curLatLng is null");
6769
return null;
6870
}
69-
List<Place> places = nearbyPlaces.radiusExpander(searchLatLng, Locale.getDefault().getLanguage(), returnClosestResult);
71+
List<Place> places = nearbyPlaces
72+
.radiusExpander(searchLatLng, Locale.getDefault().getLanguage(), returnClosestResult,
73+
shouldQueryForMonuments);
7074

7175
if (null != places && places.size() > 0) {
7276
LatLng[] boundaryCoordinates = {places.get(0).location, // south
@@ -128,11 +132,6 @@ public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng sea
128132
}
129133
}
130134

131-
public Observable<List<Place>> queryWikiDataForMonuments(
132-
final LatLng latLng, final String language) {
133-
return nearbyPlaces.queryWikiDataForMonuments(latLng, language);
134-
}
135-
136135
/**
137136
* Loads attractions from location for list view, we need to return Place data type.
138137
*

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

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package fr.free.nrw.commons.nearby;
22

3-
import io.reactivex.Observable;
43
import java.io.IOException;
5-
import java.io.InterruptedIOException;
64
import java.util.Collections;
75
import java.util.List;
86

@@ -41,12 +39,12 @@ public NearbyPlaces(OkHttpJsonApiClient okHttpJsonApiClient) {
4139
* @param lang user's language
4240
* @param returnClosestResult true if only the nearest point is desired
4341
* @return list of places obtained
44-
* @throws IOException if query fails
4542
*/
46-
List<Place> radiusExpander(LatLng curLatLng, String lang, boolean returnClosestResult) throws IOException {
43+
List<Place> radiusExpander(final LatLng curLatLng, final String lang, final boolean returnClosestResult
44+
, final boolean shouldQueryForMonuments) throws Exception {
4745

48-
int minResults;
49-
double maxRadius;
46+
final int minResults;
47+
final double maxRadius;
5048

5149
List<Place> places = Collections.emptyList();
5250

@@ -64,12 +62,7 @@ List<Place> radiusExpander(LatLng curLatLng, String lang, boolean returnClosestR
6462

6563
// Increase the radius gradually to find a satisfactory number of nearby places
6664
while (radius <= maxRadius) {
67-
try {
68-
places = getFromWikidataQuery(curLatLng, lang, radius);
69-
} catch (final Exception e) {
70-
Timber.e(e, "Exception in fetching nearby places");
71-
break;
72-
}
65+
places = getFromWikidataQuery(curLatLng, lang, radius, shouldQueryForMonuments);
7366
Timber.d("%d results at radius: %f", places.size(), radius);
7467
if (places.size() >= minResults) {
7568
break;
@@ -89,16 +82,12 @@ List<Place> radiusExpander(LatLng curLatLng, String lang, boolean returnClosestR
8982
* @param cur coordinates of search location
9083
* @param lang user's language
9184
* @param radius radius for search, as determined by radiusExpander()
85+
* @param shouldQueryForMonuments should the query include properites for monuments
9286
* @return list of places obtained
9387
* @throws IOException if query fails
9488
*/
95-
public List<Place> getFromWikidataQuery(LatLng cur, String lang, double radius) throws Exception {
96-
return okHttpJsonApiClient.getNearbyPlaces(cur, lang, radius).blockingSingle();
97-
}
98-
99-
public Observable<List<Place>> queryWikiDataForMonuments(
100-
LatLng latLng, String language) {
101-
return okHttpJsonApiClient
102-
.getNearbyMonuments(latLng, language, radius);
89+
public List<Place> getFromWikidataQuery(final LatLng cur, final String lang,
90+
final double radius, final boolean shouldQueryForMonuments) throws Exception {
91+
return okHttpJsonApiClient.getNearbyPlaces(cur, lang, radius, shouldQueryForMonuments);
10392
}
10493
}

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

Lines changed: 11 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -883,29 +883,11 @@ private void populatePlacesForCurrentLocation(final fr.free.nrw.commons.location
883883

884884
final Observable<NearbyPlacesInfo> nearbyPlacesInfoObservable = Observable
885885
.fromCallable(() -> nearbyController
886-
.loadAttractionsFromLocation(curlatLng, searchLatLng, false, true));
886+
.loadAttractionsFromLocation(curlatLng, searchLatLng,
887+
false, true, Utils.isMonumentsEnabled(new Date(), applicationKvStore)));
887888

888-
Observable<List<Place>> observableWikidataMonuments = Observable.empty();
889-
if(Utils.isMonumentsEnabled(new Date(), applicationKvStore)){
890-
observableWikidataMonuments =
891-
nearbyController
892-
.queryWikiDataForMonuments(searchLatLng, Locale.getDefault().getLanguage());
893-
894-
}
895-
896-
compositeDisposable.add(Observable.zip(nearbyPlacesInfoObservable
897-
, observableWikidataMonuments.onErrorReturn(throwable -> {
898-
showErrorMessage(getString(R.string.error_fetching_nearby_monuments) + throwable
899-
.getLocalizedMessage());
900-
return new ArrayList<>();
901-
}),
902-
(nearbyPlacesInfo, monuments) -> {
903-
final List<Place> places = mergeNearbyPlacesAndMonuments(nearbyPlacesInfo.placeList,
904-
monuments);
905-
nearbyPlacesInfo.placeList.clear();
906-
nearbyPlacesInfo.placeList.addAll(places);
907-
return nearbyPlacesInfo;
908-
}).subscribeOn(Schedulers.io())
889+
compositeDisposable.add(nearbyPlacesInfoObservable
890+
.subscribeOn(Schedulers.io())
909891
.observeOn(AndroidSchedulers.mainThread())
910892
.subscribe(nearbyPlacesInfo -> {
911893
updateMapMarkers(nearbyPlacesInfo, true);
@@ -925,28 +907,11 @@ private void populatePlacesForAnotherLocation(final fr.free.nrw.commons.location
925907

926908
final Observable<NearbyPlacesInfo> nearbyPlacesInfoObservable = Observable
927909
.fromCallable(() -> nearbyController
928-
.loadAttractionsFromLocation(curlatLng, searchLatLng, false, false));
929-
930-
Observable<List<Place>> observableWikidataMonuments = Observable.empty();
931-
if (Utils.isMonumentsEnabled(new Date(), applicationKvStore)) {
932-
observableWikidataMonuments = nearbyController
933-
.queryWikiDataForMonuments(searchLatLng, Locale.getDefault().getLanguage());
934-
935-
}
910+
.loadAttractionsFromLocation(curlatLng, searchLatLng,
911+
false, true, Utils.isMonumentsEnabled(new Date(), applicationKvStore)));
936912

937-
compositeDisposable.add(Observable.zip(nearbyPlacesInfoObservable
938-
, observableWikidataMonuments.onErrorReturn(throwable -> {
939-
showErrorMessage(getString(R.string.error_fetching_nearby_monuments) + throwable
940-
.getLocalizedMessage());
941-
return new ArrayList<>();
942-
}),
943-
(nearbyPlacesInfo, monuments) -> {
944-
final List<Place> places = mergeNearbyPlacesAndMonuments(nearbyPlacesInfo.placeList,
945-
monuments);
946-
nearbyPlacesInfo.placeList.clear();
947-
nearbyPlacesInfo.placeList.addAll(places);
948-
return nearbyPlacesInfo;
949-
}).subscribeOn(Schedulers.io())
913+
compositeDisposable.add(nearbyPlacesInfoObservable
914+
.subscribeOn(Schedulers.io())
950915
.observeOn(AndroidSchedulers.mainThread())
951916
.subscribe(nearbyPlacesInfo -> {
952917
updateMapMarkers(nearbyPlacesInfo, false);
@@ -961,25 +926,6 @@ private void populatePlacesForAnotherLocation(final fr.free.nrw.commons.location
961926
}));
962927
}
963928

964-
/**
965-
* If a nearby place happens to be a monument as well, don't make the Pin's overlap, instead
966-
* show it as a monument
967-
*
968-
* @param nearbyPlaces
969-
* @param monuments
970-
* @return
971-
*/
972-
private List<Place> mergeNearbyPlacesAndMonuments(List<Place> nearbyPlaces, List<Place> monuments){
973-
List<Place> allPlaces= new ArrayList<>();
974-
allPlaces.addAll(monuments);
975-
for (Place place : nearbyPlaces){
976-
if(!allPlaces.contains(place)){
977-
allPlaces.add(place);
978-
}
979-
}
980-
return allPlaces;
981-
}
982-
983929
/**
984930
* Populates places for your location, should be used for finding nearby places around a
985931
* location where you are.
@@ -1238,7 +1184,7 @@ public void enableFABRecenter() {
12381184
public void addCurrentLocationMarker(final fr.free.nrw.commons.location.LatLng curLatLng) {
12391185
if (null != curLatLng && !isPermissionDenied) {
12401186
ExecutorUtils.get().submit(() -> {
1241-
mapView.post(() -> removeCurrentLocationMarker());
1187+
removeCurrentLocationMarker();
12421188
Timber.d("Adds current location marker");
12431189

12441190
final Icon icon = IconFactory.getInstance(getContext())
@@ -1248,8 +1194,7 @@ public void addCurrentLocationMarker(final fr.free.nrw.commons.location.LatLng c
12481194
.position(new LatLng(curLatLng.getLatitude(),
12491195
curLatLng.getLongitude()));
12501196
currentLocationMarkerOptions.setIcon(icon); // Set custom icon
1251-
mapView.post(() -> currentLocationMarker = mapBox.addMarker(currentLocationMarkerOptions));
1252-
1197+
currentLocationMarker = mapBox.addMarker(currentLocationMarkerOptions);
12531198

12541199
final List<LatLng> circle = UiUtils
12551200
.createCircleArray(curLatLng.getLatitude(), curLatLng.getLongitude(),
@@ -1259,7 +1204,7 @@ public void addCurrentLocationMarker(final fr.free.nrw.commons.location.LatLng c
12591204
.addAll(circle)
12601205
.strokeColor(getResources().getColor(R.color.current_marker_stroke))
12611206
.fillColor(getResources().getColor(R.color.current_marker_fill));
1262-
mapView.post(() -> currentLocationPolygon = mapBox.addPolygon(currentLocationPolygonOptions));
1207+
currentLocationPolygon = mapBox.addPolygon(currentLocationPolygonOptions);
12631208

12641209
});
12651210
} else {

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ class NearbyResultItem(private val item: ResultTuple?,
1414
@field:SerializedName("pic") private val pic: ResultTuple?,
1515
@field:SerializedName("destroyed") private val destroyed: ResultTuple?,
1616
@field:SerializedName("description") private val description: ResultTuple?,
17-
@field:SerializedName("endTime") private val endTime: ResultTuple?) {
17+
@field:SerializedName("endTime") private val endTime: ResultTuple?,
18+
@field:SerializedName("monument") private val monument: ResultTuple?) {
1819

1920
fun getItem(): ResultTuple {
2021
return item ?: ResultTuple()
@@ -71,4 +72,8 @@ class NearbyResultItem(private val item: ResultTuple?,
7172
fun getAddress(): String {
7273
return address?.value?:""
7374
}
75+
76+
fun getMonument():ResultTuple?{
77+
return monument
78+
}
7479
}

0 commit comments

Comments
 (0)