From d06de27b535537327b5106c0724a4ec02bc0c2f5 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Fri, 6 Oct 2017 20:51:47 +0100 Subject: [PATCH 01/13] Add -AsHashtable switch to ConvertFrom-Json for issue 3623 --- .../Common/InvokeRestMethodCommand.Common.cs | 2 +- .../WebCmdlet/ConvertFromJsonCommand.cs | 14 +- .../commands/utility/WebCmdlet/JsonObject.cs | 120 +++++++++++++++++- .../ConvertFrom-Json.Tests.ps1 | 29 ++++- .../JsonObject.Tests.ps1 | 47 +++---- 5 files changed, 166 insertions(+), 46 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs index e9a73b46970..fda13ace9ed 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs @@ -171,7 +171,7 @@ private bool TryConvertToJson(string json, out object obj, ref Exception exRef) try { ErrorRecord error; - obj = JsonObject.ConvertFromJson(json, out error); + obj = JsonObject.ConvertFromJson(json, false, out error); if (error != null) { diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs index 4057ea82a3a..0ff72b5e77b 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs @@ -20,17 +20,23 @@ public class ConvertFromJsonCommand : Cmdlet #region parameters /// - /// gets or sets the InputString property + /// Gets or sets the InputString property. /// [Parameter(Mandatory = true, Position = 0, ValueFromPipeline = true)] [AllowEmptyString] public string InputObject { get; set; } /// - /// inputObjectBuffer buffers all InputObjet contents available in the pipeline. + /// InputObjectBuffer buffers all InputObject contents available in the pipeline. /// private List _inputObjectBuffer = new List(); + /// + /// Returned data structure is a Hashtable instead a CustomPSObject. + /// + [Parameter(Mandatory = false)] + public SwitchParameter AsHashtable { get; set; } + #endregion parameters #region overrides @@ -44,7 +50,7 @@ protected override void ProcessRecord() } /// - /// the main execution method for the convertfrom-json command + /// The main execution method for the ConvertFrom-Json command. /// protected override void EndProcessing() { @@ -95,7 +101,7 @@ protected override void EndProcessing() private bool ConvertFromJsonHelper(string input) { ErrorRecord error = null; - object result = JsonObject.ConvertFromJson(input, out error); + object result = JsonObject.ConvertFromJson(input, AsHashtable.IsPresent, out error); if (error != null) { diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs index 215fec32829..97695055f91 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs @@ -10,6 +10,7 @@ using System.Text.RegularExpressions; using Newtonsoft.Json; using Newtonsoft.Json.Linq; +using System.Collections; using System.Collections.ObjectModel; using System.IO; using System.Management.Automation.Internal; @@ -26,13 +27,14 @@ public static class JsonObject private const int maxDepthAllowed = 100; /// - /// Convert a Json string back to an object + /// Convert a Json string back to an object of type PSObject or Hashtable depending on parameter . /// /// + /// /// - /// + /// A PSObject or a Hashtable if the parameter is true. [SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly")] - public static object ConvertFromJson(string input, out ErrorRecord error) + public static object ConvertFromJson(string input, bool returnHashTable, out ErrorRecord error) { if (input == null) { @@ -65,7 +67,14 @@ public static object ConvertFromJson(string input, out ErrorRecord error) var dictionary = obj as JObject; if (dictionary != null) { - obj = PopulateFromJDictionary(dictionary, out error); + if (returnHashTable) + { + obj = PopulateHashTableFromJDictionary(dictionary, out error); + } + else + { + obj = PopulateFromJDictionary(dictionary, out error); + } } else { @@ -73,7 +82,14 @@ public static object ConvertFromJson(string input, out ErrorRecord error) var list = obj as JArray; if (list != null) { - obj = PopulateFromJArray(list, out error); + if (returnHashTable) + { + obj = PopulateHashTableFromJArray(list, out error); + } + else + { + obj = PopulateFromJArray(list, out error); + } } } } @@ -180,5 +196,99 @@ private static ICollection PopulateFromJArray(JArray list, out ErrorReco } return result.ToArray(); } + + // This function is a clone of PopulateFromDictionary using JObject as an input. + private static Hashtable PopulateHashTableFromJDictionary(JObject entries, out ErrorRecord error) + { + error = null; + Hashtable result = new Hashtable(); + foreach (var entry in entries) + { + if (result.ContainsKey(entry.Key)) + { + string errorMsg = string.Format(CultureInfo.InvariantCulture, + WebCmdletStrings.DuplicateKeysInJsonString, entry.Key, entry.Key); + error = new ErrorRecord( + new InvalidOperationException(errorMsg), + "DuplicateKeysInJsonString", + ErrorCategory.InvalidOperation, + null); + return null; + } + + // Array + else if (entry.Value is JArray) + { + JArray list = entry.Value as JArray; + ICollection listResult = PopulateHashTableFromJArray(list, out error); + if (error != null) + { + return null; + } + result.Add(entry.Key, listResult); + } + + // Dictionary + else if (entry.Value is JObject) + { + JObject dic = entry.Value as JObject; + Hashtable dicResult = PopulateHashTableFromJDictionary(dic, out error); + if (error != null) + { + return null; + } + result.Add(entry.Key, dicResult); + } + + // Value + else // (entry.Value is JValue) + { + JValue theValue = entry.Value as JValue; + result.Add(entry.Key, theValue.Value); + } + } + return result; + } + + // This function is a clone of PopulateFromList using JArray as input. + private static ICollection PopulateHashTableFromJArray(JArray list, out ErrorRecord error) + { + error = null; + List result = new List(); + + foreach (var element in list) + { + // Array + if (element is JArray) + { + JArray subList = element as JArray; + ICollection listResult = PopulateHashTableFromJArray(subList, out error); + if (error != null) + { + return null; + } + result.Add(listResult); + } + + // Dictionary + else if (element is JObject) + { + JObject dic = element as JObject; + Hashtable dicResult = PopulateHashTableFromJDictionary(dic, out error); + if (error != null) + { + return null; + } + result.Add(dicResult); + } + + // Value + else // (element is JValue) + { + result.Add(((JValue)element).Value); + } + } + return result.ToArray(); + } } } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertFrom-Json.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertFrom-Json.Tests.ps1 index 985cf608ec4..eada7a785b6 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertFrom-Json.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertFrom-Json.Tests.ps1 @@ -1,16 +1,33 @@ Describe 'ConvertFrom-Json' -tags "CI" { - It 'can convert a single-line object' { - ('{"a" : "1"}' | ConvertFrom-Json).a | Should Be 1 + + $testCasesWithAndWithoutAsHashtableSwitch = @( + @{ AsHashtable = $true } + @{ AsHashtable = $false } + ) + + It 'can convert a single-line object with AsHashtable switch set to ' -TestCase $testCasesWithAndWithoutAsHashtableSwitch { + Param($AsHashtable) + ('{"a" : "1"}' | ConvertFrom-Json -AsHashtable:$AsHashtable).a | Should Be 1 } - It 'can convert one string-per-object' { - $json = @('{"a" : "1"}', '{"a" : "x"}') | ConvertFrom-Json + It 'can convert one string-per-object with AsHashtable switch set to ' -TestCase $testCasesWithAndWithoutAsHashtableSwitch { + Param($AsHashtable) + $json = @('{"a" : "1"}', '{"a" : "x"}') | ConvertFrom-Json -AsHashtable:$AsHashtable $json.Count | Should Be 2 $json[1].a | Should Be 'x' + if ($AsHashtable) + { + $json | Should BeOfType Hashtable + } } - It 'can convert multi-line object' { - $json = @('{"a" :', '"x"}') | ConvertFrom-Json + It 'can convert multi-line object with AsHashtable switch set to ' -TestCase $testCasesWithAndWithoutAsHashtableSwitch { + Param($AsHashtable) + $json = @('{"a" :', '"x"}') | ConvertFrom-Json -AsHashtable:$AsHashtable $json.a | Should Be 'x' + if ($AsHashtable) + { + $json | Should BeOfType Hashtable + } } } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 index 525692a0779..941aed3f832 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 @@ -1,46 +1,33 @@ Describe 'Unit tests for JsonObject' -tags "CI" { - function ShouldThrow - { - param ( - [Parameter(ValueFromPipeline = $true)] - $InputObject, - [Parameter(Position = 0)] - $ExpectedException - ) - - try - { - & $InputObject - throw "Should throw exception" - } - catch - { - $_.FullyQualifiedErrorId | should be $ExpectedException - } - } + $TestCasesForReturnHashTableParameter = @($true, $false) $validStrings = @( - @{ name = "empty"; str = "" } - @{ name = "spaces"; str = " " } - @{ name = "object"; str = "{a:1}" } + @{ name = "empty"; str = ""; ReturnHashTable = $true } + @{ name = "spaces"; str = " "; ReturnHashTable = $true } + @{ name = "object"; str = "{a:1}"; ReturnHashTable = $true } + @{ name = "empty"; str = ""; ReturnHashTable = $false } + @{ name = "spaces"; str = " "; ReturnHashTable = $false } + @{ name = "object"; str = "{a:1}"; ReturnHashTable = $false } ) - It 'no error for valid string - ' -TestCase $validStrings { - param ($str) + It 'no error for valid string - when ReturnHashTable is ' -TestCase $validStrings { + param ($str, $ReturnHashTable) $errRecord = $null - [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($str, [ref]$errRecord) + [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($str, $ReturnHashTable, [ref]$errRecord) $errRecord | Should BeNullOrEmpty } $invalidStrings = @( - @{ name = "plain text"; str = "plaintext" } - @{ name = "part"; str = '{"a" :' } + @{ name = "plain text"; str = "plaintext"; ReturnHashTable = $true } + @{ name = "part"; str = '{"a" :'; ReturnHashTable = $true } + @{ name = "plain text"; str = "plaintext"; ReturnHashTable = $false } + @{ name = "part"; str = '{"a" :'; ReturnHashTable = $false } ) - It 'throw ArgumentException for invalid string - ' -TestCase $invalidStrings { - param ($str) + It 'throw ArgumentException for invalid string - when ReturnHashTable is ' -TestCase $invalidStrings { + param ($str, $ReturnHashTable) $errRecord = $null - { [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($str, [ref]$errRecord) } | ShouldThrow 'ArgumentException' + { [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($str, $ReturnHashTable, [ref]$errRecord) } | ShouldBeErrorId "ArgumentException" } } From b8dd5b60fa817d6a32cb5b2ccdbfa74bad0f8e41 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 9 Oct 2017 22:13:01 +0100 Subject: [PATCH 02/13] Keep old ConvertFromJson API for backwards compatibility and address minor suggestions in test naming --- .../commands/utility/WebCmdlet/JsonObject.cs | 12 ++++++++++++ .../JsonObject.Tests.ps1 | 6 ++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs index 97695055f91..f7ca6715e92 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs @@ -26,6 +26,18 @@ public static class JsonObject { private const int maxDepthAllowed = 100; + /// + /// Convert a Json string back to an object of type PSObject. + /// + /// + /// + /// A PSObject. + [SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly")] + public static object ConvertFromJson(string input, out ErrorRecord error) + { + return ConvertFromJson(input, false, out error); + } + /// /// Convert a Json string back to an object of type PSObject or Hashtable depending on parameter . /// diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 index 941aed3f832..e96ba461495 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 @@ -1,7 +1,5 @@ Describe 'Unit tests for JsonObject' -tags "CI" { - $TestCasesForReturnHashTableParameter = @($true, $false) - $validStrings = @( @{ name = "empty"; str = ""; ReturnHashTable = $true } @{ name = "spaces"; str = " "; ReturnHashTable = $true } @@ -11,7 +9,7 @@ @{ name = "object"; str = "{a:1}"; ReturnHashTable = $false } ) - It 'no error for valid string - when ReturnHashTable is ' -TestCase $validStrings { + It 'no error for valid string '''' with -ReturnHashTable:$' -TestCase $validStrings { param ($str, $ReturnHashTable) $errRecord = $null [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($str, $ReturnHashTable, [ref]$errRecord) @@ -25,7 +23,7 @@ @{ name = "part"; str = '{"a" :'; ReturnHashTable = $false } ) - It 'throw ArgumentException for invalid string - when ReturnHashTable is ' -TestCase $invalidStrings { + It 'throw ArgumentException for invalid string '''' with -ReturnHashTable:$' -TestCase $invalidStrings { param ($str, $ReturnHashTable) $errRecord = $null { [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($str, $ReturnHashTable, [ref]$errRecord) } | ShouldBeErrorId "ArgumentException" From a709c29b863b818b7ab76ff27f2e04805550aa0c Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 9 Oct 2017 22:49:44 +0100 Subject: [PATCH 03/13] Remove redundant '[Parameter(Mandatory = false)]' on -AsHashtable switch since the parameter is not mandatory by default --- .../commands/utility/WebCmdlet/ConvertFromJsonCommand.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs index 0ff72b5e77b..9646c1d81d0 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs @@ -34,7 +34,6 @@ public class ConvertFromJsonCommand : Cmdlet /// /// Returned data structure is a Hashtable instead a CustomPSObject. /// - [Parameter(Mandatory = false)] public SwitchParameter AsHashtable { get; set; } #endregion parameters From e1fe61987a33bc9848f9eeb4252c7fb40f302cd3 Mon Sep 17 00:00:00 2001 From: bergmeister Date: Mon, 9 Oct 2017 23:16:03 +0100 Subject: [PATCH 04/13] Last commit accidentally removed entirely [Parameter()] line, it should have been only the Mandatory=false part that should be removed. --- .../commands/utility/WebCmdlet/ConvertFromJsonCommand.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs index 9646c1d81d0..ebf9ada8a21 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs @@ -34,6 +34,7 @@ public class ConvertFromJsonCommand : Cmdlet /// /// Returned data structure is a Hashtable instead a CustomPSObject. /// + [Parameter()] public SwitchParameter AsHashtable { get; set; } #endregion parameters From c33412837fd0c8631d88411a1f815002a8831aff Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 10 Oct 2017 19:23:45 +0100 Subject: [PATCH 05/13] -Revert change in InvokeRestMethodCommand originally made in this PR that is not necessary any more since the original method signature has been restored. -Improve test to add case for $null --- .../utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs | 2 +- .../Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs index fda13ace9ed..e9a73b46970 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs @@ -171,7 +171,7 @@ private bool TryConvertToJson(string json, out object obj, ref Exception exRef) try { ErrorRecord error; - obj = JsonObject.ConvertFromJson(json, false, out error); + obj = JsonObject.ConvertFromJson(json, out error); if (error != null) { diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 index e96ba461495..b042319b125 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 @@ -1,9 +1,11 @@ Describe 'Unit tests for JsonObject' -tags "CI" { $validStrings = @( + @{ name = "null"; str = $null; ReturnHashTable = $true } @{ name = "empty"; str = ""; ReturnHashTable = $true } @{ name = "spaces"; str = " "; ReturnHashTable = $true } @{ name = "object"; str = "{a:1}"; ReturnHashTable = $true } + @{ name = "null"; str = $null; ReturnHashTable = $false } @{ name = "empty"; str = ""; ReturnHashTable = $false } @{ name = "spaces"; str = " "; ReturnHashTable = $false } @{ name = "object"; str = "{a:1}"; ReturnHashTable = $false } From 0d5a6fbd71c31043d0eda82f7b0e2d4fb0431322 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 17 Oct 2017 23:31:10 +0100 Subject: [PATCH 06/13] Detect if key is empty string to resolve 1755 issue by redirecting to -AsHashTable via error message. Fixed a typo that I found in the message strings. --- .../WebCmdlet/Common/WebRequestPSCmdlet.Common.cs | 2 +- .../commands/utility/WebCmdlet/JsonObject.cs | 12 ++++++++++++ .../resources/WebCmdletStrings.resx | 5 ++++- .../JsonObject.Tests.ps1 | 14 ++++++++++++++ 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs index e9eb8f77e56..73e23242d10 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs @@ -335,7 +335,7 @@ internal virtual void ValidateParameters() { if (Directory.Exists(providerPaths[0])) { - errorRecord = GetValidationError(WebCmdletStrings.DirecotryPathSpecified, + errorRecord = GetValidationError(WebCmdletStrings.DirectoryPathSpecified, "WebCmdletInFileNotFilePathException", InFile); } _originalFilePath = InFile; diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs index f7ca6715e92..bfbefd81b57 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs @@ -121,6 +121,18 @@ private static PSObject PopulateFromJDictionary(JObject entries, out ErrorRecord PSObject result = new PSObject(); foreach (var entry in entries) { + if (string.IsNullOrEmpty(entry.Key)) + { + string errorMsg = string.Format(CultureInfo.InvariantCulture, + WebCmdletStrings.EmptyKeyInJsonString); + error = new ErrorRecord( + new InvalidOperationException(errorMsg), + "EmptyKeyInJsonString", + ErrorCategory.InvalidOperation, + null); + return null; + } + PSPropertyInfo property = result.Properties[entry.Key]; if (property != null) { diff --git a/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx b/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx index d1fde8e4cfa..342a3f11e43 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx +++ b/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx @@ -126,9 +126,12 @@ The cmdlet cannot run because the following conflicting parameters are specified: Credential and UseDefaultCredentials. Specify either Credential or UseDefaultCredentials, then retry. - + Path '{0}' resolves to a directory. Specify a path including a file name, and then retry the command. + + The provided json includes a property whose name is an empty string, this is only supported using the -AsHashTable switch. + Cannot convert the JSON string because a dictionary that was converted from the string contains the duplicated keys '{0}' and '{1}'. diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 index b042319b125..2b02f07f258 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 @@ -30,4 +30,18 @@ $errRecord = $null { [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($str, $ReturnHashTable, [ref]$errRecord) } | ShouldBeErrorId "ArgumentException" } + + $jsonWithEmptyKey = '{"": "Value"}' + It 'throw InvalidOperationException for string ''jsonWithEmptyKey'' with empty key name' { + $errorRecord = $null + [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, [ref]$errorRecord) + $errorRecord.Exception | Should Be InvalidOperationException + $errorRecord.FullyQualifiedErrorId | Should Be EmptyKeyInJsonString + } + + It 'not throw for string ''jsonWithEmptyKey'' with empty key name when returnHashTable is used' { + $errorRecord = $null + $result = [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, $true, [ref]$errorRecord) + $result | Should Not Be $null + } } From af440722d1d6580e1db0a9697a84787fc96c6081 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 17 Oct 2017 23:56:05 +0100 Subject: [PATCH 07/13] Fix test syntax and simplify test name --- .../Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 index 2b02f07f258..0e77b249dc0 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 @@ -32,14 +32,14 @@ } $jsonWithEmptyKey = '{"": "Value"}' - It 'throw InvalidOperationException for string ''jsonWithEmptyKey'' with empty key name' { + It 'throw InvalidOperationException when json contains empty key name' { $errorRecord = $null [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, [ref]$errorRecord) - $errorRecord.Exception | Should Be InvalidOperationException + $errorRecord.Exception.GetType() | Should Be System.InvalidOperationException $errorRecord.FullyQualifiedErrorId | Should Be EmptyKeyInJsonString } - It 'not throw for string ''jsonWithEmptyKey'' with empty key name when returnHashTable is used' { + It 'not throw when json contains empty key na' { $errorRecord = $null $result = [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, $true, [ref]$errorRecord) $result | Should Not Be $null From 123128cfbd62117532bb878e8a82f443e2df10a9 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 17 Oct 2017 23:57:28 +0100 Subject: [PATCH 08/13] make 'json' uppercase 'JSON' in error message as suggested in PR review. --- .../resources/WebCmdletStrings.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx b/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx index 342a3f11e43..1aef1029f0e 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx +++ b/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx @@ -130,7 +130,7 @@ Path '{0}' resolves to a directory. Specify a path including a file name, and then retry the command. - The provided json includes a property whose name is an empty string, this is only supported using the -AsHashTable switch. + The provided JSON includes a property whose name is an empty string, this is only supported using the -AsHashTable switch. Cannot convert the JSON string because a dictionary that was converted from the string contains the duplicated keys '{0}' and '{1}'. From 69df716520194470fb692b6ddfe3060f51e108c9 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 18 Oct 2017 18:52:51 +0100 Subject: [PATCH 09/13] Address PR comments about tests (mostly style) by @iSazonov --- .../ConvertFrom-Json.Tests.ps1 | 17 +++++++----- .../JsonObject.Tests.ps1 | 27 +++++++++---------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertFrom-Json.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertFrom-Json.Tests.ps1 index eada7a785b6..a13c3067883 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertFrom-Json.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertFrom-Json.Tests.ps1 @@ -1,16 +1,19 @@ Describe 'ConvertFrom-Json' -tags "CI" { - $testCasesWithAndWithoutAsHashtableSwitch = @( - @{ AsHashtable = $true } - @{ AsHashtable = $false } - ) + BeforeAll { + $testCasesWithAndWithoutAsHashtableSwitch = @( + @{ AsHashtable = $true } + @{ AsHashtable = $false } + ) + } + - It 'can convert a single-line object with AsHashtable switch set to ' -TestCase $testCasesWithAndWithoutAsHashtableSwitch { + It 'Can convert a single-line object with AsHashtable switch set to ' -TestCase $testCasesWithAndWithoutAsHashtableSwitch { Param($AsHashtable) ('{"a" : "1"}' | ConvertFrom-Json -AsHashtable:$AsHashtable).a | Should Be 1 } - It 'can convert one string-per-object with AsHashtable switch set to ' -TestCase $testCasesWithAndWithoutAsHashtableSwitch { + It 'Can convert one string-per-object with AsHashtable switch set to ' -TestCase $testCasesWithAndWithoutAsHashtableSwitch { Param($AsHashtable) $json = @('{"a" : "1"}', '{"a" : "x"}') | ConvertFrom-Json -AsHashtable:$AsHashtable $json.Count | Should Be 2 @@ -21,7 +24,7 @@ Describe 'ConvertFrom-Json' -tags "CI" { } } - It 'can convert multi-line object with AsHashtable switch set to ' -TestCase $testCasesWithAndWithoutAsHashtableSwitch { + It 'Can convert multi-line object with AsHashtable switch set to ' -TestCase $testCasesWithAndWithoutAsHashtableSwitch { Param($AsHashtable) $json = @('{"a" :', '"x"}') | ConvertFrom-Json -AsHashtable:$AsHashtable $json.a | Should Be 'x' diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 index 0e77b249dc0..820523f440a 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 @@ -1,6 +1,11 @@ Describe 'Unit tests for JsonObject' -tags "CI" { - $validStrings = @( + BeforeAll { + $jsonWithEmptyKey = '{"": "Value"}' + } + + + It 'No error for valid string '''' with -ReturnHashTable:$' -TestCase @( @{ name = "null"; str = $null; ReturnHashTable = $true } @{ name = "empty"; str = ""; ReturnHashTable = $true } @{ name = "spaces"; str = " "; ReturnHashTable = $true } @@ -9,37 +14,31 @@ @{ name = "empty"; str = ""; ReturnHashTable = $false } @{ name = "spaces"; str = " "; ReturnHashTable = $false } @{ name = "object"; str = "{a:1}"; ReturnHashTable = $false } - ) - - It 'no error for valid string '''' with -ReturnHashTable:$' -TestCase $validStrings { + ) { param ($str, $ReturnHashTable) $errRecord = $null [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($str, $ReturnHashTable, [ref]$errRecord) $errRecord | Should BeNullOrEmpty } - $invalidStrings = @( + It 'Throw ArgumentException for invalid string '''' with -ReturnHashTable:$' -TestCase @( @{ name = "plain text"; str = "plaintext"; ReturnHashTable = $true } @{ name = "part"; str = '{"a" :'; ReturnHashTable = $true } @{ name = "plain text"; str = "plaintext"; ReturnHashTable = $false } @{ name = "part"; str = '{"a" :'; ReturnHashTable = $false } - ) - - It 'throw ArgumentException for invalid string '''' with -ReturnHashTable:$' -TestCase $invalidStrings { + ) { param ($str, $ReturnHashTable) $errRecord = $null { [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($str, $ReturnHashTable, [ref]$errRecord) } | ShouldBeErrorId "ArgumentException" } - - $jsonWithEmptyKey = '{"": "Value"}' - It 'throw InvalidOperationException when json contains empty key name' { + + It 'Throw InvalidOperationException when json contains empty key name' { $errorRecord = $null [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, [ref]$errorRecord) - $errorRecord.Exception.GetType() | Should Be System.InvalidOperationException - $errorRecord.FullyQualifiedErrorId | Should Be EmptyKeyInJsonString + $errorRecord.FullyQualifiedErrorId | Should Be 'EmptyKeyInJsonString' } - It 'not throw when json contains empty key na' { + It 'Not throw when json contains empty key na' { $errorRecord = $null $result = [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, $true, [ref]$errorRecord) $result | Should Not Be $null From 7d77ae21007c37ce041c72368557755491327e6a Mon Sep 17 00:00:00 2001 From: bergmeister Date: Thu, 19 Oct 2017 07:32:02 +0100 Subject: [PATCH 10/13] Update JsonObject.Tests.ps1 Removed extra newline in pester test as requested by @iSazonov --- .../Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 | 1 - 1 file changed, 1 deletion(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 index 820523f440a..ca98c03c029 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 @@ -4,7 +4,6 @@ $jsonWithEmptyKey = '{"": "Value"}' } - It 'No error for valid string '''' with -ReturnHashTable:$' -TestCase @( @{ name = "null"; str = $null; ReturnHashTable = $true } @{ name = "empty"; str = ""; ReturnHashTable = $true } From db0d6ceb2a78664be90a80e32783df196e476c0f Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 22 Oct 2017 16:10:35 +0100 Subject: [PATCH 11/13] Report error when keys have different casing. As part of this I also found out that newtonsoft.json would squash keys that have the same casing into one and just use the last value. I still handle this case should the behaviour change. --- .../commands/utility/WebCmdlet/JsonObject.cs | 25 +++++++++++-- .../resources/WebCmdletStrings.resx | 5 ++- .../JsonObject.Tests.ps1 | 36 ++++++++++++++----- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs index bfbefd81b57..68fc2a4f57a 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs @@ -13,6 +13,7 @@ using System.Collections; using System.Collections.ObjectModel; using System.IO; +using System.Linq; using System.Management.Automation.Internal; using System.Reflection; @@ -133,14 +134,30 @@ private static PSObject PopulateFromJDictionary(JObject entries, out ErrorRecord return null; } + // Case sensitive duplicates should normally not occur since JsonConvert.DeserializeObject + // does not throw when encountering duplicates and just uses the last entry. + if (result.Properties.Any(psPropertyInfo => psPropertyInfo.Name.Equals(entry.Key, StringComparison.InvariantCulture))) + { + string errorMsg = string.Format(CultureInfo.InvariantCulture, + WebCmdletStrings.DuplicateKeysInJsonString, entry.Key); + error = new ErrorRecord( + new InvalidOperationException(errorMsg), + "DuplicateKeysInJsonString", + ErrorCategory.InvalidOperation, + null); + return null; + } + + // Compare case insensitive to tell the user to use the -AsHashTable option instead. + // This is because PSObject cannot have keys with different casing. PSPropertyInfo property = result.Properties[entry.Key]; if (property != null) { string errorMsg = string.Format(CultureInfo.InvariantCulture, - WebCmdletStrings.DuplicateKeysInJsonString, property.Name, entry.Key); + WebCmdletStrings.KeysWithDifferentCasingInJsonString, property.Name, entry.Key); error = new ErrorRecord( new InvalidOperationException(errorMsg), - "DuplicateKeysInJsonString", + "KeysWithDifferentCasingInJsonString", ErrorCategory.InvalidOperation, null); return null; @@ -228,10 +245,12 @@ private static Hashtable PopulateHashTableFromJDictionary(JObject entries, out E Hashtable result = new Hashtable(); foreach (var entry in entries) { + // Case sensitive duplicates should normally not occur since JsonConvert.DeserializeObject + // does not throw when encountering duplicates and just uses the last entry. if (result.ContainsKey(entry.Key)) { string errorMsg = string.Format(CultureInfo.InvariantCulture, - WebCmdletStrings.DuplicateKeysInJsonString, entry.Key, entry.Key); + WebCmdletStrings.DuplicateKeysInJsonString, entry.Key); error = new ErrorRecord( new InvalidOperationException(errorMsg), "DuplicateKeysInJsonString", diff --git a/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx b/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx index 1aef1029f0e..ef130125375 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx +++ b/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx @@ -133,7 +133,10 @@ The provided JSON includes a property whose name is an empty string, this is only supported using the -AsHashTable switch. - Cannot convert the JSON string because a dictionary that was converted from the string contains the duplicated keys '{0}' and '{1}'. + Cannot convert the JSON string because a dictionary that was converted from the string contains the duplicated key '{0}'. + + + Cannot convert the JSON string because a PSObject does not support keys with different casing. Please use the -AsHashTable switch instead. The key that was attempted to be added to the exisiting key '{0}' was '{1}'. The ConvertTo-Json and ConvertFrom-Json cmdlets require the installation of the .NET Client Profile, sometimes called the .NET extended profile. diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 index ca98c03c029..d7c3d53ad4b 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 @@ -2,6 +2,7 @@ BeforeAll { $jsonWithEmptyKey = '{"": "Value"}' + $jsonContainingKeysWithDifferentCasing = '{"key1": "Value1", "Key1": "Value2"}' } It 'No error for valid string '''' with -ReturnHashTable:$' -TestCase @( @@ -30,16 +31,33 @@ $errRecord = $null { [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($str, $ReturnHashTable, [ref]$errRecord) } | ShouldBeErrorId "ArgumentException" } - - It 'Throw InvalidOperationException when json contains empty key name' { - $errorRecord = $null - [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, [ref]$errorRecord) - $errorRecord.FullyQualifiedErrorId | Should Be 'EmptyKeyInJsonString' + + Context 'Empty key name' { + It 'Throw InvalidOperationException when json contains empty key name' { + $errorRecord = $null + [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, [ref]$errorRecord) + $errorRecord.FullyQualifiedErrorId | Should Be 'EmptyKeyInJsonString' + } + + It 'Not throw when json contains empty key name when ReturnHashTable is true' { + $errorRecord = $null + $result = [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, $true, [ref]$errorRecord) + $result | Should Not Be $null + } } - It 'Not throw when json contains empty key na' { - $errorRecord = $null - $result = [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, $true, [ref]$errorRecord) - $result | Should Not Be $null + Context 'Keys with different casing ' { + + It 'Throw InvalidOperationException when json contains key with different casing' { + $errorRecord = $null + [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonContainingKeysWithDifferentCasing, [ref]$errorRecord) + $errorRecord.FullyQualifiedErrorId | Should Be 'KeysWithDifferentCasingInJsonString' + } + + It 'Not throw when json contains key (same casing) when ReturnHashTable is true' { + $errorRecord = $null + $result = [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonContainingKeysWithDifferentCasing, $true, [ref]$errorRecord) + $result | Should Not Be $null + } } } From dfa9126ce428af4cf5ed420acb470d980abf6dd0 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 22 Oct 2017 21:09:49 +0100 Subject: [PATCH 12/13] Add more validation to positive tests and shorten error message as requested in PR. Unfortunately, Pester (3.4) does not seem to support equality assertion for hashtables... --- .../resources/WebCmdletStrings.resx | 2 +- .../Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx b/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx index ef130125375..ebbd210781e 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx +++ b/src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx @@ -136,7 +136,7 @@ Cannot convert the JSON string because a dictionary that was converted from the string contains the duplicated key '{0}'. - Cannot convert the JSON string because a PSObject does not support keys with different casing. Please use the -AsHashTable switch instead. The key that was attempted to be added to the exisiting key '{0}' was '{1}'. + Cannot convert the JSON string because it contains keys with different casing. Please use the -AsHashTable switch instead. The key that was attempted to be added to the exisiting key '{0}' was '{1}'. The ConvertTo-Json and ConvertFrom-Json cmdlets require the installation of the .NET Client Profile, sometimes called the .NET extended profile. diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 index d7c3d53ad4b..be20a4bb75c 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 @@ -43,6 +43,8 @@ $errorRecord = $null $result = [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, $true, [ref]$errorRecord) $result | Should Not Be $null + $result.Count | Should Be 1 + $result.$('') | Should Be 'Value' } } @@ -58,6 +60,9 @@ $errorRecord = $null $result = [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonContainingKeysWithDifferentCasing, $true, [ref]$errorRecord) $result | Should Not Be $null + $result.Count | Should Be 2 + $result.key1 | Should Be 'Value1' + $result.Key1 | Should Be 'Value2' } } } From 935054b6b653754e0f4124916227c86c8e871cdb Mon Sep 17 00:00:00 2001 From: bergmeister Date: Sun, 22 Oct 2017 23:19:39 +0100 Subject: [PATCH 13/13] Minor syntax simplification in jsonobject.tests.ps1 --- .../Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 index be20a4bb75c..b9e83d924ec 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/JsonObject.Tests.ps1 @@ -44,7 +44,7 @@ $result = [Microsoft.PowerShell.Commands.JsonObject]::ConvertFromJson($jsonWithEmptyKey, $true, [ref]$errorRecord) $result | Should Not Be $null $result.Count | Should Be 1 - $result.$('') | Should Be 'Value' + $result.'' | Should Be 'Value' } }