Feat: Add/Retrieve Codable objects in a session #54
Conversation
| case keyNotFound(key: String) | ||
|
|
||
| //Thrown when a primative Decodable or array of primative Decodables fails to be cast to the provided type. | ||
| case failedPrimativeCast() |
There was a problem hiding this comment.
Primative -> Primitive (throughout this PR)
| // MARK StoreError | ||
|
|
||
| /// An error indicating the failure of an operation to encode/decode into/from the session `Store`. | ||
| public enum SessionCodingError: Swift.Error { |
There was a problem hiding this comment.
Please make this a struct instead of an enum so we can add to it without doing a major release.
| public func add<T: Encodable>(_ value: T, forKey key: String) throws { | ||
| let json: Any | ||
| if isPrimative(value: value) { | ||
| json = value |
There was a problem hiding this comment.
Why do we need this special casing for primitives?
There was a problem hiding this comment.
JSONEncoder and JSONDecoder do not have an .allowFragments option and will throw an error if you try and encode/decode a primitive type using them. This has been raised as a bug in SR-6163.
Until that is implemented, isPrimitive handles the case where someone wants to store/retrieve a primitive type or array of primitives type
There was a problem hiding this comment.
OK - I think we should consider not allowing primitives and throwing if the serialization fails. What do you think?
|
We have changed this to use subscript with Codable instead of now functions. the API from aboe would now be: |
| } | ||
| let user: User? = request.session?[userID] | ||
| response.status(.OK) | ||
| response.send(user) |
There was a problem hiding this comment.
Does response.send with an optional Codable do what you expect?
| guard let dict = state[key] else { | ||
| return nil | ||
| } | ||
| if let primative = dict as? T { |
| /// - Parameter key: The Codable key of the entry to retrieve/save. | ||
| public subscript<T: Codable>(key: String) -> T? { | ||
| get { | ||
| guard let dict = state[key] else { |
There was a problem hiding this comment.
Is this really a dictionary or should this be named something like value?
There was a problem hiding this comment.
This process works, but is very inefficient:
session["key"] = value calls JSONEncoder.encode(value), which creates a dictionary representation of the value, then calls JSONSerialization.data() to return you a Data.
Then we take that data and call JSONSerialization.jsonObject() to get back a dictionary. Store that in the session.
When it's time to save the session, we invoke JSONSerialization.data() on the whole session store to produce the JSON data that we will persist.
This bouncing from type -> dictionary -> data -> dictionary -> data is expensive. Ideally we'd either do:
- type -> dictionary -> data (JSONEncoder without the final call to JSONSerialization)
- type -> data -> data (if we could have JSONSerialization nest the ready-prepared Data representation inside the data it is building)
The same applies to the decode.
However, fixing this wouldn't require a public API change, so could be done later.
This PR adds two function to add or retrieve codable objects from a session as a fix for issue #53.
The two function are:
Tests, Jazzy docs and the README have been updated to cover the new API.