Skip to content

Commit

Permalink
Fix overwriting of queryItems with nil (#51)
Browse files Browse the repository at this point in the history
  • Loading branch information
@djones6
djones6 authored Jan 28, 2019
1 parent a9ff699 commit 523f389
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 15 deletions.
48 changes: 36 additions & 12 deletions Sources/SwiftyRequest/RestRequest.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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

Expand Down Expand Up @@ -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<T: JSONDecodable>(
responseToError: ((HTTPURLResponse?, Data?) -> Error?)? = nil,
Expand All @@ -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

Expand Down Expand Up @@ -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<T: Decodable>(
responseToError: ((HTTPURLResponse?, Data?) -> Error?)? = nil,
Expand All @@ -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

Expand Down Expand Up @@ -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<T: JSONDecodable>(
responseToError: ((HTTPURLResponse?, Data?) -> Error?)? = nil,
Expand All @@ -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

Expand Down Expand Up @@ -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,
Expand All @@ -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

Expand Down Expand Up @@ -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,
Expand All @@ -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

Expand Down
23 changes: 20 additions & 3 deletions Tests/SwiftyRequestTests/SwiftyRequestTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)")
Expand All @@ -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)

Expand Down Expand Up @@ -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)")
Expand All @@ -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)

Expand Down

0 comments on commit 523f389

Please sign in to comment.