Gi60s / openapi-enforcer

Apache License 2.0
94 stars 22 forks source link

Dereferencing doesn't preserve model structures #86

Closed delanym closed 4 years ago

delanym commented 4 years ago

When an external schema reference is resolved, the algorithm seems to take the properties of the external ref, place them in the position where they are first referenced, and then for the rest of the schema resolve that same external ref to the position of the first (now resolved) reference. This makes for a very confusing schema.

For example, in my model #/components/schemas/systemData I have a reference to blah.json#/components/schema/systemAuditNumber

When I dereference the schema, instead of creating a new, local model #/components/schema/systemAuditNumber, the properties are filled into #/components/schemas/systemData and sequent references to blah.json#/components/schema/systemAuditNumber instead refer to #/components/schemas/systemData/properties/systemAuditNumber

Even though the outcome is the same, or perhaps even more efficient, it does seem like a kind of obfuscation. Is this the intention? Just as I wouldn't give a minified json for someone to look at, I want to provide a json that is still consistently structured.

Gi60s commented 4 years ago

Have you tried the useNewRefParser configuration option?

https://byu-oit.github.io/openapi-enforcer/api/openapi-enforcer#enforcerconfig

delanym commented 4 years ago

Let me try that. I'm using openapi-enforcer-cli. How do I pass it config?

Gi60s commented 4 years ago

Just like the documentation shows. It's a global configuration option.

const Enforcer = require('openapi-enforcer');
Enforcer.config.useNewRefParser = true;
delanym commented 4 years ago

I don't understand. I'm in a shell, but you've given me javascript. From https://www.npmjs.com/package/openapi-enforcer-cli the output of openapi-enforcer build --help is

Usage: build [options] <oas-doc> <out-path>

Dereference and build a single OpenAPI file from multiple sources

Options:
  -h, --help  output usage information

I tried useNewRefParser=true as an option and got an error. So per https://stackabuse.com/the-ultimate-guide-to-configuring-npm/ I tried creating a file .npmrc with the line useNewRefParser=true and the output was the same. I tried export npm_config_useNewRefParser=true—same. Also tried npm config set openapi-enforcer-cli:useNewRefParser true I had a look at the package.json but couldn't find useNewRefParser in any of them. What am I doing wrong?

Gi60s commented 4 years ago

Oh sorry, you're using the CLI. I didn't realize that. I don't believe the option exists there.

I'll try to make some time tomorrow to integrate that functionality.

Gi60s commented 4 years ago

Okay, I've done some research on this and it's not going to be as easy as I thought. I built a custom ref parcer last year because the current ref parser I was using had some inadequacies. Unfortunately, the bundler portion of the app is still using the old ref parser. I can work on getting the new ref parser to also include bundling. I don't know exactly how difficult it's going to be, but it's not going to be done tonight. Just wanted to give you a heads up. I'll keep looking into it.

delanym commented 4 years ago

Thank you

Gi60s commented 4 years ago

I wanted to give you an update. I think I have this figured and am working on properly packaging it. Once it's ready I'll let you know.

Gi60s commented 4 years ago

OK. Try out the latest version of the openapi-enforcer-cli, version 0.3.3.

delanym commented 4 years ago

ok the usage line for openapi-enforcer -h doesn't have "build [options]", but build -h does. But what options are there? I tried useNewRefParser

Gi60s commented 4 years ago

The newest version of the CLI is using the new ref parser and does not have an option to use the old. In your case you're using the parser and bundler and I'm hoping it's giving you more of the output you're looking for.

delanym commented 4 years ago

Now I get errors like

One or more errors exist in the OpenApi definition at: components at: responses at: authorisation > content Value must be a plain object. Received: "#/components/responses/authorisation/content"

CLI v0.3.3 OpenAPI Enforcer v1.11.0

Gi60s commented 4 years ago

Would you be willing to share your openapi spec file with me?

delanym commented 4 years ago

I'll try post a sanitised version

delanym commented 4 years ago

Here's the base schema

openapi: 3.0.1
info:
    title: Elements
    version: 1.0.0
components:
    schemas:
        chun:
            type: string
        lllu:
            type: integer
        pebn:
            type: string
        wyan:
            type: integer
        allo:
            type: string
        kum:
            type: string
        wims:
            type: string
        feun:
            type: string
        lllu1:
            type: string
        lllu2:
            type: string
        lllu9:
            type: string
        kum7:
            type: string
        kum8:
            type: string
        qond8:
            type: string
        veni:
            type: string
        jiga:
            type: string

And the one to resolve

openapi: 3.0.1
info:
    title: Canyon
    version: 1.0.0
    description: '-'
paths:
    /waterfall:
        post:
            requestBody:
                content:
                    application/json:
                        schema:
                            $ref: '#/components/schemas/waterfall'
                required: true
            responses:
                '200':
                    content:
                        application/json:
                            schema:
                                type: integer
                    description: '-'
            operationId: waterfall
            summary: waterfall
            description: '-'
components:
    schemas:
        ag:
            description: '-'
            type: object
            properties:
                ta:
                    description: '-'
                    type: integer
        waterfall:
            description: '-'
            type: object
            properties:
                pn:
                    $ref: 'elements.yaml#/components/schemas/chun'
                tr:
                    $ref: 'elements.yaml#/components/schemas/lllu'
                td:
                    $ref: 'elements.yaml#/components/schemas/pebn'
                st:
                    $ref: 'elements.yaml#/components/schemas/wyan'
                tt:
                    $ref: 'elements.yaml#/components/schemas/allo'
                tl:
                    $ref: 'elements.yaml#/components/schemas/kum'
                to:
                    $ref: 'elements.yaml#/components/schemas/wims'
                rr:
                    $ref: 'elements.yaml#/components/schemas/feun'
                tf:
                    $ref: 'elements.yaml#/components/schemas/lllu1'
                mq:
                    $ref: 'elements.yaml#/components/schemas/lllu2'
                cc:
                    $ref: 'elements.yaml#/components/schemas/lllu9'
                ag:
                    $ref: '#/components/schemas/ag'
                aj:
                    $ref: 'elements.yaml#/components/schemas/kum7'
                ej:
                    $ref: 'elements.yaml#/components/schemas/kum8'
                bw:
                    $ref: 'elements.yaml#/components/schemas/qond8'
                ys:
                    $ref: 'elements.yaml#/components/schemas/veni'
                hh:
                    type: object
                    properties:
                        th:
                            $ref: 'elements.yaml#/components/schemas/jiga'

Command

openapi-enforcer build oas.yaml deref.json

Output

One or more errors exist in the OpenApi definition at: components > schemas at: ag at: properties Value must be a plain object. Received: "#/components/schemas/ag/properties" Unexpected error encountered: TypeError: Cannot convert undefined or null to object at Function.keys () at errors (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/enforcers/Schema.js:736:28) at fn (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/definition-validator.js:317:20) at normalize (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/definition-validator.js:294:13) at build (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/super.js:153:17) at new Schema (eval at createConstructor (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/super.js:37:15), :5:20) at runChildValidator (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/definition-validator.js:378:20) at /home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/definition-validator.js:136:44 at Array.forEach () at normalize (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/definition-validator.js:121:45) at: waterfall at: properties Value must be a plain object. Received: "#/components/schemas/waterfall/properties" Unexpected error encountered: TypeError: Cannot convert undefined or null to object at Function.keys () at errors (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/enforcers/Schema.js:736:28) at fn (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/definition-validator.js:317:20) at normalize (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/definition-validator.js:294:13) at build (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/super.js:153:17) at new Schema (eval at createConstructor (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/super.js:37:15), :5:20) at runChildValidator (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/definition-validator.js:378:20) at /home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/definition-validator.js:136:44 at Array.forEach () at normalize (/home/delany/.npm-global/lib/node_modules/openapi-enforcer-cli/node_modules/openapi-enforcer/src/definition-validator.js:121:45) at: paths at: /waterfall > post > requestBody > content > application/json > schema Value must be a plain object

Build failed

(node:491028) UnhandledPromiseRejectionWarning: [object Object] (Use node --trace-warnings ... to show where the warning was created) (node:491028) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag --unhandled-rejections=strict (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1) (node:491028) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Gi60s commented 4 years ago

Thank you for the sample to test against. That helped me to identify a few problems. I've pushed out those changes to NPM. Let me know if this does what you expect it to.

delanym commented 4 years ago

ALmost there I think. There's an issue where your tool requires specifying type object. Implicit type object gives

Properties not allowed: properties, required
Missing required property: type
Gi60s commented 4 years ago

This is a known issue that I haven't taken the time to solve yet. The work around for now is to make sure that all of your schemas have a type property.

delanym commented 4 years ago

Im not producing the schemas

Gi60s commented 4 years ago

I'm not sure what you mean by you're not producing the schemas. It's true that dereferencing and bundling move things around, but it doesn't change the schemas.

If I understand you right though we do need to get implicit types working. I've looked at this before but it's not a quick fix. I'll close this ticket for now and leave #92 open while that gets worked on.

delanym commented 4 years ago

Thanks. You said "make sure that all of your schemas have a type property", well they're not my schemas. Im not producing them, only validating and dereferencing them.