grpc / grpc-web

gRPC for Web Clients
https://grpc.io
Apache License 2.0
8.45k stars 760 forks source link

Why optional message types compile inconsistently? #1413

Closed sunny1d closed 3 months ago

sunny1d commented 3 months ago

Given a proto file like

syntax = "proto3";
import "google/protobuf/empty.proto";
import "google/protobuf/timestamp.proto";

service ProjectService {
  rpc UpdateProject(Project.Request) returns (google.protobuf.Empty);
}

message Project {
  message Request {
    google.protobuf.Timestamp update_time = 1;
    optional bool has_right = 2;
    optional string content = 3;
    optional uint32 percent = 4;
  };
}

Running the generator with options --js_out=import_style=commonjs,binary:. --grpc-web_out=import_style=typescript,mode=grpcwebtext:. in the generated _pb.d.ts file ,The parameters of setUpdateTime are optional that are not the same as the others setXXX . Why are other fields compiled with parameters that are required?

 //  _pb.d.ts file
  export class Request extends jspb.Message {
    getUpdateTime(): google_protobuf_timestamp_pb.Timestamp | undefined;
    setUpdateTime(value?: google_protobuf_timestamp_pb.Timestamp): Request; // value is optional
    hasUpdateTime(): boolean;
    clearUpdateTime(): Request;

    getHasRight(): boolean;
    setHasRight(value: boolean): Request;  // value is required
    hasHasRight(): boolean;
    clearHasRight(): Request;

    getContent(): string;
    setContent(value: string): Request;   // value is required
    hasContent(): boolean;
    clearContent(): Request;

    getPercent(): number;
    setPercent(value: number): Request;   // value is required
    hasPercent(): boolean;
    clearPercent(): Request;

  }
sampajano commented 3 months ago

Hi!

I think the difference is that "google.protobuf.Timestamp" is a message type so that it would be possible to set an "empty" message, but it's not the same for primitive types.

See: https://github.com/protocolbuffers/protobuf-javascript/blob/main/docs/index.md#singular-message-fields

Hope that makes sense!

sunny1d commented 3 months ago

Got it! Many thanks. @sampajano I have another question and need help.

The requirement I encountered was to submit only the values of fields whose values had changed.

For example (based on the proto file above): The "content" value and "percent" value have changed, and the values of the remaining fields remain unchanged. At this time, only the changed fields "content", "percent", are submitted.

Is there any solution to solve this kind of problem?

sampajano commented 3 months ago

@sunny1d You're welcome! Glad it helped! :)

I have another question and need help.

The requirement I encountered was to submit only the values of fields whose values had changed.

For example (based on the proto file above): The "content" value and "percent" value have changed, and the values of the remaining fields remain unchanged. At this time, only the changed fields "content", "percent", are submitted.

This is more of a protobuf question, but i think since all these fields are already optional, you have 2 options:

  1. Call clone() on the original proto and make corresponding changes, then send the full updated proto to server.

  2. Optionally set only the fields of the proto which needs update (essentially using this proto as a "diff"), and on server side for each field check if the field is set, and process accordingly.

Hope that helps :)

sunny1d commented 3 months ago

@sampajano I'm confused.

  1. Call clone() on the original proto and make corresponding changes, then send the full updated proto to server.

I don't understand. Do you have more details?

  1. Optionally set only the fields of the proto which needs update (essentially using this proto as a "diff"), and on server side for each field check if the field is set, and process accordingly.

How to Modify Fields to Default Values. According to the protobuf Default Values ,once a message is parsed there’s no way of telling whether a field was explicitly set to the default value (for example whether a boolean was set to false) or just not set at all

sampajano commented 3 months ago

@sampajano I'm confused.

@sunny1d no problem! I misunderstood about your original comment, so the first option i gave probably is irrelevant (it's about first cloning the entire proto then modifying necessary fields).

  1. Call clone() on the original proto and make corresponding changes, then send the full updated proto to server.

I don't understand. Do you have more details?

  1. Optionally set only the fields of the proto which needs update (essentially using this proto as a "diff"), and on server side for each field check if the field is set, and process accordingly.

How to Modify Fields to Default Values. According to the protobuf Default Values ,once a message is parsed there’s no way of telling whether a field was explicitly set to the default value (for example whether a boolean was set to false) or just not set at all

Ah that's a good point..

From my understanding, there isn't a way to distinguish between a default value (e.g. 0), with an explicitly set 0.

So if that's what you need, you could maybe use a oneof to achieve that.

Or, maybe never allow a value to be "updated" to the defaults in protobuf (but rather if you need to unset it you can set it to -1 or something). Not sure if that makes sense to you.. :)