diff --git a/lib/adapters/redis.js b/lib/adapters/redis.js index e64fa3a7..4ecfac8d 100644 --- a/lib/adapters/redis.js +++ b/lib/adapters/redis.js @@ -82,7 +82,7 @@ BridgeToRedis.prototype.create = function (model, data, callback) { }); // push the id to the list of user ids for sorting - log('SADD s:' + model + ':' + data.id + ' ...'); + log('SADD s:' + model + ' ' + data.id); this.client.sadd("s:" + model, data.id); }.bind(this)); @@ -142,6 +142,8 @@ BridgeToRedis.prototype.all = function all(model, filter, callback) { var cmd; var that = this; var sortCmd = []; + var props = this._models[model].properties; + var allNumeric = true; // ORDER if (filter && filter.order){ @@ -149,7 +151,10 @@ BridgeToRedis.prototype.all = function all(model, filter, callback) { if (typeof filter.order === "string"){ orders = [filter.order]; } - orders.forEach( function (key){ + orders.forEach(function (key){ + if (props[key].type.name !== 'Number' && props[key].type.name !== 'Date') { + allNumeric = false; + } sortCmd.push("BY", model + ":*->" + key); }); } @@ -159,24 +164,29 @@ BridgeToRedis.prototype.all = function all(model, filter, callback) { sortCmd.push("LIMIT", from, to); } + // we need ALPHA modifier when sorting string values + // the only case it's not required - we sort numbers + // TODO: check if we sort numbers + if (!allNumeric) { + sortCmd.push('ALPHA'); + } + // do we need to sort or to query normally - if(sortCmd.length){ + if (sortCmd.length) { sortCmd.unshift("s:" + model); sortCmd.push("GET", "#"); - cmd = "sort " + sortCmd.join(" "); + cmd = "SORT " + sortCmd.join(" "); sortCmd.push(function(err, ids){ - console.log( ids); if (err) { return callback(err, []); } var keys = ids.map(function (i) { return model + ":" + i; }); - console.log(keys); handleKeys(err, keys); }); - client.sort.apply(client, sortCmd); - }else{ + client.sort.apply(client, sortCmd); + } else { // Do a normal key lookup with possbible indexes var indexes = this.possibleIndexes(model, filter); if (indexes.length) { @@ -244,7 +254,9 @@ BridgeToRedis.prototype.destroyAll = function destroyAll(model, callback) { var t2 = Date.now(); this.client.multi(query).exec(function (err, replies) { this.log(query, t2); - callback(err); + this.client.del('s:' + model, function () { + callback(err); + }); }.bind(this)); }.bind(this)); }; diff --git a/test/common_test.js b/test/common_test.js index 379f8a16..a9da722a 100644 --- a/test/common_test.js +++ b/test/common_test.js @@ -171,7 +171,7 @@ function testOrm(schema) { it('should create object with initial data', function (test) { var title = 'Initial title', - date = new Date; + date = new Date; Post.create({ title: title, @@ -409,7 +409,6 @@ function testOrm(schema) { Post.count(function (err, count) { test.equal(count, 0); test.done(); - process.nextTick(allTestsDone); }); }); }); @@ -424,6 +423,61 @@ function testOrm(schema) { test.done(); }); + it('should handle ORDER clause', function (test) { + var titles = [ 'Title A', 'Title Z', 'Title M', 'Title B' ]; + var dates = [ 5, 9, 0, 17 ]; + titles.forEach(function (t, i) { + Post.create({title: t, date: dates[i]}, done); + }); + + var i = 0; + function done(err, obj) { + if (++i === titles.length) { + doStringTest(); + doNumberTest(); + } + } + + // Post.schema.log = console.log; + + function doStringTest() { + Post.all({order: 'title'}, function (err, posts) { + titles.sort().forEach(function (t, i) { + test.equal(posts[i].title, t); + }); + finished(); + }); + } + + function doNumberTest() { + Post.all({order: 'date'}, function (err, posts) { + dates.sort(numerically).forEach(function (d, i) { + test.equal(posts[i].date, d); + }); + finished(); + }); + } + + var fin = 0; + function finished() { + if (++fin === 2) { + test.done(); + } + } + + // TODO: do mixed test, do real dates tests, ensure that dates stored in UNIX timestamp format + + function numerically(a, b) { + return a - b; + } + + }); + + it('all tests done', function (test) { + test.done(); + process.nextTick(allTestsDone); + }); + function allTestsDone() { schema.disconnect(); console.log('Test done in %dms\n', Date.now() - start);