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

Adding an optional field to request body shouldn't be a breaking change #2962

Closed
schafle opened this issue Aug 17, 2023 · 1 comment · Fixed by #3011
Closed

Adding an optional field to request body shouldn't be a breaking change #2962

schafle opened this issue Aug 17, 2023 · 1 comment · Fixed by #3011
Labels
bug diff Relates to the swagger diff command pending PR

Comments

@schafle
Copy link

schafle commented Aug 17, 2023

Problem Statement

Adding an optional field to request body shouldn't be a breaking change but it seems the breaking tool identify it as a breaking change

Steps to reproduce

Before

{
  "swagger": "2.0",
  "info": {
    "title": "Swagger Fixture",
    "version": "1.0"
  },
  "paths": {
    "/a/{id}": {
      "post": {
        "parameters": [
          {
            "name": "",
            "in": "body",
            "schema": { "$ref": "#/definitions/A2" }
          }
        ],
        "responses": {
          "200": {
            "description": "200 response",
            "schema": { "$ref": "#/definitions/A3" }
          }
        }
      }
    }
  },
  "definitions": {
    "A2": {
      "type": "object",
      "required": [ "name", "description" ],
      "properties": {
        "name": { "type": "string" },
        "description": { "type": "string" }
      }
    },
    "A3": {
      "type": "object",
      "properties": {
        "id": { "type": "integer" },
        "name": { "type": "string" },
        "otherDeletedName":{"type":"string","deprecated":true},
        "description": { "type": "string" },
        "letters": {
          "type": "array",
          "items": { "type": "string" }
        },
        "attributes": {
          "type": "object",
          "additionalProperties": { "type": "string" }
        }
      }
    }
  }
}

After

{
  "swagger": "2.0",
  "info": {
    "title": "Swagger Fixture",
    "version": "1.0"
  },
  "paths": {
    "/a/{id}": {
      "post": {
        "parameters": [
          {
            "name": "",
            "in": "body",
            "schema": { "$ref": "#/definitions/A2" }
          }
        ],
        "responses": {
          "200": {
            "description": "200 response",
            "schema": { "$ref": "#/definitions/A3" }
          }
        }
      }
    }
  },
  "definitions": {
    "A2": {
      "type": "object",
      "required": [ "name", "description" ],
      "properties": {
        "name": { "type": "string" },
        "description": { "type": "string" },
        "field3": { "type": "string" }
      }
    },
    "A3": {
      "type": "object",
      "properties": {
        "id": { "type": "integer" },
        "name": { "type": "string" },
        "otherDeletedName":{"type":"string","deprecated":true},
        "description": { "type": "string" },
        "letters": {
          "type": "array",
          "items": { "type": "string" }
        },
        "attributes": {
          "type": "object",
          "additionalProperties": { "type": "string" }
        }
      }
    }
  }
}

The diff tool should say there's a non-breaking change but it says there's a breaking change.

swagger diff old.json new.json
2023/08/16 22:56:08 Run Config:
2023/08/16 22:56:08 Spec1: old.json
2023/08/16 22:56:08 Spec2: new.json
2023/08/16 22:56:08 ReportOnlyBreakingChanges (-c) :false
2023/08/16 22:56:08 OutputFormat (-f) :txt
2023/08/16 22:56:08 IgnoreFile (-i) :none specified
2023/08/16 22:56:08 Diff Report Destination (-d) :stdout
BREAKING CHANGES:
=================
/a/{id}:post - Request - Body.field3<string> - Added property
compatibility test FAILED: 1 breaking changes detected
compatibility test FAILED: 1 breaking changes detected

Environment

go version: 1.20.4
OS: macOS 13.5

@fredbi fredbi added diff Relates to the swagger diff command bug labels Dec 21, 2023
@fredbi
Copy link
Contributor

fredbi commented Dec 21, 2023

Yes, it looks like the "AddedRequiredProperty" is not fully implemented.

fredbi added a commit to fredbi/go-swagger that referenced this issue Dec 21, 2023
A non-required added property in a request should not be reported as a
breaking change.

Fixed the change status of added properties depending on being required
or not in the new specification.

* fixes go-swagger#2962

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi added a commit to fredbi/go-swagger that referenced this issue Dec 21, 2023
A non-required added property in a request should not be reported as a
breaking change.

Fixed the change status of added properties depending on being required
or not in the new specification.

* fixes go-swagger#2962

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi added a commit that referenced this issue Dec 22, 2023
A non-required added property in a request should not be reported as a
breaking change.

Fixed the change status of added properties depending on being required
or not in the new specification.

* fixes #2962

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug diff Relates to the swagger diff command pending PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants