Add RTD Denver transit support - #1
Open
jbriones95 wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds RTD Denver as a supported transit agency (static GTFS + GTFS-realtime) and updates the feed clients to correctly resolve relative redirects while maintaining HTTPS-only behavior.
Changes:
- Add RTD Denver to
GtfsAgencywith static and realtime feed endpoints. - Update GTFS static and GTFS-RT clients to resolve relative redirects (via
URI.resolve). - Update README documentation to include RTD Denver in the supported agency list.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tool/src/main/kotlin/com/thelightphone/transit/gtfs/GtfsRealtime.kt | Resolves redirect targets (including relative) for GTFS-RT fetches while enforcing HTTPS-only. |
| tool/src/main/kotlin/com/thelightphone/transit/gtfs/GtfsIngestor.kt | Resolves redirect targets (including relative) for GTFS zip HEAD/GET while enforcing HTTPS-only. |
| tool/src/main/kotlin/com/thelightphone/transit/gtfs/GtfsAgency.kt | Adds RTD Denver static + realtime endpoint URLs to the agency enum. |
| tool/README.md | Documents RTD Denver as a supported agency. |
| README.md | Documents RTD Denver as a supported agency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+186
to
+194
| /** Resolves absolute and relative redirects while never following a redirect back to plain HTTP. */ | ||
| private fun secureRealtimeRedirectUrl(currentUrl: String, location: String): String { | ||
| val resolved = URI(currentUrl).resolve(location).toString() | ||
| return if (resolved.startsWith("http://")) { | ||
| "https://" + resolved.removePrefix("http://") | ||
| } else { | ||
| resolved | ||
| } | ||
| } |
Comment on lines
+195
to
+203
| /** Resolves absolute and relative redirects while never following a redirect back to plain HTTP. */ | ||
| private fun secureRedirectUrl(currentUrl: String, location: String): String { | ||
| val resolved = URI(currentUrl).resolve(location).toString() | ||
| return if (resolved.startsWith("http://")) { | ||
| "https://" + resolved.removePrefix("http://") | ||
| } else { | ||
| resolved | ||
| } | ||
| } |
Owner
There was a problem hiding this comment.
Hey Jose! Does this change resolve the HTTP:// vs HTTPS:// issues for other agencies aswell? before I merge the change, I'd like to confirm that RIPTA will still work with these changes (HTTP:// Realtime feed)
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.
Summary
Validation
./gradlew :tool:assembleDebugcould not complete because GitHub Maven credentials are unavailable forcom.thelightphone.lp3keyboard:ui:0.0.16.,workdir