refactor: simplify config a bit more

scastlara · scastlara · commit ab72bd8ea275 · 2024-11-15T21:58:31.000+01:00
diff --git a/src/twyn/cli.py b/src/twyn/cli.py
@@ -9,7 +9,7 @@
     SELECTOR_METHOD_MAPPING,
     AvailableLoggingLevels,
 )
-from twyn.core.config_handler import ConfigHandler
+from twyn.config.config_handler import ConfigHandler
 from twyn.main import check_dependencies
 
 
diff --git a/src/twyn/config/__init__.py b/src/twyn/config/__init__.py
diff --git a/src/twyn/config/config_handler.py b/src/twyn/config/config_handler.py
@@ -3,7 +3,7 @@
 from enum import Enum
 from os import getcwd
 from pathlib import Path
-from typing import Any, Optional
+from typing import Optional
 
 from tomlkit import TOMLDocument, dumps, parse, table
 
@@ -13,7 +13,7 @@
     DEFAULT_TOP_PYPI_PACKAGES,
     AvailableLoggingLevels,
 )
-from twyn.core.exceptions import (
+from twyn.config.exceptions import (
     AllowlistPackageAlreadyExistsError,
     AllowlistPackageDoesNotExistError,
     TOMLError,
@@ -33,7 +33,7 @@ class TwynConfiguration:
     pypi_reference: str
 
 
-@dataclass(frozen=True)
+@dataclass
 class ReadTwynConfiguration:
     """Configuration for twyn as set by the user. It may have None values."""
 
@@ -65,16 +65,14 @@ def resolve_config(
         It will also handle default values, when appropriate.
         """
         toml = self._read_toml()
-        twyn_config_data = self._get_twyn_data_from_toml(toml)
+        read_config = self._get_read_config(toml)
 
-        # Resolve the configuration so that it is ready to be used by Twyn,
-        # handling defaults etc
         return TwynConfiguration(
-            dependency_file=dependency_file or twyn_config_data.get("dependency_file"),
-            selector_method=selector_method or twyn_config_data.get("selector_method", DEFAULT_SELECTOR_METHOD),
-            logging_level=_get_logging_level(verbosity, twyn_config_data.get("logging_level")),
-            allowlist=set(twyn_config_data.get("allowlist", set())),
-            pypi_reference=twyn_config_data.get("pypi_reference", DEFAULT_TOP_PYPI_PACKAGES),
+            dependency_file=dependency_file or read_config.dependency_file,
+            selector_method=selector_method or read_config.selector_method or DEFAULT_SELECTOR_METHOD,
+            logging_level=_get_logging_level(verbosity, read_config.logging_level),
+            allowlist=read_config.allowlist,
+            pypi_reference=read_config.pypi_reference or DEFAULT_TOP_PYPI_PACKAGES,
         )
 
     def add_package_to_allowlist(self, package_name: str) -> None:
@@ -84,14 +82,8 @@ def add_package_to_allowlist(self, package_name: str) -> None:
         if package_name in config.allowlist:
             raise AllowlistPackageAlreadyExistsError(package_name)
 
-        new_config = ReadTwynConfiguration(
-            dependency_file=config.dependency_file,
-            selector_method=config.selector_method,
-            logging_level=config.logging_level,
-            allowlist=config.allowlist | {package_name},
-            pypi_reference=config.pypi_reference,
-        )
-        self._write_config(toml, new_config)
+        config.allowlist.add(package_name)
+        self._write_config(toml, config)
         logger.info(f"Package '{package_name}' successfully added to allowlist")
 
     def remove_package_from_allowlist(self, package_name: str) -> None:
@@ -101,19 +93,13 @@ def remove_package_from_allowlist(self, package_name: str) -> None:
         if package_name not in config.allowlist:
             raise AllowlistPackageDoesNotExistError(package_name)
 
-        new_config = ReadTwynConfiguration(
-            dependency_file=config.dependency_file,
-            selector_method=config.selector_method,
-            logging_level=config.logging_level,
-            allowlist=config.allowlist - {package_name},
-            pypi_reference=config.pypi_reference,
-        )
-        self._write_config(toml, new_config)
+        config.allowlist.remove(package_name)
+        self._write_config(toml, config)
         logger.info(f"Package '{package_name}' successfully removed from allowlist")
 
     def _get_read_config(self, toml: TOMLDocument) -> ReadTwynConfiguration:
         """Read the twyn configuration from a provided toml document."""
-        twyn_config_data = self._get_twyn_data_from_toml(toml)
+        twyn_config_data = toml.get("tool", {}).get("twyn", {})
         return ReadTwynConfiguration(
             dependency_file=twyn_config_data.get("dependency_file"),
             selector_method=twyn_config_data.get("selector_method"),
@@ -145,10 +131,7 @@ def _read_toml(self) -> TOMLDocument:
 
         with open(fp, "r") as f:
             content = parse(f.read())
-        return parse(dumps(content))
-
-    def _get_twyn_data_from_toml(self, toml: TOMLDocument) -> dict[str, Any]:
-        return toml.get("tool", {}).get("twyn", {})
+        return content
 
     def _get_toml_file_pointer(self) -> Path:
         """Create a path for the toml file with the format <current working directory>/self.file_path."""
diff --git a/src/twyn/config/exceptions.py b/src/twyn/config/exceptions.py
diff --git a/src/twyn/main.py b/src/twyn/main.py
@@ -9,7 +9,7 @@
     SELECTOR_METHOD_MAPPING,
     AvailableLoggingLevels,
 )
-from twyn.core.config_handler import ConfigHandler
+from twyn.config.config_handler import ConfigHandler
 from twyn.dependency_parser.dependency_selector import DependencySelector
 from twyn.similarity.algorithm import EditDistance, SimilarityThreshold
 from twyn.trusted_packages import TopPyPiReference
diff --git a/tests/core/test_config_handler.py b/tests/core/test_config_handler.py
@@ -5,8 +5,8 @@
 import pytest
 from tomlkit import TOMLDocument, dumps, parse
 from twyn.base.constants import DEFAULT_TOP_PYPI_PACKAGES, AvailableLoggingLevels
-from twyn.core.config_handler import ConfigHandler, ReadTwynConfiguration, TwynConfiguration
-from twyn.core.exceptions import (
+from twyn.config.config_handler import ConfigHandler, ReadTwynConfiguration, TwynConfiguration
+from twyn.config.exceptions import (
     AllowlistPackageAlreadyExistsError,
     AllowlistPackageDoesNotExistError,
     TOMLError,
@@ -17,13 +17,13 @@ class TestConfig:
     def throw_exception(self):
         raise FileNotFoundError
 
-    @patch("twyn.core.config_handler.ConfigHandler._get_toml_file_pointer")
+    @patch("twyn.config.config_handler.ConfigHandler._get_toml_file_pointer")
     def test_enforce_file_error(self, mock_is_file):
         mock_is_file.side_effect = self.throw_exception
         with pytest.raises(TOMLError):
             ConfigHandler(enforce_file=True).resolve_config()
 
-    @patch("twyn.core.config_handler.ConfigHandler._get_toml_file_pointer")
+    @patch("twyn.config.config_handler.ConfigHandler._get_toml_file_pointer")
     def test_no_enforce_file_on_non_existent_file(self, mock_is_file):
         """Resolving the config without enforcing the file to be present gives you defaults."""
         mock_is_file.side_effect = self.throw_exception
@@ -103,8 +103,8 @@ def test_write_toml(self, pyproject_toml_file):
 
 
 class TestAllowlistConfigHandler:
-    @patch("twyn.core.config_handler.ConfigHandler._write_toml")
-    @patch("twyn.core.config_handler.ConfigHandler._read_toml")
+    @patch("twyn.config.config_handler.ConfigHandler._write_toml")
+    @patch("twyn.config.config_handler.ConfigHandler._read_toml")
     def test_allowlist_add(self, mock_toml, mock_write_toml):
         mock_toml.return_value = TOMLDocument()
 
@@ -117,8 +117,8 @@ def test_allowlist_add(self, mock_toml, mock_write_toml):
         assert final_toml == {"tool": {"twyn": {"allowlist": ["mypackage"]}}}
         assert mock_write_toml.called
 
-    @patch("twyn.core.config_handler.ConfigHandler._write_toml")
-    @patch("twyn.core.config_handler.ConfigHandler._read_toml")
+    @patch("twyn.config.config_handler.ConfigHandler._write_toml")
+    @patch("twyn.config.config_handler.ConfigHandler._read_toml")
     def test_allowlist_add_duplicate_error(self, mock_toml, mock_write_toml):
         mock_toml.return_value = parse(dumps({"tool": {"twyn": {"allowlist": ["mypackage"]}}}))
 
@@ -131,8 +131,8 @@ def test_allowlist_add_duplicate_error(self, mock_toml, mock_write_toml):
 
         assert not mock_write_toml.called
 
-    @patch("twyn.core.config_handler.ConfigHandler._write_toml")
-    @patch("twyn.core.config_handler.ConfigHandler._read_toml")
+    @patch("twyn.config.config_handler.ConfigHandler._write_toml")
+    @patch("twyn.config.config_handler.ConfigHandler._read_toml")
     def test_allowlist_remove_completely(self, mock_toml, mock_write_toml):
         mock_toml.return_value = parse(dumps({"tool": {"twyn": {"allowlist": ["mypackage"]}}}))
 
@@ -141,8 +141,8 @@ def test_allowlist_remove_completely(self, mock_toml, mock_write_toml):
         config.remove_package_from_allowlist("mypackage")
         assert config._read_toml() == {"tool": {"twyn": {}}}
 
-    @patch("twyn.core.config_handler.ConfigHandler._write_toml")
-    @patch("twyn.core.config_handler.ConfigHandler._read_toml")
+    @patch("twyn.config.config_handler.ConfigHandler._write_toml")
+    @patch("twyn.config.config_handler.ConfigHandler._read_toml")
     def test_allowlist_remove(self, mock_toml, mock_write_toml):
         mock_toml.return_value = parse(dumps({"tool": {"twyn": {"allowlist": ["mypackage", "another-package"]}}}))
 
@@ -151,8 +151,8 @@ def test_allowlist_remove(self, mock_toml, mock_write_toml):
         config.remove_package_from_allowlist("mypackage")
         assert config._read_toml() == {"tool": {"twyn": {"allowlist": ["another-package"]}}}
 
-    @patch("twyn.core.config_handler.ConfigHandler._write_toml")
-    @patch("twyn.core.config_handler.ConfigHandler._read_toml")
+    @patch("twyn.config.config_handler.ConfigHandler._write_toml")
+    @patch("twyn.config.config_handler.ConfigHandler._read_toml")
     def test_allowlist_remove_non_existent_package_error(self, mock_toml, mock_write_toml):
         mock_toml.return_value = parse(dumps({"tool": {"twyn": {"allowlist": ["mypackage"]}}}))
 
diff --git a/tests/main/test_cli.py b/tests/main/test_cli.py
@@ -7,7 +7,7 @@
 
 
 class TestCli:
-    @patch("twyn.core.config_handler.ConfigHandler.add_package_to_allowlist")
+    @patch("twyn.config.config_handler.ConfigHandler.add_package_to_allowlist")
     def test_allowlist_add_package_to_allowlist(self, mock_allowlist_add):
         runner = CliRunner()
         runner.invoke(
@@ -17,7 +17,7 @@ def test_allowlist_add_package_to_allowlist(self, mock_allowlist_add):
 
         assert mock_allowlist_add.call_args == call("requests")
 
-    @patch("twyn.core.config_handler.ConfigHandler.remove_package_from_allowlist")
+    @patch("twyn.config.config_handler.ConfigHandler.remove_package_from_allowlist")
     def test_allowlist_remove(self, mock_allowlist_add):
         runner = CliRunner()
         runner.invoke(
diff --git a/tests/main/test_main.py b/tests/main/test_main.py
@@ -1,8 +1,9 @@
 from unittest.mock import patch
 
 import pytest
+from tomlkit import dumps, parse
 from twyn.base.constants import DEFAULT_TOP_PYPI_PACKAGES, AvailableLoggingLevels
-from twyn.core.config_handler import ConfigHandler, TwynConfiguration, _get_logging_level
+from twyn.config.config_handler import ConfigHandler, TwynConfiguration, _get_logging_level
 from twyn.dependency_parser import RequirementsTxtParser
 from twyn.main import (
     check_dependencies,
@@ -27,7 +28,7 @@ class TestCheckDependencies:
                     "logging_level": "WARNING",
                     "selector_method": "nearby-letter",
                     "dependency_file": "poetry.lock",
-                    "allowlist": {"boto4", "boto2"},
+                    "allowlist": ["boto4", "boto2"],
                 },
                 TwynConfiguration(
                     dependency_file="requirements.txt",
@@ -45,7 +46,7 @@ class TestCheckDependencies:
                     "logging_level": "debug",
                     "selector_method": "nearby-letter",
                     "dependency_file": "poetry.lock",
-                    "allowlist": {"boto4", "boto2"},
+                    "allowlist": ["boto4", "boto2"],
                 },
                 TwynConfiguration(
                     dependency_file="poetry.lock",
@@ -75,13 +76,12 @@ class TestCheckDependencies:
                 },
                 {
                     "logging_level": "INFO",
-                    "selector_method": None,
                     "dependency_file": "poetry.lock",
-                    "allowlist": set(),
+                    "allowlist": [],
                 },
                 TwynConfiguration(
                     dependency_file="requirements.txt",
-                    selector_method=None,
+                    selector_method="all",
                     logging_level=AvailableLoggingLevels.debug,
                     allowlist=set(),
                     pypi_reference=DEFAULT_TOP_PYPI_PACKAGES,
@@ -106,7 +106,7 @@ def test_options_priorities_assignation(
         """
         handler = ConfigHandler(file_path=None, enforce_file=False)
 
-        with patch.object(handler, "_get_twyn_data_from_toml", return_value=file_config):
+        with patch.object(handler, "_read_toml", return_value=parse(dumps({"tool": {"twyn": file_config}}))):
             resolved = handler.resolve_config(
                 selector_method=cli_config.get("selector_method"),
                 dependency_file=cli_config.get("dependency_file"),
@@ -118,8 +118,6 @@ def test_options_priorities_assignation(
         assert resolved.logging_level == expected_resolved_config.logging_level
         assert resolved.allowlist == expected_resolved_config.allowlist
 
-        # assert mock_logging_level.call_args == call(log_level)
-
     @pytest.mark.parametrize(
         "passed_logging_level, config, logging_level",
         [

Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@`
`9`	`9`	`SELECTOR_METHOD_MAPPING,`
`10`	`10`	`AvailableLoggingLevels,`
`11`	`11`	`)`
`12`		`-from twyn.core.config_handler import ConfigHandler`
	`12`	`+from twyn.config.config_handler import ConfigHandler`
`13`	`13`	`from twyn.main import check_dependencies`
`14`	`14`
`15`	`15`