Skip to content

Commit

Permalink
refactor(Addressed PR feedback):
Browse files Browse the repository at this point in the history
  • Loading branch information
@br648
br648 committed Jan 22, 2024
1 parent f9500a0 commit 287badf
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 41 deletions.
6 changes: 3 additions & 3 deletions src/main/java/com/conveyal/gtfs/PatternBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Map;
import java.util.Set;

import static com.conveyal.gtfs.loader.JdbcGtfsLoader.copyFromFile;
import static com.conveyal.gtfs.model.Entity.INT_MISSING;
Expand All @@ -40,7 +39,7 @@ public PatternBuilder(Feed feed) throws SQLException {
connection = feed.getConnection();
}

public void create(Map<TripPatternKey, Pattern> patterns, boolean usePatternsFromFeed) {
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 @@ -54,7 +53,8 @@ public void create(Map<TripPatternKey, Pattern> patterns, boolean usePatternsFro
Statement statement = connection.createStatement();
statement.execute(String.format("alter table %s add column pattern_id varchar", tripsTableName));
if (!usePatternsFromFeed) {
// No patterns were loaded from file so the pattern table has not previously been created.
// If no patterns were loaded from file, create the pattern table. Conversely, if the patterns loaded
// from file have been superseded by generated patterns, recreate the table to start afresh.
patternsTable.createSqlTable(connection, null, true);
}
patternStopsTable.createSqlTable(connection, null, true);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/conveyal/gtfs/PatternFinder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public Map<TripPatternKey, Pattern> createPatternObjects(
patternsFromFeedIndex++;
}
if (!usePatternsFromFeed) {
// Name patterns before storing in SQL database if they have not already been provided by via a feed.
// Name patterns before storing in SQL database if they have not already been provided with a feed.
renamePatterns(patterns.values(), stopById);
}
LOG.info("Total patterns: {}", tripsForPattern.keySet().size());
Expand All @@ -116,7 +116,7 @@ public Map<TripPatternKey, Pattern> createPatternObjects(
* 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) {
public boolean canUsePatternsFromFeed(List<Pattern> patternsFromFeed) {
boolean usePatternsFromFeed = patternsFromFeed.size() == tripsForPattern.keySet().size();
LOG.info("Using patterns from feed: {}", usePatternsFromFeed);
return usePatternsFromFeed;
Expand Down
23 changes: 1 addition & 22 deletions src/main/java/com/conveyal/gtfs/validator/PatternFinderValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,20 @@
import com.conveyal.gtfs.PatternFinder;
import com.conveyal.gtfs.TripPatternKey;
import com.conveyal.gtfs.error.SQLErrorStorage;
import com.conveyal.gtfs.loader.BatchTracker;
import com.conveyal.gtfs.loader.Feed;
import com.conveyal.gtfs.loader.Requirement;
import com.conveyal.gtfs.loader.Table;
import com.conveyal.gtfs.model.Pattern;
import com.conveyal.gtfs.model.PatternStop;
import com.conveyal.gtfs.model.Route;
import com.conveyal.gtfs.model.Stop;
import com.conveyal.gtfs.model.StopTime;
import com.conveyal.gtfs.model.Trip;
import org.apache.commons.dbutils.DbUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.PrintStream;
import java.sql.Connection;
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;
import java.util.List;
import java.util.Map;
import java.util.Set;

import static com.conveyal.gtfs.loader.JdbcGtfsLoader.copyFromFile;
import static com.conveyal.gtfs.model.Entity.INT_MISSING;
import static com.conveyal.gtfs.model.Entity.setDoubleParameter;
import static com.conveyal.gtfs.model.Entity.setIntParameter;

/**
* Groups trips together into "patterns" that share the same sequence of stops.
Expand Down Expand Up @@ -84,7 +63,7 @@ public void complete(ValidationResult validationResult) {
}
// Although patterns may have already been loaded from file, the trip patterns are still required.
Map<TripPatternKey, Pattern> patterns = patternFinder.createPatternObjects(stopById, patternsFromFeed, errorStorage);
patternBuilder.create(patterns, !patternsFromFeed.isEmpty());
patternBuilder.create(patterns, patternFinder.canUsePatternsFromFeed(patternsFromFeed));
}
}

65 changes: 51 additions & 14 deletions src/test/java/com/conveyal/gtfs/validator/PatternFinderValidatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,21 @@

import com.conveyal.gtfs.GTFSFeed;
import com.conveyal.gtfs.TestUtils;
import com.conveyal.gtfs.loader.EntityPopulator;
import com.conveyal.gtfs.loader.FeedLoadResult;
import com.conveyal.gtfs.loader.JDBCTableReader;
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.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;

import static com.conveyal.gtfs.GTFS.load;
import static com.conveyal.gtfs.GTFS.validate;
Expand Down Expand Up @@ -49,36 +50,72 @@ void canUseFeedPatterns() throws SQLException {
FeedLoadResult feedLoadResult = load(fileName, testDataSource);
String testNamespace = feedLoadResult.uniqueIdentifier;
validate(testNamespace, testDataSource);
checkPatternStops(fileName, testNamespace);
checkPatternStopsAgainstFeedPatterns(fileName, testNamespace);
}

/**
* Remove one pattern from the feed so that there is a mismatch between the patterns loaded and the patterns
* generated. This will result in the generated patterns taking precedence over the loaded patterns.
*/
@Test
void canRevertToGeneratedPatterns() throws SQLException, IOException {
void canRevertToGeneratedPatterns() throws SQLException {
String fileName = getResourceFileName("real-world-gtfs-feeds/RABA.zip");
FeedLoadResult feedLoadResult = load(fileName, testDataSource);
String testNamespace = feedLoadResult.uniqueIdentifier;
String patternIdToExclude = "2k3j";
executeSqlStatement(String.format(
"delete from %s where pattern_id = '%s'",
String.format("%s.%s", testNamespace, Table.PATTERNS.name),
patternIdToExclude
));
validate(testNamespace, testDataSource);
JDBCTableReader<Pattern> patterns = new JDBCTableReader(Table.PATTERNS,
testDataSource,
testNamespace + ".",
EntityPopulator.PATTERN
);
for (Pattern pattern : patterns.getAllOrdered()) {
assertThatSqlQueryYieldsRowCountGreaterThanZero(generateSql(testNamespace, pattern.pattern_id));
}
}

@Test
void canUseGeneratedPatterns() 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);
checkPatternStopsAgainstFeedPatterns(zipFileName, testNamespace);
}

private void checkPatternStops(String zipFileName, String testNamespace) throws SQLException {
private void checkPatternStopsAgainstFeedPatterns(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
)
);
assertThatSqlQueryYieldsRowCountGreaterThanZero(generateSql(testNamespace, pattern.pattern_id));
}
}

private String generateSql(String testNamespace, String patternId) {
return String.format(
"select * from %s where pattern_id = '%s'",
String.format("%s.%s", testNamespace, Table.PATTERN_STOP.name),
patternId
);
}

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

private void executeSqlStatement(String sql) throws SQLException {
Connection connection = testDataSource.getConnection();
Statement statement = connection.createStatement();
statement.execute(sql);
statement.close();
connection.commit();
}
}

0 comments on commit 287badf

Please sign in to comment.