perf: cache ExtendedProperty parse in subcategory filter (#28)#53
Open
cocallaw wants to merge 5 commits into
Open
perf: cache ExtendedProperty parse in subcategory filter (#28)#53cocallaw wants to merge 5 commits into
cocallaw wants to merge 5 commits into
Conversation
Parse ExtendedProperty JSON once during the Where-Object filter and cache the result via Add-Member. The downstream code already checks for ExtendedPropertyObject before re-parsing, so this eliminates the redundant ConvertFrom-Json call per matching recommendation. Fixes #28 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes the Az.Advisor path in Get-AzRetirementRecommendation by caching the parsed ExtendedProperty JSON during the subcategory filtering step, preventing redundant parsing later in the pipeline.
Changes:
- Parse
ExtendedPropertyJSON once inside the subcategoryWhere-Objectfilter. - Attach the parsed object to each matching recommendation as
ExtendedPropertyObjectfor downstream reuse.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The subcategory filter now handles ExtendedProperty as string (JSON), hashtable, or PSCustomObject gracefully. Parse failures are caught silently and the item is filtered out rather than throwing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Owner
Author
|
Addressed the type checking feedback:
All 59 tests pass. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses review feedback on PR #53 by adding Pester tests that validate: - JSON string ExtendedProperty is parsed and cached as ExtendedPropertyObject - Hashtable ExtendedProperty is used directly and cached - Non-matching subcategories are filtered out - Invalid JSON is handled gracefully without throwing - Null ExtendedProperty is filtered out Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace empty catch {} with explicit $extProps = $null assignment
to satisfy PSScriptAnalyzer PSAvoidUsingEmptyCatchBlock rule
- Add function definition assertions to validate the production code
contains Add-Member caching and type checks
- Remove unused $functionDef that was assigned but never referenced
- Update test catch blocks to match production code pattern
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+159
to
+173
| # Common filter for ServiceUpgradeAndRetirement subcategory | ||
| $subcategoryFilter = { | ||
| # Parse extended properties to check subcategory | ||
| if ($_.ExtendedProperty) { | ||
| $extProps = $_.ExtendedProperty | ConvertFrom-Json | ||
| $extProps.recommendationSubCategory -eq 'ServiceUpgradeAndRetirement' | ||
| $extProps = $null | ||
| if ($_.ExtendedProperty -is [string]) { | ||
| try { $extProps = $_.ExtendedProperty | ConvertFrom-Json } | ||
| catch { $extProps = $null } | ||
| } | ||
| elseif ($_.ExtendedProperty -is [hashtable] -or $_.ExtendedProperty -is [pscustomobject]) { | ||
| $extProps = $_.ExtendedProperty | ||
| } | ||
| if ($extProps -and $extProps.recommendationSubCategory -eq 'ServiceUpgradeAndRetirement') { | ||
| $_ | Add-Member -NotePropertyName ExtendedPropertyObject -NotePropertyValue $extProps -Force | ||
| $true | ||
| } else { $false } |
Addresses review feedback: - Extract shared Resolve-ExtendedProperty private helper used by both the subcategory filter and output construction (eliminates duplication) - Tests now use InModuleScope to exercise the real helper function rather than duplicating filter logic inline - Remove unused mock of Get-AzAdvisorRecommendation from tests - Add function definition assertions scoped to the actual call sites Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+29
to
+36
| $extProps = $Recommendation.ExtendedProperty | ConvertFrom-Json | ||
| } | ||
| elseif ($Recommendation.ExtendedProperty -is [hashtable] -or $Recommendation.ExtendedProperty -is [pscustomobject]) { | ||
| $extProps = $Recommendation.ExtendedProperty | ||
| } | ||
|
|
||
| if ($extProps) { | ||
| $Recommendation | Add-Member -NotePropertyName ExtendedPropertyObject -NotePropertyValue $extProps -Force |
Comment on lines
+377
to
+387
| BeforeAll { | ||
| $functionDef = (Get-Command Get-AzRetirementRecommendation).Definition | ||
| } | ||
|
|
||
| It "Should use Resolve-ExtendedProperty helper in the subcategory filter" { | ||
| $functionDef | Should -Match 'subcategoryFilter\s*=\s*\{[^}]*Resolve-ExtendedProperty' | ||
| } | ||
|
|
||
| It "Should use Resolve-ExtendedProperty helper in the output construction" { | ||
| # Verify the helper is also used outside the filter (in the foreach loop) | ||
| $functionDef | Should -Match 'Resolve-ExtendedProperty -Recommendation \$rec' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Eliminates redundant
ConvertFrom-Jsoncall by caching the parsedExtendedPropertyduring the subcategory filter step.Changes
Where-Objectfilter and attach result asExtendedPropertyObjectviaAdd-MemberExtendedPropertyObjectbefore re-parsing, so this completes the optimizationRelated Issue
Fixes #28