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

Consider option to allow null for any graph object property during validation #13

Open
aiwilliams opened this issue May 12, 2020 · 2 comments

Comments

@aiwilliams
Copy link
Contributor

Sometimes, the data provider/API of third party systems returns a null value for properties. It doesn't seem that Ajv provides for a global configuration of allowing nullable properties, we'd have to make the schemas have nullable on every single property, or at least on those where we want to allow null.

During work on the PagerDuty integration, it seemed that a better course was to change createIntegrationEntity to avoid transferring null values to the target entity. This seemed at the time as essentially the same behavior as not receiving the property at all from the provider.

There's been another run in where the integration was using assign to directly set null and had to work around validation by value ? value : undefined.

We could consider making https://github.com/JupiterOne/data-model/blob/master/src/validateEntityWithSchema.ts#L7 support an option to remove null value properties from the graph object so that they're treated as undefined.

@aiwilliams
Copy link
Contributor Author

aiwilliams commented May 12, 2020

A few other things to keep in mind:

  1. The persister uses null values as a mechanism to direct it to remove properties from stored graph objects.

  2. The integration-sdk disables schema validation in production because provider APIs are not always consistent in providing data for every property, all the time. We have no runtime type checking of provider data so that we know when the provider is behaving "inconsistently".

  3. We have a UI driven by the schema. What implications are there by allowing null on every property just to cover for unstable provider data?

@aiwilliams
Copy link
Contributor Author

Hmm, another consideration....

Perhaps createIntegrationEntity should always transfer as-is, and we only need an option to have the validateEntityWithSchema remove the null value properties before validation, so that we don't have to muddy the schemas with nullable all over the place.

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

1 participant