EAGLESIX-2651: Value: better sync w/ 2.6.5 changes

* fix bugs and fill in a few missing type handlers
* doc any other missing type handlers
* fix inconsistencies with original implementation

lib/pipeline/Value.js

@@ -9,9 +9,9 @@
  **/
 var Value = module.exports = function Value(){
 	if(this.constructor === Value) throw new Error("Never create instances of this! Use the static helpers only.");
-}, klass = Value, base = Object, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
+}, klass = Value;
 
-var Document;  // loaded lazily below //TODO: a dirty hack; need to investigate and clean up
+var Document; // loaded lazily below //TODO: a dirty hack; need to investigate and clean up
 
 //SKIPPED: ValueStorage -- probably not required; use JSON?
 //SKIPPED: createIntOrLong -- not required; use Number
@@ -22,7 +22,7 @@ var Document;  // loaded lazily below //TODO: a dirty hack; need to investigate
 //SKIPPED: addToBsonArray -- not required; use arr.push(<val>)
 
 /** Coerce a value to a bool using BSONElement::trueValue() rules.
- * Some types unsupported.  SERVER-6120
+ * Some types unsupported. SERVER-6120
  * @method coerceToBool
  * @static
  */
@@ -31,9 +31,10 @@ klass.coerceToBool = function coerceToBool(value) {
 	return !!value;	// including null or undefined
 };
 
-/** Coercion operators to extract values with fuzzy type logic.
- *  These currently assert if called on an unconvertible type.
- *  TODO: decided how to handle unsupported types.
+/**
+ * Coercion operators to extract values with fuzzy type logic.
+ * These currently assert if called on an unconvertible type.
+ * TODO: decided how to handle unsupported types.
  */
 klass.coerceToWholeNumber = function coerceToInt(value) {
 	return klass.coerceToNumber(value) | 0;
@@ -42,36 +43,29 @@ klass.coerceToInt = klass.coerceToWholeNumber;
 klass.coerceToLong = klass.coerceToWholeNumber;
 klass.coerceToNumber = function coerceToNumber(value) {
 	if (value === null) return 0;
-	switch (typeof(value)) {
-	case "undefined":
-		return 0;
-	case "number":
-		return value;
-	case "object":
-		switch (value.constructor.name) {
-			case "Long":
-				return parseInt(value.toString(), 10);
-			case "Double":
-				return parseFloat(value.value, 10);
-			default:
-				throw new Error("can't convert from BSON type " + value.constructor.name + " to int; codes 16003, 16004, 16005");
-		}
-		return value;
-	default:
-		throw new Error("can't convert from BSON type " + typeof(value) + " to int; codes 16003, 16004, 16005");
+	switch (Value.getType(value)) {
+		case "undefined":
+			return 0;
+		case "number":
+			return value;
+		case "Long":
+			return parseInt(value.toString(), 10);
+		case "Double":
+			return parseFloat(value.value, 10);
+		default:
+			throw new Error("can't convert from BSON type " + Value.getType(value) + " to int; codes 16003, 16004, 16005");
 	}
 };
 klass.coerceToDouble = klass.coerceToNumber;
 klass.coerceToDate = function coerceToDate(value) {
 	if (value instanceof Date) return value;
-	throw new Error("can't convert from BSON type " + typeof(value) + " to Date; uassert code 16006");
+	throw new Error("can't convert from BSON type " + Value.getType(value) + " to Date; uassert code 16006");
 };
 //SKIPPED: coerceToTimeT -- not required; just use Date
 //SKIPPED: coerceToTm -- not required; just use Date
 //SKIPPED: tmToISODateString -- not required; just use Date
 klass.coerceToString = function coerceToString(value) {
-	var type = typeof(value);
-	if (type === "object") type = value === null ? "null" : value.constructor.name;
+	var type = Value.getType(value);
 	switch (type) {
 		//TODO: BSON numbers?
 		case "number":
@@ -91,7 +85,7 @@ klass.coerceToString = function coerceToString(value) {
 			return "";
 
 		default:
-			throw new Error("can't convert from BSON type " + typeof(value) + " to String; uassert code 16007");
+			throw new Error("can't convert from BSON type " + Value.getType(value) + " to String; uassert code 16007");
 	}
 };
 //SKIPPED: coerceToTimestamp
@@ -101,8 +95,16 @@ klass.coerceToString = function coerceToString(value) {
  * @method cmp
  * @static
  */
-klass.cmp = function cmp(l, r){
-	return l < r ? -1 : l > r ? 1 : 0;
+var cmp = klass.cmp = function cmp(left, right){
+	// The following is lifted directly from compareElementValues
+	// to ensure identical handling of NaN
+	if (left < right)
+		return -1;
+	if (left === right)
+		return 0;
+	if (isNaN(left))
+		return isNaN(right) ? 0 : -1;
+	return 1;
 };
 
 /** Compare two Values.
@@ -112,74 +114,84 @@ klass.cmp = function cmp(l, r){
  * Warning: may return values other than -1, 0, or 1
  */
 klass.compare = function compare(l, r) {
-	//NOTE: deviation from mongo code: we have to do some coercing for null "types" because of javascript
-	var lt = l === null ? "null" : typeof(l),
-		rt = r === null ? "null" : typeof(r),
+	var lType = Value.getType(l),
+		rType = Value.getType(r),
 		ret;
 
-	// NOTE: deviation from mongo code: javascript types do not work quite the same, so for proper results we always canonicalize, and we don't need the "speed" hack
-	ret = (klass.cmp(klass.canonicalize(l), klass.canonicalize(r)));
+	ret = lType === rType ?
+	 	0 // fast-path common case
+		: cmp(klass.canonicalize(l), klass.canonicalize(r));
 
-	if(ret !== 0) return ret;
+	if(ret !== 0)
+		return ret;
 
-	// Numbers
-	if (lt === "number" && rt === "number"){
-		//NOTE: deviation from Mongo code: they handle NaN a bit differently
-		if (isNaN(l)) return isNaN(r) ? 0 : -1;
-		if (isNaN(r)) return 1;
-		return klass.cmp(l,r);
-	}
-	// Compare MinKey and MaxKey cases
-	if (l instanceof Object && ["MinKey", "MaxKey"].indexOf(l.constructor.name) !== -1) {
-		if (l.constructor.name === r.constructor.name) {
-			return 0;
-		} else if (l.constructor.name === "MinKey") {
-			return -1;
-		} else {
-			return 1; // Must be MaxKey, which is greater than everything but MaxKey (which r cannot be)
-		}
-	}
-	// hack: These should really get converted to their BSON type ids and then compared, we use int vs object in queries
-	if (lt === "number" && rt === "object"){
-		return -1;
-	} else if (lt === "object" && rt === "number") {
-		return 1;
-	}
 	// CW TODO for now, only compare like values
-	if (lt !== rt) throw new Error("can't compare values of BSON types [" + lt + " " + l.constructor.name + "] and [" + rt + ":" + r.constructor.name + "]; code 16016");
-	// Compare everything else
-	switch (lt) {
-	case "number":
-		throw new Error("number types should have been handled earlier!");
-	case "string":
-		return klass.cmp(l, r);
-	case "boolean":
-		return l === r ? 0 : l ? 1 : -1;
-	case "undefined": //NOTE: deviation from mongo code: we are comparing null to null or undefined to undefined (otherwise the ret stuff above would have caught it)
-	case "null":
-		return 0;
-	case "object":
-		if (l instanceof Array) {
-			for (var i = 0, ll = l.length, rl = r.length; true ; ++i) {
-				if (i > ll) {
-					if (i > rl) return 0; // arrays are same length
-					return -1; // left array is shorter
-				}
-				if (i > rl) return 1; // right array is shorter
-				var cmp = Value.compare(l[i], r[i]);
-				if (cmp !== 0) return cmp;
+	if (lType !== rType)
+		throw new Error("can't compare values of BSON types [" + lType + "] and [" + rType + "]; code 16016");
+
+	switch (lType) {
+		// Order of types is the same as in compareElementValues() to make it easier to verify
+
+		// These are valueless types
+		//SKIPPED: case "EOO":
+		case "undefined":
+		case "null":
+		//SKIPPED: case "jstNULL":
+		case "MaxKey":
+		case "MinKey":
+			return ret;
+
+		case "boolean":
+			return l - r;
+
+		// WARNING: Timestamp and Date have same canonical type, but compare differently.
+		// Maintaining behavior from normal BSON.
+		//SKIPPED: case "Timestamp": //unsigned-----//TODO: handle case for bson.Timestamp()
+		case "Date": // signed
+			return cmp(l.getTime(), r.getTime());
+
+        // Numbers should compare by equivalence even if different types
+		case "number":
+			return cmp(l, r);
+
+        //SKIPPED: case "jstOID":----//TODO: handle case for bson.ObjectID()
+
+        case "Code":
+        case "Symbol":
+        case "string":
+			l = String(l);
+			r = String(r);
+			return l < r ? -1 : l > r ? 1 : 0;
+
+		case "Object":
+			if (Document === undefined) Document = require("./Document");	//TODO: a dirty hack; need to investigate and clean up
+			return Document.compare(l, r);
+
+		case "Array":
+			var lArr = l,
+				rArr = r;
+
+			var elems = Math.min(lArr.length, rArr.length);
+			for (var i = 0; i < elems; i++) {
+				// compare the two corresponding elements
+				ret = Value.compare(lArr[i], rArr[i]);
+				if (ret !== 0)
+					return ret;
 			}
+			// if we get here we are either equal or one is prefix of the other
+			return cmp(lArr.length, rArr.length);
 
-			throw new Error("logic error in Value.compare for Array types!");
-		}
-		if (l instanceof Date) return klass.cmp(l,r);
-		if (l instanceof RegExp) return klass.cmp(l,r);
-		if (Document === undefined) Document = require("./Document");	//TODO: a dirty hack; need to investigate and clean up
-		return Document.compare(l, r);
-	default:
-		throw new Error("unhandled left hand type:" + lt);
-	}
+		//SKIPPED: case "DBRef":-----//TODO: handle case for bson.DBRef()
+		//SKIPPED: case "BinData":-----//TODO: handle case for bson.BinData()
 
+		case "RegExp": // same as String in this impl but keeping order same as compareElementValues
+			l = String(l);
+			r = String(r);
+			return l < r ? -1 : l > r ? 1 : 0;
+
+		//SKIPPED: case "CodeWScope":-----//TODO: handle case for bson.CodeWScope()
+	}
+	throw new Error("Assertion failure");
 };
 
 //SKIPPED: hash_combine
@@ -201,22 +213,25 @@ klass.consume = function consume(consumed) {
 };
 
 //NOTE: DEVIATION FROM MONGO: many of these do not apply or are inlined (code where relevant)
-// missing(val):  val === undefined
-// nullish(val):  val === null || val === undefined
-// numeric(val):  typeof val === "number"
+// missing(val): val === undefined
+// nullish(val): val === null || val === undefined
+// numeric(val): typeof val === "number"
 klass.getType = function getType(v) {
 	var t = typeof v;
-	if (t === "object") t = (v === null ? "null" : v.constructor.name || t);
-	return t;
+	if (t !== "object")
+		return t;
+	if (v === null)
+		return "null";
+	return v.constructor.name || t;
 };
 // getArrayLength(arr): arr.length
-// getString(val): val.toString()   //NOTE: same for getStringData(val) I think
+// getString(val): val.toString() //NOTE: same for getStringData(val) I think
 // getOid
 // getBool
 // getDate
 // getTimestamp
-// getRegex(re):  re.source
-// getRegexFlags(re):  re.toString().slice(-re.toString().lastIndexOf('/') + 2)
+// getRegex(re): re.source
+// getRegexFlags(re): re.toString().slice(-re.toString().lastIndexOf('/') + 2)
 // getSymbol
 // getCode
 // getInt
@@ -225,8 +240,7 @@ klass.getType = function getType(v) {
 
 // from bsontypes
 klass.canonicalize = function canonicalize(x) {
-	var xType = typeof(x);
-	if (xType === "object") xType = x === null ? "null" : x.constructor.name;
+	var xType = Value.getType(x);
 	switch (xType) {
 		case "MinKey":
 			return -1;