-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
test: failing test for flattened nullable value object #58
base: master
Are you sure you want to change the base?
Conversation
Also to add to this, I noticed that in the tests the assertions happens between a Serde serialisation and deserialisation. This sort of makes sense but as you can see in this PR, doing that masks these kinds of issues because the serialisation is omitting the erroneous field. |
Oh geez. This is tricky, because null just breaks bloody well everything. 😢 If I remove the guards that cause the field to be omitted, I get a cascading set of null errors in the code that follows. |
Well, I think this works on the serialization side. I hate it, but it seems to work. But now it also fails on the deserialization side, and I... have really no idea how to resolve that nicely. |
Here's the issue for deserialization (notes for me or someone else to pick up later): Flattening works by pulling out the values of an incoming stream that are relevant for that object, and then anything "remaining" gets passed to the collecting/flattening-marked properties with a "here, pull out what you want". That's what makes it flexible and recursive. However, in this case, that means passing The problem is, we have no way to tell in advance that it's going to not work; It would require looking up the child object properties, looking to see if any are "not good" (by some definition), and then defaulting back to null. However, that is slow as it duplicates work, has no clear definition of "not good", and could possibly work only for single-value sub-objects. At the moment, I think nullable flattened objects are just not supportable. I see no not-hideously-hardcoded way to do it. |
This PR adds a failing test for a scenario that doesn't seem to be covered and I'm currently having issues with. I've had a crack at fixing it but I can't figure it out.
Quite a common pattern in my code is a nullable value object, where the parent is nullable but the value object itself contains a non-nullable value. You'll see an example of this in the test I've written.
I also make use of the flattening stuff to rename the value object to something more relevant while still retaining reusability of the value object itself.
Hopefully this helps maintainers here to identify the issue and point me in the right direction 🙏🏻