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

Malformed ObjectID #95

Closed
Sunnuld opened this issue Jun 24, 2022 · 9 comments
Closed

Malformed ObjectID #95

Sunnuld opened this issue Jun 24, 2022 · 9 comments

Comments

@Sunnuld
Copy link

Sunnuld commented Jun 24, 2022

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

kleydon commented Jun 24, 2022

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

SunburntRock89 commented Jun 24, 2022

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

Sunnuld commented Jun 24, 2022

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

kleydon commented Jun 24, 2022

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

Sunnuld commented Jun 25, 2022

@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
Copy link
Owner

kleydon commented Jun 25, 2022

@Sunnuld Ok - good to know.

@kleydon
Copy link
Owner

kleydon commented Jun 25, 2022

@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
Copy link
Contributor

SunburntRock89 commented Jun 25, 2022

@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
Copy link
Owner

kleydon commented Jun 25, 2022

👍

@kleydon kleydon closed this as completed Jun 25, 2022
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

3 participants