diff --git a/src/androidTest/java/de/dennisguse/opentracks/content/provider/CustomContentProviderUtilsTest.java b/src/androidTest/java/de/dennisguse/opentracks/content/provider/CustomContentProviderUtilsTest.java index 9e6a1f7c0..cdbdb597d 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/content/provider/CustomContentProviderUtilsTest.java +++ b/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() { - testIterator(new Track.Id(1), 50, 100); - testIterator(new Track.Id(2), 50, 50); - } - - @Test - public void testLocationIterator_batchAscending() { - testIterator(new Track.Id(1), 50, 11); - testIterator(new Track.Id(2), 50, 25); + public void testLocationIterator_noAscending() { + testIterator(new Track.Id(1), 50); + testIterator(new Track.Id(2), 50); } @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) { - long lastPointId = initializeTrack(trackId, numPoints); - contentProviderUtils.setDefaultCursorBatchSize(batchSize); + private void testIterator(Track.Id trackId, int numPoints) { + TrackPoint.Id lastPointId = initializeTrack(trackId, numPoints); List 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()); diff --git a/src/main/java/de/dennisguse/opentracks/content/provider/ContentProviderUtils.java b/src/main/java/de/dennisguse/opentracks/content/provider/ContentProviderUtils.java index dcd85c720..6aee55218 100644 --- a/src/main/java/de/dennisguse/opentracks/content/provider/ContentProviderUtils.java +++ b/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; - if (maxLocations != null) { - sortOrder += " LIMIT " + maxLocations; - } - return getTrackPointCursor(null, selection, selectionArgs, sortOrder); + return getTrackPointCursor(null, selection, selectionArgs, TrackPointsColumns.DEFAULT_SORT_ORDER); } /** @@ -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 getTrackPoints(Track.Id trackId) { - List trackPoints = null; + List trackPoints; - try (Cursor trackPointCursor = getTrackPointCursor(trackId, null, null)) { - if (trackPointCursor != null) { - trackPointCursor.moveToFirst(); - trackPoints = new ArrayList<>(trackPointCursor.getCount()); - for (int i = 0; i < trackPointCursor.getCount(); i++) { - trackPoints.add(createTrackPoint(trackPointCursor)); - trackPointCursor.moveToNext(); - } + try (Cursor trackPointCursor = getTrackPointCursor(trackId, null)) { + trackPointCursor.moveToFirst(); + trackPoints = new ArrayList<>(trackPointCursor.getCount()); + for (int i = 0; i < trackPointCursor.getCount(); i++) { + trackPoints.add(createTrackPoint(trackPointCursor)); + 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++) { diff --git a/src/main/java/de/dennisguse/opentracks/content/provider/TrackPointIterator.java b/src/main/java/de/dennisguse/opentracks/content/provider/TrackPointIterator.java index 515c1ab92..5584b224c 100644 --- a/src/main/java/de/dennisguse/opentracks/content/provider/TrackPointIterator.java +++ b/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, AutoCloseable { private static final String TAG = TrackPointIterator.class.getSimpleName(); @@ -20,7 +20,6 @@ public class TrackPointIterator implements Iterator, 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, AutoCloseable { this.trackId = trackId; cursor = getCursor(startTrackPointId); - indexes = cursor != null ? new CachedTrackPointsIndexes(cursor) - : null; + indexes = new CachedTrackPointsIndexes(cursor); } - /** - * 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()); - } - - /** - * 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; + return contentProviderUtils.getTrackPointCursor(trackId, trackPointId); } @Override @@ -57,29 +39,15 @@ public class TrackPointIterator implements Iterator, AutoCloseable { if (cursor == null) { return false; } - if (cursor.isAfterLast()) { - return false; - } - if (cursor.isLast()) { - if (cursor.getCount() != contentProviderUtils.getDefaultCursorBatchSize()) { - return false; - } - return advanceCursorToNextBatch() && !cursor.isAfterLast(); - } - return true; + return !cursor.isLast() && !cursor.isAfterLast(); } @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); } diff --git a/src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTask.java b/src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTask.java index c6f16dae6..f9ad816e6 100644 --- a/src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTask.java +++ b/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 trackPoints = contentProviderUtils.getTrackPoints(track.getId()); boolean isMetricUnits = PreferencesUtils.isMetricUnits(sharedPreferences, context); boolean isReportSpeed = PreferencesUtils.isReportSpeed(sharedPreferences, context, category);