Merge pull request #82 from canjs/difference-with-id

Fix ID and sort/page issues with can-set.difference

src/prop_tests/id_test.js

@@ -0,0 +1,46 @@
+var QUnit = require("steal-qunit");
+
+var set = require("../set-core"),
+  props = require("../props");
+
+QUnit.module("can-set props.id");
+
+test("id set.difference", function(){
+
+  var idProps = props.id("color");
+  var res;
+
+  res = set.difference({ color: "red" }, { color: "blue" }, idProps);
+  deepEqual(res, false, "id changes always false");
+
+  res = set.difference({ color: "red" }, { }, idProps);
+  deepEqual(res, false, "id removal always false");
+
+  res = set.difference({ }, { color: "blue" }, idProps);
+  deepEqual(res, true, "id addition always true");
+
+});
+
+test("id set.difference with where", function() {
+  var algebra = new set.Algebra(
+    props.id("color"),
+    props.enum("type", ["light", "dark"])
+  );
+  var res;
+
+  res = set.difference({ color: "red", type: ["light", "dark"] }, { color: "blue", type: "light" }, algebra);
+  deepEqual(res, false, "id changes always false");
+
+  res = set.difference({ color: "red", type: ["light", "dark"] }, { type: "light" }, algebra);
+  deepEqual(res, false, "id removal always false");
+
+  res = set.difference({ type: ["light", "dark"] }, {  type: "light", color: "blue" }, algebra);
+  deepEqual(res, true, "id addition always true");
+
+  res = set.difference({ type: ["light", "dark"] }, {  type: "light" }, algebra);
+  deepEqual(res, { type: "dark" }, "no id clause, fall back to where");
+
+  res = set.difference({ color: "red", type: ["light", "dark"] }, { color: "red", type: "light" }, algebra);
+  deepEqual(res, { color: "red", type: "dark" }, "no id change, fall back to where");
+
+});

src/prop_tests/sort_test.js

@@ -40,7 +40,7 @@ test('set.difference({ function })', function() {
 	var res = algebra.difference({ colors: ['red','blue'], sort: 'colors' },
 		{ colors: ['blue'] });
 
-	deepEqual(res, { colors: [ 'red' ] });
+	deepEqual(res, { colors: [ 'red' ], sort: 'colors' });
 });
 
 test('set.union', function(){

src/props_test.js

@@ -8,3 +8,4 @@ require('./prop_tests/offsetLimit_test');
 require('./prop_tests/boolean_test');
 require('./prop_tests/enum_test');
 require('./prop_tests/dotNotation_test');
+require('./prop_tests/id_test');

src/set-core.js

@@ -369,30 +369,57 @@ assign(Algebra.prototype, {
 
 		var differentClauses = this.getDifferentClauseTypes(aClauseProps, bClauseProps);
 
-		var result; // if too many clauses are different, then we won't be able
+		var clause, result; // if too many clauses are different, then we won't be able
 					// to determine the difference
 
-		switch(differentClauses.length) {
-			case 0 : {
-				// if all the clauses are the same, then there can't be a difference
-				result = false;
-				break;
-			}
-			case 1 : {
-				var clause = differentClauses[0];
-				// if there's only one clause to evaluate, then we can try to determine
-				// the difference set
-				result = compare.difference(aClauseProps[clause],
-					bClauseProps[clause], undefined, undefined, undefined,
-					this.clauses[clause], {});
-
-				if(this.translators[clause] && typeof result === "object") {
-					result = this.translators[clause].toSet({}, result);
+		if(differentClauses.length > 2) {
+			result = false;
+		} else if (differentClauses.length === 2 && (differentClauses[0] !== "where" || differentClauses[1] !== "id")) {
+			result = false;
+		} else {
+			switch(clause = differentClauses[0]) {
+				case undefined :
+					// if all the clauses are the same, then there can't be a difference
+				case "order" : {
+					// if order is the only difference, then there can't be a difference
+					// if items are paged but the order is different, though, the sets are not comparable
+					// Either way, the result is false
+					result = false;
+					break;
+				}
+				case "paginate" :
+				case "where" : {
+					// if there's only one clause to evaluate or the clauses are where + id,
+					// then we can try to determine the difference set.
+					// Note that any difference in the ID clause will cause the result to be
+					// true (if A has no ID but B has ID) or false (any case where A has ID)
+					result = compare.difference(aClauseProps[clause],
+						bClauseProps[clause], undefined, undefined, undefined,
+						this.clauses[clause], {});
+
+					if(typeof result === "object") {
+						if (this.translators[clause]) {
+							result = this.translators[clause].toSet({}, result);
+						}
+						// If where changed, don't preserve the same pagination (can't predict how many are needed),
+						//   but do preserve sort order.
+						// If pagination is what changed, preserve the same where *and* sort.
+						assign(result, aClauseProps.order);
+						if(clause === "paginate") {
+							assign(result, aClauseProps.where);
+						} else if(differentClauses[1] === "id") {
+							// If there was a possible ID difference, there is a case we need to handle where
+							// A has no ID clause but B has an ID clause, causing an incalculable subset of
+							// A where IDs aren't B's ID, so return true instead.
+							result = compare.difference(aClauseProps.id,
+								bClauseProps.id, undefined, undefined, undefined,
+								this.clauses.id, {});
+						}
+					}
+					break;
 				}
-				break;
 			}
 		}
-
 		return result;
 	},
 	/**
@@ -412,9 +439,9 @@ assign(Algebra.prototype, {
 	 *
 	 *   @param  {can-set/Set} a A set.
 	 *   @param  {can-set/Set} b A set.
-	 *   @return {can-set/Set|undefined} If an object is returned, it is the union of _A_ and _B_ (_A_ ∪ _B_).
+	 *   @return {can-set/Set|Boolean} If an object is returned, it is the union of _A_ and _B_ (_A_ ∪ _B_).
 	 *
-	 *   If `undefined` is returned, it means a union can't be created.
+	 *   If `false` is returned, it means a union can't be created.
 	 */
 	union: function(a, b){
 		// if everything is equal or has a difference
@@ -435,15 +462,15 @@ assign(Algebra.prototype, {
 	 * ) //-> {completed: true, due: "tomorrow", type: "critical"}
 	 * ```
 	 *
-	 *   @param  {[type]} a A set.
-	 *   @param  {[type]} b A set.
+	 *   @param  {can-set/Set} a A set.
+	 *   @param  {can-set/Set} b A set.
 	 *   @return {can-set/Set|Boolean} If an object is returned, it
 	 *   represents the intersection of sets _A_ and _B_ (_A_ ∩ _B_).
 	 *
 	 *   If `true` is returned, that means that an intersection exists, but no set object
 	 *   can be returned that represents that set.
 	 *
-	 *   If `false` is returned, that means there is intersection.
+	 *   If `false` is returned, that means there is no intersection.
 	 */
 	intersection: function(a, b){
 		// if everything is equal or has a difference
@@ -700,7 +727,7 @@ assign(Algebra.prototype, {
 	 * ```
 	 *
 	 *   @param  {Object} obj An instance's raw data.
-	 *   @return {*|String} If a single [can-set.props.id] is configured, it's value will be returned.
+	 *   @return {*|String} If a single [can-set.props.id] is configured, its value will be returned.
 	 *   If multiple [can-set.props.id] properties are configured a `JSON.stringify`-ed object is returned.
 	 */
 	id: function(props){