KhronosGroup / glTF

glTF – Runtime 3D Asset Delivery
Other
7.2k stars 1.14k forks source link

About using allOf for single types #2062

Open javagl opened 3 years ago

javagl commented 3 years ago

The schema currently uses allOf consistently to refer to single types. For example, in accessor.bufferView :

    "bufferView": {
        "allOf": [ { "$ref": "glTFid.schema.json" } ],
        "description": "The index of the bufferView.",
        "gltf_detailedDescription": "The index of the buffer view. When undefined, the accessor **MUST** be initialized with zeros; `sparse` property or extensions **MAY** override zeros with actual values."
    },

From what I read in the documentation about allOf and $ref, to my current understanding, it should be possible to replace this use of allOf with a plain $ref, as in

    "bufferView": {
        "$ref": "glTFid.schema.json",
        "description": "The index of the bufferView.",
        "gltf_detailedDescription": "The index of the buffer view. When undefined, the accessor **MUST** be initialized with zeros; `sparse` property or extensions **MAY** override zeros with actual values."
    },

The difficulties of modeling inheritance in general (c.f. https://github.com/KhronosGroup/glTF/issues/1144 ), the notes about "Subschema Independence" in the documentation, and (as usual:) possible changes between the Schema drafts that I may not yet have fully understood make me hesitate a bit here.

But I wondered whether there would be any semantical difference between the two options, and whether the plain $ref should be preferred for cases where allOf otherwise contains only a single type.

lexaknyazev commented 3 years ago

it should be possible to replace this use of allOf with a plain $ref

As the docs say, this behavior was introduced with draft/2019-09. Since we already moved to draft/2020-12, this change could certainly be implemented.

The difficulties of modeling inheritance

The trick is that there's no inheritance. Schemas are only composed, therefore they do not override each other's definitions. For this reason, we can stop putting empty name, extras, and extensions everywhere as long as additional properties are allowed.

lexaknyazev commented 3 years ago

For consistence, I think we should upgrade the extension schemas to draft/2020-12 (#2049) before implementing this new composition convention.

javagl commented 2 years ago

Despite trying to figure out a definite answer to this, I'm still not sure whether the new way of using $ref could also apply to the top-level objects, and there, essentially replace type. For example, in glTF.schema.json, it currently says

"type": "object",
...
"allOf": [ { "$ref": "glTFProperty.schema.json" } ],

and I think that it should be valid to replace that with

"$ref": "glTFProperty.schema.json",

But omitting the "type": "object" part looks strange.

javagl commented 2 years ago

I think I stumbled over a possible answer to the last question in https://github.com/APIDevTools/json-schema-ref-parser/issues/145 :

  • {"type": "object", "$ref": "foo"} is equivalent to {"type": "object", "allOf": [{... contents of foo schema ...}]}

(The person who wrote this may be an active maintainer of the JSON schema spec, but at least is a very active contributor)

It is now debatable whether this implies that it is OK to leave out the type, and whether

{ "$ref": "foo"} is equivalent to { "allOf": [{ "$ref": "foo" }]}

but iff omitting the type in the allOf-version is OK, then it should probably also be OK in the plain ref-version.

(In the meta-schema, the type does not have an explicit default value. I think that object is a reasonable default, and assume that many tools already do assume this, but we could consider adding the type:object when changing anything here, to ensure compliance to a strict meta-schema validation - TBD)