diff --git a/libs/deserializer/src/main/java/de/cyface/deserializer/BinaryFormatDeserializer.java b/libs/deserializer/src/main/java/de/cyface/deserializer/BinaryFormatDeserializer.java index e220e69..818fac6 100644 --- a/libs/deserializer/src/main/java/de/cyface/deserializer/BinaryFormatDeserializer.java +++ b/libs/deserializer/src/main/java/de/cyface/deserializer/BinaryFormatDeserializer.java @@ -30,6 +30,7 @@ import de.cyface.model.Measurement; import de.cyface.model.MeasurementIdentifier; import de.cyface.model.MetaData; +import de.cyface.model.NoTracksRecorded; /** * A {@link Deserializer} for a file in Cyface binary format. Constructs a new measurement from a ZLIB compressed @@ -83,7 +84,7 @@ public class BinaryFormatDeserializer implements Deserializer { } @Override - public Measurement read() throws IOException, InvalidLifecycleEvents, UnsupportedFileVersion { + public Measurement read() throws IOException, InvalidLifecycleEvents, UnsupportedFileVersion, NoTracksRecorded { try (InflaterInputStream uncompressedInput = new InflaterInputStream(compressedData, new Inflater(NOWRAP))) { final var version = BinaryFormatParser.readShort(uncompressedInput); if (version != TRANSFER_FILE_FORMAT_VERSION) { diff --git a/libs/deserializer/src/main/java/de/cyface/deserializer/Deserializer.java b/libs/deserializer/src/main/java/de/cyface/deserializer/Deserializer.java index bfa1e31..2d637a9 100644 --- a/libs/deserializer/src/main/java/de/cyface/deserializer/Deserializer.java +++ b/libs/deserializer/src/main/java/de/cyface/deserializer/Deserializer.java @@ -26,6 +26,7 @@ import de.cyface.deserializer.exceptions.NoSuchMeasurement; import de.cyface.model.Measurement; import de.cyface.model.MeasurementIdentifier; +import de.cyface.model.NoTracksRecorded; /** * Reads {@link Measurement} from the Cyface binary format. @@ -49,7 +50,7 @@ public interface Deserializer extends Serializable { * a Measurement from a source providing multiple Measurements. * @throws UnsupportedFileVersion If the binary file is from a deprecated or not yet supported file format version. */ - Measurement read() throws IOException, InvalidLifecycleEvents, NoSuchMeasurement, UnsupportedFileVersion; + Measurement read() throws IOException, InvalidLifecycleEvents, NoSuchMeasurement, UnsupportedFileVersion, NoTracksRecorded; /** * @return A list with all the valid {@link MeasurementIdentifier} within the deserializable data. * @throws IOException If reading data fails diff --git a/libs/deserializer/src/main/java/de/cyface/deserializer/UnzippedPhoneDataDeserializer.java b/libs/deserializer/src/main/java/de/cyface/deserializer/UnzippedPhoneDataDeserializer.java index 534c6e1..070520b 100644 --- a/libs/deserializer/src/main/java/de/cyface/deserializer/UnzippedPhoneDataDeserializer.java +++ b/libs/deserializer/src/main/java/de/cyface/deserializer/UnzippedPhoneDataDeserializer.java @@ -35,6 +35,7 @@ import java.util.List; import java.util.UUID; +import de.cyface.model.NoTracksRecorded; import org.apache.commons.lang3.Validate; import de.cyface.deserializer.exceptions.InvalidLifecycleEvents; @@ -152,7 +153,7 @@ public class UnzippedPhoneDataDeserializer extends PhoneDataDeserializer { } @Override - public Measurement read() throws InvalidLifecycleEvents, NoSuchMeasurement { + public Measurement read() throws InvalidLifecycleEvents, NoSuchMeasurement, NoTracksRecorded { try (final var connection = createConnection()) { PreparedStatement measurementExistsStatement = connection.prepareStatement(MEASUREMENT_QUERY); measurementExistsStatement.setLong(1, measurementNumber); diff --git a/libs/deserializer/src/main/java/de/cyface/deserializer/ZippedPhoneDataDeserializer.java b/libs/deserializer/src/main/java/de/cyface/deserializer/ZippedPhoneDataDeserializer.java index 8fc8be6..4b3ff2b 100644 --- a/libs/deserializer/src/main/java/de/cyface/deserializer/ZippedPhoneDataDeserializer.java +++ b/libs/deserializer/src/main/java/de/cyface/deserializer/ZippedPhoneDataDeserializer.java @@ -30,6 +30,7 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; +import de.cyface.model.NoTracksRecorded; import org.apache.commons.lang3.Validate; import de.cyface.deserializer.exceptions.InvalidLifecycleEvents; @@ -127,7 +128,7 @@ public class ZippedPhoneDataDeserializer extends PhoneDataDeserializer { } @Override - public Measurement read() throws IOException, InvalidLifecycleEvents, NoSuchMeasurement { + public Measurement read() throws IOException, InvalidLifecycleEvents, NoSuchMeasurement, NoTracksRecorded { if (!isUnzipped) { this.databaseFile = unzipAndReturnMatching(sqliteDatabasePath, "/measures"); this.accelerationPaths = unzip(accelerationsFilePath); diff --git a/libs/deserializer/src/test/java/de/cyface/deserializer/BinaryFormatDeserializerTest.java b/libs/deserializer/src/test/java/de/cyface/deserializer/BinaryFormatDeserializerTest.java index 6c9d8dc..3b7fe44 100644 --- a/libs/deserializer/src/test/java/de/cyface/deserializer/BinaryFormatDeserializerTest.java +++ b/libs/deserializer/src/test/java/de/cyface/deserializer/BinaryFormatDeserializerTest.java @@ -50,6 +50,7 @@ import java.util.List; import java.util.UUID; +import de.cyface.model.NoTracksRecorded; import org.hamcrest.Matcher; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -109,7 +110,7 @@ class BinaryFormatDeserializerTest { */ @Test @DisplayName("Happy Path") - void test() throws IOException, InvalidLifecycleEvents, UnsupportedFileVersion { + void test() throws IOException, InvalidLifecycleEvents, UnsupportedFileVersion, NoTracksRecorded { // Arrange final var identifier = new MeasurementIdentifier("test", 1); try (final var testData = testData(identifier)) { @@ -223,7 +224,7 @@ void test() throws IOException, InvalidLifecycleEvents, UnsupportedFileVersion { */ @DisplayName("Happy Path test for the serialization and deserialization of 3d points.") @Test - void testSerializeDeserialize() throws IOException, InvalidLifecycleEvents { + void testSerializeDeserialize() throws IOException, InvalidLifecycleEvents, NoTracksRecorded { // Arrange - Events: start, stop (1 track) final var batches = 100; diff --git a/libs/deserializer/src/test/java/de/cyface/deserializer/TrackBuilderTest.java b/libs/deserializer/src/test/java/de/cyface/deserializer/TrackBuilderTest.java index 53ee5c5..ea6639c 100644 --- a/libs/deserializer/src/test/java/de/cyface/deserializer/TrackBuilderTest.java +++ b/libs/deserializer/src/test/java/de/cyface/deserializer/TrackBuilderTest.java @@ -18,23 +18,32 @@ */ package de.cyface.deserializer; +import static de.cyface.model.Event.EventType.LIFECYCLE_PAUSE; +import static de.cyface.model.Event.EventType.LIFECYCLE_RESUME; +import static de.cyface.model.Event.EventType.LIFECYCLE_START; +import static de.cyface.model.Event.EventType.LIFECYCLE_STOP; +import static de.cyface.model.Event.EventType.MODALITY_TYPE_CHANGE; +import static de.cyface.model.MetaData.SUPPORTED_VERSIONS; import static de.cyface.model.Modality.BICYCLE; import static de.cyface.model.Modality.WALKING; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertThrows; import java.util.ArrayList; +import java.util.Date; import java.util.List; -import java.util.NoSuchElementException; import java.util.UUID; import de.cyface.deserializer.exceptions.InvalidLifecycleEvents; +import de.cyface.model.Measurement; +import de.cyface.model.MetaData; +import de.cyface.model.NoTracksRecorded; import de.cyface.model.Point3DImpl; +import de.cyface.serializer.GeoLocation; import org.apache.commons.lang3.Validate; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import de.cyface.model.Event; @@ -81,12 +90,12 @@ void testBuild() throws InvalidLifecycleEvents { final var locationRecords = generateLocationRecords(numberOfLocations, new Long[] {1000L, 1500L, 3500L, 4000L}, identifier); final var events = new ArrayList(); - events.add(new Event(Event.EventType.LIFECYCLE_START, 1000L, null)); - events.add(new Event(Event.EventType.MODALITY_TYPE_CHANGE, 1000L, WALKING.getDatabaseIdentifier())); - events.add(new Event(Event.EventType.LIFECYCLE_PAUSE, 2000L, null)); - events.add(new Event(Event.EventType.MODALITY_TYPE_CHANGE, 2500L, BICYCLE.getDatabaseIdentifier())); - events.add(new Event(Event.EventType.LIFECYCLE_RESUME, 3000L, null)); - events.add(new Event(Event.EventType.LIFECYCLE_STOP, 4000L, null)); + events.add(new Event(LIFECYCLE_START, 1000L, null)); + events.add(new Event(MODALITY_TYPE_CHANGE, 1000L, WALKING.getDatabaseIdentifier())); + events.add(new Event(LIFECYCLE_PAUSE, 2000L, null)); + events.add(new Event(MODALITY_TYPE_CHANGE, 2500L, BICYCLE.getDatabaseIdentifier())); + events.add(new Event(LIFECYCLE_RESUME, 3000L, null)); + events.add(new Event(LIFECYCLE_STOP, 4000L, null)); final var point3DS = new ArrayList(); point3DS.add(new Point3DImpl(1.0f, -2.0f, 3.0f, 1010L)); // All points >= LIFECYCLE_RESUME should be in the track, no matter when the first GPS point is captured @@ -211,10 +220,10 @@ void testBuild_withEmptyAccelerationList_shouldNotThrow() throws InvalidLifecycl ); final var events = List.of( - new Event(Event.EventType.LIFECYCLE_START, 1_000L, null), - new Event(Event.EventType.LIFECYCLE_PAUSE, 1_500L, null), - new Event(Event.EventType.LIFECYCLE_RESUME, 2_000L, null), - new Event(Event.EventType.LIFECYCLE_STOP, 2_500L, null) + new Event(LIFECYCLE_START, 1_000L, null), + new Event(LIFECYCLE_PAUSE, 1_500L, null), + new Event(LIFECYCLE_RESUME, 2_000L, null), + new Event(LIFECYCLE_STOP, 2_500L, null) ); final var empty = List.of(); // triggers .next() on empty list @@ -239,6 +248,73 @@ void testBuild_withEmptyAccelerationList_shouldNotThrow() throws InvalidLifecycl assertThat(track2.getDirections().isEmpty(), is(true)); } + /** + * Reproduced a crash which occurred in SR-2025 campaign on an Android device [STAD-712]. + * osVersion: 'Android 11', + * deviceType: 'SM-A505FN', + * appVersion: '3.3.25042964', (SR app, cyface_sdk_version = "7.13.12") + * formatVersion: 3, + * length: 0, + * locationCount: Long('2'), + *

+ * Instead of fixing the location iterator in the middle of a campaign, we just make this error fail softly + * and make it fail as INFO when we have very few locations: + * - e.g. no resume: <= (1+0+1)*2 locations => INFO + * - e.g. 1 resume: <= (1+1+1)**2 locations => INFO + * As soon as one track has > 3 locations this error should not occur. + *

+ * (1+1+1) because: with 1 resume and 2*2+1 locations it's possible, that + * - track 1: 2 location, in-between: 1 locations, track 2: 2 locations, after stop: 1 location + *

+ * If there are much more locations than resume events, this must be logged as WARN at least. (or crash) + * (and show the number of locations and resume events involved) + */ + @Test + @DisplayName("TrackBuilder creates one track for short final lifecycle segment") + void testTrackBuilderWithShortFinalSegment() throws InvalidLifecycleEvents, NoTracksRecorded { + // Given + final var identifier = new MeasurementIdentifier("test", 1); + final var events = List.of( + new Event(LIFECYCLE_START, 1746429259640L, ""), + new Event(MODALITY_TYPE_CHANGE, 1746429259640L, "BICYCLE"), + new Event(LIFECYCLE_PAUSE, 1746429328310L, ""), + new Event(LIFECYCLE_RESUME, 1746445158656L, ""), + new Event(LIFECYCLE_PAUSE, 1746445161947L, ""), + new Event(LIFECYCLE_RESUME, 1746518861627L, ""), + new Event(LIFECYCLE_STOP, 1746518872724L, "") + ); + + final var locations = List.of( + // This narrow case with only 2 locations produces the crash as we always loose the first location + new GeoLocation(48.123509, 11.372924, 1746518871000L, 1.35f, 29.5f), + new GeoLocation(48.123484, 11.372941, 1746518872000L, 1.77f, 27.0f), + // But with only 3 more location this does not happen, as 2 locations form a track + new GeoLocation(48.123485, 11.372942, 1746518872001L, 1.77f, 27.0f) + ); + + // No sensor data for this test + final var accelerations = List.of(); + final var rotations = List.of(); + final var directions = List.of(); + + final var builder = new TrackBuilder(); + + // When + final var tracks = builder.build(locations, events, accelerations, rotations, directions, identifier); + final var metaData = MetaData.Companion.create( + new MeasurementIdentifier("test", 1L), + "deviceType", + "osVersion", + "appVersion", + 0, + UUID.randomUUID(), + SUPPORTED_VERSIONS, + new Date() + ); + Measurement.Companion.create(metaData, tracks); + } + + /** * Generates GeoLocationRecords for testing. * diff --git a/libs/deserializer/src/test/java/de/cyface/deserializer/UnzippedPhoneDataDeserializerTest.java b/libs/deserializer/src/test/java/de/cyface/deserializer/UnzippedPhoneDataDeserializerTest.java index 3c022f8..f7eccbc 100644 --- a/libs/deserializer/src/test/java/de/cyface/deserializer/UnzippedPhoneDataDeserializerTest.java +++ b/libs/deserializer/src/test/java/de/cyface/deserializer/UnzippedPhoneDataDeserializerTest.java @@ -33,6 +33,7 @@ import java.util.List; import java.util.UUID; +import de.cyface.model.NoTracksRecorded; import org.apache.commons.lang3.Validate; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -52,7 +53,7 @@ public class UnzippedPhoneDataDeserializerTest { @Test @DisplayName("Test on some unzipped example data") - void test_unzipped() throws InvalidLifecycleEvents, URISyntaxException, NoSuchMeasurement { + void test_unzipped() throws InvalidLifecycleEvents, URISyntaxException, NoSuchMeasurement, NoTracksRecorded { // Arrange final var folder = "/phone-data-export/unzipped/"; final var mid = 1; @@ -90,7 +91,7 @@ void test_unzipped() throws InvalidLifecycleEvents, URISyntaxException, NoSuchMe @Test @DisplayName("Test on some zipped example data") - void test_zipped() throws URISyntaxException, InvalidLifecycleEvents, IOException, NoSuchMeasurement { + void test_zipped() throws URISyntaxException, InvalidLifecycleEvents, IOException, NoSuchMeasurement, NoTracksRecorded { // Arrange final var folder = "/phone-data-export/zipped/"; final var mid = 1; diff --git a/libs/model/src/main/kotlin/de/cyface/model/Measurement.kt b/libs/model/src/main/kotlin/de/cyface/model/Measurement.kt index 5e2b6e0..fc83262 100644 --- a/libs/model/src/main/kotlin/de/cyface/model/Measurement.kt +++ b/libs/model/src/main/kotlin/de/cyface/model/Measurement.kt @@ -513,11 +513,17 @@ open class Measurement: Serializable { * Factory to create a Measurement instance with tracks. */ @JvmStatic + @Throws(NoTracksRecorded::class) fun create( metaData: MetaData, tracks: List ): Measurement { - require(tracks.isNotEmpty()) { "Tracks list must not be empty." } + if (tracks.isEmpty()) { + // This is a known bug, for a reproducer see `TrackBuilderTest.testTrackBuilderWithShortFinalSegment` + // you just need to delete one location, it fails with 2, but succeeds with 3. As we only loose + // one location for subsequent tracks, we don't fix this during a running campaign [STAD-720]. + throw NoTracksRecorded(metaData.length) + } return Measurement().apply { this.metaData = metaData this.tracks = tracks.toMutableList() @@ -596,3 +602,6 @@ open class Measurement: Serializable { } } } + +class NoTracksRecorded(measurementLength: Double) + : Exception("No tracks for measurement with length $measurementLength")