Skip to content

Fixes #237 - support multi-dimensional arrays #238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mlankamp wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixes #237 - support multi-dimensional arrays #238

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 13 additions & 1 deletion Interfaces/Cosmos.DataTransfer.Common/DataItemJsonConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,15 @@ internal static void WriteFieldValue(Utf8JsonWriter writer, string fieldName, ob
}
else if (fieldValue is not string && fieldValue is IEnumerable children)
{
writer.WriteStartArray(propertyName);
if (!string.IsNullOrWhiteSpace(propertyName.Value))
{
writer.WriteStartArray(propertyName);
}
else
{
writer.WriteStartArray();
}

foreach (object? arrayItem in children)
{
if (arrayItem is IDataItem arrayChild)
Expand All @@ -125,6 +133,10 @@ internal static void WriteFieldValue(Utf8JsonWriter writer, string fieldName, ob
var arrayDictItem = new DictionaryDataItem(arrayDict);
WriteDataItem(writer, arrayDictItem, includeNullFields);
}
else if (arrayItem is not string && arrayItem is IEnumerable childs)
{
WriteFieldValue(writer, "", childs, includeNullFields);
Copy link
Copy Markdown
Collaborator

@philnach philnach Mar 30, 2026

Choose a reason for hiding this comment

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

Thanks for the PR — adding support for nested (multi-dimensional) arrays makes sense.

That said, I’m a bit concerned about the current approach of handling nested arrays by calling:

WriteFieldValue(writer, "", childs, includeNullFields);

relies on:

if (!string.IsNullOrWhiteSpace(propertyName.Value)) { writer.WriteStartArray(propertyName); }
else { writer.WriteStartArray(); }

Could we refactor nested-array writing so it doesn’t depend on passing fieldName == "" and the propertyName.Value check?

For example:

introduce a dedicated helper that writes an array value (no property name), e.g. WriteArrayValue(Utf8JsonWriter writer, IEnumerable items, ...)
use that helper for nested arrays so it always calls writer.WriteStartArray() / writer.WriteEndArray()

(Optional)
The nested-array detection (arrayItem is not string && arrayItem is IEnumerable) is very broad and may accidentally treat things like byte[] as arrays of numbers. It might be worth tightening this to the specific collection types we expect to see here.

}
else if (TryGetLong(arrayItem, out var longValue))
{
writer.WriteNumberValue(longValue);
Expand Down
Loading