[ios] Specify custom NSURLSessionDelegate #13491

halset · 2018-12-01T00:44:50Z

This is an attempt to solve #11888 by letting the app provide a custom NSURLSessionDelegate by setting "MGLNSURLSessionDelegateClassName" in Info.plist.

My main motivation for doing this is to be able to use tile services that uses HTTP Basic Authentication.

julianrex

Thanks for this PR @halset!

Would you mind adding an entry to the iOS and macOS changelog files?

Noting that longer term we may want to consider using NSUserDefaults (similar to how we've had to add support for MGLCollisionBehaviorPre4_0 in #13426)

halset · 2018-12-09T20:12:55Z

@julianrex does the changelogs look good?

julianrex · 2018-12-11T04:10:49Z

Looks good - thanks @halset!

1ec5 · 2018-12-20T11:35:33Z

platform/darwin/src/http_file_source.mm

+            NSDictionary *info = [[NSBundle mainBundle] infoDictionary];
+            NSString *delegateClassName = [info valueForKey:@"MGLNSURLSessionDelegateClassName"];
+            if (delegateClassName) {
+                sessionDelegate = [[NSClassFromString(delegateClassName) alloc] init];


This is great! Would you mind documenting MGLNSURLSessionDelegateClassName in the following two documents?

https://github.com/mapbox/mapbox-gl-native/blob/master/platform/ios/docs/guides/Info.plist%20Keys.md
https://github.com/mapbox/mapbox-gl-native/blob/master/platform/macos/docs/guides/Info.plist%20Keys.md

Some points that could be covered, since I think this is the first time we’ve introduced an API that represents a class name as a string:

If the class is implemented in Swift, it needs to bridge to Objective-C.

The key needs to be set to a runtime class name. (A Swift application would either need to give the class an Objective-C name or set the key to a mangled name.)

The class should conform to the NSURLSessionDelegate protocol. (Since the protocol’s methods are all optional, we can’t check for conformance at runtime. If the class doesn’t conform to the protocol, there wouldn’t be any adverse effects, but that would probably be a mistake on the part of the developer.)

See also MGLOfflineStorageDelegate. (Like the new Info.plist key, that protocol is used for any request initiated by mbgl but presumably not for Mapbox Telemetry requests, which are handled by a different dependency.)

@halset are you planning on updating this PR?

@julianrex I still want this to be merged, but the documentation work suggested by @1ec5 has not bubbled up to the top of my priority list yet. Sorry.

No worries! Was just checking in 🙂

Perhaps we could add a method mgl_applicationInfoDictionary to NSBundle+MGLAdditions.m and use that here instead.

@datwelk good idea. I've added your comment as a new issue: #13908 - so that we can keep that as a separate PR.

julianrex · 2019-02-11T20:19:49Z

@halset when you're ready for this to be reviewed again, please remove the DO NOT MERGE label.

https://github.com/mapbox/mapbox-gl-native/pull/13491/files#r243240976

halset · 2019-02-18T14:04:49Z

@julianrex Sorry for the delay. I have finally added the Info.plist documentation, but I do not know if I have privileges to remove the DO NOT MERGE label.

halset · 2019-02-18T21:40:27Z

I have updated this with the latest from #13886. Is that okay or should this be moved from a Info.plist key to something in MGLNetworkConfiguration?

lucianboboc · 2019-02-20T13:35:50Z

NSURLSessionDelegate is limited, if the backend will require an Authorization header with a Bearer xxx value there is no way to add that using NSURLSessionDelegate. It only supports Basic authentication (username + password, more details here: https://blog.cocoafrog.de/2017/11/18/how-nsurlsession-authentication-should-work.html).
A hook method before the request is sent is what's needed for the URLRequest to be configured.

julianrex · 2019-02-20T14:58:40Z

I have updated this with the latest from #13886. Is that okay or should this be moved from a Info.plist key to something in MGLNetworkConfiguration?

@fabian-guerra would you mind looking at this PR please, especially in context of your recent changes to MGLNetworkConfiguration? I'm wondering (like @halset) if this is the right time to add this to the configuration object.

@lucianboboc thanks for the link - that's very clear. We realize that this PR is limited in scope, however authentication is something that is on our radar. /cc @tmpsantos

fabian-guerra

Thank you for making this change. I'm ok with this current implementation. But now that we have made some changes to expose the network configuration object it may be worth it to consider a different approach.

fabian-guerra · 2019-02-21T22:32:56Z

platform/ios/CHANGELOG.md

@@ -69,6 +69,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 * Points of interest have clearer, localized VoiceOver hints in styles that use version 8 of the Mapbox Streets source. ([#13525](https://github.com/mapbox/mapbox-gl-native/pull/13525))
 * Added `MGLLoggingConfiguration` and `MGLLoggingBlockHandler` that handle error and fault events produced by the SDK. ([#13235](https://github.com/mapbox/mapbox-gl-native/pull/13235))
 * Fixed random crashes during app termination. ([#13367](https://github.com/mapbox/mapbox-gl-native/pull/13367))
+* Specify custom NSURLSessionDelegate with Info.plist key MGLNSURLSessionDelegateClassName. ([#13491](https://github.com/mapbox/mapbox-gl-native/pull/13491))


When using verbs at the beginning of the changelog entry use the past tense.
Enclose any class/method/keyword with ``.
This entry should be in the latest version: 4.9.0

fabian-guerra · 2019-02-21T22:33:09Z

platform/macos/CHANGELOG.md

@@ -40,6 +40,7 @@
 * Renamed `-[MGLOfflineStorage putResourceWithUrl:data:modified:expires:etag:mustRevalidate:]` to `-[MGLOfflineStorage preloadData:forURL:modificationDate:expirationDate:eTag:mustRevalidate:]`. ([#13318](https://github.com/mapbox/mapbox-gl-native/pull/13318))
 * `MGLMapSnapshotter` now respects the `MGLIdeographicFontFamilyName` key in Info.plist, which reduces bandwidth consumption when snapshotting regions that contain Chinese or Japanese characters. ([#13427](https://github.com/mapbox/mapbox-gl-native/pull/13427))
 * Added `MGLLoggingConfiguration` and `MGLLoggingBlockHandler` that handle error and fault events produced by the SDK. ([#13235](https://github.com/mapbox/mapbox-gl-native/pull/13235))
+* Specify custom NSURLSessionDelegate with Info.plist key MGLNSURLSessionDelegateClassName. ([#13491](https://github.com/mapbox/mapbox-gl-native/pull/13491))


Same as above.

fabian-guerra · 2019-02-21T22:41:52Z

platform/darwin/src/http_file_source.mm

+            // app might provide a NSURLSessionDelegate for authentication and such
+            id<NSURLSessionDelegate> sessionDelegate = nil;
+            NSDictionary *info = [[NSBundle mainBundle] infoDictionary];
+            NSString *delegateClassName = [info valueForKey:@"MGLNSURLSessionDelegateClassName"];


Now that MGLNetworkConfiguration is public what do you think if instead of reading a class from the Info file we register the class in the network configuration singleton?

Reading from the plist file is a bit hidden.

This way it's clear where to look if anyone requires custom authentication.

It's safer instantiating a class than parsing first a string.

stale · 2019-04-23T00:18:53Z

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

julianrex · 2019-04-23T13:49:03Z

Re-opening: @halset do you have any feedback on @fabian-guerra's comment above about using MGLNetworkConfiguration instead?

halset · 2019-05-21T19:13:05Z

@julianrex Looking at @fabian-guerra's comment, this pull-request has probably been out-dated by the MGLNetworkConfiguration. We should probably just close this pull-request and then hopefully someone will start a new one based on MGLNetworkConfiguration. I am continuing to use this patch in my app as it is in this pull-request.

[ios] Specify custom NSURLSessionDelegate

7461ba2

halset requested a review from 1ec5 as a code owner December 1, 2018 00:44

halset mentioned this pull request Dec 1, 2018

[ios] Provide a custom NSURLSessionDelegate? #11888

Closed

1ec5 requested review from kkaefer and julianrex December 6, 2018 06:59

kkaefer approved these changes Dec 7, 2018

View reviewed changes

julianrex suggested changes Dec 7, 2018

View reviewed changes

halset added 2 commits December 9, 2018 21:05

Merge branch 'master' into custom-session-delegate-01

bca667f

iOS/macOS Changelog entries for MGLNSURLSessionDelegateClassName

dbf6f64

julianrex approved these changes Dec 11, 2018

View reviewed changes

sebastianludwig mentioned this pull request Dec 19, 2018

ios: Support custom NSURLProtocol #12026

Closed

1ec5 reviewed Dec 20, 2018

View reviewed changes

julianrex mentioned this pull request Feb 11, 2019

Add a method mgl_applicationInfoDictionary to NSBundle+MGLAdditions.m #13908

Closed

julianrex added iOS Mapbox Maps SDK for iOS ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels Feb 11, 2019

MGLNSURLSessionDelegateClassName Info.plist documentation

d61a9ad

https://github.com/mapbox/mapbox-gl-native/pull/13491/files#r243240976

halset requested a review from a team February 18, 2019 14:02

halset added 2 commits February 18, 2019 16:35

Merge branch 'master' into custom-session-delegate-01

b226b17

Merge branch 'master' into custom-session-delegate-01

808bf0f

julianrex requested a review from fabian-guerra February 20, 2019 14:50

julianrex removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Feb 20, 2019

fabian-guerra reviewed Feb 21, 2019

View reviewed changes

stale bot added the archived Archived because of inactivity label Apr 22, 2019

stale bot closed this Apr 23, 2019

julianrex reopened this Apr 23, 2019

stale bot removed the archived Archived because of inactivity label Apr 23, 2019

halset closed this May 21, 2019

halset mentioned this pull request Jul 16, 2019

[ios][macos] Add MGLNetworkConfiguration.sessionDelegate #15128

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ios] Specify custom NSURLSessionDelegate #13491

[ios] Specify custom NSURLSessionDelegate #13491

halset commented Dec 1, 2018

julianrex left a comment

halset commented Dec 9, 2018

julianrex commented Dec 11, 2018

1ec5 Dec 20, 2018

julianrex Jan 8, 2019

halset Jan 8, 2019

julianrex Jan 8, 2019

datwelk Jan 24, 2019

julianrex Feb 11, 2019

julianrex commented Feb 11, 2019 •

edited

Loading

halset commented Feb 18, 2019

halset commented Feb 18, 2019

lucianboboc commented Feb 20, 2019

julianrex commented Feb 20, 2019

fabian-guerra left a comment

fabian-guerra Feb 21, 2019

fabian-guerra Feb 21, 2019

fabian-guerra Feb 21, 2019 •

edited

Loading

stale bot commented Apr 23, 2019

julianrex commented Apr 23, 2019

halset commented May 21, 2019

[ios] Specify custom NSURLSessionDelegate #13491

[ios] Specify custom NSURLSessionDelegate #13491

Conversation

halset commented Dec 1, 2018

julianrex left a comment

Choose a reason for hiding this comment

halset commented Dec 9, 2018

julianrex commented Dec 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julianrex commented Feb 11, 2019 • edited Loading

halset commented Feb 18, 2019

halset commented Feb 18, 2019

lucianboboc commented Feb 20, 2019

julianrex commented Feb 20, 2019

fabian-guerra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabian-guerra Feb 21, 2019 • edited Loading

Choose a reason for hiding this comment

stale bot commented Apr 23, 2019

julianrex commented Apr 23, 2019

halset commented May 21, 2019

julianrex commented Feb 11, 2019 •

edited

Loading

fabian-guerra Feb 21, 2019 •

edited

Loading