GTFS Static > agency timezone > fix infrequent issue - #195
Open
mmathieum wants to merge 12 commits into
Open
Conversation
Happened today in Prod, AI think it might come from static date format instances containing old timezone.
mmathieum
marked this pull request as ready for review
August 8, 2026 21:43
PR Summary by QodoFix GTFS static schedule timezone drift by using per-request DateFormats
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesAgency timezone flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Provider
participant AgencyUtils
participant GTFSStatusProvider
participant Schedule
Provider->>AgencyUtils: Resolve agency timezone
Provider->>GTFSStatusProvider: Create timezone-specific formatters
GTFSStatusProvider->>GTFSStatusProvider: Parse schedule or frequency timestamps
GTFSStatusProvider->>Schedule: Create timestamps with timezone
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review by Qodo
1. Formatter allocations hot path
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses an infrequent production issue where GTFS static schedule parsing/formatting could use an outdated agency timezone due to cached date formatter instances, potentially leading to cached schedule status being saved with the wrong timezone.
Changes:
- Replace cached
ThreadSafeDateFormatterinstances with newly createdSimpleDateFormat/DateFormatinstances configured per agency timezone. - Thread agency timezone ID and a preconfigured date+time formatter down into schedule parsing to avoid re-reading timezone configuration mid-flow.
- Improve agency timezone fallback behavior by logging a warning when the agency timezone resource cannot be read and falling back to the device timezone.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSStatusProvider.java | Stops caching timezone-bound formatters; creates new timezone-scoped DateFormats per request and threads them into schedule parsing. |
| src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSScheduleTimestampsProvider.java | Aligns schedule timestamp retrieval with the new per-timezone DateFormat approach. |
| src/main/java/org/mtransit/android/commons/provider/agency/AgencyUtils.kt | Adds logging and explicit fallback to device timezone when no agency timezone is configured/readable. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
mmathieum
added a commit
to mtransitapps/mtransit-for-android
that referenced
this pull request
Aug 9, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/org/mtransit/android/commons/data/Schedule.java (1)
76-101: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRestore the previous public constructor overloads.
Line 87 changes the public constructor signature by inserting
localTimeZoneIdbeforesourceLabel. Existing source callers will not compile. Existing binary callers can fail withNoSuchMethodError.Keep overloads with the previous signatures. Delegate them with
localTimeZoneIdset tonull.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/mtransit/android/commons/data/Schedule.java` around lines 76 - 101, Restore the previous public Schedule constructor overloads so callers using the original parameter order remain source- and binary-compatible. Add overloads matching the former signatures and delegate to the current constructor with localTimeZoneId set to null, while preserving the existing overloads that accept localTimeZoneId.src/main/java/org/mtransit/android/commons/data/ScheduleExt.kt (1)
24-34: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep timezone metadata when creating derived schedules.
The derived-schedule factory does not carry
localTimeZoneId. GTFS realtime no-data schedules therefore lose their agency timezone and persist without"tz".
src/main/java/org/mtransit/android/commons/data/ScheduleExt.kt#L24-L34: add timezone support toRouteDirectionStop.makeSchedule()and preserveSchedule.localTimeZoneIdinSchedule.toNoData().src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt#L210-L214: passagencyTimeZoneIdwhen creating the no-data schedule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/mtransit/android/commons/data/ScheduleExt.kt` around lines 24 - 34, The derived-schedule factories must preserve timezone metadata: in src/main/java/org/mtransit/android/commons/data/ScheduleExt.kt#L24-L34, extend RouteDirectionStop.makeSchedule() to accept and pass localTimeZoneId, and ensure Schedule.toNoData() reuses Schedule.localTimeZoneId; in src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt#L210-L214, pass agencyTimeZoneId when creating the no-data schedule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/mtransit/android/commons/data/Schedule.java`:
- Around line 113-116: Update Schedule.getTimeZone() to return localTimeZoneId
when no timestamp is available and the timestamp-derived timezone would
otherwise be null, while preserving the existing timestamp-based timezone
behavior when present.
In
`@src/main/java/org/mtransit/android/commons/provider/GTFSRealTimeProvider.java`:
- Around line 1233-1234: Update the lookup flow around the static timeParser so
its timezone is refreshed on every lookup using the current
AgencyUtils.getAgencyTimeZoneId(context) result before parsing. Do not limit
setTimeZone to first-time initialization, ensuring later device timezone changes
are reflected.
---
Outside diff comments:
In `@src/main/java/org/mtransit/android/commons/data/Schedule.java`:
- Around line 76-101: Restore the previous public Schedule constructor overloads
so callers using the original parameter order remain source- and
binary-compatible. Add overloads matching the former signatures and delegate to
the current constructor with localTimeZoneId set to null, while preserving the
existing overloads that accept localTimeZoneId.
In `@src/main/java/org/mtransit/android/commons/data/ScheduleExt.kt`:
- Around line 24-34: The derived-schedule factories must preserve timezone
metadata: in
src/main/java/org/mtransit/android/commons/data/ScheduleExt.kt#L24-L34, extend
RouteDirectionStop.makeSchedule() to accept and pass localTimeZoneId, and ensure
Schedule.toNoData() reuses Schedule.localTimeZoneId; in
src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt#L210-L214,
pass agencyTimeZoneId when creating the no-data schedule.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e6ff30e-9376-4eaf-9ff1-6887d4f25641
📒 Files selected for processing (16)
src/main/java/org/mtransit/android/commons/data/Schedule.javasrc/main/java/org/mtransit/android/commons/data/ScheduleExt.ktsrc/main/java/org/mtransit/android/commons/provider/CleverDevicesProvider.javasrc/main/java/org/mtransit/android/commons/provider/GTFSRealTimeProvider.javasrc/main/java/org/mtransit/android/commons/provider/NextBusProvider.javasrc/main/java/org/mtransit/android/commons/provider/OneBusAwayProvider.javasrc/main/java/org/mtransit/android/commons/provider/RTCQuebecProvider.javasrc/main/java/org/mtransit/android/commons/provider/ReginaTransitProvider.javasrc/main/java/org/mtransit/android/commons/provider/StmInfoApiProvider.javasrc/main/java/org/mtransit/android/commons/provider/WinnipegTransitProvider.javasrc/main/java/org/mtransit/android/commons/provider/agency/AgencyUtils.ktsrc/main/java/org/mtransit/android/commons/provider/gtfs/GTFSRealTimeProviderExt.ktsrc/main/java/org/mtransit/android/commons/provider/gtfs/GTFSScheduleTimestampsProvider.javasrc/main/java/org/mtransit/android/commons/provider/gtfs/GTFSStatusProvider.javasrc/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.ktsrc/main/res/values/gtfs_real_time_values.xml
💤 Files with no reviewable changes (2)
- src/main/res/values/gtfs_real_time_values.xml
- src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSRealTimeProviderExt.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSScheduleTimestampsProvider.java
- src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSStatusProvider.java
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/main/java/org/mtransit/android/commons/provider/GTFSRealTimeProvider.java:1234
timeParseris a static cached formatter, but its timezone is initialized from the firstContextthat hitsgetTimeParser(). If multiple agencies/providers with different timezones can be used in the same process, this can cause parsing with the wrong timezone. Making the parser instance-scoped avoids cross-provider contamination while preserving caching per provider instance.
timeParser = new ThreadSafeDateFormatter(formatter, Locale.ENGLISH);
final String agencyTimeZoneId = AgencyUtils.getAgencyTimeZoneId(context);
timeParser.setTimeZone(TimeZone.getTimeZone(agencyTimeZoneId));
src/main/java/org/mtransit/android/commons/provider/agency/AgencyUtils.kt:51
getAgencyTimeZoneId()now throws aRuntimeExceptionin DEBUG builds when no agency timezone is configured. This can crash dev/debug builds for apps/tests that relied on the previous fallback behavior (or when upgrading with older modules/resources). Consider keeping a warning but always falling back to the device timezone, even in DEBUG, to avoid hard crashes.
) ?: run {
if (BuildConfig.DEBUG) {
throw RuntimeException("No agency timezone configured!")
}
MTLog.w(LOG_TAG, "No agency timezone configured (using device timezone)!")
src/main/java/org/mtransit/android/commons/data/Schedule.java:838
Schedule.Timestamp.parseJSON()throws in DEBUG when the cached JSON is missinglocalTimeZone. Since older cached data may legitimately lack this field, this can cause crashes after upgrading while debugging. It would be safer to log and fall back to the device timezone in all build types.
if (TextUtils.isEmpty(localTimeZoneId)) {
if (BuildConfig.DEBUG) {
throw new RuntimeException("Timestamp missing timezone in JSON!");
}
MTLog.w(LOG_TAG, "Timestamp missing timezone in JSON (using device TZ) '%s'!", jTimestamp);
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Happened today in Prod, Android Studio Gemini thinks it might come from static date format instances containing old timezone.
Human Android developer is not convinced... 🤔
Some cached data (schedule status) could be saved with wrong timezone...