Skip to content

Commit

Permalink
Merge #413
Browse files Browse the repository at this point in the history
413: Add support for search ranking score r=curquiza a=Sherlouk

# Pull Request

## Related issue
Fixes #398

## What does this PR do?
- Add the ability receive a new param in the search method called showRankingScore.
- Add the ability handle the new _rankingScore key/value in the search hits' response.
- Add integration tests
- Update the code-samples accordingly

⚠️  **This is a breaking change**.

I'd love to hear some thoughts from others on how we may be able to avoid this kind of breaking change, but with #398 and #399 new values are being added to the "hit" object. The hit object is currently a generic provided by the user of the library, and as such it's impossible for us to add new attributes to that.

In this PR I've tried to get around this by creating our own encapsulation called "SearchHit" which itself holds a reference to the actual result but also any additional attributes that Meilisearch itself returns. In order to minimise the impact on users of the library I have implemented a "dynamic member lookup" with a keypath, this continues to support (in a completely type safe and compiler safe way) dot notation to variables within the hit result. Which means `hits[0].title` would still return the title on the hit, even though architecturally it's address is `hits[0].result.title`. While this does work for the vast majority of use cases, we are unable to support the case where a user tries to cast a hit to a type, you can see this in our tests where you can no longer cast `hits[0]` to a "Book", rather it is now a "SearchHit<Book>", but again... you can access variables the same and no other changes are needed to the code.

Importantly though this workaround with dynamic member lookups is limited to the capabilities of keypaths, which as documented officially by Apple [here](https://github.com/apple/swift/blob/main/test/attr/attr_dynamic_member_lookup.swift#L671-L672) does not include methods. As such where users have functions within their models, they would have to call that through `hits[0].result.someFunction()`.

I do personally think this is the best compromise. It is a breaking change, but most will not be impacted in any meaningful way. If others have a better idea though I'd love to hear it. This PR lays up the foundations for #399 which would be significantly easier once this is merged. But let's discuss.

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: James Sherlock <[email protected]>
Co-authored-by: Clémentine U. - curqui <[email protected]>
  • Loading branch information
3 people authored Sep 27, 2023
2 parents 4913acb + 4af1ac8 commit e3ba51a
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 6 deletions.
10 changes: 10 additions & 0 deletions .code-samples.meilisearch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@ search_parameter_guide_attributes_to_search_on_1: |-
print(error)
}
}
search_parameter_guide_show_ranking_score_1: |-
let searchParameters = SearchParameters(query: "dragon", showRankingScore: true)
client.index("movies").search(searchParameters) { (result: Result<Searchable<Movie>, Swift.Error>) in
switch result {
case .success(let searchResult):
print(searchResult.rankingScore)
case .failure(let error):
print(error)
}
}
authorization_header_1: |-
client = try MeiliSearch(host: "http://localhost:7700", apiKey: "masterKey")
client.getKeys { result in
Expand Down
8 changes: 7 additions & 1 deletion Sources/MeiliSearch/Model/SearchParameters.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ public struct SearchParameters: Codable, Equatable {
/// Whether to return the raw matches or not.
public let showMatchesPosition: Bool?

/// Whether to return the search ranking score or not.
public let showRankingScore: Bool?

// MARK: Initializers

public init(
Expand All @@ -81,7 +84,8 @@ public struct SearchParameters: Codable, Equatable {
filter: String? = nil,
sort: [String]? = nil,
facets: [String]? = nil,
showMatchesPosition: Bool? = nil) {
showMatchesPosition: Bool? = nil,
showRankingScore: Bool? = nil) {
self.query = query
self.offset = offset
self.limit = limit
Expand All @@ -99,6 +103,7 @@ public struct SearchParameters: Codable, Equatable {
self.sort = sort
self.facets = facets
self.showMatchesPosition = showMatchesPosition
self.showRankingScore = showRankingScore
}

// MARK: Query Initializers
Expand Down Expand Up @@ -133,5 +138,6 @@ public struct SearchParameters: Codable, Equatable {
case showMatchesPosition
case hitsPerPage
case page
case showRankingScore
}
}
33 changes: 32 additions & 1 deletion Sources/MeiliSearch/Model/SearchResult.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class Searchable<T>: Equatable, Codable where T: Codable, T: Equatable {
// MARK: Properties

/// Possible hints from the search query.
public var hits: [T] = []
public var hits: [SearchHit<T>] = []

/// Distribution of the given facets.
public var facetDistribution: [String: [String: Int]]?
Expand All @@ -34,6 +34,37 @@ public class Searchable<T>: Equatable, Codable where T: Codable, T: Equatable {
}
}

@dynamicMemberLookup
public struct SearchHit<T>: Equatable, Codable where T: Codable, T: Equatable {
public let document: T
public internal(set) var rankingScore: Double?

/// Dynamic member lookup is used to allow easy access to instance members of the hit result, maintaining a level of backwards compatibility.
public subscript<V>(dynamicMember keyPath: KeyPath<T, V>) -> V {
document[keyPath: keyPath]
}

// MARK: Codable

enum CodingKeys: String, CodingKey {
case rankingScore = "_rankingScore"
}

public init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
self.document = try T(from: decoder)
self.rankingScore = try container.decodeIfPresent(Double.self, forKey: .rankingScore)
}

public func encode(to encoder: Encoder) throws {
var container = encoder.singleValueContainer()
try container.encode(document)

var containerTwo = encoder.container(keyedBy: CodingKeys.self)
try containerTwo.encodeIfPresent(rankingScore, forKey: .rankingScore)
}
}

/**
`SearchResult` instances represent the result of a search.
Requires that the value `T` conforms to the `Codable` and `Equatable` protocols.
Expand Down
100 changes: 96 additions & 4 deletions Tests/MeiliSearchIntegrationTests/SearchTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,98 @@ class SearchTests: XCTestCase {
self.wait(for: [expectation], timeout: TESTS_TIME_OUT)
}

// MARK: Search ranking
func testSearchRankingScore() {
let expectation = XCTestExpectation(description: "Search for Books with query")

typealias MeiliResult = Result<Searchable<Book>, Swift.Error>
let query = "Moreninha"

self.index.search(SearchParameters(query: query, showRankingScore: true)) { (result: MeiliResult) in
switch result {
case .success(let response):
let result = response as! SearchResult<Book>
XCTAssertEqual(result.hits.count, 1)
XCTAssertGreaterThan(result.hits[0].rankingScore ?? 0, 0.1)
expectation.fulfill()
case .failure(let error):
dump(error)
XCTFail("Failed to search with testSearchRankingScore")
expectation.fulfill()
}
}

self.wait(for: [expectation], timeout: TESTS_TIME_OUT)
}

func testSearchBoxEncodingWithScore() {
let expectation = XCTestExpectation(description: "Search for Books with query")

let expectedValue = """
{"hits":[{"_rankingScore":0.5,"comment":"A Book from Joaquim Manuel de Macedo","genres":["Novel"],"id":1844,"title":"A Moreninha"}],"processingTimeMs":0,"query":"Moreninha"}
"""

typealias MeiliResult = Result<Searchable<Book>, Swift.Error>
let query = "Moreninha"

self.index.search(SearchParameters(query: query, showRankingScore: true)) { (result: MeiliResult) in
switch result {
case .success(let response):
do {
// the ranking score and time can change for many reasons, of which is not relevant here. we set it to a constant to test the encoding.
response.processingTimeMs = 0
response.hits[0].rankingScore = 0.5
let encoder = JSONEncoder()
encoder.outputFormatting = .sortedKeys
let data = try encoder.encode(response)
XCTAssertEqual(String(decoding: data, as: UTF8.self), expectedValue)
} catch {
XCTFail("Failed to encode search result")
}
expectation.fulfill()
case .failure(let error):
dump(error)
XCTFail("Failed to search with testSearchBoxEncodingWithScore")
expectation.fulfill()
}
}

self.wait(for: [expectation], timeout: TESTS_TIME_OUT)
}

func testSearchBoxEncodingWithoutScore() {
let expectation = XCTestExpectation(description: "Search for Books with query")

let expectedValue = """
{"hits":[{"comment":"A Book from Joaquim Manuel de Macedo","genres":["Novel"],"id":1844,"title":"A Moreninha"}],"processingTimeMs":0,"query":"Moreninha"}
"""

typealias MeiliResult = Result<Searchable<Book>, Swift.Error>
let query = "Moreninha"

self.index.search(SearchParameters(query: query, showRankingScore: false)) { (result: MeiliResult) in
switch result {
case .success(let response):
do {
let encoder = JSONEncoder()
encoder.outputFormatting = .sortedKeys
response.processingTimeMs = 0
let data = try encoder.encode(response)
XCTAssertEqual(String(decoding: data, as: UTF8.self), expectedValue)
} catch {
XCTFail("Failed to encode search result")
}
expectation.fulfill()
case .failure(let error):
dump(error)
XCTFail("Failed to search with testSearchBoxEncodingWithoutScore")
expectation.fulfill()
}
}

self.wait(for: [expectation], timeout: TESTS_TIME_OUT)
}

// MARK: Basic search with finite pagination
func testBasicSearchWithFinitePagination() {
let expectation = XCTestExpectation(description: "Search for Books with finite pagination")
Expand Down Expand Up @@ -466,7 +558,7 @@ class SearchTests: XCTestCase {

XCTAssertEqual(result.limit, limit)
XCTAssertEqual(documents.hits.count, 1)
let book: Book = documents.hits[0]
let book: SearchHit<Book> = documents.hits[0]
XCTAssertEqual("…from Joaquim Manuel de Macedo", book.formatted!.comment!)
expectation.fulfill()
case .failure(let error):
Expand Down Expand Up @@ -494,7 +586,7 @@ class SearchTests: XCTestCase {
self.index.search(searchParameters) { (result: MeiliResult) in
switch result {
case .success(let documents):
let book: Book = documents.hits[0]
let book: SearchHit<Book> = documents.hits[0]
XCTAssertEqual("(ꈍᴗꈍ)Joaquim Manuel(ꈍᴗꈍ)", book.formatted!.comment!)
expectation.fulfill()
case .failure(let error):
Expand Down Expand Up @@ -527,7 +619,7 @@ class SearchTests: XCTestCase {
XCTAssertEqual(result.limit, limit)
XCTAssertEqual(documents.hits.count, 2)

let moreninhaBook: Book = documents.hits.first(where: { book in book.id == 1844 })!
let moreninhaBook: SearchHit<Book> = documents.hits.first(where: { book in book.id == 1844 })!
XCTAssertEqual("A Book from Joaquim Manuel…", moreninhaBook.formatted!.comment!)
expectation.fulfill()
case .failure(let error):
Expand Down Expand Up @@ -878,7 +970,7 @@ class SearchTests: XCTestCase {
XCTAssertEqual(documents.query, query)
XCTAssertEqual(result.limit, limit)
XCTAssertEqual(documents.hits.count, 1)
guard let book: Book = documents.hits.first(where: { book in book.id == 1344 }) else {
guard let book: SearchHit<Book> = documents.hits.first(where: { book in book.id == 1344 }) else {
XCTFail("Failed to search with testSearchFilterWithEmptySpace")
expectation.fulfill()
return
Expand Down

0 comments on commit e3ba51a

Please sign in to comment.