From a37c6b222dc79084593bcd6d8cd855ccbf2c4d82 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 29 Apr 2026 07:39:02 -0400 Subject: [PATCH 1/6] API versioning app --- pyproject.toml | 2 + src/api_versioning/CHANGELOG.md | 5 + src/api_versioning/README.md | 5 + src/api_versioning/changelog.d/scriv.ini | 5 + .../mitol/api_versioning/__init__.py | 6 + .../mitol/api_versioning/apps.py | 48 +++ .../mitol/api_versioning/mixins.py | 222 ++++++++++++ .../mitol/api_versioning/py.typed | 0 .../mitol/api_versioning/schema_hooks.py | 118 +++++++ .../mitol/api_versioning/settings/__init__.py | 0 .../mitol/api_versioning/transforms.py | 129 +++++++ .../mitol/api_versioning/versions.py | 98 ++++++ src/api_versioning/pyproject.toml | 35 ++ testapp/main/settings/shared.py | 1 + tests/api_versioning/__init__.py | 0 tests/api_versioning/test_mixins.py | 321 ++++++++++++++++++ tests/api_versioning/test_schema_hooks.py | 321 ++++++++++++++++++ tests/api_versioning/test_transforms.py | 244 +++++++++++++ tests/api_versioning/test_versions.py | 165 +++++++++ tests/olposthog/test_features.py | 59 ++-- uv.lock | 35 +- 21 files changed, 1784 insertions(+), 35 deletions(-) create mode 100644 src/api_versioning/CHANGELOG.md create mode 100644 src/api_versioning/README.md create mode 100644 src/api_versioning/changelog.d/scriv.ini create mode 100644 src/api_versioning/mitol/api_versioning/__init__.py create mode 100644 src/api_versioning/mitol/api_versioning/apps.py create mode 100644 src/api_versioning/mitol/api_versioning/mixins.py create mode 100644 src/api_versioning/mitol/api_versioning/py.typed create mode 100644 src/api_versioning/mitol/api_versioning/schema_hooks.py create mode 100644 src/api_versioning/mitol/api_versioning/settings/__init__.py create mode 100644 src/api_versioning/mitol/api_versioning/transforms.py create mode 100644 src/api_versioning/mitol/api_versioning/versions.py create mode 100644 src/api_versioning/pyproject.toml create mode 100644 tests/api_versioning/__init__.py create mode 100644 tests/api_versioning/test_mixins.py create mode 100644 tests/api_versioning/test_schema_hooks.py create mode 100644 tests/api_versioning/test_transforms.py create mode 100644 tests/api_versioning/test_versions.py diff --git a/pyproject.toml b/pyproject.toml index 73da4981..326085c2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,6 +25,7 @@ dependencies = [ "mitol-django-scim[celery]", "mitol-django-apigateway", "mitol-django-observability", + "mitol-django-api_versioning", ] readme = "README.md" requires-python = ">= 3.11" @@ -124,6 +125,7 @@ mitol-django-scim = { workspace = true } mitol-django-apigateway = { workspace = true } mitol-django-observability = { workspace = true } mitol-drf-lint = { workspace = true } +mitol-django-api_versioning = { workspace = true } [tool.hatch.build.targets.sdist] include = ["CHANGELOG.md", "README.md", "py.typed", "**/*.py"] diff --git a/src/api_versioning/CHANGELOG.md b/src/api_versioning/CHANGELOG.md new file mode 100644 index 00000000..e323ebdf --- /dev/null +++ b/src/api_versioning/CHANGELOG.md @@ -0,0 +1,5 @@ +# Changelog + +## 2026.3.24 + +- Initial release with Transform base class, VersionedSerializerMixin, and drf-spectacular schema hook. diff --git a/src/api_versioning/README.md b/src/api_versioning/README.md new file mode 100644 index 00000000..a84e0c46 --- /dev/null +++ b/src/api_versioning/README.md @@ -0,0 +1,5 @@ +# mitol-django-api_versioning + +API versioning framework for Django REST Framework with transform-based backwards compatibility. + +Allows breaking API changes to be introduced in new versions while older versions continue to work via automatic response/request transformations — without duplicating views or serializers. diff --git a/src/api_versioning/changelog.d/scriv.ini b/src/api_versioning/changelog.d/scriv.ini new file mode 100644 index 00000000..ccf3d587 --- /dev/null +++ b/src/api_versioning/changelog.d/scriv.ini @@ -0,0 +1,5 @@ +[scriv] +format = md +md_header_level = 2 +entry_title_template = file: ../../scripts/scriv/entry_title.${config:format}.j2 +version = literal: __init__.py: __version__ diff --git a/src/api_versioning/mitol/api_versioning/__init__.py b/src/api_versioning/mitol/api_versioning/__init__.py new file mode 100644 index 00000000..ab70a3d1 --- /dev/null +++ b/src/api_versioning/mitol/api_versioning/__init__.py @@ -0,0 +1,6 @@ +"""mitol.api_versioning""" + +default_app_config = "mitol.api_versioning.apps.ApiVersioningApp" + +__version__ = "2026.3.24" +__distributionname__ = "mitol-django-api_versioning" diff --git a/src/api_versioning/mitol/api_versioning/apps.py b/src/api_versioning/mitol/api_versioning/apps.py new file mode 100644 index 00000000..7449236f --- /dev/null +++ b/src/api_versioning/mitol/api_versioning/apps.py @@ -0,0 +1,48 @@ +"""ApiVersioning app AppConfig.""" + +import importlib +import logging +import os + +from django.apps import AppConfig, apps +from django.utils.module_loading import module_has_submodule + +log = logging.getLogger(__name__) + + +class ApiVersioningApp(AppConfig): + """Default configuration for the api_versioning app.""" + + name = "mitol.api_versioning" + label = "api_versioning" + verbose_name = "API Versioning" + + # necessary because this is a namespaced app + path = os.path.dirname(os.path.abspath(__file__)) # noqa: PTH100, PTH120 + + def ready(self): + """Auto-discover transforms modules in all installed apps. + + Looks for a `transforms` submodule in each installed app + (e.g., `learning_resources.transforms`). Importing the module + triggers the Transform metaclass, which auto-registers each + Transform subclass with the version registry. + + Only attempts to import `transforms` if the submodule actually + exists, so real ImportErrors inside the module are not suppressed. + """ + for app_config in apps.get_app_configs(): + if module_has_submodule(app_config.module, "transforms"): + module_name = f"{app_config.name}.transforms" + importlib.import_module(module_name) + + +class TransitionalApiVersioningApp(AppConfig): + """AppConfig for transitioning a project with an existing app.""" + + name = "mitol.api_versioning" + label = "transitional_api_versioning" + verbose_name = "API Versioning" + + # necessary because this is a namespaced app + path = os.path.dirname(os.path.abspath(__file__)) # noqa: PTH100, PTH120 diff --git a/src/api_versioning/mitol/api_versioning/mixins.py b/src/api_versioning/mitol/api_versioning/mixins.py new file mode 100644 index 00000000..dd03f9ca --- /dev/null +++ b/src/api_versioning/mitol/api_versioning/mixins.py @@ -0,0 +1,222 @@ +"""VersionedSerializerMixin for DRF serializers. + +This mixin hooks into DRF's to_representation and to_internal_value methods +to automatically apply version transforms. It works with ModelSerializer +and any other serializer base class (it's a mixin, not a replacement). + +Usage:: + + class MySerializer(VersionedSerializerMixin, serializers.ModelSerializer): + class Meta: + model = MyModel + fields = "__all__" + +When a request comes in for an older API version, the mixin: + +- On response: applies transforms backwards (newest first) to convert + the latest-version data to the requested version's shape. +- On request: applies transforms forwards (oldest first) to convert + the older version's input to the latest version's shape before validation. +""" + +import logging + +from mitol.api_versioning.versions import ( + get_latest_version, + get_transforms_backwards, + get_transforms_forwards, +) +from rest_framework import serializers + +log = logging.getLogger(__name__) + + +class VersionedSerializerMixin: + """Mixin that applies version transforms to serializer input/output. + + Add this as the first base class (before ModelSerializer) so its + to_representation and to_internal_value are called first in the MRO. + """ + + def __init_subclass__(cls, **kwargs): + """Verify correct MRO placement at class definition time.""" + super().__init_subclass__(**kwargs) + mixin_idx = None + serializer_idx = None + for idx, klass in enumerate(cls.__mro__): + if klass is VersionedSerializerMixin and mixin_idx is None: + mixin_idx = idx + if klass is serializers.Serializer and serializer_idx is None: + serializer_idx = idx + + if ( + mixin_idx is not None + and serializer_idx is not None + and mixin_idx > serializer_idx + ): + msg = ( + f"{cls.__name__}: VersionedSerializerMixin must appear " + f"before the Serializer base class in the inheritance list. " + f"Use: class {cls.__name__}(VersionedSerializerMixin, ...)" + ) + raise TypeError(msg) + + def to_representation(self, instance): + """Serialize instance, then apply backwards transforms for older versions.""" + data = super().to_representation(instance) + request = self.context.get("request") + if not request or not hasattr(request, "version"): + return data + + latest = get_latest_version() + if not latest or request.version == latest: + return data + + transforms = get_transforms_backwards(self.__class__, request.version) + if transforms: + log.debug( + "to_representation: %s version=%s -> %s, applying %d transform(s): %s", + self.__class__.__name__, + latest, + request.version, + len(transforms), + " -> ".join(t.__name__ for t in transforms), + ) + for transform_cls in transforms: + data = transform_cls().to_representation(data, request, instance) + + return data + + def to_internal_value(self, data): + """Apply forwards transforms for older versions, then validate.""" + request = self.context.get("request") + if request and hasattr(request, "version"): + latest = get_latest_version() + if latest and request.version != latest: + data = data.copy() if hasattr(data, "copy") else dict(data) + transforms = get_transforms_forwards(self.__class__, request.version) + if transforms: + log.debug( + ( + "to_internal_value: %s version=%s -> %s, applying " + "%d transform(s): %s" + ), + self.__class__.__name__, + request.version, + latest, + len(transforms), + " -> ".join(t.__name__ for t in transforms), + ) + for transform_cls in transforms: + data = transform_cls().to_internal_value(data, request) + + return super().to_internal_value(data) + + +def transform_dict_backwards(data, serializer_class, request, *, recursive=False): + """Apply backwards transforms to a raw dict (no model instance). + + Use this for data that bypasses DRF serialization, such as + OpenSearch results that should match a serializer's output format. + + Args: + data: A dict in the latest version's format. + serializer_class: The serializer class whose transforms to apply. + request: The DRF request (used to determine the API version). + recursive: If True, also inspect the serializer's fields and + recursively apply transforms to any nested serializer fields + that have their own transforms registered. + + Returns: + The transformed dict, or the original dict if no transforms apply. + """ + if not request or not hasattr(request, "version"): + return data + + latest = get_latest_version() + if not latest or request.version == latest: + return data + + if recursive: + _transform_nested_fields(data, serializer_class, request) + + transforms = get_transforms_backwards(serializer_class, request.version) + if transforms: + log.debug( + "transform_dict_backwards: %s version=%s, applying %d transform(s): %s", + serializer_class.__name__, + request.version, + len(transforms), + " -> ".join(t.__name__ for t in transforms), + ) + for transform_cls in transforms: + data = transform_cls().to_representation(data, request, instance=None) + + return data + + +def _apply_transforms_to_data(transforms, data, request): + """Apply a list of transform classes to a dict or list of dicts.""" + if isinstance(data, dict): + for transform_cls in transforms: + transform_cls().to_representation(data, request, instance=None) + elif isinstance(data, list): + for item in data: + if isinstance(item, dict): + for transform_cls in transforms: + transform_cls().to_representation(item, request, instance=None) + + +def _transform_nested_fields(data, serializer_class, request): + """Recursively apply transforms to nested serializer fields. + + Introspects the serializer's declared fields. For each field that + is itself a serializer (not a primitive field), checks if transforms + are registered for that serializer class and applies them to the + corresponding nested dict in data. Recurses into children so that + deeply nested serializer trees are fully transformed. + """ + try: + fields = serializer_class().fields + except Exception: # noqa: BLE001 + log.debug( + "Failed to instantiate %s for nested field introspection", + serializer_class.__name__, + ) + return + + for field_name, field in fields.items(): + nested_data = data.get(field_name) + if nested_data is None: + continue + + child_serializer_class = _get_field_serializer_class(field) + if child_serializer_class is None: + continue + + # Recurse first so deepest fields are transformed before their parents + if isinstance(nested_data, dict): + _transform_nested_fields(nested_data, child_serializer_class, request) + elif isinstance(nested_data, list): + for item in nested_data: + if isinstance(item, dict): + _transform_nested_fields(item, child_serializer_class, request) + + child_transforms = get_transforms_backwards( + child_serializer_class, request.version + ) + if child_transforms: + _apply_transforms_to_data(child_transforms, nested_data, request) + + +def _get_field_serializer_class(field): + """Extract the serializer class from a DRF field, if it is a serializer. + + Handles both direct serializer fields and many=True (ListSerializer) + wrappers. + """ + if isinstance(field, serializers.ListSerializer): + return type(field.child) + if isinstance(field, serializers.Serializer): + return type(field) + return None diff --git a/src/api_versioning/mitol/api_versioning/py.typed b/src/api_versioning/mitol/api_versioning/py.typed new file mode 100644 index 00000000..e69de29b diff --git a/src/api_versioning/mitol/api_versioning/schema_hooks.py b/src/api_versioning/mitol/api_versioning/schema_hooks.py new file mode 100644 index 00000000..22aa854d --- /dev/null +++ b/src/api_versioning/mitol/api_versioning/schema_hooks.py @@ -0,0 +1,118 @@ +"""drf-spectacular postprocessing hook for API versioning. + +When generating OpenAPI specs for older API versions, this hook walks +backwards through registered transforms and applies their schema +modifications. This ensures each version's spec accurately describes +that version's response/request shapes, which is critical for correct +TypeScript client generation. + +Register this hook in SPECTACULAR_SETTINGS["POSTPROCESSING_HOOKS"]. +""" + +import logging +import re + +from mitol.api_versioning.versions import ( + get_latest_version, + get_transforms_between, +) + +log = logging.getLogger(__name__) + + +def _resolve_schema_name(serializer_ref): + """Convert a serializer reference to the drf-spectacular schema name. + + Accepts either a dotted path string or a class object. + drf-spectacular typically strips the "Serializer" suffix and uses the + remaining class name. E.g.:: + + "myapp.serializers.LearningResourceSerializer" + -> "LearningResource" + """ + if isinstance(serializer_ref, str): + class_name = serializer_ref.rsplit(".", 1)[-1] + else: + class_name = serializer_ref.__name__ + return re.sub(r"Serializer$", "", class_name) + + +# Known suffixes that drf-spectacular appends to schema component names. +# COMPONENT_SPLIT_REQUEST=True generates separate Request variants +# for writable endpoints. +_SCHEMA_SUFFIXES = ("", "Request") +_SCHEMA_PREFIXES = ("", "Patched") + + +def _get_all_schema_variants(base_name, schemas): + """Find all schema component names that match a base serializer name. + + drf-spectacular generates variants like: + - LearningResource (base) + - LearningResourceRequest (for request bodies) + - PatchedLearningResource (for PATCH requests) + - PatchedLearningResourceRequest + + Uses exact matching with known prefixes/suffixes to avoid false + positives (e.g., "CourseRun" should not match when targeting "Course"). + """ + expected = set() + for prefix in _SCHEMA_PREFIXES: + for suffix in _SCHEMA_SUFFIXES: + expected.add(f"{prefix}{base_name}{suffix}") + + return [name for name in schemas if name in expected] + + +def postprocess_versioned_schema(result, generator, request, public): # noqa: ARG001 + """Postprocessing hook for drf-spectacular. + + When generating the schema for an older API version, applies all + transform_schema() methods backwards to produce correct schema shapes + for that version. + + Args: + result: The generated OpenAPI schema dict. + generator: The drf-spectacular SchemaGenerator instance. + request: The request (may be None during CLI generation). + public: Whether this is a public schema. + + Returns: + The (possibly modified) schema dict. + """ + latest = get_latest_version() + if not latest: + return result + + api_version = getattr(generator, "api_version", None) or latest + if api_version == latest: + return result + + schemas = result.get("components", {}).get("schemas", {}) + if not schemas: + return result + + transforms = get_transforms_between(api_version, latest) + if not transforms: + return result + + log.info( + "Applying %d schema transform(s) for API version %s", + len(transforms), + api_version, + ) + + # Apply transforms in reverse order (newest first, going backwards) + for transform_cls in reversed(transforms): + # Use explicit component_name if set, otherwise derive from serializer + base_name = transform_cls.component_name or _resolve_schema_name( + transform_cls.serializer + ) + variant_names = _get_all_schema_variants(base_name, schemas) + + for schema_name in variant_names: + schemas[schema_name] = transform_cls().transform_schema( + schemas[schema_name], direction="backwards" + ) + + return result diff --git a/src/api_versioning/mitol/api_versioning/settings/__init__.py b/src/api_versioning/mitol/api_versioning/settings/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/api_versioning/mitol/api_versioning/transforms.py b/src/api_versioning/mitol/api_versioning/transforms.py new file mode 100644 index 00000000..a20463f1 --- /dev/null +++ b/src/api_versioning/mitol/api_versioning/transforms.py @@ -0,0 +1,129 @@ +"""Transform base class for API versioning. + +Each Transform represents a single breaking change introduced in a specific +API version. Transforms define how to convert data and OpenAPI schemas between +the version that introduced the change and the previous version. + +Transforms auto-register with the version registry via a metaclass. +""" + +from mitol.api_versioning.versions import register_transform + + +class TransformMeta(type): + """Metaclass that auto-registers Transform subclasses. + + When a Transform subclass is defined with a `version` attribute, it is + automatically registered so the versioning system can discover it. + Validates required attributes at class definition time to catch errors early. + """ + + def __new__(cls, name, bases, dct): + """Create a Transform subclass and auto-register concrete transforms.""" + subclass = super().__new__(cls, name, bases, dct) + if dct.get("version") is not None: + _validate_transform(subclass, name) + register_transform(subclass) + return subclass + + +def _validate_transform(transform_cls, name): + """Validate a Transform subclass has correctly configured attributes. + + Raises TypeError at class definition time for misconfiguration. + """ + version = transform_cls.version + if not isinstance(version, str) or not version: + msg = ( + f"Transform '{name}' has invalid version: {version!r}. " + f"Must be a non-empty string (e.g., 'v2')." + ) + raise TypeError(msg) + + serializer = transform_cls.serializer + if not serializer: + msg = ( + f"Transform '{name}' must define a 'serializer' attribute " + f"(dotted path to the target serializer class)." + ) + raise TypeError(msg) + + if isinstance(serializer, str) and "." not in serializer: + msg = ( + f"Transform '{name}' has serializer={serializer!r} which " + f"is not a dotted path. Use the full path like " + f"'myapp.serializers.MySerializer'." + ) + raise TypeError(msg) + + +class Transform(metaclass=TransformMeta): + """Base class for API version transforms. + + Subclasses must define: + version (str): The API version that introduced this change (e.g., "v2"). + description (str): Human-readable description of the breaking change. + serializer (str): Dotted path to the serializer class this applies to + (e.g., "myapp.serializers.MySerializer"). + component_name (str, optional): Explicit drf-spectacular schema component + name to target. If not set, the name is derived from the serializer + by stripping the "Serializer" suffix. Use this when a serializer has + custom component naming via @extend_schema(component_name=...). + + Subclasses implement whichever methods are relevant to the change: + - to_representation: for response data transforms + - to_internal_value: for request data transforms + - transform_schema: for OpenAPI schema transforms + """ + + version: str = None + description: str = "" + serializer: str = None + component_name: str = None + + def to_representation(self, data, request, instance): # noqa: ARG002 + """Transform response data backwards (latest version -> older version). + + Called when a client requests an older API version. Mutates `data` + in place to match the older version's expected response shape. + + Args: + data: The serialized response dict (latest version format). + request: The DRF request object. + instance: The model instance being serialized. + + Returns: + The transformed data dict. + """ + return data + + def to_internal_value(self, data, request): # noqa: ARG002 + """Transform request data forwards (older version -> latest version). + + Called when a client sends data using an older API version format. + Mutates `data` in place to match the latest version's expected input. + + Args: + data: The incoming request data dict (older version format). + request: The DRF request object. + + Returns: + The transformed data dict. + """ + return data + + def transform_schema(self, schema, direction): # noqa: ARG002 + """Transform an OpenAPI schema component for version compatibility. + + Called during OpenAPI spec generation to produce correct schemas + for older API versions. This ensures generated TypeScript clients + have accurate type definitions for each version. + + Args: + schema: The OpenAPI schema dict for a component. + direction: "backwards" when generating schema for an older version. + + Returns: + The transformed schema dict. + """ + return schema diff --git a/src/api_versioning/mitol/api_versioning/versions.py b/src/api_versioning/mitol/api_versioning/versions.py new file mode 100644 index 00000000..a56a1d06 --- /dev/null +++ b/src/api_versioning/mitol/api_versioning/versions.py @@ -0,0 +1,98 @@ +"""Version registry for API versioning. + +Maintains an ordered list of API versions and a registry mapping +versions to their transforms. Integrates with DRF's ALLOWED_VERSIONS +setting as the source of truth. +""" + +from collections import defaultdict + +from django.conf import settings + + +def get_allowed_versions(): + """Get the ordered list of allowed API versions from DRF settings.""" + return settings.REST_FRAMEWORK.get("ALLOWED_VERSIONS", []) + + +def get_latest_version(): + """Get the latest (newest) API version.""" + versions = get_allowed_versions() + return versions[-1] if versions else None + + +# Registry mapping version strings to lists of Transform classes. +# Populated via Transform metaclass auto-registration. +_transform_registry = defaultdict(list) +_registered_transforms = set() + + +def register_transform(transform_cls): + """ + Register a transform class for its declared version. + """ + if transform_cls in _registered_transforms: + return + _registered_transforms.add(transform_cls) + _transform_registry[transform_cls.version].append(transform_cls) + + +def get_transforms_for_version(version): + """Get all transforms introduced in a specific version.""" + return list(_transform_registry.get(version, [])) + + +def get_transforms_between(from_version, to_version): + """Get all transforms for versions > from_version and <= to_version. + + Ordered by version (oldest first). Used to collect all transforms + that need to be applied when converting between two versions. + """ + versions = get_allowed_versions() + try: + from_idx = versions.index(from_version) + to_idx = versions.index(to_version) + except ValueError: + return [] + + transforms = [] + for version in versions[from_idx + 1 : to_idx + 1]: + transforms.extend(_transform_registry.get(version, [])) + return transforms + + +def get_transforms_backwards(serializer_class, request_version): + """Get transforms to apply backwards (newest first) for a serializer. + + Returns transform classes whose serializer matches the given class + (by dotted path or direct class reference), ordered newest-version-first. + """ + latest = get_latest_version() + if not latest or request_version == latest: + return [] + + serializer_path = f"{serializer_class.__module__}.{serializer_class.__qualname__}" + transforms = get_transforms_between(request_version, latest) + + matching = [ + t for t in transforms if t.serializer in {serializer_path, serializer_class} + ] + return list(reversed(matching)) + + +def get_transforms_forwards(serializer_class, request_version): + """Get transforms to apply forwards (oldest first) for a serializer. + + Returns transform classes whose serializer matches the given class, + ordered oldest-version-first. + """ + latest = get_latest_version() + if not latest or request_version == latest: + return [] + + serializer_path = f"{serializer_class.__module__}.{serializer_class.__qualname__}" + transforms = get_transforms_between(request_version, latest) + + return [ + t for t in transforms if t.serializer in {serializer_path, serializer_class} + ] diff --git a/src/api_versioning/pyproject.toml b/src/api_versioning/pyproject.toml new file mode 100644 index 00000000..309fa0c0 --- /dev/null +++ b/src/api_versioning/pyproject.toml @@ -0,0 +1,35 @@ +[project] +name = "mitol-django-api_versioning" +version = "2026.3.24" +description = "MIT Open Learning Django API versioning framework with transform-based backwards compatibility" +dependencies = [ + "django>=3.0", + "djangorestframework>=3.14", +] +readme = "README.md" +license = "BSD-3-Clause" +requires-python = ">=3.10" + +[tool.bumpver] +current_version = "2026.3.24" +version_pattern = "YYYY.MM.DD[.INC0]" + +[tool.bumpver.file_patterns] +"pyproject.toml" = [ + 'version = "{version}"', +] +"mitol/api_versioning/__init__.py" = [ + '__version__ = "{version}"', +] + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.build.targets.sdist] +include = ["CHANGELOG.md", "README.md", "py.typed", "**/*.py"] +exclude = ["BUILD", "pyproject.toml"] + +[tool.hatch.build.targets.wheel] +include = ["CHANGELOG.md", "README.md", "py.typed", "**/*.py"] +exclude = ["BUILD", "pyproject.toml"] diff --git a/testapp/main/settings/shared.py b/testapp/main/settings/shared.py index 82120cfc..d93fcf0d 100644 --- a/testapp/main/settings/shared.py +++ b/testapp/main/settings/shared.py @@ -82,6 +82,7 @@ "mitol.scim.apps.ScimApp", "mitol.apigateway.apps.ApigatewayApp", "mitol.observability.apps.ObservabilityConfig", + "mitol.api_versioning.apps.ApiVersioningApp", # test app, integrates the reusable apps "main", "users", diff --git a/tests/api_versioning/__init__.py b/tests/api_versioning/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/api_versioning/test_mixins.py b/tests/api_versioning/test_mixins.py new file mode 100644 index 00000000..38b81fe2 --- /dev/null +++ b/tests/api_versioning/test_mixins.py @@ -0,0 +1,321 @@ +"""Tests for the VersionedSerializerMixin.""" + +from types import SimpleNamespace + +import pytest +from mitol.api_versioning.mixins import ( + VersionedSerializerMixin, + transform_dict_backwards, +) +from mitol.api_versioning.transforms import Transform +from mitol.api_versioning.versions import _registered_transforms, _transform_registry +from rest_framework import serializers + + +@pytest.fixture(autouse=True) +def _clear_registry(): + """Clear the transform registry before and after each test.""" + saved_registry = dict(_transform_registry) + saved_registered = set(_registered_transforms) + _transform_registry.clear() + _registered_transforms.clear() + yield + _transform_registry.clear() + _registered_transforms.clear() + _transform_registry.update(saved_registry) + _registered_transforms.update(saved_registered) + + +@pytest.fixture +def _versions(settings): + settings.REST_FRAMEWORK = { + **getattr(settings, "REST_FRAMEWORK", {}), + "ALLOWED_VERSIONS": ["v0", "v1", "v2"], + } + + +class SampleSerializer(VersionedSerializerMixin, serializers.Serializer): + """Sample serializer for testing.""" + + name = serializers.CharField() + value = serializers.IntegerField() + + +def test_mro_enforcement_rejects_wrong_order(): + """Mixin after Serializer in bases should raise TypeError.""" + with pytest.raises(TypeError, match="must appear before"): + + class BadSerializer(serializers.Serializer, VersionedSerializerMixin): + name = serializers.CharField() + + +def _make_request(version): + """Create a mock request with a version attribute.""" + return SimpleNamespace(version=version) + + +@pytest.mark.usefixtures("_versions") +def test_mixin_no_transforms(): + """With no transforms, mixin should be a no-op.""" + request = _make_request("v1") + context = {"request": request} + serializer = SampleSerializer(context=context) + + instance = SimpleNamespace(name="test", value=42) + data = serializer.to_representation(instance) + assert data == {"name": "test", "value": 42} + + +@pytest.mark.usefixtures("_versions") +def test_mixin_latest_version_no_transform(): + """Requests at the latest version should not apply transforms.""" + serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" + + class RenameTransform(Transform): + version = "v2" + description = "Rename name to title" + serializer = serializer_path + + def to_representation(self, data, request, instance): # noqa: ARG002 + data["name_old"] = data.pop("name") + return data + + request = _make_request("v2") # latest + context = {"request": request} + serializer = SampleSerializer(context=context) + + instance = SimpleNamespace(name="test", value=42) + data = serializer.to_representation(instance) + assert data == {"name": "test", "value": 42} + + +@pytest.mark.usefixtures("_versions") +def test_mixin_older_version_applies_transform(): + """Requests at an older version should apply transforms backwards.""" + serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" + + class RenameTransform(Transform): + version = "v2" + description = "Rename name to title" + serializer = serializer_path + + def to_representation(self, data, request, instance): # noqa: ARG002 + if "name" in data: + data["title"] = data.pop("name") + return data + + request = _make_request("v1") + context = {"request": request} + serializer = SampleSerializer(context=context) + + instance = SimpleNamespace(name="test", value=42) + data = serializer.to_representation(instance) + assert data == {"title": "test", "value": 42} + + +@pytest.mark.usefixtures("_versions") +def test_mixin_to_internal_value_applies_forward_transform(): + """Incoming data from older version should be transformed forwards.""" + serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" + + class RenameTransform(Transform): + version = "v2" + description = "Rename title to name" + serializer = serializer_path + + def to_internal_value(self, data, request): # noqa: ARG002 + if "title" in data: + data["name"] = data.pop("title") + return data + + request = _make_request("v1") + context = {"request": request} + serializer = SampleSerializer(data={"title": "test", "value": 42}, context=context) + + assert serializer.is_valid(), serializer.errors + assert serializer.validated_data["name"] == "test" + + +@pytest.mark.usefixtures("_versions") +def test_mixin_no_request_context(): + """Without a request in context, mixin should be a no-op.""" + serializer = SampleSerializer(context={}) + + instance = SimpleNamespace(name="test", value=42) + data = serializer.to_representation(instance) + assert data == {"name": "test", "value": 42} + + +@pytest.mark.usefixtures("_versions") +def test_mixin_request_without_version(): + """With a request that has no version, mixin should be a no-op.""" + request = SimpleNamespace() + serializer = SampleSerializer(context={"request": request}) + + instance = SimpleNamespace(name="test", value=42) + data = serializer.to_representation(instance) + assert data == {"name": "test", "value": 42} + + +@pytest.mark.usefixtures("_versions") +def test_transform_dict_backwards_applies_transforms(): + """transform_dict_backwards should apply transforms to a raw dict.""" + serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" + + class RenameTransform(Transform): + version = "v2" + description = "Rename name to title" + serializer = serializer_path + + def to_representation(self, data, request, instance): # noqa: ARG002 + if "name" in data: + data["title"] = data.pop("name") + return data + + request = _make_request("v1") + data = {"name": "test", "value": 42} + result = transform_dict_backwards(data, SampleSerializer, request) + assert result == {"title": "test", "value": 42} + + +@pytest.mark.usefixtures("_versions") +def test_transform_dict_backwards_no_op_for_latest(): + """transform_dict_backwards should not transform at latest version.""" + request = _make_request("v2") + data = {"name": "test", "value": 42} + result = transform_dict_backwards(data, SampleSerializer, request) + assert result == {"name": "test", "value": 42} + + +def test_transform_dict_backwards_no_request(): + """transform_dict_backwards should be a no-op with no request.""" + data = {"name": "test"} + result = transform_dict_backwards(data, SampleSerializer, None) + assert result == {"name": "test"} + + +@pytest.mark.usefixtures("_versions") +def test_transform_dict_backwards_recursive(): + """recursive=True should apply transforms to nested serializer fields.""" + + class ChildSerializer(VersionedSerializerMixin, serializers.Serializer): + """Child serializer with a field that gets transformed.""" + + color = serializers.CharField() + + child_path = f"{ChildSerializer.__module__}.{ChildSerializer.__qualname__}" + + class ParentSerializer(serializers.Serializer): + """Parent serializer with a nested child.""" + + label = serializers.CharField() + child = ChildSerializer() + + class RenameColorTransform(Transform): + version = "v2" + description = "Rename color to colour" + serializer = child_path + + def to_representation(self, data, request, instance): # noqa: ARG002 + if "color" in data: + data["colour"] = data.pop("color") + return data + + request = _make_request("v1") + data = {"label": "test", "child": {"color": "red"}} + result = transform_dict_backwards(data, ParentSerializer, request, recursive=True) + assert result["child"] == {"colour": "red"} + assert result["label"] == "test" + + +@pytest.mark.usefixtures("_versions") +def test_transform_dict_backwards_recursive_many(): + """recursive=True should handle many=True nested serializer fields.""" + + class ItemSerializer(VersionedSerializerMixin, serializers.Serializer): + """Item serializer.""" + + name = serializers.CharField() + + item_path = f"{ItemSerializer.__module__}.{ItemSerializer.__qualname__}" + + class ContainerSerializer(serializers.Serializer): + """Container with many nested items.""" + + title = serializers.CharField() + items = ItemSerializer(many=True) + + class AddFieldTransform(Transform): + version = "v2" + description = "Add score field" + serializer = item_path + + def to_representation(self, data, request, instance): # noqa: ARG002 + data.pop("score", None) + return data + + request = _make_request("v1") + data = { + "title": "container", + "items": [ + {"name": "a", "score": 10}, + {"name": "b", "score": 20}, + ], + } + result = transform_dict_backwards( + data, ContainerSerializer, request, recursive=True + ) + assert result["items"] == [{"name": "a"}, {"name": "b"}] + + +@pytest.mark.usefixtures("_versions") +def test_transform_dict_backwards_recursive_no_nested_transforms(): + """recursive=True with no child transforms should be a no-op.""" + request = _make_request("v1") + data = {"name": "test", "value": 42} + result = transform_dict_backwards(data, SampleSerializer, request, recursive=True) + # SampleSerializer has no nested serializer fields, so no change + assert result == {"name": "test", "value": 42} + + +@pytest.mark.usefixtures("_versions") +def test_transform_dict_backwards_recursive_deep_nesting(): + """recursive=True should apply transforms to deeply nested serializer fields.""" + + class GrandchildSerializer(VersionedSerializerMixin, serializers.Serializer): + color = serializers.CharField() + + grandchild_path = ( + f"{GrandchildSerializer.__module__}.{GrandchildSerializer.__qualname__}" + ) + + class ChildSerializer(VersionedSerializerMixin, serializers.Serializer): + label = serializers.CharField() + grandchild = GrandchildSerializer() + + class ParentSerializer(serializers.Serializer): + title = serializers.CharField() + child = ChildSerializer() + + class RenameColorTransform(Transform): + version = "v2" + description = "Rename color to colour" + serializer = grandchild_path + + def to_representation(self, data, request, instance): # noqa: ARG002 + if "color" in data: + data["colour"] = data.pop("color") + return data + + request = _make_request("v1") + data = { + "title": "top", + "child": { + "label": "mid", + "grandchild": {"color": "red"}, + }, + } + result = transform_dict_backwards(data, ParentSerializer, request, recursive=True) + assert result["child"]["grandchild"] == {"colour": "red"} + assert result["child"]["label"] == "mid" + assert result["title"] == "top" diff --git a/tests/api_versioning/test_schema_hooks.py b/tests/api_versioning/test_schema_hooks.py new file mode 100644 index 00000000..15aa2454 --- /dev/null +++ b/tests/api_versioning/test_schema_hooks.py @@ -0,0 +1,321 @@ +"""Tests for the drf-spectacular schema postprocessing hook.""" + +from types import SimpleNamespace + +import pytest +from mitol.api_versioning.schema_hooks import ( + _get_all_schema_variants, + _resolve_schema_name, + postprocess_versioned_schema, +) +from mitol.api_versioning.transforms import Transform +from mitol.api_versioning.versions import _transform_registry, register_transform + + +@pytest.fixture(autouse=True) +def _clear_registry(): + """Clear the transform registry before and after each test.""" + saved = dict(_transform_registry) + _transform_registry.clear() + yield + _transform_registry.clear() + _transform_registry.update(saved) + + +class TestResolveSchemaName: + """Tests for _resolve_schema_name.""" + + def test_strips_serializer_suffix(self): + """Test stripping Serializer suffix.""" + assert ( + _resolve_schema_name("myapp.serializers.LearningResourceSerializer") + == "LearningResource" + ) + + def test_no_serializer_suffix(self): + """Test name without Serializer suffix.""" + assert _resolve_schema_name("myapp.serializers.LearningResource") == ( + "LearningResource" + ) + + def test_simple_name(self): + """Test simple class name.""" + assert _resolve_schema_name("CourseSerializer") == "Course" + + def test_class_object_with_serializer_suffix(self): + """Test with an actual class object instead of a string.""" + + class CourseSerializer: + pass + + assert _resolve_schema_name(CourseSerializer) == "Course" + + def test_class_object_without_serializer_suffix(self): + """Test class object without Serializer suffix.""" + + class Course: + pass + + assert _resolve_schema_name(Course) == "Course" + + +class TestGetAllSchemaVariants: + """Tests for _get_all_schema_variants.""" + + def test_finds_base_and_variants(self): + """Test finding all expected variants.""" + schemas = { + "LearningResource": {}, + "LearningResourceRequest": {}, + "PatchedLearningResource": {}, + "PatchedLearningResourceRequest": {}, + "Course": {}, + } + result = _get_all_schema_variants("LearningResource", schemas) + assert set(result) == { + "LearningResource", + "LearningResourceRequest", + "PatchedLearningResource", + "PatchedLearningResourceRequest", + } + + def test_no_matches(self): + """Test with no matching schemas.""" + schemas = {"Course": {}, "Program": {}} + assert _get_all_schema_variants("LearningResource", schemas) == [] + + def test_does_not_over_match_similar_names(self): + """'Course' should NOT match 'CourseRun' or 'CourseFeature'.""" + schemas = { + "Course": {}, + "CourseRequest": {}, + "CourseRun": {}, + "CourseRunRequest": {}, + "CourseFeature": {}, + "PatchedCourse": {}, + "PatchedCourseRequest": {}, + "PatchedCourseRun": {}, + } + result = _get_all_schema_variants("Course", schemas) + assert set(result) == { + "Course", + "CourseRequest", + "PatchedCourse", + "PatchedCourseRequest", + } + + +class TestPostprocessVersionedSchema: + """Tests for the drf-spectacular postprocessing hook.""" + + @pytest.fixture + def _versions(self, settings): + settings.REST_FRAMEWORK = { + **getattr(settings, "REST_FRAMEWORK", {}), + "ALLOWED_VERSIONS": ["v0", "v1", "v2"], + } + + @pytest.mark.usefixtures("_versions") + def test_no_op_for_latest_version(self): + """Hook should return schema unchanged for latest version.""" + generator = SimpleNamespace(api_version="v2") + schema = { + "components": { + "schemas": { + "Course": {"properties": {"name": {"type": "string"}}}, + } + } + } + result = postprocess_versioned_schema(schema, generator, None, public=False) + assert result == schema + + @pytest.mark.usefixtures("_versions") + def test_applies_transform_for_older_version(self): + """Hook should apply schema transforms for older versions.""" + + class RenameFieldTransform(Transform): + version = "v2" + description = "Rename name to title" + serializer = "myapp.serializers.CourseSerializer" + + def transform_schema(self, schema, direction): + if direction == "backwards": + props = schema.get("properties", {}) + if "name" in props: + props["title"] = props.pop("name") + return schema + + generator = SimpleNamespace(api_version="v1") + schema = { + "components": { + "schemas": { + "Course": { + "properties": { + "name": {"type": "string"}, + "id": {"type": "integer"}, + } + }, + } + } + } + result = postprocess_versioned_schema(schema, generator, None, public=False) + props = result["components"]["schemas"]["Course"]["properties"] + assert "title" in props + assert "name" not in props + assert "id" in props + + @pytest.mark.usefixtures("_versions") + def test_applies_transform_to_variants(self): + """Hook should apply transforms to Request/Patched variants too.""" + + class AddFieldTransform(Transform): + version = "v2" + description = "Add extra_field in v2" + serializer = "myapp.serializers.CourseSerializer" + + def transform_schema(self, schema, direction): + if direction == "backwards": + schema.get("properties", {}).pop("extra_field", None) + return schema + + generator = SimpleNamespace(api_version="v1") + schema = { + "components": { + "schemas": { + "Course": { + "properties": { + "name": {"type": "string"}, + "extra_field": {"type": "string"}, + } + }, + "CourseRequest": { + "properties": { + "name": {"type": "string"}, + "extra_field": {"type": "string"}, + } + }, + } + } + } + result = postprocess_versioned_schema(schema, generator, None, public=False) + + course = result["components"]["schemas"]["Course"]["properties"] + assert "extra_field" not in course + req = result["components"]["schemas"]["CourseRequest"]["properties"] + assert "extra_field" not in req + + @pytest.mark.usefixtures("_versions") + def test_no_transforms_returns_unchanged(self): + """With no transforms registered, hook returns schema unchanged.""" + generator = SimpleNamespace(api_version="v1") + schema = { + "components": { + "schemas": { + "Course": {"properties": {"name": {"type": "string"}}}, + } + } + } + result = postprocess_versioned_schema(schema, generator, None, public=False) + assert result == schema + + @pytest.mark.usefixtures("_versions") + def test_no_components_returns_unchanged(self): + """Schema without components section should pass through.""" + generator = SimpleNamespace(api_version="v1") + schema = {"info": {"title": "API"}} + result = postprocess_versioned_schema(schema, generator, None, public=False) + assert result == schema + + def test_no_versions_configured(self, settings): + """With no allowed versions, hook should return unchanged.""" + settings.REST_FRAMEWORK = { + **getattr(settings, "REST_FRAMEWORK", {}), + "ALLOWED_VERSIONS": [], + } + generator = SimpleNamespace(api_version="v1") + schema = {"components": {"schemas": {}}} + result = postprocess_versioned_schema(schema, generator, None, public=False) + assert result == schema + + @pytest.mark.usefixtures("_versions") + def test_component_name_override(self): + """Transform with explicit component_name should target that name.""" + + class CustomNameTransform(Transform): + version = "v2" + description = "Uses custom component name" + serializer = "myapp.serializers.WeirdlyNamedSerializer" + component_name = "Course" + + def transform_schema(self, schema, direction): + if direction == "backwards": + props = schema.get("properties", {}) + if "new_field" in props: + del props["new_field"] + return schema + + generator = SimpleNamespace(api_version="v1") + schema = { + "components": { + "schemas": { + "Course": { + "properties": { + "name": {"type": "string"}, + "new_field": {"type": "string"}, + } + }, + "WeirdlyNamed": { + "properties": { + "name": {"type": "string"}, + "new_field": {"type": "string"}, + } + }, + } + } + } + result = postprocess_versioned_schema(schema, generator, None, public=False) + # Should target "Course" (component_name), not "WeirdlyNamed" (derived) + assert ( + "new_field" not in result["components"]["schemas"]["Course"]["properties"] + ) + assert ( + "new_field" in result["components"]["schemas"]["WeirdlyNamed"]["properties"] + ) + + @pytest.mark.usefixtures("_versions") + def test_class_based_serializer_reference(self): + """Transform with a class object as serializer should work in schema hook.""" + + class MySerializer: + pass + + class ClassRefTransform(Transform): + description = "Uses class ref" + serializer = "myapp.serializers.MySerializer" + + def transform_schema(self, schema, direction): + if direction == "backwards": + schema.get("properties", {}).pop("added", None) + return schema + + # Register with class ref for serializer matching, + # but use string for this test's schema targeting + ClassRefTransform.version = "v2" + ClassRefTransform.serializer = MySerializer + register_transform(ClassRefTransform) + + generator = SimpleNamespace(api_version="v1") + schema = { + "components": { + "schemas": { + "My": { + "properties": { + "name": {"type": "string"}, + "added": {"type": "string"}, + } + }, + } + } + } + result = postprocess_versioned_schema(schema, generator, None, public=False) + assert "added" not in result["components"]["schemas"]["My"]["properties"] diff --git a/tests/api_versioning/test_transforms.py b/tests/api_versioning/test_transforms.py new file mode 100644 index 00000000..8df39b7c --- /dev/null +++ b/tests/api_versioning/test_transforms.py @@ -0,0 +1,244 @@ +"""Tests for the Transform base class and auto-registration.""" + +import pytest +from mitol.api_versioning.transforms import Transform +from mitol.api_versioning.versions import ( + _registered_transforms, + _transform_registry, + get_transforms_for_version, +) + + +@pytest.fixture(autouse=True) +def _clear_registry(): + """Clear the transform registry before and after each test.""" + saved_registry = dict(_transform_registry) + saved_registered = set(_registered_transforms) + _transform_registry.clear() + _registered_transforms.clear() + yield + _transform_registry.clear() + _registered_transforms.clear() + _transform_registry.update(saved_registry) + _registered_transforms.update(saved_registered) + + +def test_transform_base_defaults(): + """Base Transform methods should return data unchanged.""" + t = Transform() + data = {"field": "value"} + assert t.to_representation(data, None, None) == data + assert t.to_internal_value(data, None) == data + assert t.transform_schema({"properties": {}}, "backwards") == {"properties": {}} + + +def test_metaclass_auto_registration(settings): + """Defining a Transform subclass with a version should auto-register it.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + + class MyTransform(Transform): + version = "v2" + description = "Test transform" + serializer = "myapp.serializers.MySerializer" + + assert MyTransform in get_transforms_for_version("v2") + + +def test_validation_rejects_missing_serializer(): + """Transform with version but no serializer should raise TypeError.""" + with pytest.raises(TypeError, match="must define a 'serializer'"): + + class BadTransform(Transform): + version = "v2" + description = "Missing serializer" + + +def test_validation_rejects_non_dotted_serializer(): + """Transform with a non-dotted serializer path should raise TypeError.""" + with pytest.raises(TypeError, match="not a dotted path"): + + class BadTransform(Transform): + version = "v2" + description = "Bad serializer path" + serializer = "NotADottedPath" + + +def test_validation_rejects_empty_version(): + """Transform with empty version string should raise TypeError.""" + with pytest.raises(TypeError, match="invalid version"): + + class BadTransform(Transform): + version = "" + serializer = "myapp.serializers.MySerializer" + + +def test_metaclass_no_registration_without_version(settings): + """A Transform subclass without a version should not be registered.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} + + class AbstractTransform(Transform): + description = "Not a real transform" + + for transforms in _transform_registry.values(): + assert AbstractTransform not in transforms + + +def test_concrete_transform_field_rename(settings): + """Test a concrete transform that renames a field.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + + class RenameFieldTransform(Transform): + version = "v2" + description = "Rename 'old_name' to 'new_name'" + serializer = "myapp.serializers.MySerializer" + + def to_representation(self, data, request, instance): # noqa: ARG002 + if "new_name" in data: + data["old_name"] = data.pop("new_name") + return data + + def to_internal_value(self, data, request): # noqa: ARG002 + if "old_name" in data: + data["new_name"] = data.pop("old_name") + return data + + def transform_schema(self, schema, direction): + props = schema.get("properties", {}) + if direction == "backwards" and "new_name" in props: + props["old_name"] = props.pop("new_name") + return schema + + t = RenameFieldTransform() + + data = {"new_name": "value", "other": "keep"} + result = t.to_representation(data, None, None) + assert result == {"old_name": "value", "other": "keep"} + + data = {"old_name": "value"} + result = t.to_internal_value(data, None) + assert result == {"new_name": "value"} + + schema = { + "properties": { + "new_name": {"type": "string"}, + "other": {"type": "int"}, + } + } + result = t.transform_schema(schema, "backwards") + assert "old_name" in result["properties"] + assert "new_name" not in result["properties"] + + +def test_concrete_transform_field_added(settings): + """Test a transform for a field added in a new version.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + + class AddFieldTransform(Transform): + version = "v2" + description = "Add 'new_field' in v2" + serializer = "myapp.serializers.MySerializer" + + def to_representation(self, data, request, instance): # noqa: ARG002 + data.pop("new_field", None) + return data + + def transform_schema(self, schema, direction): + if direction == "backwards": + schema.get("properties", {}).pop("new_field", None) + required = schema.get("required", []) + if "new_field" in required: + required.remove("new_field") + return schema + + t = AddFieldTransform() + + data = {"existing": "value", "new_field": "added"} + result = t.to_representation(data, None, None) + assert result == {"existing": "value"} + + schema = { + "properties": { + "existing": {"type": "string"}, + "new_field": {"type": "string"}, + }, + "required": ["existing", "new_field"], + } + result = t.transform_schema(schema, "backwards") + assert "new_field" not in result["properties"] + assert "new_field" not in result["required"] + + +def test_concrete_transform_field_removed(settings): + """Test a transform for a field removed in a new version.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + + class RemoveFieldTransform(Transform): + version = "v2" + description = "Remove 'legacy_field' in v2" + serializer = "myapp.serializers.MySerializer" + + def to_representation(self, data, request, instance): # noqa: ARG002 + data["legacy_field"] = getattr(instance, "legacy_field", None) + return data + + def to_internal_value(self, data, request): # noqa: ARG002 + data.pop("legacy_field", None) + return data + + def transform_schema(self, schema, direction): + if direction == "backwards": + schema.setdefault("properties", {})["legacy_field"] = {"type": "string"} + return schema + + t = RemoveFieldTransform() + + instance = type("Obj", (), {"legacy_field": "old_value"})() + data = {"current_field": "value"} + result = t.to_representation(data, None, instance) + assert result == { + "current_field": "value", + "legacy_field": "old_value", + } + + data = {"current_field": "value", "legacy_field": "old_value"} + result = t.to_internal_value(data, None) + assert result == {"current_field": "value"} + + +def test_concrete_transform_type_change(settings): + """Test a transform for a field whose type/structure changed.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + + class TypeChangeTransform(Transform): + version = "v2" + description = "Change 'topics' from list of strings to list of objects" + serializer = "myapp.serializers.MySerializer" + + def to_representation(self, data, request, instance): # noqa: ARG002 + if "topics" in data and isinstance(data["topics"], list): + data["topics"] = [ + t["name"] if isinstance(t, dict) else t for t in data["topics"] + ] + return data + + def to_internal_value(self, data, request): # noqa: ARG002 + if "topics" in data and isinstance(data["topics"], list): + data["topics"] = [ + {"name": t} if isinstance(t, str) else t for t in data["topics"] + ] + return data + + t = TypeChangeTransform() + + data = { + "topics": [ + {"id": 1, "name": "Math"}, + {"id": 2, "name": "Physics"}, + ] + } + result = t.to_representation(data, None, None) + assert result == {"topics": ["Math", "Physics"]} + + data = {"topics": ["Math", "Physics"]} + result = t.to_internal_value(data, None) + assert result == {"topics": [{"name": "Math"}, {"name": "Physics"}]} diff --git a/tests/api_versioning/test_versions.py b/tests/api_versioning/test_versions.py new file mode 100644 index 00000000..d277d128 --- /dev/null +++ b/tests/api_versioning/test_versions.py @@ -0,0 +1,165 @@ +"""Tests for the version registry.""" + +import pytest +from mitol.api_versioning.transforms import Transform +from mitol.api_versioning.versions import ( + _registered_transforms, + _transform_registry, + get_allowed_versions, + get_latest_version, + get_transforms_backwards, + get_transforms_between, + get_transforms_for_version, + get_transforms_forwards, + register_transform, +) + + +@pytest.fixture(autouse=True) +def _clear_registry(): + """Clear the transform registry before and after each test.""" + saved_registry = dict(_transform_registry) + saved_registered = set(_registered_transforms) + _transform_registry.clear() + _registered_transforms.clear() + yield + _transform_registry.clear() + _registered_transforms.clear() + _transform_registry.update(saved_registry) + _registered_transforms.update(saved_registered) + + +class DummySerializer: + """Dummy serializer class for testing.""" + + __module__ = "myapp.serializers" + __qualname__ = "DummySerializer" + + +class OtherSerializer: + """Another dummy serializer class for testing.""" + + __module__ = "myapp.serializers" + __qualname__ = "OtherSerializer" + + +def _make_transform(version, serializer_path): + """Create a Transform subclass for testing without triggering metaclass.""" + + class TestTransform(Transform): + pass + + TestTransform.version = version + TestTransform.serializer = serializer_path + register_transform(TestTransform) + return TestTransform + + +def test_get_allowed_versions(settings): + """Test reading ALLOWED_VERSIONS from DRF settings.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + assert get_allowed_versions() == ["v0", "v1", "v2"] + + +def test_get_latest_version(settings): + """Test that latest version is the last in the list.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} + assert get_latest_version() == "v1" + + +def test_get_latest_version_empty(settings): + """Test latest version with empty list returns None.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": []} + assert get_latest_version() is None + + +def test_register_and_get_transforms(settings): + """Test registering transforms and retrieving by version.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + t1 = _make_transform("v2", "myapp.serializers.DummySerializer") + t2 = _make_transform("v2", "myapp.serializers.OtherSerializer") + + result = get_transforms_for_version("v2") + assert t1 in result + assert t2 in result + assert get_transforms_for_version("v1") == [] + + +def test_get_transforms_between(settings): + """Test collecting transforms across a version range.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2", "v3"]} + t_v2 = _make_transform("v2", "myapp.serializers.DummySerializer") + t_v3 = _make_transform("v3", "myapp.serializers.DummySerializer") + + assert get_transforms_between("v1", "v3") == [t_v2, t_v3] + assert get_transforms_between("v0", "v2") == [t_v2] + assert get_transforms_between("v2", "v3") == [t_v3] + assert get_transforms_between("v2", "v2") == [] + + +def test_get_transforms_between_invalid_version(settings): + """Test that invalid versions return empty list.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} + assert get_transforms_between("v99", "v1") == [] + assert get_transforms_between("v0", "v99") == [] + + +def test_get_transforms_backwards(settings): + """Test backwards ordering (newest first) for response transforms.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2", "v3"]} + t_v2 = _make_transform("v2", "myapp.serializers.DummySerializer") + t_v3 = _make_transform("v3", "myapp.serializers.DummySerializer") + + result = get_transforms_backwards(DummySerializer, "v1") + assert result == [t_v3, t_v2] + + +def test_get_transforms_backwards_filters_by_serializer(settings): + """Test that backwards transforms filter by serializer class.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + _make_transform("v2", "myapp.serializers.DummySerializer") + t_other = _make_transform("v2", "myapp.serializers.OtherSerializer") + + result = get_transforms_backwards(OtherSerializer, "v1") + assert result == [t_other] + + +def test_get_transforms_backwards_latest_returns_empty(settings): + """Test that latest version gets no transforms.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} + _make_transform("v1", "myapp.serializers.DummySerializer") + + result = get_transforms_backwards(DummySerializer, "v1") + assert result == [] + + +def test_get_transforms_forwards(settings): + """Test forwards ordering (oldest first) for request transforms.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2", "v3"]} + t_v2 = _make_transform("v2", "myapp.serializers.DummySerializer") + t_v3 = _make_transform("v3", "myapp.serializers.DummySerializer") + + result = get_transforms_forwards(DummySerializer, "v1") + assert result == [t_v2, t_v3] + + +def test_get_transforms_forwards_latest_returns_empty(settings): + """Test that latest version gets no forwards transforms.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} + _make_transform("v1", "myapp.serializers.DummySerializer") + + result = get_transforms_forwards(DummySerializer, "v1") + assert result == [] + + +def test_register_transform_is_idempotent(settings): + """Test that registering the same transform twice only adds it once.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + t = _make_transform("v2", "myapp.serializers.DummySerializer") + + # Register again + register_transform(t) + register_transform(t) + + result = get_transforms_for_version("v2") + assert result.count(t) == 1 diff --git a/tests/olposthog/test_features.py b/tests/olposthog/test_features.py index 236ad545..1a6438db 100644 --- a/tests/olposthog/test_features.py +++ b/tests/olposthog/test_features.py @@ -23,15 +23,6 @@ """ -@pytest.fixture(autouse=True) -def posthog_settings(settings): - """Apply common settings and clear the cache for all feature flag tests.""" - settings.POSTHOG_ENABLED = True - settings.HOSTNAME = "fake_host_name" - settings.ENVIRONMENT = "prod" - caches["durable"].clear() - - def test_flags_from_cache(mocker, caplog, settings): """Test that flags are pulled from cache successfully.""" get_feature_flag_mock = mocker.patch( @@ -39,11 +30,15 @@ def test_flags_from_cache(mocker, caplog, settings): ) durable_cache = caches["durable"] settings.FEATURES["testing_function"] = True + settings.POSTHOG_ENABLED = True + settings.ENVIRONMENT = "prod" + settings.HOSTNAME = "fake_host_name" cache_key = features._generate_cache_key( # noqa: SLF001 "testing_function", features.default_unique_id(), features._get_person_properties(features.default_unique_id()), # noqa: SLF001 ) + durable_cache.clear() # Cache cleared, so we should hit Posthog. @@ -86,9 +81,16 @@ def test_cache_population(mocker, settings): }, ) + durable_cache = caches["durable"] + settings.FEATURES["testing_function_1"] = True settings.FEATURES["testing_function_2"] = True settings.FEATURES["testing_function_3"] = True + settings.POSTHOG_ENABLED = True + settings.ENVIRONMENT = "prod" + settings.HOSTNAME = "fake_host_name" + + durable_cache.clear() all_flags = features.get_all_feature_flags() @@ -107,10 +109,15 @@ def slow_flag(*_args, **_kwargs): mocker.patch("posthog.get_feature_flag", autospec=True, side_effect=slow_flag) durable_cache = caches["durable"] + settings.POSTHOG_ENABLED = True + settings.HOSTNAME = "fake_host_name" + settings.ENVIRONMENT = "prod" settings.FEATURES["testing_function"] = False settings.POSTHOG_CIRCUIT_BREAKER_COOLDOWN_SECONDS = 60 settings.POSTHOG_CIRCUIT_BREAKER_TRIP_THRESHOLD_SECONDS = 0 # trip immediately + durable_cache.clear() + with caplog.at_level(logging.DEBUG): result = features.is_enabled("testing_function") @@ -125,10 +132,15 @@ def test_circuit_breaker_skips_posthog_when_open(mocker, caplog, settings): get_feature_flag_mock = mocker.patch( "posthog.get_feature_flag", side_effect=lambda *_, **__: time.sleep(0.1) ) + durable_cache = caches["durable"] + settings.POSTHOG_ENABLED = True + settings.HOSTNAME = "fake_host_name" + settings.ENVIRONMENT = "prod" settings.FEATURES["testing_function"] = False settings.POSTHOG_CIRCUIT_BREAKER_COOLDOWN_SECONDS = 60 settings.POSTHOG_CIRCUIT_BREAKER_TRIP_THRESHOLD_SECONDS = 0 + durable_cache.clear() features.is_enabled("testing_function") # trips the circuit # Reset and verify the open circuit skips PostHog entirely @@ -144,34 +156,19 @@ def test_circuit_breaker_skips_posthog_when_open(mocker, caplog, settings): assert "circuit open" in caplog.text -def test_circuit_breaker_trips_on_exception(mocker, caplog, settings): - """Test that an unexpected PostHog exception trips the circuit and falls back.""" - mocker.patch( - "posthog.get_feature_flag", - autospec=True, - side_effect=RuntimeError("unexpected SDK error"), - ) - durable_cache = caches["durable"] - settings.FEATURES["testing_function"] = False - settings.POSTHOG_CIRCUIT_BREAKER_COOLDOWN_SECONDS = 60 - settings.POSTHOG_CIRCUIT_BREAKER_TRIP_THRESHOLD_SECONDS = 0 - - with caplog.at_level(logging.DEBUG): - result = features.is_enabled("testing_function") - - assert result is False - assert durable_cache.get(features.CIRCUIT_BREAKER_CACHE_KEY) is not None - - def test_circuit_breaker_closes_after_cooldown(mocker, settings): """Test that the circuit closes after the cooldown period expires.""" get_feature_flag_mock = mocker.patch( "posthog.get_feature_flag", autospec=True, return_value=True ) durable_cache = caches["durable"] + settings.POSTHOG_ENABLED = True + settings.HOSTNAME = "fake_host_name" + settings.ENVIRONMENT = "prod" settings.FEATURES["testing_function"] = False settings.POSTHOG_CIRCUIT_BREAKER_COOLDOWN_SECONDS = 60 + durable_cache.clear() durable_cache.set(features.CIRCUIT_BREAKER_CACHE_KEY, 1, 60) time_freezer = freeze_time(now_in_utc() + timedelta(seconds=61)) @@ -191,8 +188,14 @@ def test_posthog_flag_cache_timeout(mocker, settings): get_feature_flag_mock = mocker.patch( "posthog.get_feature_flag", autospec=True, return_value=True ) + durable_cache = caches["durable"] + settings.POSTHOG_ENABLED = True + settings.HOSTNAME = "fake_host_name" + settings.ENVIRONMENT = "prod" settings.FEATURES["testing_function"] = True + durable_cache.clear() + timeout = settings.CACHES["durable"].get("TIMEOUT", 300) time_freezer = freeze_time(now_in_utc() + timedelta(seconds=timeout * 2)) diff --git a/uv.lock b/uv.lock index f193da0f..a2192af8 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 3 +revision = 2 requires-python = ">=3.11" resolution-markers = [ "python_full_version >= '3.14'", @@ -15,6 +15,7 @@ conflicts = [[ [manifest] members = [ + "mitol-django-api-versioning", "mitol-django-apigateway", "mitol-django-authentication", "mitol-django-common", @@ -722,9 +723,9 @@ resolution-markers = [ "python_full_version < '3.13'", ] dependencies = [ - { name = "asgiref", marker = "extra == 'group-9-ol-django-django52' or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django50') or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django51') or (extra != 'group-9-ol-django-django42' and extra != 'group-9-ol-django-django50' and extra != 'group-9-ol-django-django51')" }, - { name = "sqlparse", marker = "extra == 'group-9-ol-django-django52' or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django50') or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django51') or (extra != 'group-9-ol-django-django42' and extra != 'group-9-ol-django-django50' and extra != 'group-9-ol-django-django51')" }, - { name = "tzdata", marker = "(sys_platform == 'win32' and extra != 'group-9-ol-django-django42' and extra != 'group-9-ol-django-django50' and extra != 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django50') or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django52') or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django52') or (extra == 'group-9-ol-django-django51' and extra == 'group-9-ol-django-django52')" }, + { name = "asgiref" }, + { name = "sqlparse" }, + { name = "tzdata", marker = "sys_platform == 'win32'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/1f/c5/c69e338eb2959f641045802e5ea87ca4bf5ac90c5fd08953ca10742fad51/django-5.2.13.tar.gz", hash = "sha256:a31589db5188d074c63f0945c3888fad104627dfcc236fb2b97f71f89da33bc4", size = 10890368, upload-time = "2026-04-07T14:02:15.072Z" } wheels = [ @@ -1555,9 +1556,27 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/b3/38/89ba8ad64ae25be8de66a6d463314cf1eb366222074cfda9ee839c56a4b4/mdurl-0.1.2-py3-none-any.whl", hash = "sha256:84008a41e51615a49fc9966191ff91509e3c40b939176e643fd50a5c2196b8f8", size = 9979, upload-time = "2022-08-14T12:40:09.779Z" }, ] +[[package]] +name = "mitol-django-api-versioning" +version = "2026.3.24" +source = { editable = "src/api_versioning" } +dependencies = [ + { name = "django", version = "4.2.30", source = { registry = "https://pypi.org/simple" }, marker = "extra == 'group-9-ol-django-django42' or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django52') or (extra == 'group-9-ol-django-django51' and extra == 'group-9-ol-django-django52')" }, + { name = "django", version = "5.0.14", source = { registry = "https://pypi.org/simple" }, marker = "extra == 'group-9-ol-django-django50' or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django52') or (extra == 'group-9-ol-django-django51' and extra == 'group-9-ol-django-django52')" }, + { name = "django", version = "5.1.15", source = { registry = "https://pypi.org/simple" }, marker = "extra == 'group-9-ol-django-django51' or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django50') or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django52') or (extra == 'group-9-ol-django-django42' and extra != 'group-9-ol-django-django51' and extra == 'group-9-ol-django-django52')" }, + { name = "django", version = "5.2.13", source = { registry = "https://pypi.org/simple" }, marker = "extra == 'group-9-ol-django-django52' or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django50') or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django51') or (extra != 'group-9-ol-django-django42' and extra != 'group-9-ol-django-django50' and extra != 'group-9-ol-django-django51')" }, + { name = "djangorestframework" }, +] + +[package.metadata] +requires-dist = [ + { name = "django", specifier = ">=3.0" }, + { name = "djangorestframework", specifier = ">=3.14" }, +] + [[package]] name = "mitol-django-apigateway" -version = "2026.4.2" +version = "2026.4.29" source = { editable = "src/apigateway" } dependencies = [ { name = "channels" }, @@ -1577,7 +1596,7 @@ requires-dist = [ [[package]] name = "mitol-django-authentication" -version = "2026.4.2" +version = "2026.4.29" source = { editable = "src/authentication" } dependencies = [ { name = "django", version = "4.2.30", source = { registry = "https://pypi.org/simple" }, marker = "extra == 'group-9-ol-django-django42' or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django52') or (extra == 'group-9-ol-django-django51' and extra == 'group-9-ol-django-django52')" }, @@ -1617,7 +1636,7 @@ provides-extras = ["touchstone"] [[package]] name = "mitol-django-common" -version = "2026.4.2" +version = "2026.4.29" source = { editable = "src/common" } dependencies = [ { name = "django", version = "4.2.30", source = { registry = "https://pypi.org/simple" }, marker = "extra == 'group-9-ol-django-django42' or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django52') or (extra == 'group-9-ol-django-django51' and extra == 'group-9-ol-django-django52')" }, @@ -2141,6 +2160,7 @@ name = "ol-django" version = "0.1.0" source = { editable = "." } dependencies = [ + { name = "mitol-django-api-versioning" }, { name = "mitol-django-apigateway" }, { name = "mitol-django-authentication" }, { name = "mitol-django-authentication", extra = ["touchstone"], marker = "python_full_version < '3.13' or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django50') or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django42' and extra == 'group-9-ol-django-django52') or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django51') or (extra == 'group-9-ol-django-django50' and extra == 'group-9-ol-django-django52') or (extra == 'group-9-ol-django-django51' and extra == 'group-9-ol-django-django52')" }, @@ -2214,6 +2234,7 @@ django52 = [ [package.metadata] requires-dist = [ + { name = "mitol-django-api-versioning", editable = "src/api_versioning" }, { name = "mitol-django-apigateway", editable = "src/apigateway" }, { name = "mitol-django-authentication", marker = "python_full_version >= '3.13'", editable = "src/authentication" }, { name = "mitol-django-authentication", extras = ["touchstone"], marker = "python_full_version < '3.13'", editable = "src/authentication" }, From 84b34669ca00bb69bbe7f409baecb1a6b65a2eb8 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 29 Apr 2026 12:31:37 -0400 Subject: [PATCH 2/6] Revert mystery changes --- tests/olposthog/test_features.py | 59 +++++++++++++++----------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/tests/olposthog/test_features.py b/tests/olposthog/test_features.py index 1a6438db..236ad545 100644 --- a/tests/olposthog/test_features.py +++ b/tests/olposthog/test_features.py @@ -23,6 +23,15 @@ """ +@pytest.fixture(autouse=True) +def posthog_settings(settings): + """Apply common settings and clear the cache for all feature flag tests.""" + settings.POSTHOG_ENABLED = True + settings.HOSTNAME = "fake_host_name" + settings.ENVIRONMENT = "prod" + caches["durable"].clear() + + def test_flags_from_cache(mocker, caplog, settings): """Test that flags are pulled from cache successfully.""" get_feature_flag_mock = mocker.patch( @@ -30,15 +39,11 @@ def test_flags_from_cache(mocker, caplog, settings): ) durable_cache = caches["durable"] settings.FEATURES["testing_function"] = True - settings.POSTHOG_ENABLED = True - settings.ENVIRONMENT = "prod" - settings.HOSTNAME = "fake_host_name" cache_key = features._generate_cache_key( # noqa: SLF001 "testing_function", features.default_unique_id(), features._get_person_properties(features.default_unique_id()), # noqa: SLF001 ) - durable_cache.clear() # Cache cleared, so we should hit Posthog. @@ -81,16 +86,9 @@ def test_cache_population(mocker, settings): }, ) - durable_cache = caches["durable"] - settings.FEATURES["testing_function_1"] = True settings.FEATURES["testing_function_2"] = True settings.FEATURES["testing_function_3"] = True - settings.POSTHOG_ENABLED = True - settings.ENVIRONMENT = "prod" - settings.HOSTNAME = "fake_host_name" - - durable_cache.clear() all_flags = features.get_all_feature_flags() @@ -109,15 +107,10 @@ def slow_flag(*_args, **_kwargs): mocker.patch("posthog.get_feature_flag", autospec=True, side_effect=slow_flag) durable_cache = caches["durable"] - settings.POSTHOG_ENABLED = True - settings.HOSTNAME = "fake_host_name" - settings.ENVIRONMENT = "prod" settings.FEATURES["testing_function"] = False settings.POSTHOG_CIRCUIT_BREAKER_COOLDOWN_SECONDS = 60 settings.POSTHOG_CIRCUIT_BREAKER_TRIP_THRESHOLD_SECONDS = 0 # trip immediately - durable_cache.clear() - with caplog.at_level(logging.DEBUG): result = features.is_enabled("testing_function") @@ -132,15 +125,10 @@ def test_circuit_breaker_skips_posthog_when_open(mocker, caplog, settings): get_feature_flag_mock = mocker.patch( "posthog.get_feature_flag", side_effect=lambda *_, **__: time.sleep(0.1) ) - durable_cache = caches["durable"] - settings.POSTHOG_ENABLED = True - settings.HOSTNAME = "fake_host_name" - settings.ENVIRONMENT = "prod" settings.FEATURES["testing_function"] = False settings.POSTHOG_CIRCUIT_BREAKER_COOLDOWN_SECONDS = 60 settings.POSTHOG_CIRCUIT_BREAKER_TRIP_THRESHOLD_SECONDS = 0 - durable_cache.clear() features.is_enabled("testing_function") # trips the circuit # Reset and verify the open circuit skips PostHog entirely @@ -156,19 +144,34 @@ def test_circuit_breaker_skips_posthog_when_open(mocker, caplog, settings): assert "circuit open" in caplog.text +def test_circuit_breaker_trips_on_exception(mocker, caplog, settings): + """Test that an unexpected PostHog exception trips the circuit and falls back.""" + mocker.patch( + "posthog.get_feature_flag", + autospec=True, + side_effect=RuntimeError("unexpected SDK error"), + ) + durable_cache = caches["durable"] + settings.FEATURES["testing_function"] = False + settings.POSTHOG_CIRCUIT_BREAKER_COOLDOWN_SECONDS = 60 + settings.POSTHOG_CIRCUIT_BREAKER_TRIP_THRESHOLD_SECONDS = 0 + + with caplog.at_level(logging.DEBUG): + result = features.is_enabled("testing_function") + + assert result is False + assert durable_cache.get(features.CIRCUIT_BREAKER_CACHE_KEY) is not None + + def test_circuit_breaker_closes_after_cooldown(mocker, settings): """Test that the circuit closes after the cooldown period expires.""" get_feature_flag_mock = mocker.patch( "posthog.get_feature_flag", autospec=True, return_value=True ) durable_cache = caches["durable"] - settings.POSTHOG_ENABLED = True - settings.HOSTNAME = "fake_host_name" - settings.ENVIRONMENT = "prod" settings.FEATURES["testing_function"] = False settings.POSTHOG_CIRCUIT_BREAKER_COOLDOWN_SECONDS = 60 - durable_cache.clear() durable_cache.set(features.CIRCUIT_BREAKER_CACHE_KEY, 1, 60) time_freezer = freeze_time(now_in_utc() + timedelta(seconds=61)) @@ -188,14 +191,8 @@ def test_posthog_flag_cache_timeout(mocker, settings): get_feature_flag_mock = mocker.patch( "posthog.get_feature_flag", autospec=True, return_value=True ) - durable_cache = caches["durable"] - settings.POSTHOG_ENABLED = True - settings.HOSTNAME = "fake_host_name" - settings.ENVIRONMENT = "prod" settings.FEATURES["testing_function"] = True - durable_cache.clear() - timeout = settings.CACHES["durable"].get("TIMEOUT", 300) time_freezer = freeze_time(now_in_utc() + timedelta(seconds=timeout * 2)) From 230f914509979ca0a2e66d7bceade72807ab5ee5 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 29 Apr 2026 14:00:50 -0400 Subject: [PATCH 3/6] Enhancements --- src/api_versioning/README.md | 308 +++++++++++++++++- .../mitol/api_versioning/apps.py | 2 + .../mitol/api_versioning/checks.py | 103 ++++++ .../mitol/api_versioning/mixins.py | 47 ++- .../mitol/api_versioning/schema_hooks.py | 12 +- .../mitol/api_versioning/transforms.py | 22 +- .../mitol/api_versioning/versions.py | 54 ++- tests/api_versioning/test_checks.py | 118 +++++++ tests/api_versioning/test_schema_hooks.py | 28 +- tests/api_versioning/test_transforms.py | 26 +- tests/api_versioning/test_versions.py | 12 + 11 files changed, 665 insertions(+), 67 deletions(-) create mode 100644 src/api_versioning/mitol/api_versioning/checks.py create mode 100644 tests/api_versioning/test_checks.py diff --git a/src/api_versioning/README.md b/src/api_versioning/README.md index a84e0c46..438c9b57 100644 --- a/src/api_versioning/README.md +++ b/src/api_versioning/README.md @@ -1,5 +1,309 @@ # mitol-django-api_versioning -API versioning framework for Django REST Framework with transform-based backwards compatibility. +Transform-based API versioning for Django REST Framework. -Allows breaking API changes to be introduced in new versions while older versions continue to work via automatic response/request transformations — without duplicating views or serializers. +When an API needs a breaking change, this library lets you keep one canonical +serializer (the latest version's shape) and describe the breaking change as a +small `Transform` class. Older clients keep working because the library +applies the transforms backwards on response and forwards on request; new +clients get the latest shape unmodified. The same approach also rewrites the +generated OpenAPI schema per version so generated TypeScript clients have +correct types. + +The design is modelled on [Stripe's API versioning approach](https://stripe.com/blog/api-versioning). +The advantage over per-version view/serializer modules is that bug fixes, +permissions, query plans, and serializer logic live in one place; the +disadvantage is that very behavioural divergence between versions (different +auth flows, different endpoints) is out of scope and should still use +per-version views. + +## Prerequisites + +- Django ≥ 3.0 +- Django REST Framework ≥ 3.14 +- drf-spectacular (only required if you want per-version OpenAPI schemas) + +DRF must be configured to use namespace-based versioning so each request +arrives with `request.version` set. + +## Installation + +```bash +pip install mitol-django-api_versioning +``` + +Add the app to `INSTALLED_APPS`: + +```python +INSTALLED_APPS = [ + ... + "mitol.api_versioning.apps.ApiVersioningApp", +] +``` + +The app's `ready()` hook auto-discovers a `transforms` submodule in every +installed app, so transforms registered in `/transforms.py` (or +`/transforms/__init__.py`) load at startup. + +## Configuration + +### DRF settings + +```python +REST_FRAMEWORK = { + "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.NamespaceVersioning", + "ALLOWED_VERSIONS": ["v0", "v1", "v2"], + ... +} +``` + +`ALLOWED_VERSIONS` is the source of truth. The last entry is the **latest +version**: serializers always produce that shape, and transforms run only when +the request version differs. + +### URL conf + +The same views serve every version; namespace-based versioning sets +`request.version` from the URL prefix: + +```python +v2_urls = v1_urls # same view callables, same routes + +urlpatterns = [ + re_path(r"^api/v2/", include((v2_urls, "v2"))), + re_path(r"^api/v1/", include((v1_urls, "v1"))), +] +``` + +### drf-spectacular settings (optional) + +If you want per-version OpenAPI schemas, register the postprocess hook: + +```python +SPECTACULAR_SETTINGS = { + ... + "POSTPROCESSING_HOOKS": [ + "mitol.api_versioning.schema_hooks.postprocess_versioned_schema", + ], +} +``` + +See [Per-version OpenAPI schemas](#per-version-openapi-schemas) below for the +URL conf that activates it. + +## Usage + +### Applying the mixin + +Put `VersionedSerializerMixin` first in the bases of any serializer that has +versioned transforms. The serializer always produces the latest version's +shape; the mixin intercepts `to_representation` / `to_internal_value` and +applies transforms when `request.version` is older. + +```python +from mitol.api_versioning.mixins import VersionedSerializerMixin +from rest_framework import serializers + +class CourseSerializer(VersionedSerializerMixin, serializers.ModelSerializer): + class Meta: + model = Course + fields = ["id", "title", "enrollment_url"] +``` + +The mixin's `__init_subclass__` raises `TypeError` at class-definition time if +it appears after the Serializer base class in the MRO, so misordered bases +fail loudly at import time. + +### Defining a transform + +Each breaking change is one `Transform` subclass. Implement only the methods +relevant to the change — `to_representation` for response data, +`to_internal_value` for request data, `transform_schema` for OpenAPI. + +```python +# learning_resources/transforms/v2.py + +from mitol.api_versioning.transforms import Transform + + +class RunAddEnrollmentUrl(Transform): + """v2 added enrollment_url to LearningResourceRunSerializer.""" + + version = "v2" + description = "Add enrollment_url field to Run" + serializer = "learning_resources.serializers.LearningResourceRunSerializer" + + def to_representation(self, data, request, instance): + # v1 clients didn't have enrollment_url + data.pop("enrollment_url", None) + return data + + def transform_schema(self, schema): + schema.get("properties", {}).pop("enrollment_url", None) + required = schema.get("required", []) + if "enrollment_url" in required: + required.remove("enrollment_url") + return schema +``` + +`version` is the version that **introduced** the breaking change. The +metaclass auto-registers the class with the registry, so simply defining it +is enough — no manual registration call. + +### `component_name` override + +`transform_schema` finds the OpenAPI component to mutate by stripping +`Serializer` from the class name (`MySerializer` → `My`). When a serializer +uses `@extend_schema(component_name=...)` or shares a component name with +another serializer, set `component_name` explicitly: + +```python +class CaptionUrlAddWordCount(Transform): + version = "v2" + description = "Add word_count to caption URL entries" + serializer = "learning_resources.serializers.VideoSerializer" + component_name = "CaptionUrl" # not "Video" + + def transform_schema(self, schema): + schema.get("properties", {}).pop("word_count", None) + ... +``` + +### Inherited serializers + +Registry lookup is **exact-class match** — a transform attached to a base +serializer does not automatically apply to subclasses. If both +`LearningResourceBaseDepartmentSerializer` and a derived +`LearningResourceDepartmentSerializer` rename the same field, declare two +transforms (or share the body via a no-`version` abstract base class so the +logic lives once): + +```python +class _DepartmentRenameBase(Transform): + # No `version` → not registered. Just shared logic. + description = "Rename channel_url to url" + + def to_representation(self, data, request, instance): + if "url" in data: + data["channel_url"] = data.pop("url") + return data + + +class DepartmentRenameChannelUrlToUrl(_DepartmentRenameBase): + version = "v2" + serializer = "learning_resources.serializers.LearningResourceBaseDepartmentSerializer" + + +class FullDepartmentRenameChannelUrlToUrl(_DepartmentRenameBase): + version = "v2" + serializer = "learning_resources.serializers.LearningResourceDepartmentSerializer" +``` + +### Non-DRF data (OpenSearch, raw dicts) + +For data that bypasses DRF serialization but should match a serializer's +output shape (e.g. OpenSearch hits), use `transform_dict_backwards`: + +```python +from mitol.api_versioning.mixins import transform_dict_backwards + +results = [ + transform_dict_backwards( + hit["_source"], + LearningResourceSerializer, + request, + recursive=True, # also walk nested serializer fields + ) + for hit in hits +] +``` + +## Per-version OpenAPI schemas + +drf-spectacular's `SpectacularAPIView` accepts an `api_version` kwarg that it +threads onto the generator. The postprocessing hook reads that and rewrites +the schema per version: + +```python +# openapi/urls.py +from django.urls import path +from drf_spectacular.views import ( + SpectacularAPIView, + SpectacularSwaggerView, + SpectacularRedocView, +) + +urlpatterns = [ + path("api/v1/schema/", + SpectacularAPIView.as_view(api_version="v1"), + name="v1_schema"), + path("api/v1/schema/swagger-ui/", + SpectacularSwaggerView.as_view(url_name="v1_schema"), + name="v1_swagger_ui"), + path("api/v2/schema/", + SpectacularAPIView.as_view(api_version="v2"), + name="v2_schema"), + ... +] +``` + +Each version's schema URL serves a fully transformed OpenAPI document; point +your client generator at it. + +## Debugging + +When a transform doesn't fire, three things will tell you why: + +1. **`./manage.py check`** runs three system checks at startup + (`api_versioning.E001` / `W001` / `E002`). They flag misconfigured + `serializer` paths (typos, drift) and versions outside `ALLOWED_VERSIONS`. +2. **`list_transforms_for_serializer(SerializerClass)`** returns every + registered transform for that serializer, ordered oldest-first. Useful in + a Python shell to confirm registration. +3. **`log.debug` output** from the mixin: enable `DEBUG` logging on + `mitol.api_versioning.mixins` to see the chain of transforms applied to + each request, with version transitions. + +## Limitations + +This library is scoped to **field-level shape evolution** — renames, additions, +removals, type coercions. Out of scope: + +- Different business logic per version (different auth, different ORM access) +- Endpoints that exist in some versions but not others +- Pre-serialization queryset planning (`select_related` / `prefetch_related`) + — see below. + +### Pre-serialization queryset planning + +Transforms run *after* DRF has already serialized the instance, so they can't +add ORM hints to the queryset. If an older version exposes nested data the +latest version no longer includes (or no longer prefetches), the older +version's response can degrade into N+1 queries. + +The fix lives in the view, not the transform: branch on `request.version` in +`get_queryset()` and only add the extra prefetches for versions that still +need them. The latest version pays nothing. + +```python +class CourseViewSet(viewsets.ReadOnlyModelViewSet): + serializer_class = CourseSerializer + + def get_queryset(self): + queryset = Course.objects.all() + if getattr(self.request, "version", None) == "v1": + # v1 still embeds nested runs; v2 returns runs_url instead + queryset = queryset.prefetch_related("runs__instructors") + return queryset +``` + +When an older version needs dramatically different loading behaviour, that's +a sign the change has crossed from shape-evolution into behavioural divergence +and a separate versioned view is probably cleaner. + +### Transform-chain growth + +The transform chain grows over time. Each new version adds transforms, and a +v1 request against a v5-latest serializer applies four sets of transforms in +sequence. Test every supported version against every endpoint with transforms +to catch regressions where transforms interact badly. diff --git a/src/api_versioning/mitol/api_versioning/apps.py b/src/api_versioning/mitol/api_versioning/apps.py index 7449236f..def4dea7 100644 --- a/src/api_versioning/mitol/api_versioning/apps.py +++ b/src/api_versioning/mitol/api_versioning/apps.py @@ -7,6 +7,8 @@ from django.apps import AppConfig, apps from django.utils.module_loading import module_has_submodule +from . import checks # noqa: F401 (registers Django system checks on import) + log = logging.getLogger(__name__) diff --git a/src/api_versioning/mitol/api_versioning/checks.py b/src/api_versioning/mitol/api_versioning/checks.py new file mode 100644 index 00000000..ee5488bd --- /dev/null +++ b/src/api_versioning/mitol/api_versioning/checks.py @@ -0,0 +1,103 @@ +"""Django system checks for the api_versioning app. + +Validates that every registered transform's ``serializer`` attribute +points at a real serializer class and uses a version that appears in +``REST_FRAMEWORK["ALLOWED_VERSIONS"]``. Catches typos and rename-drift +that would otherwise silently no-op at request time. + +Run via ``./manage.py check`` (also runs as part of Django startup). +""" + +from django.core.checks import Error, register +from django.core.checks import Warning as DjangoWarning +from django.utils.module_loading import import_string +from mitol.api_versioning.versions import ( + _transform_registry, + get_allowed_versions, +) + + +@register() +def check_transform_serializer_paths(app_configs, **kwargs): # noqa: ARG001 + """Validate every registered transform. + + Emits: + - api_versioning.E001 — ``serializer`` dotted path does not resolve. + - api_versioning.W001 — resolved class's canonical path differs from + the declared one (re-export drift; lookup may still work). + - api_versioning.E002 — ``version`` is not in ``ALLOWED_VERSIONS``. + """ + issues = [] + allowed_versions = set(get_allowed_versions()) + + for transforms in _transform_registry.values(): + for transform_cls in transforms: + issues.extend(_check_one(transform_cls, allowed_versions)) + + return issues + + +def _check_one(transform_cls, allowed_versions): + issues = [] + serializer_ref = transform_cls.serializer + + if isinstance(serializer_ref, str): + try: + resolved = import_string(serializer_ref) + except (ImportError, AttributeError) as exc: + issues.append( + Error( + ( + f"Transform {transform_cls.__qualname__!r} references " + f"unresolvable serializer {serializer_ref!r}: {exc}" + ), + hint=( + "Check the dotted path or rename the transform's " + "`serializer` attribute to match the actual class." + ), + obj=transform_cls, + id="api_versioning.E001", + ) + ) + resolved = None + + if resolved is not None: + canonical = f"{resolved.__module__}.{resolved.__qualname__}" + if canonical != serializer_ref: + issues.append( + DjangoWarning( + ( + f"Transform {transform_cls.__qualname__!r} declares " + f"serializer={serializer_ref!r} but the resolved " + f"class's canonical path is {canonical!r}. " + f"Runtime lookup uses the canonical path; the " + f"transform may not fire." + ), + hint=( + f"Update the transform's `serializer` attribute to " + f"{canonical!r}." + ), + obj=transform_cls, + id="api_versioning.W001", + ) + ) + + if allowed_versions and transform_cls.version not in allowed_versions: + issues.append( + Error( + ( + f"Transform {transform_cls.__qualname__!r} has " + f"version={transform_cls.version!r} which is not in " + f"REST_FRAMEWORK['ALLOWED_VERSIONS'] " + f"({sorted(allowed_versions)!r})." + ), + hint=( + "Add the version to ALLOWED_VERSIONS or fix the " + "transform's `version` attribute." + ), + obj=transform_cls, + id="api_versioning.E002", + ) + ) + + return issues diff --git a/src/api_versioning/mitol/api_versioning/mixins.py b/src/api_versioning/mitol/api_versioning/mixins.py index dd03f9ca..72674829 100644 --- a/src/api_versioning/mitol/api_versioning/mixins.py +++ b/src/api_versioning/mitol/api_versioning/mixins.py @@ -20,6 +20,7 @@ class Meta: """ import logging +from typing import Any from mitol.api_versioning.versions import ( get_latest_version, @@ -61,10 +62,10 @@ def __init_subclass__(cls, **kwargs): ) raise TypeError(msg) - def to_representation(self, instance): + def to_representation(self, instance: Any) -> dict[str, Any]: """Serialize instance, then apply backwards transforms for older versions.""" - data = super().to_representation(instance) - request = self.context.get("request") + data = super().to_representation(instance) # type: ignore[misc] + request = self.context.get("request") # type: ignore[attr-defined] if not request or not hasattr(request, "version"): return data @@ -72,7 +73,9 @@ def to_representation(self, instance): if not latest or request.version == latest: return data - transforms = get_transforms_backwards(self.__class__, request.version) + transforms = get_transforms_backwards( + self.__class__, request.version, latest=latest + ) if transforms: log.debug( "to_representation: %s version=%s -> %s, applying %d transform(s): %s", @@ -87,14 +90,16 @@ def to_representation(self, instance): return data - def to_internal_value(self, data): + def to_internal_value(self, data: Any) -> dict[str, Any]: """Apply forwards transforms for older versions, then validate.""" - request = self.context.get("request") + request = self.context.get("request") # type: ignore[attr-defined] if request and hasattr(request, "version"): latest = get_latest_version() if latest and request.version != latest: data = data.copy() if hasattr(data, "copy") else dict(data) - transforms = get_transforms_forwards(self.__class__, request.version) + transforms = get_transforms_forwards( + self.__class__, request.version, latest=latest + ) if transforms: log.debug( ( @@ -110,10 +115,16 @@ def to_internal_value(self, data): for transform_cls in transforms: data = transform_cls().to_internal_value(data, request) - return super().to_internal_value(data) + return super().to_internal_value(data) # type: ignore[misc] -def transform_dict_backwards(data, serializer_class, request, *, recursive=False): +def transform_dict_backwards( + data: dict[str, Any], + serializer_class: type, + request: Any, + *, + recursive: bool = False, +) -> dict[str, Any]: """Apply backwards transforms to a raw dict (no model instance). Use this for data that bypasses DRF serialization, such as @@ -138,9 +149,11 @@ def transform_dict_backwards(data, serializer_class, request, *, recursive=False return data if recursive: - _transform_nested_fields(data, serializer_class, request) + _transform_nested_fields(data, serializer_class, request, latest=latest) - transforms = get_transforms_backwards(serializer_class, request.version) + transforms = get_transforms_backwards( + serializer_class, request.version, latest=latest + ) if transforms: log.debug( "transform_dict_backwards: %s version=%s, applying %d transform(s): %s", @@ -167,7 +180,7 @@ def _apply_transforms_to_data(transforms, data, request): transform_cls().to_representation(item, request, instance=None) -def _transform_nested_fields(data, serializer_class, request): +def _transform_nested_fields(data, serializer_class, request, *, latest=None): """Recursively apply transforms to nested serializer fields. Introspects the serializer's declared fields. For each field that @@ -196,14 +209,18 @@ def _transform_nested_fields(data, serializer_class, request): # Recurse first so deepest fields are transformed before their parents if isinstance(nested_data, dict): - _transform_nested_fields(nested_data, child_serializer_class, request) + _transform_nested_fields( + nested_data, child_serializer_class, request, latest=latest + ) elif isinstance(nested_data, list): for item in nested_data: if isinstance(item, dict): - _transform_nested_fields(item, child_serializer_class, request) + _transform_nested_fields( + item, child_serializer_class, request, latest=latest + ) child_transforms = get_transforms_backwards( - child_serializer_class, request.version + child_serializer_class, request.version, latest=latest ) if child_transforms: _apply_transforms_to_data(child_transforms, nested_data, request) diff --git a/src/api_versioning/mitol/api_versioning/schema_hooks.py b/src/api_versioning/mitol/api_versioning/schema_hooks.py index 22aa854d..b5303c8e 100644 --- a/src/api_versioning/mitol/api_versioning/schema_hooks.py +++ b/src/api_versioning/mitol/api_versioning/schema_hooks.py @@ -11,6 +11,7 @@ import logging import re +from typing import Any from mitol.api_versioning.versions import ( get_latest_version, @@ -20,7 +21,7 @@ log = logging.getLogger(__name__) -def _resolve_schema_name(serializer_ref): +def _resolve_schema_name(serializer_ref: Any) -> str: """Convert a serializer reference to the drf-spectacular schema name. Accepts either a dotted path string or a class object. @@ -64,7 +65,12 @@ def _get_all_schema_variants(base_name, schemas): return [name for name in schemas if name in expected] -def postprocess_versioned_schema(result, generator, request, public): # noqa: ARG001 +def postprocess_versioned_schema( + result: dict[str, Any], + generator: Any, + request: Any, # noqa: ARG001 + public: bool, # noqa: ARG001, FBT001 +) -> dict[str, Any]: """Postprocessing hook for drf-spectacular. When generating the schema for an older API version, applies all @@ -112,7 +118,7 @@ def postprocess_versioned_schema(result, generator, request, public): # noqa: A for schema_name in variant_names: schemas[schema_name] = transform_cls().transform_schema( - schemas[schema_name], direction="backwards" + schemas[schema_name] ) return result diff --git a/src/api_versioning/mitol/api_versioning/transforms.py b/src/api_versioning/mitol/api_versioning/transforms.py index a20463f1..c4efa9be 100644 --- a/src/api_versioning/mitol/api_versioning/transforms.py +++ b/src/api_versioning/mitol/api_versioning/transforms.py @@ -7,6 +7,8 @@ Transforms auto-register with the version registry via a metaclass. """ +from typing import Any + from mitol.api_versioning.versions import register_transform @@ -76,12 +78,17 @@ class Transform(metaclass=TransformMeta): - transform_schema: for OpenAPI schema transforms """ - version: str = None + version: str | None = None description: str = "" - serializer: str = None - component_name: str = None - - def to_representation(self, data, request, instance): # noqa: ARG002 + serializer: str | type | None = None + component_name: str | None = None + + def to_representation( + self, + data: dict[str, Any], + request: Any, # noqa: ARG002 + instance: Any, # noqa: ARG002 + ) -> dict[str, Any]: """Transform response data backwards (latest version -> older version). Called when a client requests an older API version. Mutates `data` @@ -97,7 +104,7 @@ def to_representation(self, data, request, instance): # noqa: ARG002 """ return data - def to_internal_value(self, data, request): # noqa: ARG002 + def to_internal_value(self, data: dict[str, Any], request: Any) -> dict[str, Any]: # noqa: ARG002 """Transform request data forwards (older version -> latest version). Called when a client sends data using an older API version format. @@ -112,7 +119,7 @@ def to_internal_value(self, data, request): # noqa: ARG002 """ return data - def transform_schema(self, schema, direction): # noqa: ARG002 + def transform_schema(self, schema: dict[str, Any]) -> dict[str, Any]: """Transform an OpenAPI schema component for version compatibility. Called during OpenAPI spec generation to produce correct schemas @@ -121,7 +128,6 @@ def transform_schema(self, schema, direction): # noqa: ARG002 Args: schema: The OpenAPI schema dict for a component. - direction: "backwards" when generating schema for an older version. Returns: The transformed schema dict. diff --git a/src/api_versioning/mitol/api_versioning/versions.py b/src/api_versioning/mitol/api_versioning/versions.py index a56a1d06..e6f2faf1 100644 --- a/src/api_versioning/mitol/api_versioning/versions.py +++ b/src/api_versioning/mitol/api_versioning/versions.py @@ -6,16 +6,20 @@ """ from collections import defaultdict +from typing import TYPE_CHECKING from django.conf import settings +if TYPE_CHECKING: + from mitol.api_versioning.transforms import Transform -def get_allowed_versions(): + +def get_allowed_versions() -> list[str]: """Get the ordered list of allowed API versions from DRF settings.""" return settings.REST_FRAMEWORK.get("ALLOWED_VERSIONS", []) -def get_latest_version(): +def get_latest_version() -> str | None: """Get the latest (newest) API version.""" versions = get_allowed_versions() return versions[-1] if versions else None @@ -27,7 +31,7 @@ def get_latest_version(): _registered_transforms = set() -def register_transform(transform_cls): +def register_transform(transform_cls: type["Transform"]) -> None: """ Register a transform class for its declared version. """ @@ -37,12 +41,14 @@ def register_transform(transform_cls): _transform_registry[transform_cls.version].append(transform_cls) -def get_transforms_for_version(version): +def get_transforms_for_version(version: str) -> list[type["Transform"]]: """Get all transforms introduced in a specific version.""" return list(_transform_registry.get(version, [])) -def get_transforms_between(from_version, to_version): +def get_transforms_between( + from_version: str, to_version: str +) -> list[type["Transform"]]: """Get all transforms for versions > from_version and <= to_version. Ordered by version (oldest first). Used to collect all transforms @@ -61,13 +67,19 @@ def get_transforms_between(from_version, to_version): return transforms -def get_transforms_backwards(serializer_class, request_version): +def get_transforms_backwards( + serializer_class: type, + request_version: str, + *, + latest: str | None = None, +) -> list[type["Transform"]]: """Get transforms to apply backwards (newest first) for a serializer. Returns transform classes whose serializer matches the given class (by dotted path or direct class reference), ordered newest-version-first. """ - latest = get_latest_version() + if latest is None: + latest = get_latest_version() if not latest or request_version == latest: return [] @@ -80,13 +92,37 @@ def get_transforms_backwards(serializer_class, request_version): return list(reversed(matching)) -def get_transforms_forwards(serializer_class, request_version): +def list_transforms_for_serializer(serializer_class: type) -> list[type["Transform"]]: + """List every registered transform whose serializer matches a class. + + Debugging helper. Returns transforms across all versions, ordered + oldest-version-first. Each entry is the transform class itself; use + ``cls.version`` and ``cls.description`` for human-readable output. + """ + serializer_path = f"{serializer_class.__module__}.{serializer_class.__qualname__}" + matching = [] + for version in get_allowed_versions(): + matching.extend( + transform_cls + for transform_cls in _transform_registry.get(version, []) + if transform_cls.serializer in {serializer_path, serializer_class} + ) + return matching + + +def get_transforms_forwards( + serializer_class: type, + request_version: str, + *, + latest: str | None = None, +) -> list[type["Transform"]]: """Get transforms to apply forwards (oldest first) for a serializer. Returns transform classes whose serializer matches the given class, ordered oldest-version-first. """ - latest = get_latest_version() + if latest is None: + latest = get_latest_version() if not latest or request_version == latest: return [] diff --git a/tests/api_versioning/test_checks.py b/tests/api_versioning/test_checks.py new file mode 100644 index 00000000..eec86129 --- /dev/null +++ b/tests/api_versioning/test_checks.py @@ -0,0 +1,118 @@ +"""Tests for the api_versioning Django system checks.""" + +import pytest +from mitol.api_versioning import checks as checks_module +from mitol.api_versioning.checks import check_transform_serializer_paths +from mitol.api_versioning.transforms import Transform +from mitol.api_versioning.versions import ( + _registered_transforms, + _transform_registry, +) + + +@pytest.fixture(autouse=True) +def _clear_registry(): + """Clear the transform registry before and after each test.""" + saved_registry = dict(_transform_registry) + saved_registered = set(_registered_transforms) + _transform_registry.clear() + _registered_transforms.clear() + yield + _transform_registry.clear() + _registered_transforms.clear() + _transform_registry.update(saved_registry) + _registered_transforms.update(saved_registered) + + +# A real serializer-shaped class we can resolve via dotted path. +class _ResolvableSerializer: + pass + + +_RESOLVABLE_PATH = f"{__name__}._ResolvableSerializer" + + +def test_no_transforms_passes(settings): + """No registered transforms → no issues.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} + assert check_transform_serializer_paths(app_configs=None) == [] + + +def test_resolvable_path_passes(settings): + """A transform whose serializer resolves cleanly → no issues.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + + class GoodTransform(Transform): + version = "v2" + description = "ok" + serializer = _RESOLVABLE_PATH + + issues = check_transform_serializer_paths(app_configs=None) + assert issues == [] + + +def test_unresolvable_path_emits_e001(settings): + """Bad dotted path → E001 Error.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + + class BadTransform(Transform): + version = "v2" + description = "broken path" + serializer = "myapp.does_not_exist.NopeSerializer" + + issues = check_transform_serializer_paths(app_configs=None) + ids = [i.id for i in issues] + assert "api_versioning.E001" in ids + + +def test_canonical_mismatch_emits_w001(settings): + """Path resolves but canonical name differs → W001 Warning.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + + # Re-export the class under a different module path (simulate + # `from .real import X` in another module). We can't easily set up + # a real re-export at test time, so simulate by giving the class a + # __qualname__ that doesn't match the path. + class MismatchTransform(Transform): + version = "v2" + description = "alias path" + serializer = "tests.api_versioning.test_checks._ResolvableSerializer" + + # The class lives at this module path. Its canonical + # __module__.__qualname__ should match for this test to pass; flip the + # declared string to simulate drift. + MismatchTransform.serializer = ( + "tests.api_versioning.test_checks.AliasResolvableSerializer" + ) + + # Patch import_string to resolve to our class anyway, so we hit the + # canonical-mismatch branch. + real_import_string = checks_module.import_string + + def fake_import_string(path): + if path == "tests.api_versioning.test_checks.AliasResolvableSerializer": + return _ResolvableSerializer + return real_import_string(path) + + checks_module.import_string = fake_import_string + try: + issues = check_transform_serializer_paths(app_configs=None) + finally: + checks_module.import_string = real_import_string + + ids = [i.id for i in issues] + assert "api_versioning.W001" in ids + + +def test_version_not_in_allowed_emits_e002(settings): + """Version outside ALLOWED_VERSIONS → E002 Error.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} + + class WrongVersionTransform(Transform): + version = "v9" + description = "future version" + serializer = _RESOLVABLE_PATH + + issues = check_transform_serializer_paths(app_configs=None) + ids = [i.id for i in issues] + assert "api_versioning.E002" in ids diff --git a/tests/api_versioning/test_schema_hooks.py b/tests/api_versioning/test_schema_hooks.py index 15aa2454..fdf1e46a 100644 --- a/tests/api_versioning/test_schema_hooks.py +++ b/tests/api_versioning/test_schema_hooks.py @@ -138,11 +138,10 @@ class RenameFieldTransform(Transform): description = "Rename name to title" serializer = "myapp.serializers.CourseSerializer" - def transform_schema(self, schema, direction): - if direction == "backwards": - props = schema.get("properties", {}) - if "name" in props: - props["title"] = props.pop("name") + def transform_schema(self, schema): + props = schema.get("properties", {}) + if "name" in props: + props["title"] = props.pop("name") return schema generator = SimpleNamespace(api_version="v1") @@ -173,9 +172,8 @@ class AddFieldTransform(Transform): description = "Add extra_field in v2" serializer = "myapp.serializers.CourseSerializer" - def transform_schema(self, schema, direction): - if direction == "backwards": - schema.get("properties", {}).pop("extra_field", None) + def transform_schema(self, schema): + schema.get("properties", {}).pop("extra_field", None) return schema generator = SimpleNamespace(api_version="v1") @@ -247,11 +245,10 @@ class CustomNameTransform(Transform): serializer = "myapp.serializers.WeirdlyNamedSerializer" component_name = "Course" - def transform_schema(self, schema, direction): - if direction == "backwards": - props = schema.get("properties", {}) - if "new_field" in props: - del props["new_field"] + def transform_schema(self, schema): + props = schema.get("properties", {}) + if "new_field" in props: + del props["new_field"] return schema generator = SimpleNamespace(api_version="v1") @@ -293,9 +290,8 @@ class ClassRefTransform(Transform): description = "Uses class ref" serializer = "myapp.serializers.MySerializer" - def transform_schema(self, schema, direction): - if direction == "backwards": - schema.get("properties", {}).pop("added", None) + def transform_schema(self, schema): + schema.get("properties", {}).pop("added", None) return schema # Register with class ref for serializer matching, diff --git a/tests/api_versioning/test_transforms.py b/tests/api_versioning/test_transforms.py index 8df39b7c..f599d8c1 100644 --- a/tests/api_versioning/test_transforms.py +++ b/tests/api_versioning/test_transforms.py @@ -29,7 +29,7 @@ def test_transform_base_defaults(): data = {"field": "value"} assert t.to_representation(data, None, None) == data assert t.to_internal_value(data, None) == data - assert t.transform_schema({"properties": {}}, "backwards") == {"properties": {}} + assert t.transform_schema({"properties": {}}) == {"properties": {}} def test_metaclass_auto_registration(settings): @@ -102,9 +102,9 @@ def to_internal_value(self, data, request): # noqa: ARG002 data["new_name"] = data.pop("old_name") return data - def transform_schema(self, schema, direction): + def transform_schema(self, schema): props = schema.get("properties", {}) - if direction == "backwards" and "new_name" in props: + if "new_name" in props: props["old_name"] = props.pop("new_name") return schema @@ -124,7 +124,7 @@ def transform_schema(self, schema, direction): "other": {"type": "int"}, } } - result = t.transform_schema(schema, "backwards") + result = t.transform_schema(schema) assert "old_name" in result["properties"] assert "new_name" not in result["properties"] @@ -142,12 +142,11 @@ def to_representation(self, data, request, instance): # noqa: ARG002 data.pop("new_field", None) return data - def transform_schema(self, schema, direction): - if direction == "backwards": - schema.get("properties", {}).pop("new_field", None) - required = schema.get("required", []) - if "new_field" in required: - required.remove("new_field") + def transform_schema(self, schema): + schema.get("properties", {}).pop("new_field", None) + required = schema.get("required", []) + if "new_field" in required: + required.remove("new_field") return schema t = AddFieldTransform() @@ -163,7 +162,7 @@ def transform_schema(self, schema, direction): }, "required": ["existing", "new_field"], } - result = t.transform_schema(schema, "backwards") + result = t.transform_schema(schema) assert "new_field" not in result["properties"] assert "new_field" not in result["required"] @@ -185,9 +184,8 @@ def to_internal_value(self, data, request): # noqa: ARG002 data.pop("legacy_field", None) return data - def transform_schema(self, schema, direction): - if direction == "backwards": - schema.setdefault("properties", {})["legacy_field"] = {"type": "string"} + def transform_schema(self, schema): + schema.setdefault("properties", {})["legacy_field"] = {"type": "string"} return schema t = RemoveFieldTransform() diff --git a/tests/api_versioning/test_versions.py b/tests/api_versioning/test_versions.py index d277d128..eabf688d 100644 --- a/tests/api_versioning/test_versions.py +++ b/tests/api_versioning/test_versions.py @@ -11,6 +11,7 @@ get_transforms_between, get_transforms_for_version, get_transforms_forwards, + list_transforms_for_serializer, register_transform, ) @@ -152,6 +153,17 @@ def test_get_transforms_forwards_latest_returns_empty(settings): assert result == [] +def test_list_transforms_for_serializer(settings): + """Test that list_transforms_for_serializer returns matching transforms.""" + settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2", "v3"]} + t_v2 = _make_transform("v2", "myapp.serializers.DummySerializer") + t_v3 = _make_transform("v3", "myapp.serializers.DummySerializer") + _make_transform("v2", "myapp.serializers.OtherSerializer") + + result = list_transforms_for_serializer(DummySerializer) + assert result == [t_v2, t_v3] + + def test_register_transform_is_idempotent(settings): """Test that registering the same transform twice only adds it once.""" settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} From ff981b76a990084c5604780ce4e4881bcec39b82 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 29 Apr 2026 14:50:33 -0400 Subject: [PATCH 4/6] Enhancements --- src/api_versioning/README.md | 5 + .../mitol/api_versioning/mixins.py | 60 +++++-- .../mitol/api_versioning/transforms.py | 20 ++- tests/api_versioning/test_mixins.py | 151 ++++++++++++++++++ 4 files changed, 221 insertions(+), 15 deletions(-) diff --git a/src/api_versioning/README.md b/src/api_versioning/README.md index 438c9b57..1d079d02 100644 --- a/src/api_versioning/README.md +++ b/src/api_versioning/README.md @@ -150,6 +150,11 @@ class RunAddEnrollmentUrl(Transform): metaclass auto-registers the class with the registry, so simply defining it is enough — no manual registration call. +`to_representation` and `to_internal_value` may either mutate `data` in place +and return it, or return a new dict — both patterns work, including for +nested transforms applied via `transform_dict_backwards(..., recursive=True)`. +The only invalid return is `None`, which raises `TypeError`. + ### `component_name` override `transform_schema` finds the OpenAPI component to mutate by stripping diff --git a/src/api_versioning/mitol/api_versioning/mixins.py b/src/api_versioning/mitol/api_versioning/mixins.py index 72674829..21b53f3a 100644 --- a/src/api_versioning/mitol/api_versioning/mixins.py +++ b/src/api_versioning/mitol/api_versioning/mixins.py @@ -86,7 +86,10 @@ def to_representation(self, instance: Any) -> dict[str, Any]: " -> ".join(t.__name__ for t in transforms), ) for transform_cls in transforms: - data = transform_cls().to_representation(data, request, instance) + result = transform_cls().to_representation(data, request, instance) + data = _check_not_none( + transform_cls, result, "VersionedSerializerMixin.to_representation" + ) return data @@ -113,7 +116,12 @@ def to_internal_value(self, data: Any) -> dict[str, Any]: " -> ".join(t.__name__ for t in transforms), ) for transform_cls in transforms: - data = transform_cls().to_internal_value(data, request) + result = transform_cls().to_internal_value(data, request) + data = _check_not_none( + transform_cls, + result, + "VersionedSerializerMixin.to_internal_value", + ) return super().to_internal_value(data) # type: ignore[misc] @@ -163,21 +171,51 @@ def transform_dict_backwards( " -> ".join(t.__name__ for t in transforms), ) for transform_cls in transforms: - data = transform_cls().to_representation(data, request, instance=None) + result = transform_cls().to_representation(data, request, instance=None) + data = _check_not_none(transform_cls, result, "transform_dict_backwards") return data def _apply_transforms_to_data(transforms, data, request): - """Apply a list of transform classes to a dict or list of dicts.""" + """Apply a list of transform classes to a dict or list of dicts. + + Returns the (possibly new) data. Each transform may either mutate the + input in place and return it, or return a new dict; both are supported. + For lists, each item is rebound by index so a transform that returns a + new dict for a list item replaces it in place. + """ if isinstance(data, dict): for transform_cls in transforms: - transform_cls().to_representation(data, request, instance=None) + result = transform_cls().to_representation(data, request, instance=None) + data = _check_not_none(transform_cls, result, "_apply_transforms_to_data") elif isinstance(data, list): - for item in data: - if isinstance(item, dict): - for transform_cls in transforms: - transform_cls().to_representation(item, request, instance=None) + for i, item in enumerate(data): + if not isinstance(item, dict): + continue + for transform_cls in transforms: + result = transform_cls().to_representation(item, request, instance=None) + item = _check_not_none( # noqa: PLW2901 + transform_cls, result, "_apply_transforms_to_data[list-item]" + ) + data[i] = item + return data + + +def _check_not_none(transform_cls, result: Any, context: str) -> Any: + """Reject transforms that returned None; pass anything else through. + + Transforms may either mutate-and-return or return a new dict, but they + must always return *something* — silently swapping data for ``None`` + would hide bugs. + """ + if result is None: + msg = ( + f"Transform {transform_cls.__name__!r} in {context} returned None. " + "A transform must return the (possibly new) transformed dict." + ) + raise TypeError(msg) + return result def _transform_nested_fields(data, serializer_class, request, *, latest=None): @@ -223,7 +261,9 @@ def _transform_nested_fields(data, serializer_class, request, *, latest=None): child_serializer_class, request.version, latest=latest ) if child_transforms: - _apply_transforms_to_data(child_transforms, nested_data, request) + data[field_name] = _apply_transforms_to_data( + child_transforms, nested_data, request + ) def _get_field_serializer_class(field): diff --git a/src/api_versioning/mitol/api_versioning/transforms.py b/src/api_versioning/mitol/api_versioning/transforms.py index c4efa9be..5494be97 100644 --- a/src/api_versioning/mitol/api_versioning/transforms.py +++ b/src/api_versioning/mitol/api_versioning/transforms.py @@ -76,6 +76,10 @@ class Transform(metaclass=TransformMeta): - to_representation: for response data transforms - to_internal_value: for request data transforms - transform_schema: for OpenAPI schema transforms + + Data transform methods (``to_representation`` and ``to_internal_value``) + may either mutate ``data`` in place and return it, or return a new dict. + Both patterns are supported. The only invalid return value is ``None``. """ version: str | None = None @@ -91,8 +95,10 @@ def to_representation( ) -> dict[str, Any]: """Transform response data backwards (latest version -> older version). - Called when a client requests an older API version. Mutates `data` - in place to match the older version's expected response shape. + Called when a client requests an older API version. Reshape ``data`` + into the older version's expected response shape. You may either + mutate ``data`` in place and return it, or return a new dict; both + are supported. Args: data: The serialized response dict (latest version format). @@ -100,7 +106,8 @@ def to_representation( instance: The model instance being serialized. Returns: - The transformed data dict. + The transformed data dict (may be the same object as ``data`` or + a new one). Returning ``None`` raises ``TypeError``. """ return data @@ -108,14 +115,17 @@ def to_internal_value(self, data: dict[str, Any], request: Any) -> dict[str, Any """Transform request data forwards (older version -> latest version). Called when a client sends data using an older API version format. - Mutates `data` in place to match the latest version's expected input. + Reshape ``data`` into the latest version's expected input. You may + either mutate ``data`` in place and return it, or return a new dict; + both are supported. Args: data: The incoming request data dict (older version format). request: The DRF request object. Returns: - The transformed data dict. + The transformed data dict (may be the same object as ``data`` or + a new one). Returning ``None`` raises ``TypeError``. """ return data diff --git a/tests/api_versioning/test_mixins.py b/tests/api_versioning/test_mixins.py index 38b81fe2..2eda3781 100644 --- a/tests/api_versioning/test_mixins.py +++ b/tests/api_versioning/test_mixins.py @@ -136,6 +136,68 @@ def to_internal_value(self, data, request): # noqa: ARG002 assert serializer.validated_data["name"] == "test" +@pytest.mark.usefixtures("_versions") +def test_mixin_to_representation_rejects_none_return(): + """to_representation should reject transforms that return None.""" + serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" + + class BadTransform(Transform): + version = "v2" + description = "Bad transform" + serializer = serializer_path + + def to_representation(self, data, request, instance): # noqa: ARG002 + return None + + request = _make_request("v1") + serializer = SampleSerializer(context={"request": request}) + instance = SimpleNamespace(name="test", value=42) + + with pytest.raises(TypeError, match="to_representation"): + serializer.to_representation(instance) + + +@pytest.mark.usefixtures("_versions") +def test_mixin_to_representation_accepts_new_object_return(): + """to_representation should accept transforms that return a new dict.""" + serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" + + class FunctionalTransform(Transform): + version = "v2" + description = "Returns a new dict instead of mutating" + serializer = serializer_path + + def to_representation(self, data, request, instance): # noqa: ARG002 + return {**data, "name": data["name"].upper()} + + request = _make_request("v1") + serializer = SampleSerializer(context={"request": request}) + instance = SimpleNamespace(name="test", value=42) + + result = serializer.to_representation(instance) + assert result == {"name": "TEST", "value": 42} + + +@pytest.mark.usefixtures("_versions") +def test_mixin_to_internal_value_accepts_new_object_return(): + """to_internal_value should accept transforms that return a new dict.""" + serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" + + class FunctionalTransform(Transform): + version = "v2" + description = "Returns a new dict instead of mutating" + serializer = serializer_path + + def to_internal_value(self, data, request): # noqa: ARG002 + return {**data, "name": data["name"].lower()} + + request = _make_request("v1") + serializer = SampleSerializer(context={"request": request}) + + result = serializer.to_internal_value({"name": "TEST", "value": 42}) + assert result == {"name": "test", "value": 42} + + @pytest.mark.usefixtures("_versions") def test_mixin_no_request_context(): """Without a request in context, mixin should be a no-op.""" @@ -319,3 +381,92 @@ def to_representation(self, data, request, instance): # noqa: ARG002 assert result["child"]["grandchild"] == {"colour": "red"} assert result["child"]["label"] == "mid" assert result["title"] == "top" + + +@pytest.mark.usefixtures("_versions") +def test_transform_dict_backwards_recursive_accepts_new_object_return(): + """Nested transforms that return a new dict should be applied via parent rebind. + + This is the exact reproducer that motivated relaxing the in-place contract: + a child transform that returns ``{**data, ...}`` instead of mutating must + still be reflected in the parent's nested field. + """ + + class ChildSerializer(VersionedSerializerMixin, serializers.Serializer): + color = serializers.CharField() + + child_path = f"{ChildSerializer.__module__}.{ChildSerializer.__qualname__}" + + class ParentSerializer(serializers.Serializer): + child = ChildSerializer() + + class FunctionalChildTransform(Transform): + version = "v2" + description = "Returns a new dict instead of mutating" + serializer = child_path + + def to_representation(self, data, request, instance): # noqa: ARG002 + return {"colour": data.get("color")} + + request = _make_request("v1") + data = {"child": {"color": "red"}} + + result = transform_dict_backwards(data, ParentSerializer, request, recursive=True) + assert result["child"] == {"colour": "red"} + + +@pytest.mark.usefixtures("_versions") +def test_transform_dict_backwards_recursive_many_accepts_new_object_return(): + """List-of-dicts items returned as new dicts should be rebound by index.""" + + class ItemSerializer(VersionedSerializerMixin, serializers.Serializer): + name = serializers.CharField() + + item_path = f"{ItemSerializer.__module__}.{ItemSerializer.__qualname__}" + + class ContainerSerializer(serializers.Serializer): + items = ItemSerializer(many=True) + + class FunctionalItemTransform(Transform): + version = "v2" + description = "Returns a new dict instead of mutating" + serializer = item_path + + def to_representation(self, data, request, instance): # noqa: ARG002 + return {"label": data["name"].upper()} + + request = _make_request("v1") + data = {"items": [{"name": "a"}, {"name": "b"}]} + + result = transform_dict_backwards( + data, ContainerSerializer, request, recursive=True + ) + assert result["items"] == [{"label": "A"}, {"label": "B"}] + + +@pytest.mark.usefixtures("_versions") +def test_transform_dict_backwards_recursive_rejects_none_return(): + """Nested transforms that return None must still raise TypeError.""" + + class ChildSerializer(VersionedSerializerMixin, serializers.Serializer): + color = serializers.CharField() + + child_path = f"{ChildSerializer.__module__}.{ChildSerializer.__qualname__}" + + class ParentSerializer(serializers.Serializer): + child = ChildSerializer() + + class NoneReturningTransform(Transform): + version = "v2" + description = "Forgets to return" + serializer = child_path + + def to_representation(self, data, request, instance): # noqa: ARG002 + data["colour"] = data.pop("color") + # missing return + + request = _make_request("v1") + data = {"child": {"color": "red"}} + + with pytest.raises(TypeError, match="returned None"): + transform_dict_backwards(data, ParentSerializer, request, recursive=True) From 507e0f51cc4ddd6c05d575e0074d59c83716f0a6 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 29 Apr 2026 14:59:30 -0400 Subject: [PATCH 5/6] Simplify tests --- tests/api_versioning/conftest.py | 39 ++++ tests/api_versioning/test_checks.py | 105 ++++------ tests/api_versioning/test_mixins.py | 226 +++++++++------------- tests/api_versioning/test_schema_hooks.py | 130 +++++-------- tests/api_versioning/test_transforms.py | 90 ++++----- tests/api_versioning/test_versions.py | 142 ++++++-------- 6 files changed, 313 insertions(+), 419 deletions(-) create mode 100644 tests/api_versioning/conftest.py diff --git a/tests/api_versioning/conftest.py b/tests/api_versioning/conftest.py new file mode 100644 index 00000000..c2c37213 --- /dev/null +++ b/tests/api_versioning/conftest.py @@ -0,0 +1,39 @@ +"""Shared fixtures for api_versioning tests.""" + +import pytest +from mitol.api_versioning.versions import ( + _registered_transforms, + _transform_registry, +) + + +@pytest.fixture(autouse=True) +def _clear_registry(): + """Clear the transform registry before and after each test.""" + saved_registry = dict(_transform_registry) + saved_registered = set(_registered_transforms) + _transform_registry.clear() + _registered_transforms.clear() + yield + _transform_registry.clear() + _registered_transforms.clear() + _transform_registry.update(saved_registry) + _registered_transforms.update(saved_registered) + + +@pytest.fixture +def _versions(request, settings): + """Configure ``REST_FRAMEWORK['ALLOWED_VERSIONS']``. + + Defaults to ``['v0', 'v1', 'v2']``. Override the version list per test + with indirect parametrization:: + + @pytest.mark.parametrize("_versions", [["v0", "v1"]], indirect=True) + def test_foo(_versions): ... + """ + versions = getattr(request, "param", ["v0", "v1", "v2"]) + settings.REST_FRAMEWORK = { + **getattr(settings, "REST_FRAMEWORK", {}), + "ALLOWED_VERSIONS": list(versions), + } + return versions diff --git a/tests/api_versioning/test_checks.py b/tests/api_versioning/test_checks.py index eec86129..c2fb0e05 100644 --- a/tests/api_versioning/test_checks.py +++ b/tests/api_versioning/test_checks.py @@ -4,24 +4,6 @@ from mitol.api_versioning import checks as checks_module from mitol.api_versioning.checks import check_transform_serializer_paths from mitol.api_versioning.transforms import Transform -from mitol.api_versioning.versions import ( - _registered_transforms, - _transform_registry, -) - - -@pytest.fixture(autouse=True) -def _clear_registry(): - """Clear the transform registry before and after each test.""" - saved_registry = dict(_transform_registry) - saved_registered = set(_registered_transforms) - _transform_registry.clear() - _registered_transforms.clear() - yield - _transform_registry.clear() - _registered_transforms.clear() - _transform_registry.update(saved_registry) - _registered_transforms.update(saved_registered) # A real serializer-shaped class we can resolve via dotted path. @@ -32,87 +14,70 @@ class _ResolvableSerializer: _RESOLVABLE_PATH = f"{__name__}._ResolvableSerializer" -def test_no_transforms_passes(settings): +@pytest.mark.usefixtures("_versions") +def test_no_transforms_passes(): """No registered transforms → no issues.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} assert check_transform_serializer_paths(app_configs=None) == [] -def test_resolvable_path_passes(settings): +@pytest.mark.usefixtures("_versions") +def test_resolvable_path_passes(): """A transform whose serializer resolves cleanly → no issues.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} class GoodTransform(Transform): version = "v2" description = "ok" serializer = _RESOLVABLE_PATH - issues = check_transform_serializer_paths(app_configs=None) - assert issues == [] - + assert check_transform_serializer_paths(app_configs=None) == [] -def test_unresolvable_path_emits_e001(settings): - """Bad dotted path → E001 Error.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} - class BadTransform(Transform): - version = "v2" - description = "broken path" - serializer = "myapp.does_not_exist.NopeSerializer" +@pytest.mark.parametrize( + ("version", "serializer", "expected_id"), + [ + ("v2", "myapp.does_not_exist.NopeSerializer", "api_versioning.E001"), + ("v9", _RESOLVABLE_PATH, "api_versioning.E002"), + ], + ids=["unresolvable_path_e001", "version_not_in_allowed_e002"], +) +@pytest.mark.usefixtures("_versions") +def test_check_emits_error(version, serializer, expected_id): + """Misconfigured transforms surface the appropriate check id.""" + type( + "BadTransform", + (Transform,), + { + "version": version, + "description": "bad", + "serializer": serializer, + }, + ) issues = check_transform_serializer_paths(app_configs=None) - ids = [i.id for i in issues] - assert "api_versioning.E001" in ids + assert expected_id in [i.id for i in issues] -def test_canonical_mismatch_emits_w001(settings): +@pytest.mark.usefixtures("_versions") +def test_canonical_mismatch_emits_w001(monkeypatch): """Path resolves but canonical name differs → W001 Warning.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} + alias_path = "tests.api_versioning.test_checks.AliasResolvableSerializer" - # Re-export the class under a different module path (simulate - # `from .real import X` in another module). We can't easily set up - # a real re-export at test time, so simulate by giving the class a - # __qualname__ that doesn't match the path. class MismatchTransform(Transform): version = "v2" description = "alias path" - serializer = "tests.api_versioning.test_checks._ResolvableSerializer" + # Start with a real path so metaclass validation passes, then drift it. + serializer = _RESOLVABLE_PATH - # The class lives at this module path. Its canonical - # __module__.__qualname__ should match for this test to pass; flip the - # declared string to simulate drift. - MismatchTransform.serializer = ( - "tests.api_versioning.test_checks.AliasResolvableSerializer" - ) + MismatchTransform.serializer = alias_path - # Patch import_string to resolve to our class anyway, so we hit the - # canonical-mismatch branch. real_import_string = checks_module.import_string def fake_import_string(path): - if path == "tests.api_versioning.test_checks.AliasResolvableSerializer": + if path == alias_path: return _ResolvableSerializer return real_import_string(path) - checks_module.import_string = fake_import_string - try: - issues = check_transform_serializer_paths(app_configs=None) - finally: - checks_module.import_string = real_import_string - - ids = [i.id for i in issues] - assert "api_versioning.W001" in ids - - -def test_version_not_in_allowed_emits_e002(settings): - """Version outside ALLOWED_VERSIONS → E002 Error.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} - - class WrongVersionTransform(Transform): - version = "v9" - description = "future version" - serializer = _RESOLVABLE_PATH + monkeypatch.setattr(checks_module, "import_string", fake_import_string) issues = check_transform_serializer_paths(app_configs=None) - ids = [i.id for i in issues] - assert "api_versioning.E002" in ids + assert "api_versioning.W001" in [i.id for i in issues] diff --git a/tests/api_versioning/test_mixins.py b/tests/api_versioning/test_mixins.py index 2eda3781..d39610f9 100644 --- a/tests/api_versioning/test_mixins.py +++ b/tests/api_versioning/test_mixins.py @@ -8,32 +8,9 @@ transform_dict_backwards, ) from mitol.api_versioning.transforms import Transform -from mitol.api_versioning.versions import _registered_transforms, _transform_registry from rest_framework import serializers -@pytest.fixture(autouse=True) -def _clear_registry(): - """Clear the transform registry before and after each test.""" - saved_registry = dict(_transform_registry) - saved_registered = set(_registered_transforms) - _transform_registry.clear() - _registered_transforms.clear() - yield - _transform_registry.clear() - _registered_transforms.clear() - _transform_registry.update(saved_registry) - _registered_transforms.update(saved_registered) - - -@pytest.fixture -def _versions(settings): - settings.REST_FRAMEWORK = { - **getattr(settings, "REST_FRAMEWORK", {}), - "ALLOWED_VERSIONS": ["v0", "v1", "v2"], - } - - class SampleSerializer(VersionedSerializerMixin, serializers.Serializer): """Sample serializer for testing.""" @@ -41,6 +18,19 @@ class SampleSerializer(VersionedSerializerMixin, serializers.Serializer): value = serializers.IntegerField() +SAMPLE_PATH = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" + + +def _make_request(version): + """Create a mock request with a version attribute.""" + return SimpleNamespace(version=version) + + +@pytest.fixture +def sample_instance(): + return SimpleNamespace(name="test", value=42) + + def test_mro_enforcement_rejects_wrong_order(): """Mixin after Serializer in bases should raise TypeError.""" with pytest.raises(TypeError, match="must appear before"): @@ -49,213 +39,185 @@ class BadSerializer(serializers.Serializer, VersionedSerializerMixin): name = serializers.CharField() -def _make_request(version): - """Create a mock request with a version attribute.""" - return SimpleNamespace(version=version) - - @pytest.mark.usefixtures("_versions") -def test_mixin_no_transforms(): +def test_mixin_no_transforms(sample_instance): """With no transforms, mixin should be a no-op.""" - request = _make_request("v1") - context = {"request": request} - serializer = SampleSerializer(context=context) - - instance = SimpleNamespace(name="test", value=42) - data = serializer.to_representation(instance) - assert data == {"name": "test", "value": 42} + serializer = SampleSerializer(context={"request": _make_request("v1")}) + assert serializer.to_representation(sample_instance) == { + "name": "test", + "value": 42, + } @pytest.mark.usefixtures("_versions") -def test_mixin_latest_version_no_transform(): +def test_mixin_latest_version_no_transform(sample_instance): """Requests at the latest version should not apply transforms.""" - serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" class RenameTransform(Transform): version = "v2" description = "Rename name to title" - serializer = serializer_path + serializer = SAMPLE_PATH def to_representation(self, data, request, instance): # noqa: ARG002 data["name_old"] = data.pop("name") return data - request = _make_request("v2") # latest - context = {"request": request} - serializer = SampleSerializer(context=context) - - instance = SimpleNamespace(name="test", value=42) - data = serializer.to_representation(instance) - assert data == {"name": "test", "value": 42} + serializer = SampleSerializer(context={"request": _make_request("v2")}) + assert serializer.to_representation(sample_instance) == { + "name": "test", + "value": 42, + } @pytest.mark.usefixtures("_versions") -def test_mixin_older_version_applies_transform(): +def test_mixin_older_version_applies_transform(sample_instance): """Requests at an older version should apply transforms backwards.""" - serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" class RenameTransform(Transform): version = "v2" description = "Rename name to title" - serializer = serializer_path + serializer = SAMPLE_PATH def to_representation(self, data, request, instance): # noqa: ARG002 if "name" in data: data["title"] = data.pop("name") return data - request = _make_request("v1") - context = {"request": request} - serializer = SampleSerializer(context=context) - - instance = SimpleNamespace(name="test", value=42) - data = serializer.to_representation(instance) - assert data == {"title": "test", "value": 42} + serializer = SampleSerializer(context={"request": _make_request("v1")}) + assert serializer.to_representation(sample_instance) == { + "title": "test", + "value": 42, + } @pytest.mark.usefixtures("_versions") def test_mixin_to_internal_value_applies_forward_transform(): """Incoming data from older version should be transformed forwards.""" - serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" class RenameTransform(Transform): version = "v2" description = "Rename title to name" - serializer = serializer_path + serializer = SAMPLE_PATH def to_internal_value(self, data, request): # noqa: ARG002 if "title" in data: data["name"] = data.pop("title") return data - request = _make_request("v1") - context = {"request": request} - serializer = SampleSerializer(data={"title": "test", "value": 42}, context=context) + serializer = SampleSerializer( + data={"title": "test", "value": 42}, + context={"request": _make_request("v1")}, + ) assert serializer.is_valid(), serializer.errors assert serializer.validated_data["name"] == "test" @pytest.mark.usefixtures("_versions") -def test_mixin_to_representation_rejects_none_return(): +def test_mixin_to_representation_rejects_none_return(sample_instance): """to_representation should reject transforms that return None.""" - serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" class BadTransform(Transform): version = "v2" description = "Bad transform" - serializer = serializer_path + serializer = SAMPLE_PATH def to_representation(self, data, request, instance): # noqa: ARG002 return None - request = _make_request("v1") - serializer = SampleSerializer(context={"request": request}) - instance = SimpleNamespace(name="test", value=42) + serializer = SampleSerializer(context={"request": _make_request("v1")}) with pytest.raises(TypeError, match="to_representation"): - serializer.to_representation(instance) + serializer.to_representation(sample_instance) @pytest.mark.usefixtures("_versions") -def test_mixin_to_representation_accepts_new_object_return(): +def test_mixin_to_representation_accepts_new_object_return(sample_instance): """to_representation should accept transforms that return a new dict.""" - serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" class FunctionalTransform(Transform): version = "v2" description = "Returns a new dict instead of mutating" - serializer = serializer_path + serializer = SAMPLE_PATH def to_representation(self, data, request, instance): # noqa: ARG002 return {**data, "name": data["name"].upper()} - request = _make_request("v1") - serializer = SampleSerializer(context={"request": request}) - instance = SimpleNamespace(name="test", value=42) - - result = serializer.to_representation(instance) - assert result == {"name": "TEST", "value": 42} + serializer = SampleSerializer(context={"request": _make_request("v1")}) + assert serializer.to_representation(sample_instance) == { + "name": "TEST", + "value": 42, + } @pytest.mark.usefixtures("_versions") def test_mixin_to_internal_value_accepts_new_object_return(): """to_internal_value should accept transforms that return a new dict.""" - serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" class FunctionalTransform(Transform): version = "v2" description = "Returns a new dict instead of mutating" - serializer = serializer_path + serializer = SAMPLE_PATH def to_internal_value(self, data, request): # noqa: ARG002 return {**data, "name": data["name"].lower()} - request = _make_request("v1") - serializer = SampleSerializer(context={"request": request}) + serializer = SampleSerializer(context={"request": _make_request("v1")}) result = serializer.to_internal_value({"name": "TEST", "value": 42}) assert result == {"name": "test", "value": 42} +@pytest.mark.parametrize( + "context", + [{}, {"request": SimpleNamespace()}], + ids=["no_request", "request_without_version"], +) @pytest.mark.usefixtures("_versions") -def test_mixin_no_request_context(): - """Without a request in context, mixin should be a no-op.""" - serializer = SampleSerializer(context={}) - - instance = SimpleNamespace(name="test", value=42) - data = serializer.to_representation(instance) - assert data == {"name": "test", "value": 42} - - -@pytest.mark.usefixtures("_versions") -def test_mixin_request_without_version(): - """With a request that has no version, mixin should be a no-op.""" - request = SimpleNamespace() - serializer = SampleSerializer(context={"request": request}) - - instance = SimpleNamespace(name="test", value=42) - data = serializer.to_representation(instance) - assert data == {"name": "test", "value": 42} +def test_mixin_no_op_when_no_version(context, sample_instance): + """Mixin is a no-op when there is no resolvable request version.""" + serializer = SampleSerializer(context=context) + assert serializer.to_representation(sample_instance) == { + "name": "test", + "value": 42, + } @pytest.mark.usefixtures("_versions") def test_transform_dict_backwards_applies_transforms(): """transform_dict_backwards should apply transforms to a raw dict.""" - serializer_path = f"{SampleSerializer.__module__}.{SampleSerializer.__qualname__}" class RenameTransform(Transform): version = "v2" description = "Rename name to title" - serializer = serializer_path + serializer = SAMPLE_PATH def to_representation(self, data, request, instance): # noqa: ARG002 if "name" in data: data["title"] = data.pop("name") return data - request = _make_request("v1") data = {"name": "test", "value": 42} - result = transform_dict_backwards(data, SampleSerializer, request) + result = transform_dict_backwards(data, SampleSerializer, _make_request("v1")) assert result == {"title": "test", "value": 42} -@pytest.mark.usefixtures("_versions") -def test_transform_dict_backwards_no_op_for_latest(): - """transform_dict_backwards should not transform at latest version.""" - request = _make_request("v2") +@pytest.mark.parametrize( + ("request_obj", "_versions"), + [ + (SimpleNamespace(version="v2"), ["v0", "v1", "v2"]), + (None, ["v0", "v1", "v2"]), + ], + ids=["latest_version", "no_request"], + indirect=["_versions"], +) +def test_transform_dict_backwards_no_op(request_obj, _versions): + """transform_dict_backwards is a no-op at latest version or with no request.""" data = {"name": "test", "value": 42} - result = transform_dict_backwards(data, SampleSerializer, request) + result = transform_dict_backwards(data, SampleSerializer, request_obj) assert result == {"name": "test", "value": 42} -def test_transform_dict_backwards_no_request(): - """transform_dict_backwards should be a no-op with no request.""" - data = {"name": "test"} - result = transform_dict_backwards(data, SampleSerializer, None) - assert result == {"name": "test"} - - @pytest.mark.usefixtures("_versions") def test_transform_dict_backwards_recursive(): """recursive=True should apply transforms to nested serializer fields.""" @@ -283,9 +245,10 @@ def to_representation(self, data, request, instance): # noqa: ARG002 data["colour"] = data.pop("color") return data - request = _make_request("v1") data = {"label": "test", "child": {"color": "red"}} - result = transform_dict_backwards(data, ParentSerializer, request, recursive=True) + result = transform_dict_backwards( + data, ParentSerializer, _make_request("v1"), recursive=True + ) assert result["child"] == {"colour": "red"} assert result["label"] == "test" @@ -316,7 +279,6 @@ def to_representation(self, data, request, instance): # noqa: ARG002 data.pop("score", None) return data - request = _make_request("v1") data = { "title": "container", "items": [ @@ -325,7 +287,7 @@ def to_representation(self, data, request, instance): # noqa: ARG002 ], } result = transform_dict_backwards( - data, ContainerSerializer, request, recursive=True + data, ContainerSerializer, _make_request("v1"), recursive=True ) assert result["items"] == [{"name": "a"}, {"name": "b"}] @@ -333,9 +295,10 @@ def to_representation(self, data, request, instance): # noqa: ARG002 @pytest.mark.usefixtures("_versions") def test_transform_dict_backwards_recursive_no_nested_transforms(): """recursive=True with no child transforms should be a no-op.""" - request = _make_request("v1") data = {"name": "test", "value": 42} - result = transform_dict_backwards(data, SampleSerializer, request, recursive=True) + result = transform_dict_backwards( + data, SampleSerializer, _make_request("v1"), recursive=True + ) # SampleSerializer has no nested serializer fields, so no change assert result == {"name": "test", "value": 42} @@ -369,7 +332,6 @@ def to_representation(self, data, request, instance): # noqa: ARG002 data["colour"] = data.pop("color") return data - request = _make_request("v1") data = { "title": "top", "child": { @@ -377,7 +339,9 @@ def to_representation(self, data, request, instance): # noqa: ARG002 "grandchild": {"color": "red"}, }, } - result = transform_dict_backwards(data, ParentSerializer, request, recursive=True) + result = transform_dict_backwards( + data, ParentSerializer, _make_request("v1"), recursive=True + ) assert result["child"]["grandchild"] == {"colour": "red"} assert result["child"]["label"] == "mid" assert result["title"] == "top" @@ -408,10 +372,10 @@ class FunctionalChildTransform(Transform): def to_representation(self, data, request, instance): # noqa: ARG002 return {"colour": data.get("color")} - request = _make_request("v1") data = {"child": {"color": "red"}} - - result = transform_dict_backwards(data, ParentSerializer, request, recursive=True) + result = transform_dict_backwards( + data, ParentSerializer, _make_request("v1"), recursive=True + ) assert result["child"] == {"colour": "red"} @@ -435,11 +399,9 @@ class FunctionalItemTransform(Transform): def to_representation(self, data, request, instance): # noqa: ARG002 return {"label": data["name"].upper()} - request = _make_request("v1") data = {"items": [{"name": "a"}, {"name": "b"}]} - result = transform_dict_backwards( - data, ContainerSerializer, request, recursive=True + data, ContainerSerializer, _make_request("v1"), recursive=True ) assert result["items"] == [{"label": "A"}, {"label": "B"}] @@ -465,8 +427,8 @@ def to_representation(self, data, request, instance): # noqa: ARG002 data["colour"] = data.pop("color") # missing return - request = _make_request("v1") data = {"child": {"color": "red"}} - with pytest.raises(TypeError, match="returned None"): - transform_dict_backwards(data, ParentSerializer, request, recursive=True) + transform_dict_backwards( + data, ParentSerializer, _make_request("v1"), recursive=True + ) diff --git a/tests/api_versioning/test_schema_hooks.py b/tests/api_versioning/test_schema_hooks.py index fdf1e46a..48cecf6c 100644 --- a/tests/api_versioning/test_schema_hooks.py +++ b/tests/api_versioning/test_schema_hooks.py @@ -9,54 +9,37 @@ postprocess_versioned_schema, ) from mitol.api_versioning.transforms import Transform -from mitol.api_versioning.versions import _transform_registry, register_transform - - -@pytest.fixture(autouse=True) -def _clear_registry(): - """Clear the transform registry before and after each test.""" - saved = dict(_transform_registry) - _transform_registry.clear() - yield - _transform_registry.clear() - _transform_registry.update(saved) +from mitol.api_versioning.versions import register_transform class TestResolveSchemaName: """Tests for _resolve_schema_name.""" - def test_strips_serializer_suffix(self): - """Test stripping Serializer suffix.""" - assert ( - _resolve_schema_name("myapp.serializers.LearningResourceSerializer") - == "LearningResource" - ) - - def test_no_serializer_suffix(self): - """Test name without Serializer suffix.""" - assert _resolve_schema_name("myapp.serializers.LearningResource") == ( - "LearningResource" - ) - - def test_simple_name(self): - """Test simple class name.""" - assert _resolve_schema_name("CourseSerializer") == "Course" - - def test_class_object_with_serializer_suffix(self): - """Test with an actual class object instead of a string.""" - - class CourseSerializer: - pass - - assert _resolve_schema_name(CourseSerializer) == "Course" - - def test_class_object_without_serializer_suffix(self): - """Test class object without Serializer suffix.""" - - class Course: - pass - - assert _resolve_schema_name(Course) == "Course" + class CourseSerializer: + pass + + class Course: + pass + + @pytest.mark.parametrize( + ("name", "expected"), + [ + ("myapp.serializers.LearningResourceSerializer", "LearningResource"), + ("myapp.serializers.LearningResource", "LearningResource"), + ("CourseSerializer", "Course"), + (CourseSerializer, "Course"), + (Course, "Course"), + ], + ids=[ + "dotted_with_suffix", + "dotted_without_suffix", + "simple_with_suffix", + "class_with_suffix", + "class_without_suffix", + ], + ) + def test_resolves(self, name, expected): + assert _resolve_schema_name(name) == expected class TestGetAllSchemaVariants: @@ -108,13 +91,6 @@ def test_does_not_over_match_similar_names(self): class TestPostprocessVersionedSchema: """Tests for the drf-spectacular postprocessing hook.""" - @pytest.fixture - def _versions(self, settings): - settings.REST_FRAMEWORK = { - **getattr(settings, "REST_FRAMEWORK", {}), - "ALLOWED_VERSIONS": ["v0", "v1", "v2"], - } - @pytest.mark.usefixtures("_versions") def test_no_op_for_latest_version(self): """Hook should return schema unchanged for latest version.""" @@ -202,36 +178,32 @@ def transform_schema(self, schema): req = result["components"]["schemas"]["CourseRequest"]["properties"] assert "extra_field" not in req - @pytest.mark.usefixtures("_versions") - def test_no_transforms_returns_unchanged(self): - """With no transforms registered, hook returns schema unchanged.""" - generator = SimpleNamespace(api_version="v1") - schema = { - "components": { - "schemas": { - "Course": {"properties": {"name": {"type": "string"}}}, - } - } - } - result = postprocess_versioned_schema(schema, generator, None, public=False) - assert result == schema - - @pytest.mark.usefixtures("_versions") - def test_no_components_returns_unchanged(self): - """Schema without components section should pass through.""" - generator = SimpleNamespace(api_version="v1") - schema = {"info": {"title": "API"}} - result = postprocess_versioned_schema(schema, generator, None, public=False) - assert result == schema - - def test_no_versions_configured(self, settings): - """With no allowed versions, hook should return unchanged.""" - settings.REST_FRAMEWORK = { - **getattr(settings, "REST_FRAMEWORK", {}), - "ALLOWED_VERSIONS": [], - } + @pytest.mark.parametrize( + ("_versions", "schema"), + [ + ( + ["v0", "v1", "v2"], + { + "components": { + "schemas": { + "Course": {"properties": {"name": {"type": "string"}}}, + } + } + }, + ), + (["v0", "v1", "v2"], {"info": {"title": "API"}}), + ([], {"components": {"schemas": {}}}), + ], + ids=[ + "no_transforms_registered", + "schema_without_components", + "no_versions_configured", + ], + indirect=["_versions"], + ) + def test_returns_unchanged(self, _versions, schema): + """Hook returns schema unchanged when there is nothing to transform.""" generator = SimpleNamespace(api_version="v1") - schema = {"components": {"schemas": {}}} result = postprocess_versioned_schema(schema, generator, None, public=False) assert result == schema diff --git a/tests/api_versioning/test_transforms.py b/tests/api_versioning/test_transforms.py index f599d8c1..0200a523 100644 --- a/tests/api_versioning/test_transforms.py +++ b/tests/api_versioning/test_transforms.py @@ -3,24 +3,11 @@ import pytest from mitol.api_versioning.transforms import Transform from mitol.api_versioning.versions import ( - _registered_transforms, _transform_registry, get_transforms_for_version, ) - -@pytest.fixture(autouse=True) -def _clear_registry(): - """Clear the transform registry before and after each test.""" - saved_registry = dict(_transform_registry) - saved_registered = set(_registered_transforms) - _transform_registry.clear() - _registered_transforms.clear() - yield - _transform_registry.clear() - _registered_transforms.clear() - _transform_registry.update(saved_registry) - _registered_transforms.update(saved_registered) +SERIALIZER_PATH = "myapp.serializers.MySerializer" def test_transform_base_defaults(): @@ -32,49 +19,40 @@ def test_transform_base_defaults(): assert t.transform_schema({"properties": {}}) == {"properties": {}} -def test_metaclass_auto_registration(settings): +@pytest.mark.usefixtures("_versions") +def test_metaclass_auto_registration(): """Defining a Transform subclass with a version should auto-register it.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} class MyTransform(Transform): version = "v2" description = "Test transform" - serializer = "myapp.serializers.MySerializer" + serializer = SERIALIZER_PATH assert MyTransform in get_transforms_for_version("v2") -def test_validation_rejects_missing_serializer(): - """Transform with version but no serializer should raise TypeError.""" - with pytest.raises(TypeError, match="must define a 'serializer'"): - - class BadTransform(Transform): - version = "v2" - description = "Missing serializer" - - -def test_validation_rejects_non_dotted_serializer(): - """Transform with a non-dotted serializer path should raise TypeError.""" - with pytest.raises(TypeError, match="not a dotted path"): - - class BadTransform(Transform): - version = "v2" - description = "Bad serializer path" - serializer = "NotADottedPath" - - -def test_validation_rejects_empty_version(): - """Transform with empty version string should raise TypeError.""" - with pytest.raises(TypeError, match="invalid version"): +@pytest.mark.parametrize( + ("version", "serializer", "match"), + [ + ("v2", None, "must define a 'serializer'"), + ("v2", "NotADottedPath", "not a dotted path"), + ("", SERIALIZER_PATH, "invalid version"), + ], + ids=["missing_serializer", "non_dotted_serializer", "empty_version"], +) +def test_metaclass_validation_rejects_bad_config(version, serializer, match): + """Metaclass validation rejects misconfigured Transform subclasses.""" + attrs = {"version": version, "description": "bad"} + if serializer is not None: + attrs["serializer"] = serializer - class BadTransform(Transform): - version = "" - serializer = "myapp.serializers.MySerializer" + with pytest.raises(TypeError, match=match): + type("BadTransform", (Transform,), attrs) -def test_metaclass_no_registration_without_version(settings): +@pytest.mark.parametrize("_versions", [["v0", "v1"]], indirect=True) +def test_metaclass_no_registration_without_version(_versions): """A Transform subclass without a version should not be registered.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} class AbstractTransform(Transform): description = "Not a real transform" @@ -83,14 +61,14 @@ class AbstractTransform(Transform): assert AbstractTransform not in transforms -def test_concrete_transform_field_rename(settings): +@pytest.mark.usefixtures("_versions") +def test_concrete_transform_field_rename(): """Test a concrete transform that renames a field.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} class RenameFieldTransform(Transform): version = "v2" description = "Rename 'old_name' to 'new_name'" - serializer = "myapp.serializers.MySerializer" + serializer = SERIALIZER_PATH def to_representation(self, data, request, instance): # noqa: ARG002 if "new_name" in data: @@ -129,14 +107,14 @@ def transform_schema(self, schema): assert "new_name" not in result["properties"] -def test_concrete_transform_field_added(settings): +@pytest.mark.usefixtures("_versions") +def test_concrete_transform_field_added(): """Test a transform for a field added in a new version.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} class AddFieldTransform(Transform): version = "v2" description = "Add 'new_field' in v2" - serializer = "myapp.serializers.MySerializer" + serializer = SERIALIZER_PATH def to_representation(self, data, request, instance): # noqa: ARG002 data.pop("new_field", None) @@ -167,14 +145,14 @@ def transform_schema(self, schema): assert "new_field" not in result["required"] -def test_concrete_transform_field_removed(settings): +@pytest.mark.usefixtures("_versions") +def test_concrete_transform_field_removed(): """Test a transform for a field removed in a new version.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} class RemoveFieldTransform(Transform): version = "v2" description = "Remove 'legacy_field' in v2" - serializer = "myapp.serializers.MySerializer" + serializer = SERIALIZER_PATH def to_representation(self, data, request, instance): # noqa: ARG002 data["legacy_field"] = getattr(instance, "legacy_field", None) @@ -203,14 +181,14 @@ def transform_schema(self, schema): assert result == {"current_field": "value"} -def test_concrete_transform_type_change(settings): +@pytest.mark.usefixtures("_versions") +def test_concrete_transform_type_change(): """Test a transform for a field whose type/structure changed.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} class TypeChangeTransform(Transform): version = "v2" description = "Change 'topics' from list of strings to list of objects" - serializer = "myapp.serializers.MySerializer" + serializer = SERIALIZER_PATH def to_representation(self, data, request, instance): # noqa: ARG002 if "topics" in data and isinstance(data["topics"], list): diff --git a/tests/api_versioning/test_versions.py b/tests/api_versioning/test_versions.py index eabf688d..77a7e75d 100644 --- a/tests/api_versioning/test_versions.py +++ b/tests/api_versioning/test_versions.py @@ -3,8 +3,6 @@ import pytest from mitol.api_versioning.transforms import Transform from mitol.api_versioning.versions import ( - _registered_transforms, - _transform_registry, get_allowed_versions, get_latest_version, get_transforms_backwards, @@ -15,19 +13,11 @@ register_transform, ) +DUMMY_PATH = "myapp.serializers.DummySerializer" +OTHER_PATH = "myapp.serializers.OtherSerializer" -@pytest.fixture(autouse=True) -def _clear_registry(): - """Clear the transform registry before and after each test.""" - saved_registry = dict(_transform_registry) - saved_registered = set(_registered_transforms) - _transform_registry.clear() - _registered_transforms.clear() - yield - _transform_registry.clear() - _registered_transforms.clear() - _transform_registry.update(saved_registry) - _registered_transforms.update(saved_registered) +VERSIONS_SHORT = ["v0", "v1"] +VERSIONS_LONG = ["v0", "v1", "v2", "v3"] class DummySerializer: @@ -56,29 +46,28 @@ class TestTransform(Transform): return TestTransform -def test_get_allowed_versions(settings): +@pytest.mark.usefixtures("_versions") +def test_get_allowed_versions(): """Test reading ALLOWED_VERSIONS from DRF settings.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} assert get_allowed_versions() == ["v0", "v1", "v2"] -def test_get_latest_version(settings): - """Test that latest version is the last in the list.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} - assert get_latest_version() == "v1" - - -def test_get_latest_version_empty(settings): - """Test latest version with empty list returns None.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": []} - assert get_latest_version() is None +@pytest.mark.parametrize( + ("_versions", "expected"), + [(VERSIONS_SHORT, "v1"), ([], None)], + indirect=["_versions"], + ids=["non_empty", "empty"], +) +def test_get_latest_version(_versions, expected): + """Latest version is the last entry, or None when empty.""" + assert get_latest_version() == expected -def test_register_and_get_transforms(settings): +@pytest.mark.usefixtures("_versions") +def test_register_and_get_transforms(): """Test registering transforms and retrieving by version.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} - t1 = _make_transform("v2", "myapp.serializers.DummySerializer") - t2 = _make_transform("v2", "myapp.serializers.OtherSerializer") + t1 = _make_transform("v2", DUMMY_PATH) + t2 = _make_transform("v2", OTHER_PATH) result = get_transforms_for_version("v2") assert t1 in result @@ -86,11 +75,11 @@ def test_register_and_get_transforms(settings): assert get_transforms_for_version("v1") == [] -def test_get_transforms_between(settings): +@pytest.mark.parametrize("_versions", [VERSIONS_LONG], indirect=True) +def test_get_transforms_between(_versions): """Test collecting transforms across a version range.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2", "v3"]} - t_v2 = _make_transform("v2", "myapp.serializers.DummySerializer") - t_v3 = _make_transform("v3", "myapp.serializers.DummySerializer") + t_v2 = _make_transform("v2", DUMMY_PATH) + t_v3 = _make_transform("v3", DUMMY_PATH) assert get_transforms_between("v1", "v3") == [t_v2, t_v3] assert get_transforms_between("v0", "v2") == [t_v2] @@ -98,78 +87,67 @@ def test_get_transforms_between(settings): assert get_transforms_between("v2", "v2") == [] -def test_get_transforms_between_invalid_version(settings): +@pytest.mark.parametrize("_versions", [VERSIONS_SHORT], indirect=True) +def test_get_transforms_between_invalid_version(_versions): """Test that invalid versions return empty list.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} assert get_transforms_between("v99", "v1") == [] assert get_transforms_between("v0", "v99") == [] -def test_get_transforms_backwards(settings): - """Test backwards ordering (newest first) for response transforms.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2", "v3"]} - t_v2 = _make_transform("v2", "myapp.serializers.DummySerializer") - t_v3 = _make_transform("v3", "myapp.serializers.DummySerializer") +@pytest.mark.parametrize( + ("get_func", "reverse"), + [ + (get_transforms_backwards, True), + (get_transforms_forwards, False), + ], + ids=["backwards", "forwards"], +) +@pytest.mark.parametrize("_versions", [VERSIONS_LONG], indirect=True) +def test_get_transforms_directional_ordering(_versions, get_func, reverse): + """Backwards returns newest-first; forwards returns oldest-first.""" + t_v2 = _make_transform("v2", DUMMY_PATH) + t_v3 = _make_transform("v3", DUMMY_PATH) - result = get_transforms_backwards(DummySerializer, "v1") - assert result == [t_v3, t_v2] + expected = [t_v3, t_v2] if reverse else [t_v2, t_v3] + assert get_func(DummySerializer, "v1") == expected -def test_get_transforms_backwards_filters_by_serializer(settings): +@pytest.mark.usefixtures("_versions") +def test_get_transforms_backwards_filters_by_serializer(): """Test that backwards transforms filter by serializer class.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} - _make_transform("v2", "myapp.serializers.DummySerializer") - t_other = _make_transform("v2", "myapp.serializers.OtherSerializer") + _make_transform("v2", DUMMY_PATH) + t_other = _make_transform("v2", OTHER_PATH) result = get_transforms_backwards(OtherSerializer, "v1") assert result == [t_other] -def test_get_transforms_backwards_latest_returns_empty(settings): - """Test that latest version gets no transforms.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} - _make_transform("v1", "myapp.serializers.DummySerializer") - - result = get_transforms_backwards(DummySerializer, "v1") - assert result == [] - - -def test_get_transforms_forwards(settings): - """Test forwards ordering (oldest first) for request transforms.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2", "v3"]} - t_v2 = _make_transform("v2", "myapp.serializers.DummySerializer") - t_v3 = _make_transform("v3", "myapp.serializers.DummySerializer") - - result = get_transforms_forwards(DummySerializer, "v1") - assert result == [t_v2, t_v3] - - -def test_get_transforms_forwards_latest_returns_empty(settings): - """Test that latest version gets no forwards transforms.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1"]} - _make_transform("v1", "myapp.serializers.DummySerializer") - - result = get_transforms_forwards(DummySerializer, "v1") - assert result == [] +@pytest.mark.parametrize( + "get_func", [get_transforms_backwards, get_transforms_forwards] +) +@pytest.mark.parametrize("_versions", [VERSIONS_SHORT], indirect=True) +def test_get_transforms_at_latest_returns_empty(_versions, get_func): + """At the latest version, neither direction yields transforms.""" + _make_transform("v1", DUMMY_PATH) + assert get_func(DummySerializer, "v1") == [] -def test_list_transforms_for_serializer(settings): +@pytest.mark.parametrize("_versions", [VERSIONS_LONG], indirect=True) +def test_list_transforms_for_serializer(_versions): """Test that list_transforms_for_serializer returns matching transforms.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2", "v3"]} - t_v2 = _make_transform("v2", "myapp.serializers.DummySerializer") - t_v3 = _make_transform("v3", "myapp.serializers.DummySerializer") - _make_transform("v2", "myapp.serializers.OtherSerializer") + t_v2 = _make_transform("v2", DUMMY_PATH) + t_v3 = _make_transform("v3", DUMMY_PATH) + _make_transform("v2", OTHER_PATH) result = list_transforms_for_serializer(DummySerializer) assert result == [t_v2, t_v3] -def test_register_transform_is_idempotent(settings): +@pytest.mark.usefixtures("_versions") +def test_register_transform_is_idempotent(): """Test that registering the same transform twice only adds it once.""" - settings.REST_FRAMEWORK = {"ALLOWED_VERSIONS": ["v0", "v1", "v2"]} - t = _make_transform("v2", "myapp.serializers.DummySerializer") + t = _make_transform("v2", DUMMY_PATH) - # Register again register_transform(t) register_transform(t) From 7b87887e248c33d4cb222f1a7a74a1e93290376c Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Thu, 30 Apr 2026 21:36:23 -0400 Subject: [PATCH 6/6] feedback --- pyproject.toml | 4 +-- src/api_versioning/README.md | 18 +++++++--- .../mitol/api_versioning/__init__.py | 2 +- .../mitol/api_versioning/apps.py | 3 -- .../mitol/api_versioning/checks.py | 2 +- .../api_versioning/management/__init__.py | 0 .../management/commands/__init__.py | 0 .../commands/check_api_transforms.py | 34 +++++++++++++++++++ .../mitol/api_versioning/transforms.py | 8 +++++ src/api_versioning/pyproject.toml | 4 +-- tests/api_versioning/test_transforms.py | 8 ++++- 11 files changed, 69 insertions(+), 14 deletions(-) create mode 100644 src/api_versioning/mitol/api_versioning/management/__init__.py create mode 100644 src/api_versioning/mitol/api_versioning/management/commands/__init__.py create mode 100644 src/api_versioning/mitol/api_versioning/management/commands/check_api_transforms.py diff --git a/pyproject.toml b/pyproject.toml index 326085c2..fb0e61c0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ dependencies = [ "mitol-django-scim[celery]", "mitol-django-apigateway", "mitol-django-observability", - "mitol-django-api_versioning", + "mitol-django-api-versioning", ] readme = "README.md" requires-python = ">= 3.11" @@ -125,7 +125,7 @@ mitol-django-scim = { workspace = true } mitol-django-apigateway = { workspace = true } mitol-django-observability = { workspace = true } mitol-drf-lint = { workspace = true } -mitol-django-api_versioning = { workspace = true } +mitol-django-api-versioning = { workspace = true } [tool.hatch.build.targets.sdist] include = ["CHANGELOG.md", "README.md", "py.typed", "**/*.py"] diff --git a/src/api_versioning/README.md b/src/api_versioning/README.md index 1d079d02..d878e699 100644 --- a/src/api_versioning/README.md +++ b/src/api_versioning/README.md @@ -121,7 +121,7 @@ relevant to the change — `to_representation` for response data, `to_internal_value` for request data, `transform_schema` for OpenAPI. ```python -# learning_resources/transforms/v2.py +# learning_resources/transforms.py from mitol.api_versioning.transforms import Transform @@ -259,9 +259,19 @@ your client generator at it. When a transform doesn't fire, three things will tell you why: -1. **`./manage.py check`** runs three system checks at startup - (`api_versioning.E001` / `W001` / `E002`). They flag misconfigured - `serializer` paths (typos, drift) and versions outside `ALLOWED_VERSIONS`. +1. **`./manage.py check_api_transforms`** runs the bundled system checks for + every registered transform: + - `api_versioning.E001` — `serializer` dotted path doesn't resolve + - `api_versioning.W001` — path resolves but the resolved class's canonical + `__module__.__qualname__` differs (re-export drift; runtime lookup may + not match) + - `api_versioning.E002` — `version` is not in `ALLOWED_VERSIONS` + + Defaults to `--fail-level WARNING` so drift fails the command; pass + `--fail-level ERROR` to allow warnings. The same checks also run inside + `./manage.py check` (and on `runserver`/`migrate`/etc), filterable via + `--tag api_versioning`. Add `./manage.py check_api_transforms` to CI to + catch misconfigured transforms before they ship. 2. **`list_transforms_for_serializer(SerializerClass)`** returns every registered transform for that serializer, ordered oldest-first. Useful in a Python shell to confirm registration. diff --git a/src/api_versioning/mitol/api_versioning/__init__.py b/src/api_versioning/mitol/api_versioning/__init__.py index ab70a3d1..c853475f 100644 --- a/src/api_versioning/mitol/api_versioning/__init__.py +++ b/src/api_versioning/mitol/api_versioning/__init__.py @@ -3,4 +3,4 @@ default_app_config = "mitol.api_versioning.apps.ApiVersioningApp" __version__ = "2026.3.24" -__distributionname__ = "mitol-django-api_versioning" +__distributionname__ = "mitol-django-api-versioning" diff --git a/src/api_versioning/mitol/api_versioning/apps.py b/src/api_versioning/mitol/api_versioning/apps.py index def4dea7..5ff30ca8 100644 --- a/src/api_versioning/mitol/api_versioning/apps.py +++ b/src/api_versioning/mitol/api_versioning/apps.py @@ -1,7 +1,6 @@ """ApiVersioning app AppConfig.""" import importlib -import logging import os from django.apps import AppConfig, apps @@ -9,8 +8,6 @@ from . import checks # noqa: F401 (registers Django system checks on import) -log = logging.getLogger(__name__) - class ApiVersioningApp(AppConfig): """Default configuration for the api_versioning app.""" diff --git a/src/api_versioning/mitol/api_versioning/checks.py b/src/api_versioning/mitol/api_versioning/checks.py index ee5488bd..2980ab4b 100644 --- a/src/api_versioning/mitol/api_versioning/checks.py +++ b/src/api_versioning/mitol/api_versioning/checks.py @@ -17,7 +17,7 @@ ) -@register() +@register("api_versioning") def check_transform_serializer_paths(app_configs, **kwargs): # noqa: ARG001 """Validate every registered transform. diff --git a/src/api_versioning/mitol/api_versioning/management/__init__.py b/src/api_versioning/mitol/api_versioning/management/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/api_versioning/mitol/api_versioning/management/commands/__init__.py b/src/api_versioning/mitol/api_versioning/management/commands/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/api_versioning/mitol/api_versioning/management/commands/check_api_transforms.py b/src/api_versioning/mitol/api_versioning/management/commands/check_api_transforms.py new file mode 100644 index 00000000..0b52888d --- /dev/null +++ b/src/api_versioning/mitol/api_versioning/management/commands/check_api_transforms.py @@ -0,0 +1,34 @@ +"""Run the api_versioning system checks with a discoverable name. + +Wraps ``./manage.py check --tag api_versioning`` so consuming projects can +add a single, self-explanatory step to CI: + + ./manage.py check_api_transforms + +Defaults to ``--fail-level WARNING`` so canonical-path drift (W001) also +fails the command. Pass ``--fail-level ERROR`` to allow warnings. +""" + +from django.core.management import call_command +from django.core.management.base import BaseCommand + +_FAIL_LEVELS = ("CRITICAL", "ERROR", "WARNING", "INFO", "DEBUG") + + +class Command(BaseCommand): + help = "Validate every registered api_versioning Transform." + + def add_arguments(self, parser): + parser.add_argument( + "--fail-level", + default="WARNING", + choices=_FAIL_LEVELS, + help=("Message level that causes a non-zero exit (default: %(default)s)."), + ) + + def handle(self, *args, **options): # noqa: ARG002 + call_command( + "check", + tags=["api_versioning"], + fail_level=options["fail_level"], + ) diff --git a/src/api_versioning/mitol/api_versioning/transforms.py b/src/api_versioning/mitol/api_versioning/transforms.py index 5494be97..6311f76a 100644 --- a/src/api_versioning/mitol/api_versioning/transforms.py +++ b/src/api_versioning/mitol/api_versioning/transforms.py @@ -50,6 +50,14 @@ def _validate_transform(transform_cls, name): ) raise TypeError(msg) + if not isinstance(serializer, (str, type)): + msg = ( + f"Transform '{name}' has serializer={serializer!r} of type " + f"{type(serializer).__name__}. Must be a dotted-path string or " + f"a serializer class." + ) + raise TypeError(msg) + if isinstance(serializer, str) and "." not in serializer: msg = ( f"Transform '{name}' has serializer={serializer!r} which " diff --git a/src/api_versioning/pyproject.toml b/src/api_versioning/pyproject.toml index 309fa0c0..bec62118 100644 --- a/src/api_versioning/pyproject.toml +++ b/src/api_versioning/pyproject.toml @@ -1,5 +1,5 @@ [project] -name = "mitol-django-api_versioning" +name = "mitol-django-api-versioning" version = "2026.3.24" description = "MIT Open Learning Django API versioning framework with transform-based backwards compatibility" dependencies = [ @@ -8,7 +8,7 @@ dependencies = [ ] readme = "README.md" license = "BSD-3-Clause" -requires-python = ">=3.10" +requires-python = ">=3.11" [tool.bumpver] current_version = "2026.3.24" diff --git a/tests/api_versioning/test_transforms.py b/tests/api_versioning/test_transforms.py index 0200a523..0ff5636f 100644 --- a/tests/api_versioning/test_transforms.py +++ b/tests/api_versioning/test_transforms.py @@ -36,9 +36,15 @@ class MyTransform(Transform): [ ("v2", None, "must define a 'serializer'"), ("v2", "NotADottedPath", "not a dotted path"), + ("v2", 42, "of type int"), ("", SERIALIZER_PATH, "invalid version"), ], - ids=["missing_serializer", "non_dotted_serializer", "empty_version"], + ids=[ + "missing_serializer", + "non_dotted_serializer", + "wrong_type_serializer", + "empty_version", + ], ) def test_metaclass_validation_rejects_bad_config(version, serializer, match): """Metaclass validation rejects misconfigured Transform subclasses."""