Add Jobject serialization support to ConvertTo-Json #5141
Conversation
chunqingchen
requested review from
JamesWTruher,
PaulHigin and
dantraMSFT
as code owners
There was a problem hiding this comment.
I'm not sure this is doing what it is supposed to. Looking at the issue, The expected behavior to should be:
{
"RandomString": "A quick brown fox jumped over the lazy dog",
"DictObject": {
"JObject": {
"TestValue1": "123456",
"TestValue2": "78910",
"TestValue3": "99999"
},
"StrObject": "This is a string Object"
}
}instead, I'm seeing this:
{
"RandomString": "A quick brown fox jumped over the lazy dog",
"DictObject": {
"JObject": [
"\"TestValue1\": 123456",
"\"TestValue2\": 78910",
"\"TestValue3\": 99999"
],
"StrObject": "This is a string Object"
}
}Instead of being an Dictionary of key/value pairs, it's actually an array strings that are escaped representations of JSON key/value pairs.
Is the intent to serialize Newtonsoft.Json.Linq.Jproperty as string representations of JSON or to serialize them as a dictionary?
iSazonov
dismissed
their stale review
I agree that Jproperty should be serialized as a dictionary not string.
|
@markekraus @iSazonov Technically Jproperty cannot be equal to dictionary as it allows null value as key, while dictionary won't. |
markekraus
left a comment
markekraus
left a comment
There was a problem hiding this comment.
null keys in ConvertTo-Json should not be an issue
| else if (obj is Newtonsoft.Json.Linq.JProperty) | ||
| { | ||
| rv = obj.ToString(); | ||
| } |
There was a problem hiding this comment.
Maybe change it to
else if (obj is Newtonsoft.Json.Linq.JObject jObject)
{
rv = jObject.ToObject<Dictionary<object,object>>();
}I tested that and it can support the following:
$EgJObject = New-Object -TypeName Newtonsoft.Json.Linq.JObject
$EgJObject.Add("TestValue1", "123456")
$EgJObject.Add("TestValue2", "78910")
$EgJObject.Add("TestValue3", "99999")
$EgJObject.Add($null, "88888")
$EgJObject.Add("StringTest", "'StringToken'")
$EgJObject.Add("HashTest", '{ "innerkey": "innervalue" }')
$dict = @{}
$dict.Add('JObject', $EgJObject)
$dict.Add('StrObject', 'This is a string Object')
$properties = @{'DictObject' = $dict; 'RandomString' = 'A quick brown fox jumped over the lazy dog'}
$object = New-Object -TypeName psobject -Property $properties
$object | ConvertTo-Jsonresult
{
"DictObject": {
"StrObject": "This is a string Object",
"JObject": {
"TestValue1": 123456,
"TestValue2": 78910,
"TestValue3": 99999,
"": 88888,
"StringTest": "StringToken",
"HashTest": {
"innerkey": "innervalue"
}
}
},
"RandomString": "A quick brown fox jumped over the lazy dog"
}The null key value should only be an issue in the ConvertFrom-Json cmdlet. Since a null value key is valid JSON, we don't need to worry about it in this direction with ConvertTo-Json`.
There was a problem hiding this comment.
Interesting. Newtonsoft.Json has large internal capabilities for serialization and deserialization - it would make sense to use them in future.
| $jsonFormat.contains("TestValue2") | Should Be True | ||
| $jsonFormat.contains("78910") | Should Be True | ||
| $jsonFormat.contains("TestValue3") | Should Be True | ||
| $jsonFormat.contains("99999") | Should Be True |
There was a problem hiding this comment.
Since these tests failed to detect the issue of the object being properly transformed, i think he tests should be modified in some way that makes it more apparent if the wrong serialization took place.
|
@markekraus your solution works better and your comment is resolved ! |
|
Please use the present tense, imperative style for commit messages and title per Contribution Guide |
There was a problem hiding this comment.
Usualy we use the pattern to get good error message from Pester:
$jsonFormat | Should Match '"TestValue1": 123456')
chunqingchen
changed the title
|
@iSazonov your comment is resolved. |
|
|
|
@chunqingchen suggested title: |
chunqingchen
changed the title
|
@TravisEz13 @markekraus title updated |
4 participants
This is to resolve #4996
Newtonsoft.Json.Linq.Jproperty is not properly handled.
Adding handling for Newtonsoft.Json.Linq.Jproperty
Repro:
$EgJObject = New-Object -TypeName Newtonsoft.Json.Linq.JObject
$EgJObject.Add("TestValue1", "123456")
$EgJObject.Add("TestValue2", "78910")
$EgJObject.Add("TestValue3", "99999")
$dict = @{}
$dict.Add('JObject', $EgJObject)
$dict.Add('StrObject', 'This is a string Object')
$properties = @{'DictObject' = $dict; 'RandomString' = 'A quick brown fox jumped over the lazy dog'}
$object = New-Object -TypeName psobject -Property $properties
$object | ConvertTo-Json
Before fix:
{
"RandomString": "A quick brown fox jumped over the lazy dog",
"DictObject": {
"JObject": [
"123456",
"78910",
"99999"
],
"StrObject": "This is a string Object"
}
}
After fix:
{
"DictObject": {
"JObject": [
""TestValue1": 123456",
""TestValue2": 78910",
""TestValue3": 99999"
],
"StrObject": "This is a string Object"
},
"RandomString": "A quick brown fox jumped over the lazy dog"
}