Permalink
Browse files

Implement by property lookup, fix tests

  • Loading branch information...
1 parent 67f4c0b commit 1100bfbd5911a26939e361726f91a8dd40032513 @stefanw stefanw committed May 14, 2012
Showing with 198 additions and 56 deletions.
  1. +105 −48 lib/backbone-redis.js
  2. +93 −8 test/backbone-redis.test.js
View
@@ -27,64 +27,81 @@ module.exports = function(port, host, options) {
}
function store(id, model, options) {
+ // TODO: determine if a Lua script makes sense instead of multi
modify(id, model, function(err, rev) {
if (err) return options.error(err);
- var multi = db.multi();
- // Store the actual data.
- // TODO: When this contains references to other models,
- // we should just store the model ID
- // TODO: Handle member collections
var data = model.toJSON();
if (typeof data.id !== 'undefined' && data.id !== id) {
// TODO: Enable this by creating a copy, then deleting the current one.
return options.error(new Error('Changing the ID of an existing model is not possible'));
}
- data.id = id;
- data._rev = rev;
- var args = [ model.name + ':' + id ];
- for (var prop in data) {
- args.push(prop, JSON.stringify(data[prop]));
- }
- multi.hmset.apply(multi, args);
-
- // Store by-property lookups
- // TODO: By-property lookup for members which are collections
- // TODO: By-property lookup for string members
- // TODO: When adding these keys, also remove them again on update/destroy!
- // for (var prop in data) {
- // if (typeof data[prop] === 'number') {
- // multi.zadd(model.name + '#' + prop, data[prop], id);
- // }
- // }
-
- // Add this model to the set of models of this type if it's the
- // first time we save this model.
- if (rev === 1) {
- multi.sadd(model.name, id);
- }
+ var execMulti = function(err, previous) {
+ if (err) return options.error(err);
- // Commit everything to the database.
- multi.exec(function(err) {
- if (err) {
- options.error(err);
- } else if (rev === 1) {
- // When creating this model, we also need to set the ID.
- options.success({ id: id, _rev: rev })
- } else {
- // Otherwise, just return the updated revision number.
- options.success({ _rev: rev })
+ var multi = db.multi();
+ // Store the actual data.
+ // TODO: When this contains references to other models,
+ // we should just store the model ID
+ // TODO: Handle member collections
+
+ data.id = id;
+ data._rev = rev;
+ var args = [ model.name + ':' + id ];
+ for (var prop in data) {
+ args.push(prop, JSON.stringify(data[prop]));
}
- });
+ multi.hmset.apply(multi, args);
+
+ // Store by-property lookups
+ // as set with key model:property:value = id
+ var lookup;
+ if (previous) {
+ for (lookup in model.lookups) {
+ if (previous[lookup] !== data[lookup]) {
+ multi.srem(model.name + ':' + lookup + ':' + JSON.stringify(previous[lookup]), id);
+ }
+ }
+ }
+ for (lookup in model.lookups) {
+ multi.sadd(model.name + ':' + lookup + ':' + JSON.stringify(data[lookup]), id);
+ }
+
+ // TODO: By-property lookup for members which are collections
+
+ // Add this model to the set of models of this type if it's the
+ // first time we save this model.
+ if (rev === 1) {
+ multi.sadd(model.name, id);
+ }
+
+ // Commit everything to the database.
+ multi.exec(function(err) {
+ if (err) {
+ options.error(err);
+ } else if (rev === 1) {
+ // When creating this model, we also need to set the ID.
+ options.success({ id: id, _rev: rev });
+ } else {
+ // Otherwise, just return the updated revision number.
+ options.success({ _rev: rev });
+ }
+ });
+ };
+ if (rev === 1) {
+ return execMulti(null);
+ } else {
+ return get(model.name, id, execMulti);
+ }
});
}
function get(type, id, done) {
db.hgetall(type + ':' + id, function(err, data) {
if (err) return done(err);
- if (!data) return done(new Error('Model does not exist'));
+ if (!data) return done(new Error('Model does not exist: ' + type + ':' + id));
for (var key in data) {
data[key] = JSON.parse(data[key]);
@@ -106,22 +123,53 @@ module.exports = function(port, host, options) {
});
}
+ function retrieveByProperty(type, collection, key, value, options, done){
+ var lookup = type + ':' + key + ':' + JSON.stringify(value);
+ db.smembers(lookup, function(err, members) {
+ if (err) return options.error(err);
+
+ async.map(members, function(id, done) {
+ get(type, id, done);
+ }, function(err, results) {
+ if (err) return options.error(err);
+ options.success(results);
+ });
+ });
+ }
+
function retrieve(id, model, options) {
// Retrieves all values in that hash.
get(model.name, id, function(err, data) {
- err ? options.error(err) : options.success(data);
+ if(err) {
+ options.error(err);
+ } else {
+ options.success(data);
+ }
});
}
function destroy(id, model, options) {
modify(id, model, function(err, rev) {
- var multi = db.multi();
- multi.del(model.name + ':' + id);
- multi.srem(model.name, id);
+ get(model.name, id, function(err, data) {
+ if (err) return options.error(err);
+
+ var multi = db.multi();
+ multi.del(model.name + ':' + id);
+ multi.srem(model.name, id);
+
+ for (var lookup in model.lookups) {
+ data[lookup] = JSON.stringify(data[lookup]);
+ multi.srem(model.name + ':' + lookup + ':' + data[lookup], id);
+ }
- // Commit everything to the database.
- multi.exec(function(err) {
- err ? options.error(err) : options.success({});
+ // Commit everything to the database.
+ multi.exec(function(err) {
+ if(err) {
+ options.error(err);
+ } else {
+ options.success({});
+ }
+ });
});
});
}
@@ -141,7 +189,16 @@ module.exports = function(port, host, options) {
break;
case 'read':
if (model instanceof Backbone.Collection) {
- retrieveAll(model.model.prototype.name, model, options);
+ if (options.lookup) {
+ if (typeof model.model.prototype.lookups[options.lookup.key] === 'undefined') {
+ options.error(new Error('Cannot lookup by property "' + options.lookup.key + '"'));
+ break;
+ }
+ retrieveByProperty(model.model.prototype.name, model,
+ options.lookup.key, options.lookup.value, options);
+ } else {
+ retrieveAll(model.model.prototype.name, model, options);
+ }
} else {
// TODO: Handle loading models based on other parameters than id.
if (!model.id) {
@@ -69,7 +69,7 @@ describe('CRUD workflow', function() {
it('should fail to load the deleted model', function(done) {
new Customer({ id: customer.id }).fetch({
error: function(model, err) {
- assert.equal(err.message, 'Model does not exist');
+ assert.equal(err.message, 'Model does not exist: ' + model.name + ':' + customer.id);
done();
},
success: function(model) {
@@ -78,12 +78,6 @@ describe('CRUD workflow', function() {
});
});
- after(function(done) {
- customer.destroy({
- error: error,
- success: done.bind(null, null)
- });
- });
}); // end "CRUD workflow"
@@ -157,7 +151,98 @@ describe('collections', function() {
});
}, done);
});
-});
+}); // end "collections"
+
+
+describe('lookup by property', function() {
+ var data = [
+ new Customer({ name: "Ferdinand Müller", priority: 1, address: 'Berlin' }),
+ new Customer({ name: "Anke Müller", priority: 1, address: 'Berlin' }),
+ new Customer({ name: "Peter Müller", priority: 1, address: 'Dresden' })
+ ];
+
+ var customers = new Customers;
+
+ it('should save the entire collection', function(done) {
+ async.forEach(data, function(model, done) {
+ model.save(null, {
+ error: function(model, err) { done(err); },
+ success: function(model, resp) { done(); }
+ });
+ }, done);
+ });
+
+ it('should retrieve two customers', function(done) {
+ customers.fetch({
+ lookup: {
+ key: "address",
+ value: "Berlin"
+ },
+ error: error,
+ success: function(collection, resp) {
+ // TODO: Be sure about collection order
+ assert.deepEqual(
+ collection.toJSON(),
+ data.slice(0, 2).map(function(a) { return a.toJSON(); })
+ );
+ done();
+ }
+ });
+ });
+ it('should update lookups', function(done) {
+ var firstCustomer = customers.at(0);
+
+ firstCustomer.save({'address': 'Dortmund'}, {
+ success: function(){
+ customers.fetch({
+ lookup: {
+ key: "address",
+ value: "Dortmund"
+ },
+ error: error,
+ success: function(collection, resp){
+ assert.deepEqual(
+ collection.toJSON(),
+ [firstCustomer.toJSON()]
+ );
+ done();
+ }
+ });
+ },
+ error: error
+ });
+ });
+
+ it('should fail to retrieve', function(done) {
+ customers.fetch({
+ lookup: {
+ key: "name",
+ value: "Ferdinand Müller"
+ },
+ error: function(model, err) {
+ assert.equal(err.message, 'Cannot lookup by property "name"');
+ done();
+ },
+ success: error
+ });
+ });
+
+ after(function(done) {
+ customers.fetch({
+ error: error,
+ success: function(collection, resp){
+ // clone collection.models: can't destroy models while
+ // iterating over collection!
+ async.forEach(_.clone(collection.models), function(model, done) {
+ model.destroy({
+ error: function(model, err) { done(err); },
+ success: function(model, resp) { done(); }
+ });
+ }, done);
+ }
+ });
+ });
+}); // end "lookup by property"
}); // end "backbone redis"

0 comments on commit 1100bfb

Please sign in to comment.