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

Not picking up examples under schema #82

Open
geoffreywiseman opened this issue Jun 26, 2020 · 10 comments
Open

Not picking up examples under schema #82

geoffreywiseman opened this issue Jun 26, 2020 · 10 comments
Labels

Comments

@geoffreywiseman
Copy link

geoffreywiseman commented Jun 26, 2020

While investigating a documentation defect, I ran openapi-examples-validator and saw that it reported 13 examples, but if I ran a regexp for examples?: I got 24 matches, and my OAS spec is passing validation elsewhere, so I don't think it's malformed. I'm working my way through them, but it seems like some of these are properly positioned examples that OEV doesn't pick up?

I'm working my way through these to see if I can figure out why.

@geoffreywiseman
Copy link
Author

geoffreywiseman commented Jun 26, 2020

Most of these are an example node under schema rather than a sibling of schema. I'm not totally enthused about the way OAS allows both of those positions, to be honest, but AFAICS, it's allowed, and it looks like openapi-examples-validator won't detect an example under schema.

If I find any other missing examples, I'll call them out.

@geoffreywiseman geoffreywiseman changed the title Examples Missing? Not picking up examples under schema Jun 26, 2020
@codekie
Copy link
Owner

codekie commented Jun 27, 2020

You are right, it does not. And to be frank, this goes a bit beyond the initial purpose of this library - which is to validate "JSON response examples" against its schema - since schema can be used anywhere (not only in responses) and technically can be of any type (not only JSON).

I can and will look into this at some point (since I'd like to validate request-parameters as well), but I can't promise that this will be soon, since this is not a trivial change.

@codekie
Copy link
Owner

codekie commented Jun 27, 2020

Just in case I'm mixing something up here, can you please provide me a minimal example?

@geoffreywiseman
Copy link
Author

geoffreywiseman commented Jun 29, 2020

You can see an example in the Swagger docs: https://swagger.io/docs/specification/adding-examples/

See the example right above the words:

Note that in the code above, example is a child of schema. If schema refers to some object defined in the components section, then you should make example a child of the media type keyword:

That's an example of using an object example as a child under schema.

It's probably worth pointing out that this use looks like it's been deprecated, but only in favour of JSON Schema "examples" keyword:
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.1.0.md#schemaObject

Mostly, it's possible to have examples in an OAS spec that openapi-examples-validator doesn't even notice -- I had a bunch, not because I really really wanted them to be children of the schema node, but because either level worked ok for everything else so far, and I just hadn't worried about what level they were at until I realized that OEV was missing them altogether.

To be honest, it doesn't really bother me to move them up a level, what bothers me is that I might accidentally have broken examples in a well-formed OAS document because OEV doesn't validate them and I don't notice. Even if it had a count of schema example/examples children that it warned it wasn't going to validate, that'd make me feel better. ;)

@geoffreywiseman
Copy link
Author

Oh, and if having a really concrete example file will help, I don't mind mocking up a quick example in the Swagger editor -- not trying to dodge the work, just figured the example in the Swagger docs is both sufficient to explain and also shows that it's "authoritative" by being in a Swagger website example. ;)

@codekie
Copy link
Owner

codekie commented Jun 30, 2020

Yes, I think, the linked examples of the Swagger-docs are sufficient enough, thanks.

However, consequently OEV not only should support the examples as child of schema, but also as part of objects, properties, arrays and array-items.

As mentioned before, this change is not so trivial and probably will require extensive testing. This is why it's likely that it will take a bit until this will be implemented. However, I'll keep this issue open until then as I'd like to see this feature there as well.

As a side note to myself: Start with _getSchemaPathOfExample and also consider OAS 2, since it supports them as well.

@mknj
Copy link

mknj commented Apr 29, 2022

Any progress on this?

the following patch works fine on my examples. The given yaml contains 3 examples that were not found before.
(actually v4.6.0 finds one example and reports 0 errors, which is wrong)

example

openapi: 3.0.0
info:
  version: 1.2.3
  title: examples
  description: examples
paths:
  /demo/{id}:
    post:
      parameters:
        - name: id
          in: query
          schema:
            type: number
            example: hallo
      description: example
      requestBody:
        content:
          application/json:
            schema:
              type: object
              example: 42
      responses:
        '200':
          description: ok
          content:
            application/json:
              schema:
                type: object
                example: 42

ugly hack, that only works for me

diff --git a/src/impl/v3/index.js b/src/impl/v3/index.js
index a12392f..8ea4ba3 100644
--- a/src/impl/v3/index.js
+++ b/src/impl/v3/index.js
@@ -42,6 +42,7 @@ module.exports = {
  */
 function getJsonPathsToExamples() {
     return [
+        '$..example',
         PATH__EXAMPLE,
         PATH__EXAMPLES,
         PATH__EXAMPLE__PARAMETER,
@@ -127,7 +128,11 @@ function _getSchemaPathOfExample(pathExample) {
         idxExamples = exampleType === ExampleType.single
             ? idxExample
             : pathSegs.lastIndexOf(PROP__EXAMPLES);
-    pathSegs.splice(idxExamples, pathSegs.length - idxExamples, PROP__SCHEMA);
+    if (idxExample > -1) {
+        pathSegs.splice(idxExamples, pathSegs.length - idxExamples);
+    } else {
+        pathSegs.splice(idxExamples, pathSegs.length - idxExamples, PROP__SCHEMA);
+    }
     return {
         exampleType,
         pathSchemaAsArray: pathSegs

@codekie
Copy link
Owner

codekie commented Apr 29, 2022

Hi @mknj , thanks for your input, but I'm afraid, things are not as easy as you depict them here. The JSONPath you provided, picks up everything that is named example, even when it actually isn't an example (like a legit definition of a parameter, or a response) and is not supposed to be validated.

Please have a closer look at the OpenAPI docs, for more details: https://swagger.io/docs/specification/adding-examples/

@mknj
Copy link

mknj commented Apr 29, 2022

your are right, i should have named it hack and not patch. On the other hand i have several hundred internal openapi specs that work very reasonable with this hack.

Do we need to walk/parse the object tree so that we have more context in order to decide whether it is an example or not?

I would really love to see something upstream.

btw, did you run my example through 4.6? I wonder why it reports that it found one example and that it found no error. Should i open a separate ticket?

@codekie
Copy link
Owner

codekie commented Apr 30, 2022

I did run your example through the latest version and the reason why it found no error is because the OEV picked up the example from the parameters, but expected the schema to be on the same level (where it's not) and thus could not validate it (which is what this initially opened issue is about). That is why the output said:

Validating examples
Schemas with examples found: 0
Examples without schema found: 1
Total examples found: 1

No errors found.

I'm opposed to add hacks to this tool. And stating that you have several hundred internal OpenAPI specs that work very reasonable with it, is not going to do it, I'm afraid. Every added feature/fix needs automated tests (see test/specs-directory) to ensure that it works as intended. This includes tests that cover cases where the change applies and also make sure that it does not apply where it should't (on top of ensuring that everything else still works).

Anyhow, if I put the validation of examples as part of objects, properties, arrays and array-items out of scope for this change, things become easier and I'll look into it again.

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

No branches or pull requests

3 participants