diff --git a/phileas/policy/policy.py b/phileas/policy/policy.py index c1bdf01..29fa5ec 100644 --- a/phileas/policy/policy.py +++ b/phileas/policy/policy.py @@ -75,8 +75,12 @@ def __init__( ignored_patterns: List[str] | None = None, generators: dict | None = None, config: dict | None = None, + metadata: dict | None = None, ) -> None: self.name = name + #: Raw ``metadata`` object, kept as-is so keys the engine does not know + #: about survive a load-and-save round trip. Never read during filtering. + self.metadata: dict | None = metadata #: Raw Phileas-JSON ``identifiers`` object (entity field -> filter node). self.identifiers: dict = identifiers if identifiers is not None else {} #: Flat list of policy-level ignored terms. @@ -99,6 +103,7 @@ def from_dict(cls, data: dict) -> "Policy": ignored_patterns=_parse_ignored_patterns(data.get("ignoredPatterns", [])), generators=data.get("generators", {}) or {}, config=data.get("config", {}) or {}, + metadata=data.get("metadata"), ) def _analysis_flag(self, name: str) -> bool: @@ -129,7 +134,7 @@ def from_yaml(cls, yaml_str: str) -> "Policy": return cls.from_dict(yaml.safe_load(yaml_str)) def to_dict(self) -> dict: - return { + data = { "name": self.name, "identifiers": self.identifiers, "ignored": [{"terms": list(self.ignored)}] if self.ignored else [], @@ -137,6 +142,9 @@ def to_dict(self) -> dict: **({"generators": self.generators} if self.generators else {}), **({"config": self.config} if self.config else {}), } + if self.metadata is not None: + data["metadata"] = self.metadata + return data def to_json(self) -> str: return json.dumps(self.to_dict(), indent=2) diff --git a/phileas/services/filter_service.py b/phileas/services/filter_service.py index a759f16..e44678a 100644 --- a/phileas/services/filter_service.py +++ b/phileas/services/filter_service.py @@ -95,6 +95,20 @@ ] +def _strategies_node(node: dict, entity) -> list: + """Reads a filter's strategies array by its catalog field name, falling back to + the catalog's aliases. An alias is a name the field had in an earlier schema, so + a policy written against that schema keeps working.""" + strategies = node.get(entity.phileas_strategies_field) + if strategies is not None: + return strategies + for alias in getattr(entity, "phileas_strategies_field_aliases", ()) or (): + strategies = node.get(alias) + if strategies is not None: + return strategies + return [] + + class FilterService: def __init__(self, context_service: AbstractContextService | None = None) -> None: self._context_service = ( @@ -115,9 +129,7 @@ def filter(self, policy: Policy, context: str, document_id: str, text: str) -> F node = identifiers.get(entity.phileas_field) if not isinstance(node, dict) or node.get("enabled", True) is False: continue - strategies = self._strategies( - node.get(entity.phileas_strategies_field, []), policy - ) + strategies = self._strategies(_strategies_node(node, entity), policy) detected = filter_cls(node).detect(text, context) spans.extend(self._apply_strategies(detected, strategies, node, context)) diff --git a/tests/test_catalog.py b/tests/test_catalog.py index a1e8d90..af2ce55 100644 --- a/tests/test_catalog.py +++ b/tests/test_catalog.py @@ -17,6 +17,12 @@ from phileas.catalog import PhileasCatalog, get_catalog +def _readable_strategies_fields(entity) -> set: + """Every strategies-array name an engine must read for an entity: the catalog's + primary name plus any aliases it records for earlier names.""" + return {entity.phileas_strategies_field, *getattr(entity, "phileas_strategies_field_aliases", ())} + + class TestCatalogBridge: def test_singleton(self): assert get_catalog() is get_catalog() @@ -38,8 +44,9 @@ def test_entity_lookup_case_insensitive(self): def test_quirky_field_names_come_from_catalog(self): """The engine must use the catalog's non-obvious field names.""" cat = get_catalog() - # zip code uses a *singular* strategies field name. - assert cat.get_entity("ZIP_CODE").phileas_strategies_field == "zipCodeFilterStrategy" + # The zip code strategies field was renamed to the plural; the singular has to stay + # readable either as the primary name or as an alias, depending on the catalog version. + assert "zipCodeFilterStrategy" in _readable_strategies_fields(cat.get_entity("ZIP_CODE")) # bitcoin uses an abbreviated strategies field name. assert cat.get_entity("BITCOIN_ADDRESS").phileas_strategies_field == "bitcoinFilterStrategies" diff --git a/tests/test_catalog_extra.py b/tests/test_catalog_extra.py index eef4cda..775404b 100644 --- a/tests/test_catalog_extra.py +++ b/tests/test_catalog_extra.py @@ -92,10 +92,14 @@ def test_entity_has_strategies_field(self, name): def test_entity_name_matches_lookup(self, name): assert get_catalog().get_entity(name).name == name - def test_zip_code_quirk_singular_strategies_field(self): - # ZIP_CODE uses a *singular* strategies field name. + def test_zip_code_singular_strategies_field_stays_readable(self): + # ZIP_CODE used a singular strategies field name until schema 1.3.0 renamed it to + # the plural. Whichever the catalog calls primary, the singular must stay readable + # so a policy written against an earlier schema keeps working. entity = get_catalog().get_entity("ZIP_CODE") - assert entity.phileas_strategies_field == "zipCodeFilterStrategy" + readable = {entity.phileas_strategies_field, + *getattr(entity, "phileas_strategies_field_aliases", ())} + assert "zipCodeFilterStrategy" in readable def test_bitcoin_quirk_abbreviated_strategies_field(self): # BITCOIN_ADDRESS uses an abbreviated strategies field name. diff --git a/tests/test_integration_phisql.py b/tests/test_integration_phisql.py index c4b28d9..ae1bf70 100644 --- a/tests/test_integration_phisql.py +++ b/tests/test_integration_phisql.py @@ -22,6 +22,7 @@ from phileas.policy.policy import Policy from phileas.services.filter_service import FilterService +from phileas.catalog import get_catalog def compile_and_run(source, text, context="ctx"): @@ -83,11 +84,13 @@ def test_static_replace(self): out = compile_and_run(src, "Mail a@b.com here.").filtered_text assert "[EMAIL]" in out - def test_zip_code_quirky_field_round_trips(self): - # PhiSQL emits the singular zipCodeFilterStrategy; phileas must read it. + def test_zip_code_field_round_trips(self): + # PhiSQL emits the catalog's primary strategies field name, whichever it is; + # phileas must read what PhiSQL emits. src = "POLICY p; REDACT ZIP_CODE WITH REDACT;" policy_json = Compiler().compile(src).policy_json() - assert "zipCodeFilterStrategy" in policy_json["identifiers"]["zipCode"] + primary = get_catalog().get_entity("ZIP_CODE").phileas_strategies_field + assert primary in policy_json["identifiers"]["zipCode"] out = compile_and_run(src, "ZIP 90210 here.").filtered_text assert "90210" not in out diff --git a/tests/test_policy.py b/tests/test_policy.py index b0815bf..5c27149 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -99,3 +99,19 @@ def test_to_dict_reemits_ignored_objects(self): def test_yaml_round_trip_ignored_patterns(self): p = Policy.from_dict({"identifiers": {}, "ignoredPatterns": [{"pattern": r"\d+"}]}) assert Policy.from_yaml(p.to_yaml()).ignored_patterns == [r"\d+"] + + def test_metadata_round_trips(self): + # Keys beyond description are allowed by the schema, so they have to survive too. + metadata = { + "description": "Client intake forms.", + "author": "records team", + "labels": ["intake", "pii"], + } + p = Policy.from_dict({"metadata": metadata, "identifiers": {}}) + assert p.metadata == metadata + assert Policy.from_json(p.to_json()).metadata == metadata + + def test_policy_without_metadata_omits_it(self): + p = Policy.from_dict({"identifiers": {}}) + assert p.metadata is None + assert "metadata" not in p.to_dict() diff --git a/tests/test_service.py b/tests/test_service.py index 62bc1ec..3db2d82 100644 --- a/tests/test_service.py +++ b/tests/test_service.py @@ -14,6 +14,7 @@ """End-to-end tests for the catalog-driven FilterService.""" +from phileas.catalog import get_catalog from phileas.policy.policy import Policy from phileas.services.context.in_memory_context_service import InMemoryContextService from phileas.services.filter_service import FilterService @@ -49,8 +50,21 @@ def test_disabled_node_skipped(self): class TestCatalogDrivenFieldNames: - def test_zip_code_singular_strategy_field(self): - # The catalog says the field is `zipCodeFilterStrategy` (singular). + def test_zip_code_strategy_field(self): + # Every name the catalog says is readable must actually drive the filter: the + # primary name, and any alias recording what the field was called earlier. + entity = get_catalog().get_entity("ZIP_CODE") + names = {entity.phileas_strategies_field, + *getattr(entity, "phileas_strategies_field_aliases", ())} + for name in names: + r = run({"zipCode": {name: [{"strategy": "STATIC_REPLACE", "staticReplacement": "ZZZZZ"}]}}, + "ZIP 90210 here.") + assert "ZZZZZ" in r.filtered_text, name + assert "90210" not in r.filtered_text, name + + def test_zip_code_singular_strategy_field_still_read(self): + # The field was singular until schema 1.3.0. A policy written against an + # earlier schema must keep redacting rather than silently losing its strategies. r = run({"zipCode": {"zipCodeFilterStrategy": [{"strategy": "STATIC_REPLACE", "staticReplacement": "ZZZZZ"}]}}, "ZIP 90210 here.") assert "ZZZZZ" in r.filtered_text