Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Adding missing nullability attributes #3149

Closed
wants to merge 1 commit into from

Conversation

loudnate
Copy link
Contributor

No description provided.

@kcharwood
Copy link
Contributor

Thanks for the PR! What is your use case for nil success blocks?

@loudnate
Copy link
Contributor Author

204 response codes, using a JSON response serializer.
On Thu, Nov 12, 2015 at 10:42 AM Kevin Harwood notifications@github.com
wrote:

Thanks for the PR! What is your use case for nil success blocks?


Reply to this email directly or view it on GitHub
#3149 (comment)
.

@kcharwood
Copy link
Contributor

@cnoon Didn't you just handle 204's in Alamofire? Anything we should be aware of here?

@loudnate
Copy link
Contributor Author

Alternatively, we could route empty responses to the failure: case. Currently, though, this causes a crash when accessing the responseObject if the AFHTTPSessionManager classes are called from Swift and an empty response is received.

@kcharwood
Copy link
Contributor

Yep makes sense. I've verified this with some simple test additions to the AFHTTPSessionManager class.

Would you be willing to retarget this PR to the 3_0_0 branch, and convert to _Nullable? I'd like to take those there!

@kcharwood
Copy link
Contributor

You could also drop these tests into AFHTTPSessionManagerTests if you want:

# pragma mark - Rest Interface

- (void)testThatSuccessBlockIsCalledFor200 {
    XCTestExpectation *expectation = [self expectationWithDescription:@"Request should succeed"];
    [self.manager
     GET:@"status/200"
     parameters:nil
     success:^(NSURLSessionDataTask * _Nonnull task, id  _Nullable responseObject) {
         [expectation fulfill];
     }
     failure:nil];
    [self waitForExpectationsWithCommonTimeoutUsingHandler:nil];
}

- (void)testThatFailureBlockIsCalledFor404 {
    XCTestExpectation *expectation = [self expectationWithDescription:@"Request should succeed"];
    [self.manager
     GET:@"status/404"
     parameters:nil
     success:nil
     failure:^(NSURLSessionDataTask * _Nullable task, NSError * _Nullable error) {
         [expectation fulfill];
     }];
    [self waitForExpectationsWithCommonTimeoutUsingHandler:nil];
}

- (void)testThatResponseObjectIsEmptyFor204 {
    __block id urlResponseObject = nil;
    XCTestExpectation *expectation = [self expectationWithDescription:@"Request should succeed"];
    [self.manager
     GET:@"status/204"
     parameters:nil
     success:^(NSURLSessionDataTask * _Nonnull task, id  _Nullable responseObject) {
         urlResponseObject = responseObject;
         [expectation fulfill];
     }
     failure:nil];
    [self waitForExpectationsWithCommonTimeoutUsingHandler:nil];
    XCTAssertNil(urlResponseObject);
}

loudnate pushed a commit to chairish/AFNetworking that referenced this pull request Nov 12, 2015
@loudnate
Copy link
Contributor Author

Done, thanks! Feel free to close this one, unless you'd like me to update it so these changes go to master too?

@kcharwood
Copy link
Contributor

I'm going to close this one out and leave out of master. At this point, The 2.x branch is for critical features only. Since this only affects the NSURLSession API's, I'm going to leave it out for now.

Thanks again!

@kcharwood kcharwood closed this Nov 13, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants