Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pattern name bug fix #402

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/main/java/com/conveyal/gtfs/PatternBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public PatternBuilder(Feed feed) throws SQLException {
connection = feed.getConnection();
}

public void create(Map<TripPatternKey, Pattern> patterns, Set<String> patternIdsLoadedFromFile) {
public void create(Map<TripPatternKey, Pattern> patterns, boolean usePatternsFromFeed) {
String patternsTableName = feed.getTableNameWithSchemaPrefix("patterns");
String tripsTableName = feed.getTableNameWithSchemaPrefix("trips");
String patternStopsTableName = feed.getTableNameWithSchemaPrefix("pattern_stops");
Expand All @@ -53,13 +53,13 @@ public void create(Map<TripPatternKey, Pattern> patterns, Set<String> patternIds
LOG.info("Creating pattern and pattern stops tables.");
Statement statement = connection.createStatement();
statement.execute(String.format("alter table %s add column pattern_id varchar", tripsTableName));
if (patternIdsLoadedFromFile.isEmpty()) {
if (!usePatternsFromFeed) {
// No patterns were loaded from file so the pattern table has not previously been created.
patternsTable.createSqlTable(connection, null, true);
}
patternStopsTable.createSqlTable(connection, null, true);
try (PrintStream patternForTripsFileStream = createTempPatternForTripsTable(tempPatternForTripsTextFile, statement)) {
processPatternAndPatternStops(patternsTable, patternStopsTable, patternForTripsFileStream, patterns, patternIdsLoadedFromFile);
processPatternAndPatternStops(patternsTable, patternStopsTable, patternForTripsFileStream, patterns, usePatternsFromFeed);
}
updateTripPatternIds(tempPatternForTripsTextFile, statement, tripsTableName);
createIndexes(statement, patternsTableName, patternStopsTableName, tripsTableName);
Expand All @@ -80,7 +80,7 @@ private void processPatternAndPatternStops(
Table patternStopsTable,
PrintStream patternForTripsFileStream,
Map<TripPatternKey, Pattern> patterns,
Set<String> patternIdsLoadedFromFile
boolean usePatternsFromFeed
) throws SQLException {
// Generate prepared statements for inserts.
String insertPatternSql = patternsTable.generateInsertSql(true);
Expand All @@ -90,7 +90,7 @@ private void processPatternAndPatternStops(
for (Map.Entry<TripPatternKey, Pattern> entry : patterns.entrySet()) {
Pattern pattern = entry.getValue();
LOG.debug("Batching pattern {}", pattern.pattern_id);
if (!patternIdsLoadedFromFile.contains(pattern.pattern_id)) {
if (!usePatternsFromFeed) {
// Only insert the pattern if it has not already been imported from file.
pattern.setStatementParameters(insertPatternStatement, true);
patternTracker.addBatch();
Expand Down
38 changes: 33 additions & 5 deletions src/main/java/com/conveyal/gtfs/PatternFinder.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,34 @@ public void processTrip(Trip trip, Iterable<StopTime> orderedStopTimes) {
* Once all trips have been processed, call this method to produce the final Pattern objects representing all the
* unique sequences of stops encountered. Returns map of patterns to their keys so that downstream functions can
* make use of trip pattern keys for constructing pattern stops or other derivative objects.
*
* There is no viable relationship between patterns that are loaded from a feed (patternsFromFeed) and patterns
* generated here. Process ordering is used to update the pattern id and name if patterns from a feed are available.
* E.g. The first pattern loaded from a feed will be used to updated the first pattern created here.
*/
public Map<TripPatternKey, Pattern> createPatternObjects(Map<String, Stop> stopById, SQLErrorStorage errorStorage) {
public Map<TripPatternKey, Pattern> createPatternObjects(
Map<String, Stop> stopById,
List<Pattern> patternsFromFeed,
SQLErrorStorage errorStorage
) {
// Make pattern ID one-based to avoid any JS type confusion between an ID of zero vs. null value.
int nextPatternId = 1;
int patternsFromFeedIndex = 0;
boolean usePatternsFromFeed = canUsePatternsFromFeed(patternsFromFeed);
// Create an in-memory list of Patterns because we will later rename them before inserting them into storage.
// Use a LinkedHashMap so we can retrieve the entrySets later in the order of insertion.
Map<TripPatternKey, Pattern> patterns = new LinkedHashMap<>();
// TODO assign patterns sequential small integer IDs (may include route)
for (TripPatternKey key : tripsForPattern.keySet()) {
Collection<Trip> trips = tripsForPattern.get(key);
Pattern pattern = new Pattern(key.stops, trips, null);
// Overwrite long UUID with sequential integer pattern ID
pattern.pattern_id = Integer.toString(nextPatternId++);
if (usePatternsFromFeed) {
pattern.pattern_id = patternsFromFeed.get(patternsFromFeedIndex).pattern_id;
pattern.name = patternsFromFeed.get(patternsFromFeedIndex).name;
} else {
// Overwrite long UUID with sequential integer pattern ID
pattern.pattern_id = Integer.toString(nextPatternId++);
}
// FIXME: Should associated shapes be a single entry?
pattern.associatedShapes = new HashSet<>();
trips.stream().forEach(trip -> pattern.associatedShapes.add(trip.shape_id));
Expand All @@ -87,13 +102,26 @@ public Map<TripPatternKey, Pattern> createPatternObjects(Map<String, Stop> stopB
.setBadValue(pattern.associatedShapes.toString()));
}
patterns.put(key, pattern);
patternsFromFeedIndex++;
}
if (!usePatternsFromFeed) {
// Name patterns before storing in SQL database if they have not already been provided by via a feed.
renamePatterns(patterns.values(), stopById);
}
// Name patterns before storing in SQL database.
renamePatterns(patterns.values(), stopById);
LOG.info("Total patterns: {}", tripsForPattern.keySet().size());
return patterns;
}

/**
* If there is a difference in the number of patterns provided by a feed and the number of patterns generated here,
* the patterns provided by the feed are rejected.
*/
private boolean canUsePatternsFromFeed(List<Pattern> patternsFromFeed) {
boolean usePatternsFromFeed = patternsFromFeed.size() == tripsForPattern.keySet().size();
LOG.info("Using patterns from feed: {}", usePatternsFromFeed);
return usePatternsFromFeed;
}

/**
* Destructively rename the supplied collection of patterns.
* This process requires access to all the stops in the feed.
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/conveyal/gtfs/error/NewGTFSErrorType.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ public enum NewGTFSErrorType {
FIELD_VALUE_TOO_LONG(Priority.MEDIUM, "Field value has too many characters."),

// Shared Stops-specifc errors.
MULTIPLE_SHARED_STOPS_GROUPS(Priority.HIGH, "A GTFS stop belongs to more than one shared-stop group."),
SHARED_STOP_GROUP_MUTLIPLE_PRIMARY_STOPS(Priority.HIGH, "A Shared-stop group has multiple primary stops."),
SHARED_STOP_GROUP_ENTITY_DOES_NOT_EXIST(Priority.MEDIUM, "The stop referenced by a shared-stop does not exist."),
MULTIPLE_SHARED_STOPS_GROUPS(Priority.HIGH, "A GTFS stop belongs to more than one shared-stop group, or belongs to the same shared-stop group twice."),
SHARED_STOP_GROUP_MULTIPLE_PRIMARY_STOPS(Priority.HIGH, "A shared-stop group has multiple primary stops."),
SHARED_STOP_GROUP_ENTITY_DOES_NOT_EXIST(Priority.MEDIUM, "The stop referenced by a shared-stop does not exist in the feed it was said to exist in."),

// Unknown errors.
OTHER(Priority.LOW, "Other errors.");
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,17 @@ private void populateDefaultEditorValues(Connection connection, String tablePref
LOG.info(updateOtherSql);
int calendarsUpdated = statement.executeUpdate(updateOtherSql);
LOG.info("Updated description for {} calendars", calendarsUpdated);

// Check if there are duplicate descriptions, in which case set the description to be the service_id which is unique
String avoidDuplicateDescriptionSql = String.format(
"update %1$scalendar set description = service_id " +
"from (select description, count(*) as duplicate_count from %1$scalendar group by description) as duplicate_descriptions " +
"where %1$scalendar.description = duplicate_descriptions.description and duplicate_descriptions.duplicate_count > 1",
tablePrefix
);
LOG.info(avoidDuplicateDescriptionSql);
int duplicatesAvoided = statement.executeUpdate(avoidDuplicateDescriptionSql);
LOG.info("Updated duplicate descriptions for {} calendars", duplicatesAvoided);
}
if (Table.TRIPS.name.equals(table.name)) {
// Update use_frequency field for patterns. This sets all patterns that have a frequency trip to use
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -1502,7 +1502,7 @@ private void ensureReferentialIntegrity(
// Special case for schedule_exceptions where for exception type 10 and service_id is also a key.
String calendarDateServiceKey = "custom_schedule";
Field calendarDateServiceKeyField = table.getFieldForName(calendarDateServiceKey);
String calendarDateServiceKeyVal = jsonObject.get(calendarDateServiceKey).asText();
String calendarDateServiceKeyVal = jsonObject.get(calendarDateServiceKey).get(0).asText();
TIntSet calendarDateServiceUniqueIds = getIdsForCondition(tableName, calendarDateServiceKey, calendarDateServiceKeyVal, connection);
checkUniqueIdsAndUpdateReferencingTables(
calendarDateServiceUniqueIds,
Expand Down Expand Up @@ -1726,7 +1726,7 @@ private void updateReferencingTables(
connection.rollback();
if (entityClass.getSimpleName().equals("Stop")) {
String patternStopLookup = String.format(
"select distinct p.id, r.id " +
"select distinct p.id, r.id, r.route_short_name, r.route_id " +
"from %s.pattern_stops ps " +
"inner join " +
"%s.patterns p " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -72,19 +73,18 @@ public void validateTrip (Trip trip, Route route, List<StopTime> stopTimes, List
*/
@Override
public void complete(ValidationResult validationResult) {
Set<String> patternIds = new HashSet<>();
List<Pattern> patternsFromFeed = new ArrayList<>();
for(Pattern pattern : feed.patterns) {
patternIds.add(pattern.pattern_id);
patternsFromFeed.add(pattern);
}
LOG.info("Finding patterns...");
Map<String, Stop> stopById = new HashMap<>();
for (Stop stop : feed.stops) {
stopById.put(stop.stop_id, stop);
}
// Although patterns may have already been loaded from file, the trip patterns are still required.
Map<TripPatternKey, Pattern> patterns = patternFinder.createPatternObjects(stopById, errorStorage);
patternBuilder.create(patterns, patternIds);
Map<TripPatternKey, Pattern> patterns = patternFinder.createPatternObjects(stopById, patternsFromFeed, errorStorage);
patternBuilder.create(patterns, !patternsFromFeed.isEmpty());
}

}

Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package com.conveyal.gtfs.validator;

import com.conveyal.gtfs.GTFSFeed;
import com.conveyal.gtfs.TestUtils;
import com.conveyal.gtfs.loader.FeedLoadResult;
import com.conveyal.gtfs.loader.Table;
import com.conveyal.gtfs.model.Pattern;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.sql.DataSource;

import java.io.IOException;
import java.sql.ResultSet;
import java.sql.SQLException;

import static com.conveyal.gtfs.GTFS.load;
import static com.conveyal.gtfs.GTFS.validate;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.greaterThan;


class PatternFinderValidatorTest {

private static final Logger LOG = LoggerFactory.getLogger(PatternFinderValidatorTest.class);

private static String testDBName;

private static DataSource testDataSource;

@BeforeAll
public static void setUpClass() throws Exception {
// create a new database
testDBName = TestUtils.generateNewDB();
String dbConnectionUrl = String.format("jdbc:postgresql://localhost/%s", testDBName);
testDataSource = TestUtils.createTestDataSource(dbConnectionUrl);
}

@AfterAll
public static void tearDownClass() {
TestUtils.dropDB(testDBName);
}

@Test
void canUseFeedPatterns() throws SQLException, IOException {
String zipFileName = TestUtils.zipFolderFiles("real-world-gtfs-feeds/RABA", true);
FeedLoadResult feedLoadResult = load(zipFileName, testDataSource);
String testNamespace = feedLoadResult.uniqueIdentifier;
validate(testNamespace, testDataSource);
checkPatternStops(zipFileName, testNamespace);
}

@Test
void canRevertToGeneratedPatterns() throws SQLException, IOException {
String zipFileName = TestUtils.zipFolderFiles("fake-agency", true);
FeedLoadResult feedLoadResult = load(zipFileName, testDataSource);
String testNamespace = feedLoadResult.uniqueIdentifier;
validate(testNamespace, testDataSource);
checkPatternStops(zipFileName, testNamespace);
}

private void checkPatternStops(String zipFileName, String testNamespace) throws SQLException {
GTFSFeed feed = GTFSFeed.fromFile(zipFileName);
for (String key : feed.patterns.keySet()) {
Pattern pattern = feed.patterns.get(key);
assertThatSqlQueryYieldsRowCountGreaterThanZero(
String.format(
"select * from %s where pattern_id = '%s'",
String.format("%s.%s", testNamespace, Table.PATTERN_STOP.name),
pattern.pattern_id
)
);
}
}

private void assertThatSqlQueryYieldsRowCountGreaterThanZero(String sql) throws SQLException {
int recordCount = 0;
ResultSet rs = testDataSource.getConnection().prepareStatement(sql).executeQuery();
while (rs.next()) recordCount++;
assertThat(recordCount, greaterThan(0));
}
}
2 changes: 2 additions & 0 deletions src/test/resources/real-world-gtfs-feeds/RABA/agency.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
agency_id,agency_name,agency_url,agency_timezone,agency_lang,agency_phone,agency_branding_url,agency_fare_url,agency_email
25,Redding Area Bus Authority,http://www.rabaride.com/,America/Los_Angeles,en,530-241-2877,,,
11 changes: 11 additions & 0 deletions src/test/resources/real-world-gtfs-feeds/RABA/calendar.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
service_id,monday,tuesday,wednesday,thursday,friday,saturday,sunday,start_date,end_date
c_2713_b_80332_d_56,0,0,0,1,1,1,0,20230525,20230902
c_1658_b_18260_d_63,1,1,1,1,1,1,0,20190101,20240101
c_1658_b_18260_d_32,0,0,0,0,0,1,0,20190101,20240101
c_868_b_79978_d_31,1,1,1,1,1,0,0,20230110,20230407
c_868_b_79978_d_30,0,1,1,1,1,0,0,20230110,20230407
c_868_b_79978_d_1,1,0,0,0,0,0,0,20230110,20230407
c_868_b_79977_d_31,1,1,1,1,1,0,0,20230418,20230526
c_868_b_79977_d_30,0,1,1,1,1,0,0,20230418,20230526
c_868_b_79977_d_1,1,0,0,0,0,0,0,20230418,20230526
c_1658_b_18260_d_31,1,1,1,1,1,0,0,20190101,20240101
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
service_id,date,exception_type
c_1658_b_18260_d_31,20230529,2
c_1658_b_18260_d_31,20230904,2
c_1658_b_18260_d_31,20230704,2
c_1658_b_18260_d_63,20230529,2
c_1658_b_18260_d_63,20230904,2
c_1658_b_18260_d_63,20230704,2
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
pattern_id,route_id,name,direction_id,use_frequency,shape_id
2,151,52 stops from Masonic Transfer Center to Masonic Transfer Center (25 trips),0,,p_899986
4,3779,10 stops from Downtown Transit Center to Downtown Transit Center (5 trips),0,,p_9209
5,161,6 stops from Burney (@ Burney Sporting Goods) to Downtown Transit Center (3 trips),1,,p_2671
6,161,6 stops from Downtown Transit Center to Burney (@ Burney Sporting Goods) (3 trips),0,,p_2639
9,1887,15 stops from Downtown Transit Center to Downtown Transit Center (23 trips),1,,p_2631
10,160,31 stops from Downtown Transit Center to Downtown Transit Center (23 trips),0,,p_2637
11,159,26 stops from Canby Transfer Center to Canby Transfer Center (23 trips),1,,p_2644
12,1886,24 stops from Downtown Transit Center to Downtown Transit Center via 2510 Airpark Dr (11 trips),1,,p_2638
13,1886,24 stops from Downtown Transit Center to Downtown Transit Center via Court St at Butte St (12 trips),0,,p_2628
14,153,25 stops from Downtown Transit Center to SR 273 (frontage rd) at Happy Valley Rd (23 trips),1,,p_787964
15,153,16 stops from SR 273 (frontage rd) at Happy Valley Rd to Downtown Transit Center (23 trips),0,,p_787963
16,154,23 stops from Canby Transfer Center to Bechelli Ln at Knollcrest Dr (23 trips),1,,p_788072
17,154,21 stops from Bechelli Ln at Knollcrest Dr to Canby Transfer Center (23 trips),0,,p_788073
18,155,17 stops from Downtown Transit Center to Hartnell Ave at Shasta View Dr (23 trips),0,,p_788074
19,155,22 stops from Hartnell Ave at Shasta View Dr to Downtown Transit Center (23 trips),1,,p_788075
20,1889,12 stops from Canby Transfer Center to Victor Ave at El Vista St (23 trips),1,,p_788078
21,1889,13 stops from Victor Ave at El Vista St to Canby Transfer Center (23 trips),0,,p_788079
22,1888,13 stops from Canby Transfer Center to Shasta College (21 trips),0,,p_788076
23,1888,12 stops from Shasta College to Canby Transfer Center (21 trips),1,,p_788077
24,157,9 stops from Downtown Transit Center to Shasta College (21 trips),0,,p_788080
25,157,16 stops from Shasta College to Downtown Transit Center (21 trips),1,,p_788081
28,6460,3 stops from SR 273 (frontage rd) at Happy Valley Rd to Downtown Transit Center (1 trips),0,,p_788093
29,6460,25 stops from Downtown Transit Center to 5000 Rhonda Rd (1 trips),1,,p_788094
1,6446,21 stops from 5000 Rhonda Rd to SR 273 (frontage rd) at Happy Valley Rd (8 trips),0,,p_110998
2k3j,15,Southbound-5stops,,0,c69d
sfe0,15,Northbound-5:07pm,,0,80g2
ir25,15,Southbound-6:37pm,,0,y7ol
v7wd,6790,Westbound-6:02am,,0,s30y
3,6446,18 stops from SR 273 (frontage rd) at Happy Valley Rd to 5000 Rhonda Rd (7 trips),1,,p_111010
27,6790,3 stops from Downtown Transit Center to Canby Transfer Center (16 trips),0,,p_788089
aioj,6790,Eastbound-4:58am,,0,ce8e
26,6790,3 stops from Canby Transfer Center to Downtown Transit Center (16 trips),1,,p_788088
fm1k,15,Southbound-5:10am,,0,ygd9
0smf,15,Southbound-5:37pm,,0,lt05
mo18,15,Northbound-5stops,,0,nk8m
xadh,15,Northbound-5:35am,,0,qy7v
8,1885,Westbound to Downtown Passenger Terminal,1,,p_4556
5ske,1885,Eastbound to Shasta College,0,,c2ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fare_id,price,currency_type,payment_method,transfers,agency_id,transfer_duration
60,1.50,USD,0,,25,10000
61,2.25,USD,0,,25,10000
62,3.00,USD,0,,25,10000
106,4.50,USD,0,,25,10000
1505,2.00,USD,0,,25,10800
1506,3.50,USD,0,,25,10800
1507,5.00,USD,0,,25,10800
20 changes: 20 additions & 0 deletions src/test/resources/real-world-gtfs-feeds/RABA/fare_rules.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
fare_id,route_id,origin_id,destination_id,contains_id
60,,24,24,
60,,25,25,
60,,26,26,
61,,24,25,
61,,25,24,
61,,25,26,
61,,26,25,
62,,24,26,
62,,26,24,
1505,161,25,878,
1505,161,876,877,
1505,161,877,876,
1505,161,878,25,
1506,161,25,877,
1506,161,876,878,
1506,161,877,25,
1506,161,878,876,
1507,161,25,876,
1507,161,876,25,
Loading
Loading