dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.12k stars 4.7k forks source link

System.Text.Json doesn't support KeyedCollection when serializing with source generation #97617

Closed Septercius closed 8 months ago

Septercius commented 8 months ago

Description

I'm trying to use the source generation capabilities of System.Text.Json to serialize and deserialize a type that contains an instance of the KeyedCollection<,> class. The documentation (https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/supported-collection-types) states that this is not a fully supported type (specifically, non-string key types), but it does appear to work without problem when not using source generation (so long as I use JsonObjectCreationHandling.Populate), and I get the expected results.

I then tried using the source generation using the same object, and this is failing with a "The given key '0' was not present in the dictionary" error. Looking at the generated code, it's iterating through the collection using a standard for loop, and accessing the items in the collection by index, which doesn't work with KeyedCollection<,> as the indexer requires a key.

Deserializing using source generation does seem to work (it doesn't throw an exception).

A few questions:

  1. Is KeyedCollection<,> actually supported when not using source generation (contrary to what the documentation states)?
  2. Is there any way of getting source generation to use foreach instead?

Reproduction Steps

Paste the following into a new project:

using System;
using System.Text.Json;
using System.Collections.ObjectModel;
using System.Text.Json.Serialization;

namespace STJTest;

class Program
{

    static void Main(string[] args)
    {
        var employeeBob = new Employee() { Id = 1, Name = "Bob" };
        var employeeJane = new Employee() { Id = 2, Name = "Jane" };

        var department = new Department() { Id = 1 };
        department.Employees.Add(employeeBob);
        department.Employees.Add(employeeJane);

        // Serialize without the serializer context.
        var serializedWithoutContext = JsonSerializer.Serialize(department);

        Console.WriteLine("Serialized without context:");
        Console.WriteLine(serializedWithoutContext);

        var jsonText = "{\"Id\":2,\"Employees\":[{\"Id\":3,\"Name\":\"Sally\"},{\"Id\":2,\"Name\":\"Gordon\"}]}";

        // Deserialize without the serializer context.
        var deserializedWithoutContext = (Department) JsonSerializer.Deserialize(jsonText, typeof(Department));

        Console.WriteLine("Deserialized without context:");
        foreach (var employee in deserializedWithoutContext.Employees)
        {
            Console.WriteLine("{0} - {1}", employee.Id, employee.Name);
        }

        // Serialize with the context.  This will fail with "The given key '0' was not present in the dictionary".
        var serializedWithContext = JsonSerializer.Serialize(department, typeof(Department), DepartmentContext.Default);

        Console.WriteLine("Serialized with context:");
        Console.WriteLine(serializedWithContext);

        // Deserialize with the context.  This will work correctly.
        var deserializedWithContext = (Department) JsonSerializer.Deserialize(jsonText, typeof(Department), DepartmentContext.Default);

        Console.WriteLine("Deserialized with context:");
        foreach (var employee in deserializedWithContext.Employees)
        {
            Console.WriteLine("{0} - {1}", employee.Id, employee.Name);
        }

        Console.ReadKey();
    }
}

[JsonSerializable(typeof(Department))]
internal partial class DepartmentContext : JsonSerializerContext
{
}

public class Department
{
    public int Id { get; set; }

    [JsonObjectCreationHandling(JsonObjectCreationHandling.Populate)]
    public EmployeeCollection Employees { get; private set; }

    public Department()
    {
        Employees = new EmployeeCollection();
    }
}

public class Employee
{
    public int Id { get; set; }
    public string Name { get; set; }
}

public class EmployeeCollection : KeyedCollection<int, Employee>
{
    protected override int GetKeyForItem(Employee item)
    {
        return item.Id;
    }
}

Expected behavior

The object is successfully serialized to JSON.

Actual behavior

The code throws an exception: "The given key '0' was not present in the dictionary".

Regression?

No response

Known Workarounds

No response

Configuration

.NET 8, Mac OS Ventura 13.6.4, x86_64

Other information

The code generating the source appears to be the GenerateFastPathFuncForEnumerable method: https://github.com/dotnet/runtime/blob/7a60900cd1be3361773237d0d0d0293146db5547/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs#L406

ghost commented 8 months ago

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis See info in area-owners.md if you want to be subscribed.

Issue Details
### Description I'm trying to use the source generation capabilities of System.Text.Json to serialize and deserialize a type that contains an instance of the `KeyedCollection<,>` class. The documentation (https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/supported-collection-types) states that this is not a fully supported type (specifically, non-string types), but it does appear to work without problem when not using source generation (so long as I use `JsonObjectCreationHandling.Populate`), and I get the expected results. I then tried using the source generation using the same object, and this is failing with a "The given key '0' was not present in the dictionary" error. Looking at the generated code, it's iterating through the collection using a standard for loop, and accessing the items in the collection by index, which doesn't work with `KeyedCollection<,>` as the indexer requires a key. Deserializing using source generation does seem to work (it doesn't throw an exception). A few questions: 1. Is `KeyedCollection<,>` actually supported when not using source generation (contrary to what the documentation states)? 2. Is there any way of getting source generation to use `foreach` instead? ### Reproduction Steps Paste the following into a new project: ``` using System; using System.Text.Json; using System.Collections.ObjectModel; using System.Text.Json.Serialization; namespace STJTest; class Program { static void Main(string[] args) { var employeeBob = new Employee() { Id = 1, Name = "Bob" }; var employeeJane = new Employee() { Id = 2, Name = "Jane" }; var department = new Department() { Id = 1 }; department.Employees.Add(employeeBob); department.Employees.Add(employeeJane); // Serialize without the serializer context. var serializedWithoutContext = JsonSerializer.Serialize(department); Console.WriteLine("Serialized without context:"); Console.WriteLine(serializedWithoutContext); var jsonText = "{\"Id\":2,\"Employees\":[{\"Id\":3,\"Name\":\"Sally\"},{\"Id\":2,\"Name\":\"Gordon\"}]}"; // Deserialize without the serializer context. var deserializedWithoutContext = (Department) JsonSerializer.Deserialize(jsonText, typeof(Department)); Console.WriteLine("Deserialized without context:"); foreach (var employee in deserializedWithoutContext.Employees) { Console.WriteLine("{0} - {1}", employee.Id, employee.Name); } // Serialize with the context. This will fail with "The given key '0' was not present in the dictionary". var serializedWithContext = JsonSerializer.Serialize(department, typeof(Department), DepartmentContext.Default); Console.WriteLine("Serialized with context:"); Console.WriteLine(serializedWithContext); // Deserialize with the context. This will work correctly. var deserializedWithContext = (Department) JsonSerializer.Deserialize(jsonText, typeof(Department), DepartmentContext.Default); Console.WriteLine("Deserialized with context:"); foreach (var employee in deserializedWithContext.Employees) { Console.WriteLine("{0} - {1}", employee.Id, employee.Name); } Console.ReadKey(); } } [JsonSerializable(typeof(Department))] internal partial class DepartmentContext : JsonSerializerContext { } public class Department { public int Id { get; set; } [JsonObjectCreationHandling(JsonObjectCreationHandling.Populate)] public EmployeeCollection Employees { get; private set; } public Department() { Employees = new EmployeeCollection(); } } public class Employee { public int Id { get; set; } public string Name { get; set; } } public class EmployeeCollection : KeyedCollection { protected override int GetKeyForItem(Employee item) { return item.Id; } } ``` ### Expected behavior The object is successfully serialized to JSON. ### Actual behavior The code throws an exception: "The given key '0' was not present in the dictionary". ### Regression? _No response_ ### Known Workarounds _No response_ ### Configuration .NET 8, Mac OS Ventura 13.6.4, x86_64 ### Other information The code generating the source appears to be the `GenerateFastPathFuncForEnumerable` method: https://github.com/dotnet/runtime/blob/7a60900cd1be3361773237d0d0d0293146db5547/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs#L406
Author: Septercius
Assignees: -
Labels: `area-System.Text.Json`, `untriaged`
Milestone: -
eiriktsarpalis commented 8 months ago

I'm seeing a couple of issues:

  1. The documentation incorrectly asserts that non-string keys and deserialization aren't supported, when in fact the key type is not part of the serialization contract (the type round trips an array of values, correctly in my opinion). We should update the documentation to reflect that reality.
  2. There is an issue specifically with how the source generator handles KeyCollection<int, TValue> types, since it duck types on the indexer signature to generate an List-like serialization loop in the fast-path serializer. We might want to consider adding a check particularly for that corner case.

Minimal reproduction:

var value = new MyKeyedCollection { 1, 2, 3 };
JsonSerializer.Serialize(value, MyContext.Default.MyKeyedCollection);

[JsonSerializable(typeof(MyKeyedCollection))]
internal partial class MyContext : JsonSerializerContext;

public class MyKeyedCollection : KeyedCollection<int, int>
{
    protected override int GetKeyForItem(int k) => k;
}

A simple workaround for that issue is to disable fast-path generation for that particular type:

var value = new MyKeyedCollection { 1, 2, 3 };
var result = JsonSerializer.Serialize(value, MyContext.Default.MyKeyedCollection);
JsonSerializer.Deserialize(result, MyContext.Default.MyKeyedCollection);

[JsonSerializable(typeof(MyKeyedCollection), GenerationMode = JsonSourceGenerationMode.Metadata)]
internal partial class MyContext : JsonSerializerContext;