Making indexes generic over the contained document type

[Mubelotix]

I think the way this library handles Document types can be improved.

Current implementation

The type of the Document must be explicitly specified at each request.

let books = client.get_index("books").await?;
let results = books.search().with_query("harry pottre").execute::<Book>().await?;

That is very flexible but do we really need flexibility here? The type of the document will (and should) almost never change at runtime. The current solution may lead to bugs where the actual type of the Index does not match the specified type of the request.

let index = client.get_index("books").await?;
let book_results = index.search().with_query("harry pottre").execute::<Book>().await?;

// some code

let movie_results = index.search().with_query("harry pottre").execute::<Movie>().await?; // Bug! The programmer forgot that the index is containing books and there will be errors at runtime.

Solution

Index should be generic over its contained Document type. That would force the programmer to specify the type of the documents at the index creation.

let books = client.get_index::<Book>("books").await?;
let results = books.search().with_query("harry pottre").execute().await?;

That makes the bug described above impossible and takes advantage of Rust's type checks and inference. Note: We could also add an execute_dynamic method allowing the programmer to overwrite the type of the index (for this request only).

[ppamorim]

@Mubelotix Could be better to create a struct Book that impl a trait Document to retrieve the key from Meilisearch? Like this?

From the library:

trait Document {
  fn key(self) -> &str;
}

From the project:

struct Book {
  let foo: String
}

impl Document for Book {
  fn key(self) -> &str {
    return "books"
  }
}

It's possible to automatically do:

let books = client.get_index::<Book>().await?;

[Mubelotix]

@ppamorim Interesting idea, but that means you would not be able to have multiple indexes containing the same document type.

[ppamorim]

@Mubelotix For this case, make get_index to accept a custom index name and ignore the key from the trait. This case below makes sense for me:

let books = client.get_index::<Book>("Pedro_Book").await?;

Since it has potential to be a exceptional circumstance.

[Mubelotix]

Yes that would work but I think that adding this method to the Document trait is an unnecessary abstraction. It breaks the object hierarchy: an Index contains Documents, a Document cannot choose its Index. A Document should always be handled by its Index and does not need to be aware of its name.

[ppamorim]

If this could be possible or anything similar...

let books = client.get_index::<Book::with_key("Pedro_Book")>().await?;

By retrieving the key of the Book above, it returns Pedro_Book.

[Mubelotix]

That would result in a compiler error. Rust expects a type but this is a function call.

[ppamorim]

I know, that's a hypothetical case if we could do this in Rust.

[ppamorim]

But I still think that the developer should duplicate the structs and name them correctly, it doesn't matter if the struct is similar to another struct. Later on this can be a problem. I have this same situation in my code.

[curquiza]

Hello @Mubelotix, I checked with our CTO, your solution seems to be a good idea from his point of you! Having the type when creating the index is better.