diff --git a/.github/workflows/reusable-lint-build-test.yml b/.github/workflows/reusable-lint-build-test.yml index 81853a2..a69ee97 100644 --- a/.github/workflows/reusable-lint-build-test.yml +++ b/.github/workflows/reusable-lint-build-test.yml @@ -126,6 +126,9 @@ jobs: FORCE_COLOR: 0 TERM: dumb + - name: Validate solution IDs and model + run: uv run --frozen datam8 validate --solution datam8-sample-solution/ORAYLISDatabricksSample.dm8s --json + - name: Upload Test Artifacts if: always() uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 diff --git a/docs/backend-contract.md b/docs/backend-contract.md index 63ccac5..049e225 100644 --- a/docs/backend-contract.md +++ b/docs/backend-contract.md @@ -91,6 +91,19 @@ editor. Runtime configuration changes and process failure may require a restart. ## Entity and function invariants +- Model and folder entity IDs are assigned centrally by the backend on create, + clone, source import and migration, uniformly in `1..9007199254740991`, + independently of the solution's highest ID. Implicit folders use the same + allocator. New IDs avoid IDs already used by either entity type. + Create ignores a supplied ID; the returned wrapper's `entity.id` is authoritative. + Updates may include the unchanged ID but cannot change it. Rename/move preserve it. + Loading (including lazy load/reload) and saving reject duplicate IDs within each + entity type and IDs outside that range. Duplicate errors identify the ID and + affected locators. Existing valid IDs are retained, including historical overlap + between model and folder IDs. Solution repositories must run + `datam8 validate --solution --json` as a required merge check on + their merged solution state; independent branches cannot reserve each other's IDs. + Multiple creates use the regular entity endpoint followed by `/model/save`. - `PUT /entities/{locator}` accepts an optional `sourceFile` query parameter for Attribute Types, Data Types, Data Sources, Data Source Types, Data Products, Zones, Properties and Property Values. It selects the existing collection JSON @@ -206,6 +219,11 @@ editor. Runtime configuration changes and process failure may require a restart. before the model is changed. Source-only refresh does not read attribute metadata. - Returning `None` keeps the default one-source, one-column mapping derived from `get_table_metadata()`. +- Complete refresh adopts the entity description from source metadata. Identical + descriptions from multiple sources are accepted; missing descriptions are ignored + when another source supplies one. Conflicting descriptions are rejected. If all + external sources have no description, the existing description is cleared. + Entities without external sources and source-only refresh retain their description. - Complete refresh updates source-owned attribute metadata (data type, description, business-key flag and supplied properties), while preserving local expressions, display names, history, units, refactor names and deletion markers. Existing diff --git a/docs/releases/2.1.1.md b/docs/releases/2.1.1.md new file mode 100644 index 0000000..e54406b --- /dev/null +++ b/docs/releases/2.1.1.md @@ -0,0 +1,44 @@ +# DataM8 CLI 2.1.1 + +This patch release fixes entity description refresh and centralizes model and folder +entity ID allocation in the backend. + +## Entity description refresh + +- Complete refresh updates the entity description, including clearing a removed + description. Sources-only refresh and entities without external sources retain + their existing description. Multiple sources may supply the same description; + conflicting descriptions are rejected instead of selecting one arbitrarily. + +## Backend-assigned entity IDs + +- New model and folder entities receive random numeric IDs in the JavaScript-safe + range from one backend allocator. Create, clone, source import, migration and + implicit folders avoid IDs already used by either entity type. +- Existing valid IDs remain unchanged on update, rename and move. Loading and + saving reject IDs outside the supported range and duplicates within each entity + type, with the affected locators in the error. Historical overlap between model + and folder IDs remains supported. +- Create ignores supplied IDs; clients must use the returned entity ID. Multiple + creates continue to use the existing entity endpoint followed by one model save. + +## Other changes since 2.1.0 + +- Include the dependency and schema security corrections already merged into main, + and avoid logging secret references and registry contents. +- Clarify installation from prebuilt release packages. + +## Compatibility and release + +The solution schema version remains **2.0.0**. Existing plugin hooks and source +location handling are unchanged. Use the updated backend package or submodule +revision to receive these changes. Clients creating model or folder entities must +retain the IDs returned by the backend. Existing solutions with duplicate or +out-of-range IDs must be corrected before they can be loaded or saved; valid +existing IDs are not renumbered. + +After merging the release PR into main, push the protected tag `v2.1.1` at the merged +commit. The release workflow verifies the version, runs CI and publishes the tested +wheel and source archive to GitHub Releases. It does not publish to PyPI. + +See the [backend contract](../backend-contract.md) and [release process](../ci.md). diff --git a/pyproject.toml b/pyproject.toml index 50515bd..3936454 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "datam8" -version = "2.1.0" +version = "2.1.1" description = "ORAYLIS DataM8 CLI" license = "GPL-3.0" requires-python = ">=3.12" diff --git a/src/datam8/factory.py b/src/datam8/factory.py index 74b0a9f..fe556cc 100644 --- a/src/datam8/factory.py +++ b/src/datam8/factory.py @@ -221,7 +221,7 @@ def create_undefined_folders(_model: model.Model, /): Goes through all model entities and adds folder wrappers for every parent locator of an entity that does not exist yet. - Folder IDs are simply sequential increased during runtime. + Folder IDs use the model's central ID allocator. Parameters ---------- @@ -230,15 +230,12 @@ def create_undefined_folders(_model: model.Model, /): """ logger.debug("Create undefined folders") - existing_folder_ids = [w.entity.id for w in _model.folders.values()] - next_id = (max(existing_folder_ids) if len(existing_folder_ids) != 0 else 0) + 1 - undefined_folders: model.EntityDict[f.Folder] = {} for wrapper in _model.get_entity_iterator(): loc = wrapper.locator for ploc in loc.parents: - if ploc in _model.folders or ploc.entityName is None: + if ploc in _model.folders or ploc in undefined_folders or ploc.entityName is None: continue source_file_path = pathlib.Path( @@ -254,13 +251,11 @@ def create_undefined_folders(_model: model.Model, /): source_file=source_file_path.absolute(), locator=ploc, entity=f.Folder( - id=next_id, + id=_model.get_next_entity_id(), name=ploc.entityName, ), ) - next_id += 1 - for loc in undefined_folders: if loc not in _model.folders: _model.folders[loc] = undefined_folders[loc] diff --git a/src/datam8/migration_v1.py b/src/datam8/migration_v1.py index ab62678..728bd3e 100644 --- a/src/datam8/migration_v1.py +++ b/src/datam8/migration_v1.py @@ -7,6 +7,7 @@ from typing import Any, overload from datam8 import parser_v1, utils +from datam8.utils.ids import generate_entity_id from datam8_model import attribute as a from datam8_model import base as b from datam8_model import data_product as dp @@ -780,14 +781,14 @@ def create_new_properties(tags: Tags, output_path: pathlib.Path) -> None: with open(output_path / "property_values.json", "w", encoding="utf-8") as file: file.write(new_property_values.model_dump_json(**MODEL_DUMP_OPTIONS)) - @staticmethod def create_properties_for_folders( + self, zones: Sequence[z.Zone], data_products: Sequence[dp.DataProduct], output_path: pathlib.Path, ) -> None: """Should only be called AFTER other models have been migrated. No new folders will be created.""" - next_id = 1 + used_ids = {ref.id for ref in self.model_file_references.values()} for zone in zones: zone_path = output_path / zone.name @@ -801,9 +802,14 @@ def create_properties_for_folders( product_folder = b.Folders( type="folders", - folders=[f.Folder(id=next_id, name=product.name, dataProduct=product.name)], + folders=[ + f.Folder( + id=generate_entity_id(used_ids), + name=product.name, + dataProduct=product.name, + ) + ], ) - next_id += 1 with open(product_path / ".properties.json", "w", encoding="utf-8") as _file: _file.write(product_folder.model_dump_json(**MODEL_DUMP_OPTIONS)) @@ -817,14 +823,13 @@ def create_properties_for_folders( type="folders", folders=[ f.Folder( - id=next_id, + id=generate_entity_id(used_ids), name=module.name, dataProduct=product.name, dataModule=module.name, ) ], ) - next_id += 1 with open(module_path / ".properties.json", "w", encoding="utf-8") as _file: _file.write(module_folder.model_dump_json(**MODEL_DUMP_OPTIONS)) diff --git a/src/datam8/model/entity_wrapper.py b/src/datam8/model/entity_wrapper.py index 6007a67..96e894a 100644 --- a/src/datam8/model/entity_wrapper.py +++ b/src/datam8/model/entity_wrapper.py @@ -27,6 +27,7 @@ from pydantic import BaseModel, ConfigDict, ValidationError from datam8 import errors, logging, utils +from datam8.utils.ids import validate_entity_id from datam8_model import attribute as a from datam8_model import base as b from datam8_model import data_product as dp @@ -458,6 +459,13 @@ def has_property(self, property_name: str, /) -> bool: return False def update(self, **kwargs: Any) -> None: + if isinstance(self.entity, (m.ModelEntity, f.Folder)) and "id" in kwargs: + validate_entity_id(kwargs["id"], locator=str(self.locator)) + if kwargs["id"] != self.entity.id: + raise errors.Datam8ValidationError( + message=f"Entity ID cannot be changed at {self.locator}", + details={"id": self.entity.id, "requestedId": kwargs["id"]}, + ) new_entity = self.entity.model_copy(update=kwargs, deep=True) try: diff --git a/src/datam8/model/model.py b/src/datam8/model/model.py index dae45e8..3c2ba67 100644 --- a/src/datam8/model/model.py +++ b/src/datam8/model/model.py @@ -26,6 +26,7 @@ from pydantic import ConfigDict, Field, ValidationError from datam8 import config, errors, functions, logging, opts, utils +from datam8.utils.ids import generate_entity_id, validate_entity_id from datam8_model import attribute as a from datam8_model import base as b from datam8_model import data_product as dp @@ -146,21 +147,42 @@ def __init__(self, solution: s.Solution, /, **kwargs: EntityRepository[b.BaseEnt self._model_files: dict[Path, EntityFileRef] = {} """Internal dictionary to allow easy mapping of files and their entities""" - if len(self.modelEntities) == 0: - self.__next_model_id = 1 - else: - self.__next_model_id = ( - max(wrapper.entity.id for wrapper in self.modelEntities.values()) + 1 - ) + self.validate_entity_ids() + self._used_entity_ids = { + wrapper.entity.id + for repo in (self.modelEntities, self.folders) + for wrapper in repo.values() + } def __getitem__(self, type_: str) -> EntityRepository[b.BaseEntityType]: return getattr(self, type_) - def get_next_model_id(self) -> int: - with self.lock: - next = self.__next_model_id - self.__next_model_id += 1 - return next + def get_next_entity_id(self) -> int: + return generate_entity_id(self._used_entity_ids) + + get_next_model_id = get_next_entity_id + + def validate_entity_ids(self) -> None: + for entity_type, repo in ( + (b.EntityType.MODEL_ENTITIES, self.modelEntities), + (b.EntityType.FOLDERS, self.folders), + ): + locators_by_id: dict[int, list[str]] = {} + for wrapper in repo.values(): + if wrapper.is_deleted: + continue + entity_id = wrapper.entity.id + locator = str(wrapper.locator) + validate_entity_id(entity_id, locator=locator) + locators_by_id.setdefault(entity_id, []).append(locator) + + duplicates = {key: value for key, value in locators_by_id.items() if len(value) > 1} + if duplicates: + raise errors.Datam8ValidationError( + message=f"Duplicate IDs in {entity_type.value}: " + + "; ".join(f"{key}: {', '.join(value)}" for key, value in duplicates.items()), + details={"entityType": entity_type.value, "duplicates": duplicates}, + ) def init_file_references(self) -> None: self._model_files = {} @@ -461,15 +483,17 @@ def add_entity[T: b.BaseEntityType]( match content: case dict() as c: c["name"] = _locator.entityName - if _type == b.EntityType.MODEL_ENTITIES: - c["id"] = self.get_next_model_id() + if _type in (b.EntityType.MODEL_ENTITIES, b.EntityType.FOLDERS): + c["id"] = self.get_next_entity_id() if _type == b.EntityType.FOLDERS: c["path"] = "/".join([*_locator.folders, _locator.entityName]) new_entity = class_from_type(_type).from_dict(content) - case m.ModelEntity() as me: - new_entity = me - new_entity.id = self.get_next_model_id() + case (m.ModelEntity() | f.Folder()) as identified: + new_entity = identified.model_copy(deep=True) + new_entity.id = self.get_next_entity_id() new_entity.name = _locator.entityName + if isinstance(new_entity, f.Folder): + new_entity.path = "/".join([*_locator.folders, _locator.entityName]) case _ as rest: new_entity = rest @@ -897,6 +921,7 @@ def save(self, locator: str | None = None, /) -> None: """ Saves the current state of the model or a specific entity to the corresponding json file. """ + self.validate_entity_ids() changed_wrappers = [ w for w in self.get_entities(locator or ROOT_LOCATOR) diff --git a/src/datam8/operations.py b/src/datam8/operations.py index acb51e3..2c2cd71 100644 --- a/src/datam8/operations.py +++ b/src/datam8/operations.py @@ -32,6 +32,7 @@ from datam8.model.model import class_from_type from datam8.plugins.navigation import normalize_source_item from datam8_model import base as b +from datam8_model import folder as f from datam8_model import model as m @@ -380,7 +381,7 @@ def execute(self, operation: Operation) -> dict: assert op.new_locator is not None self.model.move_entities(locator, op.new_locator, force=op.force) result = self.persist() - if created is not None and isinstance(created.entity, m.ModelEntity): + if created is not None and isinstance(created.entity, (m.ModelEntity, f.Folder)): result["item"] = {"locator": str(created.locator), "id": created.entity.id} return result diff --git a/src/datam8/parser_v1.py b/src/datam8/parser_v1.py index 82af96d..2a645d8 100644 --- a/src/datam8/parser_v1.py +++ b/src/datam8/parser_v1.py @@ -2,6 +2,7 @@ import json import pathlib +from datam8.utils.ids import generate_entity_id from datam8_model.v1 import ( CoreModelEntry, CuratedModelEntry, @@ -24,7 +25,7 @@ def parse_solution_file( index = Index.Model.from_json_file(solution_file_path.parent / "index.json") indeces: list[IndexEntry] = [] mapping_file_id: dict[str, ModelFileReference] = {} - next_id = 1 + used_ids: set[int] = set() if index.rawIndex is not None and index.rawIndex.entry is not None: indeces.extend(index.rawIndex.entry) @@ -40,9 +41,8 @@ def parse_solution_file( for idx in indeces: mapping_file_id[idx.locator.lower()] = ModelFileReference( - id=next_id, path=pathlib.Path(idx.absPath) + id=generate_entity_id(used_ids), path=pathlib.Path(idx.absPath) ) - next_id += 1 return solution, mapping_file_id diff --git a/src/datam8/source.py b/src/datam8/source.py index 7315856..80c0d9e 100644 --- a/src/datam8/source.py +++ b/src/datam8/source.py @@ -229,6 +229,7 @@ def compare_entity_with_source( refreshed_by_index: dict[int, list[m.ExternalModelSource]] = {} described_attributes: dict[str, at.Attribute] = {} described_properties: dict[str, p.PropertyReference] = {} + described_descriptions: set[str] = set() for existing, location, index in groups: if mode == "sources-only": refreshed = read_external_sources( @@ -249,6 +250,8 @@ def compare_entity_with_source( refreshed = [ item for item in described.sources if isinstance(item, m.ExternalModelSource) ] + if described.description is not None: + described_descriptions.add(described.description) for attr in described.attributes: previous = described_attributes.get(attr.name) comparable = {"ordinalNumber", "dateAdded", "dateModified"} @@ -295,6 +298,10 @@ def compare_entity_with_source( threshold_to_diff_deeper=0, ) + if len(described_descriptions) > 1: + raise utils.create_error(ValueError("Conflicting entity descriptions")) + if groups: + wrapper.entity.description = next(iter(described_descriptions), None) if described_properties: entity_properties = {prop.property: prop for prop in wrapper.entity.properties or []} entity_properties.update(described_properties) diff --git a/src/datam8/utils/ids.py b/src/datam8/utils/ids.py new file mode 100644 index 0000000..2360c4a --- /dev/null +++ b/src/datam8/utils/ids.py @@ -0,0 +1,26 @@ +"""Central allocation and validation of model and folder entity IDs.""" + +import secrets +from threading import Lock + +from datam8.errors import Datam8ValidationError + +MAX_ENTITY_ID = (1 << 53) - 1 +_lock = Lock() + + +def generate_entity_id(used_ids: set[int]) -> int: + with _lock: + while True: + entity_id = secrets.randbelow(MAX_ENTITY_ID) + 1 + if entity_id not in used_ids: + used_ids.add(entity_id) + return entity_id + + +def validate_entity_id(value: object, *, locator: str) -> None: + if type(value) is not int or not 1 <= value <= MAX_ENTITY_ID: + raise Datam8ValidationError( + message=f"Invalid entity ID {value!r} at {locator}; expected an integer in 1..{MAX_ENTITY_ID}", + details={"id": repr(value), "locator": locator}, + ) diff --git a/tests/test_013_utf8_persistence.py b/tests/test_013_utf8_persistence.py index c445073..e78c0bb 100644 --- a/tests/test_013_utf8_persistence.py +++ b/tests/test_013_utf8_persistence.py @@ -98,6 +98,8 @@ def test_import_refresh_reload_and_output_preserve_utf8(tmp_path, builtin_enviro assert response.status_code == 200, response.text response = client.post("/model/save") assert response.status_code in (200, 204), response.text + saved_id = json.loads(entity_path.read_text(encoding="utf-8"))["id"] + assert 1 <= saved_id <= (1 << 53) - 1 assert ( json.loads(entity_path.read_bytes().decode("utf-8"))["attributes"][0]["description"] == DESCRIPTION diff --git a/tests/test_015_source_mappings.py b/tests/test_015_source_mappings.py index 7aa7308..439e0ca 100644 --- a/tests/test_015_source_mappings.py +++ b/tests/test_015_source_mappings.py @@ -370,6 +370,7 @@ def _refreshed_sources() -> list[m.ExternalModelSource]: def test_source_only_refresh_preserves_entity_attributes_and_properties(monkeypatch) -> None: model, locator, original = _source_model() + model.modelEntities[locator].entity.description = "Local description" monkeypatch.setattr( source, "read_external_sources", lambda *_args, **_kwargs: _refreshed_sources() ) @@ -382,6 +383,41 @@ def test_source_only_refresh_preserves_entity_attributes_and_properties(monkeypa attribute.name for attribute in original.attributes ] assert wrapper.entity.properties[0].value == "old" + assert wrapper.entity.description == "Local description" + + +@pytest.mark.parametrize("description", ["Updated description", "", None]) +def test_complete_refresh_updates_entity_description_without_mutating_model( + monkeypatch, description +) -> None: + model, locator, original = _source_model() + original.description = "Old description" + model.modelEntities[locator].entity = original + refreshed = original.model_copy(deep=True) + refreshed.description = description + monkeypatch.setattr(source, "read_from_data_source", lambda *_args, **_kwargs: refreshed) + + wrapper, diff = source.compare_entity_with_source(locator, model=model) + + assert wrapper.entity.description == description + assert wrapper._changed and diff + assert model.modelEntities[locator].entity.description == "Old description" + model.modelEntities[locator].entity = wrapper.entity + repeated, repeated_diff = source.compare_entity_with_source(locator, model=model) + assert repeated.entity.description == description + assert not repeated._changed and not repeated_diff + + +def test_refresh_without_external_sources_preserves_entity_description() -> None: + model, locator, original = _source_model() + original.description = "Local description" + original.sources = [] + model.modelEntities[locator].entity = original + + wrapper, diff = source.compare_entity_with_source(locator, model=model) + + assert wrapper.entity.description == "Local description" + assert not diff def test_complete_refresh_updates_entity_properties_and_sources(monkeypatch) -> None: @@ -643,6 +679,53 @@ def test_complete_refresh_merges_multiple_contracts(monkeypatch) -> None: } +@pytest.mark.parametrize( + "descriptions,expected", + [ + (("Shared", "Shared"), "Shared"), + ((None, "Shared"), "Shared"), + (("Shared", None), "Shared"), + ((None, None), None), + ], +) +def test_complete_refresh_combines_entity_descriptions(monkeypatch, descriptions, expected): + model, locator = _multi_contract_model() + model.modelEntities[locator].entity.description = "Old description" + plugin = MultiContractPlugin() + get_metadata = plugin.get_table_metadata + + def metadata_with_description(location): + metadata = get_metadata(location) + metadata.source_object.description = descriptions[location == "contract_b"] + return metadata + + monkeypatch.setattr(plugin, "get_table_metadata", metadata_with_description) + monkeypatch.setattr(factory, "get_plugin_for_data_source", lambda *_args, **_kwargs: plugin) + + wrapper, _diff = source.compare_entity_with_source(locator, model=model) + + assert wrapper.entity.description == expected + + +def test_complete_refresh_rejects_conflicting_entity_descriptions(monkeypatch) -> None: + model, locator = _multi_contract_model() + original = model.modelEntities[locator].entity.model_copy(deep=True) + plugin = MultiContractPlugin() + get_metadata = plugin.get_table_metadata + + def metadata_with_description(location): + metadata = get_metadata(location) + metadata.source_object.description = location + return metadata + + monkeypatch.setattr(plugin, "get_table_metadata", metadata_with_description) + monkeypatch.setattr(factory, "get_plugin_for_data_source", lambda *_args, **_kwargs: plugin) + + with pytest.raises(ValueError, match="Conflicting entity descriptions"): + source.compare_entity_with_source(locator, model=model) + assert model.modelEntities[locator].entity == original + + def test_complete_refresh_rejects_conflicting_entity_properties(monkeypatch) -> None: model, locator = _multi_contract_model() plugin = MultiContractPlugin(conflicting=True) diff --git a/tests/test_016_entity_tree_operations.py b/tests/test_016_entity_tree_operations.py index 277f12e..649a101 100644 --- a/tests/test_016_entity_tree_operations.py +++ b/tests/test_016_entity_tree_operations.py @@ -57,7 +57,7 @@ def test_move_folder_rebases_complete_subtree_and_metadata(model: Model) -> None def test_add_model_entity_ignores_client_id_and_uses_dedicated_file(model: Model) -> None: existing = next(iter(model.modelEntities.values())) - existing_max = max(wrapper.entity.id for wrapper in model.modelEntities.values()) + existing_ids = {wrapper.entity.id for wrapper in model.modelEntities.values()} payload = existing.entity.model_copy().model_dump(mode="json") payload["id"] = 1 payload["name"] = "ClientProvidedName" @@ -68,7 +68,8 @@ def test_add_model_entity_ignores_client_id_and_uses_dedicated_file(model: Model payload, ) - assert created.entity.id == existing_max + 1 + assert 1 <= created.entity.id <= (1 << 53) - 1 + assert created.entity.id not in existing_ids assert created.entity.name == "CreatedEntity" assert created.source_file == ( model.get_base_path_for_entity_type(EntityType.MODEL_ENTITIES) diff --git a/tests/test_022_builtin_integration.py b/tests/test_022_builtin_integration.py index 95959da..3e56247 100644 --- a/tests/test_022_builtin_integration.py +++ b/tests/test_022_builtin_integration.py @@ -121,11 +121,13 @@ def test_odcs_api_import_save_reload_and_refresh(tmp_path, builtin_environment): data["schema"][0]["properties"].append( {"name": "added", "businessName": "Neu", "logicalType": "string"} ) + data["schema"][0]["description"] = "Updated customer master" write_contract(tmp_path / "contract.yaml", data) refreshed = client.get( "/sources/compare", params={"locator": "modelEntities/Stage/Customers"} ) assert refreshed.status_code == 200, refreshed.text + assert refreshed.json()["wrapper"]["entity"]["description"] == "Updated customer master" assert refreshed.json()["wrapper"]["entity"]["attributes"][-1]["displayName"] == "Neu" sources_config["dataSources"][0]["dataTypeMapping"][0]["targetType"] = "int" sources_file.write_text(json.dumps(sources_config), encoding="utf-8") @@ -142,6 +144,7 @@ def test_odcs_api_import_save_reload_and_refresh(tmp_path, builtin_environment): params={"locator": "modelEntities/Stage/Customers", "mode": "sources-only"}, ) assert sources_only.status_code == 200, sources_only.text + assert sources_only.json()["wrapper"]["entity"]["description"] == "Customer master" assert ( sources_only.json()["wrapper"]["entity"]["attributes"][0]["dataType"]["type"] == "long" ) @@ -159,6 +162,16 @@ def test_odcs_api_import_save_reload_and_refresh(tmp_path, builtin_environment): "/sources/compare", params={"locator": "modelEntities/Stage/Customers"} ) assert unchanged.status_code == 200 and not unchanged.json()["has_changes"] + saved = json.loads((tmp_path / "Model/Stage/Customers.json").read_text()) + assert saved["description"] == "Updated customer master" + del data["schema"][0]["description"] + write_contract(tmp_path / "contract.yaml", data) + cleared = client.get( + "/sources/compare", params={"locator": "modelEntities/Stage/Customers"} + ) + assert cleared.status_code == 200, cleared.text + assert cleared.json()["has_changes"] + assert cleared.json()["wrapper"]["entity"]["description"] is None def test_odcs_directory_api_navigation_and_legacy_csv_description(tmp_path, builtin_environment): diff --git a/tests/test_025_model_entity_ids.py b/tests/test_025_model_entity_ids.py new file mode 100644 index 0000000..9df5dd6 --- /dev/null +++ b/tests/test_025_model_entity_ids.py @@ -0,0 +1,282 @@ +"""ID allocation, validation and persistence using disposable solutions.""" + +import asyncio +import json +import secrets +from concurrent.futures import ThreadPoolExecutor + +import pytest +from fastapi.testclient import TestClient +from typer.testing import CliRunner + +from datam8 import config, factory, solution +from datam8.api.app import create_app +from datam8.app import app +from datam8.errors import Datam8ValidationError +from datam8.utils.ids import MAX_ENTITY_ID + + +def entity_data(name="Existing", entity_id=7): + return { + "id": entity_id, + "name": name, + "attributes": [ + { + "name": "Id", + "ordinalNumber": 1, + "attributeType": "Regular", + "dataType": {"type": "int", "nullable": False}, + "dateAdded": "2026-01-01T00:00:00Z", + } + ], + "sources": [], + "relationships": [], + "transformations": [], + } + + +@pytest.fixture +def id_workspace(tmp_path, monkeypatch): + path = tmp_path / "IDs.dm8s" + solution.init_solution(path) + (tmp_path / "base/Zones.json").write_text( + json.dumps( + {"type": "zones", "zones": [{"name": "Raw", "targetName": "raw", "displayName": "Raw"}]} + ), + encoding="utf-8", + ) + folder = tmp_path / "model/Raw" + folder.mkdir(parents=True) + (folder / ".properties.json").write_text( + json.dumps({"type": "folders", "folders": [{"id": 11, "name": "Raw", "path": "Raw"}]}), + encoding="utf-8", + ) + (folder / "Existing.json").write_text(json.dumps(entity_data()), encoding="utf-8") + monkeypatch.setattr(config, "solution_path", path, raising=False) + monkeypatch.setattr(config, "solution_folder_path", tmp_path, raising=False) + monkeypatch.setattr(config, "lazy", True) + monkeypatch.setattr(config, "mode", config.RunMode.TEST) + monkeypatch.setattr(factory, "_model", None) + return path + + +def test_allocation_retries_loaded_and_reserved_ids(id_workspace, monkeypatch): + model = factory.get_model() + draws = iter([6, 10, MAX_ENTITY_ID - 1, MAX_ENTITY_ID - 1, 0]) + monkeypatch.setattr("datam8.utils.ids.secrets.randbelow", lambda _: next(draws)) + assert model.get_next_model_id() == MAX_ENTITY_ID + assert model.get_next_model_id() == 1 + + +def test_parallel_allocation_reserves_unique_safe_ids(id_workspace): + model = factory.get_model() + with ThreadPoolExecutor(max_workers=8) as executor: + ids = list(executor.map(lambda _: model.get_next_model_id(), range(500))) + assert len(set(ids)) == 500 + assert 7 not in ids + assert all(type(value) is int and 1 <= value <= MAX_ENTITY_ID for value in ids) + + +def test_create_clone_update_move_and_reload_keep_actual_ids(id_workspace, monkeypatch): + factory.get_model() + original_randbelow = secrets.randbelow + draws = iter([MAX_ENTITY_ID - 1, 0]) + monkeypatch.setattr("datam8.utils.ids.secrets.randbelow", lambda _: next(draws)) + with TestClient(create_app()) as client: + body = entity_data("New") + body.pop("id") + created = client.put("/entities/modelEntities/Raw/New", json=body) + assert created.status_code == 200, created.text + assert created.json()["item"]["entity"]["id"] == MAX_ENTITY_ID + cloned = client.put( + "/entities/clone", + json={"locator": "modelEntities/Raw/New", "newLocator": "modelEntities/Raw/Copy"}, + ) + assert cloned.status_code == 200, cloned.text + assert cloned.json()["items"][0]["entity"]["id"] == 1 + updated = client.patch( + "/entities/modelEntities/Raw/New", + json={"id": MAX_ENTITY_ID, "description": "Changed"}, + ) + assert updated.status_code == 200, updated.text + rejected = client.patch("/entities/modelEntities/Raw/New", json={"id": 7}) + assert rejected.status_code == 400 + assert rejected.json()["code"] == "validation_error" + moved = client.post( + "/entities/move-single", + json={"from": "modelEntities/Raw/New", "to": "modelEntities/Raw/Renamed"}, + ) + assert moved.status_code == 200, moved.text + assert moved.json()["item"]["entity"]["id"] == MAX_ENTITY_ID + assert client.post("/model/save").status_code == 204 + monkeypatch.setattr("datam8.utils.ids.secrets.randbelow", original_randbelow) + assert client.post("/model/reload").status_code == 200 + model = factory.get_model() + assert model.modelEntities.get("Raw/Renamed").entity.id == MAX_ENTITY_ID + assert model.modelEntities.get("Raw/Copy").entity.id == 1 + saved = json.loads((id_workspace.parent / "model/Raw/Renamed.json").read_text()) + assert saved["id"] == MAX_ENTITY_ID + + +@pytest.mark.parametrize( + "entity_id,exit_code", [(0, 2), (-1, 2), (MAX_ENTITY_ID + 1, 2), (MAX_ENTITY_ID, 0)] +) +def test_cli_validation_checks_id_range(id_workspace, entity_id, exit_code): + file = id_workspace.parent / "model/Raw/Invalid.json" + file.write_text(json.dumps(entity_data("Invalid", entity_id)), encoding="utf-8") + result = CliRunner().invoke(app, ["validate", "-s", str(id_workspace), "--json"]) + assert result.exit_code == exit_code, result.output + assert json.loads(result.stdout)["ok"] == (exit_code == 0) + + +def test_duplicate_ids_fail_loading_and_cli_with_locators(id_workspace): + file = id_workspace.parent / "model/Raw/Duplicate.json" + file.write_text(json.dumps(entity_data("Duplicate")), encoding="utf-8") + with pytest.raises(Datam8ValidationError, match="Duplicate IDs in modelEntities") as caught: + asyncio.run(factory.load_model(id_workspace)) + assert set(caught.value.details["duplicates"][7]) == { + "modelEntities/Raw/Existing", + "modelEntities/Raw/Duplicate", + } + result = CliRunner().invoke(app, ["validate", "-s", str(id_workspace), "--json"]) + assert result.exit_code == 2, result.output + message = json.loads(result.stdout)["error"]["message"] + assert "7" in message and "Existing" in message and "Duplicate" in message + + +def test_save_rejects_duplicate_before_writing(id_workspace): + model = factory.get_model() + new = model.add_entity("modelEntities/Raw/New", entity_data()) + new.entity.id = 7 + with pytest.raises(Datam8ValidationError, match="Duplicate"): + model.save() + assert not new.source_file.exists() + + +@pytest.mark.parametrize("supplied_id", [None, 7]) +def test_folder_create_clone_update_move_and_reload(id_workspace, supplied_id): + factory.get_model() + with TestClient(create_app()) as client: + body = {"name": "Ignored"} + if supplied_id is not None: + body["id"] = supplied_id + created = client.put("/entities/folders/Raw/New", json=body) + assert created.status_code == 200, created.text + saved = created.json()["item"]["entity"] + folder_id = saved["id"] + assert type(folder_id) is int and 1 <= folder_id <= MAX_ENTITY_ID + assert folder_id not in (7, 11) + assert saved["name"] == "New" and saved["path"] == "Raw/New" + cloned = client.put( + "/entities/clone", + json={"locator": "folders/Raw/New", "newLocator": "folders/Raw/Copy"}, + ) + assert cloned.status_code == 200, cloned.text + copy = cloned.json()["items"][0]["entity"] + assert copy["id"] not in (7, 11, folder_id) + assert copy["path"] == "Raw/Copy" + updated = client.patch( + "/entities/folders/Raw/New", json={"id": folder_id, "description": "Saved"} + ) + assert updated.status_code == 200, updated.text + assert client.patch("/entities/folders/Raw/New", json={"id": 7}).status_code == 400 + moved = client.post( + "/entities/move-single", + json={"from": "folders/Raw/New", "to": "folders/Raw/Renamed"}, + ) + assert moved.status_code == 200, moved.text + assert moved.json()["item"]["entity"]["id"] == folder_id + assert client.post("/model/save").status_code == 204 + assert client.post("/model/reload").status_code == 200 + model = factory.get_model() + assert model.folders.get("Raw/Renamed").entity.id == folder_id + assert model.folders.get("Raw/Copy").entity.id == copy["id"] + persisted = json.loads((id_workspace.parent / "model/Raw/Renamed/.properties.json").read_text()) + assert persisted["folders"][0]["id"] == folder_id + assert persisted["folders"][0]["description"] == "Saved" + + +def test_implicit_folders_use_allocator_once_per_path(id_workspace, monkeypatch): + model = factory.get_model() + model.add_entity("modelEntities/Raw/Second", entity_data("Second")) + model.folders.remove("Raw") + existing_count = len(model.folders) + # All other implicit folders have already been created; only Raw is missing. + draws = iter([6, MAX_ENTITY_ID - 1]) + monkeypatch.setattr("datam8.utils.ids.secrets.randbelow", lambda _: next(draws)) + factory.create_undefined_folders(model) + assert model.folders.get("Raw").entity.id == MAX_ENTITY_ID + assert len(model.folders) == existing_count + 1 + + +@pytest.mark.parametrize("folder_id", [7, 11, MAX_ENTITY_ID + 1]) +def test_folder_load_preserves_overlap_and_rejects_invalid_ids(id_workspace, folder_id): + folder = id_workspace.parent / "model/Raw/Nested" + folder.mkdir() + (folder / ".properties.json").write_text( + json.dumps( + { + "type": "folders", + "folders": [{"id": folder_id, "name": "Nested", "path": "Raw/Nested"}], + } + ), + encoding="utf-8", + ) + if folder_id == 7: + model = factory.get_model() + assert ( + model.folders.get("Raw/Nested").entity.id + == model.modelEntities.get("Raw/Existing").entity.id + ) + else: + with pytest.raises(Datam8ValidationError): + factory.get_model() + + +def test_typed_creation_does_not_change_existing_identity(id_workspace): + model = factory.get_model() + for locator, target in ( + ("modelEntities/Raw/Existing", "modelEntities/Raw/Copy"), + ("folders/Raw", "folders/Copy"), + ): + original = model.get_entity_by_locator(locator).entity + old_id, old_name = original.id, original.name + new = model.add_entity(target, original).entity + assert new.id != old_id + assert original.id == old_id and original.name == old_name + + +@pytest.mark.parametrize("locator", ["modelEntities/Raw/Existing", "folders/Raw"]) +def test_nonfinite_patch_id_returns_validation_error(id_workspace, locator): + factory.get_model() + with TestClient(create_app(), raise_server_exceptions=False) as client: + response = client.patch( + f"/entities/{locator}", + content='{"id":1e1000}', + headers={"Content-Type": "application/json"}, + ) + assert response.status_code == 400, response.text + assert response.json()["code"] == "validation_error" + + +def test_migration_folders_avoid_model_and_folder_ids(tmp_path, monkeypatch): + from datam8 import migration_v1, parser_v1 + from datam8_model import data_product as dp + from datam8_model import zone as z + + module_path = tmp_path / "Raw/Product/Module" + module_path.mkdir(parents=True) + migration = migration_v1.MigrationV1( + {"existing": parser_v1.ModelFileReference(id=7, path=tmp_path)} + ) + draws = iter([6, MAX_ENTITY_ID - 1, MAX_ENTITY_ID - 1, 0]) + monkeypatch.setattr("datam8.utils.ids.secrets.randbelow", lambda _: next(draws)) + migration.create_properties_for_folders( + zones=[z.Zone(name="Raw", targetName="raw", displayName="Raw")], + data_products=[dp.DataProduct(name="Product", dataModules=[dp.DataModule(name="Module")])], + output_path=tmp_path, + ) + product = json.loads((module_path.parent / ".properties.json").read_text()) + module = json.loads((module_path / ".properties.json").read_text()) + assert product["folders"][0]["id"] == MAX_ENTITY_ID + assert module["folders"][0]["id"] == 1 diff --git a/uv.lock b/uv.lock index 4580c25..3c13fe1 100644 --- a/uv.lock +++ b/uv.lock @@ -553,7 +553,7 @@ wheels = [ [[package]] name = "datam8" -version = "2.1.0" +version = "2.1.1" source = { editable = "." } dependencies = [ { name = "deepdiff" },