api: persistence, fix regressions
This commit is contained in:
@@ -9,6 +9,8 @@ let
|
||||
};
|
||||
}
|
||||
{
|
||||
# Mutable data
|
||||
_file = "deferred.json";
|
||||
foo = {
|
||||
a = { };
|
||||
b = { };
|
||||
@@ -16,7 +18,9 @@ let
|
||||
}
|
||||
|
||||
# Merge the "inventory.json"
|
||||
(builtins.fromJSON (builtins.readFile ./deferred.json))
|
||||
(lib.modules.setDefaultModuleLocation "deferred.json" (
|
||||
builtins.fromJSON (builtins.readFile ./deferred.json)
|
||||
))
|
||||
];
|
||||
};
|
||||
in
|
||||
|
||||
@@ -218,6 +218,7 @@ class InventoryStore:
|
||||
current_priority,
|
||||
dict(data_eval),
|
||||
dict(data_disk),
|
||||
inventory_file_name=self.inventory_file.name,
|
||||
)
|
||||
|
||||
return WriteInfo(write_map, data_eval, data_disk)
|
||||
|
||||
@@ -11,6 +11,7 @@ from clan_lib.errors import ClanError
|
||||
from clan_lib.nix import nix_eval
|
||||
from clan_lib.persist.inventory_store import InventoryStore
|
||||
from clan_lib.persist.path_utils import delete_by_path, set_value_by_path
|
||||
from clan_lib.persist.write_rules import PersistenceAttribute
|
||||
|
||||
|
||||
class MockFlake:
|
||||
@@ -120,16 +121,18 @@ def test_simple_read_write(setup_test_files: Path) -> None:
|
||||
invalid_data = {"protected": "foo"}
|
||||
with pytest.raises(ClanError) as e:
|
||||
store.write(invalid_data, "test", commit=False) # type: ignore[arg-type]
|
||||
assert "Path 'protected' is readonly" in str(e.value)
|
||||
assert "Cannot delete path 'foo'" in str(e.value)
|
||||
|
||||
# Test the data is not touched
|
||||
assert store.read() == data
|
||||
assert store._get_persisted() == {"foo": "foo"}
|
||||
|
||||
# Remove the foo key from the persisted data
|
||||
# Technically data = { } should also work
|
||||
# Cannot remove keys at the root level (root is always a total submodule)
|
||||
data = {"protected": "protected"} # type: ignore[typeddict-unknown-key]
|
||||
store.write(data, "test", commit=False) # type: ignore[arg-type]
|
||||
with pytest.raises(ClanError) as e:
|
||||
store.write(data, "test", commit=False) # type: ignore[arg-type]
|
||||
assert "Cannot delete path 'foo'" in str(e.value)
|
||||
|
||||
|
||||
@pytest.mark.with_core
|
||||
@@ -150,6 +153,13 @@ def test_simple_deferred(setup_test_files: Path) -> None:
|
||||
_keys={"foo"}, # disable toplevel filtering
|
||||
)
|
||||
|
||||
attribute_props = store._write_map().writeables
|
||||
assert attribute_props == {
|
||||
("foo",): {PersistenceAttribute.WRITE},
|
||||
("foo", "a"): {PersistenceAttribute.WRITE, PersistenceAttribute.DELETE},
|
||||
("foo", "b"): {PersistenceAttribute.WRITE, PersistenceAttribute.DELETE},
|
||||
}
|
||||
|
||||
data = store.read()
|
||||
assert data == {"foo": {"a": {}, "b": {}}}
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ from clan_lib.persist.path_utils import (
|
||||
)
|
||||
from clan_lib.persist.validate import (
|
||||
validate_list_uniqueness,
|
||||
validate_no_static_deletion,
|
||||
validate_patch_conflicts,
|
||||
validate_type_compatibility,
|
||||
validate_writeability,
|
||||
@@ -140,6 +141,7 @@ Please report this issue at https://git.clan.lol/clan/clan-core/issues
|
||||
static_items = list_difference(
|
||||
old_value or [], persisted_flat.get(path, [])
|
||||
)
|
||||
validate_no_static_deletion(path, new_value, static_items)
|
||||
patch_value = list_difference(new_value, static_items)
|
||||
|
||||
patches[path] = patch_value
|
||||
|
||||
@@ -311,6 +311,37 @@ def test_update_list() -> None:
|
||||
assert patchset == {("foo",): []}
|
||||
|
||||
|
||||
def test_list_delete_static() -> None:
|
||||
prios = {
|
||||
"foo": {
|
||||
"__prio": 100, # <- writeable: "foo"
|
||||
},
|
||||
}
|
||||
|
||||
data_eval = {
|
||||
# [ "A" ] is defined in nix.
|
||||
"foo": ["A", "B"],
|
||||
}
|
||||
|
||||
data_disk = {"foo": ["B"]}
|
||||
|
||||
attribute_props = compute_attribute_persistence(prios, data_eval, data_disk)
|
||||
|
||||
assert attribute_props == {
|
||||
("foo",): {PersistenceAttribute.WRITE},
|
||||
}
|
||||
|
||||
# Try to remove "A" from the list
|
||||
update = {"foo": ["B"]}
|
||||
|
||||
with pytest.raises(ClanError) as error:
|
||||
calc_patches(
|
||||
data_disk, update, all_values=data_eval, attribute_props=attribute_props
|
||||
)
|
||||
|
||||
assert "Path 'foo' doesn't contain static items ['A']" in str(error.value)
|
||||
|
||||
|
||||
def test_update_list_duplicates() -> None:
|
||||
prios = {
|
||||
"foo": {
|
||||
|
||||
@@ -10,6 +10,16 @@ from clan_lib.persist.path_utils import (
|
||||
from clan_lib.persist.write_rules import AttributeMap, is_writeable_path
|
||||
|
||||
|
||||
def validate_no_static_deletion(
|
||||
path: PathTuple, new_list: list[Any], static_items: list[Any]
|
||||
) -> None:
|
||||
"""Validate that we're not trying to delete static items from a list."""
|
||||
missing_static = [item for item in static_items if item not in new_list]
|
||||
if missing_static:
|
||||
msg = f"Path '{path_to_string(path)}' doesn't contain static items {missing_static} - They are readonly - since they are defined via a .nix file"
|
||||
raise ClanError(msg)
|
||||
|
||||
|
||||
def validate_writeability(path: PathTuple, writeables: AttributeMap) -> None:
|
||||
"""Validate that a path is writeable."""
|
||||
if not is_writeable_path(path, writeables):
|
||||
|
||||
@@ -101,7 +101,7 @@ def is_key_writeable(
|
||||
return is_mergeable_type(value_in_all) or exists_in_persisted
|
||||
|
||||
|
||||
def get_inventory_exclusive(value: dict) -> bool | None:
|
||||
def get_inventory_exclusive(value: dict, inventory_file_name: str) -> bool | None:
|
||||
if "__this" not in value:
|
||||
return None
|
||||
|
||||
@@ -110,7 +110,8 @@ def get_inventory_exclusive(value: dict) -> bool | None:
|
||||
return None
|
||||
|
||||
return (
|
||||
len(definition_locations) == 1 and definition_locations[0] == "inventory.json"
|
||||
len(definition_locations) == 1
|
||||
and definition_locations[0] == inventory_file_name
|
||||
)
|
||||
|
||||
|
||||
@@ -129,6 +130,8 @@ def _determine_props_recursive(
|
||||
parent_redonly: bool = False,
|
||||
results: AttributeMap | None = None,
|
||||
parent_total: bool = True,
|
||||
*,
|
||||
inventory_file_name: str,
|
||||
) -> AttributeMap:
|
||||
"""Recursively determine writeability for all paths in the priority structure.
|
||||
|
||||
@@ -158,7 +161,7 @@ def _determine_props_recursive(
|
||||
# Unless there is a default that applies instead, when removed. Currently we cannot test that.
|
||||
# So we assume exclusive values can be removed. In reality we might need to check defaults too. (TODO)
|
||||
# Total parents prevent deletion of immediate children.
|
||||
is_inventory_exclusive = get_inventory_exclusive(value)
|
||||
is_inventory_exclusive = get_inventory_exclusive(value, inventory_file_name)
|
||||
if not parent_total and (
|
||||
is_inventory_exclusive or is_inventory_exclusive is None
|
||||
):
|
||||
@@ -187,7 +190,8 @@ def _determine_props_recursive(
|
||||
effective_priority,
|
||||
parent_redonly=True,
|
||||
results=results,
|
||||
parent_total=value.get("__this", {}).get("total", False),
|
||||
parent_total=get_totality(value),
|
||||
inventory_file_name=inventory_file_name,
|
||||
)
|
||||
else:
|
||||
# Determine writeability based on rules
|
||||
@@ -218,13 +222,18 @@ def _determine_props_recursive(
|
||||
parent_redonly=False,
|
||||
results=results,
|
||||
parent_total=get_totality(value),
|
||||
inventory_file_name=inventory_file_name,
|
||||
)
|
||||
|
||||
return results
|
||||
|
||||
|
||||
def compute_attribute_persistence(
|
||||
priorities: dict[str, Any], all_values: dict[str, Any], persisted: dict[str, Any]
|
||||
priorities: dict[str, Any],
|
||||
all_values: dict[str, Any],
|
||||
persisted: dict[str, Any],
|
||||
*,
|
||||
inventory_file_name: str = "inventory.json",
|
||||
) -> AttributeMap:
|
||||
"""Determine writeability for all paths based on priorities and current data.
|
||||
|
||||
@@ -236,6 +245,7 @@ def compute_attribute_persistence(
|
||||
priorities: The priority structure defining writeability rules. See: 'clanInternals.inventoryClass.introspection'
|
||||
all_values: All values in the inventory, See: 'clanInternals.inventoryClass.allValues'
|
||||
persisted: The current mutable state of the inventory, see: 'readFile inventory.json'
|
||||
inventory_file_name: The name of the inventory file, defaults to "inventory.json"
|
||||
|
||||
Returns:
|
||||
Dict with sets of writeable and non-writeable paths using tuple keys
|
||||
@@ -251,4 +261,6 @@ def compute_attribute_persistence(
|
||||
)
|
||||
raise ClanError(msg)
|
||||
|
||||
return _determine_props_recursive(priorities, all_values, persisted)
|
||||
return _determine_props_recursive(
|
||||
priorities, all_values, persisted, inventory_file_name=inventory_file_name
|
||||
)
|
||||
|
||||
@@ -252,3 +252,45 @@ def test_attributes_totality() -> None:
|
||||
("foo",): {PersistenceAttribute.WRITE},
|
||||
("foo", "a"): {PersistenceAttribute.WRITE},
|
||||
}
|
||||
|
||||
|
||||
def test_totality_simple() -> None:
|
||||
introspection = {
|
||||
"foo": { # Foo is non-total all keys are potententially deleteable
|
||||
"__this": {
|
||||
"files": ["<unknown-file>"],
|
||||
"headType": "attrsOf",
|
||||
"nullable": False,
|
||||
"prio": 100,
|
||||
"total": False,
|
||||
},
|
||||
"a": { # Can delete "a" because it is defined somewhere else
|
||||
"__this": {
|
||||
"files": ["inventory.json", "<unknown-file>"],
|
||||
"headType": "deferredModule",
|
||||
"nullable": False,
|
||||
"prio": 100,
|
||||
"total": False,
|
||||
}
|
||||
},
|
||||
"b": { # Can remove "b" because it is only defined in inventory.json
|
||||
"__this": {
|
||||
"files": ["inventory.json"],
|
||||
"headType": "deferredModule",
|
||||
"nullable": False,
|
||||
"prio": 100,
|
||||
"total": False,
|
||||
}
|
||||
},
|
||||
}
|
||||
}
|
||||
data_eval: dict = {"foo": {"a": {}, "b": {}}}
|
||||
persisted: dict = {"foo": {"a": {}, "b": {}}}
|
||||
|
||||
res = compute_attribute_persistence(introspection, data_eval, persisted)
|
||||
|
||||
assert res == {
|
||||
("foo",): {PersistenceAttribute.WRITE},
|
||||
("foo", "a"): {PersistenceAttribute.WRITE},
|
||||
("foo", "b"): {PersistenceAttribute.WRITE, PersistenceAttribute.DELETE},
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user