From 971460848b4c7851c15180924cc1019e3f2e2952 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Fri, 3 Jan 2020 22:14:23 +0100 Subject: [PATCH] Extracted LocationIterator from ContentProviderUtils. Also refactored LocationFactory to TrackPointFactory. --- .../CustomContentProviderUtilsTest.java | 49 ++++++++++--------- .../content/ContentProviderUtils.java | 13 ++--- .../opentracks/content/TrackDataHub.java | 4 +- ...ionFactory.java => TrackPointFactory.java} | 9 ++-- ...nIterator.java => TrackPointIterator.java} | 27 +++++----- .../io/file/exporter/FileTrackExporter.java | 24 ++++----- .../importer/AbstractFileTrackImporter.java | 6 +-- .../services/TrackRecordingService.java | 6 +-- 8 files changed, 70 insertions(+), 68 deletions(-) rename src/main/java/de/dennisguse/opentracks/content/{LocationFactory.java => TrackPointFactory.java} (57%) rename src/main/java/de/dennisguse/opentracks/content/{LocationIterator.java => TrackPointIterator.java} (75%) diff --git a/src/androidTest/java/de/dennisguse/opentracks/content/CustomContentProviderUtilsTest.java b/src/androidTest/java/de/dennisguse/opentracks/content/CustomContentProviderUtilsTest.java index 9556a1378..5110539e2 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/content/CustomContentProviderUtilsTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/content/CustomContentProviderUtilsTest.java @@ -37,6 +37,7 @@ import java.util.concurrent.atomic.AtomicInteger; import de.dennisguse.opentracks.content.data.TestDataUtil; import de.dennisguse.opentracks.content.data.Track; +import de.dennisguse.opentracks.content.data.TrackPoint; import de.dennisguse.opentracks.content.data.TrackPointsColumns; import de.dennisguse.opentracks.content.data.TracksColumns; import de.dennisguse.opentracks.content.data.Waypoint; @@ -75,16 +76,16 @@ public class CustomContentProviderUtilsTest { @Test public void testLocationIterator_noPoints() { - testIterator(1, 0, 1, false, LocationFactory.DEFAULT_LOCATION_FACTORY); + testIterator(1, 0, 1, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY); } @Test public void testLocationIterator_customFactory() { - final Location location = new Location("test_location"); + final TrackPoint location = new TrackPoint("test_location"); final AtomicInteger counter = new AtomicInteger(); - testIterator(1, 15, 4, false, new LocationFactory() { + testIterator(1, 15, 4, false, new TrackPointFactory() { @Override - public Location createLocation() { + public TrackPoint createLocation() { counter.incrementAndGet(); return location; } @@ -105,45 +106,45 @@ public class CustomContentProviderUtilsTest { @Test public void testLocationIterator_noBatchAscending() { - testIterator(1, 50, 100, false, LocationFactory.DEFAULT_LOCATION_FACTORY); - testIterator(2, 50, 50, false, LocationFactory.DEFAULT_LOCATION_FACTORY); + testIterator(1, 50, 100, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY); + testIterator(2, 50, 50, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY); } @Test public void testLocationIterator_noBatchDescending() { - testIterator(1, 50, 100, true, LocationFactory.DEFAULT_LOCATION_FACTORY); - testIterator(2, 50, 50, true, LocationFactory.DEFAULT_LOCATION_FACTORY); + testIterator(1, 50, 100, true, TrackPointFactory.DEFAULT_LOCATION_FACTORY); + testIterator(2, 50, 50, true, TrackPointFactory.DEFAULT_LOCATION_FACTORY); } @Test public void testLocationIterator_batchAscending() { - testIterator(1, 50, 11, false, LocationFactory.DEFAULT_LOCATION_FACTORY); - testIterator(2, 50, 25, false, LocationFactory.DEFAULT_LOCATION_FACTORY); + testIterator(1, 50, 11, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY); + testIterator(2, 50, 25, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY); } @Test public void testLocationIterator_batchDescending() { - testIterator(1, 50, 11, true, LocationFactory.DEFAULT_LOCATION_FACTORY); - testIterator(2, 50, 25, true, LocationFactory.DEFAULT_LOCATION_FACTORY); + testIterator(1, 50, 11, true, TrackPointFactory.DEFAULT_LOCATION_FACTORY); + testIterator(2, 50, 25, true, TrackPointFactory.DEFAULT_LOCATION_FACTORY); } @Test public void testLocationIterator_largeTrack() { - testIterator(1, 20000, 2000, false, LocationFactory.DEFAULT_LOCATION_FACTORY); + testIterator(1, 20000, 2000, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY); } - private List testIterator(long trackId, int numPoints, int batchSize, boolean descending, LocationFactory locationFactory) { + private List testIterator(long trackId, int numPoints, int batchSize, boolean descending, TrackPointFactory trackPointFactory) { long lastPointId = initializeTrack(trackId, numPoints); ((ContentProviderUtils) contentProviderUtils).setDefaultCursorBatchSize(batchSize); List locations = new ArrayList(numPoints); - try (LocationIterator it = contentProviderUtils.getTrackPointLocationIterator(trackId, -1L, descending, locationFactory)) { + try (TrackPointIterator it = contentProviderUtils.getTrackPointLocationIterator(trackId, -1L, descending, trackPointFactory)) { while (it.hasNext()) { Location loc = it.next(); Assert.assertNotNull(loc); locations.add(loc); // Make sure the IDs are returned in the right order. Assert.assertEquals(descending ? lastPointId - locations.size() + 1 - : lastPointId - numPoints + locations.size(), it.getLocationId()); + : lastPointId - numPoints + locations.size(), it.getTrackPointId()); } Assert.assertEquals(numPoints, locations.size()); } @@ -173,10 +174,10 @@ public class CustomContentProviderUtilsTest { // Load all inserted locations. long lastPointId = -1; int counter = 0; - try (LocationIterator it = contentProviderUtils.getTrackPointLocationIterator(id, -1L, false, LocationFactory.DEFAULT_LOCATION_FACTORY)) { + try (TrackPointIterator it = contentProviderUtils.getTrackPointLocationIterator(id, -1L, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY)) { while (it.hasNext()) { it.next(); - lastPointId = it.getLocationId(); + lastPointId = it.getTrackPointId(); counter++; } } @@ -715,7 +716,7 @@ public class CustomContentProviderUtilsTest { } /** - * Tests the method {@link ContentProviderUtils#getTrackPointLocationIterator(long, long, boolean, LocationFactory)} in descending. + * Tests the method {@link ContentProviderUtils#getTrackPointLocationIterator(long, long, boolean, TrackPointFactory)} in descending. */ @Test public void testGetTrackPointLocationIterator_desc() { @@ -731,18 +732,18 @@ public class CustomContentProviderUtilsTest { long startTrackPointId = trackpointIds[9]; - LocationIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(trackId, startTrackPointId, true, LocationFactory.DEFAULT_LOCATION_FACTORY); + TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(trackId, startTrackPointId, true, TrackPointFactory.DEFAULT_LOCATION_FACTORY); for (int i = 0; i < trackpointIds.length; i++) { Assert.assertTrue(locationIterator.hasNext()); Location location = locationIterator.next(); - Assert.assertEquals(startTrackPointId - i, locationIterator.getLocationId()); + Assert.assertEquals(startTrackPointId - i, locationIterator.getTrackPointId()); checkLocation((trackpointIds.length - 1) - i, location); } Assert.assertFalse(locationIterator.hasNext()); } /** - * Tests the method {@link ContentProviderUtils#getTrackPointLocationIterator(long, long, boolean, LocationFactory)} in ascending. + * Tests the method {@link ContentProviderUtils#getTrackPointLocationIterator(long, long, boolean, TrackPointFactory)} in ascending. */ @Test public void testGetTrackPointLocationIterator_asc() { @@ -758,11 +759,11 @@ public class CustomContentProviderUtilsTest { long startTrackPointId = trackpointIds[0]; - LocationIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(trackId, startTrackPointId, false, LocationFactory.DEFAULT_LOCATION_FACTORY); + TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(trackId, startTrackPointId, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY); for (int i = 0; i < trackpointIds.length; i++) { Assert.assertTrue(locationIterator.hasNext()); Location location = locationIterator.next(); - Assert.assertEquals(startTrackPointId + i, locationIterator.getLocationId()); + Assert.assertEquals(startTrackPointId + i, locationIterator.getTrackPointId()); checkLocation(i, location); } diff --git a/src/main/java/de/dennisguse/opentracks/content/ContentProviderUtils.java b/src/main/java/de/dennisguse/opentracks/content/ContentProviderUtils.java index 5e71b5502..10e465035 100644 --- a/src/main/java/de/dennisguse/opentracks/content/ContentProviderUtils.java +++ b/src/main/java/de/dennisguse/opentracks/content/ContentProviderUtils.java @@ -928,19 +928,16 @@ public class ContentProviderUtils { * Creates a new read-only iterator over a given track's points. * It provides a lightweight way of iterating over long tracks without failing due to the underlying cursor limitations. * Since it's a read-only iterator, {@link Iterator#remove()} always throws {@link UnsupportedOperationException}. - * Each call to {@link LocationIterator#next()} may advance to the next DB record, and if so, the iterator calls {@link LocationFactory#createLocation()} and populates it with information retrieved from the record. - * When done with iteration, {@link LocationIterator#close()} must be called. + * Each call to {@link TrackPointIterator#next()} may advance to the next DB record, and if so, the iterator calls {@link TrackPointFactory#createLocation()} and populates it with information retrieved from the record. + * When done with iteration, {@link TrackPointIterator#close()} must be called. * * @param trackId the track id * @param startTrackPointId the starting track point id. -1L to ignore * @param descending true to sort the result in descending order (latest location first) - * @param locationFactory the location factory + * @param trackPointFactory the location factory */ - public LocationIterator getTrackPointLocationIterator(final long trackId, final long startTrackPointId, final boolean descending, final LocationFactory locationFactory) { - if (locationFactory == null) { - throw new IllegalArgumentException("locationFactory is null"); - } - return new LocationIterator(this, trackId, startTrackPointId, descending, locationFactory); + public TrackPointIterator getTrackPointLocationIterator(final long trackId, final long startTrackPointId, final boolean descending, final TrackPointFactory trackPointFactory) { + return new TrackPointIterator(this, trackId, startTrackPointId, descending, trackPointFactory); } private Location findTrackPointBy(String selection, String[] selectionArgs) { diff --git a/src/main/java/de/dennisguse/opentracks/content/TrackDataHub.java b/src/main/java/de/dennisguse/opentracks/content/TrackDataHub.java index 2ffeaaeec..9d4b46965 100644 --- a/src/main/java/de/dennisguse/opentracks/content/TrackDataHub.java +++ b/src/main/java/de/dennisguse/opentracks/content/TrackDataHub.java @@ -450,11 +450,11 @@ public class TrackDataHub implements DataSourceListener { int samplingFrequency = -1; boolean includeNextPoint = false; - try (LocationIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(selectedTrackId, localLastSeenLocationId + 1, false, LocationFactory.DEFAULT_LOCATION_FACTORY)) { + try (TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(selectedTrackId, localLastSeenLocationId + 1, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY)) { while (locationIterator.hasNext()) { Location location = locationIterator.next(); - long locationId = locationIterator.getLocationId(); + long locationId = locationIterator.getTrackPointId(); // Stop if past the last wanted point if (maxPointId != -1L && locationId > maxPointId) { diff --git a/src/main/java/de/dennisguse/opentracks/content/LocationFactory.java b/src/main/java/de/dennisguse/opentracks/content/TrackPointFactory.java similarity index 57% rename from src/main/java/de/dennisguse/opentracks/content/LocationFactory.java rename to src/main/java/de/dennisguse/opentracks/content/TrackPointFactory.java index ec031c180..9b714e93c 100644 --- a/src/main/java/de/dennisguse/opentracks/content/LocationFactory.java +++ b/src/main/java/de/dennisguse/opentracks/content/TrackPointFactory.java @@ -1,6 +1,5 @@ package de.dennisguse.opentracks.content; -import android.location.Location; import android.location.LocationManager; import de.dennisguse.opentracks.content.data.TrackPoint; @@ -9,14 +8,14 @@ import de.dennisguse.opentracks.content.data.TrackPoint; * Creates a new {@link TrackPoint}. * An implementation can create new instances or reuse existing instances for optimization. */ -public class LocationFactory { +public class TrackPointFactory { /** - * The default {@link LocationFactory} which creates a location each time. + * The default {@link TrackPointFactory} which creates a location each time. */ - public static LocationFactory DEFAULT_LOCATION_FACTORY = new LocationFactory(); + public static TrackPointFactory DEFAULT_LOCATION_FACTORY = new TrackPointFactory(); - public Location createLocation() { + public TrackPoint createLocation() { return new TrackPoint(LocationManager.GPS_PROVIDER); } } diff --git a/src/main/java/de/dennisguse/opentracks/content/LocationIterator.java b/src/main/java/de/dennisguse/opentracks/content/TrackPointIterator.java similarity index 75% rename from src/main/java/de/dennisguse/opentracks/content/LocationIterator.java rename to src/main/java/de/dennisguse/opentracks/content/TrackPointIterator.java index ad140ce41..4a5041be4 100644 --- a/src/main/java/de/dennisguse/opentracks/content/LocationIterator.java +++ b/src/main/java/de/dennisguse/opentracks/content/TrackPointIterator.java @@ -1,33 +1,38 @@ package de.dennisguse.opentracks.content; import android.database.Cursor; -import android.location.Location; import android.util.Log; import java.util.Iterator; import java.util.NoSuchElementException; +import de.dennisguse.opentracks.content.data.TrackPoint; + /** * A lightweight wrapper around the original {@link Cursor} with a method to clean up. */ -public class LocationIterator implements Iterator, AutoCloseable { +public class TrackPointIterator implements Iterator, AutoCloseable { - private static final String TAG = LocationIterator.class.getSimpleName(); + private static final String TAG = TrackPointIterator.class.getSimpleName(); private final ContentProviderUtils contentProviderUtils; private final long trackId; private final boolean descending; - private final LocationFactory locationFactory; + private final TrackPointFactory trackPointFactory; private final ContentProviderUtils.CachedTrackPointsIndexes indexes; private long lastTrackPointId = -1L; private Cursor cursor; - public LocationIterator(ContentProviderUtils contentProviderUtils, long trackId, long startTrackPointId, boolean descending, LocationFactory locationFactory) { + public TrackPointIterator(ContentProviderUtils contentProviderUtils, long trackId, long startTrackPointId, boolean descending, TrackPointFactory trackPointFactory) { + if (trackPointFactory == null) { + throw new IllegalArgumentException("trackPointFactory is null"); + } + this.contentProviderUtils = contentProviderUtils; this.trackId = trackId; this.descending = descending; - this.locationFactory = locationFactory; + this.trackPointFactory = trackPointFactory; cursor = getCursor(startTrackPointId); indexes = cursor != null ? new ContentProviderUtils.CachedTrackPointsIndexes(cursor) @@ -54,7 +59,7 @@ public class LocationIterator implements Iterator, AutoCloseable { return cursor != null; } - public long getLocationId() { + public long getTrackPointId() { return lastTrackPointId; } @@ -76,7 +81,7 @@ public class LocationIterator implements Iterator, AutoCloseable { } @Override - public Location next() { + public TrackPoint next() { if (cursor == null) { throw new NoSuchElementException(); } @@ -86,9 +91,9 @@ public class LocationIterator implements Iterator, AutoCloseable { } } lastTrackPointId = cursor.getLong(indexes.idIndex); - Location location = locationFactory.createLocation(); - ContentProviderUtils.fillTrackPoint(cursor, indexes, location); - return location; + TrackPoint trackPoint = trackPointFactory.createLocation(); + ContentProviderUtils.fillTrackPoint(cursor, indexes, trackPoint); + return trackPoint; } @Override diff --git a/src/main/java/de/dennisguse/opentracks/io/file/exporter/FileTrackExporter.java b/src/main/java/de/dennisguse/opentracks/io/file/exporter/FileTrackExporter.java index d9cb69c30..e842a1f45 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/exporter/FileTrackExporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/exporter/FileTrackExporter.java @@ -26,8 +26,8 @@ import androidx.annotation.NonNull; import java.io.OutputStream; import de.dennisguse.opentracks.content.ContentProviderUtils; -import de.dennisguse.opentracks.content.LocationFactory; -import de.dennisguse.opentracks.content.LocationIterator; +import de.dennisguse.opentracks.content.TrackPointFactory; +import de.dennisguse.opentracks.content.TrackPointIterator; import de.dennisguse.opentracks.content.data.Track; import de.dennisguse.opentracks.content.data.TrackPoint; import de.dennisguse.opentracks.content.data.Waypoint; @@ -126,10 +126,10 @@ public class FileTrackExporter implements TrackExporter { boolean wroteTrack = false; boolean wroteSegment = false; boolean isLastLocationValid = false; - TrackWriterLocationFactory locationFactory = new TrackWriterLocationFactory(); + TrackWriterTrackPointFactory trackPointFactory = new TrackWriterTrackPointFactory(); int locationNumber = 0; - try (LocationIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false, locationFactory)) { + try (TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false, trackPointFactory)) { while (locationIterator.hasNext()) { if (Thread.interrupted()) { @@ -144,7 +144,7 @@ public class FileTrackExporter implements TrackExporter { boolean isSegmentValid = isLocationValid && isLastLocationValid; if (!wroteTrack && isSegmentValid) { // Found the first two consecutive locations that are valid - trackWriter.writeBeginTrack(track, locationFactory.lastLocation); + trackWriter.writeBeginTrack(track, trackPointFactory.lastLocation); wroteTrack = true; } @@ -155,7 +155,7 @@ public class FileTrackExporter implements TrackExporter { wroteSegment = true; // Write the previous location, which we had previously skipped - trackWriter.writeLocation(locationFactory.lastLocation); + trackWriter.writeLocation(trackPointFactory.lastLocation); } // Write the current location @@ -169,7 +169,7 @@ public class FileTrackExporter implements TrackExporter { wroteSegment = false; } } - locationFactory.swapLocations(); + trackPointFactory.swapLocations(); isLastLocationValid = isLocationValid; } @@ -207,12 +207,12 @@ public class FileTrackExporter implements TrackExporter { * * @author Jimmy Shih */ - private class TrackWriterLocationFactory extends LocationFactory { - Location currentLocation; - Location lastLocation; + private class TrackWriterTrackPointFactory extends TrackPointFactory { + TrackPoint currentLocation; + TrackPoint lastLocation; @Override - public Location createLocation() { + public TrackPoint createLocation() { if (currentLocation == null) { currentLocation = new TrackPoint(""); } @@ -220,7 +220,7 @@ public class FileTrackExporter implements TrackExporter { } void swapLocations() { - Location tempLocation = lastLocation; + TrackPoint tempLocation = lastLocation; lastLocation = currentLocation; currentLocation = tempLocation; if (currentLocation != null) { diff --git a/src/main/java/de/dennisguse/opentracks/io/file/importer/AbstractFileTrackImporter.java b/src/main/java/de/dennisguse/opentracks/io/file/importer/AbstractFileTrackImporter.java index e552f1608..2dd8d3ccb 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/importer/AbstractFileTrackImporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/importer/AbstractFileTrackImporter.java @@ -39,8 +39,8 @@ import javax.xml.parsers.SAXParserFactory; import de.dennisguse.opentracks.R; import de.dennisguse.opentracks.content.ContentProviderUtils; -import de.dennisguse.opentracks.content.LocationFactory; -import de.dennisguse.opentracks.content.LocationIterator; +import de.dennisguse.opentracks.content.TrackPointFactory; +import de.dennisguse.opentracks.content.TrackPointIterator; import de.dennisguse.opentracks.content.data.Track; import de.dennisguse.opentracks.content.data.Waypoint; import de.dennisguse.opentracks.services.TrackRecordingService; @@ -162,7 +162,7 @@ abstract class AbstractFileTrackImporter extends DefaultHandler implements Track TripStatisticsUpdater trackTripStatisticstrackUpdater = new TripStatisticsUpdater(track.getTripStatistics().getStartTime()); TripStatisticsUpdater markerTripStatisticsUpdater = new TripStatisticsUpdater(track.getTripStatistics().getStartTime()); - try (LocationIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false, LocationFactory.DEFAULT_LOCATION_FACTORY)) { + try (TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY)) { while (true) { if (waypoint == null) { diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java index 4a4f93522..d6d92ab83 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java @@ -44,8 +44,8 @@ import de.dennisguse.opentracks.TrackDetailActivity; import de.dennisguse.opentracks.TrackListActivity; import de.dennisguse.opentracks.content.ContentProviderUtils; import de.dennisguse.opentracks.content.CustomContentProvider; -import de.dennisguse.opentracks.content.LocationFactory; -import de.dennisguse.opentracks.content.LocationIterator; +import de.dennisguse.opentracks.content.TrackPointFactory; +import de.dennisguse.opentracks.content.TrackPointIterator; import de.dennisguse.opentracks.content.data.Track; import de.dennisguse.opentracks.content.data.TrackPoint; import de.dennisguse.opentracks.content.data.Waypoint; @@ -373,7 +373,7 @@ public class TrackRecordingService extends Service { TripStatistics tripStatistics = track.getTripStatistics(); trackTripStatisticsUpdater = new TripStatisticsUpdater(tripStatistics.getStartTime()); - try (LocationIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false, LocationFactory.DEFAULT_LOCATION_FACTORY)) { + try (TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY)) { while (locationIterator.hasNext()) { Location location = locationIterator.next();