keenlabs / keen-sdk-net

A .NET SDK for the Keen IO API
MIT License
37 stars 23 forks source link

Better validation for Datasets #130

Open masojus opened 6 years ago

masojus commented 6 years ago

From review comments:

We should make sure these two names follow the rules for naming Cached Queries/Datasets: Names of Cached Datasets should be unique and can only contain alphanumeric characters, hypens ( - ), and underscores ( _ ) but the Display Name can be more human readable.

and

Both DatasetDefinition and QueryDefinition need more validation--e.g. timeframe must be relative, the timeframe units must match the interval, index_by cannot match a group_by property, etc.

masojus commented 6 years ago

The unit tests for Datasets could also be augmented to check the actual structure of the URL or POST payload that is being sent, and also make sure that in success and error cases, the right values (if/when there can be ambiguity in how JSON.net deserializes) are in the response.

masojus commented 6 years ago

Example from PR #128 :

"This test seems like it doesn't quite cover all validation. For example it doesn't really test that AnalysisType being unset will ever throw. Maybe separate tests for each invalid condition would be warranted? Maybe start with a valid dataset and then modify only one field at a time per-test and assert that it throws?"