Skip to content

Commit d5faafa

Browse files
committed
eliminate tolerance of missing threshold parameter
factor out repeated code blocks in query parameter validation
1 parent 3e953ba commit d5faafa

File tree

1 file changed

+26
-40
lines changed

1 file changed

+26
-40
lines changed

src/main/java/com/conveyal/analysis/controllers/RegionalAnalysisController.java

Lines changed: 26 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,10 @@ private RegionalAnalysis deleteRegionalAnalysis (Request req, Response res) {
161161
return analysis;
162162
}
163163

164-
private int getIntQueryParameter (Request req, String parameterName, int defaultValue) {
164+
private int getIntQueryParameter (Request req, String parameterName) {
165165
String paramValue = req.queryParams(parameterName);
166166
if (paramValue == null) {
167-
return defaultValue;
167+
throw new IllegalArgumentException("Must provide query parameter " + parameterName);
168168
}
169169
try {
170170
return Integer.parseInt(paramValue);
@@ -305,10 +305,7 @@ private Object getAllRegionalResults (Request req, Response res) throws IOExcept
305305
GridResultType gridResultType = determineGridResultType(analysis);
306306
for (String destinationPointSetId : analysis.destinationPointSetIds) {
307307
OpportunityDataset destinations = getDestinations(destinationPointSetId, userPermissions);
308-
int[] thresholds = gridResultType.equals(GridResultType.DUAL_ACCESS)
309-
? analysis.request.dualAccessThresholds
310-
: analysis.cutoffsMinutes;
311-
for (int threshold : thresholds) {
308+
for (int threshold : getValidThresholds(analysis)) {
312309
for (int percentile : analysis.travelTimePercentiles) {
313310
HumanKey gridKey = getSingleCutoffGrid(
314311
analysis, destinations, threshold, percentile, gridResultType, FileStorageFormat.GEOTIFF
@@ -393,39 +390,21 @@ private UrlWithHumanName getRegionalResults (Request req, Response res) throws I
393390
final UserPermissions userPermissions = UserPermissions.from(req);
394391
RegionalAnalysis analysis = getAnalysis(regionalAnalysisId, userPermissions);
395392
GridResultType gridResultType = determineGridResultType(analysis);
396-
// If a query parameter is supplied, range check it, otherwise use the middle value in the list.
397-
int threshold;
398-
if (gridResultType.equals(GridResultType.DUAL_ACCESS)) {
399-
int nThresholds = analysis.request.dualAccessThresholds.length;
400-
int[] thresholds = analysis.request.dualAccessThresholds;
401-
checkState(nThresholds > 0, "Regional analysis has no dual access thresholds.");
402-
threshold = getIntQueryParameter(req, "threshold", thresholds[nThresholds / 2]);
403-
checkArgument(new TIntArrayList(thresholds).contains(threshold),
404-
"Dual access thresholds for this regional analysis must be taken from this list: (%s)",
405-
Ints.join(", ", thresholds)
406-
);
407-
} else {
408-
// Handle newer regional analyses with multiple cutoffs in an array.
409-
// The cutoff variable holds the actual cutoff in minutes, not the position in the array of cutoffs.
410-
checkState(analysis.cutoffsMinutes != null, "Regional analysis has no cutoffs.");
411-
int nCutoffs = analysis.cutoffsMinutes.length;
412-
checkState(nCutoffs > 0, "Regional analysis has no cutoffs.");
413-
threshold = getIntQueryParameter(req, "threshold", analysis.cutoffsMinutes[nCutoffs / 2]);
414-
checkArgument(new TIntArrayList(analysis.cutoffsMinutes).contains(threshold),
415-
"Travel time cutoff for this regional analysis must be taken from this list: (%s)",
416-
Ints.join(", ", analysis.cutoffsMinutes)
417-
);
418-
}
419-
420-
// If a query parameter is supplied, range check it, otherwise use the middle value in the list.
421-
// The percentile variable holds the actual percentile (25, 50, 95) not the position in the array.
422-
int nPercentiles = analysis.travelTimePercentiles.length;
423-
checkState(nPercentiles > 0, "Regional analysis has no percentiles.");
424-
int percentile = getIntQueryParameter(req, "percentile", analysis.travelTimePercentiles[nPercentiles / 2]);
425-
checkArgument(new TIntArrayList(analysis.travelTimePercentiles).contains(percentile),
393+
// The threshold parameter holds the value in minutes, not the position in the array of thresholds.
394+
int threshold = getIntQueryParameter(req, "threshold");
395+
int[] thresholds = getValidThresholds(analysis);
396+
checkState(thresholds != null && thresholds.length > 0, "Regional analysis lacks thresholds for " + gridResultType);
397+
checkArgument(Ints.contains(thresholds, threshold),
398+
"Threshold parameter for this regional analysis must be taken from this list: (%s)",
399+
Ints.join(", ", thresholds)
400+
);
401+
int percentile = getIntQueryParameter(req, "percentile");
402+
int [] percentiles = analysis.travelTimePercentiles;
403+
checkState(percentiles != null && percentiles.length > 0, "Regional analysis lacks percentiles.");
404+
checkArgument(Ints.contains(percentiles, percentile),
426405
"Percentile for this regional analysis must be taken from this list: (%s)",
427-
Ints.join(", ", analysis.travelTimePercentiles));
428-
406+
Ints.join(", ", percentiles)
407+
);
429408
// Handle regional analyses with multiple destination pointsets per analysis.
430409
int nGrids = analysis.destinationPointSetIds.length;
431410
checkState(nGrids > 0, "Regional analysis has no grids.");
@@ -435,8 +414,8 @@ private UrlWithHumanName getRegionalResults (Request req, Response res) throws I
435414
}
436415
checkArgument(Arrays.asList(analysis.destinationPointSetIds).contains(destinationPointSetId),
437416
"Destination gridId must be one of: %s",
438-
String.join(",", analysis.destinationPointSetIds));
439-
417+
String.join(",", analysis.destinationPointSetIds)
418+
);
440419
// We started implementing the ability to retrieve and display partially completed analyses.
441420
// We eventually decided these should not be available here at the same endpoint as complete, immutable results.
442421
if (broker.findJob(regionalAnalysisId) != null) {
@@ -449,6 +428,13 @@ private UrlWithHumanName getRegionalResults (Request req, Response res) throws I
449428
return fileStorage.getJsonUrl(gridKey.storageKey, gridKey.humanName);
450429
}
451430

431+
private int[] getValidThresholds (RegionalAnalysis analysis) {
432+
return switch (determineGridResultType(analysis)) {
433+
case ACCESS -> analysis.cutoffsMinutes;
434+
case DUAL_ACCESS -> analysis.request.dualAccessThresholds;
435+
};
436+
}
437+
452438
// This assumes each set of regional analysis results has only primal or dual access, not both.
453439
// TODO handle regional analyses that include both regular accessibility and dual access results.
454440
private GridResultType determineGridResultType (RegionalAnalysis analysis) {

0 commit comments

Comments
 (0)