188

yurvon-screamo commented 1 month ago

Hi @VisualBean , I've finished this, please take a look.

Most of all, I am concerned about the correctness of the build of the message payload, I did not understand this contract well in places.

There is also a big disadvantage in this implementation - I do not use the component section at all, I do not yet understand what problems this can turn into, but I guess it's not good...

Please see how the time is going to be)

VisualBean commented 1 month ago

Hi @VisualBean , I've finished this, please take a look.

Most of all, I am concerned about the correctness of the build of the message payload, I did not understand this contract well in places.

There is also a big disadvantage in this implementation - I do not use the component section at all, I do not yet understand what problems this can turn into, but I guess it's not good...

Please see how the time is going to be)

Looks great! Ive only done a preliminary review on my phone (currently on holidays). But in terms of using components, if you use a reference, it must exist in components (i believe i saw this for the bindings for instance.

Components are however not necesary if you just inline everything. (Whether people want that or not is however a cause for concern on this)

For the message payload, in the current version of asyncapi.net, there is only support for jsonschema, however Avro is stewing in vNext (until i get external reference resolution to work).

Ill have s closer look when im back next week.

yurvon-screamo commented 1 month ago

Hi @VisualBean , I've finished this, please take a look. Most of all, I am concerned about the correctness of the build of the message payload, I did not understand this contract well in places. There is also a big disadvantage in this implementation - I do not use the component section at all, I do not yet understand what problems this can turn into, but I guess it's not good... Please see how the time is going to be)

Looks great! Ive only done a preliminary review on my phone (currently on holidays). But in terms of using components, if you use a reference, it must exist in components (i believe i saw this for the bindings for instance.

Components are however not necesary if you just inline everything. (Whether people want that or not is however a cause for concern on this)

For the message payload, in the current version of asyncapi.net, there is only support for jsonschema, however Avro is stewing in vNext (until i get external reference resolution to work).

Ill have s closer look when im back next week.

All right, I'll be waiting!

I've never worked with Avro, I can't tell you anything here... But this gave me the idea to check how all this will work on protobuf, I'll do it for now)

VisualBean commented 1 month ago

What i did for Avro is basically just the classes and turning message.payload into an IMessagePayload. AsyncapiSchema then inherits from this, and so does the Avro thing. (Avro is fairly complex from a parsing standpoint though as it doesnt have a top level object so to speak. But thats not what this PR is about 😁

But in terms of refs and components. All classes that inherits from IAsyncApiReferenceable will have the ability to be a reference.

So either its created as a reference (id and type -> which corresponds to a placement in the components section) (i.e new AsyncApiChannel() { Reference = new AsyncApiReference... }) or they have the data themselves.

If it has a reference, itll be looked up and inlined (depending on Writer and Reader settings) in the final document.

yurvon-screamo commented 1 month ago

I am really missing a "From this setup using Saunter, we expect this document to be produced" kind of test. or maybe i missed it?

Otherwise everything looks good i think - cant immediately spot anything.

What about bindings? Are these passed through the annotations?

The conflict should be easy as all of those files have been removed.

after this pull request, i will be implementing integration tests for the project, this will be one of them)

the lego package with bindings is connected, I will add a test that will check them + eliminate conflicts. I'll do it today or tomorrow

yurvon-screamo commented 1 month ago

I am really missing a "From this setup using Saunter, we expect this document to be produced" kind of test. or maybe i missed it?

Otherwise everything looks good i think - cant immediately spot anything.

What about bindings? Are these passed through the annotations?

The conflict should be easy as all of those files have been removed.

done:

add bindings to example project
add unit test with bindings
add docs about bindings

yurvon-screamo commented 1 week ago

@VisualBean Can we release it? It seems to me that we have already collected a lot of changes and it's time

VisualBean commented 1 week ago

@VisualBean Can we release it? It seems to me that we have already collected a lot of changes and it's time

You said something about following up with tests. I really want to ensure that the output is the same between the old version and the new. Or if not the same, is 'more' correct. Before we release this major overhaul

yurvon-screamo commented 1 week ago

@VisualBean Can we release it? It seems to me that we have already collected a lot of changes and it's time

You said something about following up with tests. I really want to ensure that the output is the same between the old version and the new. Or if not the same, is 'more' correct. Before we release this major overhaul

Yes, I plan to create a comprehensive test model of the project and implement it.

Now then I'll create an issue and start working on it)

asyncapi / saunter

LEGO DTO #221

188