Skip to content

Commit

Permalink
limit number of error/warning/info messages
Browse files Browse the repository at this point in the history 
Messages are added via a method only when set is below a certain size.
If a set reaches the maximum size, a message is added explaining that
some messages were omitted for brevity.
  • Loading branch information
@abyrd
abyrd committed Dec 21, 2022
1 parent bed8ca8 commit f0c1157
Show file tree
Hide file tree
Showing 17 changed files with 160 additions and 113 deletions.
3 changes: 3 additions & 0 deletions src/main/java/com/conveyal/r5/analyst/error/TaskError.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import java.util.Collection;
import java.util.List;

import static com.google.common.base.Preconditions.checkArgument;

/**
* This is an API model object for reporting a single error or warning that occurred on a worker back to the UI via
* the backend. The most common errors a user will see are problems applying scenario modifications, so this provides
Expand Down Expand Up @@ -35,6 +37,7 @@ public TaskError(Throwable throwable) {
public TaskError(Modification modification, Collection<String> messages) {
this.modificationId = modification.comment;
this.title = "while applying the modification entitled '" + modification.comment + "'.";
checkArgument(messages.size() <= Modification.MAX_MESSAGES + 1);
this.messages.addAll(messages);
}

Expand Down
28 changes: 14 additions & 14 deletions src/main/java/com/conveyal/r5/analyst/scenario/AddStreets.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,24 +70,24 @@ public class AddStreets extends Modification {
@Override
public boolean resolve (TransportNetwork network) {
if (allowedModes == null || allowedModes.isEmpty()) {
errors.add("You must supply at least one StreetMode.");
addError("You must supply at least one StreetMode.");
}
if (allowedModes != null && allowedModes.contains(StreetMode.CAR)) {
if (carSpeedKph == null) {
errors.add("You must supply a car speed when specifying the CAR mode.");
addError("You must supply a car speed when specifying the CAR mode.");
}
}
if (carSpeedKph != null) {
// TODO factor out repetitive range checking code into a utility function
if (carSpeedKph < 1 || carSpeedKph > 150) {
errors.add("Car speed should be in the range [1...150] kph. Specified speed is: " + carSpeedKph);
addError("Car speed should be in the range [1...150] kph. Specified speed is: " + carSpeedKph);
}
}
if (lineStrings == null || lineStrings.length < 1) {
errors.add("Modification contained no LineStrings.");
addError("Modification contained no LineStrings.");
} else for (double[][] lineString : lineStrings) {
if (lineString.length < 2) {
errors.add("LineString had less than two coordinates.");
addError("LineString had less than two coordinates.");
}
for (double[] coord : lineString) {
// Incoming coordinates use the same (x, y) axis order as JTS.
Expand All @@ -97,21 +97,21 @@ public boolean resolve (TransportNetwork network) {
}
if (walkTimeFactor != null) {
if (network.streetLayer.edgeStore.edgeTraversalTimes == null && walkTimeFactor != 1) {
errors.add("walkGenCostFactor can only be set to values other than 1 on networks that support per-edge factors.");
addError("walkGenCostFactor can only be set to values other than 1 on networks that support per-edge factors.");
}
if (walkTimeFactor <= 0 || walkTimeFactor > 10) {
errors.add("walkGenCostFactor must be in the range (0...10].");
addError("walkGenCostFactor must be in the range (0...10].");
}
}
if (bikeTimeFactor != null) {
if (network.streetLayer.edgeStore.edgeTraversalTimes == null && bikeTimeFactor != 1) {
errors.add("bikeGenCostFactor can only be set to values other than 1 on networks that support per-edge factors.");
addError("bikeGenCostFactor can only be set to values other than 1 on networks that support per-edge factors.");
}
if (bikeTimeFactor <= 0 || bikeTimeFactor > 10) {
errors.add("bikeGenCostFactor must be in the range (0...10].");
addError("bikeGenCostFactor must be in the range (0...10].");
}
}
return errors.size() > 0;
return hasErrors();
}

@Override
Expand All @@ -128,7 +128,7 @@ public boolean apply (TransportNetwork network) {
// Only first and last coordinates should be linked to the network.
int startLinkVertex = network.streetLayer.getOrCreateVertexNear(lat, lon, linkMode);
if (startLinkVertex < 0) {
errors.add(String.format("Failed to connect first vertex to streets at (%f, %f)", lat, lon));
addError(String.format("Failed to connect first vertex to streets at (%f, %f)", lat, lon));
}
prevVertex.seek(startLinkVertex);
}
Expand All @@ -142,7 +142,7 @@ public boolean apply (TransportNetwork network) {
// Only first and last coordinates should be linked to the network.
int endLinkVertex = network.streetLayer.getOrCreateVertexNear(lat, lon, linkMode);
if (endLinkVertex < 0) {
errors.add(String.format("Failed to connect last vertex to streets at (%f, %f)", lat, lon));
addError(String.format("Failed to connect last vertex to streets at (%f, %f)", lat, lon));
}
currVertex.seek(endLinkVertex);
createEdgePair(prevVertex, currVertex, network.streetLayer.edgeStore);
Expand All @@ -156,7 +156,7 @@ public boolean apply (TransportNetwork network) {
forwardEdge.seek(forwardEdgeNumber);
network.streetLayer.indexTemporaryEdgePair(forwardEdge);
}
return errors.size() > 0;
return hasErrors();
}

private void createEdgePair (VertexStore.Vertex previousVertex, VertexStore.Vertex currentVertex, EdgeStore edgeStore) {
Expand All @@ -169,7 +169,7 @@ private void createEdgePair (VertexStore.Vertex previousVertex, VertexStore.Vert
currentVertex.getLon()
);
if (edgeLengthMm > Integer.MAX_VALUE) {
errors.add("Edge length in millimeters would overflow an int (was longer than 2147km).");
addError("Edge length in millimeters would overflow an int (was longer than 2147km).");
edgeLengthMm = Integer.MAX_VALUE;
}
// TODO mono- or bidirectional
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/com/conveyal/r5/analyst/scenario/AddTrips.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,17 @@ public class AddTrips extends Modification {
@Override
public boolean resolve (TransportNetwork network) {
if (stops == null || stops.size() < 2) {
errors.add("You must provide at least two stops.");
addError("You must provide at least two stops.");
} else {
if (frequencies.isEmpty()) {
errors.add("This modification should include at least one timetable/frequency entry.");
addError("This modification should include at least one timetable/frequency entry.");
}
for (PatternTimetable entry : frequencies) {
errors.addAll(entry.validate(stops.size()));
addErrors(entry.validate(stops.size()));
}
intStopIds = findOrCreateStops(stops, network);
}
return errors.size() > 0;
return hasErrors();
}

@Override
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/com/conveyal/r5/analyst/scenario/AdjustDwellTime.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public boolean resolve (TransportNetwork network) {
for (String stringStopId : stops) {
int intStopId = network.transitLayer.indexForStopId.get(stringStopId);
if (intStopId == -1) {
errors.add("Could not find a stop to adjust with GTFS ID " + stringStopId);
addError("Could not find a stop to adjust with GTFS ID " + stringStopId);
} else {
intStops.add(intStopId);
}
Expand All @@ -64,10 +64,10 @@ public boolean resolve (TransportNetwork network) {
}
// Not bitwise operator: non-short-circuit logical XOR.
if (!((dwellSecs >= 0) ^ (scale >= 0))) {
errors.add("Dwell time or scaling factor must be specified, but not both.");
addError("Dwell time or scaling factor must be specified, but not both.");
}
checkIds(routes, patterns, trips, true, network);
return errors.size() > 0;
return hasErrors();
}

@Override
Expand All @@ -78,9 +78,9 @@ public boolean apply (TransportNetwork network) {
if (nTripsAffected > 0) {
LOG.info("Modified {} trips.", nTripsAffected);
} else {
errors.add("This modification did not affect any trips.");
addError("This modification did not affect any trips.");
}
return errors.size() > 0;
return hasErrors();
}

private TripPattern processTripPattern (TripPattern originalPattern) {
Expand Down
12 changes: 6 additions & 6 deletions src/main/java/com/conveyal/r5/analyst/scenario/AdjustFrequency.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ public class AdjustFrequency extends Modification {
@Override
public boolean resolve (TransportNetwork network) {
if (entries.isEmpty()) {
errors.add("This modification should include at least one timetable/frequency entry.");
addError("This modification should include at least one timetable/frequency entry.");
}
for (PatternTimetable entry : entries) {
errors.addAll(entry.validate(-1));
addErrors(entry.validate(-1));
}
return errors.size() > 0;
return hasErrors();
}

@Override
Expand Down Expand Up @@ -112,12 +112,12 @@ public boolean apply(TransportNetwork network) {
Set<PatternTimetable> unmatchedEntries = new HashSet<>(entries);
unmatchedEntries.removeAll(entriesMatched);
for (PatternTimetable entry : unmatchedEntries) {
errors.add("Trip not found for ID " + entry.sourceTrip + ". Ensure a trip pattern has been selected in " +
addError("Trip not found for ID " + entry.sourceTrip + ". Ensure a trip pattern has been selected in " +
"this modification.");
}
LOG.info("Cleared {} patterns, creating {} new trip schedules.", nPatternsCleared, nTripSchedulesCreated);

return errors.size() > 0;
return hasErrors();
}

/**
Expand Down Expand Up @@ -268,7 +268,7 @@ private TripPattern processPattern(TripPattern originalPattern) {
private Service blackOutService(final TripSchedule schedule) {

if (schedule.headwaySeconds != null) {
errors.add("We do not currently support retaining existing frequency entries when adjusting timetables.");
addError("We do not currently support retaining existing frequency entries when adjusting timetables.");
return null;
}

Expand Down
20 changes: 10 additions & 10 deletions src/main/java/com/conveyal/r5/analyst/scenario/AdjustSpeed.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,15 @@ public class AdjustSpeed extends Modification {
@Override
public boolean resolve(TransportNetwork network) {
if (scale <= 0) {
errors.add("Scaling factor must be a positive number.");
addError("Scaling factor must be a positive number.");
}
if (hops != null) {
// De-duplicate hops. See explanation on the hops and uniqueHops fields.
{
Set<P2<String>> uniqueHopSet = new HashSet<>();
for (String[] pair : hops) {
if (pair.length != 2) {
errors.add("Hops must all have exactly two stops.");
addError("Hops must all have exactly two stops.");
continue;
}
// Pair has equals and hashcode implementations which arrays lack
Expand All @@ -117,19 +117,19 @@ public boolean resolve(TransportNetwork network) {
int intFromId = network.transitLayer.indexForStopId.get(pair.a);
int intToId = network.transitLayer.indexForStopId.get(pair.b);
if (intFromId == -1) {
errors.add("Could not find hop origin stop " + pair.a);
addError("Could not find hop origin stop " + pair.a);
continue;
}
if (intToId == -1) {
errors.add("Could not find hop destination stop " + pair.b);
addError("Could not find hop destination stop " + pair.b);
continue;
}
hopFromStops.add(intFromId);
hopToStops.add(intToId);
}
}
checkIds(routes, patterns, trips, true, network);
return errors.size() > 0;
return hasErrors();
}

@Override
Expand All @@ -139,19 +139,19 @@ public boolean apply(TransportNetwork network) {
.collect(Collectors.toList());

if (nTripsAffected > 0) {
info.add(String.format("Changed speed on %d trips.", nTripsAffected));
addInfo(String.format("Changed speed on %d trips.", nTripsAffected));
} else {
errors.add("This modification did not cause any changes to the transport network.");
addError("This modification did not cause any changes to the transport network.");
}
if (uniqueHops != null) {
for (int h = 0; h < uniqueHops.size(); h++) {
if (nPatternsAffectedByHop[h] == 0) {
errors.add("No patterns were affected by hop: " + uniqueHops.get(h));
addError("No patterns were affected by hop: " + uniqueHops.get(h));
}
}
info.add("Number of patterns affected by each unique hop: " + Arrays.toString(nPatternsAffectedByHop));
addInfo("Number of patterns affected by each unique hop: " + Arrays.toString(nPatternsAffectedByHop));
}
return errors.size() > 0;
return hasErrors();
}

private TripPattern processTripPattern (TripPattern originalPattern) {
Expand Down
Loading

0 comments on commit f0c1157

Please sign in to comment.