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

ObjectIDs are not returned as strings when .lean() is called with a cache #191

Closed
bruce-bjorklund-sophos opened this issue Apr 3, 2024 · 10 comments

Comments

@bruce-bjorklund-sophos
Copy link

What is returned with a "findOne.cache().lean()" with no cache hydrated
{
_id: '660dd512e8532980bb5084e5'
}

What is returned with a "findOne.cache().lean()" with the cache hydrated:
{
_id: new ObjectId("660dd512e8532980bb5084e5"),
}

I would expect that we would get only JSON back if we called lean().

thoughts?

@bruce-bjorklund-sophos
Copy link
Author

Yeah you are right, it is valid JSON, but what we are expecting is the _id to be a string, not an ObjectID.

I went through your code and it looks like in the Redis driver, and you do a thing where if the value is a string, and a UUID you cast it into a ObjectID.

In Mongoose, if we don't use lean() we get back UUIDs with ObjectID types... but if we use lean() we get back full JSON. So there are no methods like .toJSON(). TBH I think the decision to cast every ObjectID string into a objectID class breaks the implementation of lean()

@ilovepixelart
Copy link
Owner

ilovepixelart commented Apr 4, 2024

Did you check example I gave you in playground mongoose returns _id as object when using lean. This is why I cast to object because lean not returning id as string.

@bruce-bjorklund-sophos
Copy link
Author

bruce-bjorklund-sophos commented Apr 4, 2024

Oh you know what I think it is?

We have the schema where the _id is a string... but we generate an ObjectID for it:

const UserSchema = new Schema<IUser>({ _id: String, organization_id: String });

const newUser = {_id: new ObjectId().toHexString()}

The redis engine will cast any string that is an ObjectID match regardless of the schema definition

@ilovepixelart
Copy link
Owner

ilovepixelart commented Apr 4, 2024

Now thats makes total sense now! Honestly speaking don't do that :) store your id properly, but yeah interesting case

@bruce-bjorklund-sophos
Copy link
Author

Yeah... Yeah I completely agree with you on that this is non-standard - I'm just like, the janitor.

I think the right thing to do would be migrate to the standard, but that LOE is too much. Would be cool if the engine used the schema for type casting but not sure what the LOE would be on that or if its even possible.

@ilovepixelart
Copy link
Owner

ilovepixelart commented Apr 4, 2024

Yep I did this default casting due to aggregate inability to properly hydrate:
Automattic/mongoose#8784 (comment)

And for cases like distinct, will check what we can do, but it's not a trivial task (just removing casting won't cut it)

But if you have sufficient ram on pod/service/vm/metal you can switch to engine: 'memory', this will solve your issue since in memory engine stores object as is without stringify hydrate etc...

@ilovepixelart
Copy link
Owner

ilovepixelart commented Apr 9, 2024

Will link here issue I created in mongoose official repo,
looks like proper hydration is not happening even on Query (only affects populated virtual objects)
Automattic/mongoose#14503

@ilovepixelart
Copy link
Owner

PR for serialization in redis engine: #198

@ilovepixelart
Copy link
Owner

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