Skip to content
This repository has been archived by the owner on Feb 18, 2021. It is now read-only.

Updating for server compatibility with fixes for reply network messages #11

Merged
merged 5 commits into from
Nov 12, 2014
Merged

Updating for server compatibility with fixes for reply network messages #11

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
25ccae9
Updating for server compatibility with fixes for reply network messages
Nov 12, 2014
8ad759e
Remove optional responses for some reply messages as per CR
Nov 12, 2014
00e8e7e
Remove all `success` fields and use `error` with nil to indicate success
Nov 12, 2014
78d3ab7
Code review feedback
Nov 12, 2014
a544793
Merge branch 'develop' into feature/server-parity
Nov 12, 2014
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
2 changes: 1 addition & 1 deletion Jetstream/ModelObject.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ struct PropertyInfo {
}

private var internalScope: Scope?
var scope: Scope? {
public internal(set) var scope: Scope? {
get {
return internalScope
}
Expand Down
4 changes: 4 additions & 0 deletions Jetstream/NetworkMessage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,14 @@ public class NetworkMessage {
return SessionCreateReplyMessage.unserialize(dictionary)
case SessionCreateReplyMessage.messageType:
return SessionCreateReplyMessage.unserialize(dictionary)
case ScopeFetchReplyMessage.messageType:
return ScopeFetchReplyMessage.unserialize(dictionary)
case ScopeStateMessage.messageType:
return ScopeStateMessage.unserialize(dictionary)
case ScopeSyncMessage.messageType:
return ScopeSyncMessage.unserialize(dictionary)
case ScopeSyncReplyMessage.messageType:
return ScopeSyncReplyMessage.unserialize(dictionary)
case PingMessage.messageType:
return PingMessage.unserialize(dictionary)
default:
Expand Down
17 changes: 13 additions & 4 deletions Jetstream/Scope.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,20 @@ import Signals
public var name: String

/// The root model associates with the scope
public var rootModel: ModelObject? {
if modelObjects.count > 0 {
return modelObjects[0]
public var root: ModelObject? {
get {
if modelObjects.count > 0 {
return modelObjects[0]
}
return nil
}
set {
if modelObjects.count == 0 {
if let definiteNewValue = newValue {
definiteNewValue.setScopeAndMakeRootModel(self)
}
}
}
return nil
}

var syncFragmentLookup = [NSUUID: SyncFragment]()
Expand Down
20 changes: 17 additions & 3 deletions Jetstream/ScopeFetchReplyMessage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@ class ScopeFetchReplyMessage: ReplyMessage {
return ScopeFetchReplyMessage.messageType
}

let success: Bool
Copy link
Contributor

Choose a reason for hiding this comment

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

So I was thinking that the absence of an error means that it was successful (wiki also updated with this)

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 it's just weird that SessionCreateReplyMessage has success and this does not

Copy link
Contributor

Choose a reason for hiding this comment

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

Acording to the Wiki it doesn't;) Maybe remove to success flag from SessionCreateReplyMessage, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

let scopeIndex: UInt?
let error: NSError?
let response: AnyObject?

init(index: UInt, replyTo: UInt, scopeIndex: UInt?, error: NSError? = nil) {
init(index: UInt, replyTo: UInt, success: Bool, scopeIndex: UInt?, error: NSError?, response: AnyObject?) {
self.success = success
self.scopeIndex = scopeIndex
self.error = error
self.response = response
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need a response?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So people can do stuff like

    session.on('fetch', function(name, params, callback) {
        // Verify fetching the scope 
        if (name !== scope.name) {
            return callback(new Error('No such scope'), {try: ["ShapesDemo"]});
        }
        callback(null, scope);
    });

And pass application specific responses for failed and/or success messages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm I'm gonna remove this

super.init(index: index, replyTo: replyTo)
}

Expand All @@ -33,16 +37,26 @@ class ScopeFetchReplyMessage: ReplyMessage {
override class func unserialize(dictionary: [String: AnyObject]) -> NetworkMessage? {
var index = dictionary["index"] as? UInt
var replyTo = dictionary["replyTo"] as? UInt
var success = dictionary["success"] as? Bool
var scopeIndex = dictionary["scopeIndex"] as? UInt

var error: NSError?
if let serializedError = dictionary["error"] as? [String: AnyObject] {
error = errorFromDictionary(.SyncFragmentApplyError, serializedError)
}

if index == nil || replyTo == nil || (scopeIndex == nil && error == nil) {
var response: AnyObject? = dictionary["response"]

if index == nil || replyTo == nil || success == nil || (scopeIndex == nil && error == nil) {
return nil
} else {
return ScopeFetchReplyMessage(index: index!, replyTo: replyTo!, scopeIndex: scopeIndex, error: error)
return ScopeFetchReplyMessage(
index: index!,
replyTo: replyTo!,
success: success!,
scopeIndex: scopeIndex,
error: error,
response: response)
}
}
}
4 changes: 2 additions & 2 deletions Jetstream/Session.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public class Session {
switch message {
case let scopeStateMessage as ScopeStateMessage:
if let scope = scopes[scopeStateMessage.scopeIndex] {
if let rootModel = scope.rootModel {
if let definiteRootModelObject = scope.root {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd keep this as "let rootModel = ..." (or rootModelObject). The convention seems to call for using the definite prefix when you're making a local optional and actual variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw, if we're not accessing rootModel after this, why don't we just do "if scope.root != nil"?

scope.startApplyingRemote()
scope.applyRootFragment(scopeStateMessage.rootFragment, additionalFragments: scopeStateMessage.syncFragments)
scope.endApplyingRemote()
Expand All @@ -104,7 +104,7 @@ public class Session {
}
case let scopeSyncMessage as ScopeSyncMessage:
if let scope = scopes[scopeSyncMessage.scopeIndex] {
if let rootModel = scope.rootModel {
if let definiteRootModelObject = scope.root {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, let's keep it as rootModel

if scopeSyncMessage.syncFragments.count > 0 {
scope.startApplyingRemote()
scope.applySyncFragments(scopeSyncMessage.syncFragments)
Expand Down
8 changes: 4 additions & 4 deletions Jetstream/SyncFragment.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,15 +235,15 @@ public class SyncFragment: Equatable {
}

func applyChangesToScope(scope: Scope, applyDefaults: Bool = false) {
if (scope.rootModel == nil) {
if scope.root == nil {
return
}

switch type {
case .Root:
if let definiteRootModel = scope.rootModel {
applyPropertiesToModelObject(definiteRootModel, scope: scope, applyDefaults: applyDefaults)
scope.updateUUIDForModel(definiteRootModel, uuid: self.objectUUID)
if let definiteRootModelObject = scope.root {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we rename this, too?

applyPropertiesToModelObject(definiteRootModelObject, scope: scope, applyDefaults: applyDefaults)
scope.updateUUIDForModel(definiteRootModelObject, uuid: self.objectUUID)
}
case .Change:
if let modelObject = scope.getObjectById(objectUUID) {
Expand Down
29 changes: 17 additions & 12 deletions JetstreamDemos/JetstreamDemos/ShapesDemoViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import UIKit
import Jetstream

class ShapesDemoViewController: UIViewController, NSURLConnectionDataDelegate {
var scope = Scope(name: "ShapesDemo")
var scope = Scope(name: "Canvas")
var canvas = Canvas()

var client: Client?
Expand All @@ -23,8 +23,8 @@ class ShapesDemoViewController: UIViewController, NSURLConnectionDataDelegate {

let tapRecognizer = UITapGestureRecognizer(target: self, action: Selector("handleTap:"))
self.view.addGestureRecognizer(tapRecognizer)
canvas.setScopeAndMakeRootModel(scope)

scope.root = canvas
canvas.observeCollectionAdd(self, key: "shapes") { (element: Shape) in
let shapeView = ShapeView(shape: element)
self.view.addSubview(shapeView)
Expand All @@ -47,15 +47,20 @@ class ShapesDemoViewController: UIViewController, NSURLConnectionDataDelegate {
let transportAdapter = WebsocketTransportAdapter(options: WebsocketConnectionOptions(url: NSURL(string: "ws://" + host + ":3000")!))
client = Client(transportAdapter: transportAdapter)
client?.connect()
client?.onSession.listenOnce(self) { (session) in
self.session = session
let scope = self.scope
session.fetch(scope) { (error) in
if error != nil {
self.alertError("Error fetching scope", message: "\(error)")
} else {
self.hideLoader()
}
client?.onSession.listenOnce(self) { [weak self] session in
Copy link
Contributor

Choose a reason for hiding this comment

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

You can do a unowned self here (and then you don't need to do the let definite dance). Signals guarantee that the listener is still alive when it fires.

if let definiteSelf = self {
definiteSelf.sessionDidStart(session)
}
}
}

func sessionDidStart(session: Session) {
self.session = session
session.fetch(scope) { (error) in
if error != nil {
self.alertError("Error fetching scope", message: "\(error)")
} else {
self.hideLoader()
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ Once you've defined your model classes, instante a canvas and mark it as a scope

```swift
var canvas = Canvas()
canvas.isScopeRoot = true

var scope = Scope(name: "Canvas")
scope.root = canvas
```
This will create a new scope and assign `canvas` as the root of the scope. The root object or any branches or leafs attached now belong to the scope. This lets you start observing changes happening to any models that have been attached to the tree:

Expand Down