From 523f389808240b2641d669d1a7c05235049b873a Mon Sep 17 00:00:00 2001 From: David Jones Date: Mon, 28 Jan 2019 16:27:55 +0000 Subject: [PATCH] Fix overwriting of queryItems with nil (#51) --- Sources/SwiftyRequest/RestRequest.swift | 48 ++++++++++++++----- .../SwiftyRequestTests.swift | 23 +++++++-- 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/Sources/SwiftyRequest/RestRequest.swift b/Sources/SwiftyRequest/RestRequest.swift index 5ff6b01..6ddff40 100644 --- a/Sources/SwiftyRequest/RestRequest.swift +++ b/Sources/SwiftyRequest/RestRequest.swift @@ -237,7 +237,7 @@ public class RestRequest: NSObject { /// /// - Parameters: /// - templateParams: URL templating parameters used for substituion if possible - /// - queryItems: array containing `URLQueryItem` objects that will be appended to the request's URL + /// - queryItems: Sets the query parameters for this RestRequest, overwriting any existing parameters. Defaults to `nil`, which means that this parameter will be ignored, and `RestRequest.queryItems` will be used instead. Note that if you wish to clear any existing query parameters, then you should set `request.queryItems = nil` before calling this function. /// - completionHandler: Callback used on completion of operation public func responseData(templateParams: [String: String]? = nil, queryItems: [URLQueryItem]? = nil, @@ -250,7 +250,11 @@ public class RestRequest: NSObject { return } - self.queryItems = queryItems + // Replace any existing query items with those provided in the queryItems + // parameter, if any were given. + if let queryItems = queryItems { + self.queryItems = queryItems + } response { data, response, error in @@ -279,7 +283,7 @@ public class RestRequest: NSObject { /// - responseToError: Error callback closure in case of request failure /// - path: Array of Json keys leading to desired Json /// - templateParams: URL templating parameters used for substituion if possible - /// - queryItems: array containing `URLQueryItem` objects that will be appended to the request's URL + /// - queryItems: Sets the query parameters for this RestRequest, overwriting any existing parameters. Defaults to `nil`, which means that this parameter will be ignored, and `RestRequest.queryItems` will be used instead. Note that if you wish to clear any existing query parameters, then you should set `request.queryItems = nil` before calling this function. /// - completionHandler: Callback used on completion of operation public func responseObject( responseToError: ((HTTPURLResponse?, Data?) -> Error?)? = nil, @@ -295,7 +299,11 @@ public class RestRequest: NSObject { return } - self.queryItems = queryItems + // Replace any existing query items with those provided in the queryItems + // parameter, if any were given. + if let queryItems = queryItems { + self.queryItems = queryItems + } response { data, response, error in @@ -357,7 +365,7 @@ public class RestRequest: NSObject { /// - responseToError: Error callback closure in case of request failure /// - path: Array of Json keys leading to desired Json /// - templateParams: URL templating parameters used for substituion if possible - /// - queryItems: array containing `URLQueryItem` objects that will be appended to the request's URL + /// - queryItems: Sets the query parameters for this RestRequest, overwriting any existing parameters. Defaults to `nil`, which means that this parameter will be ignored, and `RestRequest.queryItems` will be used instead. Note that if you wish to clear any existing query parameters, then you should set `request.queryItems = nil` before calling this function. /// - completionHandler: Callback used on completion of operation public func responseObject( responseToError: ((HTTPURLResponse?, Data?) -> Error?)? = nil, @@ -372,7 +380,11 @@ public class RestRequest: NSObject { return } - self.queryItems = queryItems + // Replace any existing query items with those provided in the queryItems + // parameter, if any were given. + if let queryItems = queryItems { + self.queryItems = queryItems + } response { data, response, error in @@ -412,7 +424,7 @@ public class RestRequest: NSObject { /// - responseToError: Error callback closure in case of request failure /// - path: Array of Json keys leading to desired Json /// - templateParams: URL templating parameters used for substituion if possible - /// - queryItems: array containing `URLQueryItem` objects that will be appended to the request's URL + /// - queryItems: Sets the query parameters for this RestRequest, overwriting any existing parameters. Defaults to `nil`, which means that this parameter will be ignored, and `RestRequest.queryItems` will be used instead. Note that if you wish to clear any existing query parameters, then you should set `request.queryItems = nil` before calling this function. /// - completionHandler: Callback used on completion of operation public func responseArray( responseToError: ((HTTPURLResponse?, Data?) -> Error?)? = nil, @@ -428,7 +440,11 @@ public class RestRequest: NSObject { return } - self.queryItems = queryItems + // Replace any existing query items with those provided in the queryItems + // parameter, if any were given. + if let queryItems = queryItems { + self.queryItems = queryItems + } response { data, response, error in @@ -490,7 +506,7 @@ public class RestRequest: NSObject { /// - Parameters: /// - responseToError: Error callback closure in case of request failure /// - templateParams: URL templating parameters used for substituion if possible - /// - queryItems: array containing `URLQueryItem` objects that will be appended to the request's URL + /// - queryItems: Sets the query parameters for this RestRequest, overwriting any existing parameters. Defaults to `nil`, which means that this parameter will be ignored, and `RestRequest.queryItems` will be used instead. Note that if you wish to clear any existing query parameters, then you should set `request.queryItems = nil` before calling this function. /// - completionHandler: Callback used on completion of operation public func responseString( responseToError: ((HTTPURLResponse?, Data?) -> Error?)? = nil, @@ -505,7 +521,11 @@ public class RestRequest: NSObject { return } - self.queryItems = queryItems + // Replace any existing query items with those provided in the queryItems + // parameter, if any were given. + if let queryItems = queryItems { + self.queryItems = queryItems + } response { data, response, error in @@ -555,7 +575,7 @@ public class RestRequest: NSObject { /// - Parameters: /// - responseToError: Error callback closure in case of request failure /// - templateParams: URL templating parameters used for substituion if possible - /// - queryItems: array containing `URLQueryItem` objects that will be appended to the request's URL + /// - queryItems: Sets the query parameters for this RestRequest, overwriting any existing parameters. Defaults to `nil`, which means that this parameter will be ignored, and `RestRequest.queryItems` will be used instead. Note that if you wish to clear any existing query parameters, then you should set `request.queryItems = nil` before calling this function. /// - completionHandler: Callback used on completion of operation public func responseVoid( responseToError: ((HTTPURLResponse?, Data?) -> Error?)? = nil, @@ -570,7 +590,11 @@ public class RestRequest: NSObject { return } - self.queryItems = queryItems + // Replace any existing query items with those provided in the queryItems + // parameter, if any were given. + if let queryItems = queryItems { + self.queryItems = queryItems + } response { data, response, error in diff --git a/Tests/SwiftyRequestTests/SwiftyRequestTests.swift b/Tests/SwiftyRequestTests/SwiftyRequestTests.swift index 887157b..ceeeffa 100644 --- a/Tests/SwiftyRequestTests/SwiftyRequestTests.swift +++ b/Tests/SwiftyRequestTests/SwiftyRequestTests.swift @@ -239,7 +239,12 @@ class SwiftyRequestTests: XCTestCase { waitForExpectations(timeout: 10) } - + + // TODO: This test does not appear to really test query parameters... + // This needs to test that a request made to an API that _requires_ a query + // parameter is successful, or that the response somehow indicates that one + // was successfully provided. Both methods of setting query items should be + // tested (setter on RestRequest, and via param to request.responseObject). func testQueryObject() { let expectation = self.expectation(description: "responseObject SwiftyRequest test") @@ -688,6 +693,8 @@ class SwiftyRequestTests: XCTestCase { if let queryItems = response.request?.url?.query { XCTAssertEqual(queryItems, "friend=darren%2Bfink") } + // Explicitly remove query items before next request + request.queryItems = nil request.responseData(completionHandler: completionHandlerThree) case .failure(let error): XCTFail("Failed to get weather response data with error: \(error)") @@ -710,7 +717,11 @@ class SwiftyRequestTests: XCTestCase { } } - request.responseData(queryItems: initialQueryItems, completionHandler: completionHandlerOne) + // Set the query items for subsequent requests + request.queryItems = initialQueryItems + + // Do not explicitly pass `queryItems` - current configuration should be picked up + request.responseData(completionHandler: completionHandlerOne) waitForExpectations(timeout: 10) @@ -764,6 +775,8 @@ class SwiftyRequestTests: XCTestCase { if let queryItems = response.request?.url?.query { XCTAssertEqual(queryItems, "friend=darren%2Bfink") } + // Explicitly remove query items before next request + request.queryItems = nil request.responseObject(completionHandler: completionHandlerThree) case .failure(let error): XCTFail("Failed to get weather response data with error: \(error)") @@ -786,7 +799,11 @@ class SwiftyRequestTests: XCTestCase { } } - request.responseObject(queryItems: initialQueryItems, completionHandler: completionHandlerOne) + // Set the query items for subsequent requests + request.queryItems = initialQueryItems + + // Do not explicitly pass `queryItems` - current configuration should be picked up + request.responseObject(completionHandler: completionHandlerOne) waitForExpectations(timeout: 10)