Required property rule sources

[StarlessNights]

Two rules about required fields may give you these errors:

• "object contains required fields but no properties"
• "required field %s is not defined in properties"

It's not clear to me that these are actual errors. Where from the spec(s) are these derived from?
To my understanding, neither of these violates plain JSON schema, but maybe there's something OpenAPI dialect specific going on?

[LasneF]

there is no strict JSON schema rules about it , as the specification wanted to be very (very) flexible and extensible ( cf the choice of having additionnalProperties : true by default )

it is more a good practice and design . How can you set a field is required without defining it properly , it means that at minimum you know its name and so could have bit a minimum specified ...

[StarlessNights]

In an OpenAPI spec you can leverage this to reduce duplication. I.e. typically POST (create) and GET requests to the same path will have largely the same properties, but the required status changes. So in this case you can put the properties in a common component schema, and then use allOf to combine it with a "dummy schema" which only defines the required fields. Otherwise you'll need to duplicate the properties.

[LasneF]

this is so related to a hot topic i was posting yesterday , today and yesterday
especially this one : OAI/learn.openapis.org#101

[daveshanley]

This is actually why this tool exists, it's to catch not only schema issues, but bad design through opinions.

Setting required without defining properties is pointless and it's confusing and bad practice in my personal opinion, I would question that design in any spec I reviewed personally.

In this case, if properties between two verbs does not change, but the required state does, the properties would be references and the required array would be the only difference between the definitions.

[clarkminor]

In an OpenAPI spec you can leverage this to reduce duplication. I.e. typically POST (create) and GET requests to the same path will have largely the same properties, but the required status changes. So in this case you can put the properties in a common component schema, and then use allOf to combine it with a "dummy schema" which only defines the required fields. Otherwise you'll need to duplicate the properties.

Yep, I came across this issue because that's how Microsoft organizes many of their OpenAPI specs. See this example from the Azure OpenAI service:

https://github.com/Azure/azure-rest-api-specs/blob/main/specification/cognitiveservices/data-plane/AzureOpenAI/inference/preview/2023-06-01-preview/inference.json#L1108

      "extensionsChatCompletionsRequest": {
        "type": "object",
        "description": "Request for the chat completions using extensions",
        "required": [
          "messages"
        ],
        "allOf": [
          {
            "$ref": "#/components/schemas/chatCompletionsRequestCommon"
          },
          {
            "properties": {
              "messages": {
                "type": "array",
                "items": {
                  "$ref": "#/components/schemas/message"
                }
              },
              "dataSources": {
                "type": "array",
                "description": "The data sources to be used for the Azure OpenAI on your data feature.",
                "items": {
                  "$ref": "#/components/schemas/dataSource"
                }
              }
            }
          }
        ],

[StarlessNights]

@daveshanley

This is actually why this tool exists, it's to catch not only schema issues, but bad design through opinions.

Setting required without defining properties is pointless and it's confusing and bad practice in my personal opinion, I would question that design in any spec I reviewed personally.

I agree that this is not an ideal idiom.

In this case, if properties between two verbs does not change, but the required state does, the properties would be references and the required array would be the only difference between the definitions.

So in this one you would need to reference each property individually (in both operations)? Or is there a way to combine them somehow?

[daveshanley]

@daveshanley

This is actually why this tool exists, it's to catch not only schema issues, but bad design through opinions.
Setting required without defining properties is pointless and it's confusing and bad practice in my personal opinion, I would question that design in any spec I reviewed personally.

I agree that this is not an ideal idiom.

In this case, if properties between two verbs does not change, but the required state does, the properties would be references and the required array would be the only difference between the definitions.

So in this one you would need to reference each property individually (in both operations)? Or is there a way to combine them somehow?

allOf would be the correct mechanism here I believe.

[StarlessNights]

Still not quite clear to me. Just to iron this out, would you consider this the correct and minimally duplicating way of writing this hypothetical schema?

# ... #
components:
  schemas:
    name:
      type: string
    disabled:
      type: boolean
paths:
  /thing:
    post:
      operationId: thing_post
      requestBody:
        content:
          application/json:
            schema:
              properties:
                name:
                  $ref: '#/components/name'
                disabled:
                  $ref: '#/components/disabled'
              # Only name is required here
              required:
                - name
    get:
      description: This service only ever stores one thing so there's no need for parameters.
      operationId: thing_get
      responses:
        '200':
          content:
            application/json:
              schema:
                properties:
                  name:
                    $ref: '#/components/name'
                  disabled:
                    $ref: '#/components/disabled'
                # Both properties always appear in the response.
                required:
                  - name
                  - disabled

[LasneF]

BTW @StarlessNights , @clarkminor
just got this morning the case

@daveshanley the team was applying being

commonTask : contains a set of data like startDate and defined a status as required but not defining the field status

then teams defines multiple combination
AllOf (commonTask + Done) = defines status as enum done + some fields
AllOf (commonTask + Error) = defines status as enum error + some others fields

I would not judge if it is bad or good design :) , but it exists , what would it means for vacuum ?

To me it should not raise a warning , vacuum should raise a lint and raise warning only definitive object used and not lint a leaf , especially that is embeddeed in a bigger object . should in fact in the case of allOf not lint individual items but merge them and then lint the result

but that may be not so easy with the "merge" of constrains in the allOf , anyOf , OneOf ...

@daveshanley to me we could apply the logic

allOf = addition / union of required statement (lint the merge)
anyOf = intersection of required statement (lint the merge)
OneOf = each of them should be consistant (lint each leaf)