While this test is sufficient to see that the server has indeed understood that this is a federated search and not a regular multi-search, there should be separate tests to see if the server returns what you would expect under these circumstances.

In addition we should check that limit and offset are sent properly and work as expected.

Does that meet your expectations?

Actually, from what I understood here @Nymuxyzo you would need to make two tests:

• one asserting the inexistence of the presence of the param federation in the response
• one asserting that you explicitly called the multi_search with a federation request, since the doc says a non-null value the value could be multi_search(data, federation: true, federation_options: {....}) # where federation options are not required

https://www.meilisearch.com/docs/learn/multi_search/multi_search_vs_federated_search#what-is-federated-search

@brunoocasali I'm sorry, but I don't understand the statement. When I look at the added test in the PHP SDK (tests/Endpoints/MultiSearchTest.php), I recognize my pattern to a very large extent.

I would very much like to bring this issue to a good end.

I've also read and reread

one asserting the inexistence of the presence of the param federation in the response
one asserting that you explicitly called the multi_search with a federation request, since the doc says a non-null value the value could be multi_search(data, federation: true, federation_options: {....}) # where federation options are not required

and I'm sorry @brunoocasali I can't make sense of it xD

I think this might be a typo:

one asserting the inexistence of or the presence of the param federation in the response

but the presence of _federation is already looked for in the test.

Meanwhile this:

multi_search(data, federation: true, federation_options: {....}) # where federation options are not required

I'm not sure I'm following with the distinction between federation and federation_options, there is only one federation option, which can either be an object of options or a truthy value to enable it with defaults.

In any case, this should work fine as long as we don't mess with it, ruby is dynamically typed and there is nothing preventing someone from passing a boolean as federation.

Yeah, @ellnix, I agree with your previous message; I will point out the change in another comment.

Actually, from what I understood here @Nymuxyzo you would need to make two tests:

• one asserting the inexistence of the presence of the param federation in the response
• one asserting that you explicitly called the multi_search with a federation request, since the doc says a non-null value the value could be multi_search(data, federation: true, federation_options: {....}) # where federation options are not required

https://www.meilisearch.com/docs/learn/multi_search/multi_search_vs_federated_search#what-is-federated-search

I am not a fan personally of two positional arguments with almost no explanation as to what each of them does in the non-existent documentation. Especially since in the reference, federation is listed first (I'm guessing it's alphabetical)

I would prefer if we took the js approach and used query: and federation: keyword arguments.

This would mean breaking code that was written to use multi_search before this PR. But, we can avoid that if we have one data=nil positional argument which we take as a query, and warn the user on its use:

def foo(bad_arg=nil, query: {}, federation: nil)
  if bad_arg
    puts "Bad arg is bad!"
  end
  
  query.update(bad_arg)
  
  {
    query: query,
    federation: federation
  }
end

foo({ q: 'hello world'}, federation: true)
# Bad arg is bad!
# => {:query=>{:q=>"hello world"}, :federation=>true}

Obviously, please don't use puts (Utils.soft_deprecate! might be appropriate) to throw warnings, and name the arguments appropriately if you choose to do this.

I've also read and reread

one asserting the inexistence of the presence of the param federation in the response
one asserting that you explicitly called the multi_search with a federation request, since the doc says a non-null value the value could be multi_search(data, federation: true, federation_options: {....}) # where federation options are not required

and I'm sorry @brunoocasali I can't make sense of it xD

I think this might be a typo:

one asserting the inexistence of or the presence of the param federation in the response

but the presence of _federation is already looked for in the test.

Meanwhile this:

multi_search(data, federation: true, federation_options: {....}) # where federation options are not required

I'm not sure I'm following with the distinction between federation and federation_options, there is only one federation option, which can either be an object of options or a truthy value to enable it with defaults.

In any case, this should work fine as long as we don't mess with it, ruby is dynamically typed and there is nothing preventing someone from passing a boolean as federation.

federation_options should be transformed to camel case with Utils.transform_attributes.

To be clear, it wasn't necessary when you first started this PR, but now there are 2 new options (facetsByIndex and mergeFacets) which won't work if you pass them as snake_cased symbols.

It would be nice if these values were not the same as the defaults, but it's not a big problem.

So, basically what @ellnix suggested is to do something like:

and I agree with that :)