Adds NSURLSession delegate to MGLNetworkConfiguration #228

julianrex · 2020-03-18T16:32:35Z

Depends on #242

This PR adds an undocumented MGLNetworkConfigurationSessionDelegate and a id delegate property to MGLNetworkConfiguration.

This allows the application to provide an NSURLSession object, which will be used instead of one being created using the existing sessionConfiguration property. It is the developer's responsibility to maintain the life (and reuse) the session object. It's also important to note that the delegate method does not get called on the main thread.

Background sessions are not supported, and an exception will fire if one is supplied - this exception is provided by the OS.

The existing sessionConfiguration behavior remains unchanged.

To enable these changes, the PR also includes:

Removal of MGLNetworkIntegrationManager (non-public type) - this was just acting as a go-between, as was superfluous.
Addition of MGLIntegrationTestCase to share existing testing functionality.

TODO:

Integration tests
update/test macOS

See also these changes in gl-native, which this PR depends on.

julianrex · 2020-03-24T01:10:15Z

cc @halset (who proposed a session delegate PR).

knov

needs tests

halset · 2020-03-24T13:23:20Z

@julianrex Thanks for the cc! Should probably recreate the NSURLSessionDelegate-PR after this is merged to be able to set a NSURLSessionDelegate or will this PR handle that as well?

julianrex · 2020-03-24T13:45:59Z

@halset I'm thinking this PR would make the existing session delegate PR redundant.

Note this is still just an experiment, and we may end up adding a delegate for MGLNetworkConfiguration which I think would be more idiomatic.

fabian-guerra

Build issues aside. LGTM I left minor comments. Thank you.

platform/darwin/src/MGLNetworkConfiguration.m

1ec5

Is it possible to split out the build system changes into a separate PR? It isn’t clear to me how they relate to MGLNetworkConfiguration.

platform/darwin/darwin.xcconfig

platform/ios/Integration Tests/MGLNetworkConfigurationIntegrationTests.mm

platform/macos/src/MGLMapView.mm

captainbarbosa

@julianrex and I went over these changes over the phone.

👍 from me, but echoing @1ec5's comments about the build changes being broken out in another PR if possible.

platform/darwin/src/MGLNetworkConfiguration.h

1ec5 · 2020-03-26T17:53:58Z

platform/darwin/src/MGLNetworkConfiguration.m

@@ -34,26 +40,46 @@ + (instancetype)sharedManager {
        _sharedManager = [[self alloc] init];
    });

-    [self setNativeNetworkManagerDelegateToDefault];
+    // Notice, this is reset for each call. This is primarily for testing purposes.
+    // TODO: Consider only calling this for testing?


Is this to-do still relevant? (×2)

I'll create a new ticket to track this.

platform/darwin/src/MGLNetworkConfiguration.h

1ec5 · 2020-03-26T17:56:20Z

platform/darwin/src/MGLNetworkConfiguration.h

@@ -23,12 +49,14 @@ MGL_EXPORT
 If this property is set to nil or if no session configuration is provided this property
 is set to the default session configuration.

- Assign this object before instantiating any `MGLMapView` object.
+ Assign this object before instantiating any `MGLMapView` object, or using
+ `MGLOfflineStorage`


What’s an example of a place where this object can be assigned safely? Consider that fixing #227 may require us to invoke +[MGLOfflineStorage sharedStorage] in an +initialize method.

Ugh. Let's address that when we fix #227. Hopefully we can find a way around without having to use an +initialize.

Build changes were refactored into #242

julianrex · 2020-03-26T20:27:04Z

When merged, the commit will need to be cherry-picked to release-vanillashake

…Manager

Add expect-to-fail test for data delegate.

Co-Authored-By: Minh Nguyễn <mxn@1ec5.org>

… property)

halset

I have tested this with a tile source that uses HTTP Basic Authentication by using a NSURLSession with a custom NSURLSessionDelegate. It works fine! So for me, this can replace my original PR. Thank you for the good work!

julianrex added enhancement New feature or request ⚠️ DO NOT MERGE skip-changelog labels Mar 18, 2020

julianrex requested review from fabian-guerra, tmpsantos, zugaldia and jmkiley and removed request for fabian-guerra March 18, 2020 16:32

knov requested a review from 1ec5 March 23, 2020 17:14

jmkiley approved these changes Mar 23, 2020

View reviewed changes

knov approved these changes Mar 24, 2020

View reviewed changes

knov assigned julianrex Mar 24, 2020

knov requested review from captainbarbosa and fabian-guerra March 25, 2020 17:35

julianrex requested review from knov and jmkiley and removed request for tmpsantos and zugaldia March 25, 2020 19:20

julianrex changed the title ~~Adds "private" NSURLSession to MGLNetworkConfiguration~~ Adds NSURLSession delegate to MGLNetworkConfiguration Mar 25, 2020

julianrex marked this pull request as ready for review March 25, 2020 19:47

julianrex requested a review from a team March 25, 2020 19:47

fabian-guerra approved these changes Mar 25, 2020

View reviewed changes

platform/darwin/src/MGLNetworkConfiguration.m Outdated Show resolved Hide resolved

platform/darwin/src/MGLNetworkConfiguration.m Outdated Show resolved Hide resolved

1ec5 previously requested changes Mar 26, 2020

View reviewed changes

platform/darwin/darwin.xcconfig Outdated Show resolved Hide resolved

platform/ios/Integration Tests/MGLNetworkConfigurationIntegrationTests.mm Show resolved Hide resolved

platform/macos/src/MGLMapView.mm Outdated Show resolved Hide resolved

julianrex force-pushed the jrex/session-poc branch from 8014911 to 8e21644 Compare March 26, 2020 16:57

julianrex requested a review from 1ec5 March 26, 2020 17:00

captainbarbosa approved these changes Mar 26, 2020

View reviewed changes

1ec5 reviewed Mar 26, 2020

View reviewed changes

julianrex force-pushed the jrex/session-poc branch from 69b78ce to 7dc7111 Compare March 26, 2020 19:33

julianrex added this to the release-vanillashake milestone Mar 26, 2020

knov approved these changes Mar 26, 2020

View reviewed changes

julianrex force-pushed the jrex/session-poc branch from ec7c140 to f6903cd Compare March 26, 2020 22:37

julianrex and others added 16 commits March 26, 2020 19:27

Experimental: Adds "private" NSURLSession to MGLNetworkConfiguration

dc180c7

Make MGLNetworkConfiguration the delegate for MGLNativeAppleInterface…

20b808b

…Manager

Remove MGLNetworkIntegrationManager

9c4e899

Adds a session delegate

4c3e375

Add tests for session delegation

e088d15

Refactor integration test - needed so that tests run triggering all.

1c30fdd

Remove packs at end of test, so we don't affect other tests :(

b969bc2

Build for mac

2e968d8

Remove commented out property

f4f8b82

Clean up setNativeNetworkManagerDelegate, comments, tests.

7673ea9

Rename set.. as reset

6cc74fa

Missing file.

0dabdbc

Update for macos.

05c5ec1

Add test for additional headers.

da6a161

Add expect-to-fail test for data delegate.

Apply suggestions from code review

e2b24c0

Co-Authored-By: Minh Nguyễn <mxn@1ec5.org>

Make delegate method optional (and add type specification to delegate…

9d2efab

… property)

julianrex force-pushed the jrex/session-poc branch from f6903cd to 9d2efab Compare March 27, 2020 03:02

julianrex mentioned this pull request Mar 27, 2020

Fix compilation warnings when compiling tests in release. #245

Merged

halset approved these changes Mar 27, 2020

View reviewed changes

julianrex mentioned this pull request Mar 27, 2020

[ios][macos] Add MGLNetworkConfiguration.sessionDelegate mapbox/mapbox-gl-native#15128

Closed

julianrex merged commit 171ff94 into master Mar 27, 2020

julianrex deleted the jrex/session-poc branch March 27, 2020 14:28

julianrex removed the ⚠️ DO NOT MERGE label Apr 9, 2020

julianrex mentioned this pull request Aug 7, 2020

Crash in logging (Debug and Error levels) #370

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds NSURLSession delegate to MGLNetworkConfiguration #228

Adds NSURLSession delegate to MGLNetworkConfiguration #228

julianrex commented Mar 18, 2020 •

edited

Loading

julianrex commented Mar 24, 2020

knov left a comment

halset commented Mar 24, 2020

julianrex commented Mar 24, 2020

fabian-guerra left a comment

1ec5 left a comment

captainbarbosa left a comment •

edited

Loading

1ec5 Mar 26, 2020

julianrex Mar 26, 2020

1ec5 Mar 26, 2020

julianrex Mar 26, 2020 •

edited

Loading

julianrex commented Mar 26, 2020

halset left a comment

Adds NSURLSession delegate to MGLNetworkConfiguration #228

Adds NSURLSession delegate to MGLNetworkConfiguration #228

Conversation

julianrex commented Mar 18, 2020 • edited Loading

julianrex commented Mar 24, 2020

knov left a comment

Choose a reason for hiding this comment

halset commented Mar 24, 2020

julianrex commented Mar 24, 2020

fabian-guerra left a comment

Choose a reason for hiding this comment

1ec5 left a comment

Choose a reason for hiding this comment

captainbarbosa left a comment • edited Loading

Choose a reason for hiding this comment

1ec5 Mar 26, 2020

Choose a reason for hiding this comment

julianrex Mar 26, 2020

Choose a reason for hiding this comment

1ec5 Mar 26, 2020

Choose a reason for hiding this comment

julianrex Mar 26, 2020 • edited Loading

Choose a reason for hiding this comment

julianrex commented Mar 26, 2020

halset left a comment

Choose a reason for hiding this comment

julianrex commented Mar 18, 2020 •

edited

Loading

captainbarbosa left a comment •

edited

Loading

julianrex Mar 26, 2020 •

edited

Loading