From f0a34bf0083dbff8967df80e8ae1a9d6a85a3962 Mon Sep 17 00:00:00 2001
From: Anatoliy Chakkaev <anatoliy.chakkaev@flatsoft.com>
Date: Wed, 18 Jan 2012 22:49:30 +0400
Subject: [PATCH] Redis-adapter: test sorting, support alpha-sort, fix
 destroyAll issue

---
 lib/adapters/redis.js | 30 +++++++++++++++-------
 test/common_test.js   | 58 +++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 77 insertions(+), 11 deletions(-)

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);