Skip to content

MTC: Validate stop code prefixes #623

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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@
<groupId>com.github.ibi-group</groupId>
<artifactId>gtfs-lib</artifactId>
<!-- Latest dev build on jitpack.io -->
<version>e9de250</version>
<version>5e004388b2473c391412fdd4954068c35186e231</version>
<!-- Exclusions added in order to silence SLF4J warnings about multiple bindings:
http://www.slf4j.org/codes.html#multiple_bindings
-->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public class MtcFeedResource implements ExternalFeedResource {
public static final String TEST_AGENCY = "test-agency";
public static final String AGENCY_ID_FIELDNAME = "AgencyId";
public static final String RESOURCE_TYPE = "MTC";
public static final String STOP_CODE_PRIMARY_PREFIX_FIELD_NAME = "PrimaryPrefix";
public static final String STOP_CODE_SECONDARY_PREFIXES_FIELD_NAME = "SecondaryPrefixes";

private String rtdApi, s3Bucket, s3Prefix;

Expand Down Expand Up @@ -337,4 +339,14 @@ static String convertRtdString(String s) {
if ("null".equals(s)) return "";
return s;
}

public static String getFieldValue(Map<String, Map<String, String>> properties, String fieldName) {
Map<String, String> values = properties.get(RESOURCE_TYPE);
return values.get(fieldName);
}

public static List<String> getSecondaryStopCodePrefixes(Map<String, Map<String, String>> properties) {
String secondaryStopPrefixValue = MtcFeedResource.getFieldValue(properties, MtcFeedResource.STOP_CODE_SECONDARY_PREFIXES_FIELD_NAME);
return (secondaryStopPrefixValue == null) ? null : List.of(secondaryStopPrefixValue.split(","));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

import java.lang.reflect.Field;

import static com.conveyal.datatools.manager.extensions.mtc.MtcFeedResource.STOP_CODE_PRIMARY_PREFIX_FIELD_NAME;
import static com.conveyal.datatools.manager.extensions.mtc.MtcFeedResource.STOP_CODE_SECONDARY_PREFIXES_FIELD_NAME;
import static com.conveyal.datatools.manager.models.ExternalFeedSourceProperty.constructId;

/**
Expand Down Expand Up @@ -70,6 +72,12 @@ public class RtdCarrier {
@JsonProperty
String EditedDate;

@JsonProperty
String PrimaryPrefix;

@JsonProperty
String SecondaryPrefixes;

/** Empty constructor needed for serialization (also used to create empty carrier). */
public RtdCarrier() {
}
Expand All @@ -94,6 +102,8 @@ public RtdCarrier(FeedSource source) {
AgencyEmail = getValueForField(source, "AgencyEmail");
AgencyUrl = getValueForField(source, "AgencyUrl");
AgencyFareUrl = getValueForField(source, "AgencyFareUrl");
PrimaryPrefix = getValueForField(source, STOP_CODE_PRIMARY_PREFIX_FIELD_NAME);
SecondaryPrefixes = getValueForField(source, STOP_CODE_SECONDARY_PREFIXES_FIELD_NAME);
}

private String getPropId(FeedSource source, String fieldName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.conveyal.datatools.common.status.MonitorableJob;
import com.conveyal.datatools.common.utils.Scheduler;
import com.conveyal.datatools.manager.DataManager;
import com.conveyal.datatools.manager.extensions.mtc.MtcFeedResource;
import com.conveyal.datatools.manager.jobs.ValidateFeedJob;
import com.conveyal.datatools.manager.jobs.ValidateMobilityDataFeedJob;
import com.conveyal.datatools.manager.jobs.validation.RouteTypeValidatorBuilder;
Expand Down Expand Up @@ -48,6 +49,8 @@
import java.text.SimpleDateFormat;
import java.time.LocalDate;
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -365,15 +368,24 @@ public void validate(MonitorableJob.Status status) {
// FIXME: pass status to validate? Or somehow listen to events?
status.update("Validating feed...", 33);

FeedSource fs = Persistence.feedSources.getById(this.feedSourceId);

// Validate the feed version.
// Certain extensions, if enabled, have extra validators.
if (isExtensionEnabled("mtc")) {
Map<String, Map<String, String>> properties = fs.externalProperties();
String primaryStopCodePrefix = MtcFeedResource.getFieldValue(properties, MtcFeedResource.STOP_CODE_PRIMARY_PREFIX_FIELD_NAME);
List<String> secondaryStopCodePrefixes = MtcFeedResource.getSecondaryStopCodePrefixes(properties);
validationResult = GTFS.validate(feedLoadResult.uniqueIdentifier, DataManager.GTFS_DATA_SOURCE,
RouteTypeValidatorBuilder::buildRouteValidator,
MTCValidator::new
(feed, errorStorage) -> new MTCValidator(
feed,
errorStorage,
primaryStopCodePrefix,
secondaryStopCodePrefixes
)
);
} else {
FeedSource fs = Persistence.feedSources.getById(this.feedSourceId);

/*
Get feed_id from feed version
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.conveyal.datatools.manager.utils;

import com.conveyal.datatools.manager.extensions.mtc.MtcFeedResource;
import org.apache.commons.codec.binary.Hex;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -14,7 +13,7 @@

public class HashUtils {

public static final Logger LOG = LoggerFactory.getLogger(MtcFeedResource.class);
public static final Logger LOG = LoggerFactory.getLogger(HashUtils.class);

/**
* Get MD5 hash for the specified file.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.conveyal.datatools.manager.models.FeedVersion;
import com.conveyal.datatools.manager.models.Project;
import com.conveyal.datatools.manager.persistence.Persistence;
import com.conveyal.datatools.manager.utils.SqlAssert;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.github.tomakehurst.wiremock.WireMockServer;
Expand All @@ -18,11 +19,15 @@
import org.junit.jupiter.params.provider.ValueSource;

import java.io.IOException;
import java.sql.SQLException;
import java.util.Date;

import static com.conveyal.datatools.TestUtils.createFeedVersion;
import static com.conveyal.datatools.TestUtils.parseJson;
import static com.conveyal.datatools.TestUtils.zipFolderFiles;
import static com.conveyal.datatools.manager.extensions.mtc.MtcFeedResource.STOP_CODE_PRIMARY_PREFIX_FIELD_NAME;
import static com.conveyal.datatools.manager.extensions.mtc.MtcFeedResource.STOP_CODE_SECONDARY_PREFIXES_FIELD_NAME;
import static com.conveyal.gtfs.error.NewGTFSErrorType.MISSING_STOP_CODE_PREFIX;
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
import static com.github.tomakehurst.wiremock.client.WireMock.get;
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
Expand Down Expand Up @@ -99,31 +104,13 @@ void canUpdateFeedExternalPropertiesToMongo() throws IOException {

// Set up some entries in the ExternalFeedSourceProperties collection.
// This one (AgencyId) should not change.
ExternalFeedSourceProperty agencyIdProp = new ExternalFeedSourceProperty(
feedSource,
"MTC",
"AgencyId",
AGENCY_CODE
);
Persistence.externalFeedSourceProperties.create(agencyIdProp);
String agencyIdPropId = createExternalFeedSourceProperties("AgencyId", AGENCY_CODE);

// This one (AgencyPublicId) should be deleted after this test (not in RTD response).
ExternalFeedSourceProperty agencyPublicIdProp = new ExternalFeedSourceProperty(
feedSource,
"MTC",
"AgencyPublicId",
AGENCY_CODE
);
Persistence.externalFeedSourceProperties.create(agencyPublicIdProp);
String agencyPublicIdPropId = createExternalFeedSourceProperties("AgencyPublicId", AGENCY_CODE);

// This one (AgencyEmail) should be updated with this test.
ExternalFeedSourceProperty agencyEmailProp = new ExternalFeedSourceProperty(
feedSource,
"MTC",
"AgencyEmail",
"[email protected]"
);
Persistence.externalFeedSourceProperties.create(agencyEmailProp);
String agencyEmailPropId = createExternalFeedSourceProperties("AgencyEmail", "[email protected]");

// make RTD request and parse the json response
JsonNode rtdResponse = parseJson(
Expand All @@ -137,55 +124,86 @@ void canUpdateFeedExternalPropertiesToMongo() throws IOException {
// Also extract desired values from response
String responseEmail = rtdResponse.get("AgencyEmail").asText();
String responseAgencyName = rtdResponse.get("AgencyName").asText();
String responsePrimaryPrefix = rtdResponse.get(STOP_CODE_PRIMARY_PREFIX_FIELD_NAME).asText();
String responseSecondaryPrefixes = rtdResponse.get(STOP_CODE_SECONDARY_PREFIXES_FIELD_NAME).asText();

// Update MTC Feed properties in Mongo based response.
new MtcFeedResource().updateMongoExternalFeedProperties(feedSource, rtdResponse);

// Existing field AgencyId should retain the same value.
ExternalFeedSourceProperty updatedAgencyIdProp = Persistence.externalFeedSourceProperties.getById(agencyIdProp.id);
assertThat(updatedAgencyIdProp.value, equalTo(agencyIdProp.value));
ExternalFeedSourceProperty updatedAgencyIdProp = Persistence.externalFeedSourceProperties.getById(agencyIdPropId);
assertThat(updatedAgencyIdProp.value, equalTo(AGENCY_CODE));

// Existing field AgencyEmail should be updated from RTD response.
ExternalFeedSourceProperty updatedEmailProp = Persistence.externalFeedSourceProperties.getById(agencyEmailProp.id);
ExternalFeedSourceProperty updatedEmailProp = Persistence.externalFeedSourceProperties.getById(agencyEmailPropId);
assertThat(updatedEmailProp.value, equalTo(responseEmail));

// New field AgencyName (not set up above) from RTD response should be added to Mongo.
ExternalFeedSourceProperty newAgencyNameProp = Persistence.externalFeedSourceProperties.getOneFiltered(
and(
eq("feedSourceId", feedSource.id),
eq("resourceType", "MTC"),
eq("name", "AgencyName") )
);
assertThat(newAgencyNameProp, notNullValue());
assertThat(newAgencyNameProp.value, equalTo(responseAgencyName));
checkForRTDPropInMongo("AgencyName", responseAgencyName);
String primaryPrefixPropId = checkForRTDPropInMongo(STOP_CODE_PRIMARY_PREFIX_FIELD_NAME, responsePrimaryPrefix);
String secondaryPrefixesPropId = checkForRTDPropInMongo(STOP_CODE_SECONDARY_PREFIXES_FIELD_NAME, responseSecondaryPrefixes);

// Removed field AgencyPublicId from RTD should be deleted from Mongo.
ExternalFeedSourceProperty removedPublicIdProp = Persistence.externalFeedSourceProperties.getById(agencyPublicIdProp.id);
ExternalFeedSourceProperty removedPublicIdProp = Persistence.externalFeedSourceProperties.getById(agencyPublicIdPropId);
assertThat(removedPublicIdProp, nullValue());

Persistence.externalFeedSourceProperties.removeById(agencyIdProp.id);
Persistence.externalFeedSourceProperties.removeById(agencyPublicIdProp.id);
Persistence.externalFeedSourceProperties.removeById(agencyEmailProp.id);
Persistence.externalFeedSourceProperties.removeById(agencyIdPropId);
Persistence.externalFeedSourceProperties.removeById(agencyPublicIdPropId);
Persistence.externalFeedSourceProperties.removeById(agencyEmailPropId);
Persistence.externalFeedSourceProperties.removeById(primaryPrefixPropId);
Persistence.externalFeedSourceProperties.removeById(secondaryPrefixesPropId);
}

/**
* Check that new fields from RTD response are added to Mongo.
*/
private String checkForRTDPropInMongo(String propName, String expectedValue) {
ExternalFeedSourceProperty prop = Persistence.externalFeedSourceProperties.getOneFiltered(
and(
eq("feedSourceId", feedSource.id),
eq("resourceType", "MTC"),
eq("name", propName)
)
);
assertThat(prop, notNullValue());
assertThat(prop.value, equalTo(expectedValue));
return prop.id;
}

@Test
void shouldTolerateNullObjectInExternalPropertyAgencyId() throws IOException {
// Add an entry in the ExternalFeedSourceProperties collection
// with AgencyId value set to null.
ExternalFeedSourceProperty agencyIdProp = new ExternalFeedSourceProperty(
feedSource,
"MTC",
"AgencyId",
null
);
Persistence.externalFeedSourceProperties.create(agencyIdProp);

String agencyIdProp = createExternalFeedSourceProperties("AgencyId", null);
// Trigger the feed update process (it should not upload anything to S3).
FeedVersion feedVersion = createFeedVersion(feedSource, zipFolderFiles("mini-bart-new"));
FeedVersion feedVersion = createFeedVersion(feedSource, zipFolderFiles("mtc-feed-resource-test"));
MtcFeedResource mtcFeedResource = new MtcFeedResource();
assertDoesNotThrow(() -> mtcFeedResource.feedVersionCreated(feedVersion, null));
Persistence.externalFeedSourceProperties.removeById(agencyIdProp);
}

Persistence.externalFeedSourceProperties.removeById(agencyIdProp.id);
@Test
void shouldValidateStopCodePrefixes() throws IOException, SQLException {
String primaryPrefixPropId = createExternalFeedSourceProperties(STOP_CODE_PRIMARY_PREFIX_FIELD_NAME, "primary-1");
String secondaryPrefixesPropId = createExternalFeedSourceProperties(STOP_CODE_SECONDARY_PREFIXES_FIELD_NAME, "secondary-1,secondary-2");

FeedVersion feedVersion = createFeedVersion(feedSource, zipFolderFiles("mtc-feed-resource-test"));
MtcFeedResource mtcFeedResource = new MtcFeedResource();
assertDoesNotThrow(() -> mtcFeedResource.feedVersionCreated(feedVersion, null));
SqlAssert sqlAssert = new SqlAssert(feedVersion);
sqlAssert.errors.assertCount(1, String.format("error_type='%s'", MISSING_STOP_CODE_PREFIX.name()));
Persistence.externalFeedSourceProperties.removeById(primaryPrefixPropId);
Persistence.externalFeedSourceProperties.removeById(secondaryPrefixesPropId);
}

private String createExternalFeedSourceProperties(String name, String value) {
ExternalFeedSourceProperty prop = new ExternalFeedSourceProperty(
feedSource,
"MTC",
name,
value
);
Persistence.externalFeedSourceProperties.create(prop);
return prop.id;
}

@ParameterizedTest
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
agency_id,agency_name,agency_url,agency_timezone,agency_lang
BART,Bay Area Rapid Transit,http://www.bart.gov,America/Los_Angeles,en
Loading
Loading