[FEATURE] Resolve $refs relative to the parsed file's directory by default

Laupetin commented 1 month ago

Why do we need this improvement?

Resolving relative paths from asyncapi documents located in different folders is currently not possible in a way that would keep the scope of the single files locally.

For example i am trying to destructure our asyncapi spec into smaller documents to reduce complexity. To do this i tried to use the following folder structure:

docs
├── common
│   └── metadata.yaml
├── index.yaml
├── receive
│   └── event-zero
│       ├── asyncapi.yaml
│       └── schema.yaml
└── send
    └── event-one
        ├── asyncapi.yaml
        └── schema.yaml

index.yaml would be the base file and our different operations would be split into separate files. Now when trying to bundle this into a single asyncapi spec i am running into a problem when trying to reference schema.yaml from any asyncapi.yaml via ./schema.yaml.

By default it will take the current working directory which is try to resolve it to docs/schema.yaml. With the help of the --baseDir arg I would only be able to set the baseDir to docs. I would still have to reference schema.yaml via a path that is not relative to the files that define each operation (event-zero, event-one): receive/event-zero/schema.yaml.

While that works, it is easy to make a mistake when creating a new operation and it also makes it impossible to validate each document separately, since validating always assumes $refs relative to the document's directory.

How will this change help?

I am able to structure each split asyncapi document i want to bundle into separate folders and each file can only be aware of its own context.
Creating new asyncapi documents for additional operations is less prone to copy paste errors due to be relative to their own directory
Validating/Generating based on each asyncapi document separately is possible
- One thing I personally want to do is to generate a markdown for each operation separately but an html documentation with all operations of the service combined. This is currently not possible like this because asyncapi generate fromTemplate parsing expects the paths to be relative to the document without the possibility to specify a "baseDir"
It is less confusing to read the documents because it is clear, what the path in $refs are relative to

Screenshots

No response

How could it be implemented/designed?

Change directory to the folder of each asyncapi document when dereferencing any $refs, unless a --baseDir option is specified (to not break the functionality of it).

Alternatively it is also possible to implement this in a non-breaking way in which it would only apply when a new option like --path-relative-to-document is specified in case a breaking change by changing default behaviour is undesired.

🚧 Breaking changes

Yes

👀 Have you checked for similar open issues?

[X] I checked and didn't find a similar issue

🏢 Have you read the Contributing Guidelines?

[X] I have read the Contributing Guidelines

Are you willing to work on this issue?

Yes I am willing to submit a PR!

github-actions[bot] commented 1 month 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.

aeworxet commented 1 month ago

@Laupetin Please share files, structured the way you mentioned as an archive, a comment, or a separate branch in https://github.com/Laupetin/asyncapi-bundler Anonymized, but as close to real as possible (or use one of the examples to keep the structure completely generic,) before we move further with this issue.

Laupetin commented 1 month ago

@aeworxet I created an example for demonstration purposes, see https://github.com/asyncapi/bundler/pull/179#issuecomment-2242420012

aeworxet commented 1 month ago

I have requested additional opinions on this subject in https://asyncapi.slack.com/archives/CQVJXFNQL/p1721883187801609

The exerpt:

#

While the default behavior of Bundler cannot be changed because it will violate the JSON standard's behavior

Resolution is performed relative to the referring document.

around which the Bundler is built, I still think that this usecase is bigger than just one user's request, because this directory structure seems useful for documenting complex APIs.

So, based on this, I lean toward adding something like the --traverse-subdirs (name is free for proposals) option, which:

if specified
- with option --base simultaneously specified, will traverse subdirectories, resolve $refs of each YAML it finds there, and merge each resulting bundled sub-YAML into the main - --base - YAML.
- with option --base NOT specified, will consider --base the 'usual' YAML (that was provided as an argument to the Bundler) and still do everything described above
if NOT specified
- Bundler will retain its current behavior, considering the flat $ref structure as it does now (dereferencing only explicit references it is able to find, relative to the referring document)

Does the addition of such an option make sense? Should it be doing what I described? Do you have other thoughts on this subject?

#

Laupetin commented 1 month ago

if NOT specified

Bundler will retain its current behavior, considering the flat $ref structure as it does now (dereferencing only explicit references it is able to find, relative to the referring document)

I think there is a misunderstanding there :sweat_smile: The issue is that the bundler does not do that currently. What I am suggesting is making him do that.

For example if your project and working directory is located in /home/example and you try to bundle docs/asyncapi/index.yaml and inside it refers to ./schema.json then:

The bundler will currently resolve it to /home/example/schema.json which is not what the linked standard suggests.
What it should resolve it to is /home/example/docs/asyncapi/schema.json (which is relative to the document) and that is also what other asyncapi tools like the generator and validator do

Laupetin commented 1 month ago

I'm sorry if me talking about about glob in the PR made things more confusing :sweat_smile: This issue was just meant for trying to change the behaviour to be resolved to "relative to the document".

Before I didn't know it was defined in the standard. Since it is I guess this could also be considered a bug then instead of a "feature"?

aeworxet commented 1 month ago

It is still not fully understandable whether you expect Bundler to do what it was initially made for, because there are no $refs in the ./docs/asyncapi/index.yaml at all.

Please read https://asyncapi.slack.com/archives/CQVJXFNQL/p1716582167100109 and suggest whether it is the Bundler's documentation that should be made clearer, or whether a user of Bundler intuitively expects something else?

Laupetin commented 1 month ago

It is still not fully understandable whether you expect Bundler to do what it was initially made for, because there are no $refs in the ./docs/asyncapi/index.yaml at all.

The index.yaml does not but the docs/asyncapi/send/lightTurnOn in Line 25 does. The example i made show what should work imo. If you clone it and run it you will see that the bundler will throw an error:

ResolverError: Error opening file "/home/example/asyncapi/bundler/schema.json" 
ENOENT: no such file or directory, open '/home/example/asyncapi/bundler/schema.json'

All other scripts like gen-markdown or validate work, just bundle does not. I did note the details for each script in the readme of the branch.

aeworxet commented 1 month ago

I'll compare how different tools resolve paths.

aeworxet commented 1 month ago

Other tools never had to deal with the resolution of $refs or the creation of a stack of YAML/JSON files that needed to be merged together. They always operate on one solid, complete AsyncAPI Document in YAML or JSON format. That's why there's a difference between handling a pack of paths and keeping calm about being fed a list of paths from an iterated array.

Bundler needs to develop its own approach to solve https://asyncapi.slack.com/archives/CQVJXFNQL/p1721921263521989?thread_ts=1721883187.801609&cid=CQVJXFNQL and I propose

{
  base: 'index.yaml',
  baseDir: 'docs/asyncapi',
  traverseSubdirsFor: 'asyncapi.yaml',
}

With these options specified, the Bundler will:

traverse nested subdirs relative to baseDir for file traverseSubdirsFor (using glob, for example, in a way it's done here)
make the directory where it found a traverseSubdirsFor file a new current Node.js dir, resolve all $refs relative to traverseSubdirsFor, repeat in case of a need (maybe creating a separate JS object with { 0: 'bundled.yaml from dir0', 1: 'bundled.yaml from dir1', 2: 'bundled.yaml from dir2', }, maybe saving down a bundled.yaml next to traverseSubdirsFor file it just dereferenced, maybe both, maybe depending on a switch in the options object)
merge all bundled YAMLs into base
pre-validate with Parser before output
output the resulting bundled.yaml if it passed pre-validation

Without traverseSubdirsFor, the Bundler will assume that it should work as usual, at a flat level, without traversing into subdirectories, only resolving $refs relative to the main YAML/JSON specified as a parameter.

@laupetin, @derberg, how does that look?

Laupetin commented 1 month ago

I am a bit confused about why you bring up a traverse subdirectories option 😅 For the issue here it is not really required as far as I saw it.

The reason why it is not required is that the bundler resolves the $refs before bundling. So you do not need to think about which folders apply to which $ref because all of them have been resolved at the bundling step already.

I did already propose a solution to the issue with the linked pull request that makes sure any loaded asyncapi document uses its own folder when resolving $refs. It is only afterwards that they are bundled. It fixes the issue in the example I have given. If i'm forgetting something and there is a reason why that approach doesnt work, please tell me. Right now im just confused about why that traversing logic would be necessary.

Without traverseSubdirsFor, the Bundler will assume that it should work as usual, at a flat level, without traversing into subdirectories, only resolving $refs relative to the main YAML/JSON specified as a parameter.

This is not what it does currently, it resolves $refs based on the current working directory by default, not based on the folder of the main yaml/json

aeworxet commented 1 month ago

@Laupetin

Your directory structure is potentially a new convention for structuring complex systems built on asynchronous APIs, where each operation is the responsibility of a separate team, up to having its own separate Git repository.

For now, there is no convention that a bundling tool should look exactly for a file asyncapi.yaml in nested directories (thus traverseSubdirsFor,) and there is no convention for how nested directories should even be named.

Should their naming repeat JSON Pointers' structure of the main AsyncAPI Document that describes the full system, so it could be parsed? Should there be a switch that lays down bundled YAMLs next to asyncapi.yaml for gathering them later by a CI? How about DE-bundling when a tool cuts the main AsyncAPI Document to pieces, using JSON Pointers' structure to FORM nested directories?

To me, the matter raised in this issue is not a feat!: because Bundler didn't reach v1.0.0 yet, but certainly not a small fix: because it nudges to rethinking the very ideology of Bundler.

Therefore, letting the current SIG group know about this idea just in case. @fmvilas @jonaslagoni @smoya

#

However, if I was fixing this issue as a plain bug, I would use one loop utilizing the tools I already have, without introducing new ones, and it would still be doing the same thing as https://github.com/asyncapi/bundler/pull/179

Please, copy&paste this code into your index.ts from master, compile, and check on your setup.

`index.ts`

```ts import { readFileSync } from 'fs'; import path from 'path'; import { toJS, resolve, versionCheck, getSpecVersion } from './util'; import { Document } from './document'; import { parse } from './parser'; import type { AsyncAPIObject } from './spec-types'; // remember the directory where execution of the program started const originDir = String(process.cwd()); /** * * @param {string | string[]} files One or more relative/absolute paths to * AsyncAPI Documents that should be bundled. * @param {Object} [options] * @param {string} [options.base] One relative/absolute path to base object whose * properties will be retained. * @param {string} [options.baseDir] One relative/absolute path to directory * relative to which paths to AsyncAPI Documents that should be bundled will be * resolved. * @param {boolean} [options.xOrigin] Pass `true` to generate properties * `x-origin` that will contain historical values of dereferenced `$ref`s. * * @return {Document} * * @example * ***TypeScript** *```ts *import { writeFileSync } from 'fs'; *import bundle from '@asyncapi/bundler'; * *async function main() { * const document = await bundle(['social-media/comments-service/main.yaml'], { * baseDir: 'example-data', * xOrigin: true, * }); * * console.log(document.yml()); // the complete bundled AsyncAPI document * writeFileSync('asyncapi.yaml', document.yml()); // the complete bundled AsyncAPI document *} * *main().catch(e => console.error(e)); *``` * ***JavaScript CJS module system** *```js *'use strict'; * *const { writeFileSync } = require('fs'); *const bundle = require('@asyncapi/bundler'); * *async function main() { * const document = await bundle(['social-media/comments-service/main.yaml'], { * baseDir: 'example-data', * xOrigin: true, * }); * writeFileSync('asyncapi.yaml', document.yml()); *} * *main().catch(e => console.error(e)); *``` * ***JavaScript ESM module system** *```js *'use strict'; * *import { writeFileSync } from 'fs'; *import bundle from '@asyncapi/bundler'; * *async function main() { * const document = await bundle(['social-media/comments-service/main.yaml'], { * baseDir: 'example-data', * xOrigin: true, * }); * writeFileSync('asyncapi.yaml', document.yml()); *} * *main().catch(e => console.error(e)); *``` * */ export default async function bundle( files: string[] | string, options: any = {} ) { // if one string was passed, convert it to an array if (typeof files === 'string') { files = Array.from(files.split(' ')); } if (options.baseDir && typeof options.baseDir === 'string') { process.chdir(path.resolve(originDir, options.baseDir)); } else if (options.baseDir && Array.isArray(options.baseDir)) { process.chdir(path.resolve(originDir, String(options.baseDir[0]))); // guard against passing an array } // -------- CHANGED CODE -------- const readFiles: AsyncAPIObject[] = [] for (const file of files) { const prevDir = process.cwd(); let filePath: any = file.split(path.sep) filePath.pop() filePath = filePath.join(path.sep) let readFile: any = readFileSync(file, 'utf-8') readFile = toJS(readFile) if (filePath) { process.chdir(filePath); } readFile = await parse(readFile, getSpecVersion(readFile), options) readFiles.push(readFile) if (prevDir) { process.chdir(prevDir); } } // -------- CHANGED CODE -------- const parsedJsons = readFiles.map(file => toJS(file)) as AsyncAPIObject[]; const majorVersion = versionCheck(parsedJsons); if (typeof options.base !== 'undefined') { if (typeof options.base === 'string') { options.base = readFileSync(options.base, 'utf-8'); // eslint-disable-line } else if (Array.isArray(options.base)) { options.base = readFileSync(String(options.base[0]), 'utf-8'); // eslint-disable-line } options.base = toJS(options.base); await parse(options.base, majorVersion, options); } const resolvedJsons: AsyncAPIObject[] = await resolve( parsedJsons, majorVersion, options ); // return to the starting directory before finishing the execution if (options.baseDir) { process.chdir(originDir); } return new Document(resolvedJsons, options.base); } // 'module.exports' is added to maintain backward compatibility with Node.js // projects, that use CJS module system. module.exports = bundle; ```

aeworxet commented 1 month ago

This functionality is implemented in Bundler v0.6.0.

aeworxet commented 1 month ago

This functionality is added in CLI v2.3.1.

asyncapi / bundler