crud icon indicating copy to clipboard operation
crud copied to clipboard

Serialized DTOs contain additional properties from DB entity

Open svorcan opened this issue 4 years ago • 6 comments

I found the following weird behaviour when using CrudOptions.serialize option.

Basically, I am trying to use DTO to hide some internal implementation details which database entities contain. What happens, is that I get the whole database entity serialized to the JSON response, which ruins the whole purpose of using DTO for me.

I tracked it to the transform method of CrudResponseInterceptor, which adds all database properties to the DTO object during mapping, instead of mapping only those actually defined in the DTO.

The proposed fix would be to provide excludeExtraneousValues flag to plainToClass method call as stated in class-transformer docs.

Here is an example:

Controller:

@Crud({
  model: {
    type: MyEntity,
  },
  routes: {
    exclude: ['createManyBase', 'replaceOneBase'],
  },
  dto: {
    create: MyEntityRequest,
    update: MyEntityRequest,
  },
  params: {
    id: {
      field: 'id',
      type: 'uuid',
      primary: true,
    },
  },
  serialize: {
    getMany: MyEntityResponse,
    get: MyEntityResponse,
    create: MyEntityResponse,
    update: MyEntityResponse,
  },
})
@Controller('my-entities')
export class MyEntityController {
  constructor(@Inject(MY_ENTITY_SERVICE) public service: MyEntityService) {}
}

Entity:

import { CreateDateColumn, PrimaryGeneratedColumn, UpdateDateColumn, Column, Entity } from 'typeorm';
import { Expose } from 'class-transformer';

@Entity({ name: 'workspace' })
export class MyEntity {
  @Expose()
  @PrimaryGeneratedColumn('uuid')
  id: string;

  @Expose()
  @CreateDateColumn()
  createdAt: Date;

  @Expose()
  @UpdateDateColumn()
  updatedAt: Date;

  @Expose()
  @Column()
  name: string;
}

DTO:

import { ApiProperty } from '@nestjs/swagger';
import { Expose } from 'class-transformer';

export class MyEntityResponse {
  @Expose()
  @ApiProperty()
  public id: string;

  @Expose()
  @ApiProperty()
  public name: string;
}

Expected response:

{
    "id": "e95e4f35-d906-42a3-a94b-851bc4572a59",
    "name": "My Name"
}

Actual response:

{
    "id": "e95e4f35-d906-42a3-a94b-851bc4572a59",
    "createdAt": "2020-11-11T10:45:45.733Z",
    "updatedAt": "2020-11-11T10:45:45.733Z",
    "name": "My Name"
}

svorcan avatar Nov 11 '20 15:11 svorcan

@svorcan I had similar issue, I've fixed it by adding @Exclude annotation on top of entity. Doing that, class-transformer will ignore all properties except those explicitly annotated with @Expose.

Example:

import { Expose, Exclude } from 'class-transformer';

@Exclude()
@Entity({ name: 'workspace' })
export class MyEntity {
  @Expose()
  @PrimaryGeneratedColumn('uuid')
  id: string;

 ....
}

alesblaznik avatar Nov 13 '20 10:11 alesblaznik

@alesblaznik I figured that out, but that is not a proper solution since you would be mixing DTO logic in the database layer.

And it won't work in case you e.g. provide 2 APIs for those entities, one internal and one external with limited scope.

I'm happy to provide a PR with the solution I described in the first comment if it makes sense to other contributors :)

svorcan avatar Nov 13 '20 11:11 svorcan

Have you tried adding @Exclude to the MyEntityResponse class - I think the default is @Expose?

tbrannam avatar Nov 20 '20 05:11 tbrannam

@tbrannam it may work, but it will couple your DTOs to your DB structure. For each endpoint, you will have a mirror of the DB structure in the DTOs, and each time you add some internal field to the DB entity it will automatically be mapped to all DTOs unless you go through all DTOs and add an exclusion of that field into all of them. This is not very efficient and prone to errors.

I mean, there are several ways to work around this I found, but I think this may be something which is handled by default in the nestjsx-crud.

svorcan avatar Nov 20 '20 11:11 svorcan

@svorcan Would you mind submitting a PR and notify me when submitted? I would like to take a look at the code you wrote for the solution. I may just pull it into my source code if the maintainers are not wanting to change that.

I agree with you that it is an anti-pattern to put the attributes on the entity itself.

aaronscribner avatar Nov 22 '20 16:11 aaronscribner

@svorcan any update?

Right now I have to set @Expose() decorator on every param in my response.dto class, like this

@Exclude()
export class MyEntityResponse {
  @Expose()
  id: string;
  
  @Expose()
  name: string;
}

and then pass this class to serialize prop inside @Crud()

@Crud({
  model: {
    type: MyEntity,
  },
  ...
  serialize: {
    get: MyEntityResponse,
    create: MyEntityResponse,
  },
})

Also, MyEntity has no class-transformer decorators, in terms of layers separation.

vladi-strilets avatar Jul 31 '21 18:07 vladi-strilets