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

Fix crash when missing stop's local timezone #298

Merged
merged 3 commits into from
Feb 6, 2025
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
5 changes: 2 additions & 3 deletions finders/dbfinder/finder.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ func (f *Finder) RouteStopBuffer(ctx context.Context, param *model.RouteStopBuff

func (f *Finder) FindFeedVersionServiceWindow(ctx context.Context, fvid int) (*model.ServiceWindow, error) {
a, _, err := f.fvslCache.Get(ctx, fvid)
if err != nil {
return nil, err
if err != nil || a == nil || a.Location == nil {
return nil, errors.New("no service window found")
}
// Get local time
nowLocal := time.Now().In(a.Location)
Expand All @@ -166,7 +166,6 @@ func (f *Finder) FindFeedVersionServiceWindow(ctx context.Context, fvid int) (*m
StartDate: a.StartDate,
EndDate: a.EndDate,
FallbackWeek: a.FallbackWeek,
Location: a.Location,
}
return ret, err
}
Expand Down
3 changes: 1 addition & 2 deletions finders/dbfinder/stop_time_select.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,11 @@ func StopTimeFilterExpand(where *model.StopTimeFilter, fvsw *model.ServiceWindow

// Further processing of the StopTimeFilter
if where != nil {
var loc *time.Location
var nowLocal time.Time
if fvsw != nil {
loc = fvsw.Location
nowLocal = fvsw.NowLocal
}
loc := nowLocal.Location()

// Set ServiceDate to local timezone
// ServiceDate is a strict GTFS calendar date
Expand Down
2 changes: 1 addition & 1 deletion finders/dbfinder/trip_select.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TripSelect(limit *int, after *model.Cursor, ids []int, active bool, permFil
if dow < 0 {
dow = 6
}
where.ServiceDate = tzTruncate(fvsw.FallbackWeek.AddDate(0, 0, dow), fvsw.Location)
where.ServiceDate = tzTruncate(fvsw.FallbackWeek.AddDate(0, 0, dow), fvsw.NowLocal.Location())
}
}
}
Expand Down
11 changes: 5 additions & 6 deletions finders/dbfinder/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package dbfinder
import (
"fmt"
"regexp"
"strconv"
"strings"
"time"
"unicode"

sq "github.com/Masterminds/squirrel"
"github.com/interline-io/log"
"github.com/interline-io/transitland-lib/tt"
"github.com/interline-io/transitland-server/model"
)
Expand All @@ -35,6 +35,10 @@ func kebabize(a string) string {
}

func tzTruncate(s time.Time, loc *time.Location) *tt.Date {
if loc == nil {
log.Error().Msg("tzTruncate: loc is nil, set to UTC")
loc = time.UTC
}
return ptr(tt.NewDate(time.Date(s.Year(), s.Month(), s.Day(), 0, 0, 0, 0, loc)))
}

Expand Down Expand Up @@ -62,11 +66,6 @@ func checkFloat(v *float64, min float64, max float64) float64 {
return *v
}

func atoi(v string) int {
a, _ := strconv.Atoi(v)
return a
}

// unicode aware remove all non-alphanumeric characters
// this is not for escaping sql; just for preparing to_tsquery
func alphanumeric(v string) string {
Expand Down
20 changes: 15 additions & 5 deletions finders/rtfinder/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,19 +105,27 @@ func (f *lookupCache) StopTimezone(ctx context.Context, id int, known string) (*

// If a timezone is provided, save it and return immediately
if known != "" {
log.For(ctx).Trace().Int("stop_id", id).Str("known", known).Msg("tz: using known timezone")
log.TraceCheck(func() {
log.For(ctx).Trace().Int("stop_id", id).Str("known", known).Msg("tz: using known timezone")
})
return f.tzCache.Add(id, known)
}

// Check the cache
if loc, ok := f.tzCache.Get(id); ok {
log.For(ctx).Trace().Int("stop_id", id).Str("known", known).Str("loc", loc.String()).Msg("tz: using cached timezone")
log.TraceCheck(func() {
log.For(ctx).Trace().Int("stop_id", id).Str("known", known).Str("loc", loc.String()).Msg("tz: using cached timezone")
})
return loc, ok
} else {
log.For(ctx).Trace().Int("stop_id", id).Str("known", known).Str("loc", loc.String()).Msg("tz: timezone not in cache")
log.TraceCheck(func() {
log.For(ctx).Trace().Int("stop_id", id).Str("known", known).Str("loc", loc.String()).Msg("tz: timezone not in cache")
})
}
if id == 0 {
log.For(ctx).Trace().Int("stop_id", id).Msg("tz: lookup failed, cant find timezone for stops with id=0 unless speciifed explicitly")
log.TraceCheck(func() {
log.For(ctx).Trace().Int("stop_id", id).Msg("tz: lookup failed, cant find timezone for stops with id=0 unless speciifed explicitly")
})
return nil, false
}
// Otherwise lookup the timezone
Expand All @@ -139,7 +147,9 @@ func (f *lookupCache) StopTimezone(ctx context.Context, id int, known string) (*
return nil, false
}
loc, ok := f.tzCache.Add(id, tz)
log.For(ctx).Trace().Int("stop_id", id).Str("known", known).Str("loc", loc.String()).Msg("tz: lookup successful")
log.TraceCheck(func() {
log.For(ctx).Trace().Int("stop_id", id).Str("known", known).Str("loc", loc.String()).Msg("tz: lookup successful")
})
return loc, ok
}

Expand Down
10 changes: 9 additions & 1 deletion internal/clock/clock.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package clock

import "time"
import (
"time"

"github.com/interline-io/log"
)

// Allow for time mocking
type Clock interface {
Expand All @@ -26,5 +30,9 @@ func (dc *Mock) Now() time.Time {
// Helpers

func tzTruncate(s time.Time, loc *time.Location) time.Time {
if loc == nil {
log.Error().Msg("tzTruncate: loc is nil, set to UTC")
loc = time.UTC
}
return time.Date(s.Year(), s.Month(), s.Day(), 0, 0, 0, 0, loc)
}
8 changes: 5 additions & 3 deletions internal/clock/swcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,19 @@ type ServiceWindow struct {
type ServiceWindowCache struct {
db sqlx.Ext
lock sync.Mutex
fvslWindows map[int]ServiceWindow
fvslWindows map[int]*ServiceWindow
tzCache *tzcache.Cache[int]
}

func NewServiceWindowCache(db sqlx.Ext) *ServiceWindowCache {
return &ServiceWindowCache{
db: db,
fvslWindows: map[int]ServiceWindow{},
fvslWindows: map[int]*ServiceWindow{},
tzCache: tzcache.NewCache[int](),
}
}

func (f *ServiceWindowCache) Get(ctx context.Context, fvid int) (ServiceWindow, bool, error) {
func (f *ServiceWindowCache) Get(ctx context.Context, fvid int) (*ServiceWindow, bool, error) {
f.lock.Lock()
defer f.lock.Unlock()
a, ok := f.fvslWindows[fvid]
Expand All @@ -51,6 +51,8 @@ func (f *ServiceWindowCache) Get(ctx context.Context, fvid int) (ServiceWindow,
if fvData.Location == nil {
return a, false, fmt.Errorf("unable to get cached default timezone for feed version %d", fvid)
}

a = &ServiceWindow{}
a.Location = fvData.Location

// Get fallback week from FVSL data
Expand Down
1 change: 0 additions & 1 deletion model/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ type ServiceWindow struct {
StartDate time.Time
EndDate time.Time
FallbackWeek time.Time
Location *time.Location
}

type StopPlaceParam struct {
Expand Down
4 changes: 2 additions & 2 deletions server/gql/stop_time_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (r *stopTimeResolver) Trip(ctx context.Context, obj *model.StopTime) (*mode
func (r *stopTimeResolver) Arrival(ctx context.Context, obj *model.StopTime) (*model.StopTimeEvent, error) {
// Lookup timezone
loc, ok := model.ForContext(ctx).RTFinder.StopTimezone(ctx, obj.StopID.Int(), "")
if !ok {
if loc == nil || !ok {
return nil, errors.New("timezone not available for stop")
}
// Create arrival; fallback to RT departure if arrival is not present
Expand All @@ -73,7 +73,7 @@ func (r *stopTimeResolver) Arrival(ctx context.Context, obj *model.StopTime) (*m
func (r *stopTimeResolver) Departure(ctx context.Context, obj *model.StopTime) (*model.StopTimeEvent, error) {
// Lookup timezone
loc, ok := model.ForContext(ctx).RTFinder.StopTimezone(ctx, obj.StopID.Int(), "")
if !ok {
if loc == nil || !ok {
return nil, errors.New("timezone not available for stop")
}
// Create departure; fallback to RT arrival if departure is not present
Expand Down
Loading