Enforce REQUIRED annotations via API call signatures

[sinelaw]

Problem:

When a field is annotated as REQUIRED, the API allows constructing objects without specifying explicitly the value of the required field.

Example:

import "google/api/field_behavior.proto";

message Child { ... }

message Parent {
  Child child = 1 [
    (google.api.field_behavior) = REQUIRED
  ];
}

The Parent will generate a Builder that has a setChild() method, but there's no compile-time way to check that this setter was called. So it's possible to write code that constructs "bad" messages which will only be manifest at runtime.

Describe the solution you'd like

Positional arguments for required fields. In the example above, to get a Parent Builder (or to create a Parent in some other way) your only option would be to call a method that requires a Child as a parameter.

To preserve the existing API, this feature could be placed behind a flag / option for the proto generator.

Describe alternatives you've considered

Don't think there's an alternative besides changing how code is generated.

Thanks!

[noahdietz]

Based on discussion in answer to yaqs/3650180692123320320.

[chanseokoh]

@noahdietz the yaqs link is broken. Nvm, it works when prefixed with a protocol.

[noahdietz]

@noahdietz the yaqs link is broken.

It works for me. If you are expecting it to be hyperlinked, I intentionally did not do that.

[chanseokoh]

Gone through the yaqs discussion. Enforcing this is a breaking change to the Java API surface, so it's unlikely that we do this for existing protos.

But more importantly, as hinted in the discussion, it is debatable that enforcing this is the right thing to do; not supplying a REQUIRED filed will make proto supply the default value, which may work well and be actually intended in most cases. For example, a backend API service annotation a boolean field with REQUIRED, indicating that the backend will read the value out of it. However, the default value when not supplied by the user is false, and the intention may be to make user set it to true only in some specific cases.

[sinelaw]

@chanseokoh -

Regarding breaking the Java API surface - we can require an option/flag to enable the new API (and maybe one day deprecate the old one), so existing code will not be affected.

The discussion here is about the field_behavior annotation, not arbitrary protobuf. Please see https://google.aip.dev/203, which dictates

The use of REQUIRED indicates that the field must be present (and set to a non-empty value) on the request or resource.

A field should only be described as required if either:

• It is a field on a resource that a user provides somewhere as input. In this case, the resource is only valid if a "truthy" value is stored.
  • When creating the resource, a value must be provided for the field on the create request.
  • When updating the resource, the user may omit the field provided that the field is also absent from the field mask, indicating no change to the field (otherwise it must be provided).
• It is a field on a request message (a message that is an argument to an RPC, with a name usually ending in Request). In this case, a value must be provided as part of the request, and failure to do so must cause an error (usually INVALID_ARGUMENT).

We define the term "truthy" above as follows:

• For primitives, values other than 0, 0.0, empty string/bytes, and false
• For repeated fields maps, values with at least one entry
• For messages, any message with at least one "truthy" field.

Based on the above it makes no sense for the client API to allow constructing a create request that simply uses default values for REQUIRED fields.

[chanseokoh]

Aha, you're right. Didn't know falsy values are not allowed for REQUIRED. So a REQUIRED boolean field must always be set to true.

A corollary to this is that a required boolean must be set to true.

[chanseokoh]

However, another aspect to take into consideration before implementing this is that, once implemented, changing REQUIRED to OPTIONAL (which is a non-breaking change on the API side) becomes a breaking change at the Java interface surface. This relaxation is what service teams may actually want to do (yaqs: 1277373026731556864), and it is hinted there too that it may cause a breaking change on end user's code. So this increases the risk of failure and everyone should be well aware of this subtlety.

[chanseokoh]

That said, IMO, as much as required proto (not annotation) fields have been discouraged and actually everything is now optional in proto3, which has good reasons, I think we need to be careful of enforcing something always required on the Java surface too. Once something becomes explicitly required, there is no turning back. Service teams cannot change their APIs, and the Java API surface just can't change at all.

[meltsufin]

@chanseokoh I agree that we should not enforce the (google.api.field_behavior) = REQUIRED at the API level. However, I think there are benefits to annotating the fields and method arguments with @NonNull or a similar annotation that IDEs and static analysis tools can use.
(Related to: b/113137879)