Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Enforce that there no extra properties in request JSON, and that non-null properties are [Required] #2328

Merged
merged 6 commits into from
Sep 2, 2022

Conversation

Porges
Copy link
Member

@Porges Porges commented Aug 31, 2022

Closes #2314 via two fixes, one for additional properties and one for missing properties:

  • Make all request types inherit from BaseRequest which has an ExtensionData property, and ensure that it is empty in ParseRequest.
  • Add [Required] attribute to non-nullable properties that do not have defaults, and add a test that ensures we have this attribute where necessary.

@Porges Porges force-pushed the enforce-no-extra-properties branch from e25de4c to da00fdb Compare August 31, 2022 00:24
@Porges Porges marked this pull request as ready for review August 31, 2022 00:27
@Porges Porges changed the title Enforce no extra properties Enforce no extra properties in request JSON Aug 31, 2022
@tevoinea
Copy link
Member

We should be sure to let this change bake a little longer than usual because it has the potential to expose a lot of small serialization bugs we missed.

@Porges Porges force-pushed the enforce-no-extra-properties branch from da00fdb to cb6abec Compare August 31, 2022 20:32
@Porges Porges changed the title Enforce no extra properties in request JSON Enforce that there no extra properties in request JSON, and that non-null properties are [Required] Aug 31, 2022
@Porges Porges force-pushed the enforce-no-extra-properties branch 2 times, most recently from ac99710 to 2e2c0e2 Compare September 1, 2022 00:00
@Porges
Copy link
Member Author

Porges commented Sep 1, 2022

Found an issue already in check-pr.

@Porges Porges force-pushed the enforce-no-extra-properties branch from 2e2c0e2 to 16d7b27 Compare September 1, 2022 02:39
@Porges
Copy link
Member Author

Porges commented Sep 1, 2022

Found during testing that the CLI does send null for some no-longer supported properties, so I’ve updated the PR to allow extraneous properties to be set iff they are null.

@Porges Porges force-pushed the enforce-no-extra-properties branch 2 times, most recently from f0b286e to db6c955 Compare September 1, 2022 20:39
@Porges Porges force-pushed the enforce-no-extra-properties branch from bfe535b to d893241 Compare September 2, 2022 01:51
@Porges Porges enabled auto-merge (squash) September 2, 2022 01:52
@Porges Porges disabled auto-merge September 2, 2022 01:53
@Porges Porges enabled auto-merge (squash) September 2, 2022 01:53
@Porges
Copy link
Member Author

Porges commented Sep 2, 2022

Passed check-pr with full set of changes.

@Porges Porges merged commit c54db04 into main Sep 2, 2022
@Porges Porges deleted the enforce-no-extra-properties branch September 2, 2022 01:59
@ghost ghost locked as resolved and limited conversation to collaborators Oct 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C# port should enforce missing fields in Json deserialization
2 participants