Relax oneof set fields validation

[adriansmares]

Summary

References TheThingsNetwork/lorawan-stack#5326

Changes

• Do not check if the source or destination oneof fields have the correct type, but instead behave as if they are nil on mismatch.
  • Note that this is allowed only if no subfields are provided

Notes for Reviewers

This is motivated by the ADR settings PR. Before this PR, it would be impossible to change the oneof field without clearing the field in between uses. This 'clear' operation (setting to nil) was required because the generated code would complain that the source and the destination oneof type are mismatching.

The effect on TTS can be seen at TheThingsNetwork/lorawan-stack@453b835

[adriansmares]

We need to check if the source type actually matches, not only that it is present.

[adriansmares]

At this point !srcTypeOk && !dstTypeOk (which is the original check) - we empty the destination in order to make sure that it matches the source.

[htdvisser]

I'm not completely sure about this one. If I want to change the type of the oneof, I should replace the entire thing. Changing a subfield of the wrong "branch" should result in an error. Replacing the thing should only happen if the field mask contains (exactly) the oneof field itself, or (exactly) one of its "branches".

[adriansmares]

Just to make sure that I understood this: partial patches (so the ones which do have subfields) should require that the source and destination types match, but full replaces may change the type freely ?

I understand the reasoning to a certain extent but keep in mind that the client does not know what is stored on the server side - It is hard to know when I need to do a replace and when I need to do a patch as a client, so requiring that I use the top level field complicates things. We require that the client fully retrieves the entity before the update if we require that the top level field is used, which is inconsistent with the rest of the API design.

Note that this PR does not allow you to provide two oneof branches at once - that one is still banned - you cannot give both oneof_field.oneof_branch_a and oneof_field.oneof_branch_b at the same time for an update.

[htdvisser]

Just to make sure that I understood this: partial patches (so the ones which do have subfields) should require that the source and destination types match, but full replaces may change the type freely?

Exactly. And to give a clear example, let's look at the ApplicationPubSub message. If the stored PubSub has nats in the provider oneof, I should not be able to set only the mqtt.password. Instead, I need to replace the entire provider by setting the entire mqtt object, so that we can also validate the entire thing.

keep in mind that the client does not know what is stored on the server side

I think it's not unreasonable to expect that clients know the state of the entity (or at least the relevant fields) before they can make changes to those fields.

[adriansmares]

I've implemented the requested change and added two test cases that should cover this.