From e13a433f4e0ba0feb4728cf398c09fab284217cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Edgar=20Ram=C3=ADrez-Mondrag=C3=B3n?= <edgarrm358@gmail.com>
Date: Thu, 18 Jul 2024 17:56:12 -0600
Subject: [PATCH] refactor: Deprecate `row` parameter of `Stream.post_process`
 in favor or `record`

Take 2 for https://github.com/meltano/sdk/pull/966
---
 .pre-commit-config.yaml                |  2 +-
 pyproject.toml                         |  1 +
 singer_sdk/helpers/_deprecation.py     | 21 ++++++++
 singer_sdk/streams/core.py             | 21 ++++++--
 singer_sdk/typing.py                   |  2 +-
 tests/core/helpers/test_deprecation.py | 70 ++++++++++++++++++++++++++
 6 files changed, 112 insertions(+), 5 deletions(-)
 create mode 100644 singer_sdk/helpers/_deprecation.py
 create mode 100644 tests/core/helpers/test_deprecation.py

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index ebf5e849c..5edfaf497 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -50,7 +50,7 @@ repos:
   - id: check-readthedocs
 
 - repo: https://github.com/astral-sh/ruff-pre-commit
-  rev: v0.5.2
+  rev: v0.5.3
   hooks:
   - id: ruff
     args: [--fix, --exit-non-zero-on-fix, --show-fixes]
diff --git a/pyproject.toml b/pyproject.toml
index eae7ecd64..6ab3d7813 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -386,6 +386,7 @@ unfixable = [
     "SLF001",
     "PLC2701",  # Allow usage of private members in tests
     "PLR6301",  # Don't suggest making test methods static, etc.
+    "INP001",   # flake8-no-pep420: implicit-namespace-package
 ]
 # Disabled some checks in samples code
 "samples/*" = ["ANN", "D"]
diff --git a/singer_sdk/helpers/_deprecation.py b/singer_sdk/helpers/_deprecation.py
new file mode 100644
index 000000000..2ecf9fdf3
--- /dev/null
+++ b/singer_sdk/helpers/_deprecation.py
@@ -0,0 +1,21 @@
+from __future__ import annotations
+
+import functools
+import typing as t
+import warnings
+
+
+def deprecate_row_param(func: t.Callable) -> t.Callable:
+    @functools.wraps(func)
+    def wrapper(*args: t.Any, **kwargs: t.Any) -> t.Any:  # noqa: ANN401
+        if "row" in kwargs:
+            warnings.warn(
+                f"The 'row' parameter for '{func.__qualname__}' is deprecated. "
+                "Use 'record' instead.",
+                category=DeprecationWarning,
+                stacklevel=2,
+            )
+            kwargs["record"] = kwargs.pop("row")
+        return func(*args, **kwargs)
+
+    return wrapper
diff --git a/singer_sdk/streams/core.py b/singer_sdk/streams/core.py
index c588a9729..5200cf53d 100644
--- a/singer_sdk/streams/core.py
+++ b/singer_sdk/streams/core.py
@@ -7,6 +7,7 @@
 import datetime
 import json
 import typing as t
+from contextlib import contextmanager
 from os import PathLike
 from pathlib import Path
 from types import MappingProxyType
@@ -28,6 +29,7 @@
 )
 from singer_sdk.helpers._catalog import pop_deselected_record_properties
 from singer_sdk.helpers._compat import datetime_fromisoformat
+from singer_sdk.helpers._deprecation import deprecate_row_param
 from singer_sdk.helpers._flattening import get_flattening_options
 from singer_sdk.helpers._state import (
     finalize_state_progress_markers,
@@ -1155,6 +1157,15 @@ def _sync_batches(
                 self._write_batch_message(encoding=encoding, manifest=manifest)
                 self._write_state_message()
 
+    @contextmanager
+    def _with_context(
+        self,
+        context: types.Context | None,
+    ) -> t.Generator[None, None, None]:
+        self.context = MappingProxyType(context) if context else None
+        yield
+        self.context = None
+
     # Public methods ("final", not recommended to be overridden)
 
     @t.final
@@ -1387,9 +1398,10 @@ def get_batches(
         for manifest in batcher.get_batches(records=records):
             yield batch_config.encoding, manifest
 
+    @deprecate_row_param
     def post_process(  # noqa: PLR6301
         self,
-        row: types.Record,
+        record: types.Record,
         context: types.Context | None = None,  # noqa: ARG002
     ) -> dict | None:
         """As needed, append or transform raw data to match expected structure.
@@ -1403,10 +1415,13 @@ def post_process(  # noqa: PLR6301
         invalid or not-applicable records from the stream.
 
         Args:
-            row: Individual record in the stream.
+            record: Individual record in the stream.
             context: Stream partition or context dictionary.
 
         Returns:
             The resulting record dict, or `None` if the record should be excluded.
+
+        .. versionchanged:: 0.39.0
+           The ``row`` parameter was renamed to ``record`` with a deprecation warning.
         """
-        return row
+        return record
diff --git a/singer_sdk/typing.py b/singer_sdk/typing.py
index ca4d917df..908850686 100644
--- a/singer_sdk/typing.py
+++ b/singer_sdk/typing.py
@@ -612,7 +612,7 @@ class Property(JSONTypeHelper[T], t.Generic[T]):
     """Generic Property. Should be nested within a `PropertiesList`."""
 
     # TODO: Make some of these arguments keyword-only. This is a breaking change.
-    def __init__(  # noqa: PLR0913
+    def __init__(
         self,
         name: str,
         wrapped: JSONTypeHelper[T] | type[JSONTypeHelper[T]],
diff --git a/tests/core/helpers/test_deprecation.py b/tests/core/helpers/test_deprecation.py
new file mode 100644
index 000000000..5c25feb17
--- /dev/null
+++ b/tests/core/helpers/test_deprecation.py
@@ -0,0 +1,70 @@
+from __future__ import annotations
+
+import warnings
+
+from singer_sdk.helpers._deprecation import deprecate_row_param
+
+
+class Deprecated:
+    @deprecate_row_param
+    def check_row(self, record):
+        pass
+
+
+class StaleSubclass(Deprecated):
+    def check_row(self, row):
+        return super().check_row(row=row)
+
+
+class OkSubclass(Deprecated):
+    def check_row(self, row):
+        return super().check_row(row)
+
+
+class NewSubclass(Deprecated):
+    def check_row(self, record):
+        return super().check_row(record)
+
+
+def test_deprecated_row_parameter():
+    d = Deprecated()
+
+    # No warning should be raised
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        d.check_row("test")
+
+    # No warning should be raised
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        d.check_row(record="test")
+
+    # Warning should be raised
+    _assert_deprecation_warning(d)
+
+    s = StaleSubclass()
+    _assert_deprecation_warning(s)
+
+    o = OkSubclass()
+    # No warning should be raised
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        o.check_row(row="test")
+
+    n = NewSubclass()
+    # No warning should be raised
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        n.check_row(record="test")
+
+
+def _assert_deprecation_warning(instance: Deprecated):
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("always")
+        instance.check_row(row="test")
+        assert len(w) == 1
+        assert issubclass(w[-1].category, DeprecationWarning)
+        assert str(w[-1].message) == (
+            "The 'row' parameter for 'Deprecated.check_row' is deprecated. "
+            "Use 'record' instead."
+        )