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

Support MongoDBStore #5

Merged
merged 5 commits into from
Sep 26, 2023
Merged

Conversation

JustMangoou
Copy link
Contributor

@JustMangoou JustMangoou commented Sep 26, 2023

This PR adds MongoDB as an option for storing sessions.

closes #4

@maxcountryman
Copy link
Owner

Wonderful! Thank you for this.

@maxcountryman
Copy link
Owner

It looks like we'll want to run cargo +nightly fmt to pass CI.

@maxcountryman
Copy link
Owner

We're getting closer--it looks like the integration tests might not be passing.

@JustMangoou
Copy link
Contributor Author

Yeah, I'm investigating why it does not work, it's been 1hr but still no progress

@maxcountryman
Copy link
Owner

Does it work for you locally?

I'm not too familiar with Mongo unfortunately, but I wonder if there could be some difference between the container's version of Mongo and a local version.

@JustMangoou
Copy link
Contributor Author

Unfortunately, it doesn't work locally. My guess is that Deserializing Object from MongoDB to Rust SessionRecord maybe the cause

@maxcountryman
Copy link
Owner

maxcountryman commented Sep 26, 2023

My guess is that Deserializing Object from MongoDB to Rust SessionRecord maybe the cause

I'm not sure exactly how you're testing, but if you're using the HandleErrorLayer, you can bind BoxError to something like err and dbg!(err) to get a better idea of what's failing.

So something like this:

[tokio::main]
 async fn main() -> Result<(), Box<dyn std::error::Error>> {
     let database_url = std::option_env!("DATABASE_URL").expect("Missing DATABASE_URL.");
     let client = mongodb::Client::with_uri_str(database_url).await.unwrap();

     let session_store = MongoDBStore::new(client, "tower-sessions".to_string());
     let session_service = ServiceBuilder::new()
         .layer(HandleErrorLayer::new(|err: BoxError| async {
             dbg!(err);
             StatusCode::BAD_REQUEST
         }))
         .layer(
             SessionManagerLayer::new(session_store)
                 .with_secure(false)
                 .with_max_age(Duration::seconds(10)),
         );

     let app = Router::new()
         .route("/", get(handler))
         .layer(session_service);

     let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
     axum::Server::bind(&addr)
         .serve(app.into_make_service())
         .await?;

     Ok(())
 }

@JustMangoou
Copy link
Contributor Author

Tests passed on local

@JustMangoou
Copy link
Contributor Author

to_string() screwed up the deserialize step, not sure why but using Binary UUID fix it
image

self.col()
.update_one(
doc! {
"_id": session_record.id().as_uuid()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being able to use the native type seems quite nice! This seems good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I noticed that sqlx providers don't use native uuid (binary), or maybe it needs to be stored as string? I'm not familiar with uuid in sql.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least with Postgres, there's a module you can enable to support UUID as a column type. But I'm not sure if it's available by default these days and so it's not something that's currently being used.

@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Merging #5 (27c5e42) into main (a37f3e9) will increase coverage by 0.31%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main       #5      +/-   ##
==========================================
+ Coverage   58.09%   58.41%   +0.31%     
==========================================
  Files           7        7              
  Lines         210      214       +4     
==========================================
+ Hits          122      125       +3     
- Misses         88       89       +1     
Files Coverage Δ
src/lib.rs 25.00% <100.00%> (+25.00%) ⬆️
src/session.rs 62.22% <0.00%> (-1.42%) ⬇️

@maxcountryman maxcountryman merged commit 2db8622 into maxcountryman:main Sep 26, 2023
13 of 14 checks passed
@JustMangoou JustMangoou deleted the mongo-store branch September 26, 2023 23:38
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

Successfully merging this pull request may close these issues.

Support MongoDB
2 participants