Validator tool is not compatible with its own schema

[np-soartech]

Background

We have a scenario where we are updating the state for a scene (for instance to update vitals). To do this we are adding a scene[].state field as specified here:

itm-scenario-validator/api_files/api.yaml

Line 870 in 4f6ebf4

state:

For example:

scenes:
  - index: 4
    state: { ... }

The scene state is of type State

itm-scenario-validator/api_files/api.yaml

Lines 870 to 871 in 4f6ebf4

	state:
	$ref: "#/components/schemas/State"

Which has the following required fields:

itm-scenario-validator/api_files/api.yaml

Lines 472 to 477 in 4f6ebf4

	State:
	required:
	- unstructured
	- environment
	- supplies
	- characters

So in order to update a small portion of the state (i.e. changing vitals), we must fully redefine unstructured, environment, supplies, and characters.

Further, scene[].state.environment (type: #/components/schemas/Environment) has the following required fields:

itm-scenario-validator/api_files/api.yaml

Lines 547 to 549 in 4f6ebf4

	Environment:
	required:
	- sim_environment

And scene[].state.environment.sim_environment (type: #/components/schemas/SimEnvironment) has the following required fields:

itm-scenario-validator/api_files/api.yaml

Lines 557 to 559 in 4f6ebf4

	SimEnvironment:
	required:
	- type

Issue

So to recap, scene[].state.environment.sim_environment.type is always required in order to update any part of the state. However, when defining this field in a scene state update, the scenario is no longer valid according to the validator tool:

It looks like this discrepancy is caused by manually swapping out the state schema here:

itm-scenario-validator/validator.py

Lines 194 to 200 in 4f6ebf4

	def validate_state_change(self, obj_to_validate):
	'''
	Under Scenes in the API, state should be defined slightly differently.
	Use state_changes.yaml and perform as before.
	'''
	schema = self.state_changes_yaml['components']['schemas']
	top_level = schema['State']['properties']

This manual schema swap makes the validator tool incompatible with its own schema file (since the schema file cannot include this manual swap) so we must either (1) define the data according to the schema and have errors from the validator tool or (2) define according to the validator tool and have errors in the schema.

Since we are using standard OpenAPI tools for our server, we cannot have (2). The libraries we are using simply won't consume the data if it doesn't match the schema. So we are currently settling for (1).

Potential Fixes

1. Move state.environment.sim_environment.type from Environment to the top-level Scenario object since it can only be used one time in each scenario anyway
2. Define explicit objects for SceneState, SceneEnviroment, and SceneSimEnvironment that can be used for state updates in a scene that encode the special rules of api/state_changes.yaml directly in the schema file.
   1. This would have the added benefit of allowing us to remove all required fields from these objects and allowing small updates without defining the entire state again
3. Make scene[].state.environment.sim_environment.type optional. This is technically incorrect since it is not optional for the first state but could be a shortcut until (1) is done to at least make it so the validator tool isn't broken

[nextcen-dgemoets]

Nick-- you should be able to configure your scenes.state with only the elements the validator requires, not everything that's "required" by the State object and its sub-components. You are not sending the State object across the wire, and that is where Swagger checks for required values. In the validator's test.yaml, you'll see we only specify environment.decision_environment.unstructured and environment.decision_environment.aid_delay, and I am able to read these in from the YAML.

That said, it's clear that this needs to be documented. We don't really have a "user's manual" for scenario creators, but there are a few things that would go there. For now, I'll add them to the Scenario YAML Documentation page on Confluence.

Let me know if this addresses the issue for you.

[np-soartech]

Hi @nextcen-dgemoets Thanks for the reply, I was on PTO last week so I've only just now had a chance to get back with you. We still have a few concerns about the schema and the issue I brought up above:

you should be able to configure your scenes.state with only the elements the validator requires, not everything that's "required" by the State object and its sub-components.

The schema specifies that the fields are required so if we do not include them, the schema will not pass and the data will be considered invalid.

itm-scenario-validator/api_files/api.yaml

Lines 472 to 477 in 4f6ebf4

	State:
	required:
	- unstructured
	- environment
	- supplies
	- characters

You are not sending the State object across the wire

We are sending state across the wire. It is included in the Scenario object which is sent to the TA3 server here via /api/v1/scenario. The endpoint is documented here: https://itm-soartech.github.io/itm-api/#/default/get_scenario_api_v1_scenario__scenario_id___get

...across the wire and that is where Swagger checks for required values.

Many standard tools check objects against the schema when they are loaded (we, for instance, are using Pydantic to do this). On start-up, before anything is sent over the wire, the TA1 server code will not load a scenario that is invalid (doesn't match the schema).

In the validator's test.yaml, you'll see we only specify environment.decision_environment.unstructured and environment.decision_environment.aid_delay, and I am able to read these in from the YAML.

I was able to check the test.yaml file using the validator tool and it passed. But this is actually the issue. The same test.yaml file is invalid when tested against the schema file using standard OpenAPI tools.

We put together a minimal example (script here) using openapi_core to validate test.yaml to show the discrepancy. Note that OpenAPI actually identifies two errors with test.yaml:

1. Required fields are missing (the issue we identified above)
2. The KDMA scores in the test file are values between 1 and 10 but the schema specifies that the values should be between 0 and 1.

Based on the comment "you should be able to configure your scenes.state with only the elements the validator requires," we assume you mean that we only need to worry about what the validator tool says and that we should ignore the required fields in the schema. We would strongly recommend that we do not to this, especially after the months of work all teams have put into building the schema. We recommend that we ensure the schema is consistent with the validator tool rather than decide to deliberately ignore certain parts of it. SoarTech is happy to help make these updates. Note also that if we ignore certain parts of the schema, we may also run into issues like the KDMA error in test.yaml where some teams are using different values for different fields because they pass the validator even though they do not conform to the schema.

I'd be happy to discuss this in more detail and we would be happy to help implement the fix, feel free to ping me on slack or email.