From 294abed1383f6790a1380f5b621d1442f7002fa9 Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 22 Apr 2026 16:00:39 +0200 Subject: [PATCH] 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 | 4 +- scrapy/core/downloader/middleware.py | 4 +- scrapy/core/spidermw.py | 4 +- scrapy/extension.py | 4 +- scrapy/pipelines/__init__.py | 4 +- scrapy/settings/__init__.py | 28 ++++++++++-- tests/test_command_check.py | 8 +++- tests/test_settings/__init__.py | 68 ++++++++++++++++++++++++---- 8 files changed, 106 insertions(+), 18 deletions(-) diff --git a/scrapy/commands/check.py b/scrapy/commands/check.py index 229e8a487..2113c19d2 100644 --- a/scrapy/commands/check.py +++ b/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) diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py index 431579856..d08c6f8e3 100644 --- a/scrapy/core/downloader/middleware.py +++ b/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"): diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index 39342ebc0..67342099d 100644 --- a/scrapy/core/spidermw.py +++ b/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) diff --git a/scrapy/extension.py b/scrapy/extension.py index 9f978fa32..05cce326b 100644 --- a/scrapy/extension.py +++ b/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") + ) diff --git a/scrapy/pipelines/__init__.py b/scrapy/pipelines/__init__.py index f58864471..18473c534 100644 --- a/scrapy/pipelines/__init__.py +++ b/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"): diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index 456e90e77..2a1b1932c 100644 --- a/scrapy/settings/__init__.py +++ b/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 + `. + :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) diff --git a/tests/test_command_check.py b/tests/test_command_check.py index 84486e717..fd1ff612c 100644 --- a/tests/test_command_check.py +++ b/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) diff --git a/tests/test_settings/__init__.py b/tests/test_settings/__init__.py index 4436f03ba..1d4c3d487 100644 --- a/tests/test_settings/__init__.py +++ b/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):