Fatal1ty / mashumaro

Fast and well tested serialization library
Apache License 2.0
751 stars 44 forks source link

Reject extra keys on deserialization #197

Closed real-or-random closed 4 months ago

real-or-random commented 5 months ago

Is your feature request related to a problem? Please describe. When deserializing, it can happen that the input (say a dict) contains extra/unknown keys, e.g., due to a typo in a configuration file, or due to a mismatch in the data model.

Describe the solution you'd like It would be great to have an option to reject extra keys during deserialization, similar to https://catt.rs/en/stable/customizing.html#forbid-extra-keys in cattr.

I was surprised that this has not been asked before, it seems a very natural thing to me. At least I couldn't find anything/ (It's very vaguely related to https://github.com/Fatal1ty/mashumaro/issues/42 .)

Describe alternatives you've considered I can't think of a good alternative that works well with this library. I could manually check that all keys are known, but doing this manually defeats the purpose of using a library for deserialization.

Fatal1ty commented 5 months ago

I was surprised that this has not been asked before, it seems a very natural thing to me.

In my experience ignoring extra keys is a blessing in scenarios when you want to be forward compatible. Maybe, I’m not the only one in this boat, so this may explain why this feature hasn’t been requested yet :)

Actually, the first one who wanted to forbid extra keys was @mishamsk, and we are waiting for him to split his valuable pull request https://github.com/Fatal1ty/mashumaro/pull/183#issuecomment-1925313960 that will probably resolve this issue.

Meanwhile, I have a list of tasks that need to be solved by the next release (and #42 is one of them). However, if this is a blocker for you, I might consider implementing it myself or taking the PR from @mishamsk as a basis if he doesn’t mind. At least we now have this feature request documented :)

mishamsk commented 5 months ago

hey all. I wrote a long comment why I won't be able to split that until April and then it got boring and I just split the code. Hopefully now you'll be ok to merge one or both of the PRs.

P.S. I'd say a just/taskfile/make with a command to run the preferred linting that would match CI test would be cool!

Fatal1ty commented 5 months ago

@mishamsk Welcome back! I will look into this as soon as possible, most likely this week.

P.S. I'd say a just/taskfile/make with a command to run the preferred linting that would match CI test would be cool!

This is a good point. I personally got used to run all linters separately on my laptop but this needs to be automated. I’ll handle this.

mishamsk commented 5 months ago

This is a good point. I personally got used to run all linters separately on my laptop but this needs to be automated. I’ll handle this.

I've recently started using taskfile (e.g. here is one of them - most of the cleanup is via pre-commit which is also called from GitHub action). Taskfile is pretty nice and easy to write tasks (I have much more complicated stuff in private repos), although one think I miss that just has - ability to pass arbitrary args without -- .... So both options seems great, especially compared with Makefile...

real-or-random commented 5 months ago

@mishamsk Nice to see a PR. :) I had missed your (now closed) PR because I only searched through issues...

In my experience ignoring extra keys is a blessing in scenarios when you want to be forward compatible.

This made me think. I assume one common thing that applications may want to do is ignore but not silently, i.e., issue a warning to the user or write into the log file.

In fact, that's true for other errors, too. One general approach is to ignore errors (e.g., skip invalid values and extra keys) but record them into a list of exceptions and finish parsing to the end. Then at the end, if they were errors, throw an exception and include the parsed object in the exception, and the list of other exceptions. This does the right thing in the sense that it raises a real error if the user asks for it, but the caller can still continue. But the approach is pretty heavy, possibly too much for this kind of library (and certainly not to be included in the current PR).

Sorry for throwing so many suggestions at you... ^^ These are really rather just my ideas, not things that I'd need.

Fatal1ty commented 5 months ago

P.S. I'd say a just/taskfile/make with a command to run the preferred linting that would match CI test would be cool!

On your request https://github.com/Fatal1ty/mashumaro/blob/master/justfile

P.S. Sadly, it seems like zsh completions work only for the first recipe. When I type the first letter of the second recipe, I don't see any suggestions on ⇥ Tab.

mishamsk commented 5 months ago

P.S. I'd say a just/taskfile/make with a command to run the preferred linting that would match CI test would be cool!

On your request https://github.com/Fatal1ty/mashumaro/blob/master/justfile

P.S. Sadly, it seems like zsh completions work only for the first recipe. When I type the first letter of the second recipe, I don't see any suggestions on ⇥ Tab.

awesome. completions or not!