From c09338ff60e6018b807c50c0dd90f5dda4e9df9f Mon Sep 17 00:00:00 2001 From: Muhammad Gibran Alfarizi Date: Wed, 29 Jul 2026 12:31:32 +0200 Subject: [PATCH 1/8] ENH: Add schema migration support for versioned resources --- ARCHITECTURE.md | 49 +- CONTRIBUTING.md | 5 + README.md | 5 + src/fmu/settings/__init__.py | 2 + src/fmu/settings/_fmu_dir.py | 29 +- src/fmu/settings/_migrations/README.md | 274 ++++++++++ src/fmu/settings/_migrations/__init__.py | 9 + src/fmu/settings/_migrations/manager.py | 193 +++++++ .../settings/_migrations/mappings/__init__.py | 7 + .../_migrations/project_config/__init__.py | 7 + .../_migrations/user_config/__init__.py | 7 + src/fmu/settings/_resources/cache_manager.py | 52 +- .../settings/_resources/config_managers.py | 21 +- .../settings/_resources/mappings_manager.py | 11 +- .../_resources/pydantic_resource_manager.py | 82 ++- .../test_migrations/test_migration_manager.py | 318 ++++++++++++ .../test_resource_migration.py | 473 ++++++++++++++++++ 17 files changed, 1524 insertions(+), 20 deletions(-) create mode 100644 src/fmu/settings/_migrations/README.md create mode 100644 src/fmu/settings/_migrations/__init__.py create mode 100644 src/fmu/settings/_migrations/manager.py create mode 100644 src/fmu/settings/_migrations/mappings/__init__.py create mode 100644 src/fmu/settings/_migrations/project_config/__init__.py create mode 100644 src/fmu/settings/_migrations/user_config/__init__.py create mode 100644 tests/test_migrations/test_migration_manager.py create mode 100644 tests/test_migrations/test_resource_migration.py diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 02b308b..b28c960 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -65,7 +65,10 @@ classDiagram class UserFMUDirectory - class CacheManager + class CacheManager { + +get_revision_content(..., migration_manager) + +restore_revision(..., migration_manager) + } class LockManager { +acquire() +ensure_can_write() @@ -75,12 +78,20 @@ classDiagram } class PydanticResourceManager~PydanticResource~ { + +migration_manager +load(force, store_cache) +save(model) +get_resource_diff(incoming_resource) +get_structured_model_diff(current_model, incoming_model) } + class MigrationManager~MigratableResource~ { + +model_class + +current_version: int + +migrate_resource(data) + +requires_backup(data) + } + class MutablePydanticResourceManager~MutablePydanticResource~ { +get(key, default) +set(key, value) @@ -129,6 +140,8 @@ classDiagram FMUDirectoryBase *-- LockManager FMUDirectoryBase *-- CacheManager + PydanticResourceManager o-- MigrationManager + CacheManager ..> MigrationManager ProjectFMUDirectory *-- ProjectConfigManager ProjectFMUDirectory *-- ChangelogManager ProjectFMUDirectory *-- MappingsManager @@ -140,10 +153,44 @@ The main library split is: - `FMUDirectoryBase` is the filesystem-centered abstraction. It owns the `.fmu` path, lock manager, cache manager, and generic read/write helpers. - `ProjectFMUDirectory` and `UserFMUDirectory` specialize the base class for project-local `.fmu/` directories and `$HOME/.fmu/`. - `PydanticResourceManager` is the generic resource engine for loading, saving, diffing, and caching JSON-backed Pydantic models. +- `MigrationManager` applies registered, forward-only schema migrations and validates the result as the current Pydantic model. - `MutablePydanticResourceManager` adds dot-notation `get`, `set`, `update`, `reset`, and merge behavior for editable resources. - `ProjectConfigManager`, `UserConfigManager`, `MappingsManager`, and `LogManager` bind specific Pydantic models to managed files inside `.fmu/`. - Directory objects compose the correct managers and delegate resource operations to them. +## Schema Migration + +`ProjectConfig`, `UserConfig`, and `InternalMappings` are versioned resources. Each +model declares its current schema version, and each resource manager has a +`MigrationManager` with the migration registry for that model. + +Migration is forward-only. Data without `schema_version` is treated as version 1. +Each registered function advances the data by one version. The manager rejects a +newer schema, a missing migration step, an invalid version, or data that does not +validate as the current model. + +The migration boundary depends on the resource operation: + +- **Load:** `PydanticResourceManager` decodes the stored JSON and asks + `MigrationManager` for a validated current model. Migration happens in memory. + Loading does not write the resource, create a cache revision, or add a changelog + entry. +- **Save:** The resource manager checks the write lock first. If the stored data is + unversioned or older, it stores the original JSON as a normal cache revision + before writing the current model. Normal cache retention applies. +- **Cache read:** `CacheManager` uses the migration manager to return old revisions + as current validated models. +- **Restore:** `CacheManager` migrates the selected revision before writing it, so + the restored resource uses the current schema. Project restore operations use the + existing restore changelog entry. + +There is no backward migration. After a current-schema resource is saved, an older +`fmu-settings` release can reject it as newer than supported. + +See the +[schema migration guide](src/fmu/settings/_migrations/README.md) +for the implementation, test, and coordinated release process. + ## Runtime Flow The full runtime spans multiple repositories, but `fmu-settings` owns the `.fmu/` directory operations used by the API and CLI. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d50e1cd..68dc209 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -32,3 +32,8 @@ ruff check ruff format --check mypy src tests ``` + +If you need to change the schema of `ProjectConfig`, `UserConfig`, or +`InternalMappings`, see the +[schema migration guide](src/fmu/settings/_migrations/README.md) for implementation +and testing details. diff --git a/README.md b/README.md index 67f4658..6163fe8 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,11 @@ ruff format --check mypy src tests ``` +If you need to change the schema of `ProjectConfig`, `UserConfig`, or +`InternalMappings`, see the +[schema migration guide](src/fmu/settings/_migrations/README.md) for implementation +and testing details. + See [CONTRIBUTING.md](CONTRIBUTING.md) for more. > [!NOTE] diff --git a/src/fmu/settings/__init__.py b/src/fmu/settings/__init__.py index b988aa2..f2e42c8 100644 --- a/src/fmu/settings/__init__.py +++ b/src/fmu/settings/__init__.py @@ -21,6 +21,7 @@ init_fmu_directory, init_user_fmu_directory, ) +from ._migrations import MigrationError from .models._enums import CacheResource from .models.mappings import ( InternalBaseMapping, @@ -47,6 +48,7 @@ "InternalWellboreIdentifierMapping", "InternalWellboreMappings", "InvalidFMUProjectPathError", + "MigrationError", "ProjectFMUDirectory", "REQUIRED_FMU_PROJECT_SUBDIRS", "UserFMUDirectory", diff --git a/src/fmu/settings/_fmu_dir.py b/src/fmu/settings/_fmu_dir.py index 6b1dc40..185c132 100644 --- a/src/fmu/settings/_fmu_dir.py +++ b/src/fmu/settings/_fmu_dir.py @@ -28,6 +28,9 @@ ) from .models.user_config import UserConfig +if TYPE_CHECKING: + from ._migrations import MigrationManager + logger: Final = null_logger(__name__) FMUConfigManager: TypeAlias = ProjectConfigManager | UserConfigManager @@ -494,13 +497,19 @@ def restore_from_cache( if manager is self.config: previous_max_revisions = self._cache_manager.max_revisions restored_config = self.cache.get_revision_content( - relative_path, revision_id, model_class=ProjectConfig + relative_path, + revision_id, + model_class=ProjectConfig, + migration_manager=self.config.migration_manager, ) self._cache_manager.max_revisions = restored_config.cache_max_revisions try: self.cache.restore_revision( - relative_path, revision_id, model_class=ProjectConfig + relative_path, + revision_id, + model_class=ProjectConfig, + migration_manager=self.config.migration_manager, ) except Exception: # Restore the previous runtime retention if config restore fails @@ -519,7 +528,13 @@ def restore_from_cache( return self.cache.restore_revision( - relative_path, revision_id, model_class=manager.model_class + relative_path, + revision_id, + model_class=cast("type[BaseModel]", manager.model_class), + migration_manager=cast( + "MigrationManager[BaseModel] | None", + manager.migration_manager, + ), ) # Refresh the resource manager's in-memory cache @@ -555,7 +570,13 @@ def get_cache_content( ) return self.cache.get_revision_content( - relative_path, revision_id, model_class=manager.model_class + relative_path, + revision_id, + model_class=cast("type[BaseModel]", manager.model_class), + migration_manager=cast( + "MigrationManager[BaseModel] | None", + manager.migration_manager, + ), ) def _cacheable_resource_managers( diff --git a/src/fmu/settings/_migrations/README.md b/src/fmu/settings/_migrations/README.md new file mode 100644 index 0000000..8f71223 --- /dev/null +++ b/src/fmu/settings/_migrations/README.md @@ -0,0 +1,274 @@ +# Schema migration guide + +Use this guide when a stored `.fmu` resource needs a new schema version. + +The supported resources and their migration registries are: + +- `ProjectConfig`: `project_config/` +- `UserConfig`: `user_config/` +- `InternalMappings`: `mappings/` + +## Decide whether to change the schema version + +Change the schema version when existing stored data needs a conversion before it +can be used by the current model. + +### Changes that need a migration + +Renaming a stored field needs a migration. For example, changing +`cache_max_revisions` to `max_cache_revisions` changes the JSON object from: + +```json +{ + "schema_version": 1, + "cache_max_revisions": 5 +} +``` + +to: + +```json +{ + "schema_version": 2, + "max_cache_revisions": 5 +} +``` + +Without a migration, the current model cannot recover the value stored under the +old field name. + +Other changes that need a migration include: + +- Removing a stored field when its value must be moved or preserved elsewhere. +- Making an optional field required and calculating its value from old data. +- Changing a field from one type to another, such as a string to a list of strings. +- Moving flat fields into a nested model. +- Changing the meaning of a value when old data must be converted to keep its + original meaning. + +### Changes that do not need a migration + +Adding an optional field with a safe default does not normally need a migration. +For example: + +```python +class ProjectConfig(ResettableBaseModel): + schema_version: Literal[1] = 1 + description: str | None = None +``` + +An existing version 1 file without `description` still validates. Pydantic supplies +`None`, so the schema version can remain 1. + +Other changes that do not normally need a migration include: + +- Making validation more permissive. +- Changing a model method that does not change stored data. +- Changing documentation or field descriptions. +- Adding a computed property that is not stored. + +Old stored data must still produce the correct current model when no migration is +added. If the old data needs conversion or its meaning would change, add a schema +version and migration. + +## Schema version contract + +Each migratable model declares one positive integer schema version. The literal and +default values must match: + +```python +schema_version: Literal[2] = 2 +``` + +Migration functions are forward-only. Each function advances the data by exactly +one version: + +```text +1 -> 2 -> 3 +``` + +Do not skip a version. Data without a `schema_version` field is treated as schema +version 1. + +## Add a migration + +The following example changes the current `ProjectConfig` from schema version 1 to +2 and renames `cache_max_revisions` to `max_cache_revisions`. + +### 1. Update the current model + +Edit the existing model in `models/project_config.py`. Do not create a second +`ProjectConfig` class. The `...` lines below represent all unchanged fields in the +current model, such as `version`, `created_at`, `created_by`, `masterdata`, and +`rms`. Only the schema version and the renamed field change in this example: + +```python +class ProjectConfig(ResettableBaseModel): + """The configuration file in a .fmu directory.""" + + schema_version: Literal[2] = 2 + # ... unchanged fields ... + max_cache_revisions: int = Field(default=5, ge=5) + # ... unchanged fields ... + + @classmethod + def reset(cls: type[Self]) -> Self: + """Reset the configuration to its defaults.""" + return cls( + # ... unchanged defaults ... + max_cache_revisions=5, + # ... unchanged defaults ... + ) +``` + +Search the source code and tests for the old field name. Update attribute access +such as `config.cache_max_revisions`, string keys such as +`set_config_value("cache_max_revisions", ...)`, API models, and test input +dictionaries. Any code that reads or writes this field must use the new name. + +### 2. Add one migration function + +Create `project_config/v1_to_v2.py`: + +```python +from typing import Any + + +def migrate_v1_to_v2(data: dict[str, Any]) -> dict[str, Any]: + """Migrate project config data from schema version 1 to 2.""" + data["max_cache_revisions"] = data.pop("cache_max_revisions", 5) + data["schema_version"] = 2 + return data +``` + +The migration manager gives each migration function a deep copy of the loaded +data. A migration function can therefore modify its input without changing the +original data. + +The returned data must: + +- Preserve all relevant stored values. +- Set `schema_version` to the next version. +- Be valid input for the next migration or the current model. + +### 3. Register the migration + +Update `project_config/__init__.py`: + +```python +from fmu.settings._migrations.manager import Migration + +from .v1_to_v2 import migrate_v1_to_v2 + +PROJECT_CONFIG_MIGRATIONS: dict[int, Migration] = { + 1: migrate_v1_to_v2, +} +``` + +The registry key is the source version. Key `1` registers the migration from +version 1 to version 2. + +Keep every migration when later versions are added: + +```python +PROJECT_CONFIG_MIGRATIONS: dict[int, Migration] = { + 1: migrate_v1_to_v2, + 2: migrate_v2_to_v3, +} +``` + +## Test the migration + +When you add a migration: + +- Keep `test_migration_manager.py` and the generic resource tests unchanged unless + the framework behavior changes. +- In `test_resource_migration.py`, update the affected resource tests that assert + its version, migration registry, or previous version data. +- Add resource-specific tests under `tests/test_migrations/`. For example, a + `ProjectConfig` migration can use `test_project_config_migration.py`. + +The resource-specific tests must cover conversion, load, save, cache restore, and +invalid data. Update the complete current version fixture used by +`tests/test_resources/test_migratable_models_up_to_date.py`. Keep the previous +version input with the resource-specific migration tests. + +Run the relevant checks: + +```text +uv run pytest tests/test_migrations +uv run pytest tests/test_resources/test_migratable_models_up_to_date.py +uv run ruff check +uv run ruff format --check +uv run mypy src tests +``` + +## Runtime behavior + +Migration is automatic during normal use: + +1. The resource manager reads old data. +2. The migration manager converts it in memory. +3. The resource manager returns the current validated model. +4. The stored file remains unchanged until a save occurs. + +On the first save: + +1. The write lock is checked. +2. The original stored data is added to the normal cache revisions. +3. The current model is written with the new schema version. +4. The new stored data is added to the normal cache revisions. + +Loading a resource does not create a changelog entry. A later user update or +restore uses the existing changelog behavior. + +When an old cache revision is restored, it is migrated before it is written. The +resource file therefore uses the current schema after the restore. + +Migrations are forward-only. After current-schema data is saved, an older +`fmu-settings` release can reject it as newer than its supported schema. + +## Release checklist for an `fmu-settings` schema version + +Use this checklist when releasing an `fmu-settings` package that contains a new +stored schema version. First prepare and publish `fmu-settings`. Then update the +downstream applications so that users receive the new package. + +### 1. Prepare the `fmu-settings` package + +- Confirm that the schema change needs a migration. +- Increase the model schema version by one. +- Add and register the migration from the previous version. +- Update `reset()` if it constructs or supplies a default for the changed field. +- Update test fixture dictionaries so that current-model fixtures use the new field + and current schema version. +- Update Python attribute access, dot-notation string keys, API code, and other + callers that use the changed field. +- Test loading, saving, caching, and restoring previous-version data. +- Run the full `fmu-settings` checks. + +### 2. Publish the `fmu-settings` package + +Publish a new GitHub release for `fmu-settings`. The publish workflow builds the +package and uploads it to PyPI. + +### 3. Release `fmu-settings-api` + +1. Set the minimum `fmu-settings` dependency in `fmu-settings-api` to the newly + released version. +2. Update the API lockfile and run the API tests. +3. Publish a new `fmu-settings-api` release. + +If the schema change affects fields exposed by the API, update and verify the +OpenAPI schema. Then regenerate and release the GUI client. + +### 4. Release `fmu-settings-cli` + +1. Set the minimum `fmu-settings-api` dependency in `fmu-settings-cli` to the newly + released API version. +2. Also set the CLI's direct `fmu-settings` dependency to the new version. +3. Update the CLI lockfile and run the CLI tests. +4. Publish a new `fmu-settings-cli` release. + +This release order keeps the schema implementation, API runtime, and CLI +distribution on compatible versions. diff --git a/src/fmu/settings/_migrations/__init__.py b/src/fmu/settings/_migrations/__init__.py new file mode 100644 index 0000000..607adfa --- /dev/null +++ b/src/fmu/settings/_migrations/__init__.py @@ -0,0 +1,9 @@ +"""Migration support for versioned resources stored in .fmu directories.""" + +from .manager import Migration, MigrationError, MigrationManager + +__all__ = [ + "Migration", + "MigrationError", + "MigrationManager", +] diff --git a/src/fmu/settings/_migrations/manager.py b/src/fmu/settings/_migrations/manager.py new file mode 100644 index 0000000..38ca76b --- /dev/null +++ b/src/fmu/settings/_migrations/manager.py @@ -0,0 +1,193 @@ +"""Forward-only migration support for versioned resource data.""" + +from __future__ import annotations + +import copy +from collections.abc import Callable, Mapping +from typing import Any, Final, Generic, Literal, TypeVar, get_args, get_origin + +from pydantic import BaseModel, ValidationError + +MigratableResource = TypeVar("MigratableResource", bound=BaseModel) +Migration = Callable[[dict[str, Any]], dict[str, Any]] + +LEGACY_SCHEMA_VERSION: Final = 1 +"""Version assigned to resources written before ``schema_version`` existed.""" + + +class MigrationError(ValueError): + """Raised when a resource cannot be migrated to the current schema.""" + + +class MigrationManager(Generic[MigratableResource]): + """Migrate versioned resource data to its current schema.""" + + def __init__( + self, + model_class: type[MigratableResource], + migrations: Mapping[int, Migration], + ) -> None: + """Initialize a migration manager. + + Args: + model_class: Current Pydantic model for the resource. + migrations: Migration functions keyed by their source version. + + Raises: + TypeError: If the model does not declare one positive integer + ``schema_version`` literal with the same default value. + """ + self.model_class = model_class + self.migrations = dict(migrations) + self.current_version = self._get_current_version() + + def migrate_resource(self, data: Any) -> MigratableResource: + """Migrate decoded JSON data and return a validated current model. + + Each migration must advance the schema by exactly one version. + + Args: + data: Decoded JSON data to migrate and validate. + + Returns: + The resource data as a validated current model. + + Raises: + MigrationError: If the data is not a JSON object, a schema version is + invalid or newer than supported, a required migration is missing or + fails, a migration does not advance by one version, or the final data + does not match the current model. + """ + if not isinstance(data, dict): + raise MigrationError( + f"{self.model_class.__name__} resource must be a JSON object" + ) + source_version = self._get_source_version(data) + migration_steps = self._get_migration_steps(source_version) + migrated_data = copy.deepcopy(data) if migration_steps else data.copy() + migrated_data.setdefault("schema_version", source_version) + for version, migration in migration_steps: + try: + migrated_data = migration(migrated_data) + except Exception as e: + raise MigrationError( + f"{self.model_class.__name__} migration from schema version " + f"{version} to {version + 1} failed" + ) from e + + result_version = self._validate_version(migrated_data.get("schema_version")) + expected_version = version + 1 + if result_version != expected_version: + raise MigrationError( + f"{self.model_class.__name__} migration from schema version " + f"{version} must set schema_version to {expected_version}, " + f"but set it to {result_version}" + ) + + try: + validated_model = self.model_class.model_validate(migrated_data) + except ValidationError as e: + raise MigrationError( + f"{self.model_class.__name__} data does not validate against current " + f"schema version {self.current_version}" + ) from e + + return validated_model + + def requires_backup(self, data: dict[str, Any]) -> bool: + """Return whether to back up the original data before a write. + + This method does not run migrations. It confirms that all required forward + migration steps exist, then requires a backup for unversioned or older data. + + Args: + data: Existing resource data that a write would replace. + + Returns: + Whether the original resource data must be backed up. + + Raises: + MigrationError: If the schema version is invalid or newer than supported, + or a required migration step is missing. + """ + source_version = self._get_source_version(data) + self._get_migration_steps(source_version) + is_unversioned = "schema_version" not in data + is_outdated = source_version < self.current_version + return is_unversioned or is_outdated + + def _get_migration_steps(self, source_version: int) -> list[tuple[int, Migration]]: + """Return and validate all migration steps needed by a source version.""" + if source_version > self.current_version: + raise MigrationError( + f"{self.model_class.__name__} schema version {source_version} is newer " + f"than supported version {self.current_version}; downgrade migration " + "is not supported" + ) + + steps: list[tuple[int, Migration]] = [] + for version in range(source_version, self.current_version): + migration = self.migrations.get(version) + if migration is None: + raise MigrationError( + f"Missing {self.model_class.__name__} migration from schema " + f"version {version} to {version + 1}" + ) + steps.append((version, migration)) + return steps + + def _get_current_version(self) -> int: + """Return the schema version declared by the current model. + + The model must define ``schema_version`` as one positive integer + ``Literal`` and use the same integer as its default value. For example, + schema version 2 must be declared as ``schema_version: Literal[2] = 2``. + + Returns: + The current schema version. + + Raises: + TypeError: If the model does not declare a valid schema version. + """ + schema_field = self.model_class.model_fields.get("schema_version") + if schema_field is None: + raise TypeError( + f"{self.model_class.__name__} must define a schema_version field" + ) + + if get_origin(schema_field.annotation) is not Literal: + raise TypeError( + f"{self.model_class.__name__}.schema_version must use Literal[int]" + ) + + literal_versions = get_args(schema_field.annotation) + if len(literal_versions) != 1: + raise TypeError( + f"{self.model_class.__name__}.schema_version must contain one version" + ) + + try: + current_version = self._validate_version(literal_versions[0]) + except MigrationError as e: + raise TypeError(str(e)) from e + + if schema_field.default != current_version: + raise TypeError( + f"{self.model_class.__name__}.schema_version must default to " + f"{current_version}" + ) + return current_version + + def _get_source_version(self, data: dict[str, Any]) -> int: + """Get the source version, using the legacy version when it is absent.""" + if "schema_version" not in data: + return LEGACY_SCHEMA_VERSION + return self._validate_version(data.get("schema_version")) + + @staticmethod + def _validate_version(value: Any) -> int: + """Validate a schema version value.""" + # bool is a subclass of int, so it must be rejected explicitly. + if isinstance(value, bool) or not isinstance(value, int) or value < 1: + raise MigrationError("Schema version must be a positive integer") + return value diff --git a/src/fmu/settings/_migrations/mappings/__init__.py b/src/fmu/settings/_migrations/mappings/__init__.py new file mode 100644 index 0000000..66b68ad --- /dev/null +++ b/src/fmu/settings/_migrations/mappings/__init__.py @@ -0,0 +1,7 @@ +"""Migration registry for mappings resources.""" + +from fmu.settings._migrations.manager import Migration + +MAPPINGS_MIGRATIONS: dict[int, Migration] = {} + +__all__ = ["MAPPINGS_MIGRATIONS"] diff --git a/src/fmu/settings/_migrations/project_config/__init__.py b/src/fmu/settings/_migrations/project_config/__init__.py new file mode 100644 index 0000000..8868a67 --- /dev/null +++ b/src/fmu/settings/_migrations/project_config/__init__.py @@ -0,0 +1,7 @@ +"""Migration registry for project config resources.""" + +from fmu.settings._migrations.manager import Migration + +PROJECT_CONFIG_MIGRATIONS: dict[int, Migration] = {} + +__all__ = ["PROJECT_CONFIG_MIGRATIONS"] diff --git a/src/fmu/settings/_migrations/user_config/__init__.py b/src/fmu/settings/_migrations/user_config/__init__.py new file mode 100644 index 0000000..450bdd7 --- /dev/null +++ b/src/fmu/settings/_migrations/user_config/__init__.py @@ -0,0 +1,7 @@ +"""Migration registry for user config resources.""" + +from fmu.settings._migrations.manager import Migration + +USER_CONFIG_MIGRATIONS: dict[int, Migration] = {} + +__all__ = ["USER_CONFIG_MIGRATIONS"] diff --git a/src/fmu/settings/_resources/cache_manager.py b/src/fmu/settings/_resources/cache_manager.py index 2e39815..483cc0f 100644 --- a/src/fmu/settings/_resources/cache_manager.py +++ b/src/fmu/settings/_resources/cache_manager.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from datetime import UTC, datetime, timedelta from pathlib import Path from typing import TYPE_CHECKING, ClassVar, Final, Self, TypeVar @@ -10,10 +11,12 @@ from pydantic import BaseModel, ValidationError from fmu.settings._logging import null_logger +from fmu.settings._migrations import MigrationError from fmu.settings._utils import path_is_dir, path_is_file if TYPE_CHECKING: from fmu.settings._fmu_dir import FMUDirectoryBase + from fmu.settings._migrations import MigrationManager logger: Final = null_logger(__name__) @@ -134,6 +137,8 @@ def get_revision_content( resource_file_path: Path | str, revision_id: str, model_class: type[RequestedModel], + *, + migration_manager: MigrationManager[RequestedModel] | None = None, ) -> RequestedModel: """Get the content of a specific cache revision as a validated model. @@ -143,12 +148,15 @@ def get_revision_content( revision_id: The revision filename (e.g., ``20260114T123045.123456Z-a1b2c3d4.json``). model_class: Pydantic model class to validate the cached content against. + migration_manager: Optional migration manager for versioned content. Returns: Validated Pydantic model instance containing the cached data. Raises: FileNotFoundError: If the cache revision doesn't exist. + MigrationError: If the cached content cannot be migrated to the current + schema. ValueError: If the cached file contains invalid JSON or fails model validation. """ @@ -164,8 +172,16 @@ def get_revision_content( content_str = self._fmu_dir.read_text_file(cache_relative) try: - return model_class.model_validate_json(content_str) - except ValidationError as e: + return self._validate_content( + content_str, + model_class, + migration_manager, + ) + except MigrationError as e: + raise MigrationError( + f"Cannot migrate cached content for '{resource_file_path}': {e}" + ) from e + except (json.JSONDecodeError, ValidationError) as e: raise ValueError( f"Invalid cached content for '{resource_file_path}': {e}" ) from e @@ -175,6 +191,8 @@ def restore_revision( resource_file_path: Path | str, revision_id: str, model_class: type[RequestedModel], + *, + migration_manager: MigrationManager[RequestedModel] | None = None, ) -> None: """Restore a resource file from a cache revision. @@ -187,15 +205,21 @@ def restore_revision( revision_id: The revision filename to restore from. model_class: Pydantic model class to validate the cached content against before restoring. + migration_manager: Optional migration manager for versioned content. Raises: FileNotFoundError: If the cache revision doesn't exist. + MigrationError: If the cached content cannot be migrated to the current + schema. ValueError: If the cached content is invalid JSON or fails model validation. """ resource_file_path = Path(resource_file_path) validated_model = self.get_revision_content( - resource_file_path, revision_id, model_class + resource_file_path, + revision_id, + model_class, + migration_manager=migration_manager, ) content_str = validated_model.model_dump_json(by_alias=True, indent=2) @@ -203,8 +227,12 @@ def restore_revision( current_content = self._fmu_dir.read_text_file(resource_file_path) try: - model_class.model_validate_json(current_content) - except ValidationError as e: + self._validate_content( + current_content, + model_class, + migration_manager, + ) + except (json.JSONDecodeError, MigrationError, ValidationError) as e: logger.warning( "Skipped caching current state of " f"{resource_file_path} before restore because it is invalid: {e}" @@ -218,6 +246,20 @@ def restore_revision( logger.info(f"Restored {resource_file_path} from cache revision {revision_id}") + @staticmethod + def _validate_content( + content: str, + model_class: type[RequestedModel], + migration_manager: MigrationManager[RequestedModel] | None, + ) -> RequestedModel: + """Validate current or versioned cached resource content.""" + if migration_manager is None: + return model_class.model_validate_json(content) + if migration_manager.model_class is not model_class: + raise TypeError("Migration manager model must match the requested model") + + return migration_manager.migrate_resource(json.loads(content)) + def _ensure_resource_cache_dir(self: Self, resource_file_path: Path) -> Path: """Create (if needed) and return the cache directory for resource file.""" self._cache_root_path(create=True) diff --git a/src/fmu/settings/_resources/config_managers.py b/src/fmu/settings/_resources/config_managers.py index 5cb5da8..11068ba 100644 --- a/src/fmu/settings/_resources/config_managers.py +++ b/src/fmu/settings/_resources/config_managers.py @@ -9,6 +9,9 @@ from typing import TYPE_CHECKING, Any, Final, Self from fmu.settings._logging import null_logger +from fmu.settings._migrations import MigrationManager +from fmu.settings._migrations.project_config import PROJECT_CONFIG_MIGRATIONS +from fmu.settings._migrations.user_config import USER_CONFIG_MIGRATIONS from fmu.settings.models.project_config import ProjectConfig from fmu.settings.models.user_config import UserConfig @@ -35,7 +38,14 @@ class ProjectConfigManager(MutablePydanticResourceManager[ProjectConfig]): def __init__(self: Self, fmu_dir: ProjectFMUDirectory) -> None: """Initializes the ProjectConfig resource manager.""" - super().__init__(fmu_dir, ProjectConfig) + super().__init__( + fmu_dir, + ProjectConfig, + migration_manager=MigrationManager( + ProjectConfig, + PROJECT_CONFIG_MIGRATIONS, + ), + ) @property def relative_path(self: Self) -> Path: @@ -94,7 +104,14 @@ class UserConfigManager(MutablePydanticResourceManager[UserConfig]): def __init__(self: Self, fmu_dir: UserFMUDirectory) -> None: """Initializes the UserConfig resource manager.""" - super().__init__(fmu_dir, UserConfig) + super().__init__( + fmu_dir, + UserConfig, + migration_manager=MigrationManager( + UserConfig, + USER_CONFIG_MIGRATIONS, + ), + ) @property def relative_path(self: Self) -> Path: diff --git a/src/fmu/settings/_resources/mappings_manager.py b/src/fmu/settings/_resources/mappings_manager.py index 0bb6d54..f557508 100644 --- a/src/fmu/settings/_resources/mappings_manager.py +++ b/src/fmu/settings/_resources/mappings_manager.py @@ -10,6 +10,8 @@ WellboreMappings, ) from fmu.datamodels.fmu_results.global_configuration import Stratigraphy +from fmu.settings._migrations import MigrationManager +from fmu.settings._migrations.mappings import MAPPINGS_MIGRATIONS from fmu.settings._resources.pydantic_resource_manager import PydanticResourceManager from fmu.settings.models.mappings import ( InternalMappings, @@ -31,7 +33,14 @@ class MappingsManager(PydanticResourceManager[InternalMappings]): def __init__(self: Self, fmu_dir: ProjectFMUDirectory) -> None: """Initializes the mappings resource manager.""" - super().__init__(fmu_dir, InternalMappings) + super().__init__( + fmu_dir, + InternalMappings, + migration_manager=MigrationManager( + InternalMappings, + MAPPINGS_MIGRATIONS, + ), + ) @property def relative_path(self: Self) -> Path: diff --git a/src/fmu/settings/_resources/pydantic_resource_manager.py b/src/fmu/settings/_resources/pydantic_resource_manager.py index 2b00dfc..66c95e8 100644 --- a/src/fmu/settings/_resources/pydantic_resource_manager.py +++ b/src/fmu/settings/_resources/pydantic_resource_manager.py @@ -3,11 +3,14 @@ from __future__ import annotations import json -from builtins import TypeError from typing import TYPE_CHECKING, Any, Generic, Self, TypeVar from pydantic import BaseModel, ValidationError +from fmu.settings._migrations import ( + MigrationError, + MigrationManager, +) from fmu.settings._utils import path_exists from fmu.settings.models.diff import ( ListFieldDiff, @@ -35,16 +38,29 @@ class PydanticResourceManager(Generic[PydanticResource]): automatic_caching: bool = True def __init__( - self: Self, fmu_dir: FMUDirectoryBase, model_class: type[PydanticResource] + self: Self, + fmu_dir: FMUDirectoryBase, + model_class: type[PydanticResource], + *, + migration_manager: MigrationManager[PydanticResource] | None = None, ) -> None: """Initializes the resource manager. Args: fmu_dir: The FMUDirectory instance model_class: The Pydantic model class this manager handles + migration_manager: Optional migration manager for versioned resources """ self.fmu_dir = fmu_dir self.model_class = model_class + if ( + migration_manager is not None + and migration_manager.model_class is not model_class + ): + raise TypeError( + "Migration manager model must match the resource manager model" + ) + self.migration_manager = migration_manager self._cache: PydanticResource | None = None @property @@ -109,8 +125,10 @@ def load( Validated Pydantic model Raises: - ValueError: If the resource file is missing or data does not match the - model schema + FileNotFoundError: If the resource file does not exist. + MigrationError: If the resource cannot be migrated to the current schema. + ValueError: If the resource file contains invalid JSON or does not match + the model schema. """ if self._cache is None or force: if not self.exists: @@ -122,7 +140,7 @@ def load( try: content = self.fmu_dir.read_text_file(self.relative_path) data = json.loads(content) - validated_model = self.model_class.model_validate(data) + validated_model = self._validate_data(data) if store_cache: self._cache = validated_model else: @@ -150,6 +168,7 @@ def save( model: Validated Pydantic model instance. """ self.fmu_dir._lock.ensure_can_write() + self._store_pre_migration_revision() json_data = model.model_dump_json(by_alias=True, indent=2) self.fmu_dir.write_text_file(self.relative_path, json_data) @@ -159,6 +178,47 @@ def save( self._cache = model + def _store_pre_migration_revision(self: Self) -> None: + """Store old disk content before a migration write. + + Invalid JSON and non-object content keep the existing save behavior, which lets + a caller replace a corrupt resource with an already validated model. + """ + if self.migration_manager is None: + return + + try: + content = self.fmu_dir.read_text_file(self.relative_path) + data = json.loads(content) + except (FileNotFoundError, json.JSONDecodeError): + return + + if not isinstance(data, dict): + return + + try: + requires_backup = self.migration_manager.requires_backup(data) + except MigrationError as e: + raise MigrationError( + f"Failed to migrate resource file for " + f"'{self.__class__.__name__}' at '{self.path}': {e}" + ) from e + + if requires_backup: + self.fmu_dir.cache.store_revision(self.relative_path, content) + + def _validate_data(self: Self, data: Any) -> PydanticResource: + """Validate decoded resource data, migrating it first when it is versioned.""" + if self.migration_manager is None: + return self.model_class.model_validate(data) + try: + return self.migration_manager.migrate_resource(data) + except MigrationError as e: + raise MigrationError( + f"Failed to migrate resource file for " + f"'{self.__class__.__name__}' at '{self.path}': {e}" + ) from e + def get_model_diff( self: Self, current_model: BaseModel, @@ -325,10 +385,18 @@ class MutablePydanticResourceManager(PydanticResourceManager[MutablePydanticReso """Manages the .fmu resource file.""" def __init__( - self: Self, fmu_dir: FMUDirectoryBase, resource: type[MutablePydanticResource] + self: Self, + fmu_dir: FMUDirectoryBase, + resource: type[MutablePydanticResource], + *, + migration_manager: MigrationManager[MutablePydanticResource] | None = None, ) -> None: """Initializes the resource manager.""" - super().__init__(fmu_dir, resource) + super().__init__( + fmu_dir, + resource, + migration_manager=migration_manager, + ) def get(self: Self, key: str, default: Any = None) -> Any: """Gets a resource value by key. diff --git a/tests/test_migrations/test_migration_manager.py b/tests/test_migrations/test_migration_manager.py new file mode 100644 index 0000000..f8b618d --- /dev/null +++ b/tests/test_migrations/test_migration_manager.py @@ -0,0 +1,318 @@ +"""Tests for forward-only resource migrations.""" + +from __future__ import annotations + +from typing import Any, Literal, cast + +import pytest +from pydantic import BaseModel + +from fmu.settings import MigrationError +from fmu.settings._migrations import MigrationManager + + +class VersionThreeModel(BaseModel): + """Test-only model for a resource at schema version three.""" + + schema_version: Literal[3] = 3 + value: str + migrations_applied: list[int] + + +class VersionOneModel(BaseModel): + """Test-only model for a resource at schema version one.""" + + schema_version: Literal[1] = 1 + value: str + + +def migrate_one_to_two(data: dict[str, Any]) -> dict[str, Any]: + """Migrate test data from version one to version two.""" + data["schema_version"] = 2 + data["migrations_applied"].append(1) + return data + + +def migrate_two_to_three(data: dict[str, Any]) -> dict[str, Any]: + """Migrate test data from version two to version three.""" + data["schema_version"] = 3 + data["migrations_applied"].append(2) + return data + + +def test_migration_manager_returns_current_data_unchanged() -> None: + """Current resource data does not need a migration.""" + data = { + "schema_version": 3, + "value": "current", + "migrations_applied": [], + } + manager = MigrationManager(VersionThreeModel, {}) + + result = manager.migrate_resource(data) + + assert result == VersionThreeModel.model_validate(data) + assert data == { + "schema_version": 3, + "value": "current", + "migrations_applied": [], + } + + +def test_migration_manager_applies_all_steps_without_mutating_input() -> None: + """Migration steps run in order and do not mutate their input.""" + data = { + "schema_version": 1, + "value": "old", + "migrations_applied": [], + } + manager = MigrationManager( + VersionThreeModel, + { + 1: migrate_one_to_two, + 2: migrate_two_to_three, + }, + ) + + result = manager.migrate_resource(data) + + assert result == VersionThreeModel( + value="old", + migrations_applied=[1, 2], + ) + assert data["migrations_applied"] == [] + assert data["schema_version"] == 1 + + +def test_migration_manager_treats_missing_version_as_version_one() -> None: + """Legacy data without schema_version starts at version one.""" + data = { + "value": "old", + "migrations_applied": [], + } + manager = MigrationManager( + VersionThreeModel, + { + 1: migrate_one_to_two, + 2: migrate_two_to_three, + }, + ) + + result = manager.migrate_resource(data) + + assert result.schema_version == 3 + assert result.migrations_applied == [1, 2] + assert "schema_version" not in data + + +def test_migration_manager_rejects_missing_step() -> None: + """Every intermediate migration must be registered.""" + manager = MigrationManager( + VersionThreeModel, + { + 2: migrate_two_to_three, + }, + ) + + with pytest.raises( + MigrationError, + match="Missing VersionThreeModel migration from schema version 1 to 2", + ): + manager.migrate_resource( + { + "schema_version": 1, + "value": "old", + "migrations_applied": [], + } + ) + + +def test_migration_manager_rejects_newer_schema() -> None: + """Forward-only migration does not accept a newer resource schema.""" + manager = MigrationManager(VersionThreeModel, {}) + + with pytest.raises( + MigrationError, + match=("VersionThreeModel schema version 4 is newer than supported version 3"), + ): + manager.migrate_resource( + { + "schema_version": 4, + "value": "future", + "migrations_applied": [], + } + ) + + +@pytest.mark.parametrize("version", [True, 0, -1, 1.0, "1", None]) +def test_migration_manager_rejects_invalid_schema_version(version: Any) -> None: + """Schema versions must be positive integers.""" + manager = MigrationManager(VersionThreeModel, {}) + + with pytest.raises( + MigrationError, match="Schema version must be a positive integer" + ): + manager.migrate_resource( + { + "schema_version": version, + "value": "invalid", + "migrations_applied": [], + } + ) + + +def test_migration_manager_rejects_wrong_step_version() -> None: + """A migration must advance by exactly one version.""" + + def skip_version(data: dict[str, Any]) -> dict[str, Any]: + data["schema_version"] = 3 + return data + + manager = MigrationManager( + VersionThreeModel, + { + 1: skip_version, + 2: migrate_two_to_three, + }, + ) + + with pytest.raises( + MigrationError, + match="must set schema_version to 2, but set it to 3", + ): + manager.migrate_resource( + { + "schema_version": 1, + "value": "old", + "migrations_applied": [], + } + ) + + +def test_migration_manager_wraps_step_error() -> None: + """An error from a migration identifies the failed version step.""" + + def failing_migration(data: dict[str, Any]) -> dict[str, Any]: + raise KeyError("missing value") + + manager = MigrationManager( + VersionThreeModel, + { + 1: failing_migration, + 2: migrate_two_to_three, + }, + ) + + with pytest.raises( + MigrationError, + match=("VersionThreeModel migration from schema version 1 to 2 failed"), + ) as error: + manager.migrate_resource( + { + "schema_version": 1, + "value": "old", + "migrations_applied": [], + } + ) + assert "missing value" not in str(error.value) + + +def test_migration_manager_validates_final_data() -> None: + """The result must validate against the current model.""" + + def remove_required_value(data: dict[str, Any]) -> dict[str, Any]: + data["schema_version"] = 2 + data.pop("value") + return data + + manager = MigrationManager( + VersionThreeModel, + { + 1: remove_required_value, + 2: migrate_two_to_three, + }, + ) + + with pytest.raises( + MigrationError, + match=( + "VersionThreeModel data does not validate against current schema version 3" + ), + ): + manager.migrate_resource( + { + "schema_version": 1, + "value": "old", + "migrations_applied": [], + } + ) + + +def test_migration_manager_requires_backup_for_unversioned_current_data() -> None: + """Normalizing an unversioned resource preserves its original form.""" + manager = MigrationManager(VersionOneModel, {}) + + assert manager.requires_backup({"value": "legacy"}) is True + assert manager.requires_backup({"schema_version": 1, "value": "current"}) is False + assert manager.migrate_resource({"value": "legacy"}).model_dump() == { + "schema_version": 1, + "value": "legacy", + } + + +def test_migration_manager_requires_literal_schema_version() -> None: + """Migratable models declare one literal current version.""" + + class InvalidModel(BaseModel): + schema_version: int = 1 + + with pytest.raises( + TypeError, match="InvalidModel.schema_version must use Literal\\[int\\]" + ): + MigrationManager(InvalidModel, {}) + + +def test_migration_manager_requires_schema_version_field() -> None: + """Migratable models must declare their current schema version.""" + + class InvalidModel(BaseModel): + value: str + + with pytest.raises( + TypeError, match="InvalidModel must define a schema_version field" + ): + MigrationManager(InvalidModel, {}) + + +def test_migration_manager_requires_one_literal_version() -> None: + """Migratable models cannot accept more than one schema version.""" + + class InvalidModel(BaseModel): + schema_version: Literal[1, 2] = 2 + + with pytest.raises( + TypeError, match="InvalidModel.schema_version must contain one version" + ): + MigrationManager(InvalidModel, {}) + + +def test_migration_manager_requires_current_version_default() -> None: + """The schema version default must equal its literal version.""" + + class InvalidModel(BaseModel): + schema_version: Literal[2] = cast("Literal[2]", 1) + + with pytest.raises( + TypeError, match="InvalidModel.schema_version must default to 2" + ): + MigrationManager(InvalidModel, {}) + + +def test_migration_manager_rejects_invalid_declared_version() -> None: + """Invalid model versions are configuration errors.""" + + class InvalidModel(BaseModel): + schema_version: Literal[0] = 0 + + with pytest.raises(TypeError, match="Schema version must be a positive integer"): + MigrationManager(InvalidModel, {}) diff --git a/tests/test_migrations/test_resource_migration.py b/tests/test_migrations/test_resource_migration.py new file mode 100644 index 0000000..3ae3e18 --- /dev/null +++ b/tests/test_migrations/test_resource_migration.py @@ -0,0 +1,473 @@ +"""Integration tests for migrations in Pydantic resource managers.""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import TYPE_CHECKING, Any, Literal, Self, cast +from unittest.mock import patch + +import pytest +from pydantic import BaseModel + +from fmu.settings import MigrationError +from fmu.settings._migrations import MigrationManager +from fmu.settings._resources.lock_manager import LockManager +from fmu.settings._resources.pydantic_resource_manager import PydanticResourceManager +from fmu.settings.models.change_info import ChangeType +from fmu.settings.models.mappings import InternalMappings +from fmu.settings.models.project_config import ProjectConfig + +if TYPE_CHECKING: + from fmu.settings._fmu_dir import ProjectFMUDirectory, UserFMUDirectory + + +class VersionTwoResource(BaseModel): + """Test-only resource model at schema version two.""" + + schema_version: Literal[2] = 2 + value: str + + +class VersionOneResource(BaseModel): + """Test-only resource model at schema version one.""" + + schema_version: Literal[1] = 1 + value: str + + +def migrate_one_to_two(data: dict[str, Any]) -> dict[str, Any]: + """Migrate the test resource to schema version two.""" + data["schema_version"] = 2 + return data + + +class MigratableResourceManager(PydanticResourceManager[VersionTwoResource]): + """Test manager with one registered migration.""" + + def __init__(self, fmu_dir: ProjectFMUDirectory) -> None: + """Initialize the test manager.""" + super().__init__( + fmu_dir, + VersionTwoResource, + migration_manager=MigrationManager( + VersionTwoResource, + {1: migrate_one_to_two}, + ), + ) + + @property + def relative_path(self: Self) -> Path: + """Return the test resource path.""" + return Path("migratable.json") + + +class VersionOneResourceManager(PydanticResourceManager[VersionOneResource]): + """Test manager for unversioned legacy resources.""" + + def __init__(self, fmu_dir: ProjectFMUDirectory) -> None: + """Initialize the test manager.""" + super().__init__( + fmu_dir, + VersionOneResource, + migration_manager=MigrationManager(VersionOneResource, {}), + ) + + @property + def relative_path(self: Self) -> Path: + """Return the test resource path.""" + return Path("version-one.json") + + +def test_load_migrates_in_memory_without_changing_disk( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Reading an old resource migrates only the in-memory representation.""" + manager = MigratableResourceManager(fmu_dir) + old_content = json.dumps({"schema_version": 1, "value": "old"}, indent=2) + fmu_dir.write_text_file(manager.relative_path, old_content) + + loaded = manager.load() + + assert loaded == VersionTwoResource(value="old") + assert manager._cache == loaded + assert fmu_dir.read_text_file(manager.relative_path) == old_content + assert fmu_dir.cache.list_revisions(manager.relative_path) == [] + + +def test_save_after_migration_backs_up_old_content( + fmu_dir: ProjectFMUDirectory, +) -> None: + """The first write backs up old data and writes the current schema.""" + manager = MigratableResourceManager(fmu_dir) + old_content = json.dumps({"schema_version": 1, "value": "old"}, indent=2) + fmu_dir.write_text_file(manager.relative_path, old_content) + loaded = manager.load() + + manager.save(loaded.model_copy(update={"value": "updated"})) + + disk_data = json.loads(fmu_dir.read_text_file(manager.relative_path)) + assert disk_data == {"schema_version": 2, "value": "updated"} + + cached_data = [ + json.loads(path.read_text(encoding="utf-8")) + for path in fmu_dir.cache.list_revisions(manager.relative_path) + ] + assert {"schema_version": 1, "value": "old"} in cached_data + assert {"schema_version": 2, "value": "updated"} in cached_data + + +def test_cached_old_schema_is_readable_and_restorable( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Existing cache APIs migrate an old backup before using its content.""" + manager = MigratableResourceManager(fmu_dir) + old_content = json.dumps({"schema_version": 1, "value": "old"}, indent=2) + fmu_dir.write_text_file(manager.relative_path, old_content) + manager.save(VersionTwoResource(value="updated")) + + old_revision = next( + path + for path in fmu_dir.cache.list_revisions(manager.relative_path) + if json.loads(path.read_text(encoding="utf-8"))["schema_version"] == 1 + ) + + cached_model = fmu_dir.cache.get_revision_content( + manager.relative_path, + old_revision.name, + manager.model_class, + migration_manager=manager.migration_manager, + ) + assert cached_model == VersionTwoResource(value="old") + + fmu_dir.cache.restore_revision( + manager.relative_path, + old_revision.name, + manager.model_class, + migration_manager=manager.migration_manager, + ) + assert json.loads(fmu_dir.read_text_file(manager.relative_path)) == { + "schema_version": 2, + "value": "old", + } + + +def test_cache_read_rejects_newer_schema_with_resource_context( + fmu_dir: ProjectFMUDirectory, +) -> None: + """A cache migration error identifies the affected resource.""" + manager = MigratableResourceManager(fmu_dir) + revision = fmu_dir.cache.store_revision( + manager.relative_path, + json.dumps({"schema_version": 3, "value": "future"}), + ) + assert revision is not None + + with pytest.raises( + MigrationError, + match=( + "Cannot migrate cached content for 'migratable.json'.*" + "schema version 3 is newer than supported version 2" + ), + ): + fmu_dir.cache.get_revision_content( + manager.relative_path, + revision.name, + manager.model_class, + migration_manager=manager.migration_manager, + ) + + +def test_unversioned_current_schema_is_backed_up_before_normalization( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Adding the current schema version preserves the unversioned source.""" + manager = VersionOneResourceManager(fmu_dir) + old_content = json.dumps({"value": "legacy"}, indent=2) + fmu_dir.write_text_file(manager.relative_path, old_content) + + manager.save(manager.load()) + + cached_data = [ + json.loads(path.read_text(encoding="utf-8")) + for path in fmu_dir.cache.list_revisions(manager.relative_path) + ] + assert {"value": "legacy"} in cached_data + assert {"schema_version": 1, "value": "legacy"} in cached_data + + +def test_force_load_without_store_cache_preserves_existing_cached_model( + fmu_dir: ProjectFMUDirectory, +) -> None: + """A forced migrated read can avoid replacing the in-memory cache.""" + manager = MigratableResourceManager(fmu_dir) + fmu_dir.write_text_file( + manager.relative_path, + json.dumps({"schema_version": 1, "value": "first"}), + ) + cached = manager.load() + fmu_dir.write_text_file( + manager.relative_path, + json.dumps({"schema_version": 1, "value": "second"}), + ) + + reloaded = manager.load(force=True, store_cache=False) + + assert reloaded == VersionTwoResource(value="second") + assert manager._cache == cached + + +def test_migration_save_respects_lock_before_backup( + fmu_dir: ProjectFMUDirectory, +) -> None: + """A foreign lock prevents both migration backup and resource write.""" + manager = MigratableResourceManager(fmu_dir) + old_content = json.dumps({"schema_version": 1, "value": "old"}, indent=2) + fmu_dir.write_text_file(manager.relative_path, old_content) + revisions_before = fmu_dir.cache.list_revisions(manager.relative_path) + lock = LockManager(fmu_dir) + + with ( + patch("socket.gethostname", return_value="other-host"), + patch("os.getpid", return_value=12345), + ): + lock.acquire() + + try: + with pytest.raises(PermissionError, match="Cannot write to .fmu directory"): + manager.save(VersionTwoResource(value="updated")) + finally: + with ( + patch("socket.gethostname", return_value="other-host"), + patch("os.getpid", return_value=12345), + ): + lock.release() + + assert fmu_dir.read_text_file(manager.relative_path) == old_content + assert fmu_dir.cache.list_revisions(manager.relative_path) == revisions_before + + +def test_current_schema_save_does_not_add_migration_backup( + fmu_dir: ProjectFMUDirectory, +) -> None: + """A normal save stores only the existing post-write revision.""" + manager = MigratableResourceManager(fmu_dir) + current = VersionTwoResource(value="current") + fmu_dir.write_text_file( + manager.relative_path, + current.model_dump_json(by_alias=True, indent=2), + ) + + manager.save(current.model_copy(update={"value": "updated"})) + + revisions = fmu_dir.cache.list_revisions(manager.relative_path) + assert len(revisions) == 1 + assert json.loads(revisions[0].read_text(encoding="utf-8")) == { + "schema_version": 2, + "value": "updated", + } + + +def test_save_replaces_non_object_json_without_migration_backup( + fmu_dir: ProjectFMUDirectory, +) -> None: + """A valid model can replace non-object JSON without preserving it.""" + manager = MigratableResourceManager(fmu_dir) + fmu_dir.write_text_file(manager.relative_path, json.dumps([])) + + manager.save(VersionTwoResource(value="current")) + + assert json.loads(fmu_dir.read_text_file(manager.relative_path)) == { + "schema_version": 2, + "value": "current", + } + revisions = fmu_dir.cache.list_revisions(manager.relative_path) + assert len(revisions) == 1 + assert json.loads(revisions[0].read_text(encoding="utf-8")) == { + "schema_version": 2, + "value": "current", + } + + +def test_save_rejects_newer_stored_schema_before_overwrite( + fmu_dir: ProjectFMUDirectory, +) -> None: + """A save does not overwrite stored data from a newer schema.""" + manager = MigratableResourceManager(fmu_dir) + future_content = json.dumps({"schema_version": 3, "value": "future"}) + fmu_dir.write_text_file(manager.relative_path, future_content) + + with pytest.raises( + MigrationError, + match=( + "Failed to migrate resource file for 'MigratableResourceManager'.*" + "schema version 3 is newer than supported version 2" + ), + ): + manager.save(VersionTwoResource(value="current")) + + assert fmu_dir.read_text_file(manager.relative_path) == future_content + assert fmu_dir.cache.list_revisions(manager.relative_path) == [] + + +def test_load_rejects_newer_schema_with_resource_context( + fmu_dir: ProjectFMUDirectory, +) -> None: + """A migration error identifies the resource that could not be loaded.""" + manager = MigratableResourceManager(fmu_dir) + fmu_dir.write_text_file( + manager.relative_path, + json.dumps({"schema_version": 3, "value": "future"}), + ) + + with pytest.raises( + ValueError, + match=( + "Failed to migrate resource file for 'MigratableResourceManager'.*" + "schema version 3 is newer than supported version 2" + ), + ): + manager.load() + + +def test_load_rejects_non_object_resource( + fmu_dir: ProjectFMUDirectory, +) -> None: + """A migratable resource must contain a JSON object.""" + manager = MigratableResourceManager(fmu_dir) + fmu_dir.write_text_file(manager.relative_path, json.dumps([])) + + with pytest.raises( + ValueError, + match=( + "Failed to migrate resource file for 'MigratableResourceManager'.*" + "VersionTwoResource resource must be a JSON object" + ), + ): + manager.load() + + +def test_project_resource_managers_have_version_one_migration_managers( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Project resources are wired without a dummy version two.""" + managers = [ + fmu_dir.config.migration_manager, + fmu_dir.mappings.migration_manager, + ] + + assert all(manager is not None for manager in managers) + assert all(manager.current_version == 1 for manager in managers if manager) + assert all(manager.migrations == {} for manager in managers if manager) + + +def test_user_config_has_version_one_migration_manager( + user_fmu_dir: UserFMUDirectory, +) -> None: + """User config is wired without a dummy version two.""" + manager = user_fmu_dir.config.migration_manager + + assert manager is not None + assert manager.current_version == 1 + assert manager.migrations == {} + + +def test_project_config_cache_boundary_handles_unversioned_revision( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Project cache APIs normalize and restore an unversioned config revision.""" + legacy_config = fmu_dir.config.load().model_dump(mode="json") + legacy_config.pop("schema_version") + legacy_config["cache_max_revisions"] = 7 + revision = fmu_dir.cache.store_revision( + "config.json", + json.dumps(legacy_config), + ) + assert revision is not None + + cached = fmu_dir.get_cache_content("config.json", revision.name) + assert isinstance(cached, ProjectConfig) + assert cached.schema_version == 1 + assert cached.cache_max_revisions == 7 # noqa: PLR2004 + + fmu_dir.restore_from_cache("config.json", revision.name) + + restored = fmu_dir.config.load() + assert restored.schema_version == 1 + assert restored.cache_max_revisions == 7 # noqa: PLR2004 + assert fmu_dir.cache_max_revisions == 7 # noqa: PLR2004 + assert fmu_dir.changelog.load().root[-1].change_type == ChangeType.restore + + +def test_mappings_cache_boundary_handles_unversioned_revision( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Project cache APIs use the mappings migration manager across the union.""" + legacy_mappings = InternalMappings().model_dump(mode="json") + legacy_mappings.pop("schema_version") + revision = fmu_dir.cache.store_revision( + "mappings.json", + json.dumps(legacy_mappings), + ) + assert revision is not None + + cached = fmu_dir.get_cache_content("mappings.json", revision.name) + assert isinstance(cached, InternalMappings) + assert cached.schema_version == 1 + + fmu_dir.restore_from_cache("mappings.json", revision.name) + + assert fmu_dir.mappings.load() == InternalMappings() + assert fmu_dir.changelog.load().root[-1].change_type == ChangeType.restore + + +def test_resource_manager_rejects_mismatched_migration_model( + fmu_dir: ProjectFMUDirectory, +) -> None: + """A resource manager cannot use migrations for a different model.""" + migration_manager = MigrationManager( + VersionTwoResource, + {1: migrate_one_to_two}, + ) + + with pytest.raises( + TypeError, + match="Migration manager model must match the resource manager model", + ): + PydanticResourceManager[VersionOneResource]( + fmu_dir, + VersionOneResource, + migration_manager=cast( + "MigrationManager[VersionOneResource]", + migration_manager, + ), + ) + + +def test_cache_manager_rejects_mismatched_migration_model( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Cache validation cannot return a model different from its annotation.""" + revision = fmu_dir.cache.store_revision( + "version-one.json", + VersionOneResource(value="current").model_dump_json(), + ) + assert revision is not None + migration_manager = MigrationManager( + VersionTwoResource, + {1: migrate_one_to_two}, + ) + + with pytest.raises( + TypeError, + match="Migration manager model must match the requested model", + ): + fmu_dir.cache.get_revision_content( + "version-one.json", + revision.name, + VersionOneResource, + migration_manager=cast( + "MigrationManager[VersionOneResource]", + migration_manager, + ), + ) From a9de6d1b4bd647f532882ce2f98b77a48b66f1e7 Mon Sep 17 00:00:00 2001 From: Muhammad Gibran Alfarizi Date: Wed, 5 Aug 2026 13:25:16 +0200 Subject: [PATCH 2/8] MAINT: Address feedbacks --- ARCHITECTURE.md | 8 ++-- src/fmu/settings/_migrations/README.md | 19 +++++---- src/fmu/settings/_migrations/__init__.py | 4 +- src/fmu/settings/_migrations/manager.py | 41 ++++++++++--------- .../settings/_migrations/mappings/__init__.py | 6 +-- .../_migrations/project_config/__init__.py | 6 +-- .../_migrations/user_config/__init__.py | 6 +-- src/fmu/settings/_resources/cache_manager.py | 8 ++-- .../_resources/pydantic_resource_manager.py | 17 ++++---- .../test_migrations/test_migration_manager.py | 13 +----- .../test_resource_migration.py | 38 +---------------- 11 files changed, 61 insertions(+), 105 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index b28c960..202f6fb 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -89,7 +89,7 @@ classDiagram +model_class +current_version: int +migrate_resource(data) - +requires_backup(data) + +requires_migration(data) } class MutablePydanticResourceManager~MutablePydanticResource~ { @@ -165,7 +165,7 @@ model declares its current schema version, and each resource manager has a `MigrationManager` with the migration registry for that model. Migration is forward-only. Data without `schema_version` is treated as version 1. -Each registered function advances the data by one version. The manager rejects a +Each registered function increments the schema version by one. The manager rejects a newer schema, a missing migration step, an invalid version, or data that does not validate as the current model. @@ -176,8 +176,8 @@ The migration boundary depends on the resource operation: Loading does not write the resource, create a cache revision, or add a changelog entry. - **Save:** The resource manager checks the write lock first. If the stored data is - unversioned or older, it stores the original JSON as a normal cache revision - before writing the current model. Normal cache retention applies. + older, it stores the original JSON as a cache revision + before writing the current model. Existing cache retention applies. - **Cache read:** `CacheManager` uses the migration manager to return old revisions as current validated models. - **Restore:** `CacheManager` migrates the selected revision before writing it, so diff --git a/src/fmu/settings/_migrations/README.md b/src/fmu/settings/_migrations/README.md index 8f71223..c2eaceb 100644 --- a/src/fmu/settings/_migrations/README.md +++ b/src/fmu/settings/_migrations/README.md @@ -69,7 +69,7 @@ Other changes that do not normally need a migration include: Old stored data must still produce the correct current model when no migration is added. If the old data needs conversion or its meaning would change, add a schema -version and migration. +version and migration function. ## Schema version contract @@ -80,8 +80,8 @@ default values must match: schema_version: Literal[2] = 2 ``` -Migration functions are forward-only. Each function advances the data by exactly -one version: +Migration functions are forward-only. Each function increments `schema_version` by +exactly one: ```text 1 -> 2 -> 3 @@ -126,7 +126,7 @@ such as `config.cache_max_revisions`, string keys such as `set_config_value("cache_max_revisions", ...)`, API models, and test input dictionaries. Any code that reads or writes this field must use the new name. -### 2. Add one migration function +### 2. Add a migration script with migration function Create `project_config/v1_to_v2.py`: @@ -156,11 +156,11 @@ The returned data must: Update `project_config/__init__.py`: ```python -from fmu.settings._migrations.manager import Migration +from fmu.settings._migrations.manager import MigrationFunction from .v1_to_v2 import migrate_v1_to_v2 -PROJECT_CONFIG_MIGRATIONS: dict[int, Migration] = { +PROJECT_CONFIG_MIGRATIONS: dict[int, MigrationFunction] = { 1: migrate_v1_to_v2, } ``` @@ -171,7 +171,7 @@ version 1 to version 2. Keep every migration when later versions are added: ```python -PROJECT_CONFIG_MIGRATIONS: dict[int, Migration] = { +PROJECT_CONFIG_MIGRATIONS: dict[int, MigrationFunction] = { 1: migrate_v1_to_v2, 2: migrate_v2_to_v3, } @@ -215,9 +215,10 @@ Migration is automatic during normal use: On the first save: 1. The write lock is checked. -2. The original stored data is added to the normal cache revisions. +2. The current stored data is added to a cache revision when it is older. 3. The current model is written with the new schema version. -4. The new stored data is added to the normal cache revisions. +4. When automatic caching is enabled, the newly written data is added to a cache + revision. Loading a resource does not create a changelog entry. A later user update or restore uses the existing changelog behavior. diff --git a/src/fmu/settings/_migrations/__init__.py b/src/fmu/settings/_migrations/__init__.py index 607adfa..a368947 100644 --- a/src/fmu/settings/_migrations/__init__.py +++ b/src/fmu/settings/_migrations/__init__.py @@ -1,9 +1,9 @@ """Migration support for versioned resources stored in .fmu directories.""" -from .manager import Migration, MigrationError, MigrationManager +from .manager import MigrationError, MigrationFunction, MigrationManager __all__ = [ - "Migration", "MigrationError", + "MigrationFunction", "MigrationManager", ] diff --git a/src/fmu/settings/_migrations/manager.py b/src/fmu/settings/_migrations/manager.py index 38ca76b..bcb2681 100644 --- a/src/fmu/settings/_migrations/manager.py +++ b/src/fmu/settings/_migrations/manager.py @@ -9,7 +9,7 @@ from pydantic import BaseModel, ValidationError MigratableResource = TypeVar("MigratableResource", bound=BaseModel) -Migration = Callable[[dict[str, Any]], dict[str, Any]] +MigrationFunction = Callable[[dict[str, Any]], dict[str, Any]] LEGACY_SCHEMA_VERSION: Final = 1 """Version assigned to resources written before ``schema_version`` existed.""" @@ -25,7 +25,7 @@ class MigrationManager(Generic[MigratableResource]): def __init__( self, model_class: type[MigratableResource], - migrations: Mapping[int, Migration], + migrations: Mapping[int, MigrationFunction], ) -> None: """Initialize a migration manager. @@ -42,9 +42,9 @@ def __init__( self.current_version = self._get_current_version() def migrate_resource(self, data: Any) -> MigratableResource: - """Migrate decoded JSON data and return a validated current model. + """Migrate decoded JSON data with predefined migration function and validate it. - Each migration must advance the schema by exactly one version. + Each migration function must increment the schema version by exactly one. Args: data: Decoded JSON data to migrate and validate. @@ -55,8 +55,8 @@ def migrate_resource(self, data: Any) -> MigratableResource: Raises: MigrationError: If the data is not a JSON object, a schema version is invalid or newer than supported, a required migration is missing or - fails, a migration does not advance by one version, or the final data - does not match the current model. + fails, a migration function does not increment the schema version by + one, or the final data does not match the current model. """ if not isinstance(data, dict): raise MigrationError( @@ -66,9 +66,9 @@ def migrate_resource(self, data: Any) -> MigratableResource: migration_steps = self._get_migration_steps(source_version) migrated_data = copy.deepcopy(data) if migration_steps else data.copy() migrated_data.setdefault("schema_version", source_version) - for version, migration in migration_steps: + for version, migration_function in migration_steps: try: - migrated_data = migration(migrated_data) + migrated_data = migration_function(migrated_data) except Exception as e: raise MigrationError( f"{self.model_class.__name__} migration from schema version " @@ -94,17 +94,18 @@ def migrate_resource(self, data: Any) -> MigratableResource: return validated_model - def requires_backup(self, data: dict[str, Any]) -> bool: - """Return whether to back up the original data before a write. + def requires_migration(self, data: dict[str, Any]) -> bool: + """Return whether the data requires a migration before a write. This method does not run migrations. It confirms that all required forward - migration steps exist, then requires a backup for unversioned or older data. + migration steps exist, then checks whether the source schema is older than + the current schema. Args: data: Existing resource data that a write would replace. Returns: - Whether the original resource data must be backed up. + Whether the resource data requires migration. Raises: MigrationError: If the schema version is invalid or newer than supported, @@ -112,11 +113,11 @@ def requires_backup(self, data: dict[str, Any]) -> bool: """ source_version = self._get_source_version(data) self._get_migration_steps(source_version) - is_unversioned = "schema_version" not in data - is_outdated = source_version < self.current_version - return is_unversioned or is_outdated + return source_version < self.current_version - def _get_migration_steps(self, source_version: int) -> list[tuple[int, Migration]]: + def _get_migration_steps( + self, source_version: int + ) -> list[tuple[int, MigrationFunction]]: """Return and validate all migration steps needed by a source version.""" if source_version > self.current_version: raise MigrationError( @@ -125,15 +126,15 @@ def _get_migration_steps(self, source_version: int) -> list[tuple[int, Migration "is not supported" ) - steps: list[tuple[int, Migration]] = [] + steps: list[tuple[int, MigrationFunction]] = [] for version in range(source_version, self.current_version): - migration = self.migrations.get(version) - if migration is None: + migration_function = self.migrations.get(version) + if migration_function is None: raise MigrationError( f"Missing {self.model_class.__name__} migration from schema " f"version {version} to {version + 1}" ) - steps.append((version, migration)) + steps.append((version, migration_function)) return steps def _get_current_version(self) -> int: diff --git a/src/fmu/settings/_migrations/mappings/__init__.py b/src/fmu/settings/_migrations/mappings/__init__.py index 66b68ad..fa3ce68 100644 --- a/src/fmu/settings/_migrations/mappings/__init__.py +++ b/src/fmu/settings/_migrations/mappings/__init__.py @@ -1,7 +1,7 @@ -"""Migration registry for mappings resources.""" +"""Migration function registry for mappings resources.""" -from fmu.settings._migrations.manager import Migration +from fmu.settings._migrations.manager import MigrationFunction -MAPPINGS_MIGRATIONS: dict[int, Migration] = {} +MAPPINGS_MIGRATIONS: dict[int, MigrationFunction] = {} __all__ = ["MAPPINGS_MIGRATIONS"] diff --git a/src/fmu/settings/_migrations/project_config/__init__.py b/src/fmu/settings/_migrations/project_config/__init__.py index 8868a67..6bfdc13 100644 --- a/src/fmu/settings/_migrations/project_config/__init__.py +++ b/src/fmu/settings/_migrations/project_config/__init__.py @@ -1,7 +1,7 @@ -"""Migration registry for project config resources.""" +"""Migration function registry for project config resources.""" -from fmu.settings._migrations.manager import Migration +from fmu.settings._migrations.manager import MigrationFunction -PROJECT_CONFIG_MIGRATIONS: dict[int, Migration] = {} +PROJECT_CONFIG_MIGRATIONS: dict[int, MigrationFunction] = {} __all__ = ["PROJECT_CONFIG_MIGRATIONS"] diff --git a/src/fmu/settings/_migrations/user_config/__init__.py b/src/fmu/settings/_migrations/user_config/__init__.py index 450bdd7..f4b04a2 100644 --- a/src/fmu/settings/_migrations/user_config/__init__.py +++ b/src/fmu/settings/_migrations/user_config/__init__.py @@ -1,7 +1,7 @@ -"""Migration registry for user config resources.""" +"""Migration function registry for user config resources.""" -from fmu.settings._migrations.manager import Migration +from fmu.settings._migrations.manager import MigrationFunction -USER_CONFIG_MIGRATIONS: dict[int, Migration] = {} +USER_CONFIG_MIGRATIONS: dict[int, MigrationFunction] = {} __all__ = ["USER_CONFIG_MIGRATIONS"] diff --git a/src/fmu/settings/_resources/cache_manager.py b/src/fmu/settings/_resources/cache_manager.py index 483cc0f..6ce9ecb 100644 --- a/src/fmu/settings/_resources/cache_manager.py +++ b/src/fmu/settings/_resources/cache_manager.py @@ -172,7 +172,7 @@ def get_revision_content( content_str = self._fmu_dir.read_text_file(cache_relative) try: - return self._validate_content( + return self._migrate_and_validate_content( content_str, model_class, migration_manager, @@ -227,7 +227,7 @@ def restore_revision( current_content = self._fmu_dir.read_text_file(resource_file_path) try: - self._validate_content( + self._migrate_and_validate_content( current_content, model_class, migration_manager, @@ -247,12 +247,12 @@ def restore_revision( logger.info(f"Restored {resource_file_path} from cache revision {revision_id}") @staticmethod - def _validate_content( + def _migrate_and_validate_content( content: str, model_class: type[RequestedModel], migration_manager: MigrationManager[RequestedModel] | None, ) -> RequestedModel: - """Validate current or versioned cached resource content.""" + """Migrate versioned content and validate it, or validate current content.""" if migration_manager is None: return model_class.model_validate_json(content) if migration_manager.model_class is not model_class: diff --git a/src/fmu/settings/_resources/pydantic_resource_manager.py b/src/fmu/settings/_resources/pydantic_resource_manager.py index 66c95e8..67644f0 100644 --- a/src/fmu/settings/_resources/pydantic_resource_manager.py +++ b/src/fmu/settings/_resources/pydantic_resource_manager.py @@ -140,7 +140,7 @@ def load( try: content = self.fmu_dir.read_text_file(self.relative_path) data = json.loads(content) - validated_model = self._validate_data(data) + validated_model = self._migrate_and_validate_data(data) if store_cache: self._cache = validated_model else: @@ -179,10 +179,9 @@ def save( self._cache = model def _store_pre_migration_revision(self: Self) -> None: - """Store old disk content before a migration write. + """Preserve current disk content before replacing it with migrated data. - Invalid JSON and non-object content keep the existing save behavior, which lets - a caller replace a corrupt resource with an already validated model. + Invalid JSON and non-object content are not stored as pre-migration revisions. """ if self.migration_manager is None: return @@ -197,18 +196,18 @@ def _store_pre_migration_revision(self: Self) -> None: return try: - requires_backup = self.migration_manager.requires_backup(data) + requires_migration = self.migration_manager.requires_migration(data) except MigrationError as e: raise MigrationError( - f"Failed to migrate resource file for " + f"Failed to check migration requirements for resource file " f"'{self.__class__.__name__}' at '{self.path}': {e}" ) from e - if requires_backup: + if requires_migration: self.fmu_dir.cache.store_revision(self.relative_path, content) - def _validate_data(self: Self, data: Any) -> PydanticResource: - """Validate decoded resource data, migrating it first when it is versioned.""" + def _migrate_and_validate_data(self: Self, data: Any) -> PydanticResource: + """Migrate decoded data with registered functions and validate the result.""" if self.migration_manager is None: return self.model_class.model_validate(data) try: diff --git a/tests/test_migrations/test_migration_manager.py b/tests/test_migrations/test_migration_manager.py index f8b618d..0ab2343 100644 --- a/tests/test_migrations/test_migration_manager.py +++ b/tests/test_migrations/test_migration_manager.py @@ -98,6 +98,7 @@ def test_migration_manager_treats_missing_version_as_version_one() -> None: }, ) + assert manager.requires_migration(data) is True result = manager.migrate_resource(data) assert result.schema_version == 3 @@ -248,18 +249,6 @@ def remove_required_value(data: dict[str, Any]) -> dict[str, Any]: ) -def test_migration_manager_requires_backup_for_unversioned_current_data() -> None: - """Normalizing an unversioned resource preserves its original form.""" - manager = MigrationManager(VersionOneModel, {}) - - assert manager.requires_backup({"value": "legacy"}) is True - assert manager.requires_backup({"schema_version": 1, "value": "current"}) is False - assert manager.migrate_resource({"value": "legacy"}).model_dump() == { - "schema_version": 1, - "value": "legacy", - } - - def test_migration_manager_requires_literal_schema_version() -> None: """Migratable models declare one literal current version.""" diff --git a/tests/test_migrations/test_resource_migration.py b/tests/test_migrations/test_resource_migration.py index 3ae3e18..d3c9fa6 100644 --- a/tests/test_migrations/test_resource_migration.py +++ b/tests/test_migrations/test_resource_migration.py @@ -62,23 +62,6 @@ def relative_path(self: Self) -> Path: return Path("migratable.json") -class VersionOneResourceManager(PydanticResourceManager[VersionOneResource]): - """Test manager for unversioned legacy resources.""" - - def __init__(self, fmu_dir: ProjectFMUDirectory) -> None: - """Initialize the test manager.""" - super().__init__( - fmu_dir, - VersionOneResource, - migration_manager=MigrationManager(VersionOneResource, {}), - ) - - @property - def relative_path(self: Self) -> Path: - """Return the test resource path.""" - return Path("version-one.json") - - def test_load_migrates_in_memory_without_changing_disk( fmu_dir: ProjectFMUDirectory, ) -> None: @@ -178,24 +161,6 @@ def test_cache_read_rejects_newer_schema_with_resource_context( ) -def test_unversioned_current_schema_is_backed_up_before_normalization( - fmu_dir: ProjectFMUDirectory, -) -> None: - """Adding the current schema version preserves the unversioned source.""" - manager = VersionOneResourceManager(fmu_dir) - old_content = json.dumps({"value": "legacy"}, indent=2) - fmu_dir.write_text_file(manager.relative_path, old_content) - - manager.save(manager.load()) - - cached_data = [ - json.loads(path.read_text(encoding="utf-8")) - for path in fmu_dir.cache.list_revisions(manager.relative_path) - ] - assert {"value": "legacy"} in cached_data - assert {"schema_version": 1, "value": "legacy"} in cached_data - - def test_force_load_without_store_cache_preserves_existing_cached_model( fmu_dir: ProjectFMUDirectory, ) -> None: @@ -300,7 +265,8 @@ def test_save_rejects_newer_stored_schema_before_overwrite( with pytest.raises( MigrationError, match=( - "Failed to migrate resource file for 'MigratableResourceManager'.*" + "Failed to check migration requirements for resource file " + "'MigratableResourceManager'.*" "schema version 3 is newer than supported version 2" ), ): From b6b4e99c3ae465409060938776a194784bc0939c Mon Sep 17 00:00:00 2001 From: Muhammad Gibran Alfarizi Date: Wed, 5 Aug 2026 15:14:24 +0200 Subject: [PATCH 3/8] ENH: Add best-effore migration backups --- ARCHITECTURE.md | 10 ++- src/fmu/settings/_migrations/README.md | 10 ++- .../_resources/pydantic_resource_manager.py | 62 +++++++++++++++---- .../test_resource_migration.py | 41 +++++++++++- 4 files changed, 105 insertions(+), 18 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 202f6fb..3ce33ca 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -176,8 +176,10 @@ The migration boundary depends on the resource operation: Loading does not write the resource, create a cache revision, or add a changelog entry. - **Save:** The resource manager checks the write lock first. If the stored data is - older, it stores the original JSON as a cache revision - before writing the current model. Existing cache retention applies. + older, it attempts to store the exact original JSON under + `.fmu/migration-backups/` before writing the current model. This backup is + best-effort: a file-system failure is logged and does not stop the save. The newly + written current data is still added to the cache. - **Cache read:** `CacheManager` uses the migration manager to return old revisions as current validated models. - **Restore:** `CacheManager` migrates the selected revision before writing it, so @@ -185,7 +187,9 @@ The migration boundary depends on the resource operation: existing restore changelog entry. There is no backward migration. After a current-schema resource is saved, an older -`fmu-settings` release can reject it as newer than supported. +`fmu-settings` release can reject it as newer than supported. Migration backups are +not loaded or restored by the library. Users must copy one back manually when they +need to use the data with an older release. See the [schema migration guide](src/fmu/settings/_migrations/README.md) diff --git a/src/fmu/settings/_migrations/README.md b/src/fmu/settings/_migrations/README.md index c2eaceb..435f6bd 100644 --- a/src/fmu/settings/_migrations/README.md +++ b/src/fmu/settings/_migrations/README.md @@ -215,7 +215,10 @@ Migration is automatic during normal use: On the first save: 1. The write lock is checked. -2. The current stored data is added to a cache revision when it is older. +2. If the stored data needs migration, the resource manager tries to save a copy of + the original JSON under `.fmu/migration-backups/`. This backup is separate from + the normal cache, is not removed automatically, and a failure to write it does not + stop the save. 3. The current model is written with the new schema version. 4. When automatic caching is enabled, the newly written data is added to a cache revision. @@ -224,7 +227,10 @@ Loading a resource does not create a changelog entry. A later user update or restore uses the existing changelog behavior. When an old cache revision is restored, it is migrated before it is written. The -resource file therefore uses the current schema after the restore. +resource file therefore uses the current schema after the restore. Migration backups +are only saved for manual recovery, and the library does not read or restore them. To +roll back, first replace the current resource file with the appropriate backup, then +run the older release. Migrations are forward-only. After current-schema data is saved, an older `fmu-settings` release can reject it as newer than its supported schema. diff --git a/src/fmu/settings/_resources/pydantic_resource_manager.py b/src/fmu/settings/_resources/pydantic_resource_manager.py index 67644f0..093e122 100644 --- a/src/fmu/settings/_resources/pydantic_resource_manager.py +++ b/src/fmu/settings/_resources/pydantic_resource_manager.py @@ -3,10 +3,14 @@ from __future__ import annotations import json -from typing import TYPE_CHECKING, Any, Generic, Self, TypeVar +from datetime import UTC, datetime +from pathlib import Path +from typing import TYPE_CHECKING, Any, Final, Generic, Self, TypeVar +from uuid import uuid4 from pydantic import BaseModel, ValidationError +from fmu.settings._logging import null_logger from fmu.settings._migrations import ( MigrationError, MigrationManager, @@ -24,13 +28,14 @@ from collections.abc import Mapping # Avoid circular dependency for type hint in __init__ only - from pathlib import Path - from fmu.settings._fmu_dir import FMUDirectoryBase PydanticResource = TypeVar("PydanticResource", bound=BaseModel) MutablePydanticResource = TypeVar("MutablePydanticResource", bound=ResettableBaseModel) +MIGRATION_BACKUP_DIRECTORY: Final = Path("migration-backups") +logger: Final = null_logger(__name__) + class PydanticResourceManager(Generic[PydanticResource]): """Base class for managing resources represented by Pydantic models.""" @@ -168,7 +173,7 @@ def save( model: Validated Pydantic model instance. """ self.fmu_dir._lock.ensure_can_write() - self._store_pre_migration_revision() + self._store_migration_backup() json_data = model.model_dump_json(by_alias=True, indent=2) self.fmu_dir.write_text_file(self.relative_path, json_data) @@ -178,12 +183,15 @@ def save( self._cache = model - def _store_pre_migration_revision(self: Self) -> None: - """Preserve current disk content before replacing it with migrated data. + def _store_migration_backup(self: Self) -> None: + """Try to back up older data before saving the migrated version. - Invalid JSON and non-object content are not stored as pre-migration revisions. + The backup is best effort. Invalid JSON and non-object content are skipped. + File-system errors while writing the backup are logged, but do not stop the + save. """ - if self.migration_manager is None: + migration_manager = self.migration_manager + if migration_manager is None: return try: @@ -196,15 +204,47 @@ def _store_pre_migration_revision(self: Self) -> None: return try: - requires_migration = self.migration_manager.requires_migration(data) + requires_migration = migration_manager.requires_migration(data) except MigrationError as e: raise MigrationError( f"Failed to check migration requirements for resource file " f"'{self.__class__.__name__}' at '{self.path}': {e}" ) from e - if requires_migration: - self.fmu_dir.cache.store_revision(self.relative_path, content) + if not requires_migration: + return + + source_version = data.get("schema_version", 1) + self._write_migration_backup(content, source_version) + + def _write_migration_backup(self: Self, content: str, source_version: int) -> None: + """Write a best-effort migration backup of the original content. + + For example, a ``config.json`` backup can be stored under + ``.fmu/migration-backups/config/`` as + ``--ProjectConfig-v1.json``. + """ + timestamp = datetime.now(UTC).strftime("%Y%m%dT%H%M%S.%fZ") + token = uuid4().hex[:8] + backup_directory = ( + MIGRATION_BACKUP_DIRECTORY + / self.relative_path.parent + / self.relative_path.stem + ) + backup_filename = ( + f"{timestamp}-{token}-{self.model_class.__name__}-v{source_version}" + f"{self.relative_path.suffix}" + ) + backup_path = backup_directory / backup_filename + + try: + self.fmu_dir.write_text_file(backup_path, content) + except OSError as e: + logger.warning( + f"Failed to save migration backup for '{self.path}' at " + f"'{self.fmu_dir.get_file_path(backup_path)}'. " + f"Continuing without it: {e}" + ) def _migrate_and_validate_data(self: Self, data: Any) -> PydanticResource: """Migrate decoded data with registered functions and validate the result.""" diff --git a/tests/test_migrations/test_resource_migration.py b/tests/test_migrations/test_resource_migration.py index d3c9fa6..c8ca0d8 100644 --- a/tests/test_migrations/test_resource_migration.py +++ b/tests/test_migrations/test_resource_migration.py @@ -81,7 +81,7 @@ def test_load_migrates_in_memory_without_changing_disk( def test_save_after_migration_backs_up_old_content( fmu_dir: ProjectFMUDirectory, ) -> None: - """The first write backs up old data and writes the current schema.""" + """The first write backs up old data outside the normal cache.""" manager = MigratableResourceManager(fmu_dir) old_content = json.dumps({"schema_version": 1, "value": "old"}, indent=2) fmu_dir.write_text_file(manager.relative_path, old_content) @@ -92,11 +92,17 @@ def test_save_after_migration_backs_up_old_content( disk_data = json.loads(fmu_dir.read_text_file(manager.relative_path)) assert disk_data == {"schema_version": 2, "value": "updated"} + backup_directory = fmu_dir.get_file_path("migration-backups/migratable") + backups = [path for path in backup_directory.iterdir() if path.is_file()] + assert len(backups) == 1 + assert backups[0].name.endswith("-VersionTwoResource-v1.json") + assert backups[0].read_text(encoding="utf-8") == old_content + cached_data = [ json.loads(path.read_text(encoding="utf-8")) for path in fmu_dir.cache.list_revisions(manager.relative_path) ] - assert {"schema_version": 1, "value": "old"} in cached_data + assert {"schema_version": 1, "value": "old"} not in cached_data assert {"schema_version": 2, "value": "updated"} in cached_data @@ -107,6 +113,8 @@ def test_cached_old_schema_is_readable_and_restorable( manager = MigratableResourceManager(fmu_dir) old_content = json.dumps({"schema_version": 1, "value": "old"}, indent=2) fmu_dir.write_text_file(manager.relative_path, old_content) + old_revision = fmu_dir.cache.store_revision(manager.relative_path, old_content) + assert old_revision is not None manager.save(VersionTwoResource(value="updated")) old_revision = next( @@ -212,6 +220,34 @@ def test_migration_save_respects_lock_before_backup( assert fmu_dir.cache.list_revisions(manager.relative_path) == revisions_before +def test_save_continues_when_migration_backup_fails( + fmu_dir: ProjectFMUDirectory, +) -> None: + """A failed migration backup does not block saving current data.""" + manager = MigratableResourceManager(fmu_dir) + old_content = json.dumps({"schema_version": 1, "value": "old"}, indent=2) + fmu_dir.write_text_file(manager.relative_path, old_content) + original_write_text_file = fmu_dir.write_text_file + + def write_text_file( + relative_path: Path | str, + content: str, + encoding: str = "utf-8", + ) -> None: + if Path(relative_path).parts[:1] == ("migration-backups",): + raise OSError("backup unavailable") + original_write_text_file(relative_path, content, encoding=encoding) + + with patch.object(fmu_dir, "write_text_file", side_effect=write_text_file): + manager.save(VersionTwoResource(value="updated")) + + assert json.loads(fmu_dir.read_text_file(manager.relative_path)) == { + "schema_version": 2, + "value": "updated", + } + assert not fmu_dir.get_file_path("migration-backups").exists() + + def test_current_schema_save_does_not_add_migration_backup( fmu_dir: ProjectFMUDirectory, ) -> None: @@ -231,6 +267,7 @@ def test_current_schema_save_does_not_add_migration_backup( "schema_version": 2, "value": "updated", } + assert not fmu_dir.get_file_path("migration-backups").exists() def test_save_replaces_non_object_json_without_migration_backup( From 6e3b88ee2e3bf77b547bde0d17936af228a799b6 Mon Sep 17 00:00:00 2001 From: Muhammad Gibran Alfarizi Date: Thu, 6 Aug 2026 15:40:27 +0200 Subject: [PATCH 4/8] ENG: Preserve pre-migration cache revisions --- ARCHITECTURE.md | 18 +++--- src/fmu/settings/_fmu_dir.py | 13 ++-- src/fmu/settings/_migrations/README.md | 46 +++++++------- src/fmu/settings/_migrations/manager.py | 61 ++++++++++--------- src/fmu/settings/_resources/cache_manager.py | 4 +- .../_resources/pydantic_resource_manager.py | 2 + src/fmu/settings/models/project_config.py | 4 +- tests/conftest.py | 6 +- tests/test_fmu_dir.py | 18 +++--- .../test_migrations/test_migration_manager.py | 2 +- .../test_resource_migration.py | 6 +- tests/test_resources/test_project_config.py | 12 ++-- 12 files changed, 97 insertions(+), 95 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 3ce33ca..bc2b94c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -178,18 +178,22 @@ The migration boundary depends on the resource operation: - **Save:** The resource manager checks the write lock first. If the stored data is older, it attempts to store the exact original JSON under `.fmu/migration-backups/` before writing the current model. This backup is - best-effort: a file-system failure is logged and does not stop the save. The newly - written current data is still added to the cache. + best-effort: a file-system failure is logged and does not stop the save. The + original JSON is also stored as a cache revision. The newly written current + data is added as another cache revision. - **Cache read:** `CacheManager` uses the migration manager to return old revisions as current validated models. - **Restore:** `CacheManager` migrates the selected revision before writing it, so - the restored resource uses the current schema. Project restore operations use the - existing restore changelog entry. + the restored resource uses the current schema. If you roll back to an older + release while the pre-migration revision is still retained, restore that revision with + the older release to return the resource file to the older schema. Project restore + operations use the existing restore changelog entry. There is no backward migration. After a current-schema resource is saved, an older -`fmu-settings` release can reject it as newer than supported. Migration backups are -not loaded or restored by the library. Users must copy one back manually when they -need to use the data with an older release. +`fmu-settings` release can reject it as newer than supported. If the pre-migration +cache revision is still retained, restore it with the older release. Migration +backups are not loaded or restored by the library. Users must copy one back manually +when the cache revision is no longer available. See the [schema migration guide](src/fmu/settings/_migrations/README.md) diff --git a/src/fmu/settings/_fmu_dir.py b/src/fmu/settings/_fmu_dir.py index 185c132..cfcfd13 100644 --- a/src/fmu/settings/_fmu_dir.py +++ b/src/fmu/settings/_fmu_dir.py @@ -47,7 +47,7 @@ class FMUDirectoryBase: def __init__( self: Self, base_path: str | Path, - cache_revisions: int = CacheManager.MIN_REVISIONS, + cache_revisions: int = 10, *, lock_timeout_seconds: int = DEFAULT_LOCK_TIMEOUT, ) -> None: @@ -56,7 +56,8 @@ def __init__( Args: base_path: The directory containing the .fmu directory or one of its parent dirs - cache_revisions: Number of revisions to retain in the cache. Minimum is 5. + cache_revisions: Number of revisions to retain in the cache. Default is 10. + Minimum is 5. lock_timeout_seconds: Lock expiration time in seconds. Default 20 minutes. Raises: @@ -334,20 +335,18 @@ def __init__( self.config = ProjectConfigManager(self) super().__init__( base_path, - CacheManager.MIN_REVISIONS, + 10, lock_timeout_seconds=lock_timeout_seconds, ) self._changelog = ChangelogManager(self) self._mappings = MappingsManager(self) try: - max_revisions = self.config.get( - "cache_max_revisions", CacheManager.MIN_REVISIONS - ) + max_revisions = self.config.get("cache_max_revisions", 10) self._cache_manager.max_revisions = max_revisions except (FileNotFoundError, ValueError) as e: logger.warning( f"Failed to load 'cache_max_revisions' from project config. " - f"Using default value '{CacheManager.MIN_REVISIONS}'. Error: {e}" + f"Using default value '10'. Error: {e}" ) def update_validation_metadata( diff --git a/src/fmu/settings/_migrations/README.md b/src/fmu/settings/_migrations/README.md index 435f6bd..c04553e 100644 --- a/src/fmu/settings/_migrations/README.md +++ b/src/fmu/settings/_migrations/README.md @@ -2,7 +2,7 @@ Use this guide when a stored `.fmu` resource needs a new schema version. -The supported resources and their migration registries are: +The supported resources and their migration function registries are: - `ProjectConfig`: `project_config/` - `UserConfig`: `user_config/` @@ -10,8 +10,8 @@ The supported resources and their migration registries are: ## Decide whether to change the schema version -Change the schema version when existing stored data needs a conversion before it -can be used by the current model. +Change the schema version when existing stored data needs a migration before it can +be used by the current model. ### Changes that need a migration @@ -40,11 +40,12 @@ old field name. Other changes that need a migration include: - Removing a stored field when its value must be moved or preserved elsewhere. -- Making an optional field required and calculating its value from old data. +- Making an optional field required and calculating its value from stored data from + an older schema. - Changing a field from one type to another, such as a string to a list of strings. - Moving flat fields into a nested model. -- Changing the meaning of a value when old data must be converted to keep its - original meaning. +- Changing the meaning of a value when stored data from an older schema must be + converted to keep its original meaning. ### Changes that do not need a migration @@ -67,10 +68,6 @@ Other changes that do not normally need a migration include: - Changing documentation or field descriptions. - Adding a computed property that is not stored. -Old stored data must still produce the correct current model when no migration is -added. If the old data needs conversion or its meaning would change, add a schema -version and migration function. - ## Schema version contract Each migratable model declares one positive integer schema version. The literal and @@ -108,7 +105,7 @@ class ProjectConfig(ResettableBaseModel): schema_version: Literal[2] = 2 # ... unchanged fields ... - max_cache_revisions: int = Field(default=5, ge=5) + max_cache_revisions: int = Field(default=10, ge=5) # ... unchanged fields ... @classmethod @@ -116,7 +113,7 @@ class ProjectConfig(ResettableBaseModel): """Reset the configuration to its defaults.""" return cls( # ... unchanged defaults ... - max_cache_revisions=5, + max_cache_revisions=10, # ... unchanged defaults ... ) ``` @@ -165,7 +162,7 @@ PROJECT_CONFIG_MIGRATIONS: dict[int, MigrationFunction] = { } ``` -The registry key is the source version. Key `1` registers the migration from +The registry key is the source schema version. Key `1` registers the migration from version 1 to version 2. Keep every migration when later versions are added: @@ -184,7 +181,7 @@ When you add a migration: - Keep `test_migration_manager.py` and the generic resource tests unchanged unless the framework behavior changes. - In `test_resource_migration.py`, update the affected resource tests that assert - its version, migration registry, or previous version data. + its version, migration function registry, or previous version data. - Add resource-specific tests under `tests/test_migrations/`. For example, a `ProjectConfig` migration can use `test_project_config_migration.py`. @@ -207,7 +204,7 @@ uv run mypy src tests Migration is automatic during normal use: -1. The resource manager reads old data. +1. The resource manager reads stored data from an older schema. 2. The migration manager converts it in memory. 3. The resource manager returns the current validated model. 4. The stored file remains unchanged until a save occurs. @@ -217,20 +214,21 @@ On the first save: 1. The write lock is checked. 2. If the stored data needs migration, the resource manager tries to save a copy of the original JSON under `.fmu/migration-backups/`. This backup is separate from - the normal cache, is not removed automatically, and a failure to write it does not - stop the save. + the cache, is not removed automatically, and a failure to write it does not + stop the save. The original JSON is also added to a cache revision before + the migrated data is written. 3. The current model is written with the new schema version. -4. When automatic caching is enabled, the newly written data is added to a cache - revision. +4. The newly written data is also added to a cache revision. Loading a resource does not create a changelog entry. A later user update or restore uses the existing changelog behavior. -When an old cache revision is restored, it is migrated before it is written. The -resource file therefore uses the current schema after the restore. Migration backups -are only saved for manual recovery, and the library does not read or restore them. To -roll back, first replace the current resource file with the appropriate backup, then -run the older release. +When an old cache revision is restored by the current release, it is migrated before +it is written. The resource file therefore uses the current schema after the restore. +If you roll back to an older release while the pre-migration revision is retained, +restore that revision with the older release to return the resource file to the older +schema. Migration backups are not read or restored by the library, copy one back +manually when the cache revision is no longer available. Migrations are forward-only. After current-schema data is saved, an older `fmu-settings` release can reject it as newer than its supported schema. diff --git a/src/fmu/settings/_migrations/manager.py b/src/fmu/settings/_migrations/manager.py index bcb2681..d83b30c 100644 --- a/src/fmu/settings/_migrations/manager.py +++ b/src/fmu/settings/_migrations/manager.py @@ -1,4 +1,4 @@ -"""Forward-only migration support for versioned resource data.""" +"""Forward-only migration support for versioned data.""" from __future__ import annotations @@ -20,7 +20,7 @@ class MigrationError(ValueError): class MigrationManager(Generic[MigratableResource]): - """Migrate versioned resource data to its current schema.""" + """Migrate stored data to its current schema.""" def __init__( self, @@ -31,7 +31,7 @@ def __init__( Args: model_class: Current Pydantic model for the resource. - migrations: Migration functions keyed by their source version. + migrations: Migration functions keyed by their source schema version. Raises: TypeError: If the model does not declare one positive integer @@ -42,7 +42,7 @@ def __init__( self.current_version = self._get_current_version() def migrate_resource(self, data: Any) -> MigratableResource: - """Migrate decoded JSON data with predefined migration function and validate it. + """Migrate decoded data with registered migration functions and validate it. Each migration function must increment the schema version by exactly one. @@ -50,22 +50,22 @@ def migrate_resource(self, data: Any) -> MigratableResource: data: Decoded JSON data to migrate and validate. Returns: - The resource data as a validated current model. + The migrated data as a validated current model. Raises: MigrationError: If the data is not a JSON object, a schema version is - invalid or newer than supported, a required migration is missing or - fails, a migration function does not increment the schema version by - one, or the final data does not match the current model. + invalid or newer than supported, a required migration function is + missing or fails, a migration function does not increment the schema + version by one, or the final data does not match the current model. """ if not isinstance(data, dict): raise MigrationError( f"{self.model_class.__name__} resource must be a JSON object" ) - source_version = self._get_source_version(data) - migration_steps = self._get_migration_steps(source_version) + source_schema_version = self._get_source_schema_version(data) + migration_steps = self._get_migration_steps(source_schema_version) migrated_data = copy.deepcopy(data) if migration_steps else data.copy() - migrated_data.setdefault("schema_version", source_version) + migrated_data.setdefault("schema_version", source_schema_version) for version, migration_function in migration_steps: try: migrated_data = migration_function(migrated_data) @@ -95,43 +95,44 @@ def migrate_resource(self, data: Any) -> MigratableResource: return validated_model def requires_migration(self, data: dict[str, Any]) -> bool: - """Return whether the data requires a migration before a write. + """Return whether stored data needs migration before a write. - This method does not run migrations. It confirms that all required forward - migration steps exist, then checks whether the source schema is older than - the current schema. + This check verifies that all required forward migration functions exist and + that the stored schema version is older than the current schema version. Args: - data: Existing resource data that a write would replace. + data: Existing stored data that a write would replace. Returns: - Whether the resource data requires migration. + Whether the stored data requires migration. Raises: MigrationError: If the schema version is invalid or newer than supported, - or a required migration step is missing. + or a required migration function is missing. """ - source_version = self._get_source_version(data) - self._get_migration_steps(source_version) - return source_version < self.current_version + source_schema_version = self._get_source_schema_version(data) + self._get_migration_steps(source_schema_version) + return source_schema_version < self.current_version def _get_migration_steps( - self, source_version: int + self, source_schema_version: int ) -> list[tuple[int, MigrationFunction]]: - """Return and validate all migration steps needed by a source version.""" - if source_version > self.current_version: + """Return and validate migration steps needed by a source schema version.""" + if source_schema_version > self.current_version: raise MigrationError( - f"{self.model_class.__name__} schema version {source_version} is newer " - f"than supported version {self.current_version}; downgrade migration " + f"{self.model_class.__name__} schema version {source_schema_version} " + "is newer than supported version " + f"{self.current_version}; downgrade migration " "is not supported" ) steps: list[tuple[int, MigrationFunction]] = [] - for version in range(source_version, self.current_version): + for version in range(source_schema_version, self.current_version): migration_function = self.migrations.get(version) if migration_function is None: raise MigrationError( - f"Missing {self.model_class.__name__} migration from schema " + f"Missing {self.model_class.__name__} migration function from " + "schema " f"version {version} to {version + 1}" ) steps.append((version, migration_function)) @@ -179,8 +180,8 @@ def _get_current_version(self) -> int: ) return current_version - def _get_source_version(self, data: dict[str, Any]) -> int: - """Get the source version, using the legacy version when it is absent.""" + def _get_source_schema_version(self, data: dict[str, Any]) -> int: + """Return the stored schema version, using the legacy version when absent.""" if "schema_version" not in data: return LEGACY_SCHEMA_VERSION return self._validate_version(data.get("schema_version")) diff --git a/src/fmu/settings/_resources/cache_manager.py b/src/fmu/settings/_resources/cache_manager.py index 6ce9ecb..cdfbf80 100644 --- a/src/fmu/settings/_resources/cache_manager.py +++ b/src/fmu/settings/_resources/cache_manager.py @@ -39,13 +39,13 @@ class CacheManager: def __init__( self: Self, fmu_dir: FMUDirectoryBase, - max_revisions: int = 5, + max_revisions: int = 10, ) -> None: """Initialize the cache manager. Args: fmu_dir: The FMUDirectory instance. - max_revisions: Maximum number of revisions to retain. Default is 5. + max_revisions: Maximum number of revisions to retain. Default is 10. Values below 5 are set to 5. """ self._fmu_dir = fmu_dir diff --git a/src/fmu/settings/_resources/pydantic_resource_manager.py b/src/fmu/settings/_resources/pydantic_resource_manager.py index 093e122..6c35fcd 100644 --- a/src/fmu/settings/_resources/pydantic_resource_manager.py +++ b/src/fmu/settings/_resources/pydantic_resource_manager.py @@ -216,6 +216,8 @@ def _store_migration_backup(self: Self) -> None: source_version = data.get("schema_version", 1) self._write_migration_backup(content, source_version) + if self.automatic_caching: + self.fmu_dir.cache.store_revision(self.relative_path, content) def _write_migration_backup(self: Self, content: str, source_version: int) -> None: """Write a best-effort migration backup of the original content. diff --git a/src/fmu/settings/models/project_config.py b/src/fmu/settings/models/project_config.py index 62f4332..f436dd0 100644 --- a/src/fmu/settings/models/project_config.py +++ b/src/fmu/settings/models/project_config.py @@ -114,7 +114,7 @@ class ProjectConfig(ResettableBaseModel): masterdata: Masterdata | None = None model: Model | None = None access: Access | None = None - cache_max_revisions: int = Field(default=5, ge=5) + cache_max_revisions: int = Field(default=10, ge=5) rms: RmsProject | None = None validation: ProjectValidation = Field(default_factory=ProjectValidation) @@ -134,7 +134,7 @@ def reset(cls: type[Self]) -> Self: masterdata=None, model=None, access=None, - cache_max_revisions=5, + cache_max_revisions=10, rms=None, validation=ProjectValidation(), ) diff --git a/tests/conftest.py b/tests/conftest.py index 161f29a..b99de46 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -94,7 +94,7 @@ def config_dict(unix_epoch_utc: datetime) -> dict[str, Any]: "created_by": "user", "last_modified_at": unix_epoch_utc, "last_modified_by": "user", - "cache_max_revisions": 5, + "cache_max_revisions": 10, "masterdata": None, "model": None, "access": None, @@ -324,7 +324,7 @@ def config_dict_with_masterdata( "created_by": "user", "last_modified_at": unix_epoch_utc, "last_modified_by": "user", - "cache_max_revisions": 5, + "cache_max_revisions": 10, "masterdata": masterdata_dict, "model": model_dict, } @@ -346,7 +346,7 @@ def mocked_project_config_with_all_fields( "created_by": "user", "last_modified_at": unix_epoch_utc, "last_modified_by": "user", - "cache_max_revisions": 5, + "cache_max_revisions": 10, "masterdata": masterdata_dict, "model": model_dict, "access": access_dict, diff --git a/tests/test_fmu_dir.py b/tests/test_fmu_dir.py index 85a132f..0e7798a 100644 --- a/tests/test_fmu_dir.py +++ b/tests/test_fmu_dir.py @@ -136,7 +136,7 @@ def test_init_logs_warning_and_uses_default_cache_revisions_on_config_read_error ): fmu = ProjectFMUDirectory(tmp_path) - assert fmu.cache.max_revisions == 5 # noqa: PLR2004 + assert fmu.cache.max_revisions == 10 # noqa: PLR2004 mock_warning.assert_called_once() warning_message = mock_warning.call_args.args[0] assert "project config" in warning_message @@ -190,7 +190,7 @@ def test_cache_property_returns_cached_manager(fmu_dir: ProjectFMUDirectory) -> assert cache is fmu_dir.cache assert fmu_dir._cache_manager is cache - assert cache.max_revisions == 5 # noqa: PLR2004 + assert cache.max_revisions == 10 # noqa: PLR2004 def test_set_cache_max_revisions_updates_manager( @@ -1140,7 +1140,7 @@ def test_fmu_directory_base_sync_runtime_variables( runtime variables should be synced accordingly. """ new_fmu_dir = extra_fmu_dir - new_cache_max_revisions = 10 + new_cache_max_revisions = 15 old_cache_max_revisions = fmu_dir.config.load().cache_max_revisions assert fmu_dir.cache_max_revisions != new_cache_max_revisions assert fmu_dir.cache_max_revisions == fmu_dir.config.load().cache_max_revisions @@ -1270,8 +1270,8 @@ def test_restore_from_cache_higher_cache_max_revisions_does_not_trim_to_old_limi ) -> None: """Restoring config.json with a higher limit keeps the pre-restore snapshot.""" current_config = fmu_dir.config.load() - current_max_revisions = 5 - restored_max_revisions = 10 + current_max_revisions = 10 + restored_max_revisions = 15 extra_config_revisions = 6 assert current_config.cache_max_revisions == current_max_revisions @@ -1499,7 +1499,7 @@ def test_fmu_directory_base_sync_dir_dont_sync_ignored_fields( last_modified_by = fmu_dir.config.load().last_modified_by new_fmu_dir = extra_fmu_dir - new_cache_max_revisions = 10 + new_cache_max_revisions = 15 new_fmu_dir.set_config_value("cache_max_revisions", new_cache_max_revisions) updates = fmu_dir.sync_dir(new_fmu_dir) @@ -1519,16 +1519,16 @@ def test_fmu_directory_base_sync_dir_dont_sync_ignored_fields( # First entry should be the cache_max_revision update in fmu_dir assert updates["_changelog"][0].key == "cache_max_revisions" - assert "Old value: 5 -> New value: 5" in updates["_changelog"][0].change + assert "Old value: 10 -> New value: 5" in updates["_changelog"][0].change # Second entry should be the cache_max_revision update from the changelog merge assert updates["_changelog"][1].key == "cache_max_revisions" - assert "Old value: 5 -> New value: 10" in updates["_changelog"][1].change + assert "Old value: 10 -> New value: 15" in updates["_changelog"][1].change assert updates["_changelog"][1].path == new_fmu_dir.path # Third entry should be the cache_max_revision update from the config merge assert updates["_changelog"][2].key == "cache_max_revisions" - assert "Old value: 5 -> New value: 10" in updates["_changelog"][2].change + assert "Old value: 5 -> New value: 15" in updates["_changelog"][2].change assert updates["_changelog"][2].path == fmu_dir.path # Fourth entry should be the logged merge details diff --git a/tests/test_migrations/test_migration_manager.py b/tests/test_migrations/test_migration_manager.py index 0ab2343..9070e77 100644 --- a/tests/test_migrations/test_migration_manager.py +++ b/tests/test_migrations/test_migration_manager.py @@ -117,7 +117,7 @@ def test_migration_manager_rejects_missing_step() -> None: with pytest.raises( MigrationError, - match="Missing VersionThreeModel migration from schema version 1 to 2", + match="Missing VersionThreeModel migration function from schema version 1 to 2", ): manager.migrate_resource( { diff --git a/tests/test_migrations/test_resource_migration.py b/tests/test_migrations/test_resource_migration.py index c8ca0d8..3a775dd 100644 --- a/tests/test_migrations/test_resource_migration.py +++ b/tests/test_migrations/test_resource_migration.py @@ -81,7 +81,7 @@ def test_load_migrates_in_memory_without_changing_disk( def test_save_after_migration_backs_up_old_content( fmu_dir: ProjectFMUDirectory, ) -> None: - """The first write backs up old data outside the normal cache.""" + """The first write backs up and caches the old data before writing current data.""" manager = MigratableResourceManager(fmu_dir) old_content = json.dumps({"schema_version": 1, "value": "old"}, indent=2) fmu_dir.write_text_file(manager.relative_path, old_content) @@ -102,7 +102,7 @@ def test_save_after_migration_backs_up_old_content( json.loads(path.read_text(encoding="utf-8")) for path in fmu_dir.cache.list_revisions(manager.relative_path) ] - assert {"schema_version": 1, "value": "old"} not in cached_data + assert {"schema_version": 1, "value": "old"} in cached_data assert {"schema_version": 2, "value": "updated"} in cached_data @@ -113,8 +113,6 @@ def test_cached_old_schema_is_readable_and_restorable( manager = MigratableResourceManager(fmu_dir) old_content = json.dumps({"schema_version": 1, "value": "old"}, indent=2) fmu_dir.write_text_file(manager.relative_path, old_content) - old_revision = fmu_dir.cache.store_revision(manager.relative_path, old_content) - assert old_revision is not None manager.save(VersionTwoResource(value="updated")) old_revision = next( diff --git a/tests/test_resources/test_project_config.py b/tests/test_resources/test_project_config.py index d9222d9..44cb0c4 100644 --- a/tests/test_resources/test_project_config.py +++ b/tests/test_resources/test_project_config.py @@ -440,17 +440,17 @@ def test_project_config_diff_with_other_config( config_model: ProjectConfig, ) -> None: """Tests getting the diff between the project config and another config resource.""" - current_cache_max_revisions = 5 + current_cache_max_revisions = 10 assert fmu_dir.config.load().cache_max_revisions == current_cache_max_revisions incoming_config = ProjectConfigManager(extra_fmu_dir) - new_cache_max_revisions = 10 + new_cache_max_revisions = 15 config_model.cache_max_revisions = new_cache_max_revisions incoming_config.save(config_model) diff = fmu_dir.config.get_resource_diff(incoming_config) assert len(diff) == 1 - assert diff == [("cache_max_revisions", 5, 10)] + assert diff == [("cache_max_revisions", 10, 15)] masterdata: Masterdata = Masterdata.model_validate(masterdata_dict) config_with_masterdata = copy.deepcopy(config_model) @@ -461,7 +461,7 @@ def test_project_config_diff_with_other_config( expected_length = 2 assert len(diff) == expected_length assert diff[0] == ("masterdata", None, masterdata) - assert diff[1] == ("cache_max_revisions", 5, 10) + assert diff[1] == ("cache_max_revisions", 10, 15) fmu_dir.config.save(config_with_masterdata) @@ -601,11 +601,11 @@ def test_project_config_merge_with_other_config( config_model: ProjectConfig, ) -> None: """Tests merging the project config with another config resource.""" - current_cache_max_revisions = 5 + current_cache_max_revisions = 10 assert fmu_dir.config.load().cache_max_revisions == current_cache_max_revisions incoming_config = ProjectConfigManager(extra_fmu_dir) - new_cache_max_revisions = 10 + new_cache_max_revisions = 15 config_model.cache_max_revisions = new_cache_max_revisions incoming_config.save(config_model) From b8937e598cb2ff79579df4dd4dc26a50205bcda0 Mon Sep 17 00:00:00 2001 From: Muhammad Gibran Alfarizi Date: Fri, 7 Aug 2026 12:27:12 +0200 Subject: [PATCH 5/8] MAINT: Address feedbacks --- ARCHITECTURE.md | 11 ++--- src/fmu/settings/_migrations/README.md | 36 ++++++++------- src/fmu/settings/_migrations/manager.py | 31 +++++++------ src/fmu/settings/_resources/cache_manager.py | 38 ++++++---------- .../_resources/pydantic_resource_manager.py | 36 +++++++-------- .../test_migrations/test_migration_manager.py | 17 +++++-- .../test_resource_migration.py | 44 +++---------------- 7 files changed, 91 insertions(+), 122 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index bc2b94c..ccfd069 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -165,9 +165,9 @@ model declares its current schema version, and each resource manager has a `MigrationManager` with the migration registry for that model. Migration is forward-only. Data without `schema_version` is treated as version 1. -Each registered function increments the schema version by one. The manager rejects a -newer schema, a missing migration step, an invalid version, or data that does not -validate as the current model. +Each registered migration function increments the schema version by one. The manager +rejects a newer schema, a missing migration step, an invalid version, or data that +does not validate as the current model. The migration boundary depends on the resource operation: @@ -192,8 +192,9 @@ The migration boundary depends on the resource operation: There is no backward migration. After a current-schema resource is saved, an older `fmu-settings` release can reject it as newer than supported. If the pre-migration cache revision is still retained, restore it with the older release. Migration -backups are not loaded or restored by the library. Users must copy one back manually -when the cache revision is no longer available. +backups are not loaded or restored by the library. If the cache revision is no longer +available, we should help users copy the appropriate migration backup back to the +resource file before running the older release. See the [schema migration guide](src/fmu/settings/_migrations/README.md) diff --git a/src/fmu/settings/_migrations/README.md b/src/fmu/settings/_migrations/README.md index c04553e..ea9b788 100644 --- a/src/fmu/settings/_migrations/README.md +++ b/src/fmu/settings/_migrations/README.md @@ -146,9 +146,9 @@ The returned data must: - Preserve all relevant stored values. - Set `schema_version` to the next version. -- Be valid input for the next migration or the current model. +- Be valid input for the next migration function or the current model. -### 3. Register the migration +### 3. Register the migration function Update `project_config/__init__.py`: @@ -162,10 +162,10 @@ PROJECT_CONFIG_MIGRATIONS: dict[int, MigrationFunction] = { } ``` -The registry key is the source schema version. Key `1` registers the migration from -version 1 to version 2. +The registry key is the source schema version. Key `1` registers the migration +function from version 1 to version 2. -Keep every migration when later versions are added: +Keep every migration function when later versions are added: ```python PROJECT_CONFIG_MIGRATIONS: dict[int, MigrationFunction] = { @@ -185,7 +185,7 @@ When you add a migration: - Add resource-specific tests under `tests/test_migrations/`. For example, a `ProjectConfig` migration can use `test_project_config_migration.py`. -The resource-specific tests must cover conversion, load, save, cache restore, and +The resource-specific tests must cover migration, load, save, cache restore, and invalid data. Update the complete current version fixture used by `tests/test_resources/test_migratable_models_up_to_date.py`. Keep the previous version input with the resource-specific migration tests. @@ -205,7 +205,7 @@ uv run mypy src tests Migration is automatic during normal use: 1. The resource manager reads stored data from an older schema. -2. The migration manager converts it in memory. +2. The migration manager migrates it in memory. 3. The resource manager returns the current validated model. 4. The stored file remains unchanged until a save occurs. @@ -223,21 +223,27 @@ On the first save: Loading a resource does not create a changelog entry. A later user update or restore uses the existing changelog behavior. +### Cache restore + When an old cache revision is restored by the current release, it is migrated before it is written. The resource file therefore uses the current schema after the restore. -If you roll back to an older release while the pre-migration revision is retained, -restore that revision with the older release to return the resource file to the older -schema. Migration backups are not read or restored by the library, copy one back -manually when the cache revision is no longer available. + +### Rollback to an older release Migrations are forward-only. After current-schema data is saved, an older -`fmu-settings` release can reject it as newer than its supported schema. +`fmu-settings` release can reject it as newer than its supported schema. If the +pre-migration revision is retained, restore that revision with the older release to +return the resource file to the older schema. Migration backups are not read or +restored by the library. If the cache revision is no longer available, we should help +users copy the appropriate migration backup back to the resource file before running +the older release. ## Release checklist for an `fmu-settings` schema version -Use this checklist when releasing an `fmu-settings` package that contains a new -stored schema version. First prepare and publish `fmu-settings`. Then update the -downstream applications so that users receive the new package. +Use this checklist when releasing an `fmu-settings` package with an updated +schema version in one of the migratable models. First prepare and publish +`fmu-settings`. Then update the downstream applications so that users receive +the new package. ### 1. Prepare the `fmu-settings` package diff --git a/src/fmu/settings/_migrations/manager.py b/src/fmu/settings/_migrations/manager.py index d83b30c..8532138 100644 --- a/src/fmu/settings/_migrations/manager.py +++ b/src/fmu/settings/_migrations/manager.py @@ -60,7 +60,7 @@ def migrate_resource(self, data: Any) -> MigratableResource: """ if not isinstance(data, dict): raise MigrationError( - f"{self.model_class.__name__} resource must be a JSON object" + f"Stored {self.model_class.__name__} data must be a JSON object" ) source_schema_version = self._get_source_schema_version(data) migration_steps = self._get_migration_steps(source_schema_version) @@ -71,37 +71,37 @@ def migrate_resource(self, data: Any) -> MigratableResource: migrated_data = migration_function(migrated_data) except Exception as e: raise MigrationError( - f"{self.model_class.__name__} migration from schema version " - f"{version} to {version + 1} failed" + f"Failed to migrate stored {self.model_class.__name__} data " + f"from schema version {version} to {version + 1}" ) from e result_version = self._validate_version(migrated_data.get("schema_version")) expected_version = version + 1 if result_version != expected_version: raise MigrationError( - f"{self.model_class.__name__} migration from schema version " - f"{version} must set schema_version to {expected_version}, " - f"but set it to {result_version}" + f"Migration of stored {self.model_class.__name__} data from " + f"schema version {version} must set schema_version to " + f"{expected_version}, but set it to {result_version}" ) try: validated_model = self.model_class.model_validate(migrated_data) except ValidationError as e: raise MigrationError( - f"{self.model_class.__name__} data does not validate against current " - f"schema version {self.current_version}" + f"Stored {self.model_class.__name__} data does not validate against " + f"current schema version {self.current_version}" ) from e return validated_model def requires_migration(self, data: dict[str, Any]) -> bool: - """Return whether stored data needs migration before a write. + """Return whether stored data needs migration. This check verifies that all required forward migration functions exist and that the stored schema version is older than the current schema version. Args: - data: Existing stored data that a write would replace. + data: Existing stored data to inspect. Returns: Whether the stored data requires migration. @@ -117,11 +117,11 @@ def requires_migration(self, data: dict[str, Any]) -> bool: def _get_migration_steps( self, source_schema_version: int ) -> list[tuple[int, MigrationFunction]]: - """Return and validate migration steps needed by a source schema version.""" + """Return migration functions from source to current schema version.""" if source_schema_version > self.current_version: raise MigrationError( - f"{self.model_class.__name__} schema version {source_schema_version} " - "is newer than supported version " + f"Stored {self.model_class.__name__} data has schema version " + f"{source_schema_version}, which is newer than supported version " f"{self.current_version}; downgrade migration " "is not supported" ) @@ -131,9 +131,8 @@ def _get_migration_steps( migration_function = self.migrations.get(version) if migration_function is None: raise MigrationError( - f"Missing {self.model_class.__name__} migration function from " - "schema " - f"version {version} to {version + 1}" + f"Missing {self.model_class.__name__} migration function for " + f"stored data from schema version {version} to {version + 1}" ) steps.append((version, migration_function)) return steps diff --git a/src/fmu/settings/_resources/cache_manager.py b/src/fmu/settings/_resources/cache_manager.py index cdfbf80..78cf983 100644 --- a/src/fmu/settings/_resources/cache_manager.py +++ b/src/fmu/settings/_resources/cache_manager.py @@ -172,11 +172,11 @@ def get_revision_content( content_str = self._fmu_dir.read_text_file(cache_relative) try: - return self._migrate_and_validate_content( - content_str, - model_class, - migration_manager, - ) + if migration_manager is not None and migration_manager.requires_migration( + json.loads(content_str) + ): + return migration_manager.migrate_resource(json.loads(content_str)) + return model_class.model_validate_json(content_str) except MigrationError as e: raise MigrationError( f"Cannot migrate cached content for '{resource_file_path}': {e}" @@ -227,11 +227,15 @@ def restore_revision( current_content = self._fmu_dir.read_text_file(resource_file_path) try: - self._migrate_and_validate_content( - current_content, - model_class, - migration_manager, - ) + if ( + migration_manager is not None + and migration_manager.requires_migration( + json.loads(current_content) + ) + ): + migration_manager.migrate_resource(json.loads(current_content)) + else: + model_class.model_validate_json(current_content) except (json.JSONDecodeError, MigrationError, ValidationError) as e: logger.warning( "Skipped caching current state of " @@ -246,20 +250,6 @@ def restore_revision( logger.info(f"Restored {resource_file_path} from cache revision {revision_id}") - @staticmethod - def _migrate_and_validate_content( - content: str, - model_class: type[RequestedModel], - migration_manager: MigrationManager[RequestedModel] | None, - ) -> RequestedModel: - """Migrate versioned content and validate it, or validate current content.""" - if migration_manager is None: - return model_class.model_validate_json(content) - if migration_manager.model_class is not model_class: - raise TypeError("Migration manager model must match the requested model") - - return migration_manager.migrate_resource(json.loads(content)) - def _ensure_resource_cache_dir(self: Self, resource_file_path: Path) -> Path: """Create (if needed) and return the cache directory for resource file.""" self._cache_root_path(create=True) diff --git a/src/fmu/settings/_resources/pydantic_resource_manager.py b/src/fmu/settings/_resources/pydantic_resource_manager.py index 6c35fcd..040cace 100644 --- a/src/fmu/settings/_resources/pydantic_resource_manager.py +++ b/src/fmu/settings/_resources/pydantic_resource_manager.py @@ -145,7 +145,13 @@ def load( try: content = self.fmu_dir.read_text_file(self.relative_path) data = json.loads(content) - validated_model = self._migrate_and_validate_data(data) + if ( + self.migration_manager is not None + and self.migration_manager.requires_migration(data) + ): + validated_model = self.migration_manager.migrate_resource(data) + else: + validated_model = self.model_class.model_validate(data) if store_cache: self._cache = validated_model else: @@ -184,11 +190,11 @@ def save( self._cache = model def _store_migration_backup(self: Self) -> None: - """Try to back up older data before saving the migrated version. + """Try to back up older stored data. The backup is best effort. Invalid JSON and non-object content are skipped. - File-system errors while writing the backup are logged, but do not stop the - save. + File-system errors while writing the backup are logged without interrupting + the caller. """ migration_manager = self.migration_manager if migration_manager is None: @@ -214,12 +220,14 @@ def _store_migration_backup(self: Self) -> None: if not requires_migration: return - source_version = data.get("schema_version", 1) - self._write_migration_backup(content, source_version) + source_schema_version = data.get("schema_version", 1) + self._write_migration_backup(content, source_schema_version) if self.automatic_caching: self.fmu_dir.cache.store_revision(self.relative_path, content) - def _write_migration_backup(self: Self, content: str, source_version: int) -> None: + def _write_migration_backup( + self: Self, content: str, source_schema_version: int + ) -> None: """Write a best-effort migration backup of the original content. For example, a ``config.json`` backup can be stored under @@ -234,7 +242,7 @@ def _write_migration_backup(self: Self, content: str, source_version: int) -> No / self.relative_path.stem ) backup_filename = ( - f"{timestamp}-{token}-{self.model_class.__name__}-v{source_version}" + f"{timestamp}-{token}-{self.model_class.__name__}-v{source_schema_version}" f"{self.relative_path.suffix}" ) backup_path = backup_directory / backup_filename @@ -248,18 +256,6 @@ def _write_migration_backup(self: Self, content: str, source_version: int) -> No f"Continuing without it: {e}" ) - def _migrate_and_validate_data(self: Self, data: Any) -> PydanticResource: - """Migrate decoded data with registered functions and validate the result.""" - if self.migration_manager is None: - return self.model_class.model_validate(data) - try: - return self.migration_manager.migrate_resource(data) - except MigrationError as e: - raise MigrationError( - f"Failed to migrate resource file for " - f"'{self.__class__.__name__}' at '{self.path}': {e}" - ) from e - def get_model_diff( self: Self, current_model: BaseModel, diff --git a/tests/test_migrations/test_migration_manager.py b/tests/test_migrations/test_migration_manager.py index 9070e77..cf73041 100644 --- a/tests/test_migrations/test_migration_manager.py +++ b/tests/test_migrations/test_migration_manager.py @@ -117,7 +117,10 @@ def test_migration_manager_rejects_missing_step() -> None: with pytest.raises( MigrationError, - match="Missing VersionThreeModel migration function from schema version 1 to 2", + match=( + "Missing VersionThreeModel migration function for stored data from " + "schema version 1 to 2" + ), ): manager.migrate_resource( { @@ -134,7 +137,10 @@ def test_migration_manager_rejects_newer_schema() -> None: with pytest.raises( MigrationError, - match=("VersionThreeModel schema version 4 is newer than supported version 3"), + match=( + "Stored VersionThreeModel data has schema version 4, which is newer " + "than supported version 3" + ), ): manager.migrate_resource( { @@ -206,7 +212,9 @@ def failing_migration(data: dict[str, Any]) -> dict[str, Any]: with pytest.raises( MigrationError, - match=("VersionThreeModel migration from schema version 1 to 2 failed"), + match=( + "Failed to migrate stored VersionThreeModel data from schema version 1 to 2" + ), ) as error: manager.migrate_resource( { @@ -237,7 +245,8 @@ def remove_required_value(data: dict[str, Any]) -> dict[str, Any]: with pytest.raises( MigrationError, match=( - "VersionThreeModel data does not validate against current schema version 3" + "Stored VersionThreeModel data does not validate against current schema " + "version 3" ), ): manager.migrate_resource( diff --git a/tests/test_migrations/test_resource_migration.py b/tests/test_migrations/test_resource_migration.py index 3a775dd..19b9848 100644 --- a/tests/test_migrations/test_resource_migration.py +++ b/tests/test_migrations/test_resource_migration.py @@ -156,7 +156,7 @@ def test_cache_read_rejects_newer_schema_with_resource_context( MigrationError, match=( "Cannot migrate cached content for 'migratable.json'.*" - "schema version 3 is newer than supported version 2" + "schema version 3, which is newer than supported version 2" ), ): fmu_dir.cache.get_revision_content( @@ -302,7 +302,7 @@ def test_save_rejects_newer_stored_schema_before_overwrite( match=( "Failed to check migration requirements for resource file " "'MigratableResourceManager'.*" - "schema version 3 is newer than supported version 2" + "schema version 3, which is newer than supported version 2" ), ): manager.save(VersionTwoResource(value="current")) @@ -311,7 +311,7 @@ def test_save_rejects_newer_stored_schema_before_overwrite( assert fmu_dir.cache.list_revisions(manager.relative_path) == [] -def test_load_rejects_newer_schema_with_resource_context( +def test_load_rejects_newer_schema( fmu_dir: ProjectFMUDirectory, ) -> None: """A migration error identifies the resource that could not be loaded.""" @@ -324,8 +324,8 @@ def test_load_rejects_newer_schema_with_resource_context( with pytest.raises( ValueError, match=( - "Failed to migrate resource file for 'MigratableResourceManager'.*" - "schema version 3 is newer than supported version 2" + "Stored VersionTwoResource data has schema version 3, which is newer " + "than supported version 2" ), ): manager.load() @@ -340,10 +340,7 @@ def test_load_rejects_non_object_resource( with pytest.raises( ValueError, - match=( - "Failed to migrate resource file for 'MigratableResourceManager'.*" - "VersionTwoResource resource must be a JSON object" - ), + match="Stored VersionTwoResource data must be a JSON object", ): manager.load() @@ -443,32 +440,3 @@ def test_resource_manager_rejects_mismatched_migration_model( migration_manager, ), ) - - -def test_cache_manager_rejects_mismatched_migration_model( - fmu_dir: ProjectFMUDirectory, -) -> None: - """Cache validation cannot return a model different from its annotation.""" - revision = fmu_dir.cache.store_revision( - "version-one.json", - VersionOneResource(value="current").model_dump_json(), - ) - assert revision is not None - migration_manager = MigrationManager( - VersionTwoResource, - {1: migrate_one_to_two}, - ) - - with pytest.raises( - TypeError, - match="Migration manager model must match the requested model", - ): - fmu_dir.cache.get_revision_content( - "version-one.json", - revision.name, - VersionOneResource, - migration_manager=cast( - "MigrationManager[VersionOneResource]", - migration_manager, - ), - ) From fb3d0b23c9c8a4719e415fa2a7219f7ba1b9eba3 Mon Sep 17 00:00:00 2001 From: Muhammad Gibran Alfarizi Date: Fri, 14 Aug 2026 11:45:16 +0200 Subject: [PATCH 6/8] MAINT: Clarify migration tests and documentation --- src/fmu/settings/_migrations/README.md | 15 +-- .../test_migrations/test_migration_manager.py | 73 +++++++++++++- .../test_resource_migration.py | 95 ++++++++----------- 3 files changed, 121 insertions(+), 62 deletions(-) diff --git a/src/fmu/settings/_migrations/README.md b/src/fmu/settings/_migrations/README.md index ea9b788..5649ff8 100644 --- a/src/fmu/settings/_migrations/README.md +++ b/src/fmu/settings/_migrations/README.md @@ -95,26 +95,27 @@ The following example changes the current `ProjectConfig` from schema version 1 ### 1. Update the current model Edit the existing model in `models/project_config.py`. Do not create a second -`ProjectConfig` class. The `...` lines below represent all unchanged fields in the -current model, such as `version`, `created_at`, `created_by`, `masterdata`, and -`rms`. Only the schema version and the renamed field change in this example: +`ProjectConfig` class. Some existing fields and default values are not shown in the +shortened example. The comments show where they belong. These fields include `version`, +`created_at`, `created_by`, `masterdata`, and `rms`. Only the schema version and the +renamed field change in this example: ```python class ProjectConfig(ResettableBaseModel): """The configuration file in a .fmu directory.""" schema_version: Literal[2] = 2 - # ... unchanged fields ... + # Existing fields before this field are not shown in this example. max_cache_revisions: int = Field(default=10, ge=5) - # ... unchanged fields ... + # Existing fields after this field are not shown in this example. @classmethod def reset(cls: type[Self]) -> Self: """Reset the configuration to its defaults.""" return cls( - # ... unchanged defaults ... + # Existing default values before this one are not shown. max_cache_revisions=10, - # ... unchanged defaults ... + # Existing default values after this one are not shown. ) ``` diff --git a/tests/test_migrations/test_migration_manager.py b/tests/test_migrations/test_migration_manager.py index cf73041..ad5c027 100644 --- a/tests/test_migrations/test_migration_manager.py +++ b/tests/test_migrations/test_migration_manager.py @@ -80,8 +80,11 @@ def test_migration_manager_applies_all_steps_without_mutating_input() -> None: value="old", migrations_applied=[1, 2], ) - assert data["migrations_applied"] == [] - assert data["schema_version"] == 1 + assert data == { + "schema_version": 1, + "value": "old", + "migrations_applied": [], + } def test_migration_manager_treats_missing_version_as_version_one() -> None: @@ -106,6 +109,72 @@ def test_migration_manager_treats_missing_version_as_version_one() -> None: assert "schema_version" not in data +def test_migration_manager_adds_legacy_version_to_returned_model() -> None: + """Unversioned data returns a version-one model without changing the input.""" + data = {"value": "old"} + manager = MigrationManager(VersionOneModel, {}) + + result = manager.migrate_resource(data) + + assert result == VersionOneModel(schema_version=1, value="old") + assert data == {"value": "old"} + + +@pytest.mark.parametrize( + ("data", "expected"), + [ + ({"schema_version": 3}, False), + ({"schema_version": 2}, True), + ({}, True), + ], +) +def test_migration_manager_reports_when_migration_is_required( + data: dict[str, Any], + expected: bool, +) -> None: + """The requirement check compares stored and current schema versions.""" + manager = MigrationManager( + VersionThreeModel, + { + 1: migrate_one_to_two, + 2: migrate_two_to_three, + }, + ) + + assert manager.requires_migration(data) is expected + + +def test_requires_migration_rejects_missing_step() -> None: + """The requirement check rejects an incomplete migration path.""" + manager = MigrationManager( + VersionThreeModel, + {2: migrate_two_to_three}, + ) + + with pytest.raises( + MigrationError, + match=( + "Missing VersionThreeModel migration function for stored data from " + "schema version 1 to 2" + ), + ): + manager.requires_migration({"schema_version": 1}) + + +def test_requires_migration_rejects_newer_schema() -> None: + """The requirement check rejects stored data from a newer schema.""" + manager = MigrationManager(VersionThreeModel, {}) + + with pytest.raises( + MigrationError, + match=( + "Stored VersionThreeModel data has schema version 4, which is newer " + "than supported version 3" + ), + ): + manager.requires_migration({"schema_version": 4}) + + def test_migration_manager_rejects_missing_step() -> None: """Every intermediate migration must be registered.""" manager = MigrationManager( diff --git a/tests/test_migrations/test_resource_migration.py b/tests/test_migrations/test_resource_migration.py index 19b9848..36964f1 100644 --- a/tests/test_migrations/test_resource_migration.py +++ b/tests/test_migrations/test_resource_migration.py @@ -19,7 +19,7 @@ from fmu.settings.models.project_config import ProjectConfig if TYPE_CHECKING: - from fmu.settings._fmu_dir import ProjectFMUDirectory, UserFMUDirectory + from fmu.settings._fmu_dir import ProjectFMUDirectory class VersionTwoResource(BaseModel): @@ -106,10 +106,10 @@ def test_save_after_migration_backs_up_old_content( assert {"schema_version": 2, "value": "updated"} in cached_data -def test_cached_old_schema_is_readable_and_restorable( +def test_cached_version_one_content_can_be_read_and_restored( fmu_dir: ProjectFMUDirectory, ) -> None: - """Existing cache APIs migrate an old backup before using its content.""" + """Reading or restoring the revision migrates its content to version two.""" manager = MigratableResourceManager(fmu_dir) old_content = json.dumps({"schema_version": 1, "value": "old"}, indent=2) fmu_dir.write_text_file(manager.relative_path, old_content) @@ -141,10 +141,10 @@ def test_cached_old_schema_is_readable_and_restorable( } -def test_cache_read_rejects_newer_schema_with_resource_context( +def test_cache_manager_cannot_read_content_from_a_newer_schema_version( fmu_dir: ProjectFMUDirectory, ) -> None: - """A cache migration error identifies the affected resource.""" + """A version-two manager cannot read a cached version-three revision.""" manager = MigratableResourceManager(fmu_dir) revision = fmu_dir.cache.store_revision( manager.relative_path, @@ -167,27 +167,6 @@ def test_cache_read_rejects_newer_schema_with_resource_context( ) -def test_force_load_without_store_cache_preserves_existing_cached_model( - fmu_dir: ProjectFMUDirectory, -) -> None: - """A forced migrated read can avoid replacing the in-memory cache.""" - manager = MigratableResourceManager(fmu_dir) - fmu_dir.write_text_file( - manager.relative_path, - json.dumps({"schema_version": 1, "value": "first"}), - ) - cached = manager.load() - fmu_dir.write_text_file( - manager.relative_path, - json.dumps({"schema_version": 1, "value": "second"}), - ) - - reloaded = manager.load(force=True, store_cache=False) - - assert reloaded == VersionTwoResource(value="second") - assert manager._cache == cached - - def test_migration_save_respects_lock_before_backup( fmu_dir: ProjectFMUDirectory, ) -> None: @@ -268,10 +247,17 @@ def test_current_schema_save_does_not_add_migration_backup( assert not fmu_dir.get_file_path("migration-backups").exists() -def test_save_replaces_non_object_json_without_migration_backup( +def test_save_overwrites_non_object_json_without_a_migration_backup( fmu_dir: ProjectFMUDirectory, ) -> None: - """A valid model can replace non-object JSON without preserving it.""" + """Allow a valid save when the existing resource contains a JSON list. + + For example, the existing file contains only ``[]`` instead of the expected JSON + object. The list has no schema version, so it cannot be migrated and is not + saved as a migration backup. The valid version-two model replaces the list with + ``{"schema_version": 2, "value": "current"}``, and only this replacement is + added to the normal cache. + """ manager = MigratableResourceManager(fmu_dir) fmu_dir.write_text_file(manager.relative_path, json.dumps([])) @@ -287,12 +273,13 @@ def test_save_replaces_non_object_json_without_migration_backup( "schema_version": 2, "value": "current", } + assert not fmu_dir.get_file_path("migration-backups").exists() -def test_save_rejects_newer_stored_schema_before_overwrite( +def test_save_does_not_overwrite_a_newer_stored_version( fmu_dir: ProjectFMUDirectory, ) -> None: - """A save does not overwrite stored data from a newer schema.""" + """A version-two manager rejects version-three data before writing or caching.""" manager = MigratableResourceManager(fmu_dir) future_content = json.dumps({"schema_version": 3, "value": "future"}) fmu_dir.write_text_file(manager.relative_path, future_content) @@ -345,35 +332,37 @@ def test_load_rejects_non_object_resource( manager.load() -def test_project_resource_managers_have_version_one_migration_managers( - fmu_dir: ProjectFMUDirectory, +@pytest.mark.parametrize( + ("directory_fixture", "resource_name"), + [ + ("fmu_dir", "config"), + ("fmu_dir", "mappings"), + ("user_fmu_dir", "config"), + ], +) +def test_resource_has_version_one_migration_manager( + request: pytest.FixtureRequest, + directory_fixture: str, + resource_name: str, ) -> None: - """Project resources are wired without a dummy version two.""" - managers = [ - fmu_dir.config.migration_manager, - fmu_dir.mappings.migration_manager, - ] - - assert all(manager is not None for manager in managers) - assert all(manager.current_version == 1 for manager in managers if manager) - assert all(manager.migrations == {} for manager in managers if manager) + """Check the current schema version and migrations for each resource. - -def test_user_config_has_version_one_migration_manager( - user_fmu_dir: UserFMUDirectory, -) -> None: - """User config is wired without a dummy version two.""" - manager = user_fmu_dir.config.migration_manager + Project config, user config, and mappings currently use schema version one, so + their migration registries are empty. Update this test when a resource gets a + new schema version and migration function. + """ + fmu_directory = request.getfixturevalue(directory_fixture) + manager = getattr(fmu_directory, resource_name).migration_manager assert manager is not None assert manager.current_version == 1 assert manager.migrations == {} -def test_project_config_cache_boundary_handles_unversioned_revision( +def test_unversioned_project_config_can_be_read_and_restored_from_cache( fmu_dir: ProjectFMUDirectory, ) -> None: - """Project cache APIs normalize and restore an unversioned config revision.""" + """Legacy config restores version-one data, runtime settings, and changelog.""" legacy_config = fmu_dir.config.load().model_dump(mode="json") legacy_config.pop("schema_version") legacy_config["cache_max_revisions"] = 7 @@ -397,10 +386,10 @@ def test_project_config_cache_boundary_handles_unversioned_revision( assert fmu_dir.changelog.load().root[-1].change_type == ChangeType.restore -def test_mappings_cache_boundary_handles_unversioned_revision( +def test_unversioned_mappings_can_be_read_and_restored_from_cache( fmu_dir: ProjectFMUDirectory, ) -> None: - """Project cache APIs use the mappings migration manager across the union.""" + """Legacy mappings restore as version one and add a changelog entry.""" legacy_mappings = InternalMappings().model_dump(mode="json") legacy_mappings.pop("schema_version") revision = fmu_dir.cache.store_revision( @@ -419,10 +408,10 @@ def test_mappings_cache_boundary_handles_unversioned_revision( assert fmu_dir.changelog.load().root[-1].change_type == ChangeType.restore -def test_resource_manager_rejects_mismatched_migration_model( +def test_resource_manager_rejects_migration_functions_for_another_model( fmu_dir: ProjectFMUDirectory, ) -> None: - """A resource manager cannot use migrations for a different model.""" + """The migration and resource managers must use the same Pydantic model.""" migration_manager = MigrationManager( VersionTwoResource, {1: migrate_one_to_two}, From 7e178ff10c7585cb07b2f27a217c8b8b8b50cc9f Mon Sep 17 00:00:00 2001 From: Muhammad Gibran Alfarizi Date: Fri, 14 Aug 2026 13:55:49 +0200 Subject: [PATCH 7/8] TST: Check `restore_revision` keeps old data in the undo snapshot --- .../test_resource_migration.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/test_migrations/test_resource_migration.py b/tests/test_migrations/test_resource_migration.py index 36964f1..2c4b905 100644 --- a/tests/test_migrations/test_resource_migration.py +++ b/tests/test_migrations/test_resource_migration.py @@ -141,6 +141,42 @@ def test_cached_version_one_content_can_be_read_and_restored( } +def test_restore_caches_older_current_state_without_migrating_it( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Restore keeps an undo snapshot of a version-one file exactly as stored. + + The current data is migrated only to validate it before it is cached. + """ + manager = MigratableResourceManager(fmu_dir) + target_revision = fmu_dir.cache.store_revision( + manager.relative_path, + json.dumps({"schema_version": 2, "value": "cached"}), + ) + assert target_revision is not None + + current_content = json.dumps({"schema_version": 1, "value": "current"}, indent=2) + fmu_dir.write_text_file(manager.relative_path, current_content) + + fmu_dir.cache.restore_revision( + manager.relative_path, + target_revision.name, + manager.model_class, + migration_manager=manager.migration_manager, + ) + + assert json.loads(fmu_dir.read_text_file(manager.relative_path)) == { + "schema_version": 2, + "value": "cached", + } + + cached_revisions = [ + path.read_text(encoding="utf-8") + for path in fmu_dir.cache.list_revisions(manager.relative_path) + ] + assert current_content in cached_revisions + + def test_cache_manager_cannot_read_content_from_a_newer_schema_version( fmu_dir: ProjectFMUDirectory, ) -> None: From 67b0fb9d5333f3ada4da210eb80050f3c41c1b36 Mon Sep 17 00:00:00 2001 From: Muhammad Gibran Alfarizi Date: Fri, 14 Aug 2026 14:19:43 +0200 Subject: [PATCH 8/8] MAINT: Address feedbacks --- .../_resources/pydantic_resource_manager.py | 76 +++++----- tests/test_resources/test_cache_manager.py | 141 ++++++++++++++++++ .../test_resources/test_resource_managers.py | 99 +++++++++++- 3 files changed, 276 insertions(+), 40 deletions(-) diff --git a/src/fmu/settings/_resources/pydantic_resource_manager.py b/src/fmu/settings/_resources/pydantic_resource_manager.py index 040cace..ecbe24d 100644 --- a/src/fmu/settings/_resources/pydantic_resource_manager.py +++ b/src/fmu/settings/_resources/pydantic_resource_manager.py @@ -173,13 +173,49 @@ def save( self: Self, model: PydanticResource, ) -> None: - """Save the Pydantic model to disk. + """Save the Pydantic model and preserve stored data that needs migration. + + Before overwriting older stored data, save its original content as a manual + migration backup and a normal cache revision. Args: model: Validated Pydantic model instance. + + Raises: + MigrationError: If the stored schema version cannot be checked or cannot + be migrated to the current version. + PermissionError: If another process holds the write lock. """ self.fmu_dir._lock.ensure_can_write() - self._store_migration_backup() + + migration_manager = self.migration_manager + if migration_manager is not None: + try: + content = self.fmu_dir.read_text_file(self.relative_path) + data = json.loads(content) + except (FileNotFoundError, json.JSONDecodeError): + pass + else: + if isinstance(data, dict): + try: + requires_migration = migration_manager.requires_migration(data) + except MigrationError as e: + raise MigrationError( + "Failed to check migration requirements for resource " + f"file '{self.__class__.__name__}' at '{self.path}': {e}" + ) from e + + if requires_migration: + source_schema_version = data.get("schema_version", 1) + self._write_migration_backup( + content, + source_schema_version, + ) + if self.automatic_caching: + self.fmu_dir.cache.store_revision( + self.relative_path, + content, + ) json_data = model.model_dump_json(by_alias=True, indent=2) self.fmu_dir.write_text_file(self.relative_path, json_data) @@ -189,42 +225,6 @@ def save( self._cache = model - def _store_migration_backup(self: Self) -> None: - """Try to back up older stored data. - - The backup is best effort. Invalid JSON and non-object content are skipped. - File-system errors while writing the backup are logged without interrupting - the caller. - """ - migration_manager = self.migration_manager - if migration_manager is None: - return - - try: - content = self.fmu_dir.read_text_file(self.relative_path) - data = json.loads(content) - except (FileNotFoundError, json.JSONDecodeError): - return - - if not isinstance(data, dict): - return - - try: - requires_migration = migration_manager.requires_migration(data) - except MigrationError as e: - raise MigrationError( - f"Failed to check migration requirements for resource file " - f"'{self.__class__.__name__}' at '{self.path}': {e}" - ) from e - - if not requires_migration: - return - - source_schema_version = data.get("schema_version", 1) - self._write_migration_backup(content, source_schema_version) - if self.automatic_caching: - self.fmu_dir.cache.store_revision(self.relative_path, content) - def _write_migration_backup( self: Self, content: str, source_schema_version: int ) -> None: diff --git a/tests/test_resources/test_cache_manager.py b/tests/test_resources/test_cache_manager.py index 03aa8dc..1b3f1d8 100644 --- a/tests/test_resources/test_cache_manager.py +++ b/tests/test_resources/test_cache_manager.py @@ -6,10 +6,14 @@ from contextlib import AbstractContextManager from datetime import UTC, datetime, timedelta from pathlib import Path +from typing import cast +from unittest.mock import Mock, patch import pytest +from fmu.settings import MigrationError from fmu.settings._fmu_dir import ProjectFMUDirectory +from fmu.settings._migrations import MigrationManager from fmu.settings._resources.cache_manager import ( _CACHEDIR_TAG_CONTENT, CacheManager, @@ -298,6 +302,65 @@ def test_cache_manager_get_revision_content_returns_model( assert restored.model_dump() == cached_model.model_dump() +def test_cache_manager_get_revision_content_uses_migration_manager( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Reading a revision migrates its data when migration is required.""" + manager = CacheManager(fmu_dir) + stored_data = {"legacy": "value"} + snapshot = manager.store_revision("config.json", json.dumps(stored_data)) + assert snapshot is not None + migrated_model = fmu_dir.config.load() + migration_manager = Mock(spec=MigrationManager) + migration_manager.requires_migration.return_value = True + migration_manager.migrate_resource.return_value = migrated_model + typed_migration_manager = cast( + "MigrationManager[ProjectConfig]", + migration_manager, + ) + + result = manager.get_revision_content( + "config.json", + snapshot.name, + ProjectConfig, + migration_manager=typed_migration_manager, + ) + + assert result == migrated_model + migration_manager.requires_migration.assert_called_once_with(stored_data) + migration_manager.migrate_resource.assert_called_once_with(stored_data) + + +def test_cache_manager_get_revision_content_wraps_migration_error( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Reading a revision adds resource context to migration errors.""" + manager = CacheManager(fmu_dir) + stored_data = {"legacy": "value"} + snapshot = manager.store_revision("config.json", json.dumps(stored_data)) + assert snapshot is not None + migration_manager = Mock(spec=MigrationManager) + migration_manager.requires_migration.side_effect = MigrationError("cannot check") + typed_migration_manager = cast( + "MigrationManager[ProjectConfig]", + migration_manager, + ) + + with pytest.raises( + MigrationError, + match="Cannot migrate cached content for 'config.json': cannot check", + ): + manager.get_revision_content( + "config.json", + snapshot.name, + ProjectConfig, + migration_manager=typed_migration_manager, + ) + + migration_manager.requires_migration.assert_called_once_with(stored_data) + migration_manager.migrate_resource.assert_not_called() + + def test_cache_manager_get_revision_content_raises_for_missing_revision( fmu_dir: ProjectFMUDirectory, ) -> None: @@ -357,6 +420,84 @@ def test_cache_manager_restore_revision_overwrites_and_caches_current( assert "2.3.4" in cached_versions +def test_cache_manager_restore_revision_uses_migration_manager( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Restore migrates the current state before storing its undo revision.""" + manager = CacheManager(fmu_dir) + restored_model = fmu_dir.config.load() + current_data = {"legacy": "current"} + current_content = json.dumps(current_data) + fmu_dir.write_text_file("config.json", current_content) + migration_manager = Mock(spec=MigrationManager) + migration_manager.requires_migration.return_value = True + migration_manager.migrate_resource.return_value = restored_model + typed_migration_manager = cast( + "MigrationManager[ProjectConfig]", + migration_manager, + ) + + with ( + patch.object( + manager, + "get_revision_content", + return_value=restored_model, + ) as get_revision_content, + patch.object( + manager, + "store_revision", + ) as store_revision, + ): + manager.restore_revision( + "config.json", + "revision.json", + ProjectConfig, + migration_manager=typed_migration_manager, + ) + + get_revision_content.assert_called_once_with( + Path("config.json"), + "revision.json", + ProjectConfig, + migration_manager=typed_migration_manager, + ) + migration_manager.requires_migration.assert_called_once_with(current_data) + migration_manager.migrate_resource.assert_called_once_with(current_data) + store_revision.assert_called_once_with( + Path("config.json"), + current_content, + skip_trim=False, + ) + + +def test_cache_manager_restore_revision_propagates_migration_error( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Restore returns a migration error raised while reading the revision.""" + manager = CacheManager(fmu_dir) + + with ( + patch.object( + manager, + "get_revision_content", + side_effect=MigrationError("cannot migrate"), + ) as get_revision_content, + pytest.raises(MigrationError, match="cannot migrate"), + ): + manager.restore_revision( + "config.json", + "revision.json", + ProjectConfig, + ) + + get_revision_content.assert_called_once_with( + Path("config.json"), + "revision.json", + ProjectConfig, + migration_manager=None, + ) + + def test_cache_manager_restore_revision_skips_invalid_current_state_cache( fmu_dir: ProjectFMUDirectory, ) -> None: diff --git a/tests/test_resources/test_resource_managers.py b/tests/test_resources/test_resource_managers.py index a4cc05e..5382caa 100644 --- a/tests/test_resources/test_resource_managers.py +++ b/tests/test_resources/test_resource_managers.py @@ -6,13 +6,15 @@ from contextlib import AbstractContextManager from datetime import UTC, datetime from pathlib import Path -from typing import Self -from unittest.mock import patch +from typing import Self, cast +from unittest.mock import Mock, patch import pytest from pydantic import AwareDatetime, BaseModel +from fmu.settings import MigrationError from fmu.settings._fmu_dir import ProjectFMUDirectory +from fmu.settings._migrations import MigrationManager from fmu.settings._resources.lock_manager import LockManager from fmu.settings._resources.pydantic_resource_manager import ( MutablePydanticResourceManager, @@ -112,6 +114,53 @@ def test_pydantic_resource_manager_save(fmu_dir: ProjectFMUDirectory) -> None: assert resource_model == PydanticResourceTest.model_validate(a_dict) +def test_pydantic_resource_manager_save_backs_up_data_that_requires_migration( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Save checks stored data and backs it up when migration is required.""" + manager = PydanticManagerTest(fmu_dir) + stored_data = {"foo": "old"} + stored_content = json.dumps(stored_data) + fmu_dir.write_text_file(manager.relative_path, stored_content) + migration_manager = Mock(spec=MigrationManager) + migration_manager.requires_migration.return_value = True + manager.migration_manager = cast( + "MigrationManager[PydanticResourceTest]", + migration_manager, + ) + + with patch.object(manager, "_write_migration_backup") as write_backup: + manager.save(PydanticResourceTest(foo="new")) + + migration_manager.requires_migration.assert_called_once_with(stored_data) + migration_manager.migrate_resource.assert_not_called() + write_backup.assert_called_once_with(stored_content, 1) + + +def test_pydantic_resource_manager_save_wraps_migration_check_error( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Save reports a migration check error before changing stored data.""" + manager = PydanticManagerTest(fmu_dir) + stored_content = json.dumps({"foo": "old"}) + fmu_dir.write_text_file(manager.relative_path, stored_content) + migration_manager = Mock(spec=MigrationManager) + migration_manager.requires_migration.side_effect = MigrationError("cannot check") + manager.migration_manager = cast( + "MigrationManager[PydanticResourceTest]", + migration_manager, + ) + + with pytest.raises( + MigrationError, + match="Failed to check migration requirements.*cannot check", + ): + manager.save(PydanticResourceTest(foo="new")) + + assert fmu_dir.read_text_file(manager.relative_path) == stored_content + migration_manager.migrate_resource.assert_not_called() + + def test_pydantic_resource_manager_save_raises_when_locked( fmu_dir: ProjectFMUDirectory, ) -> None: @@ -143,6 +192,52 @@ def test_pydantic_resource_manager_load(fmu_dir: ProjectFMUDirectory) -> None: assert test_manager._cache == test_resource +def test_pydantic_resource_manager_load_uses_migration_manager( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Load migrates stored data when the migration manager requires it.""" + manager = PydanticManagerTest(fmu_dir) + stored_data = {"foo": "old"} + fmu_dir.write_text_file(manager.relative_path, json.dumps(stored_data)) + migrated_model = PydanticResourceTest(foo="migrated") + migration_manager = Mock(spec=MigrationManager) + migration_manager.requires_migration.return_value = True + migration_manager.migrate_resource.return_value = migrated_model + manager.migration_manager = cast( + "MigrationManager[PydanticResourceTest]", + migration_manager, + ) + + loaded = manager.load() + + assert loaded == migrated_model + assert manager._cache == migrated_model + migration_manager.requires_migration.assert_called_once_with(stored_data) + migration_manager.migrate_resource.assert_called_once_with(stored_data) + + +def test_pydantic_resource_manager_load_propagates_migration_error( + fmu_dir: ProjectFMUDirectory, +) -> None: + """Load returns the migration error raised for stored data.""" + manager = PydanticManagerTest(fmu_dir) + stored_data = {"foo": "old"} + fmu_dir.write_text_file(manager.relative_path, json.dumps(stored_data)) + migration_manager = Mock(spec=MigrationManager) + migration_manager.requires_migration.return_value = True + migration_manager.migrate_resource.side_effect = MigrationError("cannot migrate") + manager.migration_manager = cast( + "MigrationManager[PydanticResourceTest]", + migration_manager, + ) + + with pytest.raises(MigrationError, match="cannot migrate"): + manager.load() + + migration_manager.requires_migration.assert_called_once_with(stored_data) + migration_manager.migrate_resource.assert_called_once_with(stored_data) + + def test_pydantic_resource_manager_load_permission_error( fmu_dir: ProjectFMUDirectory, no_permissions: Callable[[str | Path], AbstractContextManager[None]],