Skip to content

fix: polish map service API - #2683

Open
rsynek wants to merge 2 commits into
TimefoldAI:mainfrom
rsynek:fix/map-service-api-polishing
Open

fix: polish map service API#2683
rsynek wants to merge 2 commits into
TimefoldAI:mainfrom
rsynek:fix/map-service-api-polishing

Conversation

@rsynek

@rsynek rsynek commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Minor improvements identified in #2672.

@rsynek
rsynek requested review from a team and TomCools and a lite review from Copilot September 23, 2026 06:59
@rsynek
rsynek requested a review from triceo as a code owner September 23, 2026 06:59
@rsynek rsynek mentioned this pull request Sep 23, 2026
Draft
Copilot AI reviewed Sep 23, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Low severity

Open (1)
What changed in this PR

Polishes the maps service API by making location-set names optional and adding explicit validation for location lists.

Changes:

  • Adds a default empty location-set name.
  • Rejects null location lists with a clear exception.
File Description
service/​maps/​service-integration/​src/​main/​java/​ai/​timefold/​solver/​service/​maps/​service/​integration/​api/​LocationsAwareSolverModel.java Updated as part of this pull request.
service/​maps/​service-client/​src/​main/​java/​ai/​timefold/​solver/​service/​maps/​service/​client/​api/​TravelTimeMatrixEnricher.java Updated as part of this pull request.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .../java/ai/timefold/solver/service/maps/service/integration/api/LocationsAwareSolverModel.java
Copilot AI review requested due to automatic review settings September 23, 2026 09:27
Copilot AI reviewed Sep 23, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Low severity

Open (1)
Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Abort retries for invalid models throwing NullPointerException

service/​maps/​service-client/​src/​main/​java/​ai/​timefold/​solver/​service/​maps/​service/​client/​api/​TravelTimeMatrixEnricher.java:67

Because this Objects.requireNonNull throws NullPointerException inside the @Retry-annotated enrich method, an invalid model is retried five times with one-second delays instead of failing immediately; add NullPointerException.class to abortOn (or validate with the already-aborted IllegalArgumentException).

Comment thread .../java/ai/timefold/solver/service/maps/service/integration/api/LocationsAwareSolverModel.java
Copilot AI lite review requested due to automatic review settings September 30, 2026 11:25
Copilot AI reviewed Sep 30, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Comment thread ...c/main/java/ai/timefold/solver/service/maps/service/client/api/TravelTimeMatrixEnricher.java
rsynek and others added 2 commits September 30, 2026 17:03
@rsynek
fix: polish map service API
0e1ef80
@rsynek
add javadoc
388521d 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 30, 2026 15:03
@rsynek
rsynek force-pushed the fix/map-service-api-polishing branch from 7fc6921 to 388521d Compare September 30, 2026 15:03
Copilot AI reviewed Sep 30, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Comment thread ...c/main/java/ai/timefold/solver/service/maps/service/client/api/TravelTimeMatrixEnricher.java

This branch was successfully deployed

1 active deployment
internal — 388521db Deployed Sep 30, 2026 by rsynek via approval_required #887
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@TomCools TomCools Awaiting requested review from TomCools
@triceo triceo Awaiting requested review from triceo triceo is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rsynek @TomCools