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

fixes a bug where a null schema would make the generation process derail #185

Merged
merged 3 commits into from
May 31, 2021

Conversation

baywet
Copy link
Member

@baywet baywet commented May 27, 2021

partial #135

@baywet baywet added this to the Alpha milestone May 27, 2021
@baywet baywet self-assigned this May 27, 2021
@baywet baywet enabled auto-merge May 27, 2021 14:25
@sonarcloud
Copy link

sonarcloud bot commented May 27, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@PureKrome
Copy link
Contributor

Thanks @baywet - as in the screen shot above, I gave it a quick try but couldn't stay awake 💤 So you beat me to it :)

@PureKrome
Copy link
Contributor

PureKrome commented May 29, 2021

@baywet - i was just playing with this PR on localhost and the "petstore" still fails with an exception.

> System.InvalidOperationException: adding an item namespace at the root is not supported

image

image

I'm assuming that Kiota should work against the "petstore" json file?

(looks like this section is failing?? )

image

EDIT:

this was the command I used, which can be used to repo this:

launchSettings.json

{
  "profiles": {
    "kiota": {
      "commandName": "Project",
      "commandLineArgs": "--openapi https://petstore.swagger.io/v2/swagger.json -o OpenApiClient\\Generated -c PetShopGraphClient --loglevel Information"
    }
  }
}

@darrelmiller
Copy link
Member

Interestingly, when I try processing Petstore I'm getting a different issue:

            else throw new InvalidOperationException("un handled case, might be object type or array type");
            // object type array of object are technically already handled in properties but if we have a root with those we might be missing some cases here

this is caused by

    Pets:
      type: array
      items:
        $ref: "#/components/schemas/Pet"

We haven't run into this case with Graph OpenAPI descriptions because OData never allows an array at the root of a response.

@PureKrome
Copy link
Contributor

Should it be handled? Should the "petshop" be a valid sample that should generate?

@darrelmiller
Copy link
Member

@PureKrome Sorry if that wasn't clear. Absolutely it should be handled. We should handle any valid OpenAPI doc.

@baywet baywet merged commit b0ffef8 into main May 31, 2021
@baywet baywet deleted the bugfix/null-ref-exp-schema branch May 31, 2021 13:06
Copy link
Contributor

@zengin zengin left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants