From 12eac8f8fecfd3204461879a2591e1b15cee4ac8 Mon Sep 17 00:00:00 2001 From: Max Leopold Date: Sun, 5 May 2024 21:23:01 +0200 Subject: [PATCH 1/7] Introduce Valhalla costing options --- .../ferrostar/core/FerrostarCore.kt | 3 +- .../Sources/FerrostarCore/FerrostarCore.swift | 4 +- apple/Sources/UniFFI/ferrostar.swift | 43 ++++++++-- common/ferrostar/src/lib.rs | 9 +- common/ferrostar/src/routing_adapters/mod.rs | 9 +- .../src/routing_adapters/valhalla.rs | 86 ++++++++++++++++--- 6 files changed, 128 insertions(+), 26 deletions(-) diff --git a/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt b/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt index a0f587c2..3f09f32f 100644 --- a/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt +++ b/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt @@ -107,9 +107,10 @@ class FerrostarCore( profile: String, httpClient: OkHttpClient, locationProvider: LocationProvider, + costingOptions: Map> = emptyMap(), ) : this( RouteProvider.RouteAdapter( - RouteAdapter.newValhallaHttp(valhallaEndpointURL.toString(), profile)), + RouteAdapter.newValhallaHttp(valhallaEndpointURL.toString(), profile, costingOptions)), httpClient, locationProvider, ) diff --git a/apple/Sources/FerrostarCore/FerrostarCore.swift b/apple/Sources/FerrostarCore/FerrostarCore.swift index 700cbb8b..a614fdbb 100644 --- a/apple/Sources/FerrostarCore/FerrostarCore.swift +++ b/apple/Sources/FerrostarCore/FerrostarCore.swift @@ -113,11 +113,13 @@ public protocol FerrostarCoreDelegate: AnyObject { valhallaEndpointUrl: URL, profile: String, locationProvider: LocationProviding, + costingOptions: [String: [String: String]] = [:], networkSession: URLRequestLoading = URLSession.shared ) { let adapter = RouteAdapter.newValhallaHttp( endpointUrl: valhallaEndpointUrl.absoluteString, - profile: profile + profile: profile, + costingOptions: costingOptions ) self.init( routeProvider: .routeAdapter(adapter), diff --git a/apple/Sources/UniFFI/ferrostar.swift b/apple/Sources/UniFFI/ferrostar.swift index 43ecc49d..1fecb779 100644 --- a/apple/Sources/UniFFI/ferrostar.swift +++ b/apple/Sources/UniFFI/ferrostar.swift @@ -661,11 +661,14 @@ public class RouteAdapter: try! rustCall { uniffi_ferrostar_fn_free_routeadapter(pointer, $0) } } - public static func newValhallaHttp(endpointUrl: String, profile: String) -> RouteAdapter { + public static func newValhallaHttp(endpointUrl: String, profile: String, + costingOptions: [String: [String: String]]) -> RouteAdapter + { RouteAdapter(unsafeFromRawPointer: try! rustCall { uniffi_ferrostar_fn_constructor_routeadapter_new_valhalla_http( FfiConverterString.lower(endpointUrl), - FfiConverterString.lower(profile), $0 + FfiConverterString.lower(profile), + FfiConverterDictionaryStringDictionaryStringString.lower(costingOptions), $0 ) }) } @@ -3449,6 +3452,29 @@ private struct FfiConverterDictionaryStringString: FfiConverterRustBuffer { } } +private struct FfiConverterDictionaryStringDictionaryStringString: FfiConverterRustBuffer { + public static func write(_ value: [String: [String: String]], into buf: inout [UInt8]) { + let len = Int32(value.count) + writeInt(&buf, len) + for (key, value) in value { + FfiConverterString.write(key, into: &buf) + FfiConverterDictionaryStringString.write(value, into: &buf) + } + } + + public static func read(from buf: inout (data: Data, offset: Data.Index)) throws -> [String: [String: String]] { + let len: Int32 = try readInt(&buf) + var dict = [String: [String: String]]() + dict.reserveCapacity(Int(len)) + for _ in 0 ..< len { + let key = try FfiConverterString.read(from: &buf) + let value = try FfiConverterDictionaryStringString.read(from: &buf) + dict[key] = value + } + return dict + } +} + /** * Typealias from the type name used in the UDL file to the builtin type. This * is needed because the UDL type name is used in function/method signatures. @@ -3523,12 +3549,17 @@ public func createOsrmResponseParser(polylinePrecision: UInt32) -> RouteResponse * * This is provided as a convenience for use from foreign code when creating your own [routing_adapters::RouteAdapter]. */ -public func createValhallaRequestGenerator(endpointUrl: String, profile: String) -> RouteRequestGenerator { +public func createValhallaRequestGenerator( + endpointUrl: String, + profile: String, + costingOptions: [String: [String: String]] +) -> RouteRequestGenerator { try! FfiConverterTypeRouteRequestGenerator.lift( try! rustCall { uniffi_ferrostar_fn_func_create_valhalla_request_generator( FfiConverterString.lower(endpointUrl), - FfiConverterString.lower(profile), $0 + FfiConverterString.lower(profile), + FfiConverterDictionaryStringDictionaryStringString.lower(costingOptions), $0 ) } ) @@ -3625,7 +3656,7 @@ private var initializationResult: InitializationResult { if uniffi_ferrostar_checksum_func_create_osrm_response_parser() != 28097 { return InitializationResult.apiChecksumMismatch } - if uniffi_ferrostar_checksum_func_create_valhalla_request_generator() != 35701 { + if uniffi_ferrostar_checksum_func_create_valhalla_request_generator() != 47939 { return InitializationResult.apiChecksumMismatch } if uniffi_ferrostar_checksum_func_get_route_polyline() != 53320 { @@ -3670,7 +3701,7 @@ private var initializationResult: InitializationResult { if uniffi_ferrostar_checksum_constructor_routeadapter_new() != 15081 { return InitializationResult.apiChecksumMismatch } - if uniffi_ferrostar_checksum_constructor_routeadapter_new_valhalla_http() != 24553 { + if uniffi_ferrostar_checksum_constructor_routeadapter_new_valhalla_http() != 40140 { return InitializationResult.apiChecksumMismatch } diff --git a/common/ferrostar/src/lib.rs b/common/ferrostar/src/lib.rs index 5ebaa03d..38885765 100644 --- a/common/ferrostar/src/lib.rs +++ b/common/ferrostar/src/lib.rs @@ -17,8 +17,8 @@ pub mod simulation; use crate::routing_adapters::osrm::OsrmResponseParser; use crate::routing_adapters::valhalla::ValhallaHttpRequestGenerator; -use std::str::FromStr; use std::sync::Arc; +use std::{collections::HashMap, str::FromStr}; use uuid::Uuid; use routing_adapters::{RouteRequestGenerator, RouteResponseParser}; @@ -55,8 +55,13 @@ impl UniffiCustomTypeConverter for Uuid { fn create_valhalla_request_generator( endpoint_url: String, profile: String, + costing_options: HashMap>, ) -> Arc { - Arc::new(ValhallaHttpRequestGenerator::new(endpoint_url, profile)) + Arc::new(ValhallaHttpRequestGenerator::new( + endpoint_url, + profile, + costing_options, + )) } /// Creates a [RouteResponseParser] capable of parsing OSRM responses. diff --git a/common/ferrostar/src/routing_adapters/mod.rs b/common/ferrostar/src/routing_adapters/mod.rs index 346b5bd1..0ae821a9 100644 --- a/common/ferrostar/src/routing_adapters/mod.rs +++ b/common/ferrostar/src/routing_adapters/mod.rs @@ -99,8 +99,13 @@ impl RouteAdapter { } #[uniffi::constructor] - pub fn new_valhalla_http(endpoint_url: String, profile: String) -> Self { - let request_generator = create_valhalla_request_generator(endpoint_url, profile); + pub fn new_valhalla_http( + endpoint_url: String, + profile: String, + costing_options: HashMap>, + ) -> Self { + let request_generator = + create_valhalla_request_generator(endpoint_url, profile, costing_options); let response_parser = create_osrm_response_parser(6); Self::new(request_generator, response_parser) } diff --git a/common/ferrostar/src/routing_adapters/valhalla.rs b/common/ferrostar/src/routing_adapters/valhalla.rs index e1c3fcef..a75ce967 100644 --- a/common/ferrostar/src/routing_adapters/valhalla.rs +++ b/common/ferrostar/src/routing_adapters/valhalla.rs @@ -17,14 +17,19 @@ pub struct ValhallaHttpRequestGenerator { /// Users *may* include a query string with an API key. endpoint_url: String, profile: String, - // TODO: more tunable parameters; a dict that gets inserted seems like a bare minimum; we can also allow higher level ones + costing_options: HashMap>, } impl ValhallaHttpRequestGenerator { - pub fn new(endpoint_url: String, profile: String) -> Self { + pub fn new( + endpoint_url: String, + profile: String, + costing_options: HashMap>, + ) -> Self { Self { endpoint_url, profile, + costing_options, } } } @@ -82,6 +87,7 @@ impl RouteRequestGenerator for ValhallaHttpRequestGenerator { "voice_instructions": true, "costing": &self.profile, "locations": locations, + "costing_options": &self.costing_options, }); let body = serde_json::to_vec(&args)?; Ok(RouteRequest::HttpPost { @@ -131,8 +137,11 @@ mod tests { #[test] fn not_enough_locations() { - let generator = - ValhallaHttpRequestGenerator::new(ENDPOINT_URL.to_string(), COSTING.to_string()); + let generator = ValhallaHttpRequestGenerator::new( + ENDPOINT_URL.to_string(), + COSTING.to_string(), + HashMap::new(), + ); // At least two locations are required assert!(matches!( @@ -141,9 +150,16 @@ mod tests { )); } - fn generate_body(user_location: UserLocation, waypoints: Vec) -> JsonValue { - let generator = - ValhallaHttpRequestGenerator::new(ENDPOINT_URL.to_string(), COSTING.to_string()); + fn generate_body( + user_location: UserLocation, + waypoints: Vec, + costing_options: HashMap>, + ) -> JsonValue { + let generator = ValhallaHttpRequestGenerator::new( + ENDPOINT_URL.to_string(), + COSTING.to_string(), + costing_options, + ); let RouteRequest::HttpPost { url: request_url, @@ -161,7 +177,7 @@ mod tests { #[test] fn request_body_without_course() { - let body_json = generate_body(USER_LOCATION, WAYPOINTS.to_vec()); + let body_json = generate_body(USER_LOCATION, WAYPOINTS.to_vec(), HashMap::new()); assert_json_include!( actual: body_json, @@ -181,14 +197,18 @@ mod tests { "lat": 2.0, "lon": 3.0, } - ] + ], }) ); } #[test] fn request_body_with_course() { - let body_json = generate_body(USER_LOCATION_WITH_COURSE, WAYPOINTS.to_vec()); + let body_json = generate_body( + USER_LOCATION_WITH_COURSE, + WAYPOINTS.to_vec(), + HashMap::new(), + ); assert_json_include!( actual: body_json, @@ -209,15 +229,53 @@ mod tests { "lat": 2.0, "lon": 3.0, } - ] + ], + }) + ); + } + + #[test] + fn request_body_without_costing_options() { + let body_json = generate_body(USER_LOCATION, WAYPOINTS.to_vec(), HashMap::new()); + + assert_json_include!( + actual: body_json, + expected: json!({ + "costing_options": {}, + }) + ); + } + + #[test] + fn request_body_with_costing_options() { + let body_json = generate_body( + USER_LOCATION, + WAYPOINTS.to_vec(), + HashMap::from([( + "bicycle".to_string(), + HashMap::from([("bicycle_type".to_string(), "Road".to_string())]), + )]), + ); + + assert_json_include!( + actual: body_json, + expected: json!({ + "costing_options": { + "bicycle": { + "bicycle_type": "Road", + }, + }, }) ); } #[test] fn request_body_with_invalid_horizontal_accuracy() { - let generator = - ValhallaHttpRequestGenerator::new(ENDPOINT_URL.to_string(), COSTING.to_string()); + let generator = ValhallaHttpRequestGenerator::new( + ENDPOINT_URL.to_string(), + COSTING.to_string(), + HashMap::new(), + ); let location = UserLocation { coordinates: GeographicCoordinate { lat: 0.0, lng: 0.0 }, horizontal_accuracy: -6.0, @@ -256,7 +314,7 @@ mod tests { "lat": 2.0, "lon": 3.0, } - ] + ], }) ); } From 5d3d31f1a8b4336e60bfc8f39be9760746151413 Mon Sep 17 00:00:00 2001 From: Max Leopold Date: Thu, 9 May 2024 13:02:41 +0200 Subject: [PATCH 2/7] Use JSON string to pass Valhalle costing options between Swift/Kotlin and Rust --- android/core/build.gradle | 1 + .../ferrostar/core/FerrostarCore.kt | 11 +++- .../Sources/FerrostarCore/FerrostarCore.swift | 17 +++++- apple/Sources/UniFFI/ferrostar.swift | 43 +++----------- common/ferrostar/src/lib.rs | 4 +- common/ferrostar/src/routing_adapters/mod.rs | 2 +- .../src/routing_adapters/valhalla.rs | 56 +++++++++---------- 7 files changed, 62 insertions(+), 72 deletions(-) diff --git a/android/core/build.gradle b/android/core/build.gradle index 53a5d9e2..213015d9 100644 --- a/android/core/build.gradle +++ b/android/core/build.gradle @@ -47,6 +47,7 @@ dependencies { implementation 'com.squareup.okhttp3:okhttp' implementation 'org.jetbrains.kotlinx:kotlinx-coroutines-core:1.7.3' implementation 'androidx.lifecycle:lifecycle-viewmodel-ktx:2.7.0' + implementation 'org.jetbrains.kotlinx:kotlinx-serialization-json:1.6.3' implementation "net.java.dev.jna:jna:5.12.0@aar" diff --git a/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt b/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt index 3f09f32f..30f89cb9 100644 --- a/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt +++ b/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt @@ -7,6 +7,8 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.serialization.encodeToString +import kotlinx.serialization.json.Json import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.RequestBody.Companion.toRequestBody @@ -107,10 +109,15 @@ class FerrostarCore( profile: String, httpClient: OkHttpClient, locationProvider: LocationProvider, - costingOptions: Map> = emptyMap(), + costingOptions: Map = emptyMap(), ) : this( RouteProvider.RouteAdapter( - RouteAdapter.newValhallaHttp(valhallaEndpointURL.toString(), profile, costingOptions)), + RouteAdapter.newValhallaHttp( + valhallaEndpointURL.toString(), + profile, + Json.encodeToString(costingOptions) + ) + ), httpClient, locationProvider, ) diff --git a/apple/Sources/FerrostarCore/FerrostarCore.swift b/apple/Sources/FerrostarCore/FerrostarCore.swift index a614fdbb..1030d077 100644 --- a/apple/Sources/FerrostarCore/FerrostarCore.swift +++ b/apple/Sources/FerrostarCore/FerrostarCore.swift @@ -108,18 +108,29 @@ public protocol FerrostarCoreDelegate: AnyObject { // Location provider setup locationProvider.delegate = self } + + enum ValhallaError: Error { + case invalidCostingOptions + } public convenience init( valhallaEndpointUrl: URL, profile: String, locationProvider: LocationProviding, - costingOptions: [String: [String: String]] = [:], + costingOptions: [String: Any] = [:], networkSession: URLRequestLoading = URLSession.shared - ) { + ) throws { + guard let jsonCostingOptions = String( + data: try JSONSerialization.data(withJSONObject: costingOptions), + encoding: .utf8 + ) else { + throw ValhallaError.invalidCostingOptions + } + let adapter = RouteAdapter.newValhallaHttp( endpointUrl: valhallaEndpointUrl.absoluteString, profile: profile, - costingOptions: costingOptions + costingOptions: jsonCostingOptions ) self.init( routeProvider: .routeAdapter(adapter), diff --git a/apple/Sources/UniFFI/ferrostar.swift b/apple/Sources/UniFFI/ferrostar.swift index 1fecb779..31874bce 100644 --- a/apple/Sources/UniFFI/ferrostar.swift +++ b/apple/Sources/UniFFI/ferrostar.swift @@ -661,14 +661,12 @@ public class RouteAdapter: try! rustCall { uniffi_ferrostar_fn_free_routeadapter(pointer, $0) } } - public static func newValhallaHttp(endpointUrl: String, profile: String, - costingOptions: [String: [String: String]]) -> RouteAdapter - { + public static func newValhallaHttp(endpointUrl: String, profile: String, costingOptions: String) -> RouteAdapter { RouteAdapter(unsafeFromRawPointer: try! rustCall { uniffi_ferrostar_fn_constructor_routeadapter_new_valhalla_http( FfiConverterString.lower(endpointUrl), FfiConverterString.lower(profile), - FfiConverterDictionaryStringDictionaryStringString.lower(costingOptions), $0 + FfiConverterString.lower(costingOptions), $0 ) }) } @@ -3452,29 +3450,6 @@ private struct FfiConverterDictionaryStringString: FfiConverterRustBuffer { } } -private struct FfiConverterDictionaryStringDictionaryStringString: FfiConverterRustBuffer { - public static func write(_ value: [String: [String: String]], into buf: inout [UInt8]) { - let len = Int32(value.count) - writeInt(&buf, len) - for (key, value) in value { - FfiConverterString.write(key, into: &buf) - FfiConverterDictionaryStringString.write(value, into: &buf) - } - } - - public static func read(from buf: inout (data: Data, offset: Data.Index)) throws -> [String: [String: String]] { - let len: Int32 = try readInt(&buf) - var dict = [String: [String: String]]() - dict.reserveCapacity(Int(len)) - for _ in 0 ..< len { - let key = try FfiConverterString.read(from: &buf) - let value = try FfiConverterDictionaryStringString.read(from: &buf) - dict[key] = value - } - return dict - } -} - /** * Typealias from the type name used in the UDL file to the builtin type. This * is needed because the UDL type name is used in function/method signatures. @@ -3549,17 +3524,15 @@ public func createOsrmResponseParser(polylinePrecision: UInt32) -> RouteResponse * * This is provided as a convenience for use from foreign code when creating your own [routing_adapters::RouteAdapter]. */ -public func createValhallaRequestGenerator( - endpointUrl: String, - profile: String, - costingOptions: [String: [String: String]] -) -> RouteRequestGenerator { +public func createValhallaRequestGenerator(endpointUrl: String, profile: String, + costingOptions: String) -> RouteRequestGenerator +{ try! FfiConverterTypeRouteRequestGenerator.lift( try! rustCall { uniffi_ferrostar_fn_func_create_valhalla_request_generator( FfiConverterString.lower(endpointUrl), FfiConverterString.lower(profile), - FfiConverterDictionaryStringDictionaryStringString.lower(costingOptions), $0 + FfiConverterString.lower(costingOptions), $0 ) } ) @@ -3656,7 +3629,7 @@ private var initializationResult: InitializationResult { if uniffi_ferrostar_checksum_func_create_osrm_response_parser() != 28097 { return InitializationResult.apiChecksumMismatch } - if uniffi_ferrostar_checksum_func_create_valhalla_request_generator() != 47939 { + if uniffi_ferrostar_checksum_func_create_valhalla_request_generator() != 3174 { return InitializationResult.apiChecksumMismatch } if uniffi_ferrostar_checksum_func_get_route_polyline() != 53320 { @@ -3701,7 +3674,7 @@ private var initializationResult: InitializationResult { if uniffi_ferrostar_checksum_constructor_routeadapter_new() != 15081 { return InitializationResult.apiChecksumMismatch } - if uniffi_ferrostar_checksum_constructor_routeadapter_new_valhalla_http() != 40140 { + if uniffi_ferrostar_checksum_constructor_routeadapter_new_valhalla_http() != 14445 { return InitializationResult.apiChecksumMismatch } diff --git a/common/ferrostar/src/lib.rs b/common/ferrostar/src/lib.rs index 38885765..6c39f500 100644 --- a/common/ferrostar/src/lib.rs +++ b/common/ferrostar/src/lib.rs @@ -17,8 +17,8 @@ pub mod simulation; use crate::routing_adapters::osrm::OsrmResponseParser; use crate::routing_adapters::valhalla::ValhallaHttpRequestGenerator; +use std::str::FromStr; use std::sync::Arc; -use std::{collections::HashMap, str::FromStr}; use uuid::Uuid; use routing_adapters::{RouteRequestGenerator, RouteResponseParser}; @@ -55,7 +55,7 @@ impl UniffiCustomTypeConverter for Uuid { fn create_valhalla_request_generator( endpoint_url: String, profile: String, - costing_options: HashMap>, + costing_options: String, ) -> Arc { Arc::new(ValhallaHttpRequestGenerator::new( endpoint_url, diff --git a/common/ferrostar/src/routing_adapters/mod.rs b/common/ferrostar/src/routing_adapters/mod.rs index 0ae821a9..4e918504 100644 --- a/common/ferrostar/src/routing_adapters/mod.rs +++ b/common/ferrostar/src/routing_adapters/mod.rs @@ -102,7 +102,7 @@ impl RouteAdapter { pub fn new_valhalla_http( endpoint_url: String, profile: String, - costing_options: HashMap>, + costing_options: String, ) -> Self { let request_generator = create_valhalla_request_generator(endpoint_url, profile, costing_options); diff --git a/common/ferrostar/src/routing_adapters/valhalla.rs b/common/ferrostar/src/routing_adapters/valhalla.rs index a75ce967..bd76e951 100644 --- a/common/ferrostar/src/routing_adapters/valhalla.rs +++ b/common/ferrostar/src/routing_adapters/valhalla.rs @@ -17,15 +17,11 @@ pub struct ValhallaHttpRequestGenerator { /// Users *may* include a query string with an API key. endpoint_url: String, profile: String, - costing_options: HashMap>, + costing_options: String, } impl ValhallaHttpRequestGenerator { - pub fn new( - endpoint_url: String, - profile: String, - costing_options: HashMap>, - ) -> Self { + pub fn new(endpoint_url: String, profile: String, costing_options: String) -> Self { Self { endpoint_url, profile, @@ -68,6 +64,8 @@ impl RouteRequestGenerator for ValhallaHttpRequestGenerator { }) })) .collect(); + + let parsed_costing_options: JsonValue = serde_json::from_str(&self.costing_options)?; // NOTE: We currently use the OSRM format, as it is the richest one. // Though it would be nice to use PBF if we can get the required data. // However, certain info (like banners) are only available in the OSRM format. @@ -87,7 +85,7 @@ impl RouteRequestGenerator for ValhallaHttpRequestGenerator { "voice_instructions": true, "costing": &self.profile, "locations": locations, - "costing_options": &self.costing_options, + "costing_options": parsed_costing_options, }); let body = serde_json::to_vec(&args)?; Ok(RouteRequest::HttpPost { @@ -140,7 +138,7 @@ mod tests { let generator = ValhallaHttpRequestGenerator::new( ENDPOINT_URL.to_string(), COSTING.to_string(), - HashMap::new(), + String::new(), ); // At least two locations are required @@ -153,7 +151,7 @@ mod tests { fn generate_body( user_location: UserLocation, waypoints: Vec, - costing_options: HashMap>, + costing_options: String, ) -> JsonValue { let generator = ValhallaHttpRequestGenerator::new( ENDPOINT_URL.to_string(), @@ -161,23 +159,26 @@ mod tests { costing_options, ); - let RouteRequest::HttpPost { - url: request_url, - headers, - body, - } = generator - .generate_request(user_location, waypoints) - .unwrap(); - - assert_eq!(ENDPOINT_URL, request_url); - assert_eq!(headers["Content-Type"], "application/json".to_string()); - - from_slice(&body).expect("Failed to parse request body as JSON") + match generator.generate_request(user_location, waypoints) { + Ok(RouteRequest::HttpPost { + url: request_url, + headers, + body, + }) => { + assert_eq!(ENDPOINT_URL, request_url); + assert_eq!(headers["Content-Type"], "application/json".to_string()); + from_slice(&body).expect("Failed to parse request body as JSON") + } + Err(e) => { + println!("Failed to generate request: {:?}", e); + json!(null) + } + } } #[test] fn request_body_without_course() { - let body_json = generate_body(USER_LOCATION, WAYPOINTS.to_vec(), HashMap::new()); + let body_json = generate_body(USER_LOCATION, WAYPOINTS.to_vec(), "{}".to_string()); assert_json_include!( actual: body_json, @@ -207,7 +208,7 @@ mod tests { let body_json = generate_body( USER_LOCATION_WITH_COURSE, WAYPOINTS.to_vec(), - HashMap::new(), + "{}".to_string(), ); assert_json_include!( @@ -236,7 +237,7 @@ mod tests { #[test] fn request_body_without_costing_options() { - let body_json = generate_body(USER_LOCATION, WAYPOINTS.to_vec(), HashMap::new()); + let body_json = generate_body(USER_LOCATION, WAYPOINTS.to_vec(), "{}".to_string()); assert_json_include!( actual: body_json, @@ -251,10 +252,7 @@ mod tests { let body_json = generate_body( USER_LOCATION, WAYPOINTS.to_vec(), - HashMap::from([( - "bicycle".to_string(), - HashMap::from([("bicycle_type".to_string(), "Road".to_string())]), - )]), + r#"{"bicycle": {"bicycle_type": "Road"}}"#.to_string(), ); assert_json_include!( @@ -274,7 +272,7 @@ mod tests { let generator = ValhallaHttpRequestGenerator::new( ENDPOINT_URL.to_string(), COSTING.to_string(), - HashMap::new(), + "{}".to_string(), ); let location = UserLocation { coordinates: GeographicCoordinate { lat: 0.0, lng: 0.0 }, From 321fd754f0b389a3f21e85d8f9c20c30ef8650f2 Mon Sep 17 00:00:00 2001 From: Ian Wagner Date: Sun, 12 May 2024 23:22:01 +0900 Subject: [PATCH 3/7] iOS cleanup first pass --- .../Sources/FerrostarCore/FerrostarCore.swift | 10 +- apple/Sources/UniFFI/ferrostar.swift | 14 ++- .../FerrostarCoreTests.swift | 28 +++++ .../ValhallaCoreTests.swift | 2 +- .../testValhalalCostingOptionsJSON.1.txt | 105 ++++++++++++++++++ common/ferrostar/src/lib.rs | 4 +- common/ferrostar/src/routing_adapters/mod.rs | 4 +- .../src/routing_adapters/valhalla.rs | 48 ++++---- 8 files changed, 175 insertions(+), 40 deletions(-) create mode 100644 apple/Tests/FerrostarCoreTests/__Snapshots__/FerrostarCoreTests/testValhalalCostingOptionsJSON.1.txt diff --git a/apple/Sources/FerrostarCore/FerrostarCore.swift b/apple/Sources/FerrostarCore/FerrostarCore.swift index 1030d077..936dce64 100644 --- a/apple/Sources/FerrostarCore/FerrostarCore.swift +++ b/apple/Sources/FerrostarCore/FerrostarCore.swift @@ -108,9 +108,9 @@ public protocol FerrostarCoreDelegate: AnyObject { // Location provider setup locationProvider.delegate = self } - + enum ValhallaError: Error { - case invalidCostingOptions + case unserializableCostingOptions } public convenience init( @@ -124,13 +124,13 @@ public protocol FerrostarCoreDelegate: AnyObject { data: try JSONSerialization.data(withJSONObject: costingOptions), encoding: .utf8 ) else { - throw ValhallaError.invalidCostingOptions + throw ValhallaError.unserializableCostingOptions } - + let adapter = RouteAdapter.newValhallaHttp( endpointUrl: valhallaEndpointUrl.absoluteString, profile: profile, - costingOptions: jsonCostingOptions + costingOptionsJson: jsonCostingOptions ) self.init( routeProvider: .routeAdapter(adapter), diff --git a/apple/Sources/UniFFI/ferrostar.swift b/apple/Sources/UniFFI/ferrostar.swift index 31874bce..980a0475 100644 --- a/apple/Sources/UniFFI/ferrostar.swift +++ b/apple/Sources/UniFFI/ferrostar.swift @@ -661,12 +661,14 @@ public class RouteAdapter: try! rustCall { uniffi_ferrostar_fn_free_routeadapter(pointer, $0) } } - public static func newValhallaHttp(endpointUrl: String, profile: String, costingOptions: String) -> RouteAdapter { + public static func newValhallaHttp(endpointUrl: String, profile: String, + costingOptionsJson: String?) -> RouteAdapter + { RouteAdapter(unsafeFromRawPointer: try! rustCall { uniffi_ferrostar_fn_constructor_routeadapter_new_valhalla_http( FfiConverterString.lower(endpointUrl), FfiConverterString.lower(profile), - FfiConverterString.lower(costingOptions), $0 + FfiConverterOptionString.lower(costingOptionsJson), $0 ) }) } @@ -3525,14 +3527,14 @@ public func createOsrmResponseParser(polylinePrecision: UInt32) -> RouteResponse * This is provided as a convenience for use from foreign code when creating your own [routing_adapters::RouteAdapter]. */ public func createValhallaRequestGenerator(endpointUrl: String, profile: String, - costingOptions: String) -> RouteRequestGenerator + costingOptionsJson: String?) -> RouteRequestGenerator { try! FfiConverterTypeRouteRequestGenerator.lift( try! rustCall { uniffi_ferrostar_fn_func_create_valhalla_request_generator( FfiConverterString.lower(endpointUrl), FfiConverterString.lower(profile), - FfiConverterString.lower(costingOptions), $0 + FfiConverterOptionString.lower(costingOptionsJson), $0 ) } ) @@ -3629,7 +3631,7 @@ private var initializationResult: InitializationResult { if uniffi_ferrostar_checksum_func_create_osrm_response_parser() != 28097 { return InitializationResult.apiChecksumMismatch } - if uniffi_ferrostar_checksum_func_create_valhalla_request_generator() != 3174 { + if uniffi_ferrostar_checksum_func_create_valhalla_request_generator() != 40698 { return InitializationResult.apiChecksumMismatch } if uniffi_ferrostar_checksum_func_get_route_polyline() != 53320 { @@ -3674,7 +3676,7 @@ private var initializationResult: InitializationResult { if uniffi_ferrostar_checksum_constructor_routeadapter_new() != 15081 { return InitializationResult.apiChecksumMismatch } - if uniffi_ferrostar_checksum_constructor_routeadapter_new_valhalla_http() != 14445 { + if uniffi_ferrostar_checksum_constructor_routeadapter_new_valhalla_http() != 19413 { return InitializationResult.apiChecksumMismatch } diff --git a/apple/Tests/FerrostarCoreTests/FerrostarCoreTests.swift b/apple/Tests/FerrostarCoreTests/FerrostarCoreTests.swift index fd245d5b..3ad98889 100644 --- a/apple/Tests/FerrostarCoreTests/FerrostarCoreTests.swift +++ b/apple/Tests/FerrostarCoreTests/FerrostarCoreTests.swift @@ -147,6 +147,34 @@ final class FerrostarCoreTests: XCTestCase { assertSnapshot(of: routes, as: .dump) } + @MainActor + func testValhalalCostingOptionsJSON() async throws { + let mockSession = MockURLSession() + mockSession.registerMock(forURL: valhallaEndpointUrl, withData: sampleRouteData, andResponse: successfulJSONResponse) + + // The main feature of this test is that it uses this constructor, + // which can throw, and similarly getRoutes may not always work with invalid input + let core = try FerrostarCore( + valhallaEndpointUrl: valhallaEndpointUrl, + profile: "low_speed_vehicle", + locationProvider: SimulatedLocationProvider(), + costingOptions: ["low_speed_vehicle": ["vehicle_type": "golf_cart"]], + networkSession: mockSession + ) + + // Tests that the core generates a request and then the mocked parser returns the expected routes + let routes = try await core.getRoutes( + initialLocation: UserLocation( + coordinates: GeographicCoordinate(lat: 60.5347155, lng: -149.543469), + horizontalAccuracy: 0, + courseOverGround: nil, + timestamp: Date() + ), + waypoints: [Waypoint(coordinate: GeographicCoordinate(lat: 60.5349908, lng: -149.5485806), kind: .break)] + ) + assertSnapshot(of: routes, as: .dump) + } + @MainActor func testCustomRouteProvider() async throws { let expectation = expectation(description: "The custom route provider should be called once") diff --git a/apple/Tests/FerrostarCoreTests/ValhallaCoreTests.swift b/apple/Tests/FerrostarCoreTests/ValhallaCoreTests.swift index df102473..b490491d 100644 --- a/apple/Tests/FerrostarCoreTests/ValhallaCoreTests.swift +++ b/apple/Tests/FerrostarCoreTests/ValhallaCoreTests.swift @@ -14,7 +14,7 @@ final class ValhallaCoreTests: XCTestCase { andResponse: successfulJSONResponse ) - let core = FerrostarCore( + let core = try FerrostarCore( valhallaEndpointUrl: valhallaEndpointUrl, profile: "auto", locationProvider: SimulatedLocationProvider(), diff --git a/apple/Tests/FerrostarCoreTests/__Snapshots__/FerrostarCoreTests/testValhalalCostingOptionsJSON.1.txt b/apple/Tests/FerrostarCoreTests/__Snapshots__/FerrostarCoreTests/testValhalalCostingOptionsJSON.1.txt new file mode 100644 index 00000000..2fcf242d --- /dev/null +++ b/apple/Tests/FerrostarCoreTests/__Snapshots__/FerrostarCoreTests/testValhalalCostingOptionsJSON.1.txt @@ -0,0 +1,105 @@ +▿ 1 element + ▿ Route + ▿ bbox: BoundingBox + ▿ ne: GeographicCoordinate + - lat: 60.535008 + - lng: -149.543469 + ▿ sw: GeographicCoordinate + - lat: 60.534716 + - lng: -149.548581 + - distance: 284.0 + ▿ geometry: 10 elements + ▿ GeographicCoordinate + - lat: 60.534716 + - lng: -149.543469 + ▿ GeographicCoordinate + - lat: 60.534782 + - lng: -149.543879 + ▿ GeographicCoordinate + - lat: 60.534829 + - lng: -149.544134 + ▿ GeographicCoordinate + - lat: 60.534856 + - lng: -149.5443 + ▿ GeographicCoordinate + - lat: 60.534887 + - lng: -149.544533 + ▿ GeographicCoordinate + - lat: 60.534941 + - lng: -149.544976 + ▿ GeographicCoordinate + - lat: 60.534971 + - lng: -149.545485 + ▿ GeographicCoordinate + - lat: 60.535003 + - lng: -149.546177 + ▿ GeographicCoordinate + - lat: 60.535008 + - lng: -149.546937 + ▿ GeographicCoordinate + - lat: 60.534991 + - lng: -149.548581 + ▿ steps: 2 elements + ▿ RouteStep + - distance: 284.0 + ▿ geometry: 10 elements + ▿ GeographicCoordinate + - lat: 60.534716 + - lng: -149.543469 + ▿ GeographicCoordinate + - lat: 60.534782 + - lng: -149.543879 + ▿ GeographicCoordinate + - lat: 60.534829 + - lng: -149.544134 + ▿ GeographicCoordinate + - lat: 60.534856 + - lng: -149.5443 + ▿ GeographicCoordinate + - lat: 60.534887 + - lng: -149.544533 + ▿ GeographicCoordinate + - lat: 60.534941 + - lng: -149.544976 + ▿ GeographicCoordinate + - lat: 60.534971 + - lng: -149.545485 + ▿ GeographicCoordinate + - lat: 60.535003 + - lng: -149.546177 + ▿ GeographicCoordinate + - lat: 60.535008 + - lng: -149.546937 + ▿ GeographicCoordinate + - lat: 60.534991 + - lng: -149.548581 + - instruction: "Drive west on AK 1/Seward Highway." + ▿ roadName: Optional + - some: "Seward Highway" + - spokenInstructions: 0 elements + - visualInstructions: 0 elements + ▿ RouteStep + - distance: 0.0 + ▿ geometry: 2 elements + ▿ GeographicCoordinate + - lat: 60.534991 + - lng: -149.548581 + ▿ GeographicCoordinate + - lat: 60.534991 + - lng: -149.548581 + - instruction: "You have arrived at your destination." + ▿ roadName: Optional + - some: "Seward Highway" + - spokenInstructions: 0 elements + - visualInstructions: 0 elements + ▿ waypoints: 2 elements + ▿ Waypoint + ▿ coordinate: GeographicCoordinate + - lat: 60.534715 + - lng: -149.543469 + - kind: WaypointKind.break + ▿ Waypoint + ▿ coordinate: GeographicCoordinate + - lat: 60.534991 + - lng: -149.548581 + - kind: WaypointKind.break diff --git a/common/ferrostar/src/lib.rs b/common/ferrostar/src/lib.rs index 6c39f500..4c8a36d3 100644 --- a/common/ferrostar/src/lib.rs +++ b/common/ferrostar/src/lib.rs @@ -55,12 +55,12 @@ impl UniffiCustomTypeConverter for Uuid { fn create_valhalla_request_generator( endpoint_url: String, profile: String, - costing_options: String, + costing_options_json: Option, ) -> Arc { Arc::new(ValhallaHttpRequestGenerator::new( endpoint_url, profile, - costing_options, + costing_options_json, )) } diff --git a/common/ferrostar/src/routing_adapters/mod.rs b/common/ferrostar/src/routing_adapters/mod.rs index 4e918504..3873eabf 100644 --- a/common/ferrostar/src/routing_adapters/mod.rs +++ b/common/ferrostar/src/routing_adapters/mod.rs @@ -102,10 +102,10 @@ impl RouteAdapter { pub fn new_valhalla_http( endpoint_url: String, profile: String, - costing_options: String, + costing_options_json: Option, ) -> Self { let request_generator = - create_valhalla_request_generator(endpoint_url, profile, costing_options); + create_valhalla_request_generator(endpoint_url, profile, costing_options_json); let response_parser = create_osrm_response_parser(6); Self::new(request_generator, response_parser) } diff --git a/common/ferrostar/src/routing_adapters/valhalla.rs b/common/ferrostar/src/routing_adapters/valhalla.rs index bd76e951..d29e437f 100644 --- a/common/ferrostar/src/routing_adapters/valhalla.rs +++ b/common/ferrostar/src/routing_adapters/valhalla.rs @@ -16,16 +16,22 @@ pub struct ValhallaHttpRequestGenerator { /// /// Users *may* include a query string with an API key. endpoint_url: String, + /// The Valhalla costing model to use. profile: String, - costing_options: String, + /// A string serialization of the JSON options to pass as costing options. + costing_options_json: Option, } impl ValhallaHttpRequestGenerator { - pub fn new(endpoint_url: String, profile: String, costing_options: String) -> Self { + pub fn new( + endpoint_url: String, + profile: String, + costing_options_json: Option, + ) -> Self { Self { endpoint_url, profile, - costing_options, + costing_options_json, } } } @@ -65,7 +71,11 @@ impl RouteRequestGenerator for ValhallaHttpRequestGenerator { })) .collect(); - let parsed_costing_options: JsonValue = serde_json::from_str(&self.costing_options)?; + let parsed_costing_options: JsonValue = match self.costing_options_json.as_deref() { + Some(options) => serde_json::from_str(options)?, + None => json!({}), + }; + // NOTE: We currently use the OSRM format, as it is the richest one. // Though it would be nice to use PBF if we can get the required data. // However, certain info (like banners) are only available in the OSRM format. @@ -135,11 +145,8 @@ mod tests { #[test] fn not_enough_locations() { - let generator = ValhallaHttpRequestGenerator::new( - ENDPOINT_URL.to_string(), - COSTING.to_string(), - String::new(), - ); + let generator = + ValhallaHttpRequestGenerator::new(ENDPOINT_URL.to_string(), COSTING.to_string(), None); // At least two locations are required assert!(matches!( @@ -151,12 +158,12 @@ mod tests { fn generate_body( user_location: UserLocation, waypoints: Vec, - costing_options: String, + costing_options_json: Option, ) -> JsonValue { let generator = ValhallaHttpRequestGenerator::new( ENDPOINT_URL.to_string(), COSTING.to_string(), - costing_options, + costing_options_json, ); match generator.generate_request(user_location, waypoints) { @@ -178,7 +185,7 @@ mod tests { #[test] fn request_body_without_course() { - let body_json = generate_body(USER_LOCATION, WAYPOINTS.to_vec(), "{}".to_string()); + let body_json = generate_body(USER_LOCATION, WAYPOINTS.to_vec(), None); assert_json_include!( actual: body_json, @@ -205,11 +212,7 @@ mod tests { #[test] fn request_body_with_course() { - let body_json = generate_body( - USER_LOCATION_WITH_COURSE, - WAYPOINTS.to_vec(), - "{}".to_string(), - ); + let body_json = generate_body(USER_LOCATION_WITH_COURSE, WAYPOINTS.to_vec(), None); assert_json_include!( actual: body_json, @@ -237,7 +240,7 @@ mod tests { #[test] fn request_body_without_costing_options() { - let body_json = generate_body(USER_LOCATION, WAYPOINTS.to_vec(), "{}".to_string()); + let body_json = generate_body(USER_LOCATION, WAYPOINTS.to_vec(), None); assert_json_include!( actual: body_json, @@ -252,7 +255,7 @@ mod tests { let body_json = generate_body( USER_LOCATION, WAYPOINTS.to_vec(), - r#"{"bicycle": {"bicycle_type": "Road"}}"#.to_string(), + Some(r#"{"bicycle": {"bicycle_type": "Road"}}"#.to_string()), ); assert_json_include!( @@ -269,11 +272,8 @@ mod tests { #[test] fn request_body_with_invalid_horizontal_accuracy() { - let generator = ValhallaHttpRequestGenerator::new( - ENDPOINT_URL.to_string(), - COSTING.to_string(), - "{}".to_string(), - ); + let generator = + ValhallaHttpRequestGenerator::new(ENDPOINT_URL.to_string(), COSTING.to_string(), None); let location = UserLocation { coordinates: GeographicCoordinate { lat: 0.0, lng: 0.0 }, horizontal_accuracy: -6.0, From b5f34863ca9e1e7a49a22a71e923e9c0028c07c7 Mon Sep 17 00:00:00 2001 From: Ian Wagner Date: Sun, 12 May 2024 23:28:44 +0900 Subject: [PATCH 4/7] Android switch to moshi; add some test cases; move parsing to init --- android/core/build.gradle | 2 +- .../ferrostar/core/ValhallaCoreTest.kt | 33 ++++++++++++- .../ferrostar/core/FerrostarCore.kt | 15 +++--- .../com/stadiamaps/ferrostar/MainActivity.kt | 1 + apple/DemoApp/Demo/DemoNavigationView.swift | 7 +-- .../Sources/FerrostarCore/FerrostarCore.swift | 8 +--- apple/Sources/UniFFI/ferrostar.swift | 46 ++++++++++++++++--- common/ferrostar/src/lib.rs | 7 +-- .../ferrostar/src/routing_adapters/error.rs | 12 +++++ common/ferrostar/src/routing_adapters/mod.rs | 7 +-- .../src/routing_adapters/valhalla.rs | 36 ++++++++++----- 11 files changed, 131 insertions(+), 43 deletions(-) diff --git a/android/core/build.gradle b/android/core/build.gradle index 213015d9..3e1e564a 100644 --- a/android/core/build.gradle +++ b/android/core/build.gradle @@ -45,9 +45,9 @@ dependencies { implementation 'androidx.core:core-ktx:1.12.0' implementation 'androidx.appcompat:appcompat:1.6.1' implementation 'com.squareup.okhttp3:okhttp' + implementation 'com.squareup.moshi:moshi:1.15.1' implementation 'org.jetbrains.kotlinx:kotlinx-coroutines-core:1.7.3' implementation 'androidx.lifecycle:lifecycle-viewmodel-ktx:2.7.0' - implementation 'org.jetbrains.kotlinx:kotlinx-serialization-json:1.6.3' implementation "net.java.dev.jna:jna:5.12.0@aar" diff --git a/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt b/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt index 69dd8fa3..b8834264 100644 --- a/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt +++ b/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt @@ -239,7 +239,7 @@ class ValhallaCoreTest { MockInterceptor().apply { rule(post, url eq valhallaEndpointUrl) { respond(simpleRoute, MEDIATYPE_JSON) } - rule(get) { respond { throw IllegalStateException("an IO error") } } + rule(get) { respond { throw IllegalStateException("Expected only one request") } } } val core = FerrostarCore( @@ -276,4 +276,35 @@ class ValhallaCoreTest { routes.first().geometry) } } + + @Test + fun valhallaRequestWithCostingOptions(): TestResult { + val interceptor = + MockInterceptor().apply { + rule(post, url eq valhallaEndpointUrl) { respond(simpleRoute, MEDIATYPE_JSON) } + + rule(get) { respond { throw IllegalStateException("Expected only one request") } } + } + val core = + FerrostarCore( + valhallaEndpointURL = URL(valhallaEndpointUrl), + profile = "auto", + httpClient = OkHttpClient.Builder().addInterceptor(interceptor).build(), + locationProvider = SimulatedLocationProvider(), + costingOptions = mapOf("auto" to mapOf("useTolls" to 0))) + + return runTest { + val routes = + core.getRoutes( + UserLocation( + GeographicCoordinate(60.5347155, -149.543469), 12.0, null, Instant.now()), + waypoints = + listOf( + Waypoint( + coordinate = GeographicCoordinate(60.5349908, -149.5485806), + kind = WaypointKind.BREAK))) + + assertEquals(routes.count(), 1) + } + } } diff --git a/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt b/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt index 30f89cb9..cc44d14d 100644 --- a/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt +++ b/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt @@ -1,5 +1,8 @@ package com.stadiamaps.ferrostar.core +import com.squareup.moshi.JsonAdapter +import com.squareup.moshi.Moshi +import com.squareup.moshi.adapter import java.net.URL import java.util.concurrent.Executors import kotlinx.coroutines.CoroutineScope @@ -7,8 +10,6 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch -import kotlinx.serialization.encodeToString -import kotlinx.serialization.json.Json import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.RequestBody.Companion.toRequestBody @@ -30,6 +31,10 @@ data class FerrostarCoreState( val isCalculatingNewRoute: Boolean ) +private val moshi: Moshi = Moshi.Builder().build() +@OptIn(ExperimentalStdlibApi::class) +private val jsonAdapter: JsonAdapter> = moshi.adapter>() + /** * This is the entrypoint for end users of Ferrostar on Android, and is responsible for "driving" * the navigation with location updates and other events. @@ -113,11 +118,7 @@ class FerrostarCore( ) : this( RouteProvider.RouteAdapter( RouteAdapter.newValhallaHttp( - valhallaEndpointURL.toString(), - profile, - Json.encodeToString(costingOptions) - ) - ), + valhallaEndpointURL.toString(), profile, jsonAdapter.toJson(costingOptions))), httpClient, locationProvider, ) diff --git a/android/demo-app/src/main/java/com/stadiamaps/ferrostar/MainActivity.kt b/android/demo-app/src/main/java/com/stadiamaps/ferrostar/MainActivity.kt index 9bf04898..a03a6a95 100644 --- a/android/demo-app/src/main/java/com/stadiamaps/ferrostar/MainActivity.kt +++ b/android/demo-app/src/main/java/com/stadiamaps/ferrostar/MainActivity.kt @@ -72,6 +72,7 @@ class MainActivity : ComponentActivity(), AndroidTtsStatusListener { profile = "bicycle", httpClient = httpClient, locationProvider = locationProvider, + costingOptions = mapOf("bicycle" to mapOf("use_roads" to 0.2)) ) private lateinit var ttsObserver: AndroidTtsObserver diff --git a/apple/DemoApp/Demo/DemoNavigationView.swift b/apple/DemoApp/Demo/DemoNavigationView.swift index 6b112c72..3fd89aba 100644 --- a/apple/DemoApp/Demo/DemoNavigationView.swift +++ b/apple/DemoApp/Demo/DemoNavigationView.swift @@ -37,12 +37,13 @@ struct DemoNavigationView: View { let simulated = SimulatedLocationProvider(location: initialLocation) simulated.warpFactor = 2 locationProvider = simulated - ferrostarCore = FerrostarCore( + ferrostarCore = try! FerrostarCore( valhallaEndpointUrl: URL( string: "https://api.stadiamaps.com/route/v1?api_key=\(APIKeys.shared.stadiaMapsAPIKey)" )!, - profile: "pedestrian", - locationProvider: locationProvider + profile: "bicycle", + locationProvider: locationProvider, + costingOptions: ["bicycle": ["use_roads": 0.2]] ) // NOTE: Not all applications will need a delegate. Read the NavigationDelegate documentation for details. ferrostarCore.delegate = navigationDelegate diff --git a/apple/Sources/FerrostarCore/FerrostarCore.swift b/apple/Sources/FerrostarCore/FerrostarCore.swift index 936dce64..ea56da77 100644 --- a/apple/Sources/FerrostarCore/FerrostarCore.swift +++ b/apple/Sources/FerrostarCore/FerrostarCore.swift @@ -109,10 +109,6 @@ public protocol FerrostarCoreDelegate: AnyObject { locationProvider.delegate = self } - enum ValhallaError: Error { - case unserializableCostingOptions - } - public convenience init( valhallaEndpointUrl: URL, profile: String, @@ -124,10 +120,10 @@ public protocol FerrostarCoreDelegate: AnyObject { data: try JSONSerialization.data(withJSONObject: costingOptions), encoding: .utf8 ) else { - throw ValhallaError.unserializableCostingOptions + throw InstantiationError.JsonError } - let adapter = RouteAdapter.newValhallaHttp( + let adapter = try RouteAdapter.newValhallaHttp( endpointUrl: valhallaEndpointUrl.absoluteString, profile: profile, costingOptionsJson: jsonCostingOptions diff --git a/apple/Sources/UniFFI/ferrostar.swift b/apple/Sources/UniFFI/ferrostar.swift index 980a0475..4452a04d 100644 --- a/apple/Sources/UniFFI/ferrostar.swift +++ b/apple/Sources/UniFFI/ferrostar.swift @@ -662,9 +662,9 @@ public class RouteAdapter: } public static func newValhallaHttp(endpointUrl: String, profile: String, - costingOptionsJson: String?) -> RouteAdapter + costingOptionsJson: String?) throws -> RouteAdapter { - RouteAdapter(unsafeFromRawPointer: try! rustCall { + try RouteAdapter(unsafeFromRawPointer: rustCallWithError(FfiConverterTypeInstantiationError.lift) { uniffi_ferrostar_fn_constructor_routeadapter_new_valhalla_http( FfiConverterString.lower(endpointUrl), FfiConverterString.lower(profile), @@ -2292,6 +2292,38 @@ public func FfiConverterTypeWaypoint_lower(_ value: Waypoint) -> RustBuffer { FfiConverterTypeWaypoint.lower(value) } +public enum InstantiationError { + case JsonError + + fileprivate static func uniffiErrorHandler(_ error: RustBuffer) throws -> Error { + try FfiConverterTypeInstantiationError.lift(error) + } +} + +public struct FfiConverterTypeInstantiationError: FfiConverterRustBuffer { + typealias SwiftType = InstantiationError + + public static func read(from buf: inout (data: Data, offset: Data.Index)) throws -> InstantiationError { + let variant: Int32 = try readInt(&buf) + switch variant { + case 1: return .JsonError + + default: throw UniffiInternalError.unexpectedEnumCase + } + } + + public static func write(_ value: InstantiationError, into buf: inout [UInt8]) { + switch value { + case .JsonError: + writeInt(&buf, Int32(1)) + } + } +} + +extension InstantiationError: Equatable, Hashable {} + +extension InstantiationError: Error {} + // Note that we don't yet support `indirect` for enums. // See https://github.com/mozilla/uniffi-rs/issues/396 for further discussion. /** @@ -3527,10 +3559,10 @@ public func createOsrmResponseParser(polylinePrecision: UInt32) -> RouteResponse * This is provided as a convenience for use from foreign code when creating your own [routing_adapters::RouteAdapter]. */ public func createValhallaRequestGenerator(endpointUrl: String, profile: String, - costingOptionsJson: String?) -> RouteRequestGenerator + costingOptionsJson: String?) throws -> RouteRequestGenerator { - try! FfiConverterTypeRouteRequestGenerator.lift( - try! rustCall { + try FfiConverterTypeRouteRequestGenerator.lift( + rustCallWithError(FfiConverterTypeInstantiationError.lift) { uniffi_ferrostar_fn_func_create_valhalla_request_generator( FfiConverterString.lower(endpointUrl), FfiConverterString.lower(profile), @@ -3631,7 +3663,7 @@ private var initializationResult: InitializationResult { if uniffi_ferrostar_checksum_func_create_osrm_response_parser() != 28097 { return InitializationResult.apiChecksumMismatch } - if uniffi_ferrostar_checksum_func_create_valhalla_request_generator() != 40698 { + if uniffi_ferrostar_checksum_func_create_valhalla_request_generator() != 11855 { return InitializationResult.apiChecksumMismatch } if uniffi_ferrostar_checksum_func_get_route_polyline() != 53320 { @@ -3676,7 +3708,7 @@ private var initializationResult: InitializationResult { if uniffi_ferrostar_checksum_constructor_routeadapter_new() != 15081 { return InitializationResult.apiChecksumMismatch } - if uniffi_ferrostar_checksum_constructor_routeadapter_new_valhalla_http() != 19413 { + if uniffi_ferrostar_checksum_constructor_routeadapter_new_valhalla_http() != 22565 { return InitializationResult.apiChecksumMismatch } diff --git a/common/ferrostar/src/lib.rs b/common/ferrostar/src/lib.rs index 4c8a36d3..f01c339f 100644 --- a/common/ferrostar/src/lib.rs +++ b/common/ferrostar/src/lib.rs @@ -22,6 +22,7 @@ use std::sync::Arc; use uuid::Uuid; use routing_adapters::{RouteRequestGenerator, RouteResponseParser}; +use crate::routing_adapters::error::InstantiationError; uniffi::setup_scaffolding!(); @@ -56,12 +57,12 @@ fn create_valhalla_request_generator( endpoint_url: String, profile: String, costing_options_json: Option, -) -> Arc { - Arc::new(ValhallaHttpRequestGenerator::new( +) -> Result, InstantiationError> { + Ok(Arc::new(ValhallaHttpRequestGenerator::with_costing_options_json( endpoint_url, profile, costing_options_json, - )) + )?)) } /// Creates a [RouteResponseParser] capable of parsing OSRM responses. diff --git a/common/ferrostar/src/routing_adapters/error.rs b/common/ferrostar/src/routing_adapters/error.rs index c7020830..5ea41594 100644 --- a/common/ferrostar/src/routing_adapters/error.rs +++ b/common/ferrostar/src/routing_adapters/error.rs @@ -4,6 +4,12 @@ use uniffi::UnexpectedUniFFICallbackError; // The trouble appears to be with generating "flat" enum bindings that are used with callback // interfaces when the underlying actually has fields. #[derive(Debug, thiserror::Error, uniffi::Error)] +pub enum InstantiationError { + #[error("Error generating JSON for the request.")] + JsonError, } + +// TODO: See comment above +#[derive(Debug, thiserror::Error, uniffi::Error)] pub enum RoutingRequestGenerationError { #[error("Too few waypoints were provided to compute a route.")] NotEnoughWaypoints, @@ -19,6 +25,12 @@ impl From for RoutingRequestGenerationError { } } +impl From for InstantiationError { + fn from(_: serde_json::Error) -> Self { + InstantiationError::JsonError + } +} + impl From for RoutingRequestGenerationError { fn from(_: serde_json::Error) -> Self { RoutingRequestGenerationError::JsonError diff --git a/common/ferrostar/src/routing_adapters/mod.rs b/common/ferrostar/src/routing_adapters/mod.rs index 3873eabf..9182d2a8 100644 --- a/common/ferrostar/src/routing_adapters/mod.rs +++ b/common/ferrostar/src/routing_adapters/mod.rs @@ -7,6 +7,7 @@ use error::{RoutingRequestGenerationError, RoutingResponseParseError}; use std::collections::HashMap; use std::fmt::Debug; use std::sync::Arc; +use crate::routing_adapters::error::InstantiationError; pub mod error; pub mod osrm; @@ -103,11 +104,11 @@ impl RouteAdapter { endpoint_url: String, profile: String, costing_options_json: Option, - ) -> Self { + ) -> Result { let request_generator = - create_valhalla_request_generator(endpoint_url, profile, costing_options_json); + create_valhalla_request_generator(endpoint_url, profile, costing_options_json)?; let response_parser = create_osrm_response_parser(6); - Self::new(request_generator, response_parser) + Ok(Self::new(request_generator, response_parser)) } // diff --git a/common/ferrostar/src/routing_adapters/valhalla.rs b/common/ferrostar/src/routing_adapters/valhalla.rs index d29e437f..ef7476f1 100644 --- a/common/ferrostar/src/routing_adapters/valhalla.rs +++ b/common/ferrostar/src/routing_adapters/valhalla.rs @@ -18,22 +18,39 @@ pub struct ValhallaHttpRequestGenerator { endpoint_url: String, /// The Valhalla costing model to use. profile: String, - /// A string serialization of the JSON options to pass as costing options. - costing_options_json: Option, + // TODO: Language, units, and other top-level parameters + /// JSON costing options to pass through. + costing_options: JsonValue, } impl ValhallaHttpRequestGenerator { pub fn new( endpoint_url: String, profile: String, - costing_options_json: Option, + costing_options: Option, ) -> Self { Self { endpoint_url, profile, - costing_options_json, + costing_options: costing_options.unwrap_or(json!({})), } } + + pub fn with_costing_options_json( + endpoint_url: String, + profile: String, + costing_options_json: Option, + ) -> Result { + let parsed_costing_options: JsonValue = match costing_options_json.as_deref() { + Some(options) => serde_json::from_str(options)?, + None => json!({}), + }; + Ok(Self { + endpoint_url, + profile, + costing_options: parsed_costing_options, + }) + } } impl RouteRequestGenerator for ValhallaHttpRequestGenerator { @@ -71,11 +88,6 @@ impl RouteRequestGenerator for ValhallaHttpRequestGenerator { })) .collect(); - let parsed_costing_options: JsonValue = match self.costing_options_json.as_deref() { - Some(options) => serde_json::from_str(options)?, - None => json!({}), - }; - // NOTE: We currently use the OSRM format, as it is the richest one. // Though it would be nice to use PBF if we can get the required data. // However, certain info (like banners) are only available in the OSRM format. @@ -95,7 +107,7 @@ impl RouteRequestGenerator for ValhallaHttpRequestGenerator { "voice_instructions": true, "costing": &self.profile, "locations": locations, - "costing_options": parsed_costing_options, + "costing_options": &self.costing_options, }); let body = serde_json::to_vec(&args)?; Ok(RouteRequest::HttpPost { @@ -160,11 +172,11 @@ mod tests { waypoints: Vec, costing_options_json: Option, ) -> JsonValue { - let generator = ValhallaHttpRequestGenerator::new( + let generator = ValhallaHttpRequestGenerator::with_costing_options_json( ENDPOINT_URL.to_string(), COSTING.to_string(), costing_options_json, - ); + ).expect("Unable to create request generator"); match generator.generate_request(user_location, waypoints) { Ok(RouteRequest::HttpPost { From bc8eb4dc1c44744188c83f830ebff159f98efa78 Mon Sep 17 00:00:00 2001 From: Ian Wagner Date: Sun, 12 May 2024 23:52:16 +0900 Subject: [PATCH 5/7] Resolve issues after merge; bump versions --- Package.swift | 2 +- android/build.gradle | 2 +- .../java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt | 2 +- .../src/main/java/com/stadiamaps/ferrostar/MainActivity.kt | 3 +-- apple/Sources/FerrostarCore/FerrostarCore.swift | 4 ++-- apple/Tests/FerrostarCoreTests/FerrostarCoreTests.swift | 3 ++- .../FerrostarCoreTests/testValhalalCostingOptionsJSON.1.txt | 2 ++ common/Cargo.lock | 2 +- common/ferrostar/Cargo.toml | 2 +- 9 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Package.swift b/Package.swift index 3c6c9ffa..1177ccda 100644 --- a/Package.swift +++ b/Package.swift @@ -16,7 +16,7 @@ if useLocalFramework { path: "./common/target/ios/libferrostar-rs.xcframework" ) } else { - let releaseTag = "0.0.31" + let releaseTag = "0.0.32" let releaseChecksum = "a21146211e43922850a287b7cb713e3515d9b4f3863c2a1066a3f4d8af1d302c" binaryTarget = .binaryTarget( name: "FerrostarCoreRS", diff --git a/android/build.gradle b/android/build.gradle index 5c899d84..7345f8af 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -23,5 +23,5 @@ publishing { allprojects { group = "com.stadiamaps.ferrostar" - version = "0.0.31" + version = "0.0.32" } diff --git a/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt b/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt index e41d92a4..a15bcae7 100644 --- a/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt +++ b/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt @@ -297,7 +297,7 @@ class ValhallaCoreTest { val routes = core.getRoutes( UserLocation( - GeographicCoordinate(60.5347155, -149.543469), 12.0, null, Instant.now()), + GeographicCoordinate(60.5347155, -149.543469), 12.0, null, Instant.now(), null), waypoints = listOf( Waypoint( diff --git a/android/demo-app/src/main/java/com/stadiamaps/ferrostar/MainActivity.kt b/android/demo-app/src/main/java/com/stadiamaps/ferrostar/MainActivity.kt index 1bee2eb5..3122003a 100644 --- a/android/demo-app/src/main/java/com/stadiamaps/ferrostar/MainActivity.kt +++ b/android/demo-app/src/main/java/com/stadiamaps/ferrostar/MainActivity.kt @@ -76,8 +76,7 @@ class MainActivity : ComponentActivity(), AndroidTtsStatusListener { profile = "bicycle", httpClient = httpClient, locationProvider = locationProvider, - costingOptions = mapOf("bicycle" to mapOf("use_roads" to 0.2)) - ) + costingOptions = mapOf("bicycle" to mapOf("use_roads" to 0.2))) private lateinit var ttsObserver: AndroidTtsObserver diff --git a/apple/Sources/FerrostarCore/FerrostarCore.swift b/apple/Sources/FerrostarCore/FerrostarCore.swift index 39c465d3..6abe6bb8 100644 --- a/apple/Sources/FerrostarCore/FerrostarCore.swift +++ b/apple/Sources/FerrostarCore/FerrostarCore.swift @@ -116,8 +116,8 @@ public protocol FerrostarCoreDelegate: AnyObject { costingOptions: [String: Any] = [:], networkSession: URLRequestLoading = URLSession.shared ) throws { - guard let jsonCostingOptions = String( - data: try JSONSerialization.data(withJSONObject: costingOptions), + guard let jsonCostingOptions = try String( + data: JSONSerialization.data(withJSONObject: costingOptions), encoding: .utf8 ) else { throw InstantiationError.JsonError diff --git a/apple/Tests/FerrostarCoreTests/FerrostarCoreTests.swift b/apple/Tests/FerrostarCoreTests/FerrostarCoreTests.swift index 0a4c4753..54d9437f 100644 --- a/apple/Tests/FerrostarCoreTests/FerrostarCoreTests.swift +++ b/apple/Tests/FerrostarCoreTests/FerrostarCoreTests.swift @@ -175,7 +175,8 @@ final class FerrostarCoreTests: XCTestCase { coordinates: GeographicCoordinate(lat: 60.5347155, lng: -149.543469), horizontalAccuracy: 0, courseOverGround: nil, - timestamp: Date() + timestamp: Date(), + speed: nil ), waypoints: [Waypoint(coordinate: GeographicCoordinate(lat: 60.5349908, lng: -149.5485806), kind: .break)] ) diff --git a/apple/Tests/FerrostarCoreTests/__Snapshots__/FerrostarCoreTests/testValhalalCostingOptionsJSON.1.txt b/apple/Tests/FerrostarCoreTests/__Snapshots__/FerrostarCoreTests/testValhalalCostingOptionsJSON.1.txt index 2fcf242d..ce00d071 100644 --- a/apple/Tests/FerrostarCoreTests/__Snapshots__/FerrostarCoreTests/testValhalalCostingOptionsJSON.1.txt +++ b/apple/Tests/FerrostarCoreTests/__Snapshots__/FerrostarCoreTests/testValhalalCostingOptionsJSON.1.txt @@ -42,6 +42,7 @@ ▿ steps: 2 elements ▿ RouteStep - distance: 284.0 + - duration: 11.488 ▿ geometry: 10 elements ▿ GeographicCoordinate - lat: 60.534716 @@ -80,6 +81,7 @@ - visualInstructions: 0 elements ▿ RouteStep - distance: 0.0 + - duration: 0.0 ▿ geometry: 2 elements ▿ GeographicCoordinate - lat: 60.534991 diff --git a/common/Cargo.lock b/common/Cargo.lock index 09399534..8c8c72c6 100644 --- a/common/Cargo.lock +++ b/common/Cargo.lock @@ -336,7 +336,7 @@ checksum = "25cbce373ec4653f1a01a31e8a5e5ec0c622dc27ff9c4e6606eefef5cbbed4a5" [[package]] name = "ferrostar" -version = "0.0.31" +version = "0.0.32" dependencies = [ "assert-json-diff", "geo", diff --git a/common/ferrostar/Cargo.toml b/common/ferrostar/Cargo.toml index 29ae0f41..68d32820 100644 --- a/common/ferrostar/Cargo.toml +++ b/common/ferrostar/Cargo.toml @@ -2,7 +2,7 @@ lints.workspace = true [package] name = "ferrostar" -version = "0.0.31" +version = "0.0.32" readme = "README.md" description = "The core of modern turn-by-turn navigation." keywords = ["navigation", "routing", "valhalla", "osrm"] From 0274d2c0d35732bff374347d88331c4222524eea Mon Sep 17 00:00:00 2001 From: Ian Wagner Date: Sun, 12 May 2024 23:54:02 +0900 Subject: [PATCH 6/7] cargo fmt --- common/ferrostar/src/lib.rs | 14 ++++++++------ common/ferrostar/src/routing_adapters/error.rs | 3 ++- common/ferrostar/src/routing_adapters/mod.rs | 2 +- common/ferrostar/src/routing_adapters/valhalla.rs | 9 +++------ 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/common/ferrostar/src/lib.rs b/common/ferrostar/src/lib.rs index 2d41e384..4bc47d7a 100644 --- a/common/ferrostar/src/lib.rs +++ b/common/ferrostar/src/lib.rs @@ -21,8 +21,8 @@ use std::str::FromStr; use std::sync::Arc; use uuid::Uuid; -use routing_adapters::{RouteRequestGenerator, RouteResponseParser}; use crate::routing_adapters::error::InstantiationError; +use routing_adapters::{RouteRequestGenerator, RouteResponseParser}; uniffi::setup_scaffolding!(); @@ -58,11 +58,13 @@ fn create_valhalla_request_generator( profile: String, costing_options_json: Option, ) -> Result, InstantiationError> { - Ok(Arc::new(ValhallaHttpRequestGenerator::with_costing_options_json( - endpoint_url, - profile, - costing_options_json, - )?)) + Ok(Arc::new( + ValhallaHttpRequestGenerator::with_costing_options_json( + endpoint_url, + profile, + costing_options_json, + )?, + )) } /// Creates a [`RouteResponseParser`] capable of parsing OSRM responses. diff --git a/common/ferrostar/src/routing_adapters/error.rs b/common/ferrostar/src/routing_adapters/error.rs index 5ea41594..5c42208b 100644 --- a/common/ferrostar/src/routing_adapters/error.rs +++ b/common/ferrostar/src/routing_adapters/error.rs @@ -6,7 +6,8 @@ use uniffi::UnexpectedUniFFICallbackError; #[derive(Debug, thiserror::Error, uniffi::Error)] pub enum InstantiationError { #[error("Error generating JSON for the request.")] - JsonError, } + JsonError, +} // TODO: See comment above #[derive(Debug, thiserror::Error, uniffi::Error)] diff --git a/common/ferrostar/src/routing_adapters/mod.rs b/common/ferrostar/src/routing_adapters/mod.rs index 8a91980c..c5181f4a 100644 --- a/common/ferrostar/src/routing_adapters/mod.rs +++ b/common/ferrostar/src/routing_adapters/mod.rs @@ -1,4 +1,5 @@ use crate::models::Waypoint; +use crate::routing_adapters::error::InstantiationError; use crate::{ create_osrm_response_parser, create_valhalla_request_generator, models::{Route, UserLocation}, @@ -7,7 +8,6 @@ use error::{RoutingRequestGenerationError, RoutingResponseParseError}; use std::collections::HashMap; use std::fmt::Debug; use std::sync::Arc; -use crate::routing_adapters::error::InstantiationError; pub mod error; pub mod osrm; diff --git a/common/ferrostar/src/routing_adapters/valhalla.rs b/common/ferrostar/src/routing_adapters/valhalla.rs index f45997ec..a7aa7ab1 100644 --- a/common/ferrostar/src/routing_adapters/valhalla.rs +++ b/common/ferrostar/src/routing_adapters/valhalla.rs @@ -24,11 +24,7 @@ pub struct ValhallaHttpRequestGenerator { } impl ValhallaHttpRequestGenerator { - pub fn new( - endpoint_url: String, - profile: String, - costing_options: Option, - ) -> Self { + pub fn new(endpoint_url: String, profile: String, costing_options: Option) -> Self { Self { endpoint_url, profile, @@ -178,7 +174,8 @@ mod tests { ENDPOINT_URL.to_string(), COSTING.to_string(), costing_options_json, - ).expect("Unable to create request generator"); + ) + .expect("Unable to create request generator"); match generator.generate_request(user_location, waypoints) { Ok(RouteRequest::HttpPost { From fad3c167f0b7fb2a3945d16e17cc376dd6d91367 Mon Sep 17 00:00:00 2001 From: Ian Wagner Date: Sun, 12 May 2024 23:56:05 +0900 Subject: [PATCH 7/7] ktfmtFormat --- .../java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt b/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt index a15bcae7..ff7f7b51 100644 --- a/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt +++ b/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/ValhallaCoreTest.kt @@ -297,7 +297,7 @@ class ValhallaCoreTest { val routes = core.getRoutes( UserLocation( - GeographicCoordinate(60.5347155, -149.543469), 12.0, null, Instant.now(), null), + GeographicCoordinate(60.5347155, -149.543469), 12.0, null, Instant.now(), null), waypoints = listOf( Waypoint(