Refactor: libcrmcommon: Consolidate both-non-NULL in mark_xml_changes()
Move the old_child != NULL code in the new_child loop, up to the
old_child loop. This is another case where old_child and new_child are
both non-NULL; we already cover that case in the first loop.
There is a corner case here, where behavior changes but neither the old
behavior nor the new behavior seems correct. match_xml() doesn't
actually require an exact match. The signature is as follows, minus
types:
match_xml(haystack, needle)
- If needle has an id attribute, then we look for a child of haystack with the same name (element type) that has an id attribute with the same value.
- If needle does not have an id attribute, then we look for a child of haystack with the same name. We ignore the id attribute, if set.
This means the match is asymmetric. Suppose we have the following
old_xml:
<parent>
<child id="some_id"/>
</parent>
and the following new_xml:
<parent>
<child/>
</parent>
Further suppose that old_child and new_child refer to the child elements
in the respective trees above.
Then:
- match_xml(new_xml, old_child) returns NULL, because old_child has an id and there is no child of new_xml with name "child" and the same id.
- match_xml(old_xml, new_child) returns old_child, because new_child has no id, so it is free to match any child of old_xml named "child".
So we compute different changes depending on which way we check. I
believe it would work as follows:
- match_xml(new_xml, old_child) will result in marking a deletion and a creation now. I believe previously it would have resulted in a move, which is inaccurate if the element did not change position.
- match_xml(old_xml, new_child) will result in marking a change to the existing element (specifically, an attribute deletion).
This may or may not make a practical difference. Regression tests still
pass. I wouldn't be surprised if some ACL scenarios break though, for
example.
Note that things become even more complicated if there are multiple
children with the same name in old_xml, new_xml, or both. Especially if
some of them move around: match_xml() returns the FIRST matching child.
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>