Allow setting URL scheme templates

[kkaefer]

We are currently using mapbox:// URLs, but they're hardcoded into the system. This PR adds a generic setURLSchemeTemplate to OnlineFileSource and DefaultFileSource that allows you to specify templates for your own URL schemes.

[mention-bot]

@kkaefer, thanks for your PR! By analyzing this pull request, we identified @jfirebaugh, @tmcw and @yhahn to be potential reviewers.

[kkaefer]

(based on #7464)

[1ec5]

“Scheme” is redundant here. A better name would be -setURLTemplate:forScheme: or -setTemplate:forURLScheme:.

[kkaefer]

you're right; this was a victim of automated text replacement (the previous name was -setURLTemplate:forScheme:). I'll change this to -setTemplate:forURLScheme:

[1ec5]

I wonder if clang and jazzy are able to handle this nested list.

[1ec5]

Mini-languages in strings are un-Cocoa. Instead of exposing yet another mini-language that resembles but works differently than textField tokens in the runtime styling API, have you considered a lambda-based (block-based) API? The developer could specify a block (or perhaps an NSValueTransformer) that accepts an NSURL and returns a transformed NSURL. Within the block, they could easily parse and manipulate the URL via NSURLComponents.

Even more natural would be a delegate pattern: the developer could have their application delegate conform to a new MGLOfflineStorageDelegate protocol by implementing an -offlineStorage:transformedURLForTileURL: method.

[kkaefer]

While that may be true, using platform-specific URL parsing code also introduces a source for potential platform deviation. Therefore, I'd prefer using the same code for all platforms.

[1ec5]

Platform deviation would certainly be a problem when it comes to parsing URLs in JSON files, but a block would consist of platform-specific (or at least language-specific) code. Any deviation from how URLs are handled on the platform or in the language’s standard library would be more of a surprise, wouldn’t it?

In the unlikely event that incompatibilities between standard URL parsers on different platforms poses a problem to the developer, they could do minimal parsing inline. For most scenarios, however, a callback would enable developers to bring more context into the transformation logic without requiring us to add a special case for every innovation in tile URL technology.

Even if you don’t want to leave URL parsing up to the developer, why not have the block accept a dictionary mapping e.g. directory to foo/bar/ and allow the developer to perform concatenation and any other logic themselves?

[1ec5]

Google and Apple both provide callback or block-based APIs on their analogues to our raster source concept: #7471 (comment).

[1ec5]

It isn’t clear whether this API affects only tile URLs or also style URLs, sprite sheet URLs, etc.

[kkaefer]

Not sure how Any URLs encountered in styles and subsequently loaded sources could be any clearer

[1ec5]

Gah, don’t know how I missed that. 😆 I do think it would be helpful, for those who aren’t familiar with the resources that make up a style, to know exactly what “any URLs” entails. But that points to the need for a conceptual “what’s a style” guide in the documentation.

[kkaefer]

An alternative implementation (which is still lacking Android support) is in https://github.com/mapbox/mapbox-gl-native/compare/url-transform. It uses a function callback that allows manipulation of the Resource/URL.

Open questions are:

• the delegate's method is called from the FileSource thread. Is that idiomatic in Cocoa? Using a synchronous dispatch to the main thread is impossible due to deadlocks.
• Is the setter implemented correctly?

Usage is (e.g. in AppDelegate.m):

diff --git a/platform/macos/app/AppDelegate.h b/platform/macos/app/AppDelegate.h
index a1d9297..8d3e284 100644
--- a/platform/macos/app/AppDelegate.h
+++ b/platform/macos/app/AppDelegate.h
@@ -2,7 +2,7 @@
 
 extern NSString * const MGLMapboxAccessTokenDefaultsKey;
 
-@interface AppDelegate : NSObject <NSApplicationDelegate>
+@interface AppDelegate : NSObject <NSApplicationDelegate, MGLOfflineStorageDelegate>
 
 @property (weak) IBOutlet NSWindow *preferencesWindow;
 
diff --git a/platform/macos/app/AppDelegate.m b/platform/macos/app/AppDelegate.m
index d3fe2d2..cef3c1e 100644
--- a/platform/macos/app/AppDelegate.m
+++ b/platform/macos/app/AppDelegate.m
@@ -130,10 +130,19 @@ NSString * const MGLLastMapDebugMaskDefaultsKey = @"MGLLastMapDebugMask";
         [alert runModal];
         [self showPreferences:nil];
     }
+
+    [MGLOfflineStorage sharedOfflineStorage].delegate = self;
     
     [self.offlinePacksArrayController bind:@"content" toObject:[MGLOfflineStorage sharedOfflineStorage] withKeyPath:@"packs" options:nil];
 }
 
+- (NSURLComponents *)offlineStorage:(MGLOfflineStorage *)storage transformURL:(NSURLComponents *)url ofKind:(MGLResourceKind)kind {
+    if ([url.host isEqual: @"api.mapbox.com"]) {
+        url.host = @"a.tiles.mapbox.com";
+    }
+    return url;
+}
+
 - (void)applicationWillTerminate:(NSNotification *)notification {
     if (![[NSUserDefaults standardUserDefaults] boolForKey:@"NSQuitAlwaysKeepsWindows"]) {
         NSDocument *currentDocument = [NSDocumentController sharedDocumentController].currentDocument;

[1ec5]

It’s an antipattern in Objective-C (and arguably in general) for different methods on the same class to be called on different threads. That forces the class to explicitly synchronize any shared state such as ivars and avoid implicitly capturing self in any blocks. I’ve never seen it done successfully but, conversely, have had to tear such code out many times due to concurrency bugs.

If there’s no way to call a method on the main thread, the delegate cannot be AppDelegate. We should strongly recommend that the developer instead create a helper object – a subclass of NSObject that conforms to MGLOfflineStorageDelegate. Either AppDelegate (or some other application class) would own this object, or MGLOfflineStorage would own it – in which case we’d call it something else like MGLURLProtocol or even make use of the NSURLProtocol system.

[1ec5]

Related: #7455.

[jfirebaugh]

Why is the path without leading /? That is inconsistent with the definitions of path elsewhere, e.g. window.Location, node, RFC 3986.

[kkaefer]

Added pull request to change this in #7750

[jfirebaugh]

Merged #8084 instead.