Correcting or and adding not

Previously, OR filters always returned true (fixes #500).

src/ol/filter/logicalfilter.js

@@ -1,6 +1,7 @@
 goog.provide('ol.filter.Logical');
 goog.provide('ol.filter.LogicalOperator');
 
+goog.require('goog.asserts');
 goog.require('ol.filter.Filter');
 
 
@@ -9,7 +10,7 @@ goog.require('ol.filter.Filter');
  * @constructor
  * @extends {ol.filter.Filter}
  * @param {Array.<ol.filter.Filter>} filters Filters to and-combine.
- * @param {!ol.filter.LogicalOperator} operator Operator.
+ * @param {ol.filter.LogicalOperator} operator Operator.
  */
 ol.filter.Logical = function(filters, operator) {
   goog.base(this);
@@ -19,9 +20,10 @@ ol.filter.Logical = function(filters, operator) {
    * @private
    */
   this.filters_ = filters;
+  goog.asserts.assert(filters.length > 0, 'Must supply at least one filter');
 
   /**
-   * @type {!ol.filter.LogicalOperator}
+   * @type {ol.filter.LogicalOperator}
    */
   this.operator = operator;
 
@@ -35,14 +37,29 @@ goog.inherits(ol.filter.Logical, ol.filter.Filter);
 ol.filter.Logical.prototype.applies = function(feature) {
   var filters = this.filters_,
       i = 0, ii = filters.length,
-      operator = this.operator,
+      result;
-      start = operator(true, false),
+  switch (this.operator) {
-      result = start;
+    case ol.filter.LogicalOperator.AND:
-  while (result === start && i < ii) {
+      result = true;
-    result = operator(result, filters[i].applies(feature));
+      while (result && i < ii) {
-    ++i;
+        result = result && filters[i].applies(feature);
+        ++i;
+      }
+      break;
+    case ol.filter.LogicalOperator.OR:
+      result = false;
+      while (!result && i < ii) {
+        result = result || filters[i].applies(feature);
+        ++i;
+      }
+      break;
+    case ol.filter.LogicalOperator.NOT:
+      result = !filters[i].applies(feature);
+      break;
+    default:
+      goog.asserts.assert(false, 'Unsupported operation: ' + this.operator);
   }
-  return result;
+  return !!result;
 };
 
 
@@ -55,9 +72,10 @@ ol.filter.Logical.prototype.getFilters = function() {
 
 
 /**
- * @enum {!Function}
+ * @enum {string}
  */
 ol.filter.LogicalOperator = {
-  AND: /** @return {boolean} result. */ function(a, b) { return a && b; },
+  AND: '&&',
-  OR: /** @return {boolean} result. */ function(a, b) { return a || b; }
+  OR: '||',
+  NOT: '!'
 };

test/spec/ol/filter/logicalfilter.test.js

@@ -0,0 +1,97 @@
+goog.provide('ol.test.filter.Logical');
+
+
+describe('ol.filter.Logical', function() {
+
+  var OR = ol.filter.LogicalOperator.OR;
+  var AND = ol.filter.LogicalOperator.AND;
+  var NOT = ol.filter.LogicalOperator.NOT;
+  var include = new ol.filter.Filter(function() {return true});
+  var exclude = new ol.filter.Filter(function() {return false});
+
+  var apple = new ol.Feature({});
+  var orange = new ol.Feature({});
+  var duck = new ol.Feature({});
+
+  var isApple = new ol.filter.Filter(function(feature) {
+    return feature === apple;
+  });
+  var isOrange = new ol.filter.Filter(function(feature) {
+    return feature === orange;
+  });
+  var isDuck = new ol.filter.Filter(function(feature) {
+    return feature === duck;
+  });
+
+  describe('constructor', function() {
+    it('creates a new filter', function() {
+      var filter = new ol.filter.Logical([include, exclude], OR);
+      expect(filter).to.be.a(ol.filter.Logical);
+    });
+  });
+
+  describe('#operator', function() {
+    it('can be OR', function() {
+      var filter = new ol.filter.Logical([include, exclude], OR);
+      expect(filter.operator).to.be(OR);
+    });
+
+    it('can be AND', function() {
+      var filter = new ol.filter.Logical([include, exclude], AND);
+      expect(filter.operator).to.be(AND);
+    });
+
+    it('can be NOT', function() {
+      var filter = new ol.filter.Logical([include], NOT);
+      expect(filter.operator).to.be(NOT);
+    });
+  });
+
+  describe('#applies', function() {
+
+    it('works for OR', function() {
+      var isFruit = new ol.filter.Logical([isApple, isOrange], OR);
+
+      expect(isApple.applies(apple)).to.be(true);
+      expect(isOrange.applies(apple)).to.be(false);
+      expect(isFruit.applies(apple)).to.be(true);
+
+      expect(isApple.applies(duck)).to.be(false);
+      expect(isOrange.applies(duck)).to.be(false);
+      expect(isFruit.applies(duck)).to.be(false);
+    });
+
+    it('works for AND', function() {
+      expect(include.applies(apple)).to.be(true);
+      expect(isApple.applies(apple)).to.be(true);
+      expect(isDuck.applies(apple)).to.be(false);
+
+      var pass = new ol.filter.Logical([include, isApple], AND);
+      expect(pass.applies(apple)).to.be(true);
+
+      var fail = new ol.filter.Logical([isApple, isDuck], AND);
+      expect(fail.applies(apple)).to.be(false);
+    });
+
+    it('works for NOT', function() {
+      expect(isApple.applies(apple)).to.be(true);
+      expect(isDuck.applies(apple)).to.be(false);
+      expect(isDuck.applies(duck)).to.be(true);
+      expect(isDuck.applies(apple)).to.be(false);
+
+      var notApple = new ol.filter.Logical([isApple], NOT);
+      expect(notApple.applies(apple)).to.be(false);
+      expect(notApple.applies(duck)).to.be(true);
+
+      var notDuck = new ol.filter.Logical([isDuck], NOT);
+      expect(notDuck.applies(apple)).to.be(true);
+      expect(notDuck.applies(duck)).to.be(false);
+    });
+  });
+
+});
+
+goog.require('ol.Feature');
+goog.require('ol.filter.Filter');
+goog.require('ol.filter.Logical');
+goog.require('ol.filter.LogicalOperator');