diff --git a/src/main/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsHeader.java b/src/main/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsHeader.java new file mode 100644 index 000000000..9d63ae7ce --- /dev/null +++ b/src/main/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsHeader.java @@ -0,0 +1,37 @@ +package com.conveyal.datatools.manager.jobs.validation; + +import java.util.Map; + +public enum SharedStopsHeader { + + STOP_GROUP_ID_INDEX("stop_group_id"), + + STOP_ID_INDEX("stop_id"), + + IS_PRIMARY_INDEX("is_primary"), + + FEED_ID_INDEX("feed_id"); + + public final String headerName; + + SharedStopsHeader(String name) { + this.headerName = name; + } + + public String getHeaderName() { + return headerName; + } + + public static SharedStopsHeader fromValue(String headerName) { + for (SharedStopsHeader sharedStopsHeader : SharedStopsHeader.values()) { + if (sharedStopsHeader.getHeaderName().equalsIgnoreCase(headerName)) { + return sharedStopsHeader; + } + } + throw new UnsupportedOperationException(String.format("Unknown shared stops header: %s", headerName)); + } + + public static boolean hasRequiredHeaders(Map headerIndices) { + return headerIndices.size() == SharedStopsHeader.values().length; + } +} diff --git a/src/main/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidator.java b/src/main/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidator.java index dfc795879..a99e686f2 100644 --- a/src/main/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidator.java +++ b/src/main/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidator.java @@ -13,17 +13,13 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.HashMap; +import java.util.EnumMap; import java.util.HashSet; import java.util.List; import java.util.Map; -enum SharedStopsHeader { - STOP_GROUP_ID_INDEX, - STOP_ID_INDEX, - IS_PRIMARY_INDEX, - FEED_ID_INDEX -} +import static com.conveyal.datatools.manager.jobs.validation.SharedStopsHeader.STOP_GROUP_ID_INDEX; +import static com.conveyal.datatools.manager.jobs.validation.SharedStopsHeader.STOP_ID_INDEX; public class SharedStopsValidator extends FeedValidator { private static final Logger LOG = LoggerFactory.getLogger(SharedStopsValidator.class); @@ -48,6 +44,7 @@ public SharedStopsValidator buildSharedStopsValidator(Feed feed, SQLErrorStorage } return new SharedStopsValidator(feed, errorStorage, this.project, this.feedId); } + public SharedStopsValidator(Feed feed, SQLErrorStorage errorStorage, Project project, String feedId) { super(feed, errorStorage); this.feed = feed; @@ -60,8 +57,8 @@ public static String[] getHeaders(CsvReader configReader) throws IOException { return configReader.getHeaders(); } - public static Map getHeaderIncedes(CsvReader configReader) { - HashMap map = new HashMap(); + public static Map getHeaderIndices(CsvReader configReader) { + Map headerIndices = new EnumMap<>(SharedStopsHeader.class); try { configReader.readRecord(); @@ -69,36 +66,29 @@ public static Map getHeaderIncedes(CsvReader configR for (int i = 0; i < headers.length; i++) { String header = headers[i]; - switch (header) { - case "stop_group_id": - map.put(SharedStopsHeader.STOP_GROUP_ID_INDEX, i); + switch (SharedStopsHeader.fromValue(header)) { + case STOP_GROUP_ID_INDEX: + headerIndices.put(STOP_GROUP_ID_INDEX, i); break; - case "feed_id": - map.put(SharedStopsHeader.FEED_ID_INDEX, i); + case FEED_ID_INDEX: + headerIndices.put(SharedStopsHeader.FEED_ID_INDEX, i); break; - case "stop_id": - map.put(SharedStopsHeader.STOP_ID_INDEX, i); + case STOP_ID_INDEX: + headerIndices.put(STOP_ID_INDEX, i); break; - case "is_primary": - map.put(SharedStopsHeader.IS_PRIMARY_INDEX, i); + case IS_PRIMARY_INDEX: + headerIndices.put(SharedStopsHeader.IS_PRIMARY_INDEX, i); break; - default: - throw new RuntimeException("shared_stops.csv contained invalid headers!"); } } - // TODO: some way to generate this from the enum? - if (!map.containsKey(SharedStopsHeader.STOP_GROUP_ID_INDEX) || - !map.containsKey(SharedStopsHeader.FEED_ID_INDEX) || - !map.containsKey(SharedStopsHeader.STOP_ID_INDEX) || - !map.containsKey(SharedStopsHeader.IS_PRIMARY_INDEX)) { + if (!SharedStopsHeader.hasRequiredHeaders(headerIndices)) { throw new RuntimeException("shared_stops.csv is missing headers!"); } } catch (IOException e) { throw new RuntimeException("shared_stops.csv was invalid!", e); } - - return map; + return headerIndices; } @Override @@ -110,7 +100,6 @@ public void validate() { CsvReader configReader = CsvReader.parse(config); - // Build list of stop Ids. List stopIds = new ArrayList<>(); List stops = new ArrayList<>(); @@ -120,41 +109,42 @@ public void validate() { stopIds.add(stop.stop_id); } - Map indeces = getHeaderIncedes(configReader); - - // Initialize hashmaps to hold duplication info + // Initialize hashmaps to hold duplication info. HashSet seenStopIds = new HashSet<>(); HashSet stopGroupsWithPrimaryStops = new HashSet<>(); try { + Map headerIndices = getHeaderIndices(configReader); while (configReader.readRecord()) { - // TODO: Avoid casting here? It must be possible with the enums - String stopGroupId = configReader.get((Integer) indeces.get(SharedStopsHeader.STOP_GROUP_ID_INDEX)); - String stopId = configReader.get((Integer) indeces.get(SharedStopsHeader.STOP_ID_INDEX)); - String sharedStopFeedId = configReader.get((Integer) indeces.get(SharedStopsHeader.FEED_ID_INDEX)); + String stopGroupId = configReader.get(headerIndices.get(STOP_GROUP_ID_INDEX)); + String stopId = configReader.get(headerIndices.get(STOP_ID_INDEX)); + String sharedStopFeedId = configReader.get(headerIndices.get(SharedStopsHeader.FEED_ID_INDEX)); - if (stopId.equals("stop_id")) { - // Swallow header row + if (stopId.equals(STOP_ID_INDEX.headerName)) { + // Swallow header row. continue; } - // Check for SS_01 (stop id appearing in multiple stop groups) - // Make sure this error is only returned if we are inside the feed that is being checked + // Check for SS_01 (stop id appearing in multiple stop groups). + // Make sure this error is only returned if we are inside the feed that is being checked. if (seenStopIds.contains(stopId)) { if (feedId.equals(sharedStopFeedId)) { registerError(stops - .stream() - .filter(stop -> stop.stop_id.equals(stopId)) - .findFirst() - .orElse(new Stop()), NewGTFSErrorType.MULTIPLE_SHARED_STOPS_GROUPS + .stream() + .filter(stop -> stop.stop_id.equals(stopId)) + .findFirst() + .orElse(new Stop()), NewGTFSErrorType.MULTIPLE_SHARED_STOPS_GROUPS ); } } else { seenStopIds.add(stopId); } - // Check for SS_02 (multiple primary stops per stop group) - if (configReader.get((Integer) indeces.get(SharedStopsHeader.IS_PRIMARY_INDEX)).equals("1") || configReader.get((Integer) indeces.get(SharedStopsHeader.IS_PRIMARY_INDEX)).equals("true")) { + // Check for SS_02 (multiple primary stops per stop group). + if ( + configReader.get(headerIndices.get(SharedStopsHeader.IS_PRIMARY_INDEX)).equals("1") || + configReader.get(headerIndices.get(SharedStopsHeader.IS_PRIMARY_INDEX)).equals("true") + ) { if (stopGroupsWithPrimaryStops.contains(stopGroupId)) { registerError(NewGTFSError.forFeed(NewGTFSErrorType.SHARED_STOP_GROUP_MULTIPLE_PRIMARY_STOPS, stopGroupId)); } else { @@ -162,17 +152,16 @@ public void validate() { } } - // Check for SS_03 (stop_id referenced doesn't exist) - // Make sure this error is only returned if we are inside the feed that is being checked + // Check for SS_03 (stop_id referenced doesn't exist). + // Make sure this error is only returned if we are inside the feed that is being checked. if (feedId.equals(sharedStopFeedId) && !stopIds.contains(stopId)) { registerError(NewGTFSError.forFeed(NewGTFSErrorType.SHARED_STOP_GROUP_ENTITY_DOES_NOT_EXIST, stopId)); } } - } catch (IOException e) { LOG.error(e.toString()); } - finally { + } catch (IOException e) { + LOG.error("Unable to validate shared stops.", e); + } finally { configReader.close(); } - - } } diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidatorTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidatorTest.java index 9ef157393..b8a944f71 100644 --- a/src/test/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidatorTest.java +++ b/src/test/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidatorTest.java @@ -2,50 +2,60 @@ import com.conveyal.datatools.UnitTest; import com.csvreader.CsvReader; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; -import java.util.Map; +import java.util.stream.Stream; import static com.zenika.snapshotmatcher.SnapshotMatcher.matchesSnapshot; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -public class SharedStopsValidatorTest extends UnitTest { - private void attemptToParseInvalidCSV(String csv, String expectedError) { - CsvReader configReader = CsvReader.parse(csv); - Exception exception = assertThrows(RuntimeException.class, () -> { - SharedStopsValidator.getHeaderIncedes(configReader); - }); +class SharedStopsValidatorTest extends UnitTest { - String error = exception.getMessage(); - assertEquals(expectedError, error); + @ParameterizedTest + @MethodSource("createCSVHeaders") + void canHandleVariousCorrectCSV(String headers) { + assertThat(SharedStopsValidator.getHeaderIndices(CsvReader.parse(headers)), matchesSnapshot()); } - private Map parseValidCSV(String csv) { - CsvReader configReader = CsvReader.parse(csv); - return SharedStopsValidator.getHeaderIncedes(configReader); + private static Stream createCSVHeaders() { + return Stream.of( + "is_primary,feed_id,stop_id,stop_group_id", + "feed_id,stop_id,stop_group_id,is_primary", + "feed_id,is_primary,stop_group_id,stop_id,feed_id" + ); } - @Test - void canHandleIncorrectCsv() { - String invalid = "shared_stops.csv contained invalid headers!"; - String missing = "shared_stops.csv is missing headers!"; + @ParameterizedTest + @MethodSource("createInvalidCSVArguments") + void attemptToParseInvalidCSV(InvalidCSVArgument invalidCSVArgument) { + CsvReader configReader = CsvReader.parse(invalidCSVArgument.csv); + Exception exception = assertThrows(RuntimeException.class, () -> SharedStopsValidator.getHeaderIndices(configReader)); + assertEquals(invalidCSVArgument.expectedError, exception.getMessage()); + } - attemptToParseInvalidCSV("this is a great string, but it's not a shared_stops csv!", invalid); - attemptToParseInvalidCSV("", invalid); - attemptToParseInvalidCSV("is_primary,stop_id", missing); - attemptToParseInvalidCSV("is_primary,feed_id,,stop_group_id", invalid); - attemptToParseInvalidCSV("is_primary,feed_id,stop_group_id", missing); - attemptToParseInvalidCSV("feed_id, is_primary, stop_group_id,stop_id", invalid); + private static Stream createInvalidCSVArguments() { + String invalid = "Unknown shared stops header: "; + String missing = "shared_stops.csv is missing headers!"; + return Stream.of( + new InvalidCSVArgument("this is a great string, but it's not a shared_stops csv!", invalid + "this is a great string"), + new InvalidCSVArgument("", invalid), + new InvalidCSVArgument("is_primary,stop_id", missing), + new InvalidCSVArgument("is_primary,feed_id,,stop_group_id", invalid), + new InvalidCSVArgument("is_primary,feed_id,stop_group_id", missing), + new InvalidCSVArgument("feed_id, is_primary, stop_group_id,stop_id", invalid + " is_primary") + ); } - @Test - void canHandleVariousCorrectCSV() { - assertThat(parseValidCSV("is_primary,feed_id,stop_id,stop_group_id"), matchesSnapshot()); - assertThat(parseValidCSV("feed_id,stop_id,stop_group_id,is_primary"), matchesSnapshot()); + private static class InvalidCSVArgument { + public String csv; + public String expectedError; - // Only last is handled - assertThat(parseValidCSV("feed_id,is_primary,stop_group_id,stop_id,feed_id"), matchesSnapshot()); + public InvalidCSVArgument(String csv, String expectedError) { + this.csv = csv; + this.expectedError = expectedError; + } } }