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

fix: faker recursive reference pascal sanitation #1673

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

martinsafsten
Copy link
Contributor

@martinsafsten martinsafsten commented Oct 25, 2024

Status

READY

Description

Fixed bug when faker attempted to create mocked data, the schema had recursive references, where the name of the schema was changed when sanitized to pascal case. The bug was fixed with the change in packages\mock\src\faker\getters\scalar.ts:176, but I found two checks to existingReferencedProperties in packages\mock\src\faker\getters\object.ts and packages\mock\src\faker\getters\combine.ts, so I thought I would proactively fix them too

Todos

  • Tests
  • Documentation
  • Changelog Entry (unreleased)

Steps to Test or Reproduce

Here is a simplified, obfuscated version of the json that caused the issue. The sanitation changed Foo.Bar.MainMenuItemDTO to FooBarMainMenuItemDTO, causing it to miss the check in existingReferencedProperties

I ran this json in the react query sample to confirm the bug, and the fix

{
  "swagger": "2.0",
  "info": {
    "version": "v1",
    "title": "Foo Bar Api"
  },
  "host": "localhost:443",
  "basePath": "/",
  "schemes": ["https"],
  "paths": {
    "/api/foo/bar": {
      "get": {
        "tags": ["Foo"],
        "operationId": "Foo_Bar",
        "consumes": [],
        "produces": ["application/json"],
        "parameters": [],
        "responses": {
          "200": {
            "description": "OK",
            "schema": {
              "type": "array",
              "items": {
                "$ref": "#/definitions/Foo.Bar.MainMenuItemDTO"
              }
            }
          }
        },
        "deprecated": false
      }
    }
  },
  "definitions": {
    "Foo.Bar.MainMenuItemDTO": {
      "type": "object",
      "properties": {
        "id": {
          "type": "string"
        },
        "children": {
          "type": "array",
          "items": {
            "$ref": "#/definitions/Foo.Bar.MainMenuItemDTO"
          }
        }
      }
    }
  }
}

@martinsafsten
Copy link
Contributor Author

Hmm, I saw that the yarn.lock file was updated and bumped a few version locally when I did yarn install. I discarded that. Not sure why it did, since I did not change any packages.

@melloware melloware added the mock Related to mock generation label Oct 25, 2024
@melloware melloware added this to the 7.2.1 milestone Oct 25, 2024
@melloware melloware requested a review from anymaniax October 25, 2024 13:31
@melloware
Copy link
Collaborator

safe to rebase!

@melloware melloware merged commit 9a54567 into orval-labs:master Oct 25, 2024
2 checks passed
@soartec-lab soartec-lab added the bug Something isn't working label Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working mock Related to mock generation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants