Skip to content

checker: a constraint that moves into a composition is reported as removed #1309

Open

Description

@reuvenharrison

Problem

When a constraint moves into a composition instead of being removed, oasdiff judges the keyword from its own diff and reports it as removed. The verdict is a widening on a schema that did not widen, so these findings are misleading, and on the response side they are false errors.

Each keyword is judged on its own (pattern, enum, format, minLength and so on). That is sound when the constraints stay where they were, because a schema accepts a value only if every keyword accepts it. It stops being accurate when the same constraint reappears inside a subschema.

Where the constraint can move

  • Into a composition on the same schema: allOf, anyOf, oneOf, not, if/then/else, and const (enum: [red] and const: red are equivalent).
  • Into a composition on an ancestor schema, reaching the property through properties.<name>: the same keywords plus dependentSchemas. Nothing looks at ancestors today.

$ref (including 3.1 $ref with sibling keywords) is resolved before the diff and is reported correctly.

Reproductions

Request body and 200 response with the same schema.

Pattern moved into the property's own oneOf:

# base
color: {type: string, pattern: "^[a-z]+$"}
# revision
color:
  type: string
  oneOf: [{pattern: "^red$"}, {type: "null"}]

Response: response-property-pattern-removed (error), though the response narrowed.

Enum moved into the parent's allOf:

# base
Req:
  type: object
  properties:
    color: {type: string, enum: [red, green, blue]}
# revision
Req:
  type: object
  properties:
    color: {type: string}
  allOf:
    - properties:
        color: {enum: [red]}

Request: request-property-became-not-enum (info). Response: response-property-became-not-enum (warning, lowered by the allOf disclaimer; see also #1308). Both say the enum constraint was removed; it moved.

Enum moved into the parent's anyOf: same as above with anyOf. Response: response-property-became-not-enum (error), though the response narrowed to red.

Proposed direction

Recognize the move as one change, the way the nullable wrapping is recognized (#1088, #1091): a derived recognition in diff/ with an equivalence or containment guard, and an entry in the transitions table (checker/transition_claims.go) that drops the echo findings and reports the move itself. Where the move cannot be proven to keep or narrow the accepted values, report it once at WARN with a comment saying the constraint moved into a subschema, instead of as removed.

Once #1307 is in, the enum-specific guard (mayRestrictValues) can be retired in favor of this, so enum is handled like every other keyword.

Out of scope

False positives between keywords on the same schema without composition (a pattern added that every enum value already matches, format: date added to an enum of dates) come from judging each keyword separately and belong with #924, which is about reporting the effect on accepted values rather than the document change. The same applies to became-not-enum saying the property "accepts any value of its type" when a pattern or format still restricts it.

Related

#1307, #1308, #1037, #1054, #924, #1088, #1091, #1280.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions