typestack / class-transformer

Decorator-based transformation, serialization, and deserialization between objects and classes.
MIT License
6.81k stars 500 forks source link

fix: object id is transformed into a different one when use plainToClass #991

Open x-shadow-x opened 2 years ago

x-shadow-x commented 2 years ago

Description

object id is transformed into a completely different one when we add excludeExtraneousValues: true in plainToClass function

Minimal code-snippet showcasing the problem

import { Expose, plainToClass, Transform } from "class-transformer"
import { IsMongoId } from "class-validator"
import { ObjectId } from "mongodb"
import mongoose from "mongoose"

export class SampleWithObj {
  @Transform(({ obj }) => String(obj.id), { toClassOnly: true })
  @Expose()
  @IsMongoId()
  id: string

  static fromObject(obj: unknown): SampleWithObj {
    if (!obj) return null
    return plainToClass(SampleWithObj, obj)
  }
}

export class SampleWithValue {
  @Transform(({ value }) => String(value), { toClassOnly: true })
  @Expose()
  @IsMongoId()
  id: string

  static fromObject(obj: unknown): SampleWithValue {
    if (!obj) return null
    return plainToClass(SampleWithValue, obj)
  }
}

const printCls = (obj: { id: ObjectId }) => {
  console.log("plain obj: ", obj)
  console.log("deserialized obj: ", { id: String(obj.id) })

  console.log("============= test result ============")

  console.log("obj with String(): ", SampleWithObj.fromObject({ ...obj }))

  console.log("value with toString(): ", SampleWithValue.fromObject({ ...obj }))
}

const obj = { id: new mongoose.Types.ObjectId("61980e80fc741dcac3c60a83") }

printCls(obj)

pls also check this repo to run the code and check the result. https://github.com/x-shadow-x/class-transform-plainToClass-issue

Expected behavior

should get the correct string of objectId.

Actual behavior

image

alexreal1314 commented 2 years ago

I am encountering the same issue when using plainToClass method, all objectIds are changed. Any solution to the issue?

this happened after upgrading to mongoose 6.0.11.

feelgood-interface commented 2 years ago

This might be a similar issue https://github.com/typestack/class-transformer/issues/879

pacificescape commented 2 years ago

image Still the same

jaytonic commented 2 years ago

Any workaround? I've the same issue when using class-transformer within NestJs. I'm using mongoose 6.1.7 and class-transformer 0.5.1

Not sure if it can helps other foxes, but I'm currrently using

  @Transform((value) => value.obj._id.toString())
  _id: ObjectId; 

as workaround

huyphamwork commented 2 years ago

@jaytonic It's good working for me. Thanks!

chenshuiluke commented 1 year ago

Just encountered this as well

dima-mamaev commented 1 year ago

+1 it returns new ObjectId instead of provided one

tonynguyenit18 commented 1 year ago

Previously we can omit @Type(() => String) But I realised that it case this issue, we need to add @Type(() => String) when ever we use @Transform with plainToClass to transform ObjectId to String

lclarkg18 commented 1 year ago

@jaytonic and @tonynguyenit18 are right, @Type(() => String) works a charm. In my case this is a return type so transforming into a string is actually probably better than passing around a ObjectId because in any request it's going to need to come in as a string anyway.

It would be interesting to get insights from someone more versed in the workings of class transformer but something tells me this isn't an issue with the library but rather with our usage. If we put an objectId in, my guess is class transformer just creates a new one from the whole object (not just the string id) and mongoDB may not have a good way of dealing with this? @Type(() => String) works though because ObjectId will have overridden the toString() function to just return the id. I'm making a lot of assumptions here though.

If you really did want to return the id, you could do this:

  @Transform((value) => new Types.ObjectId(value.obj._id.toString()))
  _id: Types.ObjectId;

I have tested it and it will create an objectId with the same Id, but think twice wether you really want to be doing this, it seems a bit pointless to me as the only advantage that you might have from using an ObjectId instead of the string is that you store generation time, but that's going to be reset every time you run this anyway.

rkulinski commented 3 months ago

There are workarounds for this, but isn't this problematic?

  1. I don't want all developers in a team remember that this field needs special treatment. Serialization without this library wasn't causing that problem.
  2. This can lead to very serious and not obvious bugs - that's how I even found out about this problem.
  3. Even if this is documented I believe most people will find out about this same way I did (via bug).

From practical stand of a point I believe this should produce correct output without any additional adjustments. Or at least throw an error, letting my know that this will produce unexpected output.

mareksuscak commented 2 months ago

This affects Mongoose 6 and 7 based on my testing. Something's changed in v6 (broken) and then again in v8 (fixed). I can't put my finger on it as I don't see any change that'd apparently cause this behavior.