Skip to content

Commit 5edaf7b

Browse files
drewdairees
andauthored
TransferStopLocationTypeCheck (#444)
* TransferStopLocationTypeCheck * hook up to copier * // For transfer_type 4,5: fields are optional but must be location_type=0 if provided // For other transfer_types: fields are required and must be location_type=0,1 * Test data --------- Co-authored-by: Ian Rees <ian@ianrees.net>
1 parent f916dab commit 5edaf7b

File tree

6 files changed

+247
-0
lines changed

6 files changed

+247
-0
lines changed

copier/copier.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,7 @@ func NewCopier(ctx context.Context, reader adapters.Reader, writer adapters.Writ
283283
&rules.ParentStationLocationTypeCheck{},
284284
&rules.CalendarDuplicateDates{},
285285
&rules.FareProductRiderCategoryDefaultCheck{},
286+
&rules.TransferStopLocationTypeCheck{},
286287
)
287288
}
288289

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package rules
2+
3+
import (
4+
"fmt"
5+
6+
"github.com/interline-io/transitland-lib/gtfs"
7+
"github.com/interline-io/transitland-lib/tt"
8+
)
9+
10+
// TransferStopLocationTypeError reports when a transfer references a stop with an invalid location_type.
11+
type TransferStopLocationTypeError struct {
12+
StopID string
13+
FieldName string // from_stop_id or to_stop_id
14+
LocationType int
15+
TransferType int64
16+
}
17+
18+
func (e *TransferStopLocationTypeError) Error() string {
19+
var requiredType string
20+
if e.TransferType == 4 || e.TransferType == 5 {
21+
requiredType = "0 (stop/platform)"
22+
} else {
23+
requiredType = "0 (stop/platform) or 1 (station)"
24+
}
25+
return fmt.Sprintf(
26+
"transfer field '%s' references stop '%s' which has location_type %d but must be %s",
27+
e.FieldName,
28+
e.StopID,
29+
e.LocationType,
30+
requiredType,
31+
)
32+
}
33+
34+
// TransferStopLocationTypeCheck checks that stops referenced in transfers.txt have valid location_types.
35+
// According to GTFS spec:
36+
// - For transfer_type 0,1,2,3: stops must have location_type 0 (stop/platform) or 1 (station)
37+
// - For transfer_type 4,5: fields are optional, but when provided must have location_type 0 (stop/platform)
38+
type TransferStopLocationTypeCheck struct {
39+
locationTypes map[string]int
40+
}
41+
42+
func (e *TransferStopLocationTypeCheck) AfterWrite(eid string, ent tt.Entity, emap *tt.EntityMap) error {
43+
if e.locationTypes == nil {
44+
e.locationTypes = map[string]int{}
45+
}
46+
if stop, ok := ent.(*gtfs.Stop); ok {
47+
e.locationTypes[eid] = stop.LocationType.Int()
48+
}
49+
return nil
50+
}
51+
52+
// isValidTransferLocationType checks if the location_type is valid based on transfer_type:
53+
// - For transfer_type 0,1,2,3: location_type must be 0 (stop/platform) or 1 (station)
54+
// - For transfer_type 4,5: location_type must be 0 (stop/platform)
55+
func isValidTransferLocationType(locationType int, transferType int64) bool {
56+
if transferType == 4 || transferType == 5 {
57+
return locationType == 0
58+
}
59+
return locationType == 0 || locationType == 1
60+
}
61+
62+
func (e *TransferStopLocationTypeCheck) Validate(ent tt.Entity) []error {
63+
transfer, ok := ent.(*gtfs.Transfer)
64+
if !ok {
65+
return nil
66+
}
67+
68+
var errs []error
69+
70+
// For transfer_type 4,5: fields are optional but must be location_type=0 if provided
71+
// For other transfer_types: fields are required and must be location_type=0,1
72+
if fromType, ok := e.locationTypes[transfer.FromStopID.Val]; ok {
73+
if !isValidTransferLocationType(fromType, transfer.TransferType.Val) {
74+
errs = append(errs, &TransferStopLocationTypeError{
75+
StopID: transfer.FromStopID.Val,
76+
FieldName: "from_stop_id",
77+
LocationType: fromType,
78+
TransferType: transfer.TransferType.Val,
79+
})
80+
}
81+
}
82+
83+
if toType, ok := e.locationTypes[transfer.ToStopID.Val]; ok {
84+
if !isValidTransferLocationType(toType, transfer.TransferType.Val) {
85+
errs = append(errs, &TransferStopLocationTypeError{
86+
StopID: transfer.ToStopID.Val,
87+
FieldName: "to_stop_id",
88+
LocationType: toType,
89+
TransferType: transfer.TransferType.Val,
90+
})
91+
}
92+
}
93+
94+
return errs
95+
}
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package rules
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/interline-io/transitland-lib/gtfs"
8+
"github.com/interline-io/transitland-lib/tt"
9+
)
10+
11+
func TestTransferStopLocationTypeCheck(t *testing.T) {
12+
checker := TransferStopLocationTypeCheck{}
13+
14+
// Setup stops with different location types
15+
stops := []gtfs.Stop{
16+
{StopID: tt.NewString("stop0"), LocationType: tt.NewInt(0)}, // valid - stop/platform
17+
{StopID: tt.NewString("stop1"), LocationType: tt.NewInt(1)}, // valid - station
18+
{StopID: tt.NewString("stop2"), LocationType: tt.NewInt(2)}, // invalid - entrance
19+
{StopID: tt.NewString("stop3"), LocationType: tt.NewInt(3)}, // invalid - generic node
20+
{StopID: tt.NewString("stop4"), LocationType: tt.NewInt(4)}, // invalid - boarding area
21+
}
22+
23+
// Add stops to checker
24+
for _, stop := range stops {
25+
checker.AfterWrite(stop.StopID.Val, &stop, nil)
26+
}
27+
28+
testcases := []struct {
29+
name string
30+
transfer gtfs.Transfer
31+
wantErrors int // expected number of errors
32+
errorContains []string // expected substrings in error messages
33+
}{
34+
{
35+
name: "stop_to_stop_transfer",
36+
transfer: gtfs.Transfer{
37+
FromStopID: tt.NewKey("stop0"),
38+
ToStopID: tt.NewKey("stop0"),
39+
TransferType: tt.NewInt(2),
40+
},
41+
wantErrors: 0,
42+
},
43+
{
44+
name: "stop_to_station_transfer",
45+
transfer: gtfs.Transfer{
46+
FromStopID: tt.NewKey("stop0"),
47+
ToStopID: tt.NewKey("stop1"),
48+
TransferType: tt.NewInt(2),
49+
},
50+
wantErrors: 0,
51+
},
52+
{
53+
name: "station_to_station_transfer",
54+
transfer: gtfs.Transfer{
55+
FromStopID: tt.NewKey("stop1"),
56+
ToStopID: tt.NewKey("stop1"),
57+
TransferType: tt.NewInt(2),
58+
},
59+
wantErrors: 0,
60+
},
61+
{
62+
name: "entrance_to_generic_node_transfer",
63+
transfer: gtfs.Transfer{
64+
FromStopID: tt.NewKey("stop2"),
65+
ToStopID: tt.NewKey("stop3"),
66+
TransferType: tt.NewInt(2),
67+
},
68+
wantErrors: 2,
69+
errorContains: []string{
70+
"transfer field 'from_stop_id' references stop 'stop2'",
71+
"transfer field 'to_stop_id' references stop 'stop3'",
72+
},
73+
},
74+
{
75+
name: "stop_to_boarding_area_transfer",
76+
transfer: gtfs.Transfer{
77+
FromStopID: tt.NewKey("stop0"),
78+
ToStopID: tt.NewKey("stop4"),
79+
TransferType: tt.NewInt(2),
80+
},
81+
wantErrors: 1,
82+
errorContains: []string{
83+
"transfer field 'to_stop_id' references stop 'stop4'",
84+
},
85+
},
86+
{
87+
name: "transfer_type_no_stops",
88+
transfer: gtfs.Transfer{
89+
FromTripID: tt.NewKey("trip1"),
90+
ToTripID: tt.NewKey("trip2"),
91+
TransferType: tt.NewInt(4),
92+
},
93+
wantErrors: 0,
94+
},
95+
}
96+
97+
for _, tc := range testcases {
98+
t.Run(tc.name, func(t *testing.T) {
99+
errs := checker.Validate(&tc.transfer)
100+
101+
// Check error count
102+
if got := len(errs); got != tc.wantErrors {
103+
t.Errorf("got %d errors, want %d", got, tc.wantErrors)
104+
}
105+
106+
// Check error messages if specified
107+
if len(tc.errorContains) > 0 {
108+
if len(errs) != len(tc.errorContains) {
109+
t.Errorf("got %d errors, want %d", len(errs), len(tc.errorContains))
110+
}
111+
for i, want := range tc.errorContains {
112+
if i >= len(errs) {
113+
break
114+
}
115+
if !strings.Contains(errs[i].Error(), want) {
116+
t.Errorf("error %d: got %q, want it to contain %q", i, errs[i].Error(), want)
117+
}
118+
}
119+
}
120+
})
121+
}
122+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Test for TransferStopLocationTypeCheck
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
stop_id,stop_name,stop_desc,stop_lat,stop_lon,zone_id,stop_url,location_type,parent_station,stop_timezone,wheelchair_boarding
2+
12TH,12th St. Oakland City Center,,37.803768,-122.271450,12TH,http://www.bart.gov/stations/12TH/,0,12TH_station,,1
3+
12TH_entrance,test,,37.803768,-122.271450,,,2,12TH_station,,
4+
12TH_node,test,,37.803768,-122.271450,,,3,12TH_station,,
5+
12TH_station,12th St. Oakland City Center,,37.803768,-122.271450,,,1,,,1
6+
19TH,19th St. Oakland,,37.808350,-122.268602,19TH,http://www.bart.gov/stations/19TH/,0,,,1
7+
LAKE,Lake Merritt,,37.797027,-122.265180,LAKE,http://www.bart.gov/stations/LAKE/,0,,,1
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
from_stop_id,to_stop_id,transfer_type,min_transfer_time,from_trip_id,to_trip_id,expect_error
2+
12TH,LAKE,0,600,,
3+
12TH_station,LAKE,0,600,,
4+
12TH_station,LAKE,1,600,,
5+
12TH,LAKE,2,,,
6+
12TH,LAKE,3,,,
7+
12TH,LAKE,4,600,2230435WKDY,2230435WKDY
8+
12TH,LAKE,5,600,2230435WKDY,2230435WKDY
9+
10+
12TH_node,LAKE,0,,,,TransferStopLocationTypeError
11+
12TH_node,LAKE,1,,,,TransferStopLocationTypeError
12+
12TH_node,LAKE,2,,,,TransferStopLocationTypeError
13+
12TH_node,LAKE,3,,,,TransferStopLocationTypeError
14+
12TH_entrance,LAKE,0,,,,TransferStopLocationTypeError
15+
12TH_entrance,LAKE,1,,,,TransferStopLocationTypeError
16+
12TH_entrance,LAKE,2,,,,TransferStopLocationTypeError
17+
12TH_entrance,LAKE,3,,,,TransferStopLocationTypeError
18+
12TH_station,LAKE,4,600,2230435WKDY,2230435WKDY,TransferStopLocationTypeError
19+
12TH_station,LAKE,5,600,2230435WKDY,2230435WKDY,TransferStopLocationTypeError
20+
12TH_node,LAKE,5,600,2230435WKDY,2230435WKDY,TransferStopLocationTypeError
21+
12TH_entrance,LAKE,5,600,2230435WKDY,2230435WKDY,TransferStopLocationTypeError

0 commit comments

Comments
 (0)