Hard limit the number of results returned by a search

[Kerollmops]

This PR fixes #2133 by hard-limiting the number of results that a search request can return at any time. I would like the guidance of @MarinPostma to test that, should I use a mocking test here? Or should I do anything else?

I talked about touching the nb_hits value with @qdequele and we concluded that it was not correct to do so.

Could you please confirm that it is the right place to change that?

[MarinPostma]

I don't think that this is how it should be done.

what if I ask for 1 document at offset 10000?

I think we need to pass the following params to milli:

offset = min(offset, HARD_RESULT_LIMIT);
limit = min(limit, HARD_RESULT_LIMIT - offset);

wdyt?

[Kerollmops]

Ahha yeah, I'm dumb, you are absolutely right. That's probably due to jet lag. I will update my PR accordingly. Thanks!

[MarinPostma]

@Kerollmops, for testability, you could in fact do a mock here, but maybe the simple option would be to make a function like compute_offset_limit(offset: usize, limit: usize) -> (usize, uszie), and unit test that function instead, wdyt?

If we wanted to make integration tests, having a way to lower the value of MAX_RESULT_LIMIT would be useful so we don't have to create search results with 1000 hits, but who should own this setting is another question...

[Kerollmops]

@Kerollmops, for testability, you could in fact do a mock here, but maybe the simple option would be to make a function like compute_offset_limit(offset: usize, limit: usize) -> (usize, uszie), and unit test that function instead, wdyt?

I don't think that we need to move this simple logic in another function, it is not used in any other place.

If we wanted to make integration tests, having a way to lower the value of MAX_RESULT_LIMIT would be useful so we don't have to create search results with 1000 hits, but who should own this setting is another question...

I just added two integration tests that check that a placeholder or normal search is limited and even changing the offset to a big number doesn't expose documents past our hard 1000 limit. Thank you for that by the way 😂

[curquiza]

Ok for me ✅

Wait for the final Marin's approval for the merge

[Kerollmops]

Thank you for the reviews, merging now!
bors merge

[bors]

Build succeeded:

• Run Clippy
• Run Rustfmt
• Run tests in debug
• Tests on macos-latest
• Tests on ubuntu-18.04
• Tests on windows-latest