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.

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

Updates to enum use and parameterized unit tests #585

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -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<SharedStopsHeader, Integer> headerIndices) {
return headerIndices.size() == SharedStopsHeader.values().length;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -60,45 +57,38 @@ public static String[] getHeaders(CsvReader configReader) throws IOException {
return configReader.getHeaders();
}

public static Map<SharedStopsHeader, Integer> getHeaderIncedes(CsvReader configReader) {
HashMap map = new HashMap();
public static Map<SharedStopsHeader, Integer> getHeaderIndices(CsvReader configReader) {
Map<SharedStopsHeader, Integer> headerIndices = new EnumMap<>(SharedStopsHeader.class);

try {
configReader.readRecord();
String[] headers = getHeaders(configReader);

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
Expand All @@ -110,7 +100,6 @@ public void validate() {

CsvReader configReader = CsvReader.parse(config);


// Build list of stop Ids.
List<String> stopIds = new ArrayList<>();
List<Stop> stops = new ArrayList<>();
Expand All @@ -120,59 +109,59 @@ 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<String> seenStopIds = new HashSet<>();
HashSet<String> stopGroupsWithPrimaryStops = new HashSet<>();

try {
Map<SharedStopsHeader, Integer> 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 {
stopGroupsWithPrimaryStops.add(stopGroupId);
}
}

// 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();
}


}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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<InvalidCSVArgument> 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;
}
}
}
Loading