Skip to content

Commit 51d499a

Browse files
committed
clean up and reduce repetition in validation code
- factor out checks against arrays of valid values - use Arrays.toString, List.of, Ints.contains, static imports
1 parent d5faafa commit 51d499a

File tree

1 file changed

+32
-28
lines changed

1 file changed

+32
-28
lines changed

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

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@
6060
import static com.conveyal.analysis.util.JsonUtil.toJson;
6161
import static com.conveyal.file.FileCategory.BUNDLES;
6262
import static com.conveyal.file.FileCategory.RESULTS;
63+
import static com.conveyal.file.FileStorageFormat.GEOTIFF;
64+
import static com.conveyal.file.FileStorageFormat.GRID;
65+
import static com.conveyal.file.FileStorageFormat.PNG;
6366
import static com.conveyal.file.UrlWithHumanName.filenameCleanString;
6467
import static com.conveyal.r5.transit.TransportNetworkCache.getScenarioFilename;
6568
import static com.google.common.base.Preconditions.checkArgument;
@@ -308,7 +311,7 @@ private Object getAllRegionalResults (Request req, Response res) throws IOExcept
308311
for (int threshold : getValidThresholds(analysis)) {
309312
for (int percentile : analysis.travelTimePercentiles) {
310313
HumanKey gridKey = getSingleCutoffGrid(
311-
analysis, destinations, threshold, percentile, gridResultType, FileStorageFormat.GEOTIFF
314+
analysis, destinations, threshold, percentile, gridResultType, GEOTIFF
312315
);
313316
humanKeys.add(gridKey);
314317
progressListener.increment();
@@ -384,38 +387,17 @@ private UrlWithHumanName getRegionalResults (Request req, Response res) throws I
384387
// expected to have no gridded results and cleanly return a 404?
385388
final String regionalAnalysisId = req.params("_id");
386389
FileStorageFormat format = FileStorageFormat.valueOf(req.params("format").toUpperCase());
387-
if (!FileStorageFormat.GRID.equals(format) && !FileStorageFormat.PNG.equals(format) && !FileStorageFormat.GEOTIFF.equals(format)) {
388-
throw AnalysisServerException.badRequest("Format \"" + format + "\" is invalid. Request format must be \"grid\", \"png\", or \"geotiff\".");
390+
if (!List.of(GRID, PNG, GEOTIFF).contains(format)) {
391+
throw AnalysisServerException.badRequest("Parameter 'format' must be one of [grid, png, geotiff].");
389392
}
390393
final UserPermissions userPermissions = UserPermissions.from(req);
391394
RegionalAnalysis analysis = getAnalysis(regionalAnalysisId, userPermissions);
392395
GridResultType gridResultType = determineGridResultType(analysis);
393396
// 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),
405-
"Percentile for this regional analysis must be taken from this list: (%s)",
406-
Ints.join(", ", percentiles)
407-
);
408-
// Handle regional analyses with multiple destination pointsets per analysis.
409-
int nGrids = analysis.destinationPointSetIds.length;
410-
checkState(nGrids > 0, "Regional analysis has no grids.");
411-
String destinationPointSetId = req.queryParams("destinationPointSetId");
412-
if (destinationPointSetId == null) {
413-
destinationPointSetId = analysis.destinationPointSetIds[0];
414-
}
415-
checkArgument(Arrays.asList(analysis.destinationPointSetIds).contains(destinationPointSetId),
416-
"Destination gridId must be one of: %s",
417-
String.join(",", analysis.destinationPointSetIds)
418-
);
397+
int threshold = getAndValidateIntParameter(req, "threshold", getValidThresholds(analysis));
398+
int percentile = getAndValidateIntParameter(req, "percentile", analysis.travelTimePercentiles);
399+
String destinationPointSetId = getAndValidateStringParameter(
400+
req, "destinationPointSetId", analysis.destinationPointSetIds);
419401
// We started implementing the ability to retrieve and display partially completed analyses.
420402
// We eventually decided these should not be available here at the same endpoint as complete, immutable results.
421403
if (broker.findJob(regionalAnalysisId) != null) {
@@ -441,6 +423,28 @@ private GridResultType determineGridResultType (RegionalAnalysis analysis) {
441423
return analysis.request.includeTemporalDensity ? GridResultType.DUAL_ACCESS : GridResultType.ACCESS;
442424
}
443425

426+
/// Get the value for a given query parameter name, check that it's non-null and can be parsed
427+
/// as an integer, and check that the value is present in an array of valid values.
428+
private int getAndValidateIntParameter (Request req, String parameterName, int[] allowedValues) {
429+
int value = getIntQueryParameter(req, parameterName);
430+
checkState(allowedValues != null && allowedValues.length > 0, "Lacking values for " + parameterName);
431+
checkArgument(Ints.contains(allowedValues, value), "Parameter '%s' must be one of: %s",
432+
parameterName, Arrays.toString(allowedValues));
433+
return value;
434+
}
435+
436+
/// Should behave identically to getAndValidateIntParameter, but for Strings.
437+
private String getAndValidateStringParameter (Request req, String parameterName, String[] allowedValues) {
438+
checkState(allowedValues != null && allowedValues.length > 0, "Lacking values for " + parameterName);
439+
String value = req.queryParams(parameterName);
440+
if (value == null || value.isEmpty()) {
441+
throw new IllegalArgumentException("Must provide query parameter " + parameterName);
442+
}
443+
checkArgument(List.of(allowedValues).contains(value), "Parameter '%s' must be one of: %s",
444+
parameterName, Arrays.toString(allowedValues));
445+
return value;
446+
}
447+
444448
private Object getCsvResults (Request req, Response res) {
445449
final String regionalAnalysisId = req.params("_id");
446450
final CsvResultType resultType = CsvResultType.valueOf(req.params("resultType").toUpperCase());

0 commit comments

Comments
 (0)