From 9decee111706d37841a1cbfe84a704a174697d17 Mon Sep 17 00:00:00 2001 From: Guillaume Mulocher Date: Fri, 2 Aug 2024 16:54:39 +0200 Subject: [PATCH] Feat(plugins): Add strict mode and ignore_case flags to natural_sort filter (#4298) Co-authored-by: Claus Holbech Co-authored-by: Carl Buchmann --- .../plugins/Filter_plugins/natural_sort.md | 2 + .../arista/avd/plugins/filter/natural_sort.py | 10 ++ .../application-traffic-recognition.j2 | 6 +- .../eos/application-traffic-recognition.j2 | 4 +- python-avd/pyavd/j2filters/natural_sort.py | 52 ++++++--- .../pyavd/j2filters/test_natural_sort.py | 110 ++++++++++++++---- 6 files changed, 142 insertions(+), 42 deletions(-) diff --git a/ansible_collections/arista/avd/docs/plugins/Filter_plugins/natural_sort.md b/ansible_collections/arista/avd/docs/plugins/Filter_plugins/natural_sort.md index e952c85ca1d..d96025085cb 100644 --- a/ansible_collections/arista/avd/docs/plugins/Filter_plugins/natural_sort.md +++ b/ansible_collections/arista/avd/docs/plugins/Filter_plugins/natural_sort.md @@ -29,6 +29,8 @@ The filter will return an empty list if the value parsed to `arista.avd.natural_ | -------- | ---- | -------- | ------- | ------------------ | ----------- | | _input | any | True | None | | List or dictionary | | sort_key | string | optional | None | | Key to sort on when sorting a list of dictionaries | +| strict | bool | optional | True | | When `sort_key` is set, setting strict to true will trigger an exception if the `sort_key` is missing in any items in the input. | +| ignore_case | bool | optional | True | | When true, strings are coerced to lower case before being compared. | ## Examples diff --git a/ansible_collections/arista/avd/plugins/filter/natural_sort.py b/ansible_collections/arista/avd/plugins/filter/natural_sort.py index e8777fa214d..d707bad44f6 100644 --- a/ansible_collections/arista/avd/plugins/filter/natural_sort.py +++ b/ansible_collections/arista/avd/plugins/filter/natural_sort.py @@ -47,6 +47,16 @@ description: Key to sort on when sorting a list of dictionaries type: string version_added: "3.0.0" + strict: + description: When `sort_key` is set, setting strict to true will trigger an exception if the `sort_key` is missing in any items in the input. + type: bool + default: true + version_added: "5.0.0" + ignore_case: + description: When true, strings are coerced to lower case before being compared. + type: bool + default: true + version_added: "5.0.0" """ EXAMPLES = r""" diff --git a/python-avd/pyavd/_eos_cli_config_gen/j2templates/documentation/application-traffic-recognition.j2 b/python-avd/pyavd/_eos_cli_config_gen/j2templates/documentation/application-traffic-recognition.j2 index 475c0e948bc..05e75cf1113 100644 --- a/python-avd/pyavd/_eos_cli_config_gen/j2templates/documentation/application-traffic-recognition.j2 +++ b/python-avd/pyavd/_eos_cli_config_gen/j2templates/documentation/application-traffic-recognition.j2 @@ -56,10 +56,10 @@ | Type | Name | Service | | ---- | ---- | ------- | -{% for application in application_profile.applications | arista.avd.natural_sort('service') | arista.avd.natural_sort('name') %} +{% for application in application_profile.applications | arista.avd.natural_sort('service', strict=False) | arista.avd.natural_sort('name') %} | application | {{ application.name }} | {{ application.service | arista.avd.default("-") }} | {% endfor %} -{% for category in application_profile.categories | arista.avd.natural_sort('service') | arista.avd.natural_sort('name') %} +{% for category in application_profile.categories | arista.avd.natural_sort('service', strict=False) | arista.avd.natural_sort('name') %} | category | {{ category.name }} | {{ category.service | arista.avd.default("-") }} | {% endfor %} {% for transport in application_profile.application_transports | arista.avd.natural_sort %} @@ -76,7 +76,7 @@ | -------- | -------------------- | {% for category in application_traffic_recognition.categories | arista.avd.natural_sort('name') %} {% set apps = [] %} -{% for app_details in category.applications | arista.avd.natural_sort('service') | arista.avd.natural_sort('name') %} +{% for app_details in category.applications | arista.avd.natural_sort('service', strict=False) | arista.avd.natural_sort('name') %} {% if app_details.service is arista.avd.defined %} {% do apps.append( app_details.name + "(" + app_details.service | arista.avd.default("-") + ")" ) %} {% else %} diff --git a/python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/application-traffic-recognition.j2 b/python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/application-traffic-recognition.j2 index d7d69651cab..f3167ccc670 100644 --- a/python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/application-traffic-recognition.j2 +++ b/python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/application-traffic-recognition.j2 @@ -76,7 +76,7 @@ application traffic recognition {% for category in application_traffic_recognition.categories | arista.avd.natural_sort('name') %} ! category {{ category.name }} -{% for app_details in category.applications | arista.avd.natural_sort('name') | arista.avd.natural_sort('service') %} +{% for app_details in category.applications | arista.avd.natural_sort('name') | arista.avd.natural_sort('service', strict=False) %} {% if app_details.service is arista.avd.defined %} application {{ app_details.name }} service {{ app_details.service }} {% else %} @@ -87,7 +87,7 @@ application traffic recognition {% for application_profile in application_traffic_recognition.application_profiles | arista.avd.natural_sort('name') %} ! application-profile {{ application_profile.name }} -{% for application in application_profile.applications | arista.avd.natural_sort('name') | arista.avd.natural_sort('service') %} +{% for application in application_profile.applications | arista.avd.natural_sort('name') | arista.avd.natural_sort('service', strict=False) %} {% if application.service is arista.avd.defined %} application {{ application.name }} service {{ application.service }} {% else %} diff --git a/python-avd/pyavd/j2filters/natural_sort.py b/python-avd/pyavd/j2filters/natural_sort.py index 76f0dcc3b4a..1d331fbdb29 100644 --- a/python-avd/pyavd/j2filters/natural_sort.py +++ b/python-avd/pyavd/j2filters/natural_sort.py @@ -9,25 +9,43 @@ from jinja2.utils import Namespace -def convert(text: str) -> int | str: - """ - Converts the string to an integer if it is a digit, otherwise converts it to lower case. - Args: +def convert(text: str, ignore_case: bool) -> int | str: + """Converts the input string to be sorted. + + Converts the string to an integer if it is a digit, otherwise converts + it to lower case if ignore_case is True. + + Parameters + ---------- text (str): Input string. - Returns: + ignore_case (bool): If ignore_case is True, strings are applied lower() function. + + Returns + ------- int | str: Converted string. """ - return int(text) if text.isdigit() else text.lower() + if text.isdigit(): + return int(text) + return text.lower() if ignore_case else text -def natural_sort(iterable: list | dict | str | None, sort_key: str | None = None) -> list: - """ - Sorts an iterable in a natural (alphanumeric) order. - Args: +def natural_sort(iterable: list | dict | str | None, sort_key: str | None = None, *, strict: bool = True, ignore_case: bool = True) -> list: + """Sorts an iterable in a natural (alphanumeric) order. + + Parameters + ---------- iterable (list | dict | str | None): Input iterable. sort_key (str | None, optional): Key to sort by, defaults to None. - Returns: + strict (bool, optional): If strict is True, raise an error is the sort_key is missing. + ignore_case (bool, optional): If ignore_case is True, strings are applied lower() function. + + Returns + ------- list: Sorted iterable. + + Raises + ------ + KeyError, AttributeError: if strict=True and sort_key is not present in an item in the iterable. """ if isinstance(iterable, Undefined) or iterable is None: return [] @@ -35,9 +53,15 @@ def natural_sort(iterable: list | dict | str | None, sort_key: str | None = None def alphanum_key(key): pattern = r"(\d+)" if sort_key is not None and isinstance(key, dict): - return [convert(c) for c in re.split(pattern, str(key.get(sort_key, key)))] + if strict and sort_key not in key: + msg = f"Missing key '{sort_key}' in item to sort {key}." + raise KeyError(msg) + return [convert(c, ignore_case) for c in re.split(pattern, str(key.get(sort_key, key)))] if sort_key is not None and isinstance(key, Namespace): - return [convert(c) for c in re.split(pattern, getattr(key, sort_key))] - return [convert(c) for c in re.split(pattern, str(key))] + if strict and not hasattr(key, sort_key): + msg = f"Missing attribute '{sort_key}' in item to sort {key}." + raise AttributeError(msg) + return [convert(c, ignore_case) for c in re.split(pattern, getattr(key, sort_key))] + return [convert(c, ignore_case) for c in re.split(pattern, str(key))] return sorted(iterable, key=alphanum_key) diff --git a/python-avd/tests/pyavd/j2filters/test_natural_sort.py b/python-avd/tests/pyavd/j2filters/test_natural_sort.py index ee356554adc..52a3d947ffc 100644 --- a/python-avd/tests/pyavd/j2filters/test_natural_sort.py +++ b/python-avd/tests/pyavd/j2filters/test_natural_sort.py @@ -1,47 +1,55 @@ # Copyright (c) 2023-2024 Arista Networks, Inc. # Use of this source code is governed by the Apache License 2.0 # that can be found in the LICENSE file. + +from contextlib import nullcontext as does_not_raise + import pytest from pyavd.j2filters.natural_sort import convert, natural_sort class TestNaturalSortFilter: @pytest.mark.parametrize( - "item_to_convert, converted_item", + ("item_to_convert, converted_item, ignore_case"), [ - ("100", 100), - ("200", 200), - ("ABC", "abc"), + ("100", 100, True), + ("200", 200, True), + ("ABC", "abc", True), + ("ABC", "ABC", False), ], ) - def test_convert(self, item_to_convert, converted_item): - resp = convert(item_to_convert) + def test_convert(self, item_to_convert, converted_item, ignore_case): + resp = convert(item_to_convert, ignore_case) assert resp == converted_item @pytest.mark.parametrize( - "item_to_natural_sort, sort_key, sorted_list", + ("item_to_natural_sort, sort_key, strict, ignore_case, sorted_list, expected_raise"), [ - (None, None, []), # test with None - ([], None, []), # test with blank list - ({}, "", []), # test with blank dict - ("", None, []), # test with blank string - ("access_list", None, ["_", "a", "c", "c", "e", "i", "l", "s", "s", "s", "t"]), # test with string - (["1,2,3,4", "11,2,3,4", "5.6.7.8"], None, ["1,2,3,4", "5.6.7.8", "11,2,3,4"]), # test with list of integers - ({"a1": 123, "a10": 333, "a2": 2, "a11": 4456}, None, ["a1", "a2", "a10", "a11"]), # test with dict - ( + pytest.param(None, None, False, True, [], does_not_raise(), id="None"), + pytest.param([], None, False, True, [], does_not_raise(), id="empty-list"), + pytest.param({}, "", False, True, [], does_not_raise(), id="empty-dict"), + pytest.param("", "", False, True, [], does_not_raise(), id="empty-string"), + pytest.param("access_list", None, False, True, ["_", "a", "c", "c", "e", "i", "l", "s", "s", "s", "t"], does_not_raise(), id="string-input"), + pytest.param(["1,2,3,4", "11,2,3,4", "5.6.7.8"], None, False, True, ["1,2,3,4", "5.6.7.8", "11,2,3,4"], does_not_raise(), id="list-of-integers"), + pytest.param({"a1": 123, "a10": 333, "a2": 2, "a11": 4456}, None, False, True, ["a1", "a2", "a10", "a11"], does_not_raise(), id="dict"), + pytest.param( [ {"name": "ACL-10", "counters_per_entry": True}, {"name": "ACL-01", "counters_per_entry": True}, {"name": "ACL-05", "counters_per_entry": False}, ], "name", + False, + True, [ {"name": "ACL-01", "counters_per_entry": True}, {"name": "ACL-05", "counters_per_entry": False}, {"name": "ACL-10", "counters_per_entry": True}, - ], # test list of dict with "name" as sort_key + ], + does_not_raise(), + id="list-of-dict-with-sort-key", ), - ( + pytest.param( [ {"name": "ACL-10", "counters_per_entry": True}, {"sequence_numbers": {"sequence": 10}}, @@ -49,28 +57,84 @@ def test_convert(self, item_to_convert, converted_item): {"name": "ACL-05", "counters_per_entry": False}, ], "name", + False, + True, [ {"name": "ACL-05", "counters_per_entry": False}, {"name": "ACL-10", "counters_per_entry": True}, {"counters_per_entry": False}, {"sequence_numbers": {"sequence": 10}}, - ], # test list of dict without "name" sort_key in some entries + ], + does_not_raise(), + id="list-of-dict-with-sort-key-some-entries-dont-have-the-key", ), - ( + pytest.param( [ {"sequence_numbers": {"sequence": 10}}, {"counters_per_entry": False}, {"action": "action_command"}, ], "name", + False, + True, [ {"action": "action_command"}, {"counters_per_entry": False}, {"sequence_numbers": {"sequence": 10}}, ], - ), # test test list of dict without "name" sort_key in all entries + does_not_raise(), + id="list-of-dict-with-sort-key-all-entries-dont-have-the-key", + ), + pytest.param( + [ + {"name": "default"}, + {"name": "D"}, + {"name": "E"}, + ], + "name", + False, + True, + [ + {"name": "D"}, + {"name": "default"}, + {"name": "E"}, + ], + does_not_raise(), + id="list-of-dict-with-sort-key-ignore-case", + ), + pytest.param( + [ + {"name": "default"}, + {"name": "D"}, + {"name": "E"}, + ], + "name", + False, + False, + [ + {"name": "D"}, + {"name": "E"}, + {"name": "default"}, + ], + does_not_raise(), + id="list-of-dict-with-sort-key-respect-case", + ), + pytest.param( + [ + {"name": "ACL-10", "counters_per_entry": True}, + {"counters_per_entry": False}, + {"name": "ACL-05", "counters_per_entry": False}, + ], + "name", + True, + True, + None, + pytest.raises(Exception, match="Missing key 'name' in item to sort "), + id="list-of-dict-with-sort-key-strict-mode", + ), ], ) - def test_natural_sort(self, item_to_natural_sort, sort_key, sorted_list): - resp = natural_sort(item_to_natural_sort, sort_key) - assert resp == sorted_list + def test_natural_sort(self, item_to_natural_sort, sort_key, strict, ignore_case, sorted_list, expected_raise): + with expected_raise: + resp = natural_sort(item_to_natural_sort, sort_key, strict=strict, ignore_case=ignore_case) + assert resp == sorted_list