Commit c249436
Update config validation logic for entities (#3306)
## Why make this change?
Closes #3267
## What is this change?
Alters the validation logic in the following way.
Is top-level config with data-source-files? (we call this a `Root`
config file)
├── YES
│ ├── Has datasource? → ValidateEntityPresence (same rules as non-root)
│ ├── No datasource but has entities/autoentities? → ERROR
│ └── No datasource, no entities → VALID (children provide everything)
│ └── For each child → ValidateNonRootConfig(child, filename)
│
└── NO (standalone or child config)
├── No datasource? → ERROR: "data source is required"
└── Has datasource → ValidateEntityPresence
Note: A top-level config file without any children data-source files is
NOT considered a root. And an intermediary config file, ie: is a child,
that also has child configs is NOT a root. Only a top-level config with
children configs is a Root.
#### ValidateEntityPresence
Count resolved autoentities from AutoentityResolutionCounts
total = manual entities + resolved autoentities
total == 0? → ERROR: "No entities found"
total > 0 but autoentities discovered nothing? → WARN: "Autoentities
configured but none discovered"
No double messaging. If total is 0, only the error is recorded, not the
warning.
## How was this tested?
### Truth table — top-level config
Variables (`1` = present / non-empty, `0` = absent / empty):
- **DSF** — `data-source-files` present
- **DS** — `data-source` present
- **E** — manual `entities` count > 0
- **AE** — `autoentities` count > 0 (presence, *not* resolved count)
Path is determined by `IsRootConfig = (DSF == 1) && !IsChildConfig`.
| # | DSF | DS | E | AE | AE resolved | Path | Expected | Test |
|---|:---:|:--:|:-:|:--:|:-----------:|------|----------|------|
| 1 | 0 | 0 | 0 | 0 | — | Non-root | **Error**: "data source is
required" | `TestNonRootWithNoDataSourceProducesError` |
| 2 | 0 | 0 | 0 | 1 | — | Non-root | **Error**: "data source is
required" | _covered by #1 — DS check fires first_ |
| 3 | 0 | 0 | 1 | 0 | — | Non-root | **Error**: "data source is
required" | _covered by #1_ |
| 4 | 0 | 0 | 1 | 1 | — | Non-root | **Error**: "data source is
required" | _covered by #1_ |
| 5 | 0 | 1 | 0 | 0 | — | Non-root | **Error**: "No entities found" |
`TestNonRootWithDataSourceAndNoEntitiesProducesError` |
| 6a | 0 | 1 | 0 | 1 | 0 | Non-root | **Error**: "No entities found" |
`TestNonRootWithDataSourceAndAutoentitiesResolvingZeroProducesError` |
| 6b | 0 | 1 | 0 | 1 | >0 | Non-root | **Valid** |
`TestNonRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid` |
| 7 | 0 | 1 | 1 | 0 | — | Non-root | **Valid** |
`TestNonRootWithDataSourceAndEntitiesIsValid` |
| 8a | 0 | 1 | 1 | 1 | 0 | Non-root | **Valid** + **Warn** |
`TestNonRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning` |
| 8b | 0 | 1 | 1 | 1 | >0 | Non-root | **Valid** | _covered by #7 / #6b
combined_ |
| 9 | 1 | 0 | 0 | 0 | — | Root | **Valid** (children carry the load) |
`TestRootWithNoDataSourceAndNoEntitiesIsValid`,
`TestRootConfigWithNoDataSourceAndNoEntitiesParses` |
| 10 | 1 | 0 | 0 | 1 | — | Root | **Error**: "must not define entities
or autoentities" |
`TestRootWithNoDataSourceButAutoentitiesProducesError` |
| 11 | 1 | 0 | 1 | 0 | — | Root | **Error**: "must not define entities"
| `TestRootWithNoDataSourceButEntitiesProducesError` |
| 12 | 1 | 0 | 1 | 1 | — | Root | **Error** | _covered by #11_ |
| 13 | 1 | 1 | 0 | 0 | — | Root (with own DS) | **Error**: "No entities
found" | `TestRootWithDataSourceAndNoEntitiesProducesError` |
| 14a | 1 | 1 | 0 | 1 | 0 | Root (with own DS) | **Error**: "No entities
found" |
`TestRootWithDataSourceAndAutoentitiesResolvingZeroProducesError` |
| 14b | 1 | 1 | 0 | 1 | >0 | Root (with own DS) | **Valid** |
`TestRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid` |
| 15 | 1 | 1 | 1 | 0 | — | Root (with own DS) | **Valid** |
`TestRootWithDataSourceAndEntitiesIsValid` |
| 16a | 1 | 1 | 1 | 1 | 0 | Root (with own DS) | **Valid** + **Warn** |
`TestRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning` |
| 16b | 1 | 1 | 1 | 1 | >0 | Root (with own DS) | **Valid** | _covered
by #15 / #14b combined_ |
### Truth table — child config (validated when iterating
`root.ChildConfigs`)
Children are always treated as non-root regardless of their own
`data-source-files`.
| # | DS | E | AE | AE resolved | Expected | Test |
|---|:--:|:-:|:--:|:-----------:|----------|------|
| C1 | 0 | 0 | 0 | — | **Error** naming the child file: "data source is
required" | `TestChildWithNoDataSourceProducesNamedError` |
| C2 | 0 | * | * | — | **Error** naming the child file: "data source is
required" | _covered by C1_ |
| C3 | 1 | 0 | 0 | — | **Error** naming the child file: "No entities
found" | `TestChildWithDataSourceAndNoEntitiesProducesNamedError` |
| C4a | 1 | 0 | 1 | 0 | **Error** naming the child file: "No entities
found" |
`TestChildWithDataSourceAndAutoentitiesResolvingZeroProducesNamedError`
|
| C4b | 1 | 0 | 1 | >0 | **Valid** | _covered by C5 (resolved entities
behave the same as manual entities)_ |
| C5 | 1 | 1 | 0 | — | **Valid** | _implicitly via
`TestRootWithDataSourceAndEntitiesIsValid` setup_ |
| C6a | 1 | 1 | 1 | 0 | **Valid** + **Warn** naming the child file |
`TestChildWithEntitiesAndAutoentitiesResolvingZeroLogsNamedWarning` |
| C6b | 1 | 1 | 1 | >0 | **Valid** | _covered by C5_ |
### Other scenarios
| Scenario | Expected | Test |
|----------|----------|------|
| Connection-string error gates entity validation (no entity error fires
when DB unreachable) | `IsConfigValid == false` due to connection error
only | `TestValidateNonRootZeroEntitiesWithInvalidConnectionString` |
| Config with no entities parses cleanly (constructor no longer throws)
and `IsConfigValid` returns false without throwing | parse OK, validate
fails | `TestValidateConfigWithNoEntitiesProducesCleanError`
_(modified)_ |
| Root parses successfully without a data source | parse OK,
`IsRootConfig == true` |
`TestRootConfigWithNoDataSourceAndNoEntitiesParses` |
| Non-root with DS and no entities parses successfully | parse OK,
`IsRootConfig == false` |
`TestNonRootConfigWithDataSourceAndNoEntitiesParses` |
| Autoentities present but resolve to nothing — must not crash, must not
double-message with "No entities found" | no crash; only "No entities
found" if total = 0 | `ValidateAutoentitiesConfiguration` _(modified to
`isValidateOnly: true`)_ |
New tests:
`TestRootConfigWithNoDataSourceAndNoEntitiesParses` Root config (has
data-source-files) without datasource parses OK
`TestNonRootConfigWithDataSourceAndNoEntitiesParses` Non-root config
with datasource + no entities parses OK (validation catches it later)
`TestNonRootWithDataSourceAndNoEntitiesProducesError` Calls
ValidateDataSourceAndEntityPresence directly, error recorded
`TestNonRootWithNoDataSourceProducesError` No datasource, error with
"data source is required"
`TestNonRootWithDataSourceAndEntitiesIsValid` Datasource + entities, no
errors
`TestRootWithNoDataSourceAndNoEntitiesIsValid` Root with child, no own
datasource, valid
`TestRootWithNoDataSourceButEntitiesProducesError` Root with entities
but no datasource, error
`TestRootWithDataSourceAndEntitiesIsValid` Root with own datasource +
entities, valid
`TestChildWithDataSourceAndNoEntitiesProducesNamedError` Child with no
entities, error names the child file
`TestChildWithNoDataSourceProducesNamedError` Child with no datasource,
error names the child file
`TestNonRootWithDataSourceAndAutoentitiesResolvingZeroProducesError`
Non-root with only autoentities that resolve to 0
`TestNonRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid`
Non-root with only autoentities resolving > 0 entities
`TestNonRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning`
Non-root with manual entities + autoentities resolving 0
`TestRootWithNoDataSourceButAutoentitiesProducesError` Root with no
datasource but autoentities defined
`TestRootWithDataSourceAndNoEntitiesProducesError` Root with own
datasource and zero entities/autoentities
`TestRootWithDataSourceAndAutoentitiesResolvingZeroProducesError` Root
with own datasource and autoentities resolving 0
`TestRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid` Root
with own datasource and autoentities resolving > 0
`TestRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning` Root with
own datasource, manual entities, and autoentities resolving 0
`TestChildWithDataSourceAndAutoentitiesResolvingZeroProducesNamedError`
Child with autoentities-only resolving 0
`TestChildWithEntitiesAndAutoentitiesResolvingZeroLogsNamedWarning`
Child with manual entities + autoentities resolving 0
Modified tests:
`TestValidateConfigWithNoEntitiesProducesCleanError` Replaced main's
version (expected parse failure) with ours: parse succeeds,
IsConfigValid returns false
`ValidateAutoentitiesConfiguration` Changed to isValidateOnly: true,
asserts no crashes instead of zero errors
---------
Co-authored-by: Anusha Kolan <anushakolan10@gmail.com>1 parent ba3fd40 commit c249436
18 files changed
Lines changed: 948 additions & 109 deletions
File tree
- src
- Cli.Tests
- Cli
- Config
- ObjectModel
- Core
- Configurations
- Services/MetadataProviders
- Service.Tests
- Configuration
- Service/HealthCheck
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
811 | 811 | | |
812 | 812 | | |
813 | 813 | | |
814 | | - | |
| 814 | + | |
815 | 815 | | |
816 | 816 | | |
817 | 817 | | |
| |||
841 | 841 | | |
842 | 842 | | |
843 | 843 | | |
844 | | - | |
| 844 | + | |
845 | 845 | | |
846 | 846 | | |
847 | 847 | | |
| |||
877 | 877 | | |
878 | 878 | | |
879 | 879 | | |
880 | | - | |
| 880 | + | |
881 | 881 | | |
882 | 882 | | |
883 | 883 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
68 | 68 | | |
69 | 69 | | |
70 | 70 | | |
71 | | - | |
| 71 | + | |
72 | 72 | | |
73 | 73 | | |
74 | 74 | | |
| |||
96 | 96 | | |
97 | 97 | | |
98 | 98 | | |
99 | | - | |
| 99 | + | |
100 | 100 | | |
101 | 101 | | |
102 | 102 | | |
| |||
127 | 127 | | |
128 | 128 | | |
129 | 129 | | |
130 | | - | |
| 130 | + | |
131 | 131 | | |
132 | 132 | | |
133 | 133 | | |
| |||
208 | 208 | | |
209 | 209 | | |
210 | 210 | | |
211 | | - | |
| 211 | + | |
212 | 212 | | |
213 | 213 | | |
214 | 214 | | |
| |||
247 | 247 | | |
248 | 248 | | |
249 | 249 | | |
250 | | - | |
| 250 | + | |
251 | 251 | | |
252 | 252 | | |
253 | 253 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
119 | 119 | | |
120 | 120 | | |
121 | 121 | | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
122 | 130 | | |
123 | 131 | | |
124 | 132 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
53 | | - | |
| 53 | + | |
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
| |||
95 | 95 | | |
96 | 96 | | |
97 | 97 | | |
98 | | - | |
| 98 | + | |
99 | 99 | | |
100 | 100 | | |
101 | 101 | | |
0 commit comments