Restore 2.14 getwithbase, add a new method for class key deduplication (#7449)

* Restore 2.14 getwithbase, add a new method for class key deduplication

* Use the original getwithbase code, not some equivalent

* Fix mocking

* Test key exceptions

scrapy/commands/check.py

@@ -73,7 +73,9 @@ class Command(ScrapyCommand):
     def run(self, args: list[str], opts: argparse.Namespace) -> None:
         # load contracts
         assert self.settings is not None
-        contracts = build_component_list(self.settings.getwithbase("SPIDER_CONTRACTS"))
+        contracts = build_component_list(
+            self.settings.get_component_priority_dict_with_base("SPIDER_CONTRACTS")
+        )
         conman = ContractsManager(load_object(c) for c in contracts)
         runner = TextTestRunner(verbosity=2 if opts.verbose else 1)
         result = TextTestResult(runner.stream, runner.descriptions, runner.verbosity)

scrapy/core/downloader/middleware.py

@@ -36,7 +36,9 @@ class DownloaderMiddlewareManager(MiddlewareManager):
 
     @classmethod
     def _get_mwlist_from_settings(cls, settings: BaseSettings) -> list[Any]:
-        return build_component_list(settings.getwithbase("DOWNLOADER_MIDDLEWARES"))
+        return build_component_list(
+            settings.get_component_priority_dict_with_base("DOWNLOADER_MIDDLEWARES")
+        )
 
     def _add_middleware(self, mw: Any) -> None:
         if hasattr(mw, "process_request"):

scrapy/core/spidermw.py

@@ -56,7 +56,9 @@ class SpiderMiddlewareManager(MiddlewareManager):
 
     @classmethod
     def _get_mwlist_from_settings(cls, settings: BaseSettings) -> list[Any]:
-        return build_component_list(settings.getwithbase("SPIDER_MIDDLEWARES"))
+        return build_component_list(
+            settings.get_component_priority_dict_with_base("SPIDER_MIDDLEWARES")
+        )
 
     def __init__(self, *middlewares: Any, crawler: Crawler | None = None) -> None:
         self._check_deprecated_process_start_requests_use(middlewares)

scrapy/extension.py

@@ -20,4 +20,6 @@ class ExtensionManager(MiddlewareManager):
 
     @classmethod
     def _get_mwlist_from_settings(cls, settings: Settings) -> list[Any]:
-        return build_component_list(settings.getwithbase("EXTENSIONS"))
+        return build_component_list(
+            settings.get_component_priority_dict_with_base("EXTENSIONS")
+        )

scrapy/pipelines/__init__.py

@@ -33,7 +33,9 @@ class ItemPipelineManager(MiddlewareManager):
 
     @classmethod
     def _get_mwlist_from_settings(cls, settings: Settings) -> list[Any]:
-        return build_component_list(settings.getwithbase("ITEM_PIPELINES"))
+        return build_component_list(
+            settings.get_component_priority_dict_with_base("ITEM_PIPELINES")
+        )
 
     def _add_middleware(self, pipe: Any) -> None:
         if hasattr(pipe, "open_spider"):

scrapy/settings/__init__.py

@@ -323,12 +323,34 @@ class BaseSettings(MutableMapping[_SettingsKey, Any]):
         return copy.deepcopy(value)
 
     def getwithbase(self, name: _SettingsKey) -> BaseSettings:
-        """Get a composition of a dictionary-like setting and its `_BASE`
+        """Get a composition of a dictionary-like setting and its ``_BASE``
         counterpart.
 
+        Use
+        :meth:`~scrapy.settings.BaseSettings.get_component_priority_dict_with_base`
+        instead if the setting is a :ref:`component priority dictionary
+        <component-priority-dictionaries>`.
+
         :param name: name of the dictionary-like setting
         :type name: str
         """
+        if not isinstance(name, str):
+            raise ValueError(f"Base setting key must be a string, got {name}")
+        compbs = BaseSettings()
+        compbs.update(self[name + "_BASE"])
+        compbs.update(self[name])
+        return compbs
+
+    def get_component_priority_dict_with_base(self, name: _SettingsKey) -> BaseSettings:
+        """Get a composition of a component priority dictionary setting and
+        its ``_BASE`` counterpart.
+
+        Keys are resolved to their import path for deduplication and then
+        restored to their latest input representation.
+
+        :param name: name of the component priority dictionary setting
+        :type name: str
+        """
         if not isinstance(name, str):
             raise ValueError(f"Base setting key must be a string, got {name}")
 
@@ -345,10 +367,10 @@ class BaseSettings(MutableMapping[_SettingsKey, Any]):
                 f"be kept."
             )
 
-        def normalize_key(key: Any) -> str:
+        def normalize_key(key: Any) -> Any:
             try:
                 loaded_key = load_object(key)
-            except (AttributeError, TypeError, ValueError):
+            except (NameError, TypeError, ValueError):
                 loaded_key = key
             else:
                 import_path = global_object_name(loaded_key)

tests/test_command_check.py

@@ -186,7 +186,9 @@ class CheckSpider(scrapy.Spider):
         output = StringIO()
         sys.stdout = output
         cmd = Command()
-        cmd.settings = Mock(getwithbase=Mock(return_value={}))
+        cmd.settings = Mock(
+            get_component_priority_dict_with_base=Mock(return_value={}),
+        )
         cm_cls_mock.return_value = cm_mock = Mock()
         spider_loader_mock = Mock()
         cmd.crawler_process = Mock(spider_loader=spider_loader_mock)
@@ -211,7 +213,9 @@ class CheckSpider(scrapy.Spider):
         self, cm_cls_mock
     ) -> None:
         cmd = Command()
-        cmd.settings = Mock(getwithbase=Mock(return_value={}))
+        cmd.settings = Mock(
+            get_component_priority_dict_with_base=Mock(return_value={}),
+        )
         cm_cls_mock.return_value = cm_mock = Mock()
         spider_loader_mock = Mock()
         cmd.crawler_process = Mock(spider_loader=spider_loader_mock)

tests/test_settings/__init__.py

@@ -410,7 +410,45 @@ class TestBaseSettings:
         assert frozencopy.frozen
         assert frozencopy is not self.settings
 
-    def test_getwithbase_override_none_by_type(self):
+    def test_getwithbase_for_dotted_keys(self):
+        settings = BaseSettings(
+            {
+                "FEED_EXPORTERS_BASE": BaseSettings({"json": "foo"}),
+                "FEED_EXPORTERS": BaseSettings({"csv.gz": "bar"}),
+            }
+        )
+        value = settings.getwithbase("FEED_EXPORTERS")
+        assert isinstance(value, BaseSettings)
+        assert dict(value) == {
+            "json": "foo",
+            "csv.gz": "bar",
+        }
+
+    @pytest.mark.parametrize(
+        ("key", "exception"),
+        [
+            pytest.param(1, TypeError, id="type-error"),
+            pytest.param("foo", ValueError, id="value-error"),
+            pytest.param("csv.gz", NameError, id="name-error"),
+        ],
+    )
+    def test_get_component_priority_dict_with_base_handles_load_object_exceptions(
+        self, key, exception
+    ):
+        with pytest.raises(exception):
+            load_object(key)
+
+        settings = BaseSettings(
+            {
+                "FOO": BaseSettings({key: 1}),
+            }
+        )
+        value = settings.get_component_priority_dict_with_base("FOO")
+
+        assert isinstance(value, BaseSettings)
+        assert dict(value) == {key: 1}
+
+    def test_get_component_priority_dict_with_base_override_none_by_type(self):
         settings = BaseSettings()
         setting_names = set()
         for k, v in scrapy_default_settings.__dict__.items():
@@ -426,10 +464,10 @@ class TestBaseSettings:
                 load_object(import_path): None for import_path in v
             }
         for setting_name in setting_names:
-            value = settings.getwithbase(setting_name)
+            value = settings.get_component_priority_dict_with_base(setting_name)
             assert not dict(value)
 
-    def test_getwithbase_override_value_by_type(self):
+    def test_get_component_priority_dict_with_base_override_value_by_type(self):
         settings = BaseSettings()
         setting_names = set()
         value = 0
@@ -446,7 +484,10 @@ class TestBaseSettings:
                 load_object(import_path): value for import_path in v
             }
         for setting_name in setting_names:
-            assert settings.getwithbase(setting_name) == settings[setting_name]
+            assert (
+                settings.get_component_priority_dict_with_base(setting_name)
+                == settings[setting_name]
+            )
 
     def test_getwithbase_for_non_component_priority_dicts(self):
         settings = BaseSettings()
@@ -465,7 +506,9 @@ class TestBaseSettings:
             assert isinstance(value, BaseSettings)
             assert dict(value) == expected
 
-    def test_getwithbase_warns_on_duplicate_import_paths(self, caplog):
+    def test_get_component_priority_dict_with_base_warns_on_duplicate_import_paths(
+        self, caplog
+    ):
         settings = BaseSettings()
         settings["FOO"] = BaseSettings(
             {
@@ -474,20 +517,22 @@ class TestBaseSettings:
             }
         )
         with caplog.at_level(logging.WARNING):
-            value = settings.getwithbase("FOO")
+            value = settings.get_component_priority_dict_with_base("FOO")
         assert isinstance(value, BaseSettings)
         assert dict(value) == {"scrapy.http.Request": 2}
         assert caplog.records, "Expected a warning to be logged"
         msg = caplog.records[0].message
         assert "scrapy.http.request.Request" in msg
 
-    def test_getwithbase_warns_on_duplicate_mixed_type_and_path(self, caplog):
+    def test_get_component_priority_dict_with_base_warns_on_duplicate_mixed_type_and_path(
+        self, caplog
+    ):
         settings = BaseSettings()
         settings["FOO"] = BaseSettings(
             {Component1: 1, "tests.test_settings.Component1": 2}
         )
         with caplog.at_level(logging.WARNING):
-            value = settings.getwithbase("FOO")
+            value = settings.get_component_priority_dict_with_base("FOO")
         assert isinstance(value, BaseSettings)
         assert dict(value) == {"tests.test_settings.Component1": 2}
         assert caplog.records, "Expected a warning to be logged"
@@ -501,6 +546,13 @@ class TestBaseSettings:
         ):
             settings.getwithbase(123)
 
+    def test_get_component_priority_dict_with_base_invalid_setting_name(self):
+        settings = BaseSettings()
+        with pytest.raises(
+            ValueError, match="Base setting key must be a string, got 123"
+        ):
+            settings.get_component_priority_dict_with_base(123)
+
 
 class TestSettings:
     def setup_method(self):