Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of use of State #174

Open
Drvanon opened this issue Jan 3, 2023 · 4 comments
Open

Lack of use of State #174

Drvanon opened this issue Jan 3, 2023 · 4 comments

Comments

@Drvanon
Copy link

Drvanon commented Jan 3, 2023

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.

@ndelvalle
Copy link
Owner

ndelvalle commented Jan 4, 2023

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.

@Drvanon
Copy link
Author

Drvanon commented Jan 4, 2023

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
Copy link
Author

Drvanon commented Jan 6, 2023

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;
}

@ndelvalle
Copy link
Owner

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants