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

Implement NSURLProtocol.stopLoading() for delayed requests #96

Merged
merged 17 commits into from
Aug 20, 2016
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
57e42fa
implemented NSURLProtocol stopLoading method
leviathan Aug 10, 2016
2106533
added IntelliJ AppCode meta file directory to git ignore
leviathan Aug 10, 2016
9c98c60
using `indexOf` instead of swift array `filter` to reduce roundtrips
leviathan Aug 10, 2016
a6d40f0
Detailed the code documentation to make the internal NSURLProtocol (O…
leviathan Aug 11, 2016
2415924
Fixed spelling mistakes in code docs
leviathan Aug 11, 2016
c10b110
Router now remembers the 'to be canceled' URLs & added router unit te…
leviathan Aug 11, 2016
2d0203d
Merge remote-tracking branch 'devlucky/master'
leviathan Aug 11, 2016
f5d61a9
Move request cancelation to private method and fixed unit test for Ro…
leviathan Aug 11, 2016
ea5a349
Check that the url client does not deliver notifications when the rou…
leviathan Aug 11, 2016
c65872f
Added stopLoading tests to cover the cancellation logic
leviathan Aug 12, 2016
69338ca
Cancel the request immediately
leviathan Aug 12, 2016
b91867f
Using simple flag on the KakapoServer instance to let the OS mark a s…
leviathan Aug 12, 2016
60417ca
Removed code comments, which expose too much of internal implementati…
leviathan Aug 19, 2016
cd83ba7
fixed spelling mistake in test description
leviathan Aug 19, 2016
1e6d40b
Removed error prone initialization of error response objects in test …
leviathan Aug 19, 2016
1672e55
fixed duplicate URL request test
leviathan Aug 20, 2016
e6ea7c8
Merge remote-tracking branch 'devlucky/master'
leviathan Aug 20, 2016
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
6 changes: 5 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ xcuserdata
timeline.xctimeline
playground.xcworkspace

# AppCode
.idea
.idea/

Copy link
Member

Choose a reason for hiding this comment

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

What is this for? Some auto generated files from AppCode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, these are meta files generated by AppCode (don't want to have them in the repo).

# Swift Package Manager
#
# Add this line if you want to avoid checking in source code from Swift Package Manager dependencies.
Expand All @@ -56,7 +60,7 @@ Carthage/Build

# fastlane
#
# It is recommended to not store the screenshots in the git repo. Instead, use fastlane to re-generate the
# It is recommended to not store the screenshots in the git repo. Instead, use fastlane to re-generate the
# screenshots whenever they are needed.
# For more information about the recommended setup visit:
# https://github.com/fastlane/fastlane/blob/master/docs/Gitignore.md
Expand Down
2 changes: 1 addition & 1 deletion Source/JSONAPIError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import Foundation

/// A convenince error object that conform to JSON API
/// A convenience error object that conform to JSON API
public struct JSONAPIError: ResponseFieldsProvider {

/// An object containing references to the source of the error, optionally including any of the following members
Expand Down
4 changes: 2 additions & 2 deletions Source/JSONAPILinks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public enum JSONAPILink: CustomSerializable {
}
```

Appart from their own entity links, and in order to provide extra links for relationships, `User` must specify them for each relationship key:
Apart from their own entity links, and in order to provide extra links for relationships, `User` must specify them for each relationship key:

```swift
let cats = [Cat(id: "33", name: "Stancho", links: nil),
Expand Down Expand Up @@ -111,7 +111,7 @@ public enum JSONAPILink: CustomSerializable {
public protocol JSONAPILinkedEntity {
/// The related links, must use link-names as keys and links as values.
var links: [String : JSONAPILink]? { get }
/// The relationships links, an object containing relationsips can specify top level links for every relationship type. The object must provide a Dictionary where keys are the relationships types and values are dictionaries with link-names as keys and link as values.
/// The relationships links, an object containing relationships can specify top level links for every relationship type. The object must provide a Dictionary where keys are the relationships types and values are dictionaries with link-names as keys and link as values.
var relationshipsLinks: [String : [String : JSONAPILink]]? { get }
}

Expand Down
4 changes: 2 additions & 2 deletions Source/JSONAPISerializer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ public extension JSONAPIEntity {

// MARK: JSONAPIEntity

/// returns the lowercased class name as string by default
/// returns the lower-cased class name as string by default
static var type: String {
return String(self).lowercaseString
}
Expand Down Expand Up @@ -332,7 +332,7 @@ public extension JSONAPIEntity {
return data
}

/// returns the `included` relationsips field conforming to JSON API
/// returns the `included` relationships field conforming to JSON API
public func includedRelationships(includeChildren: Bool, keyTransformer: KeyTransformer?) -> [AnyObject]? {
let mirror = Mirror(reflecting: self)
let includedRelationships = mirror.children.flatMap { (label, value) -> [AnyObject] in
Expand Down
12 changes: 6 additions & 6 deletions Source/KakapoDB.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public protocol Storable {
An initializer that is used by `KakapoDB` to create objects to be stored in the db
- parameter id: The unique identifier provided by `KakapoDB`, objects shouldn't generate ids themselves. `KakapoDB` generate `Int` ids converted to String for better compatibilities with standards like JSONAPI, in case you need `Int` ids is safe to ssume that the conversion will always succeeed.
- parameter db: The db that is creating the object, can be used to generate other `Storable` objects, for example relationsips of the object: `myRelationship = db.create(MyRelationshipType)` or `myrelationship = db.insert { MyRelationshipType(id: $0, db: db) }`. The relationsip will also recieve the `db` instance to eventually initialize its relationships.
- parameter db: The db that is creating the object, can be used to generate other `Storable` objects, for example relationships of the object: `myRelationship = db.create(MyRelationshipType)` or `myrelationship = db.insert { MyRelationshipType(id: $0, db: db) }`. The relationsip will also recieve the `db` instance to eventually initialize its relationships.
- returns: A configured object stored in the db.
*/
Expand Down Expand Up @@ -93,7 +93,7 @@ public final class KakapoDB {
/**
Creates and inserts Storable objects based on their default initializer
- parameter (unamed): The Storable Type to be created
- parameter (unnamed): The Storable Type to be created
- parameter number: The number of elements to create, defaults to 1
- returns: An array containing the new inserted Storable objects
Expand Down Expand Up @@ -181,7 +181,7 @@ public final class KakapoDB {
/**
Find all the objects in the store of a given Storable Type
- parameter (unamed): The Storable Type to be found
- parameter (unnamed): The Storable Type to be found
- returns: An array containing the found Storable objects
*/
Expand All @@ -194,7 +194,7 @@ public final class KakapoDB {
/**
Filter all the objects in the store of a given Storable Type that satisfy the a given handler
- parameter (unamed): The Storable Type to be filtered
- parameter (unnamed): The Storable Type to be filtered
- parameter includeElement: The predicate to satisfy the filtering
- returns: An array containing the filtered Storable objects
Expand All @@ -206,10 +206,10 @@ public final class KakapoDB {
/**
Find the object in the store by a given id
- parameter (unamed): The Storable Type to be filtered
- parameter (unnamed): The Storable Type to be filtered
- parameter id: The id to search for
- returns: An optional thay may (or not) contain the found Storable object
- returns: An optional, which may (or may not) contain the found Storable object
*/
public func find<T: Storable>(_: T.Type, id: String) -> T? {
return filter(T.self) { $0.id == id }.first
Expand Down
34 changes: 25 additions & 9 deletions Source/KakapoServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
import Foundation

/**
A server that conforms to NSURLProtocol in order to intercept outgoing network communication.
You shouldn't use this class directly but register a `Router` instead. Since frameworks like **AFNetworking** and **Alamofire** require manual registration of the `NSURLProtocol` classes you will need to register this class when needed.
A server that conforms to `NSURLProtocol` in order to intercept outgoing network communication.
You shouldn't use this class directly but register a `Router` instead.
Since frameworks like **AFNetworking** and **Alamofire** require manual registration of the `NSURLProtocol` classes
you will need to register this class when needed.

### Examples

Expand All @@ -37,15 +39,20 @@ import Foundation
```
*/
public final class KakapoServer: NSURLProtocol {


/**
The overall list of `Router` objects, which is know to all `KakapoServer` instances.
Visibility is on the `KakapoServer` class level, because `NSURLProtocol` just allows us to register
class elements (no instances are possible).
*/
Copy link
Member

Choose a reason for hiding this comment

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

No need to add documentation here since it's a private var -> implementation detail on how the Router and Server communicate and Server keeps storage of routers, nothing that the client needs to be aware of.

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 would like to keep this comment. How shall I mark it that its not visible to the public? /!* ??

Copy link
Member

Choose a reason for hiding this comment

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

Nothing special needed, private prop/methods/types are not accessible anyway. However we try to limit comments , only when really needed

Copy link
Member

Choose a reason for hiding this comment

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

This comment is not clear/correct IMHO. Not sure if it's even needed

Copy link
Member

Choose a reason for hiding this comment

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

Oh important: routers should remain private and yes, I will also say that this comment should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed comment in 60417ca

private static var routers: [Router] = []

/**
Register and return a new Router in the Server

- parameter baseURL: The base URL that this Router will use

- returns: An new initializcaRouter objects can hold the same baseURL.
- returns: A newly initialized Router object, which is configured to use the `baseURL`.
*/
class func register(baseURL: String) -> Router {
NSURLProtocol.registerClass(self)
Expand Down Expand Up @@ -74,14 +81,19 @@ public final class KakapoServer: NSURLProtocol {
}

/**
KakapoServer checks if the given request matches any of the registered routes and determines if the request should be intercepted
`KakapoServer` checks if the given request matches any of the registered routes
and determines if the request should be intercepted.

Note: If this method returns `true`, then the OS will create a new `KakapoServer` instance for the `request`
via the `init(request:cachedResponse:client:)` initializer. So, this `request` is the same, which
we'll have access to later on in the `startLoading()` and `stopLoading()` methods.

- parameter request: A request

- returns: true if any of the registered route match the request URL
*/
override public class func canInitWithRequest(request: NSURLRequest) -> Bool {
return routers.filter { $0.canInitWithRequest(request) }.first != nil
return routers.indexOf({ $0.canInitWithRequest(request) }) != nil
}

/// Just returns the given request without changes
Expand All @@ -91,11 +103,15 @@ public final class KakapoServer: NSURLProtocol {

/// Start loading the matched requested, the route handler will be called and the returned object will be serialized.
override public func startLoading() {
KakapoServer.routers.filter { $0.canInitWithRequest(request) }.first!.startLoading(self)
if let routerIndex = KakapoServer.routers.indexOf({ $0.canInitWithRequest(request) }) {
KakapoServer.routers[routerIndex].startLoading(self)
}
}

/// Not implemented yet, does nothing ATM. https://github.com/devlucky/Kakapo/issues/88
/// Stops the loading of the matched request.
override public func stopLoading() {
/* TODO: implement stopLoading for delayed requests https://github.com/devlucky/Kakapo/issues/88 */
if let routerIndex = KakapoServer.routers.indexOf({ $0.canInitWithRequest(request) }) {
KakapoServer.routers[routerIndex].stopLoading(self)
}
Copy link
Member

@joanromano joanromano Aug 11, 2016

Choose a reason for hiding this comment

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

So, if our assumption is right that Foundation is creating a new KakapoServer instance per request since we return true for canInitWithRequest: could we, as suggested by @MP0w, just use an internal boolean flag here to later not start loading on startLoading?

That sounds like the most reasonable and simple approach for now, though we can discuss if it's the best one. But it will, almost 100% sure, prevent to modify the Routerclass at all right?

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've added a test case in ea5a349 for the case when the URLRequest gets cancelled directly.

However, Router is handling the trigger of the NSURLProtocolClient#URLProtocolDidFinishLoading... delegate call. Apple doc requires that no further delegate calls are performed when the request has been stopped.
Soo, I added some test cases for this behaviour as well, which basically ensures that the URLProtocolDidFinishLoading does not get called by the Router when the request has been marked as stopped by the OS.

Copy link
Member

Choose a reason for hiding this comment

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

As mentioned, can you try to just keep a private boolean flag that, when set, will prevent the Server to forward the call to the routers in startLoading()?

  1. stopLoading -> set flag to true
  2. startLoading -> check if flag is set; if it's the case, we don't even forward calls to router, just do nothing.

Copy link
Member

Choose a reason for hiding this comment

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

This way, we can just leave the Router as it was before. No changes would be needed as far as I know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I will implement it like this.

}
}
8 changes: 4 additions & 4 deletions Source/RouteMatcher.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import Foundation
public typealias URLInfo = (components: [String : String], queryParameters: [NSURLQueryItem])

/**
Match a route and a requestURL. A route is composed by a baseURL and a path, togheter they should match the given requestURL.
Match a route and a requestURL. A route is composed by a baseURL and a path, together they should match the given requestURL.
To match a route the baseURL must be contained in the requestURL, the substring of the requestURL following the baseURL then is tested against the path to check if they match.
A baseURL can contain a scheme, and the requestURL must match the scheme; if it doesn't contain a scheme then the baseURL is a wildcard and will be matched by any subdomain or any scheme:
Expand All @@ -23,7 +23,7 @@ public typealias URLInfo = (components: [String : String], queryParameters: [NSU
- base: `kakapo.com`, path: "any", requestURL: "https://kakapo.com/any" ✅
- base: `kakapo.com`, path: "any", requestURL: "https://api.kakapo.com/any" ✅
A path can contain wildcard components prefixed with ":" (e.g. /users/:userid) that are used to build the component dictionary, the wildcard is then used as key and the repsective component of the requestURL is used as value.
A path can contain wildcard components prefixed with ":" (e.g. /users/:userid) that are used to build the component dictionary, the wildcard is then used as key and the respective component of the requestURL is used as value.
Any component that is not a wildcard have to be exactly the same in both the path and the request, otherwise the route won't match.
- `/users/:userid` and `/users/1234` ✅ -> `[userid: 1234]`
Expand All @@ -35,7 +35,7 @@ public typealias URLInfo = (components: [String : String], queryParameters: [NSU
- parameter path: The path of the request, can contain wildcards components prefixed with ":" (e.g. /users/:id/)
- parameter requestURL: The URL of the request (e.g. https://kakapo.com/api/users/1234)
- returns: A URL info object containing `components` and `queryParamaters` or nil if `requestURL`doesn't match the route.
- returns: A URL info object containing `components` and `queryParameters` or nil if `requestURL`doesn't match the route.
*/
func matchRoute(baseURL: String, path: String, requestURL: NSURL) -> URLInfo? {

Expand Down Expand Up @@ -91,7 +91,7 @@ private extension String {
}

/**
Retrun the substring From/To a given string or nil if the string is not contained.
Return the substring From/To a given string or nil if the string is not contained.
- **From**: return the substring following the given string (e.g. `kakapo.com/users`, `kakapo.com` -> `/users`)
- **To**: return the substring preceding the given string (e.g. `kakapo.com/users?a=b`, `?` -> `kakapo.com/users`)
*/
Expand Down
64 changes: 49 additions & 15 deletions Source/Router.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ public struct Request {

/**
A protocol to adopt when a `Serializable object needs to also provide response status code and/or headerFields
For example you may use `Response` to wrap your `Serializable` object to just achieve the result or directly implement the protocol. For examply `JSONAPISerializer` implement the protocol in order to be able to provide custom status code in the response.
For example you may use `Response` to wrap your `Serializable` object to just achieve the result or directly implement the protocol.
For example `JSONAPISerializer` implement the protocol in order to be able to provide custom status code in the response.
*/
public protocol ResponseFieldsProvider: CustomSerializable {
/// The response status code
Expand All @@ -59,7 +60,7 @@ extension ResponseFieldsProvider {
/**
A ResponseFieldsProvider implementation which can be used in `RouteHandlers` to provide valid responses that can return different status code than the default (200) or headerFields.

The struct provides, appart from a Serializable `body` object, a status code and header fields.
The struct provides, apart from a Serializable `body` object, a status code and header fields.
*/
public struct Response: ResponseFieldsProvider {
/// The response status code
Expand All @@ -72,11 +73,11 @@ public struct Response: ResponseFieldsProvider {
public let headerFields: [String : String]?

/**
Initialize `Response` object that wraps another `Serializable` object for the serialization but, implemententing `ResponseFieldsProvider` can affect some parameters of the HTTP response
Initialize `Response` object that wraps another `Serializable` object for the serialization but, implementing `ResponseFieldsProvider` can affect some parameters of the HTTP response

- parameter statusCode: the status code that the response should provide to the HTTP repsonse
- parameter statusCode: the status code that the response should provide to the HTTP response
- parameter body: the body that will be serialized
- parameter headerFields: the headerFields that the response should provide to the HTTP repsonse
- parameter headerFields: the headerFields that the response should provide to the HTTP response
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for fixing all those typos! 👍

Copy link
Member

Choose a reason for hiding this comment

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

Ty! I'm really bad with typos I will implement Danger that apparently has typo checks!


- returns: A wrapper `Serializable` object that affect http requests.
*/
Expand All @@ -101,17 +102,19 @@ public final class Router {
}

private var routes: [String : Route] = [:]

private var canceledRequests: [NSURL] = []

/// The `baseURL` of the Router
public let baseURL: String

/// The desired latency to delay the mocked responses. Default value is 0.
/// The desired latency (in seconds) to delay the mocked responses. Default value is 0.
public var latency: NSTimeInterval = 0


/**
Register a new Router in the KakapoServer.
The baseURL can contain a scheme, and the requestURL must match the scheme; if it doesn't contain a scheme then the baseURL is a wildcard and will be matched by any subdomain or any scheme:
Register a new Router in the `KakapoServer`.
The `baseURL` can contain a scheme, and the requestURL must match the scheme; if it doesn't contain a scheme then
the `baseURL` is a wildcard and will be matched by any subdomain or any scheme:

- base: `http://kakapo.com`, path: "any", requestURL: "http://kakapo.com/any" ✅
- base: `http://kakapo.com`, path: "any", requestURL: "https://kakapo.com/any" ❌ because it's **https**
Expand Down Expand Up @@ -210,14 +213,28 @@ public final class Router {

let delayTime = dispatch_time(DISPATCH_TIME_NOW, Int64(latency * Double(NSEC_PER_SEC)))
dispatch_after(delayTime, dispatch_get_main_queue()) {
didFinishLoading(server)
[weak self] in
// before reporting "finished", check if request has been canceled in the meantime
if self?.cancelRequest(requestURL) == false {
didFinishLoading(server)
}
}
}


func stopLoading(server: NSURLProtocol) {
guard let requestURL = server.request.URL else {
return
}
// if request URL not in the list of "to be canceled" requests -> enqueue it
if canceledRequests.contains(requestURL) == false {
canceledRequests.append(requestURL)
}
}

/**
Registers a GET request with the given path.

The path is used togheter with the `Router.baseURL` to match requests. It can contain wildcard components prefixed by ":" that are later used to retreive the components of the request:
The path is used together with the `Router.baseURL` to match requests. It can contain wildcard components prefixed by ":" that are later used to retrieve the components of the request:

- "/users/:userid" and "/users/1234" will produce [userid: 1234]

Expand All @@ -239,7 +256,7 @@ public final class Router {
/**
Registers a POST request with the given path

The path is used togheter with the `Router.baseURL` to match requests. It can contain wildcard components prefixed by ":" that are later used to retreive the components of the request:
The path is used together with the `Router.baseURL` to match requests. It can contain wildcard components prefixed by ":" that are later used to retrieve the components of the request:

- "/users/:userid" and "/users/1234" will produce [userid: 1234]

Expand All @@ -261,7 +278,7 @@ public final class Router {
/**
Registers a DEL request with the given path

The path is used togheter with the `Router.baseURL` to match requests. It can contain wildcard components prefixed by ":" that are later used to retreive the components of the request:
The path is used together with the `Router.baseURL` to match requests. It can contain wildcard components prefixed by ":" that are later used to retrieve the components of the request:

- "/users/:userid" and "/users/1234" will produce [userid: 1234]

Expand All @@ -283,7 +300,7 @@ public final class Router {
/**
Registers a PUT request with the given path

The path is used togheter with the `Router.baseURL` to match requests. It can contain wildcard components prefixed by ":" that are later used to retreive the components of the request:
The path is used together with the `Router.baseURL` to match requests. It can contain wildcard components prefixed by ":" that are later used to retrieve the components of the request:

- "/users/:userid" and "/users/1234" will produce [userid: 1234]

Expand All @@ -301,5 +318,22 @@ public final class Router {
public func put(path: String, handler: RouteHandler) {
routes[path] = (.PUT, handler)
}

/**
Determines, if the `requestURL` has been marked as "should be canceled" or not.

- return: `true` if the `requestURL` should be canceled, otherwise `false`
*/
private func cancelRequest(requestURL: NSURL) -> Bool {
var requestUrlCanceled = false

if let canceledRequestIndex = canceledRequests.indexOf(requestURL) {
// remove request URL from the list of "canceled requests" and DO NOT send notification(s)
canceledRequests.removeAtIndex(canceledRequestIndex)
requestUrlCanceled = true
}

return requestUrlCanceled
}

}
Loading