Prevent ConvertTo-SmarterObject from flattening arrays (#56)

* Ensure that `ConvertTo-SmarterObject` does not cause any side-effects on `InputObject` * Ensure that `ConvertTo-SmarterObject` does not flatten any arrays * Switched all calls of `ConvertTo-Json` to not use pipelining in order to avoid PowerShell's array-flattening logic when piping the InputObject in, as opposed to when passing it as a parameter. * Ensured that we do best-effort Date conversion in `ConvertTo-SmarterObject` (a failed date conversion should never cause an exception/failure). * Used a workaround described in [Pester's Wiki](# https://github.com/pester/Pester/wiki/Testing-different-module-types) to force the module to be a `Script Module` instead of a `Manifest Module` so that `Mock` and `InModuleScope` (which lets you test private methods) work. * Added UT's for `ConvertTo-SmarterObject` core scenarios. Resolves PowerShell#55: ConvertTo-SmarterObject is flattening arrays Thanks to @danbelcher-MSFT for the assist on this one.
microsoft · Nov 30, 2018 · 6cf344f · 6cf344f
1 parent 35b06ce
commit 6cf344f
Show file tree

Hide file tree

Showing 11 changed files with 315 additions and 28 deletions.
diff --git a/GitHubConfiguration.ps1 b/GitHubConfiguration.ps1
@@ -309,8 +309,7 @@ function Save-GitHubConfiguration
     )
 
     $null = New-Item -Path $Path -Force
-    $Configuration |
-        ConvertTo-Json |
+    ConvertTo-Json -InputObject $Configuration |
         Set-Content -Path $Path -Force -ErrorAction SilentlyContinue -ErrorVariable ev
 
     if (($null -ne $ev) -and ($ev.Count -gt 0))
@@ -654,7 +653,7 @@ function Backup-GitHubConfiguration
     }
     else
     {
-        @{} | ConvertTo-Json | Set-Content -Path $Path -Force:$Force
+        ConvertTo-Json -InputObject @{} | Set-Content -Path $Path -Force:$Force
     }
 }
 

diff --git a/GitHubCore.ps1 b/GitHubCore.ps1
@@ -261,7 +261,7 @@ function Invoke-GHRestMethod
                             Write-Log -Message "Unable to retrieve the raw HTTP Web Response:" -Exception $_ -Level Warning
                         }
 
-                        throw ($ex | ConvertTo-Json -Depth 20)
+                        throw (ConvertTo-Json -InputObject $ex -Depth 20)
                     }
                 }
 
@@ -840,8 +840,12 @@ filter ConvertTo-SmarterObject
     .PARAMETER InputObject
         The object to update
 #>
+    [CmdletBinding()]
     param(
-        [Parameter(Mandatory)]
+        [Parameter(
+            Mandatory,
+            ValueFromPipeline,
+            ValueFromPipelineByPropertyName)]
         [AllowNull()]
         [object] $InputObject
     )
@@ -851,31 +855,44 @@ filter ConvertTo-SmarterObject
         return $null
     }
 
-    if ($InputObject -is [array])
+    if ($InputObject -is [System.Collections.IList])
     {
-        foreach ($object in $InputObject)
-        {
-            Write-Output -InputObject (ConvertTo-SmarterObject -InputObject $object)
-        }
+        $InputObject |
+            ConvertTo-SmarterObject |
+            Write-Output
     }
     elseif ($InputObject -is [PSCustomObject])
     {
-        $properties = $InputObject.PSObject.Properties | Where-Object { $null -ne $_.Value }
+        $clone = DeepCopy-Object -InputObject $InputObject
+        $properties = $clone.PSObject.Properties | Where-Object { $null -ne $_.Value }
         foreach ($property in $properties)
         {
             # Convert known date properties from dates to real DateTime objects
-            if ($property.Name -in $script:datePropertyNames)
+            if (($property.Name -in $script:datePropertyNames) -and
+                ($property.Value -is [String]) -and
+                (-not [String]::IsNullOrWhiteSpace($property.Value)))
             {
-                $property.Value = Get-Date -Date $property.Value
+                try
+                {
+                    $property.Value = Get-Date -Date $property.Value
+                }
+                catch
+                {
+                    Write-Log -Message "Unable to convert $($property.Name) value of $($property.Value) to a [DateTime] object.  Leaving as-is." -Level Verbose
+                }
             }
 
-            if (($property.Value -is [array]) -or ($property.Value -is [PSCustomObject]))
+            if ($property.Value -is [System.Collections.IList])
+            {
+                $property.Value = @(ConvertTo-SmarterObject -InputObject $property.Value)
+            }
+            elseif ($property.Value -is [PSCustomObject])
             {
                 $property.Value = ConvertTo-SmarterObject -InputObject $property.Value
             }
         }
 
-        Write-Output -InputObject $InputObject
+        Write-Output -InputObject $clone
     }
     else
     {

diff --git a/GitHubIssues.ps1 b/GitHubIssues.ps1
@@ -514,7 +514,7 @@ function New-GitHubIssue
 
     $params = @{
         'UriFragment' = "/repos/$OwnerName/$RepositoryName/issues"
-        'Body' = ($hashBody | ConvertTo-Json)
+        'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Method' = 'Post'
         'Description' =  "Creating new Issue ""$Title"" on $RepositoryName"
         'AcceptHeader' = 'application/vnd.github.symmetra-preview+json'
@@ -654,7 +654,7 @@ function Update-GitHubIssue
 
     $params = @{
         'UriFragment' = "/repos/$OwnerName/$RepositoryName/issues/$Issue"
-        'Body' = ($hashBody | ConvertTo-Json)
+        'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Method' = 'Patch'
         'Description' =  "Updating Issue #$Issue on $RepositoryName"
         'AcceptHeader' = 'application/vnd.github.symmetra-preview+json'
@@ -760,7 +760,7 @@ function Lock-GitHubIssue
 
     $params = @{
         'UriFragment' = "/repos/$OwnerName/$RepositoryName/issues/$Issue/lock"
-        'Body' = ($hashBody | ConvertTo-Json)
+        'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Method' = 'Put'
         'Description' =  "Locking Issue #$Issue on $RepositoryName"
         'AcceptHeader' = 'application/vnd.github.sailor-v-preview+json'

diff --git a/GitHubLabels.ps1 b/GitHubLabels.ps1
@@ -210,7 +210,7 @@ function New-GitHubLabel
 
     $params = @{
         'UriFragment' = "repos/$OwnerName/$RepositoryName/labels"
-        'Body' = ($hashBody | ConvertTo-Json)
+        'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Method' = 'Post'
         'Description' =  "Creating label $Name in $RepositoryName"
         'AcceptHeader' = 'application/vnd.github.symmetra-preview+json'
@@ -425,7 +425,7 @@ function Update-GitHubLabel
 
     $params = @{
         'UriFragment' = "repos/$OwnerName/$RepositoryName/labels/$Name"
-        'Body' = ($hashBody | ConvertTo-Json)
+        'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Method' = 'Patch'
         'Description' =  "Updating label $Name"
         'AcceptHeader' = 'application/vnd.github.symmetra-preview+json'

diff --git a/GitHubMiscellaneous.ps1 b/GitHubMiscellaneous.ps1
@@ -150,7 +150,7 @@ function ConvertFrom-Markdown
 
     $params = @{
         'UriFragment' = 'markdown'
-        'Body' = ($hashBody | ConvertTo-Json)
+        'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Method' = 'Post'
         'Description' =  "Converting Markdown to HTML"
         'AccessToken' = $AccessToken

diff --git a/GitHubRepositories.ps1 b/GitHubRepositories.ps1
@@ -166,7 +166,7 @@ function New-GitHubRepository
 
     $params = @{
         'UriFragment' = $uriFragment
-        'Body' = ($hashBody | ConvertTo-Json)
+        'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Method' = 'Post'
         'Description' =  "Creating $RepositoryName"
         'AccessToken' = $AccessToken
@@ -609,7 +609,7 @@ function Update-GitHubRepository
 
     $params = @{
         'UriFragment' = "repos/$OwnerName/$ReposistoryName"
-        'Body' = ($hashBody | ConvertTo-Json)
+        'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Method' = 'Patch'
         'Description' =  "Updating $RepositoryName"
         'AccessToken' = $AccessToken
@@ -817,7 +817,7 @@ function Set-GitHubRepositoryTopic
 
     $params = @{
         'UriFragment' = "repos/$OwnerName/$RepositoryName/topics"
-        'Body' = ($hashBody | ConvertTo-Json)
+        'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Method' = 'Put'
         'Description' =  $description
         'AcceptHeader' = 'application/vnd.github.mercy-preview+json'
@@ -1299,7 +1299,7 @@ function Move-GitHubRepositoryOwnership
 
     $params = @{
         'UriFragment' = "repos/$OwnerName/$RepositoryName/transfer"
-        'Body' = ($hashBody | ConvertTo-Json)
+        'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Method' = 'Post'
         'Description' =  "Transferring ownership of $RepositoryName to $NewOwnerName"
         'AccessToken' = $AccessToken

diff --git a/GitHubUsers.ps1 b/GitHubUsers.ps1
@@ -259,7 +259,7 @@ function Update-GitHubCurrentUser
     $params = @{
         'UriFragment' = 'user'
         'Method' = 'Patch'
-        'Body' = ($hashBody | ConvertTo-Json)
+        'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Description' =  "Updating current authenticated user"
         'AccessToken' = $AccessToken
         'TelemetryEventName' = $MyInvocation.MyCommand.Name

diff --git a/Helpers.ps1 b/Helpers.ps1
@@ -467,7 +467,7 @@ function Write-InvocationLog
             }
             else
             {
-                $params += "-$($param.Key) $($param.Value | ConvertTo-Json -Depth $jsonConversionDepth -Compress)"
+                $params += "-$($param.Key) $(ConvertTo-Json -InputObject $param.Value -Depth $jsonConversionDepth -Compress)"
             }
         }
     }

diff --git a/PowerShellForGitHub.psd1 b/PowerShellForGitHub.psd1
@@ -11,7 +11,7 @@
     Description = 'PowerShell wrapper for GitHub API'
 
     # Script module or binary module file associated with this manifest.
-    # RootModule = 'GitHubCore.psm1'
+    RootModule = 'PowerShellForGitHub.psm1'
 
     # Modules to import as nested modules of the module specified in RootModule/ModuleToProcess
     NestedModules = @(

diff --git a/PowerShellForGitHub.psm1 b/PowerShellForGitHub.psm1
@@ -0,0 +1,4 @@
+# This file only exists to enable making this a "Script" module instead of a "Manifest" module
+# so that Pester tests are able to use Pester's Mock and InModuleScope features for internal
+# methods.  For more information, refer to:
+# https://github.com/pester/Pester/wiki/Testing-different-module-types
-Original file line number
+Diff line change
@@ Expand Up / @@ -467,7 +467,7 @@ function Write-InvocationLog @@
                 }
                 else
                 {
-                    $params += "-$($param.Key) $($param.Value | ConvertTo-Json -Depth $jsonConversionDepth -Compress)"
+                    $params += "-$($param.Key) $(ConvertTo-Json -InputObject $param.Value -Depth $jsonConversionDepth -Compress)"
                 }
             }
         }
@@ Expand Down @@