Discriminated Union discriminators should be required

[lewisjkl]

https://github.com/disneystreaming/smithy4s/blob/main/modules/protocol/src/smithy4s/api/validation/DiscriminatedUnionValidator.java

We should be validating that the target structures containing the discriminator have the discriminating field marked as required. If the field is optional, then that means the discriminator may not be present which defeats the purpose.

[Baccata]

I disagree, the field should not be part of the model. If anything we should validate that the field doesn't exist, to avoid collision with the serialisation mechanism

[Baccata]

The openapi conversion however should ensure that the field exists and is marked as required in the output openapi model

[lewisjkl]

I disagree, the field should not be part of the model. If anything we should validate that the field doesn't exist, to avoid collision with the serialisation mechanism

This works for me 👍

The openapi conversion however should ensure that the field exists and is marked as required in the output openapi model

In openapi, it seems like the field that is a discriminator is almost implied to be required, not sure though... I guess it wouldn't hurt to mark it as required in any case.

[Baccata]

In openapi, it seems like the field that is a discriminator is almost implied to be required, not sure though... I guess it wouldn't hurt to mark it as required in any case.

Yup. In fact, not only should the field be required, but it should also be constrained as enumeration with a single value : the one corresponding to the alternative

[Baccata]

The validation of the discriminated trait has been added there : #86