fix: preserve the order of elements of the list extract from defaults.ini

Author: Trong Nhan Mai

src/macaron/config/defaults.py

@@ -18,13 +18,12 @@ class ConfigParser(configparser.ConfigParser):
     def get_list(
         self,
         section: str,
-        item: str,
+        option: str,
         delimiter: str | None = "\n",
         fallback: list[str] | None = None,
-        duplicated_ok: bool = False,
         strip: bool = True,
     ) -> list[str]:
-        r"""Parse and return a list of strings from an item in ``defaults.ini``.
+        r"""Parse and return a list of strings from an ``option`` for ``section`` in ``defaults.ini``.
 
         This method uses str.split() to split the value into list of strings.
         References: https://docs.python.org/3/library/stdtypes.html#str.split.
@@ -37,22 +36,19 @@ def get_list(
         If ``strip`` is True  (default: True), strings are whitespace-stripped and empty strings
         are removed from the final result.
 
-        If ``duplicated_ok`` is True (default: False), duplicated values are not removed from the final list.
-
+        The order of non-empty elements in the list is preserved.
         The content of each string in the list is not validated and should be handled separately.
 
         Parameters
         ----------
         section : str
             The section in ``defaults.ini``.
-        item : str
-            The item to parse the list.
+        option : str
+            The option whose value will be split into the return list of strings.
         delimiter : str | None
             The delimiter used to split the strings.
         fallback : list | None
             The fallback value in case of errors.
-        duplicated_ok : bool
-            If True allow duplicate values.
         strip: bool
             If True, strings are whitespace-stripped and any empty strings are removed.
 
@@ -79,20 +75,15 @@ def get_list(
             allowed_hosts == ["github.com", "boo.com gitlab.com", "host com"]
         """
         try:
-            value = self.get(section, item)
+            value = self.get(section, option)
             if isinstance(value, str):
                 content = value.split(sep=delimiter)
 
                 if strip:
                     content = [x.strip() for x in content if x.strip()]
 
-                if duplicated_ok:
-                    return content
-
-                distinct_values = set()
-                distinct_values.update(content)
-                return list(distinct_values)
-        except configparser.NoOptionError as error:
+                return content
+        except (configparser.NoOptionError, configparser.NoSectionError) as error:
             logger.error(error)
 
         return fallback or []

src/macaron/repo_finder/repo_finder_java.py

@@ -119,7 +119,6 @@ def _create_urls(self, group: str, artifact: str, version: str) -> list[str]:
             "repofinder.java",
             "artifact_repositories",
             fallback=["https://repo.maven.apache.org/maven2"],
-            duplicated_ok=True,
         )
         urls = []
         for repo in repositories:
@@ -163,7 +162,7 @@ def _read_pom(self, pom: str) -> list[str]:
             The extracted contents as a list of strings.
         """
         # Retrieve tags
-        tags = defaults.get_list("repofinder.java", "repo_pom_paths", duplicated_ok=True)
+        tags = defaults.get_list("repofinder.java", "repo_pom_paths")
         if not any(tags):
             logger.debug("No POM tags found for URL discovery.")
             return []

src/macaron/slsa_analyzer/registry.py

@@ -680,8 +680,8 @@ def prepare(self) -> bool:
             logger.error("Found circular dependencies in registered checks: %s", str(error))
             return False
 
-        ex_pats = defaults.get_list(section="analysis.checks", item="exclude", fallback=[])
-        in_pats = defaults.get_list(section="analysis.checks", item="include", fallback=["*"])
+        ex_pats = defaults.get_list(section="analysis.checks", option="exclude", fallback=[])
+        in_pats = defaults.get_list(section="analysis.checks", option="include", fallback=["*"])
         try:
             checks_to_run = self.get_final_checks(ex_pats, in_pats)
         except CheckRegistryError as error:

tests/config/test_defaults.py

@@ -1,13 +1,14 @@
-# Copyright (c) 2022 - 2023, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2022 - 2024, Oracle and/or its affiliates. All rights reserved.
 # Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl/.
 
 """This module tests the defaults module."""
 
 import os
+from pathlib import Path
 
 import pytest
 
-from macaron.config.defaults import ConfigParser, create_defaults, defaults, load_defaults
+from macaron.config.defaults import create_defaults, defaults, load_defaults
 from macaron.config.global_config import global_config
 
 
@@ -41,93 +42,194 @@ def test_create_defaults_without_permission() -> None:
 
 
 @pytest.mark.parametrize(
-    ("section", "item", "delimiter", "strip", "duplicated_ok", "expect"),
+    ("user_config_input", "delimiter", "strip", "expect"),
     [
         (
-            "test.list",
-            "commas_string",
+            """
+            [test.list]
+            list = ,github.com, gitlab.com, space string, space string
+            """,
             ",",
             False,
+            ["", "github.com", " gitlab.com", " space string", " space string"],
+        ),
+        (
+            """
+            [test.list]
+            list = ,github.com, gitlab.com, space string, space string
+            """,
+            ",",
             True,
-            ["", " gitlab.com", " space string", " space string", "github.com"],
+            ["github.com", "gitlab.com", "space string", "space string"],
         ),
-        ("test.list", "commas_string", ",", False, False, ["", " gitlab.com", " space string", "github.com"]),
-        ("test.list", "commas_string", ",", True, True, ["github.com", "gitlab.com", "space string", "space string"]),
-        ("test.list", "commas_string", ",", True, False, ["github.com", "gitlab.com", "space string"]),
-        # Using None as the delimiter parameter will ignore cleanup
-        ("test.list", "default", None, True, False, ["comma_ended,", "github.com", "space", "string"]),
-        ("test.list", "default", None, False, False, ["comma_ended,", "github.com", "space", "string"]),
+        # Using None as the `delimiter` will also remove leading and trailing spaces of elements. Therefore,
+        # the results must be the same whether `strip` is set to True or False.
         (
-            "test.list",
-            "default",
+            """
+            [test.list]
+            list =
+                github.com
+                comma_ended,
+                space string
+                space string
+            """,
             None,
-            False,
             True,
-            ["comma_ended,", "github.com", "space", "space", "string", "string"],
+            ["github.com", "comma_ended,", "space", "string", "space", "string"],
         ),
-        ("test.list", "one_line", None, True, False, ["comma_ended,", "github.com", "space", "string"]),
         (
-            "test.list",
-            "one_line",
+            """
+            [test.list]
+            list =
+                github.com
+                comma_ended,
+                space string
+                space string
+            """,
+            None,
+            False,
+            ["github.com", "comma_ended,", "space", "string", "space", "string"],
+        ),
+        (
+            """
+            [test.list]
+            list = github.com comma_ended, space string space string
+            """,
             None,
             True,
-            True,
-            ["comma_ended,", "github.com", "space", "space", "string", "string"],
+            ["github.com", "comma_ended,", "space", "string", "space", "string"],
         ),
     ],
 )
 def test_get_str_list_with_custom_delimiter(
-    section: str, item: str, delimiter: str, strip: bool, duplicated_ok: bool, expect: list[str]
+    user_config_input: str,
+    delimiter: str,
+    strip: bool,
+    expect: list[str],
+    tmp_path: Path,
 ) -> None:
     """Test getting a list of strings from defaults.ini using a custom delimiter."""
-    content = """
-    [test.list]
-    default =
-        github.com
-        comma_ended,
-        space string
-        space string
-    empty =
-    one_line = github.com comma_ended, space string space string
-    commas_string = ,github.com, gitlab.com, space string, space string
-    """
-    custom_defaults = ConfigParser()
-    custom_defaults.read_string(content)
+    user_config_path = os.path.join(tmp_path, "config.ini")
+    with open(user_config_path, "w", encoding="utf-8") as user_config_file:
+        user_config_file.write(user_config_input)
+    load_defaults(user_config_path)
 
-    results = custom_defaults.get_list(section, item, delimiter=delimiter, strip=strip, duplicated_ok=duplicated_ok)
-    results.sort()
+    results = defaults.get_list(section="test.list", option="list", delimiter=delimiter, strip=strip)
     assert results == expect
 
 
 @pytest.mark.parametrize(
-    ("section", "item", "strip", "duplicated_ok", "fallback", "expect"),
+    ("user_config_input", "expect"),
     [
-        ("test.list", "default", True, False, [], ["comma_ended,", "github.com", "space string"]),
-        ("test.list", "default", True, True, [], ["comma_ended,", "github.com", "space string", "space string"]),
-        ("test.list", "empty", False, True, [], [""]),
-        ("test.list", "empty", True, True, [], []),
-        # Test for an item that does not exist in defaults.ini
-        ("test.list", "item_not_exist", True, True, [], []),
-        # Test value for fallback. The fallback value must be returned as is and shouldn't be modified by the method.
-        ("test.list", "item_not_exist", True, True, ["", "fallback_val"], ["", "fallback_val"]),
+        (
+            """
+            [test.list]
+            list = ,github.com, gitlab.com, space string, space string
+            """,
+            [",github.com, gitlab.com, space string, space string"],
+        ),
+        (
+            """
+            [test.list]
+            list =
+                github.com
+                comma_ended,
+                space string
+                space string
+            """,
+            ["github.com", "comma_ended,", "space string", "space string"],
+        ),
+        (
+            """
+            [test.list]
+            list =
+            """,
+            [],
+        ),
     ],
 )
-def test_get_str_list_with_default_delimiter(
-    section: str, item: str, strip: bool, duplicated_ok: bool, fallback: list[str], expect: list[str]
+def test_get_str_list_default(
+    user_config_input: str,
+    expect: list[str],
+    tmp_path: Path,
 ) -> None:
-    """Test getting a list of strings from defaults.ini using the default delimiter."""
+    """Test default behavior of getting a list of strings from an option in defaults.ini."""
+    user_config_path = os.path.join(tmp_path, "config.ini")
+    with open(user_config_path, "w", encoding="utf-8") as user_config_file:
+        user_config_file.write(user_config_input)
+    load_defaults(user_config_path)
+
+    results = defaults.get_list(section="test.list", option="list")
+    assert results == expect
+
+
+@pytest.mark.parametrize(
+    ("section", "option", "fallback", "expect"),
+    [
+        (
+            "section",
+            "non-existing",
+            None,
+            [],
+        ),
+        (
+            "non-existing",
+            "option",
+            None,
+            [],
+        ),
+        (
+            "non-existing",
+            "non-existing",
+            None,
+            [],
+        ),
+        (
+            "section",
+            "non-existing",
+            ["some", "fallback", "value"],
+            ["some", "fallback", "value"],
+        ),
+        (
+            "non-existing",
+            "option",
+            ["some", "fallback", "value"],
+            ["some", "fallback", "value"],
+        ),
+        (
+            "non-existing",
+            "non-existing",
+            ["some", "fallback", "value"],
+            ["some", "fallback", "value"],
+        ),
+    ],
+)
+def test_get_str_list_default_with_errors(
+    section: str,
+    option: str,
+    fallback: list[str] | None,
+    expect: list[str],
+    tmp_path: Path,
+) -> None:
+    """Test default behavior of getting a list of strings with errors from defaults.ini."""
     content = """
-    [test.list]
-    default =
+    [section]
+    option =
         github.com
         comma_ended,
         space string
         space string
-    empty =
     """
-    custom_defaults = ConfigParser()
-    custom_defaults.read_string(content)
-
-    results = custom_defaults.get_list(section, item, strip=strip, fallback=fallback, duplicated_ok=duplicated_ok)
-    results.sort()
-    assert results == expect
+    user_config_path = os.path.join(tmp_path, "config.ini")
+    with open(user_config_path, "w", encoding="utf-8") as user_config_file:
+        user_config_file.write(content)
+    load_defaults(user_config_path)
+
+    assert (
+        defaults.get_list(
+            section=section,
+            option=option,
+            fallback=fallback,
+        )
+        == expect
+    )