Bus API Redesign #40
Bus API Redesign #40
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds JSpecify nullability annotations to the public API of the CTA4J Java SDK to improve type safety and establish clearer nullability contracts. The version is bumped from 3.0.4 to 3.0.5 with the addition of the org.jspecify:jspecify:1.0.0 dependency.
- Added
@NonNullannotations to return types of public API methods inTrainClient,BusClient, and their implementations - Reorganized field declarations in implementation classes to group constants together
- Updated version and changelog to reflect the changes
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Bumped version to 3.0.5 and added JSpecify dependency with provided scope |
| CHANGELOG.md | Added release notes for version 3.0.5 documenting the addition of JSpecify annotations |
| src/main/java/com/cta4j/train/client/TrainClient.java | Added @NonNull annotations to method return types and builder methods |
| src/main/java/com/cta4j/train/client/internal/TrainClientImpl.java | Added @NonNull annotations to match interface, reorganized fields to group constants |
| src/main/java/com/cta4j/bus/client/BusClient.java | Added @NonNull annotations to method return types and builder methods |
| src/main/java/com/cta4j/bus/client/internal/BusClientImpl.java | Added @NonNull annotations to match interface, reorganized fields to group constants |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/com/cta4j/train/client/internal/TrainClientImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cta4j/train/client/internal/TrainClientImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 100 out of 100 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/com/cta4j/bus/prediction/internal/impl/PredictionsApiImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 100 out of 100 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Mapping(source = "gtfsseq", target = "metadata.gtfsSequence") | ||
| @Mapping(source = "nbus", target = "metadata.nextBus") | ||
| @Mapping(source = "stst", target = "metadata.scheduledStartSeconds") | ||
| @Mapping(source = "stsd", target = "metadata.scheduledStartDate") |
There was a problem hiding this comment.
stsd (scheduledStartDate) is mapped from a String without a qualifier. If the API returns stsd as yyyyMMdd (common for BusTime), MapStruct’s default LocalDate conversion (LocalDate.parse) will fail. Add a @Named date-mapping qualifier (e.g., mapDate) and use qualifiedByName on this mapping.
src/main/java/com/cta4j/bus/prediction/internal/impl/PredictionsApiImpl.java
Show resolved
Hide resolved
src/main/java/com/cta4j/bus/prediction/internal/impl/PredictionsApiImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 100 out of 100 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 100 out of 100 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/main/java/com/cta4j/bus/direction/internal/wire/CtaDirection.java:16
CtaDirectionuses record componentsid/namewith no Jackson field annotations, but the BusTime API wire models in this PR consistently use CTA field names (e.g.,rt,stpid,rtdir, etc.). If the directions payload uses a different field name (commonlydir), Jackson will deserializeid/nameas null and the compact constructor will throw. Rename the components to match the JSON field name(s) and/or add@JsonProperty/@JsonAliasso deserialization is resilient.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 100 out of 100 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 100 out of 100 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/main/java/com/cta4j/bus/direction/internal/wire/CtaDirection.java:16
CtaDirectionrecord component names (id,name) don’t appear to match the CTA BusTimegetdirectionspayload (which typically uses adirfield, and may not include a separatename). With the currentrequireNonNullchecks, Jackson deserialization will fail if the JSON doesn’t contain bothidandname. Consider renaming the components to match the JSON field names (e.g.,dir) and/or adding@JsonPropertyannotations, and relax/remove thenamerequirement if it isn’t provided by the API.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.