Skip to content

Fix GraphQL query sent with hierarchical nodes #149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions infrahub_sdk/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,14 @@ async def generate_query_data_node(

if rel_schema and rel_schema.cardinality == "one":
rel_data = RelatedNode._generate_query_data(peer_data=peer_data)
# Nodes involved in a hierarchy are required to inherit from a common ancestor node, and graphql
# tries to resolve attributes in this ancestor instead of actual node. To avoid
# invalid queries issues when attribute is missing in the common ancestor, we use a fragment
# to explicit actual node kind we are querying.
if rel_schema.kind == RelationshipKind.HIERARCHY:
data_node = rel_data["node"]
rel_data["node"] = {}
rel_data["node"][f"...on {rel_schema.peer}"] = data_node
elif rel_schema and rel_schema.cardinality == "many":
rel_data = RelationshipManager._generate_query_data(peer_data=peer_data)

Expand Down Expand Up @@ -1764,6 +1772,14 @@ def generate_query_data_node(

if rel_schema and rel_schema.cardinality == "one":
rel_data = RelatedNodeSync._generate_query_data(peer_data=peer_data)
# Nodes involved in a hierarchy are required to inherit from a common ancestor node, and graphql
# tries to resolve attributes in this ancestor instead of actual node. To avoid
# invalid queries issues when attribute is missing in the common ancestor, we use a fragment
# to explicit actual node kind we are querying.
if rel_schema.kind == RelationshipKind.HIERARCHY:
data_node = rel_data["node"]
rel_data["node"] = {}
rel_data["node"][f"...on {rel_schema.peer}"] = data_node
elif rel_schema and rel_schema.cardinality == "many":
rel_data = RelationshipManagerSync._generate_query_data(peer_data=peer_data)

Expand Down
48 changes: 48 additions & 0 deletions tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,54 @@ async def ipam_schema() -> SchemaRoot:
return SCHEMA


@pytest.fixture(scope="module")
async def hierarchical_schema() -> dict:
schema = {
"version": "1.0",
"generics": [
{
"name": "Generic",
"namespace": "Location",
"description": "Generic hierarchical location",
"label": "Location",
"hierarchical": True,
"human_friendly_id": ["name__value"],
"include_in_menu": True,
"attributes": [
{"name": "name", "kind": "Text", "unique": True, "order_weight": 900},
],
}
],
"nodes": [
{
"name": "Country",
"namespace": "Location",
"description": "A country within a continent.",
"inherit_from": ["LocationGeneric"],
"generate_profile": False,
"default_filter": "name__value",
"order_by": ["name__value"],
"display_labels": ["name__value"],
"children": "LocationSite",
"attributes": [{"name": "shortname", "kind": "Text"}],
},
{
"name": "Site",
"namespace": "Location",
"description": "A site within a country.",
"inherit_from": ["LocationGeneric"],
"default_filter": "name__value",
"order_by": ["name__value"],
"display_labels": ["name__value"],
"children": "",
"parent": "LocationCountry",
"attributes": [{"name": "shortname", "kind": "Text"}],
},
],
}
return schema


class BusRecorder(InfrahubMessageBus):
def __init__(self, component_type: Optional[ComponentType] = None):
self.messages: list[InfrahubMessage] = []
Expand Down
25 changes: 25 additions & 0 deletions tests/integration/test_infrahub_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,3 +293,28 @@ async def test_create_branch(self, client: InfrahubClient, db: InfrahubDatabase,
async def test_create_branch_async(self, client: InfrahubClient, db: InfrahubDatabase, init_db_base, base_dataset):
task_id = await client.branch.create(branch_name="new-branch-2", wait_until_completion=False)
assert isinstance(task_id, str)

# See issue #148.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked myself this once: should we just put the entire link to the issue as a comment to a test that solves it? I think it makes sense to have it/

Copy link
Contributor Author

@LucasG0 LucasG0 Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends on our preferencies, actually, I think I prefer full link too. It's a bit more verbose but it allows quicker access to corresponding issue, and it is even more relevant as we have now multiple repositories (at least sdk/server), and it should be easy to find/replace these links in case repositories names change.

async def test_hierarchical(
self, client: InfrahubClient, db: InfrahubDatabase, init_db_base, base_dataset, hierarchical_schema
):
await client.schema.load(schemas=[hierarchical_schema])

location_country = await client.create(
kind="LocationCountry", name="country_name", shortname="country_shortname"
)
await location_country.save()

location_site = await client.create(
kind="LocationSite", name="site_name", shortname="site_shortname", parent=location_country
)
await location_site.save()

nodes = await client.all(kind="LocationSite", prefetch_relationships=True, populate_store=True)
assert len(nodes) == 1
site_node = nodes[0]
assert site_node.name.value == "site_name"
assert site_node.shortname.value == "site_shortname"
country_node = site_node.parent.get()
assert country_node.name.value == "country_name"
assert country_node.shortname.value == "country_shortname"
23 changes: 23 additions & 0 deletions tests/integration/test_infrahub_client_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,3 +295,26 @@ def test_create_branch(self, client: InfrahubClientSync, db: InfrahubDatabase, i
def test_create_branch_async(self, client: InfrahubClientSync, db: InfrahubDatabase, init_db_base, base_dataset):
task_id = client.branch.create(branch_name="new-branch-2", wait_until_completion=False)
assert isinstance(task_id, str)

# See issue #148.
def test_hierarchical(
self, client: InfrahubClientSync, db: InfrahubDatabase, init_db_base, base_dataset, hierarchical_schema
):
client.schema.load(schemas=[hierarchical_schema])

location_country = client.create(kind="LocationCountry", name="country_name", shortname="country_shortname")
location_country.save()

location_site = client.create(
kind="LocationSite", name="site_name", shortname="site_shortname", parent=location_country
)
location_site.save()

nodes = client.all(kind="LocationSite", prefetch_relationships=True, populate_store=True)
assert len(nodes) == 1
site_node = nodes[0]
assert site_node.name.value == "site_name"
assert site_node.shortname.value == "site_shortname"
country_node = site_node.parent.get()
assert country_node.name.value == "country_name"
assert country_node.shortname.value == "country_shortname"