Getting rid of TrackBuffer.

MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProviderUtils.java

@@ -175,7 +175,7 @@ public interface MyTracksProviderUtils {
    * @return A cursor over the selected range of locations
    */
   Cursor getWaypointsCursor(long trackId, long minWaypointId,
-      long maxWaypoints);
+      int maxWaypoints);
 
   /**
    * Finds a track by given unique track id.
@@ -206,32 +206,6 @@ public interface MyTracksProviderUtils {
    */
   long getTrackPoints(Track track, int maxPoints);
 
-  /**
-   * Fetches some number of locations for the given track.
-   *
-   * This is designed to be used to stream through large tracks without loading
-   * all points into memory.
-   *
-   * @param track to load locations for
-   * @param buffer an array of locations to fill
-   * @deprecated use {@link #fillTrackPoints} instead
-   */
-  @Deprecated
-  void getTrackPoints(Track track, TrackBuffer buffer);
-
-  /**
-   * Fetches some number of locations for the given track.
-   *
-   * This is designed to be used to stream through large tracks without loading
-   * all points into memory.
-   * This method will reuse the Location objects in the buffer.  If you need a
-   * Location object copy the object.
-   *
-   * @param track to load locations for
-   * @param buffer an array of locations to fill
-   */
-  void fillTrackPoints(Track track, TrackBuffer buffer);
-
   /**
    * Creates a cursor over the tracks provider with a given selection.
    * Caller gets to own the returned cursor. Don't forget to close it.
@@ -319,7 +293,7 @@ public interface MyTracksProviderUtils {
    * @return a new location object
    */
   Location createLocation(Cursor cursor);
-  
+
   /**
    * Fill a location object with values from a given cursor.
    * 
@@ -327,7 +301,7 @@ public interface MyTracksProviderUtils {
    * @param location a location object to be overwritten
    */
   void fillLocation(Cursor cursor, Location location);
-  
+
   /**
    * Creates a waypoint object from a given cursor.
    *

MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProviderUtilsImpl.java

@@ -775,7 +775,7 @@ public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils {
 
   @Override
   public Cursor getWaypointsCursor(long trackId, long minWaypointId,
-      long maxWaypoints) {
+      int maxWaypoints) {
     String selection;
     if (minWaypointId > 0) {
       selection = String.format("%s=%d AND %s>=%d",
@@ -845,67 +845,6 @@ public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils {
     return lastId;
   }
 
-  @Override
-  public void getTrackPoints(Track track, TrackBuffer buffer) {
-    getTrackPoints(track, buffer, false);
-  }
-  
-  @Override
-  public void fillTrackPoints(Track track, TrackBuffer buffer) {
-    getTrackPoints(track, buffer, true);
-  }
-
-  public void getTrackPoints(Track track, TrackBuffer buffer,
-      boolean reuseLocations) {
-    long startingPoint = buffer.getLastLocationRead() == 0 ? track.getStartId()
-        : buffer.getLastLocationRead();
-    buffer.reset();
-    Cursor cursor = getLocationsCursor(track.getId(),
-                                       startingPoint,
-                                       buffer.getSize(), false);
-    if (cursor == null) {
-      Log.w(MyTracksProvider.TAG, "Cannot get a locations cursor!");
-      buffer.setInvalid();
-      return;
-    }
-    try {
-      if (cursor.getCount() == 0) {
-        Log.w(MyTracksProvider.TAG, "No matching locations found.");
-        buffer.resetAt(startingPoint + buffer.getSize());
-        return;
-      }
-
-      if (!cursor.moveToFirst()) {
-        Log.w(MyTracksProvider.TAG, "Could not move to first.");
-        buffer.setInvalid();
-        return;
-      }
-
-      final int idColumnIdx =
-          cursor.getColumnIndexOrThrow(TrackPointsColumns._ID);
-      do {
-        if (reuseLocations) {
-          fillLocation(cursor, buffer.add(cursor.getLong(idColumnIdx)));
-        } else {
-          Location location = createLocation(cursor);
-          if (location == null) {
-            continue;
-          }
-          buffer.add(location, cursor.getLong(idColumnIdx));
-        }
-      } while (cursor.moveToNext());
-
-      if (buffer.getLocationsLoaded() == 0) {
-        Log.w(MyTracksProvider.TAG, "No locations read.");
-        buffer.resetAt(startingPoint + buffer.getSize());
-      }
-    } catch (RuntimeException e) {
-      Log.w(MyTracksProvider.TAG, "Caught unexpected exception.", e);
-    } finally {
-      cursor.close();
-    }
-  }
-
   @Override
   public Cursor getTracksCursor(String selection) {
     Cursor cursor = context.getContentResolver().query(

MyTracks/src/com/google/android/apps/mytracks/content/TrackBuffer.java

@@ -1,149 +0,0 @@
-/*
- * Copyright 2009 Google Inc.
- * 
- * Licensed under the Apache License, Version 2.0 (the "License"); you may not
- * use this file except in compliance with the License. You may obtain a copy of
- * the License at
- * 
- * http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations under
- * the License.
- */
-package com.google.android.apps.mytracks.content;
-
-import com.google.android.apps.mytracks.util.MyTracksUtils;
-
-import android.location.Location;
-
-/**
- * A buffer of Locations from a track.
- * This class contains some additional information about the loaded points.
- *
- * @author Sandor Dornbush
- */
-public class TrackBuffer {
-
-  /**
-   * An array of location objects that are part of a track.
-   */
-  private final Location[] buffer;
-
-  /**
-   * The id of the last location read from the track.
-   */
-  private long lastLocationRead = 0;
-
-  /**
-   * The number of locations loaded into the buffer.
-   */
-  private int locationsLoaded = 0;
-
-  /**
-   * Create a TrackBuffer with size elements.
-   *
-   * @param size The size of the buffer
-   */
-  public TrackBuffer(int size) {
-    buffer = new Location[size];
-  }
-
-  /**
-   * Reset the track to a state with no locations.
-   */
-  public void reset() {
-    resetAt(0);
-  }
-
-  /**
-   * Reset the track at the given starting location id.
-   */
-  public void resetAt(long lastLocation) {
-    lastLocationRead = lastLocation;
-    locationsLoaded = 0;
-  }
-
-  public void setInvalid() {
-    lastLocationRead = Integer.MAX_VALUE;
-  }
-
-  /**
-   * @return The number of locations that can be stored in this buffer
-   */
-  public int getSize() {
-    return buffer.length;
-  }
-
-  /**
-   * @param index The index of the location to fetch
-   * @return The location for the given index
-   */
-  public Location get(int index) {
-    return buffer[index];
-  }
-
-  /**
-   * Adds a location to the end of the buffer.
-   * @param location The location to add.
-   * @param id The id of the location to be added.
-   */
-  public void add(Location location, long id) {
-    buffer[locationsLoaded++] = location;
-    lastLocationRead = Math.max(lastLocationRead, id);
-  }
-  
-  /**
-   * Get a location from the end of the buffer.
-   * This may reuse Location objects already allocated to the buffer.
-   * @param locationId The of the location to be added.
-   * @return The location object associated with the give id.
-   */
-  public Location add(long locationId) {
-    lastLocationRead = Math.max(lastLocationRead, locationId);
-    if (buffer[locationsLoaded] == null) {
-      buffer[locationsLoaded] = new Location("");
-    } else {
-      buffer[locationsLoaded].reset();
-    }
-    return buffer[locationsLoaded++];
-  }
-
-  /**
-   * @return The id of the last location loaded into the buffer
-   */
-  public long getLastLocationRead() {
-    return lastLocationRead;
-  }
-
-  /**
-   * @return The number of locations loaded into the buffer.
-   */
-  public int getLocationsLoaded() {
-    return locationsLoaded;
-  }
-
-  /**
-   * Finds the start location, i.e. the one which is the first point of a
-   * segment with at least two points.
-   *
-   * @return the start location
-   */
-  public Location findStartLocation() {
-    int numValidLocations = 0;
-    for (int i = 0; i < getLocationsLoaded(); i++) {
-      Location location = buffer[i];
-      if (MyTracksUtils.isValidLocation(location)) {
-        numValidLocations++;
-        if (numValidLocations == 2) {
-          return buffer[i - 1];
-        }
-      } else {
-        numValidLocations = 0;
-      }
-    }
-    return null;
-  }
-}

MyTracks/src/com/google/android/apps/mytracks/io/TrackWriter.java

@@ -18,7 +18,6 @@ package com.google.android.apps.mytracks.io;
 import com.google.android.apps.mytracks.MyTracksConstants;
 import com.google.android.apps.mytracks.content.MyTracksProviderUtils;
 import com.google.android.apps.mytracks.content.Track;
-import com.google.android.apps.mytracks.content.TrackBuffer;
 import com.google.android.apps.mytracks.content.Waypoint;
 import com.google.android.apps.mytracks.util.FileUtils;
 import com.google.android.apps.mytracks.util.MyTracksUtils;
@@ -254,55 +253,8 @@ public class TrackWriter {
   void writeDocument() {
     Log.d(MyTracksConstants.TAG, "Started writing track.");
     writer.writeHeader();
-    TrackBuffer buffer = new TrackBuffer(1024);
-    Location last = null;
-    boolean wroteFirst = false;
-    boolean segmentOpen = false;
-    int nValidLocations = 0;
-
-    // Fetch small pieces of the track.
-    while (buffer.getLastLocationRead() < track.getStopId()) {
-      Log.d(MyTracksConstants.TAG,
-          "Reading track points starting at: " + buffer.getLastLocationRead());
-      providerUtils.fillTrackPoints(track, buffer);
-      if (!wroteFirst) {
-        Location first = buffer.findStartLocation();
-        writer.writeBeginTrack(first);
-        wroteFirst = true;
-      }
-
-      Log.d(MyTracksConstants.TAG,
-            "Reading " + buffer.getLocationsLoaded()
-            + " Ending at: " + buffer.getLastLocationRead());
-      for (int i = 0; i < buffer.getLocationsLoaded(); i++) {
-        Location location = buffer.get(i);
-        if (MyTracksUtils.isValidLocation(location)) {
-          nValidLocations++;
-          if (!segmentOpen) {
-            writer.writeOpenSegment();
-            segmentOpen = true;
-          }
-          writer.writeLocation(location);
-          if (nValidLocations >= 2) {
-            last = location;
-          }
-        } else {
-          nValidLocations = 0;
-          last = null;
-          if (segmentOpen) {
-            writer.writeCloseSegment();
-            segmentOpen = false;
-          }
-        }
-      }
-    }
-    if (segmentOpen) {
-      writer.writeCloseSegment();
-      segmentOpen = false;
-    }
-    if (wroteFirst) {
-      writer.writeEndTrack(last);
-    }
+    // TODO: Fix ordering (in GPX waypoints should come first)
+    writeLocations();
     writeWaypoints(track.getId());
     writer.writeFooter();
     writer.close();
@@ -310,4 +262,60 @@ public class TrackWriter {
     Log.d(MyTracksConstants.TAG, "Done writing track.");
     errorMessage = R.string.io_write_finished;
   }
+
+  private void writeLocations() {
+    boolean wroteFirst = false;
+    boolean segmentOpen = false;
+    Location lastLoc = null;
+    boolean isLastValid = false;
+    Cursor locationsCursor = providerUtils.getLocationsCursor(track.getId(), 0, -1, false);
+    
+    if (locationsCursor == null || !locationsCursor.moveToFirst()) {
+      Log.w(MyTracksConstants.TAG, "Unable to get any points to write");
+      return;
+    }
+
+    do {
+      // TODO: Use fillLocation instead
+      Location loc = providerUtils.createLocation(locationsCursor);
+
+      boolean isValid = MyTracksUtils.isValidLocation(loc);
+      boolean validSegment = isValid && isLastValid;
+      if (!wroteFirst && validSegment) {
+        // Found the first two consecutive points which are valid
+        writer.writeBeginTrack(lastLoc);
+        wroteFirst = true;
+      }
+
+      if (validSegment) {
+        if (!segmentOpen) {
+          // Start a segment for this point
+          writer.writeOpenSegment();
+          segmentOpen = true;
+
+          // Write the previous point, which we had previously skipped
+          writer.writeLocation(lastLoc);
+        }
+
+        // Write the current point
+        writer.writeLocation(loc);
+      } else {
+        if (segmentOpen) {
+          writer.writeCloseSegment();
+          segmentOpen = false;
+        }
+      }
+
+      lastLoc = loc;
+      isLastValid = isValid;
+    } while (locationsCursor.moveToNext());
+
+    if (segmentOpen) {
+      writer.writeCloseSegment();
+      segmentOpen = false;
+    }
+    if (wroteFirst) {
+      writer.writeEndTrack(lastLoc);
+    }
+  }
 }

MyTracks/src/com/google/android/apps/mytracks/io/TrackWriterFactory.java

@@ -102,7 +102,7 @@ public class TrackWriterFactory {
    * @param format the output format to write in
    * @return the new track writer
    */
-  public static TrackWriter newWriter(Context context,
+  private static TrackWriter newWriter(Context context,
       MyTracksProviderUtils providerUtils,
       Track track, TrackFileFormat format) {
     TrackFormatWriter writer = format.newFormatWriter(context);

MyTracksTest/src/com/google/android/apps/mytracks/io/TrackWriterTest.java

@@ -2,21 +2,21 @@
 
 package com.google.android.apps.mytracks.io;
 
+import static org.easymock.EasyMock.eq;
 import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.isA;
-import static org.easymock.EasyMock.same;
+import static org.easymock.EasyMock.leq;
 
 import com.google.android.apps.mytracks.MyTracksConstants;
 import com.google.android.apps.mytracks.content.MyTracksProviderUtils;
 import com.google.android.apps.mytracks.content.MyTracksProviderUtils.Factory;
 import com.google.android.apps.mytracks.content.Track;
-import com.google.android.apps.mytracks.content.TrackBuffer;
 import com.google.android.apps.mytracks.content.Waypoint;
 import com.google.android.apps.mytracks.testing.TestingProviderUtilsFactory;
 
 import android.content.Context;
-import android.database.Cursor;
+import android.database.MatrixCursor;
 import android.location.Location;
+import android.provider.BaseColumns;
 import android.test.AndroidTestCase;
 
 import java.io.ByteArrayOutputStream;
@@ -226,6 +226,8 @@ public class TrackWriterTest extends AndroidTestCase {
     expect(providerUtils.getWaypointsCursor(
         TRACK_ID, 0, MyTracksConstants.MAX_LOADED_WAYPOINTS_POINTS))
         .andStubReturn(null);
+    expect(providerUtils.getLocationsCursor(
+        eq(TRACK_ID), leq(0L), leq(0), eq(false))).andStubReturn(null);
 
     // Set expected mock behavior
     formatWriter.writeHeader();
@@ -242,61 +244,71 @@ public class TrackWriterTest extends AndroidTestCase {
   public void testWriteDocument() {
     writer = new TrackWriter(getContext(), providerUtils, track, formatWriter);
 
-    Location l1 = new Location("fake1");
-    Location l2 = new Location("fake2");
-    Location l3 = new Location("fake3");
-    Location l4 = new Location("fake4");
-    Location l5 = new Location("fake5");
-    Location l6 = new Location("fake6");
-    Waypoint p1 = new Waypoint();
-    Waypoint p2 = new Waypoint();
+    final Location[] locs = {
+        new Location("fake0"),
+        new Location("fake1"),
+        new Location("fake2"),
+        new Location("fake3"),
+        new Location("fake4"),
+        new Location("fake5"),
+    };
+    Waypoint[] wps = { new Waypoint(), new Waypoint(), new Waypoint() };
 
-    addLocations(l1, l2, l3, l4, l5, l6);
-    stubBufferFill(
-        new Location[] { l1, l2, l3, l4 },
-        new Location[] { l5, l6 });
-
-    track.setStopId(6L);
+    // Fill locations with valid values
+    fillLocations(locs);
 
     // Make location 3 invalid
-    l3.setLatitude(100);
+    locs[2].setLatitude(100);
+
+    // Set up cursors
+    // We use fake columns since the cursor is only read by the provider utils
+    final MatrixCursor locCursor =
+        new MatrixCursor(new String[] { BaseColumns._ID }, 6);
+    for (int i = 1; i <= 6; i++) {
+      locCursor.newRow().add(i);
+    }
+    expect(providerUtils.getLocationsCursor(
+        eq(TRACK_ID), leq(0L), leq(0), eq(false))).andStubReturn(locCursor);
+    expect(providerUtils.createLocation(locCursor))
+        .andStubAnswer(stubCursorToArray(locCursor, locs));
+
+    MatrixCursor wpCursor =
+        new MatrixCursor(new String[] { BaseColumns._ID }, 3);
+    wpCursor.newRow().add(1);
+    wpCursor.newRow().add(2);
+    wpCursor.newRow().add(3);
+    expect(providerUtils.getWaypointsCursor(
+        eq(TRACK_ID), leq(0L),
+        eq(MyTracksConstants.MAX_LOADED_WAYPOINTS_POINTS)))
+        .andStubReturn(wpCursor);
+    expect(providerUtils.createWaypoint(wpCursor))
+        .andStubAnswer(stubCursorToArray(wpCursor, wps));
 
     // Begin the track
     formatWriter.writeHeader();
-    formatWriter.writeBeginTrack(l1);
+    formatWriter.writeBeginTrack(locs[0]);
 
     // Write locations 1-2
     formatWriter.writeOpenSegment();
-    formatWriter.writeLocation(l1);
-    formatWriter.writeLocation(l2);
+    formatWriter.writeLocation(locs[0]);
+    formatWriter.writeLocation(locs[1]);
     formatWriter.writeCloseSegment();
 
     // Location 3 is not written - it's invalid
 
     // Write locations 4-6
     formatWriter.writeOpenSegment();
-    formatWriter.writeLocation(l4);
-    formatWriter.writeLocation(l5);
-    formatWriter.writeLocation(l6);
+    formatWriter.writeLocation(locs[3]);
+    formatWriter.writeLocation(locs[4]);
+    formatWriter.writeLocation(locs[5]);
     formatWriter.writeCloseSegment();
 
     // End the track
-    formatWriter.writeEndTrack(l6);
+    formatWriter.writeEndTrack(locs[5]);
 
-    // Expect reading/writing of the waypoints
-    Cursor cursor = mocksControl.createMock(Cursor.class);
-    expect(providerUtils.getWaypointsCursor(
-        TRACK_ID, 0, MyTracksConstants.MAX_LOADED_WAYPOINTS_POINTS))
-        .andStubReturn(cursor);
-    expect(cursor.moveToFirst()).andReturn(true);
-    expect(cursor.moveToNext()).andReturn(true);
-    expect(providerUtils.createWaypoint(cursor)).andReturn(p1);
-    formatWriter.writeWaypoint(p1);
-    expect(cursor.moveToNext()).andReturn(true);
-    expect(providerUtils.createWaypoint(cursor)).andReturn(p2);
-    formatWriter.writeWaypoint(p2);
-    expect(cursor.moveToNext()).andReturn(false).anyTimes();
-    cursor.close();
+    // Expect reading/writing of the waypoints (except the first)
+    formatWriter.writeWaypoint(wps[1]);
+    formatWriter.writeWaypoint(wps[2]);
 
     formatWriter.writeFooter();
     formatWriter.close();
@@ -308,7 +320,17 @@ public class TrackWriterTest extends AndroidTestCase {
     mocksControl.verify();
   }
 
-  private void addLocations(Location... locs) {
+  private <T> IAnswer<T> stubCursorToArray(
+      final MatrixCursor cursor, final T[] values) {
+    return new IAnswer<T>() {
+      @Override
+      public T answer() throws Throwable {
+        return values[cursor.getPosition()];
+      }
+    };
+  }
+
+  private void fillLocations(Location... locs) {
     assertTrue(locs.length < 90);
     for (int i = 0; i < locs.length; i++) {
       Location location = locs[i];
@@ -316,46 +338,4 @@ public class TrackWriterTest extends AndroidTestCase {
       location.setLongitude(i + 1);
     }
   }
-
-  /**
-   * Defines the behaviour of filling the track buffer when a read is
-   * requested.
-   * The IDs of the locations will be their sequential number.
-   *
-   * @param feeds is a list of location arrays, each element of which
-   *        will be fed into the track buffer on each call
-   */
-  private void stubBufferFill(final Location[]... feeds) {
-    providerUtils.fillTrackPoints(same(track), isA(TrackBuffer.class));
-    EasyMock.expectLastCall().andStubAnswer(new IAnswer<Void>() {
-      private int lastId = 1;
-      private int reads = 0;
-
-      @Override
-      public Void answer() throws Throwable {
-        // Get the buffer from the arguments
-        Object[] args = EasyMock.getCurrentArguments();
-        assertEquals(2, args.length);
-        TrackBuffer buffer = (TrackBuffer) args[1];
-        assertNotNull(buffer);
-
-        // Check that we still have data to feed to the buffer
-        if (reads >= feeds.length) {
-          fail("More buffer reads than expected");
-        }
-
-        // Fill the buffer
-        buffer.reset();
-        Location[] locations = feeds[reads];
-        for (int i = 0; i < locations.length; i++) {
-          buffer.add(locations[i], lastId + i);
-        }
-
-        // Update internal state
-        lastId += locations.length;
-        reads++;
-        return null;
-      }
-    });
-  }
 }