Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot reference yaml alias if anchor is in other comment #152

Open
psxpaul opened this Issue Feb 14, 2019 · 11 comments

Comments

Projects
None yet
2 participants
@psxpaul
Copy link

psxpaul commented Feb 14, 2019

I would like to use a yaml anchor to define in my swagger doc an object that is used in multiple places. If I have a comment like this in one file:

/**
 * @swagger
 * '/helloworld':
 *   get:
 *     summary: The helloworld API
 *     description: This is a description.
 *     security: []
 *     responses:
 *       200:
 *         description: OK
 *         content:
 *           application/json:
 *             schema:
 *               $ref: '#/components/schemas/HelloWorld'
 *     x-amazon-apigateway-integration: *default-integration
 */

And I have in my definition file:

x-amazon-apigateway-integrations:
  default-integration: &default-integration
    type: object
    x-amazon-apigateway-integration:
      httpMethod: POST
      passthroughBehavior: when_no_match
      type: aws_proxy
      uri: 'arn:aws:apigateway:us-east-1:lambda:path/2015-03-31/functions/arn:aws:lambda:us-east-1:123456789:function:helloworldlambda/invocations'

I get the following output when running swagger-jsdoc:

'/helloworld':
  get:
    summary: The helloworld API
    description: This is a description.
    security: []
    responses:
      200:
        description: OK
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/HelloWorld'
    x-amazon-apigateway-integration: *default-integration -------------- Pay attention at this place

/Users/proberts/src/hello-world/node_modules/swagger-jsdoc/lib/index.js:32
    throw new Error(err);
    ^

Error: YAMLException: unidentified alias "default-integration" at line 13, column 58:
     ... ntegration: *default-integration
                                         ^
    at module.exports.options (/Users/proberts/src/hello-world/node_modules/swagger-jsdoc/lib/index.js:32:11)
    at createSpecification (/Users/proberts/src/hello-world/node_modules/swagger-jsdoc/bin/swagger-jsdoc.js:39:31)
    at fs.readFile (/Users/proberts/src/hello-world/node_modules/swagger-jsdoc/bin/swagger-jsdoc.js:170:10)
    at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:511:3)

If I add the anchor to within the same swagger-jsdoc comment, it works as expected. I'm guessing the problem is that lib/helpers/filterJsDocComments.js is loading each comment as individual yaml documents.

I can help with a fix, if there's a good idea on how to solve this. Maybe a new optional anchors file, or root definition property, that can be concatenated to every swagger-jsdoc comment in lib/helpers/filterJsDocComments.js?

@kalinchernev

This comment has been minimized.

Copy link
Collaborator

kalinchernev commented Feb 16, 2019

Hi @psxpaul I spent a few hours trying to understand the problem, but couldn't.

From the snippets you shared I ended up trying to put things in one file trying to make a working example as following:

const express = require("express");
const swaggerJSDoc = require("swagger-jsdoc");

const port = process.env.PORT || 3000;
const app = express();

/**
 * @swagger
 *
 * x-amazon-apigateway-integrations:
 *  default-integration: &default-integration
 *   type: object
 *   x-amazon-apigateway-integration:
 *     httpMethod: POST
 *     passthroughBehavior: when_no_match
 *     type: aws_proxy
 *     uri: 'arn:aws:apigateway:us-east-1:lambda:path/2015-03-31/functions/arn:aws:lambda:us-east-1:123456789:function:helloworldlambda/invocations'
 *
 * '/helloworld':
 *   get:
 *     summary: The helloworld API
 *     description: This is a description.
 *     security: []
 *     responses:
 *       200:
 *         description: OK
 *         content:
 *           application/json:
 *             schema:
 *               $ref: '#/components/schemas/HelloWorld'
 *     x-amazon-apigateway-integration: *default-integration
 */
app.get("/", () => {});

const swaggerDefinition = {
  openapi: "3.0.0",
  info: {
    title: "AWS API Gateway",
    version: "1.0.0"
  }
};

const options = {
  swaggerDefinition,
  apis: ["./*.js", "./components.yaml"]
};

const swaggerSpec = swaggerJSDoc(options);

app.get("/api", (req, res) => {
  res.send(swaggerSpec);
});

app.listen(port, () => console.log(`http://localhost:${port}`));

The resulting spec is:

{
  "openapi": "3.0.0",
  "info": { "title": "AWS API Gateway", "version": "1.0.0" },
  "paths": {
    "x-amazon-apigateway-integrations": {
      "default-integration": {
        "type": "object",
        "x-amazon-apigateway-integration": {
          "httpMethod": "POST",
          "passthroughBehavior": "when_no_match",
          "type": "aws_proxy",
          "uri": "arn:aws:apigateway:us-east-1:lambda:path/2015-03-31/functions/arn:aws:lambda:us-east-1:123456789:function:helloworldlambda/invocations"
        }
      }
    },
    "/helloworld": {
      "get": {
        "summary": "The helloworld API",
        "description": "This is a description.",
        "security": [],
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "application/json": {
                "schema": { "$ref": "#/components/schemas/HelloWorld" }
              }
            }
          }
        },
        "x-amazon-apigateway-integration": {
          "type": "object",
          "x-amazon-apigateway-integration": {
            "httpMethod": "POST",
            "passthroughBehavior": "when_no_match",
            "type": "aws_proxy",
            "uri": "arn:aws:apigateway:us-east-1:lambda:path/2015-03-31/functions/arn:aws:lambda:us-east-1:123456789:function:helloworldlambda/invocations"
          }
        }
      }
    }
  },
  "components": {
    "HelloWorld": {
      "type": "object",
      "properties": { "data": { "type": "string" } }
    }
  },
  "tags": []
}

At this stage I don't even know which spec you're targeting, which isn't important, but from what I googled, version 3 has been supported since a few months.

Reflecting only results so far, the fact that x-amazon-apigateway-integrations is parsed as a path is obviously wrong. I am not sure at all whether you actually have your code in this way or not. Sharing your code which I can copy-paste and try would be useful.

This is the pull request which enabled anchors between yamls. Not sure whether your issue is coming from filterJsDocComments, it's taking comments, not touching references.

An idea which comes to mind is for you to try to take what you share in @swagger comment and place it in a separate yaml file. That would be an easy path to validate whether it's the comments parser or the yaml parser.

@psxpaul

This comment has been minimized.

Copy link
Author

psxpaul commented Feb 16, 2019

Hi @kalinchernev , thanks for the quick response. Here's a modified version of what you posted:

index.js:

const express = require("express");
const swaggerJSDoc = require("swagger-jsdoc");

const port = process.env.PORT || 3000;
const app = express();

/**
 * @swagger
 * '/helloworld':
 *   get:
 *     summary: The helloworld API
 *     description: This is a description.
 *     security: []
 *     responses:
 *       200:
 *         description: OK
 *         content:
 *           application/json:
 *             schema:
 *               $ref: '#/components/schemas/HelloWorld'
 *     x-amazon-apigateway-integration: *default-integration
 */
app.get("/", () => {});

const swaggerDefinition = {
  openapi: "3.0.0",
  info: {
    title: "AWS API Gateway",
    version: "1.0.0"
  }
};

const options = {
  swaggerDefinition,
  apis: ["./*.js", "./x-amazon-apigateway-integrations.yaml"]
};

const swaggerSpec = swaggerJSDoc(options);

app.get("/api", (req, res) => {
  res.send(swaggerSpec);
});

app.listen(port, () => console.log(`http://localhost:${port}`));

x-amazon-apigateway-integrations.yaml:

x-amazon-apigateway-integrations:
  default-integration: &default-integration
    type: object
    x-amazon-apigateway-integration:
      httpMethod: POST
      passthroughBehavior: when_no_match
      type: aws_proxy
      uri: 'arn:aws:apigateway:us-east-1:lambda:path/2015-03-31/functions/arn:aws:lambda:us-east-1:123456789:function:helloworldlambda/invocations'

Running node index.js results in:

[paul]:[/tmp/test-swagger-jsdoc]$ npm start

> test-swagger-jsdoc@1.0.0 start /tmp/test-swagger-jsdoc
> node index.js

'/helloworld':
  get:
    summary: The helloworld API
    description: This is a description.
    security: []
    responses:
      200:
        description: OK
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/HelloWorld'
    x-amazon-apigateway-integration: *default-integration -------------- Pay attention at this place

/tmp/test-swagger-jsdoc/node_modules/swagger-jsdoc/lib/index.js:32
    throw new Error(err);
    ^

Error: YAMLException: unidentified alias "default-integration" at line 13, column 58:
     ... ntegration: *default-integration
                                         ^
    at module.exports.options (/tmp/test-swagger-jsdoc/node_modules/swagger-jsdoc/lib/index.js:32:11)
    at Object.<anonymous> (/tmp/test-swagger-jsdoc/index.js:38:21)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Function.Module.runMain (module.js:693:10)
    at startup (bootstrap_node.js:188:16)
    at bootstrap_node.js:609:3
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! test-swagger-jsdoc@1.0.0 start: `node index.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the test-swagger-jsdoc@1.0.0 start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/paul/.npm/_logs/2019-02-16T21_48_59_868Z-debug.log

I believe it is failing at this line because each jsdoc comment is parsed as individual yaml documents. The comment contains an alias to an anchor that is defined somewhere else, so jsYaml throws an "unidentified alias" exception.

My project has several express mappings across different files, and I would like to avoid copy-pasting the same 7 lines of yaml for each one.

@kalinchernev

This comment has been minimized.

Copy link
Collaborator

kalinchernev commented Feb 17, 2019

Hi @psxpaul thanks for helping me to reproduce the issue!

Indeed, the error is saying that there's an unidentified alias. Debugging it, however, I get the feeling it might be an issue of order, rather than the two parsers swagger-jsdoc is using.

My first thought was: maybe the items in the comment are not valid, because the information/definition from the yaml file is not available yet. So I changed this part to

  for (let i = 0; i < apiPaths.length; i += 1) {
    const files = parseApiFile(apiPaths[i]);

    specHelper.addDataToSwaggerObject(specification, files.yaml);

    const swaggerJsDocComments = filterJsDocComments(files.jsdoc);
    specHelper.addDataToSwaggerObject(specification, swaggerJsDocComments);
  }

This results in the same issue.

Then, I told myself, maybe if I put the comment into another yaml file, then the files.yaml part will be "together at one place" at the time of the parsing, but this didn't help.

That's the point where I made a search about the generic issue and found this which brings me to the thought that the issue could be coming from the order.

Lastly, I checked the yaml parsing library if there would be any possible options to wait for interpreting anchors later. There appeared to be several existing issues regarding this.

@psxpaul

This comment has been minimized.

Copy link
Author

psxpaul commented Feb 17, 2019

If I change this part to:

      if (tag.title === 'swagger') {
        console.log('parsing: ');
        console.dir(tag.description);
        swaggerJsDocComments.push(jsYaml.safeLoad(tag.description));
        console.log('parsed');
      }

Then run it again:

[paul]:[/tmp/test-swagger-jsdoc]$ npm start

> test-swagger-jsdoc@1.0.0 start /tmp/test-swagger-jsdoc
> node index.js

parsing: 
'\'/helloworld\':\n  get:\n    summary: The helloworld API\n    description: This is a description.\n    security: []\n    responses:\n      200:\n        description: OK\n        content:\n          application/json:\n            schema:\n              $ref: \'#/components/schemas/HelloWorld\'\n    x-amazon-apigateway-integration: *default-integration'
'/helloworld':
  get:
    summary: The helloworld API
    description: This is a description.
    security: []
    responses:
      200:
        description: OK
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/HelloWorld'
    x-amazon-apigateway-integration: *default-integration -------------- Pay attention at this place

/tmp/test-swagger-jsdoc/node_modules/swagger-jsdoc/lib/index.js:32
    throw new Error(err);
    ^

Error: YAMLException: unidentified alias "default-integration" at line 13, column 58:
     ... ntegration: *default-integration
                                         ^
    at module.exports.options (/tmp/test-swagger-jsdoc/node_modules/swagger-jsdoc/lib/index.js:32:11)
    at Object.<anonymous> (/tmp/test-swagger-jsdoc/index.js:38:21)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Function.Module.runMain (module.js:693:10)
    at startup (bootstrap_node.js:188:16)
    at bootstrap_node.js:609:3
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! test-swagger-jsdoc@1.0.0 start: `node index.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the test-swagger-jsdoc@1.0.0 start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/paul/.npm/_logs/2019-02-17T07_11_47_041Z-debug.log

It looks to me like jsYaml.safeLoad(tag.description) is parsing the yaml format of one single comment, without any context from any other files. As far as I can tell, jsYaml doesn't re-use state between calls, so anchors that were parsed previously wouldn't be resolved on subsequent calls.

I also did a brief test with one yaml file that contains an alias to an anchor in a second file and saw similar results. I suspect the same thing is happening at this line.

If I get some time on Monday, I'll try to hack something together to read a 'x-anchors' root property on the swaggerDefinition, and prepend that yaml to all jsdoc comments and yaml api files before parsing.

@kalinchernev

This comment has been minimized.

Copy link
Collaborator

kalinchernev commented Feb 17, 2019

@kalinchernev

This comment has been minimized.

Copy link
Collaborator

kalinchernev commented Feb 18, 2019

Hi @psxpaul I made a few changes to illustrate my idea at #153

The idea is to save the parts in an array and get them together before the parsing. What do you think?

screenshot from 2019-02-18 08-41-44

@psxpaul

This comment has been minimized.

Copy link
Author

psxpaul commented Feb 18, 2019

Is the idea to do something like specHelper.addDataToSwaggerObject(specification, jsYaml.safeLoad(specParts.join('\n'))); where the debugger line is? I think that could work, and it would be very helpful for my purposes.

@kalinchernev

This comment has been minimized.

Copy link
Collaborator

kalinchernev commented Feb 19, 2019

Hi @psxpaul

addDataToSwaggerObject adds a parsed object to the specification which is delivered to the user as an end result. It has to be called after being sure that the jsYaml can do the parsing.

I removed the two parsing events (here and here) in order to defer the parsing for when the a string contains all the necessary references.

specParts.join('\n') won't be enough I think, as the structure of the specParts contains both jsdoc and yaml which is what we have so far from both types of sources: comments and yaml files correspondingly. Probably a map before the join would be necessary in order for the strings to be in one place.

Then, yes, you can even make another helper with a test. It's easier and faster to iterate on an array of objects, rather than re-running the whole thing each time you want to test.

Here's my example:

[
  {
    "yaml": [],
    "jsdoc": [
      {
        "description": "",
        "tags": [
          {
            "title": "swagger",
            "description": "'/helloworld':\n  get:\n    summary: The helloworld API\n    description: This is a description.\n    security: []\n    responses:\n      200:\n        description: OK\n        content:\n          application/json:\n            schema:\n              $ref: '#/components/schemas/HelloWorld'\n    x-amazon-apigateway-integration: *default-integration"
          }
        ]
      }
    ]
  },
  {
    "yaml": [
      "components:\n  HelloWorld:\n    type: object\n    properties:\n      data:\n        type: string\n"
    ],
    "jsdoc": []
  },
  {
    "yaml": [
      "x-amazon-apigateway-integrations:\n  default-integration: &default-integration\n    type: object\n    x-amazon-apigateway-integration:\n      httpMethod: POST\n      passthroughBehavior: when_no_match\n      type: aws_proxy\n      uri: \"arn:aws:apigateway:us-east-1:lambda:path/2015-03-31/functions/arn:aws:lambda:us-east-1:123456789:function:helloworldlambda/invocations\"\n"
    ],
    "jsdoc": []
  }
]

Lastly, yes, the parts being combined into one string which will contain the necessary references and aliases, etc., hopefully it will be able to be parsed with jsYaml.safeLoad().

@kalinchernev

This comment has been minimized.

Copy link
Collaborator

kalinchernev commented Feb 23, 2019

@psxpaul it's quite possible you have already come to the same conclusion, seems like it's a different issue.

After combining the strings before parsing them:

'/helloworld':
  get:
    summary: The helloworld API
    description: This is a description.
    security: []
    responses:
      200:
        description: OK
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/HelloWorld'
    x-amazon-apigateway-integration: *default-integration


components:
  HelloWorld:
    type: object
    properties:
      data:
        type: string


x-amazon-apigateway-integrations:
  default-integration: &default-integration
    type: object
    x-amazon-apigateway-integration:
      httpMethod: POST
      passthroughBehavior: when_no_match
      type: aws_proxy
      uri: "arn:aws:apigateway:us-east-1:lambda:path/2015-03-31/functions/arn:aws:lambda:us-east-1:123456789:function:helloworldlambda/invocations"

Placing it in the first online validator, I get the following error:

Error : Reference "default-integration" does not exist.
Line : x-amazon-apigateway-integration: *default-integration  undefined

This is valid, however :D

x-amazon-apigateway-integrations:
  default-integration: &default-integration
    type: object
    x-amazon-apigateway-integration:
      httpMethod: POST
      passthroughBehavior: when_no_match
      type: aws_proxy
      uri: "arn:aws:apigateway:us-east-1:lambda:path/2015-03-31/functions/arn:aws:lambda:us-east-1:123456789:function:helloworldlambda/invocations"
      
      
'/helloworld':
  get:
    summary: The helloworld API
    description: This is a description.
    security: []
    responses:
      200:
        description: OK
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/HelloWorld'
    x-amazon-apigateway-integration: *default-integration


components:
  HelloWorld:
    type: object
    properties:
      data:
        type: string

So, it's not only about having all parts at once for the parsing to be successful. Order is important as well.

@kalinchernev

This comment has been minimized.

Copy link
Collaborator

kalinchernev commented Feb 23, 2019

That's a very ugly try. The problem now changing order is that something else down the road doesn't work.

Such as

tags:
  name: Users
  description: User management and login
tags: -------------- Pay attention at this place
  - name: Login
    description: Login
  - name: Accounts
    description: Accounts

/home/kalin/Projects/swagger-jsdoc/lib/index.js:32
    throw new Error(err);
    ^

Error: YAMLException: duplicated mapping key at line 36, column -89:

Having reached this point, knowing that order is important, but the fix won't be as easy as I thought, I start to have this feeling we'll need to think about solving order issues, not so much parsing such ...

@kalinchernev

This comment has been minimized.

Copy link
Collaborator

kalinchernev commented Feb 24, 2019

Tags have specifics and for not going too far, maybe the best way to work out the problem at this stage is to continue on the path of preparing the items for the yaml parser, but only reorganize and then split again and let the rest of the logic work like it does now.

Meaning, this (maybe) could make sense to an extend, but shouldn't do as much as this. Instead of simply finding anchors and hoisting them irresponsibly, a possible better way would be to parse and find anchors and merge them only with parts containing references to the same named anchors. This way, we ensure the parts which need to be in one place are together, but then we split them again and let the rest of logic work as it is now to lower the impact.

The helper I've started can go here without the rest of the things I've drafted in the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.