Repository: basho/riak-nodejs-client (js)

Issue:
Riak Search, empty string "" becomes integer 0. [JIRA: CLIENTS-881]
From the code it looks like isNaN is used to check the "" empty string, https://github.com/basho/riak-nodejs-client/blob/master/lib/commands/yokozuna/search.js#L98

But `isNaN("");        // false: the empty string is converted to 0 which is not NaN.` from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/isNaN.



Pull request description:
READY: fix conversion from YZ search results
Fixes #160 (CLIENTS-881)

Patch:
diff --git a/lib/commands/ts/utils.js b/lib/commands/ts/utils.js
index 959871c..b33c062 100644
--- a/lib/commands/ts/utils.js
+++ b/lib/commands/ts/utils.js
@@ -9,18 +9,6 @@ var rpb = require('../../protobuf/riakprotobuf');
 var TsCell = rpb.getProtoFor('TsCell');
 var TsColumnType = rpb.getProtoFor('TsColumnType');
 
-function isFloat(n) {
-    return typeof n === 'number' && n % 1 !== 0;
-}
-
-function isInteger(n) {
-    if (n instanceof Long) {
-        return true;
-    } else {
-        return typeof n === 'number' && n % 1 === 0;
-    }
-}
-
 function isBoolean(v) {
     return (typeof v === 'boolean' || v instanceof Boolean);
 }
@@ -63,9 +51,9 @@ function convertToTsCells(row) {
                 tsc.setBooleanValue(cell);
             } else if (utils.isString(cell)) { // NB: should come before isInteger / isFloat
                 tsc.setVarcharValue(new Buffer(cell, 'utf8'));
-            } else if (isInteger(cell)) {
+            } else if (utils.isInteger(cell)) {
                 tsc.setSint64Value(cell);
-            } else if (isFloat(cell)) {
+            } else if (utils.isFloat(cell)) {
                 tsc.setDoubleValue(cell);
             } else {
                 var msg = util.format("could not serialize: %s, type: %s",
diff --git a/lib/commands/yokozuna/search.js b/lib/commands/yokozuna/search.js
index d52d3ef..3716b7a 100644
--- a/lib/commands/yokozuna/search.js
+++ b/lib/commands/yokozuna/search.js
@@ -4,6 +4,8 @@ var CommandBase = require('../commandbase');
 var inherits = require('util').inherits;
 var Joi = require('joi');
 
+var utils = require('../../utils');
+
 /**
  * Provides the (Yokozuna) Search class, its builder, and its response.
  * @module YZ
@@ -95,9 +97,10 @@ Search.prototype.onSuccess = function(rpbSearchQueryResp) {
             var value = rpbSearchQueryResp.docs[i].fields[j].value.toString('utf8');
             // Search and MapReduce are effectively broken with the PB API because
             // everything is returned as a string.
-            if (!isNaN(value)) {
-                // it's really a number.
-                value = +value;
+            var valAsNum = utils.maybeIsNumber(value);
+            if (valAsNum) {
+                // it's a Long or number.
+                value = valAsNum;
             } else if (value === 'null') {
                 // It's actually null
                 value = null;
diff --git a/lib/utils.js b/lib/utils.js
index cbaf475..64a263d 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -1,5 +1,7 @@
 'use strict';
 
+var Long = require('long');
+
 function isString(v) {
     return (typeof v === 'string' || v instanceof String);
 }
@@ -8,5 +10,31 @@ function isNullOrUndefined(v) {
     return (v === null || typeof v === 'undefined');
 }
 
+function maybeIsNumber(n) {
+    if (n instanceof Long) {
+        return n;
+    }
+    var asNum = parseFloat(n);
+    if (isNaN(asNum)) {
+        return undefined;
+    }
+    return asNum;
+}
+
+function isInteger(n) {
+    if (n instanceof Long) {
+        return true;
+    } else {
+        return typeof n === 'number' && n % 1 === 0;
+    }
+}
+
+function isFloat(n) {
+    return typeof n === 'number' && n % 1 !== 0;
+}
+
 module.exports.isString = isString;
 module.exports.isNullOrUndefined = isNullOrUndefined;
+module.exports.maybeIsNumber = maybeIsNumber;
+module.exports.isInteger = isInteger;
+module.exports.isFloat = isFloat;


Test patch:
diff --git a/test/unit/yokozuna/search.js b/test/unit/yokozuna/search.js
index 523b863..9b1e315 100644
--- a/test/unit/yokozuna/search.js
+++ b/test/unit/yokozuna/search.js
@@ -9,8 +9,7 @@ var assert = require('assert');
 
 describe('Search', function() {
     describe('Build', function() {
-        it('should build a RpbSearchQueryReq correctly', function(done) {
-        
+        it('builds-RpbSearchQueryReq', function(done) {
             var search = new Search.Builder()
                     .withIndexName('indexName')
                     .withQuery('some solr query')
@@ -24,9 +23,9 @@ describe('Search', function() {
                     .withPresort('key')
                     .withCallback(function(){})
                     .build();
-            
+
             var protobuf = search.constructPbRequest();
-            
+
             assert.equal(protobuf.index.toString('utf8'), 'indexName');
             assert.equal(protobuf.q.toString('utf8'), 'some solr query');
             assert.equal(protobuf.rows, 20);
@@ -40,85 +39,93 @@ describe('Search', function() {
             assert.equal(protobuf.fl[1].toString('utf8'), 'field2');
             assert.equal(protobuf.presort.toString('utf8'), 'key');
             done();
-            
-        
         });
-        
-        it('should take a RpbSearchQueryResp and call the users callback with the response', function(done) {
-            
+
+        it('processes-RpbSearchQueryResp', function(done) {
             var resp = new RpbSearchQueryResp();
-            
+
             var doc = new RpbSearchDoc();
-            // The PB API is broken in that it returns everything as strings. The 
+            // The PB API is broken in that it returns everything as strings. The
             // search command should convert boolean and numeric values properly
             var pair = new RpbPair();
             pair.key = new Buffer('leader_b');
             pair.value = new Buffer('true');
             doc.fields.push(pair);
-            
+
+            pair = new RpbPair();
+            pair.key = new Buffer('leader2_b');
+            pair.value = new Buffer('false');
+            doc.fields.push(pair);
+
             pair = new RpbPair();
             pair.key = new Buffer('age_i');
             pair.value = new Buffer('30');
             doc.fields.push(pair);
-            
+
+            pair = new RpbPair();
+            pair.key = new Buffer('age_f');
+            pair.value = new Buffer('12.34');
+            doc.fields.push(pair);
+
             pair = new RpbPair();
             pair.key = new Buffer('_yz_id');
             pair.value = new Buffer('default_cats_liono_37');
             doc.fields.push(pair);
-            
+
             pair = new RpbPair();
             pair.key = new Buffer('nullValue');
             pair.value = new Buffer('null');
             doc.fields.push(pair);
-            
+
+            pair = new RpbPair();
+            pair.key = new Buffer('emptyString');
+            pair.value = new Buffer('');
+            doc.fields.push(pair);
+
             resp.docs.push(doc);
             resp.max_score = 1.123;
             resp.num_found = 1;
-            
+
             var callback = function(err, response) {
-              
                 assert.equal(response.numFound, 1);
                 assert.equal(response.maxScore, 1.123);
                 assert.equal(response.docs.length, 1);
                 var doc = response.docs[0];
-                assert.equal(doc.leader_b, true); // converted to boolean
-                assert.equal(doc.age_i, 30); // converted to number
-                assert.equal(doc.nullValue, null); // converted to null
-                assert.equal(doc._yz_id, 'default_cats_liono_37'); // left as string
+                assert.strictEqual(doc.leader_b, true);
+                assert.strictEqual(doc.leader2_b, false);
+                assert.strictEqual(doc.age_i, 30);
+                assert.strictEqual(doc.age_f, 12.34);
+                assert.strictEqual(doc.nullValue, null);
+                assert.strictEqual(doc.emptyString, '');
+                assert.strictEqual(doc._yz_id, 'default_cats_liono_37');
                 done();
             };
-            
+
             var search = new Search.Builder()
                     .withIndexName('indexName')
                     .withQuery('some solr query')
                     .withCallback(callback)
                     .build();
-            
             search.onSuccess(resp);
-            
         });
-        
-        it ('should take a RpbErrorResp and call the users callback with the error message', function(done) {
+
+        it ('processes-RpbErrorResp', function(done) {
            var rpbErrorResp = new RpbErrorResp();
            rpbErrorResp.setErrmsg(new Buffer('this is an error'));
-           
+
            var callback = function(err, response) {
                if (err) {
                    assert.equal(err,'this is an error');
                    done();
                }
            };
-           
+
            var search = new Search.Builder()
                     .withIndexName('indexName')
                     .withQuery('some solr query')
                     .withCallback(callback)
                     .build();
-            
            search.onRiakError(rpbErrorResp);
-           
-           
        });
-        
     });
 });