Extend Document trait with function to get index
amaihoefner opened this issue · 3 comments
Description
Add a index()
function to the Document
trait to make it easier to get access to the index.
Basic example
Right now, something that implments Into<String>
needs to be provided when one wants to get access to an index:
let movie_index = client.index("movies");
With this change one could instead write:
let movie_index = Movie::index(&client);
(Provided Movie
implements the Document
trait)
This way accessing an index becomes less prone to typos and one would'nt have to keep track of the name of the index, only the associated struct.
Other
It might also make sense to add a constant INDEX_STR
to the trait.
This would have the following benefits:
- it could be used to provide a default implementation for the proposed
Document::index
function - it could be used when implementing
Document::generate_index
by hand
Adding this to the derive macro would also not require much effort, as the index name is already being extracted there.
I will provide a draft merge request with these changes to act as a base for further discussion.
Hey, I get why you would want to do that.
But a Document
type should not be tied to an index.
You should be able to use the same type over multiple indexes.
wdyt @bidoubiwa?
Hm, I admit, I had not originally considered that.
But when we implement the Document
trait for a type, don't we already create a sort of "binding" to a specific index because of the generate_index
function of the trait?
And we would still be able to use the type (including the settings from generate_settings
) for other indices (although the primary_key would not be included with those settings).
But yes, we may want to mention in the documentation of the proposed index
function, that it refers to the same "default" index that generate_index
is referring to.
Thinking about it, there might be a nice way of allowing multiple indices with a const generic str argument. So one could use something like Movie::index::<"my_index">(&client)
.
But those currently require nightly and the incomplete adt_const_params
feature.
And that would work against the idea of this issue, by again requiring the index to be specified.
Of course, there might also be another, better way of accomplishing that.
Hey @amaihoefner
thanks for raising this issue.
But a Document type should not be tied to an index.
It is already the case, currently, the index_name is generated by snake_casing the Document structure's name. For example the Movie structure will have movie
as an index_name.
This confusion is due to the trait being very badly named. It helps generate the settings based on the schema of the document. Thus it's more an Index configuration/schema than a Document.
My suggestion would be:
Rename the Document
trait to IndexConfig
or something better (if it's ok for you to do that in your PR).
In your PR, change
fn index(client: &crate::client::Client) -> Index {
to
fn index(client: &Client) -> Index {