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

Map/Dictionary Parsing Failing #71

Closed
ryanskinner opened this issue Apr 20, 2020 · 4 comments · Fixed by #72
Closed

Map/Dictionary Parsing Failing #71

ryanskinner opened this issue Apr 20, 2020 · 4 comments · Fixed by #72
Labels
bug Something isn't working released

Comments

@ryanskinner
Copy link

Describe the bug

I am not 100% sure this is a bug but I've been able the whittle this down to a simple document to duplicate it. I am only able to duplicate it using the CLI for generator, but I believe the issue is in the parser, hence I'm posting the issue here. The issue does not show up when using the playground online.

When I try to generate either an html or MD template from the YML file below, I get the following error:

Something went wrong:
TypeError: Cannot convert undefined or null to object
    at Function.entries (<anonymous>)
    at recursiveSchema (/Users/ryan/.nvm/versions/node/v13.13.0/lib/node_modules/@asyncapi/generator/node_modules/@asyncapi/parser/lib/models/asyncapi.js:272:49)
    at /Users/ryan/.nvm/versions/node/v13.13.0/lib/node_modules/@asyncapi/generator/node_modules/@asyncapi/parser/lib/models/asyncapi.js:327:11
    at Array.forEach (<anonymous>)
    at /Users/ryan/.nvm/versions/node/v13.13.0/lib/node_modules/@asyncapi/generator/node_modules/@asyncapi/parser/lib/models/asyncapi.js:325:40
    at Array.forEach (<anonymous>)
    at schemaDocument (/Users/ryan/.nvm/versions/node/v13.13.0/lib/node_modules/@asyncapi/generator/node_modules/@asyncapi/parser/lib/models/asyncapi.js:311:24)
    at assignIdToAnonymousSchemas (/Users/ryan/.nvm/versions/node/v13.13.0/lib/node_modules/@asyncapi/generator/node_modules/@asyncapi/parser/lib/models/asyncapi.js:346:3)
    at new AsyncAPIDocument (/Users/ryan/.nvm/versions/node/v13.13.0/lib/node_modules/@asyncapi/generator/node_modules/@asyncapi/parser/lib/models/asyncapi.js:25:5)
    at parse (/Users/ryan/.nvm/versions/node/v13.13.0/lib/node_modules/@asyncapi/generator/node_modules/@asyncapi/parser/lib/parser.js:128:10)

The generator I'm using is 0.39.1. I was able to hit the same issue with version 0.37.0.

I understand it very well could be that I'm not using the standard correctly. The documentation on how to do a map/dictionary, while present, is hard to find. And I believe there is not much regarding doing list/arrays. In the end I need to do a Map<String,List<Object>> but this example is just Map<String, String> to keep it simple.

How to Reproduce

asyncapi: '2.0.0'
defaultContentType: application/json
info:
  title: Map Object Test
  version: '1'

channels:
  some_channel:
    subscribe:
      message:
        name: some_map
        payload:
          type: object
          additionalProperties:
            type: string
          #properties:
          #  name:
          #    type: string

If I comment the additionalProperties branch and uncomment the properties branch, it works.

@ryanskinner ryanskinner added the bug Something isn't working label Apr 20, 2020
@github-actions
Copy link

Welcome to AsyncAPI. Thanks a lot for reporting your first issue.

Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@derberg
Copy link
Member

derberg commented Apr 21, 2020

@ryanskinner thanks for the issue and especially the example that made it super easy to find a root cause. The playground is a few releases behind, and this is why it might look strange to you that some things are working in the Playground while at the same time fail in the generator.

Imho your case is valid, json schema is a valid schema if there are no properties but additionalProperties is declared.

The part of the code that fails is https://github.com/asyncapi/parser-js/blob/master/lib/models/asyncapi.js#L271 where we expect that schema always have props and then we recursively go through all of them. I think we should simply return when there are no properties.

const props = schema.properties();
if(!props) return;

Tests work fine with the above change. I think we only would have to add an additional unit test for future. @jonaslagoni what do you think?

@jonaslagoni
Copy link
Member

Yep, the function assumes properties are always sat. Heard @derberg volunteered to change the library to property based testing 😉

Also, additional properties are not recognized as a "schema", that needs to change as well 🤔Didn't think of this when I implemented it...

I'll create a PR today fixing this.

jonaslagoni added a commit to jonaslagoni/parser-js that referenced this issue Apr 23, 2020
and fixes a problem with "additionalItems" are never checked
fmvilas added a commit that referenced this issue Apr 27, 2020
…ailing on windows (#72)

* fixes #71
and fixes a problem with "additionalItems" are never checked

* Fixed a problem with the parse_test on windows where path is required even if not on linux.
Fixed a problem with the parse_test where offsets are not caluclated.

* Removed forgotten console log.

* Update lib/models/asyncapi.js

Co-Authored-By: Fran Méndez <[email protected]>

* Update lib/models/asyncapi.js

Co-Authored-By: Fran Méndez <[email protected]>

Co-authored-by: Fran Méndez <[email protected]>
@asyncapi-bot
Copy link
Contributor

🎉 This issue has been resolved in version 0.18.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
4 participants