[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: |
|
||||||||||||||||||||
| 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:
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! | |||||||||||
| 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. | |||||||||||
| 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:
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. |