elastic / elasticsearch-rs

Official Elasticsearch Rust Client
https://www.elastic.co/guide/en/elasticsearch/client/rust-api/current/index.html
Apache License 2.0
702 stars 72 forks source link

[ENHANCEMENT] Pass reference to Elasticsearch instance #36

Closed russcam closed 4 years ago

russcam commented 4 years ago

Each builder struct currently takes ownership of Elasticsearch, with instantiation of a builder struct from the root or a namespace client accepting a clone of the Elasticsearch instance on which the associated function is called. For example, in the case of search

#[derive(Clone, Debug)]
#[doc = "Builder for the [Search API](https://www.elastic.co/guide/en/elasticsearch/reference/master/search-search.html). Returns results matching a query."]
pub struct Search<'a, B> {
    client: Elasticsearch,
    parts: SearchParts<'a>,
    // ....
}

impl<'a, B> Search<'a, B>
where
    B: Body,
{
    #[doc = "Creates a new instance of [Search] with the specified API parts"]
    pub fn new(client: Elasticsearch, parts: SearchParts<'a>) -> Self {
        Search {
            client,
            parts,
            // ...
         }
    }
}

impl Elasticsearch {
    #[doc = "Returns results matching a query."]
    pub fn search<'a>(&self, parts: SearchParts<'a>) -> Search<'a, ()> {
        Search::new(self.clone(), parts)
    }
}

With an instance of Elasticsearch now instantiated with a Transport that has a ConnectionPool of Connections with which to make API calls to Elasticsearch, it is desirable for builder structs to share the same ConnectionPool, such that in the future, when other ConnectionPool implementations can refresh the collection of connections, this would be reflected in all builders.

Possible implementations

Same lifetime as SearchParts<'a>

Making Elasticsearch now a reference, &Elasticsearch, requires giving it an explicit lifetime. One implementation of this would be to give it the same lifetime as SearchParts<'a>

pub struct Search<'a, B> {
    client: &'a Elasticsearch,
    parts: SearchParts<'a>,
    // ....
}

impl<'a, B> Search<'a, B>
where
    B: Body,
{
    pub fn new(client: &'a Elasticsearch, parts: SearchParts<'a>) -> Self {
        Search {
            client,
            parts,
            // ...
         }
    }
}

impl Elasticsearch {
    pub fn search<'a>(&'a self, parts: SearchParts<'a>) -> Search<'a, ()> {
        Search::new(&self, parts)
    }
}

New lifetime

An alternative is introducing a new lifetime for &Elasticsearch

pub struct Search<'a, 'b, B> {
    client: &'a Elasticsearch,
    parts: SearchParts<'b>,
    // ...
}

impl<'a, 'b, B> Search<'a, 'b, B>
where
    B: Body,
{
    pub fn new(client: &'a Elasticsearch, parts: SearchParts<'b>) -> Self {
        Search {
            client,
            parts,
            // ...
        }
    }
}

pub fn search<'a, 'b>(&'a self, parts: SearchParts<'b>) -> Search<'a, 'b, ()> {
    Search::new(&self, parts)
}

I think that &Elasticsearch should have a different lifetime to *Parts<'a> (second implementation) because it can be alive for a different (longer) scope than both *Parts<'a> and the returned builder struct.