kleydon / prisma-session-store

Express session store for Prisma
MIT License
116 stars 18 forks source link

Malformed ObjectID #95

Closed Sunnuld closed 1 year ago

Sunnuld commented 1 year ago

As far as I can tell I cannot get this to work when using MongoDB as the database.

Mongo expects its '_id' parameter to be of type ObjectId (bson), but I have found no way of making dbRecordIdFunction return a valid ObjectId

Why does this package rely on inserting the id field into the database, instead of allowing Prisma/Mongo to generate its own, via @default()?

kleydon commented 1 year ago

Hi @Sunnuld - thanks for reporting this limitation.

I think when the library was created, MongoDB was not yet supported by Prisma. The library has been used with Postgres and SQL Lite - but I'm just now realizing I don't know if anyone is using it with MongoDB yet.

Why does this package rely on inserting the id field into the database, instead of allowing Prisma/Mongo to generate its own, via @default()?

I'm pretty limited time-wise right now, and may not get a chance to explore this properly. If you feel at all like trying this out though, and perhaps posting a PR if it works, I'll bet there are others who would appreciate the result.

SunburntRock89 commented 1 year ago

Mongo expects its '_id' parameter to be of type ObjectId (bson)

There's actually no such restriction, as long as you @map the field to _id it'll work.

You get a different issue when you try to use the id field as _id (can't quite remember what it is, but it's described in #83) A (completely untested but working enough to bypass the issue) solution can be to just define an Object ID as the main id.

model Session {
  objId     String   @id @map("_id") @db.ObjectId @default(auto())
  id        String   @unique
  sid       String   @unique
  data      String
  expiresAt DateTime
}
Sunnuld commented 1 year ago

Thanks for the replies! @kleydon - I hadn't considered it, but looking now; it does seem that MongoDB has only recently been officially supported!

@SunburntRock89 - I had tried literally everything, except maybe the concept of saving the _id in a separate field to the id that this package passes 🙈

This was as far as got before I edited the source files (untested) to allow this

model Session {
  id           String   @id @map("_id") @default(auto()) @db.ObjectId 
  sid          String   @unique
  data         String
  expiresAt DateTime
}
kleydon commented 1 year ago

This was as far as got before I edited the source files (untested) to allow this

model Session { id String @id @map("_id") @default(auto()) @db.ObjectId sid String @unique data String expiresAt DateTime }

Curious: Is this working for you so far?

Sunnuld commented 1 year ago

@kleydon - No, that was as far as I got. Using that layout still produces the malformed ObjectId Error. I believe what SunBurntRock has said would be a usable workaround, just means adding an extra 'objId' column into the database (making for three id fields; _id, id & sid )!.

I brute-forced a fix by removing:

id: this.dbRecordIdIsSessionId ? sid : this.dbRecordIdFunction(sid),

From the data object @line 391, prisma-session-store.ts,

I haven't had time to test this, beyond successfully writing/reading sessions from the database

kleydon commented 1 year ago

@Sunnuld Ok - good to know.

kleydon commented 1 year ago

@SunburntRock89 - Do you think this can be closed, now that PR #96 is merged, or are there other details that need to be thought through?

SunburntRock89 commented 1 year ago

@kleydon In my (limted) testing you don't need a specific object ID field anymore. In fact I think I ended up encountering another, bigger issue because of using the workaround whilst testing this change.

Fairly sure this is resolved.

kleydon commented 1 year ago

👍