feat: add mongodb session store #1804
Conversation
belfhi
changed the title
belfhi
force-pushed
the
add-mongodb-session-store
branch
from
7bdd0a1 to
8ebf888
Compare
belfhi
force-pushed
the
add-mongodb-session-store
branch
from
8ebf888 to
e80cc61
Compare
| store: MongoStore.create({ | ||
| mongoUrl, // MongoDB connection string | ||
| collectionName: "sessions", // Collection name for storing sessions | ||
| ttl: 24 * 60 * 60, // Session TTL (24 hours) |
| if (expressSessionSecret) { | ||
| app.use( | ||
| session({ | ||
| secret: expressSessionSecret, | ||
| resave: false, | ||
| saveUninitialized: true, | ||
| store: MongoStore.create({ | ||
| mongoUrl, // MongoDB connection string | ||
| collectionName: "sessions", // Collection name for storing sessions |
There was a problem hiding this comment.
I think "sessions" is the default, so maybe we can avoid specifying it here?
| if (expressSessionSecret) { | ||
| app.use( | ||
| session({ | ||
| secret: expressSessionSecret, | ||
| resave: false, | ||
| saveUninitialized: true, | ||
| store: MongoStore.create({ |
There was a problem hiding this comment.
shall we leave space for choosing if to use mongoStore or in memory one?
There was a problem hiding this comment.
+1
I would add a property in configuration to select the current behavior or session in mongodb.
| @@ -1,4 +1,5 @@ | |||
| import session from "express-session"; | |||
| import MongoStore from "connect-mongo"; | |||
There was a problem hiding this comment.
I wonder if we need to have both mongoose and connect-mongo?
Can we achieve the same functionality just with mongoose?
| store: MongoStore.create({ | ||
| mongoUrl, // MongoDB connection string | ||
| collectionName: "sessions", // Collection name for storing sessions | ||
| ttl: 24 * 60 * 60, // Session TTL (24 hours) | ||
| }), | ||
| cookie: { | ||
| secure: true, | ||
| }, |
There was a problem hiding this comment.
According to this post, it is possible to have only one connection to the database.
https://stackoverflow.com/questions/66731669/mongoose-and-connect-mongo
Can you look into it and see if we can reduce duplication?
There was a problem hiding this comment.
There is this mongoose example in the connect-mongo package (https://github.com/jdesboeufs/connect-mongo/blob/master/example/mongoose.js). But as we use the wrapper for mongoose @nestjs/mongoose then there might be some way to get the connection in a "nestjs" way. So I guess the idea is to reuse the existing mongoose connection if possible here instead of creating a new connection for the connect-mongo.
|
I'm second to nitrosx's comment, to avoid 2 db connections. |
|
@belfhi do you have any update on this PR? Thank you so much |
resolves #1803