Skip to content
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.

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

Questions about JsonSerializer.Deserialize #83704

Closed
bunnyi116 opened this issue Mar 21, 2023 · 8 comments
Closed

Questions about JsonSerializer.Deserialize #83704

bunnyi116 opened this issue Mar 21, 2023 · 8 comments

Comments

@bunnyi116
Copy link

I'm having a problem where I have a record base type (not abstract and interface) and I use JsonPolymorphic, putting TypeDiscriminatorPropertyName = "type", and IgnoreUnrecognizedTypeDiscriminators = true when an unknown type is encountered, He will return the base type, but in fact it does not assign a value to the base type "type", is this a bug?

image
image

Because the service program belongs to someone else's, I am worried that he will introduce unknown types, so I hope to accept it so that I can add new types so that I can better analyze the data.

Also, I wonder if at JsonSerializer.Deserialize can bind unresolved content data to custom properties like JsonPolymorphic(TypeDiscriminatorPropertyName = "type"), so that the data can be analyzed

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Mar 21, 2023
@ghost
Copy link

ghost commented Mar 21, 2023

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

I'm having a problem where I have a record base type (not abstract and interface) and I use JsonPolymorphic, putting TypeDiscriminatorPropertyName = "type", and IgnoreUnrecognizedTypeDiscriminators = true when an unknown type is encountered, He will return the base type, but in fact it does not assign a value to the base type "type", is this a bug?

image
image

Because the service program belongs to someone else's, I am worried that he will introduce unknown types, so I hope to accept it so that I can add new types so that I can better analyze the data.

Also, I wonder if at JsonSerializer.Deserialize can bind unresolved content data to custom properties like JsonPolymorphic(TypeDiscriminatorPropertyName = "type"), so that the data can be analyzed

Author: Bunnui
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@pinkfloydx33
Copy link

Last I can recall, you cannot have am actual property for the discriminator field. There's definitely some older issues in this repo requesting that it be supported, so it's quite possible it's changed by now, but I don't remember seeing any updates just yet.

Essentially what I'm saying is: remove the property. If you really want/need it, one solution is to make it an abstract property that you decorate with JsonIgnore and then require all sub types to override. We did something similar before deciding to remove it altogether.

@krwq
Copy link
Member

krwq commented Mar 21, 2023

it's currently not supported. My suggestion will be to directly implement static abstract member with the same type discriminators.

There is currently couple of parallel issues with treating metadata similar to properties:

possibly we should expose polymorphic metadata as regular JsonPropertyInfo to allow these scenarios to work.

@krwq krwq added the bug label Mar 21, 2023
@krwq krwq added this to the Future milestone Mar 21, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Mar 21, 2023
@krwq
Copy link
Member

krwq commented Mar 21, 2023

Marking this as Future because we will unlikely pick this up in 8.0 timeline

@krwq
Copy link
Member

krwq commented Mar 21, 2023

In order to move this forward sooner it would be good if we created specific design proposal on how to modernize metadata handling (i.e. show them as JsonPropertyInfo, then perhaps trying to prototype that and seeing if that is sound plan). Once we have specific plan we all agree on we can consider that for vNext or we can leave it for volounteers to pick up.

@eiriktsarpalis
Copy link
Member

This is by design -- type discriminators are identifiers associated with particular derived types on the type level and not values that can be read from or written to object properties. Such a feature would not be polymorphism per se, rather, it is informing the contract of a serialized type dynamically based on contents of its values (and not its runtime type). As such, it is not in scope for the polymorphism feature.

@eiriktsarpalis eiriktsarpalis closed this as not planned Won't fix, can't repro, duplicate, stale Mar 21, 2023
@bunnyi116
Copy link
Author

bunnyi116 commented Mar 21, 2023

This is by design -- type discriminators are identifiers associated with particular derived types on the type level and not values that can be read from or written to object properties. Such a feature would not be polymorphism per se, rather, it is informing the contract of a serialized type dynamically based on contents of its values (and not its runtime type). As such, it is not in scope for the polymorphism feature.

请不要拿This is by design 当借口,这个设计本存在缺陷。
至少遇到无法识别的类型鉴别器的时候,应该反序列化一个基本类型,然后将鉴别器关联的标识符名称传入,而不是反序列化一个基本类型而不传入值,我觉得这正是鉴别器该做的工作IgnoreUnrecognizedTypeDiscriminators = true 我觉得忽略无法识别的类型鉴别器没有必要,真正需要的是能指定无法识别的类型鉴别器指定一个派生类。否则直接捕捉异常不是更好?虽然它忽略了无法识别的类型,但是它又反序列化出一个类型空白的类型,那这个属性的存在意义何在?

直接捕捉异常,当派生类拥有某个属性中存在其他的多态类型,这样我只能将整段原始数据进行分析,很不方便,因为他无法将原始类型与原始数据写入。

获取原始数据:考虑到JSON解析是使用Utf8JsonReader,在对象的开头记录开始位置,在对象接收记录结束位置,然后将这段无法识别的数据写入到自定义属性(前提有设置这个属性名称)

微软想让人使用 System.Text.Json,但每次建议提问都以 This is by design 理由为借口,而不作出改变,明明可以让 System.Text.Json 使用起来更加方便,反而每次弄得更加难用使得开发者自己重写一大堆内容,还有Enum自定义名称也是不支持自定义Json名称。

Please don't use 'This is by design' as an excuse. It's flawed.
Instead of deserializing a primitive type without a value, at least when you encounter an unrecognized type discriminator, you should deserialize a primitive type and pass the identifier name associated with the discriminator. I think this is what the discriminator to do their job IgnoreUnrecognizedTypeDiscriminators = true I think ignoring the type of unrecognized discriminator is not necessary, really need is to specify the type of the unrecognized discriminator to assign a derived class. Otherwise wouldn't it be better to catch the exception directly? It ignores types it doesn't recognize, but it deserializes a blank type, so what's the point of this property?

Directly catching the exception, when the derived class has a property with other polymorphic types, then I can only analyze the whole original data, which is inconvenient, because he can't write the original type and original data.

Get the raw data: considering JSON parsing is using Utf8JsonReader, record the start position at the beginning of the object and the end position at the end of the object receiving record, then write this unrecognized data to the custom property (provided the property name is set)

Microsoft wants people to use System.Text.Json, but every time they ask a question, they useThis is by design as an excuse, instead of making changes to make System.Text.Json easier to use. On the other hand, it makes it harder for developers to rewrite a lot of things themselves each time, and Enum custom names don't support custom Json names.

@eiriktsarpalis
Copy link
Member

I'm sorry you feel that way, but as already mentioned, the type discriminator is designed to be a type-level mapping and does not bind to runtime values of properties (either in serialization or deserialization). As with any feature of such a nature, it is necessarily opinionated so it can't cater to every use case (similar to how the ReferenceHandler.Preserve doesn't bind reference metadata to properties). I would suggest writing your own custom converter that handles polymorphism in a way that satisfies your use case.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants