From 52fa2aace91e36eeea5e0eddedcfd37106646283 Mon Sep 17 00:00:00 2001 From: bartn Date: Mon, 20 Dec 2010 14:50:27 -0800 Subject: [PATCH] Finalize the new interface, fix bugs and add unit tests. --- .../content/MyTracksProviderUtilsImpl.java | 142 ++++++++++++------ .../services/TrackRecordingServiceTest.java | 6 +- 2 files changed, 99 insertions(+), 49 deletions(-) diff --git a/MyTracksLib/src/com/google/android/apps/mytracks/content/MyTracksProviderUtilsImpl.java b/MyTracksLib/src/com/google/android/apps/mytracks/content/MyTracksProviderUtilsImpl.java index c3793de09..a0a48c9af 100644 --- a/MyTracksLib/src/com/google/android/apps/mytracks/content/MyTracksProviderUtilsImpl.java +++ b/MyTracksLib/src/com/google/android/apps/mytracks/content/MyTracksProviderUtilsImpl.java @@ -40,6 +40,8 @@ import java.util.NoSuchElementException; public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils { private final ContentResolver contentResolver; + + private int defaultCursorBatchSize = 2000; public MyTracksProviderUtilsImpl(ContentResolver contentResolver) { this.contentResolver = contentResolver; @@ -185,55 +187,78 @@ public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils { fillLocation(cursor, location); return location; } + + /** + * A cache of track column indices. + */ + private static class CachedTrackColumnIndices { + public final int idxId; + public final int idxLatitude; + public final int idxLongitude; + public final int idxAltitude; + public final int idxTime; + public final int idxBearing; + public final int idxAccuracy; + public final int idxSpeed; + public final int idxSensor; - @Override - public void fillLocation(Cursor cursor, Location location) { + public CachedTrackColumnIndices(Cursor cursor) { + idxId = cursor.getColumnIndex(TrackPointsColumns._ID); + idxLatitude = cursor.getColumnIndexOrThrow(TrackPointsColumns.LATITUDE); + idxLongitude = cursor.getColumnIndexOrThrow(TrackPointsColumns.LONGITUDE); + idxAltitude = cursor.getColumnIndexOrThrow(TrackPointsColumns.ALTITUDE); + idxTime = cursor.getColumnIndexOrThrow(TrackPointsColumns.TIME); + idxBearing = cursor.getColumnIndexOrThrow(TrackPointsColumns.BEARING); + idxAccuracy = cursor.getColumnIndexOrThrow(TrackPointsColumns.ACCURACY); + idxSpeed = cursor.getColumnIndexOrThrow(TrackPointsColumns.SPEED); + idxSensor = cursor.getColumnIndexOrThrow(TrackPointsColumns.SENSOR); + } + } + + private void fillLocation(Cursor cursor, CachedTrackColumnIndices columnIndices, + Location location) { location.reset(); - - int idxLatitude = cursor.getColumnIndexOrThrow(TrackPointsColumns.LATITUDE); - int idxLongitude = - cursor.getColumnIndexOrThrow(TrackPointsColumns.LONGITUDE); - int idxAltitude = cursor.getColumnIndexOrThrow(TrackPointsColumns.ALTITUDE); - int idxTime = cursor.getColumnIndexOrThrow(TrackPointsColumns.TIME); - int idxBearing = cursor.getColumnIndexOrThrow(TrackPointsColumns.BEARING); - int idxAccuracy = cursor.getColumnIndexOrThrow(TrackPointsColumns.ACCURACY); - int idxSpeed = cursor.getColumnIndexOrThrow(TrackPointsColumns.SPEED); - int idxSensor = cursor.getColumnIndexOrThrow(TrackPointsColumns.SENSOR); - - if (!cursor.isNull(idxLatitude)) { - location.setLatitude(1. * cursor.getInt(idxLatitude) / 1E6); + + if (!cursor.isNull(columnIndices.idxLatitude)) { + location.setLatitude(1. * cursor.getInt(columnIndices.idxLatitude) / 1E6); } - if (!cursor.isNull(idxLongitude)) { - location.setLongitude(1. * cursor.getInt(idxLongitude) / 1E6); + if (!cursor.isNull(columnIndices.idxLongitude)) { + location.setLongitude(1. * cursor.getInt(columnIndices.idxLongitude) / 1E6); } - if (!cursor.isNull(idxAltitude)) { - location.setAltitude(cursor.getFloat(idxAltitude)); + if (!cursor.isNull(columnIndices.idxAltitude)) { + location.setAltitude(cursor.getFloat(columnIndices.idxAltitude)); } - if (!cursor.isNull(idxTime)) { - location.setTime(cursor.getLong(idxTime)); + if (!cursor.isNull(columnIndices.idxTime)) { + location.setTime(cursor.getLong(columnIndices.idxTime)); } - if (!cursor.isNull(idxBearing)) { - location.setBearing(cursor.getFloat(idxBearing)); + if (!cursor.isNull(columnIndices.idxBearing)) { + location.setBearing(cursor.getFloat(columnIndices.idxBearing)); } - if (!cursor.isNull(idxSpeed)) { - location.setSpeed(cursor.getFloat(idxSpeed)); + if (!cursor.isNull(columnIndices.idxSpeed)) { + location.setSpeed(cursor.getFloat(columnIndices.idxSpeed)); } - if (!cursor.isNull(idxAccuracy)) { - location.setAccuracy(cursor.getFloat(idxAccuracy)); + if (!cursor.isNull(columnIndices.idxAccuracy)) { + location.setAccuracy(cursor.getFloat(columnIndices.idxAccuracy)); } if (location instanceof MyTracksLocation && - !cursor.isNull(idxSensor)) { + !cursor.isNull(columnIndices.idxSensor)) { MyTracksLocation mtLocation = (MyTracksLocation) location; // TODO get the right buffer. Sensor.SensorDataSet sensorData; try { - sensorData = Sensor.SensorDataSet.parseFrom(cursor.getBlob(idxSensor)); + sensorData = Sensor.SensorDataSet.parseFrom(cursor.getBlob(columnIndices.idxSensor)); mtLocation.setSensorData(sensorData); } catch (InvalidProtocolBufferException e) { Log.w(TAG, "Failed to parse sensor data.", e); } } } + + @Override + public void fillLocation(Cursor cursor, Location location) { + CachedTrackColumnIndices columnIndicies = new CachedTrackColumnIndices(cursor); + fillLocation(cursor, columnIndicies, location); + } @Override public Track createTrack(Cursor cursor) { @@ -776,13 +801,12 @@ public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils { public Cursor getLocationsCursor(long trackId, long minTrackPointId, int maxLocations, boolean descending) { String selection; - if (minTrackPointId > 0) { - selection = String.format("%s=%d AND %s>=%d", - TrackPointsColumns.TRACKID, trackId, - TrackPointsColumns._ID, minTrackPointId); + if (minTrackPointId >= 0) { + selection = String.format("%s=%d AND %s%s%d", + TrackPointsColumns.TRACKID, trackId, TrackPointsColumns._ID, + descending ? "<=" : ">=", minTrackPointId); } else { - selection = String.format("%s=%d", - TrackPointsColumns.TRACKID, trackId); + selection = String.format("%s=%d", TrackPointsColumns.TRACKID, trackId); } String sortOrder = "_id " + (descending ? "DESC" : "ASC"); @@ -790,8 +814,7 @@ public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils { sortOrder += " LIMIT " + maxLocations; } - return contentResolver.query( - TrackPointsColumns.CONTENT_URI, null, selection, null, sortOrder); + return contentResolver.query(TrackPointsColumns.CONTENT_URI, null, selection, null, sortOrder); } @Override @@ -939,20 +962,25 @@ public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils { @Override public LocationIterator getLocationIterator(final long trackId, final long startTrackPointId, final boolean descending, final LocationFactory locationFactory) { + if (locationFactory == null) { + throw new IllegalArgumentException("Expecting non-null locationFactory"); + } return new LocationIterator() { private long lastTrackPointId = startTrackPointId; private Cursor cursor = getCursor(startTrackPointId); - private final int idColumnIdx = cursor != null ? - cursor.getColumnIndexOrThrow(TrackPointsColumns._ID) : -1; + private final CachedTrackColumnIndices columnIndices = cursor != null ? + new CachedTrackColumnIndices(cursor) : null; private Cursor getCursor(long trackPointId) { - return getLocationsCursor(trackId, trackPointId, 2000, descending); + return getLocationsCursor(trackId, trackPointId, defaultCursorBatchSize, descending); } private boolean advanceCursorToNextBatch() { + long pointId = lastTrackPointId + (descending ? -1 : 1); + Log.d(TAG, "Advancing cursor point ID: " + pointId); cursor.close(); - cursor = getCursor(lastTrackPointId + (descending ? -1 : 1)); - return cursor != null && cursor.moveToNext(); + cursor = getCursor(pointId); + return cursor != null; } @Override @@ -962,18 +990,33 @@ public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils { @Override public boolean hasNext() { - return cursor != null && !cursor.isAfterLast(); + if (cursor == null) { + return false; + } + if (cursor.isAfterLast()) { + return false; + } + if (cursor.isLast()) { + // If the current batch size was less that max, we can safely return, otherwise + // we need to advance to the next batch. + return cursor.getCount() == defaultCursorBatchSize && + advanceCursorToNextBatch() && !cursor.isAfterLast(); + } + + return true; } @Override public Location next() { - if (!cursor.moveToNext() || !advanceCursorToNextBatch()) { + if (cursor == null || + !(cursor.moveToNext() || advanceCursorToNextBatch() || cursor.moveToNext())) { throw new NoSuchElementException(); } - lastTrackPointId = cursor.getLong(idColumnIdx); + + lastTrackPointId = cursor.getLong(columnIndices.idxId); Location location = locationFactory.createLocation(); - // TODO: Cache column indices. - fillLocation(cursor, location); + fillLocation(cursor, columnIndices, location); + return location; } @@ -991,4 +1034,9 @@ public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils { } }; } + + // @VisibleForTesting + void setDefaultCursorBatchSize(int defaultCursorBatchSize) { + this.defaultCursorBatchSize = defaultCursorBatchSize; + } } diff --git a/MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java b/MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java index f54f6d6c1..b3ce5637a 100644 --- a/MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java +++ b/MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java @@ -77,9 +77,11 @@ public class TrackRecordingServiceTest } /** - * A context wrapper with the user provided {@link ContentResolver}. + * A context wrapper with the user provided {@link ContentResolver}. + * + * TODO: Move to test utils package. */ - private static class MockContext extends ContextWrapper { + public static class MockContext extends ContextWrapper { private final ContentResolver contentResolver; public MockContext(ContentResolver contentResolver, Context base) {