From c891c2b0dfbdb4d2abf47140298333b18b4c6dbc Mon Sep 17 00:00:00 2001 From: Tanmay Gupta <119003089+savsch@users.noreply.github.com> Date: Fri, 20 Dec 2024 18:00:53 +0530 Subject: [PATCH] Nearby: Fix race condition and lag when loading pin details, faster overlay management (#6047) * temporary fixes part one * temporary fixes part two * temporary fixes part three * temporary fixes part four * temporary fixes part five * reformatting * remove code no longer in use * Migrate NearbyParentFragmentPresenter to Kotlin * Partially replace temporary experimental fixes * Replace temporary experimental fixes part 2 * Replace temporary experimental fixes part 3 * Replace temporary fixes completely * Fix caching and loading places in Nearby list * Add place bookmarking logic, Remove all old code * Nearby Presenter: Close channel properly * Nearby pins now load starting from the center Fixes #6049 * Add comments and javadoc for Nearby Presenter * Fix warnings, Fix formatting, Add javadoc * Pass unit tests --- .../nrw/commons/nearby/NearbyController.java | 2 +- .../NearbyParentFragmentContract.java | 6 +- .../fragments/NearbyParentFragment.java | 365 ++--------- .../NearbyParentFragmentPresenter.java | 368 ----------- .../NearbyParentFragmentPresenter.kt | 597 ++++++++++++++++++ .../NearbyParentFragmentPresenterTest.kt | 17 +- 6 files changed, 676 insertions(+), 679 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.java create mode 100644 app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java index 7bb311961..cc2442db6 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java @@ -20,8 +20,8 @@ import timber.log.Timber; public class NearbyController extends MapController { - private static final int MAX_RESULTS = 1000; private final NearbyPlaces nearbyPlaces; + public static final int MAX_RESULTS = 1000; public static double currentLocationSearchRadius = 10.0; //in kilometers public static LatLng currentLocation; // Users latest fetched location public static LatLng latestSearchLocation; // Can be current and camera target on search this area button is used diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/contract/NearbyParentFragmentContract.java b/app/src/main/java/fr/free/nrw/commons/nearby/contract/NearbyParentFragmentContract.java index bcf8d8421..3b08cf7cb 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/contract/NearbyParentFragmentContract.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/contract/NearbyParentFragmentContract.java @@ -2,11 +2,13 @@ package fr.free.nrw.commons.nearby.contract; import android.content.Context; import androidx.annotation.Nullable; +import androidx.lifecycle.LifecycleCoroutineScope; import fr.free.nrw.commons.BaseMarker; import fr.free.nrw.commons.kvstore.JsonKvStore; import fr.free.nrw.commons.location.LatLng; import fr.free.nrw.commons.location.LocationServiceManager.LocationChangeType; import fr.free.nrw.commons.nearby.Label; +import fr.free.nrw.commons.nearby.MarkerPlaceGroup; import fr.free.nrw.commons.nearby.Place; import java.util.List; @@ -68,7 +70,7 @@ public interface NearbyParentFragmentContract { Context getContext(); - void updateMapMarkers(List BaseMarkers); + void replaceMarkerOverlays(List markerPlaceGroups); void filterOutAllMarkers(); @@ -127,5 +129,7 @@ public interface NearbyParentFragmentContract { void setCheckboxUnknown(); void setAdvancedQuery(String query); + + void toggleBookmarkedStatus(Place place); } } diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java index fff4e4ca7..6b21681ab 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java @@ -23,8 +23,6 @@ import android.graphics.drawable.Drawable; import android.location.Location; import android.location.LocationManager; import android.net.Uri; -import android.os.Build.VERSION; -import android.os.Build.VERSION_CODES; import android.os.Bundle; import android.os.Environment; import android.os.Handler; @@ -56,6 +54,7 @@ import androidx.appcompat.app.AlertDialog.Builder; import androidx.constraintlayout.widget.ConstraintLayout; import androidx.core.content.ContextCompat; import androidx.core.content.FileProvider; +import androidx.lifecycle.LifecycleOwnerKt; import androidx.recyclerview.widget.DividerItemDecoration; import androidx.recyclerview.widget.GridLayoutManager; import androidx.recyclerview.widget.LinearLayoutManager; @@ -110,16 +109,12 @@ import fr.free.nrw.commons.wikidata.WikidataEditListener; import io.reactivex.Completable; import io.reactivex.Observable; import io.reactivex.android.schedulers.AndroidSchedulers; -import io.reactivex.disposables.Disposable; import io.reactivex.schedulers.Schedulers; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.text.SimpleDateFormat; import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Comparator; import java.util.Date; import java.util.List; import java.util.Locale; @@ -155,6 +150,29 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment FragmentNearbyParentBinding binding; + public final MapEventsOverlay mapEventsOverlay = new MapEventsOverlay(new MapEventsReceiver() { + @Override + public boolean singleTapConfirmedHelper(GeoPoint p) { + if (clickedMarker != null) { + clickedMarker.closeInfoWindow(); + } else { + Timber.e("CLICKED MARKER IS NULL"); + } + if (isListBottomSheetExpanded()) { + // Back should first hide the bottom sheet if it is expanded + hideBottomSheet(); + } else if (isDetailsBottomSheetVisible()) { + hideBottomDetailsSheet(); + } + return true; + } + + @Override + public boolean longPressHelper(GeoPoint p) { + return false; + } + }); + @Inject LocationServiceManager locationManager; @Inject @@ -217,7 +235,6 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment private Runnable searchRunnable; private static final long SCROLL_DELAY = 800; // Delay for debounce of onscroll, in milliseconds. - private List updatedPlacesList; private LatLng updatedLatLng; private boolean searchable; @@ -308,10 +325,6 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment * WLM URL */ public static final String WLM_URL = "https://commons.wikimedia.org/wiki/Commons:Mobile_app/Contributing_to_WLM_using_the_app"; - /** - * Saves response of list of places for the first time - */ - private List places = new ArrayList<>(); @NonNull public static NearbyParentFragment newInstance() { @@ -327,7 +340,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment view = binding.getRoot(); initNetworkBroadCastReceiver(); - presenter = new NearbyParentFragmentPresenter(bookmarkLocationDao); + presenter = new NearbyParentFragmentPresenter(bookmarkLocationDao, placesRepository, nearbyController); progressDialog = new ProgressDialog(getActivity()); progressDialog.setCancelable(false); progressDialog.setMessage("Saving in progress..."); @@ -452,28 +465,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment binding.map.getOverlays().add(scaleBarOverlay); binding.map.getZoomController().setVisibility(Visibility.NEVER); binding.map.getController().setZoom(ZOOM_LEVEL); - binding.map.getOverlays().add(new MapEventsOverlay(new MapEventsReceiver() { - @Override - public boolean singleTapConfirmedHelper(GeoPoint p) { - if (clickedMarker != null) { - clickedMarker.closeInfoWindow(); - } else { - Timber.e("CLICKED MARKER IS NULL"); - } - if (isListBottomSheetExpanded()) { - // Back should first hide the bottom sheet if it is expanded - hideBottomSheet(); - } else if (isDetailsBottomSheetVisible()) { - hideBottomDetailsSheet(); - } - return true; - } - - @Override - public boolean longPressHelper(GeoPoint p) { - return false; - } - })); + binding.map.getOverlays().add(mapEventsOverlay); binding.map.addMapListener(new MapListener() { @Override @@ -605,8 +597,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment return Unit.INSTANCE; }, (place, isBookmarked) -> { - updateMarker(isBookmarked, place, null); - binding.map.invalidate(); + presenter.toggleBookmarkedStatus(place); return Unit.INSTANCE; }, commonPlaceClickActions, @@ -670,19 +661,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment registerNetworkReceiver(); if (isResumed() && ((MainActivity) getActivity()).activeFragment == ActiveFragment.NEARBY) { if (locationPermissionsHelper.checkLocationPermission(getActivity())) { - if (lastFocusLocation == null && lastKnownLocation == null) { - locationPermissionGranted(); - } else{ - if (updatedPlacesList != null) { - if (!updatedPlacesList.isEmpty()) { - loadPlacesDataAsync(updatedPlacesList, updatedLatLng); - } else { - updateMapMarkers(updatedPlacesList, getLastMapFocus(), false); - } - }else { - locationPermissionGranted(); - } - } + locationPermissionGranted(); } else { startMapWithoutPermission(); } @@ -973,7 +952,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment @Override public void updateListFragment(final List placeList) { - places = placeList; + adapter.clear(); adapter.setItems(placeList); binding.bottomSheetNearby.noResultsMessage.setVisibility( placeList.isEmpty() ? View.VISIBLE : View.GONE); @@ -1367,13 +1346,8 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment ? getTextBetweenParentheses( updatedPlace.getLongDescription()) : updatedPlace.getLongDescription()); marker.showInfoWindow(); - for (int i = 0; i < updatedPlacesList.size(); i++) { - Place pl = updatedPlacesList.get(i); - if (pl.location == updatedPlace.location) { - updatedPlacesList.set(i, updatedPlace); - savePlaceToDatabase(place); - } - } + presenter.handlePinClicked(updatedPlace); + savePlaceToDatabase(place); Drawable icon = ContextCompat.getDrawable(getContext(), getIconFor(updatedPlace, isBookMarked)); marker.setIcon(icon); @@ -1412,12 +1386,10 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment setProgressBarVisibility(false); presenter.lockUnlockNearby(false); } else { - updateMapMarkers(nearbyPlacesInfo.placeList, nearbyPlacesInfo.currentLatLng, - true); + updateMapMarkers(nearbyPlacesInfo.placeList, searchLatLng, true); lastFocusLocation = searchLatLng; lastMapFocus = new GeoPoint(searchLatLng.getLatitude(), searchLatLng.getLongitude()); - loadPlacesDataAsync(nearbyPlacesInfo.placeList, nearbyPlacesInfo.currentLatLng); } }, throwable -> { @@ -1457,12 +1429,10 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment // curLatLng is used to calculate distance from the current location to the place // and distance is later on populated to the place - updateMapMarkers(nearbyPlacesInfo.placeList, nearbyPlacesInfo.currentLatLng, - false); + updateMapMarkers(nearbyPlacesInfo.placeList, searchLatLng, false); lastMapFocus = new GeoPoint(searchLatLng.getLatitude(), searchLatLng.getLongitude()); stopQuery(); - loadPlacesDataAsync(nearbyPlacesInfo.placeList, nearbyPlacesInfo.currentLatLng); } }, throwable -> { @@ -1475,167 +1445,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment })); } - public void loadPlacesDataAsync(List placeList, LatLng curLatLng) { - List places = new ArrayList<>(placeList); - - // Instead of loading all pins in a single SPARQL query, we query in batches. - // This variable controls the number of pins queried per batch. - int batchSize = 3; - - updatedLatLng = curLatLng; - updatedPlacesList = new ArrayList<>(placeList); - - // Sorts the places by distance to ensure the nearest pins are ready for the user as soon - // as possible. - if (VERSION.SDK_INT >= VERSION_CODES.N) { - Collections.sort(places, - Comparator.comparingDouble(place -> place.getDistanceInDouble(getMapFocus()))); - } - stopQuery = false; - processBatchesSequentially(places, batchSize, updatedPlacesList, curLatLng, 0); - } - - /** - * Processes a list of places in batches sequentially. This method handles the asynchronous - * processing of places, updating the map markers and updates the list of updated places accordingly. - * - * @param places The list of Place objects to be processed. - * @param batchSize The size of each batch to be processed. - * @param updatedPlaceList The list of Place objects to be updated. - * @param curLatLng The current location of the user. - * @param startIndex The starting index for the current batch. - */ - @SuppressLint("CheckResult") - private void processBatchesSequentially(List places, int batchSize, - List updatedPlaceList, LatLng curLatLng, int startIndex) { - if (startIndex >= places.size() || stopQuery) { - return; - } - - int endIndex = Math.min(startIndex + batchSize, places.size()); - List batch = places.subList(startIndex, endIndex); - for (int i = 0; i < batch.size(); i++) { - if (i == batch.size() - 1 && batch.get(i).name != "") { - processBatchesSequentially(places, batchSize, updatedPlaceList, curLatLng, - endIndex + batchSize); - return; - } - if (batch.get(i).name == "") { - if (i == 0) { - break; - } - processBatchesSequentially(places, batchSize, updatedPlaceList, curLatLng, - endIndex + i); - return; - } - } - - Disposable disposable = processBatch(batch, updatedPlaceList) - .subscribe(p -> { - if (stopQuery) { - return; - } - if (!p.isEmpty() && p != updatedPlaceList) { - synchronized (updatedPlaceList) { - updatedPlaceList.clear(); - updatedPlaceList.addAll((Collection) p); - } - } - updateMapMarkers(new ArrayList<>(updatedPlaceList), curLatLng, false); - processBatchesSequentially(places, batchSize, updatedPlaceList, curLatLng, endIndex); - }, throwable -> { - Timber.e(throwable); - showErrorMessage(getString(R.string.error_fetching_nearby_places) + throwable.getLocalizedMessage()); - setFilterState(); - }); - - compositeDisposable.add(disposable); - } - - /** - * Processes a batch of places, updating the provided place list with fetched or updated data. - * This method handles the asynchronous fetching and updating of places from the repository. - * - * @param batch The batch of Place objects to be processed. - * @param placeList The list of Place objects to be updated. - * @return An Observable emitting the updated list of Place objects. - */ - private Observable> processBatch(List batch, List placeList) { - List toBeProcessed = new ArrayList<>(); - - List> placeObservables = new ArrayList<>(); - - for (Place place : batch) { - Observable placeObservable = Observable - .fromCallable(() -> { - Place fetchedPlace = placesRepository.fetchPlace(place.entityID); - return fetchedPlace != null ? fetchedPlace : place; - }) - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .doOnNext(placeData -> { - if (placeData.equals(place)) { - toBeProcessed.add(place); - } else { - for (int i = 0; i < placeList.size(); i++) { - Place pl = placeList.get(i); - if (pl.location.equals(place.location)) { - placeList.set(i, placeData); - break; - } - } - } - }); - - placeObservables.add(placeObservable); - } - - return Observable.zip(placeObservables, objects -> toBeProcessed) - .flatMap(processedList -> { - if (processedList.isEmpty()) { - return Observable.just(placeList); - } - return Observable.fromCallable(() -> nearbyController.getPlaces(processedList)) - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .map(places -> { - if (stopQuery) { - return Collections.emptyList(); - } - if (places == null || places.isEmpty()) { - return Collections.emptyList(); - } else { - List updatedPlaceList = new ArrayList<>(placeList); - for (Place place : places) { - for (Place foundPlace : placeList) { - if (place.siteLinks.getWikidataLink() - .equals(foundPlace.siteLinks.getWikidataLink())) { - place.location = foundPlace.location; - place.distance = foundPlace.distance; - place.setMonument(foundPlace.isMonument()); - int index = updatedPlaceList.indexOf(foundPlace); - if (index != -1) { - updatedPlaceList.set(index, place); - savePlaceToDatabase(place); - } - break; - } - } - } - return updatedPlaceList; - } - }) - .onErrorReturn(throwable -> { - Timber.e(throwable); - showErrorMessage(getString(R.string.error_fetching_nearby_places) + " " - + throwable.getLocalizedMessage()); - setFilterState(); - return Collections.emptyList(); - }); - }); - } - - private void savePlaceToDatabase(Place place) { + public void savePlaceToDatabase(Place place) { compositeDisposable.add(placesRepository .save(place) .subscribeOn(Schedulers.io()) @@ -1661,8 +1471,8 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment */ private void updateMapMarkers(final List nearbyPlaces, final LatLng curLatLng, final boolean shouldUpdateSelectedMarker) { - presenter.updateMapMarkers(nearbyPlaces, curLatLng, shouldUpdateSelectedMarker); - setFilterState(); + presenter.updateMapMarkers(nearbyPlaces, curLatLng, + LifecycleOwnerKt.getLifecycleScope(getViewLifecycleOwner())); } @@ -1899,13 +1709,6 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment } } - @Override - public void updateMapMarkers(final List BaseMarkers) { - if (binding.map != null) { - presenter.updateMapMarkersToController(BaseMarkers); - } - } - @Override public void filterOutAllMarkers() { clearAllMarkers(); @@ -1925,8 +1728,11 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment final boolean displayExists = false; final boolean displayNeedsPhoto= false; final boolean displayWlm = false; - // Remove the previous markers before updating them - clearAllMarkers(); + if (selectedLabels == null || selectedLabels.size() == 0) { + replaceMarkerOverlays(NearbyController.markerLabelList); + return; + } + final ArrayList placeGroupsToShow = new ArrayList<>(); for (final MarkerPlaceGroup markerPlaceGroup : NearbyController.markerLabelList) { final Place place = markerPlaceGroup.getPlace(); // When label filter is engaged @@ -1967,19 +1773,12 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment } if (shouldUpdateMarker) { - updateMarker(markerPlaceGroup.getIsBookmarked(), place, - NearbyController.currentLocation); + placeGroupsToShow.add( + new MarkerPlaceGroup(markerPlaceGroup.getIsBookmarked(), place) + ); } } - if (selectedLabels == null || selectedLabels.size() == 0) { - ArrayList markerArrayList = new ArrayList<>(); - for (final MarkerPlaceGroup markerPlaceGroup : NearbyController.markerLabelList) { - BaseMarker nearbyBaseMarker = new BaseMarker(); - nearbyBaseMarker.setPlace(markerPlaceGroup.getPlace()); - markerArrayList.add(nearbyBaseMarker); - } - addMarkersToMap(markerArrayList); - } + replaceMarkerOverlays(placeGroupsToShow); } @Override @@ -1987,18 +1786,6 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment return binding.map == null ? null : getMapFocus(); } - /** - * Sets marker icon according to marker status. Sets title and distance. - * - * @param isBookmarked true if place is bookmarked - * @param place - * @param currentLatLng current location - */ - public void updateMarker(final boolean isBookmarked, final Place place, - @Nullable final LatLng currentLatLng) { - addMarkerToMap(place, isBookmarked); - } - /** * Highlights nearest place when user clicks on home nearby banner * @@ -2052,13 +1839,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment ); } - /** - * Adds a marker representing a place to the map with optional bookmark icon. - * - * @param place The Place object containing information about the location. - * @param isBookMarked A Boolean flag indicating whether the place is bookmarked or not. - */ - private void addMarkerToMap(Place place, Boolean isBookMarked) { + public Marker convertToMarker(Place place, Boolean isBookMarked) { Drawable icon = ContextCompat.getDrawable(getContext(), getIconFor(place, isBookMarked)); GeoPoint point = new GeoPoint(place.location.getLatitude(), place.location.getLongitude()); Marker marker = new Marker(binding.map); @@ -2094,22 +1875,27 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment bottomSheetDetailsBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED); return true; }); - binding.map.getOverlays().add(marker); + return marker; } /** * Adds multiple markers representing places to the map and handles item gestures. * - * @param nearbyBaseMarkers The list of Place objects containing information about the - * locations. + * @param markerPlaceGroups The list of marker place groups containing the places and + * their bookmarked status */ - private void addMarkersToMap(List nearbyBaseMarkers) { - - for(int i = 0; i< nearbyBaseMarkers.size(); i++){ - addMarkerToMap(nearbyBaseMarkers.get(i).getPlace(), false); + @Override + public void replaceMarkerOverlays(final List markerPlaceGroups) { + ArrayList newMarkers = new ArrayList<>(markerPlaceGroups.size()); + for (MarkerPlaceGroup markerPlaceGroup : markerPlaceGroups) { + newMarkers.add( + convertToMarker(markerPlaceGroup.getPlace(), markerPlaceGroup.getIsBookmarked())); } + clearAllMarkers(); + binding.map.getOverlays().addAll(newMarkers); } + /** * Extracts text between the first occurrence of '(' and its corresponding ')' in the input * string. @@ -2400,7 +2186,6 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment binding.map.invalidate(); GeoPoint geoPoint = mapCenter; if (geoPoint != null) { - List overlays = binding.map.getOverlays(); ScaleDiskOverlay diskOverlay = new ScaleDiskOverlay(this.getContext(), geoPoint, 2000, UnitOfMeasure.foot); @@ -2434,28 +2219,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment scaleBarOverlay.setBackgroundPaint(barPaint); scaleBarOverlay.enableScaleBar(); binding.map.getOverlays().add(scaleBarOverlay); - binding.map.getOverlays().add(new MapEventsOverlay(new MapEventsReceiver() { - @Override - public boolean singleTapConfirmedHelper(GeoPoint p) { - if (clickedMarker != null) { - clickedMarker.closeInfoWindow(); - } else { - Timber.e("CLICKED MARKER IS NULL"); - } - if (isListBottomSheetExpanded()) { - // Back should first hide the bottom sheet if it is expanded - hideBottomSheet(); - } else if (isDetailsBottomSheetVisible()) { - hideBottomDetailsSheet(); - } - return true; - } - - @Override - public boolean longPressHelper(GeoPoint p) { - return false; - } - })); + binding.map.getOverlays().add(mapEventsOverlay); binding.map.setMultiTouchControls(true); } @@ -2510,21 +2274,14 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment @Override public void onBottomSheetItemClick(@Nullable View view, int position) { BottomSheetItem item = dataList.get(position); - boolean isBookmarked = bookmarkLocationDao.findBookmarkLocation(selectedPlace); switch (item.getImageResourceId()) { case R.drawable.ic_round_star_border_24px: - bookmarkLocationDao.updateBookmarkLocation(selectedPlace); + presenter.toggleBookmarkedStatus(selectedPlace); updateBookmarkButtonImage(selectedPlace); - isBookmarked = bookmarkLocationDao.findBookmarkLocation(selectedPlace); - updateMarker(isBookmarked, selectedPlace, locationManager.getLastLocation()); - binding.map.invalidate(); break; case R.drawable.ic_round_star_filled_24px: - bookmarkLocationDao.updateBookmarkLocation(selectedPlace); + presenter.toggleBookmarkedStatus(selectedPlace); updateBookmarkButtonImage(selectedPlace); - isBookmarked = bookmarkLocationDao.findBookmarkLocation(selectedPlace); - updateMarker(isBookmarked, selectedPlace, locationManager.getLastLocation()); - binding.map.invalidate(); break; case R.drawable.ic_directions_black_24dp: Utils.handleGeoCoordinates(this.getContext(), selectedPlace.getLocation()); diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.java b/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.java deleted file mode 100644 index 00a491e68..000000000 --- a/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.java +++ /dev/null @@ -1,368 +0,0 @@ -package fr.free.nrw.commons.nearby.presenter; - -import static fr.free.nrw.commons.location.LocationServiceManager.LocationChangeType.CUSTOM_QUERY; -import static fr.free.nrw.commons.location.LocationServiceManager.LocationChangeType.LOCATION_SIGNIFICANTLY_CHANGED; -import static fr.free.nrw.commons.location.LocationServiceManager.LocationChangeType.LOCATION_SLIGHTLY_CHANGED; -import static fr.free.nrw.commons.location.LocationServiceManager.LocationChangeType.MAP_UPDATED; -import static fr.free.nrw.commons.location.LocationServiceManager.LocationChangeType.SEARCH_CUSTOM_AREA; -import static fr.free.nrw.commons.nearby.CheckBoxTriStates.CHECKED; -import static fr.free.nrw.commons.nearby.CheckBoxTriStates.UNCHECKED; -import static fr.free.nrw.commons.nearby.CheckBoxTriStates.UNKNOWN; -import static fr.free.nrw.commons.wikidata.WikidataConstants.PLACE_OBJECT; - -import android.location.Location; -import androidx.annotation.MainThread; -import androidx.annotation.Nullable; -import fr.free.nrw.commons.BaseMarker; -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao; -import fr.free.nrw.commons.kvstore.JsonKvStore; -import fr.free.nrw.commons.location.LatLng; -import fr.free.nrw.commons.location.LocationServiceManager.LocationChangeType; -import fr.free.nrw.commons.location.LocationUpdateListener; -import fr.free.nrw.commons.nearby.CheckBoxTriStates; -import fr.free.nrw.commons.nearby.Label; -import fr.free.nrw.commons.nearby.MarkerPlaceGroup; -import fr.free.nrw.commons.nearby.NearbyController; -import fr.free.nrw.commons.nearby.Place; -import fr.free.nrw.commons.nearby.contract.NearbyParentFragmentContract; -import fr.free.nrw.commons.utils.LocationUtils; -import fr.free.nrw.commons.wikidata.WikidataEditListener; -import java.lang.reflect.Proxy; -import java.util.List; -import timber.log.Timber; - -public class NearbyParentFragmentPresenter - implements NearbyParentFragmentContract.UserActions, - WikidataEditListener.WikidataP18EditListener, - LocationUpdateListener { - - private boolean isNearbyLocked; - private LatLng currentLatLng; - - private boolean placesLoadedOnce; - - BookmarkLocationsDao bookmarkLocationDao; - - private @Nullable String customQuery; - - private static final NearbyParentFragmentContract.View DUMMY = (NearbyParentFragmentContract.View) Proxy.newProxyInstance( - NearbyParentFragmentContract.View.class.getClassLoader(), - new Class[]{NearbyParentFragmentContract.View.class}, (proxy, method, args) -> { - if (method.getName().equals("onMyEvent")) { - return null; - } else if (String.class == method.getReturnType()) { - return ""; - } else if (Integer.class == method.getReturnType()) { - return Integer.valueOf(0); - } else if (int.class == method.getReturnType()) { - return 0; - } else if (Boolean.class == method.getReturnType()) { - return Boolean.FALSE; - } else if (boolean.class == method.getReturnType()) { - return false; - } else { - return null; - } - } - ); - private NearbyParentFragmentContract.View nearbyParentFragmentView = DUMMY; - - - public NearbyParentFragmentPresenter(BookmarkLocationsDao bookmarkLocationDao) { - this.bookmarkLocationDao = bookmarkLocationDao; - } - - @Override - public void attachView(NearbyParentFragmentContract.View view) { - this.nearbyParentFragmentView = view; - } - - @Override - public void detachView() { - this.nearbyParentFragmentView = DUMMY; - } - - @Override - public void removeNearbyPreferences(JsonKvStore applicationKvStore) { - Timber.d("Remove place objects"); - applicationKvStore.remove(PLACE_OBJECT); - } - - public void initializeMapOperations() { - lockUnlockNearby(false); - updateMapAndList(LOCATION_SIGNIFICANTLY_CHANGED); - nearbyParentFragmentView.setCheckBoxAction(); - } - - /** - * Sets click listeners of FABs, and 2 bottom sheets - */ - @Override - public void setActionListeners(JsonKvStore applicationKvStore) { - nearbyParentFragmentView.setFABPlusAction(v -> { - if (applicationKvStore.getBoolean("login_skipped", false)) { - // prompt the user to login - nearbyParentFragmentView.displayLoginSkippedWarning(); - } else { - nearbyParentFragmentView.animateFABs(); - } - }); - - nearbyParentFragmentView.setFABRecenterAction(v -> { - nearbyParentFragmentView.recenterMap(currentLatLng); - }); - - } - - @Override - public boolean backButtonClicked() { - if (nearbyParentFragmentView.isAdvancedQueryFragmentVisible()) { - nearbyParentFragmentView.showHideAdvancedQueryFragment(false); - return true; - } else if (nearbyParentFragmentView.isListBottomSheetExpanded()) { - // Back should first hide the bottom sheet if it is expanded - nearbyParentFragmentView.listOptionMenuItemClicked(); - return true; - } else if (nearbyParentFragmentView.isDetailsBottomSheetVisible()) { - nearbyParentFragmentView.setBottomSheetDetailsSmaller(); - return true; - } - return false; - } - - public void markerUnselected() { - nearbyParentFragmentView.hideBottomSheet(); - } - - /** - * Nearby updates takes time, since they are network operations. During update time, we don't - * want to get any other calls from user. So locking nearby. - * - * @param isNearbyLocked true means lock, false means unlock - */ - @Override - public void lockUnlockNearby(boolean isNearbyLocked) { - this.isNearbyLocked = isNearbyLocked; - if (isNearbyLocked) { - nearbyParentFragmentView.disableFABRecenter(); - } else { - nearbyParentFragmentView.enableFABRecenter(); - } - } - - /** - * This method should be the single point to update Map and List. Triggered by location changes - * - * @param locationChangeType defines if location changed significantly or slightly - */ - @Override - public void updateMapAndList(LocationChangeType locationChangeType) { - Timber.d("Presenter updates map and list"); - if (isNearbyLocked) { - Timber.d("Nearby is locked, so updateMapAndList returns"); - return; - } - - if (!nearbyParentFragmentView.isNetworkConnectionEstablished()) { - Timber.d("Network connection is not established"); - return; - } - - LatLng lastLocation = nearbyParentFragmentView.getLastMapFocus(); - if (nearbyParentFragmentView.getMapCenter() != null) { - currentLatLng = nearbyParentFragmentView.getMapCenter(); - } else { - currentLatLng = lastLocation; - } - if (currentLatLng == null) { - Timber.d("Skipping update of nearby places as location is unavailable"); - return; - } - - /** - * Significant changed - Markers and current location will be updated together - * Slightly changed - Only current position marker will be updated - */ - if (locationChangeType.equals(CUSTOM_QUERY)) { - Timber.d("ADVANCED_QUERY_SEARCH"); - lockUnlockNearby(true); - nearbyParentFragmentView.setProgressBarVisibility(true); - LatLng updatedLocationByUser = LocationUtils.deriveUpdatedLocationFromSearchQuery( - customQuery); - if (updatedLocationByUser == null) { - updatedLocationByUser = lastLocation; - } - nearbyParentFragmentView.populatePlaces(updatedLocationByUser, customQuery); - } else if (locationChangeType.equals(LOCATION_SIGNIFICANTLY_CHANGED) - || locationChangeType.equals(MAP_UPDATED)) { - lockUnlockNearby(true); - nearbyParentFragmentView.setProgressBarVisibility(true); - nearbyParentFragmentView.populatePlaces(nearbyParentFragmentView.getMapCenter()); - } else if (locationChangeType.equals(SEARCH_CUSTOM_AREA)) { - Timber.d("SEARCH_CUSTOM_AREA"); - lockUnlockNearby(true); - nearbyParentFragmentView.setProgressBarVisibility(true); - nearbyParentFragmentView.populatePlaces(nearbyParentFragmentView.getMapFocus()); - } else { // Means location changed slightly, ie user is walking or driving. - Timber.d("Means location changed slightly"); - } - } - - /** - * Populates places for custom location, should be used for finding nearby places around a - * location where you are not at. - * - * @param nearbyPlaces This variable has placeToCenter list information and distances. - */ - public void updateMapMarkers(List nearbyPlaces, LatLng currentLatLng, - boolean shouldTrackPosition) { - if (null != nearbyParentFragmentView) { - nearbyParentFragmentView.clearAllMarkers(); - List baseMarkers = NearbyController - .loadAttractionsFromLocationToBaseMarkerOptions(currentLatLng, - // Curlatlang will be used to calculate distances - nearbyPlaces); - nearbyParentFragmentView.updateMapMarkers(baseMarkers); - lockUnlockNearby(false); // So that new location updates wont come - nearbyParentFragmentView.setProgressBarVisibility(false); - nearbyParentFragmentView.updateListFragment(nearbyPlaces); - } - } - - /** - * Some centering task may need to wait for map to be ready, if they are requested before map is - * ready. So we will remember it when the map is ready - */ - private void handleCenteringTaskIfAny() { - if (!placesLoadedOnce) { - placesLoadedOnce = true; - nearbyParentFragmentView.centerMapToPlace(null); - } - } - - @Override - public void onWikidataEditSuccessful() { - updateMapAndList(MAP_UPDATED); - } - - @Override - public void onLocationChangedSignificantly(LatLng latLng) { - Timber.d("Location significantly changed"); - updateMapAndList(LOCATION_SIGNIFICANTLY_CHANGED); - } - - @Override - public void onLocationChangedSlightly(LatLng latLng) { - Timber.d("Location significantly changed"); - updateMapAndList(LOCATION_SLIGHTLY_CHANGED); - } - - @Override - public void onLocationChangedMedium(LatLng latLng) { - Timber.d("Location changed medium"); - } - - @Override - public void filterByMarkerType(List