TrackPointIterator: removed manual batching as it should not be necessary.

src/androidTest/java/de/dennisguse/opentracks/content/provider/CustomContentProviderUtilsTest.java

@@ -94,29 +94,22 @@ public class CustomContentProviderUtilsTest {
 
     @Test
     public void testLocationIterator_noPoints() {
-        testIterator(new Track.Id(1), 0, 1);
+        testIterator(new Track.Id(1), 0);
     }
 
     @Test
-    public void testLocationIterator_noBatchAscending() {
+    public void testLocationIterator_noAscending() {
-        testIterator(new Track.Id(1), 50, 100);
+        testIterator(new Track.Id(1), 50);
-        testIterator(new Track.Id(2), 50, 50);
+        testIterator(new Track.Id(2), 50);
-    }
-
-    @Test
-    public void testLocationIterator_batchAscending() {
-        testIterator(new Track.Id(1), 50, 11);
-        testIterator(new Track.Id(2), 50, 25);
     }
 
     @Test
     public void testLocationIterator_largeTrack() {
-        testIterator(new Track.Id(1), 20000, 2000);
+        testIterator(new Track.Id(1), 20000);
     }
 
-    private void testIterator(Track.Id trackId, int numPoints, int batchSize) {
+    private void testIterator(Track.Id trackId, int numPoints) {
-        long lastPointId = initializeTrack(trackId, numPoints);
+        TrackPoint.Id lastPointId = initializeTrack(trackId, numPoints);
-        contentProviderUtils.setDefaultCursorBatchSize(batchSize);
         List<TrackPoint> locations = new ArrayList<>(numPoints);
         try (TrackPointIterator it = contentProviderUtils.getTrackPointLocationIterator(trackId, null)) {
             while (it.hasNext()) {
@@ -124,13 +117,13 @@ public class CustomContentProviderUtilsTest {
                 assertNotNull(trackPoint);
                 locations.add(trackPoint);
                 // Make sure the IDs are returned in the right order.
-                assertEquals(lastPointId - numPoints + locations.size(), trackPoint.getId().getId());
+                assertEquals(lastPointId.getId() - numPoints + locations.size(), trackPoint.getId().getId());
             }
             assertEquals(numPoints, locations.size());
         }
     }
 
-    private long initializeTrack(Track.Id id, int numPoints) {
+    private TrackPoint.Id initializeTrack(Track.Id id, int numPoints) {
         Track track = new Track();
         track.setId(id);
         track.setName("Test: " + id.getId());
@@ -150,17 +143,17 @@ public class CustomContentProviderUtilsTest {
         contentProviderUtils.bulkInsertTrackPoint(trackPoints, id);
 
         // Load all inserted trackPoints.
-        long lastPointId = -1;
+        TrackPoint.Id lastPointId = null;
         int counter = 0;
         try (TrackPointIterator it = contentProviderUtils.getTrackPointLocationIterator(id, null)) {
             while (it.hasNext()) {
                 TrackPoint trackPoint = it.next();
-                lastPointId = trackPoint.getId().getId();
+                lastPointId = trackPoint.getId();
                 counter++;
             }
         }
 
-        assertTrue(numPoints == 0 || lastPointId > 0);
+        assertTrue(numPoints == 0 || lastPointId.getId() > 0);
         assertEquals(numPoints, counter);
 
         return lastPointId;
@@ -770,9 +763,9 @@ public class CustomContentProviderUtilsTest {
 
         // when / then
         contentProviderUtils.bulkInsertTrackPoint(track.second, trackId);
-        assertEquals(20, contentProviderUtils.getTrackPointCursor(trackId, null, 1000).getCount());
+        assertEquals(20, contentProviderUtils.getTrackPointCursor(trackId, null).getCount());
         contentProviderUtils.bulkInsertTrackPoint(track.second.subList(0, 8), trackId);
-        assertEquals(28, contentProviderUtils.getTrackPointCursor(trackId, null, 1000).getCount());
+        assertEquals(28, contentProviderUtils.getTrackPointCursor(trackId, null).getCount());
     }
 
     /**
@@ -879,7 +872,7 @@ public class CustomContentProviderUtilsTest {
                 .map(TrackPoint.Id::new).collect(Collectors.toList());
 
         // when
-        Cursor cursor = contentProviderUtils.getTrackPointCursor(trackId, trackpointIds.get(8), 5);
+        Cursor cursor = contentProviderUtils.getTrackPointCursor(trackId, trackpointIds.get(8));
 
         // then
         assertEquals(2, cursor.getCount());

src/main/java/de/dennisguse/opentracks/content/provider/ContentProviderUtils.java

@@ -70,7 +70,6 @@ public class ContentProviderUtils {
     private static final String ID_SEPARATOR = ",";
 
     private final ContentResolver contentResolver;
-    private int defaultCursorBatchSize = 2000;
 
     public ContentProviderUtils(Context context) {
         contentResolver = context.getContentResolver();
@@ -176,7 +175,7 @@ public class ContentProviderUtils {
 
         // Delete track last since it triggers a database vacuum call
         String whereClause = String.format(TracksColumns._ID + " IN (%s)", TextUtils.join(",", Collections.nCopies(trackIds.size(), "?")));
-        contentResolver.delete(TracksColumns.CONTENT_URI, whereClause, trackIds.stream().map(id->Long.toString(id.getId())).toArray(String[]::new));
+        contentResolver.delete(TracksColumns.CONTENT_URI, whereClause, trackIds.stream().map(id -> Long.toString(id.getId())).toArray(String[]::new));
     }
 
     public void deleteTrack(Context context, @NonNull Track.Id trackId) {
@@ -632,9 +631,9 @@ public class ContentProviderUtils {
      *
      * @param trackId           the track id
      * @param startTrackPointId the starting trackPoint id. `null` to ignore
-     * @param maxLocations      maximum number of locations to return. `null` for no limit
      */
-    public Cursor getTrackPointCursor(Track.Id trackId, TrackPoint.Id startTrackPointId, Integer maxLocations) {
+    @NonNull
+    public Cursor getTrackPointCursor(@NonNull Track.Id trackId, TrackPoint.Id startTrackPointId) {
         String selection;
         String[] selectionArgs;
         if (startTrackPointId != null) {
@@ -645,11 +644,7 @@ public class ContentProviderUtils {
             selectionArgs = new String[]{Long.toString(trackId.getId())};
         }
 
-        String sortOrder = TrackPointsColumns.DEFAULT_SORT_ORDER;
+        return getTrackPointCursor(null, selection, selectionArgs, TrackPointsColumns.DEFAULT_SORT_ORDER);
-        if (maxLocations != null) {
-            sortOrder += " LIMIT " + maxLocations;
-        }
-        return getTrackPointCursor(null, selection, selectionArgs, sortOrder);
     }
 
     /**
@@ -763,39 +758,23 @@ public class ContentProviderUtils {
         return contentResolver.query(TrackPointsColumns.CONTENT_URI_BY_ID, projection, selection, selectionArgs, sortOrder);
     }
 
+    @Deprecated //Use TrackPointIterator instead
     @VisibleForTesting
     public List<TrackPoint> getTrackPoints(Track.Id trackId) {
-        List<TrackPoint> trackPoints = null;
+        List<TrackPoint> trackPoints;
 
-        try (Cursor trackPointCursor = getTrackPointCursor(trackId, null, null)) {
+        try (Cursor trackPointCursor = getTrackPointCursor(trackId, null)) {
-            if (trackPointCursor != null) {
+            trackPointCursor.moveToFirst();
-                trackPointCursor.moveToFirst();
+            trackPoints = new ArrayList<>(trackPointCursor.getCount());
-                trackPoints = new ArrayList<>(trackPointCursor.getCount());
+            for (int i = 0; i < trackPointCursor.getCount(); i++) {
-                for (int i = 0; i < trackPointCursor.getCount(); i++) {
+                trackPoints.add(createTrackPoint(trackPointCursor));
-                    trackPoints.add(createTrackPoint(trackPointCursor));
+                trackPointCursor.moveToNext();
-                    trackPointCursor.moveToNext();
-                }
             }
         }
 
         return trackPoints;
     }
 
-    int getDefaultCursorBatchSize() {
-        return defaultCursorBatchSize;
-    }
-
-    /**
-     * Sets the default cursor batch size. For testing purpose.
-     *
-     * @param defaultCursorBatchSize the default cursor batch size
-     */
-    @VisibleForTesting
-    void setDefaultCursorBatchSize(int defaultCursorBatchSize) {
-        this.defaultCursorBatchSize = defaultCursorBatchSize;
-    }
-
-
     public static String formatIdListForUri(Track.Id... trackIds) {
         long[] ids = new long[trackIds.length];
         for (int i = 0; i < trackIds.length; i++) {

src/main/java/de/dennisguse/opentracks/content/provider/TrackPointIterator.java

@@ -1,7 +1,8 @@
 package de.dennisguse.opentracks.content.provider;
 
 import android.database.Cursor;
-import android.util.Log;
+
+import androidx.annotation.NonNull;
 
 import java.util.Iterator;
 import java.util.NoSuchElementException;
@@ -10,9 +11,8 @@ import de.dennisguse.opentracks.content.data.Track;
 import de.dennisguse.opentracks.content.data.TrackPoint;
 
 /**
- * A lightweight wrapper around the original {@link Cursor} with a method to clean up.
+ * A lightweight wrapper around the original {@link Cursor}.
  */
-//TODO Remove batching; that should be handled by the database/contentprovider (i.e., already in place as we use a cursor)!
 public class TrackPointIterator implements Iterator<TrackPoint>, AutoCloseable {
 
     private static final String TAG = TrackPointIterator.class.getSimpleName();
@@ -20,7 +20,6 @@ public class TrackPointIterator implements Iterator<TrackPoint>, AutoCloseable {
     private final ContentProviderUtils contentProviderUtils;
     private final Track.Id trackId;
     private final CachedTrackPointsIndexes indexes;
-    private TrackPoint.Id lastTrackPointId = null;
     private Cursor cursor;
 
     public TrackPointIterator(ContentProviderUtils contentProviderUtils, Track.Id trackId, TrackPoint.Id startTrackPointId) {
@@ -28,28 +27,11 @@ public class TrackPointIterator implements Iterator<TrackPoint>, AutoCloseable {
         this.trackId = trackId;
 
         cursor = getCursor(startTrackPointId);
-        indexes = cursor != null ? new CachedTrackPointsIndexes(cursor)
+        indexes = new CachedTrackPointsIndexes(cursor);
-                : null;
     }
 
-    /**
-     * Gets the track point cursor.
-     *
-     * @param trackPointId the starting track point id
-     */
     private Cursor getCursor(TrackPoint.Id trackPointId) {
-        return contentProviderUtils.getTrackPointCursor(trackId, trackPointId, contentProviderUtils.getDefaultCursorBatchSize());
+        return contentProviderUtils.getTrackPointCursor(trackId, trackPointId);
-    }
-
-    /**
-     * Advances the cursor to the next batch. Returns true if successful.
-     */
-    private boolean advanceCursorToNextBatch() {
-        TrackPoint.Id trackPointId = lastTrackPointId == null ? null : new TrackPoint.Id(lastTrackPointId.getId() + 1);
-        Log.d(TAG, "Advancing track point id: " + trackPointId);
-        cursor.close();
-        cursor = getCursor(trackPointId);
-        return cursor != null;
     }
 
     @Override
@@ -57,29 +39,15 @@ public class TrackPointIterator implements Iterator<TrackPoint>, AutoCloseable {
         if (cursor == null) {
             return false;
         }
-        if (cursor.isAfterLast()) {
+        return !cursor.isLast() && !cursor.isAfterLast();
-            return false;
-        }
-        if (cursor.isLast()) {
-            if (cursor.getCount() != contentProviderUtils.getDefaultCursorBatchSize()) {
-                return false;
-            }
-            return advanceCursorToNextBatch() && !cursor.isAfterLast();
-        }
-        return true;
     }
 
     @Override
+    @NonNull
     public TrackPoint next() {
-        if (cursor == null) {
+        if (cursor == null || !cursor.moveToNext()) {
             throw new NoSuchElementException();
         }
-        if (!cursor.moveToNext()) {
-            if (!advanceCursorToNextBatch() || !cursor.moveToNext()) {
-                throw new NoSuchElementException();
-            }
-        }
-        lastTrackPointId = new TrackPoint.Id(cursor.getLong(indexes.idIndex));
         return ContentProviderUtils.fillTrackPoint(cursor, indexes);
     }
 

src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTask.java

@@ -172,6 +172,7 @@ public class AnnouncementPeriodicTask implements PeriodicTask {
         Track track = contentProviderUtils.getTrack(PreferencesUtils.getRecordingTrackId(sharedPreferences, context));
         String category = track != null ? track.getCategory() : "";
 
+        //TODO Querying all TrackPoints all the time is inefficient; use TrackDataHub
         List<TrackPoint> trackPoints = contentProviderUtils.getTrackPoints(track.getId());
         boolean isMetricUnits = PreferencesUtils.isMetricUnits(sharedPreferences, context);
         boolean isReportSpeed = PreferencesUtils.isReportSpeed(sharedPreferences, context, category);