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

Update how requests are created so custom subclasses of HTTP requests can be used #1405

Merged
merged 8 commits into from
Sep 22, 2020

Conversation

designatednerd
Copy link
Contributor

@designatednerd designatednerd commented Sep 21, 2020

In this PR:

  • Made RequestChainNetworkTransport subclassable and changed two methods to be open so they can be subclassed in order to facilitate using subclasses of HTTPRequest when needed. (thanks for the suggestion, Networking Beta: Convenience constructors for HTTPRequest and JSONRequest #1404)
  • Realized after doing this that uploading setup could be moved to UploadRequest, which meant:
    • UploadRequest is now open so it can be subclassed.
    • The default creation of a multipart form request now lives in UploadRequest instead of RequestCreator, and non-default creation should be moved from an implementation of RequestCreator to a subclass of UploadRequest.
    • RequestCreator protocol has now been renamed RequestBodyCreator, since it only creates the request body and not a full HTTPRequest (which didn't exist in the old networking stack, so the name made more sense then). Associated implementations and property names have been changed accordingly.
    • The protocol requirement for multipart form data has consequently been removed from the RequestBodyCreator protocol.

import XCTest
import Apollo

class MultipartFormDataTests: XCTestCase {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this out of the request creator tests to disambiguate what's actually being tested here

@@ -27,40 +27,4 @@ struct TestCustomRequestCreator: RequestCreator {

return body
}

public func requestMultipartFormData<Operation: GraphQLOperation>(for operation: Operation,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is no longer a requirement of the RequestBodyCreator protocol, so it's been removed

@@ -198,6 +199,225 @@ class UploadTests: XCTestCase {
}

self.wait(for: [expectation], timeout: 10)
}

// MARK: - UploadRequest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests were moved over from the RequestCreator tests since they no longer have anything to do with the request creator

@designatednerd designatednerd changed the title Update/creating requests Update how requests are created so custom subclasses of HTTP requests can be used Sep 21, 2020
@designatednerd
Copy link
Contributor Author

These are slightly larger changes than I'd normally like to make at the RC stage, but I've been itching to get rid of the request creator and this gave me an opening to get rid of its most convoluted bits.

Copy link

@knox knox left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@designatednerd designatednerd merged commit 357bc9b into betas/networking-stack Sep 22, 2020
@designatednerd designatednerd deleted the update/creating-requests branch September 22, 2020 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants