Bug Report: The TCK needs to check for presence of a sequence number in all required messages

[wes-johnson]

It appears Protobuf 3 can exclude optional fields and if not present can be considered their default value. Null and default are not the same and should not be treated as such. Ensure the TCK uses the 'has' Methods to verify presence and value.

[mattiasbe]

Scalar values' default is 0, not null. It's a protobuf encoding feature, or optimisation if you will. Sparkplug is defined to use syntax 2, so syntax 3 is not an issue anyway, though 3 can now be made to work the same way with the re-introduced feature.

[mattiasbe]

So to be clear: This shouldn't be changed in the TCK.

I wrote the same in a tahu issue:

Primitives in protobuf can't be null, if the seq is not sent it is 0. seq is defined as:

optional uint64 seq = 3; // Sequence number

Protobuf has this very well defined. It’s not null.

This section explains it well:

“A well-formed message may or may not contain an optional element. When a message is parsed, if it does not contain an optional element, accessing the corresponding field in the parsed object returns the default value for that field. The default value can be specified as part of the message description.”

“If the default value is not specified for an optional element, a type-specific default value is used instead: for strings, the default value is the empty string. For bytes, the default value is the empty byte string. For bools, the default value is false. For numeric types, the default value is zero. For enums, the default value is the first value listed in the enum’s type definition. “

From: https://protobuf.dev/programming-guides/proto/#optional

[icraggs-sparkplug]

@wes-johnson do you have any more information about where this bug was experienced, and any thoughts about Mattias' points? Thanks.

[wes-johnson]

So it is true that Protobuf has 'default values'. As noted above, for a numeric type, the default value is zero. However, the Sparkplug spec states this (among other statements around sequence numbers):

[tck-id-payloads-sequence-num-zero-nbirth] A NBIRTH message from an Edge Node MUST always contain a sequence number between 0 and 255 (inclusive).

[tck-id-payloads-sequence-num-incrementing] All subsequent messages after an NBIRTH from an Edge Node MUST contain a sequence number that is continually increasing by one in each message from that Edge Node until a value of 255 is reached.

It doesn't say if the value is zero, it may be omitted so it can assume the default value of zero as defined by Protobuf. I think the Sparkplug spec is very clear in that it must always contain a sequence number and must be between 0 and 255.

[mattiasbe]

From sparkplug's point of view, the value is 0. The layer below it, protobuf, says it's 0.

[icraggs-sparkplug]

PR created. #515

[mattiasbe]

Again please consider the protobuf 3 spec that if you set the value to 0 it will not be serialized:

Note that for scalar message fields, once a message is parsed there’s no way of telling whether a field was explicitly set to the default value (for example whether a boolean was set to false) or just not set at all

"Also note that if a scalar message field is set to its default, the value will not be serialized on the wire."

Example with float follows that statement:

"If a float or double value is set to +0 it will not be serialized."

From: https://protobuf.dev/programming-guides/proto3/#default