Remove filter overlap handling -- PPS doesn't actually impl gt/lt so …

…it's pointless
internetarchive · Nov 11, 2022 · 907911e · 907911e
1 parent c023aed
commit 907911e
Show file tree

Hide file tree

Showing 2 changed files with 3 additions and 92 deletions.
diff --git a/src/filter-map-builder.ts b/src/filter-map-builder.ts
@@ -1,12 +1,5 @@
 import { FilterConstraint, FilterMap } from './search-params';
 
-const NUMERIC_CONSTRAINTS = [
-  FilterConstraint.GREATER_OR_EQUAL,
-  FilterConstraint.GREATER_THAN,
-  FilterConstraint.LESS_OR_EQUAL,
-  FilterConstraint.LESS_THAN,
-];
-
 /**
  * A utility class for building filter maps
  */
@@ -15,7 +8,7 @@ export class FilterMapBuilder {
 
   /**
    * Adds a filter to the FilterMap under construction.
-   * The new filter may overwrite or modify existing filters already added to the builder.
+   * The new filter may overwrite existing filters already added to the builder.
    * @param field The field to filter on (e.g., 'subject', 'year', ...)
    * @param value The value of the field to filter on (e.g., 'Cicero', '1920', ...)
    * @param constraint The constraint to apply to the `field`, with respect to the given `value`.
@@ -26,39 +19,7 @@ export class FilterMapBuilder {
       this.filterMap[field] = {};
     }
 
-    // Edge case for numeric fields (i.e., year)
-    // Logically it would be valid for excluded values to overlap with gt/gte/lt/lte values.
-    // For instance, specifying 1950 <= year <= 2000, but also excluding year = 1950.
-    // But since we can only have one constraint for a given value, these would overwrite each other.
-    // To work around this, we adjust the resulting map to maintain the correct logic.
-    const currentConstraint = this.filterMap[field][value];
-    if (currentConstraint && constraint === FilterConstraint.EXCLUDE) {
-      if (currentConstraint === FilterConstraint.GREATER_OR_EQUAL) {
-        // gte and exclude on the same value -> gt
-        this.filterMap[field][value] = FilterConstraint.GREATER_THAN;
-        return this;
-      } else if (currentConstraint === FilterConstraint.LESS_OR_EQUAL) {
-        // lte and exclude on the same value -> lt
-        this.filterMap[field][value] = FilterConstraint.LESS_THAN;
-        return this;
-      } else if (
-        currentConstraint === FilterConstraint.GREATER_THAN ||
-        currentConstraint === FilterConstraint.LESS_THAN
-      ) {
-        // gt/lt and exclude -> no additional filter
-        return this;
-      }
-    }
-
-    // An 'include' constraint should always take precedence over a gt/gte/lt/lte constraint on the same value.
-    if (currentConstraint === FilterConstraint.INCLUDE) {
-      if (NUMERIC_CONSTRAINTS.includes(constraint)) {
-        // Don't overwrite the existing 'include' constraint
-        return this;
-      }
-    }
-
-    // Otherwise, overwrite any existing filter for this value
+    // Overwrite any existing filter for this value
     this.filterMap[field][value] = constraint;
     return this;
   }
@@ -93,7 +54,7 @@ export class FilterMapBuilder {
 
   /**
    * Adds all filters from an existing filter map to the one being built.
-   * Filters from the provided map may overwrite or modify existing filters already added to the builder.
+   * Filters from the provided map may overwrite existing filters already added to the builder.
    * @param map The FilterMap to merge into the one being built.
    */
   mergeFilterMap(map: FilterMap): this {

diff --git a/test/filter-map-builder.test.ts b/test/filter-map-builder.test.ts
@@ -89,54 +89,4 @@ describe('filter map builder', () => {
       baz: { boop: 'exc' },
     });
   });
-
-  it('correctly resolves overlap between exclude and gte constraints', () => {
-    const builder = new FilterMapBuilder();
-    builder.addFilter('year', '1950', FilterConstraint.GREATER_OR_EQUAL);
-    expect(builder.build()).to.deep.equal({ year: { 1950: 'gte' } });
-
-    builder.addFilter('year', '1950', FilterConstraint.EXCLUDE);
-    // Overwrites constraint with GREATER_THAN
-    expect(builder.build()).to.deep.equal({ year: { 1950: 'gt' } });
-  });
-
-  it('correctly resolves overlap between exclude and lte constraints', () => {
-    const builder = new FilterMapBuilder();
-    builder.addFilter('year', '1950', FilterConstraint.LESS_OR_EQUAL);
-    expect(builder.build()).to.deep.equal({ year: { 1950: 'lte' } });
-
-    builder.addFilter('year', '1950', FilterConstraint.EXCLUDE);
-    // Overwrites constraint with LESS_THAN
-    expect(builder.build()).to.deep.equal({ year: { 1950: 'lt' } });
-  });
-
-  it('correctly resolves overlap between exclude and gt constraints', () => {
-    const builder = new FilterMapBuilder();
-    builder.addFilter('year', '1950', FilterConstraint.GREATER_THAN);
-    expect(builder.build()).to.deep.equal({ year: { 1950: 'gt' } });
-
-    builder.addFilter('year', '1950', FilterConstraint.EXCLUDE);
-    // Leaves constraint unchanged
-    expect(builder.build()).to.deep.equal({ year: { 1950: 'gt' } });
-  });
-
-  it('correctly resolves overlap between exclude and lt constraints', () => {
-    const builder = new FilterMapBuilder();
-    builder.addFilter('year', '1950', FilterConstraint.LESS_THAN);
-    expect(builder.build()).to.deep.equal({ year: { 1950: 'lt' } });
-
-    builder.addFilter('year', '1950', FilterConstraint.EXCLUDE);
-    // Leaves constraint unchanged
-    expect(builder.build()).to.deep.equal({ year: { 1950: 'lt' } });
-  });
-
-  it('correctly resolves overlap between include and numeric constraints', () => {
-    const builder = new FilterMapBuilder();
-    builder.addFilter('year', '1950', FilterConstraint.INCLUDE);
-    expect(builder.build()).to.deep.equal({ year: { 1950: 'inc' } });
-
-    builder.addFilter('year', '1950', FilterConstraint.GREATER_THAN);
-    // Existing include constraint takes precedence
-    expect(builder.build()).to.deep.equal({ year: { 1950: 'inc' } });
-  });
 });