Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Limit number of error/warning/info messages #845

Merged
merged 2 commits into from
Dec 21, 2022
Merged

Limit number of error/warning/info messages #845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d8515a
limit number of error/warning/info messages
abyrd Dec 21, 2022
2febaed
fix whitespace from search and replace
abyrd Dec 21, 2022
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
3 changes: 3 additions & 0 deletions src/main/java/com/conveyal/r5/analyst/error/TaskError.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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