diff --git a/src/sentry/issues/derived/features.py b/src/sentry/issues/derived/features.py index 2371eebd3781..00fa9fb7ed81 100644 --- a/src/sentry/issues/derived/features.py +++ b/src/sentry/issues/derived/features.py @@ -1,7 +1,7 @@ from datetime import datetime from enum import StrEnum -from sentry.issues.derived.framework import EnumCodec, Feature, OptionalCodec +from sentry.issues.derived.framework import DateTimeCodec, EnumCodec, Feature, OptionalCodec from sentry.issues.progress_state import IssueProgressState @@ -24,4 +24,6 @@ class IssueStatus(StrEnum): ) # The last time the progress was advanced. -LAST_PROGRESSED_AT = Feature[datetime | None]("last_progressed_at", default=None) +LAST_PROGRESSED_AT = Feature[datetime | None]( + "last_progressed_at", default=None, codec=OptionalCodec(DateTimeCodec()) +) diff --git a/src/sentry/issues/derived/framework.py b/src/sentry/issues/derived/framework.py index b499730e7a87..d98773e4c64f 100644 --- a/src/sentry/issues/derived/framework.py +++ b/src/sentry/issues/derived/framework.py @@ -6,6 +6,7 @@ import copy from collections.abc import Callable, Iterable, Iterator from dataclasses import dataclass +from datetime import datetime from enum import IntEnum, StrEnum from typing import Any, Protocol, runtime_checkable @@ -18,12 +19,28 @@ class Codec[T]: - """Handles serialization of a Feature value to/from JSON-compatible form.""" + """Converts a Feature's Python value to/from storage representations. - def dump(self, value: T) -> Any: + Two pairs of methods handle different storage backends: + + * ``to_json`` / ``from_json`` — for the ``data`` JSONField blob. + * ``to_column`` / ``from_column`` — for dedicated Django model columns. + + The default implementation is identity for both pairs. Override as + needed (e.g. ``EnumCodec`` wraps raw strings back into enum members + on ``from_column`` so that column-loaded values are real enum instances). + """ + + def to_json(self, value: T) -> Any: + return value + + def from_json(self, raw: Any) -> T: + return raw + + def to_column(self, value: T) -> Any: return value - def load(self, raw: Any) -> T: + def from_column(self, raw: Any) -> T: return raw @@ -34,22 +51,42 @@ class EnumCodec[E: StrEnum](Codec[E]): def __init__(self, enum_cls: type[E]) -> None: self._enum_cls = enum_cls - def load(self, raw: Any) -> E: + def to_json(self, value: E) -> str: + return value.value + + def from_json(self, raw: Any) -> E: return self._enum_cls(raw) - def dump(self, value: E) -> str: + def to_column(self, value: E) -> str: return value.value + def from_column(self, raw: Any) -> E: + return self._enum_cls(raw) + + +class DateTimeCodec(Codec[datetime]): + def to_json(self, value: datetime) -> str: + return value.isoformat() + + def from_json(self, raw: Any) -> datetime: + return datetime.fromisoformat(raw) + class OptionalCodec[T](Codec[T | None]): def __init__(self, inner: Codec[T]) -> None: self._inner = inner - def load(self, raw: Any) -> T | None: - return self._inner.load(raw) if raw is not None else None + def to_json(self, value: T | None) -> Any: + return self._inner.to_json(value) if value is not None else None - def dump(self, value: T | None) -> Any: - return self._inner.dump(value) if value is not None else None + def from_json(self, raw: Any) -> T | None: + return self._inner.from_json(raw) if raw is not None else None + + def to_column(self, value: T | None) -> Any: + return self._inner.to_column(value) if value is not None else None + + def from_column(self, raw: Any) -> T | None: + return self._inner.from_column(raw) if raw is not None else None # --------------------------------------------------------------------------- @@ -62,8 +99,9 @@ def dump(self, value: T | None) -> Any: class Feature[T]: """A named, typed slot in derived state with a default value. - The `codec` handles conversion to/from JSON-compatible representations. - Defaults to identity (pass-through) for JSON-native types. + The ``codec`` handles conversion to/from storage representations. + JSON-blob features use ``to_json`` / ``from_json``; column-backed + features use ``to_column`` / ``from_column``. """ def __init__( @@ -87,11 +125,17 @@ def initial_value(self) -> T: return self._default_factory() return self._default - def dump(self, value: T) -> Any: - return self._codec.dump(value) + def to_json(self, value: T) -> Any: + return self._codec.to_json(value) + + def from_json(self, raw: Any) -> T: + return self._codec.from_json(raw) + + def to_column(self, value: T) -> Any: + return self._codec.to_column(value) - def load(self, raw: Any) -> T: - return self._codec.load(raw) + def from_column(self, raw: Any) -> T: + return self._codec.from_column(raw) def value(self, val: T) -> FeatureEntry: return (self, val) diff --git a/src/sentry/issues/derived/store.py b/src/sentry/issues/derived/store.py index e639d7c45203..1e585a43533b 100644 --- a/src/sentry/issues/derived/store.py +++ b/src/sentry/issues/derived/store.py @@ -27,7 +27,9 @@ class GroupDerivedDataStore: """Translates between Pipeline State and GroupDerivedData storage. Features listed in COLUMN_MAP are read from / written to dedicated - model columns. All other features use the `data` JSON blob. + model columns via the Feature's ``from_column`` / ``to_column`` codec + methods. All other features use the ``data`` JSON blob via + ``from_json`` / ``to_json``. """ @staticmethod @@ -37,9 +39,9 @@ def load(pipeline: Pipeline[Any], derived: GroupDerivedData) -> State: for f in pipeline.features: column = COLUMN_MAP.get(f) if column: - result[f] = f.load(getattr(derived, column)) + result[f] = f.from_column(getattr(derived, column)) elif f.name in data: - result[f] = f.load(data[f.name]) + result[f] = f.from_json(data[f.name]) else: result[f] = f.initial_value() return State(result) @@ -58,10 +60,10 @@ def build_update(pipeline: Pipeline[Any], state: State) -> dict[str, Any]: for f in pipeline.features: column = COLUMN_MAP.get(f) if column and f in updated: - update[column] = f.dump(state[f]) + update[column] = f.to_column(state[f]) # If any JSON feature was updated, include all of them (the blob is replaced wholesale) if updated.intersection(json_features): - update["data"] = {f.name: f.dump(state[f]) for f in json_features} + update["data"] = {f.name: f.to_json(state[f]) for f in json_features} return update @staticmethod diff --git a/tests/sentry/issues/derived/test_processing.py b/tests/sentry/issues/derived/test_processing.py index 0f165e2c48ee..968fdf16f265 100644 --- a/tests/sentry/issues/derived/test_processing.py +++ b/tests/sentry/issues/derived/test_processing.py @@ -1,3 +1,5 @@ +from datetime import datetime, timezone + import pytest from django.db import router, transaction @@ -26,7 +28,10 @@ ) from sentry.issues.derived.framework import ( AggregatorResult, + DateTimeCodec, + EnumCodec, Feature, + OptionalCodec, Pipeline, State, StateUpdate, @@ -46,6 +51,7 @@ from sentry.testutils.cases import TestCase from sentry.testutils.helpers.features import with_feature from sentry.testutils.outbox import outbox_runner +from sentry.utils import json SOURCE = ActionSource.API @@ -333,6 +339,98 @@ def test_store_apply_to_instance() -> None: assert derived.view_count == 5 +def test_all_feature_defaults_round_trip_through_json() -> None: + state = PIPELINE.initial_state() + blob = {f.name: f.to_json(state[f]) for f in PIPELINE.features} + serialized = json.loads(json.dumps(blob)) + for f in PIPELINE.features: + assert f.from_json(serialized[f.name]) == state[f], f"round-trip failed for {f.name}" + + +# --- Codec tests --- + + +class TestDateTimeCodec: + def test_json_round_trip(self) -> None: + codec = DateTimeCodec() + dt = datetime(2025, 3, 15, 12, 30, 45, tzinfo=timezone.utc) + assert codec.from_json(codec.to_json(dt)) == dt + + def test_to_json_produces_iso_string(self) -> None: + codec = DateTimeCodec() + dt = datetime(2025, 3, 15, 12, 30, 45, tzinfo=timezone.utc) + dumped = codec.to_json(dt) + assert isinstance(dumped, str) + assert dumped == dt.isoformat() + + def test_column_round_trip_is_identity(self) -> None: + codec = DateTimeCodec() + dt = datetime(2025, 3, 15, 12, 30, 45, tzinfo=timezone.utc) + assert codec.from_column(codec.to_column(dt)) == dt + assert codec.to_column(dt) is dt + + def test_optional_none(self) -> None: + codec = OptionalCodec(DateTimeCodec()) + assert codec.to_json(None) is None + assert codec.from_json(None) is None + + def test_optional_json_round_trip(self) -> None: + codec = OptionalCodec(DateTimeCodec()) + dt = datetime(2025, 3, 15, 12, 30, 45, tzinfo=timezone.utc) + assert codec.from_json(codec.to_json(dt)) == dt + + +class TestEnumCodecCoverage: + @pytest.mark.parametrize("raw", ["open", "closed"]) + def test_issue_status_json_round_trip(self, raw: str) -> None: + codec = EnumCodec(IssueStatus) + loaded = codec.from_json(raw) + assert codec.to_json(loaded) == raw + + @pytest.mark.parametrize("raw", ["open", "closed"]) + def test_issue_status_column_round_trip(self, raw: str) -> None: + codec = EnumCodec(IssueStatus) + loaded = codec.from_column(raw) + assert isinstance(loaded, IssueStatus) + assert codec.to_column(loaded) == raw + + @pytest.mark.parametrize( + "raw", ["identified", "assigned", "diagnosed", "fix_proposed", "fix_applied"] + ) + def test_issue_progress_state_json_round_trip(self, raw: str) -> None: + codec = EnumCodec(IssueProgressState) + loaded = codec.from_json(raw) + assert codec.to_json(loaded) == raw + + @pytest.mark.parametrize( + "raw", ["identified", "assigned", "diagnosed", "fix_proposed", "fix_applied"] + ) + def test_issue_progress_state_column_produces_enum(self, raw: str) -> None: + codec = EnumCodec(IssueProgressState) + loaded = codec.from_column(raw) + assert isinstance(loaded, IssueProgressState) + + @pytest.mark.parametrize( + "raw", + [None, "identified", "assigned", "diagnosed", "fix_proposed", "fix_applied"], + ) + def test_optional_progress_json_round_trip(self, raw: str | None) -> None: + codec = OptionalCodec(EnumCodec(IssueProgressState)) + loaded = codec.from_json(raw) + assert codec.to_json(loaded) == raw + + @pytest.mark.parametrize( + "raw", + [None, "identified", "assigned", "diagnosed", "fix_proposed", "fix_applied"], + ) + def test_optional_progress_column_round_trip(self, raw: str | None) -> None: + codec = OptionalCodec(EnumCodec(IssueProgressState)) + loaded = codec.from_column(raw) + if raw is not None: + assert isinstance(loaded, IssueProgressState) + assert codec.to_column(loaded) == raw + + # --- Store tests (need DB) --- @@ -363,6 +461,7 @@ def test_load_populates_columns_and_json(self) -> None: state = GroupDerivedDataStore.load(PIPELINE, derived) assert state[VIEW_COUNT] == 3 assert state[PROGRESS] == IssueProgressState.DIAGNOSED + assert isinstance(state[PROGRESS], IssueProgressState) assert state[STATUS] == IssueStatus.CLOSED def test_load_null_progress(self) -> None: