gapic: non-optional number fields are excluded from REST query params if value is 0

[noahdietz]

Currently, REST transport implementations check if a number field (e.g. int32) is 0 or not, and will only include the field's value in request params if it is not 0. Example.

This could be a problem since 0 is both a valid value in many cases and the default empty/unset value for type int32 (which is why it is being checked in the first place).

Specifically, in DIREGAPIC compute, the only non-optional number fields are also annotated REQUIRED, and in all cases 0 is a valid value. This results in BadRequest errors because a field that is required is not sent when it is 0, which is a valid value. See googleapis/google-cloud-go#5174.

[noahdietz]

I think this will be a problem for both DIREGAPIC and REGAPIC.

With DIREGAPIC, it is easier to handle because the protos are generated as either optional (and thus become a pointer, e.g. *int32, in Go) or are non-optional and are annotated as REQUIRED.

With REGAPIC, the optional label is not often used, which means non-REQUIRED fields are still raw primitives, not pointers, and default to the empty value i.e. 0.

I think we can start by continuing to use the same if != 0 check for non-REQUIRED REGAPIC number fields.

For all (REGAPIC and DIREGAPIC) number fields annotated as REQUIRED should send their value no matter what. This could open users up to some weird scenarios where they didn't actually set the REQUIRED field and the 0 is sent anyways.

To help reduce the chance of the aforementioned unset REQUIRED field issue happening, I think we will want to beef up documentation in Go clients for REQUIRED fields with a combination of extra docs on methods and comments for required fields in example payloads. This will be a separate change from request param building to fix the actual issue.

[noahdietz]

#820 for required field documentation improvements.

[vchudnov-g]

That is indeed the behavior we should have. Sorry it wasn't called out explicitly for query params in the internal design doc...or included in integration tests. I will address those two points. Thanks for catching this!