Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Runtime support of union types is limited to TypeScript clients #1411

Closed
darrelmiller opened this issue Mar 18, 2022 · 6 comments · Fixed by #1603
Closed

Runtime support of union types is limited to TypeScript clients #1411

darrelmiller opened this issue Mar 18, 2022 · 6 comments · Fixed by #1603
Assignees
Labels
fixed generator Issues or improvements relater to generation capabilities.

Comments

@darrelmiller
Copy link
Member

Due to lack of support for union types in other languages, we do not have an easy way to map in runtime response to the appropriate property on the wrapper created for union types. Support for this would require taking a dependency on a JSON Schema validation library to determine which of the schemas map to the response.

Users can always decorate their API with a discriminator for non-scalar types.

Further investigation is required.

@darrelmiller darrelmiller self-assigned this Mar 18, 2022
@baywet baywet added type:bug A broken experience needs more information generator Issues or improvements relater to generation capabilities. labels Mar 18, 2022
@darrelmiller
Copy link
Member Author

darrelmiller commented May 20, 2022

For union and exclusion types that are non-scalar, a discriminator is required to enable runtime differentiation between the returned type.

MyResponseType:
  oneOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'
  - $ref: '#/components/schemas/Lizard'
  - $ref: 'https://gigantic-server.com/schemas/Monster/schema.json'
  discriminator:
    propertyName: petType
    mapping:
      dog: '#/components/schemas/Dog'
      monster: 'https://gigantic-server.com/schemas/Monster/schema.json'

For scalar types, we would brute force match the returned value against the schema types listed in the oneOf until we find a match.

If a oneOf is present but there is no discriminator, look into the schemas in the oneOf for the discriminator. By convention when we generate the OpenAPI from Graph, we put the base type as the first schema in the oneOf and it contains the discriminator. Only use the mappings in from the discriminator that are used by the oneOf.

@baywet
Copy link
Member

baywet commented Jun 2, 2022

in case we have one of more complex types in the oneOf/anyOf but are not able to find discriminator information, an error should be logged but the generation process should continue.

@darrelmiller
Copy link
Member Author

If possible we should prevent the RequestFactory from being emitted, but probably still emit the model or we could cause cascading errors.

@baywet
Copy link
Member

baywet commented Jun 30, 2022

From our most recent design session:

AllOfs in schemas included in an anyOf cause an issue if we merge anyOfs into a single wrapper.

Union type maps to oneOf
Intersection type maps to anyOf

To support anyOf we have two choices:
A) Merge all the properties of the child schemas into the wrapper type
B) Create properties in the wrapper type for each of the child schemas

Option A doesn't work cleanly for child schemas that are scalar types and it doesn't work for child schemas that use allOf for inheritance.

Option B has a problem where properties exist in multiple child schemas.
We are preferring Option B and telling customers that if a property exists in multiple child schemas then we will only deserialize to one of the properties.

This means that oneOf and anyOf wrappers are the same shape.

oneOfs should come with a discriminator to know which property to deserialize the object into.

We think we need the additionalProperties dictionary on the wrapper class.

If a oneOf has a child schema of type object, then a discriminator is required.

https://spec.openapis.org/registry/format/
Formats Registry
Extensible data value repository

public class ComposedWrapper : IUnionType (oneOf) | IIntersectionType (anyOf) {
	public ComposedType1 ComposedType1 { get; set; }
	public ComposedType2 ComposedType2 { get; set; }
	public string StringResult { get; set; }
	public string WhichFieldInfo {get; set;} // only for union type

	public static ComposedWrapper CreateFromDiscriminator(IParseNode parseNode) {
		var result = new ComposedWrapper();
		var discriminator = parseNode.GetDiscriminator();

		// when any of
		result.composedType1 = new();
		result.composedType2 = new();
		whichFieldInfo = "composedType1,composedType1;composedType2,composedType1;";
		// when one of
		if("#microsoft.graph.composedType1".Equals(discriminator)) {
			result.composedType1 = new();
			whichFieldInfo = "composedType1,composedType1"; // (symbol name, serialization name)
		}
		if("#microsoft.graph.composedType2".Equals(discriminator)) {
			result.composedType2 = new();
			whichFieldInfo = "composedType2,composedType2";
		}
		// end when
		if(parseNode.GetStringValue() is string stringValue) {
			result.StringResult = stringValue;
			whichFieldInfo = "noop"; // only for union types/one of
		}
		return result;
	}

	public Dictionary<string, Action<ParseNode>> GetFieldDeserializers() {
		return new Dictionary<string, Action<ParseNode>> {
			{ "composedType1", (node) => {
				this.composedType1 = node.GetObjectValue<ComposedType1>();
			} },
			{ "composedType2", (node) => {
				this.composedType1 = node.GetObjectValue<ComposedType2>();
			} },
			{ "string", (node) => {
				this.string = node.GetString();
			} },
		};
	}
}

public class JsonParseNode {
	public T GetObjectValue<T>(Func<T> factory) {
		var rawJsonValue = new ();
		var instance = factory(this);
		if(instance is IIntersectionType) {
			var fieldDeserializers = instance.ComposedType1.GetFieldDeserializers().Union(instance.ComposedType1.GetFieldDeserializers()); // AnyOf get the properties by reflection using the whichFieldInfo
			// use a multi-map to get the field deserializers in case fields with the same name are defined on different members
		} else if(instance is IUnionType) {
			var fieldDeserializers = instance.ComposedType1.GetFieldDeserializers(); // one of (get the right property by reflection using the whichFieldInfo)
		} else {
			var fieldDeserializers = instance.GetFieldDeserializers(); // regular object and all of
		}

		foreach(var field in rawJsonValue) {
			var fieldDeserializer = fieldDeserializers[field.Key];
			fieldDeserializer(field.Value);
		}
		// additional data
		return JsonConvert.DeserializeObject<T>(this.Value);
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed generator Issues or improvements relater to generation capabilities.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants