From 3d2afbe017334231c7b12766d0f6d89fa0486b91 Mon Sep 17 00:00:00 2001 From: hmmwhatsthisdo Date: Tue, 13 Aug 2024 23:51:37 -0700 Subject: [PATCH] Improve performance of `New-`/`Set-JiraIssue` on instances with large quantities of fields (#516) * Refactor Set-JiraIssue to only call Get-JiraField once * Reduce log-spam and add explicit debug message before calling Get-JiraField * Refactor New-JiraIssue to only call Get-JiraField once * Re-add missing variable declarations * Reunify parameter formatting in logs * Fix New-JiraIssue unit tests * Add mock for Get-JiraField * Remove unnecessary mock --------- Co-authored-by: hmmwhatsthisdo <2093321+hmmwhatsthisdo@users.noreply.github.com> --- JiraPS/Public/New-JiraIssue.ps1 | 69 +++++++++++++++----- JiraPS/Public/Set-JiraIssue.ps1 | 47 +++++++++++-- Tests/Functions/New-JiraIssue.Unit.Tests.ps1 | 15 ++++- Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 | 34 ++++++++-- 4 files changed, 135 insertions(+), 30 deletions(-) diff --git a/JiraPS/Public/New-JiraIssue.ps1 b/JiraPS/Public/New-JiraIssue.ps1 index 8d5477cf..6e42b581 100644 --- a/JiraPS/Public/New-JiraIssue.ps1 +++ b/JiraPS/Public/New-JiraIssue.ps1 @@ -126,29 +126,62 @@ function New-JiraIssue { } } - Write-Debug "[$($MyInvocation.MyCommand.Name)] Resolving `$Fields" - foreach ($_key in $Fields.Keys) { - $name = $_key - $value = $Fields.$_key - - if ($field = Get-JiraField -Field $name -Credential $Credential -Debug:$false) { - # For some reason, this was coming through as a hashtable instead of a String, - # which was causing ConvertTo-Json to crash later. - # Not sure why, but this forces $id to be a String and not a hashtable. + If ($Fields) { + + Write-Debug "[$($MyInvocation.MyCommand.Name)] Enumerating fields for server" + + # Fetch all available fields ahead-of-time to avoid repeated API calls in the upcoming loop. + # Eventually, this may be better to extract from CreateMeta. + $AvailableFields = Get-JiraField -Credential $Credential -ErrorAction Stop -Debug:$false + + $AvailableFieldsById = $AvailableFields | Group-Object -Property Id -AsHashTable -AsString + $AvailableFieldsByName = $AvailableFields | Group-Object -Property Name -AsHashTable -AsString + + + Write-Debug "[$($MyInvocation.MyCommand.Name)] Resolving `$Fields" + + foreach ($_key in $Fields.Keys) { + + $name = $_key + $value = $Fields.$_key + # The Fields hashtable supports both name- and ID-based lookup for custom fields, so we have to search both. + if ($AvailableFieldsById.ContainsKey($name)) { + $field = $AvailableFieldsById[$name][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a field ID" + } elseif ($AvailableFieldsById.ContainsKey("customfield_$name")) { + $field = $AvailableFieldsById["customfield_$name"][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a numerical field ID (customfield_$name)" + } elseif ($AvailableFieldsByName.ContainsKey($name) -and $AvailableFieldsByName[$name].Count -eq 1) { + $field = $AvailableFieldsByName[$name][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a human-readable field name ($($field.ID))" + } elseif ($AvailableFieldsByName.ContainsKey($name)) { + # Jira does not prevent multiple custom fields with the same name, so we have to ensure + # any name references are unambiguous. + + # More than one value in $AvailableFieldsByName (i.e. .Count -gt 1) indicates two duplicate custom fields. + $exception = ([System.ArgumentException]"Ambiguously Referenced Parameter") + $errorId = 'ParameterValue.AmbiguousParameter' + $errorCategory = 'InvalidArgument' + $errorTarget = $Fields + $errorItem = New-Object -TypeName System.Management.Automation.ErrorRecord $exception, $errorId, $errorCategory, $errorTarget + $errorItem.ErrorDetails = "Field name [$name] in -Fields hashtable ambiguously refers to more than one field. Use Get-JiraField for more information, or specify the custom field by its ID." + $PSCmdlet.ThrowTerminatingError($errorItem) + } else { + $exception = ([System.ArgumentException]"Invalid value for Parameter") + $errorId = 'ParameterValue.InvalidFields' + $errorCategory = 'InvalidArgument' + $errorTarget = $Fields + $errorItem = New-Object -TypeName System.Management.Automation.ErrorRecord $exception, $errorId, $errorCategory, $errorTarget + $errorItem.ErrorDetails = "Unable to identify field [$name] from -Fields hashtable. Use Get-JiraField for more information." + $PSCmdlet.ThrowTerminatingError($errorItem) + } + $id = $field.Id $requestBody["$id"] = $value } - else { - $exception = ([System.ArgumentException]"Invalid value for Parameter") - $errorId = 'ParameterValue.InvalidFields' - $errorCategory = 'InvalidArgument' - $errorTarget = $Fields - $errorItem = New-Object -TypeName System.Management.Automation.ErrorRecord $exception, $errorId, $errorCategory, $errorTarget - $errorItem.ErrorDetails = "Unable to identify field [$name] from -Fields hashtable. Use Get-JiraField for more information." - $PSCmdlet.ThrowTerminatingError($errorItem) - } } + Write-Verbose "[$($MyInvocation.MyCommand.Name)] Validating fields with metadata" foreach ($c in $createmeta) { Write-Debug "[$($MyInvocation.MyCommand.Name)] Checking metadata for `$c [$c]" diff --git a/JiraPS/Public/Set-JiraIssue.ps1 b/JiraPS/Public/Set-JiraIssue.ps1 index 33d6f633..bd6f4b93 100644 --- a/JiraPS/Public/Set-JiraIssue.ps1 +++ b/JiraPS/Public/Set-JiraIssue.ps1 @@ -156,17 +156,56 @@ function Set-JiraIssue { ) } + if ($Fields) { + + Write-Debug "[$($MyInvocation.MyCommand.Name)] Enumerating fields defined on the server" + + # Fetch all available fields ahead-of-time to avoid repeated API calls in the upcoming loop. + # Eventually, this may be better to extract from EditMeta. + $AvailableFields = Get-JiraField -Credential $Credential -ErrorAction Stop -Debug:$false + + $AvailableFieldsById = $AvailableFields | Group-Object -Property Id -AsHashTable -AsString + $AvailableFieldsByName = $AvailableFields | Group-Object -Property Name -AsHashTable -AsString + Write-Debug "[$($MyInvocation.MyCommand.Name)] Resolving `$Fields" foreach ($_key in $Fields.Keys) { + $name = $_key $value = $Fields.$_key - $field = Get-JiraField -Field $name -Credential $Credential -ErrorAction Stop + # The Fields hashtable supports both name- and ID-based lookup for custom fields, so we have to search both. + if ($AvailableFieldsById.ContainsKey($name)) { + $field = $AvailableFieldsById[$name][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a field ID" + } elseif ($AvailableFieldsById.ContainsKey("customfield_$name")) { + $field = $AvailableFieldsById["customfield_$name"][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a numerical field ID (customfield_$name)" + } elseif ($AvailableFieldsByName.ContainsKey($name) -and $AvailableFieldsByName[$name].Count -eq 1) { + $field = $AvailableFieldsByName[$name][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a human-readable field name ($($field.ID))" + } elseif ($AvailableFieldsByName.ContainsKey($name)) { + # Jira does not prevent multiple custom fields with the same name, so we have to ensure + # any name references are unambiguous. + + # More than one value in $AvailableFieldsByName (i.e. .Count -gt 1) indicates two duplicate custom fields. + $exception = ([System.ArgumentException]"Ambiguously Referenced Parameter") + $errorId = 'ParameterValue.AmbiguousParameter' + $errorCategory = 'InvalidArgument' + $errorTarget = $Fields + $errorItem = New-Object -TypeName System.Management.Automation.ErrorRecord $exception, $errorId, $errorCategory, $errorTarget + $errorItem.ErrorDetails = "Field name [$name] in -Fields hashtable ambiguously refers to more than one field. Use Get-JiraField for more information, or specify the custom field by its ID." + $PSCmdlet.ThrowTerminatingError($errorItem) + } else { + $exception = ([System.ArgumentException]"Invalid value for Parameter") + $errorId = 'ParameterValue.InvalidFields' + $errorCategory = 'InvalidArgument' + $errorTarget = $Fields + $errorItem = New-Object -TypeName System.Management.Automation.ErrorRecord $exception, $errorId, $errorCategory, $errorTarget + $errorItem.ErrorDetails = "Unable to identify field [$name] from -Fields hashtable. Use Get-JiraField for more information." + $PSCmdlet.ThrowTerminatingError($errorItem) + } - # For some reason, this was coming through as a hashtable instead of a String, - # which was causing ConvertTo-Json to crash later. - # Not sure why, but this forces $id to be a String and not a hashtable. $id = [string]$field.Id $issueProps.update[$id] = @(@{ 'set' = $value }) } diff --git a/Tests/Functions/New-JiraIssue.Unit.Tests.ps1 b/Tests/Functions/New-JiraIssue.Unit.Tests.ps1 index 6b76ae53..3b580c79 100644 --- a/Tests/Functions/New-JiraIssue.Unit.Tests.ps1 +++ b/Tests/Functions/New-JiraIssue.Unit.Tests.ps1 @@ -81,7 +81,20 @@ Describe "New-JiraIssue" -Tag 'Unit' { # This one needs to be able to output multiple objects Mock Get-JiraField { - $Field | % { + + $(If ($null -eq $Field) { + @( + 'Project' + 'IssueType' + 'Priority' + 'Summary' + 'Description' + 'Reporter' + 'CustomField' + ) + } Else { + $Field + }) | % { $object = [PSCustomObject] @{ 'Id' = $_ } diff --git a/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 b/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 index 1dd2cd00..c94b3268 100644 --- a/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 +++ b/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 @@ -58,6 +58,32 @@ Describe "Set-JiraIssue" -Tag 'Unit' { return $object } + Mock Get-JiraField { + + $(If ($null -eq $Field) { + @( + 'Project' + 'IssueType' + 'Priority' + 'Summary' + 'Description' + 'Reporter' + 'CustomField' + 'customfield_12345' + 'customfield_67890' + 'customfield_111222' + ) + } Else { + $Field + }) | % { + $object = [PSCustomObject] @{ + 'Id' = $_ + } + $object.PSObject.TypeNames.Insert(0, 'JiraPS.Field') + $object + } + } + Mock Resolve-JiraIssueObject -ModuleName JiraPS { Get-JiraIssue -Key $Issue } @@ -73,7 +99,7 @@ Describe "Set-JiraIssue" -Tag 'Unit' { # actually try to query a JIRA instance Mock Invoke-JiraMethod { ShowMockInfo 'Invoke-JiraMethod' 'Method', 'Uri' - throw "Unidentified call to Invoke-JiraMethod" + throw "Unidentified call ($Method $Uri) to Invoke-JiraMethod" } Context "Sanity checking" { @@ -146,12 +172,6 @@ Describe "Set-JiraIssue" -Tag 'Unit' { } It "Updates custom fields if provided to the -Fields parameter" { - Mock Get-JiraField { - [PSCustomObject] @{ - 'Name' = $Field - 'ID' = $Field - } - } { Set-JiraIssue -Issue TEST-001 -Fields @{'customfield_12345' = 'foo'; 'customfield_67890' = 'bar'; 'customfield_111222' = @(@{'value' = 'foobar'})} } | Should Not Throw Assert-MockCalled -CommandName Invoke-JiraMethod -ModuleName JiraPS -Times 1 -Scope It -ParameterFilter { $Method -eq 'Put' -and $URI -like "$jiraServer/rest/api/*/issue/12345" -and $Body -like '*customfield_12345*set*foo*' } Assert-MockCalled -CommandName Invoke-JiraMethod -ModuleName JiraPS -Times 1 -Scope It -ParameterFilter { $Method -eq 'Put' -and $URI -like "$jiraServer/rest/api/*/issue/12345" -and $Body -like '*customfield_67890*set*bar*' }