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.
This commit is contained in:
Howard Wolosky 2018-11-30 11:26:13 -08:00 коммит произвёл GitHub
Родитель 35b06ce168
Коммит 6cf344fb38
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
11 изменённых файлов: 315 добавлений и 28 удалений

Просмотреть файл

@ -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
}
}

Просмотреть файл

@ -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
{

Просмотреть файл

@ -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'

Просмотреть файл

@ -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'

Просмотреть файл

@ -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

Просмотреть файл

@ -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

Просмотреть файл

@ -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

Просмотреть файл

@ -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)"
}
}
}

Просмотреть файл

@ -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 = @(

4
PowerShellForGitHub.psm1 Normal file
Просмотреть файл

@ -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

267
Tests/GitHubCore.Tests.ps1 Normal file
Просмотреть файл

@ -0,0 +1,267 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.
<#
.Synopsis
Tests for GitHubCore.ps1 module
#>
[String] $root = Split-Path -Parent (Split-Path -Parent $Script:MyInvocation.MyCommand.Path)
. (Join-Path -Path $root -ChildPath 'Tests\Config\Settings.ps1')
Import-Module -Name $root -Force
function Initialize-AppVeyor
{
<#
.SYNOPSIS
Configures the tests to run with the authentication information stored in AppVeyor
(if that information exists in the environment).
.DESCRIPTION
Configures the tests to run with the authentication information stored in AppVeyor
(if that information exists in the environment).
The Git repo for this module can be found here: http://aka.ms/PowerShellForGitHub
.NOTES
Internal-only helper method.
The only reason this exists is so that we can leverage CodeAnalysis.SuppressMessageAttribute,
which can only be applied to functions.
We call this immediately after the declaration so that AppVeyor initialization can heppen
(if applicable).
#>
[CmdletBinding()]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingConvertToSecureStringWithPlainText", "", Justification="Needed to configure with the stored, encrypted string value in AppVeyor.")]
param()
if ($env:AppVeyor)
{
$secureString = $env:avAccessToken | ConvertTo-SecureString -AsPlainText -Force
$cred = New-Object System.Management.Automation.PSCredential "<username is ignored>", $secureString
Set-GitHubAuthentication -Credential $cred
$script:ownerName = $env:avOwnerName
$script:organizationName = $env:avOrganizationName
$message = @(
'This run is executed in the AppVeyor environment.',
'The GitHub Api Token won''t be decrypted in PR runs causing some tests to fail.',
'403 errors possible due to GitHub hourly limit for unauthenticated queries.',
'Use Set-GitHubAuthentication manually. modify the values in Tests\Config\Settings.ps1,',
'and run tests on your machine first.')
Write-Warning -Message ($message -join [Environment]::NewLine)
}
}
Initialize-AppVeyor
$script:accessTokenConfigured = Test-GitHubAuthenticationConfigured
if (-not $script:accessTokenConfigured)
{
$message = @(
'GitHub API Token not defined, some of the tests will be skipped.',
'403 errors possible due to GitHub hourly limit for unauthenticated queries.')
Write-Warning -Message ($message -join [Environment]::NewLine)
}
try
{
# Backup the user's configuration before we begin, and ensure we're at a pure state before running
# the tests. We'll restore it at the end.
$configFile = New-TemporaryFile
Backup-GitHubConfiguration -Path $configFile
Reset-GitHubConfiguration
Set-GitHubConfiguration -DisableTelemetry # We don't want UT's to impact telemetry
Describe 'Testing ConvertTo-SmarterObject behavior' {
InModuleScope PowerShellForGitHub {
$jsonConversionDepth = 20
Context 'When a property is a simple type' {
$original = [PSCustomObject]@{
'prop1' = 'value1'
'prop2' = 3
'prop3' = $null
}
$converted = ConvertTo-SmarterObject -InputObject $original
It 'Should return the same values' {
$originalJson = (ConvertTo-Json -InputObject $original -Depth $jsonConversionDepth)
$convertedJson = (ConvertTo-Json -InputObject $converted -Depth $jsonConversionDepth)
$originalJson -eq $convertedJson | Should be $true
}
}
Context 'When a property is a PSCustomObject' {
$original = [PSCustomObject]@{
'prop1' = [PSCustomObject]@{
'prop1' = 'value1'
'prop2' = 3
'prop3' = $null
}
'prop2' = 3
'prop3' = $null
}
$converted = ConvertTo-SmarterObject -InputObject $original
It 'Should return the correct values' {
$originalJson = (ConvertTo-Json -InputObject $original -Depth $jsonConversionDepth)
$convertedJson = (ConvertTo-Json -InputObject $converted -Depth $jsonConversionDepth)
$originalJson -eq $convertedJson | Should be $true
}
}
Context 'When a known date property has a date string' {
$date = Get-Date
$dateString = $date.ToUniversalTime().ToString('o')
$original = [PSCustomObject]@{
'prop1' = $dateString
'closed_at' = $dateString
'committed_at' = $dateString
'completed_at' = $dateString
'created_at' = $dateString
'date' = $dateString
'due_on' = $dateString
'last_edited_at' = $dateString
'last_read_at' = $dateString
'merged_at' = $dateString
'published_at' = $dateString
'pushed_at' = $dateString
'starred_at' = $dateString
'started_at' = $dateString
'submitted_at' = $dateString
'timestamp' = $dateString
'updated_at' = $dateString
}
$converted = ConvertTo-SmarterObject -InputObject $original
It 'Should convert the value to a [DateTime]' {
$converted.closed_at -is [DateTime] | Should be $true
$converted.committed_at -is [DateTime] | Should be $true
$converted.completed_at -is [DateTime] | Should be $true
$converted.created_at -is [DateTime] | Should be $true
$converted.date -is [DateTime] | Should be $true
$converted.due_on -is [DateTime] | Should be $true
$converted.last_edited_at -is [DateTime] | Should be $true
$converted.last_read_at -is [DateTime] | Should be $true
$converted.merged_at -is [DateTime] | Should be $true
$converted.published_at -is [DateTime] | Should be $true
$converted.pushed_at -is [DateTime] | Should be $true
$converted.starred_at -is [DateTime] | Should be $true
$converted.started_at -is [DateTime] | Should be $true
$converted.submitted_at -is [DateTime] | Should be $true
$converted.timestamp -is [DateTime] | Should be $true
$converted.updated_at -is [DateTime] | Should be $true
}
It 'Should NOT convert the value to a [DateTime] if it''s not a known property' {
$converted.prop1 -is [DateTime] | Should be $false
}
}
Context 'When a known date property has a null, empty or invalid date string' {
$original = [PSCustomObject]@{
'closed_at' = $null
'committed_at' = '123'
'completed_at' = ''
'created_at' = 123
'date' = $null
'due_on' = '123'
'last_edited_at' = ''
'last_read_at' = 123
'merged_at' = $null
'published_at' = '123'
'pushed_at' = ''
'starred_at' = 123
'started_at' = $null
'submitted_at' = '123'
'timestamp' = ''
'updated_at' = 123
}
$converted = ConvertTo-SmarterObject -InputObject $original
It 'Should keep the existing value' {
$original.closed_at -eq $converted.closed_at | Should be $true
$original.committed_at -eq $converted.committed_at | Should be $true
$original.completed_at -eq $converted.completed_at | Should be $true
$original.created_at -eq $converted.created_at | Should be $true
$original.date -eq $converted.date | Should be $true
$original.due_on -eq $converted.due_on | Should be $true
$original.last_edited_at -eq $converted.last_edited_at | Should be $true
$original.last_read_at -eq $converted.last_read_at | Should be $true
$original.merged_at -eq $converted.merged_at | Should be $true
$original.published_at -eq $converted.published_at | Should be $true
$original.pushed_at -eq $converted.pushed_at | Should be $true
$original.starred_at -eq $converted.starred_at | Should be $true
$original.started_at -eq $converted.started_at | Should be $true
$original.submitted_at -eq $converted.submitted_at | Should be $true
$original.timestamp -eq $converted.timestamp | Should be $true
$original.updated_at -eq $converted.updated_at | Should be $true
}
}
Context 'When an object has an empty array' {
$original = [PSCustomObject]@{
'prop1' = 'value1'
'prop2' = 3
'prop3' = @()
'prop4' = $null
}
$converted = ConvertTo-SmarterObject -InputObject $original
It 'Should still be an empty array after conversion' {
$originalJson = (ConvertTo-Json -InputObject $original -Depth $jsonConversionDepth)
$convertedJson = (ConvertTo-Json -InputObject $converted -Depth $jsonConversionDepth)
$originalJson -eq $convertedJson | Should be $true
}
}
Context 'When an object has a single item array' {
$original = [PSCustomObject]@{
'prop1' = 'value1'
'prop2' = 3
'prop3' = @(1)
'prop4' = $null
}
$converted = ConvertTo-SmarterObject -InputObject $original
It 'Should still be a single item array after conversion' {
$originalJson = (ConvertTo-Json -InputObject $original -Depth $jsonConversionDepth)
$convertedJson = (ConvertTo-Json -InputObject $converted -Depth $jsonConversionDepth)
$originalJson -eq $convertedJson | Should be $true
}
}
Context 'When an object has a multi-item array' {
$original = [PSCustomObject]@{
'prop1' = 'value1'
'prop2' = 3
'prop3' = @(1, 2)
'prop4' = $null
}
$converted = ConvertTo-SmarterObject -InputObject $original
It 'Should still be a multi item array after conversion' {
$originalJson = (ConvertTo-Json -InputObject $original -Depth $jsonConversionDepth)
$convertedJson = (ConvertTo-Json -InputObject $converted -Depth $jsonConversionDepth)
$originalJson -eq $convertedJson | Should be $true
}
}
}
}
}
finally
{
# Restore the user's configuration to its pre-test state
Restore-GitHubConfiguration -Path $configFile
}