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

Naming convention for function names doesn't make sense #117

Open
Sereger13 opened this issue Jan 22, 2024 · 4 comments
Open

Naming convention for function names doesn't make sense #117

Sereger13 opened this issue Jan 22, 2024 · 4 comments
Assignees

Comments

@Sereger13
Copy link

The guideline here https://protobuf.dev/programming-guides/style/#services suggests that RPC parameter names should mirrow RPC method names:
service FooService {
rpc GetSomething(GetSomethingRequest) returns (GetSomethingResponse);
rpc ListSomething(ListSomethingRequest) returns (ListSomethingResponse);
}
This makes it really inconvenient to follow when you want to re-use SomethingRequest/SomethingResponse in two different methods. Could you please change the guideline to remove Get/List prefixes, otherwise this leads to artificial limitation that each RPC function has to have a unique parameter type, which isn't mentioned anywhere in the documentation.

@Logofile Logofile self-assigned this Feb 2, 2024
@Logofile
Copy link
Member

Logofile commented Feb 2, 2024

I've forwarded your question to the folks on my team who are working on new style guidelines to get their input on your request.

@googleberg
Copy link
Member

Hello @Sereger13,

Actually, we recommend that you create a unique request and response proto for each RPC method. Discovering later on that you need to diverge the top-level request or response can be prohibitively expensive. This includes "empty" responses -- create a unique empty response proto rather than reusing the well-known Empty message type.

To achieve reuse -- create shared "domain" message types that can be included in multple Request and Response protos. Your application logic can be written in terms of those types rather than the request and response types.

This gives you the flexibility to evolve your method request/response types independently, but share code for logical sub-units.

As a corollary to this, we recommend that top level request and response types contain only sub-messages (no primitive fields). Even when you only need a single primitive type today, having it wrapped in a message gives you a clear path to expand that type and share the type among other methods that return the similar values.

@Sereger13
Copy link
Author

Sereger13 commented Feb 22, 2024

Thanks for the comprehensive answers - appreciate your answers, that's really useful.

Just to confirm on the naming - the original example from the guidance:

service FooService {
  rpc GetSomething(GetSomethingRequest) returns (GetSomethingResponse);
  rpc ListSomething(ListSomethingRequest) returns (ListSomethingResponse);
}

//  Is the recommended way then to have separate proto messages/files like this?
message SomethingRequest{
}

message SomethingResponse{
}

message GetSomethingRequest{
  SomethingRequest thing = 1;
}

message GetSomethingResponse{
  SomethingResponse thing = 1;
}

message ListSomethingRequest{
  repeated SomethingRequest thing = 1;
}
message ListSomethingResponse{
  repeated SomethingResponse thing = 1;
}

@googleberg
Copy link
Member

Definitely on the right track. Let's try something a little more concrete like this:

service StudentService {
  rpc CreateStudent(CreateStudentRequest) returns (CreateStudentResponse);
  rpc GetStudent(GetStudentRequest) returns (GetStudentResponse);
}

message StudentID {
  string value = 1;
}

message FullName {
  string family_name = 1;
  string given_name = 2;
}

message Student {
  StudentId id = 1;
  FullName name = 2;
}

message CreateStudentRequest {
  FullName name = 1;
}

message CreateStudentResponse {
  Student student = 1;
}

message GetStudentRequest {
  StudentID id = 1;
}

message GetStudentResponse {
  Student student = 1;
}

In this example, Student, StudentID, and FullName are domain types that are reusable across requests and responses. But the top-level request and response protos are unique to each service+method.

Now, if we later need to add a middle_name field to the FullName message, we don't need to update every individual top-level message with that new field.

Likewise, if we need to update Student with more information (and we definitely will), all the requests and responses get the update.

Even though it may be unlikely StudentID might update to be a multi-part ID especially if you have to do something to sequester certain student records in certain geographic regions -- something that can happen when governments introduce new regulations.

Also, having even simple types like StudentID wrapped as a message means that you have created a type that has semantics and consolidated documentation. For something like FullName you'll often want to be careful with where the information gets logged -- this is another advantage of not repeating these fields in multiple top-level messages -- you can tag those fields once as sensitive and exclude them from logging.

Hope this helps.

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

3 participants