Open aeworxet opened 1 month ago
Hey @aeworxet If we leave the bundler out of the picture here, can you add a an example on which the optimizer is not producing the right output and how this PR resolves the issue?
Right now, Optimizer
produces output from the left side of the diff. The code in the PR adds components/
to make the $ref
correct (right side of the diff.)
My main question is whether $ref
in line 29 should be left intact or should it also be added the components/
, because both versions are correct.
It's just that in the current version, the channel
component in lines 6-8 has meaning because, with it, the AsyncAPI Document retains readability for humans in general. And if even one $ref
stops referring to it, there will not be even one reason it's still there (it's not mandatory to have #/channels
according to the Spec.)
@aeworxet based on the specification of Operation Object in v3:
If the operation is located in the root Operations Object, it MUST point to a channel definition located in the root Channels Object, and MUST NOT point to a channel definition located in the Components Object or anywhere else. If the operation is located in the Components Object, it MAY point to a Channel Object in any location.
and for messages:
It MUST contain a subset of the messages defined in the channel referenced in this operation, and MUST NOT point to a subset of message definitions located in the Messages Object in the Components Object or anywhere else.
I think the issue is bigger.
If the operation is located in the root Operations Object, it MUST point to a channel definition located in the root Channels Object, and MUST NOT point to a channel definition located in the Components Object or anywhere else.
Operation Object
in lines 9-11 doesn't point to a channel. It points to a copy of itself in the Components Object
, which is either OK or not allowed at all.
It passed Parser's validation, so it's OK.
Spec doesn't mention such a possibility, so it's not OK.
Which one is it?
@derberg @jonaslagoni @smoya
If the operation is located in the
Components Object
, it MAY point to aChannel Object
in any location.
So it is allowed to move Operation Object
to Components Object
, but the Operation Object
is not supposed to be in the root of the AsyncAPI Document after moveAllToComponents()
then, as I understand.
Or should it be there in the root, but pointing only to channels
in the root? And if channels
are deleted from root after moving? Should the root Operations Object
be tied to the root Channels Object
and either they are existing both in the root or none of them do?
And not only the fact that the root contains Operation Object
pointing to a copy of itself in Components Object
, but also the fact that it contains an invalid $ref
(pointing to operations
which is not mentioned in Spec) passed Parser's validation.
Optimizer
's output.yaml
didn't pass separate Parser's validation with asyncapi validate
, but then it wasn't supposed to be given out by Optimizer
as an output file to begin with.
$ asyncapi validate output.yaml
File output.yaml and/or referenced documents have governance issues.
Errors
output.yaml
31:11 error asyncapi3-operation-messages-from-referred-channel Operation message does not belong to the specified channel. components.operations.onUserSignUp.messages[0]
And when I remove components/
from the 31:11 $ref
to make it incorrect indeed, the file (which now has an intentionally wrong $ref
) passes validation...
So I think that Parser's validation rules need to be reviewed, and there needs to be implemented Parser's pre-validation, which will validate optimized files before giving them outside, as I did it with Bundler
.
Comments?
Issues
1 New issue
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code
Implemented pre-validation of the optimized AsyncAPI Documents before their output. Of course, some tests immediately started failing (which is good, because this means validation works.)
Fixing the mechanism of testing.
Checked outputs manually and here are some remarks.
@asyncapi/parser
v3.0.14
inputYAML
/ inputJSON
*_mATCTrue_mDTCFalse_schemaFalse
, *_mATCTrue_mDTCFalse_schemaTrue
Input file validates OK.
Output file gives validation errors:
[
{
code: 'asyncapi-document-resolved',
message: '"user~1deleteAccount" property must have required property "action"',
path: [ 'operations', 'user/deleteAccount' ],
severity: 0,
range: { start: [Object], end: [Object] }
// there is no `user~1deleteAccount` property, there is a `user/deleteAccount.subscribe` property
// that has child property `action` in both input and output YAMLs,
// and there is a `user/deleteAccount` which DOES NOT have the child property `action`.
// which one does Parser validate?
},
{
code: 'asyncapi-document-resolved',
message: 'Property "subscribe" is not expected to be here',
path: [ 'operations', 'user/deleteAccount', 'subscribe' ],
severity: 0,
source: undefined,
range: { start: [Object], end: [Object] }
// output file contains
operations:
user/deleteAccount.subscribe:
action: send
channel:
$ref: '#/channels/deleteAccount'
messages:
- $ref: '#/channels/deleteAccount/messages/deleteUser'
user/deleteAccount:
subscribe:
$ref: '#/components/operations/subscribe'
// so property "subscribe" is expected or not expected to be here?
// double parsing is on me because it should not have happened,
// the `dot` was parsed incorrectly during optimization
// Parser validates `user/deleteAccount.subscribe` OK in the input file.
},
{
code: 'invalid-ref',
path: [ 'operations', 'user/deleteAccount', 'subscribe', '$ref' ],
message: "'#/components/operations/subscribe' does not exist",
severity: 0,
range: { start: [Object], end: [Object] }
}
// this is obviously about
user/deleteAccount:
subscribe:
$ref: '#/components/operations/subscribe'
// this on is on me too because $ref IS wrong
]
There is no limitation on the usage of .
(dot) for naming properties in an AsyncAPI Document, and YAML containing the property user/deleteAccount.subscribe
passes validation with Parser
.
What mask should I use to find property
user/deleteAccount.subscribe
in this file, then?
{
path: 'channels.withFullFormMessage.messages.canBeReused',
action: 'move',
target: 'components.messages.canBeReused'
},
{
path: 'channels.withDuplicatedMessage1.messages.duped1',
action: 'move',
target: 'components.messages.duped1'
},
{
path: 'channels.withDuplicatedMessage2.messages.duped2',
action: 'move',
target: 'components.messages.duped2'
},
{
path: 'operations.user/deleteAccount.subscribe', // <-- I need to find this 'user/deleteAccount.subscribe'
action: 'move',
target: 'components.operations.subscribe'
},
{
path: 'channels.withDuplicatedMessage1',
action: 'move',
target: 'components.channels.withDuplicatedMessage1FromXOrigin'
},
{
path: 'channels.withDuplicatedMessage2',
action: 'move',
target: 'components.channels.withDuplicatedMessage2'
},
This PR updates
$ref
s of the AsyncAPI Document after moving the AsyncAPI components to thecomponents
section by prefixing the corresponding JSON Pointers with the characterscomponents/
.Addresses issue described in https://github.com/asyncapi/cli/issues/1323#issuecomment-2117488533.