handle feeds with 1 agency & routes.agency_id = null 🐛✅

fixes #45

Author: derhuerst

cli.js

@@ -165,9 +165,9 @@ pipeline(
 		if (!err) return;
 		if (err instanceof DataError) {
 			console.error(String(err))
-			return;
+		} else if (err.code !== 'EPIPE') {
+			console.error(err)
 		}
-		if (err.code !== 'EPIPE') console.error(err)
 		process.exit(1)
 	}
 )

lib/data-error.js

@@ -5,8 +5,7 @@ class DataError extends Error {
 		super(`${fileName}: ${message}`)
 		this.fileName = fileName
 		const expl = Array.isArray(explanation)
-			// todo [breaking]: indent with \t
-			? `\n  ${explanation.join('\n\t')}`
+			? `\n  ${explanation.join('\n  ')}`
 			: ''
 		const asString = `${fileName}: ${message}${expl}`
 		this.toString = () => asString

lib/routes.js

@@ -1,5 +1,7 @@
 'use strict'
 
+const DataError = require('./data-error')
+
 // Google's "Extended GTFS Route Types"
 // https://developers.google.com/transit/gtfs/reference/extended-route-types
 const googleExtendedRouteTypes = [
@@ -288,10 +290,27 @@ COPY "${opt.schema}".routes (
 `
 }
 
-const formatRoutesRow = (r) => {
+const formatRoutesRow = (r, opt, workingState) => {
+	const agency_id = r.agency_id || null
+	if (agency_id === null) {
+		// The GTFS spec allows routes.agency_id to be empty/null if there is exactly one agency in the feed.
+		// It seems that GTFS has allowed this at least since 2016:
+		// https://github.com/google/transit/blame/217e9bf/gtfs/spec/en/reference.md#L544-L554
+		if (workingState.nrOfRowsByName.get('agency') !== 1) {
+			// todo: throw special error indicating an error in the input data
+			throw new DataError(
+				'routes',
+				'agency_id must not be empty/null',
+				[
+					'The GTFS spec allows routes.agency_id to be empty/null only if there is exactly one agency in the feed.'
+				],
+			)
+		}
+	}
+
 	return [
 		r.route_id || null,
-		r.agency_id || null,
+		agency_id,
 		r.route_short_name || null,
 		r.route_long_name || null,
 		r.route_desc || null,

lib/stop_times.js

@@ -161,7 +161,7 @@ $$ LANGUAGE SQL IMMUTABLE;
 CREATE OR REPLACE VIEW "${opt.schema}".arrivals_departures AS
 WITH stop_times_based AS NOT MATERIALIZED (
 	SELECT
-		routes.agency_id,
+		agency.agency_id,
 		trips.route_id,
 		route_short_name,
 		route_long_name,
@@ -212,7 +212,14 @@ WITH stop_times_based AS NOT MATERIALIZED (
 		LEFT JOIN "${opt.schema}".stops stations ON stops.parent_station = stations.stop_id
 		JOIN "${opt.schema}".trips ON s.trip_id = trips.trip_id
 		JOIN "${opt.schema}".routes ON trips.route_id = routes.route_id
-		LEFT JOIN "${opt.schema}".agency ON routes.agency_id = agency.agency_id
+		LEFT JOIN "${opt.schema}".agency ON (
+			-- The GTFS spec allows routes.agency_id to be NULL if there is exactly one agency in the feed.
+			-- Note: We implicitly rely on other parts of the code base to validate that agency has just one row!
+			-- It seems that GTFS has allowed this at least since 2016:
+			-- https://github.com/google/transit/blame/217e9bf/gtfs/spec/en/reference.md#L544-L554
+			routes.agency_id IS NULL -- match first (and only) agency
+			OR routes.agency_id = agency.agency_id -- match by ID
+		)
 		JOIN "${opt.schema}".service_days ON trips.service_id = service_days.service_id
 	)
 	-- todo: this slows down slightly
@@ -422,7 +429,14 @@ WITH stop_times_based AS NOT MATERIALIZED (
 			to_stations.stop_timezone as to_station_tz
 		FROM "${opt.schema}".trips
 		LEFT JOIN "${opt.schema}".routes ON trips.route_id = routes.route_id
-		LEFT JOIN "${opt.schema}".agency ON routes.agency_id = agency.agency_id
+		LEFT JOIN "${opt.schema}".agency ON (
+			-- The GTFS spec allows routes.agency_id to be NULL if there is exactly one agency in the feed.
+			-- Note: We implicitly rely on other parts of the code base to validate that agency has just one row!
+			-- It seems that GTFS has allowed this at least since 2016:
+			-- https://github.com/google/transit/blame/217e9bf/gtfs/spec/en/reference.md#L544-L554
+			routes.agency_id IS NULL -- match first (and only) agency
+			OR routes.agency_id = agency.agency_id -- match by ID
+		)
 		LEFT JOIN "${opt.schema}".stop_times ON trips.trip_id = stop_times.trip_id
 		LEFT JOIN "${opt.schema}".stops from_stops ON stop_times.stop_id = from_stops.stop_id
 		LEFT JOIN "${opt.schema}".stops from_stations ON from_stops.parent_station = from_stations.stop_id

test/calendar-dates-only.sh

@@ -34,3 +34,14 @@ if [[ "$arrN" != "1563629400" ]]; then
 	echo "invalid 2nd t_arrival: $arrN" 1>&2
 	exit 1
 fi
+
+agency_id_null=$(cat << EOF
+select count(*)
+from arrivals_departures
+where agency_id IS NULL
+EOF)
+agency_id_null_count="$(psql --csv -t -c "$agency_id_null")"
+if [[ "$agency_id_null_count" != "0" ]]; then
+	echo ">0 rows with agency_id = null" 1>&2
+	exit 1
+fi

test/calendar-dates-only/routes.txt

@@ -1,2 +1,2 @@
 route_id,agency_id,route_short_name,route_long_name,route_type,route_desc,route_url,route_color,route_text_color,route_sort_order
-A,MTA,Ada,Ada Lovelace Bus Line,3,,,,,
+A,,Ada,Ada Lovelace Bus Line,3,,,,,

test/index.sh

@@ -13,5 +13,6 @@ psql -t -c 'SELECT version()'
 ./postgraphile.sh
 ./routes-without-agency-id.sh
 ./stops-without-level-id.sh
+./invalid-empty-agency-id.sh
 
 echo -e "\n\n✔︎ tests passing"

test/invalid-empty-agency-id.sh

@@ -0,0 +1,14 @@
+#!/bin/bash
+
+set -e
+set -o pipefail
+cd "$(dirname $0)"
+set -x
+
+if ../cli.js -d --trips-without-shape-id -s -- \
+	invalid-empty-agency-id/*.txt >/dev/null; then
+	echo "import didn't fail" 1>&2
+	exit 1
+else
+	echo 'import failed ✔'
+fi

test/invalid-empty-agency-id/agency.txt

@@ -0,0 +1,3 @@
+agency_id,agency_name,agency_url,agency_timezone,agency_lang,agency_phone,agency_fare_url,agency_email
+MTA,Minimal Transit Agency,https://mta.example.org/,Europe/Berlin,,,,
+another,another transit agency,https://another.example.org/,Asia/Kolkata,,,,

test/invalid-empty-agency-id/calendar_dates.txt

@@ -0,0 +1,2 @@
+service_id,date,exception_type
+some-days,20230713,1