search

InteropIO / FDC3

Open standards for the financial desktop.
https://fdc3.finos.org
Other
2 stars 1 forks source link

544 refactor schemas to separate agent and bridge messages #69

Closed kriswest closed 1 year ago

kriswest commented 1 year ago

Improves the schemas and generated code for bridging messages by spliting each message schema into a separate schema for agents to return to the bridge and for the bridge to send onto agents, ensuring that a precise schema is available to validate each message type.

Also adds better titles and descriptions to the schemas, which improve generated element names and source code comments in generated TypeScript code. I went through far too many versions before arriving at this structure in the schemas, which seems to work and deals with the differences between the agent and bridge messages as well as quicktype is able (IMHO).

@mattjamieson requested this change. @Vivek-NatWest & @mhmcclung please do review and feedback @tpina to review if possible.

I'd like to merge this on Monday, and then focus on the talk we're giving next week, although there are still a couple of other minor todos in the main PR:

Vivek-NatWest commented 1 year ago

@kriswest, Doing the review. Have some findings, and will post them here once finished.

Vivek-NatWest commented 1 year ago

@kriswest, Doing the review. Have some findings, and will post them here once finished. Have uploaded Async API def. This needs to be placed at the root folder AsyncApi.zip.

Generated files from async api def-> Below is HTML output of API definition output.zip

Client generation using the generators: https://github.com/asyncapi/generator C# client has issues where a lot of files are generated with Anonymous class names overriding implementations like 'MessageType' being shared by many messages, resulting in inconsistent output. It looks like inheritance behavior imposed through 'allOf' is not respected well in schemas.

kriswest commented 1 year ago

@Vivek-NatWest not being able to use allOf for schema composition would be a huge disability! We'll need to figure out if its fundamental to asyncAPI or whether its the particular template (e.g. does it generate docs correctly but not C# code?).

We might have some time to look into this after the event (so friday at earliest). Is it worth setting up a call/works session to do that?

Finally, are you ok with these proposed changes to split the schemas (apart from the allOf issue, which I assume is wider than this PR)? It makes sense to me as doing so means we can validate the subtly different messages to and from the bridge separately.

Vivek-NatWest commented 1 year ago

@Vivek-NatWest not being able to use allOf for schema composition would be a huge disability! We'll need to figure out if its fundamental to asyncAPI or whether its the particular template (e.g. does it generate docs correctly but not C# code?).

We might have some time to look into this after the event (so friday at earliest). Is it worth setting up a call/works session to do that?

Finally, are you ok with these proposed changes to split the schemas (apart from the allOf issue, which I assume is wider than this PR)? It makes sense to me as doing so means we can validate the subtly different messages to and from the bridge separately.

Thanks @kriswest. Yes, since the desktop agent and bridge play both the role of client and server for each other, it makes sense to have the separation in terms of message exchanges and API.

kriswest commented 1 year ago

@Vivek-NatWest re: the asyncAPI schema, assuming the whole thing is set up from the perspective of a Desktop Agent, then all publishes should use agent schemas and all subscriptions need to use the bridge schemas (as all messages received will come through the bridge).

Hence, I think it should end up referencing every 'top-level' schema file. E.g. We should see it publish broadcastAgentRequest and subscribe to broadcastBridgeRequest, right?