From 31d44cff64e88d16be50ddae714ae555f8fd9e43 Mon Sep 17 00:00:00 2001 From: Ben Avrahami Date: Tue, 28 Dec 2021 09:49:20 +0200 Subject: [PATCH] New query api (#44) * behavior done * updated docs * lint * fixes * fix --- .github/workflows/pull-requests.yml | 6 +- CHANGELOG.md | 19 +- ...13520cb02b_removed_setting_s_touch_time.py | 30 ++ docs/api.rst | 74 ++- heksher/api/v1/context_features.py | 7 +- heksher/api/v1/rules.py | 229 +++++----- heksher/api/v1/settings.py | 8 +- heksher/api/v1/util.py | 30 +- heksher/db_logic/metadata.py | 3 +- heksher/db_logic/rule.py | 50 +- heksher/db_logic/setting.py | 16 +- scripts/blackbox_test.sh | 1 - scripts/coverage.sh | 7 + scripts/unittest.sh | 1 - tests/blackbox/app/test_v1api_aliases.py | 26 +- .../app/test_v1api_context_features.py | 24 + tests/blackbox/app/test_v1api_rules.py | 429 ++++++++++-------- tools/mk_revision.py | 17 + 18 files changed, 555 insertions(+), 422 deletions(-) create mode 100644 alembic/versions/7713520cb02b_removed_setting_s_touch_time.py delete mode 100755 scripts/blackbox_test.sh create mode 100644 scripts/coverage.sh delete mode 100755 scripts/unittest.sh create mode 100644 tools/mk_revision.py diff --git a/.github/workflows/pull-requests.yml b/.github/workflows/pull-requests.yml index b994b0e..e503e07 100644 --- a/.github/workflows/pull-requests.yml +++ b/.github/workflows/pull-requests.yml @@ -28,9 +28,7 @@ jobs: uses: docker-practice/actions-setup-docker@0.0.1 - name: Linting run: poetry run sh scripts/lint.sh - - name: Run unittests - run: poetry run sh scripts/unittest.sh - - name: Run blackbox tests - run: poetry run sh scripts/blackbox_test.sh + - name: Run unittests + app tests + run: poetry run sh scripts/coverage.sh - name: Run image test run: poetry run sh scripts/image_test.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 45404f5..f979b3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,23 @@ # Heksher Changelog -## Next -### Added -* declarations are now tolerant of subtypes (to account for previous type upgrade) -* documentation -* The api endpoint PUT /api/v1/rules//value to change a rule's value +## Next (REQ alembic upgrade) +### 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 method of the endpoint /api/v1/rules/search has been changed to GET. ### 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 +### Added +* declarations are now tolerant of subtypes (to account for previous type upgrade) +* 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) +### Fixed +* A bug where patching a context feature's index using "to_before" would use the incorrect target. +### Internal +* a new script to test and correctly report coverage +* tools/mk_revision.py to easily create alembic revisions ## 0.4.1 ### Removed * removed the alembic extra, it's now a requirement diff --git a/alembic/versions/7713520cb02b_removed_setting_s_touch_time.py b/alembic/versions/7713520cb02b_removed_setting_s_touch_time.py new file mode 100644 index 0000000..64bafde --- /dev/null +++ b/alembic/versions/7713520cb02b_removed_setting_s_touch_time.py @@ -0,0 +1,30 @@ +""""removed setting's touch time" + +Revision ID: 7713520cb02b +Revises: 37b28f79cccf +Create Date: 2021-12-26 14:07:09.427397 + +""" +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +from alembic import op + +# revision identifiers, used by Alembic. +revision = '7713520cb02b' +down_revision = '37b28f79cccf' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('settings', 'last_touch_time') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('settings', sa.Column('last_touch_time', postgresql.TIMESTAMP(), autoincrement=False, nullable=False, + server_default=sa.func.now())) + # ### end Alembic commands ### diff --git a/docs/api.rst b/docs/api.rst index aa5e34a..45f9df4 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -10,7 +10,7 @@ Unless otherwise noted, all responses have the status code 200. Since Heksher is a FastAPI service, the API can also be accessed via the redoc endpoint ``/redoc``. The most common endpoints for users are :ref:`setting declaration `, -and :ref:`rule querying ` +and :ref:`rule querying ` General ------- @@ -124,15 +124,16 @@ Delete a rule. Responds with a 204 response. -POST /api/v1/rules/search +GET /api/v1/rules/search *************************** Find a rule by its setting and feature_values. -Request: +Query parameters: * **setting**: The name of the setting the rule to applies to. -* **feature_values**: A dictionary of the values of the context features that the rule should apply to. +* **feature_values**: A comma-seperated list of colon-seperated pairs context features and their values that the rule + should apply to. Example: ``../api/v1/rules/search?setting=foo&feature_values=bar:a,baz:b`` If a rule does not exists to that setting and feature_values, returns a 404 response. @@ -156,39 +157,66 @@ PATCH /api/v1/rules/ A deprecated route that is equivalent to `PUT /api/v1/rules//value`_. -POST /api/v1/rules/query +GET /api/v1/rules/query ************************** .. note:: This should be the primary endpoint that users call to get rules. +.. note:: + + This endpoint supports the + `If-None-Match `_ header. It also returns + an `ETag `_ header on successful responses. + Query the rules in the service, filtering in only rules pertaining to specific settings and contexts. -Request: +Query parameters: + +* **settings**: A comma-seperated list of the names of the settings to query. If specified only rules that apply to one + of the settings in this list will be returned. Example: ``../api/v1/rules/query?settings=foo,bar`` +* **context_filters**: A comma-seperated list of filters to filter rules by their context. If any filters are specified + only rules all of whose exact-match context conditions match the relevant filters will be returned. Each filter is + is a colon-separated pair. The first element of the pair is the context feature name, the second element is either + the special character ``*`` to accept all values of the context feature, or a comma-seperated list of the values + in parentheses. Example: ``../api/v1/rules/query?context_filters=foo:*,bar:(a,b)``. Alternatively, the context_filters + can be the special character ``*`` to accept all context features (this is the default behaviour). -* **setting_names**: A list of the names of the settings to query. Only rules that apply to one of the - settings in this list will be returned. -* **context_feature_options**: A dictionary that maps context feature names to arrays of values to consider when - querying. Only rules whose exact-match conditions are all in the respective arrays will be returned. Alternatively, - a context feature value list can be replaced with the string "*" to indicate that all values of that context feature - should be considered. Finally, the entire dictionary can be replaced with the string "*" to indicate that all rules - should be returned, regardless of their condition. -* **cache_time** (optional): The timestamp of the user's cache for this query. If provided, then only settings that have - been changed since this timestamp will be returned (the rest will be omitted from the results). -* **include_metadata** (optional, default false): If true, then the metadata associated with each rule will be included in + .. note:: Context Filter Example + + Assuming a setting has the context features ``X``, ``Y``, and ``Z``, and the following rules: + + .. csv-table:: + :header: "X", "Y", "Z", "**rule_id**" + + "x_0", "\*", "\*", "1" + "x_1", "\*", "\*", "2" + "x_0", "y_0", "\*", "3" + "x_0", "y_1", "\*", "4" + "x_2", "y_0", "\*", "5" + "\*", "\*", "z_0", "6" + "x_0", "\*", "z_0", "7" + + The the context filter: ``X:(x_0,x_1),Y:*`` will only allow the rules ``1``, ``2``, ``3``, and ``4``. Rule ``5`` will + be rejected because it's X condition is not in the X filter's list of values. Rules ``6`` and ``7`` will be rejected + because they have a Z condition and there is no Z filter. + +* **include_metadata** (default false): If true, then the metadata associated with each rule will be included in the results. Response: -* **rules**: A dictionary that maps setting names to arrays of rules that apply to that setting and pass the filters in the - request. If a setting has not been changed since the cache_time, then it will not be in the result. - Each rule is a dictionary with the following keys: +* **settings**: A dictionary that maps setting names to query results of that setting and pass the filters in the + request. Each rule is a dictionary with the following keys: + + * **rules**: A list of rule dictionaries, that contains all teh rules that met the query criteria. Each rule + dictionary has the following keys: - * **value**: The value a setting should take if the rule is matched. - * **feature_values**: An array of 2-str-arrays of the context feature names and values that the rule applies to, in order - of the context features. - * **metadata**: A dictionary of metadata associated with the rule. Only present if include_metadata is true. + * **value**: The value a setting should take if the rule is matched. + * **feature_values**: An array of 2-str-arrays of the context feature names and values that the rule applies to, in order + of the context features. + * **metadata**: A dictionary of metadata associated with the rule. Only present if include_metadata is true. GET /api/v1/rules/ *************************** diff --git a/heksher/api/v1/context_features.py b/heksher/api/v1/context_features.py index 7a4e060..4052f67 100644 --- a/heksher/api/v1/context_features.py +++ b/heksher/api/v1/context_features.py @@ -87,12 +87,11 @@ async def patch_context_feature(name: str, input: Union[PatchAfterContextFeature target_index = await app.db_logic.get_context_feature_index(input.target) if index_to_move is None or target_index is None: return Response(status_code=status.HTTP_404_NOT_FOUND) + if isinstance(input, PatchBeforeContextFeatureInput): + target_index -= 1 if index_to_move == target_index: return None - if isinstance(input, PatchAfterContextFeatureInput): - await app.db_logic.move_after_context_feature(index_to_move, target_index) - else: # move before target - await app.db_logic.move_after_context_feature(index_to_move, target_index - 1) + await app.db_logic.move_after_context_feature(index_to_move, target_index) class AddContextFeatureInput(ORJSONModel): diff --git a/heksher/api/v1/rules.py b/heksher/api/v1/rules.py index 2b07d9a..f4f081d 100644 --- a/heksher/api/v1/rules.py +++ b/heksher/api/v1/rules.py @@ -1,14 +1,15 @@ -from datetime import datetime +import re from logging import getLogger -from typing import Any, Dict, List, Literal, Optional, Tuple, Union +from typing import Any, Dict, List, Optional, Tuple, Union -from fastapi import APIRouter, Response +from fastapi import APIRouter, Query, Response from pydantic import Field, validator from starlette import status +from starlette.requests import Request from starlette.responses import PlainTextResponse from heksher.api.v1.rules_metadata import router as metadata_router -from heksher.api.v1.util import ORJSONModel, application, router as v1_router +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 @@ -28,35 +29,30 @@ async def delete_rule(rule_id: int, app: HeksherApp = application): return Response(status_code=status.HTTP_404_NOT_FOUND) await app.db_logic.delete_rule(rule_id) - await app.db_logic.touch_setting(rule_spec.setting) - - -class SearchRuleInput(ORJSONModel): - setting: SettingName = Field(description="the setting name the rule belongs to") - feature_values: Dict[ContextFeatureName, ContextFeatureValue] = \ - Field(description="the exact-match conditions of the rule") - - @validator('feature_values') - @classmethod - def feature_values_not_empty(cls, v): - if not v: - raise ValueError('feature_values must not be empty') - return v class SearchRuleOutput(ORJSONModel): rule_id: int -@router.post('/search', response_model=SearchRuleOutput) -async def search_rule(input: SearchRuleInput, app: HeksherApp = application): +@router.get('/search', response_model=SearchRuleOutput) +async def search_rule(app: HeksherApp = application, + setting: str = Query(..., + description="the name of the setting to search", + regex='[a-zA-Z_0-9.]+$'), + feature_values: str = Query(..., + description="a comma-separated list of context features for the " + "rule", + regex='[a-zA-Z_0-9]+:[a-zA-Z_0-9]+(,[a-zA-Z_0-9]+:[a-zA-Z_0-9]+)*$', + example=["a:X,c:Z"]), ): """ Get the ID of a rule with specific conditions. """ - setting = await app.db_logic.get_setting(input.setting, include_metadata=False) # for aliasing - if not setting: + canon_setting = await app.db_logic.get_setting(setting, include_metadata=False) # for aliasing + if not canon_setting: return Response(status_code=status.HTTP_404_NOT_FOUND) - rule_id = await app.db_logic.get_rule_id(setting.name, input.feature_values) + feature_values_dict: Dict[str, str] = dict(part.split(':') for part in feature_values.split(',')) # type: ignore + rule_id = await app.db_logic.get_rule_id(canon_setting.name, feature_values_dict) if not rule_id: return Response(status_code=status.HTTP_404_NOT_FOUND) return SearchRuleOutput(rule_id=rule_id) @@ -104,7 +100,6 @@ async def add_rule(input: AddRuleInput, app: HeksherApp = application): return PlainTextResponse('rule already exists', status_code=status.HTTP_409_CONFLICT) new_id = await app.db_logic.add_rule(setting.name, input.value, input.metadata, input.feature_values) - await app.db_logic.touch_setting(setting.name) return AddRuleOutput(rule_id=new_id) @@ -131,57 +126,6 @@ async def patch_rule(rule_id: int, input: PatchRuleInput, app: HeksherApp = appl status_code=status.HTTP_400_BAD_REQUEST) await app.db_logic.patch_rule(rule_id, input.value) - await app.db_logic.touch_setting(setting.name) - - -class QueryRulesInput(ORJSONModel): - setting_names: List[SettingName] = Field(description="a list of setting names to return the rules for") - context_features_options: Union[ - Dict[ContextFeatureName, Union[ - List[ContextFeatureValue], Literal['*'] - ]], - Literal['*'] - ] = Field( - description="a mapping of context features and possible values. Any rule with an exact-match condition not in" - " this mapping will not be returned. Optionally can be set to '*' to return all rules, or set an" - " individual context feature to '*' to not disqualify rules with conditions on that context" - " feature." - ) - cache_time: Optional[datetime] = Field(None, description="if provided, any settings that have not been changed" - " since this time will be ignored") - include_metadata: bool = Field(False, description="whether to load and include the metadata of each rule in" - " the results") - - @validator('context_features_options') - @classmethod - def wildcard(cls, v): - if v == '*': - return None - for k in v.keys(): - if v[k] == '*': - v[k] = None - elif not v[k]: - raise ValueError('cannot accept an empty option') - return v - - @validator('cache_time') - @classmethod - def no_tz(cls, v: Optional[datetime]): - if v and v.tzinfo: - raise ValueError('cannot accept datetime with timezone') - return v - - @validator('cache_time') - @classmethod - def no_future(cls, v: Optional[datetime]): - now = datetime.utcnow() - if now < v: - raise ValueError('got cache time in the future') - return v - - @property - def valid_context_features_options(cls) -> Optional[Dict[str, Optional[List[str]]]]: - return cls.context_features_options # type: ignore # https://github.com/tiangolo/fastapi/issues/2724 @@ -193,64 +137,117 @@ class QueryRulesOutput_Rule(ORJSONModel): rule_id: int = Field(description="unique identifier of the rule.") +class QueryRulesOutput_Setting(ORJSONModel): + rules: List[QueryRulesOutput_Rule] = Field(description="a list of rules for the setting") + + class QueryRulesOutput(ORJSONModel): - rules: Dict[str, List[QueryRulesOutput_Rule]] = Field(description="a list of rules for each setting that was not" - " filtered out") + settings: Dict[str, QueryRulesOutput_Setting] = Field(description="query results for each setting") class QueryRulesOutputWithMetadata_Rule(QueryRulesOutput_Rule): metadata: Dict[MetadataKey, Any] = Field(description="the metadata of the rule, if requested") -class QueryRulesOutputWithMetadata(ORJSONModel): - rules: Dict[str, List[QueryRulesOutputWithMetadata_Rule]] = Field( - description="a list of rules for each setting that was not filtered out" - ) +class QueryRulesOutputWithMetadata_Setting(ORJSONModel): + rules: List[QueryRulesOutputWithMetadata_Rule] = Field(description="a list of rules for the setting") -@router.post('/query', response_model=Union[QueryRulesOutputWithMetadata, QueryRulesOutput]) # type: ignore -async def query_rules(input: QueryRulesInput, app: HeksherApp = application): - """ - Query settings for rules for a specific set of potential contexts. - """ - if input.context_features_options is not None: - not_context_features = await app.db_logic.get_not_found_context_features(input.context_features_options) - if not_context_features: - return PlainTextResponse(f'the following are not valid context features: {not_context_features}', - status_code=status.HTTP_404_NOT_FOUND) - - if input.setting_names: - names = await app.db_logic.get_canonical_names(input.setting_names) - not_settings = [k for k, v in names.items() if not v] +class QueryRulesOutputWithMetadata(ORJSONModel): + settings: Dict[str, QueryRulesOutputWithMetadata_Setting] = Field(description="query results for each setting") + + +raw_context_feature_filters_pattern = r'''(?x) +( + \* # we allow an explicit global wildcard + | + [a-zA-Z_0-9]+: # context feature name + ( + \* # accept any value + | + \( + [a-zA-Z_0-9]+ # value + (,[a-zA-Z_0-9]+)* # additional values + \) + ) + ( + ,[a-zA-Z_0-9]+: (\* | \([a-zA-Z_0-9]+ (,[a-zA-Z_0-9]+)*\)) # additional filters + )* +)? # we also allow empty string to signify no rules could match +$ +''' + + +@router.get('/query', response_model=Union[QueryRulesOutputWithMetadata, QueryRulesOutput]) # type: ignore +async def query_rules(request: Request, app: HeksherApp = application, + raw_settings: str = Query(None, alias='settings', + description="a comma-separated list of setting names", + regex='([a-zA-Z_0-9.]+(,[a-zA-Z_0-9.]+)*)?$'), + raw_context_filters: str = Query( + '*', alias='context_filters', + description="a comma-separated list of context feature filters", + regex=raw_context_feature_filters_pattern, example=["a:(X,Y),b:(Z),c:*", '*', 'a:*']), + include_metadata: bool = Query(False, description="whether to include rule metadata in the" + " response"), + ): + if raw_settings is None: + settings = await app.db_logic.get_all_settings_names() + elif not raw_settings: + settings = [] + else: + names = raw_settings.split(',') + aliases = await app.db_logic.get_canonical_names(names) + not_settings = [k for k, v in aliases.items() if not v] if not_settings: return PlainTextResponse(f'the following are not setting names: {not_settings}', status_code=status.HTTP_404_NOT_FOUND) + settings = list(aliases.values()) - query_result = await app.db_logic.query_rules(list(names.values()), - input.valid_context_features_options, - input.cache_time, input.include_metadata) + if raw_context_filters == '*': + context_features_options: Optional[Dict[str, Optional[List[str]]]] = None else: - query_result = {} - - if input.include_metadata: - return QueryRulesOutputWithMetadata( - rules={ - setting: [ - QueryRulesOutputWithMetadata_Rule( - value=rule.value, context_features=rule.feature_values, metadata=rule.metadata, - rule_id=rule.rule_id - ) - for rule in rules - ] for setting, rules in query_result.items() + context_filter_items = ((match['key'], (None if match['values'] is None else match['values'].split(','))) + for match in + re.finditer(r'(?P[a-z]+):(\((?P[^)]+)\)|\*)', raw_context_filters)) + context_features_options = {} + for k, v in context_filter_items: + if k in context_features_options: + return PlainTextResponse(f'context name repeated in context filter: {k}', + status_code=status.HTTP_400_BAD_REQUEST) + context_features_options[k] = v + + not_context_features = await app.db_logic.get_not_found_context_features(context_features_options) + if not_context_features: + return PlainTextResponse(f'the following are not valid context features: {not_context_features}', + status_code=status.HTTP_404_NOT_FOUND) + results = await app.db_logic.query_rules(settings, context_features_options, include_metadata) + if include_metadata: + ret: Union[QueryRulesOutputWithMetadata, QueryRulesOutput] = QueryRulesOutputWithMetadata( + settings={ + setting: + QueryRulesOutputWithMetadata_Setting(rules=[ + QueryRulesOutputWithMetadata_Rule( + value=rule.value, context_features=rule.feature_values, metadata=rule.metadata, + rule_id=rule.rule_id + ) + for rule in rules + ]) for setting, rules in results.items() }) else: - return QueryRulesOutput( - rules={ - setting: [ - QueryRulesOutput_Rule(value=rule.value, context_features=rule.feature_values, rule_id=rule.rule_id) - for rule in rules - ] for setting, rules in query_result.items() + ret = QueryRulesOutput( + settings={ + setting: + QueryRulesOutput_Setting(rules=[ + QueryRulesOutput_Rule( + value=rule.value, context_features=rule.feature_values, metadata=rule.metadata, + rule_id=rule.rule_id + ) + for rule in rules + ]) for setting, rules in results.items() }) + response = PydanticResponse(ret) + handle_etag(response, request) + return response class GetRuleOutput(ORJSONModel): diff --git a/heksher/api/v1/settings.py b/heksher/api/v1/settings.py index e6bfa1c..8cacf47 100644 --- a/heksher/api/v1/settings.py +++ b/heksher/api/v1/settings.py @@ -1,4 +1,3 @@ -from datetime import datetime from logging import getLogger from typing import Any, Dict, List, Optional, Union @@ -91,7 +90,7 @@ async def declare_setting(input: DeclareSettingInput, app: HeksherApp = applicat await app.db_logic.add_setting(new_setting, alias=input.alias) return DeclareSettingOutput(created=True, changed=[], incomplete={}) - to_change: Dict[str, Any] = {'last_touch_time': datetime.utcnow()} + to_change: Dict[str, Any] = {} changed = [] incomplete: Dict[str, Any] = {} @@ -131,11 +130,13 @@ async def declare_setting(input: DeclareSettingInput, app: HeksherApp = applicat 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() @@ -150,7 +151,7 @@ async def declare_setting(input: DeclareSettingInput, app: HeksherApp = applicat else: new_metadata = None - if to_change or new_configurable_features: + 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) @@ -328,7 +329,6 @@ async def rename_setting(name: str, input: RenameSettingInput, app: HeksherApp = 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.touch_setting(input.name) return None diff --git a/heksher/api/v1/util.py b/heksher/api/v1/util.py index ba96993..430d13a 100644 --- a/heksher/api/v1/util.py +++ b/heksher/api/v1/util.py @@ -1,8 +1,11 @@ from __future__ import annotations +from hashlib import md5 + import orjson -from fastapi import APIRouter, Depends, Request +from fastapi import APIRouter, Depends, HTTPException, Request from pydantic import BaseModel +from starlette.responses import JSONResponse, Response def orjson_dumps(v, **kwargs): @@ -20,12 +23,35 @@ class Config: json_loads = orjson.loads +class PydanticResponse(JSONResponse): + media_type = "application/json" + + def render(self, content: BaseModel) -> bytes: + return content.json().encode("utf-8") + + @Depends def application(request: Request): """ - A helper dependancy to get the app instance + A helper dependency to get the app instance """ return request.app +def handle_etag(response: Response, request: Request): + """ + A utility function to add etag to a response, and to check whether the request has the same etag. + """ + response_etag = response.headers.get('ETag') + if response_etag is None: + hasher = md5() + hasher.update(response.body) + response_etag = response.headers['ETag'] = f'"{hasher.hexdigest()}"' + if_none_match = request.headers.get('If-None-Match') + if if_none_match is None: + return + if if_none_match == '*' or response_etag in if_none_match: + raise HTTPException(status_code=304, headers=response.headers) # type: ignore[arg-type] + + router = APIRouter(prefix='/api/v1') diff --git a/heksher/db_logic/metadata.py b/heksher/db_logic/metadata.py index 123cd06..960bb5d 100644 --- a/heksher/db_logic/metadata.py +++ b/heksher/db_logic/metadata.py @@ -1,4 +1,4 @@ -from sqlalchemy import TIMESTAMP, Column, ForeignKey, Integer, MetaData, String, Table, UniqueConstraint +from sqlalchemy import Column, ForeignKey, Integer, MetaData, String, Table, UniqueConstraint from sqlalchemy.dialects.postgresql import JSONB metadata = MetaData() @@ -7,7 +7,6 @@ Column('name', String, primary_key=True), Column('type', String, nullable=False), Column('default_value', String, nullable=True), - Column('last_touch_time', TIMESTAMP(), nullable=False), ) context_features = Table('context_features', metadata, diff --git a/heksher/db_logic/rule.py b/heksher/db_logic/rule.py index eb74bc1..a46bc4d 100644 --- a/heksher/db_logic/rule.py +++ b/heksher/db_logic/rule.py @@ -1,6 +1,5 @@ from __future__ import annotations -from datetime import datetime from itertools import groupby from operator import itemgetter from typing import Any, Dict, List, Mapping, NamedTuple, Optional, Sequence, Tuple @@ -9,7 +8,7 @@ from sqlalchemy import and_, func, join, not_, select, tuple_ from heksher.db_logic.logic_base import DBLogicBase -from heksher.db_logic.metadata import conditions, context_features, rule_metadata, rules, settings +from heksher.db_logic.metadata import conditions, context_features, rule_metadata, rules class RuleSpec(NamedTuple): @@ -194,7 +193,6 @@ async def patch_rule(self, rule_id: int, value: Any) -> None: async def query_rules(self, setting_names: List[str], feature_value_options: Optional[Dict[str, Optional[List[str]]]], - setting_touch_time_cutoff: Optional[datetime], include_metadata: bool) -> Dict[str, List[InnerRuleSpec]]: """ Search the rules of multiple settings @@ -211,23 +209,11 @@ async def query_rules(self, setting_names: List[str], A mapping of non-filtered settings to rules """ - if setting_touch_time_cutoff: - async with self.db_engine.connect() as conn: - settings_results = (await conn.execute( - select([settings.c.name]) - .where( - settings.c.name.in_(setting_names) - & (settings.c.last_touch_time >= setting_touch_time_cutoff) - ) - )).scalars().all() - else: - settings_results = setting_names - applicable_rules: Dict[int, Tuple[Tuple[str, str], ...]] = {} conditions_ = conditions.alias() - if not settings_results: - # shortcut in case all settings are up-to-date + if not setting_names: + # shortcut in case no settings are selected rule_results = [] else: # inv_match is a mixin condition, if an exact-match condition returns True for it, the rule associated with @@ -255,22 +241,22 @@ async def query_rules(self, setting_names: List[str], else: inv_match = tuple_conditions if exact_tuple_conditions else cf_conditions - settings_container = [name for name in settings_results] + clauses = [ + not_(conditions_.select() + .where( + and_( + conditions.c.rule == conditions_.c.rule, inv_match + )) + .exists()), + ] + 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) .select_from(rules.outerjoin(conditions, rules.c.id == conditions.c.rule)) .outerjoin(context_features, context_features.c.name == conditions.c.context_feature) - .where( - and_(rules.c.setting.in_(settings_container), - not_(conditions_.select() - .where( - and_( - conditions.c.rule == conditions_.c.rule, inv_match - )) - .exists()) - ) - ) + .where(*clauses) .order_by(rules.c.id, context_features.c.index)) async with self.db_engine.connect() as conn: @@ -290,7 +276,7 @@ async def query_rules(self, setting_names: List[str], rule_query = ( select([rules.c.id, rules.c.setting, rules.c.value]) .where(rules.c.id.in_(applicable_rules)) - .order_by(rules.c.setting) + .order_by(rules.c.setting, rules.c.id) ) async with self.db_engine.connect() as conn: rule_results = (await conn.execute(rule_query)).mappings().all() @@ -308,8 +294,7 @@ async def query_rules(self, setting_names: List[str], else: metadata = None - ret = {} - missing_settings = set(settings_results) + ret: Dict[str, List[InnerRuleSpec]] = {setting: [] for setting in setting_names} for setting, rows in groupby(rule_results, key=itemgetter('setting')): rule_list = [ InnerRuleSpec( @@ -321,9 +306,6 @@ async def query_rules(self, setting_names: List[str], for row in rows ] ret[setting] = rule_list - missing_settings.remove(setting) - for setting in missing_settings: - ret[setting] = [] return ret async def get_rules_for_setting(self, setting_name: str) -> Sequence[BareRuleSpec]: diff --git a/heksher/db_logic/setting.py b/heksher/db_logic/setting.py index 73bd6dd..9569530 100644 --- a/heksher/db_logic/setting.py +++ b/heksher/db_logic/setting.py @@ -1,4 +1,3 @@ -from datetime import datetime from itertools import groupby from typing import Any, Dict, Iterable, List, Mapping, NamedTuple, Optional @@ -67,6 +66,7 @@ async def get_setting(self, name_or_alias: str, *, include_metadata: bool) -> Op join(settings, setting_aliases, settings.c.name == setting_aliases.c.setting, isouter=True) ) .where(or_(settings.c.name == name_or_alias, setting_aliases.c.alias == name_or_alias)) + .limit(1) ) data_row = (await conn.execute(stmt)).mappings().first() @@ -116,7 +116,6 @@ 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'), - last_touch_time=datetime.utcnow(), ) ) await conn.execute( @@ -174,19 +173,6 @@ async def update_setting(self, name: str, changed: Mapping[str, Any], new_contex ).on_conflict_do_nothing() ) - async def touch_setting(self, name: str, timestamp: Optional[datetime] = None): - """ - Update a setting's last_touch_time - Args: - name: the name of the setting to update - timestamp: the time to set the last_touch_time, defaults to datetime.now - """ - timestamp = timestamp or datetime.utcnow() - async with self.db_engine.begin() as conn: - await conn.execute( - settings.update().where(settings.c.name == name).values(last_touch_time=timestamp) - ) - async def delete_setting(self, name: str) -> bool: """ Remove a setting from the DB diff --git a/scripts/blackbox_test.sh b/scripts/blackbox_test.sh deleted file mode 100755 index 68b65d4..0000000 --- a/scripts/blackbox_test.sh +++ /dev/null @@ -1 +0,0 @@ -python -m pytest tests/blackbox/app -s -x --cov=heksher --cov-report=xml --cov-report=term-missing \ No newline at end of file diff --git a/scripts/coverage.sh b/scripts/coverage.sh new file mode 100644 index 0000000..a72c409 --- /dev/null +++ b/scripts/coverage.sh @@ -0,0 +1,7 @@ +#!/bin/sh +set -e +coverage run --branch --context=ut --include="heksher/*" --concurrency=greenlet -m pytest tests/unittests +coverage run -a --branch --context=blackbox --include="heksher/*" --concurrency=greenlet -m pytest tests/blackbox/app +coverage html +coverage report -m +coverage xml \ No newline at end of file diff --git a/scripts/unittest.sh b/scripts/unittest.sh deleted file mode 100755 index fed2087..0000000 --- a/scripts/unittest.sh +++ /dev/null @@ -1 +0,0 @@ -python -m pytest tests/unittests -s -x --cov=heksher --cov-report=xml --cov-report=term-missing \ No newline at end of file diff --git a/tests/blackbox/app/test_v1api_aliases.py b/tests/blackbox/app/test_v1api_aliases.py index e9d5a59..ec85ac1 100644 --- a/tests/blackbox/app/test_v1api_aliases.py +++ b/tests/blackbox/app/test_v1api_aliases.py @@ -1,5 +1,5 @@ import json -from typing import Iterable, Optional +from typing import Optional from pytest import fixture, mark, raises from requests import HTTPError @@ -75,16 +75,16 @@ async def add(setting: str, theme: str): }))) async def search(setting: str, theme: str): - return _get_ok_data(await app_client.post('/api/v1/rules/search', data=json.dumps({ + return _get_ok_data(await app_client.get('/api/v1/rules/search', query_string={ 'setting': setting, - 'feature_values': {'theme': theme}, - }))) + 'feature_values': f'theme:{theme}', + })) - async def query(*settings: Iterable[str]): - return _get_ok_data(await app_client.post('/api/v1/rules/query', data=json.dumps({ - 'setting_names': settings, - 'context_features_options': "*", - }))) + async def query(*settings: str): + return _get_ok_data(await app_client.get('/api/v1/rules/query', query_string={ + 'settings': ','.join(settings), + '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)) @@ -101,10 +101,10 @@ async def query(*settings: Iterable[str]): assert (await search("kelev", "dracula"))["rule_id"] == kelev_rule with raises(HTTPError): await query("cat", "kelev", "yanshuf") - assert (await query("cat", "kelev"))['rules'] == { - 'cat': [{'value': 10, 'context_features': [['theme', 'bright']], 'rule_id': cat_rule}, - {'value': 10, 'context_features': [['theme', 'dark']], 'rule_id': hatul_rule}], - 'dog': [{'value': 10, 'context_features': [['theme', 'dracula']], 'rule_id': kelev_rule}] + 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}]} } diff --git a/tests/blackbox/app/test_v1api_context_features.py b/tests/blackbox/app/test_v1api_context_features.py index ffc2489..ba6c6bc 100644 --- a/tests/blackbox/app/test_v1api_context_features.py +++ b/tests/blackbox/app/test_v1api_context_features.py @@ -67,6 +67,30 @@ async def test_patch_before_context_feature(app_client): } +@mark.asyncio +async def test_patch_to_middle_context_feature(app_client): + response = await app_client.patch('/api/v1/context_features/trust/index', data=json.dumps( + {"to_before": "theme"} + )) + response.raise_for_status() + context_features = await app_client.get('/api/v1/context_features') + assert context_features.json() == { + 'context_features': ["user", "trust", "theme"] + } + + +@mark.asyncio +async def test_patch_same_spot_context_feature(app_client): + response = await app_client.patch('/api/v1/context_features/trust/index', data=json.dumps( + {"to_before": "user"} + )) + response.raise_for_status() + context_features = await app_client.get('/api/v1/context_features') + assert context_features.json() == { + 'context_features': ["trust", "user", "theme"] + } + + @mark.asyncio async def test_patch_context_feature_doesnt_exists(app_client): response = await app_client.patch('/api/v1/context_features/trust/index', data=json.dumps( diff --git a/tests/blackbox/app/test_v1api_rules.py b/tests/blackbox/app/test_v1api_rules.py index c57938e..7e035ba 100644 --- a/tests/blackbox/app/test_v1api_rules.py +++ b/tests/blackbox/app/test_v1api_rules.py @@ -1,5 +1,4 @@ import json -from datetime import datetime, timedelta from itertools import chain from pytest import fixture, mark @@ -118,10 +117,10 @@ async def test_get_rule_missing(example_rule, app_client): @mark.asyncio async def test_search_rule(example_rule, app_client, sql_service): - res = await app_client.post('/api/v1/rules/search', data=json.dumps({ + res = await app_client.get('/api/v1/rules/search', query_string={ 'setting': 'size_limit', - 'feature_values': [('theme', 'bright')] - })) + 'feature_values': 'theme:bright' + }) res.raise_for_status() assert res.json() == { 'rule_id': example_rule @@ -130,19 +129,19 @@ async def test_search_rule(example_rule, app_client, sql_service): @mark.asyncio async def test_search_rule_empty(example_rule, app_client, sql_service): - res = await app_client.get('/api/v1/rules/search', data=json.dumps({ + res = await app_client.get('/api/v1/rules/search', query_string={ 'setting': 'size_limit', - 'feature_values': [] - })) + 'feature_values': '' + }) assert res.status_code == 422 @mark.asyncio async def test_search_rule_missing(example_rule, app_client): - res = await app_client.post('/api/v1/rules/search', data=json.dumps({ + res = await app_client.get('/api/v1/rules/search', query_string={ 'setting': 'size_limit', - 'feature_values': [('theme', 'dark')] - })) + 'feature_values': 'theme:dark' + }) assert res.status_code == 404 @@ -194,96 +193,34 @@ async def setup_rules(mk_setting, mk_rule): await mk_rule('a', {'theme': 'black', 'user': 'admin'}, 7) -@mark.asyncio -@mark.parametrize('metadata', [False, True]) -async def test_query_rules(metadata: bool, app_client, setup_rules): - res = await app_client.post('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': {'trust': ['full', 'part'], 'theme': ['black']}, - 'include_metadata': metadata - })) - - expected = { - 'rules': { - 'a': [ - {'context_features': [['trust', 'full']], 'value': 1, 'rule_id': 1}, - {'context_features': [['theme', 'black']], 'value': 2, 'rule_id': 2}, - {'context_features': [['trust', 'full'], ['theme', 'black']], 'value': 3, 'rule_id': 3} - ], - 'long_setting_name': [ - {'context_features': [['trust', 'part']], 'value': 5, 'rule_id': 5} - ] - } - } - if metadata: - for rule in chain.from_iterable(expected['rules'].values()): - rule['metadata'] = {'test': 'yes'} - - assert res.json() == expected +def patch_rule_expectation_with_metadata(expected_rules): + for rule in chain.from_iterable(s['rules'] for s in expected_rules['settings'].values()): + rule['metadata'] = {'test': 'yes'} @mark.asyncio @mark.parametrize('metadata', [False, True]) -async def test_query_rules_time_cache(metadata: bool, app_client, setup_rules, mk_rule): - current_time = datetime.utcnow() - # touch a to change its last_touch_time - await mk_rule('a', {'theme': 'grey', 'user': 'admin'}, 8) - - res = await app_client.post('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': {'trust': ['full', 'part'], 'theme': ['black']}, - 'include_metadata': metadata, - 'cache_time': current_time.isoformat(), - })) +async def test_query_rules(metadata: bool, app_client, setup_rules): + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'trust:(full,part),theme:(black)', + 'include_metadata': str(metadata) + }) expected = { - 'rules': { - 'a': [ + 'settings': { + 'a': {'rules': [ {'context_features': [['trust', 'full']], 'value': 1, 'rule_id': 1}, {'context_features': [['theme', 'black']], 'value': 2, 'rule_id': 2}, {'context_features': [['trust', 'full'], ['theme', 'black']], 'value': 3, 'rule_id': 3} - ], + ]}, + 'long_setting_name': {'rules': [ + {'context_features': [['trust', 'part']], 'value': 5, 'rule_id': 5} + ]} } } if metadata: - for rule in chain.from_iterable(expected['rules'].values()): - rule['metadata'] = {'test': 'yes'} - - assert res.json() == expected - - -@mark.asyncio -@mark.parametrize('metadata', [False, True]) -@mark.parametrize('suffix', ['Z', '+00:00', '+01:02', '-06:05']) -async def test_query_rules_bad_cache_time_zone(metadata: bool, suffix: str, app_client, setup_rules, mk_rule): - current_time = datetime.utcnow() - # touch a to change its last_touch_time - await mk_rule('a', {'theme': 'grey', 'user': 'admin'}, 8) - - res = await app_client.get('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': {'trust': ['full', 'part'], 'theme': ['black']}, - 'include_metadata': metadata, - 'cache_time': current_time.isoformat() + suffix, - })) - assert res.status_code == 422 - - -@mark.asyncio -@mark.parametrize('metadata', [False, True]) -async def test_query_rules_fully_cached(metadata: bool, app_client, setup_rules, mk_rule): - current_time = datetime.utcnow() - - res = await app_client.post('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': {'trust': ['full', 'part'], 'theme': ['black']}, - 'include_metadata': metadata, - 'cache_time': current_time.isoformat(), - })) - - expected = { - 'rules': {} - } + patch_rule_expectation_with_metadata(expected) assert res.json() == expected @@ -299,28 +236,27 @@ async def test_query_rules_with_empty(metadata: bool, app_client, setup_rules, s INSERT INTO rule_metadata (rule, key, value) VALUES (8, 'test', '"yes"') """) - res = await app_client.post('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': {'trust': ['full', 'part'], 'theme': ['black']}, - 'include_metadata': metadata - })) + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'trust:(full,part),theme:(black)', + 'include_metadata': str(metadata) + }) expected = { - 'rules': { - 'a': [ + 'settings': { + 'a': {'rules': [ {'context_features': [['trust', 'full']], 'rule_id': 1, 'value': 1}, {'context_features': [['theme', 'black']], 'rule_id': 2, 'value': 2}, {'context_features': [['trust', 'full'], ['theme', 'black']], 'rule_id': 3, 'value': 3} - ], - 'long_setting_name': [ + ]}, + 'long_setting_name': {'rules': [ {'context_features': [['trust', 'part']], 'rule_id': 5, 'value': 5}, {'context_features': [], 'rule_id': 8, 'value': 10} - ] + ]} } } if metadata: - for rule in chain.from_iterable(expected['rules'].values()): - rule['metadata'] = {'test': 'yes'} + patch_rule_expectation_with_metadata(expected) assert res.json() == expected @@ -328,15 +264,16 @@ async def test_query_rules_with_empty(metadata: bool, app_client, setup_rules, s @mark.asyncio @mark.parametrize('metadata', [False, True]) async def test_query_rules_nooptions(metadata: bool, app_client, setup_rules): - res = await app_client.post('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a'], - 'context_features_options': {}, + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a', + 'context_filters': '', 'include_metadata': metadata - })) + }) + res.raise_for_status() expected = { - 'rules': { - 'a': [] + 'settings': { + 'a': {'rules': []} } } @@ -354,24 +291,23 @@ async def test_query_rules_nooptions_with_matchall(metadata: bool, app_client, s INSERT INTO rule_metadata (rule, key, value) VALUES (8, 'test', '"yes"') """) - res = await app_client.post('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': {}, - 'include_metadata': metadata - })) + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': '', + 'include_metadata': str(metadata) + }) expected = { - 'rules': { - 'a': [], - 'long_setting_name': [ + 'settings': { + 'a': {'rules': []}, + 'long_setting_name': {'rules': [ {'context_features': [], 'value': 10, 'rule_id': 8}, - ] + ]} } } if metadata: - for rule in chain.from_iterable(expected['rules'].values()): - rule['metadata'] = {'test': 'yes'} + patch_rule_expectation_with_metadata(expected) assert res.json() == expected @@ -379,29 +315,28 @@ async def test_query_rules_nooptions_with_matchall(metadata: bool, app_client, s @mark.asyncio @mark.parametrize('metadata', [False, True]) async def test_query_rules_matchall(metadata: bool, app_client, setup_rules): - res = await app_client.post('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': '*', - 'include_metadata': metadata - })) + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': '*', + 'include_metadata': str(metadata) + }) expected = { - 'rules': { - 'a': [ + 'settings': { + 'a': {'rules': [ {'context_features': [['trust', 'full']], 'value': 1, 'rule_id': 1}, {'context_features': [['theme', 'black']], 'value': 2, 'rule_id': 2}, {'context_features': [['trust', 'full'], ['theme', 'black']], 'rule_id': 3, 'value': 3}, {'context_features': [['user', 'admin'], ['theme', 'black']], 'rule_id': 7, 'value': 7} - ], - 'long_setting_name': [ + ]}, + 'long_setting_name': {'rules': [ {'context_features': [['trust', 'none']], 'rule_id': 4, 'value': 4}, {'context_features': [['trust', 'part']], 'rule_id': 5, 'value': 5} - ] + ]} } } if metadata: - for rule in chain.from_iterable(expected['rules'].values()): - rule['metadata'] = {'test': 'yes'} + patch_rule_expectation_with_metadata(expected) assert res.json() == expected @@ -409,27 +344,26 @@ async def test_query_rules_matchall(metadata: bool, app_client, setup_rules): @mark.asyncio @mark.parametrize('metadata', [False, True]) async def test_query_rules_wildcard_some(metadata: bool, app_client, setup_rules): - res = await app_client.post('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': {'theme': '*', 'trust': ['full', 'none']}, - 'include_metadata': metadata - })) + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'theme:*,trust:(full,none)', + 'include_metadata': str(metadata) + }) expected = { - 'rules': { - 'a': [ + 'settings': { + 'a': {'rules': [ {'context_features': [['trust', 'full']], 'value': 1, 'rule_id': 1}, {'context_features': [['theme', 'black']], 'value': 2, 'rule_id': 2}, {'context_features': [['trust', 'full'], ['theme', 'black']], 'value': 3, 'rule_id': 3}, - ], - 'long_setting_name': [ + ]}, + 'long_setting_name': {'rules': [ {'context_features': [['trust', 'none']], 'value': 4, 'rule_id': 4}, - ] + ]} } } if metadata: - for rule in chain.from_iterable(expected['rules'].values()): - rule['metadata'] = {'test': 'yes'} + patch_rule_expectation_with_metadata(expected) assert res.json() == expected @@ -437,102 +371,130 @@ async def test_query_rules_wildcard_some(metadata: bool, app_client, setup_rules @mark.asyncio @mark.parametrize('metadata', [False, True]) async def test_query_rules_wildcard_only(metadata: bool, app_client, setup_rules): - res = await app_client.post('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': {'theme': '*'}, - 'include_metadata': metadata - })) + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'theme:*', + 'include_metadata': str(metadata) + }) expected = { - 'rules': { - 'a': [ + 'settings': { + 'a': {'rules': [ {'context_features': [['theme', 'black']], 'value': 2, 'rule_id': 2}, - ], - 'long_setting_name': [] + ]}, + 'long_setting_name': {'rules': []} } } if metadata: - for rule in chain.from_iterable(expected['rules'].values()): - rule['metadata'] = {'test': 'yes'} + patch_rule_expectation_with_metadata(expected) assert res.json() == expected @mark.asyncio async def test_query_rules_bad_contexts(app_client, setup_rules): - res = await app_client.get('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': {'trust': ['full', 'part'], 'theme': ['black'], 'love': ['overflowing']}, - })) - assert 400 <= res.status_code <= 499 + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'theme:(black),trust:(full,part),love:(overflowing)' + }) + assert res.status_code == 404 @mark.asyncio async def test_query_rules_empty_contexts(app_client, setup_rules): - res = await app_client.get('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': {'trust': []} - })) - assert 400 <= res.status_code <= 499 + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'trust:()' + }) + assert res.status_code == 422 @mark.asyncio async def test_query_rules_bad_settings(app_client, setup_rules): - res = await app_client.get('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'd'], - 'context_features_options': {'trust': ['full', 'part'], 'theme': ['black']}, - })) - assert 400 <= res.status_code <= 499 + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,d', + 'context_filters': 'trust:(full,part),theme:(black)' + }) + assert res.status_code == 404 @mark.asyncio -@mark.parametrize('options', [None, '**', 'wildcard']) +@mark.parametrize('options', ['null', '**', 'wildcard']) async def test_query_rules_bad_options(app_client, setup_rules, options): - res = await app_client.get('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a'], - 'context_features_options': options, - })) + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a', + 'context_filters': options, + }) assert res.status_code == 422, res.content @mark.asyncio -@mark.parametrize('options', [None, '**', 'wildcard']) -async def test_query_rules_bad_inner_option(app_client, setup_rules, options): - res = await app_client.get('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a'], - 'context_features_options': {'trust': options}, - })) - assert res.status_code == 422, res.content +@mark.parametrize('metadata', [False, True]) +async def test_query_rules_nosettings(metadata: bool, app_client, setup_rules): + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': '', + 'include_metadata': str(metadata) + }) + res.raise_for_status() + + expected = {'settings': {}} + + assert res.json() == expected @mark.asyncio @mark.parametrize('metadata', [False, True]) -async def test_query_rules_nosettings(metadata: bool, app_client, setup_rules): - res = await app_client.post('/api/v1/rules/query', data=json.dumps({ - 'setting_names': [], - 'context_features_options': '*', - 'include_metadata': metadata - })) +async def test_query_rules_allsettings_no_filter(metadata: bool, app_client, setup_rules): + res = await app_client.get('/api/v1/rules/query', query_string={ + 'include_metadata': str(metadata) + }) + res.raise_for_status() - expected = {'rules': {}} + expected = {'settings': { + 'a': {'rules': [ + {'context_features': [['trust', 'full']], 'value': 1, 'rule_id': 1}, + {'context_features': [['theme', 'black']], 'value': 2, 'rule_id': 2}, + {'context_features': [['trust', 'full'], ['theme', 'black']], 'value': 3, 'rule_id': 3}, + {'context_features': [['user', 'admin'], ['theme', 'black']], 'value': 7, 'rule_id': 7}, + ]}, + 'b': {'rules': [ + {'context_features': [['trust', 'full']], 'value': 6, 'rule_id': 6}, + ]}, + 'long_setting_name': {'rules': [ + {'context_features': [['trust', 'none']], 'value': 4, 'rule_id': 4}, + {'context_features': [['trust', 'part']], 'value': 5, 'rule_id': 5}, + ]} + }} + + if metadata: + patch_rule_expectation_with_metadata(expected) assert res.json() == expected @mark.asyncio @mark.parametrize('metadata', [False, True]) -async def test_query_rules_bad_cache_future(metadata: bool, app_client, setup_rules, mk_rule): - future_time = datetime.utcnow() + timedelta(hours=2) - # touch a to change its last_touch_time - await mk_rule('a', {'theme': 'grey', 'user': 'admin'}, 8) - - res = await app_client.get('/api/v1/rules/query', data=json.dumps({ - 'setting_names': ['a', 'long_setting_name'], - 'context_features_options': {'trust': ['full', 'part'], 'theme': ['black']}, - 'include_metadata': metadata, - 'cache_time': future_time.isoformat(), - })) - assert res.status_code == 422 +async def test_query_rules_allsettings_with_filter(metadata: bool, app_client, setup_rules): + res = await app_client.get('/api/v1/rules/query', query_string={ + 'context_filters': 'user:*,trust:(full),theme:(blue)', + 'include_metadata': str(metadata) + }) + res.raise_for_status() + + expected = {'settings': { + 'a': {'rules': [ + {'context_features': [['trust', 'full']], 'value': 1, 'rule_id': 1}, + ]}, + 'b': {'rules': [ + {'context_features': [['trust', 'full']], 'value': 6, 'rule_id': 6}, + ]}, + 'long_setting_name': {'rules': []} + }} + + if metadata: + patch_rule_expectation_with_metadata(expected) + + assert res.json() == expected @fixture(params=['deprecated', 'new']) @@ -571,3 +533,74 @@ async def test_patch_rule_missing(patch_callback, app_client): async def test_patch_rule_bad_data(patch_callback, example_rule, app_client): res = await patch_callback(app_client, example_rule, ['d5']) assert res.status_code == 400 + + +@mark.asyncio +@mark.parametrize('metadata', [False, True]) +async def test_query_etag(app_client, setup_rules, metadata): + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'theme:*,trust:(full,none)', + 'include_metadata': str(metadata) + }) + res.raise_for_status() + + etag = res.headers['ETag'] + + repeat_resp = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'theme:*,trust:(full,none)', + 'include_metadata': str(metadata) + }, headers={'If-None-Match': etag}) + + assert repeat_resp.status_code == 304 + assert repeat_resp.headers['ETag'] == etag + assert repeat_resp.content == b'{"detail":"Not Modified"}' + + +@mark.asyncio +@mark.parametrize('metadata', [False, True]) +async def test_query_wrong_etag(app_client, setup_rules, metadata): + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'theme:*,trust:(full,none)', + 'include_metadata': str(metadata) + }) + res.raise_for_status() + + etag = res.headers['ETag'] + wrong_etag = etag[:5] + '%' + etag[5:] + + repeat_resp = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'theme:*,trust:(full,none)', + 'include_metadata': str(metadata) + }, headers={'If-None-Match': wrong_etag}) + + assert repeat_resp.status_code == 200 + assert repeat_resp.headers['ETag'] == etag + assert repeat_resp.content + + +@mark.asyncio +@mark.parametrize('metadata', [False, True]) +async def test_query_etag_wildcard(app_client, setup_rules, metadata): + repeat_resp = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'theme:*,trust:(full,none)', + 'include_metadata': str(metadata) + }, headers={'If-None-Match': '*'}) + + assert repeat_resp.status_code == 304 + assert repeat_resp.content == b'{"detail":"Not Modified"}' + + +@mark.asyncio +@mark.parametrize('metadata', [False, True]) +async def test_query_repeat_filter(app_client, setup_rules, metadata): + res = await app_client.get('/api/v1/rules/query', query_string={ + 'settings': 'a,long_setting_name', + 'context_filters': 'theme:*,trust:(full,none),theme:*', + 'include_metadata': str(metadata) + }) + assert res.status_code == 400 diff --git a/tools/mk_revision.py b/tools/mk_revision.py new file mode 100644 index 0000000..1bc5222 --- /dev/null +++ b/tools/mk_revision.py @@ -0,0 +1,17 @@ +import subprocess +import sys +from os import environ + +from yellowbox import docker_client +from yellowbox.extras.postgresql import PostgreSQLService + +service: PostgreSQLService +with docker_client() as DC, PostgreSQLService.run(DC) as sql_service: + environ['HEKSHER_DB_CONNECTION_STRING'] = sql_service.local_connection_string() + environ['HEKSHER_STARTUP_CONTEXT_FEATURES'] = 'user;trust;theme' + + head_suffix = input('enter head suffix:\n') + message = input('enter revision message:\n') + + subprocess.run([sys.executable, "-m", "alembic", "upgrade", f"head{head_suffix}"]) + subprocess.run([sys.executable, "-m", "alembic", "revision", "--autogenerate", "-m", f'"{message}"'])