Unnecessary allOfs in OpenAPI spec

[david-crespo]

name doesn't need to be an allOf here, right? There are a few of these, most are Name but not all. @ahl

omicron/openapi/nexus.json

Lines 3862 to 3879 in 3d132cc

	"OrganizationUpdate": {
	"description": "Updateable properties of an [`Organization`]",
	"type": "object",
	"properties": {
	"description": {
	"nullable": true,
	"type": "string"
	},
	"name": {
	"nullable": true,
	"allOf": [
	{
	"$ref": "#/components/schemas/Name"
	}
	]
	}
	}
	},

[ahl]

I see two classes of these allOf uses: adding a description field and adding a nullable field.

In both cases, the use of allOf is necessary to add the additional field because in JSON Schema Draft 7 a $ref must be alone in an object. OpenAPI 3.0.x (which we're using) is based on JSON draft 7. OpenAPI 3.1 is based on a later draft which has greater flexibility, but the tooling ecosystem seems to be anaemic--more so than 3.0.x.

For description we need this in situations where the parameter itself is documented. I don't think this can be avoided without removing important information from the schema.

The nullable case may be avoidable, but I'm not certain of the right approach. Consider the case above of OrganizationUpdate which is really just a (flattened) wrapper around IdentityMetadataUpdateParams:

#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
pub struct IdentityMetadataUpdateParams {
    pub name: Option<Name>,
    pub description: Option<String>,
}

Both of these properties are optional and we can see this in the schema because neither of them is in the set of required properties. By default null is a valid value for an Option type. We could change our use of schemars so that it doesn't generate the nullable property, and that would certainly lead to a simpler spec, but since we do actually accept null as a valid value for these, we may want to preserve that expressivity.

Is the use of allOf somehow impeding you? Is this causing problems with a generator?

[david-crespo]

It's not causing me any problems, it's an easy case to handle — at least in our case where there's just one arm to the allOf. I just noticed it in the spec and didn't understand why it was there. Now I do. Thanks for the explanation!

Some of the PUT body ones might go away because of #333, but not all.

[ahl]

Glad to hear that it's not an obstacle. I agree that it's goofy! And it's good at least that if/when we go to OpenAPI 3.1 we won't have to deal with it.