feat(valhalla): add multimodal support with the date_time API parameter#124
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an optional date_time query parameter for Valhalla multimodal computations and wires it through the service parameter model, Simple API controller, and Valhalla request payload, with accompanying documentation updates.
Changes:
- Added a new
date_timeservice parameter and exposed it onrouteandisochroneoperations. - Propagated
date_timethrough Simple API parsing/validation, request objects, and Valhalla request JSON construction. - Introduced a new
stringparameter type (StringParameter) and enabled it inparameterManager, plus updated docs/samples/changelog.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/resources/parameters/date_time.json | Adds the date_time service parameter definition. |
| src/resources/operations/route.json | Exposes date_time on the route operation. |
| src/resources/operations/isochrone.json | Exposes date_time on the isochrone operation. |
| src/js/sources/valhallaSource.js | Maps request.date_time into Valhalla date_time payload. |
| src/js/requests/routeRequest.js | Adds date_time field + accessors to route request model. |
| src/js/requests/isochroneRequest.js | Adds date_time field + accessors to isochrone request model. |
| src/js/parameters/stringParameter.js | Introduces a resource parameter implementation for string. |
| src/js/parameters/parameterManager.js | Allows string type, validates config, instantiates StringParameter. |
| src/js/apis/simple/1.0.0/init.js | Adds date_time to generated Simple API parameter descriptions/capabilities. |
| src/js/apis/simple/1.0.0/controller/controller.js | Parses/validates date_time for route and isochrone requests. |
| readme.md | Markdown whitespace/formatting adjustments. |
| readme_en.md | Markdown whitespace/formatting adjustments. |
| documentation/index.md | Mentions date_time availability for multimodal Valhalla resources. |
| documentation/configuration/resources/valhalla.resource | Updates sample Valhalla resource config (multimodal + date_time). |
| documentation/apis/simple/1.0.0/api.json | Documents date_time parameter and schema constraints (FR). |
| documentation_en/index.md | Mentions date_time availability (EN). |
| documentation_en/apis/simple/1.0.0/api.json | Documents date_time parameter and schema constraints (EN). |
| changelog.md | Records the new date_time parameter and multimodal exposure. |
Comments suppressed due to low confidence (2)
src/js/requests/routeRequest.js:370
- Accent manquant dans le commentaire JSDoc (départ).
* @description Attribuer la date et heure de depart
src/js/requests/isochroneRequest.js:317
- Le type du paramètre
dtdans le JSDoc est indiqué comme{object}alors quedate_timeest une chaîne (cf. regex/paramètre OpenAPI).
* @function
* @name set date_time
* @description Attribuer la date et heure de depart.
* @param {object} dt - Date et heure de depart.
*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Amaury Zarzelli <amaury.zarzelli@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (2)
src/js/apis/simple/1.0.0/controller/controller.js:935
- Same issue as route:
if (parameters.date_time)ignores an explicitly provided empty value (date_time=). This should validate and return 400 rather than being treated as missing.
// date_time
if (parameters.date_time) {
documentation/configuration/resources/valhalla.resource:176
- Same as the route operation section above: adding a
values.patternfordate_timein the sample config keeps the validation contract explicit and aligned with the API docs/controller regex.
{
"id": "date_time"
},
| if (this._values.pattern) { | ||
| const regex = new RegExp(this._values.pattern); | ||
| if (!regex.test(userValue)) { | ||
| return errorManager.createErrorMessage("value does not match required pattern"); | ||
| } | ||
| } |
| // date_time | ||
| if (parameters.date_time) { | ||
|
|
| { | ||
| "id": "date_time" | ||
| }, |
| * @description Classe modelisant un parametre de type string dans une operation. | ||
| * | ||
| */ | ||
| module.exports = class StringParameter extends ResourceParameter { |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/js/parameters/stringParameter.js:109
new RegExp(this._values.pattern)can throw if the configured pattern is not a valid regex, which would crash request validation at runtime. Catch the error and return a validation message instead.
if (this._values.pattern) {
const regex = new RegExp(this._values.pattern);
if (!regex.test(userValue)) {
return errorManager.createErrorMessage("value does not match required pattern");
}
}
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
No description provided.