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

ModelSync.Local implementation (final) - passing on A-grade browsers #1218

Merged
merged 17 commits into from
Sep 27, 2013
Merged
Show file tree
Hide file tree
Changes from 12 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
5 changes: 5 additions & 0 deletions src/app/build.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
"model.js"
]
},
"model-sync-local": {
"jsfiles": [
"model-extensions/model-sync-local.js"
]
},
"model-sync-rest": {
"jsfiles": [
"model-extensions/model-sync-rest.js"
Expand Down
73 changes: 73 additions & 0 deletions src/app/docs/model/index.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -1020,6 +1020,79 @@ If you're sending and receiving content other than JSON, you can override these
For more information on the `Y.ModelSync.REST` extension, refer to its <a href="{{apiDocs}}/classes/ModelSync.REST.html">API docs</a>.
</p>

<h3>Local Storage Synchronization</h3>

<p>
`Y.ModelSync.Local` is an extension which provides a sync implementation through locally stored key value pairs, either through the HTML localStorage API or falling back onto an in-memory cache, that can be mixed into a Model or ModelList subclass.
</p>

<p>
This follows a similar API as `Y.ModelSync.REST`, so in most cases, you'll again only need to provide a value for `root` when sub-classing `Y.Model`.
</p>

```javascript
// Create `Y.User`, a `Y.Model` subclass and mix-in the Local Storage sync layer
Y.User = Y.Base.create('user', Y.Model, [Y.ModelSync.Local], {
// The root key for local storage or the in-memory cache
root: 'users'
});

var existingUser = new Y.User({id: 'users-1'}),
oldUser = new Y.User({id: 'users-2'}),
newUser = new Y.User({name: 'Eric Ferraiuolo'});

// Get the existing user data from: "{'users': 'users-1': { /* data */ }}"
existingUser.load(function () {
Y.log(existingUser.get('name')); // => "Ron Swannson"

// Correct the user's `name` and update the set of key-value pairs
existingUser.set('name', 'Ron Swanson').save();
});

// Destroy the old user data at: "{'users': 'users-2': { /* data */ }}"
oldUser.destroy({remove: true});

// Set the new user data to "{'users'}"
newUser.save(function () {
// The sync layer can return the user data with an `id` assigned.
// By default, this uses the root and Y.guid() internally
Y.log(newUser.get('id'));
// => "users_yui_3_8_0_1_1357185522298_106"
});
```

<p>
As mentioned in the example, the IDs that are automatically generated and used inside of either Local Storage or the in-memory cache are provided by Y.guid() internally. If you would like to use your own method of generating unique IDs with this sync layer, you can override the `generateID` method to do so.
</p>

<p>
For Model Lists, the `root` property is by convention inherited from the `root` property found in the default Model class that can be provided as a configuration parameter. Otherwise, it defaults to an empty string.
</p>

```
Y.User = Y.Base.create('user', Y.Model, [Y.ModelSync.Local], {
root: 'users'
});

Y.Users = Y.Base.create('users', Y.ModelList, [Y.ModelSync.Local], {
// By convention `Y.User`'s `root` will be used for `Y.Users` as well.
model: Y.User
});

var users = new Y.Users();

// Get users list from: {"users": /* data */ }
users.load(function () {
var firstUser = users.item(0);

Y.log(firstUser.get('id'));
// => "users-1"

// Update user data at "users-1"
firstUser.set('name', 'Eric').save();
});
```

<h3>Implementing a Model Sync Layer</h3>

<p>
Expand Down
294 changes: 294 additions & 0 deletions src/app/js/model-extensions/model-sync-local.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,294 @@
/*
An extension which provides a sync implementation through locally stored
key value pairs, either through the HTML localStorage API or falling back
onto an in-memory cache, that can be mixed into a Model or ModelList subclass.

@module app
@submodule model-sync-local
@since @VERSION@
**/

/**
An extension which provides a sync implementation through locally stored
key value pairs, either through the HTML localStorage API or falling back
onto an in-memory cache, that can be mixed into a Model or ModelList subclass.

A group of Models/ModelLists is serialized in localStorage by either its
class name, or a specified 'root' that is provided.

var User = Y.Base.create('user', Y.Model, [Y.ModelSync.Local], {
root: 'user'
});

var Users = Y.Base.create('users', Y.ModelList, [Y.ModelSync.Local], {
model: User,
});

@class ModelSync.Local
@extensionfor Model
@extensionfor ModelList
@since @VERSION@
**/
function LocalSync() {}

/**
Properties that shouldn't be turned into ad-hoc attributes when passed to a
Model or ModelList constructor.

@property _NON_ATTRS_CFG
@type Array
@default ['root'']
Copy link
Contributor

Choose a reason for hiding this comment

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

['root']

@static
@protected
@since @VERSION@
**/
LocalSync._NON_ATTRS_CFG = ['root'];

/**
Feature detection for `localStorage` availability.

@property _hasLocalStorage
@type Boolean
@private
**/
LocalSync._hasLocalStorage = (function () {
try {
return 'localStorage' in Y.config.win && Y.config.win.localStorage !== null;
} catch (e) {
return false;
}
})(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we should advocate the use of feature testing, rather than simply detecting whether localStorage exists or not. For example, we should test if we're actually able to set and remove items from localStorage, like so:

if (localStorage is detected and the following pass):
    localStorage.setItem(..., ...);
    localStorage.removeItem(...);
then localStorage is available and useable

The main reason being there are times when localStorage is detected, but is actually unusable. For example, when browsing in private mode on an iOS device, localStorage is available but cannot be used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This sounds good to me, I'll push this up in just a bit.


/**
Object of key/value pairs to fall back on when localStorage is not available.

@property _data
@type Object
@private
**/
LocalSync._data = {};

LocalSync.prototype = {

// -- Public Methods -------------------------------------------------------

/**
Root used as the key inside of localStorage and/or the in-memory store.

@property root
@type String
@default ""
@since @VERSION@
**/
root: '',

/**
Shortcut for access to localStorage.

@property storage
@type Storage
@default null
@since @VERSION@
**/
storage: null,

// -- Lifecycle Methods -----------------------------------------------------
initializer: function (config) {
var store;

config || (config = {});

if ('root' in config) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this assumes a root property may not ever exist in config's prototype chain, which is a fair assumption. However, I believe it's the wrong kind of test in this case.

config.hasOwnProperty("root");

seems more correct. Then again, since root is expected to be a string of length greater than one:

if (config.root) { ... }

suffices.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, but I think we need to check the prototype, since this is a mix-in for a Y.Base class. In that case, if something along the prototype chain has a root property, then it should use that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. in is the correct choice, then.

this.root = config.root || '';
Copy link
Contributor

Choose a reason for hiding this comment

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

Since root already defaults to an empty string, having || '' is unnecessary. It can be safely removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess this might protect against the case where someone actually does explicitly specify '' in their root, which would return falsey.

Copy link
Member

Choose a reason for hiding this comment

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

I like this pattern of coercing all falsy values to an empty string. This way the rest of the code can always assume root is a string, even if the person had {root: false}.

}

if (this.model && this.model.prototype.root) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a code comment there that this is checking if the sync layer is being applied to a ModelList and if so, trying to look for a root on its model's propto.

this.root = this.model.prototype.root;
Copy link
Member

Choose a reason for hiding this comment

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

config.root should trump a ModelList's model's root. I'd add an !this.root to the if statement.

}

if (LocalSync._hasLocalStorage) {
this.storage = Y.config.win.localStorage;
store = this.storage.getItem(this.root);
} else {
Y.log("Could not access localStorage.", "warn");
}

// Pull in existing data from localStorage, if possible.
// Otherwise, see if there's existing data on the local cache.
if (store) {
LocalSync._data[this.root] = Y.JSON.parse(store);
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to try/catch around Y.JSON.parse() since it can throw?

} else {
LocalSync._data[this.root] = (LocalSync._data[this.root] || {});
Copy link
Member

Choose a reason for hiding this comment

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

Could use conditional assignment here and swap the = and || in this line. Just a style thing…

Or use an else if

}
},

// -- Public Methods -----------------------------------------------------------

/**
Creates a synchronization layer with the localStorage API, if available.
Otherwise, falls back to a in-memory data store.

This method is called internally by load(), save(), and destroy().

@method sync
@param {String} action Sync action to perform. May be one of the following:

* **create**: Store a newly-created model for the first time.
* **read** : Load an existing model.
* **update**: Update an existing model.
* **delete**: Delete an existing model.

@param {Object} [options] Sync options
@param {callback} [callback] Called when the sync operation finishes.
@param {Error|null} callback.err If an error occurred, this parameter will
contain the error. If the sync operation succeeded, _err_ will be
falsy.
Copy link
Contributor

Choose a reason for hiding this comment

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

falsey

@param {Any} [callback.response] The response from our sync. This value will
be passed to the parse() method, which is expected to parse it and
return an attribute hash.
**/
sync: function (action, options, callback) {
options || (options = {});
var response, errorInfo;

try {
switch (action) {
case 'read':
if (this._isYUIModelList) {
response = this._index(options);
} else {
response = this._show(options);
Copy link
Member

Choose a reason for hiding this comment

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

What about switching _show to _read? Or did you do this to disambiguate "read" since it's different for models and lists?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that was the main decision behind that. I could switch it to _read if it's more clear though.

Copy link
Member

Choose a reason for hiding this comment

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

Either way it's fine, it's a protected helper method so since you explicitly called it _show() for a reason, I won't bikeshed it.

}
break;
case 'create':
response = this._create(options);
break;
case 'update':
response = this._update(options);
break;
case 'delete':
response = this._destroy(options);
break;
}
} catch (error) {
errorInfo = error.message;
}

if (response) {
callback(null, response);
} else {
if (errorInfo) {
callback(errorInfo);
} else {
callback("Data not found in LocalStorage");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The above is logically equivalent to the following simplified version:

if (response) {
    callback(null, response);
} else if (errorInfo) {
    callback(errorInfo);
} else {
    callback("Data not found in LocalStage.");
}

},

/**
Generate a random GUID for our Models. This can be overriden if you have
another method of generating different IDs.

@method generateID
@protected
@param {String} pre Optional GUID prefix
**/
generateID: function (pre) {
return Y.guid(pre + '_');
},

// -- Protected Methods ----------------------------------------------------

/**
Sync method correlating to the "read" operation, for a Model List

@method _index
@return {Object[]} Array of objects found for that root key
@protected
@since @VERSION@
**/
_index: function (options) {
return Y.Object.values(LocalSync._data[this.root]);
Copy link
Member

Choose a reason for hiding this comment

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

This is effectively saying that storage order is not significant for a model list's data. If we want to make it significant and consistent, then I don't want us to store it as an object whose key-order is not guaranteed, especially when going to/from JSON.

},

/**
Sync method correlating to the "read" operation, for a Model

@method _show
@return {Object} Object found for that root key and model ID
@protected
@since @VERSION@
**/
_show: function (options) {
return LocalSync._data[this.root][this.get('id')];
},

/**
Sync method correlating to the "create" operation

@method _show
@return {Object} The new object created.
@protected
@since @VERSION@
**/
_create: function (options) {
var hash = this.toJSON();
hash.id = this.generateID(this.root);
LocalSync._data[this.root][hash.id] = hash;

this._save();
return hash;
},

/**
Sync method correlating to the "update" operation

@method _update
@return {Object} The updated object.
@protected
@since @VERSION@
**/
_update: function (options) {
var hash = Y.merge(this.toJSON(), options);
Copy link
Member

Choose a reason for hiding this comment

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

Why are you merging in the options here?

LocalSync._data[this.root][this.get('id')] = hash;

this._save();
return hash;
},

/**
Sync method correlating to the "delete" operation. Deletes the data
from the in-memory object, and saves into localStorage if available.

@method _destroy
@return {Object} The deleted object.
@protected
@since @VERSION@
**/
_destroy: function (options) {
delete LocalSync._data[this.root][this.get('id')];
this._save();
return this.toJSON();
},

/**
Saves the current in-memory store into a localStorage key/value pair
if localStorage is available; otherwise, does nothing.

@method _save
@protected
@since @VERSION@
**/
_save: function () {
if (LocalSync._hasLocalStorage) {
this.storage && this.storage.setItem(
this.root,
Y.JSON.stringify(LocalSync._data[this.root])
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious to know why you're serializing all the objects under this root on every save, instead of only serializing the one model or the entire list on save.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

localStorage is an un-nested string-based key/value mapping. In the system here (and with the TodoMVC spec), each Model/ModelList root provides a single key, and the entire class of objects it represents needs to be stringified into one value.

You could serialize all of the objects into individual key/value pairs, but that would be less performant once you have to access all of the objects at once, which is a pretty common use case.

Copy link
Member

Choose a reason for hiding this comment

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

Okay cool, seems like the right tradeoff to keep the state for the whole root in memory to make loads/reads faster and you have do deal with the key -> string limitation anyways.

What's happens when there's several classes and instances that use the same root, are they both pointing to the same structure in memory, is there a situation where they overwriting each other's operations?

);
}
}
};

// -- Namespace ---------------------------------------------------------------

Y.namespace('ModelSync').Local = LocalSync;
Loading