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

Expose information about JsonPropertyInfo being readable/writable #77307

Open
Tracked by #71944
krwq opened this issue Oct 21, 2022 · 5 comments
Open
Tracked by #71944

Expose information about JsonPropertyInfo being readable/writable #77307

krwq opened this issue Oct 21, 2022 · 5 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json
Milestone

Comments

@krwq
Copy link
Member

krwq commented Oct 21, 2022

It's very useful information to know if property is readable/writable (i.e. generating docs). But this information is currently not exactly obvious.

  • If JsonPropertyInfo.Get != null then we know property is readable
  • If JsonPropertyInfo.Set != null then we know property is writable

but above is not complete:

  • If JsonPropertyInfo.Get != null and there is parametrized ctor with related property property is also writable (technically it's init but that's transparent to user)

while first two are easy to check the last one is not exactly trivial due to this information not being exposed.

Somewhat related to #71944 - it could potentially be solved as part of that issue but this issue is much more scoped than that.

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

ghost commented Oct 21, 2022

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

It's very useful information to know if property is readable/writable (i.e. generating docs). But this information is currently not exactly obvious.

  • If JsonPropertyInfo.Get != null then we know property is readable
  • If JsonPropertyInfo.Set != null then we know property is writable

but above is not complete:

  • If JsonPropertyInfo.Get != null and there is parametrized ctor with related property property is also writable (technically it's init but that's transparent to user)

while first two are easy to check the last one is not exactly trivial due to this information not being exposed.

Somewhat related to #71944 - it could potentially be solved as part of that issue but this issue is much more scoped than that.

Author: krwq
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@eiriktsarpalis
Copy link
Member

It will largely depend on how #71944 gets implemented. Clearly, we need to expose JsonParameterInfo. Currently the implementation requires each parameter to be bound to a corresponding JsonPropertyInfo, however this is only required for validating that each parameter corresponds to a property and is not strictly necessary for deserialization to succeed.

We have received feedback from users who believe that the current restriction is too strict, so perhaps it makes sense to completely decouple JsonPropertyInfo from JsonParameterInfo, at least in the contract model layer. If we do it that way, then clearly a property is writeable only if it defines a setter.

@krwq krwq added this to the 8.0.0 milestone Oct 21, 2022
@krwq krwq removed the untriaged New issue has not been triaged by the area owner label Oct 21, 2022
@krwq
Copy link
Member Author

krwq commented Oct 21, 2022

changing milestone to 8.0 - we should at least consider this for 8.0

@krwq
Copy link
Member Author

krwq commented Oct 21, 2022

@eiriktsarpalis I think I'd go the other way - from JSON point of view there are really no parameters. Only serialization has such concept - I'd rather go in direction of adding bool flag on the JsonPropertyInfo, i.e. IsConstructorParameter or nullable ConstructorParameterName and internally we can still do what we used to. IMO JsonTypeInfo.Properties should be exhaustive list of what can occur in the payload

@eiriktsarpalis
Copy link
Member

I'd rather go in direction of adding bool flag on the JsonPropertyInfo

Note that ordering of constructor parameters is significant and might not match the declared serialization order (which is also influenced by the Order property). Assuming we fix #44428, it may well be the case that the type of JsonParameterInfo does not match the type of the corresponding JsonPropertyInfo.

@eiriktsarpalis eiriktsarpalis added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 23, 2023
@eiriktsarpalis eiriktsarpalis modified the milestones: 8.0.0, Future Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json
Projects
None yet
Development

No branches or pull requests

2 participants