Train API refactor by lbkulinski · Pull Request #45 · lbkulinski/cta4j-java-sdk

lbkulinski · 2026-03-07T20:08:10Z

No description provided.

Copilot

Pull request overview

Refactors the train portion of the SDK from the legacy TrainClient/model/mapper set to a new TrainApi with dedicated sub-APIs (StationsApi, ArrivalsApi), MapStruct-based wire→domain mapping, and shared internal utilities (HTTP client, object mapper, geo constants).

Changes:

Introduces TrainApi with StationsApi/ArrivalsApi, new domain models, wire DTOs, and MapStruct mappers/qualifiers.
Replaces/removes legacy train client and related models/mappers/external DTOs.
Unifies internal infrastructure by moving HTTP GET to com.cta4j.internal.http.HttpClient, adding a shared Cta4jObjectMapper, and centralizing geo validation constants.

Reviewed changes

Copilot reviewed 77 out of 77 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/main/java/com/cta4j/util/DateTimeUtils.java	Removed legacy train timestamp parsing utility (logic moved into train mapping qualifiers).
src/main/java/com/cta4j/train/station/model/Station.java	Adds new station domain model with immutability/null checks.
src/main/java/com/cta4j/train/station/model/Location.java	Adds station location domain model (lat/lon + optional human address).
src/main/java/com/cta4j/train/station/model/HumanAddress.java	Adds parsed “human address” domain model.
src/main/java/com/cta4j/train/station/model/CardinalDirection.java	Adds cardinal direction enum for station direction.
src/main/java/com/cta4j/train/station/internal/wire/CtaStation.java	Adds wire DTO for Chicago Data Portal station payload.
src/main/java/com/cta4j/train/station/internal/wire/CtaLocation.java	Adds wire DTO for station location payload.
src/main/java/com/cta4j/train/station/internal/mapper/StationMapper.java	Adds MapStruct mapper for station wire→domain mapping.
src/main/java/com/cta4j/train/station/internal/impl/StationsApiImpl.java	Implements station listing via configured stations URL.
src/main/java/com/cta4j/train/station/StationsApi.java	Adds public Stations API surface.
src/main/java/com/cta4j/train/model/UpcomingTrainArrival.java	Removes legacy train arrival model.
src/main/java/com/cta4j/train/model/TrainCoordinates.java	Removes legacy train coordinates model.
src/main/java/com/cta4j/train/model/Train.java	Removes legacy train aggregate model.
src/main/java/com/cta4j/train/model/StationArrival.java	Removes legacy station arrival model.
src/main/java/com/cta4j/train/model/Route.java	Removes legacy train route enum (replaced by TrainLine).
src/main/java/com/cta4j/train/mapper/UpcomingTrainArrivalMapper.java	Removes legacy follow-ETA mapper.
src/main/java/com/cta4j/train/mapper/TrainCoordinatesMapper.java	Removes legacy position mapper.
src/main/java/com/cta4j/train/mapper/StationArrivalMapper.java	Removes legacy arrivals mapper.
src/main/java/com/cta4j/train/mapper/RouteMapper.java	Removes legacy route mapping helper.
src/main/java/com/cta4j/train/internal/wire/CtaResponse.java	Adds generic “ctatt” response wrapper for train endpoints.
src/main/java/com/cta4j/train/internal/wire/CtaError.java	Adds structured error type for train endpoint error handling.
src/main/java/com/cta4j/train/internal/util/ApiUtils.java	Adds shared train API constants and error-message formatting.
src/main/java/com/cta4j/train/internal/mapper/Qualifiers.java	Adds MapStruct qualifiers for train mapping (directions, lines, timestamps, booleans, etc.).
src/main/java/com/cta4j/train/internal/impl/TrainApiImpl.java	Implements TrainApi composition + builder defaults (host/stationsUrl/objectMapper).
src/main/java/com/cta4j/train/internal/context/TrainApiContext.java	Adds train API context record (host/url/key/objectMapper).
src/main/java/com/cta4j/train/external/follow/CtaFollowResponse.java	Removes legacy follow endpoint DTO.
src/main/java/com/cta4j/train/external/follow/CtaFollowPosition.java	Removes legacy follow endpoint DTO.
src/main/java/com/cta4j/train/external/follow/CtaFollowEta.java	Removes legacy follow endpoint DTO.
src/main/java/com/cta4j/train/external/follow/CtaFollowCtatt.java	Removes legacy follow endpoint DTO.
src/main/java/com/cta4j/train/external/arrival/CtaArrivalsResponse.java	Removes legacy arrivals endpoint DTO.
src/main/java/com/cta4j/train/external/arrival/CtaArrivalsEta.java	Removes legacy arrivals endpoint DTO.
src/main/java/com/cta4j/train/external/arrival/CtaArrivalsCtatt.java	Removes legacy arrivals endpoint DTO.
src/main/java/com/cta4j/train/common/model/TrainLine.java	Adds unified train line enum (code + color).
src/main/java/com/cta4j/train/client/internal/TrainClientImpl.java	Removes legacy TrainClient implementation.
src/main/java/com/cta4j/train/client/TrainClient.java	Removes legacy TrainClient public API.
src/main/java/com/cta4j/train/arrival/query/StopArrivalQuery.java	Adds stop-based arrivals query type + builder.
src/main/java/com/cta4j/train/arrival/query/MapArrivalQuery.java	Adds map-based arrivals query type + builder.
src/main/java/com/cta4j/train/arrival/model/TrainDirection.java	Adds operational train direction enum (CTA codes 1/5).
src/main/java/com/cta4j/train/arrival/model/ArrivalMetadata.java	Adds arrival metadata (direction/run/optional coords/flags) + validation.
src/main/java/com/cta4j/train/arrival/model/Arrival.java	Adds new arrival domain model.
src/main/java/com/cta4j/train/arrival/internal/wire/CtaArrivalsResponse.java	Adds new train arrivals wire DTO (errCd/errNm/eta).
src/main/java/com/cta4j/train/arrival/internal/wire/CtaArrival.java	Adds new train arrival wire DTO for ETA entries.
src/main/java/com/cta4j/train/arrival/internal/mapper/ArrivalMapper.java	Adds MapStruct mapper for arrivals wire→domain mapping.
src/main/java/com/cta4j/train/arrival/internal/impl/ArrivalsApiImpl.java	Implements arrivals fetching by mapId/stopId with query params and error mapping.
src/main/java/com/cta4j/train/arrival/ArrivalsApi.java	Adds public Arrivals API surface + convenience overloads.
src/main/java/com/cta4j/train/TrainApi.java	Adds new top-level Train API surface + builder entrypoint.
src/main/java/com/cta4j/internal/json/Cta4jObjectMapper.java	Adds shared singleton ObjectMapper provider.
src/main/java/com/cta4j/internal/http/HttpClient.java	Moves/renames HTTP utility into internal package and standardizes usage.
src/main/java/com/cta4j/internal/geo/GeoConstants.java	Centralizes geo bounds constants used across bus/train models.
src/main/java/com/cta4j/bus/vehicle/model/VehicleCoordinates.java	Replaces inline heading bounds with GeoConstants; adds lat/lon range validation.
src/main/java/com/cta4j/bus/vehicle/internal/mapper/VehicleMapper.java	Normalizes MapStruct `@Mapping` argument order (target/source).
src/main/java/com/cta4j/bus/vehicle/internal/impl/VehiclesApiImpl.java	Switches HTTP calls from legacy HttpUtils to internal HttpClient.
src/main/java/com/cta4j/bus/stop/model/Stop.java	Relies on List.copyOf for null-element checks (removes explicit forEach checks).
src/main/java/com/cta4j/bus/stop/internal/wire/CtaStop.java	Relies on List.copyOf for null-element checks (removes explicit forEach checks).
src/main/java/com/cta4j/bus/stop/internal/mapper/StopMapper.java	Normalizes MapStruct `@Mapping` argument order (target/source).
src/main/java/com/cta4j/bus/stop/internal/impl/StopsApiImpl.java	Switches HTTP calls from legacy HttpUtils to internal HttpClient.
src/main/java/com/cta4j/bus/route/internal/mapper/RouteMapper.java	Normalizes MapStruct `@Mapping` argument order (target/source).
src/main/java/com/cta4j/bus/route/internal/impl/RoutesApiImpl.java	Switches HTTP calls from legacy HttpUtils to internal HttpClient.
src/main/java/com/cta4j/bus/prediction/query/VehiclesPredictionsQuery.java	Relies on List.copyOf for null-element checks; builder copies list up-front.
src/main/java/com/cta4j/bus/prediction/query/StopsPredictionsQuery.java	Relies on List.copyOf for null-element checks; builder copies lists up-front.
src/main/java/com/cta4j/bus/prediction/internal/mapper/PredictionMapper.java	Normalizes MapStruct `@Mapping` argument order (target/source).
src/main/java/com/cta4j/bus/prediction/internal/impl/PredictionsApiImpl.java	Switches HTTP calls from legacy HttpUtils to internal HttpClient.
src/main/java/com/cta4j/bus/pattern/model/RoutePattern.java	Relies on List.copyOf for null-element checks (removes explicit forEach checks).
src/main/java/com/cta4j/bus/pattern/internal/wire/CtaPattern.java	Relies on List.copyOf for null-element checks (removes explicit forEach checks).
src/main/java/com/cta4j/bus/pattern/internal/mapper/RoutePatternMapper.java	Normalizes MapStruct `@Mapping` argument order (target/source).
src/main/java/com/cta4j/bus/pattern/internal/impl/PatternsApiImpl.java	Switches HTTP calls from legacy HttpUtils to internal HttpClient.
src/main/java/com/cta4j/bus/locale/internal/mapper/SupportedLocaleMapper.java	Normalizes MapStruct `@Mapping` argument order (target/source).
src/main/java/com/cta4j/bus/locale/internal/impl/LocalesApiImpl.java	Renames getLocales→list and switches HTTP to HttpClient.
src/main/java/com/cta4j/bus/locale/LocalesApi.java	Renames getLocales→list methods in public API.
src/main/java/com/cta4j/bus/internal/impl/BusApiImpl.java	Injects shared ObjectMapper and switches HTTP to HttpClient.
src/main/java/com/cta4j/bus/direction/internal/impl/DirectionsApiImpl.java	Switches HTTP calls from legacy HttpUtils to internal HttpClient.
src/main/java/com/cta4j/bus/detour/model/Detour.java	Relies on List.copyOf for null-element checks (removes explicit forEach checks).
src/main/java/com/cta4j/bus/detour/internal/wire/CtaDetour.java	Relies on List.copyOf for null-element checks (removes explicit forEach checks).
src/main/java/com/cta4j/bus/detour/internal/mapper/DetourMapper.java	Normalizes MapStruct `@Mapping` argument order (target/source).
src/main/java/com/cta4j/bus/detour/internal/impl/DetoursApiImpl.java	Switches HTTP calls from legacy HttpUtils to internal HttpClient.
pom.xml	Updates jackson-databind dependency version/scope.
CHANGELOG.md	Updates changelog entry to reflect new internal http package naming.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/main/java/com/cta4j/train/arrival/query/MapArrivalQuery.java

src/main/java/com/cta4j/train/arrival/query/StopArrivalQuery.java

src/main/java/com/cta4j/train/arrival/query/MapArrivalQuery.java

src/main/java/com/cta4j/train/arrival/internal/mapper/ArrivalMapper.java

src/main/java/com/cta4j/train/arrival/model/ArrivalMetadata.java

src/main/java/com/cta4j/train/arrival/model/Arrival.java

Copilot

Pull request overview

Copilot reviewed 77 out of 77 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-07T20:26:24Z

src/main/java/com/cta4j/train/arrival/internal/wire/CtaArrivalsResponse.java

+        if ((errCd != 0) && (errNm == null)) {
+            throw new IllegalArgumentException("errNm must be provided when errCd is non-zero");
+        }
+
+        if ((errCd == 0) && (errNm != null)) {
+            throw new IllegalArgumentException("errNm must be null when errCd is zero");
+        }


errNm is rejected when errCd == 0 (success), but the previous wire model treated errNm as always present. If the API returns errNm as an empty string on success (common for CTA responses), deserialization will throw and break all successful requests. Consider allowing blank/empty errNm when errCd == 0 (e.g., normalize blank to null) and only enforcing non-blank errNm when errCd != 0.

Copilot · 2026-03-07T20:26:24Z

src/main/java/com/cta4j/train/internal/mapper/Qualifiers.java

+    @Named("mapHumanAddress")
+    public static HumanAddress mapHumanAddress(String humanAddress) {
+        Objects.requireNonNull(humanAddress);
+
+        ObjectMapper objectMapper = Cta4jObjectMapper.instance();


humanAddress is modeled as @Nullable in CtaLocation, but mapHumanAddress unconditionally requireNonNulls the input. If MapStruct invokes this qualifier with a null value, this will throw. Consider making the qualifier null-safe (accept/return @Nullable and return null when the input is null/blank) to match the wire model and mapper usage.

Copilot · 2026-03-07T20:26:24Z

src/main/java/com/cta4j/train/station/internal/impl/StationsApiImpl.java

+        try {
+            stations = this.context.objectMapper()
+                                   .readValue(response, typeReference);
+        } catch (Exception e) {
+            String message = String.format("Failed to parse response from %s", this.context.stationsUrl());
+
+            throw new Cta4jException(message, e);
+        }


This catch is currently catch (Exception e), which will also wrap unexpected runtime issues (e.g., mapper bugs) as parse failures. Consider narrowing this to the Jackson parsing exception type used elsewhere (e.g., JacksonException) so non-parsing failures aren’t misreported.

Copilot · 2026-03-07T20:26:25Z

CHANGELOG.md

 - `fromExternal` method from `TrainCoordinates` class.
 - `fromExternal` method from `UpcomingTrainArrival` enum.
- `com.cta4j.util` export from `module-info.java`.
+- `com.cta4j.internal.http` export from `module-info.java`.


This changelog entry mentions exporting com.cta4j.internal.http from module-info.java, but there is no module-info.java in the current source tree. Please update the entry to reflect the actual packaging/module setup so the changelog doesn’t reference a non-existent file.

Suggested change

- `com.cta4j.internal.http` export from `module-info.java`.

- Public exposure of the internal `com.cta4j.internal.http` package (it is now internal-only).

Logan Kulinski and others added 13 commits February 1, 2026 12:50

Train API Refactor

99863c8

Train API Refactor

2ec080e

Train API Refactor

342626f

Train API Refactor

2208324

Put "target" attribute before "source"

f6b8fdf

Train API refactor

ff48196

Train API refactor

23230da

Train API refactor

8a30ff1

Train API refactor

d41730c

Train API refactor

50088ad

Train API refactor

4a264c9

Train API refactor

2e63350

Train API refactor

e8a3f63

Copilot AI review requested due to automatic review settings March 7, 2026 20:08

Copilot started reviewing on behalf of lbkulinski March 7, 2026 20:08 View session

Copilot AI reviewed Mar 7, 2026

View reviewed changes

Train API refactor

b09a45d

lbkulinski requested a review from Copilot March 7, 2026 20:21

Copilot started reviewing on behalf of lbkulinski March 7, 2026 20:21 View session

Copilot AI reviewed Mar 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Train API refactor#45

Train API refactor#45
lbkulinski wants to merge 14 commits intomainfrom
train-api-refactor

lbkulinski commented Mar 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	- `com.cta4j.internal.http` export from `module-info.java`.
	- Public exposure of the internal `com.cta4j.internal.http` package (it is now internal-only).

Conversation

lbkulinski commented Mar 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants