From 250d49d755a1520ec9ca31cf834d6cd82ececb4c Mon Sep 17 00:00:00 2001 From: Sam Dods Date: Thu, 11 Jan 2018 12:47:14 +0000 Subject: [PATCH 1/2] Provide resource service subclasses with the ability to set resource-specific properties of the request --- .../Resource types/NetworkResourceType.swift | 15 ++++---- .../Services/NetworkDataResourceService.swift | 23 ++++++++--- TABResourceLoader.podspec | 2 +- ...MockNilURLRequestNetworkJSONResource.swift | 2 +- .../Protocols/NetworkResourceTypeTests.swift | 10 ++--- .../NetworkDataResourceServiceTests.swift | 4 +- ...assedNetworkDataResourceServiceTests.swift | 38 ++++++++++++++++--- 7 files changed, 67 insertions(+), 27 deletions(-) diff --git a/Sources/TABResourceLoader/Protocols/Resource types/NetworkResourceType.swift b/Sources/TABResourceLoader/Protocols/Resource types/NetworkResourceType.swift index effba77..5267411 100644 --- a/Sources/TABResourceLoader/Protocols/Resource types/NetworkResourceType.swift +++ b/Sources/TABResourceLoader/Protocols/Resource types/NetworkResourceType.swift @@ -62,9 +62,11 @@ public protocol NetworkResourceType { /** Convenience function that builds a URLRequest for this resource + - parameter additionalQueryParameters: Query parameters to be added to the URL - returns: A URLRequest or nil if the construction of the request failed */ - func urlRequest() -> URLRequest? + + func urlRequest(with: [URLQueryItem]) -> URLRequest? } // MARK: - NetworkJSONResource defaults @@ -75,10 +77,10 @@ public extension NetworkResourceType { public var jsonBody: Any? { return nil } public var queryItems: [URLQueryItem]? { return nil } - public func urlRequest() -> URLRequest? { + public func urlRequest(with additionalQueryParameters: [URLQueryItem]) -> URLRequest? { var urlComponents = URLComponents(url: url, resolvingAgainstBaseURL: true) let initialItems = urlComponents?.queryItems - urlComponents?.queryItems = allQueryItems(initialItems: initialItems) + urlComponents?.queryItems = allQueryItems(initialItems: initialItems, additional: additionalQueryParameters) guard let urlFromComponents = urlComponents?.url else { return nil } @@ -93,10 +95,9 @@ public extension NetworkResourceType { return request } - private func allQueryItems(initialItems: [URLQueryItem]?) -> [URLQueryItem]? { - let combinedQueryItems = (initialItems ?? []) + (queryItems ?? []) - let allQueryItems = combinedQueryItems.isEmpty ? nil : combinedQueryItems - return allQueryItems + private func allQueryItems(initialItems: [URLQueryItem]?, additional: [URLQueryItem]) -> [URLQueryItem]? { + let combinedQueryItems = (initialItems ?? []) + (queryItems ?? []) + additional + return combinedQueryItems.isEmpty ? nil : combinedQueryItems } } diff --git a/Sources/TABResourceLoader/Services/NetworkDataResourceService.swift b/Sources/TABResourceLoader/Services/NetworkDataResourceService.swift index f24db39..5f68a06 100644 --- a/Sources/TABResourceLoader/Services/NetworkDataResourceService.swift +++ b/Sources/TABResourceLoader/Services/NetworkDataResourceService.swift @@ -51,13 +51,24 @@ open class NetworkDataResourceService { /** Method designed to be implemented on subclasses, these fields will be overriden by any HTTP header field - key that is defined in the resource (in case of conflicts) + key that is defined in the resource (in case of conflicts). This gives the resource service subclass a chance to add query parameters based on information retrieved from the resource - - returns: Return any additional header fields that need to be added to the url request + - parameter resource: The resource being used to create the URL request + - returns: Return any additional header fields that need to be added to the URL request */ - open func additionalHeaderFields() -> [String: String] { + open func additionalHeaderFields(for resource: Resource) -> [String: String] { return [:] } + + /** + Method designed to be implemented on subclasses to provide additional query parameters to be added to the URL. This gives the resource service subclass a chance to add query parameters based on information retrieved from the resource + + - parameter resource: The resource that provides the URL for the URL request + - returns: Return any query parameters that need to be appended to the URL + */ + open func additionalQueryParameters(for resource: Resource) -> [URLQueryItem] { + return [] + } let session: URLSessionType @@ -91,12 +102,14 @@ open class NetworkDataResourceService { // Method used for injecting the NetworkServiceActivity for testing @discardableResult func fetch(resource: Resource, networkServiceActivity: NetworkServiceActivity, completion: @escaping (NetworkResponse) -> Void) -> Cancellable? { - guard var urlRequest = resource.urlRequest() else { + let queryParameters = additionalQueryParameters(for: resource) + guard var urlRequest = resource.urlRequest(with: queryParameters) else { completion(.failure(.couldNotCreateURLRequest, nil)) return nil } - urlRequest.allHTTPHeaderFields = merge(additionalHeaderFields(), urlRequest.allHTTPHeaderFields) + let additionalHeaderFields = self.additionalHeaderFields(for: resource) + urlRequest.allHTTPHeaderFields = merge(additionalHeaderFields, urlRequest.allHTTPHeaderFields) networkServiceActivity.increaseActiveRequest() let cancellable = session.perform(request: urlRequest) { (data, URLResponse, error) in networkServiceActivity.decreaseActiveRequest() diff --git a/TABResourceLoader.podspec b/TABResourceLoader.podspec index bb8db98..37a383c 100644 --- a/TABResourceLoader.podspec +++ b/TABResourceLoader.podspec @@ -1,7 +1,7 @@ Pod::Spec.new do |spec| spec.name = 'TABResourceLoader' spec.homepage = 'https://github.com/theappbusiness/TABResourceLoader' - spec.version = '7.2.0' + spec.version = '8.0.0' spec.license = { :type => 'MIT' } spec.authors = { 'Luciano Marisi' => 'luciano@techbrewers.com' } spec.summary = 'Framework for loading resources from a network service' diff --git a/Tests/TABResourceLoaderTests/Mocks/MockNilURLRequestNetworkJSONResource.swift b/Tests/TABResourceLoaderTests/Mocks/MockNilURLRequestNetworkJSONResource.swift index 6899f85..a24f7e4 100644 --- a/Tests/TABResourceLoaderTests/Mocks/MockNilURLRequestNetworkJSONResource.swift +++ b/Tests/TABResourceLoaderTests/Mocks/MockNilURLRequestNetworkJSONResource.swift @@ -13,7 +13,7 @@ struct MockNilURLRequestNetworkJSONResource: NetworkDataResourceType { typealias Model = String let url: URL = URL(string: "www.test.com")! - func urlRequest() -> URLRequest? { + func urlRequest(with: [URLQueryItem]) -> URLRequest? { return nil } diff --git a/Tests/TABResourceLoaderTests/Protocols/NetworkResourceTypeTests.swift b/Tests/TABResourceLoaderTests/Protocols/NetworkResourceTypeTests.swift index eb6a7ec..d0bbcaa 100644 --- a/Tests/TABResourceLoaderTests/Protocols/NetworkResourceTypeTests.swift +++ b/Tests/TABResourceLoaderTests/Protocols/NetworkResourceTypeTests.swift @@ -52,7 +52,7 @@ class NetworkResourceTypeTests: XCTestCase { let expectedURL = "\(url)?query-name=query-value" let mockNetworkResource = MockCustomNetworkResource(url: url, httpRequestMethod: expectedHTTPMethod, httpHeaderFields: expectedAllHTTPHeaderFields, jsonBody: expectedJSONBody, queryItems: mockedURLQueryItems) - let urlRequest = mockNetworkResource.urlRequest() + let urlRequest = mockNetworkResource.urlRequest(with: []) XCTAssertNotNil(urlRequest) XCTAssertEqual(urlRequest?.url?.absoluteString, expectedURL) XCTAssertEqual(urlRequest?.httpMethod, expectedHTTPMethod.rawValue) @@ -65,7 +65,7 @@ class NetworkResourceTypeTests: XCTestCase { let urlWithQueryParameters = URL(string: "www.test.com?query-name=query-value")! let resource = MockCustomNetworkResource(url: urlWithQueryParameters) - let urlRequest = resource.urlRequest() + let urlRequest = resource.urlRequest(with: []) XCTAssertEqual(urlRequest?.url?.absoluteString, urlWithQueryParameters.absoluteString) } @@ -73,7 +73,7 @@ class NetworkResourceTypeTests: XCTestCase { let urlWithQueryParameters = URL(string: "www.test.com")! let resource = MockCustomNetworkResource(url: urlWithQueryParameters) - let urlRequest = resource.urlRequest() + let urlRequest = resource.urlRequest(with: []) XCTAssertEqual(urlRequest?.url?.absoluteString, "www.test.com") } @@ -82,7 +82,7 @@ class NetworkResourceTypeTests: XCTestCase { let resource = MockCustomNetworkResource(url: urlWithQueryItem, queryItems: mockedURLQueryItems) let expectedURLString = "\(urlWithQueryItem)&query-name-a=query-value-a" - let urlRequest = resource.urlRequest() + let urlRequest = resource.urlRequest(with: []) XCTAssertEqual(urlRequest?.url?.absoluteString, expectedURLString) } @@ -91,7 +91,7 @@ class NetworkResourceTypeTests: XCTestCase { let resource = MockCustomNetworkResource(url: urlWithQueryItem, queryItems: mockedURLQueryItems) let expectedURLString = "\(urlWithQueryItem)&query-name=query-value-a" - let urlRequest = resource.urlRequest() + let urlRequest = resource.urlRequest(with: []) XCTAssertEqual(urlRequest?.url?.absoluteString, expectedURLString) } diff --git a/Tests/TABResourceLoaderTests/Services/NetworkDataResourceServiceTests.swift b/Tests/TABResourceLoaderTests/Services/NetworkDataResourceServiceTests.swift index 7e8150e..7afcf4a 100644 --- a/Tests/TABResourceLoaderTests/Services/NetworkDataResourceServiceTests.swift +++ b/Tests/TABResourceLoaderTests/Services/NetworkDataResourceServiceTests.swift @@ -45,7 +45,7 @@ class NetworkDataResourceServiceTests: XCTestCase { func test_fetch_callsPerformRequestOnSessionWithCorrectURLRequest() { testService.fetch(resource: mockResource) { _ in } let capturedRequest = mockSession.capturedRequest - let expectedRequest = mockResource.urlRequest() + let expectedRequest = mockResource.urlRequest(with: []) XCTAssertNotNil(expectedRequest?.allHTTPHeaderFields) XCTAssertEqual(capturedRequest, expectedRequest) } @@ -71,7 +71,7 @@ class NetworkDataResourceServiceTests: XCTestCase { func test_fetch_withInvalidURLRequest_callsFailureWithCorrectError() { let mockInvalidURLResource = MockNilURLRequestNetworkJSONResource() let newTestRequestManager = GenericNetworkDataResourceService(session: mockSession) - XCTAssertNil(mockInvalidURLResource.urlRequest()) + XCTAssertNil(mockInvalidURLResource.urlRequest(with: [])) performAsyncTest { expectation in newTestRequestManager.fetch(resource: mockInvalidURLResource) { result in switch result { diff --git a/Tests/TABResourceLoaderTests/Services/SubclassedNetworkDataResourceServiceTests.swift b/Tests/TABResourceLoaderTests/Services/SubclassedNetworkDataResourceServiceTests.swift index 0d1fa82..9e0482b 100644 --- a/Tests/TABResourceLoaderTests/Services/SubclassedNetworkDataResourceServiceTests.swift +++ b/Tests/TABResourceLoaderTests/Services/SubclassedNetworkDataResourceServiceTests.swift @@ -10,18 +10,31 @@ import XCTest @testable import TABResourceLoader private let commonKey = "common" +private let resourceNameKey = "resourceName" +private let mockResourceName = "Mock" +private let queryKey = "query" +private let mockQuery = "example" -struct MockHTTPHeaderFieldsNetworkDataResource: NetworkResourceType, DataResourceType { +protocol MockResourceType: NetworkResourceType, DataResourceType { + var resourceName: String { get } +} + +struct MockHTTPHeaderFieldsNetworkDataResource: MockResourceType { typealias Model = String let url: URL let httpHeaderFields: [String: String]? + let resourceName = mockResourceName + + var queryItems: [URLQueryItem]? { + return [URLQueryItem(name: queryKey, value: mockQuery)] + } func model(from data: Data) throws -> String { return "" } } -final class SubclassedNetworkDataResourceService: GenericNetworkDataResourceService { +final class SubclassedNetworkDataResourceService: GenericNetworkDataResourceService { required init() { super.init() @@ -31,8 +44,15 @@ final class SubclassedNetworkDataResourceService [String: String] { - return [commonKey: "subclass"] + override func additionalHeaderFields(for resource: Resource) -> [String: String] { + return [ + commonKey: "subclass", + resourceNameKey: mockResourceName, + ] + } + + override func additionalQueryParameters(for resource: Resource) -> [URLQueryItem] where Resource : DataResourceType, Resource : NetworkResourceType { + return [URLQueryItem(name: resourceNameKey, value: mockResourceName)] } } @@ -54,14 +74,20 @@ class SubclassedNetworkJSONResourceServiceTests: XCTestCase { //swiftlint:disabl let resourceHTTPHeaderFields = [commonKey: "resource"] mockResource = MockHTTPHeaderFieldsNetworkDataResource(url: mockURL, httpHeaderFields: resourceHTTPHeaderFields) testService.fetch(resource: mockResource) { _ in } - XCTAssertEqual(mockSession.capturedRequest!.allHTTPHeaderFields!, resourceHTTPHeaderFields) + let expectedHeaderFields = [commonKey: "resource", resourceNameKey: mockResourceName] + XCTAssertEqual(mockSession.capturedRequest!.allHTTPHeaderFields!, expectedHeaderFields) + + let expectedQuery1 = "\(queryKey)=\(mockQuery)" + let expectedQuery2 = "\(resourceNameKey)=\(mockResourceName)" + let expectedQuery = "\(expectedQuery1)&\(expectedQuery2)" + XCTAssertEqual(mockSession.capturedRequest?.url?.query, expectedQuery) } func test_finalRequestInclude_subclassHTTPHeaderFields_and_resourceHTTPHeaderFields() { let resourceHTTPHeaderFields = ["resource_key": "resource"] mockResource = MockHTTPHeaderFieldsNetworkDataResource(url: mockURL, httpHeaderFields: resourceHTTPHeaderFields) testService.fetch(resource: mockResource) { _ in } - let expectedHeaderFields = [commonKey: "subclass", "resource_key": "resource"] + let expectedHeaderFields = [commonKey: "subclass", resourceNameKey: mockResourceName, "resource_key": "resource"] XCTAssertEqual(mockSession.capturedRequest!.allHTTPHeaderFields!, expectedHeaderFields) } From bf7c656215349df1ec85a594868fc73b8d02d462 Mon Sep 17 00:00:00 2001 From: Sam Dods Date: Thu, 11 Jan 2018 13:11:46 +0000 Subject: [PATCH 2/2] Updated CHANGELOG --- CHANGELOG.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7749e21..b55128d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Change Log +## 8.0.0 + +- Adding a `resource` argument to the "additional" functions of the resource service - the functions that are intended to be overridden by subclasses. + +Providing the subclass with the resource gives that subclass an opportunity to set parameters of the URL request based on the resource. This is different to the resource providing those parameters itself, e.g. the resource could implement the `httpHeaderFields` and `queryItems` properties. + +Consider a subclass that adds the requirement that its resources must conform to a protocol `GraphQLResouce`. The `GraphQLResouce` protocol requires a property `queryName: String` to be implemented by its conformers. + +The `NetworkDataResourceService` can now add in the `queryName` to either the HTTP header fields or the URL query items. It would be unreliable to expect all resources to implement the `queryItems` and `httpHeaderFields` to add the `queryName` key/value. + +> _**NOTE:** This only impacts consumers that subclass `NetworkDataResourceService` and override the `additionalHeaderFields` implementation._ + +API-Breaking Change: This release modifies the `NetworkDataResourceService.additionalHeaderFields` function by adding an argument to the function. + ## 7.2.0 - Adds the ability to cancel all requests from the resource service.