Skip to content

Commit 7420ad3

Browse files
committed
Add name to Valhalla waypoint properties
Exposes the Valhalla 'name' location property on ValhallaWaypointProperties so waypoints can carry a location/business name, which Valhalla may use in route narration (e.g. "You have arrived at <business name>"). Notes: - ValhallaWaypointProperties loses Copy since String is not Copy. - The field is serialized as 'waypoint_name' in the internal waypoint properties blob (the request still sends 'name'). This avoids a key collision with OsrmWaypointProperties: waypoints rebuilt from a parsed route response reuse the same properties blob, and on reroute the OSRM snapped street name would otherwise deserialize into the Valhalla name and be sent to the server. A regression test covers this.
1 parent c163960 commit 7420ad3

1 file changed

Lines changed: 61 additions & 1 deletion

File tree

common/ferrostar/src/routing_adapters/valhalla.rs

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,24 @@ use tsify::Tsify;
3030
///
3131
/// NOTE: Waypoint properties will NOT currently be echoed back in OSRM format,
3232
/// so these are sent to the server one time.
33-
#[derive(Copy, Clone, PartialEq, Debug, Default, Serialize, Deserialize)]
33+
#[derive(Clone, PartialEq, Debug, Default, Serialize, Deserialize)]
3434
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))]
3535
#[cfg_attr(feature = "wasm-bindgen", derive(Tsify))]
3636
#[cfg_attr(feature = "wasm-bindgen", tsify(into_wasm_abi, from_wasm_abi))]
3737
pub struct ValhallaWaypointProperties {
38+
/// Location or business name.
39+
///
40+
/// The name may be used in the route narration directions,
41+
/// such as "You have arrived at &lt;business name&gt;."
42+
///
43+
/// NOTE: Serialized as `waypoint_name` in the waypoint properties blob so it cannot
44+
/// collide with the `name` of [`OsrmWaypointProperties`](crate::routing_adapters::osrm::models::OsrmWaypointProperties)
45+
/// (the snapped street name), which occupies the same blob on waypoints rebuilt from a
46+
/// route response (e.g. the remaining waypoints used for reroute requests).
47+
/// It is still sent to Valhalla as `name`.
48+
#[serde(rename = "waypoint_name")]
49+
#[cfg_attr(feature = "uniffi", uniffi(default))]
50+
pub name: Option<String>,
3851
/// Preferred direction of travel for the start from the location.
3952
///
4053
/// The heading is indicated in degrees from north in a clockwise direction, where north is 0°, east is 90°, south is 180°, and west is 270°.
@@ -503,6 +516,7 @@ fn merge_optional_waypoint_properties(
503516
waypoint_properties: Option<ValhallaWaypointProperties>,
504517
) -> JsonValue {
505518
let Some(ValhallaWaypointProperties {
519+
name,
506520
heading,
507521
heading_tolerance,
508522
minimum_reachability,
@@ -531,6 +545,10 @@ fn merge_optional_waypoint_properties(
531545
}
532546
}
533547

548+
if let Some(name) = name {
549+
result["name"] = name.into();
550+
}
551+
534552
if let Some(heading) = heading {
535553
result["heading"] = heading.into();
536554
}
@@ -629,6 +647,7 @@ mod tests {
629647
GeographicCoordinate { lat: 2.0, lng: 3.0 },
630648
WaypointKind::Break,
631649
ValhallaWaypointProperties {
650+
name: Some("Main Street Stop".to_string()),
632651
preferred_side: Some(ValhallaWaypointPreferredSide::Same),
633652
search_filter: Some(ValhallaLocationSearchFilter {
634653
exclude_bridge: Some(true),
@@ -702,6 +721,7 @@ mod tests {
702721
{
703722
"lat": 2.0,
704723
"lon": 3.0,
724+
"name": "Main Street Stop",
705725
}
706726
],
707727
})
@@ -928,4 +948,44 @@ mod tests {
928948
})
929949
);
930950
}
951+
952+
/// Waypoints rebuilt from a parsed route (e.g. the remaining waypoints used when rerouting)
953+
/// carry [`OsrmWaypointProperties`] in the properties blob, whose `name` is the snapped
954+
/// street name. That name must NOT leak into the generated request as a Valhalla
955+
/// location `name`.
956+
#[test]
957+
fn osrm_response_properties_do_not_leak_into_request() {
958+
use crate::routing_adapters::osrm::models::OsrmWaypointProperties;
959+
960+
let waypoint = Waypoint {
961+
coordinate: GeographicCoordinate { lat: 2.0, lng: 3.0 },
962+
kind: WaypointKind::Break,
963+
properties: Some(
964+
serde_json::to_vec(&OsrmWaypointProperties {
965+
name: Some("Snapped Street".to_string()),
966+
distance: Some(4.2),
967+
})
968+
.expect("Infallible JSON serialization"),
969+
),
970+
};
971+
972+
let body_json = generate_body(
973+
USER_LOCATION,
974+
vec![
975+
Waypoint {
976+
coordinate: GeographicCoordinate { lat: 0.0, lng: 1.0 },
977+
kind: WaypointKind::Break,
978+
properties: None,
979+
},
980+
waypoint,
981+
],
982+
None,
983+
);
984+
985+
assert_eq!(
986+
body_json["locations"][2].get("name"),
987+
None,
988+
"OSRM snapped street name must not become a Valhalla location name"
989+
);
990+
}
931991
}

0 commit comments

Comments
 (0)