stephenh / ts-proto

An idiomatic protobuf generator for TypeScript
Apache License 2.0
2.13k stars 345 forks source link

required field validation not working for nestjs grpc microservice #466

Closed AmreeshTyagi closed 2 years ago

AmreeshTyagi commented 2 years ago

Request made to grpc method is not doing class validation nor default schema validation.

export class CreateTodoDto {
  @IsString()
  @IsNotEmpty()
  title: string;
  description: string;
}

controller.js

@Controller('todo-grpc')
@TodoServiceControllerMethods()
export class TodoGrpcController implements TodoServiceController {
  constructor(private readonly todoService: TodoService) {}

  create(
    request: CreateTodoDto,
  ): CreateTodoResponse | Promise<CreateTodoResponse> {
    console.log('call service');
    return this.todoService.create(request);
  }

Request using grpc client

Request body:

{
 description: 'Test description'
}

Current behaviour

console log is executed & service method is also executed without validation.

Expectation:

Request should throw validation error and should not hit service method.

stephenh commented 2 years ago

@AmreeshTyagi huh, this:

export class CreateTodoDto {
  @IsString()
  @IsNotEmpty()
  title: string;
  description: string;
}

Doesn't even look like ts-proto output. ts-proto does not have any annotations.

Did you mean to file this in NestJS?

Some users of ts-proto have extended ts-proto to generate output for using in NestJS's grpc services, but other than generating the DTOs, ts-proto doesn't do anything else regarding processing/validating input.

AmreeshTyagi commented 2 years ago

Yes. It is not generated by ts-proto. Following interface was generated by ts-proto.

proto

message CreateTodoRequest {
    string title = 2;
    optional string description =3;
}
export interface CreateTodoRequest {
  title: string;
  description?: string | undefined;
}

I have also checked this on stackoverflow and got my answer. required field has been removed from proto3 . I will check again & see if this issue can be raised on nextjs repo.

First link from comment has also mentioned the same as follows:

The right answer is for applications to do validation as-needed in application-level code. If you want to detect when a client fails to set a particular field, give the field an invalid default value and then check for that value on the server. Low-level infrastructure that doesn’t care about message content should not validate it at all.

Thanks @stephenh for the response. Closing this issue now as it is not related to ts-proto.