Propagate nullable properties from OpenAPI 3 to CodeModel schemas

[daviwil]

Issue #269 reports that x-nullable extensions in Swagger inputs don't have any effect on the outputted CodeModel. This change merely propagates the value for nullable on OpenAPI 3 schemas to their CodeModel counterparts. Let me know if I missed any cases!

[daviwil]

Forgot this needs a change to @azure-tools/codemodel too, will send a PR for that.

[azure-pipelines]

You may test this build by running autorest --reset and then either:

---

add --use: to the command line:

autorest --use:http://tinyurl.com/ybjk7ysa ...

---

or use the following in your autorest configuration:

use-extension:
  "@autorest/modelerfour": "http://tinyurl.com/ybjk7ysa" 

---

If this build is good for you, give this comment a thumbs up. (👍)

And you should run autorest --reset again once you're finished testing to remove it.

[daviwil]

OK, finally got this updated and added some unit tests (check out modelerfour/test/unit/modelerfour.test.ts at the end of the Changes list). This is ready for final review!

[MiYanni]

I'm a bit confused. I see the code related to nullable and nullableItems (I'm assuming this is processing them for the code model), but shouldn't there be changes related to processing x-nullable from the swagger? Meaning, turning x-nullable into either nullable or nullableItems?

[daviwil]

That happens in AutoRest Core via @azure-tools/oai2-to-oai3:

https://github.com/azure/perks/blob/master/oai2-to-oai3/main.ts#L532-L541

x-nullable gets moved to nullable on the OpenAPI 3 document which we use as the input to modelerfour

[MiYanni]

@daviwil So, looking at that, makes sense for nullable. But, how does x-nullable become nullableItems?

[daviwil]

There are two ways that can happen for ArraySchema or DictionarySchema in this change:

• x-nullable on an array schema (or schema with additionalProperties which becomes a dictionary schema) gets copied over to the OpenAPI v3 nullable property on the schema. That nullable value gets placed into nullableItems (now that I say it, this might not be correct behavior).
• The OpenAPI v3 element schema of the ArraySchema or DictionarySchema has nullable set to true, that value gets copied over to nullableItems.

Maybe I should remove the former since the assumption would be that the Array/DictionarySchema would itself be nullable, which we're trying to avoid.

[MiYanni]

@daviwil After reading this, what you said makes a lot more sense: https://help.apiary.io/api_101/swagger-extensions/#x-nullable
This also helped understand null in OAPI3: https://swagger.io/docs/specification/data-models/data-types/#null
Based on what Garrett said, I believe the idea is only usage of a schema can have nullable. In OAPI2/3 (x-/nullable), you are saying "along with type: [something], you can also provide null". So, we represent that with our Value type in the code model, which has a Schema property alongside with a Nullable property. In this case for DictionarySchema/ArraySchema, it only applies to the ElementType (which is a schema). So, what is needed is an ElementNullable property on DictionarySchema/ArraySchema to make this work properly. Does that make sense?

[daviwil]

nullableItems == elementNullable, I called it nullableItems because ArraySchema also has properies called minItems, maxItems, and uniqueItems, so I'm following the established naming pattern.

https://github.com/Azure/perks/blob/47eb351e582950c26d265e0d15f7e5b1a867ddda/codemodel/model/common/schemas/array.ts#L14-L24

[MiYanni]

@daviwil Ah, right. Derp. I got lost in my own thinking there. So, the question is, in additionalProperties (for dictionarties) or items (in arrays), can you put x-/nullable in OAPI2/3? Because that is the correct usage here. Having the array schema or dictionary schema itself with those attributes would not be what we want to use for nullableItems. Instead, that dictionary or array reference itself would be nullable (via Value in the code model).

[daviwil]

In OpenAPI 3, nullable is available on all schema types so an OAI3 array schema or one with additionalProperties could be marked as nullable and then the nullable handling in modelerfour would be activated when those schemas are used in a context where we support nullable (properties, parameters, elements of a collection).

I'll remove the behavior I mentioned above about setting nullableItems to true when the Array/DictionarySchema itself has nullable: true.

[MiYanni]

@daviwil Perfect. Sounds great. Thanks for clarifying! 😊

[daviwil]

No problem! Fixed the unwanted nullable propagation to ArraySchema and DictionarySchema, this PR is ready to go.

[azure-pipelines]

You may test this build by running autorest --reset and then either:

---

add --use: to the command line:

autorest --use:http://tinyurl.com/y93qvals ...

---

or use the following in your autorest configuration:

use-extension:
  "@autorest/modelerfour": "http://tinyurl.com/y93qvals" 

---

If this build is good for you, give this comment a thumbs up. (👍)

And you should run autorest --reset again once you're finished testing to remove it.

[fearthecowboy]

TS has null propagation now 😄 ei?.nullable || false

[daviwil]

Old habits die hard :) I'll simplify this, thanks!

[daviwil]

Ahhh, I had to do this because .additionalProperties is typed to false | Schema, so the optional chaining operator fails the type check in this case after the schema has been resolved. It's either leave the current logic or use some type coercion to convince the compiler that it's definitely a Schema, so I think I'll leave this logic as-is for now.