Pinging @giovannipizzi and @greschd for discussion

Yes, I think the validation should be recursive.

As a side note: the behavior is different again for cases where the JSON serializer doesn't automatically convert to string. The following code raises an error when storing the node:

from aiida.orm import Dict

class Test:
    def __hash__(self):
         return 0

d = Dict(dict={'a': {Test(): 2}}) # works
d.store() # "TypeError: keys must be a string" from json/encoder.py

Your example is the expected behavior though. I delayed the validation through serialization to when the node is stored. This is for efficiency reasons, because as long as the node is not stored, attributes can be freely mutated. Validating the whole set of attributes every time is not efficient and only validating the changed attribute may not be sufficient. So therefore we chose to push validation down to just before storing the node.

Ah, I see. Yeah, the problematic case is when it is just mutated silently, because that can go undetected.

Indeed, we moved validation upon storing.
I agree that silent mutation should be avoided.
Beside the very first example, are there other cases? We can run the key validation recursively, but we need to check that it's fast enough.

I am pinging @ltalirz because we changed the validation code as it was very slow, so before we merge any change we should double check the performance impact.

See also #733 for some reference on some decision choices

If I remember correctly the biggest slowdown was that the entire set of attributes was cleaned in its entirely every time a single attribute was changed. When building up StructureData nodes then, where you call append_atom often, which will call set_attr many times, we ended up validating everything multiple times. I think validating the entire nested structure once just before storing should be acceptable. Anyway, like you said, the alternative is silent mutation which is perhaps worse than a slightly worse performance

I agree that validation should be recursive upon store (I think there is still the flag to skip validation entirely if maximum performance is needed, right?).
And yes, the original issue was that validation happened whenever the node was modified before storing.

The original performance issue was noticed when looping over a few hundred structure files to import them into StructureData.
I suggest this as a performance test here as well.