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

[STJ] Disallow property names that conflict with metadata property names #106390

Closed
eiriktsarpalis opened this issue Aug 14, 2024 · 5 comments · Fixed by #106460
Closed

[STJ] Disallow property names that conflict with metadata property names #106390

eiriktsarpalis opened this issue Aug 14, 2024 · 5 comments · Fixed by #106460
Assignees
Labels
area-System.Text.Json breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. enhancement Product code improvement that does NOT require public API changes/additions in-pr There is an active PR which will close this issue when it is merged needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet
Milestone

Comments

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Aug 14, 2024

I think the best course of action is to disallow properties that conflict with type discriminators. It produces JSON with duplicate properties that cannot be roundtripped -- the failure of the schema exporter is merely a side-effect of configuration being invalid.

Originally posted by @eiriktsarpalis in dotnet/aspnetcore#56575 (comment)

This should include conflicting type discriminator id's, as well as reference types containing $id or $ref in cases where reference preservation has been enabled.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Aug 14, 2024
Copy link
Contributor

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

@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label Aug 14, 2024
@eiriktsarpalis eiriktsarpalis added this to the 10.0.0 milestone Aug 14, 2024
@eiriktsarpalis eiriktsarpalis added enhancement Product code improvement that does NOT require public API changes/additions breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. labels Aug 14, 2024
@dotnet-policy-service dotnet-policy-service bot added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Aug 14, 2024
Copy link
Contributor

Added needs-breaking-change-doc-created label because this issue has the breaking-change label.

  1. Create and link to this issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.

Tagging @dotnet/compat for awareness of the breaking change.

@gregsdennis
Copy link
Contributor

Does the usage of $id or $ref have anything to do with JSON Schema, or is it independent/other functionality?

@eiriktsarpalis
Copy link
Member Author

Unrelated, it's metadata being emitted when serializing with ReferenceHandler.Preserve.

@gregsdennis
Copy link
Contributor

Okay. I understand it's existing behavior, but the overlap with JSON Schema is a bit concerning for me.

@eiriktsarpalis eiriktsarpalis self-assigned this Aug 15, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Aug 15, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. enhancement Product code improvement that does NOT require public API changes/additions in-pr There is an active PR which will close this issue when it is merged needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants