Protos Don't Indicate If Field Is Required or Optional

[grant]

Expected Behavior

I'd expect that CloudEvent required proto fields have the required field rule somehow. This is so we can make JSON fields required or not.

I understand proto3 has limitations with required fields.

Actual Behavior

The protos don't seem to state if fields are required or not. This turns out to be a little annoying as it causes tooling to accept empty objects as valid events.

I'd like to be able to mark in the JSON schema if a field is required or not. Currently all fields are optional. We could make all fields required, but that is likely too strict for all events.

---

For more context, I'm fuzzing with our (JSON) schemas and see lots of invalid events / false positives. This is because our JSON schemas don't have any required fields because our protos don't have any required fields (even {} is a valid Pub/Sub event for example).

[jskeet]

Given that these are only consumed by users rather than actually produced, I think I'd prefer to just document whether they're always expected to be present or not.

Making all fields required would definitely be too strict - I expect many fields are absent from many events, particularly CAL and Storage.

Note that even when fields are always produced, making the libraries reject events that don't include them could be annoying for developers. Suppose there are 10 fields in StorageObjectData that are always populated (which isn't at all unreasonable) but my application only looks at 1 of them... it's annoying to have to populate the other 9 in tests.

[grant]

@jskeet Can you tell me how we would make a field required or not in proto3? As in, is it technically possible? How?

I'm asking for validation tooling purposes. I would ensure any strict requirement is not added for our libraries.

[jskeet]

You can't make fields required "natively" in proto3, no. We could add our own annotation (or reuse an existing one in google.api - I'd need to take a closer look to see if I could see one) and then generate code based on that for validation purposes.

[grant]

@jskeet Can we try adding optional to our protos ans using --experimental_allow_proto3_optional I believe our proto version used in this repo already supports this.

https://github.com/protocolbuffers/protobuf/blob/v3.12.0/docs/field_presence.md

[jskeet]

We could do that, but I'm pretty skeptical about it, to be honest. Firstly, I'm not sure how genuinely useful it will be - this isn't a typical use case for proto3 optional fields, where you really want to be able to tell the difference between a value set explicitly and it just keeping the default value. Secondly, I'm very nervous about the quality of information we'll end up with, both now and later on when the set of events expands a lot. Unless we're really confident that this is something that all event providers will annotate correctly, I don't think it's worth providing information that might or might not be accurate.

[grant]

Perhaps we could apply a field option like [(google.api.field_behavior) = OPTIONAL].

[jskeet]

I'd prefer not to reuse the field_behavior annotation which is explicitly designed for APIs, mostly in terms of what clients need to provide in a request. This is more about what the server may or may not provide. So if we want to do anything, I'd suggest a new annotation.

But as per my earlier comment, I'd rather not do anything at all, because I'm skeptical about the accuracy of the information. Experience suggests that event providers won't meticulously check each field and decide how to annotate it.

[grant]

Thanks for the discussion. Will close this for now. Will re-open if needed.