Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace deprecated methods #146

Merged
merged 6 commits into from
Sep 18, 2017
Merged

Replace deprecated methods #146

merged 6 commits into from
Sep 18, 2017

Conversation

djih
Copy link
Member

@djih djih commented Sep 15, 2017

Removing 2 deprecated methods (#67) since Xcode is now enforcing that you don't use these methods, will likely block iOS 11 updates.

CFURLCreateStringByAddingPercentEscapes is simple to replace with stringByAddingPercentEncodingWithAllowedCharacters

NSURLConnection is more involved to replace with NSURLSession. Since we aren't altering any of the connection configuration settings, we can just use [NSURLSession sharedSession]. It's a little trickier with SSL pinning. I added an AMPURLSession class that extends ISPPinnedNSURLSessionDelegate and wraps NSURLSession. I'm having to create a new NSURLSession each time you try to upload events, similar to how the previous AMPURLConnection creates a new connection with each request. Although I think you're supposed to try to re-use NSURLSessions, but I wasn't having much luck with a static instance in AMPURLSession.

This will bump the min iOS version to 9. This also verifies that the unit tests pass in iOS 11 (on iPhone X emulator). Will need to do some more additional testing on the demo app for the SSL pinning

@djih djih requested a review from curtisliu September 15, 2017 08:47
@curtisliu
Copy link
Member

What was the issue you were running into with a static instance in AMPURLSession? Was it not possible to mirror the sharedSession call of NSURLSession?

@djih
Copy link
Member Author

djih commented Sep 15, 2017

Switched to using a static instance of the AMPURLSession delegate that maintains a persistent sharedSession that it can pass function calls to. Verified that the pinning works in our demo app running on an iphone X emulator with iOS 11. Also bumped the iOS target in podfile to 9.0 (no more 6.0). PTAL @curtisliu

Copy link
Member

@curtisliu curtisliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@djih djih merged commit c495909 into master Sep 18, 2017
@djih djih deleted the replace-deprecated-methods branch September 18, 2017 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants