Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/System.Text.Json/tests/Serialization/CyclicTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,16 @@ public static void WriteCyclicFail()
// We don't allow cycles; we throw InvalidOperation instead of an unrecoverable StackOverflow.
Assert.Throws<InvalidOperationException>(() => JsonSerializer.ToString(obj));
}

[Fact]
[ActiveIssue(37313)]
public static void WriteTestClassWithArrayOfElementsOfTheSameClassWithoutCyclesDoesNotFail()
{
TestClassWithArrayOfElementsOfTheSameClass obj = new TestClassWithArrayOfElementsOfTheSameClass();

//It shouldn't throw when there is no real cycle reference, and just empty object is created
string json = JsonSerializer.ToString(obj);
Assert.Equal(@"{}", json);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pranavkm, @rynowak - what do you expect the behavior to be here? Is returning an empty object JSON fine or should the serializer throw ArgumentException/InvalidOperationException?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty object for certain. This is a legal and useful way to define a data-type 👍

Copy link
Copy Markdown

@ahsonkhan ahsonkhan May 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty object for certain

Is there any value in the output being something like the following instead of {} (if it was feasible)?
{"Array":[]}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be empty object or empty object with inner array initialized with undefined value. For sure it shouldn't be empty array, as property was not initialized.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tried, but does this reproduce with a Dictionary property:

public TestClassWithDictionaryOfElementsOfTheSameClass
{
    public string Dictionary<string, TestClassWithDictionaryOfElementsOfTheSameClass> Values { get; set; }
}

Might be useful to also capture this as a test scenario to make sure this works too.

Copy link
Copy Markdown
Contributor Author

@oskardudycz oskardudycz May 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it's happening for all collections. My initial issue was for arrays (https://github.com/dotnet/corefx/issues/37610) while it's duplicate of the issue with lists (https://github.com/dotnet/corefx/issues/37313)

}
}
}
5 changes: 5 additions & 0 deletions src/System.Text.Json/tests/Serialization/TestClasses.cs
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,11 @@ public void Initialize()
}
}

public class TestClassWithArrayOfElementsOfTheSameClass
{
public TestClassWithArrayOfElementsOfTheSameClass[] Array { get; set; }
}

public class TestClassWithGenericList : ITestClass
{
public List<string> MyData { get; set; }
Expand Down