From cd0d0f85de9d12bce3ef06ce826f9b2615ff628b Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 14 Apr 2023 17:12:48 +0100 Subject: [PATCH 01/40] refactor(Check for pinned deployment in mongo db): Check if a project has a pinned deployment and re --- .../datatools/manager/models/FeedSource.java | 4 +- .../datatools/manager/models/Project.java | 43 ++++++++++++++++++- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java index 555414f71..f90ad15a7 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java @@ -49,6 +49,7 @@ import java.util.stream.Collectors; import static com.conveyal.datatools.manager.models.FeedRetrievalMethod.FETCHED_AUTOMATICALLY; +import static com.conveyal.datatools.manager.models.Project.hasPinnedDeployment; import static com.conveyal.datatools.manager.utils.StringUtils.getCleanName; import static com.mongodb.client.model.Filters.and; import static com.mongodb.client.model.Filters.eq; @@ -516,8 +517,7 @@ public FeedVersionDeployed retrieveDeployedFeedVersion() { if (deployedFeedVersionDefined) { return deployedFeedVersion; } - Project project = Persistence.projects.getById(projectId); - deployedFeedVersion = (project.pinnedDeploymentId != null && !project.pinnedDeploymentId.isEmpty()) + deployedFeedVersion = hasPinnedDeployment(projectId) ? FeedVersionDeployed.getFeedVersionFromPinnedDeployment(projectId, id) : FeedVersionDeployed.getFeedVersionFromLatestDeployment(projectId, id); deployedFeedVersionDefined = true; diff --git a/src/main/java/com/conveyal/datatools/manager/models/Project.java b/src/main/java/com/conveyal/datatools/manager/models/Project.java index 110618cb5..ddd2298a2 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/Project.java +++ b/src/main/java/com/conveyal/datatools/manager/models/Project.java @@ -2,13 +2,17 @@ import com.conveyal.datatools.manager.jobs.AutoDeployType; import com.conveyal.datatools.manager.persistence.Persistence; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.collect.Lists; +import com.mongodb.client.model.Projections; +import org.bson.Document; import org.bson.codecs.pojo.annotations.BsonIgnore; +import org.bson.conversions.Bson; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; import java.util.Collection; import java.util.Date; import java.util.HashSet; @@ -16,8 +20,11 @@ import java.util.Set; import java.util.stream.Collectors; +import static com.mongodb.client.model.Aggregates.match; +import static com.mongodb.client.model.Aggregates.project; import static com.mongodb.client.model.Filters.and; import static com.mongodb.client.model.Filters.eq; +import static com.mongodb.client.model.Filters.in; import static com.mongodb.client.model.Filters.or; /** @@ -191,4 +198,38 @@ public abstract static class ProjectWithOtpServers { @JsonProperty("otpServers") public abstract Collection availableOtpServers (); } + + @BsonIgnore + @JsonIgnore + public static boolean hasPinnedDeployment(String projectId) { + /* + db.getCollection('Project').aggregate([ + { + // Match provided project id. + $match: { + _id: "project-with-latest-deployment" + } + }, + { + $project: { + _id: 0, + pinnedDeploymentId: 1 + } + } + ]) + */ + List stages = Lists.newArrayList( + match( + in("_id", projectId) + ), + project(Projections.excludeId()), + project(Projections.include("pinnedDeploymentId")) + ); + Document project = Persistence + .getMongoDatabase() + .getCollection("Project") + .aggregate(stages) + .first(); + return !project.isEmpty(); + } } From 438a3c04addf017cedb32039afa26a288dbc7875 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 3 May 2023 15:55:53 +0100 Subject: [PATCH 02/40] refactor(Added feed source summary): All feed source summaries are available via a new end point --- .../controllers/api/FeedSourceController.java | 17 + .../datatools/manager/models/FeedSource.java | 66 --- .../manager/models/FeedSourceSummary.java | 527 ++++++++++++++++++ .../manager/models/FeedVersionDeployed.java | 239 -------- .../datatools/manager/models/Project.java | 19 + .../manager/persistence/Persistence.java | 14 +- .../api/FeedSourceControllerTest.java | 51 +- 7 files changed, 605 insertions(+), 328 deletions(-) create mode 100644 src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java delete mode 100644 src/main/java/com/conveyal/datatools/manager/models/FeedVersionDeployed.java diff --git a/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java b/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java index cc8807c87..badedda76 100644 --- a/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java +++ b/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java @@ -9,9 +9,11 @@ import com.conveyal.datatools.manager.extensions.ExternalFeedResource; import com.conveyal.datatools.manager.jobs.FetchSingleFeedJob; import com.conveyal.datatools.manager.jobs.NotifyUsersForSubscriptionJob; +import com.conveyal.datatools.manager.models.DeploymentSummary; import com.conveyal.datatools.manager.models.ExternalFeedSourceProperty; import com.conveyal.datatools.manager.models.FeedRetrievalMethod; import com.conveyal.datatools.manager.models.FeedSource; +import com.conveyal.datatools.manager.models.FeedSourceSummary; import com.conveyal.datatools.manager.models.JsonViews; import com.conveyal.datatools.manager.models.Project; import com.conveyal.datatools.manager.models.transform.NormalizeFieldTransformation; @@ -395,6 +397,20 @@ protected static FeedSource cleanFeedSourceForNonAdmins(FeedSource feedSource, b return feedSource; } + private static Collection getAllFeedSourceSummaries(Request req, Response res) { + Auth0UserProfile userProfile = req.attribute("user"); + String projectId = req.queryParams("projectId"); + Project project = Persistence.projects.getById(projectId); + if (project == null) { + logMessageAndHalt(req, 400, "Must provide valid projectId value."); + } + if (!userProfile.canAdministerProject(project)) { + logMessageAndHalt(req, 401, "User not authorized to view project feed sources."); + } + return project.retrieveFeedSourceSummaries(); + } + + // FIXME: use generic API controller and return JSON documents via BSON/Mongo public static void register (String apiPrefix) { get(apiPrefix + "secure/feedsource/:id", FeedSourceController::getFeedSource, json::write); @@ -404,5 +420,6 @@ public static void register (String apiPrefix) { put(apiPrefix + "secure/feedsource/:id/updateExternal", FeedSourceController::updateExternalFeedResource, json::write); delete(apiPrefix + "secure/feedsource/:id", FeedSourceController::deleteFeedSource, json::write); post(apiPrefix + "secure/feedsource/:id/fetch", FeedSourceController::fetch, json::write); + get(apiPrefix + "secure/feedsourceSummaries", FeedSourceController::getAllFeedSourceSummaries, json::write); } } diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java index f90ad15a7..db7eea7a9 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java @@ -38,7 +38,6 @@ import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; -import java.time.LocalDate; import java.util.ArrayList; import java.util.Collection; import java.util.Date; @@ -49,7 +48,6 @@ import java.util.stream.Collectors; import static com.conveyal.datatools.manager.models.FeedRetrievalMethod.FETCHED_AUTOMATICALLY; -import static com.conveyal.datatools.manager.models.Project.hasPinnedDeployment; import static com.conveyal.datatools.manager.utils.StringUtils.getCleanName; import static com.mongodb.client.model.Filters.and; import static com.mongodb.client.model.Filters.eq; @@ -460,70 +458,6 @@ public String latestVersionId() { return latest != null ? latest.id : null; } - /** - * The deployed feed version. - * This cannot be returned because of a circular reference between feed source and feed version. Instead, individual - * parameters (version id, start date and end date) are returned. - */ - @JsonIgnore - @BsonIgnore - private FeedVersionDeployed deployedFeedVersion; - - /** - * This value is set to true once an attempt has been made to get the deployed feed version. This prevents subsequent - * attempts by Json annotated properties to get a deployed feed version that is not available. - */ - @JsonIgnore - @BsonIgnore - private boolean deployedFeedVersionDefined; - - @JsonInclude(JsonInclude.Include.NON_NULL) - @JsonView(JsonViews.UserInterface.class) - @JsonProperty("deployedFeedVersionId") - @BsonIgnore - public String getDeployedFeedVersionId() { - deployedFeedVersion = retrieveDeployedFeedVersion(); - return deployedFeedVersion != null ? deployedFeedVersion.id : null; - } - - @JsonInclude(JsonInclude.Include.NON_NULL) - @JsonView(JsonViews.UserInterface.class) - @JsonProperty("deployedFeedVersionStartDate") - @BsonIgnore - public LocalDate getDeployedFeedVersionStartDate() { - deployedFeedVersion = retrieveDeployedFeedVersion(); - return deployedFeedVersion != null ? deployedFeedVersion.startDate : null; - } - - @JsonInclude(JsonInclude.Include.NON_NULL) - @JsonView(JsonViews.UserInterface.class) - @JsonProperty("deployedFeedVersionEndDate") - @BsonIgnore - public LocalDate getDeployedFeedVersionEndDate() { - deployedFeedVersion = retrieveDeployedFeedVersion(); - return deployedFeedVersion != null ? deployedFeedVersion.endDate : null; - } - - /** - * Get deployed feed version for this feed source. - * - * If a project has a "pinned" deployment, return the feed version from this pinned deployment. If it is not - * available return null and don't attempt to get the feed version from the latest deployment. - * - * If a project does not have a "pinned" deployment, return the latest deployment's feed versions for this feed - * source, if available. - */ - public FeedVersionDeployed retrieveDeployedFeedVersion() { - if (deployedFeedVersionDefined) { - return deployedFeedVersion; - } - deployedFeedVersion = hasPinnedDeployment(projectId) - ? FeedVersionDeployed.getFeedVersionFromPinnedDeployment(projectId, id) - : FeedVersionDeployed.getFeedVersionFromLatestDeployment(projectId, id); - deployedFeedVersionDefined = true; - return deployedFeedVersion; - } - /** * Number of {@link FeedVersion}s that exist for the feed source. */ diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java new file mode 100644 index 000000000..484760c32 --- /dev/null +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java @@ -0,0 +1,527 @@ +package com.conveyal.datatools.manager.models; + +import com.conveyal.datatools.editor.utils.JacksonSerializers; +import com.conveyal.datatools.manager.persistence.Persistence; +import com.conveyal.gtfs.validator.ValidationResult; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.google.common.collect.Lists; +import com.mongodb.client.model.Accumulators; +import com.mongodb.client.model.Projections; +import com.mongodb.client.model.Sorts; +import org.bson.Document; +import org.bson.conversions.Bson; + +import java.time.LocalDate; +import java.time.ZoneId; +import java.time.format.DateTimeFormatter; +import java.util.ArrayList; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static com.mongodb.client.model.Aggregates.group; +import static com.mongodb.client.model.Aggregates.limit; +import static com.mongodb.client.model.Aggregates.lookup; +import static com.mongodb.client.model.Aggregates.match; +import static com.mongodb.client.model.Aggregates.project; +import static com.mongodb.client.model.Aggregates.replaceRoot; +import static com.mongodb.client.model.Aggregates.sort; +import static com.mongodb.client.model.Aggregates.unwind; +import static com.mongodb.client.model.Filters.in; + +public class FeedSourceSummary { + private static final DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMdd"); + + public String id; + public String name; + public boolean deployable; + public boolean isPublic; + + @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) + @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) + public LocalDate lastUpdated; + + public String deployedFeedVersionId; + + @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) + @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) + public LocalDate deployedFeedVersionStartDate; + + @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) + @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) + public LocalDate deployedFeedVersionEndDate; + + public Integer deployedFeedVersionIssues; + + public String latestFeedVersionId; + + @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) + @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) + public LocalDate latestFeedVersionStartDate; + + @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) + @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) + public LocalDate latestFeedVersionEndDate; + + public Integer latestFeedVersionIssues; + + public FeedSourceSummary() { + } + + public FeedSourceSummary(Document feedSourceDocument) { + this.id = feedSourceDocument.getString("_id"); + this.name = feedSourceDocument.getString("name"); + this.deployable = feedSourceDocument.getBoolean("deployable"); + this.isPublic = feedSourceDocument.getBoolean("isPublic"); + // Convert to local date type for consistency. + this.lastUpdated = getLocalDateFromDate(feedSourceDocument.getDate("lastUpdated")); + } + + /** + * Set the appropriate feed version. For consistency, if no error count is available set the related number of + * issues to null. + */ + public void setFeedVersion(FeedVersionSummary feedVersionSummary, boolean isDeployed) { + if (feedVersionSummary != null) { + if (isDeployed) { + this.deployedFeedVersionId = feedVersionSummary.id; + this.deployedFeedVersionStartDate = feedVersionSummary.validationResult.firstCalendarDate; + this.deployedFeedVersionEndDate = feedVersionSummary.validationResult.lastCalendarDate; + this.deployedFeedVersionIssues = (feedVersionSummary.validationResult.errorCount == -1) + ? null + : feedVersionSummary.validationResult.errorCount; + } else { + this.latestFeedVersionId = feedVersionSummary.id; + this.latestFeedVersionStartDate = feedVersionSummary.validationResult.firstCalendarDate; + this.latestFeedVersionEndDate = feedVersionSummary.validationResult.lastCalendarDate; + this.latestFeedVersionIssues = (feedVersionSummary.validationResult.errorCount == -1) + ? null + : feedVersionSummary.validationResult.errorCount; + } + } + } + + /** + * Get all feed source summaries matching the project id. + */ + public static List getFeedSourceSummaries(String projectId) { + /* + db.getCollection('FeedSource').aggregate([ + { + // Match provided project id. + $match: { + projectId: "" + } + }, + { + $project: { + "_id": 1, + "name": 1, + "deployable": 1, + "isPublic": 1, + "lastUpdated": 1 + } + }, + { + $sort: { + "name": 1 + } + } + ]) + */ + List stages = Lists.newArrayList( + match( + in("projectId", projectId) + ), + project( + Projections.fields(Projections.include( + "_id", + "name", + "deployable", + "isPublic", + "lastUpdated") + ) + ), + sort(Sorts.ascending("name")) + ); + return extractFeedSourceSummaries(stages); + } + + /** + * Get the latest feed version from all feed sources for this project. + */ + public static Map getLatestFeedVersionForFeedSources(String projectId) { + /* + Note: To test this script: + 1) Comment out the call to tearDownDeployedFeedVersion() in FeedSourceControllerTest -> tearDown(). + 2) Run FeedSourceControllerTest to created required objects referenced here. + 3) Once complete, delete documents via MongoDB. + 4) Uncomment the call to tearDownDeployedFeedVersion() in FeedSourceControllerTest -> tearDown(). + 5) Re-run FeedSourceControllerTest to confirm deletion of objects. + + db.getCollection('FeedSource').aggregate([ + { + // Match provided project id. + $match: { + projectId: "project-with-latest-deployment" + } + }, + { + $lookup: { + from: "FeedVersion", + localField: "_id", + foreignField: "feedSourceId", + as: "feedVersions" + } + }, + { + $unwind: "$feedVersions" + }, + { + $group: { + _id: "$_id", + doc: { + $max: { + version: "$feedVersions.version", + feedVersionId: "$feedVersions._id", + firstCalendarDate: "$feedVersions.validationResult.firstCalendarDate", + lastCalendarDate: "$feedVersions.validationResult.lastCalendarDate", + issues: "$feedVersions.validationResult.errorCount" + } + } + } + } + ]) + */ + List stages = Lists.newArrayList( + match( + in("projectId", projectId) + ), + lookup("FeedVersion", "_id", "feedSourceId", "feedVersions"), + unwind("$feedVersions"), + group( + "$_id", + Accumulators.first("feedVersionId", "$feedVersions._id"), + Accumulators.first("firstCalendarDate", "$feedVersions.validationResult.firstCalendarDate"), + Accumulators.first("lastCalendarDate", "$feedVersions.validationResult.lastCalendarDate"), + Accumulators.first("errorCount", "$feedVersions.validationResult.errorCount") + ) + ); + return extractFeedVersionSummaries( + "FeedSource", + "feedVersionId", + "_id", + false, + stages); + } + + /** + * Get the deployed feed versions from the latest deployment for this project. + */ + public static Map getFeedVersionsFromLatestDeployment(String projectId) { + /* + Note: To test this script: + 1) Comment out the call to tearDownDeployedFeedVersion() in FeedSourceControllerTest -> tearDown(). + 2) Run FeedSourceControllerTest to created required objects referenced here. + 3) Once complete, delete documents via MongoDB. + 4) Uncomment the call to tearDownDeployedFeedVersion() in FeedSourceControllerTest -> tearDown(). + 5) Re-run FeedSourceControllerTest to confirm deletion of objects. + + db.getCollection('Project').aggregate([ + { + // Match provided project id. + $match: { + _id: "project-with-latest-deployment" + } + }, + { + // Get all deployments for this project. + $lookup:{ + from:"Deployment", + localField:"_id", + foreignField:"projectId", + as:"deployment" + } + }, + { + // Deconstruct deployments array to a document for each element. + $unwind: "$deployment" + }, + { + // Make the deployment documents the input/root document. + "$replaceRoot": { + "newRoot": "$deployment" + } + }, + { + // Sort descending. + $sort: { + lastUpdated : -1 + } + }, + { + // At this point we will have the latest deployment for a project. + $limit: 1 + }, + { + $lookup:{ + from:"FeedVersion", + localField:"feedVersionIds", + foreignField:"_id", + as:"feedVersions" + } + }, + { + // Deconstruct feedVersions array to a document for each element. + $unwind: "$feedVersions" + }, + { + // Make the feed version documents the input/root document. + "$replaceRoot": { + "newRoot": "$feedVersions" + } + }, + { + $project: { + "_id": 1, + "feedSourceId": 1, + "validationResult.firstCalendarDate": 1, + "validationResult.lastCalendarDate": 1, + "validationResult.errorCount": 1 + } + } + ]) + */ + List stages = Lists.newArrayList( + match( + in("_id", projectId) + ), + lookup("Deployment", "_id", "projectId", "deployments"), + unwind("$deployments"), + replaceRoot("$deployments"), + sort(Sorts.descending("lastUpdated")), + limit(1), + lookup("FeedVersion", "feedVersionIds", "_id", "feedVersions"), + unwind("$feedVersions"), + replaceRoot("$feedVersions"), + project( + Projections.fields(Projections.include( + "feedSourceId", + "validationResult.firstCalendarDate", + "validationResult.lastCalendarDate", + "validationResult.errorCount") + ) + ) + ); + return extractFeedVersionSummaries( + "Project", + "_id", + "feedSourceId", + true, + stages); + } + + /** + * Get the deployed feed version from the pinned deployment for this feed source. + */ + public static Map getFeedVersionsFromPinnedDeployment(String projectId) { + /* + Note: To test this script: + 1) Comment out the call to tearDownDeployedFeedVersion() in FeedSourceControllerTest -> tearDown(). + 2) Run FeedSourceControllerTest to created required objects referenced here. + 3) Once complete, delete documents via MongoDB. + 4) Uncomment the call to tearDownDeployedFeedVersion() in FeedSourceControllerTest -> tearDown(). + 5) Re-run FeedSourceControllerTest to confirm deletion of objects. + + db.getCollection('Project').aggregate([ + { + // Match provided project id. + $match: { + _id: "project-with-pinned-deployment" + } + }, + { + $project: { + pinnedDeploymentId: 1 + } + }, + { + $lookup:{ + from:"Deployment", + localField:"pinnedDeploymentId", + foreignField:"_id", + as:"deployment" + } + }, + { + $unwind: "$deployment" + }, + { + $lookup:{ + from:"FeedVersion", + localField:"deployment.feedVersionIds", + foreignField:"_id", + as:"feedVersions" + } + }, + { + // Deconstruct feedVersions array to a document for each element. + $unwind: "$feedVersions" + }, + { + // Make the feed version documents the input/root document. + "$replaceRoot": { + "newRoot": "$feedVersions" + } + }, + { + $project: { + "_id": 1, + "feedSourceId": 1, + "validationResult.firstCalendarDate": 1, + "validationResult.lastCalendarDate": 1, + "validationResult.errorCount": 1 + } + } + ]) + */ + + List stages = Lists.newArrayList( + match( + in("_id", projectId) + ), + project( + Projections.fields(Projections.include("pinnedDeploymentId")) + ), + lookup("Deployment", "pinnedDeploymentId", "_id", "deployment"), + unwind("$deployment"), + lookup("FeedVersion", "deployment.feedVersionIds", "_id", "feedVersions"), + unwind("$feedVersions"), + replaceRoot("$feedVersions"), + project( + Projections.fields(Projections.include( + "feedSourceId", + "validationResult.firstCalendarDate", + "validationResult.lastCalendarDate", + "validationResult.errorCount") + ) + ) + ); + return extractFeedVersionSummaries( + "Project", + "_id", + "feedSourceId", + true, + stages); + } + + + /** + * Produce a list of all feed source summaries for a project. + */ + private static List extractFeedSourceSummaries(List stages) { + List feedSourceSummaries = new ArrayList<>(); + for (Document feedSourceDocument : Persistence.getDocuments("FeedSource", stages)) { + feedSourceSummaries.add(new FeedSourceSummary(feedSourceDocument)); + } + return feedSourceSummaries; + } + + /** + * Extract feed version summaries from feed version documents. Each feed version is held against the matching feed + * source. + */ + private static Map extractFeedVersionSummaries( + String collection, + String feedVersionKey, + String feedSourceKey, + boolean hasChildValidationResultDocument, + List stages + ) { + Map feedVersionSummaries = new HashMap<>(); + for (Document feedVersionDocument : Persistence.getDocuments(collection, stages)) { + FeedVersionSummary feedVersionSummary = new FeedVersionSummary(); + feedVersionSummary.id = feedVersionDocument.getString(feedVersionKey); + feedVersionSummary.validationResult = getValidationResult(hasChildValidationResultDocument, feedVersionDocument); + feedVersionSummaries.put(feedVersionDocument.getString(feedSourceKey), feedVersionSummary); + } + return feedVersionSummaries; + } + + /** + * Build validation result from feed version document. + */ + private static ValidationResult getValidationResult(boolean hasChildValidationResultDocument, Document feedVersionDocument) { + ValidationResult validationResult = new ValidationResult(); + validationResult.errorCount = getValidationResultErrorCount(hasChildValidationResultDocument, feedVersionDocument); + validationResult.firstCalendarDate = getValidationResultDate(hasChildValidationResultDocument, feedVersionDocument, "firstCalendarDate"); + validationResult.lastCalendarDate = getValidationResultDate(hasChildValidationResultDocument, feedVersionDocument, "lastCalendarDate"); + return validationResult; + } + + private static LocalDate getValidationResultDate( + boolean hasChildValidationResultDocument, + Document feedVersionDocument, + String key + ) { + return (hasChildValidationResultDocument) + ? getDateFieldFromDocument(feedVersionDocument, key) + : getDateFromString(feedVersionDocument.getString(key)); + } + + /** + * Extract date value from validation result document. + */ + private static LocalDate getDateFieldFromDocument(Document document, String dateKey) { + Document validationResult = getDocumentChild(document, "validationResult"); + return (validationResult != null) + ? getDateFromString(validationResult.getString(dateKey)) + : null; + } + + /** + * Extract the error count from the parent document or child validation result document. If the error count is not + * available, return -1. + */ + private static int getValidationResultErrorCount(boolean hasChildValidationResultDocument, Document feedVersionDocument) { + int errorCount; + try { + errorCount = (hasChildValidationResultDocument) + ? getErrorCount(feedVersionDocument) + : feedVersionDocument.getInteger("errorCount"); + } catch (NullPointerException e) { + errorCount = -1; + } + return errorCount; + } + + /** + * Get the child validation result document and extract the error count from this. + */ + private static int getErrorCount(Document document) { + return getDocumentChild(document, "validationResult").getInteger("errorCount"); + } + + /** + * Extract child document matching provided name. + */ + private static Document getDocumentChild(Document document, String name) { + return (Document) document.get(name); + } + + /** + * Convert String date (if not null) into LocalDate. + */ + private static LocalDate getDateFromString(String date) { + return (date == null) ? null : LocalDate.parse(date, formatter); + } + + /** + * Convert Date object into LocalDate object. + */ + private static LocalDate getLocalDateFromDate(Date date) { + return date.toInstant().atZone(ZoneId.systemDefault()).toLocalDate(); + } +} \ No newline at end of file diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedVersionDeployed.java b/src/main/java/com/conveyal/datatools/manager/models/FeedVersionDeployed.java deleted file mode 100644 index 4fafc888d..000000000 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedVersionDeployed.java +++ /dev/null @@ -1,239 +0,0 @@ -package com.conveyal.datatools.manager.models; - -import com.conveyal.datatools.editor.utils.JacksonSerializers; -import com.conveyal.datatools.manager.persistence.Persistence; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import com.fasterxml.jackson.databind.annotation.JsonSerialize; -import com.google.common.collect.Lists; -import com.mongodb.client.model.Sorts; -import org.bson.Document; -import org.bson.conversions.Bson; - -import java.time.LocalDate; -import java.time.format.DateTimeFormatter; -import java.util.List; - -import static com.mongodb.client.model.Aggregates.limit; -import static com.mongodb.client.model.Aggregates.lookup; -import static com.mongodb.client.model.Aggregates.match; -import static com.mongodb.client.model.Aggregates.replaceRoot; -import static com.mongodb.client.model.Aggregates.sort; -import static com.mongodb.client.model.Aggregates.unwind; -import static com.mongodb.client.model.Filters.in; - -public class FeedVersionDeployed { - public String id; - - @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) - @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) - public LocalDate startDate; - - @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) - @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) - public LocalDate endDate; - - public FeedVersionDeployed() { - } - - public FeedVersionDeployed(Document feedVersionDocument) { - this.id = feedVersionDocument.getString("_id"); - DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMdd"); - Document validationResult = (Document) feedVersionDocument.get("validationResult"); - if (validationResult != null) { - String first = validationResult.getString("firstCalendarDate"); - String last = validationResult.getString("lastCalendarDate"); - this.startDate = (first == null) ? null : LocalDate.parse(first, formatter); - this.endDate = (last == null) ? null : LocalDate.parse(last, formatter); - } - } - - /** - * Get the deployed feed version from the pinned deployment for this feed source. - */ - public static FeedVersionDeployed getFeedVersionFromPinnedDeployment(String projectId, String feedSourceId) { - /* - Note: To test this script: - 1) Comment out the call to tearDownDeployedFeedVersion() in FeedSourceControllerTest -> tearDown(). - 2) Run FeedSourceControllerTest to created required objects referenced here. - 3) Once complete, delete documents via MongoDB. - 4) Uncomment the call to tearDownDeployedFeedVersion() in FeedSourceControllerTest -> tearDown(). - 5) Re-run FeedSourceControllerTest to confirm deletion of objects. - - db.getCollection('Project').aggregate([ - { - $match: { - _id: "project-with-pinned-deployment" - } - }, - { - $lookup:{ - from:"Deployment", - localField:"pinnedDeploymentId", - foreignField:"_id", - as:"deployment" - } - }, - { - $lookup:{ - from:"FeedVersion", - localField:"deployment.feedVersionIds", - foreignField:"_id", - as:"feedVersions" - } - }, - { - $unwind: "$feedVersions" - }, - { - "$replaceRoot": { - "newRoot": "$feedVersions" - } - }, - { - $match: { - feedSourceId: "feed-source-with-pinned-deployment-feed-version" - } - }, - { - $sort: { - lastUpdated : -1 - } - }, - { - $limit: 1 - } - ]) - */ - List stages = Lists.newArrayList( - match( - in("_id", projectId) - ), - lookup("Deployment", "pinnedDeploymentId", "_id", "deployment"), - lookup("FeedVersion", "deployment.feedVersionIds", "_id", "feedVersions"), - unwind("$feedVersions"), - replaceRoot("$feedVersions"), - match( - in("feedSourceId", feedSourceId) - ), - // If more than one feed version for a feed source is held against a deployment the latest is used. - sort(Sorts.descending("lastUpdated")), - limit(1) - ); - return getFeedVersionDeployed(stages); - } - - /** - * Get the deployed feed version from the latest deployment for this feed source. - */ - public static FeedVersionDeployed getFeedVersionFromLatestDeployment(String projectId, String feedSourceId) { - /* - Note: To test this script: - 1) Comment out the call to tearDownDeployedFeedVersion() in FeedSourceControllerTest -> tearDown(). - 2) Run FeedSourceControllerTest to created required objects referenced here. - 3) Once complete, delete documents via MongoDB. - 4) Uncomment the call to tearDownDeployedFeedVersion() in FeedSourceControllerTest -> tearDown(). - 5) Re-run FeedSourceControllerTest to confirm deletion of objects. - - db.getCollection('Project').aggregate([ - { - // Match provided project id. - $match: { - _id: "project-with-latest-deployment" - } - }, - { - // Get all deployments for this project. - $lookup:{ - from:"Deployment", - localField:"_id", - foreignField:"projectId", - as:"deployments" - } - }, - { - // Deconstruct deployments array to a document for each element. - $unwind: "$deployments" - }, - { - // Make the deployment documents the input/root document. - "$replaceRoot": { - "newRoot": "$deployments" - } - }, - { - // Sort descending. - $sort: { - lastUpdated : -1 - } - }, - { - // At this point we will have the latest deployment for a project. - $limit: 1 - }, - { - // Get all feed versions that have been deployed as part of the latest deployment. - $lookup:{ - from:"FeedVersion", - localField:"feedVersionIds", - foreignField:"_id", - as:"feedVersions" - } - }, - { - // Deconstruct feedVersions array to a document for each element. - $unwind: "$feedVersions" - }, - { - // Make the feed version documents the input/root document. - "$replaceRoot": { - "newRoot": "$feedVersions" - } - }, - { - // Match the required feed source. - $match: { - feedSourceId: "feed-source-with-latest-deployment-feed-version" - } - }, - { - $sort: { - lastUpdated : -1 - } - }, - { - // At this point we will have the latest feed version from the latest deployment for a feed source. - $limit: 1 - } - ]) - */ - List stages = Lists.newArrayList( - match( - in("_id", projectId) - ), - lookup("Deployment", "_id", "projectId", "deployments"), - unwind("$deployments"), - replaceRoot("$deployments"), - sort(Sorts.descending("lastUpdated")), - limit(1), - lookup("FeedVersion", "feedVersionIds", "_id", "feedVersions"), - unwind("$feedVersions"), - replaceRoot("$feedVersions"), - match( - in("feedSourceId", feedSourceId) - ), - // If more than one feed version for a feed source is held against a deployment the latest is used. - sort(Sorts.descending("lastUpdated")), - limit(1) - ); - return getFeedVersionDeployed(stages); - } - - private static FeedVersionDeployed getFeedVersionDeployed(List stages) { - Document feedVersionDocument = Persistence - .getMongoDatabase() - .getCollection("Project") - .aggregate(stages) - .first(); - return (feedVersionDocument == null) ? null : new FeedVersionDeployed(feedVersionDocument); - } -} diff --git a/src/main/java/com/conveyal/datatools/manager/models/Project.java b/src/main/java/com/conveyal/datatools/manager/models/Project.java index ddd2298a2..b868c6b45 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/Project.java +++ b/src/main/java/com/conveyal/datatools/manager/models/Project.java @@ -17,6 +17,7 @@ import java.util.Date; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -164,6 +165,24 @@ public Collection retrieveDeploymentSummaries() { .collect(Collectors.toList()); } + /** + * Get all feed source summaries for this project. + */ + public Collection retrieveFeedSourceSummaries() { + List feedSourceSummaries = FeedSourceSummary.getFeedSourceSummaries(id); + Map latestFeedVersionForFeedSources = FeedSourceSummary.getLatestFeedVersionForFeedSources(id); + Map deployedFeedVersions = FeedSourceSummary.getFeedVersionsFromPinnedDeployment(id); + if (deployedFeedVersions.isEmpty()) { + // No pinned deployments, instead, get the deployed feed versions from the latest deployment. + deployedFeedVersions = FeedSourceSummary.getFeedVersionsFromLatestDeployment(id); + } + for (FeedSourceSummary feedSourceSummary : feedSourceSummaries) { + feedSourceSummary.setFeedVersion(latestFeedVersionForFeedSources.get(feedSourceSummary.id), false); + feedSourceSummary.setFeedVersion(deployedFeedVersions.get(feedSourceSummary.id), true); + } + return feedSourceSummaries; + } + // TODO: Does this need to be returned with JSON API response public Organization retrieveOrganization() { if (organizationId != null) { diff --git a/src/main/java/com/conveyal/datatools/manager/persistence/Persistence.java b/src/main/java/com/conveyal/datatools/manager/persistence/Persistence.java index 2bd79ef6f..e215c6742 100644 --- a/src/main/java/com/conveyal/datatools/manager/persistence/Persistence.java +++ b/src/main/java/com/conveyal/datatools/manager/persistence/Persistence.java @@ -18,15 +18,20 @@ import com.conveyal.datatools.manager.models.Snapshot; import com.mongodb.ConnectionString; import com.mongodb.MongoClientSettings; +import com.mongodb.client.AggregateIterable; import com.mongodb.client.MongoClient; import com.mongodb.client.MongoClients; import com.mongodb.client.MongoDatabase; +import org.bson.Document; import org.bson.codecs.configuration.CodecRegistries; import org.bson.codecs.configuration.CodecRegistry; import org.bson.codecs.pojo.PojoCodecProvider; +import org.bson.conversions.Bson; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.List; + import static com.conveyal.datatools.manager.DataManager.getConfigPropertyAsText; /** @@ -135,5 +140,12 @@ public static void initialize () { public static MongoDatabase getMongoDatabase() { return mongoDatabase; } - + + /** + * Get all bespoke documents matching query. These documents are tailored to the query response and are not tied + * directly to a persistence type. + */ + public static AggregateIterable getDocuments(String collection, List stages) { + return getMongoDatabase().getCollection(collection).aggregate(stages); + } } diff --git a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java index e93b9baaf..a32474337 100644 --- a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java +++ b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java @@ -7,6 +7,7 @@ import com.conveyal.datatools.manager.models.Deployment; import com.conveyal.datatools.manager.models.FeedRetrievalMethod; import com.conveyal.datatools.manager.models.FeedSource; +import com.conveyal.datatools.manager.models.FeedSourceSummary; import com.conveyal.datatools.manager.models.FeedVersion; import com.conveyal.datatools.manager.models.FetchFrequency; import com.conveyal.datatools.manager.models.Label; @@ -323,46 +324,51 @@ public void createFeedSourceWithLabels() { void canRetrieveDeployedFeedVersionFromLatestDeployment() throws IOException { SimpleHttpResponse response = TestUtils.makeRequest( String.format( - "/api/manager/secure/feedsource/%s", - feedSourceWithLatestDeploymentFeedVersion.id + "/api/manager/secure/feedsourceSummaries?projectId=%s", + feedSourceWithLatestDeploymentFeedVersion.projectId ), null, HttpUtils.REQUEST_METHOD.GET ); assertEquals(OK_200, response.status); - FeedSource feedSource = - JsonUtil.getPOJOFromResponse( - response, - FeedSource.class + + List feedSourceSummaries = + JsonUtil.getPOJOFromJSONAsList( + JsonUtil.getJsonNodeFromResponse(response), + FeedSourceSummary.class ); - assertNotNull(feedSource); - assertEquals(feedSourceWithLatestDeploymentFeedVersion.id, feedSource.id); - assertEquals(feedVersionFromLatestDeployment.id, feedSource.getDeployedFeedVersionId()); - assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSource.getDeployedFeedVersionStartDate()); - assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSource.getDeployedFeedVersionEndDate()); + + assertNotNull(feedSourceSummaries); + assertEquals(feedSourceWithLatestDeploymentFeedVersion.id, feedSourceSummaries.get(0).id); + assertEquals(feedVersionFromLatestDeployment.id, feedSourceSummaries.get(0).deployedFeedVersionId); + assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); + assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); + assertEquals(feedVersionFromLatestDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).deployedFeedVersionIssues); } @Test void canRetrieveDeployedFeedVersionFromPinnedDeployment() throws IOException { SimpleHttpResponse response = TestUtils.makeRequest( String.format( - "/api/manager/secure/feedsource/%s", - feedSourceWithPinnedDeploymentFeedVersion.id + "/api/manager/secure/feedsourceSummaries?projectId=%s", + feedSourceWithPinnedDeploymentFeedVersion.projectId ), null, HttpUtils.REQUEST_METHOD.GET ); assertEquals(OK_200, response.status); - FeedSource feedSource = - JsonUtil.getPOJOFromResponse( - response, - FeedSource.class + + List feedSourceSummaries = + JsonUtil.getPOJOFromJSONAsList( + JsonUtil.getJsonNodeFromResponse(response), + FeedSourceSummary.class ); - assertNotNull(feedSource); - assertEquals(feedSourceWithPinnedDeploymentFeedVersion.id, feedSource.id); - assertEquals(feedVersionFromPinnedDeployment.id, feedSource.getDeployedFeedVersionId()); - assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSource.getDeployedFeedVersionEndDate()); - assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSource.getDeployedFeedVersionStartDate()); + assertNotNull(feedSourceSummaries); + assertEquals(feedSourceWithPinnedDeploymentFeedVersion.id, feedSourceSummaries.get(0).id); + assertEquals(feedVersionFromPinnedDeployment.id, feedSourceSummaries.get(0).deployedFeedVersionId); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).deployedFeedVersionIssues); } private static FeedSource createFeedSource(String name, URL url, Project project) { @@ -421,6 +427,7 @@ private static FeedVersion createFeedVersion(String id, String feedSourceId, Loc ValidationResult validationResult = new ValidationResult(); validationResult.firstCalendarDate = startDate; validationResult.lastCalendarDate = endDate; + validationResult.errorCount = 5 + (int)(Math.random() * ((1000 - 5) + 1)); feedVersion.validationResult = validationResult; Persistence.feedVersions.create(feedVersion); return feedVersion; From 62eb590cc32e95baa65cac54a515ebb30b2d49c9 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 20 Jun 2023 11:52:09 +0100 Subject: [PATCH 03/40] refactor(Update to include label ids): Updated to get label ids from DB for a feed source --- .../manager/models/FeedSourceSummary.java | 9 +++++-- .../api/FeedSourceControllerTest.java | 27 +++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java index 484760c32..ccce4a0da 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java @@ -43,6 +43,8 @@ public class FeedSourceSummary { @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) public LocalDate lastUpdated; + public List labelIds = new ArrayList<>(); + public String deployedFeedVersionId; @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) @@ -75,6 +77,7 @@ public FeedSourceSummary(Document feedSourceDocument) { this.name = feedSourceDocument.getString("name"); this.deployable = feedSourceDocument.getBoolean("deployable"); this.isPublic = feedSourceDocument.getBoolean("isPublic"); + this.labelIds = feedSourceDocument.getList("labelIds", String.class); // Convert to local date type for consistency. this.lastUpdated = getLocalDateFromDate(feedSourceDocument.getDate("lastUpdated")); } @@ -121,7 +124,8 @@ public static List getFeedSourceSummaries(String projectId) { "name": 1, "deployable": 1, "isPublic": 1, - "lastUpdated": 1 + "lastUpdated": 1, + "labelIds": 1 } }, { @@ -141,7 +145,8 @@ public static List getFeedSourceSummaries(String projectId) { "name", "deployable", "isPublic", - "lastUpdated") + "lastUpdated", + "labelIds") ) ), sort(Sorts.ascending("name")) diff --git a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java index a32474337..217b05586 100644 --- a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java +++ b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java @@ -91,7 +91,8 @@ public static void setUp() throws IOException { "FeedSource", null, projectWithLatestDeployment, - true + true, + List.of("labelOne", "labelTwo") ); LocalDate deployedSuperseded = LocalDate.of(2020, Month.MARCH, 12); LocalDate deployedEndDate = LocalDate.of(2021, Month.MARCH, 12); @@ -124,7 +125,8 @@ public static void setUp() throws IOException { "FeedSourceWithPinnedFeedVersion", null, projectWithPinnedDeployment, - true + true, + List.of("labelOne", "labelTwo") ); feedVersionFromPinnedDeployment = createFeedVersion( "feed-version-from-pinned-deployment", @@ -340,10 +342,15 @@ void canRetrieveDeployedFeedVersionFromLatestDeployment() throws IOException { assertNotNull(feedSourceSummaries); assertEquals(feedSourceWithLatestDeploymentFeedVersion.id, feedSourceSummaries.get(0).id); + assertEquals(feedSourceWithLatestDeploymentFeedVersion.labelIds, feedSourceSummaries.get(0).labelIds); assertEquals(feedVersionFromLatestDeployment.id, feedSourceSummaries.get(0).deployedFeedVersionId); assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); assertEquals(feedVersionFromLatestDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).deployedFeedVersionIssues); + assertEquals(feedVersionFromLatestDeployment.id, feedSourceSummaries.get(0).latestFeedVersionId); + assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSourceSummaries.get(0).latestFeedVersionStartDate); + assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSourceSummaries.get(0).latestFeedVersionEndDate); + assertEquals(feedVersionFromLatestDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).latestFeedVersionIssues); } @Test @@ -365,10 +372,15 @@ void canRetrieveDeployedFeedVersionFromPinnedDeployment() throws IOException { ); assertNotNull(feedSourceSummaries); assertEquals(feedSourceWithPinnedDeploymentFeedVersion.id, feedSourceSummaries.get(0).id); + assertEquals(feedSourceWithPinnedDeploymentFeedVersion.labelIds, feedSourceSummaries.get(0).labelIds); assertEquals(feedVersionFromPinnedDeployment.id, feedSourceSummaries.get(0).deployedFeedVersionId); assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); assertEquals(feedVersionFromPinnedDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).deployedFeedVersionIssues); + assertEquals(feedVersionFromPinnedDeployment.id, feedSourceSummaries.get(0).latestFeedVersionId); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSourceSummaries.get(0).latestFeedVersionStartDate); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSourceSummaries.get(0).latestFeedVersionEndDate); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).latestFeedVersionIssues); } private static FeedSource createFeedSource(String name, URL url, Project project) { @@ -379,6 +391,16 @@ private static FeedSource createFeedSource(String name, URL url, Project project * Helper method to create feed source. */ private static FeedSource createFeedSource(String id, String name, URL url, Project project, boolean persist) { + return createFeedSource(id, name, url, project, persist, null); + } + private static FeedSource createFeedSource( + String id, + String name, + URL url, + Project project, + boolean persist, + List labels + ) { FeedSource feedSource = new FeedSource(); if (id != null) feedSource.id = id; feedSource.fetchFrequency = FetchFrequency.MINUTES; @@ -388,6 +410,7 @@ private static FeedSource createFeedSource(String id, String name, URL url, Proj feedSource.projectId = project.id; feedSource.retrievalMethod = FeedRetrievalMethod.FETCHED_AUTOMATICALLY; feedSource.url = url; + if (labels != null) feedSource.labelIds = labels; if (persist) Persistence.feedSources.create(feedSource); return feedSource; } From 2024a9ac7b7c5d2a6aac9e30c45b26659c9d8f4c Mon Sep 17 00:00:00 2001 From: miles-grant-ibi Date: Wed, 26 Jul 2023 18:28:34 +0200 Subject: [PATCH 04/40] fix: add Shared Stops Validator --- .../jobs/validation/SharedStopsValidator.java | 41 +++++++++++++++++++ .../datatools/manager/models/FeedVersion.java | 10 ++++- 2 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 src/main/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidator.java 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 new file mode 100644 index 000000000..de94a5667 --- /dev/null +++ b/src/main/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidator.java @@ -0,0 +1,41 @@ +package com.conveyal.datatools.manager.jobs.validation; + +import com.conveyal.datatools.manager.models.Project; +import com.conveyal.gtfs.error.NewGTFSErrorType; +import com.conveyal.gtfs.error.SQLErrorStorage; +import com.conveyal.gtfs.loader.Feed; +import com.conveyal.gtfs.model.Route; +import com.conveyal.gtfs.validator.FeedValidator; + + +public class SharedStopsValidator extends FeedValidator { + Feed feed; + Project project; + + public SharedStopsValidator(Project project) { + super(null, null); + this.project = project; + } + + // This method can only be run on a SharedStopsValidator that has been set up with a project only + public SharedStopsValidator buildSharedStopsValidator(Feed feed, SQLErrorStorage errorStorage) { + if (this.project == null) { + throw new RuntimeException("buildSharedStopsValidator can only be called with a project already set!"); + } + return new SharedStopsValidator(feed, errorStorage, this.project); + } + public SharedStopsValidator(Feed feed, SQLErrorStorage errorStorage, Project project) { + super(feed, errorStorage); + this.feed = feed; + this.project = project; + } + + @Override + public void validate() { + + for (Route route : feed.routes) { + registerError(route, NewGTFSErrorType.MULTIPLE_SHARED_STOPS_GROUPS); + } + + } +} diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java index 523ee181a..e160b1a9c 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java @@ -6,6 +6,7 @@ import com.conveyal.datatools.manager.jobs.ValidateFeedJob; import com.conveyal.datatools.manager.jobs.ValidateMobilityDataFeedJob; import com.conveyal.datatools.manager.jobs.validation.RouteTypeValidatorBuilder; +import com.conveyal.datatools.manager.jobs.validation.SharedStopsValidator; import com.conveyal.datatools.manager.persistence.FeedStore; import com.conveyal.datatools.manager.persistence.Persistence; import com.conveyal.datatools.manager.utils.HashUtils; @@ -370,8 +371,11 @@ public void validate(MonitorableJob.Status status) { MTCValidator::new ); } else { + FeedSource fs = Persistence.feedSources.getById(this.feedSourceId); + SharedStopsValidator ssv = new SharedStopsValidator(fs.retrieveProject()); + validationResult = GTFS.validate(feedLoadResult.uniqueIdentifier, DataManager.GTFS_DATA_SOURCE, - RouteTypeValidatorBuilder::buildRouteValidator + RouteTypeValidatorBuilder::buildRouteValidator, ssv::buildSharedStopsValidator ); } } catch (Exception e) { @@ -489,7 +493,9 @@ public boolean hasBlockingIssuesForPublishing() { NewGTFSErrorType.SERVICE_WITHOUT_DAYS_OF_WEEK, NewGTFSErrorType.TABLE_MISSING_COLUMN_HEADERS, NewGTFSErrorType.TABLE_IN_SUBDIRECTORY, - NewGTFSErrorType.WRONG_NUMBER_OF_FIELDS + NewGTFSErrorType.WRONG_NUMBER_OF_FIELDS, + NewGTFSErrorType.MULTIPLE_SHARED_STOPS_GROUPS, + NewGTFSErrorType.SHARED_STOP_GROUP_MUTLIPLE_PRIMARY_STOPS )); } From 75c1904c2f8016606b6d144a2055d365b39e0a5a Mon Sep 17 00:00:00 2001 From: miles-grant-ibi Date: Thu, 27 Jul 2023 01:18:56 +0200 Subject: [PATCH 05/40] refactor: shared stops use all error types --- .../jobs/validation/SharedStopsValidator.java | 17 +++++++++++++++-- .../datatools/manager/models/Project.java | 4 ++++ 2 files changed, 19 insertions(+), 2 deletions(-) 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 de94a5667..d9ce9a9a6 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 @@ -1,10 +1,15 @@ package com.conveyal.datatools.manager.jobs.validation; import com.conveyal.datatools.manager.models.Project; +import com.conveyal.gtfs.error.GTFSError; +import com.conveyal.gtfs.error.NewGTFSError; import com.conveyal.gtfs.error.NewGTFSErrorType; import com.conveyal.gtfs.error.SQLErrorStorage; import com.conveyal.gtfs.loader.Feed; +import com.conveyal.gtfs.loader.Table; +import com.conveyal.gtfs.model.Entity; import com.conveyal.gtfs.model.Route; +import com.conveyal.gtfs.model.Stop; import com.conveyal.gtfs.validator.FeedValidator; @@ -33,8 +38,16 @@ public SharedStopsValidator(Feed feed, SQLErrorStorage errorStorage, Project pro @Override public void validate() { - for (Route route : feed.routes) { - registerError(route, NewGTFSErrorType.MULTIPLE_SHARED_STOPS_GROUPS); + String config = project.sharedStopsConfig; + + + for (Stop stop : feed.stops) { + registerError(stop, NewGTFSErrorType.MULTIPLE_SHARED_STOPS_GROUPS); + } + if (config == null) { + registerError(NewGTFSError.forFeed(NewGTFSErrorType.SHARED_STOP_GROUP_ENTITY_DOES_NOT_EXIST, "uh oh")); + registerError(NewGTFSError.forFeed(NewGTFSErrorType.SHARED_STOP_GROUP_MUTLIPLE_PRIMARY_STOPS, "good god")); + } } diff --git a/src/main/java/com/conveyal/datatools/manager/models/Project.java b/src/main/java/com/conveyal/datatools/manager/models/Project.java index 110618cb5..15654bcd2 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/Project.java +++ b/src/main/java/com/conveyal/datatools/manager/models/Project.java @@ -48,6 +48,10 @@ public class Project extends Model { /** Last successful auto deploy. **/ public Date lastAutoDeploy; + /** CSV formatted shared stops config. **/ + public String sharedStopsConfig; + + /** * A list of servers that are available to deploy project feeds/OSM to. This includes servers assigned to this * project as well as those that belong to no project. From 9613395beeda98e8277b9be6fe8ee2cecf694ce4 Mon Sep 17 00:00:00 2001 From: miles-grant-ibi Date: Fri, 28 Jul 2023 13:57:15 +0200 Subject: [PATCH 06/40] fix: initial shared stops validation --- .../jobs/validation/SharedStopsValidator.java | 74 ++++++++++++++++--- 1 file changed, 65 insertions(+), 9 deletions(-) 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 d9ce9a9a6..c4e7ec86e 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 @@ -1,16 +1,20 @@ package com.conveyal.datatools.manager.jobs.validation; import com.conveyal.datatools.manager.models.Project; -import com.conveyal.gtfs.error.GTFSError; import com.conveyal.gtfs.error.NewGTFSError; import com.conveyal.gtfs.error.NewGTFSErrorType; import com.conveyal.gtfs.error.SQLErrorStorage; import com.conveyal.gtfs.loader.Feed; -import com.conveyal.gtfs.loader.Table; -import com.conveyal.gtfs.model.Entity; -import com.conveyal.gtfs.model.Route; import com.conveyal.gtfs.model.Stop; import com.conveyal.gtfs.validator.FeedValidator; +import com.csvreader.CsvReader; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Set; public class SharedStopsValidator extends FeedValidator { @@ -37,18 +41,70 @@ public SharedStopsValidator(Feed feed, SQLErrorStorage errorStorage, Project pro @Override public void validate() { - String config = project.sharedStopsConfig; + if (config == null) { + return; + } + CsvReader configReader = CsvReader.parse(config); + + int STOP_GROUP_ID_INDEX = 0; + int STOP_ID_INDEX = 2; + int IS_PRIMARY_INDEX = 3; + + // Build list of stop Ids. + List stopIds = new ArrayList<>(); + List stops = new ArrayList<>(); for (Stop stop : feed.stops) { - registerError(stop, NewGTFSErrorType.MULTIPLE_SHARED_STOPS_GROUPS); + stops.add(stop); + stopIds.add(stop.stop_id); } - if (config == null) { - registerError(NewGTFSError.forFeed(NewGTFSErrorType.SHARED_STOP_GROUP_ENTITY_DOES_NOT_EXIST, "uh oh")); - registerError(NewGTFSError.forFeed(NewGTFSErrorType.SHARED_STOP_GROUP_MUTLIPLE_PRIMARY_STOPS, "good god")); + // Initialize hashmaps to hold duplication info + HashMap> stopGroupStops = new HashMap<>(); + HashSet stopGroupsWithPrimaryStops = new HashSet<>(); + + try { + while (configReader.readRecord()) { + String stopGroupId = configReader.get(STOP_GROUP_ID_INDEX); + String stopId = configReader.get(STOP_ID_INDEX); + + if (stopId.equals("stop_id")) { + // Swallow header row + continue; + } + + if (!stopGroupStops.containsKey(stopGroupId)) { + stopGroupStops.put(stopGroupId, new HashSet<>()); + } + Set seenStopIds = stopGroupStops.get(stopGroupId); + + // Check for SS_01 (stop id appearing in multiple stop groups) + if (seenStopIds.contains(stopId)) { + registerError(stops.stream().filter(stop -> stop.stop_id.equals(stopId)).findFirst().orElse(stops.get(0)), NewGTFSErrorType.MULTIPLE_SHARED_STOPS_GROUPS); + } else { + seenStopIds.add(stopId); + } + + // Check for SS_02 (multiple primary stops per stop group) + if (stopGroupsWithPrimaryStops.contains(stopGroupId)) { + registerError(NewGTFSError.forFeed(NewGTFSErrorType.SHARED_STOP_GROUP_MUTLIPLE_PRIMARY_STOPS, stopGroupId)); + } else if (configReader.get(IS_PRIMARY_INDEX).equals("true")) { + stopGroupsWithPrimaryStops.add(stopGroupId); + } + + // Check for SS_03 (stop_id referenced doesn't exist) + // TODO: CHECK FEED ID + if (!stopIds.contains(stopId)) { + registerError(NewGTFSError.forFeed(NewGTFSErrorType.SHARED_STOP_GROUP_ENTITY_DOES_NOT_EXIST, stopId)); + } + } + } catch (IOException e) { + // TODO Fail gracefully + throw new RuntimeException(e); } + } } From 461deefd2401bc845bfda407c5a404ea97eb75ad Mon Sep 17 00:00:00 2001 From: miles-grant-ibi Date: Thu, 3 Aug 2023 15:51:36 -0400 Subject: [PATCH 07/40] refactor: include shared stops file in deployment bundle --- .../com/conveyal/datatools/manager/jobs/DeployJob.java | 4 ++++ .../manager/jobs/validation/SharedStopsValidator.java | 2 +- .../conveyal/datatools/manager/models/Deployment.java | 10 ++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/datatools/manager/jobs/DeployJob.java b/src/main/java/com/conveyal/datatools/manager/jobs/DeployJob.java index 4083dfc72..162f28525 100644 --- a/src/main/java/com/conveyal/datatools/manager/jobs/DeployJob.java +++ b/src/main/java/com/conveyal/datatools/manager/jobs/DeployJob.java @@ -1292,6 +1292,10 @@ public OtpRunnerManifest createAndUploadManifestAndConfigs(boolean graphAlreadyB } } + // upload shared stops file + String sharedStopsConfig = this.deployment.parentProject().sharedStopsConfig; + if (sharedStopsConfig != null) addStringContentsAsBaseFolderDownload(manifest, "shared_stops.csv", sharedStopsConfig); + // upload otp-runner manifest to s3 try { ObjectMapper mapper = new ObjectMapper(); 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 c4e7ec86e..2795aa726 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 @@ -95,7 +95,7 @@ public void validate() { } // Check for SS_03 (stop_id referenced doesn't exist) - // TODO: CHECK FEED ID + // TODO: CHECK FEED ID (adjust the pre-build constructor to include feed_id) if (!stopIds.contains(stopId)) { registerError(NewGTFSError.forFeed(NewGTFSErrorType.SHARED_STOP_GROUP_ENTITY_DOES_NOT_EXIST, stopId)); } diff --git a/src/main/java/com/conveyal/datatools/manager/models/Deployment.java b/src/main/java/com/conveyal/datatools/manager/models/Deployment.java index b43092f87..04731841a 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/Deployment.java +++ b/src/main/java/com/conveyal/datatools/manager/models/Deployment.java @@ -400,6 +400,16 @@ public void dump (File output, boolean includeManifest, boolean includeOsm, bool out.closeEntry(); } } + + // Include shared_stops.csv, if present + if (parentProject().sharedStopsConfig != null) { + byte[] sharedStopsConfigAsBytes = parentProject().sharedStopsConfig.getBytes(StandardCharsets.UTF_8); + ZipEntry sharedStopsEntry = new ZipEntry("shared_stops.csv"); + out.putNextEntry(sharedStopsEntry); + out.write(sharedStopsConfigAsBytes); + out.closeEntry(); + } + // Finally close the zip output stream. The dump file is now complete. out.close(); } From 53fe24363bdfeaad4ab2bc232bdb38c5bae84407 Mon Sep 17 00:00:00 2001 From: miles-grant-ibi Date: Fri, 11 Aug 2023 11:20:48 -0400 Subject: [PATCH 08/40] refactor: update gtfs-lib --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a265b5442..ff0612eb5 100644 --- a/pom.xml +++ b/pom.xml @@ -268,7 +268,7 @@ com.github.conveyal gtfs-lib - f2ceb59 + 9837b6499796a0eeb5de76314e6a1f3125d695fb From f6deff34def8901ddb4e544ece9a53d122025c5b Mon Sep 17 00:00:00 2001 From: miles-grant-ibi Date: Fri, 11 Aug 2023 12:25:55 -0400 Subject: [PATCH 09/40] refactor: address pr feedback --- .../jobs/validation/SharedStopsValidator.java | 30 ++++++++++++------- .../datatools/manager/models/FeedVersion.java | 7 +++-- 2 files changed, 25 insertions(+), 12 deletions(-) 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 2795aa726..41084d0e5 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 @@ -1,5 +1,6 @@ package com.conveyal.datatools.manager.jobs.validation; +import com.conveyal.datatools.common.utils.aws.S3Utils; import com.conveyal.datatools.manager.models.Project; import com.conveyal.gtfs.error.NewGTFSError; import com.conveyal.gtfs.error.NewGTFSErrorType; @@ -8,6 +9,8 @@ import com.conveyal.gtfs.model.Stop; import com.conveyal.gtfs.validator.FeedValidator; import com.csvreader.CsvReader; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.ArrayList; @@ -18,6 +21,8 @@ public class SharedStopsValidator extends FeedValidator { + private static final Logger LOG = LoggerFactory.getLogger(S3Utils.class); + Feed feed; Project project; @@ -29,7 +34,7 @@ public SharedStopsValidator(Project project) { // This method can only be run on a SharedStopsValidator that has been set up with a project only public SharedStopsValidator buildSharedStopsValidator(Feed feed, SQLErrorStorage errorStorage) { if (this.project == null) { - throw new RuntimeException("buildSharedStopsValidator can only be called with a project already set!"); + throw new RuntimeException("Shared stops validator can not be called because no project has been set!"); } return new SharedStopsValidator(feed, errorStorage, this.project); } @@ -48,9 +53,9 @@ public void validate() { CsvReader configReader = CsvReader.parse(config); - int STOP_GROUP_ID_INDEX = 0; - int STOP_ID_INDEX = 2; - int IS_PRIMARY_INDEX = 3; + final int STOP_GROUP_ID_INDEX = 0; + final int STOP_ID_INDEX = 2; + final int IS_PRIMARY_INDEX = 3; // Build list of stop Ids. List stopIds = new ArrayList<>(); @@ -75,14 +80,17 @@ public void validate() { continue; } - if (!stopGroupStops.containsKey(stopGroupId)) { - stopGroupStops.put(stopGroupId, new HashSet<>()); - } + stopGroupStops.putIfAbsent(stopGroupId, new HashSet<>()); Set seenStopIds = stopGroupStops.get(stopGroupId); // Check for SS_01 (stop id appearing in multiple stop groups) if (seenStopIds.contains(stopId)) { - registerError(stops.stream().filter(stop -> stop.stop_id.equals(stopId)).findFirst().orElse(stops.get(0)), NewGTFSErrorType.MULTIPLE_SHARED_STOPS_GROUPS); + registerError(stops + .stream() + .filter(stop -> stop.stop_id.equals(stopId)) + .findFirst() + .orElse(stops.get(0)), NewGTFSErrorType.MULTIPLE_SHARED_STOPS_GROUPS + ); } else { seenStopIds.add(stopId); } @@ -101,8 +109,10 @@ public void validate() { } } } catch (IOException e) { - // TODO Fail gracefully - throw new RuntimeException(e); + LOG.error(e.toString()); + } + finally { + configReader.close(); } diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java index e160b1a9c..69edac06f 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java @@ -374,8 +374,11 @@ public void validate(MonitorableJob.Status status) { FeedSource fs = Persistence.feedSources.getById(this.feedSourceId); SharedStopsValidator ssv = new SharedStopsValidator(fs.retrieveProject()); - validationResult = GTFS.validate(feedLoadResult.uniqueIdentifier, DataManager.GTFS_DATA_SOURCE, - RouteTypeValidatorBuilder::buildRouteValidator, ssv::buildSharedStopsValidator + validationResult = GTFS.validate( + feedLoadResult.uniqueIdentifier, + DataManager.GTFS_DATA_SOURCE, + RouteTypeValidatorBuilder::buildRouteValidator, + ssv::buildSharedStopsValidator ); } } catch (Exception e) { From 268b949ba5a8a22b43dc9d59795848696ec9f898 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Mon, 14 Aug 2023 12:08:54 +0100 Subject: [PATCH 10/40] refactor(Added project id to feed source summary): --- .../datatools/manager/models/FeedSourceSummary.java | 12 ++++++++---- .../controllers/api/FeedSourceControllerTest.java | 2 ++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java index ccce4a0da..1f380d541 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java @@ -34,7 +34,10 @@ public class FeedSourceSummary { private static final DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMdd"); + public String projectId; + public String id; + public String name; public boolean deployable; public boolean isPublic; @@ -72,7 +75,8 @@ public class FeedSourceSummary { public FeedSourceSummary() { } - public FeedSourceSummary(Document feedSourceDocument) { + public FeedSourceSummary(String projectId, Document feedSourceDocument) { + this.projectId = projectId; this.id = feedSourceDocument.getString("_id"); this.name = feedSourceDocument.getString("name"); this.deployable = feedSourceDocument.getBoolean("deployable"); @@ -151,7 +155,7 @@ public static List getFeedSourceSummaries(String projectId) { ), sort(Sorts.ascending("name")) ); - return extractFeedSourceSummaries(stages); + return extractFeedSourceSummaries(projectId, stages); } /** @@ -426,10 +430,10 @@ public static Map getFeedVersionsFromPinnedDeploymen /** * Produce a list of all feed source summaries for a project. */ - private static List extractFeedSourceSummaries(List stages) { + private static List extractFeedSourceSummaries(String projectId, List stages) { List feedSourceSummaries = new ArrayList<>(); for (Document feedSourceDocument : Persistence.getDocuments("FeedSource", stages)) { - feedSourceSummaries.add(new FeedSourceSummary(feedSourceDocument)); + feedSourceSummaries.add(new FeedSourceSummary(projectId, feedSourceDocument)); } return feedSourceSummaries; } diff --git a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java index 217b05586..1f58ea1ce 100644 --- a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java +++ b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java @@ -342,6 +342,7 @@ void canRetrieveDeployedFeedVersionFromLatestDeployment() throws IOException { assertNotNull(feedSourceSummaries); assertEquals(feedSourceWithLatestDeploymentFeedVersion.id, feedSourceSummaries.get(0).id); + assertEquals(feedSourceWithLatestDeploymentFeedVersion.projectId, feedSourceSummaries.get(0).projectId); assertEquals(feedSourceWithLatestDeploymentFeedVersion.labelIds, feedSourceSummaries.get(0).labelIds); assertEquals(feedVersionFromLatestDeployment.id, feedSourceSummaries.get(0).deployedFeedVersionId); assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); @@ -372,6 +373,7 @@ void canRetrieveDeployedFeedVersionFromPinnedDeployment() throws IOException { ); assertNotNull(feedSourceSummaries); assertEquals(feedSourceWithPinnedDeploymentFeedVersion.id, feedSourceSummaries.get(0).id); + assertEquals(feedSourceWithPinnedDeploymentFeedVersion.projectId, feedSourceSummaries.get(0).projectId); assertEquals(feedSourceWithPinnedDeploymentFeedVersion.labelIds, feedSourceSummaries.get(0).labelIds); assertEquals(feedVersionFromPinnedDeployment.id, feedSourceSummaries.get(0).deployedFeedVersionId); assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); From 1f580545d80a57f456ee337492f5b5fb6a3d05d5 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Mon, 14 Aug 2023 12:19:27 +0100 Subject: [PATCH 11/40] refactor(pom.xml): Updated gtfs-lib version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a265b5442..1566ccefa 100644 --- a/pom.xml +++ b/pom.xml @@ -268,7 +268,7 @@ com.github.conveyal gtfs-lib - f2ceb59 + f2ceb59027 From 2cc5aab3ea2126d67d5c650920b4e76a11ef732d Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 16 Aug 2023 10:39:30 +0100 Subject: [PATCH 12/40] refactor(pom.xml): Addressed short hash for gtfs lib Jitpack now requires 10 instead of 7 characters --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a265b5442..1566ccefa 100644 --- a/pom.xml +++ b/pom.xml @@ -268,7 +268,7 @@ com.github.conveyal gtfs-lib - f2ceb59 + f2ceb59027 From d43f40afa415fc4a112395f1ee0fea23ee6da06e Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 16 Aug 2023 11:28:22 +0100 Subject: [PATCH 13/40] refactor(Disable auth for tests): --- pom.xml | 2 +- .../manager/gtfsplus/GtfsPlusValidationTest.java | 3 +++ .../datatools/manager/jobs/ArbitraryTransformJobTest.java | 3 +++ .../datatools/manager/jobs/AutoPublishJobTest.java | 3 +++ .../manager/jobs/DeploymentGisExportJobTest.java | 8 ++++++++ .../manager/jobs/FetchLoadFeedCombinationTest.java | 3 +++ .../conveyal/datatools/manager/jobs/GisExportJobTest.java | 8 ++++++++ .../datatools/manager/jobs/MergeFeedsJobTest.java | 3 +++ .../datatools/manager/models/FeedVersionTest.java | 3 +++ 9 files changed, 35 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a265b5442..1566ccefa 100644 --- a/pom.xml +++ b/pom.xml @@ -268,7 +268,7 @@ com.github.conveyal gtfs-lib - f2ceb59 + f2ceb59027 diff --git a/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java b/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java index ac24c29bb..d1fe8924b 100644 --- a/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java +++ b/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java @@ -3,6 +3,7 @@ import com.conveyal.datatools.DatatoolsTest; import com.conveyal.datatools.UnitTest; import com.conveyal.datatools.manager.DataManager; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.jobs.MergeFeedsJobTest; import com.conveyal.datatools.manager.models.FeedSource; import com.conveyal.datatools.manager.models.FeedVersion; @@ -44,6 +45,7 @@ public class GtfsPlusValidationTest extends UnitTest { public static void setUp() throws IOException { // Start server if it isn't already running. DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); // Create a project, feed sources, and feed versions to merge. project = new Project(); project.name = String.format("Test %s", new Date()); @@ -65,6 +67,7 @@ public static void setUp() throws IOException { @AfterAll static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); project.delete(); } diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/ArbitraryTransformJobTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/ArbitraryTransformJobTest.java index 3bb40f228..a8924973e 100644 --- a/src/test/java/com/conveyal/datatools/manager/jobs/ArbitraryTransformJobTest.java +++ b/src/test/java/com/conveyal/datatools/manager/jobs/ArbitraryTransformJobTest.java @@ -2,6 +2,7 @@ import com.conveyal.datatools.DatatoolsTest; import com.conveyal.datatools.UnitTest; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.auth.Auth0UserProfile; import com.conveyal.datatools.manager.models.FeedRetrievalMethod; import com.conveyal.datatools.manager.models.FeedSource; @@ -65,6 +66,7 @@ public class ArbitraryTransformJobTest extends UnitTest { public static void setUp() throws IOException { // start server if it isn't already running DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); // Create a project, feed sources, and feed versions to merge. project = new Project(); @@ -81,6 +83,7 @@ public static void setUp() throws IOException { */ @AfterAll public static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); // Project delete cascades to feed sources. project.delete(); } diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/AutoPublishJobTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/AutoPublishJobTest.java index 3c21a1599..4b15cbd56 100644 --- a/src/test/java/com/conveyal/datatools/manager/jobs/AutoPublishJobTest.java +++ b/src/test/java/com/conveyal/datatools/manager/jobs/AutoPublishJobTest.java @@ -3,6 +3,7 @@ import com.amazonaws.services.s3.model.S3ObjectSummary; import com.conveyal.datatools.DatatoolsTest; import com.conveyal.datatools.UnitTest; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.auth.Auth0UserProfile; import com.conveyal.datatools.manager.models.ExternalFeedSourceProperty; import com.conveyal.datatools.manager.models.FeedSource; @@ -52,6 +53,7 @@ public class AutoPublishJobTest extends UnitTest { public static void setUp() throws IOException { // start server if it isn't already running DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); // Create a project, feed sources, and feed versions to merge. project = new Project(); @@ -76,6 +78,7 @@ public static void setUp() throws IOException { @AfterAll public static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); if (project != null) { project.delete(); } diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/DeploymentGisExportJobTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/DeploymentGisExportJobTest.java index aea3a6f16..133864c31 100644 --- a/src/test/java/com/conveyal/datatools/manager/jobs/DeploymentGisExportJobTest.java +++ b/src/test/java/com/conveyal/datatools/manager/jobs/DeploymentGisExportJobTest.java @@ -1,11 +1,13 @@ package com.conveyal.datatools.manager.jobs; import com.conveyal.datatools.DatatoolsTest; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.auth.Auth0UserProfile; import com.conveyal.datatools.manager.models.*; import com.conveyal.datatools.manager.persistence.Persistence; import com.google.common.io.Files; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -63,6 +65,7 @@ private File[] getFoldersFromZippedShapefile(File zipFile) throws IOException { @BeforeAll public static void setUp() throws IOException { DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); // Calling GisExportJobTest.setUp() creates project and feeds GisExportJobTest.setUp(); user = Auth0UserProfile.createTestAdminUser(); @@ -72,6 +75,11 @@ public static void setUp() throws IOException { Persistence.deployments.create(deployment); } + @AfterAll + public static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); + } + /** * Ensures that shapefiles containing stop features for a deployment can be exported and * contain geometry for each stop. diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/FetchLoadFeedCombinationTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/FetchLoadFeedCombinationTest.java index 56c99d32c..f7dc2a33b 100644 --- a/src/test/java/com/conveyal/datatools/manager/jobs/FetchLoadFeedCombinationTest.java +++ b/src/test/java/com/conveyal/datatools/manager/jobs/FetchLoadFeedCombinationTest.java @@ -3,6 +3,7 @@ import com.conveyal.datatools.DatatoolsTest; import com.conveyal.datatools.UnitTest; import com.conveyal.datatools.common.status.MonitorableJob; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.auth.Auth0UserProfile; import com.conveyal.datatools.manager.models.FeedSource; import com.conveyal.datatools.manager.models.FeedVersion; @@ -53,6 +54,7 @@ public class FetchLoadFeedCombinationTest extends UnitTest { public static void setUp() throws IOException { // start server if it isn't already running DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); // Create a project and feed sources. project = new Project(); @@ -70,6 +72,7 @@ public static void setUp() throws IOException { @AfterAll public static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); wireMockServer.stop(); if (project != null) { project.delete(); diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/GisExportJobTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/GisExportJobTest.java index e42976802..6d5fd163d 100644 --- a/src/test/java/com/conveyal/datatools/manager/jobs/GisExportJobTest.java +++ b/src/test/java/com/conveyal/datatools/manager/jobs/GisExportJobTest.java @@ -2,6 +2,7 @@ import com.conveyal.datatools.DatatoolsTest; import com.conveyal.datatools.UnitTest; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.auth.Auth0UserProfile; import com.conveyal.datatools.manager.models.FeedRetrievalMethod; import com.conveyal.datatools.manager.models.FeedSource; @@ -11,6 +12,7 @@ import com.conveyal.datatools.manager.utils.SqlAssert; import com.google.common.base.Strings; import com.google.common.io.Files; +import org.junit.jupiter.api.AfterAll; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.MultiLineString; import org.locationtech.jts.geom.Point; @@ -149,6 +151,7 @@ public void validateShapefiles(File[] files, String agencyName, GisExportJob.Exp @BeforeAll public static void setUp() throws IOException { DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); LOG.info("{} setup", GisExportJobTest.class.getSimpleName()); // Create a project, feed sources, and feed versions to merge. @@ -165,6 +168,11 @@ public static void setUp() throws IOException { hawaiiVersion = createFeedVersionFromGtfsZip(hawaii, "hawaii_fake_no_shapes.zip"); } + @AfterAll + public static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); + } + /** * Ensures that a shapefile containing stop features for a feed version can be exported and * contains geometry for each stop. diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java index 430adbfcc..50ae60586 100644 --- a/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java +++ b/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java @@ -2,6 +2,7 @@ import com.conveyal.datatools.DatatoolsTest; import com.conveyal.datatools.UnitTest; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.auth.Auth0UserProfile; import com.conveyal.datatools.manager.gtfsplus.GtfsPlusValidation; import com.conveyal.datatools.manager.jobs.feedmerge.MergeFeedsType; @@ -86,6 +87,7 @@ public class MergeFeedsJobTest extends UnitTest { public static void setUp() throws IOException { // start server if it isn't already running DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); // Create a project, feed sources, and feed versions to merge. project = new Project(); @@ -157,6 +159,7 @@ public static void setUp() throws IOException { */ @AfterAll public static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); if (project != null) { project.delete(); } diff --git a/src/test/java/com/conveyal/datatools/manager/models/FeedVersionTest.java b/src/test/java/com/conveyal/datatools/manager/models/FeedVersionTest.java index 9bbca2755..13a90b0da 100644 --- a/src/test/java/com/conveyal/datatools/manager/models/FeedVersionTest.java +++ b/src/test/java/com/conveyal/datatools/manager/models/FeedVersionTest.java @@ -2,6 +2,7 @@ import com.conveyal.datatools.DatatoolsTest; import com.conveyal.datatools.UnitTest; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.persistence.Persistence; import com.conveyal.gtfs.error.NewGTFSError; import com.conveyal.gtfs.error.NewGTFSErrorType; @@ -35,6 +36,7 @@ public class FeedVersionTest extends UnitTest { public static void setUp() throws Exception { // start server if it isn't already running DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); // set up project project = new Project(); @@ -48,6 +50,7 @@ public static void setUp() throws Exception { @AfterAll public static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); if (project != null) { project.delete(); } From ca60d4bfedb370ad0e7fc6b726c7527002c16499 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 16 Aug 2023 11:44:37 +0100 Subject: [PATCH 14/40] refactor(NormalizeFieldTransformJobTest): Disabled auth --- .../datatools/manager/jobs/NormalizeFieldTransformJobTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/NormalizeFieldTransformJobTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/NormalizeFieldTransformJobTest.java index 8eb99259e..6a35edb56 100644 --- a/src/test/java/com/conveyal/datatools/manager/jobs/NormalizeFieldTransformJobTest.java +++ b/src/test/java/com/conveyal/datatools/manager/jobs/NormalizeFieldTransformJobTest.java @@ -1,6 +1,7 @@ package com.conveyal.datatools.manager.jobs; import com.conveyal.datatools.DatatoolsTest; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.models.FeedRetrievalMethod; import com.conveyal.datatools.manager.models.FeedSource; import com.conveyal.datatools.manager.models.FeedVersion; @@ -48,6 +49,7 @@ public class NormalizeFieldTransformJobTest extends DatatoolsTest { public static void setUp() throws IOException { // start server if it isn't already running DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); // Create a project. project = createProject(); @@ -58,6 +60,7 @@ public static void setUp() throws IOException { */ @AfterAll public static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); // Project delete cascades to feed sources. project.delete(); } From 89f6f11c0ac44da433ab4689797da71afbb2419c Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 16 Aug 2023 12:13:07 +0100 Subject: [PATCH 15/40] refactor(Disabled auth): --- .../java/com/conveyal/datatools/HandleCorruptGTFSFileTest.java | 3 +++ .../datatools/manager/extensions/mtc/MtcFeedResourceTest.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/test/java/com/conveyal/datatools/HandleCorruptGTFSFileTest.java b/src/test/java/com/conveyal/datatools/HandleCorruptGTFSFileTest.java index 31aefd4fa..72e536dd4 100644 --- a/src/test/java/com/conveyal/datatools/HandleCorruptGTFSFileTest.java +++ b/src/test/java/com/conveyal/datatools/HandleCorruptGTFSFileTest.java @@ -1,6 +1,7 @@ package com.conveyal.datatools; import com.conveyal.datatools.common.status.MonitorableJob; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.jobs.LoadFeedJob; import com.conveyal.datatools.manager.jobs.ProcessSingleFeedJob; import com.conveyal.datatools.manager.jobs.ValidateFeedJob; @@ -30,10 +31,12 @@ public class HandleCorruptGTFSFileTest { public static void setUp() throws IOException { // start server if it isn't already running DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); } @AfterAll public static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); mockProject.delete(); } diff --git a/src/test/java/com/conveyal/datatools/manager/extensions/mtc/MtcFeedResourceTest.java b/src/test/java/com/conveyal/datatools/manager/extensions/mtc/MtcFeedResourceTest.java index 741627985..3a6cb6a31 100644 --- a/src/test/java/com/conveyal/datatools/manager/extensions/mtc/MtcFeedResourceTest.java +++ b/src/test/java/com/conveyal/datatools/manager/extensions/mtc/MtcFeedResourceTest.java @@ -2,6 +2,7 @@ import com.conveyal.datatools.DatatoolsTest; import com.conveyal.datatools.UnitTest; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.models.ExternalFeedSourceProperty; import com.conveyal.datatools.manager.models.FeedSource; import com.conveyal.datatools.manager.models.FeedVersion; @@ -50,6 +51,7 @@ class MtcFeedResourceTest extends UnitTest { static void setUp() throws IOException { // start server if it isn't already running DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); // Create a project, feed sources. project = new Project(); project.name = String.format("Test %s", new Date()); @@ -69,6 +71,7 @@ static void setUp() throws IOException { @AfterAll static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); wireMockServer.stop(); if (project != null) { project.delete(); From 026fa2f70d66d6a42650236cad41dda7718beb8f Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 16 Aug 2023 13:52:49 +0100 Subject: [PATCH 16/40] refactor(SqlSchemaUpdaterTest): Disabled auth --- .../datatools/manager/utils/sql/SqlSchemaUpdaterTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/java/com/conveyal/datatools/manager/utils/sql/SqlSchemaUpdaterTest.java b/src/test/java/com/conveyal/datatools/manager/utils/sql/SqlSchemaUpdaterTest.java index d3050493f..c374cc010 100644 --- a/src/test/java/com/conveyal/datatools/manager/utils/sql/SqlSchemaUpdaterTest.java +++ b/src/test/java/com/conveyal/datatools/manager/utils/sql/SqlSchemaUpdaterTest.java @@ -3,6 +3,7 @@ import com.conveyal.datatools.DatatoolsTest; import com.conveyal.datatools.UnitTest; import com.conveyal.datatools.manager.DataManager; +import com.conveyal.datatools.manager.auth.Auth0Connection; import com.conveyal.datatools.manager.models.FeedSource; import com.conveyal.datatools.manager.models.FeedVersion; import com.conveyal.datatools.manager.models.Project; @@ -40,6 +41,7 @@ class SqlSchemaUpdaterTest extends UnitTest { public static void setUp() throws IOException { // start server if it isn't already running. DatatoolsTest.setUp(); + Auth0Connection.setAuthDisabled(true); // Create a project and feed sources. project = new Project(); @@ -60,6 +62,7 @@ public static void setUp() throws IOException { */ @AfterAll public static void tearDown() { + Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled()); // Project delete cascades to feed sources. project.delete(); } From b76e31831e1ad70961dee799682892fab2498c3a Mon Sep 17 00:00:00 2001 From: miles-grant-ibi Date: Wed, 16 Aug 2023 13:41:55 -0400 Subject: [PATCH 17/40] refactor: address pr feedback --- .../manager/jobs/validation/SharedStopsValidator.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 41084d0e5..4e791b608 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 @@ -108,9 +108,7 @@ public void validate() { registerError(NewGTFSError.forFeed(NewGTFSErrorType.SHARED_STOP_GROUP_ENTITY_DOES_NOT_EXIST, stopId)); } } - } catch (IOException e) { - LOG.error(e.toString()); - } + } catch (IOException e) { LOG.error(e.toString()); } finally { configReader.close(); } From 36391dc84c6034b612acb1171b1138c8334f3b05 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 17 Aug 2023 15:07:17 +0000 Subject: [PATCH 18/40] chore(deps): bump com.google.guava:guava from 30.0-jre to 32.0.0-jre Bumps [com.google.guava:guava](https://github.com/google/guava) from 30.0-jre to 32.0.0-jre. - [Release notes](https://github.com/google/guava/releases) - [Commits](https://github.com/google/guava/commits) --- updated-dependencies: - dependency-name: com.google.guava:guava dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 1566ccefa..658ae2d4a 100644 --- a/pom.xml +++ b/pom.xml @@ -291,7 +291,7 @@ com.google.guava guava - 30.0-jre + 32.0.0-jre javax.xml.bind From be6f44030750a5734372436e775f174dd0dcee52 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 18 Aug 2023 09:59:15 +0100 Subject: [PATCH 19/40] refactor(Update to include a new private class to hold latest validation result): This new class ali --- .../manager/models/FeedSourceSummary.java | 46 ++++++++++++------- .../api/FeedSourceControllerTest.java | 16 +++---- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java index 1f380d541..931fb49f3 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java @@ -60,17 +60,7 @@ public class FeedSourceSummary { public Integer deployedFeedVersionIssues; - public String latestFeedVersionId; - - @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) - @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) - public LocalDate latestFeedVersionStartDate; - - @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) - @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) - public LocalDate latestFeedVersionEndDate; - - public Integer latestFeedVersionIssues; + public LatestValidationResult latestValidation; public FeedSourceSummary() { } @@ -100,12 +90,7 @@ public void setFeedVersion(FeedVersionSummary feedVersionSummary, boolean isDepl ? null : feedVersionSummary.validationResult.errorCount; } else { - this.latestFeedVersionId = feedVersionSummary.id; - this.latestFeedVersionStartDate = feedVersionSummary.validationResult.firstCalendarDate; - this.latestFeedVersionEndDate = feedVersionSummary.validationResult.lastCalendarDate; - this.latestFeedVersionIssues = (feedVersionSummary.validationResult.errorCount == -1) - ? null - : feedVersionSummary.validationResult.errorCount; + this.latestValidation = new LatestValidationResult(feedVersionSummary); } } } @@ -533,4 +518,31 @@ private static LocalDate getDateFromString(String date) { private static LocalDate getLocalDateFromDate(Date date) { return date.toInstant().atZone(ZoneId.systemDefault()).toLocalDate(); } + + public static class LatestValidationResult { + + public String feedVersionId; + @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) + @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) + public LocalDate feedVersionStartDate; + + @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) + @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) + public LocalDate feedVersionEndDate; + + public Integer feedVersionIssues; + + /** Required for JSON de/serializing. **/ + public LatestValidationResult() {} + + LatestValidationResult(FeedVersionSummary feedVersionSummary) { + this.feedVersionId = feedVersionSummary.id; + this.feedVersionStartDate = feedVersionSummary.validationResult.firstCalendarDate; + this.feedVersionEndDate = feedVersionSummary.validationResult.lastCalendarDate; + this.feedVersionIssues = (feedVersionSummary.validationResult.errorCount == -1) + ? null + : feedVersionSummary.validationResult.errorCount; + } + } + } \ No newline at end of file diff --git a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java index 1f58ea1ce..0026e15fc 100644 --- a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java +++ b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java @@ -348,10 +348,10 @@ void canRetrieveDeployedFeedVersionFromLatestDeployment() throws IOException { assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); assertEquals(feedVersionFromLatestDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).deployedFeedVersionIssues); - assertEquals(feedVersionFromLatestDeployment.id, feedSourceSummaries.get(0).latestFeedVersionId); - assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSourceSummaries.get(0).latestFeedVersionStartDate); - assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSourceSummaries.get(0).latestFeedVersionEndDate); - assertEquals(feedVersionFromLatestDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).latestFeedVersionIssues); + assertEquals(feedVersionFromLatestDeployment.id, feedSourceSummaries.get(0).latestValidation.feedVersionId); + assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSourceSummaries.get(0).latestValidation.feedVersionStartDate); + assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSourceSummaries.get(0).latestValidation.feedVersionEndDate); + assertEquals(feedVersionFromLatestDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).latestValidation.feedVersionIssues); } @Test @@ -379,10 +379,10 @@ void canRetrieveDeployedFeedVersionFromPinnedDeployment() throws IOException { assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); assertEquals(feedVersionFromPinnedDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).deployedFeedVersionIssues); - assertEquals(feedVersionFromPinnedDeployment.id, feedSourceSummaries.get(0).latestFeedVersionId); - assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSourceSummaries.get(0).latestFeedVersionStartDate); - assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSourceSummaries.get(0).latestFeedVersionEndDate); - assertEquals(feedVersionFromPinnedDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).latestFeedVersionIssues); + assertEquals(feedVersionFromPinnedDeployment.id, feedSourceSummaries.get(0).latestValidation.feedVersionId); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSourceSummaries.get(0).latestValidation.feedVersionStartDate); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSourceSummaries.get(0).latestValidation.feedVersionEndDate); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).latestValidation.feedVersionIssues); } private static FeedSource createFeedSource(String name, URL url, Project project) { From 15d43e7cc8aac9cec6b831a8ac507332b7813200 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Fri, 18 Aug 2023 15:10:22 -0400 Subject: [PATCH 20/40] refactor(feedsourceSummary): rename fields to agree with old format --- .../datatools/manager/models/FeedSourceSummary.java | 12 ++++++------ .../controllers/api/FeedSourceControllerTest.java | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java index 931fb49f3..d472b05c4 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java @@ -524,22 +524,22 @@ public static class LatestValidationResult { public String feedVersionId; @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) - public LocalDate feedVersionStartDate; + public LocalDate startDate; @JsonSerialize(using = JacksonSerializers.LocalDateIsoSerializer.class) @JsonDeserialize(using = JacksonSerializers.LocalDateIsoDeserializer.class) - public LocalDate feedVersionEndDate; + public LocalDate endDate; - public Integer feedVersionIssues; + public Integer errorCount; /** Required for JSON de/serializing. **/ public LatestValidationResult() {} LatestValidationResult(FeedVersionSummary feedVersionSummary) { this.feedVersionId = feedVersionSummary.id; - this.feedVersionStartDate = feedVersionSummary.validationResult.firstCalendarDate; - this.feedVersionEndDate = feedVersionSummary.validationResult.lastCalendarDate; - this.feedVersionIssues = (feedVersionSummary.validationResult.errorCount == -1) + this.startDate = feedVersionSummary.validationResult.firstCalendarDate; + this.endDate = feedVersionSummary.validationResult.lastCalendarDate; + this.errorCount = (feedVersionSummary.validationResult.errorCount == -1) ? null : feedVersionSummary.validationResult.errorCount; } diff --git a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java index 0026e15fc..21faa0dd9 100644 --- a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java +++ b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java @@ -349,9 +349,9 @@ void canRetrieveDeployedFeedVersionFromLatestDeployment() throws IOException { assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); assertEquals(feedVersionFromLatestDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).deployedFeedVersionIssues); assertEquals(feedVersionFromLatestDeployment.id, feedSourceSummaries.get(0).latestValidation.feedVersionId); - assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSourceSummaries.get(0).latestValidation.feedVersionStartDate); - assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSourceSummaries.get(0).latestValidation.feedVersionEndDate); - assertEquals(feedVersionFromLatestDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).latestValidation.feedVersionIssues); + assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSourceSummaries.get(0).latestValidation.startDate); + assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSourceSummaries.get(0).latestValidation.endDate); + assertEquals(feedVersionFromLatestDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).latestValidation.errorCount); } @Test @@ -380,9 +380,9 @@ void canRetrieveDeployedFeedVersionFromPinnedDeployment() throws IOException { assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); assertEquals(feedVersionFromPinnedDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).deployedFeedVersionIssues); assertEquals(feedVersionFromPinnedDeployment.id, feedSourceSummaries.get(0).latestValidation.feedVersionId); - assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSourceSummaries.get(0).latestValidation.feedVersionStartDate); - assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSourceSummaries.get(0).latestValidation.feedVersionEndDate); - assertEquals(feedVersionFromPinnedDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).latestValidation.feedVersionIssues); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSourceSummaries.get(0).latestValidation.startDate); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSourceSummaries.get(0).latestValidation.endDate); + assertEquals(feedVersionFromPinnedDeployment.validationSummary().errorCount, feedSourceSummaries.get(0).latestValidation.errorCount); } private static FeedSource createFeedSource(String name, URL url, Project project) { From ab782d57e8e8178fcfd6e3262d4141a80c8c7944 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 24 Aug 2023 09:09:59 +0100 Subject: [PATCH 21/40] refactor(FeedSourceSummary.java): Addressed PR feedback --- .../manager/models/FeedSourceSummary.java | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java index d472b05c4..b9917e94a 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java @@ -71,7 +71,10 @@ public FeedSourceSummary(String projectId, Document feedSourceDocument) { this.name = feedSourceDocument.getString("name"); this.deployable = feedSourceDocument.getBoolean("deployable"); this.isPublic = feedSourceDocument.getBoolean("isPublic"); - this.labelIds = feedSourceDocument.getList("labelIds", String.class); + List documentLabelIds = feedSourceDocument.getList("labelIds", String.class); + if (documentLabelIds != null) { + this.labelIds = documentLabelIds; + } // Convert to local date type for consistency. this.lastUpdated = getLocalDateFromDate(feedSourceDocument.getDate("lastUpdated")); } @@ -87,7 +90,7 @@ public void setFeedVersion(FeedVersionSummary feedVersionSummary, boolean isDepl this.deployedFeedVersionStartDate = feedVersionSummary.validationResult.firstCalendarDate; this.deployedFeedVersionEndDate = feedVersionSummary.validationResult.lastCalendarDate; this.deployedFeedVersionIssues = (feedVersionSummary.validationResult.errorCount == -1) - ? null + ? 0 : feedVersionSummary.validationResult.errorCount; } else { this.latestValidation = new LatestValidationResult(feedVersionSummary); @@ -341,25 +344,25 @@ public static Map getFeedVersionsFromPinnedDeploymen pinnedDeploymentId: 1 } }, - { + { $lookup:{ from:"Deployment", localField:"pinnedDeploymentId", foreignField:"_id", as:"deployment" } - }, - { - $unwind: "$deployment" - }, - { + }, + { + $unwind: "$deployment" + }, + { $lookup:{ from:"FeedVersion", localField:"deployment.feedVersionIds", foreignField:"_id", as:"feedVersions" } - }, + }, { // Deconstruct feedVersions array to a document for each element. $unwind: "$feedVersions" From c860bce849316eff0cc78a3fd171ffdb98182048 Mon Sep 17 00:00:00 2001 From: miles-grant-ibi Date: Thu, 24 Aug 2023 11:28:31 -0400 Subject: [PATCH 22/40] shared stops validator: validate feed_id --- pom.xml | 2 +- .../jobs/validation/SharedStopsValidator.java | 19 +++++++++++++------ .../datatools/manager/models/FeedVersion.java | 10 +++++++++- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index ff0612eb5..3a3708edd 100644 --- a/pom.xml +++ b/pom.xml @@ -268,7 +268,7 @@ com.github.conveyal gtfs-lib - 9837b6499796a0eeb5de76314e6a1f3125d695fb + cb8d03c1ee4b3137377986b795de406ab8e1a825 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 b4d4db0a2..a69175532 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 @@ -1,6 +1,5 @@ package com.conveyal.datatools.manager.jobs.validation; -import com.conveyal.datatools.common.utils.aws.S3Utils; import com.conveyal.datatools.manager.models.Project; import com.conveyal.gtfs.error.NewGTFSError; import com.conveyal.gtfs.error.NewGTFSErrorType; @@ -24,11 +23,13 @@ public class SharedStopsValidator extends FeedValidator { private static final Logger LOG = LoggerFactory.getLogger(SharedStopsValidator.class); Feed feed; + String feedId; Project project; - public SharedStopsValidator(Project project) { + public SharedStopsValidator(Project project, String feedId) { super(null, null); this.project = project; + this.feedId = feedId; } // This method can only be run on a SharedStopsValidator that has been set up with a project only @@ -36,12 +37,16 @@ public SharedStopsValidator buildSharedStopsValidator(Feed feed, SQLErrorStorage if (this.project == null) { throw new RuntimeException("Shared stops validator can not be called because no project has been set!"); } - return new SharedStopsValidator(feed, errorStorage, this.project); + if (this.feedId == null) { + throw new RuntimeException("Shared stops validator can not be called because no feed ID has been set!"); + } + return new SharedStopsValidator(feed, errorStorage, this.project, this.feedId); } - public SharedStopsValidator(Feed feed, SQLErrorStorage errorStorage, Project project) { + public SharedStopsValidator(Feed feed, SQLErrorStorage errorStorage, Project project, String feedId) { super(feed, errorStorage); this.feed = feed; this.project = project; + this.feedId = feedId; } @Override @@ -56,6 +61,7 @@ public void validate() { final int STOP_GROUP_ID_INDEX = 0; final int STOP_ID_INDEX = 2; final int IS_PRIMARY_INDEX = 3; + final int FEED_ID_INDEX = 1; // Build list of stop Ids. List stopIds = new ArrayList<>(); @@ -74,6 +80,7 @@ public void validate() { while (configReader.readRecord()) { String stopGroupId = configReader.get(STOP_GROUP_ID_INDEX); String stopId = configReader.get(STOP_ID_INDEX); + String sharedStopFeedId = configReader.get(FEED_ID_INDEX); if (stopId.equals("stop_id")) { // Swallow header row @@ -103,8 +110,8 @@ public void validate() { } // Check for SS_03 (stop_id referenced doesn't exist) - // TODO: CHECK FEED ID (adjust the pre-build constructor to include feed_id) - if (!stopIds.contains(stopId)) { + // 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)); } } diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java index 69edac06f..ce1a3e442 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java @@ -13,6 +13,7 @@ import com.conveyal.gtfs.BaseGTFSCache; import com.conveyal.gtfs.GTFS; import com.conveyal.gtfs.error.NewGTFSErrorType; +import com.conveyal.gtfs.graphql.fetchers.JDBCFetcher; import com.conveyal.gtfs.loader.Feed; import com.conveyal.gtfs.loader.FeedLoadResult; import com.conveyal.gtfs.validator.MTCValidator; @@ -372,7 +373,14 @@ public void validate(MonitorableJob.Status status) { ); } else { FeedSource fs = Persistence.feedSources.getById(this.feedSourceId); - SharedStopsValidator ssv = new SharedStopsValidator(fs.retrieveProject()); + + // Get feed_id from feed version... Really awful hack! + JDBCFetcher feedFetcher = new JDBCFetcher("feed_info", null); + Object gtfsFeedId = feedFetcher.getResults(this.namespace, null, null).get(0).get("feed_id"); + + + String feedId = gtfsFeedId == null ? "" : gtfsFeedId.toString(); + SharedStopsValidator ssv = new SharedStopsValidator(fs.retrieveProject(), feedId); validationResult = GTFS.validate( feedLoadResult.uniqueIdentifier, From 04c99d708d7ee7860c56c3b2769e0db6f9c1184b Mon Sep 17 00:00:00 2001 From: miles-grant-ibi Date: Fri, 8 Sep 2023 11:38:40 -0400 Subject: [PATCH 23/40] address pr feedback --- .../java/com/conveyal/datatools/manager/models/FeedVersion.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java index ce1a3e442..a15c486b2 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java @@ -375,7 +375,7 @@ public void validate(MonitorableJob.Status status) { FeedSource fs = Persistence.feedSources.getById(this.feedSourceId); // Get feed_id from feed version... Really awful hack! - JDBCFetcher feedFetcher = new JDBCFetcher("feed_info", null); + JDBCFetcher feedFetcher = new JDBCFetcher("feed_info"); Object gtfsFeedId = feedFetcher.getResults(this.namespace, null, null).get(0).get("feed_id"); From 4c9cfc08cc487a1881520620ed3e8384c77ffafc Mon Sep 17 00:00:00 2001 From: miles-grant-ibi Date: Mon, 11 Sep 2023 12:39:46 -0400 Subject: [PATCH 24/40] clarify feed_id retrieval comment --- .../conveyal/datatools/manager/models/FeedVersion.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java index a15c486b2..44cf2dc04 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java @@ -374,7 +374,15 @@ public void validate(MonitorableJob.Status status) { } else { FeedSource fs = Persistence.feedSources.getById(this.feedSourceId); - // Get feed_id from feed version... Really awful hack! + /* + Get feed_id from feed version + + This could potentially happen inside gtfs-lib, however + because this functionality is specific to datatools and the + shared stops feature, it lives only here instead. Changes to + gtfs-lib have been avoided, so that gtfs-lib isn't being modified + to support proprietary features. + */ JDBCFetcher feedFetcher = new JDBCFetcher("feed_info"); Object gtfsFeedId = feedFetcher.getResults(this.namespace, null, null).get(0).get("feed_id"); From 5ea408b9b5bfbb542445ec4878c661fb497e7ddb Mon Sep 17 00:00:00 2001 From: miles-grant-ibi Date: Thu, 14 Sep 2023 11:38:30 -0400 Subject: [PATCH 25/40] address pr feedback --- .../datatools/manager/jobs/validation/SharedStopsValidator.java | 1 + 1 file changed, 1 insertion(+) 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 a69175532..d608fe249 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 @@ -58,6 +58,7 @@ public void validate() { CsvReader configReader = CsvReader.parse(config); + // TODO: pull indicies from the CSV header final int STOP_GROUP_ID_INDEX = 0; final int STOP_ID_INDEX = 2; final int IS_PRIMARY_INDEX = 3; From d66c5477d04d2c07e3f8eb5159d961ffe0c73b2b Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Tue, 26 Sep 2023 11:26:03 -0400 Subject: [PATCH 26/40] feat(GtfsPlusValidation): add validation for directions.txt --- .../manager/gtfsplus/GtfsPlusValidation.java | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java b/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java index c0b76d959..a357bb9c1 100644 --- a/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java +++ b/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java @@ -5,6 +5,7 @@ import com.conveyal.datatools.manager.persistence.FeedStore; import com.conveyal.datatools.manager.persistence.Persistence; import com.conveyal.gtfs.GTFSFeed; +import com.conveyal.gtfs.model.Route; import com.csvreader.CsvReader; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; @@ -21,8 +22,10 @@ import java.util.Arrays; import java.util.Collection; import java.util.Enumeration; +import java.util.HashMap; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; @@ -122,6 +125,13 @@ private static void validateTable( GTFSFeed gtfsFeed ) throws IOException { String tableId = specTable.get("id").asText(); + Boolean tableIsDirections = tableId.equals("directions"); + + Map gtfsRoutes = new HashMap<>(); + if (tableIsDirections) { + // Copy the gtfs routes into a map we can "check them off" in (remove them) + gtfsRoutes.putAll(gtfsFeed.routes); + } // Read in table data from input stream. CsvReader csvReader = new CsvReader(inputStreamToValidate, ',', StandardCharsets.UTF_8); @@ -167,15 +177,24 @@ private static void validateTable( // Validate each value in row. Note: we iterate over the fields and not values because a row may be missing // columns, but we still want to validate that missing value (e.g., if it is missing a required field). for (int f = 0; f < fieldsFound.length; f++) { + JsonNode specField = fieldsFound[f]; // If value exists for index, use that. Otherwise, default to null to avoid out of bounds exception. String val = f < recordColumnCount ? rowValues[f] : null; - validateTableValue(issues, tableId, rowIndex, rowValues, val, fieldsFound, fieldsFound[f], gtfsFeed); + if (tableIsDirections && specField.get("name").asText().equals("route_id")) { + validateTableValue(issues, tableId, rowIndex, rowValues, val, fieldsFound, specField, gtfsFeed, gtfsRoutes); + } else { + validateTableValue(issues, tableId, rowIndex, rowValues, val, fieldsFound, specField, gtfsFeed); + } } + // After we're done validating all the table values, check if every route was checked off in directions.txt } rowIndex++; } csvReader.close(); + if (tableIsDirections && !gtfsRoutes.isEmpty()) { + issues.add(new ValidationIssue(tableId, "route_id", -1, "Directions table does not define direction names for all routes.")); + } // Add issues for wrong number of columns and for empty rows after processing all rows. // Note: We considered adding an issue for each row, but opted for the single error approach because there's no // concept of a row-level issue in the UI right now. So we would potentially need to add that to the UI @@ -299,7 +318,32 @@ private static void validateTableValue( } break; } + } + + /** Validate a single route_id value for the directions.txt GTFS+ table. */ + private static void validateTableValue( + Collection issues, + String tableId, + int rowIndex, + String[] allValues, + String value, + JsonNode[] specFieldsFound, + JsonNode specField, + GTFSFeed gtfsFeed, + Map gtfsRoutes + ) { + if (specField == null) return; + validateTableValue(issues, tableId, rowIndex, allValues, value, specFieldsFound, specField, gtfsFeed); + if (!gtfsRoutes.containsKey(value)) { + if (!gtfsFeed.routes.containsKey(value)) { + issues.add(new ValidationIssue(tableId, specField.get("name").asText(), rowIndex, "Duplicate route entry in directions table.")); + } else { + return; + } + } + // "Check off" the route_id from the list to verify every route id has a direction + gtfsRoutes.remove(value); } /** Construct missing ID text for validation issue description. */ From 1b462bc520c6115ee12c7b9ae3eecd28b1e0f9d8 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Tue, 26 Sep 2023 13:34:31 -0400 Subject: [PATCH 27/40] refactor(GtfsPlusValidation): fix tests --- .../manager/gtfsplus/GtfsPlusValidationTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java b/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java index d1fe8924b..45eb504aa 100644 --- a/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java +++ b/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java @@ -75,8 +75,8 @@ static void tearDown() { void canValidateCleanGtfsPlus() throws Exception { LOG.info("Validation BART GTFS+"); GtfsPlusValidation validation = GtfsPlusValidation.validate(bartVersion1.id); - // Expect issues to be zero. - assertThat("Issues count for clean BART feed is zero", validation.issues.size(), equalTo(0)); + // Expect issues to be only one with directions.txt file + assertThat("Issues count for clean BART feed is zero", validation.issues.size(), equalTo(1)); } @Test @@ -85,8 +85,8 @@ void canValidateGtfsPlusWithQuotedValues() throws Exception { GtfsPlusValidation validation = GtfsPlusValidation.validate(bartVersion1WithQuotedValues.id); // Expect issues to be zero. assertThat( - "Issues count for clean BART feed (quoted values) is zero", - validation.issues.size(), equalTo(0) + "Issues count for clean BART feed (quoted values) is equal to 1 (as above)", + validation.issues.size(), equalTo(1) ); } From 345627dd8f83e449b7091fe6787a70304e2d4612 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Tue, 26 Sep 2023 14:11:45 -0400 Subject: [PATCH 28/40] refactor(GtfsPlusValidation): remove duplicate condition --- .../datatools/manager/gtfsplus/GtfsPlusValidation.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java b/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java index a357bb9c1..d120c839d 100644 --- a/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java +++ b/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java @@ -335,13 +335,7 @@ private static void validateTableValue( if (specField == null) return; validateTableValue(issues, tableId, rowIndex, allValues, value, specFieldsFound, specField, gtfsFeed); - if (!gtfsRoutes.containsKey(value)) { - if (!gtfsFeed.routes.containsKey(value)) { - issues.add(new ValidationIssue(tableId, specField.get("name").asText(), rowIndex, "Duplicate route entry in directions table.")); - } else { - return; - } - } + if (!gtfsRoutes.containsKey(value)) return; // "Check off" the route_id from the list to verify every route id has a direction gtfsRoutes.remove(value); } From 8bc5bcaaef3ed2f55626b057aa4108cfda34d25d Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Wed, 27 Sep 2023 11:12:47 -0400 Subject: [PATCH 29/40] feat(deps): update gtfs lib --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 15507bde8..816f0f47a 100644 --- a/pom.xml +++ b/pom.xml @@ -268,7 +268,7 @@ com.github.conveyal gtfs-lib - cb8d03c1ee4b3137377986b795de406ab8e1a825 + d2469631f9fddfeecdd22c81d218479ca32a2bae From 91afbca3cd1f2ddf7c4658f42ed6ce132db174c6 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Wed, 27 Sep 2023 11:20:57 -0400 Subject: [PATCH 30/40] feat(deps): update gtfs lib to merge commit --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 816f0f47a..7e384a78c 100644 --- a/pom.xml +++ b/pom.xml @@ -268,7 +268,7 @@ com.github.conveyal gtfs-lib - d2469631f9fddfeecdd22c81d218479ca32a2bae + 91b05180e68955ba41355cc6dc43e67c17eec8ca From cf83716376c894677cddf3752eb439962f1f4f62 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Wed, 27 Sep 2023 14:19:40 -0400 Subject: [PATCH 31/40] refactor(JdbcTableWriter): respond to PR comments" --- .../manager/gtfsplus/GtfsPlusValidation.java | 47 ++++++------------- 1 file changed, 14 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java b/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java index d120c839d..eb434923f 100644 --- a/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java +++ b/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java @@ -5,7 +5,6 @@ import com.conveyal.datatools.manager.persistence.FeedStore; import com.conveyal.datatools.manager.persistence.Persistence; import com.conveyal.gtfs.GTFSFeed; -import com.conveyal.gtfs.model.Route; import com.csvreader.CsvReader; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; @@ -22,10 +21,11 @@ import java.util.Arrays; import java.util.Collection; import java.util.Enumeration; -import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; -import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; @@ -125,12 +125,12 @@ private static void validateTable( GTFSFeed gtfsFeed ) throws IOException { String tableId = specTable.get("id").asText(); - Boolean tableIsDirections = tableId.equals("directions"); + boolean tableIsDirections = tableId.equals("directions"); - Map gtfsRoutes = new HashMap<>(); + Set gtfsRoutes = new HashSet<>(); if (tableIsDirections) { - // Copy the gtfs routes into a map we can "check them off" in (remove them) - gtfsRoutes.putAll(gtfsFeed.routes); + // Copy the gtfs routes into a map we can "check them off" in (remove them). Stream is required in order to copy keys. + gtfsRoutes.addAll(gtfsFeed.routes.keySet()); } // Read in table data from input stream. @@ -180,19 +180,15 @@ private static void validateTable( JsonNode specField = fieldsFound[f]; // If value exists for index, use that. Otherwise, default to null to avoid out of bounds exception. String val = f < recordColumnCount ? rowValues[f] : null; - if (tableIsDirections && specField.get("name").asText().equals("route_id")) { - validateTableValue(issues, tableId, rowIndex, rowValues, val, fieldsFound, specField, gtfsFeed, gtfsRoutes); - } else { - validateTableValue(issues, tableId, rowIndex, rowValues, val, fieldsFound, specField, gtfsFeed); - } + validateTableValue(issues, tableId, rowIndex, rowValues, val, fieldsFound, specField, gtfsFeed, gtfsRoutes, tableIsDirections); } - // After we're done validating all the table values, check if every route was checked off in directions.txt } rowIndex++; } csvReader.close(); if (tableIsDirections && !gtfsRoutes.isEmpty()) { + // After we're done validating all the table values, check if every route was checked off in directions.txt issues.add(new ValidationIssue(tableId, "route_id", -1, "Directions table does not define direction names for all routes.")); } // Add issues for wrong number of columns and for empty rows after processing all rows. @@ -224,7 +220,9 @@ private static void validateTableValue( String value, JsonNode[] specFieldsFound, JsonNode specField, - GTFSFeed gtfsFeed + GTFSFeed gtfsFeed, + Set gtfsRoutes, + boolean tableIsDirections ) { if (specField == null) return; String fieldName = specField.get("name").asText(); @@ -318,26 +316,9 @@ private static void validateTableValue( } break; } - } - - /** Validate a single route_id value for the directions.txt GTFS+ table. */ - private static void validateTableValue( - Collection issues, - String tableId, - int rowIndex, - String[] allValues, - String value, - JsonNode[] specFieldsFound, - JsonNode specField, - GTFSFeed gtfsFeed, - Map gtfsRoutes - ) { - if (specField == null) return; - validateTableValue(issues, tableId, rowIndex, allValues, value, specFieldsFound, specField, gtfsFeed); - if (!gtfsRoutes.containsKey(value)) return; - // "Check off" the route_id from the list to verify every route id has a direction - gtfsRoutes.remove(value); + // "Check off" the route_id in directions.txt from the list to verify every route id has a direction + if (tableIsDirections && fieldName.equals("route_id")) gtfsRoutes.remove(value); } /** Construct missing ID text for validation issue description. */ From 5529162bc1e832f356b5110cb2f7e5b9f2f971a9 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Thu, 28 Sep 2023 10:29:46 -0400 Subject: [PATCH 32/40] refactor(GtfsPlusValidation): Update wording --- .../datatools/manager/gtfsplus/GtfsPlusValidation.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java b/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java index eb434923f..c71f5d2f2 100644 --- a/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java +++ b/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java @@ -25,7 +25,6 @@ import java.util.LinkedList; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; @@ -129,7 +128,7 @@ private static void validateTable( Set gtfsRoutes = new HashSet<>(); if (tableIsDirections) { - // Copy the gtfs routes into a map we can "check them off" in (remove them). Stream is required in order to copy keys. + // Copy the gtfs routes into a set so that we can "check them off" (remove them). gtfsRoutes.addAll(gtfsFeed.routes.keySet()); } From e9cd115e533211873ab85826c82df95f07a232d9 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Thu, 28 Sep 2023 10:30:43 -0400 Subject: [PATCH 33/40] refactor(GtfsPlusValidationTest): Update test wording --- .../datatools/manager/gtfsplus/GtfsPlusValidationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java b/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java index 45eb504aa..2ed4d6381 100644 --- a/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java +++ b/src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java @@ -76,7 +76,7 @@ void canValidateCleanGtfsPlus() throws Exception { LOG.info("Validation BART GTFS+"); GtfsPlusValidation validation = GtfsPlusValidation.validate(bartVersion1.id); // Expect issues to be only one with directions.txt file - assertThat("Issues count for clean BART feed is zero", validation.issues.size(), equalTo(1)); + assertThat("Clean BART feed and incomplete directions.txt results in one issue", validation.issues.size(), equalTo(1)); } @Test From b3b32997919795eb2272aeb450424399fc7dcf9a Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Thu, 28 Sep 2023 15:49:48 -0400 Subject: [PATCH 34/40] feat(deps): update gtfs lib again --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 7e384a78c..5f5a80fe2 100644 --- a/pom.xml +++ b/pom.xml @@ -268,7 +268,7 @@ com.github.conveyal gtfs-lib - 91b05180e68955ba41355cc6dc43e67c17eec8ca + 80a968cd3a071aa46a994ce2632535303a6e4384 From 2cc547dcfffd1519669a3f47251ddbeb5ba86491 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Fri, 29 Sep 2023 14:52:35 -0400 Subject: [PATCH 35/40] fix(shared stops): fix everything --- .../datatools/manager/models/FeedSourceSummary.java | 8 ++++---- .../datatools/manager/models/FeedVersion.java | 11 +++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java index b9917e94a..9cf031df7 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java @@ -200,10 +200,10 @@ public static Map getLatestFeedVersionForFeedSources unwind("$feedVersions"), group( "$_id", - Accumulators.first("feedVersionId", "$feedVersions._id"), - Accumulators.first("firstCalendarDate", "$feedVersions.validationResult.firstCalendarDate"), - Accumulators.first("lastCalendarDate", "$feedVersions.validationResult.lastCalendarDate"), - Accumulators.first("errorCount", "$feedVersions.validationResult.errorCount") + Accumulators.last("feedVersionId", "$feedVersions._id"), + Accumulators.last("firstCalendarDate", "$feedVersions.validationResult.firstCalendarDate"), + Accumulators.last("lastCalendarDate", "$feedVersions.validationResult.lastCalendarDate"), + Accumulators.last("errorCount", "$feedVersions.validationResult.errorCount") ) ); return extractFeedVersionSummaries( diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java index 44cf2dc04..060f8589b 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java @@ -365,7 +365,7 @@ public void validate(MonitorableJob.Status status) { status.update("Validating feed...", 33); // Validate the feed version. - // Certain extensions, if enabled, have extra validators + // Certain extensions, if enabled, have extra validators. if (isExtensionEnabled("mtc")) { validationResult = GTFS.validate(feedLoadResult.uniqueIdentifier, DataManager.GTFS_DATA_SOURCE, RouteTypeValidatorBuilder::buildRouteValidator, @@ -384,9 +384,12 @@ public void validate(MonitorableJob.Status status) { to support proprietary features. */ JDBCFetcher feedFetcher = new JDBCFetcher("feed_info"); - Object gtfsFeedId = feedFetcher.getResults(this.namespace, null, null).get(0).get("feed_id"); - - + Object gtfsFeedId = new Object(); + try { + gtfsFeedId = feedFetcher.getResults(this.namespace, null, null).get(0).get("feed_id"); + } catch (RuntimeException e) { + LOG.warn("RuntimeException occurred while fetching feedId"); + } String feedId = gtfsFeedId == null ? "" : gtfsFeedId.toString(); SharedStopsValidator ssv = new SharedStopsValidator(fs.retrieveProject(), feedId); From ebbfdaf444339365550b9e9f965437232a633e8e Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 3 Oct 2023 17:33:30 +0100 Subject: [PATCH 36/40] refactor(Add url to feed source summary): --- .../datatools/manager/models/FeedSourceSummary.java | 9 +++++++-- .../controllers/api/FeedSourceControllerTest.java | 10 ++++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java index 9cf031df7..2c5d6197f 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java @@ -62,6 +62,8 @@ public class FeedSourceSummary { public LatestValidationResult latestValidation; + public String url; + public FeedSourceSummary() { } @@ -77,6 +79,7 @@ public FeedSourceSummary(String projectId, Document feedSourceDocument) { } // Convert to local date type for consistency. this.lastUpdated = getLocalDateFromDate(feedSourceDocument.getDate("lastUpdated")); + this.url = feedSourceDocument.getString("url"); } /** @@ -117,7 +120,8 @@ public static List getFeedSourceSummaries(String projectId) { "deployable": 1, "isPublic": 1, "lastUpdated": 1, - "labelIds": 1 + "labelIds": 1, + "url": 1 } }, { @@ -138,7 +142,8 @@ public static List getFeedSourceSummaries(String projectId) { "deployable", "isPublic", "lastUpdated", - "labelIds") + "labelIds", + "url") ) ), sort(Sorts.ascending("name")) diff --git a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java index 21faa0dd9..a93f14630 100644 --- a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java +++ b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java @@ -75,8 +75,8 @@ public static void setUp() throws IOException { feedSourceWithUrl = createFeedSource("FeedSourceOne", new URL("http://www.feedsource.com"), project); feedSourceWithNoUrl = createFeedSource("FeedSourceTwo", null, project); - feedSourceWithLabels = createFeedSource("FeedSourceThree", null, projectToBeDeleted); - feedSourceWithInvalidLabels = createFeedSource("FeedSourceFour", null, project); + feedSourceWithLabels = createFeedSource("FeedSourceThree", new URL("http://www.feedsource.com"), projectToBeDeleted); + feedSourceWithInvalidLabels = createFeedSource("FeedSourceFour", new URL("http://www.feedsource.com"), project); adminOnlyLabel = createLabel("Admin Only Label"); adminOnlyLabel.adminOnly = true; @@ -89,7 +89,7 @@ public static void setUp() throws IOException { feedSourceWithLatestDeploymentFeedVersion = createFeedSource( "feed-source-with-latest-deployment-feed-version", "FeedSource", - null, + new URL("http://www.feedsource.com"), projectWithLatestDeployment, true, List.of("labelOne", "labelTwo") @@ -123,7 +123,7 @@ public static void setUp() throws IOException { feedSourceWithPinnedDeploymentFeedVersion = createFeedSource( "feed-source-with-pinned-deployment-feed-version", "FeedSourceWithPinnedFeedVersion", - null, + new URL("http://www.feedsource.com"), projectWithPinnedDeployment, true, List.of("labelOne", "labelTwo") @@ -344,6 +344,7 @@ void canRetrieveDeployedFeedVersionFromLatestDeployment() throws IOException { assertEquals(feedSourceWithLatestDeploymentFeedVersion.id, feedSourceSummaries.get(0).id); assertEquals(feedSourceWithLatestDeploymentFeedVersion.projectId, feedSourceSummaries.get(0).projectId); assertEquals(feedSourceWithLatestDeploymentFeedVersion.labelIds, feedSourceSummaries.get(0).labelIds); + assertEquals(feedSourceWithLatestDeploymentFeedVersion.url.toString(), feedSourceSummaries.get(0).url); assertEquals(feedVersionFromLatestDeployment.id, feedSourceSummaries.get(0).deployedFeedVersionId); assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); @@ -375,6 +376,7 @@ void canRetrieveDeployedFeedVersionFromPinnedDeployment() throws IOException { assertEquals(feedSourceWithPinnedDeploymentFeedVersion.id, feedSourceSummaries.get(0).id); assertEquals(feedSourceWithPinnedDeploymentFeedVersion.projectId, feedSourceSummaries.get(0).projectId); assertEquals(feedSourceWithPinnedDeploymentFeedVersion.labelIds, feedSourceSummaries.get(0).labelIds); + assertEquals(feedSourceWithPinnedDeploymentFeedVersion.url.toString(), feedSourceSummaries.get(0).url); assertEquals(feedVersionFromPinnedDeployment.id, feedSourceSummaries.get(0).deployedFeedVersionId); assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); From 69a9b653158849e89708a239db4a7bb7b426926c Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 13 Oct 2023 11:08:58 +0100 Subject: [PATCH 37/40] refactor(Added auth checks): Auth checks and added notes and org id to feed source summary --- .../controllers/api/FeedSourceController.java | 40 +++++--- .../manager/models/FeedSourceSummary.java | 47 +++++++-- .../datatools/manager/models/Label.java | 13 ++- .../datatools/manager/models/Note.java | 15 +++ .../datatools/manager/models/Project.java | 2 +- .../api/FeedSourceControllerTest.java | 97 ++++++++++++++++--- 6 files changed, 177 insertions(+), 37 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java b/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java index badedda76..080c79915 100644 --- a/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java +++ b/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java @@ -45,6 +45,7 @@ import static com.conveyal.datatools.common.utils.SparkUtils.getPOJOFromRequestBody; import static com.conveyal.datatools.common.utils.SparkUtils.logMessageAndHalt; import static com.conveyal.datatools.manager.models.ExternalFeedSourceProperty.constructId; +import static com.conveyal.datatools.manager.models.FeedSourceSummary.cleanFeedSourceSummaryForNonAdmins; import static com.conveyal.datatools.manager.models.transform.NormalizeFieldTransformation.getInvalidSubstitutionMessage; import static com.conveyal.datatools.manager.models.transform.NormalizeFieldTransformation.getInvalidSubstitutionPatterns; import static com.mongodb.client.model.Filters.in; @@ -103,6 +104,31 @@ private static Collection getProjectFeedSources(Request req, Respons return feedSourcesToReturn; } + private static Collection getAllFeedSourceSummaries(Request req, Response res) { + Collection feedSourcesToReturn = new ArrayList<>(); + Auth0UserProfile user = req.attribute("user"); + String projectId = req.queryParams("projectId"); + + Project project = Persistence.projects.getById(projectId); + + if (project == null) { + logMessageAndHalt(req, 400, "Must provide valid projectId value."); + } else { + boolean isAdmin = user.canAdministerProject(project); + Collection feedSourceSummaries = project.retrieveFeedSourceSummaries(); + for (FeedSourceSummary feedSourceSummary : feedSourceSummaries) { + // If user can view or manage feed, add to list of feeds to return. NOTE: By default most users with access + // to a project should be able to view all feed sources. Custom privileges would need to be provided to + // override this behavior. + if (user.canManageOrViewFeed(project.organizationId, feedSourceSummary.projectId, feedSourceSummary.id)) { + // Remove labels user can't view, then add to list of feeds to return. + feedSourcesToReturn.add(cleanFeedSourceSummaryForNonAdmins(feedSourceSummary, isAdmin)); + } + } + } + return feedSourcesToReturn; + } + /** * HTTP endpoint to create a new feed source. */ @@ -397,20 +423,6 @@ protected static FeedSource cleanFeedSourceForNonAdmins(FeedSource feedSource, b return feedSource; } - private static Collection getAllFeedSourceSummaries(Request req, Response res) { - Auth0UserProfile userProfile = req.attribute("user"); - String projectId = req.queryParams("projectId"); - Project project = Persistence.projects.getById(projectId); - if (project == null) { - logMessageAndHalt(req, 400, "Must provide valid projectId value."); - } - if (!userProfile.canAdministerProject(project)) { - logMessageAndHalt(req, 401, "User not authorized to view project feed sources."); - } - return project.retrieveFeedSourceSummaries(); - } - - // FIXME: use generic API controller and return JSON documents via BSON/Mongo public static void register (String apiPrefix) { get(apiPrefix + "secure/feedsource/:id", FeedSourceController::getFeedSource, json::write); diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java index 2c5d6197f..6116bac3f 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java @@ -2,6 +2,7 @@ import com.conveyal.datatools.editor.utils.JacksonSerializers; import com.conveyal.datatools.manager.persistence.Persistence; +import com.conveyal.datatools.manager.utils.PersistenceUtils; import com.conveyal.gtfs.validator.ValidationResult; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; @@ -20,6 +21,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import static com.mongodb.client.model.Aggregates.group; import static com.mongodb.client.model.Aggregates.limit; @@ -64,11 +66,16 @@ public class FeedSourceSummary { public String url; + public List noteIds = new ArrayList<>(); + + public String organizationId; + public FeedSourceSummary() { } - public FeedSourceSummary(String projectId, Document feedSourceDocument) { + public FeedSourceSummary(String projectId, String organizationId, Document feedSourceDocument) { this.projectId = projectId; + this.organizationId = organizationId; this.id = feedSourceDocument.getString("_id"); this.name = feedSourceDocument.getString("name"); this.deployable = feedSourceDocument.getBoolean("deployable"); @@ -77,11 +84,35 @@ public FeedSourceSummary(String projectId, Document feedSourceDocument) { if (documentLabelIds != null) { this.labelIds = documentLabelIds; } + List documentNoteIds = feedSourceDocument.getList("noteIds", String.class); + if (documentNoteIds != null) { + this.noteIds = documentNoteIds; + } // Convert to local date type for consistency. this.lastUpdated = getLocalDateFromDate(feedSourceDocument.getDate("lastUpdated")); this.url = feedSourceDocument.getString("url"); } + /** + * Removes labels and notes from a feed that a user is not allowed to view. Returns cleaned feed source. + * @param feedSourceSummary The feed source to clean + * @param isAdmin Is the user an admin? Changes what is returned. + * @return A feed source containing only labels/notes the user is allowed to see + */ + public static FeedSourceSummary cleanFeedSourceSummaryForNonAdmins(FeedSourceSummary feedSourceSummary, boolean isAdmin) { + // Admin can view all feed labels, but a non-admin should only see those with adminOnly=false + feedSourceSummary.labelIds = Persistence.labels + .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", feedSourceSummary.labelIds), isAdmin)).stream() + .map(label -> label.id) + .collect(Collectors.toList()); + feedSourceSummary.noteIds = Persistence.notes + .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", feedSourceSummary.noteIds), isAdmin)).stream() + .map(note -> note.id) + .collect(Collectors.toList()); + return feedSourceSummary; + } + + /** * Set the appropriate feed version. For consistency, if no error count is available set the related number of * issues to null. @@ -104,7 +135,7 @@ public void setFeedVersion(FeedVersionSummary feedVersionSummary, boolean isDepl /** * Get all feed source summaries matching the project id. */ - public static List getFeedSourceSummaries(String projectId) { + public static List getFeedSourceSummaries(String projectId, String organizationId) { /* db.getCollection('FeedSource').aggregate([ { @@ -121,7 +152,8 @@ public static List getFeedSourceSummaries(String projectId) { "isPublic": 1, "lastUpdated": 1, "labelIds": 1, - "url": 1 + "url": 1, + "noteIds": 1 } }, { @@ -143,12 +175,13 @@ public static List getFeedSourceSummaries(String projectId) { "isPublic", "lastUpdated", "labelIds", - "url") + "url", + "noteIds") ) ), sort(Sorts.ascending("name")) ); - return extractFeedSourceSummaries(projectId, stages); + return extractFeedSourceSummaries(projectId, organizationId, stages); } /** @@ -423,10 +456,10 @@ public static Map getFeedVersionsFromPinnedDeploymen /** * Produce a list of all feed source summaries for a project. */ - private static List extractFeedSourceSummaries(String projectId, List stages) { + private static List extractFeedSourceSummaries(String projectId, String organizationId, List stages) { List feedSourceSummaries = new ArrayList<>(); for (Document feedSourceDocument : Persistence.getDocuments("FeedSource", stages)) { - feedSourceSummaries.add(new FeedSourceSummary(projectId, feedSourceDocument)); + feedSourceSummaries.add(new FeedSourceSummary(projectId, organizationId, feedSourceDocument)); } return feedSourceSummaries; } diff --git a/src/main/java/com/conveyal/datatools/manager/models/Label.java b/src/main/java/com/conveyal/datatools/manager/models/Label.java index 8bf239798..0afe2563e 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/Label.java +++ b/src/main/java/com/conveyal/datatools/manager/models/Label.java @@ -39,15 +39,24 @@ public String organizationId () { public Auth0UserProfile user; /** - * Create a new label + * Create a new label with auto-gen id. */ public Label (String name, String description, String color, boolean adminOnly, String projectId) { + this(null, name, description, color, adminOnly, projectId); + } + + /** + * Create a new label with provided id. + */ + public Label (String id, String name, String description, String color, boolean adminOnly, String projectId) { super(); + if (id != null) { + this.id = id; + } this.name = name; this.description = description != null ? description : ""; this.color = color != null ? color : "#000000"; this.adminOnly = adminOnly; - this.projectId = projectId; } diff --git a/src/main/java/com/conveyal/datatools/manager/models/Note.java b/src/main/java/com/conveyal/datatools/manager/models/Note.java index 87b5f452f..438a784b5 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/Note.java +++ b/src/main/java/com/conveyal/datatools/manager/models/Note.java @@ -32,6 +32,21 @@ public class Note extends Model implements Serializable { /** Whether the note should be visible to project admins only */ public boolean adminOnly; + /** + * Create a new note with provided id. + */ + public Note(String id, String body, boolean adminOnly) { + super(); + if (id != null) { + this.id = id; + } + this.body = body; + this.adminOnly = adminOnly; + } + + public Note() { + } + /** * The types of object that can have notes recorded on them. */ diff --git a/src/main/java/com/conveyal/datatools/manager/models/Project.java b/src/main/java/com/conveyal/datatools/manager/models/Project.java index 4ccad306f..eccb7d33a 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/Project.java +++ b/src/main/java/com/conveyal/datatools/manager/models/Project.java @@ -173,7 +173,7 @@ public Collection retrieveDeploymentSummaries() { * Get all feed source summaries for this project. */ public Collection retrieveFeedSourceSummaries() { - List feedSourceSummaries = FeedSourceSummary.getFeedSourceSummaries(id); + List feedSourceSummaries = FeedSourceSummary.getFeedSourceSummaries(id, organizationId); Map latestFeedVersionForFeedSources = FeedSourceSummary.getLatestFeedVersionForFeedSources(id); Map deployedFeedVersions = FeedSourceSummary.getFeedVersionsFromPinnedDeployment(id); if (deployedFeedVersions.isEmpty()) { diff --git a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java index a93f14630..a4c6d8d0a 100644 --- a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java +++ b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java @@ -11,6 +11,7 @@ import com.conveyal.datatools.manager.models.FeedVersion; import com.conveyal.datatools.manager.models.FetchFrequency; import com.conveyal.datatools.manager.models.Label; +import com.conveyal.datatools.manager.models.Note; import com.conveyal.datatools.manager.models.Project; import com.conveyal.datatools.manager.persistence.Persistence; import com.conveyal.datatools.manager.utils.HttpUtils; @@ -22,6 +23,7 @@ import org.junit.jupiter.api.Test; import java.io.IOException; +import java.net.MalformedURLException; import java.net.URL; import java.time.LocalDate; import java.time.Month; @@ -46,7 +48,10 @@ public class FeedSourceControllerTest extends DatatoolsTest { private static FeedSource feedSourceWithInvalidLabels = null; private static Label publicLabel = null; private static Label adminOnlyLabel = null; - + private static Label feedSourceWithLatestDeploymentAdminOnlyLabel = null; + private static Label feedSourceWithPinnedDeploymentAdminOnlyLabel = null; + private static Note feedSourceWithLatestDeploymentAdminOnlyNote = null; + private static Note feedSourceWithPinnedDeploymentAdminOnlyNote = null; private static Project projectWithLatestDeployment = null; private static FeedSource feedSourceWithLatestDeploymentFeedVersion = null; private static FeedVersion feedVersionFromLatestDeployment = null; @@ -78,22 +83,37 @@ public static void setUp() throws IOException { feedSourceWithLabels = createFeedSource("FeedSourceThree", new URL("http://www.feedsource.com"), projectToBeDeleted); feedSourceWithInvalidLabels = createFeedSource("FeedSourceFour", new URL("http://www.feedsource.com"), project); - adminOnlyLabel = createLabel("Admin Only Label"); + adminOnlyLabel = createLabel("Admin Only Label", projectToBeDeleted.id); adminOnlyLabel.adminOnly = true; - publicLabel = createLabel("Public Label"); + publicLabel = createLabel("Public Label", projectToBeDeleted.id); + + setUpFeedVersionFromLatestDeployment(); + setUpFeedVersionFromPinnedDeployment(); + + } - // Feed version from latest deployment. + /** + * Create all the required objects to test a feed version from the latest deployment. + */ + private static void setUpFeedVersionFromLatestDeployment() throws MalformedURLException { projectWithLatestDeployment = new Project(); projectWithLatestDeployment.id = "project-with-latest-deployment"; + projectWithLatestDeployment.organizationId = "project-with-latest-deployment-org-id"; Persistence.projects.create(projectWithLatestDeployment); + + feedSourceWithLatestDeploymentAdminOnlyLabel = createLabel("label-id-latest-deployment", "Admin Only Label", projectWithLatestDeployment.id); + feedSourceWithLatestDeploymentAdminOnlyNote = createNote("note-id-latest-deployment", "A test note"); + feedSourceWithLatestDeploymentFeedVersion = createFeedSource( "feed-source-with-latest-deployment-feed-version", "FeedSource", new URL("http://www.feedsource.com"), projectWithLatestDeployment, true, - List.of("labelOne", "labelTwo") + List.of(feedSourceWithLatestDeploymentAdminOnlyLabel.id), + List.of(feedSourceWithLatestDeploymentAdminOnlyNote.id) ); + LocalDate deployedSuperseded = LocalDate.of(2020, Month.MARCH, 12); LocalDate deployedEndDate = LocalDate.of(2021, Month.MARCH, 12); LocalDate deployedStartDate = LocalDate.of(2021, Month.MARCH, 1); @@ -115,18 +135,28 @@ public static void setUp() throws IOException { feedVersionFromLatestDeployment.id, deployedEndDate ); + } - // Feed version from pinned deployment. + /** + * Create all the required objects to test a feed version from a pinned deployment. + */ + private static void setUpFeedVersionFromPinnedDeployment() throws MalformedURLException { projectWithPinnedDeployment = new Project(); projectWithPinnedDeployment.id = "project-with-pinned-deployment"; + projectWithPinnedDeployment.organizationId = "project-with-pinned-deployment-org-id"; Persistence.projects.create(projectWithPinnedDeployment); + + feedSourceWithPinnedDeploymentAdminOnlyLabel = createLabel("label-id-pinned-deployment", "Admin Only Label", projectWithPinnedDeployment.id); + feedSourceWithPinnedDeploymentAdminOnlyNote = createNote("note-id-pinned-deployment", "A test note"); + feedSourceWithPinnedDeploymentFeedVersion = createFeedSource( "feed-source-with-pinned-deployment-feed-version", "FeedSourceWithPinnedFeedVersion", new URL("http://www.feedsource.com"), projectWithPinnedDeployment, true, - List.of("labelOne", "labelTwo") + List.of(feedSourceWithPinnedDeploymentAdminOnlyLabel.id), + List.of(feedSourceWithPinnedDeploymentAdminOnlyNote.id) ); feedVersionFromPinnedDeployment = createFeedVersion( "feed-version-from-pinned-deployment", @@ -137,7 +167,7 @@ public static void setUp() throws IOException { "deployment-pinned", projectWithPinnedDeployment, feedVersionFromPinnedDeployment.id, - deployedEndDate + LocalDate.of(2021, Month.MARCH, 12) ); projectWithPinnedDeployment.pinnedDeploymentId = deploymentPinned.id; Persistence.projects.replace(projectWithPinnedDeployment.id, projectWithPinnedDeployment); @@ -200,6 +230,18 @@ private static void tearDownDeployedFeedVersion() { if (deploymentSuperseded != null) { Persistence.deployments.removeById(deploymentSuperseded.id); } + if (feedSourceWithPinnedDeploymentAdminOnlyLabel != null) { + Persistence.labels.removeById(feedSourceWithPinnedDeploymentAdminOnlyLabel.id); + } + if (feedSourceWithLatestDeploymentAdminOnlyLabel != null) { + Persistence.labels.removeById(feedSourceWithLatestDeploymentAdminOnlyLabel.id); + } + if (feedSourceWithPinnedDeploymentAdminOnlyNote != null) { + Persistence.notes.removeById(feedSourceWithPinnedDeploymentAdminOnlyNote.id); + } + if (feedSourceWithLatestDeploymentAdminOnlyNote != null) { + Persistence.notes.removeById(feedSourceWithLatestDeploymentAdminOnlyNote.id); + } } /** @@ -345,6 +387,8 @@ void canRetrieveDeployedFeedVersionFromLatestDeployment() throws IOException { assertEquals(feedSourceWithLatestDeploymentFeedVersion.projectId, feedSourceSummaries.get(0).projectId); assertEquals(feedSourceWithLatestDeploymentFeedVersion.labelIds, feedSourceSummaries.get(0).labelIds); assertEquals(feedSourceWithLatestDeploymentFeedVersion.url.toString(), feedSourceSummaries.get(0).url); + assertEquals(feedSourceWithLatestDeploymentFeedVersion.noteIds, feedSourceSummaries.get(0).noteIds); + assertEquals(feedSourceWithLatestDeploymentFeedVersion.organizationId(), feedSourceSummaries.get(0).organizationId); assertEquals(feedVersionFromLatestDeployment.id, feedSourceSummaries.get(0).deployedFeedVersionId); assertEquals(feedVersionFromLatestDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); assertEquals(feedVersionFromLatestDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); @@ -377,6 +421,8 @@ void canRetrieveDeployedFeedVersionFromPinnedDeployment() throws IOException { assertEquals(feedSourceWithPinnedDeploymentFeedVersion.projectId, feedSourceSummaries.get(0).projectId); assertEquals(feedSourceWithPinnedDeploymentFeedVersion.labelIds, feedSourceSummaries.get(0).labelIds); assertEquals(feedSourceWithPinnedDeploymentFeedVersion.url.toString(), feedSourceSummaries.get(0).url); + assertEquals(feedSourceWithPinnedDeploymentFeedVersion.noteIds, feedSourceSummaries.get(0).noteIds); + assertEquals(feedSourceWithPinnedDeploymentFeedVersion.organizationId(), feedSourceSummaries.get(0).organizationId); assertEquals(feedVersionFromPinnedDeployment.id, feedSourceSummaries.get(0).deployedFeedVersionId); assertEquals(feedVersionFromPinnedDeployment.validationSummary().startDate, feedSourceSummaries.get(0).deployedFeedVersionStartDate); assertEquals(feedVersionFromPinnedDeployment.validationSummary().endDate, feedSourceSummaries.get(0).deployedFeedVersionEndDate); @@ -395,7 +441,7 @@ private static FeedSource createFeedSource(String name, URL url, Project project * Helper method to create feed source. */ private static FeedSource createFeedSource(String id, String name, URL url, Project project, boolean persist) { - return createFeedSource(id, name, url, project, persist, null); + return createFeedSource(id, name, url, project, persist, null, null); } private static FeedSource createFeedSource( String id, @@ -403,7 +449,8 @@ private static FeedSource createFeedSource( URL url, Project project, boolean persist, - List labels + List labels, + List notes ) { FeedSource feedSource = new FeedSource(); if (id != null) feedSource.id = id; @@ -415,6 +462,7 @@ private static FeedSource createFeedSource( feedSource.retrievalMethod = FeedRetrievalMethod.FETCHED_AUTOMATICALLY; feedSource.url = url; if (labels != null) feedSource.labelIds = labels; + if (notes != null) feedSource.noteIds = notes; if (persist) Persistence.feedSources.create(feedSource); return feedSource; } @@ -461,10 +509,33 @@ private static FeedVersion createFeedVersion(String id, String feedSourceId, Loc } /** - * Helper method to create label + * Helper method to create note. + */ + private static Note createNote(String id, String body) { + Note note = new Note(id, body, false); + Persistence.notes.create(note); + return note; + } + + /** + * Helper method to create label. If the id is provided save the label now if not deffer to test to save. + */ + private static Label createLabel(String id, String name, String projectId) { + Label label; + if (id != null) { + label = new Label(id, name, "A label used during testing", "#123", false, projectId); + Persistence.labels.create(label); + } else { + label = new Label(name, "A label used during testing", "#123", false, projectId); + } + return label; + } + + /** + * Helper method to create label. */ - private static Label createLabel(String name) { - return new Label(name, "A label used during testing", "#123", false, projectToBeDeleted.id); + private static Label createLabel(String name, String projectId) { + return createLabel(null, name, projectId); } /** From 6695674e23dda07cd1f3a9eb32e347338286b179 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 13 Oct 2023 12:47:13 +0100 Subject: [PATCH 38/40] refactor(Refactored label and note auth checks): --- .../controllers/api/FeedSourceController.java | 77 ++++++++++++++----- .../manager/models/FeedSourceSummary.java | 29 ++----- 2 files changed, 63 insertions(+), 43 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java b/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java index 080c79915..628d7815c 100644 --- a/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java +++ b/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java @@ -9,7 +9,6 @@ import com.conveyal.datatools.manager.extensions.ExternalFeedResource; import com.conveyal.datatools.manager.jobs.FetchSingleFeedJob; import com.conveyal.datatools.manager.jobs.NotifyUsersForSubscriptionJob; -import com.conveyal.datatools.manager.models.DeploymentSummary; import com.conveyal.datatools.manager.models.ExternalFeedSourceProperty; import com.conveyal.datatools.manager.models.FeedRetrievalMethod; import com.conveyal.datatools.manager.models.FeedSource; @@ -45,7 +44,6 @@ import static com.conveyal.datatools.common.utils.SparkUtils.getPOJOFromRequestBody; import static com.conveyal.datatools.common.utils.SparkUtils.logMessageAndHalt; import static com.conveyal.datatools.manager.models.ExternalFeedSourceProperty.constructId; -import static com.conveyal.datatools.manager.models.FeedSourceSummary.cleanFeedSourceSummaryForNonAdmins; import static com.conveyal.datatools.manager.models.transform.NormalizeFieldTransformation.getInvalidSubstitutionMessage; import static com.conveyal.datatools.manager.models.transform.NormalizeFieldTransformation.getInvalidSubstitutionPatterns; import static com.mongodb.client.model.Filters.in; @@ -88,7 +86,7 @@ private static Collection getProjectFeedSources(Request req, Respons boolean isAdmin = user.canAdministerProject(project); Collection projectFeedSources = project.retrieveProjectFeedSources(); - for (FeedSource source: projectFeedSources) { + for (FeedSource source : projectFeedSources) { String orgId = source.organizationId(); // If user can view or manage feed, add to list of feeds to return. NOTE: By default most users with access // to a project should be able to view all feed sources. Custom privileges would need to be provided to @@ -323,7 +321,7 @@ private static FeedSource deleteFeedSource(Request req, Response res) { /** * Re-fetch this feed from the feed source URL. */ - private static String fetch (Request req, Response res) { + private static String fetch(Request req, Response res) { FeedSource s = requestFeedSourceById(req, Actions.MANAGE); if (s.url == null) { logMessageAndHalt(req, HttpStatus.BAD_REQUEST_400, "Cannot fetch feed source with null URL."); @@ -341,7 +339,8 @@ private static String fetch (Request req, Response res) { /** * Helper function returns feed source if user has permission for specified action. - * @param req spark Request object from API request + * + * @param req spark Request object from API request * @param action action type (either "view" or Permission.MANAGE) * @return feedsource object for ID */ @@ -392,39 +391,79 @@ public static FeedSource checkFeedSourcePermissions(Request req, FeedSource feed return cleanFeedSourceForNonAdmins(feedSource, isProjectAdmin); } - /** Determines whether a change to a feed source is significant enough that it warrants sending a notification + /** + * Determines whether a change to a feed source is significant enough that it warrants sending a notification * * @param formerFeedSource A feed source object, without new changes * @param updatedFeedSource A feed source object, with new changes - * @return A boolean value indicating if the updated feed source is changed enough to warrant sending a notification. + * @return A boolean value indicating if the updated feed source is changed enough to warrant sending a notification. */ private static boolean shouldNotifyUsersOnFeedUpdated(FeedSource formerFeedSource, FeedSource updatedFeedSource) { - return - // If only labels have changed, don't send out an email - formerFeedSource.equalsExceptLabels(updatedFeedSource); + // If only labels have changed, don't send out an email. + return formerFeedSource.equalsExceptLabels(updatedFeedSource); } /** * Removes labels and notes from a feed that a user is not allowed to view. Returns cleaned feed source. - * @param feedSource The feed source to clean - * @param isAdmin Is the user an admin? Changes what is returned. - * @return A feed source containing only labels/notes the user is allowed to see + * + * @param feedSource The feed source to clean. + * @param isAdmin Is the user an admin? Changes what is returned. + * @return A feed source containing only labels/notes the user is allowed to see. */ protected static FeedSource cleanFeedSourceForNonAdmins(FeedSource feedSource, boolean isAdmin) { // Admin can view all feed labels, but a non-admin should only see those with adminOnly=false - feedSource.labelIds = Persistence.labels - .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", feedSource.labelIds), isAdmin)).stream() + feedSource.labelIds = cleanFeedSourceLabelIdsForNonAdmins(feedSource.labelIds, isAdmin); + feedSource.noteIds = cleanFeedSourceNotesForNonAdmins(feedSource.noteIds, isAdmin); + return feedSource; + } + + /** + * Removes labels and notes from a feed that a user is not allowed to view. Returns cleaned feed source summary. + * + * @param feedSourceSummary The feed source to clean. + * @param isAdmin Is the user an admin? Changes what is returned. + * @return A feed source summary containing only labels/notes the user is allowed to see. + */ + protected static FeedSourceSummary cleanFeedSourceSummaryForNonAdmins(FeedSourceSummary feedSourceSummary, boolean isAdmin) { + // Admin can view all feed labels, but a non-admin should only see those with adminOnly=false + feedSourceSummary.labelIds = cleanFeedSourceLabelIdsForNonAdmins(feedSourceSummary.labelIds, isAdmin); + feedSourceSummary.noteIds = cleanFeedSourceNotesForNonAdmins(feedSourceSummary.noteIds, isAdmin); + return feedSourceSummary; + } + + /** + * Removes labels from a feed that a user is not allowed to view. Returns cleaned notes. + * + * @param labelIds The labels to clean. + * @param isAdmin Is the user an admin? Changes what is returned. + * @return Labels the user is allowed to see. + */ + protected static List cleanFeedSourceLabelIdsForNonAdmins(List labelIds, boolean isAdmin) { + // Admin can view all feed labels, but a non-admin should only see those with adminOnly=false. + return Persistence.labels + .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", labelIds), isAdmin)).stream() .map(label -> label.id) .collect(Collectors.toList()); - feedSource.noteIds = Persistence.notes - .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", feedSource.noteIds), isAdmin)).stream() + } + + /** + * Removes notes from a feed that a user is not allowed to view. Returns cleaned notes. + * + * @param noteIds The notes to clean. + * @param isAdmin Is the user an admin? Changes what is returned. + * @return Notes the user is allowed to see. + */ + protected static List cleanFeedSourceNotesForNonAdmins(List noteIds, boolean isAdmin) { + // Admin can view all feed notes, but a non-admin should only see those with adminOnly=false. + return Persistence.notes + .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", noteIds), isAdmin)).stream() .map(note -> note.id) .collect(Collectors.toList()); - return feedSource; } + // FIXME: use generic API controller and return JSON documents via BSON/Mongo - public static void register (String apiPrefix) { + public static void register(String apiPrefix) { get(apiPrefix + "secure/feedsource/:id", FeedSourceController::getFeedSource, json::write); get(apiPrefix + "secure/feedsource", FeedSourceController::getProjectFeedSources, json::write); post(apiPrefix + "secure/feedsource", FeedSourceController::createFeedSource, json::write); diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java index 6116bac3f..15f93e57b 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSourceSummary.java @@ -2,7 +2,6 @@ import com.conveyal.datatools.editor.utils.JacksonSerializers; import com.conveyal.datatools.manager.persistence.Persistence; -import com.conveyal.datatools.manager.utils.PersistenceUtils; import com.conveyal.gtfs.validator.ValidationResult; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; @@ -21,7 +20,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import static com.mongodb.client.model.Aggregates.group; import static com.mongodb.client.model.Aggregates.limit; @@ -93,26 +91,6 @@ public FeedSourceSummary(String projectId, String organizationId, Document feedS this.url = feedSourceDocument.getString("url"); } - /** - * Removes labels and notes from a feed that a user is not allowed to view. Returns cleaned feed source. - * @param feedSourceSummary The feed source to clean - * @param isAdmin Is the user an admin? Changes what is returned. - * @return A feed source containing only labels/notes the user is allowed to see - */ - public static FeedSourceSummary cleanFeedSourceSummaryForNonAdmins(FeedSourceSummary feedSourceSummary, boolean isAdmin) { - // Admin can view all feed labels, but a non-admin should only see those with adminOnly=false - feedSourceSummary.labelIds = Persistence.labels - .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", feedSourceSummary.labelIds), isAdmin)).stream() - .map(label -> label.id) - .collect(Collectors.toList()); - feedSourceSummary.noteIds = Persistence.notes - .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", feedSourceSummary.noteIds), isAdmin)).stream() - .map(note -> note.id) - .collect(Collectors.toList()); - return feedSourceSummary; - } - - /** * Set the appropriate feed version. For consistency, if no error count is available set the related number of * issues to null. @@ -573,8 +551,11 @@ public static class LatestValidationResult { public Integer errorCount; - /** Required for JSON de/serializing. **/ - public LatestValidationResult() {} + /** + * Required for JSON de/serializing. + **/ + public LatestValidationResult() { + } LatestValidationResult(FeedVersionSummary feedVersionSummary) { this.feedVersionId = feedVersionSummary.id; From 9c78f73247e650a45f2975c97925bbe6b769f0c4 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Fri, 20 Oct 2023 16:13:36 -0700 Subject: [PATCH 39/40] refactor(GtfsPlusValidation): update wording to match mtc branch --- .../conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java b/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java index c71f5d2f2..55504120c 100644 --- a/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java +++ b/src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java @@ -188,7 +188,7 @@ private static void validateTable( if (tableIsDirections && !gtfsRoutes.isEmpty()) { // After we're done validating all the table values, check if every route was checked off in directions.txt - issues.add(new ValidationIssue(tableId, "route_id", -1, "Directions table does not define direction names for all routes.")); + issues.add(new ValidationIssue(tableId, null, -1, "Directions file doesn't define directions for all routes listed in routes.txt")); } // Add issues for wrong number of columns and for empty rows after processing all rows. // Note: We considered adding an issue for each row, but opted for the single error approach because there's no From 3fc322f1674209faf3aaccc961c5b14e79c02942 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 24 Oct 2023 17:13:16 +0100 Subject: [PATCH 40/40] refactor(Addressed PR feedback): Cleaned constructors --- .../controllers/api/FeedSourceController.java | 6 ++++-- .../datatools/manager/models/Label.java | 18 ++++++++---------- .../datatools/manager/models/Note.java | 4 +--- .../api/FeedSourceControllerTest.java | 2 +- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java b/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java index 628d7815c..361c74f6d 100644 --- a/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java +++ b/src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java @@ -441,7 +441,8 @@ protected static FeedSourceSummary cleanFeedSourceSummaryForNonAdmins(FeedSource protected static List cleanFeedSourceLabelIdsForNonAdmins(List labelIds, boolean isAdmin) { // Admin can view all feed labels, but a non-admin should only see those with adminOnly=false. return Persistence.labels - .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", labelIds), isAdmin)).stream() + .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", labelIds), isAdmin)) + .stream() .map(label -> label.id) .collect(Collectors.toList()); } @@ -456,7 +457,8 @@ protected static List cleanFeedSourceLabelIdsForNonAdmins(List l protected static List cleanFeedSourceNotesForNonAdmins(List noteIds, boolean isAdmin) { // Admin can view all feed notes, but a non-admin should only see those with adminOnly=false. return Persistence.notes - .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", noteIds), isAdmin)).stream() + .getFiltered(PersistenceUtils.applyAdminFilter(in("_id", noteIds), isAdmin)) + .stream() .map(note -> note.id) .collect(Collectors.toList()); } diff --git a/src/main/java/com/conveyal/datatools/manager/models/Label.java b/src/main/java/com/conveyal/datatools/manager/models/Label.java index 0afe2563e..46c265f54 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/Label.java +++ b/src/main/java/com/conveyal/datatools/manager/models/Label.java @@ -42,17 +42,7 @@ public String organizationId () { * Create a new label with auto-gen id. */ public Label (String name, String description, String color, boolean adminOnly, String projectId) { - this(null, name, description, color, adminOnly, projectId); - } - - /** - * Create a new label with provided id. - */ - public Label (String id, String name, String description, String color, boolean adminOnly, String projectId) { super(); - if (id != null) { - this.id = id; - } this.name = name; this.description = description != null ? description : ""; this.color = color != null ? color : "#000000"; @@ -60,6 +50,14 @@ public Label (String id, String name, String description, String color, boolean this.projectId = projectId; } + /** + * Create a new label with provided id. + */ + public Label (String id, String name, String description, String color, boolean adminOnly, String projectId) { + this(name, description, color, adminOnly, projectId); + this.id = id; + } + /** * No-arg constructor to yield an uninitialized label, for dump/restore. * Should not be used in general code. diff --git a/src/main/java/com/conveyal/datatools/manager/models/Note.java b/src/main/java/com/conveyal/datatools/manager/models/Note.java index 438a784b5..d6a745dd6 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/Note.java +++ b/src/main/java/com/conveyal/datatools/manager/models/Note.java @@ -37,9 +37,7 @@ public class Note extends Model implements Serializable { */ public Note(String id, String body, boolean adminOnly) { super(); - if (id != null) { - this.id = id; - } + this.id = id; this.body = body; this.adminOnly = adminOnly; } diff --git a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java index a4c6d8d0a..1350fbe52 100644 --- a/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java +++ b/src/test/java/com/conveyal/datatools/manager/controllers/api/FeedSourceControllerTest.java @@ -518,7 +518,7 @@ private static Note createNote(String id, String body) { } /** - * Helper method to create label. If the id is provided save the label now if not deffer to test to save. + * Helper method to create label. If the id is provided save the label now if not defer to test to save. */ private static Label createLabel(String id, String name, String projectId) { Label label;