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

Avoid duplication in schema - consider a metadataEntity #607

Closed
javagl opened this issue Dec 10, 2021 · 2 comments · Fixed by #608
Closed

Avoid duplication in schema - consider a metadataEntity #607

javagl opened this issue Dec 10, 2021 · 2 comments · Fixed by #608

Comments

@javagl
Copy link
Contributor

javagl commented Dec 10, 2021

The specification currently says

Entities are instantiations of a class, populated with property values conforming to the class definition

Such entities can appear on different levels. Right now, the JSON schema definition for the structure of an entity exists in tileset.schema.json, tile.3DTILES_metadata.schema.json and group.schema.json.

These definitions are structurally equal, and only differ in the title and description. (Except for one accidental difference - and that's one argument for avoiding this sort of duplication...). We should consider combining these to something like a metadataEntity, to only have the schema definition once. I think this could avoid confusion and it could be nicely in line with the textual description. Such a metadataEntity could be created by just moving the common class/properties blocks from the schemas into a common metadataEntity.schema.json.


If we decide to do that, then we have to sort out whether the respective objects (i.e. the tileset, tile.3DTILES_metadata, and group) should then be a metadataEntity, or whether they should contain a metadataEntity.

For example, the tileset.schema.json currently is

{
    "$id": "tileset.schema.json",
    "properties": {
        "class": { ... }
        "properties": { ... }
    }
}

And an instance of that could be

      "tileset": {
        "class": "Example",
        "properties": {
          "numberOfTiles": "100",
          "date": "2021-03-23"
         }
      }

We could let the tileset then be a metadataEntity. This could happen by letting tileset be "allOf": [ { "$ref": "metadataEntity.schema.json" } ]. In this case, the instance JSON would not change.

Alternatively, the tileset could contain a metadataEntity, as in

{
    "$id": "tileset.schema.json",
    "properties": {
        "metadataEntity": { "type" : "metadataEntity.schema.json" }
    }
}

This would mean that the instance would become

      "tileset": {
        "metadataEntity": {
          "class": "Example",
          "properties": {
            "numberOfTiles": "100",
            "date": "2021-03-23"
          }
        }
      }

The drawback of this, at the first glance, could be a deeper nesting (and having to update existing sample data and inlined snippets). But letting these objects contain a metadataEntity could make it simpler to add specific properties to the tileset, tile.3DTILES_metadata, and group later, and dedicatedly keep these additional properties separate from the metadataEntity.


(An aside: I also thought about whether it could be possible to let the tileset be a metadataEntity, and going so far to remove the tileset.schema.json altogether, and always directly refer to the metadataEntity.schema.json instead. But I haven't yet thought through all possible implications of that, so this is not a suggestion, but only mentioned here as something that we might consider).

@javagl
Copy link
Contributor Author

javagl commented Dec 12, 2021

I have created a DRAFT for such a metadataEntity in #608 .

Until now, the tileset, tile.3DTILES_metadata, and group are a metadataEntity. If this is the preferred solution, then it raises another question about the structure: Until now, these types are all declared to be

"allOf": [
    {
        "$ref": "tilesetProperty.schema.json"
    }
],

and that makes sense. The new metadataEntity should, in some way, not be a tilesetProperty. There is no reason for that, and these structures could be independent. (And if they could be independent, then they should be independent, because fewer dependencies are usually better).

The problem is that then, the existing structures would have to be

"allOf": [
    {
        "$ref": "tilesetProperty.schema.json"
    },
    {
        "$ref": "metadataEntity.schema.json"
    }
],

but this implies "multiple inheritance". (Yes, I know, it's JSON Schema, and thus, not really "inheritance", but let's keep the real world and some code generators in mind here). That's another question where I haven't thought through all possible paths, options, and implications, but maybe we can find a sensible solution here.

@lilleyse
Copy link
Contributor

I'm good with the approach in #608. Each metadata entity still gets its own schema which allows additional properties to be added in the future. I prefer not having the nested metadataEntity object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants