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

Recursive sample generation #1561

Merged
merged 3 commits into from May 3, 2023

Conversation

leflings
Copy link
Contributor

@leflings leflings commented Oct 2, 2022

Fixes #1560

This PR modifies SampleJsonDataGenerator to support re-use of definitions, and by extension, recursion.

See issue for the problem before.

Example 1

Now the following schema:

{
    "definitions": {
        "withNumber": {
            "type": "object",
            "required": ["value"],
            "properties": {
                "value": {
                    "type": "number"
                }
            }
        }
    },
    "type": "object",
    "required": ["number1", "number2"],
    "properties": {
        "number1": { "$ref": "#/definitions/withNumber" },
        "number2": { "$ref": "#/definitions/withNumber" }
    }
}

generates

{
  "number1": {
    "value": 0.0
  },
  "number2": {
    "value": 0.0
  }
}

Example 2

Schema:

{
    "definitions": {
        "data": {
            "type": "object",
            "required": ["body"],
            "properties": {
                "body": { "$ref": "#/definitions/data" }
            }
        }
    },
    "type": "object",
    "required": ["data"],
    "properties": {
        "data": { "$ref": "#/definitions/data" },
    }
}

generates:

{
  "data": {
    "body": {
      "body": {
        "body": null
      }
    }
  }
}

The level of recursion is controlled by a new setting on SampleJsonDataGeneratorSettings

return null;
}
schemaStack.Push(schema);
if (schemaStack.Count(s => s == schema) > _settings.MaxRecursionLevel)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this kill performance? capturing lambda and used inside recursion, having O(n) characteristics?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be very surprised if people use this functionality in performance sensitive scenarios. It's easy to rewrite this without the use of a capturing lambda if you deem this to be a big performance issue

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah true, this is only for sample data indeed. Anything called by NSwag in normal generation process usually is quite performance sensitive.

@RicoSuter RicoSuter merged commit f1d244b into RicoSuter:master May 3, 2023
shuruev added a commit to servicetitan/NJsonSchema that referenced this pull request Nov 16, 2023
* Update README.md

* Handle single quote in properties names (RicoSuter#1574)

* added abstract schema checking to CSharpValueGenerator GetDefaultValue (RicoSuter#1570)

Co-authored-by: Gergo Vandor <gergo.vandor.dev@outlook.com>

* Recursive sample generation (RicoSuter#1561)

* Add tests that show generator doesnt handle definition re-use/recursion

* Allow definition re-use/recursion in SampleJsonDataGenerator

Adds a MaxRecursionLevel to SampleJsonDataGeneratorSettings

* Add test of recursion level

* Fix race condition in GetName (RicoSuter#1571)

* Update Namotion.Reflection

* v11.1.0

* Revert "v11.1.0"

This reverts commit 1015b01.

* Post-merge adjustments

---------

Co-authored-by: Rico Suter <mail@rsuter.com>
Co-authored-by: kal <35899782+kalilistic@users.noreply.github.com>
Co-authored-by: Gergo Vandor <vandor.gergoo@gmail.com>
Co-authored-by: Gergo Vandor <gergo.vandor.dev@outlook.com>
Co-authored-by: Flemming Madsen <mail@leflings.dk>
Co-authored-by: SeongChan Lee <foriequal@gmail.com>
Co-authored-by: Rico Suter <rico.suter@buhlergroup.com>
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

Successfully merging this pull request may close these issues.

Sample generation fails to generate values when re-using definitions in schema
3 participants