Skip to content

Commit

Permalink
Add validation for hierarchy in RelationshipManager
Browse files Browse the repository at this point in the history
  • Loading branch information
dgarros committed Sep 22, 2024
1 parent 15e92ab commit 0e4cb63
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 0 deletions.
17 changes: 17 additions & 0 deletions backend/infrahub/core/relationship/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,8 @@ async def init(
if not isinstance(data, list):
data = [data]

await rm._validate_hierarchy()

for item in data:
if not isinstance(item, (rm.rel_class, str, dict)) and not hasattr(item, "_schema"):
raise ValidationError({rm.name: f"Invalid data provided to form a relationship {item}"})
Expand Down Expand Up @@ -966,6 +968,8 @@ async def update( # pylint: disable=too-many-branches
else:
list_data = data

await self._validate_hierarchy()

# Reset the list of relationship and save the previous one to see if we can reuse some
previous_relationships = {rel.peer_id: rel for rel in await self.get_relationships(db=db) if rel.peer_id}
self._relationships.clear()
Expand Down Expand Up @@ -1023,6 +1027,8 @@ async def add(self, data: Union[dict[str, Any], Node], db: InfrahubDatabase) ->
if not isinstance(data, (self.rel_class, dict)) and not hasattr(data, "_schema"):
raise ValidationError({self.name: f"Invalid data provided to form a relationship {data}"})

await self._validate_hierarchy()

previous_relationships = {rel.peer_id for rel in await self.get_relationships(db=db) if rel.peer_id}

item_id = getattr(data, "id", None)
Expand Down Expand Up @@ -1148,3 +1154,14 @@ async def to_graphql(
return None

return await relationships[0].to_graphql(fields=fields, db=db, related_node_ids=related_node_ids)

async def _validate_hierarchy(self) -> None:
schema = self.node.get_schema()
if schema.is_profile_schema or not schema.hierarchy: # type: ignore[union-attr]
return

if self.name == "parent" and not schema.parent: # type: ignore[union-attr]
raise ValidationError({self.name: f"Not supported to assign a value to parent for {schema.kind}"})

if self.name == "children" and not schema.children: # type: ignore[union-attr]
raise ValidationError({self.name: f"Not supported to assign some children for {schema.kind}"})
37 changes: 37 additions & 0 deletions backend/tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
from infrahub.core.node.resource_manager.ip_address_pool import CoreIPAddressPool
from infrahub.core.node.resource_manager.ip_prefix_pool import CoreIPPrefixPool
from infrahub.core.protocols_base import CoreNode
from infrahub.core.relationship import RelationshipManager
from infrahub.core.schema import (
GenericSchema,
NodeSchema,
Expand Down Expand Up @@ -2062,6 +2063,42 @@ async def hierarchical_location_schema_simple(db: InfrahubDatabase, default_bran
registry.schema.register_schema(schema=schema, branch=default_branch.name)


@pytest.fixture
async def location_generic_protocol():
class LocationGeneric(CoreNode):
name: String
status: StringOptional
things: RelationshipManager
parent: RelationshipManager
children: RelationshipManager

return LocationGeneric


@pytest.fixture
async def location_site_protocol(location_generic_protocol):
class LocationSite(location_generic_protocol):
pass

return LocationSite


@pytest.fixture
async def location_region_protocol(location_generic_protocol):
class LocationRegion(location_generic_protocol):
pass

return LocationRegion


@pytest.fixture
async def location_rack_protocol(location_generic_protocol):
class LocationRack(location_generic_protocol):
pass

return LocationRack


@pytest.fixture
async def hierarchical_location_schema(
db: InfrahubDatabase, default_branch: Branch, hierarchical_location_schema_simple, register_core_models_schema
Expand Down
24 changes: 24 additions & 0 deletions backend/tests/unit/core/hierarchy/test_hierarchy_create.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import pytest

from infrahub.core import registry
from infrahub.core.manager import NodeManager
from infrahub.core.node import Node
from infrahub.database import InfrahubDatabase
from infrahub.exceptions import ValidationError


async def test_create_node_with_invalid_hierarchy(db: InfrahubDatabase, hierarchical_location_data):
region_schema = registry.schema.get_node_schema(name="LocationRegion", duplicate=True)
rack_schema = registry.schema.get_node_schema(name="LocationRack", duplicate=True)
europe = await NodeManager.get_one(db=db, id=hierarchical_location_data["europe"].id)
city = await NodeManager.get_one(db=db, id=hierarchical_location_data["seattle"].id)

with pytest.raises(ValidationError) as exc:
region = await Node.init(db=db, schema=region_schema)
await region.new(db=db, name="region2", parent=city)
assert "Not supported to assign a value to parent" in str(exc.value)

with pytest.raises(ValidationError) as exc:
rack = await Node.init(db=db, schema=rack_schema)
await rack.new(db=db, name="rack2", parent=city, children=[europe])
assert "Not supported to assign some children" in str(exc.value)
25 changes: 25 additions & 0 deletions backend/tests/unit/core/hierarchy/test_hierarchy_update.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import pytest

from infrahub.core import registry
from infrahub.core.manager import NodeManager
from infrahub.database import InfrahubDatabase
from infrahub.exceptions import ValidationError

CHECK_HIERARCHY_QUERY = """
MATCH ({uuid: $node_uuid})-[rel:IS_RELATED]-(rel_node:Relationship {name: "parent__child"})
Expand Down Expand Up @@ -30,3 +33,25 @@ async def test_update_node_with_hierarchy(db: InfrahubDatabase, hierarchical_loc
assert result.get("rel").get("hierarchy") == site_schema.hierarchy
nodes = await NodeManager.query(db=db, schema=site_schema, filters={"parent__name__value": "europe"})
assert {node.name.value for node in nodes} == {"paris", "london", "seattle"}


async def test_update_node_invalid_hierarchy(db: InfrahubDatabase, hierarchical_location_data):
city = await NodeManager.get_one(db=db, id=hierarchical_location_data["seattle"].id, raise_on_error=True)
region = await NodeManager.get_one(db=db, id=hierarchical_location_data["europe"].id, raise_on_error=True)
rack = await NodeManager.get_one(db=db, id=hierarchical_location_data["paris-r1"].id, raise_on_error=True)

with pytest.raises(ValidationError) as exc:
await region.parent.update(db=db, data=city)
assert "Not supported to assign a value to parent" in str(exc.value)

with pytest.raises(ValidationError) as exc:
await region.parent.add(db=db, data=city)
assert "Not supported to assign a value to parent" in str(exc.value)

with pytest.raises(ValidationError) as exc:
await rack.children.update(db=db, data=[region])
assert "Not supported to assign some children" in str(exc.value)

with pytest.raises(ValidationError) as exc:
await rack.children.update(db=db, data=[region])
assert "Not supported to assign some children" in str(exc.value)
1 change: 1 addition & 0 deletions changelog/4325.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hierarchical node that don't have a parent or a children defined in the schema will properly enforce that constraint

0 comments on commit 0e4cb63

Please sign in to comment.