Fix set filters to use set operations (ansible#81639)

* Fix set filters to use set operations

* Fix integration tests

* Update filter documentation

Author: mattclay

changelogs/fragments/set-filters.yml

@@ -0,0 +1,8 @@
+bugfixes:
+  - Set filters ``intersect``, ``difference``, ``symmetric_difference`` and ``union`` now use set operations when the given items are hashable.
+    Previously, list operations were performed unless the inputs were a hashable type such as ``str``, instead of a collection, such as a ``list`` or ``tuple``.
+  - Set filters ``intersect``, ``difference``, ``symmetric_difference`` and ``union`` now always return a ``list``, never a ``set``.
+    Previously, a ``set`` would be returned if the inputs were a hashable type such as ``str``, instead of a collection, such as a ``list`` or ``tuple``.
+minor_changes:
+  - Documentation for set filters ``intersect``, ``difference``, ``symmetric_difference`` and ``union`` now states
+    that the returned list items are in arbitrary order.

lib/ansible/plugins/filter/difference.yml

@@ -5,6 +5,7 @@ DOCUMENTATION:
   short_description: the difference of one list from another
   description:
     - Provide a unique list of all the elements of the first list that do not appear in the second one.
+    - Items in the resulting list are returned in arbitrary order.
   options:
     _input:
       description: A list.

lib/ansible/plugins/filter/intersect.yml

@@ -5,6 +5,7 @@ DOCUMENTATION:
   short_description: intersection of lists
   description:
     - Provide a list with the common elements from other lists.
+    - Items in the resulting list are returned in arbitrary order.
   options:
     _input:
       description: A list.

lib/ansible/plugins/filter/mathstuff.py

@@ -18,14 +18,12 @@
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
-# Make coding more python3-ish
-from __future__ import (absolute_import, division, print_function)
-__metaclass__ = type
+from __future__ import annotations
 
 import itertools
 import math
 
-from collections.abc import Hashable, Mapping, Iterable
+from collections.abc import Mapping, Iterable
 
 from jinja2.filters import pass_environment
 
@@ -84,37 +82,37 @@ def _do_fail(e):
 
 @pass_environment
 def intersect(environment, a, b):
-    if isinstance(a, Hashable) and isinstance(b, Hashable):
-        c = set(a) & set(b)
-    else:
+    try:
+        c = list(set(a) & set(b))
+    except TypeError:
         c = unique(environment, [x for x in a if x in b], True)
     return c
 
 
 @pass_environment
 def difference(environment, a, b):
-    if isinstance(a, Hashable) and isinstance(b, Hashable):
-        c = set(a) - set(b)
-    else:
+    try:
+        c = list(set(a) - set(b))
+    except TypeError:
         c = unique(environment, [x for x in a if x not in b], True)
     return c
 
 
 @pass_environment
 def symmetric_difference(environment, a, b):
-    if isinstance(a, Hashable) and isinstance(b, Hashable):
-        c = set(a) ^ set(b)
-    else:
+    try:
+        c = list(set(a) ^ set(b))
+    except TypeError:
         isect = intersect(environment, a, b)
         c = [x for x in union(environment, a, b) if x not in isect]
     return c
 
 
 @pass_environment
 def union(environment, a, b):
-    if isinstance(a, Hashable) and isinstance(b, Hashable):
-        c = set(a) | set(b)
-    else:
+    try:
+        c = list(set(a) | set(b))
+    except TypeError:
         c = unique(environment, a + b, True)
     return c
 

lib/ansible/plugins/filter/symmetric_difference.yml

@@ -5,6 +5,7 @@ DOCUMENTATION:
   short_description: different items from two lists
   description:
     - Provide a unique list of all the elements unique to each list.
+    - Items in the resulting list are returned in arbitrary order.
   options:
     _input:
       description: A list.

lib/ansible/plugins/filter/union.yml

@@ -5,6 +5,7 @@ DOCUMENTATION:
   short_description: union of lists
   description:
     - Provide a unique list of all the elements of two lists.
+    - Items in the resulting list are returned in arbitrary order.
   options:
     _input:
       description: A list.

test/integration/targets/filter_mathstuff/tasks/main.yml

@@ -64,44 +64,44 @@
     that:
       - '[1,2,3]|intersect([4,5,6]) == []'
       - '[1,2,3]|intersect([3,4,5,6]) == [3]'
-      - '[1,2,3]|intersect([3,2,1]) == [1,2,3]'
-      - '(1,2,3)|intersect((4,5,6))|list == []'
-      - '(1,2,3)|intersect((3,4,5,6))|list == [3]'
+      - '[1,2,3]|intersect([3,2,1]) | sort == [1,2,3]'
+      - '(1,2,3)|intersect((4,5,6)) == []'
+      - '(1,2,3)|intersect((3,4,5,6)) == [3]'
       - '["a","A","b"]|intersect(["B","c","C"]) == []'
       - '["a","A","b"]|intersect(["b","B","c","C"]) == ["b"]'
-      - '["a","A","b"]|intersect(["b","A","a"]) == ["a","A","b"]'
-      - '("a","A","b")|intersect(("B","c","C"))|list == []'
-      - '("a","A","b")|intersect(("b","B","c","C"))|list == ["b"]'
+      - '["a","A","b"]|intersect(["b","A","a"]) | sort(case_sensitive=True) == ["A","a","b"]'
+      - '("a","A","b")|intersect(("B","c","C")) == []'
+      - '("a","A","b")|intersect(("b","B","c","C")) == ["b"]'
 
 - name: Verify difference
   tags: difference
   assert:
     that:
-      - '[1,2,3]|difference([4,5,6]) == [1,2,3]'
-      - '[1,2,3]|difference([3,4,5,6]) == [1,2]'
+      - '[1,2,3]|difference([4,5,6]) | sort == [1,2,3]'
+      - '[1,2,3]|difference([3,4,5,6]) | sort == [1,2]'
       - '[1,2,3]|difference([3,2,1]) == []'
-      - '(1,2,3)|difference((4,5,6))|list == [1,2,3]'
-      - '(1,2,3)|difference((3,4,5,6))|list == [1,2]'
-      - '["a","A","b"]|difference(["B","c","C"]) == ["a","A","b"]'
-      - '["a","A","b"]|difference(["b","B","c","C"]) == ["a","A"]'
+      - '(1,2,3)|difference((4,5,6)) | sort == [1,2,3]'
+      - '(1,2,3)|difference((3,4,5,6)) | sort == [1,2]'
+      - '["a","A","b"]|difference(["B","c","C"]) | sort(case_sensitive=True) == ["A","a","b"]'
+      - '["a","A","b"]|difference(["b","B","c","C"]) | sort(case_sensitive=True) == ["A","a"]'
       - '["a","A","b"]|difference(["b","A","a"]) == []'
-      - '("a","A","b")|difference(("B","c","C"))|list|sort(case_sensitive=True) == ["A","a","b"]'
-      - '("a","A","b")|difference(("b","B","c","C"))|list|sort(case_sensitive=True) == ["A","a"]'
+      - '("a","A","b")|difference(("B","c","C")) | sort(case_sensitive=True) == ["A","a","b"]'
+      - '("a","A","b")|difference(("b","B","c","C")) | sort(case_sensitive=True) == ["A","a"]'
 
 - name: Verify symmetric_difference
   tags: symmetric_difference
   assert:
     that:
-      - '[1,2,3]|symmetric_difference([4,5,6]) == [1,2,3,4,5,6]'
-      - '[1,2,3]|symmetric_difference([3,4,5,6]) == [1,2,4,5,6]'
+      - '[1,2,3]|symmetric_difference([4,5,6]) | sort == [1,2,3,4,5,6]'
+      - '[1,2,3]|symmetric_difference([3,4,5,6]) | sort == [1,2,4,5,6]'
       - '[1,2,3]|symmetric_difference([3,2,1]) == []'
-      - '(1,2,3)|symmetric_difference((4,5,6))|list == [1,2,3,4,5,6]'
-      - '(1,2,3)|symmetric_difference((3,4,5,6))|list == [1,2,4,5,6]'
-      - '["a","A","b"]|symmetric_difference(["B","c","C"]) == ["a","A","b","B","c","C"]'
-      - '["a","A","b"]|symmetric_difference(["b","B","c","C"]) == ["a","A","B","c","C"]'
+      - '(1,2,3)|symmetric_difference((4,5,6)) | sort == [1,2,3,4,5,6]'
+      - '(1,2,3)|symmetric_difference((3,4,5,6)) | sort == [1,2,4,5,6]'
+      - '["a","A","b"]|symmetric_difference(["B","c","C"]) | sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
+      - '["a","A","b"]|symmetric_difference(["b","B","c","C"]) | sort(case_sensitive=True) == ["A","B","C","a","c"]'
       - '["a","A","b"]|symmetric_difference(["b","A","a"]) == []'
-      - '("a","A","b")|symmetric_difference(("B","c","C"))|list|sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
-      - '("a","A","b")|symmetric_difference(("b","B","c","C"))|list|sort(case_sensitive=True) == ["A","B","C","a","c"]'
+      - '("a","A","b")|symmetric_difference(("B","c","C")) | sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
+      - '("a","A","b")|symmetric_difference(("b","B","c","C")) | sort(case_sensitive=True) == ["A","B","C","a","c"]'
 
 - name: Verify union
   tags: union
@@ -112,11 +112,11 @@
       - '[1,2,3]|union([3,2,1]) == [1,2,3]'
       - '(1,2,3)|union((4,5,6))|list == [1,2,3,4,5,6]'
       - '(1,2,3)|union((3,4,5,6))|list == [1,2,3,4,5,6]'
-      - '["a","A","b"]|union(["B","c","C"]) == ["a","A","b","B","c","C"]'
-      - '["a","A","b"]|union(["b","B","c","C"]) == ["a","A","b","B","c","C"]'
-      - '["a","A","b"]|union(["b","A","a"]) == ["a","A","b"]'
-      - '("a","A","b")|union(("B","c","C"))|list|sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
-      - '("a","A","b")|union(("b","B","c","C"))|list|sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
+      - '["a","A","b"]|union(["B","c","C"]) | sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
+      - '["a","A","b"]|union(["b","B","c","C"]) | sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
+      - '["a","A","b"]|union(["b","A","a"]) | sort(case_sensitive=True) == ["A","a","b"]'
+      - '("a","A","b")|union(("B","c","C")) | sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
+      - '("a","A","b")|union(("b","B","c","C")) | sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
 
 - name: Verify min
   tags: min

test/units/plugins/filter/test_mathstuff.py

@@ -1,9 +1,8 @@
 # Copyright: (c) 2017, Ansible Project
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-# Make coding more python3-ish
-from __future__ import (absolute_import, division, print_function)
-__metaclass__ = type
+from __future__ import annotations
+
 import pytest
 
 from jinja2 import Environment
@@ -12,54 +11,68 @@
 from ansible.errors import AnsibleFilterError, AnsibleFilterTypeError
 
 
-UNIQUE_DATA = (([1, 3, 4, 2], [1, 3, 4, 2]),
-               ([1, 3, 2, 4, 2, 3], [1, 3, 2, 4]),
-               (['a', 'b', 'c', 'd'], ['a', 'b', 'c', 'd']),
-               (['a', 'a', 'd', 'b', 'a', 'd', 'c', 'b'], ['a', 'd', 'b', 'c']),
-               )
+UNIQUE_DATA = [
+    ([], []),
+    ([1, 3, 4, 2], [1, 3, 4, 2]),
+    ([1, 3, 2, 4, 2, 3], [1, 3, 2, 4]),
+    ([1, 2, 3, 4], [1, 2, 3, 4]),
+    ([1, 1, 4, 2, 1, 4, 3, 2], [1, 4, 2, 3]),
+]
+
+TWO_SETS_DATA = [
+    ([], [], ([], [], [])),
+    ([1, 2], [1, 2], ([1, 2], [], [])),
+    ([1, 2], [3, 4], ([], [1, 2], [1, 2, 3, 4])),
+    ([1, 2, 3], [5, 3, 4], ([3], [1, 2], [1, 2, 5, 4])),
+    ([1, 2, 3], [4, 3, 5], ([3], [1, 2], [1, 2, 4, 5])),
+]
+
+
+def dict_values(values: list[int]) -> list[dict[str, int]]:
+    """Return a list of non-hashable values derived from the given list."""
+    return [dict(x=value) for value in values]
+
+
+for _data, _expected in list(UNIQUE_DATA):
+    UNIQUE_DATA.append((dict_values(_data), dict_values(_expected)))
+
+for _dataset1, _dataset2, _expected in list(TWO_SETS_DATA):
+    TWO_SETS_DATA.append((dict_values(_dataset1), dict_values(_dataset2), tuple(dict_values(answer) for answer in _expected)))
 
-TWO_SETS_DATA = (([1, 2], [3, 4], ([], sorted([1, 2]), sorted([1, 2, 3, 4]), sorted([1, 2, 3, 4]))),
-                 ([1, 2, 3], [5, 3, 4], ([3], sorted([1, 2]), sorted([1, 2, 5, 4]), sorted([1, 2, 3, 4, 5]))),
-                 (['a', 'b', 'c'], ['d', 'c', 'e'], (['c'], sorted(['a', 'b']), sorted(['a', 'b', 'd', 'e']), sorted(['a', 'b', 'c', 'e', 'd']))),
-                 )
 
 env = Environment()
 
 
-@pytest.mark.parametrize('data, expected', UNIQUE_DATA)
-class TestUnique:
-    def test_unhashable(self, data, expected):
-        assert ms.unique(env, list(data)) == expected
+def assert_lists_contain_same_elements(a, b) -> None:
+    """Assert that the two values given are lists that contain the same elements, even when the elements cannot be sorted or hashed."""
+    assert isinstance(a, list)
+    assert isinstance(b, list)
 
-    def test_hashable(self, data, expected):
-        assert ms.unique(env, tuple(data)) == expected
+    missing_from_a = [item for item in b if item not in a]
+    missing_from_b = [item for item in a if item not in b]
 
+    assert not missing_from_a, f'elements from `b` {missing_from_a} missing from `a` {a}'
+    assert not missing_from_b, f'elements from `a` {missing_from_b} missing from `b` {b}'
 
-@pytest.mark.parametrize('dataset1, dataset2, expected', TWO_SETS_DATA)
-class TestIntersect:
-    def test_unhashable(self, dataset1, dataset2, expected):
-        assert sorted(ms.intersect(env, list(dataset1), list(dataset2))) == expected[0]
 
-    def test_hashable(self, dataset1, dataset2, expected):
-        assert sorted(ms.intersect(env, tuple(dataset1), tuple(dataset2))) == expected[0]
+@pytest.mark.parametrize('data, expected', UNIQUE_DATA, ids=str)
+def test_unique(data, expected):
+    assert_lists_contain_same_elements(ms.unique(env, data), expected)
 
 
-@pytest.mark.parametrize('dataset1, dataset2, expected', TWO_SETS_DATA)
-class TestDifference:
-    def test_unhashable(self, dataset1, dataset2, expected):
-        assert sorted(ms.difference(env, list(dataset1), list(dataset2))) == expected[1]
+@pytest.mark.parametrize('dataset1, dataset2, expected', TWO_SETS_DATA, ids=str)
+def test_intersect(dataset1, dataset2, expected):
+    assert_lists_contain_same_elements(ms.intersect(env, dataset1, dataset2), expected[0])
 
-    def test_hashable(self, dataset1, dataset2, expected):
-        assert sorted(ms.difference(env, tuple(dataset1), tuple(dataset2))) == expected[1]
 
+@pytest.mark.parametrize('dataset1, dataset2, expected', TWO_SETS_DATA, ids=str)
+def test_difference(dataset1, dataset2, expected):
+    assert_lists_contain_same_elements(ms.difference(env, dataset1, dataset2), expected[1])
 
-@pytest.mark.parametrize('dataset1, dataset2, expected', TWO_SETS_DATA)
-class TestSymmetricDifference:
-    def test_unhashable(self, dataset1, dataset2, expected):
-        assert sorted(ms.symmetric_difference(env, list(dataset1), list(dataset2))) == expected[2]
 
-    def test_hashable(self, dataset1, dataset2, expected):
-        assert sorted(ms.symmetric_difference(env, tuple(dataset1), tuple(dataset2))) == expected[2]
+@pytest.mark.parametrize('dataset1, dataset2, expected', TWO_SETS_DATA, ids=str)
+def test_symmetric_difference(dataset1, dataset2, expected):
+    assert_lists_contain_same_elements(ms.symmetric_difference(env, dataset1, dataset2), expected[2])
 
 
 class TestLogarithm: