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

Resource-specific properties set by resource service subclass #67

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Copy link

@manojmahapatra manojmahapatra Jan 26, 2018

Choose a reason for hiding this comment

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

can you also please include additionalQueryParameters in the function signature as the parameter name just to go w/ the comment above.

}

// MARK: - NetworkJSONResource defaults
Expand All @@ -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 }

Expand All @@ -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
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Resource: NetworkResourceType & DataResourceType>(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<Resource: NetworkResourceType & DataResourceType>(for resource: Resource) -> [URLQueryItem] {
return []
}

let session: URLSessionType

Expand Down Expand Up @@ -91,12 +102,14 @@ open class NetworkDataResourceService {
// Method used for injecting the NetworkServiceActivity for testing
@discardableResult
func fetch<Resource: NetworkResourceType & DataResourceType>(resource: Resource, networkServiceActivity: NetworkServiceActivity, completion: @escaping (NetworkResponse<Resource.Model>) -> 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()
Expand Down
2 changes: 1 addition & 1 deletion TABResourceLoader.podspec
Original file line number Diff line number Diff line change
@@ -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' => '[email protected]' }
spec.summary = 'Framework for loading resources from a network service'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -65,15 +65,15 @@ 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)
}

func test_urlRequest_resourceURLWithNoQueryItemsHasNoQuestionMark() {
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")
}

Expand All @@ -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)
}

Expand All @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -71,7 +71,7 @@ class NetworkDataResourceServiceTests: XCTestCase {
func test_fetch_withInvalidURLRequest_callsFailureWithCorrectError() {
let mockInvalidURLResource = MockNilURLRequestNetworkJSONResource()
let newTestRequestManager = GenericNetworkDataResourceService<MockNilURLRequestNetworkJSONResource>(session: mockSession)
XCTAssertNil(mockInvalidURLResource.urlRequest())
XCTAssertNil(mockInvalidURLResource.urlRequest(with: []))
performAsyncTest { expectation in
newTestRequestManager.fetch(resource: mockInvalidURLResource) { result in
switch result {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Resource: NetworkResourceType & DataResourceType>: GenericNetworkDataResourceService<Resource> {
final class SubclassedNetworkDataResourceService<Resource: MockResourceType>: GenericNetworkDataResourceService<Resource> {

required init() {
super.init()
Expand All @@ -31,8 +44,15 @@ final class SubclassedNetworkDataResourceService<Resource: NetworkResourceType &
super.init(session: session)
}

override func additionalHeaderFields() -> [String: String] {
return [commonKey: "subclass"]
override func additionalHeaderFields<Resource: MockResourceType>(for resource: Resource) -> [String: String] {
return [
commonKey: "subclass",
resourceNameKey: mockResourceName,
]
}

override func additionalQueryParameters<Resource>(for resource: Resource) -> [URLQueryItem] where Resource : DataResourceType, Resource : NetworkResourceType {
return [URLQueryItem(name: resourceNameKey, value: mockResourceName)]
}
}

Expand All @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

If you wanted to avoid the force unwraps here you could use optional chaining with a default value?

mockSession.capturedRequest?.allHTTPHeaderFields ?? [:] should work? It'll still fail but won't crash the 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.

good point @KaneCheshire. i just modified what was already there. didn't even notice the force unwraps.


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)
}

Expand Down