Add JSON schema caching#1637
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an in-memory, process-wide JSON Schema cache keyed by resource identity so repeated schema validations don’t re-fetch schemas (e.g., during test/set/get flows), addressing #1513.
Changes:
- Introduces a new
schema_cachemodule with a staticRESOURCE_SCHEMASstore and a lookup helper. - Adds cache lookup/logging and cache population to schema retrieval paths for command resources / resource schema invocation.
- Adds Pester coverage that exercises cache-hit behavior and updates localization strings; bumps the bundled jsonschema patch version metadata.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/dsc-lib/src/dscresources/dscresource.rs | Adds cache lookup in schema() and (currently) also attempts to populate the cache. |
| lib/dsc-lib/src/dscresources/command_resource.rs | Adds cache lookup and cache population inside get_schema(). |
| lib/dsc-lib/src/configure/schema_cache.rs | New module defining the global in-memory schema cache and lookup helper. |
| lib/dsc-lib/src/configure/mod.rs | Exposes the new schema_cache module. |
| lib/dsc-lib/locales/en-us.toml | Adds log strings for “retrieved schema from cache” messages. |
| lib/dsc-lib-jsonschema/.versions.json | Updates latest patch metadata and list to include V3_2_3. |
| dsc/tests/dsc_config_get.tests.ps1 | Adds tests that validate cache-hit logging for repeated resource usage. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
dsc/tests/dsc_config_get.tests.ps1:205
- This test name states the embedded schema is retrieved "only once", but the assertion only checks that a cache-hit message appears at least once. That can still pass even if the schema is retrieved multiple times (as long as there is at least one cache hit). Consider also asserting that the cache-miss log ("Invoking schema for ...") appears exactly once for this resource type during the run.
$LASTEXITCODE | Should -Be 0 -Because $errorLog
$result.hadErrors | Should -BeFalse
$result.results.Count | Should -Be 2
$errorLog | Should -BeLike "*Retrieved schema for resource 'Microsoft.DSC.Debug/Echo' with version '1.0.0' from cache*"
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
dsc/tests/dsc_config_get.tests.ps1:204
- This test claims the embedded schema should be retrieved "only" once, but the current assertion only checks that a cache-hit message appears at least once. That would still pass even if the cache is hit multiple times (or if the message is logged more than once), so it doesn't actually verify the "only once" requirement.
$errorLog | Should -BeLike "*Retrieved schema for resource 'Microsoft.DSC.Debug/Echo' with version '1.0.0' from cache*"
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
dsc/tests/dsc_config_get.tests.ps1:205
- This test name says the embedded schema should be retrieved "only" once, but the current assertion only checks that the cache-hit message appears somewhere in the log. If the message appears multiple times (or the schema is retrieved more than once), this would still pass. Consider asserting the exact number of cache-hit log entries to match the test intent.
$errorLog = Get-Content $TestDrive/error.log -Raw
$LASTEXITCODE | Should -Be 0 -Because $errorLog
$result.hadErrors | Should -BeFalse
$result.results.Count | Should -Be 2
$errorLog | Should -BeLike "*Retrieved schema for resource 'Microsoft.DSC.Debug/Echo' with version '1.0.0' from cache*"
}
c2ad345 to
994cffb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
dsc/tests/dsc_config_get.tests.ps1:186
- This
Itdescription claims the embedded schema is retrieved "only once", but the assertions only verify that a cache-hit message appears (at least once). Rename the test to match what it actually asserts (that the schema cache is used).
It 'embedded schema should only be retrieved once for multiple instances of the same resource' {
dsc/tests/dsc_config_get.tests.ps1:207
- This
Itdescription doesn’t match the assertions: the test checks for cache-hit log lines, not that the schema is retrieved "only once". Rename the test to describe the cache usage being asserted.
It 'retrieves command resource schema from cache for multiple instances of the same resource' {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
dsc/tests/dsc_config_get.tests.ps1:187
- This test claims the schema is retrieved "only" once, but it only asserts a cache-hit message appears. It also calls this an "embedded" schema even though
Microsoft.DSC.Debug/Echoretrieves schema via a schema command. Tighten the assertions (e.g., exactly one "Invoking schema" for this resource) and/or adjust the name to match what is being validated.
It 'embedded schema should only be retrieved once for multiple instances of the same resource' {
$config_yaml = @"
dsc/tests/dsc_config_get.tests.ps1:233
- These assertions only check that the cache-hit messages appear somewhere in the debug log. To validate the intended behavior (cache is keyed by version and avoids repeated retrieval), assert the expected number of schema-cache misses as well (one miss per version => two total "Invoking schema" lines here).
$result.results.Count | Should -Be 3
$errorLog | Should -BeLike "*Retrieved schema for resource 'Test/Version' with version '1.1.0' from cache*"
$errorLog | Should -BeLike "*Retrieved schema for resource 'Test/Version' with version '2.0.0' from cache*"
PR Summary
Add an in-memory static cache of retrieved JSON schema for resources so if the resource is used more than once it doesn't need to be retrieved from the resource itself.
PR Context
Fix #1513