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.

Sign up for GitHub

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

Remove delegate from HTTPNetworkTransport initializer #1002

Merged
merged 2 commits into from
Feb 6, 2020
Merged

Remove delegate from HTTPNetworkTransport initializer #1002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3aa0824
possible fix for overcalling issues
designatednerd Feb 5, 2020
b02fd7d
make weak delegate a fully breaking change by removing from initializer
designatednerd Feb 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions Sources/Apollo/HTTPNetworkTransport.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,22 +117,19 @@ public class HTTPNetworkTransport {
/// - useGETForQueries: If query operation should be sent using GET instead of POST. Defaults to false.
/// - enableAutoPersistedQueries: Whether to send persistedQuery extension. QueryDocument will be absent at 1st request, retry with QueryDocument if server respond PersistedQueryNotFound or PersistedQueryNotSupport. Defaults to false.
/// - useGETForPersistedQueryRetry: Whether to retry persistedQuery request with HttpGetMethod. Defaults to false.
/// - delegate: [Optional] A delegate which can conform to any or all of `HTTPNetworkTransportPreflightDelegate`, `HTTPNetworkTransportTaskCompletedDelegate`, and `HTTPNetworkTransportRetryDelegate`. Defaults to nil.
public init(url: URL,
session: URLSession = .shared,
sendOperationIdentifiers: Bool = false,
useGETForQueries: Bool = false,
enableAutoPersistedQueries: Bool = false,
useGETForPersistedQueryRetry: Bool = false,
delegate: HTTPNetworkTransportDelegate? = nil,
requestCreator: RequestCreator = ApolloRequestCreator()) {
self.url = url
self.session = session
self.sendOperationIdentifiers = sendOperationIdentifiers
self.useGETForQueries = useGETForQueries
self.enableAutoPersistedQueries = enableAutoPersistedQueries
self.useGETForPersistedQueryRetry = useGETForPersistedQueryRetry
self.delegate = delegate
self.requestCreator = requestCreator
}

Expand Down
3 changes: 3 additions & 0 deletions Tests/ApolloCacheDependentTests/FetchQueryTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,5 +473,8 @@ class FetchQueryTests: XCTestCase {
}
self.wait(for: [expectation], timeout: 10)

for watcher in watchers {
watcher.cancel()
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a sneaking suspicion that the watchers are getting overcalled because they're not necessarily getting killed off by ARC in time to avoid calls from other tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope - still failing occasionally

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if its the case here, but we had to add to wrap to autoreleasepool { ... } to workaround ARC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what'd you need to add that to?

}
}
7 changes: 4 additions & 3 deletions Tests/ApolloCacheDependentTests/StarWarsServerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@ class APQsWithGetMethodConfig: TestConfig, HTTPNetworkTransportRetryDelegate{
}

func network() -> HTTPNetworkTransport {
return HTTPNetworkTransport(url: URL(string: "http://localhost:8080/graphql")!,
let transport = HTTPNetworkTransport(url: URL(string: "http://localhost:8080/graphql")!,
enableAutoPersistedQueries: true,
useGETForPersistedQueryRetry: true,
delegate: self)
useGETForPersistedQueryRetry: true)
transport.delegate = self
return transport
}

}
Expand Down
23 changes: 14 additions & 9 deletions Tests/ApolloTests/HTTPTransportTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@ class HTTPTransportTests: XCTestCase {
private var graphQlErrors = [GraphQLError]()

private lazy var url = URL(string: "http://localhost:8080/graphql")!
private lazy var networkTransport = HTTPNetworkTransport(url: self.url,
useGETForQueries: true,
delegate: self)
private lazy var networkTransport: HTTPNetworkTransport = {
let transport = HTTPNetworkTransport(url: self.url,
useGETForQueries: true)
transport.delegate = self
return transport
}()

private func validateHeroNameQueryResponse<Operation: GraphQLOperation>(result: Result<GraphQLResponse<Operation>, Error>,
expectation: XCTestExpectation,
Expand Down Expand Up @@ -189,12 +192,10 @@ class HTTPTransportTests: XCTestCase {

func testEquality() {
let identicalTransport = HTTPNetworkTransport(url: self.url,
useGETForQueries: true,
delegate: self)
useGETForQueries: true)
XCTAssertEqual(self.networkTransport, identicalTransport)

let nonIdenticalTransport = HTTPNetworkTransport(url: self.url,
delegate: self)
let nonIdenticalTransport = HTTPNetworkTransport(url: self.url)
XCTAssertNotEqual(self.networkTransport, nonIdenticalTransport)
}

Expand All @@ -208,7 +209,9 @@ class HTTPTransportTests: XCTestCase {
let mockSession = MockURLSession()
mockSession.response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: nil)
mockSession.data = try JSONSerialization.data(withJSONObject: body, options: .prettyPrinted)
let network = HTTPNetworkTransport(url: url, session: mockSession, delegate: self)
let network = HTTPNetworkTransport(url: url,
session: mockSession)
network.delegate = self
let expectation = self.expectation(description: "Send operation completed")

let _ = network.send(operation: query) { result in
Expand Down Expand Up @@ -241,7 +244,9 @@ class HTTPTransportTests: XCTestCase {
let mockSession = MockURLSession()
mockSession.response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: nil)
mockSession.data = try JSONSerialization.data(withJSONObject: body, options: .prettyPrinted)
let network = HTTPNetworkTransport(url: url, session: mockSession, delegate: self)
let network = HTTPNetworkTransport(url: url,
session: mockSession)
network.delegate = self
let expectation = self.expectation(description: "Send operation completed")

let _ = network.send(operation: query) { result in
Expand Down