stoplightio / prism

Turn any OpenAPI2/3 and Postman Collection file into an API server with mocking, transformations and validations.
https://stoplight.io/open-source/prism
Apache License 2.0
4.12k stars 338 forks source link

Circular $ref pointer not supported #1456

Closed marcoboi closed 3 years ago

marcoboi commented 3 years ago

Describe the bug

A clear and concise description of what the bug is. Pointing prism to a file that contains components that use recursive field definition, Prism fails with the message:

Circular $ref pointer found at /tmp/test.yaml#/components/schemas/Person/properties/spouse

To Reproduce

A minimal API definition triggering the error message:

info:
  title: "Circular reference example"
  description: "Exemplifies a schema with circular references"
  version: 0.1.0

paths:
  /directory:
    get:
      responses:
        "200":
          description: "The request was successfully processed"
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Person"
              example:
                name: John
                spouse:
                  name: Cynthia

components:

  schemas:

    Person:
      nullable: true
      properties:
        name:
          type: string
          nullable: false
        spouse:
          $ref: "#/components/schemas/Person"

Expected behavior

As the OAS file specifies a valid API, the server should be able to mock that API. While having a circular reference can theoretically lead to a infinite recursion, this can be in practice avoided by having a nullable reference to the child object.

Environment

akmoulai commented 3 years ago

I just experienced the same problem trying Prism on a real-life contract running in production. I've found https://github.com/stoplightio/prism/issues/891 and https://github.com/stoplightio/prism/pull/1271 for more context. Can't really remove the circular reference as it's how the data is :-/ and really want to give Prism a go!

XVincentX commented 3 years ago

Hey people,

Essentially the reason why we decided to disable the circular reference support is because no matter how much we can support them, we will always ultimately fail to serialise a circular response body and we haven't been able to come up with a proper story to handle such. Consider this example:

    Person:
      properties:
        name:
          type: string
        spouse:
          $ref: "#/components/schemas/Person"

There is no way a data structure like this can go over the wire. Both the serialisation (input) and deserialisation (output) is not possible.

While the serialisation is not a problem from a Prism perspective, the deserialisation is (as in — Prism cannot produce such output).

We also have some other edge corner cases we did not have the time to investigate. It seems to be kinda important for you, so we'll see if I can find some time to look into this and…relax this hard constraint a bit.

Cheers!

akmoulai commented 3 years ago

@XVincentX thanks for your reply :)

If I can add: in the provided example, in practice would become Person (A) -> Spouse -> Person (B) -> Spouse -> Person (A) (can be a copy object, not memory reference to the previously defined one) - maybe there could be an option to resolve an arbitrary number of recursive levels and then give up (with the difference that this number could be overridden in the config)?

After all, the contract doesn't specify how deep the circular reference needs to be (therefore one level would always be enough to match the contract?).

XVincentX commented 3 years ago

If I can add: in the provided example, in practice would become Person (A) -> Spouse -> Person (B) -> Spouse -> Person (A)

That is anecdotical. spouse was just a name I come up with, it might not be the same and the chain could be infinite (impossibility to serialise the payload)

there could be an option to resolve an arbitrary number of recursive levels and then give up

That would be a tradeoff, although this would be essentially like forcing a nullable: true after a certain depth; that will make the output validator fail, making Prism scream at you (and you regretting your life choices)

After all, the contract doesn't specify how deep the circular reference needs to be

I think that is inaccurate. The example I wrote clearly specifies an infinite circular chain — I do not see any doubt about it.

marcoboi commented 3 years ago

@XVincentX , thanks for replying so promptly. While I understand why it is not possible to serialize A -> B -> A, I do not get what the problem would be in serializing A -> B where both A and B are instances of the same type. What I am interested here is simply having A -> B as it happens in a parent-child relationship. In my previous example, such a relationship would serialize as: {"name":"John","spouse":{"name":"Cynthia","spouse":null}}

Again, the example I chose might be poor in that it indeed also allows B to link back to A, which was not the point I wanted to make here.

blemasle commented 3 years ago

Hi,

I just faced the same issue today. I can understand that dynamically mocking and serializing a circular reference causes an issue. In my case though, the open api file provides a response example and prism would never have to do such a thing.

Nonetheless, just describing the circular reference in the OpenApi specification file prevent prism from booting :/ Is there any way to work around this ?

XVincentX commented 3 years ago

Not at the moment; however I can see there's a lot of pressure here so I'll probably circle back on this and see if we can improve the situation somehow

linkdd commented 3 years ago

FYI, the Kubernetes OpenAPI spec contains circular references.

Mocking the API server with Prism would help testing operators and other tools interacting with the Kubernetes API Server.

Looking forward to it!

philsturgeon commented 3 years ago

@linkdd could you narrow it down a bit? Haha can't go digging through all that! :)

@blemasle I don't understand this JSON:

{"name":"John","spouse":{"name":"Cynthia","spouse":null}}

This looks like John is married to Cynthia but Cynthia is not married to John.

Poor John.

This doesn't make any sense fundamentally so it's going to lead to confusing conversations trying to use it as an example.

If you wanted a null in there, then sure your OpenAPI should be this:

    Person:
      properties:
        name:
          type: string
        spouse:
          oneOf: 
            - $ref: "#/components/schemas/Person"
            - nullable: true

Supporting this in Prism would just be a case of picking oneOf's at random, which I remember discussing in the past. Some people want oneOfs to always show the first, which is where you'd get into trouble, because again the chain would never be broken by a null.

Possibly a more logical example would be parent.

Person:
  properties:
    name:
      type: string
    manager:
      oneOf: 
        - $ref: "#/components/schemas/Person"
        - nullable: true

This is another chain which is not infinite but could be. It makes me wonder why people design APIs like this because clients should be able to follow a link if they want that information instead of having recursion thrust upon them.

{
  "name":"John",
  "manager": {
    "name":"Cynthia",
    "manger":{
      "name":"Frank",
      "manger":{
        "name":"Mary",
        "manger":{
          "name":"George",
          "manger":{
            "name":"Big BOSS CEO",
            "manger":null
          }
        }
      }
    }
  }
}

If you're using Prism you're probably designing an API right now, and if you're doing that its not too late to change the design. Use any Hypermedia Format and insert links, which might look as simple as this:

{
  "name":"John",
  "links": {
    "manager":"https://api.example.com/people/cynthia"
  }
}

Prism is happy. Your API is happy. Your clients are happy.

If anyone has any other examples of circular references which should exist, lmk, and in the men time @XVincentX can we look into that oneOf shortcut with null? We can't do anything about any of these other situations so far but we can t least handle that.

linkdd commented 3 years ago

@philsturgeon Here is the error I get:

$ prism mock https://raw.githubusercontent.com/kubernetes/kubernetes/master/api/openapi-spec/swagger.json
[18:15:53] » [CLI] ...  awaiting  Starting Prism…
[18:15:56] » [CLI] ×  fatal     Circular $ref pointer found at https://raw.githubusercontent.com/kubernetes/kubernetes/master/api/openapi-spec/swagger.json#/definitions/io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.JSONSchemaProps/properties    
prism mock <document>

Start a mock server with the given document file

Arguments positionnels :
  document  Path to a document file. Can be both a file or a fetchable resource on the web.      [chaîne de caractères] [requis]
Options :
      --version       Affiche le numéro de version                                                                     [booléen]      --help          Affiche l'aide                                                                                   [booléen]  -p, --port          Port that Prism will run on.                                             [nombre] [requis] [défaut : 4010]  -h, --host          Host that Prism will listen to.                     [chaîne de caractères] [requis] [défaut : "127.0.0.1"]      --cors          Enables CORS headers.                                                            [booléen] [défaut : true]  -m, --multiprocess  Forks the http server from the CLI for faster log processing.                   [booléen] [défaut : false]      --errors        Specifies whether request/response violations marked as errors will produce an error response
                                                                                             [booléen] [requis] [défaut : false]  -d, --dynamic       Dynamically generate examples.                                                  [booléen] [défaut : false]
{
  stack: 'ReferenceError: Circular $ref pointer found at https://raw.githubusercontent.com/kubernetes/kubernetes/master/api/openapi-spec/swagger.json#/definitions/io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.JSONSchemaProps/properties\n' +     
    '    at foundCircularReference (C:\\Users\\david\\AppData\\Local\\Yarn\\Data\\global\\node_modules\\@apidevtools\\json-schema-ref-parser\\lib\\dereference.js:159:15)\n' +
    '    at crawl (C:\\Users\\david\\AppData\\Local\\Yarn\\Data\\global\\node_modules\\@apidevtools\\json-schema-ref-parser\\lib\\dereference.js:75:24)\n' +
    '    at dereference$Ref (C:\\Users\\david\\AppData\\Local\\Yarn\\Data\\global\\node_modules\\@apidevtools\\json-schema-ref-parser\\lib\\dereference.js:125:24)\n' +
    '    at crawl (C:\\Users\\david\\AppData\\Local\\Yarn\\Data\\global\\node_modules\\@apidevtools\\json-schema-ref-parser\\lib\\dereference.js:58:26)\n' +
    '    at crawl (C:\\Users\\david\\AppData\\Local\\Yarn\\Data\\global\\node_modules\\@apidevtools\\json-schema-ref-parser\\lib\\dereference.js:67:28)\n' +
    '    at crawl (C:\\Users\\david\\AppData\\Local\\Yarn\\Data\\global\\node_modules\\@apidevtools\\json-schema-ref-parser\\lib\\dereference.js:67:28)\n' +
    '    at dereference$Ref (C:\\Users\\david\\AppData\\Local\\Yarn\\Data\\global\\node_modules\\@apidevtools\\json-schema-ref-parser\\lib\\dereference.js:125:24)\n' +
    '    at crawl (C:\\Users\\david\\AppData\\Local\\Yarn\\Data\\global\\node_modules\\@apidevtools\\json-schema-ref-parser\\lib\\dereference.js:58:26)\n' +
    '    at crawl (C:\\Users\\david\\AppData\\Local\\Yarn\\Data\\global\\node_modules\\@apidevtools\\json-schema-ref-parser\\lib\\dereference.js:67:28)',
  message: 'Circular $ref pointer found at https://raw.githubusercontent.com/kubernetes/kubernetes/master/api/openapi-spec/swagger.json#/definitions/io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.JSONSchemaProps/properties',
  toJSON: [Function: toJSON],
  name: 'ReferenceError',
  toString: [Function: toString]
}

The culprit is at this path #/definitions/io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.JSONSchemaProps/properties

I invite you to run this command to have the full schema:

$ wget https://raw.githubusercontent.com/kubernetes/kubernetes/master/api/openapi-spec/swagger.json
$ cat swagger.json | jq '.definitions["io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.JSONSchemaProps"].properties'

This definition is used within the CustomResourceDefinition resource schema, which allows the developer to provide an OpenAPI schema of their custom resource for validation by the Kubernetes API Server. Hence the circular reference.

I bet it's very difficult to generate an OpenAPI schema from nothing in order to mock this endpoint 😆

A solution would be to provide the user some option on what to do when detecting a circular reference:

Let me know if I can be of any help on that subject.

blemasle commented 3 years ago

@philsturgeon Wrong mention here. I'm not the author of the spouse example :)

philsturgeon commented 3 years ago

Ok, here's something we could try.

Other than nullable (or type: null in v3.1 🥳) there's another way we can conceptually forceably short-circuit an infinite loop like this.

If a property is not in the required: [manager] array, we can conceptually just not show it.

How to decide if we show it or not? Well, we can probably pattern match by looking at the ancestors. A B A ok fine. A B A B probably bad. A B A B A STOP IT.

The trouble here is when you get to a few levels of this sort of nonsense. A B C A B C that's already quite big and fits inside the "dont repeat yourself a third time". A B C A B C A B C would trigger the short circuit but would probably blow a fuse already depending on big those resources are.

Anyway, perhaps we can take a swing at something along these lines, by shimming something up around json-schema-ref-resolver to see if we can add this tracking, and if it works maybe we can bake it into the tool as a "hook to fire when you hit a circular loop" type exception thrower. Whatever, that's for later.

If we can find a way to get the code working, it would theoretically work for "belongs to" (manager > manager > manager), and can work for nested has one nested has one.... business > website > business > website...

If somebody has put it in required, then yeah, sure, they get the error they have right now, because aint no way we can show an infinity recursion in JSON.

But seriously please folks link to other resources instead of embedding them. HTTP/2 is great. https://apisyouwonthate.com/blog/lets-stop-building-apis-around-a-network-hack

dargiri commented 3 years ago

3 cents from me.

Maybe if you have circular references and it's hard(or maybe even impossible) to generate response but you have an example - then stick with it - otherwise - fail?

Sample schema:

swagger: "2.0"
info:
  description: "Test"
  version: "1.0.0"
  title: "Swagger Petstore"
  termsOfService: "http://swagger.io/terms/"
  contact:
    email: "apiteam@swagger.io"
  license:
    name: "Apache 2.0"
    url: "http://www.apache.org/licenses/LICENSE-2.0.html"
host: "petstore.swagger.io"
basePath: "/v2"
tags:
- name: "users"
  description: "Everything about your Users"
  externalDocs:
    description: "Find out more"
    url: "http://swagger.io"
schemes:
- "http"
paths:
  /users/me:
    get:
      tags:
      - "users"
      summary: "get any pet"
      description: "no description for this endpoint"
      operationId: "get"
      produces:
      - "application/json"
      responses:
        "200":
          description: "successful operation"
          schema:
            type: "array"
            items:
              $ref: "#/definitions/User"
        "400":
          description: "Invalid status value"
definitions:
  User:
    type: "object"
    required:
    - "id"
    - "name"
    - "friends"
    properties:
      id:
        type: "integer"
        format: "int64"
        example: 1
      name:
        type: "string"
        example: "Lincoln"
      friends:
        type: "array"
        items:
          $ref: "#/definitions/User"
        example: [{
          id: 2,
          name: "John",
          friends: []
        }]

Generated example by swagger editor: image

Actual frustrating picture that I get with current version of prism:

image

P.S. there's a difference between living in an ideal world(building ideal API) vs adopting. P.P.S. actually prism 3.x was doing this trick with examples. I don't know why this functionality was removed :(

BR, Dionis

philsturgeon commented 3 years ago

Yep! Thanks Dionis. @XVincentX will get on top of this soon, we have an approach that can help solve some more of these circular references, remember we’ve fixed several but not all circular references are equal.

XVincentX commented 3 years ago

Hey @dargiri,

I've explained why the functionality was disabled in https://github.com/stoplightio/prism/issues/1456#issuecomment-702176386

TL;DR: it's not easy to know where we can handle circular references successfully and where not.

Rather than trying to do something according to common sense, ship something for the sake of and then get tons of issues with edge cases or one scenario making another one not work correctly — I decided it was a better idea to admit we do not have a story flashed out yet and, as long we don't have it, I am not going to let it happen.

If this takes the priority, more than happy to work seriously on this.

bdotzour commented 3 years ago

I'd also like to voice my support for finding some kind of solution. I can appreciate that it's really challenging to solve all the edge cases but, frankly, your stance on this issue seems pretty hard-lined. I myself have a terribly simple use case which is just modeling a nested structure of objects, which is never going to be circular in practice but I am prevented from even starting the mock server. Being a long-time Java user I'm perfectly used to seeing the StackOverflowException when I'm doing something silly and trying to serialize a true circular reference. I would love to see prism do the same thing: let the users shoot themselves in the foot if that's what they're determined to do.

Love the tool everyone, best regards!

In case it's relevant, here is the trimmed down example of the spec file I'm working with:

openapi: 3.0.1
info:
  title: Example Case
  description: Nested $refs to same schema
  version: 1.0.0
servers:
  - url: 'http://localhost:8080'

paths:
  /report:
    get:
      operationId: getReportContent
      summary: 'Report Content'
      responses:
        200:
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ReportContent'

components:
  schemas:
    ReportContent:
      description: 'The detailed content of a given report'
      type: object
      properties:
        title:
          description: 'The title of the report'
          type: string
        sections:
          description: 'The sections that contain the content for the body of the report.'
          type: array
          items:
            $ref: '#/components/schemas/ReportSection'

    ReportSection:
      description: 'A logical grouping of content within the report'
      type: object
      properties:
        title:
          description: 'The title of the section'
          type: string
        content:
          description: 'The textual content of the section'
          type: string
        childSections:
          description: 'Sections can be nested inside one another for building a hierarchical report structure'
          type: array
          items:
            $ref: '#/components/schemas/ReportSection'
      required:
        - title
        - content
philsturgeon commented 3 years ago

I’ve suggested some approaches we could try to solve a few more scenarios involving circular references, and we’ll get to them when we can. In the mean time if anyone would like to send a pull request that would be very welcome. Happy holidays folks!

marcoboi commented 3 years ago

Hi @philsturgeon Just wanted to reiterate one more time what I already mentioned here: https://github.com/stoplightio/prism/issues/1456#issuecomment-704009063

My spouse example is not the best example for this issue, as it semantically entails a circular dependency (A -> B -> A). Yet in the real world there are many instances where parent and children will have the same type but will not be referencing each other. You pointed out the manager example, other people in this thread suggested more examples along the same line.

What we're pointing out in this thread is that it should be possible for Prism to generate data for a schema where a parent object references an object of the same type. That is a valid schema, therefore one would expect you can generate data for it.

If you think the spouse example creates confusion, I'm happy to start a new ticket with an example you find more suitable.

philsturgeon commented 3 years ago

Thanks @marcoboi. As I said I think we have some examples that work and we can take a swing as solving a few more types of circular reference in the new year.

andrewsomething commented 3 years ago

Reading over the related issues, most of the conversation seems to be focused on the difficulties of serializing responses for the mock server. I'm curious about the status of circular refs in the validation proxy. Any pointers?

XVincentX commented 3 years ago

@andrewsomething We do not have any update on this unfortunately. We're working on other stuff and I am not in a position of giving you an ETA on when we'll tame this.

philsturgeon commented 3 years ago

@XVincentX I don't think @andrewsomething was chasing for an update, just asking for insight into how circular references work in the validation proxy.

@andrewsomething Hello! Validation and response generation are two very different concepts. Trying to create JSON based on schemas which have self-referencing relationships and knowing when to stop trying to create more JSON in that infinite loop is the main problem discussed in this thread. Looking at an instance of JSON and figuring out if it's valid against the schema is waaaaaaay easier.

I'd recommend giving it a try and see if you have any problems. If you do, let us know. If not, enjoy!

andrewsomething commented 3 years ago

As the docs read:

Even though Prism is technically able to internally handle circular references, the CLI will refuse to mock the provided document in case any circular reference is detected.

I had naively hoped that something like this is all it would take to use them with the proxy server: https://github.com/stoplightio/prism/compare/master...andrewsomething:enable-circular-proxy While Prism doesn't fall over proxying an API w/ a circular ref, the schema for the operation does not seem to make it through the transformation to HTTP Spec. So requests can be made and response are passed through, they are not actually validated for operations w/ a circular ref.

reidmv commented 3 years ago

The Swagger parser's bundle method is alleged to produce a Swagger object and also "eliminate the risk of circular references".

  1. Is that good enough for prism?
  2. Does dereference do something bundle does not, which prism depends on?
diff --git a/packages/cli/src/operations.ts b/packages/cli/src/operations.ts
index 8bb49ad..1199fe5 100644
--- a/packages/cli/src/operations.ts
+++ b/packages/cli/src/operations.ts
@@ -9,7 +9,7 @@ import type { OpenAPIObject } from 'openapi3-ts';
 import type { CollectionDefinition } from 'postman-collection';

 export async function getHttpOperationsFromSpec(specFilePathOrObject: string | object): Promise<IHttpOperation[]> {
-  const result = await dereference(specFilePathOrObject, { dereference: { circular: false } });
+  const result = await bundle(specFilePathOrObject, { dereference: { circular: false } });

   if (isOpenAPI2(result)) return transformOas2Operations(result);
   if (isOpenAPI3(result)) return transformOas3Operations(result);
garethj-msft commented 3 years ago

Another $0.02 to ask for prioritizing support for stopping at non-required recursive fields or null.

We're building OData APIs where this is incredibly commonplace, because the schema is full of non-required fields that allow you to explicitly use a $expand query parameter to add a navigation across a link relationship (which doesn't implicitly cause the reciprocal link to get $expanded, so no recursion in the response body in practice).

However the schema needs to include all the possible combinations that could be $expanded, thus frequently producing recursion.

I'm blocked from any chance of adopting Prism without this support.

philsturgeon commented 3 years ago

Hey @garethj-msft, could you help us out with some repro cases for OData $expand? I don't think any of us have extensive experience with OData, but if you can clearly show off a real world problem we can use it as a test case and make sure Prism can handle it.

pytlesk4 commented 3 years ago

Hey Everyone,

Finally got some time to work on circular ref support in prism, if you update prism to the latest, circular refs should be supported. Please create a new issue if something is not working as expected!