Filter matchAny function changes value order
Description
Environment
is related to
Activity
Rod Widdowson September 28, 2022 at 2:20 PM
Seems safe to close.
Rod Widdowson August 23, 2022 at 1:04 PM
Fixed as noted in the attribute filter.
This is a zero API change and so allows the fix to be back ported to V4.3
Not resolving just yet
Rod Widdowson August 23, 2022 at 12:49 PM
Having done al; this work to make the filters work with lists and not sets, which always felt a wee bit odd given that filtering is more set-like, I suddenly had an inspiration that this bug should have little to do with the order from filtering. the code in AttributeFilterImpl goes
Which is to say “clone the attribute and overwrite the values with the result of filtering”.
What is should is “clone the attribute and remove from it all values which did not get past the fltering, so
Except of course that we his immutability so it becomes
I’ll save what I have done to date into a branch and try this out.
Rod Widdowson August 23, 2022 at 10:08 AMEdited
Having coded up all the matchers to do the deduplication on the fly (using hashsets so as to avoid N^2 inserts) it turns out that the AttributeFilterWorkContext is doing the work as well so I'll probably just add the deduplication there and allow Matchers to return duplicates.
I’ll see what the code looks like
Rod Widdowson August 22, 2022 at 3:32 PM
I’ve started looking at this again. there is some V4.3 work to be done to change the type returned from a ScriptedMatcher to be a Collection (not a Set).
we can make a deprecation warning on it being a Set and tell people to make it a list.
But I want to see that the code to do this in V5 works first
Related to the consent bug that popped up because of attribute value order changing, I'm wondering if the problem/cause of this was the use of Streams in the setValues method.
I know it says the stream is sequential, but I wonder if the collector isn't.
We could probably reproduce this in unit testing, maybe, but either way since this is purely a syntax thing, I think we might want to unroll that back into a loop and ensure we don't change the order inadvertently.
This isn't because of consent, but if this happened in a DataConnector, you'd be in very big trouble. In fact, I do that and I'm wondering why I don't see the problem, so I'll dig a bit more, but I am concerned.
With a multi-attribute result set, it's fatal if each individual one has its values reordered, whereas the only obvious problem with the single Attribute case is consent.