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

Restrict the schemas to valid values #187

Closed
hugoledoux opened this issue Nov 17, 2023 Discussed in #186 · 7 comments
Closed

Restrict the schemas to valid values #187

hugoledoux opened this issue Nov 17, 2023 Discussed in #186 · 7 comments
Milestone

Comments

@hugoledoux
Copy link
Member

Discussed in #186

Originally posted by balazsdukai November 17, 2023
I experimented with the popular JSON Schema Faker tool to mock CityJSON data.
I used the combined v2.0 schema to fake values for the CityJSON members.
It does generate a seemingly schema-valid CityJSON object, however many members have invalid values, for instance negative vertex indices in the boundaries.
The reason for this is that in several cases the schema does not restrict the values sufficiently.

Below is are the issues and potential fixes that I discovered from the generated CityJSON.

  • The version number is invalid. It should be restricted to the CityJSON version that is described by the schema.
"version": "3.4"
  • CityObject family references point to nonexistent objects. This is probably not possible to prescribe solely with JSON Schema.
"CityObjects": {
  "et_": {
    "type": "BuildingInstallation",
    "parents": [
      "ex fugiat",
      "sed veniam ut dolore"
    ],
    "children": [
      "ipsum nostrud"
    ]
  }
}
  • Geometry LoD has invalid value. The lod should be restricted to the allowed values.
"lod": "9"
  • Vertex indices in the boundaries have negative values. The vertex indices should be restricted to positive integers. This example shows that this should be possible.
"boundaries": [
  [
    44174456,
    49920625,
    -95552225,
    -13564149
  ]
]
  • Semantic object does not have type. In the example below, the second semantic object does not have a type member, which is required according to the specs.
"semantics": {
  "surfaces": [
    {
      "type": "ClosureSurface",
      "laboris_4": "sit enim in",
      "amet62": true
    },
    {
      "deserunt_af": 18338638,
      "quis_2": -44755250,
      "in_e": 51750499,
      "adb_": "enim irure",
      "velit__": 73779662,
      "tempor_73": false
    }
  ]
}

I don't see drawbacks from applying these restrictions where possible, but maybe others see it differently?

@hugoledoux
Copy link
Member Author

I fixed the 4th above (semantic surfaces) there: 5544e1b

in a branch for v2.0.1

@hugoledoux hugoledoux added this to the 2.0.1 milestone Nov 21, 2023
hugoledoux added a commit that referenced this issue Apr 8, 2024
@hugoledoux
Copy link
Member Author

Point 1 above (version of CityJSON) has been fixed in 1dbd856

@hugoledoux
Copy link
Member Author

@balazsdukai for the #3 above ("The lod should be restricted to the allowed values"), I am not sure we want to do this... The specs state:

The value must be a string with the LoD identifying the level-of-detail (LoD) of the geometry. This can be either a single digit (following the CityGML standards), or "X.Y"-formatted if the improved LoDs by TU Delft are used.

If we restrict, we go up to what? LoD3.3? What if someone wants LoD4.2?

Maybe @fbiljecki has something smart to add here?

@fbiljecki
Copy link
Collaborator

If it follows the two standards, then it is better to constrain it with their possible values and ranges, that is, 3.3 being the maximum in the latter case.

@hugoledoux
Copy link
Member Author

this is the ones that would be accepted with CityGML3 (no LoD4 here)

  "Lods": {
      "enum": [
        "0",   "1",   "2",   "3", 
        "0.0", "0.1", "0.2", "0.3", 
        "1.0", "1.1", "1.2", "1.3", 
        "2.0", "2.1", "2.2", "2.3", 
        "3.0", "3.1", "3.2", "3.3"
      ]
  }

hugoledoux added a commit that referenced this issue Apr 8, 2024
@balazsdukai
Copy link
Member

balazsdukai commented Apr 9, 2024

What if someone wants LoD4.2?

Then it would be an extended CityJSON file, with its own schema that allows LoD4.2 I think. 👍 for 29dfdc4

@hugoledoux
Copy link
Member Author

Hmmm, no that would mean no geometry types since "lod" is a child of "geometry" and this is not possible to modify.

But CityJSON is CityGML (thus only 0-1-2-3) and the TUDelft-LoDs, so restricting is fine. If you want more than you can use IFC I guess

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

No branches or pull requests

3 participants