Skip to content

allOf required property not composing properly #3328

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

Closed
webron opened this issue Jul 3, 2017 · 9 comments
Closed

allOf required property not composing properly #3328

webron opened this issue Jul 3, 2017 · 9 comments

Comments

@webron
Copy link
Contributor

webron commented Jul 3, 2017

From @baynezy on July 3, 2017 13:46

If you take the following swagger definition:-

{
    "swagger": "2.0",
    "info": {
        "title": "An API Definition",
        "version": "0.1"
    },
    "paths": {
        "/users": {
            "post": {
                "operationId": "getUser",
                "produces": [
                    "application/json"
                ],
                "parameters": [{
                    "name": "newUser",
                    "in": "body",
                    "required": true,
                    "schema": {
                        "$ref": "#/definitions/newUser"
                    }
                }],
                "responses": {
                    "201": {
                        "description": "Details of the user"
                    }
                }
            }
        }
    },
    "definitions": {
        "user": {
            "type": "object",
            "required": ["username", "firstName", "lastName"],
            "properties": {
                "username": {
                    "type": "string"
                },
                "firstName": {
                    "type": "string"
                },
                "lastName": {
                    "type": "string"
                }
            }
        },
        "newUser": {
            "allOf": [{
                    "$ref": "#/definitions/user"
                },
                {
                    "type": "object",
                    "required": ["email", "roles"],
                    "properties": {
                        "email": {
                            "type": "string"
                        },
                        "roles": {
                            "type": "array",
                            "items": {
                                "type": "string"
                            }
                        }
                    }
                }
            ]
        }
    }
}

Then newUser is a composite of user and the additional properties defined on newUser. In the Swagger Editor the UI shows this correctly.

image

However, it should also compose the required property which it is failing to do. user states that username, firstName, and lastName are required, and newUser states that email, and roles are required. However, it is only marking those on newUser as required.

image

This should be a super-set of these.

6.26. allOf

This keyword's value MUST be a non-empty array. Each item of the array MUST be a valid JSON Schema.

An instance validates successfully against this keyword if it validates successfully against all schemas defined by this keyword's value.

Copied from original issue: swagger-api/swagger-editor#1390

@owenconti
Copy link
Contributor

I have a fix for this issue in the swagger-js repository. You can track the PR here: swagger-api/swagger-js#1101

@baynezy
Copy link

baynezy commented Jul 11, 2017

@webron and @owenconti - thanks for the quick response. How often do you guys release to production?

@webron
Copy link
Contributor Author

webron commented Jul 11, 2017

We try to every Friday, meaning the fix is already included in the release. However, I'm still seeing the issue. @owenconti?

@owenconti
Copy link
Contributor

@webron The PR in the swagger-js repo was merged just last night. It should be in the upcoming release then, shouldn't it?

@webron
Copy link
Contributor Author

webron commented Jul 11, 2017

Oh! I thought it was merged last week, my bad. @baynezy expect the fix to be live this upcoming Friday.

@baynezy
Copy link

baynezy commented Jul 13, 2017

Perfect thanks.

@webron
Copy link
Contributor Author

webron commented Jul 17, 2017

@baynezy mind testing with the latest version?

@baynezy
Copy link

baynezy commented Jul 18, 2017

@webron - that works as I had hoped. Thank you very much.

@webron
Copy link
Contributor Author

webron commented Jul 19, 2017

Thanks for verifying! Closing.

@webron webron closed this as completed Jul 19, 2017
@lock lock bot locked and limited conversation to collaborators Jul 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants