Skip to content

chore: remove the MapEnrichmentContext bean from definition - #2747

Open
rsynek wants to merge 1 commit into
TimefoldAI:mainfrom
rsynek:chore/map-enrichment-context
Open

rsynek wants to merge 1 commit into
TimefoldAI:mainfrom
rsynek:chore/map-enrichment-context

Conversation

@rsynek

@rsynek rsynek commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Removes the MapEnrichmentContext bean from the definition module, which should stay safe to depend on with no side effect. Also, places the class into a more fitting package.

@rsynek
rsynek requested review from a team and a balanced review from Copilot October 13, 2026 11:18
@rsynek
rsynek requested a review from triceo as a code owner October 13, 2026 11:18
/**
* Produces StorageObjectMapper instances with storage-specific (de)serialization settings.
*/
@ApplicationScoped

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This again breaks the rule about no beans in the definition model. However, it was there silently, as it contains the Producer annotation and CDI pick it up if there are other beans in the module.

Removing this bean will be a subject of additional refactoring of AbstractStorageService.

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.

🟡 Changes recommended

The relocated parameter type breaks a public API constructor despite the compatibility suppression.

1 open finding
What changed in this PR

Moves map enrichment CDI ownership from service definition to the maps client.

Changes:

  • Relocates MapEnrichmentContext.
  • Adds a maps-client producer and optional worker fallback.
  • Updates imports, tests, and compatibility metadata.
File Description
SolverWorker.java Resolves the context optionally.
TravelTimeMatrixEnricherTrafficTest.java Updates the context import.
TravelTimeMatrixEnricherTest.java Updates the injected context import.
MapEnrichmentContextProducer.java Produces the maps-client CDI bean.
TravelTimeMatrixEnricher.java Uses the relocated context.
revapi-differences.json Suppresses the constructor API break.
definition/​internal/​MapEnrichmentContext.java Removes the old context class.
StorageObjectMapperProducer.java Adds an explicit CDI scope.
definition/​impl/​enrichment/​MapEnrichmentContext.java Adds the relocated non-bean context.

🧠 Review effort: Balanced


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

Comment thread service/maps/service-client/src/build/revapi-differences.json
Copilot AI balanced review requested due to automatic review settings October 14, 2026 12:01
@rsynek
rsynek force-pushed the chore/map-enrichment-context branch from 72eaf53 to 501349d Compare October 14, 2026 12:01

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.

🟡 Changes recommended

It suppresses a public constructor compatibility break and lacks coverage for the worker fallback path.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

This branch was successfully deployed

1 active deployment
internal — 501349d8 Deployed Oct 14, 2026 by rsynek via approval_required #5076
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants