Skip to content
This repository has been archived by the owner on Jul 30, 2020. It is now read-only.

Commit

Permalink
Fix handling of numeric refinements when disjunctive faceting is used
Browse files Browse the repository at this point in the history 
They were ignored. The fix involves introducing specific methods to generate `Query.filters` from numeric refinements or facet refinements alone.
  • Loading branch information
Clément Le Provost committed Dec 13, 2016
1 parent b785931 commit d3519ac
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 28 deletions.
116 changes: 91 additions & 25 deletions Sources/SearchParameters.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,38 +376,53 @@ import Foundation

// MARK: - Generate filters

/// Build a URL query string from the current parameters, including numeric and facet refinements.
///
/// - returns: A URL query string.
///
@objc override open func build() -> String {
// Override the `filters` parameter with the current refinements.
var parameters = self.parameters
parameters["filters"] = buildFilters()
return AbstractQuery.build(parameters: parameters)
}

/// Generate a filter expression from the current filters.
/// Generate a filter expression from the current refinements.
/// This will combine numeric refinements and facet refinements into the same expression.
///
/// - returns: An expression suitable for use with `Query.filters`. The expression may be nil if no refinements
/// are being used.
/// - returns: An expression suitable for use with `Query.filters`. The expression may be `nil` if no refinements
/// are currently set.
///
@objc public func buildFilters() -> String? {
if !hasRefinements() {
return nil
}
// NOTE: We sort attribute names to get predictable output.
// Facet filters.
let facetExpression = facetRefinements.keys.sorted().flatMap({ (facetName: String) -> String? in
let refinements = self.facetRefinements[facetName]!
if refinements.isEmpty {
return nil
}
if self.isDisjunctiveFacet(name: facetName) {
let innerFilters = refinements.map({ return $0.buildFilter() }).joined(separator: " OR ")
return "(\(innerFilters))"
let facetExpression = buildFiltersFromFacets()
let numericExpression = buildFiltersFromNumerics()
if let facetExpression = facetExpression {
if let numericExpression = numericExpression {
return facetExpression + " AND " + numericExpression
} else {
return refinements.map({ return $0.buildFilter() }).joined(separator: " AND ")
return facetExpression
}
}).joined(separator: " AND ")
// Numeric filters.
let numericExpression = numericRefinements.keys.sorted().flatMap({ (attributeName: String) -> String? in
} else if let numericExpression = numericExpression {
return numericExpression
} else {
assert(false) // should not happen, as per above
return nil
}
}

/// Generate a filter expression from the current numeric refinements.
///
/// - returns: An expression suitable for use with `Query.filters`. The expression may be `nil` if no numeric
/// refinements are currently set.
///
@objc public func buildFiltersFromNumerics() -> String? {
if (!hasNumericRefinements()) {
return nil
}
// NOTE: We sort attribute names to get predictable output.
let expression = numericRefinements.keys.sorted().flatMap({ (attributeName: String) -> String? in
let filters = self.numericRefinements[attributeName]!
if filters.isEmpty {
return nil
Expand All @@ -419,19 +434,41 @@ import Foundation
return filters.map({ return $0.buildFilter() }).joined(separator: " AND ")
}
}).joined(separator: " AND ")
// Combine everything together.
var expression = facetExpression
if !numericExpression.isEmpty {
if !facetExpression.isEmpty {
expression += " AND "
}
expression += numericExpression
assert(!expression.isEmpty)
return expression
}

/// Generate a filter expression from the current facet refinements.
///
/// - returns: An expression suitable for use with `Query.filters`. The expression may be `nil` if no facet
/// refinements are currently set.
///
@objc public func buildFiltersFromFacets() -> String? {
if (!hasFacetRefinements()) {
return nil
}
// NOTE: We sort attribute names to get predictable output.
let expression = facetRefinements.keys.sorted().flatMap({ (facetName: String) -> String? in
let refinements = self.facetRefinements[facetName]!
if refinements.isEmpty {
return nil
}
if self.isDisjunctiveFacet(name: facetName) {
let innerFilters = refinements.map({ return $0.buildFilter() }).joined(separator: " OR ")
return "(\(innerFilters))"
} else {
return refinements.map({ return $0.buildFilter() }).joined(separator: " AND ")
}
}).joined(separator: " AND ")
assert(!expression.isEmpty)
return expression
}

/// Generate facet refinements from the current filters.
///
/// + Note: Those are intended for use with `Index.searchDisjunctiveFaceting(...)`. In the general case, please
/// use `buildFilters()` instead.
///
/// - returns: Facet refinements suitable for use with `Index.searchDisjunctiveFaceting(...)`.
///
@objc public func buildFacetRefinements() -> [String: [String]] {
Expand All @@ -444,6 +481,9 @@ import Foundation

/// Generate facet filters from the current filters.
///
/// + Note: Those are intended for use with `Query.facetFilters`. In the general case, please use
/// `buildFiltersFromNumerics()` instead.
///
/// - returns: An expression suitable for use with `Query.facetFilters`.
///
@objc public func buildFacetFilters() -> [Any] {
Expand All @@ -463,7 +503,33 @@ import Foundation
}
return facetFilters
}

// MARK: - Update filters in place

/// Update the parameters from the current refinements.
///
/// + Warning: This will override the `filters` parameter.
///
@objc public func update() {
self["filters"] = buildFilters()
}

/// Update the parameters from the current *facet* refinements. Numeric refinements are ignored.
///
/// + Warning: This will override the `filters` parameter.
///
@objc public func updateFromFacets() {
self["filters"] = buildFiltersFromFacets()
}

/// Update the parameters from the current *numeric* refinements. Facet refinements are ignored.
///
/// + Warning: This will override the `filters` parameter.
///
@objc public func updateFromNumerics() {
self["filters"] = buildFiltersFromNumerics()
}

// MARK: - Facets

/// Set a given facet as disjunctive or conjunctive.
Expand Down
7 changes: 4 additions & 3 deletions Sources/Searcher.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,12 +367,13 @@ import Foundation
this.handleResults(content: content, error: error, userInfo: userInfo)
}
if state.disjunctiveFacets.isEmpty {
// All facets are conjunctive; build facet filters accordingly.
// TODO: Automate this?
params.facetFilters = params.buildFacetFilters()
// All facets are conjunctive; build regular filters combining numeric and facet refinements.
// NOTE: Not strictly necessary since `Index.search(...)` calls `Query.build()`, but let's not rely on that.
params.update()
operation = index.search(params, completionHandler: completionHandler)
} else {
// Facet filters are built directly by the disjunctive faceting search helper method.
params.updateFromNumerics() // this is really necessary (in contrast to the above)
let refinements = params.buildFacetRefinements()
operation = index.searchDisjunctiveFaceting(params, disjunctiveFacets: state.disjunctiveFacets, refinements: refinements, completionHandler: completionHandler)
}
Expand Down
71 changes: 71 additions & 0 deletions Tests/SearchParametersTest.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,42 +50,52 @@ class SearchParametersTest: XCTestCase {

// Empty params should produce empty string.
XCTAssertNil(params.buildFilters())
XCTAssertNil(params.buildFiltersFromFacets())

// One conjunctive facet with one refinement.
params.addFacetRefinement(name: "foo", value: "bar1")
XCTAssertEqual(params.buildFilters(), "\"foo\":\"bar1\"")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromFacets())

// One conjunctive facet with two refinements.
params.addFacetRefinement(name: "foo", value: "bar2")
XCTAssertEqual(params.buildFilters(), "\"foo\":\"bar1\" AND \"foo\":\"bar2\"")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromFacets())

// Two conjunctive facets with one refinement.
params.removeFacetRefinement(name: "foo", value: "bar1")
params.addFacetRefinement(name: "abc", value: "xyz")
XCTAssertEqual(params.buildFilters(), "\"abc\":\"xyz\" AND \"foo\":\"bar2\"")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromFacets())

// Two conjunctive facets with two refinements (one negated).
params.addFacetRefinement(name: "foo", value: "bar3")
params.addFacetRefinement(name: "abc", value: "tuv", inclusive: false)
XCTAssertEqual(params.buildFilters(), "\"abc\":\"xyz\" AND NOT \"abc\":\"tuv\" AND \"foo\":\"bar2\" AND \"foo\":\"bar3\"")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromFacets())

// One conjunctive facet and one disjunctive facet.
params.setFacet(withName: "abc", disjunctive: true)
XCTAssertEqual(params.buildFilters(), "(\"abc\":\"xyz\" OR NOT \"abc\":\"tuv\") AND \"foo\":\"bar2\" AND \"foo\":\"bar3\"")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromFacets())

// Two disjunctive facets.
params.setFacet(withName: "foo", disjunctive: true)
XCTAssertEqual(params.buildFilters(), "(\"abc\":\"xyz\" OR NOT \"abc\":\"tuv\") AND (\"foo\":\"bar2\" OR \"foo\":\"bar3\")")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromFacets())

// Disjunctive facet with only one refinement.
params.removeFacetRefinement(name: "abc", value: "tuv")
XCTAssertEqual(params.buildFilters(), "(\"abc\":\"xyz\") AND (\"foo\":\"bar2\" OR \"foo\":\"bar3\")")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromFacets())

// Remove all refinements: facet should disappear from params.
params.removeFacetRefinement(name: "abc", value: "xyz")
XCTAssertEqual(params.buildFilters(), "(\"foo\":\"bar2\" OR \"foo\":\"bar3\")")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromFacets())
params.clearFacetRefinements(name: "foo")
XCTAssertNil(params.buildFilters())
XCTAssertNil(params.buildFiltersFromFacets())
}

func testFacetExistence() {
Expand Down Expand Up @@ -121,42 +131,52 @@ class SearchParametersTest: XCTestCase {

// Empty params should produce empty string.
XCTAssertNil(params.buildFilters())
XCTAssertNil(params.buildFiltersFromNumerics())

// One conjunctive numeric with one refinement.
params.addNumericRefinement("foo", .greaterThanOrEqual, 3)
XCTAssertEqual(params.buildFilters(), "\"foo\" >= 3")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromNumerics())

// One conjunctive numeric with two refinements.
params.addNumericRefinement("foo", .lessThan, 4.0)
XCTAssertEqual(params.buildFilters(), "\"foo\" >= 3 AND \"foo\" < 4")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromNumerics())

// Two conjunctive numeric with one refinement.
params.removeNumericRefinement(NumericRefinement("foo", .greaterThanOrEqual, 3.0))
params.addNumericRefinement(NumericRefinement("bar", .greaterThan, 456.789))
XCTAssertEqual(params.buildFilters(), "\"bar\" > 456.789 AND \"foo\" < 4")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromNumerics())

// Two conjunctive numerics with two refinements (one negated).
params.addNumericRefinement("foo", .notEqual, 0)
params.addNumericRefinement("bar", .equal, 0, inclusive: false)
XCTAssertEqual(params.buildFilters(), "\"bar\" > 456.789 AND NOT \"bar\" = 0 AND \"foo\" < 4 AND \"foo\" != 0")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromNumerics())

// One conjunctive numeric and one disjunctive.
params.setNumeric(withName: "foo", disjunctive: true)
XCTAssertEqual(params.buildFilters(), "\"bar\" > 456.789 AND NOT \"bar\" = 0 AND (\"foo\" < 4 OR \"foo\" != 0)")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromNumerics())

// Two disjunctive numeric.
params.setNumeric(withName: "bar", disjunctive: true)
XCTAssertEqual(params.buildFilters(), "(\"bar\" > 456.789 OR NOT \"bar\" = 0) AND (\"foo\" < 4 OR \"foo\" != 0)")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromNumerics())

// Disjunctive numeric with only one refinement.
params.removeNumericRefinement("foo", .lessThan, 4)
XCTAssertEqual(params.buildFilters(), "(\"bar\" > 456.789 OR NOT \"bar\" = 0) AND (\"foo\" != 0)")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromNumerics())

// Remove all refinements: numerics should disappear from params.
params.removeNumericRefinement("foo", .notEqual, 0.0)
XCTAssertEqual(params.buildFilters(), "(\"bar\" > 456.789 OR NOT \"bar\" = 0)")
XCTAssertEqual(params.buildFilters(), params.buildFiltersFromNumerics())
params.clearNumericRefinements(name: "bar")
XCTAssertNil(params.buildFilters())
XCTAssertNil(params.buildFiltersFromNumerics())
}

func testBooleanNumeric() {
Expand Down Expand Up @@ -188,16 +208,67 @@ class SearchParametersTest: XCTestCase {
XCTAssertFalse(params.hasNumericRefinements(name: "foo"))
}

/// Test combining facet refinements and numeric refinements.
///
func testFacetAndNumeric() {
let params = SearchParameters()
params.addNumericRefinement("foo", .greaterThanOrEqual, 123)
params.addFacetRefinement(name: "abc", value: "something")
params.addNumericRefinement("bar", .lessThan, 456.789)
params.addFacetRefinement(name: "xyz", value: "other")
XCTAssertEqual(params.buildFilters(), "\"abc\":\"something\" AND \"xyz\":\"other\" AND \"bar\" < 456.789 AND \"foo\" >= 123")
XCTAssertEqual(params.buildFiltersFromFacets(), "\"abc\":\"something\" AND \"xyz\":\"other\"")
XCTAssertEqual(params.buildFiltersFromNumerics(), "\"bar\" < 456.789 AND \"foo\" >= 123")

let params2 = SearchParameters(from: params)
params2.clearNumericRefinements()
XCTAssertEqual(params2.buildFilters(), "\"abc\":\"something\" AND \"xyz\":\"other\"")
XCTAssertEqual(params2.buildFiltersFromFacets(), "\"abc\":\"something\" AND \"xyz\":\"other\"")
XCTAssertNil(params2.buildFiltersFromNumerics())

let params3 = SearchParameters(from: params)
params3.clearFacetRefinements()
XCTAssertEqual(params3.buildFilters(), "\"bar\" < 456.789 AND \"foo\" >= 123")
XCTAssertEqual(params3.buildFiltersFromNumerics(), "\"bar\" < 456.789 AND \"foo\" >= 123")
XCTAssertNil(params3.buildFiltersFromFacets())
}

/// Test updating parameters in place.
///
func testUpdate() {
let params = SearchParameters()
params.addNumericRefinement("foo", .greaterThanOrEqual, 123)
params.addFacetRefinement(name: "abc", value: "something")
params.addNumericRefinement("bar", .lessThan, 456.789)
params.addFacetRefinement(name: "xyz", value: "other")
params.update()
XCTAssertEqual(params["filters"], "\"abc\":\"something\" AND \"xyz\":\"other\" AND \"bar\" < 456.789 AND \"foo\" >= 123")
params.updateFromFacets()
XCTAssertEqual(params["filters"], "\"abc\":\"something\" AND \"xyz\":\"other\"")
params.updateFromNumerics()
XCTAssertEqual(params["filters"], "\"bar\" < 456.789 AND \"foo\" >= 123")

let params2 = SearchParameters(from: params)
params2.clearNumericRefinements()
params2.update()
XCTAssertEqual(params2["filters"], "\"abc\":\"something\" AND \"xyz\":\"other\"")
params2.updateFromFacets()
XCTAssertEqual(params2["filters"], "\"abc\":\"something\" AND \"xyz\":\"other\"")
params2.updateFromNumerics()
XCTAssertNil(params2["filters"])

let params3 = SearchParameters(from: params)
params3.clearFacetRefinements()
params3.update()
XCTAssertEqual(params3["filters"], "\"bar\" < 456.789 AND \"foo\" >= 123")
params3.updateFromFacets()
XCTAssertNil(params3["filters"])
params3.updateFromNumerics()
XCTAssertEqual(params3["filters"], "\"bar\" < 456.789 AND \"foo\" >= 123")
}

/// Test interaction of params with other search parameters.
///
func testOtherParams() {
let params = SearchParameters()

Expand Down

0 comments on commit d3519ac

Please sign in to comment.