From f5f1cd5bf31a9ae86178993fbba8c4026d993881 Mon Sep 17 00:00:00 2001 From: Aaron Holmes Date: Tue, 17 Sep 2024 14:17:37 -0700 Subject: [PATCH] Make feature flag PATCH request work and add test. --- .../web/middleware/feature_flags/__init__.py | 127 +++++++++--------- .../BL_Python/web/middleware/openapi/cors.py | 2 +- .../test_feature_flags_middleware.py | 46 ++++++- 3 files changed, 105 insertions(+), 70 deletions(-) diff --git a/src/web/BL_Python/web/middleware/feature_flags/__init__.py b/src/web/BL_Python/web/middleware/feature_flags/__init__.py index 0bc0e2bb..7a53af93 100644 --- a/src/web/BL_Python/web/middleware/feature_flags/__init__.py +++ b/src/web/BL_Python/web/middleware/feature_flags/__init__.py @@ -1,5 +1,6 @@ +from dataclasses import dataclass from logging import Logger -from typing import Any, Callable, Generic, Sequence, cast +from typing import Any, Callable, Generic, Sequence, TypedDict, cast from BL_Python.platform.feature_flag.caching_feature_flag_router import ( CachingFeatureFlagRouter, @@ -24,8 +25,8 @@ from BL_Python.programming.config import AbstractConfig from BL_Python.programming.patterns.dependency_injection import ConfigurableModule from BL_Python.web.middleware.sso import login_required -from connexion import FlaskApp -from flask import Blueprint, Flask, request +from connexion import FlaskApp, request +from flask import Blueprint, Flask from injector import Binder, Injector, Module, inject, provider, singleton from pydantic import BaseModel from starlette.types import ASGIApp, Receive, Scope, Send @@ -45,6 +46,17 @@ def post_load(self) -> None: feature_flag: FeatureFlagConfig +class FeatureFlagPatchRequest(TypedDict): + name: str + enabled: bool + + +@dataclass +class FeatureFlagPatch: + name: str + enabled: bool + + class FeatureFlagRouterModule(ConfigurableModule, Generic[TFeatureFlag]): def __init__(self, t_feature_flag: type[FeatureFlagRouter[TFeatureFlag]]) -> None: self._t_feature_flag = t_feature_flag @@ -115,9 +127,7 @@ def _login_required(fn: Callable[..., Any]): @_login_required @inject def feature_flag(feature_flag_router: FeatureFlagRouter[FeatureFlag]): # pyright: ignore[reportUnusedFunction] - request_query_names: list[str] | None = request.args.to_dict(flat=False).get( - "name" - ) + request_query_names: list[str] | None = request.query_params.get("name") feature_flags: Sequence[FeatureFlag] missing_flags: set[str] | None = None @@ -146,17 +156,13 @@ def feature_flag(feature_flag_router: FeatureFlagRouter[FeatureFlag]): # pyrigh response["problems"] = problems elif not feature_flags: - problems.append( - # ResponseProblem( - { - "title": "No feature flags found", - "detail": "Queried feature flags do not exist.", - "instance": "", - "status": 404, - "type": None, - } - # ) - ) + problems.append({ + "title": "No feature flags found", + "detail": "Queried feature flags do not exist.", + "instance": "", + "status": 404, + "type": None, + }) response["problems"] = problems if feature_flags: @@ -165,57 +171,48 @@ def feature_flag(feature_flag_router: FeatureFlagRouter[FeatureFlag]): # pyrigh else: return response, 404 - # - # - ## @server_blueprint.route("/server/feature_flag", methods=("PATCH",)) + @feature_flag_blueprint.route("/feature_flag", methods=("PATCH",)) # pyright: ignore[reportArgumentType,reportUntypedFunctionDecorator] # @login_required([UserRole.Operator]) - # @inject - # def feature_flag_patch(feature_flag_router: FeatureFlagRouter[DBFeatureFlag]): - # post_request_feature_flag_schema = PatchRequestFeatureFlagSchema() - # - # feature_flags: list[PatchRequestFeatureFlag] = cast( - # list[PatchRequestFeatureFlag], - # post_request_feature_flag_schema.load( - # flask.request.json, # pyright: ignore[reportArgumentType] why is `flask.request.json` wrong here? - # many=True, - # ), - # ) - # - # changes: list[FeatureFlagChange] = [] - # problems: list[ResponseProblem] = [] - # for flag in feature_flags: - # try: - # change = feature_flag_router.set_feature_is_enabled(flag.name, flag.enabled) - # changes.append(change) - # except LookupError: - # problems.append( - # ResponseProblem( - # title=_FEATURE_FLAG_NOT_FOUND_PROBLEM_TITLE, - # detail="Feature flag to PATCH does not exist. It must be created first.", - # instance=flag.name, - # status=_FEATURE_FLAG_NOT_FOUND_PROBLEM_STATUS, - # type=None, - # ) - # ) - # - # response: dict[str, Any] = {} - # - # if problems: - # response["problems"] = ResponseProblemSchema().dump(problems, many=True) - # - # if changes: - # response["data"] = PatchResponseFeatureFlagSchema().dump(changes, many=True) - # return response - # else: - # return response, _FEATURE_FLAG_NOT_FOUND_PROBLEM_STATUS - # - return feature_flag_blueprint + # TODO assign a specific role ? + @_login_required + @inject + async def feature_flag_patch(feature_flag_router: FeatureFlagRouter[FeatureFlag]): # pyright: ignore[reportUnusedFunction] + feature_flags_request: list[FeatureFlagPatchRequest] = await request.json() + feature_flags = [ + FeatureFlagPatch(name=flag["name"], enabled=flag["enabled"]) + for flag in feature_flags_request + ] -# class FeatureFlagModule(Module): -# def __init__(self): -# """ """ -# super().__init__() + changes: list[Any] = [] + problems: list[Any] = [] + for flag in feature_flags: + try: + change = feature_flag_router.set_feature_is_enabled( + flag.name, flag.enabled + ) + changes.append(change) + except LookupError: + problems.append({ + "title": "feature flag not found", + "detail": "Feature flag to PATCH does not exist. It must be created first.", + "instance": flag.name, + "status": 404, + "type": None, + }) + + response: dict[str, Any] = {} + + if problems: + response["problems"] = problems + + if changes: + response["data"] = changes + return response + else: + return response, 404 + + return feature_flag_blueprint class FeatureFlagMiddlewareModule(Module): diff --git a/src/web/BL_Python/web/middleware/openapi/cors.py b/src/web/BL_Python/web/middleware/openapi/cors.py index d1ace0c2..621591f4 100644 --- a/src/web/BL_Python/web/middleware/openapi/cors.py +++ b/src/web/BL_Python/web/middleware/openapi/cors.py @@ -13,7 +13,7 @@ def register_middleware(self, app: FlaskApp, config: Config): app.add_middleware( CORSMiddleware, position=MiddlewarePosition.BEFORE_EXCEPTION, - allow_origins=cors_config.origins, + allow_origins=cors_config.origins or [], allow_credentials=cors_config.allow_credentials, allow_methods=cors_config.allow_methods, allow_headers=cors_config.allow_headers, diff --git a/src/web/test/unit/middleware/test_feature_flags_middleware.py b/src/web/test/unit/middleware/test_feature_flags_middleware.py index 528b9986..3fe3f785 100644 --- a/src/web/test/unit/middleware/test_feature_flags_middleware.py +++ b/src/web/test/unit/middleware/test_feature_flags_middleware.py @@ -4,10 +4,7 @@ import pytest from BL_Python.platform.dependency_injection import UserLoaderModule -from BL_Python.platform.feature_flag import FeatureFlag, caching_feature_flag_router -from BL_Python.platform.feature_flag.caching_feature_flag_router import ( - CachingFeatureFlagRouter, -) +from BL_Python.platform.feature_flag import FeatureFlag from BL_Python.platform.feature_flag.feature_flag_router import FeatureFlagRouter from BL_Python.platform.identity.user_loader import Role as LoaderRole from BL_Python.programming.config import AbstractConfig @@ -226,6 +223,7 @@ def test__FeatureFlagMiddleware__feature_flag_api_GET_returns_no_feature_flags_w assert (title := problems[0].get("title", None)) is not None assert title == "No feature flags found" + # TODO need test for querying for specific flag def test__FeatureFlagMiddleware__feature_flag_api_GET_returns_feature_flags_when_they_exist( self, openapi_config: Config, @@ -261,3 +259,43 @@ def client_init_hook(app: CreateAppResult[FlaskApp]): assert len(data) == 1 assert data[0].get("enabled", None) is True assert data[0].get("name", None) == "foo_feature" + + def test__FeatureFlagMiddleware__feature_flag_api_PATCH_modifies_something( + self, + openapi_config: Config, + openapi_client_configurable: OpenAPIClientInjectorConfigurable, + openapi_mock_controller: OpenAPIMockController, + mocker: MockerFixture, + ): + def client_init_hook(app: CreateAppResult[FlaskApp]): + caching_feature_flag_router = app.app_injector.flask_injector.injector.get( + FeatureFlagRouter[FeatureFlag] + ) + _ = caching_feature_flag_router.set_feature_is_enabled("foo_feature", True) + + openapi_mock_controller.begin() + app = next( + openapi_client_configurable( + openapi_config, + client_init_hook, + self._user_session_app_init_hook, + ) + ) + + with self.get_authenticated_request_context( + app, + User, # pyright: ignore[reportArgumentType] + mocker, + ): + response = app.client.patch( + "/server/feature_flag", + json=[{"name": "foo_feature", "enabled": False}], + ) + + assert response.status_code == 200 + response_json = response.json() + assert (data := response_json.get("data", None)) is not None + assert len(data) == 1 + assert data[0].get("name", None) == "foo_feature" + assert data[0].get("new_value", None) == False + assert data[0].get("old_value", None) == True