OpenApiSchema.SerializeAsV2WithoutReference does not recursively serialize references

[nth-commit]

Version 1.1.4

Given the document:

{
  "swagger": "2.0",
  "paths": {
    "/foos": {
      "get": {
        "produces": [
          "application/json"
        ],
        "responses": {
          "200": {
            "schema": {
              "$ref": "#/definitions/Foo"
            }
          }
        }
      }
    }
  },
  "definitions": {
    "Foo": {
      "type": "object",
      "properties": {
        "bar": {
          "$ref": "#/definitions/Bar"
        }
      }
    },
    "Bar": {
      "type": "object",
      "properties": {
        "baz": {}
      }
    }
  }
}

When serializing the schema referenced by the path "/foos"

Expected:

{
  "type": "object",
  "properties": {
    "bar": {
      "type": "object",
      "properties": {
        "baz": {}
      }
    }
  }
}

Actual:

{
  "type": "object",
  "properties": {
    "bar": {
      "$ref": "#/definitions/Bar"
    }
  }
}

[nth-commit]

I was trying to imagine a use-case where someone would call SerializeAsV2WithoutReference and not want nested references to be denormalized, and I couldn't. However, in the code, there is a situation where this would happen.

Given the JSON schema:

{
    "type": "object",
    "properties": {
        "foo": {
            "$ref": "#/$defs/foo"
        }
    }
}

When SerializeAsV2 is called on the root, the code will see that there's no reference, and then go into SerializeAsV2WithoutReference, which will go and write the properties of that schema. As the caller, you would expect the JSON schema to be something equivalent to the above, but given the naive fix, this would break (and denormalize where not intended). What I mean by "the naive fix": nth-commit@b95d871

It would seem it's necessary for SerializeAsV2 to not call SerializeAsV2WithoutReference, so the caller's intention is not lost. Might need a private version of WriteAsSchemaProperties that takes an Action to handle writing nested schemas.

I'm happy to have a go at making this change.

[DerekChasse]

I'm actually looking at accomplishing exactly this, so thank you for starting the discussion.

Is there a solution which would account for circular references?

{
  "swagger": "2.0",
  "paths": {
    "/foos": {
      "get": {
        "produces": [
          "application/json"
        ],
        "responses": {
          "200": {
            "schema": {
              "$ref": "#/definitions/Foo"
            }
          }
        }
      }
    }
  },
  "definitions": {
    "foo": {
      "type": "object",
      "properties": {
        "bar": {
          "$ref": "#/definitions/bar"
        }
      }
    },
    "bar": {
      "type": "object",
      "properties": {
        "gaz": {
          "$ref": "#/definitions/gaz"
        }
      }
    },
    "gaz": {
      "type": "object",
      "properties": {
        "foo": {
          "$ref": "#/definitions/foo"
        }
      }
    }
  }
}

[nth-commit]

@DerekChasse ahhhhhhh good point. I think my change in #430 would stack overflow on circular references. I think that is what the applicator $recusiveRef is for? I'm not a JSON schema expert, however.

So given this Open API schema (simplified slightly from mine and your examples above):

{
  "swagger": "2.0",
  "paths": {
    "/foos": {
      "get": {
        "produces": [
          "application/json"
        ],
        "responses": {
          "200": {
            "schema": {
              "$ref": "#/definitions/Foo"
            }
          }
        }
      }
    }
  },
  "definitions": {
    "Foo": {
      "type": "object",
      "properties": {
        "bar": {
          "$ref": "#/definitions/Bar"
        }
      }
    },
    "Bar": {
      "type": "object",
      "properties": {
        "foo": {
          "$ref": "#/definitions/Foo"
        }
      }
    }
  }
}

Then serializing the JSON schema at the solo path should output:

{
  "type": "object",
  "properties": {
    "bar": {
      "type": "object",
      "properties": {
        "foo": {
          "$recursiveRef": "#"
        }
      }
    }
  }
}

[nth-commit]

FYI @DerekChasse, I updated my PR to at least throw when a circular reference is detected. Will wait to hear from the maintainers before I implement anything fancy to do with denormalizing circular references.

[DerekChasse]

@darrelmiller Could you provide some insight as to how IOpenApiReferenceable is intended to behave here? Is it's intent to do what @nth-commit is attempting to solve here?

[darrelmiller]

Hey folks! Sorry I don't have the time right now that this issue deserves but I think I might be able to shed some light.

"SerializeAsV2WithoutReference" is not supposed to do what it sounds like you want it to do. You want to inline all references. Actually, so do I as we need that to support PowerPlatform. However, my plan is to implement that with a new Visitor.

If my memory is correct, the SerializeXXXWithoutReference method is specifically to ensure that when serializing objects in the components section that we don't just write out a reference for the top level object. It was not intended to be a "deep resolve reference". This is necessary because the object referenced in the components collection is actually the same instance that is used in the main document. (At least it is after calling ResolveReferences). The same object needs to be serialized two different ways depending on where it is in the OpenAPIDocument.

I'm not sure how easy it is going to be to update the writer to also support serializing full references. There are a bunch of edge cases that are quite hard to work around.

My plan was to just clone the referenced objects and remove the references completely and then serialize out that. Dealing with recursion is a bit of a pain, but I think we have code in the ResolveReferences visitor that does that.

The "$recursiveRef" keyword is not currently supported in OpenAPI V3.

The IOpenApiReferencable is just a utility interface to make it easy to write generic code with things that could be referenced.

[DerekChasse]

Awesome, thanks for the clarification, and shining the light on how difficult this could end up being. It's not hard to imagine how it could easily become a very slippery slope, especially with external references.

That being said, I think that there is still value in pursuing a solution to this, perhaps without re-inventing how SerializeAsXXXWithoutReference works.

Thanks,
~Derek

[darrelmiller]

@DerekChasse Yeah, external references would start to get a bit wild. FYI, there is a branch here called OpenApiWorkspace that I'm hoping one day will support multiple OpenAPI documents and fragments that can all be referenced.
I think either me or one of the devs on our team is going to try and implement an inlining visitor sometime before the end of the year. If you wanted to give it a shot before that, I'd be happy to help out when I can.

[nth-commit]

Hey @darrelmiller, thanks for the thought out response.

I'm not sure how easy it is going to be to update the writer to also support serializing full references. There are a bunch of edge cases that are quite hard to work around.

My fairly uneducated implementation in #430 simply writes out what has already been resolved in the component tree. This is possible, because during deserialization, not only are the particular objects created with a reference field set, the schema properties from the reference are actually copied over. Because of the way that it currently works, I figured resolving references was not a concern of the serialization code.

The "$recursiveRef" keyword is not currently supported in OpenAPI V3.

Aight. { "$ref": "#" } seems to work when I feed it back into Newtonsoft.Json.Schema and do some validations against it.