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

Flag when empty object is used for "schema" value in spec #443

Closed
nschonni opened this issue Jul 17, 2019 · 12 comments
Closed

Flag when empty object is used for "schema" value in spec #443

nschonni opened this issue Jul 17, 2019 · 12 comments

Comments

@nschonni
Copy link

In #436, OAV started to flag schema mismatches with the examples. Once case where these will still pass, is when the schema is defined as

        "responses": {
          "200": {
            "description": "The annotation that was successfully deleted.",
            "schema": {}
          }
        },

Then things like "body": "" are still passing.

EX: Azure/azure-rest-api-specs#6636

@sergey-shandar
Copy link
Contributor

That's correct, because empty schema means that it can be any value, ASFAIK.

@nschonni
Copy link
Author

This is one that I think was using it as the "any" value https://github.com/Azure/azure-rest-api-specs/blob/master/specification/web/resource-manager/Microsoft.Web/stable/2016-06-01/logicAppsManagementClient.json#L634-L639
But the others seemed to be masking the delete/empty body issue. Should it be a warning an suppressed if it is the exception instead of the rule?

@raych1
Copy link
Contributor

raych1 commented Jul 17, 2019

@nschonni , for the spec you referenced, this example would not fail. Can you clarify?

schema
"responses": {
          "200": {
            "description": "The annotation that was successfully deleted.",
            "schema": {}
          }
        }
example
{
  "parameters": {},
  "responses": {
    "200": {
      "body": {}
    }
  }
}

@nschonni
Copy link
Author

@raych1 the first one for application insights or the second web one?
What I mostly saw with the empty schemas was body="" which OAV now flags. I didn't check for an empty object body

@raych1
Copy link
Contributor

raych1 commented Jul 17, 2019

@nschonni , I updated above comment. Pls check. OAV would not flag when example body=""(which is empty case) and schema is empty.

@nschonni
Copy link
Author

@raych1 yeah, I think it should flag it unless it's

{
  "parameters": {},
  "responses": {
    "200": {}
  }

Which is what passes if the empty schema is removed

@raych1
Copy link
Contributor

raych1 commented Jul 21, 2019

@nschonni , like @sergey-shandar said empty schema means it can be any value. That's why undefined body would be flagged but empty body wouldn't be flagged.

@nschonni
Copy link
Author

Yeah, what I was suggesting here, is that the empty schema object is a valid value, but is more likely used incorrectly (single probably correct use case in the repo) vs. incorrectly used to hide other OAV warnings on DELETE operations. Since it's more likely not correct, it could be a warning and authors would get exceptions if they actually needed it

@raych1
Copy link
Contributor

raych1 commented Jul 29, 2019

@nschonni , can you share me an example of DELETE operation which incorrectly use the empty schema?
I don't see any schema defined in
https://github.com/Azure/azure-rest-api-specs/blob/master/specification/web/resource-manager/Microsoft.Web/stable/2016-06-01/logicAppsManagementClient.json for delete opreations.

@nschonni
Copy link
Author

The PR linked in the first post has the empty schema to hide the empty response

@raych1
Copy link
Contributor

raych1 commented Jul 29, 2019

Thanks @nschonni , I commented on the linked PR.
I would leave this issue as won't fix and will not add warning on misuse case for DELETE operation.

@raych1 raych1 closed this as completed Jul 29, 2019
@nschonni
Copy link
Author

OK, the original issue wasn't just about deletes, but I also doesn't look like it was that wide-spread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants