object_changed(): fix list comparison

[obfusk]

Currently, e.g. setting a list value (that was not empty before) to [] will never be detected as a change as the for loop is never run. Neither will changing a list to any prefix of the old one. And it will crash if changing a list to a longer one if the list contains something other than lists or dicts as the if value[i] != value2[i] doesn't check the length of value2:

# wrong
>>> bool(object_changed(dict(k=[1,2,3]),dict(k=[])))
False
# wrong
>>> bool(object_changed(dict(k=[1,2,3]),dict(k=[1,2])))
False

# crash
>>> bool(object_changed(dict(k=[1,2,3]),dict(k=[1,2,3,4])))
[...]
    if value[i] != value2[i]:
                   ~~~~~~^^^
IndexError: list index out of range

# wrong
>>> bool(object_changed(dict(k=[dict(a=1),dict(b=2)]),dict(k=[dict(a=1)])))
False
# correct and doesn't crash
>>> bool(object_changed(dict(k=[dict(a=1),dict(b=2)]),dict(k=[dict(a=1), dict(b=2), dict(b=3)])))
True

All of those correctly return True with this patch.

I'm not sure this has any unintended side-effects as some existing code might accidentally rely on the existing behaviour, but it's clearly a bug.

Not yet tested with our actual ansible config, just those examples. Will test soon.

[obfusk]

I found more bugs with dict comparisons. The latest version should fix those.

Would probably be good if someone can run the test suite to make sure nothing breaks.

[obfusk]

Deployed our ansible config with these changes. LGTM :)

[obfusk]

I also looked at the code using this as best I could. I can't rule out it'll break something. And someone should definitely run the test suite. But it looks to me like none of the usages of object_changed() in the ansible role should be adversely affected. And all of the edit_ functions in the API seem to use the same pattern of

data = self.get_monitor(id_)
data.update(kwargs)

Which means that there should be no unintended side-effects from the changes that now check that nested dicts and lists will always be considered changed if they are not the same size. The top-level dict is still considered a superset and subset, as it should be, with extra keys present in the superset not counting as changes.

[obfusk]

So unless something breaks as a result of actual changes now being recognised as such when they were not before -- which means it relied on broken behaviour -- I do not expect any issues.

[obfusk]

FWIW we've been using this in production since I opened this PR and have had zero issues whilst the bug (unable to clear notifications as setting the list to [] is not detected as a change) is indeed fixed.