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

Update GraphQL a little bit #149

Merged
merged 1 commit into from
Dec 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@
<dependency>
<groupId>com.graphql-java</groupId>
<artifactId>graphql-java</artifactId>
<version>4.2</version>
<version>11.0</version>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ public class GraphQLGtfsSchema {
.type(new GraphQLList(routeType))
.argument(stringArg("namespace"))
.argument(stringArg(SEARCH_ARG))
.argument(intArg(LIMIT_ARG))
.dataFetcher(new NestedJDBCFetcher(
new JDBCFetcher("pattern_stops", "stop_id", null, false),
new JDBCFetcher("patterns", "pattern_id", null, false),
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/conveyal/gtfs/graphql/GraphQLUtil.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package com.conveyal.gtfs.graphql;

import graphql.schema.FieldDataFetcher;
import graphql.schema.GraphQLArgument;
import graphql.schema.GraphQLFieldDefinition;
import graphql.schema.GraphQLList;
import graphql.schema.PropertyDataFetcher;

import static graphql.Scalars.GraphQLFloat;
import static graphql.Scalars.GraphQLInt;
Expand All @@ -17,15 +17,15 @@ public static GraphQLFieldDefinition string (String name) {
return newFieldDefinition()
.name(name)
.type(GraphQLString)
.dataFetcher(new FieldDataFetcher(name))
.dataFetcher(new PropertyDataFetcher(name))
.build();
}

public static GraphQLFieldDefinition intt (String name) {
return newFieldDefinition()
.name(name)
.type(GraphQLInt)
.dataFetcher(new FieldDataFetcher(name))
.dataFetcher(new PropertyDataFetcher(name))
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import java.util.HashMap;
import java.util.Map;

import static com.conveyal.gtfs.graphql.fetchers.JDBCFetcher.validateNamespace;

/**
* Fetch the summary row for a particular loaded feed, based on its namespace.
* This essentially gets the row from the top-level summary table of all feeds that have been loaded into the database.
Expand All @@ -25,6 +27,7 @@ public class FeedFetcher implements DataFetcher {
@Override
public Map<String, Object> get (DataFetchingEnvironment environment) {
String namespace = environment.getArgument("namespace"); // This is the unique table prefix (the "schema").
validateNamespace(namespace);
StringBuilder sqlBuilder = new StringBuilder();
sqlBuilder.append(String.format("select * from feeds where namespace = '%s'", namespace));
Connection connection = null;
Expand Down
65 changes: 35 additions & 30 deletions src/main/java/com/conveyal/gtfs/graphql/fetchers/JDBCFetcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,16 @@ public List<Map<String, Object>> get (DataFetchingEnvironment environment) {
* Handle fetching functionality for a given namespace, set of join values, and arguments. This is broken out from
* the standard get function so that it can be reused in other fetchers (i.e., NestedJdbcFetcher)
*/
List<Map<String, Object>> getResults (String namespace, List<String> parentJoinValues, Map<String, Object> arguments) {
List<Map<String, Object>> getResults (
String namespace,
List<String> parentJoinValues,
Map<String, Object> graphQLQueryArguments
) {
// Track the parameters for setting prepared statement parameters
List<String> parameters = new ArrayList<>();
List<String> preparedStatementParameters = new ArrayList<>();
// This will contain one Map<String, Object> for each row fetched from the database table.
List<Map<String, Object>> results = new ArrayList<>();
if (arguments == null) arguments = new HashMap<>();
if (graphQLQueryArguments == null) graphQLQueryArguments = new HashMap<>();
// Ensure namespace exists and is clean. Note: FeedFetcher will have executed before this and validated that an
// entry exists in the feeds table and the schema actually exists in the database.
validateNamespace(namespace);
Expand All @@ -188,31 +192,32 @@ List<Map<String, Object>> getResults (String namespace, List<String> parentJoinV
sqlBuilder.append("select *");

// We will build up additional sql clauses in this List (note: must be a List so that the order is preserved).
List<String> conditions = new ArrayList<>();
List<String> whereConditions = new ArrayList<>();
// The order by clause will go here.
String sortBy = "";
// If we are fetching an item nested within a GTFS entity in the GraphQL query, we want to add an SQL "where"
// clause. This could conceivably be done automatically, but it's clearer to just express the intent.
// Note, this is assuming the type of the field in the parent is a string.
if (parentJoinField != null && parentJoinValues != null && !parentJoinValues.isEmpty()) {
conditions.add(makeInClause(parentJoinField, parentJoinValues, parameters));
whereConditions.add(makeInClause(parentJoinField, parentJoinValues, preparedStatementParameters));
}
if (sortField != null) {
// Sort field is not provided by user input, so it's ok to add here (i.e., it's not prone to SQL injection).
sortBy = String.format(" order by %s", sortField);
}
Set<String> argumentKeys = arguments.keySet();
Set<String> argumentKeys = graphQLQueryArguments.keySet();
for (String key : argumentKeys) {
// The pagination, bounding box, and date/time args should all be skipped here because they are handled
// separately below from standard args (pagination becomes limit/offset clauses, bounding box applies to
// stops table, and date/time args filter stop times. All other args become "where X in A, B, C" clauses.
if (argsToSkip.contains(key)) continue;
if (ID_ARG.equals(key)) {
Integer value = (Integer) arguments.get(key);
conditions.add(String.join(" = ", "id", value.toString()));
Integer value = (Integer) graphQLQueryArguments.get(key);
whereConditions.add(String.join(" = ", "id", value.toString()));
} else {
List<String> values = (List<String>) arguments.get(key);
if (values != null && !values.isEmpty()) conditions.add(makeInClause(key, values, parameters));
List<String> values = (List<String>) graphQLQueryArguments.get(key);
if (values != null && !values.isEmpty())
whereConditions.add(makeInClause(key, values, preparedStatementParameters));
}
}
if (argumentKeys.containsAll(boundingBoxArgs)) {
Expand All @@ -222,7 +227,7 @@ List<Map<String, Object>> getResults (String namespace, List<String> parentJoinV
// operating on the patterns table, a SELECT DISTINCT patterns query will be constructed with a join to
// stops and pattern stops.
for (String bound : boundingBoxArgs) {
Double value = (Double) arguments.get(bound);
Double value = (Double) graphQLQueryArguments.get(bound);
// Determine delimiter/equality operator based on min/max
String delimiter = bound.startsWith("max") ? " <= " : " >= ";
// Determine field based on lat/lon
Expand All @@ -232,7 +237,7 @@ List<Map<String, Object>> getResults (String namespace, List<String> parentJoinV
boundsConditions.add(String.join(delimiter, fieldWithNamespace, value.toString()));
}
if ("stops".equals(tableName)) {
conditions.addAll(boundsConditions);
whereConditions.addAll(boundsConditions);
} else if ("patterns".equals(tableName)) {
// Add from table as unique_pattern_ids_in_bounds to match patterns table -> pattern stops -> stops
fromTables.add(
Expand All @@ -243,7 +248,7 @@ List<Map<String, Object>> getResults (String namespace, List<String> parentJoinV
String.join(" and ", boundsConditions),
namespace
));
conditions.add(String.format("%s.patterns.pattern_id = unique_pattern_ids_in_bounds.pattern_id", namespace));
whereConditions.add(String.format("%s.patterns.pattern_id = unique_pattern_ids_in_bounds.pattern_id", namespace));
}
}
if (argumentKeys.contains(DATE_ARG)) {
Expand All @@ -253,36 +258,36 @@ List<Map<String, Object>> getResults (String namespace, List<String> parentJoinV
// service_dates table. In other words, feeds generated by the editor cannot be queried with the date/time args.
String tripsTable = String.format("%s.trips", namespace);
fromTables.add(tripsTable);
String date = getDateArgument(arguments);
String date = getDateArgument(graphQLQueryArguments);
// Gather all service IDs that run on the provided date.
fromTables.add(String.format(
"(select distinct service_id from %s.service_dates where service_date = ?) as unique_service_ids_in_operation",
namespace)
);
// Add date to beginning of parameters list (it is used to pre-select a table in the from clause before any
// other conditions or parameters are appended).
parameters.add(0, date);
preparedStatementParameters.add(0, date);
if (argumentKeys.contains(FROM_ARG) && argumentKeys.contains(TO_ARG)) {
// Determine which trips start in the specified time window by joining to filtered stop times.
String timeFilteredTrips = "trips_beginning_in_time_period";
conditions.add(String.format("%s.trip_id = %s.trip_id", timeFilteredTrips, tripsTable));
whereConditions.add(String.format("%s.trip_id = %s.trip_id", timeFilteredTrips, tripsTable));
// Select all trip IDs that start during the specified time window. Note: the departure and arrival times
// are divided by 86399 to account for trips that begin after midnight. FIXME: Should this be 86400?
fromTables.add(String.format(
"(select trip_id " +
"from (select distinct on (trip_id) * from %s.stop_times order by trip_id, stop_sequence) as first_stop_times " +
"where departure_time %% 86399 >= %d and departure_time %% 86399 <= %d) as %s",
namespace,
(int) arguments.get(FROM_ARG),
(int) arguments.get(TO_ARG),
(int) graphQLQueryArguments.get(FROM_ARG),
(int) graphQLQueryArguments.get(TO_ARG),
timeFilteredTrips));
}
// Join trips to service_dates (unique_service_ids_in_operation).
conditions.add(String.format("%s.service_id = unique_service_ids_in_operation.service_id", tripsTable));
whereConditions.add(String.format("%s.service_id = unique_service_ids_in_operation.service_id", tripsTable));
}
if (argumentKeys.contains(SEARCH_ARG)) {
// Handle string search argument
String value = (String) arguments.get(SEARCH_ARG);
String value = (String) graphQLQueryArguments.get(SEARCH_ARG);
if (!value.isEmpty()) {
// Only apply string search if string is not empty.
Set<String> searchFields = getSearchFields(namespace);
Expand All @@ -292,23 +297,23 @@ List<Map<String, Object>> getResults (String namespace, List<String> parentJoinV
// FIXME: is ILIKE compatible with non-Postgres? LIKE doesn't work well enough (even when setting
// the strings to lower case).
searchClauses.add(String.format("%s ILIKE ?", field));
parameters.add(String.format("%%%s%%", value));
preparedStatementParameters.add(String.format("%%%s%%", value));
}
if (!searchClauses.isEmpty()) {
// Wrap string search in parentheses to isolate from other conditions.
conditions.add(String.format(("(%s)"), String.join(" OR ", searchClauses)));
whereConditions.add(String.format(("(%s)"), String.join(" OR ", searchClauses)));
}
}
}
sqlBuilder.append(String.format(" from %s", String.join(", ", fromTables)));
if (!conditions.isEmpty()) {
if (!whereConditions.isEmpty()) {
sqlBuilder.append(" where ");
sqlBuilder.append(String.join(" and ", conditions));
sqlBuilder.append(String.join(" and ", whereConditions));
}
// The default value for sortBy is an empty string, so it's safe to always append it here. Also, there is no
// threat of SQL injection because the sort field value is not user input.
sqlBuilder.append(sortBy);
Integer limit = (Integer) arguments.get(LIMIT_ARG);
Integer limit = (Integer) graphQLQueryArguments.get(LIMIT_ARG);
if (limit == null) {
limit = autoLimit ? DEFAULT_ROWS_TO_FETCH : -1;
}
Expand All @@ -322,7 +327,7 @@ List<Map<String, Object>> getResults (String namespace, List<String> parentJoinV
} else {
sqlBuilder.append(" limit ").append(limit);
}
Integer offset = (Integer) arguments.get(OFFSET_ARG);
Integer offset = (Integer) graphQLQueryArguments.get(OFFSET_ARG);
if (offset != null && offset >= 0) {
sqlBuilder.append(" offset ").append(offset);
}
Expand All @@ -331,7 +336,7 @@ List<Map<String, Object>> getResults (String namespace, List<String> parentJoinV
connection = GTFSGraphQL.getConnection();
PreparedStatement preparedStatement = connection.prepareStatement(sqlBuilder.toString());
int oneBasedIndex = 1;
for (String parameter : parameters) {
for (String parameter : preparedStatementParameters) {
preparedStatement.setString(oneBasedIndex++, parameter);
}
// This logging produces a lot of noise during testing due to large numbers of joined sub-queries
Expand Down Expand Up @@ -381,7 +386,7 @@ private static String getDateArgument(Map<String, Object> arguments) {
* @param namespace database schema namespace/table prefix
* @return
*/
private static void validateNamespace(String namespace) {
public static void validateNamespace(String namespace) {
if (namespace == null) {
// If namespace is null, do no attempt a query on a namespace that does not exist.
throw new IllegalArgumentException("Namespace prefix must be provided.");
Expand Down Expand Up @@ -437,7 +442,7 @@ private Set<String> getSearchFields(String namespace) {
/**
* Construct filter clause with '=' (single string) and add values to list of parameters.
* */
static String makeInClause(String filterField, String string, List<String> parameters) {
static String filterEquals(String filterField, String string, List<String> parameters) {
// Add string to list of parameters (to be later used to set parameters for prepared statement).
parameters.add(string);
return String.format("%s = ?", filterField);
Expand All @@ -448,7 +453,7 @@ static String makeInClause(String filterField, String string, List<String> param
* */
static String makeInClause(String filterField, List<String> strings, List<String> parameters) {
if (strings.size() == 1) {
return makeInClause(filterField, strings.get(0), parameters);
return filterEquals(filterField, strings.get(0), parameters);
} else {
// Add strings to list of parameters (to be later used to set parameters for prepared statement).
parameters.addAll(strings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import static com.conveyal.gtfs.graphql.GraphQLUtil.intt;
import static com.conveyal.gtfs.graphql.GraphQLUtil.string;
import static com.conveyal.gtfs.graphql.GraphQLUtil.stringArg;
import static com.conveyal.gtfs.graphql.fetchers.JDBCFetcher.makeInClause;
import static com.conveyal.gtfs.graphql.fetchers.JDBCFetcher.filterEquals;
import static graphql.Scalars.GraphQLInt;
import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition;
import static graphql.schema.GraphQLObjectType.newObject;
Expand Down Expand Up @@ -71,8 +70,13 @@ public Object get(DataFetchingEnvironment environment) {
// FIXME Does this handle null cases?
// Add where clause to filter out non-matching results.
String filterValue = (String) parentFeedMap.get(filterField);
String inClause = makeInClause(filterField, filterValue, parameters);
clauses.add(String.join(" ", "where", inClause));
clauses.add(
String.join(
" ",
"where",
filterEquals(filterField, filterValue, parameters)
)
);
} else if (groupByField != null) {
// Handle group by field and optionally handle any filter arguments passed in.
if (!argKeys.isEmpty()) {
Expand All @@ -86,8 +90,13 @@ public Object get(DataFetchingEnvironment environment) {
}
String filterValue = (String) arguments.get(groupedFilterField);
if (filterValue != null) {
String inClause = makeInClause(groupedFilterField, filterValue, parameters);
clauses.add(String.join(" ", "where", inClause));
clauses.add(
String.join(
" ",
"where",
filterEquals(groupedFilterField, filterValue, parameters)
)
);
}
}
// Finally, add group by clause.
Expand Down
Loading