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

[C++] Required numerics rule #853

Closed
xamix opened this issue Apr 25, 2023 · 2 comments
Closed

[C++] Required numerics rule #853

xamix opened this issue Apr 25, 2023 · 2 comments
Labels
C++ C++ language support Enhancement Extend or improve functionality

Comments

@xamix
Copy link

xamix commented Apr 25, 2023

Hi,

I use proto3 and latest protoc 22.3 and latest pgv 1.0.0
I have a need to check the presence of numerics fields like bool or others.

It is possible to get protoc generating the has_field accessor on thoses numerics types by using the keyword optional, for example in the following definition, the scheduled field have the optional keyword:

message Event {
        optional bool   scheduled = 1; 
}

And the corresponding function event.has_scheduled() do exist.

What I want is a rule something like [(validate.rules).bool.required = true]

I know that I can use the WKT (Well Known Type) google.protobuf.BoolValue in place of bool however it's a pain to use it in C++ since it will force to create an object and set it's value instead of just setting the boolean value directly:

With google.protobuf.BoolValue scheduled = 1;:

event.mutable_scheduled()->set_value(true);

With optional bool scheduled = 1; :

event.set_scheduled(true);

Moreover the serialized size of object using WKT here (google.protobuf.BoolValue ) is bigger than the solution with optional bool when you serialize non default value, for example on boolean true value take 4 byte to encode with google.protobuf.BoolValue whereas it takes always 2 byte to encode false or true with the optional bool

So if it's possible to add required rules on the optional field it will be a gain in coding and also in serialized size.

EDIT:
To help here is a link to the explanation of the usage of optional

While reading this I came to a dirty solution to keep serialized size to minimum and accessing field easily, by wrapping each numerics type in a single oneof, and applying validate.required rule on it for example:

oneof scheduled_value {
    option (validate.required) = true;
    bool scheduled = 1;
}

As a side note, if the name of the oneof start or end with underscore, the validator generate code which do not compile.

Now my proto file is very ugly to my point of view, so implementing optional and handling rule required in PGV should be a better option

@rodaine rodaine added Enhancement Extend or improve functionality C++ C++ language support labels Apr 26, 2023
@C0mbatwombat
Copy link

+1

@chrispine
Copy link
Contributor

Hello, thanks for your interest! As explained in the README, this project is in maintenance mode. We recommend upgrading to protovalidate.

I'm going to go ahead and close this issue, but feel free to open one against protovalidate if needed. Thanks!

@chrispine chrispine closed this as not planned Won't fix, can't repro, duplicate, stale Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ C++ language support Enhancement Extend or improve functionality
Projects
None yet
Development

No branches or pull requests

4 participants