From bb06758a7b737befcb28b8fe202d2c6da767146d Mon Sep 17 00:00:00 2001 From: Martin Fischer Date: Tue, 4 Jan 2022 11:53:42 +0100 Subject: [PATCH 1/4] [refactor] add type hints & remove Setting._post_init Previously the Setting classes used a horrible _post_init hack that prevented proper type checking. --- searx/engines/__init__.py | 16 +-- searx/plugins/__init__.py | 12 +- searx/preferences.py | 229 ++++++++++++++++----------------- tests/unit/test_preferences.py | 15 +-- 4 files changed, 133 insertions(+), 139 deletions(-) diff --git a/searx/engines/__init__.py b/searx/engines/__init__.py index 7e8336e01..ca3b5d4a8 100644 --- a/searx/engines/__init__.py +++ b/searx/engines/__init__.py @@ -13,7 +13,7 @@ usage:: import sys import copy -from typing import List +from typing import Dict, List, Optional from os.path import realpath, dirname from babel.localedata import locale_identifiers @@ -67,10 +67,10 @@ class Engine: # pylint: disable=too-few-public-methods timeout: float -# Defaults for the namespace of an engine module, see :py:func:`load_engine`` +# Defaults for the namespace of an engine module, see :py:func:`load_engine` categories = {'general': []} -engines = {} +engines: Dict[str, Engine] = {} engine_shortcuts = {} """Simple map of registered *shortcuts* to name of the engine (or ``None``). @@ -81,7 +81,7 @@ engine_shortcuts = {} """ -def load_engine(engine_data): +def load_engine(engine_data: dict) -> Optional[Engine]: """Load engine from ``engine_data``. :param dict engine_data: Attributes from YAML ``settings:engines/`` @@ -157,7 +157,7 @@ def set_loggers(engine, engine_name): module.logger = logger.getChild(module_engine_name) -def update_engine_attributes(engine, engine_data): +def update_engine_attributes(engine: Engine, engine_data): # set engine attributes from engine_data for param_name, param_value in engine_data.items(): if param_name == 'categories': @@ -175,7 +175,7 @@ def update_engine_attributes(engine, engine_data): setattr(engine, arg_name, copy.deepcopy(arg_value)) -def set_language_attributes(engine): +def set_language_attributes(engine: Engine): # assign supported languages from json file if engine.name in ENGINES_LANGUAGES: engine.supported_languages = ENGINES_LANGUAGES[engine.name] @@ -248,7 +248,7 @@ def is_missing_required_attributes(engine): return missing -def is_engine_active(engine): +def is_engine_active(engine: Engine): # check if engine is inactive if engine.inactive is True: return False @@ -260,7 +260,7 @@ def is_engine_active(engine): return True -def register_engine(engine): +def register_engine(engine: Engine): if engine.name in engines: logger.error('Engine config error: ambigious name: {0}'.format(engine.name)) sys.exit(1) diff --git a/searx/plugins/__init__.py b/searx/plugins/__init__.py index 7815c2099..6c1bea8d0 100644 --- a/searx/plugins/__init__.py +++ b/searx/plugins/__init__.py @@ -10,10 +10,20 @@ from os.path import abspath, basename, dirname, exists, join from shutil import copyfile from pkgutil import iter_modules from logging import getLogger +from typing import List from searx import logger, settings +class Plugin: # pylint: disable=too-few-public-methods + """This class is currently never initialized and only used for type hinting.""" + + id: str + name: str + description: str + default_on: bool + + logger = logger.getChild("plugins") required_attrs = ( @@ -175,7 +185,7 @@ def load_and_initialize_plugin(plugin_module_name, external, init_args): class PluginStore: def __init__(self): - self.plugins = [] + self.plugins: List[Plugin] = [] def __iter__(self): for plugin in self.plugins: diff --git a/searx/preferences.py b/searx/preferences.py index b550e1ebd..bbcfc524e 100644 --- a/searx/preferences.py +++ b/searx/preferences.py @@ -8,8 +8,14 @@ from base64 import urlsafe_b64encode, urlsafe_b64decode from zlib import compress, decompress from urllib.parse import parse_qs, urlencode +from typing import Iterable, Dict, List, Set +from dataclasses import dataclass + +import flask from searx import settings, autocomplete +from searx.engines import Engine +from searx.plugins import Plugin from searx.locales import LOCALE_NAMES from searx.webutils import VALID_LANGUAGE_CODE from searx.engines import OTHER_CATEGORY @@ -21,31 +27,20 @@ ENABLED = 1 DOI_RESOLVERS = list(settings['doi_resolvers']) -class MissingArgumentException(Exception): - """Exption from ``cls._post_init`` when a argument is missed.""" - - class ValidationException(Exception): - """Exption from ``cls._post_init`` when configuration value is invalid.""" + """Exption from ``cls.__init__`` when configuration value is invalid.""" class Setting: """Base class of user settings""" - def __init__(self, default_value, locked=False, **kwargs): + def __init__(self, default_value, locked: bool = False): super().__init__() self.value = default_value self.locked = locked - for key, value in kwargs.items(): - setattr(self, key, value) - self._post_init() - - def _post_init(self): - pass - - def parse(self, data): + def parse(self, data: str): """Parse ``data`` and store the result at ``self.value`` If needed, its overwritten in the inheritance. @@ -59,7 +54,7 @@ class Setting: """ return self.value - def save(self, name, resp): + def save(self, name: str, resp: flask.Response): """Save cookie ``name`` in the HTTP reponse obect If needed, its overwritten in the inheritance.""" @@ -73,35 +68,35 @@ class StringSetting(Setting): class EnumStringSetting(Setting): """Setting of a value which can only come from the given choices""" - def _post_init(self): - if not hasattr(self, 'choices'): - raise MissingArgumentException('Missing argument: choices') + def __init__(self, default_value: str, choices: Iterable[str], locked=False): + super().__init__(default_value, locked) + self.choices = choices self._validate_selection(self.value) - def _validate_selection(self, selection): - if selection not in self.choices: # pylint: disable=no-member + def _validate_selection(self, selection: str): + if selection not in self.choices: raise ValidationException('Invalid value: "{0}"'.format(selection)) - def parse(self, data): + def parse(self, data: str): """Parse and validate ``data`` and store the result at ``self.value``""" self._validate_selection(data) self.value = data -class MultipleChoiceSetting(EnumStringSetting): +class MultipleChoiceSetting(Setting): """Setting of values which can only come from the given choices""" - def _validate_selections(self, selections): - for item in selections: - if item not in self.choices: # pylint: disable=no-member - raise ValidationException('Invalid value: "{0}"'.format(selections)) - - def _post_init(self): - if not hasattr(self, 'choices'): - raise MissingArgumentException('Missing argument: choices') + def __init__(self, default_value: List[str], choices: Iterable[str], locked=False): + super().__init__(default_value, locked) + self.choices = choices self._validate_selections(self.value) - def parse(self, data): + def _validate_selections(self, selections: List[str]): + for item in selections: + if item not in self.choices: + raise ValidationException('Invalid value: "{0}"'.format(selections)) + + def parse(self, data: str): """Parse and validate ``data`` and store the result at ``self.value``""" if data == '': self.value = [] @@ -111,16 +106,16 @@ class MultipleChoiceSetting(EnumStringSetting): self._validate_selections(elements) self.value = elements - def parse_form(self, data): + def parse_form(self, data: List[str]): if self.locked: return self.value = [] for choice in data: - if choice in self.choices and choice not in self.value: # pylint: disable=no-member + if choice in self.choices and choice not in self.value: self.value.append(choice) - def save(self, name, resp): + def save(self, name: str, resp: flask.Response): """Save cookie ``name`` in the HTTP reponse obect""" resp.set_cookie(name, ','.join(self.value), max_age=COOKIE_MAX_AGE) @@ -128,32 +123,32 @@ class MultipleChoiceSetting(EnumStringSetting): class SetSetting(Setting): """Setting of values of type ``set`` (comma separated string)""" - def _post_init(self): - if not hasattr(self, 'values'): - self.values = set() + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.values = set() def get_value(self): """Returns a string with comma separated values.""" return ','.join(self.values) - def parse(self, data): + def parse(self, data: str): """Parse and validate ``data`` and store the result at ``self.value``""" if data == '': - self.values = set() # pylint: disable=attribute-defined-outside-init + self.values = set() return elements = data.split(',') for element in elements: self.values.add(element) - def parse_form(self, data): + def parse_form(self, data: str): if self.locked: return elements = data.split(',') - self.values = set(elements) # pylint: disable=attribute-defined-outside-init + self.values = set(elements) - def save(self, name, resp): + def save(self, name: str, resp: flask.Response): """Save cookie ``name`` in the HTTP reponse obect""" resp.set_cookie(name, ','.join(self.values), max_age=COOKIE_MAX_AGE) @@ -165,13 +160,13 @@ class SearchLanguageSetting(EnumStringSetting): if selection != '' and not VALID_LANGUAGE_CODE.match(selection): raise ValidationException('Invalid language code: "{0}"'.format(selection)) - def parse(self, data): + def parse(self, data: str): """Parse and validate ``data`` and store the result at ``self.value``""" - if data not in self.choices and data != self.value: # pylint: disable=no-member + if data not in self.choices and data != self.value: # hack to give some backwards compatibility with old language cookies data = str(data).replace('_', '-') lang = data.split('-', maxsplit=1)[0] - # pylint: disable=no-member + if data in self.choices: pass elif lang in self.choices: @@ -185,34 +180,43 @@ class SearchLanguageSetting(EnumStringSetting): class MapSetting(Setting): """Setting of a value that has to be translated in order to be storable""" - def _post_init(self): - if not hasattr(self, 'map'): - raise MissingArgumentException('missing argument: map') - if self.value not in self.map.values(): # pylint: disable=no-member + def __init__(self, default_value, map: Dict[str, object], locked=False): # pylint: disable=redefined-builtin + super().__init__(default_value, locked) + self.map = map + + if self.value not in self.map.values(): raise ValidationException('Invalid default value') - def parse(self, data): + def parse(self, data: str): """Parse and validate ``data`` and store the result at ``self.value``""" - # pylint: disable=no-member + if data not in self.map: raise ValidationException('Invalid choice: {0}'.format(data)) self.value = self.map[data] self.key = data # pylint: disable=attribute-defined-outside-init - def save(self, name, resp): + def save(self, name: str, resp: flask.Response): """Save cookie ``name`` in the HTTP reponse obect""" if hasattr(self, 'key'): resp.set_cookie(name, self.key, max_age=COOKIE_MAX_AGE) +@dataclass +class Choice: + """A choice for a ``SwitchableSetting``.""" + + default_on: bool + id: str + + class SwitchableSetting(Setting): """Base class for settings that can be turned on && off""" - def _post_init(self): - self.disabled = set() - self.enabled = set() - if not hasattr(self, 'choices'): - raise MissingArgumentException('missing argument: choices') + def __init__(self, default_value, locked: bool, choices: Iterable[Choice]): + super().__init__(default_value, locked) + self.choices = choices + self.enabled: Set[str] = set() + self.disabled: Set[str] = set() def transform_form_items(self, items): # pylint: disable=no-self-use @@ -223,62 +227,57 @@ class SwitchableSetting(Setting): return values def parse_cookie(self, data): - # pylint: disable=attribute-defined-outside-init if data[DISABLED] != '': self.disabled = set(data[DISABLED].split(',')) if data[ENABLED] != '': self.enabled = set(data[ENABLED].split(',')) - def parse_form(self, items): + def parse_form(self, items: List[str]): if self.locked: return items = self.transform_form_items(items) - self.disabled = set() # pylint: disable=attribute-defined-outside-init - self.enabled = set() # pylint: disable=attribute-defined-outside-init - for choice in self.choices: # pylint: disable=no-member - if choice['default_on']: - if choice['id'] in items: - self.disabled.add(choice['id']) + self.disabled = set() + self.enabled = set() + for choice in self.choices: + if choice.default_on: + if choice.id in items: + self.disabled.add(choice.id) else: - if choice['id'] not in items: - self.enabled.add(choice['id']) + if choice.id not in items: + self.enabled.add(choice.id) - def save(self, resp): # pylint: disable=arguments-differ + def save(self, resp: flask.Response): # pylint: disable=arguments-differ """Save cookie in the HTTP reponse obect""" resp.set_cookie('disabled_{0}'.format(self.value), ','.join(self.disabled), max_age=COOKIE_MAX_AGE) resp.set_cookie('enabled_{0}'.format(self.value), ','.join(self.enabled), max_age=COOKIE_MAX_AGE) def get_disabled(self): disabled = self.disabled - for choice in self.choices: # pylint: disable=no-member - if not choice['default_on'] and choice['id'] not in self.enabled: - disabled.add(choice['id']) + for choice in self.choices: + if not choice.default_on and choice.id not in self.enabled: + disabled.add(choice.id) return self.transform_values(disabled) def get_enabled(self): enabled = self.enabled - for choice in self.choices: # pylint: disable=no-member - if choice['default_on'] and choice['id'] not in self.disabled: - enabled.add(choice['id']) + for choice in self.choices: + if choice.default_on and choice.id not in self.disabled: + enabled.add(choice.id) return self.transform_values(enabled) class EnginesSetting(SwitchableSetting): """Engine settings""" - def _post_init(self): - super()._post_init() - transformed_choices = [] - for engine_name, engine in self.choices.items(): # pylint: disable=no-member,access-member-before-definition + def __init__(self, default_value, engines: Iterable[Engine]): + choices = [] + for engine in engines: for category in engine.categories: if not category in list(settings['categories_as_tabs'].keys()) + [OTHER_CATEGORY]: continue - transformed_choice = {} - transformed_choice['default_on'] = not engine.disabled - transformed_choice['id'] = '{}__{}'.format(engine_name, category) - transformed_choices.append(transformed_choice) - self.choices = transformed_choices + choices.append(Choice(default_on=not engine.disabled, id='{}__{}'.format(engine.name, category))) + super().__init__(default_value, False, choices) def transform_form_items(self, items): return [item[len('engine_') :].replace('_', ' ').replace(' ', '__') for item in items] @@ -296,15 +295,11 @@ class EnginesSetting(SwitchableSetting): class PluginsSetting(SwitchableSetting): """Plugin settings""" - def _post_init(self): - super()._post_init() - transformed_choices = [] - for plugin in self.choices: # pylint: disable=access-member-before-definition - transformed_choice = {} - transformed_choice['default_on'] = plugin.default_on - transformed_choice['id'] = plugin.id - transformed_choices.append(transformed_choice) - self.choices = transformed_choices + def __init__(self, default_value, plugins: Iterable[Plugin]): + choices = [] + for plugin in plugins: + choices.append(Choice(default_on=plugin.default_on, id=plugin.id)) + super().__init__(default_value, False, choices) def transform_form_items(self, items): return [item[len('plugin_') :] for item in items] @@ -313,34 +308,34 @@ class PluginsSetting(SwitchableSetting): class Preferences: """Validates and saves preferences to cookies""" - def __init__(self, themes, categories, engines, plugins): + def __init__(self, themes: List[str], categories: List[str], engines: Dict[str, Engine], plugins: Iterable[Plugin]): super().__init__() - self.key_value_settings = { + self.key_value_settings: Dict[str, Setting] = { # fmt: off 'categories': MultipleChoiceSetting( ['general'], - is_locked('categories'), + locked=is_locked('categories'), choices=categories + ['none'] ), 'language': SearchLanguageSetting( settings['search']['default_lang'], - is_locked('language'), + locked=is_locked('language'), choices=settings['search']['languages'] + [''] ), 'locale': EnumStringSetting( settings['ui']['default_locale'], - is_locked('locale'), + locked=is_locked('locale'), choices=list(LOCALE_NAMES.keys()) + [''] ), 'autocomplete': EnumStringSetting( settings['search']['autocomplete'], - is_locked('autocomplete'), + locked=is_locked('autocomplete'), choices=list(autocomplete.backends.keys()) + [''] ), 'image_proxy': MapSetting( settings['server']['image_proxy'], - is_locked('image_proxy'), + locked=is_locked('image_proxy'), map={ '': settings['server']['image_proxy'], '0': False, @@ -351,12 +346,12 @@ class Preferences: ), 'method': EnumStringSetting( settings['server']['method'], - is_locked('method'), + locked=is_locked('method'), choices=('GET', 'POST') ), 'safesearch': MapSetting( settings['search']['safe_search'], - is_locked('safesearch'), + locked=is_locked('safesearch'), map={ '0': 0, '1': 1, @@ -365,12 +360,12 @@ class Preferences: ), 'theme': EnumStringSetting( settings['ui']['default_theme'], - is_locked('theme'), + locked=is_locked('theme'), choices=themes ), 'results_on_new_tab': MapSetting( settings['ui']['results_on_new_tab'], - is_locked('results_on_new_tab'), + locked=is_locked('results_on_new_tab'), map={ '0': False, '1': True, @@ -380,22 +375,22 @@ class Preferences: ), 'doi_resolver': MultipleChoiceSetting( [settings['default_doi_resolver'], ], - is_locked('doi_resolver'), + locked=is_locked('doi_resolver'), choices=DOI_RESOLVERS ), 'oscar-style': EnumStringSetting( settings['ui']['theme_args']['oscar_style'], - is_locked('oscar-style'), + locked=is_locked('oscar-style'), choices=['', 'logicodev', 'logicodev-dark', 'pointhi'] ), 'simple_style': EnumStringSetting( settings['ui']['theme_args']['simple_style'], - is_locked('simple_style'), + locked=is_locked('simple_style'), choices=['', 'auto', 'light', 'dark'] ), 'advanced_search': MapSetting( settings['ui']['advanced_search'], - is_locked('advanced_search'), + locked=is_locked('advanced_search'), map={ '0': False, '1': True, @@ -406,7 +401,7 @@ class Preferences: ), 'query_in_title': MapSetting( settings['ui']['query_in_title'], - is_locked('query_in_title'), + locked=is_locked('query_in_title'), map={ '': settings['ui']['query_in_title'], '0': False, @@ -418,10 +413,10 @@ class Preferences: # fmt: on } - self.engines = EnginesSetting('engines', choices=engines) - self.plugins = PluginsSetting('plugins', choices=plugins) + self.engines = EnginesSetting('engines', engines=engines.values()) + self.plugins = PluginsSetting('plugins', plugins=plugins) self.tokens = SetSetting('tokens') - self.unknown_params = {} + self.unknown_params: Dict[str, str] = {} def get_as_url_params(self): """Return preferences as URL parameters""" @@ -444,7 +439,7 @@ class Preferences: return urlsafe_b64encode(compress(urlencode(settings_kv).encode())).decode() - def parse_encoded_data(self, input_data): + def parse_encoded_data(self, input_data: str): """parse (base64) preferences from request (``flask.request.form['preferences']``)""" bin_data = decompress(urlsafe_b64decode(input_data)) dict_data = {} @@ -452,7 +447,7 @@ class Preferences: dict_data[x] = y[0] self.parse_dict(dict_data) - def parse_dict(self, input_data): + def parse_dict(self, input_data: Dict[str, str]): """parse preferences from request (``flask.request.form``)""" for user_setting_name, user_setting in input_data.items(): if user_setting_name in self.key_value_settings: @@ -474,7 +469,7 @@ class Preferences: ): self.unknown_params[user_setting_name] = user_setting - def parse_form(self, input_data): + def parse_form(self, input_data: Dict[str, str]): """Parse formular (````) data from a ``flask.request.form``""" disabled_engines = [] enabled_categories = [] @@ -497,7 +492,7 @@ class Preferences: self.plugins.parse_form(disabled_plugins) # cannot be used in case of engines or plugins - def get_value(self, user_setting_name): + def get_value(self, user_setting_name: str): """Returns the value for ``user_setting_name``""" ret_val = None if user_setting_name in self.key_value_settings: @@ -506,7 +501,7 @@ class Preferences: ret_val = self.unknown_params[user_setting_name] return ret_val - def save(self, resp): + def save(self, resp: flask.Response): """Save cookie in the HTTP reponse obect""" for user_setting_name, user_setting in self.key_value_settings.items(): # pylint: disable=unnecessary-dict-index-lookup @@ -532,7 +527,7 @@ class Preferences: return valid -def is_locked(setting_name): +def is_locked(setting_name: str): """Checks if a given setting name is locked by settings.yml""" if 'preferences' not in settings: return False diff --git a/tests/unit/test_preferences.py b/tests/unit/test_preferences.py index 1ffed5c1a..18323940b 100644 --- a/tests/unit/test_preferences.py +++ b/tests/unit/test_preferences.py @@ -1,7 +1,6 @@ from searx.preferences import ( EnumStringSetting, MapSetting, - MissingArgumentException, SearchLanguageSetting, MultipleChoiceSetting, PluginsSetting, @@ -19,10 +18,6 @@ class PluginStub: class TestSettings(SearxTestCase): # map settings - def test_map_setting_invalid_initialization(self): - with self.assertRaises(MissingArgumentException): - MapSetting(3, wrong_argument={'0': 0}) - def test_map_setting_invalid_default_value(self): with self.assertRaises(ValidationException): MapSetting(3, map={'dog': 1, 'bat': 2}) @@ -43,9 +38,6 @@ class TestSettings(SearxTestCase): self.assertEqual(setting.get_value(), 2) # enum settings - def test_enum_setting_invalid_initialization(self): - with self.assertRaises(MissingArgumentException): - EnumStringSetting('cat', wrong_argument=[0, 1, 2]) def test_enum_setting_invalid_default_value(self): with self.assertRaises(ValidationException): @@ -67,9 +59,6 @@ class TestSettings(SearxTestCase): self.assertEqual(setting.get_value(), 2) # multiple choice settings - def test_multiple_setting_invalid_initialization(self): - with self.assertRaises(MissingArgumentException): - MultipleChoiceSetting(['2'], wrong_argument=['0', '1', '2']) def test_multiple_setting_invalid_default_value(self): with self.assertRaises(ValidationException): @@ -115,14 +104,14 @@ class TestSettings(SearxTestCase): def test_plugins_setting_all_default_enabled(self): plugin1 = PluginStub('plugin1', True) plugin2 = PluginStub('plugin2', True) - setting = PluginsSetting(['3'], choices=[plugin1, plugin2]) + setting = PluginsSetting(['3'], plugins=[plugin1, plugin2]) self.assertEqual(setting.get_enabled(), set(['plugin1', 'plugin2'])) def test_plugins_setting_few_default_enabled(self): plugin1 = PluginStub('plugin1', True) plugin2 = PluginStub('plugin2', False) plugin3 = PluginStub('plugin3', True) - setting = PluginsSetting('name', choices=[plugin1, plugin2, plugin3]) + setting = PluginsSetting('name', plugins=[plugin1, plugin2, plugin3]) self.assertEqual(setting.get_enabled(), set(['plugin1', 'plugin3'])) From 83f8a8fc6dc7658e062ddcbe779cba57dfaa1cb3 Mon Sep 17 00:00:00 2001 From: Martin Fischer Date: Tue, 4 Jan 2022 14:14:37 +0100 Subject: [PATCH 2/4] [refactor] remove pointless tuple --- searx/preferences.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/searx/preferences.py b/searx/preferences.py index bbcfc524e..ae158773a 100644 --- a/searx/preferences.py +++ b/searx/preferences.py @@ -22,8 +22,6 @@ from searx.engines import OTHER_CATEGORY COOKIE_MAX_AGE = 60 * 60 * 24 * 365 * 5 # 5 years -DISABLED = 0 -ENABLED = 1 DOI_RESOLVERS = list(settings['doi_resolvers']) @@ -226,11 +224,11 @@ class SwitchableSetting(Setting): # pylint: disable=no-self-use return values - def parse_cookie(self, data): - if data[DISABLED] != '': - self.disabled = set(data[DISABLED].split(',')) - if data[ENABLED] != '': - self.enabled = set(data[ENABLED].split(',')) + def parse_cookie(self, data_disabled: str, data_enabled: str): + if data_disabled != '': + self.disabled = set(data_disabled.split(',')) + if data_enabled != '': + self.enabled = set(data_enabled.split(',')) def parse_form(self, items: List[str]): if self.locked: @@ -455,13 +453,9 @@ class Preferences: continue self.key_value_settings[user_setting_name].parse(user_setting) elif user_setting_name == 'disabled_engines': - self.engines.parse_cookie( - (input_data.get('disabled_engines', ''), input_data.get('enabled_engines', '')) - ) + self.engines.parse_cookie(input_data.get('disabled_engines', ''), input_data.get('enabled_engines', '')) elif user_setting_name == 'disabled_plugins': - self.plugins.parse_cookie( - (input_data.get('disabled_plugins', ''), input_data.get('enabled_plugins', '')) - ) + self.plugins.parse_cookie(input_data.get('disabled_plugins', ''), input_data.get('enabled_plugins', '')) elif user_setting_name == 'tokens': self.tokens.parse(user_setting) elif not any( From 56fbf221082bce439fef6e975058367cf4d2ce79 Mon Sep 17 00:00:00 2001 From: Martin Fischer Date: Tue, 4 Jan 2022 14:57:39 +0100 Subject: [PATCH 3/4] [refactor] stop SwitchableSetting from subclassing Setting Previously the default_value was abused for the cookie name. Having SwitchableSetting subclass Setting doesn't even make sense in the first place since none of the Setting methods apply. --- searx/preferences.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/searx/preferences.py b/searx/preferences.py index ae158773a..7ea81ce46 100644 --- a/searx/preferences.py +++ b/searx/preferences.py @@ -207,11 +207,12 @@ class Choice: id: str -class SwitchableSetting(Setting): +class SwitchableSetting: """Base class for settings that can be turned on && off""" - def __init__(self, default_value, locked: bool, choices: Iterable[Choice]): - super().__init__(default_value, locked) + def __init__(self, name: str, locked: bool, choices: Iterable[Choice]): + self.name = name + self.locked = locked self.choices = choices self.enabled: Set[str] = set() self.disabled: Set[str] = set() @@ -245,10 +246,10 @@ class SwitchableSetting(Setting): if choice.id not in items: self.enabled.add(choice.id) - def save(self, resp: flask.Response): # pylint: disable=arguments-differ + def save(self, resp: flask.Response): """Save cookie in the HTTP reponse obect""" - resp.set_cookie('disabled_{0}'.format(self.value), ','.join(self.disabled), max_age=COOKIE_MAX_AGE) - resp.set_cookie('enabled_{0}'.format(self.value), ','.join(self.enabled), max_age=COOKIE_MAX_AGE) + resp.set_cookie('disabled_{0}'.format(self.name), ','.join(self.disabled), max_age=COOKIE_MAX_AGE) + resp.set_cookie('enabled_{0}'.format(self.name), ','.join(self.enabled), max_age=COOKIE_MAX_AGE) def get_disabled(self): disabled = self.disabled From 180d4d068b4c629ab99876b55046f98455b88149 Mon Sep 17 00:00:00 2001 From: Martin Fischer Date: Thu, 6 Jan 2022 18:45:50 +0100 Subject: [PATCH 4/4] [refactor] refactor SwitchableSetting The previous implementation used two hash sets and a list. ... that's not necessary ... a single hash map suffices. And it's also less error prone ... because the previous data structure allowed a setting to be enabled and disabled at the same time. --- searx/preferences.py | 80 +++++++++++++--------------------- tests/unit/test_preferences.py | 4 +- 2 files changed, 33 insertions(+), 51 deletions(-) diff --git a/searx/preferences.py b/searx/preferences.py index 7ea81ce46..223858a5b 100644 --- a/searx/preferences.py +++ b/searx/preferences.py @@ -8,8 +8,7 @@ from base64 import urlsafe_b64encode, urlsafe_b64decode from zlib import compress, decompress from urllib.parse import parse_qs, urlencode -from typing import Iterable, Dict, List, Set -from dataclasses import dataclass +from typing import Iterable, Dict, List import flask @@ -199,23 +198,13 @@ class MapSetting(Setting): resp.set_cookie(name, self.key, max_age=COOKIE_MAX_AGE) -@dataclass -class Choice: - """A choice for a ``SwitchableSetting``.""" +class BooleanChoices: + """Maps strings to booleans that are either true or false.""" - default_on: bool - id: str - - -class SwitchableSetting: - """Base class for settings that can be turned on && off""" - - def __init__(self, name: str, locked: bool, choices: Iterable[Choice]): + def __init__(self, name: str, choices: Dict[str, bool], locked: bool = False): self.name = name - self.locked = locked self.choices = choices - self.enabled: Set[str] = set() - self.disabled: Set[str] = set() + self.locked = locked def transform_form_items(self, items): # pylint: disable=no-self-use @@ -226,25 +215,29 @@ class SwitchableSetting: return values def parse_cookie(self, data_disabled: str, data_enabled: str): - if data_disabled != '': - self.disabled = set(data_disabled.split(',')) - if data_enabled != '': - self.enabled = set(data_enabled.split(',')) + for disabled in data_disabled.split(','): + if disabled in self.choices: + self.choices[disabled] = False + + for enabled in data_enabled.split(','): + if enabled in self.choices: + self.choices[enabled] = True def parse_form(self, items: List[str]): if self.locked: return - items = self.transform_form_items(items) - self.disabled = set() - self.enabled = set() - for choice in self.choices: - if choice.default_on: - if choice.id in items: - self.disabled.add(choice.id) - else: - if choice.id not in items: - self.enabled.add(choice.id) + disabled = self.transform_form_items(items) + for setting in self.choices: + self.choices[setting] = setting not in disabled + + @property + def enabled(self): + return (k for k, v in self.choices.items() if v) + + @property + def disabled(self): + return (k for k, v in self.choices.items() if not v) def save(self, resp: flask.Response): """Save cookie in the HTTP reponse obect""" @@ -252,31 +245,23 @@ class SwitchableSetting: resp.set_cookie('enabled_{0}'.format(self.name), ','.join(self.enabled), max_age=COOKIE_MAX_AGE) def get_disabled(self): - disabled = self.disabled - for choice in self.choices: - if not choice.default_on and choice.id not in self.enabled: - disabled.add(choice.id) - return self.transform_values(disabled) + return self.transform_values(list(self.disabled)) def get_enabled(self): - enabled = self.enabled - for choice in self.choices: - if choice.default_on and choice.id not in self.disabled: - enabled.add(choice.id) - return self.transform_values(enabled) + return self.transform_values(list(self.enabled)) -class EnginesSetting(SwitchableSetting): +class EnginesSetting(BooleanChoices): """Engine settings""" def __init__(self, default_value, engines: Iterable[Engine]): - choices = [] + choices = {} for engine in engines: for category in engine.categories: if not category in list(settings['categories_as_tabs'].keys()) + [OTHER_CATEGORY]: continue - choices.append(Choice(default_on=not engine.disabled, id='{}__{}'.format(engine.name, category))) - super().__init__(default_value, False, choices) + choices['{}__{}'.format(engine.name, category)] = not engine.disabled + super().__init__(default_value, choices) def transform_form_items(self, items): return [item[len('engine_') :].replace('_', ' ').replace(' ', '__') for item in items] @@ -291,14 +276,11 @@ class EnginesSetting(SwitchableSetting): return transformed_values -class PluginsSetting(SwitchableSetting): +class PluginsSetting(BooleanChoices): """Plugin settings""" def __init__(self, default_value, plugins: Iterable[Plugin]): - choices = [] - for plugin in plugins: - choices.append(Choice(default_on=plugin.default_on, id=plugin.id)) - super().__init__(default_value, False, choices) + super().__init__(default_value, {plugin.id: plugin.default_on for plugin in plugins}) def transform_form_items(self, items): return [item[len('plugin_') :] for item in items] diff --git a/tests/unit/test_preferences.py b/tests/unit/test_preferences.py index 18323940b..a69c45178 100644 --- a/tests/unit/test_preferences.py +++ b/tests/unit/test_preferences.py @@ -105,14 +105,14 @@ class TestSettings(SearxTestCase): plugin1 = PluginStub('plugin1', True) plugin2 = PluginStub('plugin2', True) setting = PluginsSetting(['3'], plugins=[plugin1, plugin2]) - self.assertEqual(setting.get_enabled(), set(['plugin1', 'plugin2'])) + self.assertEqual(set(setting.get_enabled()), set(['plugin1', 'plugin2'])) def test_plugins_setting_few_default_enabled(self): plugin1 = PluginStub('plugin1', True) plugin2 = PluginStub('plugin2', False) plugin3 = PluginStub('plugin3', True) setting = PluginsSetting('name', plugins=[plugin1, plugin2, plugin3]) - self.assertEqual(setting.get_enabled(), set(['plugin1', 'plugin3'])) + self.assertEqual(set(setting.get_enabled()), set(['plugin1', 'plugin3'])) class TestPreferences(SearxTestCase):