Autorest: object type vs wildcard type and other Swagger fun (thanks to JSON schema)

[olydis]

This issue is about the difference between modeling stuff as

• wildcard types: object (C#), any (TypeScript), i.e. accepting anything including 42, "asd", [42], { "asd": 42 } etc.
• object types: Dictionary<string, object> (C#), object (TypeScript), i.e. accepting any JSON object { ... }

We (specs reviewers, validation tools, AutoRest) misinterpreted what it means for a schema to have properties, but no type: object. We thought presence of properties implies type: object, but it does not, see here.

Specs Repo

Situation

almost 9000 schemas that mistakenly do not have type: object on them

Impact:

• validation based on those schemas is ultra weak (may not catch errors in examples, in live validation, etc.)
• an input schema being too loose actually means it's just wrong: it documents that people may send stuff to the API which the API does not accept!
• will likely cause other tools (that interpret the Swagger spec correctly) to generate types that are too loose => horrible code => customers suffer

Work Items

• fix all schemas, making them correct in JSON schema's eye while also not changing them in AutoRest's eye (=> 0 impact on code generation)
• expected type of changes: add type: object (e.g. where we have properties), remove type: object (where wildcard/"untyped" behavior was expected)

Example method to validate the above using AutoRest:

1. snapshot the repo
2. generate all CodeModels for all Swaggers using current AutoRest modeler (call that "set A")
3. attempt to fix Swaggers
4. generate all CodeModels for all Swaggers using current AutoRest modeler (call that "set B")
5. generate all CodeModels for all Swaggers using "future" AutoRest modeler that treats schemas correctly (call that "set C")
6. if set A and set B don't match, you broke something by changing the intended meaning of a Swagger; reiterate from 3
7. if set A and set C don't match, there's still a type in there that is too loose; reiterate from 3

How to generate code models: --use=<modeler> (for latest: @microsoft.azure/[email protected], for fixed one: olydis/autorest.modeler#object-types) and --standalone-modeler --output-artifact=code-model-v1.norm.yaml

Validation Tools

The goal is to force people to clearly express their intended semantics. We should do so by disallowing certain vague (but technically valid) patterns.

Work Items

• error-level rule to always expect a type (next to properties, ...) since that is what 99.9% of all APIs want; exceptions like data factory can gladly get a suppression...
• error-level rule if there's type: object but no properties and no additionalProperties: true/{...} (you likely meant this to be a wildcard type - if you really meant object as in JSON object, add additionalProperties: {})
• also see here

AutoRest Common & Modeler

DONE: there is now a MultiType (AutoRest.Common version 2.4.40 and above) to represent the case when there was no type specified in Swagger
DONE: tag @objecttypes of the modeler is updated to treat Swagger spec compliantly (see table below for new treatment)

Work Items

• publish the above fixed modeler AFTER specs repo is fixed (so no breaking changes are introduced) and validation is in place

Generators

Situation

If in the Swagger there's a schema containing properties and either no type: object, we generate a strong type, even though we should not.
If in the Swagger there is type: object but no properties, the C# generator generates object in the code, which is (see above) a wildcard type, i.e. also accepts numbers and stuff! I think some other languages follow that (bad) history and service teams started to build on that behavior. In other words, we have Swaggers out there that model parameters/properties as type: object but expect it to accept numbers too. And we made it work. Except that it happens that Go is still sane and does not have this weird interpretation.

How generators treat PrimaryType with KnownPrimaryType object:

| language | behavior |
| - | - |
| C# | wildcard (also serializes known models correctly) |
| Go | object (serialization is broken for non-conformant JSON) |
| Java | TODO |
| NodeJS | TODO |
| Python | wildcard (also serializes known models correctly) |
| Ruby | TODO |
| TypeScript | TODO |

Work Items (once specs repo sanitized)

• all generators should map PrimaryType with KnownPrimaryType object to a JSONy object type and map MultiType to a true wildcard type

CodeModel changes of the fixed modeler (tag @objecttypes, to be promoted to latest once everything is fixed) compared to the latest stable

| Schema | Old Model (latest stable) | New Model (to be released after specs repo fixed) |
| - | - | - |
| {                                                                                                } | PrimaryType with KnownPrimaryType object | MultiType |
| {                                                        properties: {}                          } | PrimaryType with KnownPrimaryType object | MultiType |
| {                                                        properties: { prop: { type: integer } } } | CompositeType | MultiType |
| {               additionalProperties: {}                                                         } | DictionaryType | MultiType |
| {               additionalProperties: {},                properties: {}                          } | DictionaryType | MultiType |
| {               additionalProperties: {},                properties: { prop: { type: integer } } } | CompositeType | MultiType |
| {               additionalProperties: { type: boolean }                                          } | DictionaryType | MultiType |
| {               additionalProperties: { type: boolean }, properties: {}                          } | DictionaryType | MultiType |
| {               additionalProperties: { type: boolean }, properties: { prop: { type: integer } } } | CompositeType | MultiType |
| { type: object                                                                                   } | PrimaryType with KnownPrimaryType object | PrimaryType with KnownPrimaryType object |
| { type: object,                                          properties: {}                          } | PrimaryType with KnownPrimaryType object | PrimaryType with KnownPrimaryType object (CompositeType if --generate-empty-classes) |
| { type: object,                                          properties: { prop: { type: integer } } } | CompositeType | CompositeType |
| { type: object, additionalProperties: {}                                                         } | DictionaryType | DictionaryType |
| { type: object, additionalProperties: {},                properties: {}                          } | DictionaryType | CompositeType |
| { type: object, additionalProperties: {},                properties: { prop: { type: integer } } } | CompositeType | CompositeType |
| { type: object, additionalProperties: { type: boolean }                                          } | DictionaryType | DictionaryType |
| { type: object, additionalProperties: { type: boolean }, properties: {}                          } | DictionaryType | CompositeType |
| { type: object, additionalProperties: { type: boolean }, properties: { prop: { type: integer } } } | CompositeType | CompositeType |
| { type: string                                                                                   } | PrimaryType with KnownPrimaryType string | PrimaryType with KnownPrimaryType string |
| { type: string,                                          properties: {}                          } | PrimaryType with KnownPrimaryType string | PrimaryType with KnownPrimaryType string |
| { type: string,                                          properties: { prop: { type: integer } } } | PrimaryType with KnownPrimaryType string | PrimaryType with KnownPrimaryType string |
| { type: string, additionalProperties: {}                                                         } | DictionaryType | PrimaryType with KnownPrimaryType string |
| { type: string, additionalProperties: {},                properties: {}                          } | DictionaryType | PrimaryType with KnownPrimaryType string |
| { type: string, additionalProperties: {},                properties: { prop: { type: integer } } } | DictionaryType | PrimaryType with KnownPrimaryType string |
| { type: string, additionalProperties: { type: boolean }                                          } | DictionaryType | PrimaryType with KnownPrimaryType string |
| { type: string, additionalProperties: { type: boolean }, properties: {}                          } | DictionaryType | PrimaryType with KnownPrimaryType string |
| { type: string, additionalProperties: { type: boolean }, properties: { prop: { type: integer } } } | DictionaryType | PrimaryType with KnownPrimaryType string |

[lmazuel]

Interesting. Right now Python has the wildcards semantic and accept everything without any constraints.
Side note, if type is object and the user gives to Python a Model defined in the Swagger, Python serializes it correctly. It was a requirement for Data Factory. Of course this doesn't work on deserialization, since I don't know how to deserialize into a Model.

[olydis]

Thanks for the feedback, C# does the same - let's actually collect that information!

[olydis]

@johanste FYI

[fearthecowboy]

Yes;

We should run thru the rest-specs-repo and find whats' there, and how it's going to change and retro fit the definitions to generate what it currently does, so that we introduce no changes.

I think we should also figure out which of the permutations need a validation rule saying "Just what the hell do you think you're doing?!"