rustapi icon indicating copy to clipboard operation
rustapi copied to clipboard

Lack of use of State

Open Drvanon opened this issue 3 years ago • 4 comments

Hi, This is a super awesome project and it has been incredibly useful for me to structure the web API part of my project. Thank you so much.

I have a few comments in mind, but one of the first things that jumped out at me was your use of the Singleton pattern for settings and the database connection. I don't really like the singleton pattern (though I am sure you took care of the strongest arguments against it). So I was looking into what other options there are in the frameworks that you use, and once I learned more about tower, I expected it to be able to deliver the database/settings as a middleware quite nicely. I even found that a database connection is the example provided for state in the axum documentation.

I have a couple more thoughts that I can make issues for if you would like me to, but I thought I'd kick it off with this one.

Drvanon avatar Jan 03 '23 21:01 Drvanon

Hi @Drvanon, Yeah, I know ideally the better pattern is not to have a singleton connection. I actually started not using the singleton, I used two approaches before the singleton.

  1. Create the database connection on the main function, then store it on the state and access the state connection on each route

  2. Create the database connection and instantiate the models with this connection on main, then store this on the state and access it from each route. You can see an example from this patter here: main and the route example. Note that this example is using actix web, but migrating this part to axum is really easy.

These two approaches worked to some extent, but at some point it became pretty annoying sending the connection over from the route to specifics methods that needed to use the database connection. I think ideally the route function would not handle all the business logic, so I needed to always send the connection a couple levels deep to other functions.

I am used to using Mongoose + Nodejs, Mongoose allows having a singleton instance and use it everywhere. Considering that my use cases always use a single database having the singleton instance removes a lot of complexity and adds a nicer developer experience, this is of course IMO.

Anyway, I think you are in the right track there, take a look at this example from the framework I am using in the starter. The approach matches what you are looking for. I think you need to:

  • Update the src/utils/models.rs file to receive a mongo connection
  • Instantiate a mongo connection the way the wither library suggests and the store this connection on the axum state like you said
  • Get the mongo connection from each route and call the model methods passing the mongo connection

I hope this helps, if not let me know, and we can work on a specific code example / POC.

ndelvalle avatar Jan 04 '23 12:01 ndelvalle

Hi, thanks for your response! I think personally I prefer "correctness" over developer experience (it's not a phase, mom!), so I think I am going to try to apply it for myself like that. I am curious to see how awful it is going to be XD.

Drvanon avatar Jan 04 '23 13:01 Drvanon

So, I continued to mess around and I kind of like what I have for state right now. It feels "rustic" to me, though feel free to disagree.

use async_trait::async_trait;
use bson::{doc, oid::ObjectId, Document};
use mongodb::{self, Cursor, Database};
use serde::{de::DeserializeOwned, Deserialize, Serialize};

#[derive(Serialize, Deserialize)]
pub struct Model<Data>
where
    Data: Sized + Serialize + ModelData + Send + Sync,
{
    #[serde(rename = "_id")]
    pub id: ObjectId,
    #[serde(flatten)]
    inner: Data,
}

impl<Data> Model<Data>
where
    Data: Sized + Serialize + DeserializeOwned + ModelData + Send + Sync + Unpin,
{
    /// Apply a function to the contained data and store it to the server.
    pub async fn try_apply<F>(self, db: Database, f: F) -> Result<Self, Data::Error>
    where
        F: FnOnce(Data) -> Result<Data, Data::Error>,
    {
        let inner = f(self.inner)?;

        db.collection::<Self>(Data::COLLECTION_NAME.as_ref())
            .update_one(
                doc! {
                    "_id": self.id
                },
                bson::to_document(&inner).expect("Could not convert model inner to bson."),
                None,
            )
            .await?;

        Ok(Self { inner, ..self })
    }

    /// Gets a reference to the contained data.
    pub fn get_ref(&self) -> &Data {
        &self.inner
    }

    /// Find an item in the collection with the id.
    pub async fn find_by_id(db: Database, id: ObjectId) -> Result<Option<Self>, Data::Error> {
        db.collection(Data::COLLECTION_NAME.as_ref())
            .find_one(doc! { "_id": id}, None)
            .await
            .map_err(|e| e.into())
    }

    /// Finds an item with the doc
    pub async fn find_by(db: Database, doc: Document) -> Result<Cursor<Self>, Data::Error> {
        db.collection(Data::COLLECTION_NAME.as_ref())
            .find(doc, None)
            .await
            .map_err(|e| e.into())
    }
}

#[async_trait]
pub trait TryIntoModel: ModelData + Serialize + Send + Sync {
    /// RAII version of saving a model.
    async fn try_into_model(self, db: Database) -> Result<Model<Self>, Self::Error> {
        db.collection::<Self>(Self::COLLECTION_NAME)
            .insert_one(&self, None)
            .await
            .map_err(|e| -> Self::Error { e.into() })
            .map(|res| Model {
                id: res
                    .inserted_id
                    .as_object_id()
                    .expect("Did not receive object id from database."),
                inner: self,
            })
    }
}

pub trait ModelData: Sized {
    const COLLECTION_NAME: &'static str;
    type Error: From<mongodb::error::Error> + Send;
}

Drvanon avatar Jan 06 '23 21:01 Drvanon

@Drvanon do you mind posting a usage example with this struct and trait?

Also, I have on my to-do list to add some state to the Axum router, at least a dummy date to have it as an example. I was wondering if you mind adding a basic implementation. If you can't, I will add it next week. I should probably also update axum version because the way state is used changed recently.

ndelvalle avatar Jan 20 '23 22:01 ndelvalle