Closed stefanerwinmayer closed 1 year ago
Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
Thanks for the issue @stefanerwinmayer. I am in the process of rewriting the underlying model, as this scenario is currently not possible and too tightly coupled with JSON Schema. Once the rewrite is complete I will take a look at this 👍
Thank you @jonaslagoni. I will eagerly await your rewrite.
Related BlackBox tests that currently is being blacklisted - https://github.com/asyncapi/modelina/blob/3f14b6cc44eb10af4c74fbba9d8d12037ae66ad4/test/blackbox/blackbox.spec.ts#L60
@all-contributors please add @stefanerwinmayer for bug
@jonaslagoni
I've put up a pull request to add @stefanerwinmayer! :tada:
As the union is a typescript specific feature, maybe it could help to think about how the expected output should look like with other generators. For example with Java: Generated outcome:
public class A {
private AnonymousSchema_1 prop;
private Map<String, Object> additionalProperties;
public AnonymousSchema_1 getProp() { return this.prop; }
public void setProp(AnonymousSchema_1 prop) { this.prop = prop; }
public Map<String, Object> getAdditionalProperties() { return this.additionalProperties; }
public void setAdditionalProperties(Map<String, Object> additionalProperties) { this.additionalProperties = additionalProperties; }
}
public class AnonymousSchema_1 {
private Integer someProp;
private Integer anotherProp;
private Map<String, Object> additionalProperties;
public Integer getSomeProp() { return this.someProp; }
public void setSomeProp(Integer someProp) { this.someProp = someProp; }
public Integer getAnotherProp() { return this.anotherProp; }
public void setAnotherProp(Integer anotherProp) { this.anotherProp = anotherProp; }
public Map<String, Object> getAdditionalProperties() { return this.additionalProperties; }
public void setAdditionalProperties(Map<String, Object> additionalProperties) { this.additionalProperties = additionalProperties; }
}
Expected outcome:
?
Good point @panwauu.
My immediate thoughts about how we can support it:
public class A {
private APropUnion prop;
...
}
public class APropUnion {
private B b;
private C c;
...
}
Object
. We might be able to provide some comments to help the developer "know" which union types can be created, i.e.
public class A {
/** This must either be B or C */
private Object prop;
...
}
public class A {
private Object prop;
public B getPropBType() { return <B>this.prop; }
public boolean isPropBType() { ... }
public C getPropCType() { return <C>this.prop; }
public boolean isPropCType() { ... }
}
Can you think of any other options? 🤔
We can of course always have multiple, to fit the users use-case, but think a default behavior is good starting point 🙂
I would suggest that union types in java could be done if asyncapi adopted the openapi spec in regards to discriminators
https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/
then you could do :
discriminator:
propertyName: objectType
mapping:
b: '#/components/schemas/b'
c: '#/components/schemas/c'
public abstract class A {
abstract String getObjectType();
Map<String, Object> additionalProperties;
}
public class B extends A {
Long someProp;
String getObjectType() { return "a" }
}
public class C extends A {
Long anotherProp;
String getObjectType() { return "b" }
}
And you can use jackson annotations on A to do the mappings
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include=JsonTypeInfo.As.PROPERTY, property = "objectType")
@JsonSubTypes({
@JsonSubTypes.Type(value=B.class, name="b"),
@JsonSubTypes.Type(value=C.class, name="c"),
})
})
This issue has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.
There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.
Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.
Thank you for your patience :heart:
Still relevant
At Sportradar we're trying to adopt AsyncAPI with Modelina, and we are facing this issue. Is this being worked on in the next branch? If not, I can try to solve it for TypeScript and possibly Java at least.
At Sportradar we're trying to adopt AsyncAPI with Modelina, and we are facing this issue. Is this being worked on in the next branch? If not, I can try to solve it for TypeScript and possibly Java at least.
It is yes 🙂
Awesome! I'll share our use case in case you need a spec to test with. I would expect a data model for Message, Goal, YellowCard, and hopefully Event using inheritance.
asyncapi: "2.4.0"
info:
title: Soccer
version: "v1.0.0"
description: Soccer
servers: {}
defaultContentType: application/json
channels: {}
components:
messages:
Message:
name: Message
payload:
$ref: "#/components/schemas/Message"
schemas:
Message:
$id: Message
type: object
additionalProperties: false
properties:
event:
$id: Message::Event
oneOf:
- $ref: "#/components/schemas/Goal"
- $ref: "#/components/schemas/YellowCard"
Event:
$id: Event
type: object
additionalProperties: false
properties:
id:
type: string
required:
- id
Goal:
$id: Event::Goal
type: object
additionalProperties: false
allOf:
- $ref: "#/components/schemas/Event"
- type: object
properties:
scored_by_team_id:
type: string
scored_by_player_id:
type: string
required:
- scored_by_team_id
- scored_by_player_id
YellowCard:
$id: Event::YellowCard
type: object
additionalProperties: false
allOf:
- $ref: "#/components/schemas/Event"
- type: object
properties:
for_team_id:
type: string
for_player_id:
type: string
required:
- for_team_id
- for_player_id
I think the problem is here https://github.com/asyncapi/modelina/blob/7a2394956f8f82670e10859efd6b1c67c4487033/src/interpreter/Interpreter.ts#L85-L86
It shouldn't interpret and combine the schemas for oneOf but should just reference the underlying schemas https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/#anyof-vs-oneof
hey folks, I made some changes to address this. I'll open a PR later today/tomorrow after running the tests. feel free to reference the feature branch and let me know if this works for you https://github.com/asyncapi/modelina/compare/master...jacopomaroli:modelina:fix_oneOf_impl?expand=1
edit: PR opened here https://github.com/asyncapi/modelina/pull/814
:tada: This issue has been resolved in version 1.0.0-next.10 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
Describe the bug
Typescript: Using oneOf leads to creation and use of an anonymous schema rather than use of a union type.
How to Reproduce
api.yaml
Example code to consume the api:
Generated outcome:
Expected behavior
According to my understanding, this should be generated instead: