search

mapbox / mapbox-gl-native

Interactive, thoroughly customizable maps in native Android, iOS, macOS, Node.js, and Qt applications, powered by vector tiles and OpenGL
https://mapbox.com/mobile
Other
4.37k stars 1.33k forks source link

Consider adding mbgl-offline executable to iOS releases #13314

Closed julianrex closed 5 years ago

julianrex commented 5 years ago

As we continue to improve support for sideloading offline databases, we should consider bundling mbgl-offline with our iOS releases.

1ec5 commented 5 years ago

mbgl-offline is written in cross-platform C++, so it’s incapable of providing offline region metadata that iOS applications can read using NSKeyedArchiver (as is customary). For iOS developers, the only benefit of mbgl-offline over Mapbox GL.app, which is bundled with the macOS map SDK, is that it can be automated. We could add AppleScript scriptability or command line option handling to Mapbox GL.app.

julianrex commented 5 years ago

Re metadata - see also https://github.com/mapbox/mapbox-gl-native/issues/13275

asheemmamoowala commented 5 years ago

We could add AppleScript scriptability or command line option handling to Mapbox GL.app.

I always assumed that it is up to the client to determine what/how that metadata would be encoding into the region. If that is the case, any built-in tools we provide will need some customization to be used. Even more so, with sideloading support, packs created on macOS may be downloaded to non-darwin platforms, and would have to build support for decoding the metadata appropriately. Maybe a default cross-platfrom encoding method ( UTF-8 string perhaps) can be used in the off-the-shelf tooling we provide, and developers can swap it with their own at runtime.

1ec5 commented 5 years ago

As far as I know, the offline database format was designed to be portable in the sense that the implementation was portable. However, making the metadata field a binary blob meant that field would never truly be portable. We were primarily focused on the use case of a device downloading an offline region for its own use, rather than redistribution. Any redistribution use case does make a strong case for portability.

The other consideration is that, although most of our examples only stick a name in the metadata for identification, the intent was always that the developer could insert application-specific information beyond the name. On iOS and macOS, the standard practice has been to use NSKeyedArchiver, which allows the developer to safely encode and decode an arbitrary data structure, basically anything that could go into a plist (so more flexible than JSON). It would be overkill to add a full-fledged plist editor to macosapp, but it would be trivial to bind a NSTableView or NSOutlineView to an NSDictionaryController with an NSSecureUnarchiveFromDataTransformer.

If mbgl-offline provides a metadata option that’s limited to a UTF-8 string, that might satisfy basic use cases that don’t require additional metadata. However, decoding the metadata on iOS becomes somewhat less safe if the application can’t assume the use of NSKeyedArchiver.

asheemmamoowala commented 5 years ago

However, decoding the metadata on iOS becomes somewhat less safe if the application can’t assume the use of NSKeyedArchiver.

@1ec5 would a base64 encoded string address the security concern? I see built-in functions in NSData and NSString.

Another way to do this might be to provide an additional varchar column in the database for a name or title.

1ec5 commented 5 years ago

would a base64 encoded string address the security concern? I see built-in functions in NSData and NSString.

Given an arbitrary binary blob, an application may be able to interpret it as a string without crashing or generating an error, but that doesn’t guarantee that a string is the correct interpretation. The fact that this is a binary blob means we allow it to contain anything. As soon as we start assigning meaning to the blob, that’s a good argument for a separate, more strongly typed column…

Another way to do this might be to provide an additional varchar column in the database for a name or title.

This is getting pretty far off-topic, but I think a more strongly-typed column would be a very positive improvement to the schema; it seems pretty clear to me that the name is the main thing people use the metadata field for. For the other kinds of metadata a developer might want to include (which they can currently stuff in the dictionary that gets archived), we could add a column for that dictionary encoded as a JSON source string. The only thing that can be represented in plist but not JSON is… binary data blobs. 🐢🐢🐢

julianrex commented 5 years ago

This is getting pretty far off-topic, but I think a more strongly-typed column would be a very positive improvement to the schema

Agreed. Though I do like the flexibility of an arbitrary blob - so perhaps something like a "Content-Type" column for the blob would be good.

zugaldia commented 5 years ago

If not bundling it with iOS/Android release, at least we need to make sure we test it as part of our builds https://github.com/mapbox/mapbox-gl-native/pull/14272.

stale[bot] commented 5 years ago

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.