[CSHARP-1240] Type name store in the _t field missing generic type argument Created: 13/Apr/15  Updated: 20/Jan/23  Resolved: 21/Apr/22

Status: Closed
Project: C# Driver
Component/s: Serialization
Affects Version/s: 2.0
Fix Version/s: None

Type: Bug Priority: Minor - P4
Reporter: Marcin Floryan Assignee: Unassigned
Resolution: Duplicate Votes: 1
Labels: Serialization, generics
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
duplicates CSHARP-3494 Discriminator conventions don't work ... Backlog
is duplicated by CSHARP-1751 Generic Collection throws "{document}... Closed
Problem/Incident
is caused by CSHARP-3494 Discriminator conventions don't work ... Backlog
Epic Link: Improve Serialization

 Description   

When an object is serialised to Bson and its type is included in the _t property - for types with a generic argument only the part up to `1 is persisted. For example a type of Container<Wrapped> is written as Container`1



 Comments   
Comment by Craig Wilson [ 22/Aug/16 ]

I think, also, that for now, users could create a convention to do this. Something like below:

    public class GenericDiscriminatorClassMapConvention : ConventionBase, IClassMapConvention
    {
        public void Apply(BsonClassMap classMap)
        {
            if (classMap.ClassType.IsGenericType) 
            {
                var discriminatorName = GetGenericDiscriminatorName(classMap.ClassType);
                classMap.SetDiscriminator(discriminatorName);
            }
        }
    }

Then register the convention. And things should just work, provided you don't have any existing data stored in the "wrong" way.

Comment by Craig Wilson [ 11/May/15 ]

I think that's an excellent idea. The only problem would be when these users are querying for their documents. There would be, in this case, 2 different discriminators representing the same type, which would make querying difficult. We'll have to discuss this and see if this is a big enough problem to not pursue this idea.

Thanks!
Craig

Comment by Kieren Johnstone [ 11/May/15 ]

I found a TypeNameDiscriminator helper class, which generates a generic-safe discriminator - though as above BsonClassMap only uses the .Name of the type. I have a fork which conditionally uses the TypeNameDiscriminator class if the type is generic - but I hadn't considered the 'one-generic-type' issue above. Could it be that classes should have multiple discriminator options to cover this - we create an array for them, the preferred one first?

For instance, SomeGeneric<ParamType> has a list of discriminators (driver-side only): the "full" generic string and the simple "`1" string. When serialising, we pick the first. When deserialising, we can choose any. In the case of the 'one-generic-type' users, the second option is picked up.

Too big a change for a problem with such limited scope?

K

Comment by Marcin Floryan [ 10/May/15 ]

Fair enough. Any change would certainly the existing data stores and I suspect, like you say, they are few and far between.
One solution might be to introduce a better behaviour by default and provide a compatibility convention for the class map?

Comment by Craig Wilson [ 09/May/15 ]

You're absolutely right... However, it's that user who does only store one version of the generic type that we'll be breaking. Perhaps those guys are few and far between, but that's the guy I'm worried about.

Comment by Marcin Floryan [ 09/May/15 ]

Yet I have a strong impression that the current behaviour is already broken anyway. People can't read back any generic types because the type argument is missing from the discriminator. So I'm baffled as to how anyone can have it working unless they only store one generic type argument (though that would defy the point of generics somewhat).

Comment by Craig Wilson [ 08/May/15 ]

No, you aren't missing anything. There are a number of ways to fix this. The problem is that all of them are backwards breaking to applications that have already saved any amount of data. If we change the default discriminator for generic types, then we won't be able to read back in types written with the previous default discriminator.

Comment by Kieren Johnstone [ 08/May/15 ]

It looks like this could be fixed by having BsonClassMap default the _discriminator to the type's .FullName rather than just .Name - or am I missing something?

Happy to put together a pull request for this one if I'm on the right track?

Comment by Marcin Floryan [ 14/Apr/15 ]

Thank you Robert. This workaround is sufficient for us now.

Comment by Robert Stam [ 13/Apr/15 ]

As a temporary workaround, you can register your own discriminator values for generic sub-classes:

BsonClassMap.RegisterClassMap<Container<int>>(cm =>
{
    cm.AutoMap();
    cm.SetDiscriminator("Container<int>");
});
BsonClassMap.RegisterClassMap<Container<string>>(cm =>
{
    cm.AutoMap();
    cm.SetDiscriminator("Container<string>");
});

This would only really be feasible if you know all the possible <T> types in advance, but might be viable as a workaround until this issue gets resolved.

One concern with using this workaround is that we don't yet know what the discriminator value for generic sub-classes might actually end up looking like, and it might not end up being like "Container<int>". So once you store data with these discriminator values your workaround might end up being permanent.

Note: comment edited to reflect a more likely representation for the discriminator values.

Comment by Robert Stam [ 13/Apr/15 ]

I can repro this easily, so we have sufficient information to figure out how to address this.

Thanks for reporting the issue.

Generated at Wed Feb 07 21:39:03 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.