diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index c330c4c35..598a746a9 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -5,12 +5,16 @@ import json import warnings from collections.abc import Iterable, Iterator, Mapping, MutableMapping from importlib import import_module +from logging import getLogger from pprint import pformat from typing import TYPE_CHECKING, Any, TypeAlias, cast from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.settings import default_settings from scrapy.utils.misc import load_object +from scrapy.utils.python import global_object_name + +logger = getLogger(__name__) # The key types are restricted in BaseSettings._get_key() to ones supported by JSON, # see https://github.com/scrapy/scrapy/issues/5383. @@ -319,10 +323,41 @@ class BaseSettings(MutableMapping[_SettingsKey, Any]): """ 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 + + normalized_keys = {} + obj_keys = set() + + def track_loaded_key(k: Any) -> None: + if k not in obj_keys: + obj_keys.add(k) + return + logger.warning( + f"Setting {name} contains multiple keys that refer to the " + f"same object: {global_object_name(k)}. Only the last one will " + f"be kept." + ) + + def normalize_key(key: Any) -> str: + try: + loaded_key = load_object(key) + except (AttributeError, TypeError, ValueError): + loaded_key = key + else: + import_path = global_object_name(loaded_key) + normalized_keys[import_path] = key + key = import_path + track_loaded_key(loaded_key) + return key + + def restore_key(k: str) -> Any: + return normalized_keys.get(k, k) + + result = dict(self[name + "_BASE"] or {}) + override = {normalize_key(k): v for k, v in (self[name] or {}).items()} + result.update(override) + return BaseSettings( + {restore_key(k): v for k, v in result.items() if v is not None} + ) def getpriority(self, name: _SettingsKey) -> int | None: """ diff --git a/tests/test_settings/__init__.py b/tests/test_settings/__init__.py index e97e114d6..4436f03ba 100644 --- a/tests/test_settings/__init__.py +++ b/tests/test_settings/__init__.py @@ -1,6 +1,7 @@ # pylint: disable=unsubscriptable-object,unsupported-membership-test,use-implicit-booleaness-not-comparison # (too many false positives) +import logging import warnings from unittest import mock @@ -14,11 +15,18 @@ from scrapy.settings import ( SettingsAttribute, get_settings_priority, ) -from scrapy.utils.misc import build_from_crawler +from scrapy.settings import default_settings as scrapy_default_settings +from scrapy.utils.misc import build_from_crawler, load_object from scrapy.utils.test import get_crawler from . import default_settings +NON_COMPONENT_PRIORITY_DICT_BASE_SETTING_NAMES = { + "DOWNLOAD_HANDLERS_BASE", + "FEED_EXPORTERS_BASE", + "FEED_STORAGES_BASE", +} + class TestSettingsGlobalFuncs: def test_get_settings_priority(self): @@ -402,6 +410,97 @@ class TestBaseSettings: assert frozencopy.frozen assert frozencopy is not self.settings + def test_getwithbase_override_none_by_type(self): + settings = BaseSettings() + setting_names = set() + for k, v in scrapy_default_settings.__dict__.items(): + if ( + not k.endswith("_BASE") + or k in NON_COMPONENT_PRIORITY_DICT_BASE_SETTING_NAMES + ): + continue + settings[k] = v + setting_name = k[: -len("_BASE")] + setting_names.add(setting_name) + settings[setting_name] = { + load_object(import_path): None for import_path in v + } + for setting_name in setting_names: + value = settings.getwithbase(setting_name) + assert not dict(value) + + def test_getwithbase_override_value_by_type(self): + settings = BaseSettings() + setting_names = set() + value = 0 + for k, v in scrapy_default_settings.__dict__.items(): + if ( + not k.endswith("_BASE") + or k in NON_COMPONENT_PRIORITY_DICT_BASE_SETTING_NAMES + ): + continue + settings[k] = v + setting_name = k[: -len("_BASE")] + setting_names.add(setting_name) + settings[setting_name] = { + load_object(import_path): value for import_path in v + } + for setting_name in setting_names: + assert settings.getwithbase(setting_name) == settings[setting_name] + + def test_getwithbase_for_non_component_priority_dicts(self): + settings = BaseSettings() + for base_name in NON_COMPONENT_PRIORITY_DICT_BASE_SETTING_NAMES: + base_value = getattr(scrapy_default_settings, base_name) + settings[base_name] = BaseSettings(base_value) + assert len(base_value) >= 2 + keys = list(base_value) + values = list(base_value.values()) + override = {keys[0]: values[1]} + expected = dict(base_value) + expected[keys[0]] = values[1] + name = base_name[: -len("_BASE")] + settings[name] = BaseSettings(override) + value = settings.getwithbase(name) + assert isinstance(value, BaseSettings) + assert dict(value) == expected + + def test_getwithbase_warns_on_duplicate_import_paths(self, caplog): + settings = BaseSettings() + settings["FOO"] = BaseSettings( + { + "scrapy.Request": 1, + "scrapy.http.Request": 2, + } + ) + with caplog.at_level(logging.WARNING): + value = settings.getwithbase("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): + settings = BaseSettings() + settings["FOO"] = BaseSettings( + {Component1: 1, "tests.test_settings.Component1": 2} + ) + with caplog.at_level(logging.WARNING): + value = settings.getwithbase("FOO") + assert isinstance(value, BaseSettings) + assert dict(value) == {"tests.test_settings.Component1": 2} + assert caplog.records, "Expected a warning to be logged" + msg = caplog.records[0].message + assert "tests.test_settings.Component1" in msg + + def test_getwithbase_invalid_setting_name(self): + settings = BaseSettings() + with pytest.raises( + ValueError, match="Base setting key must be a string, got 123" + ): + settings.getwithbase(123) + class TestSettings: def setup_method(self):