Skip to content

Commit

Permalink
Merge pull request #194 from cyface-de/fix/4.2.4/VIC-78_IndexOutOfBou…
Browse files Browse the repository at this point in the history 
…ndsException

Fix/4.2.4/vic 78 index out of bounds exception
  • Loading branch information
@hb0
hb0 authored Sep 30, 2019
2 parents 9557be3 + bb569d7 commit 84f2526
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 22 deletions.
4 changes: 2 additions & 2 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*
* @author Armin Schnabel
* @author Klemens Muthmann
* @version 2.4.0
* @version 2.4.1
* @since 1.0.0
*/

Expand Down Expand Up @@ -33,7 +33,7 @@ buildscript {

ext {
groupId = 'de.cyface'
version = "4.2.3"
version = "4.2.4"

minSdkVersion = 16
targetSdkVersion = 28
Expand Down
69 changes: 54 additions & 15 deletions datacapturing/src/androidTest/java/de/cyface/datacapturing/model/CapturedDataWriterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
import static de.cyface.persistence.Utils.getIdentifierUri;
import static de.cyface.persistence.Utils.getMeasurementUri;
import static de.cyface.persistence.model.MeasurementStatus.FINISHED;
import static de.cyface.persistence.model.Vehicle.UNKNOWN;
import static de.cyface.testutils.SharedTestUtils.clearPersistenceLayer;
import static de.cyface.testutils.SharedTestUtils.deserialize;
import static org.hamcrest.core.Is.is;
Expand All @@ -91,7 +92,7 @@
*
* @author Klemens Muthmann
* @author Armin Schnabel
* @version 5.5.3
* @version 5.6.0
* @since 1.0.0
*/
@RunWith(AndroidJUnit4.class)
Expand Down Expand Up @@ -160,7 +161,7 @@ public void tearDown() {
public void testCreateNewMeasurement() throws NoSuchMeasurementException, CursorIsNullException {

// Create a measurement
Measurement measurement = oocut.newMeasurement(Vehicle.UNKNOWN);
Measurement measurement = oocut.newMeasurement(UNKNOWN);
assertThat(measurement.getIdentifier() > 0L, is(equalTo(true)));

// Try to load the created measurement and check its properties
Expand All @@ -179,7 +180,7 @@ public void testCreateNewMeasurement() throws NoSuchMeasurementException, Cursor
assertThat(result.moveToFirst(), is(equalTo(true)));

assertThat(result.getString(result.getColumnIndex(MeasurementTable.COLUMN_VEHICLE)),
is(equalTo(Vehicle.UNKNOWN.getDatabaseIdentifier())));
is(equalTo(UNKNOWN.getDatabaseIdentifier())));
assertThat(result.getString(result.getColumnIndex(MeasurementTable.COLUMN_STATUS)),
is(equalTo(MeasurementStatus.OPEN.getDatabaseIdentifier())));
assertThat(result.getShort(result.getColumnIndex(MeasurementTable.COLUMN_PERSISTENCE_FILE_FORMAT_VERSION)),
Expand Down Expand Up @@ -213,7 +214,7 @@ public void testCreateNewMeasurement() throws NoSuchMeasurementException, Cursor
assertThat(finishingResult.moveToFirst(), is(equalTo(true)));

assertThat(finishingResult.getString(finishingResult.getColumnIndex(MeasurementTable.COLUMN_VEHICLE)),
is(equalTo(Vehicle.UNKNOWN.getDatabaseIdentifier())));
is(equalTo(UNKNOWN.getDatabaseIdentifier())));
assertThat(finishingResult.getString(finishingResult.getColumnIndex(MeasurementTable.COLUMN_STATUS)),
is(equalTo(FINISHED.getDatabaseIdentifier())));
} finally {
Expand All @@ -229,7 +230,7 @@ public void testCreateNewMeasurement() throws NoSuchMeasurementException, Cursor
@Test
public void testStoreData() throws NoSuchFileException {
// Manually trigger data capturing (new measurement with sensor data and a location)
Measurement measurement = oocut.newMeasurement(Vehicle.UNKNOWN);
Measurement measurement = oocut.newMeasurement(UNKNOWN);

final Lock lock = new ReentrantLock();
final Condition condition = lock.newCondition();
Expand Down Expand Up @@ -302,7 +303,7 @@ public void testCascadingClearMeasurements() {

// Insert test measurements
final int testMeasurements = 2;
oocut.newMeasurement(Vehicle.UNKNOWN);
oocut.newMeasurement(UNKNOWN);
Measurement measurement = oocut.newMeasurement(Vehicle.CAR);

final Lock lock = new ReentrantLock();
Expand Down Expand Up @@ -397,7 +398,7 @@ public void writingDataCompleted() {
*/
@Test
public void testLoadMeasurements() throws CursorIsNullException {
oocut.newMeasurement(Vehicle.UNKNOWN);
oocut.newMeasurement(UNKNOWN);
oocut.newMeasurement(Vehicle.CAR);

List<Measurement> loadedMeasurements = oocut.loadMeasurements();
Expand All @@ -415,7 +416,7 @@ public void testLoadMeasurements() throws CursorIsNullException {
public void testDeleteMeasurement() throws CursorIsNullException {

// Arrange
Measurement measurement = oocut.newMeasurement(Vehicle.UNKNOWN);
Measurement measurement = oocut.newMeasurement(UNKNOWN);
final long measurementId = measurement.getIdentifier();

final Lock lock = new ReentrantLock();
Expand Down Expand Up @@ -502,7 +503,7 @@ public void writingDataCompleted() {
public void testLoadTrack_startPauseResumeStop() throws CursorIsNullException {

// Arrange
final Measurement measurement = oocut.newMeasurement(Vehicle.UNKNOWN);
final Measurement measurement = oocut.newMeasurement(UNKNOWN);

// Start event and 2 locations
oocut.logEvent(Event.EventType.LIFECYCLE_START, measurement, 1L);
Expand Down Expand Up @@ -548,10 +549,11 @@ public void testLoadTrack_startPauseResumeStop() throws CursorIsNullException {
* @throws CursorIsNullException If {@link ContentProvider} was inaccessible.
*/
@Test
public void testLoadTrack_startPauseResumeStop_withoutGeoLocationsAfterResume() throws CursorIsNullException {
public void testLoadTrack_startPauseResumeStop_withGeoLocationAfterStartAndAfterPause_withoutGeoLocationsAfterResume()
throws CursorIsNullException {

// Arrange
final Measurement measurement = oocut.newMeasurement(Vehicle.UNKNOWN);
final Measurement measurement = oocut.newMeasurement(UNKNOWN);

// Start event and 2 locations
oocut.logEvent(Event.EventType.LIFECYCLE_START, measurement, 1L);
Expand Down Expand Up @@ -580,6 +582,43 @@ public void testLoadTrack_startPauseResumeStop_withoutGeoLocationsAfterResume()
assertThat(tracks.get(0).getGeoLocations().get(1).getTimestamp(), is(equalTo(2L)));
}

/**
* Tests whether loading a track of geo locations is possible via the {@link PersistenceLayer} object.
* <p>
* This test uses predefined timestamps or else it will be flaky, e.g. when storing a location is faster than
* storing an event when the test assumes otherwise.
* <p>
* This test reproduced VIC-78 which occurred after refactoring loadTracks() in commit
* #0673fb3fc81f00438d063114273f17f7ed17298f where we forgot to check the result of moveToNext() in
* collectNextSubTrack().
*
* @throws CursorIsNullException If {@link ContentProvider} was inaccessible.
*/
@Test
public void testLoadTrack_startPauseResumeStop_withGeoLocationAfterStart_withoutGeoLocationsAfterPauseAndAfterResume()
throws CursorIsNullException {

// Arrange
final Measurement measurement = oocut.newMeasurement(UNKNOWN);

// Start event and at least one location between start and pause
oocut.logEvent(Event.EventType.LIFECYCLE_START, measurement, 1L);
capturingBehaviour.storeLocation(testLocation(2L), measurement.getIdentifier());
oocut.logEvent(Event.EventType.LIFECYCLE_PAUSE, measurement, 3L);
oocut.logEvent(Event.EventType.LIFECYCLE_RESUME, measurement, 4L);
oocut.logEvent(Event.EventType.LIFECYCLE_STOP, measurement, 5L);

// Act
final List<Measurement> loadedMeasurements = oocut.loadMeasurements();
assertThat(loadedMeasurements.size(), is(equalTo(1)));
List<Track> tracks = oocut.loadTracks(loadedMeasurements.get(0).getIdentifier());

// Assert
assertThat(tracks.size(), is(equalTo(1)));
assertThat(tracks.get(0).getGeoLocations().size(), is(equalTo(1)));
assertThat(tracks.get(0).getGeoLocations().get(0).getTimestamp(), is(equalTo(2L)));
}

/**
* Tests whether loading a track of geo locations is possible via the {@link PersistenceLayer} object.
* <p>
Expand All @@ -592,7 +631,7 @@ public void testLoadTrack_startPauseResumeStop_withoutGeoLocationsAfterResume()
public void testLoadTrack_startPauseResumePauseStop() throws CursorIsNullException {

// Arrange
final Measurement measurement = oocut.newMeasurement(Vehicle.UNKNOWN);
final Measurement measurement = oocut.newMeasurement(UNKNOWN);

// Start event and 2 locations
oocut.logEvent(Event.EventType.LIFECYCLE_START, measurement, 1L);
Expand Down Expand Up @@ -640,7 +679,7 @@ public void testLoadTrack_startPauseResumePauseStop() throws CursorIsNullExcepti
public void testLoadTrack_startPauseStop() throws CursorIsNullException {

// Arrange
final Measurement measurement = oocut.newMeasurement(Vehicle.UNKNOWN);
final Measurement measurement = oocut.newMeasurement(UNKNOWN);

oocut.logEvent(Event.EventType.LIFECYCLE_START, measurement, System.currentTimeMillis());
capturingBehaviour.storeLocation(testLocation(System.currentTimeMillis()), measurement.getIdentifier());
Expand Down Expand Up @@ -672,7 +711,7 @@ public void testLoadTrack_startPauseStop() throws CursorIsNullException {
public void testLoadTrack_startStop() throws CursorIsNullException {

// Arrange
final Measurement measurement = oocut.newMeasurement(Vehicle.UNKNOWN);
final Measurement measurement = oocut.newMeasurement(UNKNOWN);

oocut.logEvent(Event.EventType.LIFECYCLE_START, measurement, System.currentTimeMillis());
capturingBehaviour.storeLocation(testLocation(System.currentTimeMillis()), measurement.getIdentifier());
Expand Down Expand Up @@ -701,7 +740,7 @@ public void testLoadTrack_startStop() throws CursorIsNullException {
public void testLoadCleanedTrack() throws CursorIsNullException {

// Arrange
final Measurement measurement = oocut.newMeasurement(Vehicle.UNKNOWN);
final Measurement measurement = oocut.newMeasurement(UNKNOWN);
final long startTime = 1000000000L;
final GeoLocation locationWithJustTooBadAccuracy = new GeoLocation(51.1, 13.1,
startTime + 1, 5.0, 2000f);
Expand Down
19 changes: 14 additions & 5 deletions persistence/src/main/java/de/cyface/persistence/PersistenceLayer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
*
* @author Klemens Muthmann
* @author Armin Schnabel
* @version 16.0.0
* @version 17.0.2
* @since 2.0.0
*/
public class PersistenceLayer<B extends PersistenceBehaviour> {
Expand Down Expand Up @@ -720,7 +720,9 @@ private List<Track> loadTracks(@NonNull final Cursor geoLocationCursor, @NonNull
final Track track = new Track();
do {
final GeoLocation location = loadGeoLocation(geoLocationCursor);
track.add(location);
if (location != null) {
track.add(location);
}

} while (geoLocationCursor.moveToNext());
Validate.isTrue(track.getGeoLocations().size() > 0);
Expand All @@ -742,9 +744,11 @@ private Track collectNextSubTrack(@NonNull final Cursor geoLocationCursor, @NonN
final Track track = new Track();

GeoLocation location = loadGeoLocation(geoLocationCursor);
while (location.getTimestamp() <= pauseEventTime) {
while (location != null && location.getTimestamp() <= pauseEventTime) {

track.add(location);

// Load next GeoLocation to check it's timestamp in next iteration
geoLocationCursor.moveToNext();
location = loadGeoLocation(geoLocationCursor);
}
Expand All @@ -762,8 +766,9 @@ private Track collectNextSubTrack(@NonNull final Cursor geoLocationCursor, @NonN
*/
private void moveCursorToFirstAfter(@NonNull final Cursor geoLocationCursor, final long resumeEventTime) {

@Nullable
GeoLocation location = loadGeoLocation(geoLocationCursor);
while (location.getTimestamp() < resumeEventTime && geoLocationCursor.moveToNext()) {
while (location != null && location.getTimestamp() < resumeEventTime && geoLocationCursor.moveToNext()) {

// Load next location to check it's timestamp
location = loadGeoLocation(geoLocationCursor);
Expand All @@ -774,9 +779,13 @@ private void moveCursorToFirstAfter(@NonNull final Cursor geoLocationCursor, fin
* Loads the {@link GeoLocation} at the {@code Cursor}'s current position.
*
* @param geoLocationCursor the {@code Cursor} pointing to a {@code GeoLocation} in the database.
* @return the {@code GeoLocation} loaded.
* @return the {@code GeoLocation} loaded or {@code Null} if the cursor points to the entry after the last one.
*/
@Nullable
private GeoLocation loadGeoLocation(@NonNull final Cursor geoLocationCursor) {
if (geoLocationCursor.isAfterLast()) {
return null;
}

final double lat = geoLocationCursor.getDouble(geoLocationCursor.getColumnIndex(GeoLocationsTable.COLUMN_LAT));
final double lon = geoLocationCursor.getDouble(geoLocationCursor.getColumnIndex(GeoLocationsTable.COLUMN_LON));
Expand Down

0 comments on commit 84f2526

Please sign in to comment.