handle Graphhopper requests

[karussell]

Please review carefully as I'm not a native speaker of Rust or Kotlin :)

My initial approach was to convert the "native" GraphHopper response inside ferrostar, but due to some problems and because it makes other situations also simpler I've created a server-side solution, where the endpoint accepts a "native" GraphHopper (POST) request and returns the OSRM-alike response, which is already supported from ferrostar and e.g. maxspeed annotations worked out of the box. (I kept the early commits mainly for myself and I think this is fine as you are squashing commits anyway?)

Currently there are no tests as I wasn't sure what to test and also there is this notice "move out of Android Tests ASAP". Is this still valid? I could test e.g. that the request is properly formed - just let me know if I should e.g. copy ValhallaCoreTest.kt or if I should do something else.

And should we create a more generic route adapter creation like this?

RouteAdapter.newHttp(routingEngine, routingEndpointURL.toString(), profile, jsonAdapter.toJson(options))

Otherwise we have to duplicate the cases of the different routing engines at a higher level in all different environment. One tricky part for other engines could be the authentication as it is often not done via a simple API key in the URL (like used from the Stadia Maps and the Graphhopper API).

Let me know which one you prefer and I could try to make the web/wasm environment working.

Currently there is also no support for iOS and react-native, but it should be only a few lines as you can see in the Kotlin code. Unfortunately, I probably won't be of much help here.

[ianthetechie]

Thanks for the PR @karussell! Will try to get this reviewed in greater detail within the next day or so!

Please review carefully as I'm not a native speaker of Rust or Kotlin :)

Haha no worries :)

(I kept the early commits mainly for myself and I think this is fine as you are squashing commits anyway?)

Correct; no complaints with extra commits from us!

Currently there are no tests as I wasn't sure what to test and also there is this notice "move out of Android Tests ASAP". Is this still valid?

Yeaaaaaaahhhhh.... there were some complications we ran into with trying to test the full stack, JNA bits and all, from a "regular" JUnit test. These questions were never fully resolved. So, for Android, we have to run a lot of tests that should theoretically be boring JUnit tests as "connected Android checks" on an emulator / device.

And should we create a more generic route adapter creation like this?

RouteAdapter.newHttp(routingEngine, routingEndpointURL.toString(), profile, jsonAdapter.toJson(options))

I'll have to give that a bit of thought as a review this.

One tricky part for other engines could be the authentication as it is often not done via a simple API key in the URL (like used from the Stadia Maps and the Graphhopper API).

Do you have a specific example? The methods I'm aware of are query string and header-based auth, both of which are supported. JWT is also starting to be used (Apple Maps for JavaScript is the only geospatial related one I can think of), but I think the current set of interfaces still work for that.

Currently there is also no support for iOS and react-native, but it should be only a few lines as you can see in the Kotlin code.

No worries; we can handle the other platforms :)

[Archdoog]

Some initial comments. Overall looking simple enough. Will let @ianthetechie critique the rust stuff.

[Archdoog]

We should probably consider a strategy where this is a sealed class with options. E.g.

sealed class RoutingEngine {
    data class Valhalla(val endpoint: String) : RoutingEngine
    data class GraphHopper(val endpoint: String): RoutingEngine
    // TODO: we might be able to move the CustomRouteProvider in here somehow as well.
}

For swift this would look something like:

enum RoutingEngine {
    case valhalla(String)
    case graphHopper(String)
}

[ianthetechie]

Yeah, agreed. I can take another pass at that. This is one of those weird Kotlin things ;)

[karussell]

Not sure if I fully understand this but I have added this approach in the last commit. Please see the change and let me know if this is what you meant. (I also included the routing profile as it is dependent on the routing engine)

[Archdoog]

Accepting the sealed class above, this would become. when statement.

[ianthetechie]

Sorry this took a bit for me to get around to! This is really great! I've just done some cleanup + added docs. If you could review the questions (esp verifying the JSON output :D), we can get this merged pretty quickly + I'll handle the iOS side.

[ianthetechie]

Not sure why this was commented out...

[karussell]

Please have a look. I made it configurable with a boolean now.

[ianthetechie]

With Valhalla, we leave locale and voice units unspecified, but you had them hard-coded, which makes me wonder if these are required and/or strongly encouraged for every request. I've made some changes to the interface assuming the answer is "yes" but please let me know your thoughts. For Valhalla, we currently just handle this with extra JSON options, but I'd like to move toward making the options easier to use with strong types, so here's a first pass at this.

cc @Archdoog in case you have any comments.

[karussell]

Thanks for the review! (Sorry for the delay :) )

With Valhalla, we leave locale and voice units unspecified, but you had them hard-coded, which makes me wonder if these are required and/or strongly encouraged for every request.

They are not required but I thought I make them explicit as they are often the first things users want to change beside profile and points. (Let's remove them to make it more similar to valhalla.)

The methods I'm aware of are query string and header-based auth, both of which are supported.

Ah, ok. Thought only query string auth was supported.

[karussell]

@ianthetechie @Archdoog anything missing that I can do? I can e.g. fix the merge conflicts but would prefer to do it close to the merge :)