ConsumerDataStandardsAustralia / standards-maintenance

This repository houses the interactions, consultations and work management to support the maintenance of baselined components of the Consumer Data Right API Standards and Information Security profile.
41 stars 9 forks source link

Payload conventions; optional fields with null values aren't defined in schemas #538

Closed ghost closed 1 year ago

ghost commented 2 years ago

Description

When validating data payloads against the CDR schemas, we are seeing validation errors when optional fields are being included with a null value in DH payloads. This is because there is an inconsistency between the defined data schemas and the described payload conventions for optional fields. The data schemas don't allow null values, whereas the conventions do.

Take the BankingAccount.nickname field for example; the schema defines this field as not required, but if it is chosen to be included then its value must be of type string:

          "nickname" : {
            "description" : "A customer supplied nick name for the account",
            "type" : "string"
          }

While the payload conventions describe null as a valid value for optional fields if they are chosen to be included:

Optional fields MAY be present but this is not guaranteed. It is also valid for these fields to be present but have a null value.

An empty field (ie. a field that is not present in a payload) will be considered equivalent with a field that is present with a null value.

If the field is optional a null value or omission of the field in the response is accepted.

Therefore, "nickname": null is valid according to payload conventions, but invalid according to the schema.

Area Affected

As the described payload conventions apply to all optional fields, this inconsistency affects all non-required fields defined in the data schemas without "null" as an acceptable type.

Change Proposed

If the defined schemas are updated to reflect the described payload convention for optional fields by defining "null" as an acceptable type for all optional fields, then schema-based data validation would operate per described standards. Continuing with our example, this would see the schema definition for BankingAccount.nickname change to:

          "nickname" : {
            "description" : "A customer supplied nick name for the account",
            "type" : ["string", "null"]
          }

The alternative is to adjust the payload conventions to align with JSON conventions such that optional fields cannot be null valued. While perhaps more standards aligned, this would require change by existing data holders.

I would suggest updating the schemas to reflect the described and already implemented payload conventions.

perlboy commented 2 years ago

If we were still dealing with Swagger docs ("OpenAPI 2") this would "all be fine" as type is deferred directly to draft-zyp-json-schema-04 which includes null type although a type with an array value I don't believe is explicitly permitted.

However, the OpenAPI spec was bumped to OpenAPI 3 in 1.16.1 and as such the above proposal is not functional.

OpenAPI 3.0.3 specification is a subset of the JSON Schema that states the following:

null is not supported as a type (see nullable for an alternative solution)

And:

type - Value MUST be a string. Multiple types via an array are not supported.

I believe the correct approach is:

          "nickname" : {
            "description" : "A customer supplied nick name for the account",
            "type" : "string",
            "nullable": true
          }
ghost commented 2 years ago

Yes, good pickup - thanks @perlboy!

nils-work commented 1 year ago

Hi @perlboy

Thanks for those details.

While the OpenAPI Specs. in the CDS are currently version 3.0.3, if I'm reading correctly now, it seems that version 3.1 would allow "null" as a type, according to 4.2.1. Instance Data Model (linked from 4.4 Data Types) and,

OpenAPI 3.1 support in Swagger Core:

while fields only part of 3.0 version (such as Schema.nullable) are annotated with @OpenAPI3.0.

image

perlboy commented 1 year ago

Support for OpenAPI 3.1 in libraries and products is still almost non-existent and it also calls into question what the Standards intent is for OpenAPI publishing. I raise this because the further along the OpenAPI upgrade path one goes the less likely it is to cleanly downgrade to Swagger. If downgrading to Swagger is intended to be abandoned then there is a valuable opportunity to actively use oneOf support which brings us into discriminator support.

I think the DSB needs to formally decide what its actual intent is regarding the specifications it publishes because there seems to be some oscillating between historical attitudes and easing of implementation.

biza-io commented 1 year ago

We believe the outcome of this item is dependent upon Native OAS Versioning Support #578 and so defer to that to determine the pathway forward here.

nils-work commented 1 year ago

DSB Item - OpenAPI Spec. (OAS) version plan #131 has been raised as a Future-Plan Decision Proposal placeholder to be addressed outside the Maintenance cycle.