[Story] Customise maps provider

[Johennes]

• As a maintainer of a fork
• I want to be able to use my own provider for
  • the map tile server
  • the static map images API
• so that I can pick the option that works for my budget and privacy concerns

Size estimate

M

Dependencies

• [Prep] Set up SDK & reusable map view component #1684

Acceptance criteria

• The app does not value the tile server setting from .well-known because this doesn't cover static maps and will likely require a holistic redesign later
• Setting the API key or replacing the map provider URLs in forks is easy enough (e.g. dedicated file or interface)
• There is documentation in the repositories that explains how to replace the API key and URLs in forks
• The API keys used for Element's apps must not be put in the source code but rather they must be baked into the .apk/.ipa at build time using a GitHub Actions secret
  • For local builds (during development) a dev-only API key must be used in place of the platform-specific ones

Other

• It looks like we are already using a "provider" mechanism for allowing different push notification providers. In the android code is called "pushproviders" perhaps we could create a similar architecture for maps (e.g. a map providers interface which will have multiple implementations: maptiler... and perhaps others)

Out of scope

• Nothing

Subtasks

Android

Beta Give feedback

1. Location sharing: don't hardcode API key vector-im/element-x-android#886
   +0

iOS

Beta Give feedback

1. Move Map Libre API key to Xcode Cloud vector-im/element-x-ios#1313
   +0
2. Refactor map styles for forking vector-im/element-x-ios#1364
   +0
   

Other

Beta Give feedback

No tasks being tracked yet.

[alfogrillo]

@Johennes @julioromano should we do anything in this epic for this?
I fell like the code on iOS is already modular enough to enable a map provider switch very easily.
In other words boundaries between MapTiler and EX are very clear ATM.

[Johennes]

@alfogrillo what you say sounds like customizing the map provider by changing the app source code? I think ideally, there'd be a way to control the map provider via the home server's .well-known. That would allow users that are unable or unwilling to build and distribute the app themselves to opt out from our default map provider. MSC3488 does support this but only for the tile server URL, not for the static maps API.

[alfogrillo]

@alfogrillo what you say sounds like customizing the map provider by changing the app source code

Yep, we are quite ready for that.

I mean I think ideally, there'd be a way to control the map provider via the home server's .well-known

I'm not sure how we can make this generic across all the map providers.
Afaik other providers may use different endpoints for static/tiled maps for example.
And even if the format is the same, work needs be done on the source code to switch to the wanted provider sdk.

Maybe we may change the MSC3488 to introduce map provider specific information?
Would that be helpful?

Example (.well-known)

{
    "m.tile_server": { 
        "map_tiler": {
            "map_light_style_url": "https://www.example.com/style_light.json",
            "map_dark_style_url": "https://www.example.com/style_dark.json",
            "static_map_light_style_url": "https://www.example.com/static/style_light.json",
            "static_map_dark_style_url": "https://www.example.com/staic/style_dark.json"
        },
        "google_maps": {
            "some_url": "https://www.example.com/style.json"
        }
    },
    
    "m.homeserver": {
        "base_url": "https://matrix-client.matrix.org"
    },
    "m.identity_server": {
        "base_url": "https://vector.im"
    }
} 

[Johennes]

Maybe we may change the matrix-org/matrix-spec-proposals#3488 to introduce map provider specific information?

Yes, we'd likely need to change this in the MSC, too. It currently only includes the tile server configuration via .well-known.

"m.tile_server": { 
    "map_style_url": "https://www.example.com/style.json"
}

Your idea of having different objects for different map providers in .well-known sounds interesting. I suppose there would also have to be something in there that instructs the app which provider to choose if there are several?

I think a general problem there is that e.g. MapTiler and Mapbox use different formats for the static maps endpoints. MapTiler uses

https://api.maptiler.com/maps/{mapId}/static/{lon},{lat},{zoom}/{width}x{height}{scale}.{format}

while Mapbox uses

https://api.mapbox.com/styles/v1/{username}/{style_id}/static/{overlay}/{lon},{lat},{zoom},{bearing},{pitch}|{bbox}|{auto}/{width}x{height}{@2x}

So I think in addition to the URL to request we'd also need to know where to insert the various parameters.

One slightly wild idea @julioromano and I had discussed a while ago was to add a URL template for the static maps endpoint in .well-known. Then to use your custom MapTiler account, you would specify something like

"template": "https://api.maptiler.com/maps/.../static/${lon},${lat},${zoom}/${width}x${height}${2x}.png"

whereas to use your Mapbox account, you could specify something like

"template": "https://api.mapbox.com/styles/v1/.../.../static/.../${lon},${lat},${zoom},auto/${width}x${height}${2x}"

The app could then replace all the various parameters (e.g. ${lon} with the actual longitude).

This isn't quite perfect though. It may not work for other static map providers and it definitely won't work when we add the geocoding API later because those return JSON that needs to be parsed differently for each provider. So that would probably only be a stopgap solution at best.

The MSC also doesn't currently support different tile server URLs for light and dark mode which is a similar problem and might require changes to the existing m.tile_server field.

[Johennes]

I've updated the description above with what we aligned on in the weekly today.

[csmith]

Open question: what do we do when there's no API key set?

If we don't do anything, the existing error state allows you to retry the map load, but that will never work.

Do we want to add a custom state for when the API key is missing? A placeholder image with text along the lines of "Viewing locations is not configured in this version"?

[Johennes]

Good find. Making the error explicit sounds like a good idea to me. It'll prevent us getting vague error reports about this from forkers later.

[Johennes]

As for the wording, I think we can probably keep it tech-y because it should not be a common error users see. @callumu do you have opinions?

[callumu]

As for the wording, I think we can probably keep it tech-y because it should not be a common error users see. @callumu do you have opinions?

Could you clarify when the error would show and what it exactly says? I'm mainly confused about when it will show

[julioromano]

Open question: what do we do when there's no API key set?

If we don't do anything, the existing error state allows you to retry the map load, but that will never work.

Do we want to add a custom state for when the API key is missing? A placeholder image with text along the lines of "Viewing locations is not configured in this version"?

AFAIK the app requires a key to be set or it wont' build. So at runtime there will always be one.

[csmith]

Could you clarify when the error would show and what it exactly says? I'm mainly confused about when it will show

@callumu it would never show in the apps we release, just if someone makes a version themselves and doesn't add their own mapbox key. So, say, an open source contributor testing/using their changes, or someone creating their own branded version of EX.

If we're going with technical wording it'd be something like "Mapbox API key must be configured when building the app".

---

AFAIK the app requires a key to be set or it wont' build. So at runtime there will always be one.

If that's the case I think we should change it as part of this ticket. It puts a massive barrier in the way of anyone trying to make an open source contribution 😱

[julioromano]

If that's the case I think we should change it as part of this ticket. It puts a massive barrier in the way of anyone trying to make an open source contribution 😱

Disabling or changing the feature when there's no API key adds unnecessary effort to our tasks and adds more points of failure, so perhaps it makes sense to still use our current api key as default and don't worry about our maptiler until we have reason to.

cc: @Johennes

[Johennes]

I agree. We ideally shouldn't require people to set up a map provider just to contribute to the app. At the same time, building in special UI to cover for a case that should never happen in production also feels wasteful.

What if we disabled all map rendering entirely when no API key is set? This would mean not showing the location button in the composer menu and rendering location events in the timeline using their textual fallback.

[julioromano]

What if we disabled all map rendering entirely when no API key is set? This would mean not showing the location button in the composer menu and rendering location events in the timeline using their textual fallback.

That's precisely what I had in mind when referring to "adds unnecessary effort to our tasks" :)
Even simple logic that does this is critical and must be well tested because we don't want to risk to show a crippled feature even when there is an api key.
The question is, are we willing to take this effort now, considering that API key usage is not a concern right now?

[callumu]

Okay that wording sounds good then

[csmith]

What if we disabled all map rendering entirely when no API key is set? This would mean not showing the location button in the composer menu and rendering location events in the timeline using their textual fallback.

That actually feels more complicated 😁. We'd have to expose the "Are maps configured?" state in a bunch of places (timeline items factory, composer actions, etc).

Showing a custom error on top of the existing elements is a small, self contained change. IMO it's well worth it to make the open source experience much better. (If I was trying to patch an open source app and it made me go and get an API key from a third party, I'd either just give up or hack out that API key entirely.)

[julioromano]

What if we disabled all map rendering entirely when no API key is set? This would mean not showing the location button in the composer menu and rendering location events in the timeline using their textual fallback.

That actually feels more complicated 😁. We'd have to expose the "Are maps configured?" state in a bunch of places (timeline items factory, composer actions, etc).

Showing a custom error on top of the existing elements is a small, self contained change. IMO it's well worth it to make the open source experience much better. (If I was trying to patch an open source app and it made me go and get an API key from a third party, I'd either just give up or hack out that API key entirely.)

I'm starting to lean on you side Chris :) Such an approach seems sensible enough. @Johennes what d'you think?

[Johennes]

Agreed, if we think this is little risk and overhead, let's do the error messaging instead. 👍

[VolkerJunginger]

@kittykat no clue how to test that…

[kittykat]

@alfogrillo ^ can you help?

[julioromano]

Not really a user story :) Should have been marked as a tech task.
It allows a 3rd party developer to develop other map providers easily.
But there's no user facing part to this.

[alfogrillo]

@alfogrillo ^ can you help?

Hopefully Marco clarified already.
I remember we decided to skip it for now anyway.

[julioromano]

It's been done AFAIK (at least on android) but not in the "super easy" way that was at first envisioned.
It still requires the 3rd party to fork the app and change some code.

[kittykat]

Agreed, this doesn't need signoff from us

[alfogrillo]

It's been done AFAIK (at least on android)

What do you mean by that?

On EXI map tiler urls are still hard coded.

[julioromano]

It's been done AFAIK (at least on android)

What do you mean by that?

On EXI map tiler urls are still hard coded.

https://github.com/vector-im/element-x-android/blob/develop/features/location/api/src/main/kotlin/io/element/android/features/location/api/internal/StaticMapUrlBuilder.kt

Static map loading is behind an interface, we provide one implementation for maptiler's static map api but 3rd party devs can create additional implementations if they want.

[alfogrillo]

Static map loading is behind an interface, we provide one implementation for maptiler's static map api but 3rd party devs can create additional implementations if they want.

That's the same on EXI.
I meant we don't have in place dynamic url from the .well-known file.

[julioromano]

Static map loading is behind an interface, we provide one implementation for maptiler's static map api but 3rd party devs can create additional implementations if they want.

That's the same on EXI. I meant we don't have in place dynamic url from the .well-known file.

Same here, so the platforms are on par.