Refs #3430. Fixed AndMatchExpression test cases. Moved ElementPath iteration to static function.

lib/pipeline/matcher/AllElemMatchOp.js

@@ -152,66 +152,6 @@ proto.init = function init( path ){ //  const StringData& path
 	return s;
 };
 
-
-/**
- *
- * _matches exists because we don't have pathIterators, so we need a recursive function call
- * through the path pieces
- * @method _matches
- * @param doc
- * @param path
- * @param details
- *
- */
-proto._matches = function _matches(doc, path, details){
-	var element, k, item,
-		curr = doc;
-	for (k = 0; k < path.length; k++) {
-		item = curr[path[k]];
-		if (item instanceof Object && item.constructor === Object) {
-			curr = item;
-			continue;
-		} else if (item instanceof Object && item.constructor === Array) {
-			if (k == path.length-1) {
-				curr = item;
-				break; // this is the end of the path, so check this array
-			} else if (!(isNaN(parseInt(path[k+1], 10)))) {
-				curr = item;
-				continue; // the *next* path section is an item in the array so we don't check this whole array
-			}
-			// otherwise, check each item in the array against the rest of the path
-			for(var ii = 0, il = item.length; ii < il; ii++){
-				var subitem = item[ii];
-				if (subitem.constructor !== Object) continue;	// can't look for a subfield in a non-object value.
-				if (this._matches(subitem, path.slice(k), null)) { // check the item against the rest of the path
-					if (details && details.needRecord())
-						details.setElemMatchKey(ii.toString());
-					return true;
-				}
-			}
-			return false; // checked all items in the array and found no matches
-		}
-	}
-
-	// we got the whole path, now check it
-	element = curr;
-	if (!(element instanceof Array))
-		return false;
-
-	//var amIRoot = (element.length === 0);
-
-	if (this._allMatch(element))
-		return true;
-
-	/*
-	if (!amIRoot && details && details.needRecord() {
-		details.setElemMatchKey(element);
-	}
-	*/
-	return false;
-};
-
-
 /**
  *
  * matches checks the input doc against the internal path to see if it is a match
@@ -222,7 +162,24 @@ proto._matches = function _matches(doc, path, details){
  */
 proto.matches = function matches(doc, details){
 	// File: expression_array.cpp lines: 189-198
-	return this._matches(doc, this._elementPath.fieldRef()._array, details);
+	var self = this,
+		checker = function(element) {
+			if (!(element instanceof Array))
+				return false;
+
+			//var amIRoot = (element.length === 0);
+
+			if (self._allMatch(element))
+				return true;
+
+			/*
+			if (!amIRoot && details && details.needRecord() {
+				details.setElemMatchKey(element);
+			}
+			*/
+			return false;
+		};
+	return this._elementPath._matches(doc, details, checker);
 };
 
 /**

lib/pipeline/matcher/AndMatchExpression.js

@@ -12,7 +12,7 @@ var AndMatchExpression = module.exports = function AndMatchExpression(){
 
 
 /**
- * 
+ *
  * This documentation was automatically generated. Please update when you touch this function.
  * @method debugString
  * @param
@@ -23,20 +23,20 @@ proto.debugString = function debugString( level ) { //  StringBuilder& debug, in
 	return this._debugAddSpace(level) + "$and\n" + this._debugList(level);
 };
 
-
 /**
- * 
- * This documentation was automatically generated. Please update when you touch this function.
+ *
+ * matches checks the input doc against the internal path to see if it is a match
  * @method matches
- * @param
+ * @param doc
+ * @param details
  *
  */
-proto.matches = function matches( doc, details ) { //  const MatchableDocument* doc, MatchDetails* details
-// File: expression_tree.cpp lines: 64-72	
+proto.matches = function matches(doc, details) { //  const MatchableDocument* doc, MatchDetails* details
+	// File: expression_tree.cpp lines: 64-72
 	var tChild;
 	for (var i = 0; i < this.numChildren(); i++) {
 		tChild = this.getChild(i);
-		if ( ! tChild.matches(doc,details)) {
+		if (!tChild.matches(doc, details)) {
 			if (details) {
 				details.resetOutput();
 			}
@@ -48,14 +48,14 @@ proto.matches = function matches( doc, details ) { //  const MatchableDocument*
 
 
 /**
- * 
+ *
  * This documentation was automatically generated. Please update when you touch this function.
  * @method matchesSingleElement
  * @param
  *
  */
 proto.matchesSingleElement = function matchesSingleElement( e ){ //  const BSONElement& e
-// File: expression_tree.cpp lines: 75-81
+	// File: expression_tree.cpp lines: 75-81
 	for (var i = 0; i < this.numChildren(); i++) {
 		if (!this.getChild(i).matchesSingleElement(e)) {
 			return false;
@@ -66,7 +66,7 @@ proto.matchesSingleElement = function matchesSingleElement( e ){ //  const BSONE
 
 
 /**
- * 
+ *
  * This documentation was automatically generated. Please update when you touch this function.
  * @method shallowClone
  * @param

lib/pipeline/matcher/ArrayMatchingMatchExpression.js

@@ -59,64 +59,6 @@ proto.initPath = function initPath(path){
 	return status;
 };
 
-/**
- *
- * _matches exists because we don't have pathIterators, so we need a recursive function call
- * through the path pieces
- * @method _matches
- * @param doc
- * @param path
- * @param details
- *
- */
-proto._matches = function _matches(doc, path, details){
-	// File: expression_array.cpp lines: 34-53
-	var element, k, item,
-		curr = doc;
-	for (k = 0; k < path.length; k++) {
-		item = curr[path[k]];
-		if (item instanceof Object && item.constructor === Object) {
-			curr = item;
-			continue;
-		} else if (item instanceof Object && item.constructor === Array) {
-			if (k == path.length-1) {
-				curr = item;
-				break; // this is the end of the path, so check this array
-			} else if (!(isNaN(parseInt(path[k+1], 10)))) {
-				curr = item;
-				continue; // the *next* path section is an item in the array so we don't check this whole array
-			}
-			// otherwise, check each item in the array against the rest of the path
-			for(var ii = 0, il = item.length; ii < il; ii++){
-				var subitem = item[ii];
-				if (subitem.constructor !== Object) continue;	// can't look for a subfield in a non-object value.
-				if (this._matches(subitem, path.slice(k), null)) { // check the item against the rest of the path
-					if (details && details.needRecord())
-						details.setElemMatchKey(ii.toString());
-					return true;
-				}
-			}
-			return false; // checked all items in the array and found no matches
-		}
-	}
-
-	// we got the whole path, now check it
-	element = curr;
-	if (!(element instanceof Array))
-		return false;
-
-	//var amIRoot = (element.length === 0);
-
-	if (!this.matchesArray(element, details))
-		return false;
-
-	/*
-	if (!amIRoot && details && details.needRecord() {
-		details.setElemMatchKey(element);
-	}
-	*/
-	return true;
-};
 
 
 /**
@@ -128,7 +70,25 @@ proto._matches = function _matches(doc, path, details){
  *
  */
 proto.matches = function matches(doc, details){
-	return this._matches(doc, this._elementPath.fieldRef()._array, details);
+	var self = this,
+		checker = function(element) {
+			// we got the whole path, now check it
+			if (!(element instanceof Array))
+				return false;
+
+			//var amIRoot = (element.length === 0);
+
+			if (!self.matchesArray(element, details))
+				return false;
+
+			/*
+			if (!amIRoot && details && details.needRecord() {
+				details.setElemMatchKey(element);
+			}
+			*/
+			return true;
+		};
+	return this._elementPath._matches(doc, details, checker);
 };
 
 /**

lib/pipeline/matcher/CLUDGES

@@ -20,3 +20,4 @@ verify is a macro function that throws an exception if the input is falsey, with
 
 obj.reset( arg ) is replaced with obj = arg; Everything else that happens in reset is manual memory management.
 
+elementPath iteration is now encapsulated inside the elementPath class as a static method. It needs an input function to check the item at the end of the path.

lib/pipeline/matcher/ComparisonMatchExpression.js

@@ -135,7 +135,7 @@ proto.init = function init( path,rhs ) { //  const StringData& path, const BSONE
  *
  */
 proto.matchesSingleElement = function matchesSingleElement( e ){ //  const BSONElement& e
-// File: expression_leaf.cpp lines: 91-132
+	// File: expression_leaf.cpp lines: 91-132
 	if( typeof(e) != typeof(this._rhs) ){
 		if ((e === null || e === undefined) && (this._rhs ===null || this._rhs === undefined)) {
 			return ["EQ","LTE","GTE"].indexOf(this._matchType) != -1;

lib/pipeline/matcher/ElementPath.js

@@ -145,3 +145,61 @@ klass.objAtPath = function objAtPath(doc, path) {
 	var tpath = path.split('.');
 	return klass.objAtPath(doc[tpath[0]],tpath.slice(1).join('.'));
 };
+
+
+/**
+ *
+ * Helper to wrap our path into the static method
+ * @method _matches
+ * @param doc
+ * @param details
+ * @param function checker this function is used to check for a valid item at the end of the path
+ *
+ */
+proto._matches = function _matches(doc, details, checker) {
+	return klass._matches(doc, this._fieldRef._array, details, checker);
+};
+
+/**
+ *
+ * _matches exists because we don't have pathIterators, so we need a recursive function call
+ * through the path pieces
+ * @method _matches
+ * @param doc
+ * @param path
+ * @param details
+ * @param function checker this function is used to check for a valid item at the end of the path
+ *
+ */
+klass._matches = function _matches(doc, path, details, checker){
+	// File: expression_array.cpp lines: 34-53
+	var k, item,
+		curr = doc;
+	for (k = 0; k < path.length; k++) {
+		item = curr[path[k]];
+		if (item instanceof Object && item.constructor === Object) {
+			curr = item;
+			continue;
+		} else if (item instanceof Object && item.constructor === Array) {
+			if (k == path.length-1) {
+				curr = item;
+				break; // this is the end of the path, so check this array
+			} else if (!(isNaN(parseInt(path[k+1], 10)))) {
+				curr = item;
+				continue; // the *next* path section is an item in the array so we don't check this whole array
+			}
+			// otherwise, check each item in the array against the rest of the path
+			for(var ii = 0, il = item.length; ii < il; ii++){
+				var subitem = item[ii];
+				if (subitem.constructor !== Object) continue;	// can't look for a subfield in a non-object value.
+				if (this._matches(subitem, path.slice(k), null, checker)) { // check the item against the rest of the path
+					if (details && details.needRecord())
+						details.setElemMatchKey(ii.toString());
+					return true;
+				}
+			}
+			return false; // checked all items in the array and found no matches
+		}
+	}
+	return checker(item);
+};

lib/pipeline/matcher/LeafMatchExpression.js

@@ -48,21 +48,46 @@ proto.initPath = function initPath( path ) { //  const StringData& path
  *
  */
 proto.matches = function matches( doc, details ) { //  const MatchableDocument* doc, MatchDetails* details
-// File: expression_leaf.cpp lines: 37-48
+	// File: expression_leaf.cpp lines: 37-48
+	var self = this,
+		checker = function(element) {
+			if (element instanceof Array) {
+				for (var i = 0; i < element.length; i++) {
+					if(self.matchesSingleElement(element[i])) {
+						if(details && details.needRecord()) {
+							details.setElemMatchKey(i.toString());
+						}
+						return true;
+					}
+				}
+				return false;
+			}
+			if (!self.matchesSingleElement(element)) {
+				return false;
+			}
+			/*
+			if( details && details.needRecord() && (element instanceof Array)) {
+				details.setElemMatchKey( docKeys[i+1] );
+			}
+			*/
+			return true;
+		};
+	return this._elementPath._matches(doc, details, checker);
+	/*
 	var tDoc = ElementPath.objAtPath( doc, this._path );
 	if(tDoc instanceof RegExp || typeof(tDoc) != 'object') {
 		return this.matchesSingleElement(tDoc);
 	}
-	var docKeys = Object.keys(tDoc);
-	for(var i = 0;i < docKeys.length; i++ ) {
 		if(!this.matchesSingleElement( tDoc[docKeys[i]] ))
 			continue;
 		if( details && details.needRecord() && (tDoc instanceof Array) && i < docKeys.length-1 ) {
+			console.log('test2');
 			details.setElemMatchKey( docKeys[i+1] );
 		}
 		return true;
 	}
 	return false;
+	*/
 };
 
 

lib/pipeline/matcher/NotMatchExpression.js

@@ -15,7 +15,7 @@ var NotMatchExpression = module.exports = function NotMatchExpression(){
 proto._exp = undefined;
 
 /**
- * 
+ *
  * This documentation was automatically generated. Please update when you touch this function.
  * @method debugString
  * @param
@@ -28,20 +28,20 @@ proto.debugString = function debugString( level ) { //  StringBuilder& debug, in
 
 
 /**
- * 
+ *
  * This documentation was automatically generated. Please update when you touch this function.
  * @method equivalent
  * @param
  *
  */
-proto.equivalent = function equivalent( other ) { //  const MatchExpression* other 
+proto.equivalent = function equivalent( other ) { //  const MatchExpression* other
 // File: expression_tree.cpp lines: 152-156
 	return other._matchType == 'NOT' && this._exp.equivalent(other.getChild(0));
 };
 
 
 /**
- * 
+ *
  * This documentation was automatically generated. Please update when you touch this function.
  * @method getChild
  * @param
@@ -55,21 +55,21 @@ proto.getChild = function getChild( i ) { //  size_t i
 
 
 /**
- * 
+ *
  * This documentation was automatically generated. Please update when you touch this function.
  * @method init
  * @param
  *
  */
-proto.init = function init( exp ) { //  MatchExpression* exp 
-// File: expression_tree.h lines: 123-125
+proto.init = function init( exp ) { //  MatchExpression* exp
+	// File: expression_tree.h lines: 123-125
 	this._exp = exp;
 	return {'code':'OK'};
 };
 
 
 /**
- * 
+ *
  * This documentation was automatically generated. Please update when you touch this function.
  * @method matches
  * @param
@@ -82,7 +82,7 @@ proto.matches = function matches( doc,details ) { //  const MatchableDocument* d
 
 
 /**
- * 
+ *
  * This documentation was automatically generated. Please update when you touch this function.
  * @method matchesSingleElement
  * @param
@@ -95,7 +95,7 @@ proto.matchesSingleElement = function matchesSingleElement( e ) { //  const BSON
 
 
 /**
- * 
+ *
  * This documentation was automatically generated. Please update when you touch this function.
  * @method numChildren
  * @param
@@ -108,7 +108,7 @@ proto.numChildren = function numChildren( /*  */ ){
 
 
 /**
- * 
+ *
  * This documentation was automatically generated. Please update when you touch this function.
  * @method shallowClone
  * @param

test/lib/pipeline/matcher/AndMatchExpression.js

@@ -1,12 +1,12 @@
 "use strict";
 var assert = require("assert"),
-	AndMatchExpression = require("../../../../lib/pipeline/matcher/AndMatchExpression"),
-	LTMatchExpression = require("../../../../lib/pipeline/matcher/LTMatchExpression"),
-	GTMatchExpression = require("../../../../lib/pipeline/matcher/GTMatchExpression"),
-	RegexMatchExpression = require("../../../../lib/pipeline/matcher/RegexMatchExpression"),
-	EqualityMatchExpression = require("../../../../lib/pipeline/matcher/EqualityMatchExpression"),
-	NotMatchExpression = require("../../../../lib/pipeline/matcher/EqualityMatchExpression");
-
+	AndMatchExpression = require("../../../../lib/pipeline/matcher/AndMatchExpression.js"),
+	LTMatchExpression = require("../../../../lib/pipeline/matcher/LTMatchExpression.js"),
+	GTMatchExpression = require("../../../../lib/pipeline/matcher/GTMatchExpression.js"),
+	MatchDetails = require("../../../../lib/pipeline/matcher/MatchDetails.js"),
+	RegexMatchExpression = require("../../../../lib/pipeline/matcher/RegexMatchExpression.js"),
+	EqualityMatchExpression = require("../../../../lib/pipeline/matcher/EqualityMatchExpression.js"),
+	NotMatchExpression = require("../../../../lib/pipeline/matcher/NotMatchExpression.js");
 
 module.exports = {
 	"AndMatchExpression": {
@@ -35,8 +35,8 @@ module.exports = {
 			var eq = new EqualityMatchExpression();
 			var op = new AndMatchExpression();
 
-			assert.strictEqual( eq.init('a', 5).code,'OK');
-			assert.strictEqual( nop.init(eq).code,'OK');
+			assert.strictEqual(eq.init('a', 5).code, 'OK');
+			assert.strictEqual(nop.init(eq).code, 'OK');
 			op.add(nop);
 			assert.ok( op.matches({'a':4}) );
 			assert.ok( op.matches({'a':[4,6]}) );
@@ -44,15 +44,51 @@ module.exports = {
 			assert.ok( !op.matches({'a':[4,5]}) );
 		},
 		"Should match three clauses": function(){
-		// File expression_tree_test.cpp lines 144-168
+			var baseOperand1 = {"$gt":1},
+				baseOperand2 = {"$lt":10},
+				baseOperand3 = {"$lt":100},
+				sub1 = new GTMatchExpression(),
+				sub2 = new LTMatchExpression(),
+				sub3 = new LTMatchExpression(),
+				andOp = new AndMatchExpression();
+
+			assert.strictEqual(sub1.init("a", baseOperand1.$gt).code, 'OK');
+			assert.strictEqual(sub2.init("a", baseOperand2.$lt).code, 'OK');
+			assert.strictEqual(sub3.init("b", baseOperand3.$lt).code, 'OK');
 
-			assert.ok(false, 'Fill out test');
+			andOp.add(sub1);
+			andOp.add(sub2);
+			andOp.add(sub3);
 
+			assert.ok(andOp.matchesBSON({"a":5, "b":6}, null));
+			assert.ok(!andOp.matchesBSON({"a":5}, null));
+			assert.ok(!andOp.matchesBSON({"b":6}, null ));
+			assert.ok(!andOp.matchesBSON({"a":1, "b":6}, null));
+			assert.ok(!andOp.matchesBSON({"a":10, "b":6}, null));
 		},
 		"Should have an elemMatchKey": function(){
-		// File expression_tree_test.cpp lines 170 - 195
+			var baseOperand1 = {"a":1},
+				baseOperand2 = {"b":2},
+				sub1 = new EqualityMatchExpression(),
+				sub2 = new EqualityMatchExpression(),
+				andOp = new AndMatchExpression(),
+				details = new MatchDetails();
+
+			assert.strictEqual(sub1.init("a", baseOperand1.a).code, 'OK');
+			assert.strictEqual(sub2.init("b", baseOperand2.b).code, 'OK');
+
+			andOp.add(sub1);
+			andOp.add(sub2);
 
-			assert.ok(false, 'Fill out test');
+			details.requestElemMatchKey();
+			assert.ok(!andOp.matchesBSON({"a":[1]}, details));
+			assert.ok(!details.hasElemMatchKey());
+			assert.ok(!andOp.matchesBSON({"b":[2]}, details));
+			assert.ok(!details.hasElemMatchKey());
+			assert.ok(andOp.matchesBSON({"a":[1], "b":[1, 2]}, details));
+			assert.ok(details.hasElemMatchKey());
+			// The elem match key for the second $and clause is recorded.
+			assert.strictEqual("1", details.elemMatchKey());
 		}