apiaryio / dredd

Language-agnostic HTTP API Testing Tool
https://dredd.org
MIT License
4.19k stars 280 forks source link

Swagger "tags" are misused as groups of resources, transaction paths could allow querying by tags #998

Open VaN-dev opened 6 years ago

VaN-dev commented 6 years ago

UPDATE: after some investigation, it appears that dredd is not using my hookfile when using "tags" in swagger. That's why all my POST/PUT tests fail, request body is supposed to be configured in the hookfile to make them pass.


Describe your problem

When using swagger "tags" option, POST and PUT tests fail.

What command line options do you use?

$ dredd apps/admin-api/swagger/api-description.bundle.json \
        "http://web:8000" \
        --language=vendor/bin/dredd-hooks-php \
        --hookfiles=apps/admin-api/swagger/dredd/hooks/*.php

What is in your dredd.yml?

none

What's your dredd --version output?

debug: Dredd version: 5.1.5
debug: Node.js version: v8.9.4
debug: Node.js environment: http_parser=2.7.0, node=8.9.4, v8=6.1.534.50, uv=1.15.0, zlib=1.2.11, ares=1.10.1-DEV, modules=57, nghttp2=1.25.0, openssl=1.0.2n, icu=59.1, unicode=9.0, cldr=31.0.1, tz=2017b
debug: System version: Linux 4.10.0-38-generic x64
debug: npm version: 5.6.0

Does dredd --level=debug uncover something?

nothing interesting

Can you send us failing test in a Pull Request?

no.

With the following json description, everything works fine :

    "/admin/auth/login": {
      "post": {
        "summary": "That's the entrypoint of the API, which gives a JSON Web Token to the client",
        "parameters": [
          {
            "in": "body",
            "name": "credentials",
            "schema": {
              "type": "object",
              "required": [
                "email",
                "password"
              ],
              "properties": {
                "email": {
                  "type": "string",
                  "format": "email"
                },
                "password": {
                  "type": "string"
                }
              }
            }
          }
        ],
        "responses": {
          "200": {
            "description": "OK",
            "schema": {
              "type": "object",
              "properties": {
                "token": {
                  "type": "string"
                },
                "user": {
                  "type": "object",
                  "properties": {
                    "id": {
                      "type": "string"
                    },
                    "email": {
                      "type": "string",
                      "format": "email"
                    },
                    "firstName": {
                      "type": "string"
                    },
                    "lastName": {
                      "type": "string"
                    }
                  }
                }
              }
            }
          }
        }
      }
    },

And if I had a "tags" parameter, the same test fails :

    "/admin/auth/login": {
      "post": {
        "summary": "That's the entrypoint of the API, which gives a JSON Web Token to the client",
        "tags": [
          "auth"
        ],
        "parameters": [...]
fail: POST (200) /admin/auth/login duration: 273ms

Another difference between using or not using tags :

When NOT using tags, got the following test :

pass: POST (204) /admin/auth/password-reset/74e8b54c-2d84-4b99-8ac2-a198bc3f2f95

When using tags :

warn: POST (204) /admin/auth/password-reset/74e8b54c-2d84-4b99-8ac2-a198bc3f2f95 HTTP 204 and 205 responses must not include a message body: https://tools.ietf.org/html/rfc7231#section-6.3
fail: POST (204) /admin/auth/password-reset/74e8b54c-2d84-4b99-8ac2-a198bc3f2f95 duration: 253ms

The 2 tests use the same following json description :

    "/admin/auth/password-reset/{passwordResetProcessId}": {
      "post": {
        "summary": "Commits the password reset process.",
        "parameters": [
          {
            "in": "path",
            "name": "passwordResetProcessId",
            "required": true,
            "type": "string",
            "x-example": "74e8b54c-2d84-4b99-8ac2-a198bc3f2f95"
          }
        ],
        "responses": {
          "204": {
            "description": "OK"
          }
        }
      }
    },
michalholasek commented 6 years ago

Hello @VaN-dev, thank you for the report. I don't have much time to look into this at the moment unfortunately, it would be a great help if you would be willing to provide failing test case - if you would be interested, but don't know where to start / look into, I can provide you with guidence.

Thanks!

honzajavorek commented 6 years ago

This is interesting. What is the output of running Dredd with --names when you try it on your Swagger files with and without tags?

VaN-dev commented 6 years ago

Well, something strange happens :

when NOT using tags, everything looks normal :

info: Successfully connected to hooks handler. Waiting 0.1s to start testing.
info: /admin/auth/login > That's the entrypoint of the API, which gives a JSON Web Token to the client > 200 > application/json
skip: POST (200) /admin/auth/login
info: /admin/auth/token/about > Gives some info about a JSON Web Token, like its TTL > 200 > application/json
skip: GET (200) /admin/auth/token/about
info: /admin/auth/token/renew > Renew a token before its expiration (gives the client a new, fresh JWT) > 200 > application/json
skip: POST (200) /admin/auth/token/renew
[...]
info: /admin/clients > Creates a Client > 201 > application/json
skip: POST (201) /admin/clients
info: /admin/clients > Returns a list of Clients. > 200 > application/json
skip: GET (200) /admin/clients?page=1&limit=10&sort=asc&order=id

When I add the tag auth-and-security to the route POST /admin/auth/login and the tag clients to the route POST /admin/clients, I get the following output :

info: Successfully connected to hooks handler. Waiting 0.1s to start testing.
info: auth-and-security > /admin/auth/login > That's the entrypoint of the API, which gives a JSON Web Token to the client > 200 > application/json
skip: POST (200) /admin/auth/login
info: auth-and-security > /admin/auth/token/about > Gives some info about a JSON Web Token, like its TTL > 200 > application/json
skip: GET (200) /admin/auth/token/about
info: auth-and-security > /admin/auth/token/renew > Renew a token before its expiration (gives the client a new, fresh JWT) > 200 > application/json
skip: POST (200) /admin/auth/token/renew
[...]
info: clients > /admin/clients > Creates a Client > 201 > application/json
skip: POST (201) /admin/clients
info: clients > /admin/clients > Returns a list of Clients. > 200 > application/json
skip: GET (200) /admin/clients?page=1&limit=10&sort=asc&order=id

it's like tag is inherited from one route to the following, until it is overwritten by a new tag. Is the the normal behavior ?

honzajavorek commented 6 years ago

I suspect this is a problem in the underlying toolchain for parsing the Swagger. @apiaryio/adt, are you aware of any special treatment tags would get? From the output @VaN-dev shares it looks like they behave like resource groups or something like that. Is that an expected behavior?

pksunkara commented 6 years ago

Yes, tags are resource groups.

honzajavorek commented 6 years ago

Isn't it a misinterpretation of the tags feature?

honzajavorek commented 6 years ago

@VaN-dev So it seems when you use tags, they put the resources into logical groups. This affects the "transaction paths" used in Dredd hooks to address requests. If you update the "transaction paths" in hooks to reflect the output of dredd --names, everything should work as expected.

Follow-up discussion could be whether tags should be interpreted this way (up to @apiaryio/adt and the Swagger adapter project), or whether resource groups should appear in the paths at all (IMHO they should not and their planned redesign in #227 should have that in mind).

Please let us know if the solution works for you.

kylef commented 6 years ago

Isn't it a misinterpretation of the tags feature?

@honzajavorek It is, and the way they are interpreted can be a little confusing. Especially when a resource is contained within multiple tags. It will only be in the first tag. This was a decision made during the initial development of Swagger support in Apiary.

We've been wanting to do this right, but that unfortunately is blocked by documentation rendering in Apiary to be on API Elements so that we can rehaul this section and introduce a tag concept into API Elements. Then tags can be supported in documentation render and we can move to them in the parser so we don't introduce a regression by removing this implicit resource group creation.

kylef commented 6 years ago

@honzajavorek When it's time for this change, this will have implications in Dredd too so we will have to think through it all and a path forward once we're unblocked. This could allow users to create hooks at the tag level in Dredd which could be neat!

honzajavorek commented 6 years ago

I agree. I think this really is related to #227 than anything else then.

VaN-dev commented 6 years ago

@honzajavorek

If you update the "transaction paths" in hooks to reflect the output of dredd --names, everything should work as expected.

I updated my hooks with your solution and it works : ) Thank you.