Refactored REST Requests

[Nightsd01]

• Instead of manually building NSURLRequests, where we need to manually set different request properties (like the HTTP method) each time we make the request in code, I've refactored the project to use subclassed representations of the different requests.
• This results in less code duplication and less potential for bugs
• Refactored the unit tests and added a new test to make sure the new requests are being built & formatted correctly.
• Also removes usage of NSURLConnection (NSURLConnection is deprecated, should switch to NSURLSession #292 )

---

This change is 

[jkasten2]

Review status: 0 of 19 files reviewed at latest revision, 8 unresolved discussions.

---

iOS_SDK/OneSignalSDK/Source/OneSignal.m, line 464 at r1 (raw file):

+(void)downloadIOSParams {
    [[OneSignalClient sharedClient] executeRequest:[OSRequestGetIosParams withUserId:self.currentSubscriptionState.userId appId:self.app_id] onSuccess:^(NSDictionary *result) {

[OneSignalClient sharedClient] could be cleaned up to be OneSignalClient.sharedClient to improve readability.

---

iOS_SDK/OneSignalSDK/Source/OneSignalHelper.m, line 769 at r1 (raw file):

}

+ (void)handleJSONNSURLResponse:(NSURLResponse*) response data:(NSData*) data error:(NSError*) error onSuccess:(OSResultSuccessBlock)successBlock onFailure:(OSFailureBlock)failureBlock {

We should be able to move handleJSONNSURLResponse out of OneSignalHelper.m now that there is more fitting classes.

---

iOS_SDK/OneSignalSDK/Source/OneSignalRequest.h, line 1 at r1 (raw file):

//

Update this head file copyright to the one found in OneSignal.h. There are a few other new files that are missing this in this pull.

---

iOS_SDK/OneSignalSDK/Source/OneSignalRequest.m, line 46 at r1 (raw file):

    }
    
    NSLog(@"BUILT %@ HTTP REQUEST: \nURL: %@\nMETHOD: %@\nPARAMETERS: %@\nHEADER FIELDS: %@", NSStringFromClass([self class]), request.URL.absoluteString, request.HTTPMethod, self.parameters, request.allHTTPHeaderFields);

NSLog should be removed. We can keep this logging but it should be switch to onesignal_Log with DEBUG or VERBOSE.

---

iOS_SDK/OneSignalSDK/Source/Requests.m, line 22 at r1 (raw file):

@implementation OSRequestRegisterDevice
+ (instancetype)withAppId:(NSString *)appId withDeviceType:(NSNumber *)deviceType withAdId:(NSString *)adId withSDKVersion:(NSString *)sdkVersion withIdentifier:(NSString *)identifier {

There doesn't seem to be pattern of what is passed in vs what is collected internally. Can we pass everything in as more parameters or maybe create some kind of device / user state object to pass in?

---

iOS_SDK/OneSignalSDK/Source/Requests.m, line 122 at r1 (raw file):

    let request = [OSRequestUpdateNotificationTypes new];
    
    request.parameters = @{@"app_id" : appId, @"notificatioon_types" : notificationTypes};

Typo in endpoint

---

iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.m, line 46 at r1 (raw file):

    
    if (!parameters[@"app_id"] && ![request.request.URL.absoluteString containsString:@"/apps/"])
        _XCTPrimitiveFail(currentTestInstance, @"All requesst should include an app_id");

Typo error message

---

iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.m, line 48 at r1 (raw file):

        _XCTPrimitiveFail(currentTestInstance, @"All requesst should include an app_id");
    
    if (!parameters[@"app_id"] && ![request.request.URL.absoluteString containsString:@"/apps/"])

There 2 lines of code are duplicated

---

Comments from Reviewable

[Nightsd01]

iOS_SDK/OneSignalSDK/Source/Requests.m, line 22 at r1 (raw file):

Previously, jkasten2 (Josh Kasten) wrote…

There doesn't seem to be pattern of what is passed in vs what is collected internally. Can we pass everything in as more parameters or maybe create some kind of device / user state object to pass in?

This request should actually have been removed, as it is now implemented as OSRequestRegisterUser. I noticed how many parameters there were and decided to simply implement it by letting the client pass in an NSDictionary containing all of the different parameters, since it's less verbose.

---

Comments from Reviewable

[Nightsd01]

iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.m, line 46 at r1 (raw file):

Previously, jkasten2 (Josh Kasten) wrote…

Typo error message

Good catch! I actually just copy and pasted from the previous [OneSignalHelperOverrider enqueueRequest: ...] method. Fixed it.

---

Comments from Reviewable

[jkasten2]

Reply

---

Reviewed 7 of 19 files at r1, 12 of 12 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable