From 84130a5523d66d5f39129ebc4706c2ea64aa3de0 Mon Sep 17 00:00:00 2001 From: Ben Avrahami Date: Mon, 3 Jan 2022 16:07:36 +0200 Subject: [PATCH] Setting Versionsing (#45) * documentation * in progress * . * ready for PR * lint * cr --- CHANGELOG.md | 5 +- docs/index.rst | 1 + docs/setting_types.rst | 1 - docs/setting_versions.rst | 53 + heksher/api/v1/rules.py | 21 +- heksher/api/v1/rules_metadata.py | 2 +- heksher/api/v1/setting_declaration.py | 309 ++++++ heksher/api/v1/settings.py | 280 +++--- heksher/api/v1/settings_metadata.py | 75 +- heksher/api/v1/validation.py | 4 +- heksher/db_logic/logic_base.py | 3 + heksher/db_logic/metadata.py | 24 +- heksher/db_logic/rule.py | 18 +- heksher/db_logic/setting.py | 265 ++--- .../db_logic/setting_configurable_features.py | 13 + heksher/db_logic/setting_metadata.py | 69 +- heksher/db_logic/util.py | 5 +- heksher/setting.py | 18 - tests/blackbox/app/conftest.py | 8 +- tests/blackbox/app/test_v1api_aliases.py | 114 +-- tests/blackbox/app/test_v1api_rules.py | 11 +- .../blackbox/app/test_v1api_rules_metadata.py | 7 +- tests/blackbox/app/test_v1api_settings.py | 926 +++++++++++++++--- .../app/test_v1api_settings_metadata.py | 55 +- 24 files changed, 1683 insertions(+), 604 deletions(-) create mode 100644 docs/setting_versions.rst create mode 100644 heksher/api/v1/setting_declaration.py create mode 100644 heksher/db_logic/setting_configurable_features.py delete mode 100644 heksher/setting.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f979b3a..f36014b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,10 @@ ### Removed * old api endpoint POST /api/v1/rules/query has been removed and replaced with GET /api/v1/rules/query ### Changed -* the rename api endpoint has been changed to PUT /api/v1//name +* the rename api endpoint has been changed to PUT /api/v1//name. * the method of the endpoint /api/v1/rules/search has been changed to GET. +* All setting now must have a default value. +* Setting declarations are now versioned. ### Deprecated * The api endpoint PATCH /api/v1/rules/ to change a rule's value is now deprecated, new users should use PUT /api/v1/rules//value @@ -13,6 +15,7 @@ * documentation * The api endpoint PUT /api/v1/rules//value to change a rule's value * The api endpoint GET /api/v1/rules/query to query rules (replaces the old query endpoint) +* POST /api/v1/rules now returns the rule location in the header ### Fixed * A bug where patching a context feature's index using "to_before" would use the incorrect target. ### Internal diff --git a/docs/index.rst b/docs/index.rst index 429548e..fa0d679 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -10,6 +10,7 @@ Welcome to the Heksher documentation! running setting_types api + setting_versions setting_aliases cookbook libraries diff --git a/docs/setting_types.rst b/docs/setting_types.rst index 0c20946..0f41bf9 100644 --- a/docs/setting_types.rst +++ b/docs/setting_types.rst @@ -98,7 +98,6 @@ hold any dictionary mapping strings to dictionaries that map strings to integers Type Order ---------- - Setting types have a partial ordering over them. This when we want to safely change a setting's type. We say that type A is a supertype of type B if every value that can be stored in type B can also be stored in type A. This will help us when :ref:`declaring settings `. diff --git a/docs/setting_versions.rst b/docs/setting_versions.rst new file mode 100644 index 0000000..b90962f --- /dev/null +++ b/docs/setting_versions.rst @@ -0,0 +1,53 @@ +Setting Versions +=================== + +Over time, attributes of settings may change. These changes might be metadata differences, setting type changes, or +even wholesale renaming of the setting. These changes must be as backwards compatible as possible, so that users using a +setting's older attributes might still function. At the same time, we want to make sure that these older declarations +don't regress the settings back to their old attributes. We consolidate those two needs with setting versions. + +Whenever a setting is declared, it is declared with a version (the default version is ``1.0``).If the setting does not +yet exist, it is created (in this case, we expect the version to be ``1.0``). If the setting already exists, we check +the latest declared version of the setting. + +* If the latest declared version is the same as the current version, we assert that the values are the same as the + latest declaration. If the assertion fails, we inform the user of an attribute mismatch. +* If the latest declared version is higher than the declaration version, we inform the user that they are declaring with + outdated attributes. + .. warning:: + + Differing attributes are not checked for older versions. If a user purposely declares a setting with an older + version but with different attributes then those used for that version, no issue will be reported (but this will not + affect other users whatsoever). This behavior might change in the future. + +* If the latest declared version is lower than the declaration version, we update the setting attributes to reflect the + new declaration. + + +Note that not all changes are automatically accepted. If the new version is higher than the current version only in the +second number (what we call a **minor change**), only the following changes are accepted: + +* Changing metadata. +* Changing the setting type to a :ref:`subtype ` of the current setting type. +* Renaming the setting (while defining the old name as an alias). +* Removing a configurable feature that no rule of the setting is configured by. +* Changing a default value. + +.. note:: + + These are changes that we expect to be fully backwards compatible. i.e. if a setting is declared with an older + version, we expect to be fully functional. + +If the new version is higher than the current version in the first number (what we call a **major change**), we accept +the following changes: + + * Changing metadata. + * Changing the setting type. + * Renaming the setting (while defining the old name as an alias). + * Changing configurable features. + * Changing a default value. + +There are some changes that are never acceptable, as they would break the logic of the application. These are: + +* Changing a setting type to a value that does not accept the value of at least one rule of the setting. +* Removing configurable features that are matched by at least one rule of the setting. \ No newline at end of file diff --git a/heksher/api/v1/rules.py b/heksher/api/v1/rules.py index f4f081d..cf3327c 100644 --- a/heksher/api/v1/rules.py +++ b/heksher/api/v1/rules.py @@ -12,7 +12,6 @@ from heksher.api.v1.util import ORJSONModel, PydanticResponse, application, handle_etag, router as v1_router from heksher.api.v1.validation import ContextFeatureName, ContextFeatureValue, MetadataKey, SettingName from heksher.app import HeksherApp -from heksher.setting import Setting router = APIRouter(prefix='/rules') logger = getLogger(__name__) @@ -48,7 +47,8 @@ async def search_rule(app: HeksherApp = application, """ Get the ID of a rule with specific conditions. """ - canon_setting = await app.db_logic.get_setting(setting, include_metadata=False) # for aliasing + canon_setting = await app.db_logic.get_setting(setting, include_metadata=False, include_configurable_features=False, + include_aliases=False) # for aliasing if not canon_setting: return Response(status_code=status.HTTP_404_NOT_FOUND) feature_values_dict: Dict[str, str] = dict(part.split(':') for part in feature_values.split(',')) # type: ignore @@ -62,7 +62,7 @@ class AddRuleInput(ORJSONModel): setting: SettingName = Field(description="the setting name the rule should apply to") feature_values: Dict[ContextFeatureName, ContextFeatureValue] = \ Field(description="the exact-match conditions of the rule") - value: Any = Field(description="the value of the setting in contexts that match the rule") + value: Any = Field(..., description="the value of the setting in contexts that match the rule") metadata: Dict[MetadataKey, Any] = Field(default_factory=dict, description="additional metadata of the rule") @validator('feature_values') @@ -82,7 +82,8 @@ async def add_rule(input: AddRuleInput, app: HeksherApp = application): """ Add a rule, and get its ID. """ - setting: Optional[Setting] = await app.db_logic.get_setting(input.setting, include_metadata=False) + setting = await app.db_logic.get_setting(input.setting, include_metadata=False, include_configurable_features=True, + include_aliases=False) if not setting: return PlainTextResponse(f'setting not found with name {input.setting}', status_code=status.HTTP_422_UNPROCESSABLE_ENTITY) @@ -101,11 +102,12 @@ async def add_rule(input: AddRuleInput, app: HeksherApp = application): new_id = await app.db_logic.add_rule(setting.name, input.value, input.metadata, input.feature_values) - return AddRuleOutput(rule_id=new_id) + return PydanticResponse(AddRuleOutput(rule_id=new_id), + headers={'Location': f'/{new_id}'}, status_code=status.HTTP_201_CREATED) class PatchRuleInput(ORJSONModel): - value: Any = Field(description="the value of the setting in contexts that match the rule") + value: Any = Field(..., description="the value of the setting in contexts that match the rule") @router.patch('/{rule_id}', status_code=status.HTTP_204_NO_CONTENT, response_class=Response) @@ -118,7 +120,8 @@ async def patch_rule(rule_id: int, input: PatchRuleInput, app: HeksherApp = appl if not rule: return PlainTextResponse(status_code=status.HTTP_404_NOT_FOUND) - setting = await app.db_logic.get_setting(rule.setting, include_metadata=False) + setting = await app.db_logic.get_setting(rule.setting, include_metadata=False, include_aliases=False, + include_configurable_features=False) assert setting if not setting.type.validate(input.value): @@ -191,7 +194,9 @@ async def query_rules(request: Request, app: HeksherApp = application, " response"), ): if raw_settings is None: - settings = await app.db_logic.get_all_settings_names() + settings = [spec.name for spec in await app.db_logic.get_all_settings(include_configurable_features=False, + include_aliases=False, + include_metadata=False)] elif not raw_settings: settings = [] else: diff --git a/heksher/api/v1/rules_metadata.py b/heksher/api/v1/rules_metadata.py index 4649e9f..00f85b2 100644 --- a/heksher/api/v1/rules_metadata.py +++ b/heksher/api/v1/rules_metadata.py @@ -44,7 +44,7 @@ async def replace_rule_metadata(rule_id: int, input: InputRuleMetadata, app: Hek class PutRuleMetadataKey(ORJSONModel): - value: Any = Field(description="the new value of the given key and rule in the rule's metadata") + value: Any = Field(..., description="the new value of the given key and rule in the rule's metadata") @router.put('/{rule_id}/metadata/{key}', status_code=status.HTTP_204_NO_CONTENT, response_class=Response) diff --git a/heksher/api/v1/setting_declaration.py b/heksher/api/v1/setting_declaration.py new file mode 100644 index 0000000..79da690 --- /dev/null +++ b/heksher/api/v1/setting_declaration.py @@ -0,0 +1,309 @@ +from asyncio import gather +from dataclasses import dataclass +from itertools import chain +from logging import getLogger +from typing import Any, Dict, List, Literal, Optional, Tuple, Union + +import orjson +from fastapi import HTTPException +from pydantic import Field, root_validator +from starlette import status +from starlette.responses import PlainTextResponse + +from heksher.api.v1.util import ORJSONModel, PydanticResponse, application +from heksher.api.v1.validation import ContextFeatureName, MetadataKey, SettingName, SettingVersion +from heksher.app import HeksherApp +from heksher.db_logic.setting import SettingSpec +from heksher.db_logic.util import parse_setting_version +from heksher.setting_types import SettingType + +logger = getLogger(__name__) + + +class DeclareSettingInput(ORJSONModel): + name: SettingName = Field(description="the name of the setting") + configurable_features: List[ContextFeatureName] = Field( + description="a list of context features that the setting should allow rules to match by", min_items=1, + unique=True + ) + type: SettingType = Field(description="the type of the setting") + default_value: Any = Field(..., description="the default value of the rule, must be applicable to the setting's " + "value") + metadata: Dict[MetadataKey, Any] = Field(default_factory=dict, description="user-defined metadata of the setting") + alias: Optional[SettingName] = Field( + description="an alias for the setting name, that can be used interchangeably with the original name" + ) + version: SettingVersion = Field('1.0', description="the version of the setting") + + @root_validator(skip_on_failure=True) + @classmethod + def default_value_matches(cls, values: Dict[str, Any]): + default = values['default_value'] + type_: SettingType = values['type'] + if not type_.validate(default): + raise TypeError(f'type ({type_}) and default value ({default}) must match') + return values + + @root_validator(skip_on_failure=True) + @classmethod + def alias_and_name_differ(cls, values: Dict[str, Any]): + alias = values['alias'] + if alias is None: + return values + name = values['name'] + if name == alias: + raise ValueError(f'name ({name}) and alias ({alias}) must differ') + return values + + +class AttributeDifferenceOutput(ORJSONModel): + level: Literal['minor', 'major', 'mismatch'] = Field(description="the level of difference") + attribute: str = Field(description="the attribute that is different") + latest_value: Any = Field(description="the value of the attribute in the latest declaration") + + +class MessageDifferenceOutput(ORJSONModel): + level: Literal['minor', 'major', 'mismatch'] = Field(description="the level of difference") + message: str = Field(description="message describing the difference") + + +class UpToDateDeclareSettingOutput(ORJSONModel): + outcome: Literal['created', 'uptodate'] = Field(description="'created' if the setting is newly created, or " + "'uptodate' if it already existed at the same version") + + +class OutdatedDeclareSettingOutput(ORJSONModel): + outcome: Literal['outdated'] = Field('outdated', description="'outdated' if the setting is already defined at a" + " higher version") + latest_version: str = Field(description="the latest version of the setting") + differences: List[Union[MessageDifferenceOutput, AttributeDifferenceOutput]] = Field( + description="a list of differences between the declaration and the setting attributes") + + +class UpgradeDeclareSettingOutput(ORJSONModel): + outcome: Literal['upgraded', 'rejected'] = Field(description="'upgraded' if the setting was successfully upgraded, " + "'rejected' if the upgrade was rejected due to " + "breaking changes") + previous_version: str = Field(description="the previous version of the setting") + differences: List[Union[MessageDifferenceOutput, AttributeDifferenceOutput]] = Field( + description="a list of differences between the declaration and the setting attributes") + + +class MismatchDeclareSettingOutput(ORJSONModel): + outcome: Literal['mismatch'] = Field('mismatch', description="'mismatch' if the setting was declared at the same " + "version, but with different attributes") + differences: List[Union[MessageDifferenceOutput, AttributeDifferenceOutput]] = Field( + description="a list of differences between the declaration and the setting attributes") + + +@dataclass +class AttrDifference: + attribute: str + latest_value: Any + + +@dataclass +class MessageDifference: + message: str + + +# type aliases for internal logic +DifferenceCategory = Literal['minor', 'major', 'mismatch'] +DifferenceSpec = Union[MessageDifference, AttrDifference] +DifferencesDict = Dict[DifferenceCategory, List[DifferenceSpec]] +NewSettingAttributes = Tuple[Optional[List[str]], Optional[SettingType], Optional[str], Optional[Any], + Optional[Dict[str, Any]]] + + +async def declare_setting_endpoint(input: DeclareSettingInput, app: HeksherApp = application): + """ + Ensure that a setting exists, creating it if necessary. + """ + existing = await app.db_logic.get_setting(input.name, include_aliases=True, include_metadata=True, + include_configurable_features=True) + if input.alias: + alias_canonical_name = (await app.db_logic.get_canonical_names([input.alias]))[input.alias] + if alias_canonical_name is None: + raise HTTPException(status_code=404, detail=f'alias {input.alias} does not exist') + # we only accept two options: either the setting does not exist and the alias is a canonical name, or it is an + # existing alias of the setting + if not ( + (not existing and input.alias == alias_canonical_name) + or (existing and existing.name == alias_canonical_name) + ): + raise HTTPException(status_code=409, detail=f'alias {input.alias} is an alias of unrelated setting ' + f'{alias_canonical_name}') + else: + alias_canonical_name = None + + if existing is None: + if alias_canonical_name: + existing = await app.db_logic.get_setting(alias_canonical_name, include_aliases=True, + include_metadata=True, + include_configurable_features=True) + else: + if input.version != '1.0': + return PlainTextResponse('newly created settings must have version 1.0', status_code=400) + not_cf = await app.db_logic.get_not_found_context_features(input.configurable_features) + if not_cf: + return PlainTextResponse(f'{not_cf} are not acceptable context features', + status_code=status.HTTP_404_NOT_FOUND) + logger.info('creating new setting', extra={'setting_name': input.name}) + aliases = [input.alias] if input.alias else None + raw_default_value = str(orjson.dumps(input.default_value), 'utf-8') + spec = SettingSpec(input.name, str(input.type), raw_default_value, input.metadata, + input.configurable_features, aliases, input.version) + await app.db_logic.add_setting(spec) + return PydanticResponse(UpToDateDeclareSettingOutput(outcome='created')) + + async def get_diffs(is_outdated: bool) -> Tuple[NewSettingAttributes, DifferencesDict]: + differences: DifferencesDict = {k: [] for k in ['minor', 'major', 'mismatch']} # type: ignore[misc] + + # all the functions below handle different attributes of the setting. They append the differences to the dict, + # and return True if there are any differences. + + async def handle_cf_diff(is_outdated: bool) -> bool: + existing_setting_cfs = frozenset(existing.configurable_features) + new_setting_cfs = frozenset(input.configurable_features) + if existing_setting_cfs == new_setting_cfs: + return False + if is_outdated: + # all changes are flipped in direction + existing_setting_cfs, new_setting_cfs = new_setting_cfs, existing_setting_cfs + removed_cfs = existing_setting_cfs - new_setting_cfs + if not is_outdated and removed_cfs: + actual_cfs_in_use = await app.db_logic.get_actual_configurable_features(existing.name) + removed_cfs_in_use = removed_cfs & actual_cfs_in_use.keys() + if removed_cfs_in_use: + rule_ids = list(chain.from_iterable(actual_cfs_in_use[cf] for cf in removed_cfs_in_use)) + differences['mismatch'].append(MessageDifference( + f'configurable features {sorted(removed_cfs)} are still in use by rules {rule_ids}')) + return True + if existing_setting_cfs > new_setting_cfs: + differences['minor'].append( + MessageDifference(f'removal of configurable features {sorted(removed_cfs)}')) + else: + if not is_outdated: + not_cf = await app.db_logic.get_not_found_context_features(new_setting_cfs) + if not_cf: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, + detail=f'{not_cf} are not acceptable context features') + differences['major'].append(AttrDifference('configurable_features', existing.configurable_features)) + return True + + async def handle_type_diff(is_outdated: bool) -> bool: + if existing.type == input.type: + return False + existing_type = existing.type + new_type = input.type + if is_outdated: + # all changes are flipped in direction + existing_type, new_type = new_type, existing_type + if new_type < existing_type: + differences['minor'].append(AttrDifference('type', str(existing.type))) + else: + # only check the rules if we're not outdated + rules = await app.db_logic.get_rules_for_setting(input.name) if not is_outdated else [] + mismatched_rule_ids = [rule.rule_id for rule in rules if not new_type.validate(rule.value)] + if mismatched_rule_ids: + differences['mismatch'].append( + MessageDifference(f'setting type incompatible with values for rules: ' + f'{sorted(mismatched_rule_ids)}')) + else: + differences['major'].append(AttrDifference('type', str(existing.type))) + return True + + def handle_rename(is_outdated: bool) -> bool: + if input.name == existing.name: + # the names are equal, and we have a guarantee that the alias is already an alias of the setting + return False + if is_outdated: + differences['minor'].append(AttrDifference('name', existing.name)) + return True + assert input.alias == existing.name + differences['minor'].append(AttrDifference('name', existing.name)) + return True + + def handle_default_value_diff(is_outdated: bool) -> bool: + if existing.default_value == input.default_value: + return False + differences['minor'].append(AttrDifference('default_value', existing.default_value)) + return True + + def handle_metadata_diff(is_outdated: bool) -> bool: + if existing.metadata == input.metadata: + return False + differences['minor'].append(AttrDifference('metadata', existing.metadata)) + return True + + is_cf_diff, is_type_diff = await gather(handle_cf_diff(is_outdated), handle_type_diff(is_outdated)) + return (input.configurable_features if is_cf_diff else None, + input.type if is_type_diff else None, + input.name if handle_rename(is_outdated) else None, + input.default_value if handle_default_value_diff(is_outdated) else None, + input.metadata if handle_metadata_diff(is_outdated) else None), differences + + def diff_list(differences: DifferencesDict) -> List[Union[MessageDifferenceOutput, AttributeDifferenceOutput]]: + diff_list: List[Union[MessageDifferenceOutput, AttributeDifferenceOutput]] = [] + for level, diffs in differences.items(): + for diff in diffs: + if isinstance(diff, AttrDifference): + diff_list.append(AttributeDifferenceOutput(level=level, attribute=diff.attribute, + latest_value=diff.latest_value)) + else: + diff_list.append(MessageDifferenceOutput(level=level, message=diff.message)) + return diff_list + + if input.version == existing.version: + _, diffs = await get_diffs(False) + if any(v for v in diffs.values()): + return PydanticResponse(MismatchDeclareSettingOutput(differences=diff_list(diffs)), + status_code=status.HTTP_409_CONFLICT) + return PydanticResponse(UpToDateDeclareSettingOutput(outcome='uptodate')) + + input_version = parse_setting_version(input.version) + existing_version = parse_setting_version(existing.version) + + if input_version < existing_version: + _, diffs = await get_diffs(True) + return PydanticResponse(OutdatedDeclareSettingOutput(latest_version=existing.version, + differences=diff_list(diffs))) + + # now the user is definitely attempting an upgrade + (new_cfs, new_type, new_name, new_default_value, new_metadata), differences = await get_diffs(False) + if differences['mismatch']: + accepted = False + elif input_version[0] > existing_version[0]: + # we perform a major upgrade without mismatches + accepted = True + else: + assert input_version[1] > existing_version[1] + # we perform a minor upgrade, so long as there are no major differences + accepted = not differences['major'] + + if not accepted: + return PydanticResponse( + UpgradeDeclareSettingOutput(outcome='rejected', previous_version=existing.version, + differences=diff_list(differences)), + status_code=status.HTTP_409_CONFLICT) + await app.db_logic.update_setting(existing.name, new_name, new_cfs, new_type, new_default_value, + new_metadata, input.version) + return PydanticResponse(UpgradeDeclareSettingOutput(outcome='upgraded', previous_version=existing.version, + differences=diff_list(differences))) + + +declare_setting_enpoint_args: Dict[str, Any] = dict( + methods=['POST'], + responses={ + status.HTTP_200_OK: { + "model": Union[UpToDateDeclareSettingOutput, OutdatedDeclareSettingOutput, + UpgradeDeclareSettingOutput, MismatchDeclareSettingOutput] + }, + status.HTTP_422_UNPROCESSABLE_ENTITY: { + "description": "Configurable features are not acceptable.", + }, + status.HTTP_409_CONFLICT: { + "description": "The given alias is used by another setting." + }, + } +) diff --git a/heksher/api/v1/settings.py b/heksher/api/v1/settings.py index 8cacf47..b92fe8c 100644 --- a/heksher/api/v1/settings.py +++ b/heksher/api/v1/settings.py @@ -1,160 +1,26 @@ +from itertools import chain from logging import getLogger -from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List, Union -import orjson from fastapi import APIRouter, Response -from pydantic import Field, root_validator +from pydantic import Field from starlette import status from starlette.responses import JSONResponse, PlainTextResponse +from heksher.api.v1.setting_declaration import declare_setting_endpoint, declare_setting_enpoint_args from heksher.api.v1.settings_metadata import router as metadata_router from heksher.api.v1.util import ORJSONModel, application, router as v1_router -from heksher.api.v1.validation import ContextFeatureName, MetadataKey, SettingName +from heksher.api.v1.validation import MetadataKey, SettingName, SettingVersion from heksher.app import HeksherApp -from heksher.setting import Setting -from heksher.setting_types import SettingType, setting_type +from heksher.db_logic.setting_configurable_features import set_settings_configurable_features +from heksher.db_logic.util import parse_setting_version +from heksher.setting_types import SettingType router = APIRouter(prefix='/settings') logger = getLogger(__name__) - -class DeclareSettingInput(ORJSONModel): - name: SettingName = Field(description="the name of the setting") - configurable_features: List[ContextFeatureName] = Field( - description="a list of context features that the setting should allow rules to match by" - ) - type: SettingType = Field(description="the type of the setting") - default_value: Any = Field(None, - description="the default value of the rule, must be applicable to the setting's value") - metadata: Dict[MetadataKey, Any] = Field(default_factory=dict, description="user-defined metadata of the setting") - alias: Optional[SettingName] = Field( - description="an alias for the setting name, that can be used interchangeably with the original name" - ) - - def to_setting(self) -> Setting: - return Setting(self.name, self.type, self.default_value, self.configurable_features, self.metadata) - - @root_validator(skip_on_failure=True) - @classmethod - def default_value_matches(cls, values: Dict[str, Any]): - default = values['default_value'] - if default is None: - return values - type_: SettingType = values['type'] - if not type_.validate(default): - raise TypeError(f'type ({type_}) and default value ({default}) must match') - return values - - -class DeclareSettingOutput(ORJSONModel): - created: bool = Field(description="whether the fields was newly created by the request") - changed: List[str] = Field(description="a list of fields that were changed by the request") - incomplete: Dict[str, Any] = Field( - description="a mapping of fields that were declared with incomplete data. The values in the mapping represent" - " the complete data with (which remains unchanged)" - ) - - -@router.put('/declare', - responses={ - status.HTTP_200_OK: { - "model": DeclareSettingOutput, - }, - status.HTTP_422_UNPROCESSABLE_ENTITY: { - "description": "Configurable features are not acceptable.", - }, - status.HTTP_409_CONFLICT: { - "description": "The given alias is used by another setting." - }, - }) -async def declare_setting(input: DeclareSettingInput, app: HeksherApp = application): - """ - Ensure that a setting exists, creating it if necessary. - """ - new_setting = input.to_setting() - existing = await app.db_logic.get_setting_full(input.name) - if input.alias: - alias_canonical_name = (await app.db_logic.get_canonical_names([input.alias]))[input.alias] - else: - alias_canonical_name = None - if existing is None: - not_cf = await app.db_logic.get_not_found_context_features(input.configurable_features) - if not_cf: - return PlainTextResponse(f'{not_cf} are not acceptable context features', - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY) - if alias_canonical_name: - return PlainTextResponse(f"alias '{input.alias}' used by another setting", - status_code=status.HTTP_409_CONFLICT) - logger.info('creating new setting', extra={'setting_name': new_setting.name}) - await app.db_logic.add_setting(new_setting, alias=input.alias) - return DeclareSettingOutput(created=True, changed=[], incomplete={}) - - to_change: Dict[str, Any] = {} - changed = [] - incomplete: Dict[str, Any] = {} - - existing_setting_cfs = frozenset(existing.configurable_features) - new_setting_cfs = frozenset(new_setting.configurable_features) - - new_configurable_features = new_setting_cfs - existing_setting_cfs - if new_configurable_features: - # we need to make sure the new CFs are actually CFs - not_cf = await app.db_logic.get_not_found_context_features(new_configurable_features) - if not_cf: - return PlainTextResponse(f'{not_cf} are not acceptable context features', - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY) - changed.append('configurable_features') - logger.info("adding new configurable features to setting", - extra={'setting_name': existing.name, 'new_configurable_features': new_configurable_features}) - - missing_cf = existing_setting_cfs - new_setting_cfs - if missing_cf: - # note: there is a slight potential mislead here. If a user both declares new CFs and omits existing - # CFs, the new CFs will not appear in the response. This is fine for now - incomplete['configurable_features'] = existing.configurable_features - - type_ = setting_type(existing.raw_type) - if new_setting.type < type_: - incomplete['type'] = existing.raw_type - elif not type_ <= new_setting.type: - return PlainTextResponse( - f'Setting already exists with conflicting type. Expected {type_} (or upgradable one), ' - f'got {new_setting.type}', status_code=status.HTTP_409_CONFLICT - ) - if type_ != new_setting.type: - to_change['type'] = str(new_setting.type) - changed.append('type') - - if existing.default_value != new_setting.default_value: - to_change['default_value'] = str(orjson.dumps(new_setting.default_value), 'utf-8') - changed.append('default_value') - - is_new_alias = False - if input.alias and input.alias not in existing.aliases: - if alias_canonical_name and alias_canonical_name != existing.name: - return PlainTextResponse(f"alias '{input.alias}' used by another setting", - status_code=status.HTTP_409_CONFLICT) - changed.append('alias') - is_new_alias = True - - # we need to get which metadata keys are changed - metadata_changed = existing.metadata.keys() ^ new_setting.metadata.keys() - metadata_changed.update( - k for (k, v) in existing.metadata.items() if (k in new_setting.metadata and new_setting.metadata[k] != v) - ) - if metadata_changed: - logger.info('changing setting metadata', - extra={'setting_name': existing.name, 'new_metadata': new_setting.metadata}) - changed.extend('metadata.' + k for k in sorted(metadata_changed)) - new_metadata = new_setting.metadata - else: - new_metadata = None - - if to_change or new_configurable_features or is_new_alias: - await app.db_logic.update_setting(existing.name, to_change, new_configurable_features, new_metadata, - input.alias) - return DeclareSettingOutput(created=False, changed=changed, incomplete=incomplete) +router.add_api_route('/declare', declare_setting_endpoint, **declare_setting_enpoint_args) @router.delete('/{name}', status_code=status.HTTP_204_NO_CONTENT, response_class=Response) @@ -162,7 +28,8 @@ async def delete_setting(name: str, app: HeksherApp = application): """ Delete a setting. """ - setting = await app.db_logic.get_setting(name, include_metadata=False) # for aliasing + setting = await app.db_logic.get_setting(name, include_metadata=False, include_aliases=False, + include_configurable_features=False) # for aliasing if not setting: return PlainTextResponse('setting name not found', status_code=status.HTTP_404_NOT_FOUND) deleted = await app.db_logic.delete_setting(setting.name) @@ -178,6 +45,7 @@ class GetSettingOutput(ORJSONModel): default_value: Any = Field(description="the default value of the setting") metadata: Dict[MetadataKey, Any] = Field(description="additional metadata of the setting") aliases: List[str] = Field(description="aliases for the setting's name") + version: str = Field(description="the version of the setting") @router.get('/{name}', response_model=GetSettingOutput, @@ -190,17 +58,21 @@ async def get_setting(name: str, app: HeksherApp = application): """ Get details on a setting. """ - setting = await app.db_logic.get_setting_full(name) + setting = await app.db_logic.get_setting(name, include_metadata=True, include_aliases=True, + include_configurable_features=True) if not setting: return PlainTextResponse(f'the setting {name} does not exist', status_code=status.HTTP_404_NOT_FOUND) return GetSettingOutput(name=setting.name, configurable_features=setting.configurable_features, type=setting.raw_type, default_value=setting.default_value, metadata=setting.metadata, - aliases=setting.aliases) + aliases=setting.aliases, version=setting.version) # https://github.com/tiangolo/fastapi/issues/2724 class GetSettingsOutput_Setting(ORJSONModel): name: str = Field(description="The name of the setting") + type: str = Field(description="the type of the setting") + default_value: Any = Field(description="the default value of the setting") + version: str = Field(description="the version of the setting") class GetSettingsOutput(ORJSONModel): @@ -211,8 +83,6 @@ class GetSettingsOutputWithData_Setting(GetSettingsOutput_Setting): configurable_features: List[str] = Field( description="a list of the context features the setting can be configured by" ) - type: str = Field(description="the type of the setting") - default_value: Any = Field(description="the default value of the setting") metadata: Dict[MetadataKey, Any] = Field(description="additional metadata of the setting") aliases: List[SettingName] = Field(description="aliases for the setting's name") @@ -227,7 +97,8 @@ async def get_settings(include_additional_data: bool = False, app: HeksherApp = List all the settings in the service """ if include_additional_data: - full_results = await app.db_logic.get_all_settings_full() + full_results = await app.db_logic.get_all_settings(include_metadata=True, include_aliases=True, + include_configurable_features=True) return GetSettingsOutputWithData(settings=[ GetSettingsOutputWithData_Setting( name=spec.name, @@ -236,19 +107,25 @@ async def get_settings(include_additional_data: bool = False, app: HeksherApp = default_value=spec.default_value, metadata=spec.metadata, aliases=spec.aliases, + version=spec.version, ) for spec in full_results ]) else: - results = await app.db_logic.get_all_settings_names() + results = await app.db_logic.get_all_settings(include_metadata=False, include_aliases=False, + include_configurable_features=False) return GetSettingsOutput(settings=[ GetSettingsOutput_Setting( - name=name - ) for name in results + name=spec.name, + type=spec.raw_type, + default_value=spec.default_value, + version=spec.version, + ) for spec in results ]) class PutSettingTypeInput(ORJSONModel): type: SettingType = Field(description='the new setting type to set') + version: SettingVersion = Field(description='the new version of the setting') class PutSettingTypeConflictOutput(ORJSONModel): @@ -268,30 +145,43 @@ async def set_setting_type(name: str, input: PutSettingTypeInput, app: HeksherAp """ Change The type of a setting """ - setting = await app.db_logic.get_setting(name, include_metadata=False) + setting = await app.db_logic.get_setting(name, include_metadata=False, include_aliases=False, + include_configurable_features=False) if not setting: return PlainTextResponse(f'the setting {name} does not exist', status_code=status.HTTP_404_NOT_FOUND) + existing_version = parse_setting_version(setting.version) + new_version = parse_setting_version(input.version) + if existing_version == new_version and input.type == setting.type: + return Response(status_code=status.HTTP_204_NO_CONTENT) + if existing_version >= new_version: + return PlainTextResponse(f'the setting {name} is at a higher version than the request ({existing_version})', + status_code=status.HTTP_409_CONFLICT) + if input.type == setting.type: + # we only need to do a version bump + async with app.db_logic.db_engine.begin() as conn: + await app.db_logic.bump_setting_version(conn, name, input.version) + return Response(status_code=status.HTTP_204_NO_CONTENT) conflicts = [] - new_type = input.type - if new_type == setting.type: - return None - if not new_type.validate(setting.default_value): + if not input.type.validate(setting.default_value): conflicts.append(f'the default value {setting.default_value!r} does not match the new type') rules = await app.db_logic.get_rules_for_setting(setting.name) - bad_rules = {rule_id: rule_value for (rule_id, rule_value) in rules if not new_type.validate(rule_value)} + bad_rules = {rule_id: rule_value for (rule_id, rule_value) in rules if not input.type.validate(rule_value)} if bad_rules: conditions = await app.db_logic.get_rules_feature_values(list(bad_rules.keys())) for rule_id, value in bad_rules.items(): conflicts.append(f'rule {rule_id} ({conditions[rule_id]}) has incompatible value {value}') + if not (input.type < setting.type) and existing_version[0] == new_version[0]: + conflicts.append(f'cannot change type to non-subtype {input.type} in a minor version bump') if conflicts: return JSONResponse(PutSettingTypeConflictOutput(conflicts=conflicts).dict(), status_code=status.HTTP_409_CONFLICT) - await app.db_logic.set_setting_type(setting.name, new_type) + await app.db_logic.set_setting_type(setting.name, input.type, input.version) return None class RenameSettingInput(ORJSONModel): name: SettingName = Field(description="the new name for the setting") + version: SettingVersion = Field(description="the new version for the setting") @router.put('/{name}/name', status_code=status.HTTP_204_NO_CONTENT, response_class=Response, @@ -316,9 +206,21 @@ async def rename_setting(name: str, input: RenameSettingInput, app: HeksherApp = # if the canonical name is None - this setting does not exist if not canonical_name: return PlainTextResponse('setting does not exist', status_code=status.HTTP_404_NOT_FOUND) - # we check that the names differ, otherwise nothing should be done + setting = await app.db_logic.get_setting(canonical_name, include_metadata=False, include_aliases=False, + include_configurable_features=False) + existing_version = parse_setting_version(setting.version) + new_version = parse_setting_version(input.version) + if existing_version == new_version and canonical_name == input.name: + return Response(status_code=status.HTTP_204_NO_CONTENT) + elif existing_version > new_version: + return PlainTextResponse(f'The setting {name} already has a newer version ({setting.version})', + status_code=status.HTTP_409_CONFLICT) + if input.name == canonical_name: - return None + # we just need to version bump + async with app.db_logic.db_engine.begin() as conn: + await app.db_logic.bump_setting_version(conn, canonical_name, input.version) + return Response(status_code=status.HTTP_204_NO_CONTENT) # the second entry: given new name -> None # if the value is not None - this name/alias already exists if names_map[input.name] is not None: @@ -327,10 +229,62 @@ async def rename_setting(name: str, input: RenameSettingInput, app: HeksherApp = # otherwise - the operation cannot be done since the new name already exists as another setting's name or alias if names_map[input.name] != canonical_name: return PlainTextResponse('name already exists', status_code=status.HTTP_409_CONFLICT) - - await app.db_logic.rename_setting(canonical_name, input.name) + await app.db_logic.rename_setting(canonical_name, input.name, input.version) return None +class ConfigurableFeaturesInput(ORJSONModel): + configurable_features: List[str] = Field(description="the new configurable features for the setting", min_items=1, + unique=True) + version: SettingVersion = Field(description="the new version for the setting") + + +@router.put('/{name}/configurable_features', status_code=status.HTTP_204_NO_CONTENT, response_class=Response, + responses={ + status.HTTP_404_NOT_FOUND: { + "description": "The setting does not exist." + }, + status.HTTP_409_CONFLICT: { + "description": "One of the configurable features removed is in use by a rule. Or the version is " + "incompatible with the setting state." + } + }) +async def set_configurable_features(name: str, input: ConfigurableFeaturesInput, app: HeksherApp = application): + setting = await app.db_logic.get_setting(name, include_metadata=False, include_aliases=False, + include_configurable_features=True) + if not setting: + return PlainTextResponse(f'the setting {name} does not exist', status_code=status.HTTP_404_NOT_FOUND) + existing_version = parse_setting_version(setting.version) + new_version = parse_setting_version(input.version) + existing_cfs = frozenset(setting.configurable_features) + new_cfs = frozenset(input.configurable_features) + if existing_version == new_version and new_cfs == existing_cfs: + return Response(status_code=status.HTTP_204_NO_CONTENT) + if existing_version >= new_version: + return PlainTextResponse(f'the setting {name} is at a higher version than the request ({existing_version})', + status_code=status.HTTP_409_CONFLICT) + if new_cfs == existing_cfs: + # we only need to do a version bump + async with app.db_logic.db_engine.begin() as conn: + await app.db_logic.bump_setting_version(conn, name, input.version) + return Response(status_code=status.HTTP_204_NO_CONTENT) + removed_cfs = existing_cfs - new_cfs + if removed_cfs: + # check if any rules are using the removed configurable features + actual_cfs_in_use = await app.db_logic.get_actual_configurable_features(setting.name) + removed_cfs_in_use = removed_cfs & actual_cfs_in_use.keys() + if removed_cfs_in_use: + rule_ids = list(chain.from_iterable(actual_cfs_in_use[cf] for cf in removed_cfs_in_use)) + return PlainTextResponse(f'Configurable features {removed_cfs_in_use} are in use by rules {rule_ids}', + status_code=status.HTTP_409_CONFLICT) + if not (existing_cfs > new_cfs) and existing_version[0] == new_version[0]: + # can't add new cfs on the same major + return PlainTextResponse('Cannot add new configurable features on a minor version bump', + status_code=status.HTTP_409_CONFLICT) + async with app.db_logic.db_engine.begin() as conn: + await set_settings_configurable_features(conn, setting.name, input.configurable_features, input.version) + return Response(status_code=status.HTTP_204_NO_CONTENT) + + router.include_router(metadata_router) v1_router.include_router(router) diff --git a/heksher/api/v1/settings_metadata.py b/heksher/api/v1/settings_metadata.py index 8579e44..608f76d 100644 --- a/heksher/api/v1/settings_metadata.py +++ b/heksher/api/v1/settings_metadata.py @@ -7,8 +7,9 @@ from starlette.responses import PlainTextResponse from heksher.api.v1.util import ORJSONModel, application -from heksher.api.v1.validation import MetadataKey +from heksher.api.v1.validation import MetadataKey, SettingVersion from heksher.app import HeksherApp +from heksher.db_logic.util import parse_setting_version router = APIRouter() logger = getLogger(__name__) @@ -16,6 +17,7 @@ class InputSettingMetadata(ORJSONModel): metadata: Dict[MetadataKey, Any] = Field(default_factory=dict, description="user-defined metadata of the setting") + version: SettingVersion = Field(description='the new version to assign to the setting') @router.post('/{name}/metadata', status_code=status.HTTP_204_NO_CONTENT, response_class=Response) @@ -23,12 +25,16 @@ async def update_setting_metadata(name: str, input: InputSettingMetadata, app: H """ Update the setting's metadata """ - if not input.metadata: - return None - setting = await app.db_logic.get_setting(name, include_metadata=False) + setting = await app.db_logic.get_setting(name, include_metadata=False, include_aliases=False, + include_configurable_features=False) if not setting: return PlainTextResponse(f'the setting {name} does not exist', status_code=status.HTTP_404_NOT_FOUND) - await app.db_logic.update_setting_metadata(setting.name, input.metadata) + existing_version = parse_setting_version(setting.version) + new_version = parse_setting_version(input.version) + if existing_version >= new_version: + return PlainTextResponse(f'The setting {name} already has a newer version ({setting.version})', + status_code=status.HTTP_409_CONFLICT) + await app.db_logic.update_setting_metadata(setting.name, input.metadata, input.version) @router.put('/{name}/metadata', status_code=status.HTTP_204_NO_CONTENT, response_class=Response) @@ -36,18 +42,21 @@ async def replace_setting_metadata(name: str, input: InputSettingMetadata, app: """ Change the current metadata of the setting. """ - setting = await app.db_logic.get_setting(name, include_metadata=False) + setting = await app.db_logic.get_setting(name, include_metadata=False, include_aliases=False, + include_configurable_features=False) if not setting: return PlainTextResponse(f'the setting {name} does not exist', status_code=status.HTTP_404_NOT_FOUND) - if not input.metadata: - # empty dictionary equals to deleting the metadata - await app.db_logic.delete_setting_metadata(setting.name) - else: - await app.db_logic.replace_setting_metadata(setting.name, input.metadata) + existing_version = parse_setting_version(setting.version) + new_version = parse_setting_version(input.version) + if existing_version >= new_version: + return PlainTextResponse(f'The setting {name} already has a newer version ({setting.version})', + status_code=status.HTTP_409_CONFLICT) + await app.db_logic.replace_setting_metadata(setting.name, input.metadata, input.version) class PutSettingMetadataKey(ORJSONModel): - value: Any = Field(description="the new value of the given key and setting in the setting's metadata") + value: Any = Field(..., description="the new value of the given key and setting in the setting's metadata") + version: SettingVersion = Field(description='the new version to assign to the setting') @router.put('/{name}/metadata/{key}', status_code=status.HTTP_204_NO_CONTENT, response_class=Response) @@ -56,32 +65,55 @@ async def update_setting_metadata_key(name: str, key: MetadataKey, input: PutSet """ Updates the current metadata of the setting. Existing keys won't be deleted. """ - setting = await app.db_logic.get_setting(name, include_metadata=False) + setting = await app.db_logic.get_setting(name, include_metadata=False, include_aliases=False, + include_configurable_features=False) if not setting: return PlainTextResponse(f'the setting {name} does not exist', status_code=status.HTTP_404_NOT_FOUND) - await app.db_logic.update_setting_metadata_key(setting.name, key, input.value) + existing_version = parse_setting_version(setting.version) + new_version = parse_setting_version(input.version) + if existing_version >= new_version: + return PlainTextResponse(f'The setting {name} already has a newer version ({setting.version})', + status_code=status.HTTP_409_CONFLICT) + await app.db_logic.update_setting_metadata_key(setting.name, key, input.value, input.version) + + +class DeleteSettingMetadataInput(ORJSONModel): + version: SettingVersion = Field(description='the new version to assign to the setting') @router.delete('/{name}/metadata', status_code=status.HTTP_204_NO_CONTENT, response_class=Response) -async def delete_setting_metadata(name: str, app: HeksherApp = application): +async def delete_setting_metadata(name: str, input: DeleteSettingMetadataInput, app: HeksherApp = application): """ Delete a setting's metadata. """ - setting = await app.db_logic.get_setting(name, include_metadata=False) + setting = await app.db_logic.get_setting(name, include_metadata=False, include_aliases=False, + include_configurable_features=False) if not setting: return PlainTextResponse(f'the setting {name} does not exist', status_code=status.HTTP_404_NOT_FOUND) - await app.db_logic.delete_setting_metadata(setting.name) + existing_version = parse_setting_version(setting.version) + new_version = parse_setting_version(input.version) + if existing_version >= new_version: + return PlainTextResponse(f'The setting {name} already has a newer version ({setting.version})', + status_code=status.HTTP_409_CONFLICT) + await app.db_logic.delete_setting_metadata(setting.name, input.version) @router.delete('/{name}/metadata/{key}', status_code=status.HTTP_204_NO_CONTENT, response_class=Response) -async def delete_rule_key_from_metadata(name: str, key: MetadataKey, app: HeksherApp = application): +async def delete_rule_key_from_metadata(name: str, key: MetadataKey, input: DeleteSettingMetadataInput, + app: HeksherApp = application): """ Delete a specific key from the setting's metadata. """ - setting = await app.db_logic.get_setting(name, include_metadata=False) + setting = await app.db_logic.get_setting(name, include_metadata=False, include_aliases=False, + include_configurable_features=False) if not setting: return PlainTextResponse(f'the setting {name} does not exist', status_code=status.HTTP_404_NOT_FOUND) - await app.db_logic.delete_setting_metadata_key(setting.name, key) + existing_version = parse_setting_version(setting.version) + new_version = parse_setting_version(input.version) + if existing_version >= new_version: + return PlainTextResponse(f'The setting {name} already has a newer version ({setting.version})', + status_code=status.HTTP_409_CONFLICT) + await app.db_logic.delete_setting_metadata_key(setting.name, key, input.version) class GetSettingMetadataOutput(ORJSONModel): @@ -99,6 +131,7 @@ async def get_setting_metadata(name: str, app: HeksherApp = application): """ Get metadata of a setting. """ - if not (setting := await app.db_logic.get_setting(name, include_metadata=True)): + if not (setting := await app.db_logic.get_setting(name, include_metadata=True, include_aliases=False, + include_configurable_features=False)): return PlainTextResponse(f'the setting {name} does not exist', status_code=status.HTTP_404_NOT_FOUND) return GetSettingMetadataOutput(metadata=setting.metadata) diff --git a/heksher/api/v1/validation.py b/heksher/api/v1/validation.py index ee75584..f3cd12c 100644 --- a/heksher/api/v1/validation.py +++ b/heksher/api/v1/validation.py @@ -2,13 +2,15 @@ from pydantic import constr -if TYPE_CHECKING: +if TYPE_CHECKING: # pragma: nocover ContextFeatureName = str ContextFeatureValue = str SettingName = str MetadataKey = str + SettingVersion = str else: ContextFeatureName = constr(regex='[a-zA-Z_0-9]+$') ContextFeatureValue = constr(regex='[a-zA-Z_0-9]+$') SettingName = constr(regex='[a-zA-Z_0-9.]+$') MetadataKey = constr(regex='[a-zA-Z0-9_-]+$') + SettingVersion = constr(regex=r'^[0-9]+\.[0-9]+$') diff --git a/heksher/db_logic/logic_base.py b/heksher/db_logic/logic_base.py index aa23160..6520919 100644 --- a/heksher/db_logic/logic_base.py +++ b/heksher/db_logic/logic_base.py @@ -6,3 +6,6 @@ class DBLogicBase: A base class for DBLogic mixins """ db_engine: AsyncEngine + + async def bump_setting_version(self, conn, setting_name: str, new_version: str) -> None: + pass diff --git a/heksher/db_logic/metadata.py b/heksher/db_logic/metadata.py index 960bb5d..2c8f0af 100644 --- a/heksher/db_logic/metadata.py +++ b/heksher/db_logic/metadata.py @@ -6,7 +6,8 @@ settings = Table('settings', metadata, Column('name', String, primary_key=True), Column('type', String, nullable=False), - Column('default_value', String, nullable=True), + Column('default_value', String, nullable=False), + Column('version', String, nullable=False), ) context_features = Table('context_features', metadata, @@ -16,34 +17,38 @@ configurable = Table('configurable', metadata, Column('setting', ForeignKey(settings.columns.name, - ondelete="CASCADE", onupdate="CASCADE"), index=True), - Column('context_feature', ForeignKey(context_features.columns.name)), + ondelete="CASCADE", onupdate="CASCADE"), index=True, nullable=False), + Column('context_feature', ForeignKey(context_features.columns.name), nullable=False), UniqueConstraint('setting', 'context_feature') ) rules = Table('rules', metadata, Column('id', Integer, primary_key=True, autoincrement=True), - Column('setting', ForeignKey(settings.columns.name, ondelete="CASCADE", onupdate="CASCADE"), index=True), + Column('setting', ForeignKey(settings.columns.name, ondelete="CASCADE", onupdate="CASCADE"), index=True, + nullable=False), Column('value', String, nullable=False), ) conditions = Table('conditions', metadata, - Column('rule', ForeignKey(rules.columns.id, ondelete="CASCADE", onupdate="CASCADE"), index=True), - Column('context_feature', ForeignKey(context_features.columns.name)), + Column('rule', ForeignKey(rules.columns.id, ondelete="CASCADE", onupdate="CASCADE"), index=True, + nullable=False), + Column('context_feature', ForeignKey(context_features.columns.name), nullable=False), Column('feature_value', String, nullable=False), UniqueConstraint('rule', 'context_feature'), ) setting_metadata = Table('setting_metadata', metadata, Column('setting', ForeignKey(settings.columns.name, - ondelete="CASCADE", onupdate="CASCADE"), index=True), + ondelete="CASCADE", onupdate="CASCADE"), index=True, + nullable=False), Column('key', String, nullable=False), Column('value', JSONB, nullable=False), UniqueConstraint('setting', 'key'), ) rule_metadata = Table('rule_metadata', metadata, - Column('rule', ForeignKey(rules.columns.id, ondelete="CASCADE", onupdate="CASCADE"), index=True), + Column('rule', ForeignKey(rules.columns.id, ondelete="CASCADE", onupdate="CASCADE"), index=True, + nullable=False), Column('key', String, nullable=False), Column('value', JSONB, nullable=False), UniqueConstraint('rule', 'key'), @@ -51,6 +56,7 @@ setting_aliases = Table('setting_aliases', metadata, Column('setting', ForeignKey(settings.columns.name, - ondelete="CASCADE", onupdate="CASCADE"), index=True), + ondelete="CASCADE", onupdate="CASCADE"), index=True, + nullable=False), Column('alias', String, primary_key=True), ) diff --git a/heksher/db_logic/rule.py b/heksher/db_logic/rule.py index a46bc4d..51431a2 100644 --- a/heksher/db_logic/rule.py +++ b/heksher/db_logic/rule.py @@ -248,9 +248,8 @@ async def query_rules(self, setting_names: List[str], conditions.c.rule == conditions_.c.rule, inv_match )) .exists()), + rules.c.setting.in_(setting_names) ] - if setting_names is not None: - clauses.append(rules.c.setting.in_(setting_names)) query = (select() .add_columns(rules.c.id, conditions.c.context_feature, conditions.c.feature_value) @@ -323,3 +322,18 @@ async def get_rules_for_setting(self, setting_name: str) -> Sequence[BareRuleSpe result = ((await conn.execute(select([rules.c.id, rules.c.value]).where(rules.c.setting == setting_name))) .all()) return [BareRuleSpec(id_, orjson.loads(v)) for id_, v in result] + + async def get_actual_configurable_features(self, setting_name: str) -> Dict[str, List[int]]: + """ + Get all the rules that are configured for each context feature for a setting + """ + async with self.db_engine.connect() as conn: + results = (await conn.execute( + select([conditions.c.context_feature, rules.c.id]) + .select_from(rules.join(conditions, rules.c.id == conditions.c.rule)) + .where(rules.c.setting == setting_name) + .order_by(conditions.c.context_feature) + )).all() + + return {context_feature: [rule_id for (_, rule_id) in rows] + for (context_feature, rows) in groupby(results, key=itemgetter(0))} diff --git a/heksher/db_logic/setting.py b/heksher/db_logic/setting.py index 9569530..83d6e8b 100644 --- a/heksher/db_logic/setting.py +++ b/heksher/db_logic/setting.py @@ -1,24 +1,36 @@ +from dataclasses import dataclass +from functools import cached_property from itertools import groupby -from typing import Any, Dict, Iterable, List, Mapping, NamedTuple, Optional +from typing import Any, Dict, Iterable, List, Optional import orjson from _operator import itemgetter from sqlalchemy import String, column, delete, join, or_, select, values from sqlalchemy.dialects.postgresql import insert +from sqlalchemy.ext.asyncio import AsyncConnection from heksher.db_logic.logic_base import DBLogicBase from heksher.db_logic.metadata import configurable, context_features, setting_aliases, setting_metadata, settings -from heksher.setting import Setting from heksher.setting_types import SettingType, setting_type -class SettingSpec(NamedTuple): +@dataclass +class SettingSpec: name: str raw_type: Optional[str] - default_value: Optional[Any] + raw_default_value: Any metadata: Optional[Dict[str, Any]] configurable_features: Optional[List[str]] - aliases: List[str] + aliases: Optional[List[str]] + version: str + + @cached_property + def type(self) -> SettingType: + return setting_type(self.raw_type) + + @cached_property + def default_value(self): + return orjson.loads(self.raw_default_value) class SettingMixin(DBLogicBase): @@ -44,13 +56,14 @@ async def get_canonical_names(self, names_or_aliases: Iterable[str]) -> Dict[str isouter=True), or_(settings.c.name == names_table.c.n, setting_aliases.c.alias == names_table.c.n), isouter=True) - ) + ) .distinct() ) results = (await conn.execute(stmt)).all() return dict(results) - async def get_setting(self, name_or_alias: str, *, include_metadata: bool) -> Optional[Setting]: + async def get_setting(self, name_or_alias: str, *, include_metadata: bool, include_aliases: bool, + include_configurable_features: bool) -> Optional[SettingSpec]: """ Args: name_or_alias: The name/alias of a setting @@ -61,7 +74,7 @@ async def get_setting(self, name_or_alias: str, *, include_metadata: bool) -> Op """ async with self.db_engine.connect() as conn: stmt = ( - select([settings.c.name, settings.c.type, settings.c.default_value]) + select([settings.c.name, settings.c.type, settings.c.default_value, settings.c.version]) .select_from( join(settings, setting_aliases, settings.c.name == setting_aliases.c.setting, isouter=True) ) @@ -74,16 +87,31 @@ async def get_setting(self, name_or_alias: str, *, include_metadata: bool) -> Op return None setting_name = data_row['name'] + if include_aliases: + stmt = ( + select([setting_aliases.c.alias]) + .select_from( + join(settings, setting_aliases, settings.c.name == setting_aliases.c.setting) + ) + .where(settings.c.name == setting_name) + .order_by(setting_aliases.c.alias) + ) + aliases = (await conn.execute(stmt)).scalars().all() + else: + aliases = None - stmt = ( - select([configurable.c.context_feature]) - .select_from( - join(configurable, context_features, configurable.c.context_feature == context_features.c.name) + if include_configurable_features: + stmt = ( + select([configurable.c.context_feature]) + .select_from( + join(configurable, context_features, configurable.c.context_feature == context_features.c.name) + ) + .where(configurable.c.setting == setting_name) + .order_by(context_features.c.index) ) - .where(configurable.c.setting == setting_name) - .order_by(context_features.c.index) - ) - configurable_rows = (await conn.execute(stmt)).scalars().all() + configurable_features = (await conn.execute(stmt)).scalars().all() + else: + configurable_features = None if include_metadata: stmt = ( @@ -94,17 +122,17 @@ async def get_setting(self, name_or_alias: str, *, include_metadata: bool) -> Op else: metadata_ = None - type_ = setting_type(data_row['type']) - default_value_ = orjson.loads(data_row['default_value']) - return Setting( + return SettingSpec( setting_name, - type_, - default_value_, - configurable_rows, - metadata_ + data_row['type'], + data_row['default_value'], + metadata_, + configurable_features, + aliases, + data_row['version'] ) - async def add_setting(self, setting: Setting, alias: Optional[str] = None): + async def add_setting(self, setting: SettingSpec): """ Add a setting to the DB Args: @@ -116,6 +144,7 @@ async def add_setting(self, setting: Setting, alias: Optional[str] = None): name=setting.name, type=str(setting.type), default_value=str(orjson.dumps(setting.default_value), 'utf-8'), + version=setting.version ) ) await conn.execute( @@ -129,48 +158,52 @@ async def add_setting(self, setting: Setting, alias: Optional[str] = None): [{'setting': setting.name, 'key': k, 'value': v} for (k, v) in setting.metadata.items()] ) ) - if alias: - await conn.execute( - insert(setting_aliases).values( - [{'setting': setting.name, 'alias': alias}] - ) - ) + assert not setting.aliases # newly added settings can't have aliases - async def update_setting(self, name: str, changed: Mapping[str, Any], new_contexts: Iterable[str], - new_metadata: Optional[Dict[str, Any]], alias: Optional[str]): - """ - Edit an existing setting in the DB - Args: - name: The name of the setting to edit - changed: The fields of the setting to change - new_contexts: An iterable of new context names to assign to the setting as configurable - new_metadata: Optional, new metadata to assign to the setting - """ + async def update_setting(self, old_name: str, new_name: Optional[str], configurable_features: Optional[List[str]], + type: Optional[SettingType], default_value: Optional[Any], + metadata: Optional[Dict[str, Any]], version: str): async with self.db_engine.begin() as conn: - if changed: + if configurable_features is not None: await conn.execute( - settings.update().where(settings.c.name == name).values(**changed) + configurable.delete().where(configurable.c.setting == old_name) ) - if new_contexts: await conn.execute( configurable.insert().values( - [{'setting': name, 'context_feature': cf} for cf in new_contexts] + [{'setting': old_name, 'context_feature': cf} for cf in configurable_features] ) ) - if new_metadata is not None: + if metadata is not None: await conn.execute( - setting_metadata.delete().where(setting_metadata.c.setting == name) + setting_metadata.delete().where(setting_metadata.c.setting == old_name) ) await conn.execute( setting_metadata.insert().values( - [{'setting': name, 'key': k, 'value': v} for (k, v) in new_metadata.items()] + [{'setting': old_name, 'key': k, 'value': v} for (k, v) in metadata.items()] ) ) - if alias: + + # we change the row last, so that the other tables can still refer to the setting by it's old name + row_changes = {'version': version} + if new_name: + row_changes['name'] = new_name + if type: + row_changes['type'] = str(type) + if default_value: + row_changes['default_value'] = str(orjson.dumps(default_value), 'utf-8') + await conn.execute( + settings.update().where(settings.c.name == old_name).values(**row_changes) + ) + + if new_name: + # remove the new name from the aliases table, if it exists + await conn.execute( + setting_aliases.delete().where(setting_aliases.c.alias == new_name) + ) await conn.execute( insert(setting_aliases).values( - [{'setting': name, 'alias': alias}] - ).on_conflict_do_nothing() + [{'setting': new_name, 'alias': old_name}] + ) ) async def delete_setting(self, name: str) -> bool: @@ -185,88 +218,72 @@ async def delete_setting(self, name: str) -> bool: resp = (await conn.execute(settings.delete().where(settings.c.name == name))).rowcount return resp == 1 - async def get_setting_full(self, name_or_alias: str) -> Optional[SettingSpec]: - """ - Args: - name_or_alias: The name/alias of a setting - Returns: - The setting specs object for the setting in the DB with the same name, or None if it does not exist - """ - setting = await self.get_setting(name_or_alias, include_metadata=True) - - if not setting: - return None - - async with self.db_engine.connect() as conn: - alias_rows = (await conn.execute( - select([setting_aliases.c.alias]) - .where(setting_aliases.c.setting == setting.name) - )).scalars().all() - - return SettingSpec( - setting.name, - str(setting.type), - setting.default_value, - setting.metadata, - list(setting.configurable_features), - list(alias_rows) - ) - - async def get_all_settings_full(self) -> List[SettingSpec]: + async def get_all_settings(self, include_configurable_features: bool, include_metadata: bool, + include_aliases: bool) -> List[SettingSpec]: """ Returns: A list of all setting specs in the DB """ - select_query = select([settings.c.name, settings.c.type, settings.c.default_value]).order_by(settings.c.name) + select_query = select([settings.c.name, settings.c.type, settings.c.default_value, settings.c.version]) \ + .order_by(settings.c.name) async with self.db_engine.connect() as conn: records = (await conn.execute(select_query)).mappings().all() - configurable_rows = (await conn.execute( - select([configurable.c.setting, configurable.c.context_feature]) - .select_from(join(configurable, context_features, - configurable.c.context_feature == context_features.c.name)) - .order_by(configurable.c.setting, context_features.c.index) - )).mappings().all() - metadata_rows = await conn.execute( - setting_metadata.select().order_by(setting_metadata.c.setting) - ) - alias_rows = await conn.execute( - setting_aliases.select().order_by(setting_aliases.c.setting) - ) + if include_configurable_features: + configurable_rows = (await conn.execute( + select([configurable.c.setting, configurable.c.context_feature]) + .select_from(join(configurable, context_features, + configurable.c.context_feature == context_features.c.name)) + .order_by(configurable.c.setting, context_features.c.index) + )).mappings().all() + else: + configurable_rows = None + if include_metadata: + metadata_rows = await conn.execute( + setting_metadata.select().order_by(setting_metadata.c.setting) + ) + else: + metadata_rows = None + if include_aliases: + alias_rows = await conn.execute( + setting_aliases.select().order_by(setting_aliases.c.setting) + ) + else: + alias_rows = None - configurables = { - setting: [row['context_feature'] for row in rows] - for (setting, rows) in groupby(configurable_rows, key=itemgetter('setting')) - } - metadata = { - setting: {k: v for (_, k, v) in rows} for (setting, rows) in groupby(metadata_rows, key=itemgetter(0)) - } - aliases = { - setting: [v for (_, v) in rows] for (setting, rows) in groupby(alias_rows, key=itemgetter(0)) - } + if include_configurable_features: + configurable_features = { + setting: [row['context_feature'] for row in rows] + for (setting, rows) in groupby(configurable_rows, key=itemgetter('setting')) + } + else: + configurable_features = None + if include_metadata: + metadata = { + setting: {k: v for (_, k, v) in rows} for (setting, rows) in groupby(metadata_rows, key=itemgetter(0)) + } + else: + metadata = None + if include_aliases: + aliases = { + setting: [v for (_, v) in rows] for (setting, rows) in groupby(alias_rows, key=itemgetter(0)) + } + else: + aliases = None return [ SettingSpec( row['name'], row['type'], - orjson.loads(row['default_value']), - metadata.get(row['name'], {}), - configurables[row['name']], - aliases.get(row['name'], []), + row['default_value'], + metadata.get(row['name'], {}) if include_metadata else None, + configurable_features[row['name']] if include_configurable_features else None, + aliases.get(row['name'], []) if include_aliases else None, + row['version'] ) for row in records ] - async def get_all_settings_names(self) -> List[str]: - """ - Returns: - A list of all setting names in the DB - """ - select_query = select([settings.c.name]).order_by(settings.c.name) - - async with self.db_engine.connect() as conn: - return list((await conn.execute(select_query)).scalars().all()) - - async def set_setting_type(self, setting_name: str, new_type: SettingType): + async def set_setting_type(self, setting_name: str, new_type: SettingType, new_version: str): """ Change the type of a setting. Does not check validity. Args: @@ -274,21 +291,16 @@ async def set_setting_type(self, setting_name: str, new_type: SettingType): new_type: the new type of the setting """ async with self.db_engine.begin() as conn: - await conn.execute(settings.update().where(settings.c.name == setting_name).values(type=str(new_type))) + await conn.execute(settings.update().where(settings.c.name == setting_name) + .values(type=str(new_type), version=new_version)) - async def rename_setting(self, old_name: str, new_name: str): - """ - Change a canonical setting name to another one, adding the old one as an alias - Args: - old_name: The name of the setting to rename - new_name: The new name to set for the setting - """ + async def rename_setting(self, old_name: str, new_name: str, new_version: str): async with self.db_engine.begin() as conn: # this should cascade through all other tables await conn.execute( settings.update() .where(settings.c.name == old_name) - .values({"name": new_name}) + .values({"name": new_name, 'version': new_version}) ) # add the old name as an alias of the new one await conn.execute( @@ -301,3 +313,6 @@ async def rename_setting(self, old_name: str, new_name: str): delete(setting_aliases) .where(setting_aliases.c.setting == new_name, setting_aliases.c.alias == new_name) ) + + async def bump_setting_version(self, conn: AsyncConnection, setting_name: str, new_version: str): + await conn.execute(settings.update().where(settings.c.name == setting_name).values(version=new_version)) diff --git a/heksher/db_logic/setting_configurable_features.py b/heksher/db_logic/setting_configurable_features.py new file mode 100644 index 0000000..0f3be7f --- /dev/null +++ b/heksher/db_logic/setting_configurable_features.py @@ -0,0 +1,13 @@ +from typing import List + +from sqlalchemy.ext.asyncio import AsyncConnection + +from heksher.db_logic.metadata import configurable, settings + + +async def set_settings_configurable_features(conn: AsyncConnection, setting_name: str, configurable_features: List[str], + version: str): + await conn.execute(configurable.delete().where(configurable.c.setting == setting_name)) + await conn.execute(configurable.insert().values([ + {'setting': setting_name, 'context_feature': cf} for cf in configurable_features])) + await conn.execute(settings.update().values({'version': version}).where(settings.c.name == setting_name)) diff --git a/heksher/db_logic/setting_metadata.py b/heksher/db_logic/setting_metadata.py index 6ce36fa..174e6d0 100644 --- a/heksher/db_logic/setting_metadata.py +++ b/heksher/db_logic/setting_metadata.py @@ -8,72 +8,47 @@ class SettingMetadataMixin(DBLogicBase): - - async def update_setting_metadata(self, name: str, metadata: Dict[str, Any]): - """ - Update the metadata of the given setting. Similar to the dict.update() method, meaning that for existing keys, - the value will be updated, and new keys will be added as well. - Args: - name: the name of the setting to update it's metadata. - metadata: the metadata to update. - """ + async def update_setting_metadata(self, name: str, metadata: Dict[str, Any], new_version: str): async with self.db_engine.begin() as conn: - stmt = insert(setting_metadata).values( - [{'setting': name, 'key': k, 'value': v} for (k, v) in metadata.items()] - ) - await conn.execute(stmt.on_conflict_do_update(index_elements=[setting_metadata.c.setting, - setting_metadata.c.key], - set_={"value": stmt.excluded.value})) + if metadata: + stmt = insert(setting_metadata).values( + [{'setting': name, 'key': k, 'value': v} for (k, v) in metadata.items()] + ) + await conn.execute(stmt.on_conflict_do_update(index_elements=[setting_metadata.c.setting, + setting_metadata.c.key], + set_={"value": stmt.excluded.value})) + await self.bump_setting_version(conn, name, new_version) - async def replace_setting_metadata(self, name: str, new_metadata: Dict[str, Any]): - """ - Replace the metadata of the given setting with new metadata. - Args: - name: the name of the setting to change its metadata. - new_metadata: the new metadata for the setting. - """ + async def replace_setting_metadata(self, name: str, new_metadata: Dict[str, Any], new_version: str): async with self.db_engine.begin() as conn: await conn.execute(setting_metadata.delete() .where(setting_metadata.c.setting == name) ) - await conn.execute( - setting_metadata.insert().values( - [{'setting': name, 'key': k, 'value': v} for (k, v) in new_metadata.items()] + if new_metadata: + await conn.execute( + setting_metadata.insert().values( + [{'setting': name, 'key': k, 'value': v} for (k, v) in new_metadata.items()] + ) ) - ) + await self.bump_setting_version(conn, name, new_version) - async def update_setting_metadata_key(self, name: str, key: str, new_value: Any): - """ - Updates a specific key of the setting's metadata. - Args: - name: the name of the setting to change its metadata. - key: the key to update. - new_value: the value to update for the given key. - """ + async def update_setting_metadata_key(self, name: str, key: str, new_value: Any, new_version: str): async with self.db_engine.begin() as conn: await conn.execute(insert(setting_metadata) .values([{'setting': name, 'key': key, 'value': new_value}]) .on_conflict_do_update(index_elements=[setting_metadata.c.setting, setting_metadata.c.key], set_={"value": new_value})) + await self.bump_setting_version(conn, name, new_version) - async def delete_setting_metadata(self, name: str): - """ - Remove a setting's metadata from the DB - Args: - name: the name of the setting to remove its metadata - """ + async def delete_setting_metadata(self, name: str, new_version: str): async with self.db_engine.begin() as conn: await conn.execute(setting_metadata.delete().where(setting_metadata.c.setting == name)) + await self.bump_setting_version(conn, name, new_version) - async def delete_setting_metadata_key(self, name: str, key: str): - """ - Remove a specific key from the setting's metadata - Args: - name: the name of the setting - key: the name of the key to be deleted from the setting metadata - """ + async def delete_setting_metadata_key(self, name: str, key: str, new_version: str): async with self.db_engine.begin() as conn: await conn.execute(setting_metadata.delete() .where(and_(setting_metadata.c.setting == name, setting_metadata.c.key == key))) + await self.bump_setting_version(conn, name, new_version) diff --git a/heksher/db_logic/util.py b/heksher/db_logic/util.py index a8f10c2..783c0c8 100644 --- a/heksher/db_logic/util.py +++ b/heksher/db_logic/util.py @@ -1,4 +1,3 @@ -import re from typing import Collection, Dict, Hashable, Sequence, Tuple, TypeVar, Union T = TypeVar('T', bound=Hashable) @@ -49,4 +48,6 @@ def supersequence_new_elements(supersequence: Sequence[T], subsequence: Collecti return tuple(new_elements.items()) -INLINE_VALIDATION_PATTERN = re.compile(r'[a-zA-Z0-9_\s]+') +def parse_setting_version(version: str) -> Tuple[int, int]: + major, _, minor = version.partition('.') + return int(major), int(minor) diff --git a/heksher/setting.py b/heksher/setting.py deleted file mode 100644 index cfc9c39..0000000 --- a/heksher/setting.py +++ /dev/null @@ -1,18 +0,0 @@ -from dataclasses import dataclass -from typing import Any, Dict, Sequence - -from heksher.setting_types import SettingType - - -@dataclass -class Setting: - name: str - """The name of a setting""" - type: SettingType - """The type of the setting""" - default_value: Any - """The default value of the setting, or None if there is no default value""" - configurable_features: Sequence[str] - """The configurable features of the setting, should be in hierarchical order""" - metadata: Dict[str, Any] - """user-defined metadata""" diff --git a/tests/blackbox/app/conftest.py b/tests/blackbox/app/conftest.py index a4eb72f..2a49474 100644 --- a/tests/blackbox/app/conftest.py +++ b/tests/blackbox/app/conftest.py @@ -56,7 +56,7 @@ async def app_client(monkeypatch, sql_service, purge_sql, check_indexes_of_cf): @fixture async def size_limit_setting(app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'size_limit', 'configurable_features': ['user', 'theme'], 'type': 'int', @@ -64,11 +64,7 @@ async def size_limit_setting(app_client): 'metadata': {'testing': True} })) res.raise_for_status() - assert res.json() == { - 'created': True, - 'changed': [], - 'incomplete': {} - } + assert res.json() == {'outcome': 'created'} @fixture diff --git a/tests/blackbox/app/test_v1api_aliases.py b/tests/blackbox/app/test_v1api_aliases.py index ec85ac1..30beb6f 100644 --- a/tests/blackbox/app/test_v1api_aliases.py +++ b/tests/blackbox/app/test_v1api_aliases.py @@ -1,6 +1,7 @@ import json from typing import Optional +from async_asgi_testclient.response import Response from pytest import fixture, mark, raises from requests import HTTPError @@ -15,53 +16,42 @@ def default_declare_params() -> dict: } -def _get_ok_data(response): - response.raise_for_status() +def _get_ok_data(response: Response): + if not response.ok: + raise HTTPError(f"Response is not OK {response.status_code}: {response.content}") return response.json() if response.content else None @mark.asyncio -async def test_settings(app_client, default_declare_params): - async def declare(name: str, alias: str): - default_declare_params.update({'name': name, 'alias': alias}) - return _get_ok_data(await app_client.put('/api/v1/settings/declare', data=json.dumps(default_declare_params))) +@mark.parametrize('first_version', ['1.0', None]) +async def test_settings(app_client, default_declare_params, first_version): + async def declare(name: str, alias: Optional[str], version: Optional[str]): + params = dict(default_declare_params) + params['name'] = name + if alias: + params['alias'] = alias + if version: + params['version'] = version + return _get_ok_data(await app_client.post('/api/v1/settings/declare', json=params)) async def get(name_or_alias: str): return _get_ok_data(await app_client.get(f'/api/v1/settings/{name_or_alias}')) - async def get_all(): - return _get_ok_data(await app_client.get('/api/v1/settings')) - - async def get_all_full(): - return _get_ok_data(await app_client.get('/api/v1/settings', query_string={'include_additional_data': True})) - - assert (await declare('god', 'elohim'))['created'] is True + assert (await declare('elohim', None, first_version))['outcome'] == 'created' + assert (await declare('god', 'elohim', '1.1'))['outcome'] == 'upgraded' res = await get('elohim') assert res['name'] == "god" assert res['aliases'] == ["elohim"] assert await get('god') == await get('elohim') - res = await declare('elohim', 'yahweh') - assert res['created'] is False - assert res['changed'] == ['alias'] + res = await declare('yahweh', 'god', '2.0') + assert res['outcome'] == 'upgraded' + assert res['previous_version'] == '1.1' + assert sum(diff.get('attribute') == 'name' for diff in res['differences']) == 1 res = await get('yahweh') - assert res['name'] == "god" - assert res['aliases'] == ["elohim", "yahweh"] - assert (await declare('moses', 'moshe'))['created'] is True - resp = await app_client.put('/api/v1/settings/moshe/type', data=json.dumps({'type': 'float'})) - resp.raise_for_status() - assert (await get('moshe'))['type'] == "float" - - assert (await get_all())["settings"] == [{"name": "god"}, {"name": "moses"}] - assert (await get_all_full())["settings"] == [ - {'name': 'god', 'configurable_features': ['user', 'theme'], 'type': 'int', 'default_value': 5, - 'metadata': {'testing': True}, 'aliases': ['elohim', 'yahweh']}, - {'name': 'moses', 'configurable_features': ['user', 'theme'], 'type': 'float', 'default_value': 5, - 'metadata': {'testing': True}, 'aliases': ['moshe']} - ] + assert res['name'] == "yahweh" + assert res['aliases'] == ["elohim", "god"] - resp = await app_client.delete('/api/v1/settings/moshe') - resp.raise_for_status() - assert (await get_all())["settings"] == [{"name": "god"}] + assert (await get('yahweh')) == (await get('god')) == (await get('elohim')) @mark.asyncio @@ -86,25 +76,32 @@ async def query(*settings: str): 'context_filters': "*", })) - default_declare_params.update({'name': 'cat', 'alias': 'hatul'}) - resp = await app_client.put('/api/v1/settings/declare', data=json.dumps(default_declare_params)) + default_declare_params.update({'name': 'cat'}) + resp = await app_client.post('/api/v1/settings/declare', data=json.dumps(default_declare_params)) resp.raise_for_status() - default_declare_params.update({'name': 'dog', 'alias': 'kelev'}) - resp = await app_client.put('/api/v1/settings/declare', data=json.dumps(default_declare_params)) + default_declare_params.update({'name': 'hatul', 'alias': 'cat', 'version': '1.3'}) + resp = await app_client.post('/api/v1/settings/declare', data=json.dumps(default_declare_params)) + resp.raise_for_status() + + default_declare_params.update({'name': 'dog', 'alias': None, 'version': '1.0'}) + resp = await app_client.post('/api/v1/settings/declare', data=json.dumps(default_declare_params)) + resp.raise_for_status() + default_declare_params.update({'name': 'kelev', 'alias': 'dog', 'version': '2.6'}) + resp = await app_client.post('/api/v1/settings/declare', data=json.dumps(default_declare_params)) resp.raise_for_status() cat_rule = (await add('cat', 'bright'))["rule_id"] hatul_rule = (await add('hatul', 'dark'))["rule_id"] kelev_rule = (await add('kelev', 'dracula'))["rule_id"] - assert (await search("hatul", "bright"))["rule_id"] == cat_rule - assert (await search("hatul", "dark"))["rule_id"] == hatul_rule - assert (await search("kelev", "dracula"))["rule_id"] == kelev_rule + assert (await search("hatul", "bright"))["rule_id"] == (await search("cat", "bright"))["rule_id"] == cat_rule + assert (await search("hatul", "dark"))["rule_id"] == (await search("cat", "dark"))["rule_id"] == hatul_rule + assert (await search("kelev", "dracula"))["rule_id"] == (await search("dog", "dracula"))["rule_id"] == kelev_rule with raises(HTTPError): await query("cat", "kelev", "yanshuf") assert (await query("cat", "kelev"))['settings'] == { - 'cat': {'rules': [{'value': 10, 'context_features': [['theme', 'bright']], 'rule_id': cat_rule}, - {'value': 10, 'context_features': [['theme', 'dark']], 'rule_id': hatul_rule}]}, - 'dog': {'rules': [{'value': 10, 'context_features': [['theme', 'dracula']], 'rule_id': kelev_rule}]} + 'hatul': {'rules': [{'value': 10, 'context_features': [['theme', 'bright']], 'rule_id': cat_rule}, + {'value': 10, 'context_features': [['theme', 'dark']], 'rule_id': hatul_rule}]}, + 'kelev': {'rules': [{'value': 10, 'context_features': [['theme', 'dracula']], 'rule_id': kelev_rule}]} } @@ -113,39 +110,42 @@ async def test_metadata(app_client, default_declare_params): async def get(): return _get_ok_data(await app_client.get('/api/v1/settings/yayin/metadata'))['metadata'] - default_declare_params.update({'name': 'wine', 'alias': 'yayin'}) - _get_ok_data(await app_client.put('/api/v1/settings/declare', data=json.dumps(default_declare_params))) + default_declare_params.update({'name': 'yayin', 'alias': None}) + _get_ok_data(await app_client.post('/api/v1/settings/declare', data=json.dumps(default_declare_params))) + default_declare_params.update({'name': 'wine', 'alias': 'yayin', 'version': '1.1'}) + _get_ok_data(await app_client.post('/api/v1/settings/declare', data=json.dumps(default_declare_params))) assert await get() == {'testing': True} - resp = await app_client.post('/api/v1/settings/yayin/metadata', data=json.dumps({"metadata": {"alcohol": "6%"}})) + resp = await app_client.post('/api/v1/settings/yayin/metadata', data=json.dumps({"metadata": {"alcohol": "6%"}, + 'version': '1.2'})) resp.raise_for_status() assert await get() == {'testing': True, "alcohol": "6%"} resp = await app_client.put('/api/v1/settings/yayin/metadata', data=json.dumps({"metadata": { "price": 50, "experimenting": True, "should_drink": True, - }})) + }, "version": "1.3"})) resp.raise_for_status() assert await get() == {"price": 50, "experimenting": True, "should_drink": True} - resp = await app_client.put('/api/v1/settings/yayin/metadata/should_drink', data=json.dumps({"value": False})) + resp = await app_client.put('/api/v1/settings/yayin/metadata/should_drink', data=json.dumps({"value": False, + 'version': '1.4'})) resp.raise_for_status() assert await get() == {"price": 50, "experimenting": True, "should_drink": False} - resp = await app_client.delete('/api/v1/settings/yayin/metadata/should_drink') + resp = await app_client.delete('/api/v1/settings/yayin/metadata/should_drink', data=json.dumps({"version": "1.5"})) resp.raise_for_status() assert await get() == {"price": 50, "experimenting": True} - resp = await app_client.delete('/api/v1/settings/yayin/metadata') + resp = await app_client.delete('/api/v1/settings/yayin/metadata', data=json.dumps({"version": "1.6"})) resp.raise_for_status() assert await get() == {} @mark.asyncio async def test_declare_existing_alias(app_client, default_declare_params): - async def declare(name: str, alias: Optional[str]): - default_declare_params.update({'name': name, 'alias': alias}) - return _get_ok_data(await app_client.put('/api/v1/settings/declare', data=json.dumps(default_declare_params))) + async def declare(name: str, alias: Optional[str], version: str): + default_declare_params.update({'name': name, 'alias': alias, 'version': version}) + return _get_ok_data(await app_client.post('/api/v1/settings/declare', data=json.dumps(default_declare_params))) - assert (await declare('god', 'elohim'))['created'] is True - with raises(HTTPError): - await declare('dios', 'god') + assert (await declare('god', None, '1.0'))['outcome'] == 'created' + assert (await declare('elohim', 'god', '1.1'))['outcome'] == 'upgraded' with raises(HTTPError): - await declare('dios', 'elohim') + await declare('dios', 'god', '1.0') diff --git a/tests/blackbox/app/test_v1api_rules.py b/tests/blackbox/app/test_v1api_rules.py index 7e035ba..6cea505 100644 --- a/tests/blackbox/app/test_v1api_rules.py +++ b/tests/blackbox/app/test_v1api_rules.py @@ -148,17 +148,14 @@ async def test_search_rule_missing(example_rule, app_client): @fixture def mk_setting(app_client): async def mk_setting(name: str): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': name, 'configurable_features': ['theme', 'trust', 'user'], - 'type': 'int' + 'type': 'int', + 'default_value': 0, })) res.raise_for_status() - assert res.json() == { - 'created': True, - 'changed': [], - 'incomplete': {} - } + assert res.json() == {'outcome': 'created'} return mk_setting diff --git a/tests/blackbox/app/test_v1api_rules_metadata.py b/tests/blackbox/app/test_v1api_rules_metadata.py index be85809..972893f 100644 --- a/tests/blackbox/app/test_v1api_rules_metadata.py +++ b/tests/blackbox/app/test_v1api_rules_metadata.py @@ -45,7 +45,7 @@ async def test_post_not_existing_rule_metadata(app_client): @mark.asyncio async def test_post_rule_first_metadata(example_rule, app_client): - await app_client.put('/api/v1/settings/declare', data=json.dumps({ + await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'test_setting', 'configurable_features': ['user', 'theme'], 'type': 'int', @@ -196,10 +196,11 @@ async def test_get_rule_metadata(example_rule, app_client): @mark.asyncio async def test_get_rule_no_metadata(app_client): - await app_client.put('/api/v1/settings/declare', data=json.dumps({ + await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'test_setting', 'configurable_features': ['theme', 'user'], - 'type': 'int' + 'type': 'int', + 'default_value': 10 })) post_rule_rep = await app_client.post('/api/v1/rules', data=json.dumps({ diff --git a/tests/blackbox/app/test_v1api_settings.py b/tests/blackbox/app/test_v1api_settings.py index daabee3..72de69a 100644 --- a/tests/blackbox/app/test_v1api_settings.py +++ b/tests/blackbox/app/test_v1api_settings.py @@ -14,14 +14,28 @@ async def test_declare_new_setting(size_limit_setting, app_client): 'type': 'int', 'default_value': 200, 'metadata': {'testing': True}, - 'aliases': [] + 'aliases': [], + 'version': '1.0', } +@mark.asyncio +async def test_declare_new_setting_bad_version(app_client): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'version': '1.1', + })) + assert res.status_code == 400 + + @mark.asyncio @mark.parametrize('type_', [15, 'Flags{1,2,3}', 'enum[1,2,3]', 'Flags[[0]]', 'Flags[]']) async def test_declare_new_setting_bad_type(app_client, type_): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'size_limit', 'configurable_features': ['user', 'theme'], 'type': type_, @@ -33,7 +47,7 @@ async def test_declare_new_setting_bad_type(app_client, type_): @mark.asyncio async def test_declare_new_setting_bad_default(app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'size_limit', 'configurable_features': ['user', 'theme'], 'type': 'int', @@ -45,31 +59,32 @@ async def test_declare_new_setting_bad_default(app_client): @mark.asyncio async def test_declare_new_setting_bad_cf(app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'size_limit', 'configurable_features': ['user', 'theme', 'color'], 'type': 'int', 'default_value': 200, 'metadata': {'testing': True} })) - assert res.status_code == 422 + assert res.status_code == 404 @mark.asyncio async def test_declare_new_setting_modify_bad_cf(size_limit_setting, app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'size_limit', 'configurable_features': ['user', 'theme', 'color'], 'type': 'int', 'default_value': 200, - 'metadata': {'testing': True} + 'metadata': {'testing': True}, + 'version': '2.0', })) - assert res.status_code == 422 + assert res.status_code == 404 @mark.asyncio async def test_declare_no_modify(size_limit_setting, app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'size_limit', 'configurable_features': ['user', 'theme'], 'type': 'int', @@ -78,62 +93,148 @@ async def test_declare_no_modify(size_limit_setting, app_client): })) res.raise_for_status() assert res.json() == { - 'created': False, - 'changed': [], - 'incomplete': {} + 'outcome': 'uptodate', } @mark.asyncio async def test_declare_modify(app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'size_limit', 'configurable_features': ['user'], 'type': 'int', 'default_value': 200, - 'metadata': {'testing': True, 'ctr': 1, 'dummy': 2} + 'metadata': {'testing': True, 'ctr': 1, 'dummy': 2}, + 'version': '1.0', })) res.raise_for_status() - assert res.json() == { - 'created': True, - 'changed': [], - 'incomplete': {} + assert res.json() == {'outcome': 'created', } + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 300, + 'metadata': {'testing': True, 'ctr': 2}, + 'version': '2.0', + })) + res.raise_for_status() + resp_body = res.json() + assert {diff['attribute'] for diff in resp_body.pop('differences')} == {'default_value', 'metadata', + 'configurable_features'} + assert resp_body == { + 'outcome': 'upgraded', + 'previous_version': '1.0', } - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.get('/api/v1/settings/size_limit') + res.raise_for_status() + assert res.json() == { 'name': 'size_limit', 'configurable_features': ['user', 'theme'], 'type': 'int', 'default_value': 300, - 'metadata': {'testing': True, 'ctr': 2} + 'metadata': {'testing': True, 'ctr': 2}, + 'aliases': [], + 'version': '2.0', + } + + +@mark.asyncio +@mark.parametrize('version', ['1.1', '2.0']) +async def test_declare_vbump(app_client, version): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True, 'ctr': 1, 'dummy': 2}, + 'version': '1.0', + })) + res.raise_for_status() + assert res.json() == {'outcome': 'created', } + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True, 'ctr': 1, 'dummy': 2}, + 'version': version, })) res.raise_for_status() assert res.json() == { - 'created': False, - 'changed': [ - 'configurable_features', - 'default_value', - 'metadata.ctr', - 'metadata.dummy', - ], - 'incomplete': {} + 'outcome': 'upgraded', + 'previous_version': '1.0', + 'differences': [], } res = await app_client.get('/api/v1/settings/size_limit') res.raise_for_status() assert res.json() == { + 'name': 'size_limit', + 'configurable_features': ['user'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True, 'ctr': 1, 'dummy': 2}, + 'aliases': [], + 'version': version, + } + + +@mark.asyncio +async def test_declare_same_as_alias(app_client): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True, 'ctr': 1, 'dummy': 2}, + 'version': '1.0', + })) + res.raise_for_status() + assert res.json() == {'outcome': 'created', } + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'size_limit', 'configurable_features': ['user', 'theme'], 'type': 'int', 'default_value': 300, 'metadata': {'testing': True, 'ctr': 2}, - 'aliases': [] - } + 'alias': 'size_limit', + 'version': '2.0', + })) + assert res.status_code == 422 + + +@mark.asyncio +async def test_declare_nonexistant_alias(app_client): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True, 'ctr': 1, 'dummy': 2}, + 'version': '1.0', + })) + res.raise_for_status() + assert res.json() == {'outcome': 'created', } + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 300, + 'metadata': {'testing': True, 'ctr': 2}, + 'alias': 'foobar', + 'version': '2.0', + })) + assert res.status_code == 404 @mark.asyncio async def test_declare_conflict(size_limit_setting, app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'size_limit', 'configurable_features': ['user', 'theme'], 'type': 'str', @@ -143,47 +244,305 @@ async def test_declare_conflict(size_limit_setting, app_client): assert res.status_code == 409 +@mark.asyncio +async def test_modify_with_fcs_in_use_safe(size_limit_setting, app_client): + (await app_client.post('/api/v1/rules', data=json.dumps({ + 'setting': 'size_limit', + 'feature_values': {'user': '1'}, + 'value': 10, + }))).raise_for_status() + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'version': '1.1', + })) + assert res.json() == { + 'outcome': 'upgraded', + 'previous_version': '1.0', + 'differences': [ + {'level': 'minor', 'message': "removal of configurable features ['theme']"} + ] + } + + +@mark.asyncio +async def test_modify_with_fcs_in_use(size_limit_setting, app_client): + (await app_client.post('/api/v1/rules', data=json.dumps({ + 'setting': 'size_limit', + 'feature_values': {'user': '1', 'theme': '2'}, + 'value': 10, + }))).raise_for_status() + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'version': '2.0', + })) + assert res.status_code == 409 + assert res.json()['differences'] == [ + {'level': 'mismatch', 'message': "configurable features ['theme'] are still in use by rules [1]"} + ] + + +@mark.asyncio +async def test_declare_major_changes_on_minor_vbump(size_limit_setting, app_client): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user', 'theme', 'trust'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'version': '1.1', + })) + assert res.status_code == 409 + + +@mark.asyncio +async def test_declare_major_changes_on_major_vbump(size_limit_setting, app_client): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user', 'theme', 'trust'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'version': '2.0', + })) + res.raise_for_status() + assert res.json() == { + 'outcome': 'upgraded', + 'previous_version': '1.0', + 'differences': [ + {'level': 'major', 'attribute': "configurable_features", 'latest_value': ['user', 'theme']} + ] + } + + res = await app_client.get('/api/v1/settings/size_limit') + res.raise_for_status() + assert res.json() == { + 'name': 'size_limit', + 'configurable_features': ['user', 'trust', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'aliases': [], + 'version': '2.0', + } + + @mark.asyncio async def test_declare_type_upgrade(size_limit_setting, app_client): - upgraded_type = 'float' - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'size_limit', 'configurable_features': ['user', 'theme'], - 'type': upgraded_type, + 'type': 'float', 'default_value': 200, - 'metadata': {'testing': True} + 'metadata': {'testing': True}, + 'version': '2.0', })) res.raise_for_status() - assert res.json() == {'changed': ['type'], 'created': False, 'incomplete': {}} + assert res.json() == { + 'outcome': 'upgraded', + 'previous_version': '1.0', + 'differences': [{'level': 'major', 'attribute': 'type', 'latest_value': 'int'}] + } res = await app_client.get('/api/v1/settings/size_limit') res.raise_for_status() assert res.json() == { 'name': 'size_limit', 'configurable_features': ['user', 'theme'], - 'type': upgraded_type, + 'type': 'float', 'default_value': 200, 'metadata': {'testing': True}, 'aliases': [], + 'version': '2.0', + } + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'version': '1.0', + })) + assert res.json() == { + 'outcome': 'outdated', + 'latest_version': '2.0', + 'differences': [{'level': 'major', 'attribute': 'type', 'latest_value': 'float'}] + } + + +@mark.asyncio +async def test_declare_type_upgrade_to_subtype(app_client): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'color', + 'configurable_features': ['user', 'theme'], + 'type': 'Enum[0,1,2]', + 'default_value': 1, + 'metadata': {'testing': True}, + 'version': '1.0', + })) + res.raise_for_status() + assert res.json() == { + 'outcome': 'created', + } + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'color', + 'configurable_features': ['user', 'theme'], + 'type': 'Enum[0,1]', + 'default_value': 1, + 'metadata': {'testing': True}, + 'version': '1.1', + })) + res.raise_for_status() + assert res.json() == { + 'outcome': 'upgraded', + 'previous_version': '1.0', + 'differences': [{'level': 'minor', 'attribute': 'type', 'latest_value': 'Enum[0,1,2]'}] + } + + res = await app_client.get('/api/v1/settings/color') + res.raise_for_status() + assert res.json() == { + 'name': 'color', + 'configurable_features': ['user', 'theme'], + 'type': 'Enum[0,1]', + 'default_value': 1, + 'metadata': {'testing': True}, + 'aliases': [], + 'version': '1.1', + } + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'color', + 'configurable_features': ['user', 'theme'], + 'type': 'Enum[0,1,2]', + 'default_value': 1, + 'metadata': {'testing': True}, + 'version': '1.0', + })) + assert res.json() == { + 'outcome': 'outdated', + 'latest_version': '1.1', + 'differences': [{'level': 'minor', 'attribute': 'type', 'latest_value': 'Enum[0,1]'}] } +@mark.asyncio +async def test_modify_type_incompatible(size_limit_setting, app_client): + (await app_client.post('/api/v1/rules', data=json.dumps({ + 'setting': 'size_limit', + 'feature_values': {'user': '1', 'theme': '2'}, + 'value': 10, + }))).raise_for_status() + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user'], + 'type': 'Enum[0,11,12]', + 'default_value': 0, + 'metadata': {'testing': True}, + 'version': '2.0', + })) + assert res.status_code == 409 + assert {'level': 'mismatch', 'message': 'setting type incompatible with values for rules: [1]'} \ + in res.json()['differences'] + + +@mark.asyncio +async def test_declare_multiversion(size_limit_setting, app_client): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'foo', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'alias': 'size_limit', + 'version': '2.0', + })) + res.raise_for_status() + assert res.json()['outcome'] == 'upgraded' + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'version': '1.0', + })) + res.raise_for_status() + assert res.json()['outcome'] == 'outdated' + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'bar', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'alias': 'foo', + 'version': '3.0', + })) + res.raise_for_status() + assert res.json()['outcome'] == 'upgraded' + + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'size_limit', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'version': '1.0', + })) + res.raise_for_status() + assert res.json()['outcome'] == 'outdated' + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'foo', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'alias': 'size_limit', + 'version': '2.0', + })) + res.raise_for_status() + assert res.json()['outcome'] == 'outdated' + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'bar', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'alias': 'foo', + 'version': '3.0', + })) + res.raise_for_status() + assert res.json()['outcome'] == 'uptodate' + + @mark.asyncio async def test_declare_incomplete(size_limit_setting, app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'size_limit', 'configurable_features': ['user'], 'type': 'int', 'default_value': 200, - 'metadata': {'testing': True} + 'metadata': {'testing': True}, + 'version': '0.1', })) res.raise_for_status() assert res.json() == { - 'created': False, - 'changed': [], - 'incomplete': { - 'configurable_features': ['user', 'theme'] - } + 'outcome': 'outdated', + 'latest_version': '1.0', + 'differences': [{'level': 'major', 'attribute': 'configurable_features', 'latest_value': ['user', 'theme']}] } res = await app_client.get('/api/v1/settings/size_limit') @@ -195,6 +554,7 @@ async def test_declare_incomplete(size_limit_setting, app_client): 'default_value': 200, 'metadata': {'testing': True}, 'aliases': [], + 'version': '1.0', } @@ -209,6 +569,7 @@ async def test_get_setting(size_limit_setting, app_client): 'default_value': 200, 'metadata': {'testing': True}, 'aliases': [], + 'version': '1.0', } @@ -237,16 +598,15 @@ async def test_delete_setting_missing(size_limit_setting, app_client): @mark.parametrize('additional_data', [False, None]) async def test_get_settings(app_client, additional_data): async def mk_setting(name: str): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': name, - 'configurable_features': [], - 'type': 'int' + 'configurable_features': ['user'], + 'type': 'int', + 'default_value': 200, })) res.raise_for_status() assert res.json() == { - 'created': True, - 'changed': [], - 'incomplete': {} + 'outcome': 'created', } await mk_setting('a') @@ -257,35 +617,34 @@ async def mk_setting(name: str): if additional_data is not None: request_data['include_additional_data'] = additional_data - res = await app_client.get('/api/v1/settings', data=json.dumps(request_data)) + res = await app_client.get('/api/v1/settings', query_string=request_data) res.raise_for_status() assert res.json() == { 'settings': [ - {'name': 'a'}, - {'name': 'b'}, - {'name': 'c'}, + {'name': 'a', 'type': 'int', 'default_value': 200, 'version': '1.0'}, + {'name': 'b', 'type': 'int', 'default_value': 200, 'version': '1.0'}, + {'name': 'c', 'type': 'int', 'default_value': 200, 'version': '1.0'}, ] } @mark.asyncio async def test_get_settings_additional_data(app_client): - async def mk_setting(name: str, type: str): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + async def mk_setting(name: str, type: str, default): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': name, 'configurable_features': ['theme', 'user'], - 'type': type + 'type': type, + 'default_value': default, })) res.raise_for_status() assert res.json() == { - 'created': True, - 'changed': [], - 'incomplete': {} + 'outcome': 'created', } - await mk_setting('a', 'int') - await mk_setting('c', 'float') - await mk_setting('b', 'str') + await mk_setting('a', 'int', 200) + await mk_setting('c', 'float', 200.0) + await mk_setting('b', 'str', '200') res = await app_client.get('/api/v1/settings', query_string={ 'include_additional_data': True @@ -294,18 +653,18 @@ async def mk_setting(name: str, type: str): assert res.json() == { 'settings': [ {'name': 'a', 'type': 'int', 'configurable_features': ['user', 'theme'], - 'metadata': {}, 'default_value': None, 'aliases': []}, + 'metadata': {}, 'default_value': 200, 'aliases': [], 'version': '1.0'}, {'name': 'b', 'type': 'str', 'configurable_features': ['user', 'theme'], - 'metadata': {}, 'default_value': None, 'aliases': []}, + 'metadata': {}, 'default_value': '200', 'aliases': [], 'version': '1.0'}, {'name': 'c', 'type': 'float', 'configurable_features': ['user', 'theme'], - 'metadata': {}, 'default_value': None, 'aliases': []}, + 'metadata': {}, 'default_value': 200.0, 'aliases': [], 'version': '1.0'}, ] } @fixture async def interval_setting(app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'interval', 'configurable_features': ['user', 'theme'], 'type': 'float', @@ -314,9 +673,7 @@ async def interval_setting(app_client): })) res.raise_for_status() assert res.json() == { - 'created': True, - 'changed': [], - 'incomplete': {} + 'outcome': 'created', } @@ -324,7 +681,8 @@ async def interval_setting(app_client): @mark.parametrize('new_type', ['float', 'int']) async def test_type_downgrade_no_rules(interval_setting, app_client, new_type): res = await app_client.put('/api/v1/settings/interval/type', json={ - 'type': new_type + 'type': new_type, + 'version': '2.0', }) assert res.status_code == 204 assert not res.content @@ -334,12 +692,14 @@ async def test_type_downgrade_no_rules(interval_setting, app_client, new_type): resp_json = res.json() assert resp_json['type'] == new_type assert resp_json['default_value'] == 5 + assert resp_json['version'] == '2.0' @mark.asyncio async def test_type_downgrade_no_rules_default_conflict(interval_setting, app_client): res = await app_client.put('/api/v1/settings/interval/type', json={ - 'type': 'bool' + 'type': 'bool', + 'version': '2.0', }) assert res.status_code == 409 assert sum('default value' in reason for reason in res.json()['conflicts']) == 1 @@ -362,7 +722,8 @@ async def test_type_downgrade_with_rules(interval_setting, app_client): res.raise_for_status() res = await app_client.put('/api/v1/settings/interval/type', json={ - 'type': 'int' + 'type': 'int', + 'version': '2.0', }) assert res.status_code == 204 assert not res.content @@ -387,7 +748,8 @@ async def test_declare_downgrade_with_rules_conflict(interval_setting, app_clien rule_id = j_result.pop('rule_id') res = await app_client.put('/api/v1/settings/interval/type', json={ - 'type': 'int' + 'type': 'int', + 'version': '2.0', }) assert res.status_code == 409 assert sum(str(rule_id) in reason for reason in res.json()['conflicts']) == 1 @@ -403,14 +765,15 @@ async def test_declare_downgrade_with_rules_conflict(interval_setting, app_clien @mark.parametrize('new_type', ['float', 'int']) async def test_type_downgrade_missing(interval_setting, app_client, new_type): res = await app_client.put('/api/v1/settings/intervalloo/type', json={ - 'type': new_type + 'type': new_type, + 'version': '2.0', }) assert res.status_code == 404 @mark.asyncio async def test_type_downgrade_with_rules_enum(app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'background_color', 'configurable_features': ['theme'], 'type': 'Enum["red", "blue", "green"]', @@ -419,9 +782,7 @@ async def test_type_downgrade_with_rules_enum(app_client): })) res.raise_for_status() assert res.json() == { - 'created': True, - 'changed': [], - 'incomplete': {} + 'outcome': 'created', } res = await app_client.post('/api/v1/rules', data=json.dumps({ @@ -441,7 +802,8 @@ async def test_type_downgrade_with_rules_enum(app_client): res.raise_for_status() res = await app_client.put('/api/v1/settings/background_color/type', json={ - 'type': 'Enum["blue", "green", "yellow"]' + 'type': 'Enum["blue", "green", "yellow"]', + 'version': '2.0', }) assert res.status_code == 204 assert not res.content @@ -455,7 +817,7 @@ async def test_type_downgrade_with_rules_enum(app_client): @mark.asyncio async def test_type_downgrade_with_rules_enum_bad(app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'background_color', 'configurable_features': ['theme'], 'type': 'Enum["red", "blue", "green"]', @@ -464,9 +826,7 @@ async def test_type_downgrade_with_rules_enum_bad(app_client): })) res.raise_for_status() assert res.json() == { - 'created': True, - 'changed': [], - 'incomplete': {} + 'outcome': 'created', } res = await app_client.post('/api/v1/rules', data=json.dumps({ @@ -486,7 +846,8 @@ async def test_type_downgrade_with_rules_enum_bad(app_client): res.raise_for_status() res = await app_client.put('/api/v1/settings/background_color/type', json={ - 'type': 'Enum["blue", "green", "yellow"]' + 'type': 'Enum["blue", "green", "yellow"]', + 'version': '2.0', }) assert res.status_code == 409 @@ -498,75 +859,134 @@ async def test_type_downgrade_with_rules_enum_bad(app_client): @mark.asyncio -@mark.parametrize('old,new', [ - ('A', 'Z'), - ('A1', 'Z'), -]) -async def test_rename_setting(app_client, old, new): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ +@mark.parametrize('old', ['A1', 'A']) +async def test_rename_setting(app_client, old): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'A1', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + })) + res.raise_for_status() + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'A', 'configurable_features': ['user', 'theme'], 'type': 'int', 'default_value': 200, 'metadata': {'testing': True}, - 'alias': 'A1' + 'alias': 'A1', + 'version': '2.0', })) res.raise_for_status() - res = await app_client.put(f'/api/v1/settings/{old}/name', data=json.dumps({'name': new})) + res = await app_client.put(f'/api/v1/settings/{old}/name', data=json.dumps({'name': 'Z', 'version': '2.1'})) res.raise_for_status() res = await app_client.get('/api/v1/settings/Z') res.raise_for_status() data = res.json() assert data['name'] == 'Z' assert set(data['aliases']) == {'A', 'A1'} + assert data['version'] == '2.1' @mark.asyncio -@mark.parametrize('old,new', [ - ('A', 'A'), - ('A1', 'A'), -]) -async def test_rename_setting_no_action_needed(app_client, old, new): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ +@mark.parametrize('old', ['A', 'A1']) +async def test_rename_setting_no_action_needed(app_client, old): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'A1', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + })) + res.raise_for_status() + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'A', 'configurable_features': ['user', 'theme'], 'type': 'int', 'default_value': 200, 'metadata': {'testing': True}, - 'alias': 'A1' + 'alias': 'A1', + 'version': '2.0', })) res.raise_for_status() - res = await app_client.put(f'/api/v1/settings/{old}/name', data=json.dumps({'name': new})) + res = await app_client.put(f'/api/v1/settings/{old}/name', data=json.dumps({'name': 'A', 'version': '2.1'})) res.raise_for_status() res = await app_client.get('/api/v1/settings/A') res.raise_for_status() data = res.json() assert data['name'] == 'A' assert set(data['aliases']) == {'A1'} + assert data['version'] == '2.1' @mark.asyncio -@mark.parametrize('old,new', [ - ('A', 'A1'), - ('A1', 'A1'), -]) -async def test_rename_setting_to_alias(app_client, old, new): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ +@mark.parametrize('old', ['A', 'A1', ]) +async def test_rename_setting_to_alias(app_client, old): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'A1', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + })) + res.raise_for_status() + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'A', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'alias': 'A1', + 'version': '2.0', + })) + res.raise_for_status() + res = await app_client.put(f'/api/v1/settings/{old}/name', data=json.dumps({'name': 'A1', 'version': '2.1'})) + res.raise_for_status() + res = await app_client.get('/api/v1/settings/A1') + res.raise_for_status() + data = res.json() + assert data['name'] == 'A1' + assert set(data['aliases']) == {'A'} + assert data['version'] == '2.1' + + +@mark.asyncio +async def test_rename_setting_to_alias_with_declaration(app_client): + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'A1', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + })) + res.raise_for_status() + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'A', 'configurable_features': ['user', 'theme'], 'type': 'int', 'default_value': 200, 'metadata': {'testing': True}, - 'alias': 'A1' + 'alias': 'A1', + 'version': '2.0', })) res.raise_for_status() - res = await app_client.put(f'/api/v1/settings/{old}/name', data=json.dumps({'name': new})) + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'A1', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'alias': 'A', + 'version': '2.1', + })) res.raise_for_status() res = await app_client.get('/api/v1/settings/A1') res.raise_for_status() data = res.json() assert data['name'] == 'A1' assert set(data['aliases']) == {'A'} + assert data['version'] == '2.1' @mark.asyncio @@ -577,25 +997,43 @@ async def test_rename_setting_to_alias(app_client, old, new): ('A1', 'B1'), ]) async def test_rename_setting_existing(app_client, old, new): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'A1', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + })) + res.raise_for_status() + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'A', 'configurable_features': ['user', 'theme'], 'type': 'int', 'default_value': 200, 'metadata': {'testing': True}, - 'alias': 'A1' + 'alias': 'A1', + 'version': '2.0', })) res.raise_for_status() - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ + 'name': 'B1', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + })) + res.raise_for_status() + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'B', 'configurable_features': ['user', 'theme'], 'type': 'int', 'default_value': 200, 'metadata': {'testing': True}, - 'alias': 'B1' + 'alias': 'B1', + 'version': '2.0', })) res.raise_for_status() - res = await app_client.put(f'/api/v1/settings/{old}/name', data=json.dumps({'name': new})) + res = await app_client.put(f'/api/v1/settings/{old}/name', data=json.dumps({'name': new, 'version': '2.1'})) with raises(HTTPError): res.raise_for_status() res = await app_client.get('/api/v1/settings/A') @@ -612,14 +1050,14 @@ async def test_rename_setting_existing(app_client, old, new): @mark.asyncio async def test_rename_setting_not_existing(app_client): - res = await app_client.put('/api/v1/settings/X', data=json.dumps({'name': 'Y'})) + res = await app_client.put('/api/v1/settings/X/name', data=json.dumps({'name': 'Y', 'version': '2.1'})) with raises(HTTPError): res.raise_for_status() @mark.asyncio async def test_rename_setting_cascade(app_client): - res = await app_client.put('/api/v1/settings/declare', data=json.dumps({ + res = await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'A', 'configurable_features': ['user', 'theme'], 'type': 'int', @@ -635,8 +1073,262 @@ async def test_rename_setting_cascade(app_client): })) res.raise_for_status() rule_id = res.json()["rule_id"] - res = await app_client.put('/api/v1/settings/A/name', data=json.dumps({'name': 'Z'})) + res = await app_client.put('/api/v1/settings/A/name', data=json.dumps({'name': 'Z', 'version': '2.1'})) res.raise_for_status() res = await app_client.get(f'/api/v1/rules/{rule_id}') res.raise_for_status() assert res.json()["setting"] == "Z" + + +@mark.asyncio +async def test_set_cfs_minor(size_limit_setting, app_client): + res = await app_client.put('/api/v1/settings/size_limit/configurable_features', data=json.dumps({ + 'configurable_features': ['user'], + 'version': '1.1' + })) + assert res.status_code == 204 + assert not res.content + + res = await app_client.get('/api/v1/settings/size_limit') + res.raise_for_status() + assert res.json() == { + 'name': 'size_limit', + 'configurable_features': ['user'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'aliases': [], + 'version': '1.1', + } + + +@mark.asyncio +async def test_set_cfs_minor_rejected(size_limit_setting, app_client): + res = await app_client.put('/api/v1/settings/size_limit/configurable_features', data=json.dumps({ + 'configurable_features': ['user', 'theme', 'trust'], + 'version': '1.1' + })) + assert res.status_code == 409 + + res = await app_client.get('/api/v1/settings/size_limit') + res.raise_for_status() + assert res.json() == { + 'name': 'size_limit', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'aliases': [], + 'version': '1.0', + } + + +@mark.asyncio +async def test_set_cfs_minor_conflict(size_limit_setting, app_client): + (await app_client.post('/api/v1/rules', data=json.dumps({ + 'setting': 'size_limit', + 'feature_values': {'user': '1', 'theme': '2'}, + 'value': 10, + }))).raise_for_status() + + res = await app_client.put('/api/v1/settings/size_limit/configurable_features', data=json.dumps({ + 'configurable_features': ['user'], + 'version': '1.1' + })) + assert res.status_code == 409 + + res = await app_client.get('/api/v1/settings/size_limit') + res.raise_for_status() + assert res.json() == { + 'name': 'size_limit', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'aliases': [], + 'version': '1.0', + } + + +@mark.asyncio +async def test_set_cfs_no_change(size_limit_setting, app_client): + res = await app_client.put('/api/v1/settings/size_limit/configurable_features', data=json.dumps({ + 'configurable_features': ['user', 'theme'], + 'version': '1.1' + })) + assert res.status_code == 204 + assert not res.content + + res = await app_client.get('/api/v1/settings/size_limit') + res.raise_for_status() + assert res.json() == { + 'name': 'size_limit', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'aliases': [], + 'version': '1.1', + } + + +@mark.asyncio +async def test_set_cfs_outdated(size_limit_setting, app_client): + res = await app_client.put('/api/v1/settings/size_limit/configurable_features', data=json.dumps({ + 'configurable_features': ['user', 'theme'], + 'version': '0.1' + })) + assert res.status_code == 409 + + res = await app_client.get('/api/v1/settings/size_limit') + res.raise_for_status() + assert res.json() == { + 'name': 'size_limit', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'aliases': [], + 'version': '1.0', + } + + +@mark.asyncio +async def test_set_cfs_same(size_limit_setting, app_client): + res = await app_client.put('/api/v1/settings/size_limit/configurable_features', data=json.dumps({ + 'configurable_features': ['user', 'theme'], + 'version': '1.0' + })) + assert res.status_code == 204 + assert not res.content + + res = await app_client.get('/api/v1/settings/size_limit') + res.raise_for_status() + assert res.json() == { + 'name': 'size_limit', + 'configurable_features': ['user', 'theme'], + 'type': 'int', + 'default_value': 200, + 'metadata': {'testing': True}, + 'aliases': [], + 'version': '1.0', + } + + +@mark.asyncio +@mark.parametrize('new_type', ['float', 'int']) +async def test_type_change_outdated(interval_setting, app_client, new_type): + res = await app_client.put('/api/v1/settings/interval/type', json={ + 'type': new_type, + 'version': '0.1', + }) + assert res.status_code == 409 + + res = await app_client.get('/api/v1/settings/interval') + res.raise_for_status() + resp_json = res.json() + assert resp_json['type'] == 'float' + assert resp_json['default_value'] == 5 + assert resp_json['version'] == '1.0' + + +@mark.asyncio +@mark.parametrize('new_version', ['1.0', '1.1']) +async def test_type_no_change(interval_setting, app_client, new_version): + res = await app_client.put('/api/v1/settings/interval/type', json={ + 'type': 'float', + 'version': new_version, + }) + assert res.status_code == 204 + assert not res.content + + res = await app_client.get('/api/v1/settings/interval') + res.raise_for_status() + resp_json = res.json() + assert resp_json['type'] == 'float' + assert resp_json['default_value'] == 5 + assert resp_json['version'] == new_version + + +@mark.asyncio +async def test_type_major_on_minor(size_limit_setting, app_client): + res = await app_client.put('/api/v1/settings/size_limit/type', json={ + 'type': 'float', + 'version': '1.1', + }) + assert res.status_code == 409 + + res = await app_client.get('/api/v1/settings/size_limit') + res.raise_for_status() + resp_json = res.json() + assert resp_json['type'] == 'int' + assert resp_json['default_value'] == 200 + assert resp_json['version'] == '1.0' + + +@mark.asyncio +async def test_type_outdated(size_limit_setting, app_client): + res = await app_client.put('/api/v1/settings/size_limit/type', json={ + 'type': 'float', + 'version': '0.1', + }) + assert res.status_code == 409 + + res = await app_client.get('/api/v1/settings/size_limit') + res.raise_for_status() + resp_json = res.json() + assert resp_json['type'] == 'int' + assert resp_json['default_value'] == 200 + assert resp_json['version'] == '1.0' + + +@mark.asyncio +@mark.parametrize('new_version', ['1.0', '1.1']) +async def test_rename_no_change(interval_setting, app_client, new_version): + res = await app_client.put('/api/v1/settings/interval/name', json={ + 'name': 'interval', + 'version': new_version, + }) + assert res.status_code == 204 + assert not res.content + + res = await app_client.get('/api/v1/settings/interval') + res.raise_for_status() + resp_json = res.json() + assert resp_json['type'] == 'float' + assert resp_json['default_value'] == 5 + assert resp_json['version'] == new_version + + +@mark.asyncio +@mark.parametrize('new_name', ['interval', 'fooobar']) +async def test_rename_outdated(interval_setting, app_client, new_name): + res = await app_client.put('/api/v1/settings/interval/name', json={ + 'name': new_name, + 'version': '0.1', + }) + assert res.status_code == 409 + + res = await app_client.get('/api/v1/settings/interval') + res.raise_for_status() + resp_json = res.json() + assert resp_json['type'] == 'float' + assert resp_json['default_value'] == 5 + assert resp_json['version'] == '1.0' + + +@mark.asyncio +@mark.parametrize('new_name', ['interval', 'fooobar']) +async def test_rename_back(interval_setting, app_client, new_name): + res = await app_client.put('/api/v1/settings/interval/name', json={ + 'name': new_name, + 'version': '0.1', + }) + assert res.status_code == 409 + + res = await app_client.get('/api/v1/settings/interval') + res.raise_for_status() + resp_json = res.json() + assert resp_json['type'] == 'float' + assert resp_json['default_value'] == 5 + assert resp_json['version'] == '1.0' diff --git a/tests/blackbox/app/test_v1api_settings_metadata.py b/tests/blackbox/app/test_v1api_settings_metadata.py index 7cd5d64..7f794df 100644 --- a/tests/blackbox/app/test_v1api_settings_metadata.py +++ b/tests/blackbox/app/test_v1api_settings_metadata.py @@ -6,7 +6,8 @@ @mark.asyncio async def test_post_setting_metadata(size_limit_setting, app_client): res = await app_client.post('/api/v1/settings/size_limit/metadata', data=json.dumps({ - 'metadata': {'testing': False, 'second_key': 12} + 'metadata': {'testing': False, 'second_key': 12}, + 'version': '2.0' })) res.raise_for_status() setting = await app_client.get('/api/v1/settings/size_limit') @@ -17,13 +18,15 @@ async def test_post_setting_metadata(size_limit_setting, app_client): 'default_value': 200, 'metadata': {'testing': False, 'second_key': 12}, 'aliases': [], + 'version': '2.0', } @mark.asyncio async def test_post_setting_metadata_new_key(size_limit_setting, app_client): res = await app_client.post('/api/v1/settings/size_limit/metadata', data=json.dumps({ - 'metadata': {'second_key': 12} + 'metadata': {'second_key': 12}, + 'version': '2.0' })) res.raise_for_status() setting = await app_client.get('/api/v1/settings/size_limit') @@ -34,20 +37,22 @@ async def test_post_setting_metadata_new_key(size_limit_setting, app_client): 'default_value': 200, 'metadata': {'testing': True, 'second_key': 12}, 'aliases': [], + 'version': '2.0', } @mark.asyncio async def test_post_not_existing_setting_metadata(app_client): res = await app_client.post('/api/v1/settings/no_setting/metadata', data=json.dumps({ - 'metadata': {'testing': True} + 'metadata': {'testing': True}, + 'version': '2.0' })) assert res.status_code == 404 @mark.asyncio async def test_post_setting_first_metadata(app_client): - await app_client.put('/api/v1/settings/declare', data=json.dumps({ + await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'test_setting', 'configurable_features': ['user'], 'type': 'int', @@ -55,7 +60,8 @@ async def test_post_setting_first_metadata(app_client): 'metadata': {} })) res = await app_client.post('/api/v1/settings/test_setting/metadata', data=json.dumps({ - 'metadata': {'testing': True} + 'metadata': {'testing': True}, + 'version': '2.0' })) res.raise_for_status() setting = await app_client.get('/api/v1/settings/test_setting') @@ -66,13 +72,15 @@ async def test_post_setting_first_metadata(app_client): 'default_value': 10, 'metadata': {'testing': True}, 'aliases': [], + 'version': '2.0', } @mark.asyncio async def test_put_setting_metadata(size_limit_setting, app_client): res = await app_client.put('/api/v1/settings/size_limit/metadata', data=json.dumps({ - 'metadata': {'first': 'yes', 'second': 'no'} + 'metadata': {'first': 'yes', 'second': 'no'}, + 'version': '2.0' })) res.raise_for_status() setting = await app_client.get('/api/v1/settings/size_limit') @@ -83,13 +91,15 @@ async def test_put_setting_metadata(size_limit_setting, app_client): 'default_value': 200, 'metadata': {'first': 'yes', 'second': 'no'}, 'aliases': [], + 'version': '2.0', } @mark.asyncio async def test_put_not_existing_setting_metadata(app_client): res = await app_client.put('/api/v1/settings/no_setting/metadata', data=json.dumps({ - 'metadata': {'testing': True} + 'metadata': {'testing': True}, + 'version': '2.0' })) assert res.status_code == 404 @@ -97,7 +107,8 @@ async def test_put_not_existing_setting_metadata(app_client): @mark.asyncio async def test_put_setting_empty_metadata(size_limit_setting, app_client): res = await app_client.put('/api/v1/settings/size_limit/metadata', data=json.dumps({ - 'metadata': {} + 'metadata': {}, + 'version': '2.0' })) res.raise_for_status() setting = await app_client.get('/api/v1/settings/size_limit') @@ -108,13 +119,15 @@ async def test_put_setting_empty_metadata(size_limit_setting, app_client): 'default_value': 200, 'metadata': {}, 'aliases': [], + 'version': '2.0', } @mark.asyncio async def test_put_setting_metadata_existing_key(size_limit_setting, app_client): res = await app_client.put('/api/v1/settings/size_limit/metadata/testing', data=json.dumps({ - 'value': 1000 + 'value': 1000, + 'version': '2.0' })) res.raise_for_status() setting = await app_client.get('/api/v1/settings/size_limit') @@ -125,13 +138,15 @@ async def test_put_setting_metadata_existing_key(size_limit_setting, app_client) 'default_value': 200, 'metadata': {'testing': 1000}, 'aliases': [], + 'version': '2.0', } @mark.asyncio async def test_put_setting_metadata_not_existing_key(size_limit_setting, app_client): res = await app_client.put('/api/v1/settings/size_limit/metadata/hello', data=json.dumps({ - 'value': 'world' + 'value': 'world', + 'version': '2.0' })) res.raise_for_status() setting = await app_client.get('/api/v1/settings/size_limit') @@ -142,12 +157,15 @@ async def test_put_setting_metadata_not_existing_key(size_limit_setting, app_cli 'default_value': 200, 'metadata': {'testing': True, 'hello': 'world'}, 'aliases': [], + 'version': '2.0', } @mark.asyncio async def test_delete_setting_metadata(size_limit_setting, app_client): - res = await app_client.delete('/api/v1/settings/size_limit/metadata') + res = await app_client.delete('/api/v1/settings/size_limit/metadata', data=json.dumps({ + 'version': '2.0' + })) res.raise_for_status() setting = await app_client.get('/api/v1/settings/size_limit') assert setting.json() == { @@ -157,21 +175,27 @@ async def test_delete_setting_metadata(size_limit_setting, app_client): 'default_value': 200, 'metadata': {}, 'aliases': [], + 'version': '2.0', } @mark.asyncio async def test_delete_not_existing_setting_metadata(app_client): - res = await app_client.delete('/api/v1/settings/test_setting/metadata') + res = await app_client.delete('/api/v1/settings/test_setting/metadata', data=json.dumps({ + 'version': '2.0' + })) assert res.status_code == 404 @mark.asyncio async def test_delete_specific_key_from_setting_metadata(size_limit_setting, app_client): await app_client.put('/api/v1/settings/size_limit/metadata/hello', data=json.dumps({ - 'value': 'world' + 'value': 'world', + 'version': '2.0' + })) + res = await app_client.delete('/api/v1/settings/size_limit/metadata/testing', data=json.dumps({ + 'version': '3.0' })) - res = await app_client.delete('/api/v1/settings/size_limit/metadata/testing') res.raise_for_status() setting = await app_client.get('/api/v1/settings/size_limit') assert setting.json() == { @@ -181,6 +205,7 @@ async def test_delete_specific_key_from_setting_metadata(size_limit_setting, app 'default_value': 200, 'metadata': {'hello': 'world'}, 'aliases': [], + 'version': '3.0', } @@ -195,7 +220,7 @@ async def test_get_setting_metadata(size_limit_setting, app_client): @mark.asyncio async def test_get_setting_no_metadata(app_client): - await app_client.put('/api/v1/settings/declare', data=json.dumps({ + await app_client.post('/api/v1/settings/declare', data=json.dumps({ 'name': 'test_setting', 'configurable_features': ['user'], 'type': 'int',