Nearby: show cached pins even when internet is unavailable (fixes #6051) (#6081)

* Place: Made location @Embedded

* Nearby: Move handling map scroll to presenter

* PlacesRepository: Add methods for fetching places in geo bounds

* Nearby: add getScreenTopRight/BottomLeft and refactor old code

* PlacesRepository: Add methods for fetching places in map bounds

* Nearby: Complete offline pins implementation

* Nearby offline pins: Add snackbar message

* Nearby offline pins: Add javadoc

---------

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

app/src/main/java/fr/free/nrw/commons/data/DBOpenHelper.kt

@@ -18,7 +18,7 @@ class DBOpenHelper(
 
     companion object {
         private const val DATABASE_NAME = "commons.db"
-        private const val DATABASE_VERSION = 20
+        private const val DATABASE_VERSION = 21
         const val CONTRIBUTIONS_TABLE = "contributions"
         private const val DROP_TABLE_STATEMENT = "DROP TABLE IF EXISTS %s"
     }

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

@@ -5,6 +5,7 @@ import android.os.Parcel;
 import android.os.Parcelable;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.room.Embedded;
 import androidx.room.Entity;
 import androidx.room.PrimaryKey;
 import fr.free.nrw.commons.location.LatLng;
@@ -24,6 +25,7 @@ public class Place implements Parcelable {
     public String name;
     private Label label;
     private String longDescription;
+    @Embedded
     public LatLng location;
     @PrimaryKey @NonNull
     public String entityID;

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

@@ -5,6 +5,7 @@ import androidx.room.Insert;
 import androidx.room.OnConflictStrategy;
 import androidx.room.Query;
 import io.reactivex.Completable;
+import java.util.List;
 
 /**
  * Data Access Object (DAO) for accessing the Place entity in the database.
@@ -32,6 +33,20 @@ public abstract class PlaceDao {
     @Query("SELECT * from place WHERE entityID=:entity")
     public abstract Place getPlace(String entity);
 
+    /**
+     * Retrieves a list of places within the specified rectangular area.
+     *
+     * @param latBegin Latitudinal lower bound
+     * @param lngBegin Longitudinal lower bound
+     * @param latEnd Latitudinal upper bound, should be greater than `latBegin`
+     * @param lngEnd Longitudinal upper bound, should be greater than `lngBegin`
+     * @return The list of places within the specified rectangular geographical area.
+     */
+    @Query("SELECT * from place WHERE name!='' AND latitude>=:latBegin AND longitude>=:lngBegin "
+        + "AND latitude<:latEnd AND longitude<:lngEnd")
+    public abstract List<Place> fetchPlaces(double latBegin, double lngBegin,
+        double latEnd, double lngEnd);
+
     /**
      * Saves a Place object asynchronously into the database.
      */

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

@@ -1,7 +1,11 @@
 package fr.free.nrw.commons.nearby;
 
+import fr.free.nrw.commons.location.LatLng;
 import io.reactivex.Completable;
+import java.util.ArrayList;
+import java.util.List;
 import javax.inject.Inject;
+import timber.log.Timber;
 
 /**
  * The LocalDataSource class for Places
@@ -26,6 +30,81 @@ public class PlacesLocalDataSource {
         return placeDao.getPlace(entityID);
     }
 
+    /**
+     * Retrieves a list of places from the database within the geographical area
+     * specified by map's opposite corners.
+     *
+     * @param mapBottomLeft Bottom left corner of the map.
+     * @param mapTopRight Top right corner of the map.
+     * @return The list of saved places within the map's view.
+     */
+    public List<Place> fetchPlaces(final LatLng mapBottomLeft, final LatLng mapTopRight) {
+        class Constraint {
+
+            final double latBegin;
+            final double lngBegin;
+            final double latEnd;
+            final double lngEnd;
+
+            public Constraint(final double latBegin, final double lngBegin, final double latEnd,
+                final double lngEnd) {
+                this.latBegin = latBegin;
+                this.lngBegin = lngBegin;
+                this.latEnd = latEnd;
+                this.lngEnd = lngEnd;
+            }
+        }
+
+        final List<Constraint> constraints = new ArrayList<>();
+
+        if (mapTopRight.getLatitude() < mapBottomLeft.getLatitude()) {
+            if (mapTopRight.getLongitude() < mapBottomLeft.getLongitude()) {
+                constraints.add(
+                    new Constraint(mapBottomLeft.getLatitude(), mapBottomLeft.getLongitude(), 90.0,
+                        180.0));
+                constraints.add(new Constraint(mapBottomLeft.getLatitude(), -180.0, 90.0,
+                    mapTopRight.getLongitude()));
+                constraints.add(
+                    new Constraint(-90.0, mapBottomLeft.getLongitude(), mapTopRight.getLatitude(),
+                        180.0));
+                constraints.add(new Constraint(-90.0, -180.0, mapTopRight.getLatitude(),
+                    mapTopRight.getLongitude()));
+            } else {
+                constraints.add(
+                    new Constraint(mapBottomLeft.getLatitude(), mapBottomLeft.getLongitude(), 90.0,
+                        mapTopRight.getLongitude()));
+                constraints.add(
+                    new Constraint(-90.0, mapBottomLeft.getLongitude(), mapTopRight.getLatitude(),
+                        mapTopRight.getLongitude()));
+            }
+        } else {
+            if (mapTopRight.getLongitude() < mapBottomLeft.getLongitude()) {
+                constraints.add(
+                    new Constraint(mapBottomLeft.getLatitude(), mapBottomLeft.getLongitude(),
+                        mapTopRight.getLatitude(), 180.0));
+                constraints.add(
+                    new Constraint(mapBottomLeft.getLatitude(), -180.0, mapTopRight.getLatitude(),
+                        mapTopRight.getLongitude()));
+            } else {
+                constraints.add(
+                    new Constraint(mapBottomLeft.getLatitude(), mapBottomLeft.getLongitude(),
+                        mapTopRight.getLatitude(), mapTopRight.getLongitude()));
+            }
+        }
+
+        final List<Place> cachedPlaces = new ArrayList<>();
+        for (final Constraint constraint : constraints) {
+            cachedPlaces.addAll(placeDao.fetchPlaces(
+                constraint.latBegin,
+                constraint.lngBegin,
+                constraint.latEnd,
+                constraint.lngEnd
+            ));
+        }
+
+        return cachedPlaces;
+    }
+
     /**
      * Saves a Place object asynchronously into the database.
      *

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

@@ -4,6 +4,7 @@ import fr.free.nrw.commons.contributions.Contribution;
 import fr.free.nrw.commons.location.LatLng;
 import io.reactivex.Completable;
 import io.reactivex.schedulers.Schedulers;
+import java.util.List;
 import javax.inject.Inject;
 
 /**
@@ -39,6 +40,17 @@ public class PlacesRepository {
         return localDataSource.fetchPlace(entityID);
     }
 
+    /**
+     * Retrieves a list of places within the geographical area specified by map's opposite corners.
+     *
+     * @param mapBottomLeft Bottom left corner of the map.
+     * @param mapTopRight Top right corner of the map.
+     * @return The list of saved places within the map's view.
+     */
+    public List<Place> fetchPlaces(final LatLng mapBottomLeft, final LatLng mapTopRight) {
+        return localDataSource.fetchPlaces(mapBottomLeft, mapTopRight);
+    }
+
     /**
      * Clears the Nearby cache on an IO thread.
      *

app/src/main/java/fr/free/nrw/commons/nearby/contract/NearbyParentFragmentContract.java

@@ -18,6 +18,8 @@ public interface NearbyParentFragmentContract {
 
         boolean isNetworkConnectionEstablished();
 
+        void updateSnackbar(boolean offlinePinsShown);
+
         void listOptionMenuItemClicked();
 
         void populatePlaces(LatLng currentLatLng);
@@ -91,6 +93,10 @@ public interface NearbyParentFragmentContract {
 
         LatLng getMapFocus();
 
+        LatLng getScreenTopRight();
+
+        LatLng getScreenBottomLeft();
+
         boolean isAdvancedQueryFragmentVisible();
 
         void showHideAdvancedQueryFragment(boolean shouldShow);
@@ -122,8 +128,6 @@ public interface NearbyParentFragmentContract {
         void filterByMarkerType(List<Label> selectedLabels, int state, boolean filterForPlaceState,
             boolean filterForAllNoneType);
 
-        void updateMapMarkersToController(List<BaseMarker> baseMarkers);
-
         void searchViewGainedFocus();
 
         void setCheckboxUnknown();
@@ -131,5 +135,7 @@ public interface NearbyParentFragmentContract {
         void setAdvancedQuery(String query);
 
         void toggleBookmarkedStatus(Place place);
+
+        void handleMapScrolled(LifecycleCoroutineScope scope, boolean isNetworkAvailable);
     }
 }

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

@@ -55,6 +55,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.LifecycleCoroutineScope;
 import androidx.lifecycle.LifecycleOwnerKt;
 import androidx.recyclerview.widget.DividerItemDecoration;
 import androidx.recyclerview.widget.GridLayoutManager;
@@ -64,7 +65,6 @@ import com.google.android.material.bottomsheet.BottomSheetBehavior.BottomSheetCa
 import com.google.android.material.snackbar.Snackbar;
 import com.jakewharton.rxbinding2.view.RxView;
 import com.jakewharton.rxbinding3.appcompat.RxSearchView;
-import fr.free.nrw.commons.BaseMarker;
 import fr.free.nrw.commons.CommonsApplication;
 import fr.free.nrw.commons.CommonsApplication.BaseLogoutListener;
 import fr.free.nrw.commons.MapController.NearbyPlacesInfo;
@@ -208,6 +208,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
     private boolean isNetworkErrorOccurred;
     private Snackbar snackbar;
     private View view;
+    private LifecycleCoroutineScope scope;
     private NearbyParentFragmentPresenter presenter;
     private boolean isDarkTheme;
     private boolean isFABsExpanded;
@@ -231,10 +232,8 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
     private Place nearestPlace;
     private volatile boolean stopQuery;
 
-    private boolean isSearchInProgress = false;
     private final Handler searchHandler = new Handler();
     private Runnable searchRunnable;
-    private static final long SCROLL_DELAY = 800; // Delay for debounce of onscroll, in milliseconds.
 
     private LatLng updatedLatLng;
     private boolean searchable;
@@ -341,6 +340,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
         view = binding.getRoot();
 
         initNetworkBroadCastReceiver();
+        scope = LifecycleOwnerKt.getLifecycleScope(getViewLifecycleOwner());
         presenter = new NearbyParentFragmentPresenter(bookmarkLocationDao, placesRepository, nearbyController);
         progressDialog = new ProgressDialog(getActivity());
         progressDialog.setCancelable(false);
@@ -471,28 +471,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
         binding.map.addMapListener(new MapListener() {
             @Override
             public boolean onScroll(ScrollEvent event) {
-
-                // Remove any pending search runnables
-                searchHandler.removeCallbacks(searchRunnable);
-
-                // Set a runnable to call the Search after a delay
-                searchRunnable = new Runnable() {
-                    @Override
-                    public void run() {
-                        if (!isSearchInProgress) {
-                            isSearchInProgress = true; // search executing flag
-                            // Start Search
-                            try {
-                                presenter.searchInTheArea();
-                            } finally {
-                                isSearchInProgress = false;
-                            }
-                        }
-                    }
-                };
-                // post runnable with configured SCROLL_DELAY
-                searchHandler.postDelayed(searchRunnable, SCROLL_DELAY);
-
+                presenter.handleMapScrolled(scope, !isNetworkErrorOccurred);
                 return true;
             }
 
@@ -1060,6 +1039,23 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
         };
     }
 
+    /**
+     * Updates the internet unavailable snackbar to reflect whether cached pins are shown.
+     *
+     * @param offlinePinsShown Whether there are pins currently being shown on map.
+     */
+    @Override
+    public void updateSnackbar(final boolean offlinePinsShown) {
+        if (!isNetworkErrorOccurred || snackbar == null) {
+            return;
+        }
+        if (offlinePinsShown) {
+            snackbar.setText(R.string.nearby_showing_pins_offline);
+        } else {
+            snackbar.setText(R.string.no_internet);
+        }
+    }
+
     /**
      * Hide or expand bottom sheet according to states of all sheets
      */
@@ -1074,16 +1070,38 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
         }
     }
 
+    /**
+     * Returns the location of the top right corner of the map view.
+     *
+     * @return a `LatLng` object denoting the location of the top right corner of the map.
+     */
+    @Override
+    public LatLng getScreenTopRight() {
+        final IGeoPoint screenTopRight = binding.map.getProjection()
+            .fromPixels(binding.map.getWidth(), 0);
+        return new LatLng(
+            screenTopRight.getLatitude(), screenTopRight.getLongitude(), 0);
+    }
+
+    /**
+     * Returns the location of the bottom left corner of the map view.
+     *
+     * @return a `LatLng` object denoting the location of the bottom left corner of the map.
+     */
+    @Override
+    public LatLng getScreenBottomLeft() {
+        final IGeoPoint screenBottomLeft = binding.map.getProjection()
+            .fromPixels(0, binding.map.getHeight());
+        return new LatLng(
+            screenBottomLeft.getLatitude(), screenBottomLeft.getLongitude(), 0);
+    }
+
     @Override
     public void populatePlaces(final LatLng currentLatLng) {
-            IGeoPoint screenTopRight = binding.map.getProjection()
-            .fromPixels(binding.map.getWidth(), 0);
-        IGeoPoint screenBottomLeft = binding.map.getProjection()
-            .fromPixels(0, binding.map.getHeight());
-        LatLng screenTopRightLatLng = new LatLng(
-            screenBottomLeft.getLatitude(), screenBottomLeft.getLongitude(), 0);
-        LatLng screenBottomLeftLatLng = new LatLng(
-            screenTopRight.getLatitude(), screenTopRight.getLongitude(), 0);
+        // these two variables have historically been assigned values the opposite of what their
+        // names imply, and quite some existing code depends on this fact
+        LatLng screenTopRightLatLng = getScreenBottomLeft();
+        LatLng screenBottomLeftLatLng = getScreenTopRight();
 
         // When the nearby fragment is opened immediately upon app launch, the {screenTopRightLatLng}
         // and {screenBottomLeftLatLng} variables return {LatLng(0.0,0.0)} as output.
@@ -1136,14 +1154,10 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
             populatePlaces(currentLatLng);
             return;
         }
-        IGeoPoint screenTopRight = binding.map.getProjection()
-            .fromPixels(binding.map.getWidth(), 0);
-        IGeoPoint screenBottomLeft = binding.map.getProjection()
-            .fromPixels(0, binding.map.getHeight());
-        LatLng screenTopRightLatLng = new LatLng(
-            screenBottomLeft.getLatitude(), screenBottomLeft.getLongitude(), 0);
-        LatLng screenBottomLeftLatLng = new LatLng(
-            screenTopRight.getLatitude(), screenTopRight.getLongitude(), 0);
+        // these two variables have historically been assigned values the opposite of what their
+        // names imply, and quite some existing code depends on this fact
+        final LatLng screenTopRightLatLng = getScreenBottomLeft();
+        final LatLng screenBottomLeftLatLng = getScreenTopRight();
 
         if (currentLatLng.equals(lastFocusLocation) || lastFocusLocation == null
             || recenterToUserLocation) { // Means we are checking around current location
@@ -1477,8 +1491,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
      */
     private void updateMapMarkers(final List<Place> nearbyPlaces, final LatLng curLatLng,
         final boolean shouldUpdateSelectedMarker) {
-        presenter.updateMapMarkers(nearbyPlaces, curLatLng,
-            LifecycleOwnerKt.getLifecycleScope(getViewLifecycleOwner()));
+        presenter.updateMapMarkers(nearbyPlaces, curLatLng, scope);
     }
 
 
@@ -1865,20 +1878,24 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
                 clickedMarker.closeInfoWindow();
             }
             clickedMarker = marker1;
-            binding.bottomSheetDetails.dataCircularProgress.setVisibility(View.VISIBLE);
-            binding.bottomSheetDetails.icon.setVisibility(View.GONE);
-            binding.bottomSheetDetails.wikiDataLl.setVisibility(View.GONE);
-            if (Objects.equals(place.name, "")) {
-                getPlaceData(place.getWikiDataEntityId(), place, marker1, isBookMarked);
+            if (!isNetworkErrorOccurred) {
+                binding.bottomSheetDetails.dataCircularProgress.setVisibility(View.VISIBLE);
+                binding.bottomSheetDetails.icon.setVisibility(View.GONE);
+                binding.bottomSheetDetails.wikiDataLl.setVisibility(View.GONE);
+                if (Objects.equals(place.name, "")) {
+                    getPlaceData(place.getWikiDataEntityId(), place, marker1, isBookMarked);
+                } else {
+                    marker.showInfoWindow();
+                    binding.bottomSheetDetails.dataCircularProgress.setVisibility(View.GONE);
+                    binding.bottomSheetDetails.icon.setVisibility(View.VISIBLE);
+                    binding.bottomSheetDetails.wikiDataLl.setVisibility(View.VISIBLE);
+                    passInfoToSheet(place);
+                    hideBottomSheet();
+                }
+                bottomSheetDetailsBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED);
             } else {
                 marker.showInfoWindow();
-                binding.bottomSheetDetails.dataCircularProgress.setVisibility(View.GONE);
-                binding.bottomSheetDetails.icon.setVisibility(View.VISIBLE);
-                binding.bottomSheetDetails.wikiDataLl.setVisibility(View.VISIBLE);
-                passInfoToSheet(place);
-                hideBottomSheet();
             }
-            bottomSheetDetailsBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED);
             return true;
         });
         return marker;

app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt

@@ -4,7 +4,6 @@ import android.location.Location
 import android.view.View
 import androidx.annotation.MainThread
 import androidx.lifecycle.LifecycleCoroutineScope
-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
@@ -52,6 +51,10 @@ class NearbyParentFragmentPresenter
 
     private var nearbyParentFragmentView: NearbyParentFragmentContract.View = DUMMY
 
+    private var placeSearchJob: Job? = null
+    private var isSearchInProgress = false
+    private var localPlaceSearchJob: Job? = null
+
     private val clickedPlaces = CopyOnWriteArrayList<Place>()
 
     /**
@@ -489,17 +492,52 @@ class NearbyParentFragmentPresenter
         }
     }
 
-    @MainThread
-    override fun updateMapMarkersToController(baseMarkers: MutableList<BaseMarker>) {
-        NearbyController.markerLabelList.clear()
-        for (i in baseMarkers.indices) {
-            val nearbyBaseMarker = baseMarkers[i]
-            NearbyController.markerLabelList.add(
-                MarkerPlaceGroup(
-                    bookmarkLocationDao.findBookmarkLocation(nearbyBaseMarker.place),
-                    nearbyBaseMarker.place
-                )
-            )
+    /**
+     * Handles the map scroll user action for `NearbyParentFragment`
+     *
+     * @param scope The lifecycle scope of `nearbyParentFragment`'s `viewLifecycleOwner`
+     * @param isNetworkAvailable Whether to load pins from the internet or from the cache.
+     */
+    @Override
+    override fun handleMapScrolled(scope: LifecycleCoroutineScope?, isNetworkAvailable: Boolean) {
+        scope ?: return
+
+        placeSearchJob?.cancel()
+        localPlaceSearchJob?.cancel()
+        if (isNetworkAvailable) {
+            placeSearchJob = scope.launch(Dispatchers.Main) {
+                delay(SCROLL_DELAY)
+                if (!isSearchInProgress) {
+                    isSearchInProgress = true; // search executing flag
+                    // Start Search
+                    try {
+                        searchInTheArea();
+                    } finally {
+                        isSearchInProgress = false;
+                    }
+                }
+            }
+        } else {
+            loadPlacesDataAyncJob?.cancel()
+            localPlaceSearchJob = scope.launch(Dispatchers.IO) {
+                delay(LOCAL_SCROLL_DELAY)
+                val mapFocus = nearbyParentFragmentView.mapFocus
+                val markerPlaceGroups = placesRepository.fetchPlaces(
+                    nearbyParentFragmentView.screenBottomLeft,
+                    nearbyParentFragmentView.screenTopRight
+                ).sortedBy { it.getDistanceInDouble(mapFocus) }.take(NearbyController.MAX_RESULTS)
+                    .map {
+                        MarkerPlaceGroup(
+                            bookmarkLocationDao.findBookmarkLocation(it), it
+                        )
+                    }
+                ensureActive()
+                NearbyController.currentLocation = mapFocus
+                schedulePlacesUpdate(markerPlaceGroups, force = true)
+                withContext(Dispatchers.Main) {
+                    nearbyParentFragmentView.updateSnackbar(!markerPlaceGroups.isEmpty())
+                }
+            }
         }
     }
 
@@ -575,6 +613,8 @@ class NearbyParentFragmentPresenter
     }
 
     companion object {
+        private const val SCROLL_DELAY = 800L; // Delay for debounce of onscroll, in milliseconds.
+        private const val LOCAL_SCROLL_DELAY = 200L; // SCROLL_DELAY but for local db place search
         private val DUMMY = Proxy.newProxyInstance(
             NearbyParentFragmentContract.View::class.java.getClassLoader(),
             arrayOf<Class<*>>(NearbyParentFragmentContract.View::class.java),

app/src/main/res/values/strings.xml

@@ -282,6 +282,7 @@
   <string name="copy_wikicode">Copy the wikitext to the clipboard</string>
   <string name="wikicode_copied">The wikitext was copied to the clipboard</string>
   <string name="nearby_location_not_available">Nearby might not work properly, Location not available.</string>
+  <string name="nearby_showing_pins_offline">Internet unavailable. Showing only cached places.</string>
   <string name="upload_location_access_denied">Location access denied. Please set your location manually to use this feature.</string>
   <string name="location_permission_rationale_nearby">Permission required to display a list of nearby places</string>
   <string name="location_permission_rationale_explore">Permission required to display a list of nearby images</string>