Skip to content
This repository was archived by the owner on May 27, 2024. It is now read-only.
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions RULES.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ Rules are declared in the [`ValidationRules` class](https://github.com/CUTR-at-U
| [E050](#E050) | `timestamp` is in the future
| [E051](#E051) | GTFS-rt `stop_sequence` not found in GTFS data
| [E052](#E052) | `vehicle.id` is not unique
| [E053](#E053) | `start_time` for trip has changed

### Table of Warnings

Expand Down Expand Up @@ -717,6 +718,16 @@ From [VehiclePosition.VehicleDescriptor](https://github.com/google/transit/blob/
#### References:
* [`vehicle.id`](https://github.com/google/transit/blob/master/gtfs-realtime/spec/en/reference.md#message-vehicledescriptor)

<a name="E053"/>

### E053 - `start_time` for frequency-based trip has changed

A trip that is defined in frequencies.txt and has exact_times=0 or empty should have the same start_time in the descriptor for it's entire trip instance. A trip instance is defined as one journey of the vehicle from the starting stop_sequence for a trip_id in stop_times.txt to the ending stop_sequence for that same trip_id.

From [TripUpdate.TripDescriptor](https://github.com/google/transit/blob/master/gtfs-realtime/spec/en/reference.md#message-tripdescriptor) for `start_time`:

>If the trip corresponds to exact_times=0, then its start_time may be arbitrary, and is initially expected to be the first departure of the trip. Once established, the start_time of this frequency-based exact_times=0 trip should be considered immutable, even if the first departure time changes

# Warnings

<a name="W001"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ public class ValidationRules {
public static final ValidationRule E052 = new ValidationRule("E052", "ERROR", "vehicle.id is not unique",
"Each vehicle should have a unique ID",
"which is used by more than one vehicle in the feed");

public static final ValidationRule E053 = new ValidationRule("E053", "ERROR", "start_time for frequency-based trip changed",
"An exact_times = 0 frequencies.txt trip should have the same start_time through it's trip",
"exact_times=0 must be immutable");

private static List<ValidationRule> mAllRules = new ArrayList<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
package edu.usf.cutr.gtfsrtvalidator.lib.validation.rules;

import com.google.transit.realtime.GtfsRealtime;
import com.google.transit.realtime.GtfsRealtime.TripUpdate;

import edu.usf.cutr.gtfsrtvalidator.lib.model.MessageLogModel;
import edu.usf.cutr.gtfsrtvalidator.lib.model.OccurrenceModel;
import edu.usf.cutr.gtfsrtvalidator.lib.model.helper.ErrorListHelperModel;
Expand All @@ -26,8 +28,9 @@
import org.onebusaway.gtfs.services.GtfsMutableDao;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.List;
import java.text.ParseException;
import java.util.*;
import java.text.SimpleDateFormat;

import static edu.usf.cutr.gtfsrtvalidator.lib.validation.ValidationRules.*;

Expand All @@ -37,6 +40,7 @@
* E006 - Missing required vehicle_position trip field for frequency-based exact_times = 0
* E013 - Frequency type 0 trip schedule_relationship should be UNSCHEDULED or empty
* W005 - Missing vehicle_id in trip_update for frequency-based exact_times = 0
* E053 - An exact_times = 0 frequencies.txt trip should have the same start_time through it's trip
*/
public class FrequencyTypeZeroValidator implements FeedEntityValidator {

Expand All @@ -47,9 +51,22 @@ public List<ErrorListHelperModel> validate(long currentTimeMillis, GtfsMutableDa
List<OccurrenceModel> errorListE006 = new ArrayList<>();
List<OccurrenceModel> errorListE013 = new ArrayList<>();
List<OccurrenceModel> errorListW005 = new ArrayList<>();
List<OccurrenceModel> errorListE053 = new ArrayList<>();

HashMap<TripUpdateKey, List<TripUpdate>> previousTripUpdates = new HashMap<TripUpdateKey, List<TripUpdate>>();

if (previousFeedMessage != null) {
for (GtfsRealtime.FeedEntity entity : previousFeedMessage.getEntityList()) {
if (entity.hasTripUpdate()) {
GtfsRealtime.TripUpdate tripUpdate = entity.getTripUpdate();
addTripUpdate(previousTripUpdates, tripUpdate);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to have addTripUpdate() return the HashMap, rather than passing it as a parameter.

}
}
}

for (GtfsRealtime.FeedEntity entity : feedMessage.getEntityList()) {
if (entity.hasTripUpdate()) {

GtfsRealtime.TripUpdate tripUpdate = entity.getTripUpdate();

if (gtfsMetadata.getExactTimesZeroTripIds().contains(tripUpdate.getTrip().getTripId())) {
Expand All @@ -75,9 +92,11 @@ public List<ErrorListHelperModel> validate(long currentTimeMillis, GtfsMutableDa
// W005 - Missing vehicle_id in trip_update for frequency-based exact_times = 0
RuleUtils.addOccurrence(W005, "trip_id " + tripUpdate.getTrip().getTripId(), errorListW005, _log);
}

if(previousFeedMessage!=null)
checkE053(tripUpdate, previousTripUpdates, errorListE053);
}
}

if (entity.hasVehicle()) {
GtfsRealtime.VehiclePosition vehiclePosition = entity.getVehicle();
if (vehiclePosition.hasTrip() &&
Expand Down Expand Up @@ -109,6 +128,7 @@ public List<ErrorListHelperModel> validate(long currentTimeMillis, GtfsMutableDa
}
}
}

List<ErrorListHelperModel> errors = new ArrayList<>();
if (!errorListE006.isEmpty()) {
errors.add(new ErrorListHelperModel(new MessageLogModel(E006), errorListE006));
Expand All @@ -119,6 +139,126 @@ public List<ErrorListHelperModel> validate(long currentTimeMillis, GtfsMutableDa
if (!errorListW005.isEmpty()) {
errors.add(new ErrorListHelperModel(new MessageLogModel(W005), errorListW005));
}
if (!errorListE053.isEmpty()) {
errors.add(new ErrorListHelperModel(new MessageLogModel(E053), errorListE053));
}
return errors;

}

private void checkE053(TripUpdate currentTripUpdate, HashMap<TripUpdateKey, List<TripUpdate>> previousTripUpdates, List<OccurrenceModel> errorListE053) {

if (!inTripUpdates(currentTripUpdate, previousTripUpdates)) {
if (!isNewTrip(previousTripUpdates, currentTripUpdate)) {
String errorMessage = "vehicle_id " + currentTripUpdate.getVehicle().getId() + " startTime has changed and is now " + currentTripUpdate.getTrip().getStartTime();
RuleUtils.addOccurrence(E053, errorMessage, errorListE053, _log);
}
}

}

private boolean isNewTrip(HashMap<TripUpdateKey, List<TripUpdate>> previousTripUpdates, TripUpdate tripUpdate) {
TripUpdate latest = null;
List<TripUpdate> tripUpdates = previousTripUpdates.get(new TripUpdateKey(tripUpdate.getVehicle().getId(), tripUpdate.getTrip().getTripId()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some comments here to make it a bit easier to follow the logic?

if (tripUpdates != null) {
for (TripUpdate previousTripUpdate : tripUpdates) {
if (latest == null) {
latest = previousTripUpdate;
}
if (new TripUpdateStartTimeComparator().compare(latest, previousTripUpdate) > 0) {
latest = previousTripUpdate;
}
}
if (new TripUpdateStartTimeComparator().compare(tripUpdate, latest) > 0) {
return true;
} else {
return false;
}
}
return true;
}

private boolean inTripUpdates(TripUpdate tripUpdate, HashMap<TripUpdateKey, List<TripUpdate>> previousTripUpdates ) {

List<TripUpdate> tripUpdates = previousTripUpdates.get(new TripUpdateKey(tripUpdate.getVehicle().getId(), tripUpdate.getTrip().getTripId()));
if(tripUpdates !=null )
{
for (TripUpdate previousTripUpdate : tripUpdates) {
if (previousTripUpdate.getTrip().getStartTime().equals(tripUpdate.getTrip().getStartTime())) {
return true;
}
}
}
return false;
}


private void addTripUpdate(HashMap<TripUpdateKey, List<TripUpdate>> tripUpdatesMap, GtfsRealtime.TripUpdate tripUpdate) {
if (tripUpdate.hasVehicle() && tripUpdate.getVehicle().hasId()) {

List<TripUpdate> tripUpdates = null;
if (!tripUpdatesMap.containsKey(new TripUpdateKey(tripUpdate.getVehicle().getId(), tripUpdate.getTrip().getTripId()))) {
tripUpdates = new ArrayList<TripUpdate>();
} else {
tripUpdates = tripUpdatesMap.get(new TripUpdateKey(tripUpdate.getVehicle().getId(), tripUpdate.getTrip().getTripId()));
}
tripUpdates.add(tripUpdate);

//Collections.sort(tripUpdates, new TripUpdateStartTimeComparator());

tripUpdatesMap.put(new TripUpdateKey(tripUpdate.getVehicle().getId(), tripUpdate.getTrip().getTripId()), tripUpdates);
}
}
private class TripUpdateKey
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could just concatenate the vehicle_id and trip_id strings and that by itself would be fine for a composite key? So "1234_400" or something similar. Having an entire class for this seems overblown to me.

{
String vehicle_id;
String trip_id;

public TripUpdateKey(String vehicle_id, String trip_id)
{
this.vehicle_id=vehicle_id;
this.trip_id=trip_id;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

TripUpdateKey that = (TripUpdateKey) o;

if (vehicle_id != null ? !vehicle_id.equals(that.vehicle_id) : that.vehicle_id != null) return false;
return trip_id != null ? trip_id.equals(that.trip_id) : that.trip_id == null;
}

@Override
public int hashCode() {
int result = vehicle_id != null ? vehicle_id.hashCode() : 0;
result = 31 * result + (trip_id != null ? trip_id.hashCode() : 0);
return result;
}
}

private class TripUpdateStartTimeComparator implements Comparator<TripUpdate> {
SimpleDateFormat formatter = new SimpleDateFormat("hh:mm:ssa");

@Override
public int compare(TripUpdate t1, TripUpdate t2) {

try {
if(t1.hasTrip()&&t1.getTrip().hasStartTime()&&t2.hasTrip()&&t2.getTrip().hasStartTime()) {

Date t1_date = formatter.parse(t1.getTrip().getStartTime());

Date t2_date = formatter.parse(t2.getTrip().getStartTime());

return t1_date.compareTo(t2_date);
}
} catch (ParseException e) {
_log.error(e.getMessage(), e);
}
return -1;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,6 @@ public void testIsInFuture() {
@Test
public void testGetAllRules() {
List<ValidationRule> rules = ValidationRules.getRules();
assertEquals(61, rules.size());
assertEquals(62, rules.size());
}
}
Loading