Feature flag support (#787)

spiffworkflow-backend/migrations/versions/1b5a9f7af28e_.py

@@ -0,0 +1,42 @@
+"""empty message
+
+Revision ID: 1b5a9f7af28e
+Revises: d8901960326e
+Create Date: 2023-12-05 09:46:36.417973
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '1b5a9f7af28e'
+down_revision = 'd8901960326e'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.create_table('feature_flag',
+    sa.Column('id', sa.Integer(), nullable=False),
+    sa.Column('generation_id', sa.Integer(), nullable=False),
+    sa.Column('value', sa.JSON(), nullable=False),
+    sa.Column('updated_at_in_seconds', sa.Integer(), nullable=False),
+    sa.Column('created_at_in_seconds', sa.Integer(), nullable=False),
+    sa.ForeignKeyConstraint(['generation_id'], ['cache_generation.id'], ),
+    sa.PrimaryKeyConstraint('id')
+    )
+    with op.batch_alter_table('feature_flag', schema=None) as batch_op:
+        batch_op.create_index(batch_op.f('ix_feature_flag_generation_id'), ['generation_id'], unique=True)
+
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    with op.batch_alter_table('feature_flag', schema=None) as batch_op:
+        batch_op.drop_index(batch_op.f('ix_feature_flag_generation_id'))
+
+    op.drop_table('feature_flag')
+    # ### end Alembic commands ###

spiffworkflow-backend/src/spiffworkflow_backend/config/default.py

@@ -153,7 +153,6 @@ config_from_env("SPIFFWORKFLOW_BACKEND_ENGINE_STEP_DEFAULT_STRATEGY_WEB", defaul
 
 ### element units
 # disabling until we fix the "no such directory" error so we do not keep sending cypress errors
-config_from_env("SPIFFWORKFLOW_BACKEND_FEATURE_ELEMENT_UNITS_ENABLED", default=False)
 config_from_env("SPIFFWORKFLOW_BACKEND_ELEMENT_UNITS_CACHE_DIR", default="src/instance/element-unit-cache")
 
 ### cryptography (to encrypt) or no_op_cipher (to not encrypt)

spiffworkflow-backend/src/spiffworkflow_backend/load_database_models.py

@@ -106,5 +106,8 @@ from spiffworkflow_backend.models.user_property import (
 from spiffworkflow_backend.models.service_account import (
     ServiceAccountModel,
 )  # noqa: F401
+from spiffworkflow_backend.models.feature_flag import (
+    FeatureFlagModel,
+)  # noqa: F401
 
 add_listeners()

spiffworkflow-backend/src/spiffworkflow_backend/models/cache_generation.py

@@ -11,6 +11,7 @@ from spiffworkflow_backend.models.db import db
 
 class CacheGenerationTable(SpiffEnum):
     reference_cache = "reference_cache"
+    feature_flag = "feature_flag"
 
 
 class CacheGenerationModel(SpiffworkflowBaseDBModel):

spiffworkflow-backend/src/spiffworkflow_backend/models/feature_flag.py

@@ -0,0 +1,39 @@
+from dataclasses import dataclass
+from typing import Any
+
+from sqlalchemy import ForeignKey
+from sqlalchemy.orm import relationship
+
+from spiffworkflow_backend.models.cache_generation import CacheGenerationModel
+from spiffworkflow_backend.models.db import SpiffworkflowBaseDBModel
+from spiffworkflow_backend.models.db import db
+
+
+@dataclass
+class FeatureFlagModel(SpiffworkflowBaseDBModel):
+    __tablename__ = "feature_flag"
+
+    id: int = db.Column(db.Integer, primary_key=True)
+    generation_id: int = db.Column(ForeignKey(CacheGenerationModel.id), nullable=False, unique=True, index=True)  # type: ignore
+    value: dict = db.Column(db.JSON, nullable=False)
+    updated_at_in_seconds: int = db.Column(db.Integer, nullable=False)
+    created_at_in_seconds: int = db.Column(db.Integer, nullable=False)
+
+    generation = relationship(CacheGenerationModel)
+
+    @classmethod
+    def most_recent_feature_flags(cls) -> dict[str, Any]:
+        cache_generation = CacheGenerationModel.newest_generation_for_table(cls.__tablename__)
+        if cache_generation is None:
+            return {}
+
+        result = cls.query.filter_by(generation_id=cache_generation.id).first()
+        return {} if result is None else result.value  # type: ignore
+
+    @classmethod
+    def set_most_recent_feature_flags(cls, value: dict[str, Any]) -> None:
+        cache_generation = CacheGenerationModel(cache_table=cls.__tablename__)
+        feature_flags = FeatureFlagModel(generation=cache_generation, value=value)
+        db.session.add(cache_generation)
+        db.session.add(feature_flags)
+        db.session.commit()

spiffworkflow-backend/src/spiffworkflow_backend/scripts/set_feature_flags.py

@@ -0,0 +1,15 @@
+from typing import Any
+
+from spiffworkflow_backend.models.script_attributes_context import ScriptAttributesContext
+from spiffworkflow_backend.scripts.script import Script
+from spiffworkflow_backend.services.feature_flag_service import FeatureFlagService
+
+
+class SetFeatureFlags(Script):
+    def get_description(self) -> str:
+        return """Allows updating default and process model specific feature flags."""
+
+    def run(self, script_attributes_context: ScriptAttributesContext, *args: Any, **kwargs: Any) -> Any:
+        default_feature_flags = args[0]
+        process_model_overrides = args[1]
+        return FeatureFlagService.set_feature_flags(default_feature_flags, process_model_overrides)

spiffworkflow-backend/src/spiffworkflow_backend/services/element_units_service.py

@@ -3,6 +3,8 @@ from typing import Any
 
 from flask import current_app
 
+from spiffworkflow_backend.services.feature_flag_service import FeatureFlagService
+
 BpmnSpecDict = dict[str, Any]
 
 
@@ -15,7 +17,7 @@ class ElementUnitsService:
 
     @classmethod
     def _enabled(cls) -> bool:
-        enabled = current_app.config["SPIFFWORKFLOW_BACKEND_FEATURE_ELEMENT_UNITS_ENABLED"]
+        enabled = FeatureFlagService.feature_enabled("element_units", False)
         return enabled and cls._cache_dir() is not None
 
     @classmethod

spiffworkflow-backend/src/spiffworkflow_backend/services/feature_flag_service.py

@@ -0,0 +1,37 @@
+from typing import Any
+
+from flask import current_app
+from flask import g
+
+from spiffworkflow_backend.models.feature_flag import FeatureFlagModel
+
+DEFAULT_FEATURE_FLAGS = "__spiff_default_feature_flags"
+
+
+class FeatureFlagService:
+    @classmethod
+    def _process_model_specific_value(cls, name: str, feature_flags: dict[str, Any]) -> bool | None:
+        tld = current_app.config.get("THREAD_LOCAL_DATA")
+
+        if not tld or not hasattr(tld, "process_model_identifier"):
+            return None
+
+        return feature_flags.get(tld.process_model_identifier, {}).get(name)  # type: ignore
+
+    @classmethod
+    def feature_enabled(cls, name: str, enabled_by_default: bool = False) -> bool:
+        if not hasattr(g, "feature_flags"):
+            g.feature_flags = FeatureFlagModel.most_recent_feature_flags()
+
+        value = cls._process_model_specific_value(name, g.feature_flags)
+        if value is not None:
+            return value
+
+        return g.feature_flags.get(DEFAULT_FEATURE_FLAGS, {}).get(name, enabled_by_default)  # type: ignore
+
+    @classmethod
+    def set_feature_flags(cls, default_feature_flags: dict[str, Any], process_model_overrides: dict[str, Any]) -> None:
+        feature_flags = {}
+        feature_flags.update(process_model_overrides)
+        feature_flags[DEFAULT_FEATURE_FLAGS] = default_feature_flags
+        FeatureFlagModel.set_most_recent_feature_flags(feature_flags)

spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_element_units_service.py

@@ -5,8 +5,11 @@ from collections.abc import Generator
 
 import pytest
 from flask.app import Flask
+from spiffworkflow_backend.models.db import db
+from spiffworkflow_backend.models.feature_flag import FeatureFlagModel
 from spiffworkflow_backend.services.element_units_service import BpmnSpecDict
 from spiffworkflow_backend.services.element_units_service import ElementUnitsService
+from spiffworkflow_backend.services.feature_flag_service import FeatureFlagService
 
 from tests.spiffworkflow_backend.helpers.base_test import BaseTest
 
@@ -17,6 +20,22 @@ from tests.spiffworkflow_backend.helpers.base_test import BaseTest
 #
 
 
+@pytest.fixture()
+def feature_enabled(app: Flask, with_db_and_bpmn_file_cleanup: None) -> Generator[None, None, None]:
+    db.session.query(FeatureFlagModel).delete()
+    db.session.commit()
+    FeatureFlagService.set_feature_flags({"element_units": True}, {})
+    yield
+
+
+@pytest.fixture()
+def feature_disabled(app: Flask, with_db_and_bpmn_file_cleanup: None) -> Generator[None, None, None]:
+    db.session.query(FeatureFlagModel).delete()
+    db.session.commit()
+    FeatureFlagService.set_feature_flags({"element_units": False}, {})
+    yield
+
+
 @pytest.fixture()
 def app_no_cache_dir(app: Flask) -> Generator[Flask, None, None]:
     with BaseTest().app_config_mock(app, "SPIFFWORKFLOW_BACKEND_ELEMENT_UNITS_CACHE_DIR", None):
@@ -30,22 +49,10 @@ def app_some_cache_dir(app: Flask) -> Generator[Flask, None, None]:
 
 
 @pytest.fixture()
-def app_disabled(app: Flask) -> Generator[Flask, None, None]:
+def app_tmp_cache_dir(app: Flask) -> Generator[Flask, None, None]:
-    with BaseTest().app_config_mock(app, "SPIFFWORKFLOW_BACKEND_FEATURE_ELEMENT_UNITS_ENABLED", False):
-        yield app
-
-
-@pytest.fixture()
-def app_enabled(app_some_cache_dir: Flask) -> Generator[Flask, None, None]:
-    with BaseTest().app_config_mock(app_some_cache_dir, "SPIFFWORKFLOW_BACKEND_FEATURE_ELEMENT_UNITS_ENABLED", True):
-        yield app_some_cache_dir
-
-
-@pytest.fixture()
-def app_enabled_tmp_cache_dir(app_enabled: Flask) -> Generator[Flask, None, None]:
     with tempfile.TemporaryDirectory() as tmpdirname:
-        with BaseTest().app_config_mock(app_enabled, "SPIFFWORKFLOW_BACKEND_ELEMENT_UNITS_CACHE_DIR", tmpdirname):
+        with BaseTest().app_config_mock(app, "SPIFFWORKFLOW_BACKEND_ELEMENT_UNITS_CACHE_DIR", tmpdirname):
-            yield app_enabled
+            yield app
 
 
 @pytest.fixture()
@@ -64,15 +71,15 @@ class TestElementUnitsService(BaseTest):
     ) -> None:
         assert ElementUnitsService._cache_dir() == "some_cache_dir"
 
-    def test_feature_disabled_if_env_is_false(
+    def test_feature_disabled_if_feature_flag_is_false(
         self,
-        app_disabled: Flask,
+        feature_disabled: None,
     ) -> None:
         assert not ElementUnitsService._enabled()
 
     def test_feature_enabled_if_env_is_true(
         self,
-        app_enabled: Flask,
+        feature_enabled: None,
     ) -> None:
         assert ElementUnitsService._enabled()
 
@@ -84,21 +91,22 @@ class TestElementUnitsService(BaseTest):
 
     def test_ok_to_cache_when_disabled(
         self,
-        app_disabled: Flask,
+        feature_disabled: None,
     ) -> None:
         result = ElementUnitsService.cache_element_units_for_workflow("", {})
         assert result is None
 
     def test_ok_to_read_workflow_from_cached_element_unit_when_disabled(
         self,
-        app_disabled: Flask,
+        feature_disabled: None,
     ) -> None:
         result = ElementUnitsService.workflow_from_cached_element_unit("", "", "")
         assert result is None
 
     def test_can_write_to_cache(
         self,
-        app_enabled_tmp_cache_dir: Flask,
+        app_tmp_cache_dir: Flask,
+        feature_enabled: None,
         example_specs_dict: BpmnSpecDict,
     ) -> None:
         result = ElementUnitsService.cache_element_units_for_workflow("testing", example_specs_dict)
@@ -106,7 +114,8 @@ class TestElementUnitsService(BaseTest):
 
     def test_can_write_to_cache_multiple_times(
         self,
-        app_enabled_tmp_cache_dir: Flask,
+        app_tmp_cache_dir: Flask,
+        feature_enabled: None,
         example_specs_dict: BpmnSpecDict,
     ) -> None:
         result = ElementUnitsService.cache_element_units_for_workflow("testing", example_specs_dict)
@@ -118,7 +127,8 @@ class TestElementUnitsService(BaseTest):
 
     def test_can_read_element_unit_for_process_from_cache(
         self,
-        app_enabled_tmp_cache_dir: Flask,
+        app_tmp_cache_dir: Flask,
+        feature_enabled: None,
         example_specs_dict: BpmnSpecDict,
     ) -> None:
         ElementUnitsService.cache_element_units_for_workflow("testing", example_specs_dict)
@@ -127,7 +137,8 @@ class TestElementUnitsService(BaseTest):
 
     def test_reading_element_unit_for_uncached_process_returns_none(
         self,
-        app_enabled_tmp_cache_dir: Flask,
+        app_tmp_cache_dir: Flask,
+        feature_enabled: None,
     ) -> None:
         cached_specs_dict = ElementUnitsService.workflow_from_cached_element_unit("testing", "no_tasks", "")
         assert cached_specs_dict is None

spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_feature_flag_service.py

@@ -0,0 +1,66 @@
+from collections.abc import Generator
+
+import pytest
+from flask.app import Flask
+from spiffworkflow_backend.services.feature_flag_service import FeatureFlagService
+
+from tests.spiffworkflow_backend.helpers.base_test import BaseTest
+
+
+@pytest.fixture()
+def no_feature_flags(app: Flask, with_db_and_bpmn_file_cleanup: None) -> Generator[None, None, None]:
+    yield
+
+
+class TestFeatureFlagService(BaseTest):
+    """Tests the FeatureFlagService."""
+
+    def test_default_enabled_is_respected_when_no_feature_flag_exists(
+        self,
+        no_feature_flags: None,
+    ) -> None:
+        assert FeatureFlagService.feature_enabled("some_feature", True)
+        assert not FeatureFlagService.feature_enabled("another_feature", False)
+
+    def test_default_feature_flag_value_overrides_passed_in_default_enabled(
+        self,
+        no_feature_flags: None,
+    ) -> None:
+        FeatureFlagService.set_feature_flags({"some_feature": False}, {})
+        assert not FeatureFlagService.feature_enabled("some_feature", True)
+
+    def test_process_model_override_feature_flag_value_overrides_passed_in_default_enabled(
+        self,
+        app: Flask,
+        no_feature_flags: None,
+    ) -> None:
+        app.config.get("THREAD_LOCAL_DATA").process_model_identifier = "a/b/c"  # type: ignore
+        FeatureFlagService.set_feature_flags(
+            {},
+            {"a/b/c": {"some_feature": False}},
+        )
+        assert not FeatureFlagService.feature_enabled("some_feature", True)
+
+    def test_process_model_override_feature_flag_value_overrides_default_feature_flag_value(
+        self,
+        app: Flask,
+        no_feature_flags: None,
+    ) -> None:
+        app.config.get("THREAD_LOCAL_DATA").process_model_identifier = "a/b/c"  # type: ignore
+        FeatureFlagService.set_feature_flags(
+            {"some_feature": True},
+            {"a/b/c": {"some_feature": False}},
+        )
+        assert not FeatureFlagService.feature_enabled("some_feature", True)
+
+    def test_does_not_consider_other_features(
+        self,
+        app: Flask,
+        no_feature_flags: None,
+    ) -> None:
+        app.config.get("THREAD_LOCAL_DATA").process_model_identifier = "a/b/c"  # type: ignore
+        FeatureFlagService.set_feature_flags(
+            {"one_feature": False},
+            {"a/b/c": {"two_feature": False}},
+        )
+        assert FeatureFlagService.feature_enabled("some_feature", True)