fix: prevent NPEs from sparse API arrays, add coordinate validation, and enforce Optional discipline #34

WilliamAGH · 2026-01-23T08:29:36Z

Summary

This PR hardens the SDK against real-world Apple Maps API responses that contain unexpected nulls, adds fail-fast coordinate validation, and aligns the codebase with project null/Optional rules (NO1a-e).

Bug Fixes

Null-safe list normalization across all domain models

Problem: List.copyOf() throws NullPointerException when the source list contains null elements. Apple Maps API responses occasionally return sparse arrays with null entries (e.g., a routes array like [route1, null, route3]).

Solution: Replaced List.copyOf() with a stream-and-filter pattern in 17 domain model records:

// Before (throws NPE on null elements)
routes = List.copyOf(routes);

// After (filters nulls safely)
routes = rawList.stream().filter(Objects::nonNull).toList();

Affected models: AlternateIdsEntry, AlternateIdsResponse, AutocompleteResult, DirectionsResponse, DirectionsRoute, ErrorResponse, EtaResponse, Place, PlaceLookupError, PlaceResults, PlacesResponse, SearchAutocompleteResponse, SearchResponse, SearchResponsePlace, StructuredAddress

Preserve stepPaths index alignment with steps array

Problem: When filtering null stepPath entries, the positional relationship between stepPaths.get(i) and steps.get(i) was broken, causing route rendering bugs.

Solution: Null stepPath entries are now converted to empty lists (List.of()) instead of being removed, maintaining index alignment. Null Location elements within individual paths are still filtered.

URL-encode origin and destinations in EtaInput

Problem: Coordinates contain commas (37.7749,-122.4194) and destinations are pipe-separated (loc1|loc2), but these reserved characters were not URL-encoded per RFC 3986, causing malformed URIs.

Solution: Apply URLEncoder.encode() to origin and destinations parameters in toQueryString().

Make optional API response fields nullable

Problem: PlaceLookupError.id and SearchResponse.displayMapRegion were required fields, but the API sometimes omits them, causing deserialization NPEs.

Solution:

PlaceLookupError: Changed constructor to accept nullable String (renamed to rawId) with an Optional<String> id() accessor—per NO1c, Optional should not be used as constructor parameters
SearchResponse.displayMapRegion: Changed from required to Optional<MapRegion>

Return Optional from getOrigin() per NO1a

Problem: AppleMapsAuthorizationService.getOrigin() returned nullable String, violating project rule NO1a (public methods never return null).

Solution: Changed return type to Optional<String>, normalized at construction time, and updated HttpAppleMapsGateway to use ifPresent() pattern.

Fix Sonatype snapshot publishing

Problem: CI workflow was publishing version 0.1.5 to the snapshot repository, but Sonatype Central requires the -SNAPSHOT suffix, causing HTTP 400 errors.

Solution: Extract base version from gradle.properties and append -SNAPSHOT suffix before passing to Gradle publish task.

New Features

Fail-fast coordinate validation

Benefit: Invalid coordinates (out of range, NaN, Infinity) now fail immediately at Location or DirectionsEndpoint construction with clear error messages, rather than propagating to the API and returning cryptic errors.

Validation rules:

Latitude must be in range [-90, 90]
Longitude must be in range [-180, 180]
Both must be finite (not NaN or Infinity)

new Location(91.0, 0.0);  // throws IllegalArgumentException: "latitude must be between -90.0 and 90.0"
new Location(Double.NaN, 0.0);  // throws IllegalArgumentException: "latitude must be a finite value"

Documentation

Added Javadoc for origin parameter in AppleMapsAuthorizationService and HttpAppleMapsGateway constructors
Added Javadoc for getOrigin() method explaining return behavior

Other Changes

Dependencies: Jackson BOM 3.0.4, NMCP plugin 1.4.3
Tooling: Added Spotless plugin with make lint target for code formatting
Cleanup: Normalized trailing whitespace across files

Adds a convenient `make lint` target that runs the full Gradle check task with --rerun-tasks to ensure all checks execute regardless of cache state.

List.copyOf() throws NullPointerException when the source list contains null elements. The Apple Maps API responses may include sparse arrays with null entries. This replaces List.copyOf() with a stream-and-filter pattern that safely removes null elements before creating an immutable list. - Update normalizeList in all domain model records - Add explicit null check before streaming to handle null input - Use .filter(Objects::nonNull) to remove null elements

Geographic coordinates must be within valid ranges: latitude [-90, 90] and longitude [-180, 180]. Previously, invalid coordinates would propagate through the system and only fail when reaching the Apple Maps API. This adds fail-fast validation at construction time with clear error messages. - Add validateLatitudeLongitude to Location record with range checking - Validate coordinates are finite (not NaN or Infinity) - Call validation from DirectionsEndpoint.fromLatitudeLongitude

Some API response fields documented as required may be absent in certain edge cases. This makes PlaceLookupError.id and SearchResponse.displayMapRegion optional to handle these cases gracefully rather than throwing NPE during deserialization. - Change PlaceLookupError.id from String to Optional<String> - Change SearchResponse.displayMapRegion from required to Optional - Add normalizeOptional helper to handle null Optional inputs

The coordinates contain commas and destinations are pipe-separated, but these characters were not being URL-encoded. While many HTTP clients handle this, RFC 3986 requires encoding reserved characters in query parameters. This ensures the generated query strings create valid URIs. - Encode origin and destinations parameters in toQueryString - Add test verifying URI.create accepts the generated query string

The Origin header support was added but lacked documentation explaining its purpose. This adds Javadoc for the origin parameter in constructors and the getOrigin() accessor method. Also normalizes trailing whitespace and removes stray blank lines for consistency. - Document origin parameter in AppleMapsAuthorizationService constructor - Document origin parameter in HttpAppleMapsGateway constructor - Add Javadoc for getOrigin() method explaining return behavior - Remove trailing whitespace and normalize blank lines

Sonatype Central's snapshot repository requires versions ending with -SNAPSHOT suffix. The CI workflow was publishing version 0.1.5 without the suffix, causing HTTP 400 errors from the snapshot repository. - Extract base version from gradle.properties in new workflow step - Append -SNAPSHOT suffix to create valid snapshot version - Pass snapshot version via -Pversion to Gradle publish task

coderabbitai · 2026-01-23T08:29:55Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Summary by CodeRabbit

New Features
- Optional Origin header can be specified for API requests.
- Enforced code formatting via added formatting tooling and lint target.
Bug Fixes
- Improved null-safety: list fields tolerate null inputs and drop null elements.
- Added coordinate validation for geographic inputs.
- Search map region and place lookup ID are now optional.
- ETA query string now URL-encodes origin/destinations.
Tests
- Added extensive unit tests for validations and null-element normalization.
Chores
- CI: dynamic snapshot versioning and made dependency-graph step resilient.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Walkthrough

This PR adds CI snapshot extraction and Spotless formatting, tightens domain model null handling by filtering null list elements, adds Location coordinate validation and related tests, makes several fields Optional (PlaceLookupError.id, SearchResponse.displayMapRegion), percent-encodes ETA query params, and wires optional Origin handling into auth/gateway code.

Changes

Cohort / File(s)	Summary
CI & Build `.github/workflows/CI.yaml`, `Makefile`, `build.gradle.kts`	Extract snapshot VERSION in CI and pass to publish; continue-on-error on dependency graph update; add `lint` Make target; add Spotless plugin and hook `spotlessCheck` into `check`; bump nmcp and jackson-bom versions.
Auth & HTTP Gateway `src/main/java/.../AppleMapsAuthorizationService.java`, `src/main/java/.../HttpAppleMapsGateway.java`	Make origin optional (store as `Optional<String>`), add constructor accepting origin, and set Origin header via `Optional.ifPresent` when present.
Location & Endpoint Validation `src/main/java/.../Location.java`, `src/main/java/.../DirectionsEndpoint.java`	Add latitude/longitude finiteness and range validation in `Location` and call validation from `DirectionsEndpoint.fromLatitudeLongitude`.
Null-safe List Normalization (many models) `src/main/java/.../{AlternateIdsEntry,AlternateIdsResponse,AutocompleteResult,DirectionsResponse,DirectionsRoute,ErrorResponse,EtaResponse,Place,PlaceResults,PlacesResponse,SearchAutocompleteResponse,SearchResponsePlace,StructuredAddress,SearchResponse}.java`	Replace `List.copyOf(Objects.requireNonNullElse(...))` with explicit null checks and stream-based filtering: null input → empty list; drop null elements. Make `SearchResponse.displayMapRegion` an `Optional` with a `normalizeOptional` helper.
PlaceLookupError change `src/main/java/.../PlaceLookupError.java`	Replace required `id` with `@JsonProperty("id") String rawId` and expose `id()` as `Optional<String>`; relax canonical constructor null requirement.
ETA request encoding `src/main/java/.../EtaInput.java`	Percent-encode origin and destinations when building the ETA query string.
Minor cleanup `src/main/java/.../AppleMaps.java`	Small whitespace/formatting adjustment; removed an extra blank line in a resolver helper.
Tests — validation & normalization `src/test/java/.../{LocationValidationTest,DirectionsEndpointValidationTest,NullElementListNormalizationTest,PlaceLookupErrorTest,SearchResponseDisplayMapRegionTest,EtaInputTest}.java`	Add tests for Location validation and DirectionsEndpoint, broad null-element filtering across models, PlaceLookupError Optional id deserialization, SearchResponse.displayMapRegion optional handling, and percent-encoded ETA query string/URI creation.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related issues

[Detail Bug] PlaceLookupError requires non-null id, causing deserialization failure when API omits id #29 — Matches PlaceLookupError.id→Optional deserialization changes.
[Detail Bug] DirectionsEndpoint.fromLatitudeLongitude accepts invalid coordinates without validation #19 — Matches addition of coordinate validation invoked by DirectionsEndpoint.
[Detail Bug] DirectionsResponse normalization crashes on null elements (List.copyOf NPE) #14 — Matches safer DirectionsResponse normalization and null-element handling.
[Detail Bug] EtaInput produces unencoded query string; URI.create() fails for multi-destination ETAs #28 — Matches EtaInput.toQueryString percent-encoding of origin/destinations.
[Detail Bug] SearchResponse deserialization fails when displayMapRegion is missing or null #17 — Matches making SearchResponse.displayMapRegion optional and normalization changes.

Possibly related PRs

Add Origin header support, switch to nmcp publishing, comprehensive Javadoc #9 — Overlaps origin handling changes in authorization/gateway code.
chore: bump version to 0.1.2-SNAPSHOT #7 — Related to VERSION_NAME/SNAPSHOT handling in CI workflows.

Poem

Lists now filter out the stray,
Lat/longs checked the proper way,
Optionals cradle missing ids,
CI snapshots set their keys,
Tests march on — robust by day 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: fixing NPE issues in sparse API arrays, adding coordinate validation, and enforcing Optional discipline per project rules.
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, detailing bug fixes, new features, and other changes with clear explanations and code examples.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch dev

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e1becbc5b0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

src/main/java/com/williamcallahan/applemaps/domain/model/DirectionsResponse.java

Copilot

Pull request overview

This PR implements defensive null handling across domain models, adds coordinate validation, improves query string encoding, integrates code formatting tools, and updates dependencies. The changes include two breaking API changes that convert non-optional fields to Optional fields to handle missing/null API responses more gracefully.

Changes:

Changed SearchResponse.displayMapRegion and PlaceLookupError.id from required to optional fields (breaking changes)
Implemented consistent null filtering for list elements across all domain models
Added coordinate validation to Location and DirectionsEndpoint with proper bounds checking
Fixed ETA query string encoding to properly URL-encode special characters (commas and pipes)
Integrated Spotless code formatter and updated build dependencies

Reviewed changes

Copilot reviewed 28 out of 30 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
EtaInputTest.java	Updated test expectations for proper URL encoding and added URI validity test
SearchResponseDisplayMapRegionTest.java	New test verifying Optional handling for missing/null displayMapRegion
PlaceLookupErrorTest.java	New test verifying Optional handling for missing/null error IDs
NullElementListNormalizationTest.java	Comprehensive test suite covering null element filtering across all model classes
LocationValidationTest.java	New test verifying coordinate validation (bounds and finiteness)
DirectionsEndpointValidationTest.java	New test verifying coordinate validation in factory method
EtaInput.java	Added URL encoding for origin and destination coordinates
SearchResponse.java	Changed displayMapRegion from SearchMapRegion to Optional, added null filtering
PlaceLookupError.java	Changed id from String to Optional
Location.java	Added coordinate validation logic with bounds and finiteness checks
DirectionsEndpoint.java	Added coordinate validation call in fromLatitudeLongitude factory
Multiple model files	Implemented consistent null element filtering in normalizeList methods
HttpAppleMapsGateway.java	Added documentation for Origin header parameter, removed trailing whitespace
AppleMapsAuthorizationService.java	Added documentation for Origin parameter and getOrigin method
AppleMaps.java	Removed trailing whitespace
build.gradle.kts	Updated Jackson BOM to 3.0.4, nmcp plugin to 1.4.3, added Spotless plugin configuration
Makefile	Added lint target for running checks
CI.yaml	Improved snapshot version extraction and publishing

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

src/main/java/com/williamcallahan/applemaps/domain/model/SearchResponse.java

src/main/java/com/williamcallahan/applemaps/domain/model/PlaceLookupError.java

coderabbitai

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@build.gradle.kts`:
- Around line 71-73: Update the Jackson BOM version used in the Gradle
dependencies: replace the non-released version string
"tools.jackson:jackson-bom:3.0.4" with the stable "3.0.3" (the entry is in the
implementation(platform(...)) line), and ensure the transitive
implementation("tools.jackson.core:jackson-databind") continues to align with
that BOM; alternatively, if 3.0.4 is intentionally required, add a comment
documenting that it's a forward-looking pin and verify the artifact is available
in your registries before keeping "3.0.4".

In
`@src/main/java/com/williamcallahan/applemaps/adapters/mapsserver/AppleMapsAuthorizationService.java`:
- Around line 58-65: Change the public API to avoid returning null by making
getOrigin() return Optional<String> and normalizing the value once in the
AppleMapsAuthorizationService constructor: store a non-null Optional (e.g.,
Optional.ofNullable(orig)) as a private field or convert the existing origin
field to an Optional during construction, and update getOrigin() to return that
Optional; also apply the same normalization pattern to the other public getters
mentioned (lines 97-99) so no public method ever returns null.

In
`@src/main/java/com/williamcallahan/applemaps/domain/model/PlaceLookupError.java`:
- Around line 9-19: Change the record component type in PlaceLookupError from
Optional<String> to a nullable String parameter and normalize it in the
canonical constructor: update the record header to use PlaceLookupErrorCode
errorCode, `@Nullable` String id, keep Objects.requireNonNull(errorCode,
"errorCode") in the canonical constructor (remove the Objects.requireNonNullElse
call that handled Optional), and add a public Optional<String> id() accessor
that returns Optional.ofNullable(id) so callers receive an Optional but callers
constructing the record can pass a plain String or null.

🧹 Nitpick comments (2)

src/main/java/com/williamcallahan/applemaps/adapters/mapsserver/HttpAppleMapsGateway.java (1)
71-79: Consider normalizing blank origin to null. This avoids accidentally sending an empty Origin header, which some servers treat as invalid.
♻️ Suggested tweak
 public HttpAppleMapsGateway(String authToken, Duration timeout, String origin) {
-    this(new Dependencies(authToken, timeout, origin));
+    String normalizedOrigin = (origin == null || origin.isBlank()) ? null : origin.trim();
+    this(new Dependencies(authToken, timeout, normalizedOrigin));
 }
src/test/java/com/williamcallahan/applemaps/domain/model/DirectionsEndpointValidationTest.java (1)

8-26: Nice foundation for coordinate validation tests! 🎯

The test structure is clean and the naming is behavior-focused, which makes it easy to understand what's being verified. One small learning opportunity: you're testing the lower bound for latitude (-90.1), but there are a few more edge cases that could strengthen confidence:

Latitude too high (90.1)

Longitude out of bounds (-180.1, 180.1)

Special floating-point values (Double.NaN, Double.POSITIVE_INFINITY)

These would ensure the validation logic is comprehensive. That said, the current tests do validate the core behavior nicely!

build.gradle.kts

...in/java/com/williamcallahan/applemaps/adapters/mapsserver/AppleMapsAuthorizationService.java

src/main/java/com/williamcallahan/applemaps/domain/model/PlaceLookupError.java

Null stepPath entries are now converted to empty lists instead of being filtered out. This maintains positional alignment between stepPaths and steps, so stepPaths.get(i) always corresponds to steps.get(i). Null Location elements within individual paths are still filtered.

Split test into two: one verifying null filtering for routes/steps, and another explicitly testing that null stepPath entries become empty lists to preserve index alignment with the steps array.

…r NO1a Public methods must not return null per project rules. Changed getOrigin() to return Optional<String> and normalized at construction time. Also filters out blank origin values. Updated callers to use ifPresent pattern.

Updated to use ifPresent with the Optional<String> returned by AppleMapsAuthorizationService.getOrigin() instead of null check.

@JsonProperty

Per NO1c, Optional should not be used as public constructor parameters. Changed record to accept nullable String (renamed to rawId for clarity) and provide an Optional<String> id() accessor that wraps it. This allows callers to pass null directly instead of Optional.empty(). Uses @JsonProperty("id") to map JSON field to rawId.

…r id Updated test helper to pass null instead of Optional.empty() following the PlaceLookupError API change where the constructor now accepts a nullable String.

WilliamAGH added 8 commits January 23, 2026 00:07

chore(deps): update Jackson BOM to 3.0.4 and NMCP plugin to 1.4.3

1057802

chore(make): add lint target for running Gradle check

22cf6b5

Adds a convenient `make lint` target that runs the full Gradle check task with --rerun-tasks to ensure all checks execute regardless of cache state.

WilliamAGH self-assigned this Jan 23, 2026

Copilot AI review requested due to automatic review settings January 23, 2026 08:29

Copilot started reviewing on behalf of WilliamAGH January 23, 2026 08:29 View session

WilliamAGH changed the title ~~merge~~ fix: improve API response robustness with null-safe list handling and coordinate validation Jan 23, 2026

chatgpt-codex-connector bot reviewed Jan 23, 2026

View reviewed changes

src/main/java/com/williamcallahan/applemaps/domain/model/DirectionsResponse.java Show resolved Hide resolved

Copilot AI reviewed Jan 23, 2026

View reviewed changes

src/main/java/com/williamcallahan/applemaps/domain/model/SearchResponse.java Show resolved Hide resolved

src/main/java/com/williamcallahan/applemaps/domain/model/PlaceLookupError.java Outdated Show resolved Hide resolved

coderabbitai bot reviewed Jan 23, 2026

View reviewed changes

build.gradle.kts Show resolved Hide resolved

...in/java/com/williamcallahan/applemaps/adapters/mapsserver/AppleMapsAuthorizationService.java Show resolved Hide resolved

src/main/java/com/williamcallahan/applemaps/domain/model/PlaceLookupError.java Outdated Show resolved Hide resolved

WilliamAGH added 6 commits January 23, 2026 00:41

test(DirectionsResponse): verify stepPaths index alignment preservation

e59d91e

Split test into two: one verifying null filtering for routes/steps, and another explicitly testing that null stepPath entries become empty lists to preserve index alignment with the steps array.

fix(HttpAppleMapsGateway): use Optional pattern for origin header

8f0a733

Updated to use ifPresent with the Optional<String> returned by AppleMapsAuthorizationService.getOrigin() instead of null check.

test(NullElementListNormalizationTest): pass null for PlaceLookupErro…

6d7f6f3

…r id Updated test helper to pass null instead of Optional.empty() following the PlaceLookupError API change where the constructor now accepts a nullable String.

WilliamAGH changed the title ~~fix: improve API response robustness with null-safe list handling and coordinate validation~~ fix: prevent NPEs from sparse API arrays, add coordinate validation, and enforce Optional discipline Jan 23, 2026

WilliamAGH merged commit 6dab01f into main Jan 23, 2026
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: prevent NPEs from sparse API arrays, add coordinate validation, and enforce Optional discipline #34

fix: prevent NPEs from sparse API arrays, add coordinate validation, and enforce Optional discipline #34

Uh oh!

WilliamAGH commented Jan 23, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Jan 23, 2026 •

edited

Loading

Other AI code review bot(s) detected

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related issues

Possibly related PRs

Poem

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: prevent NPEs from sparse API arrays, add coordinate validation, and enforce Optional discipline #34

fix: prevent NPEs from sparse API arrays, add coordinate validation, and enforce Optional discipline #34

Uh oh!

Conversation

WilliamAGH commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Bug Fixes

Null-safe list normalization across all domain models

Preserve stepPaths index alignment with steps array

URL-encode origin and destinations in EtaInput

Make optional API response fields nullable

Return Optional from getOrigin() per NO1a

Fix Sonatype snapshot publishing

New Features

Fail-fast coordinate validation

Documentation

Other Changes

Uh oh!

coderabbitai bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Other AI code review bot(s) detected

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related issues

Possibly related PRs

Poem

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

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!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

WilliamAGH commented Jan 23, 2026 •

edited

Loading

coderabbitai bot commented Jan 23, 2026 •

edited

Loading