From 2ba04648874c212a507d0527de4717443e7be45b Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Fri, 29 May 2026 15:12:34 -0700 Subject: [PATCH 1/3] allow passing ordering override through many relationship in gql query --- backend/infrahub/core/manager.py | 2 + backend/infrahub/core/query/relationship.py | 44 ++++++++++++++++- backend/infrahub/graphql/loaders/peers.py | 5 ++ .../graphql/resolvers/many_relationship.py | 11 +++++ .../metadata/test_graphql_query_metadata.py | 49 +++++++++++++++++++ .../tasks.md | 4 +- 6 files changed, 112 insertions(+), 3 deletions(-) diff --git a/backend/infrahub/core/manager.py b/backend/infrahub/core/manager.py index cfcfacc6e46..8e720cd3afa 100644 --- a/backend/infrahub/core/manager.py +++ b/backend/infrahub/core/manager.py @@ -286,6 +286,7 @@ async def query_peers( branch_agnostic: bool = False, fetch_peers: bool = False, include_metadata: MetadataQueryOptions | MetadataOptions = MetadataOptions.NONE, + order: OrderModel | None = None, ) -> list[Relationship]: branch = await registry.get_branch(branch=branch, db=db) at = Timestamp(at) @@ -308,6 +309,7 @@ async def query_peers( at=at, branch_agnostic=branch_agnostic, include_metadata=relationship_metadata_options, + requested_order=order, ) await query.execute(db=db) diff --git a/backend/infrahub/core/query/relationship.py b/backend/infrahub/core/query/relationship.py index 8446dbcb388..10488150db4 100644 --- a/backend/infrahub/core/query/relationship.py +++ b/backend/infrahub/core/query/relationship.py @@ -14,6 +14,8 @@ ) from infrahub.core.constants import InfrahubKind, MetadataOptions, RelationshipDirection, RelationshipStatus from infrahub.core.constants.database import DatabaseEdgeType +from infrahub.constants.enums import OrderDirection +from infrahub.core.order import METADATA_CREATED_AT, METADATA_UPDATED_AT, OrderModel from infrahub.core.query import Query, QueryResult, QueryType from infrahub.core.query.subquery import build_subquery_filter, build_subquery_order, build_subquery_order_metadata from infrahub.core.schema.order_by import OrderByTargetKind, parse_order_by_entry @@ -689,6 +691,7 @@ def __init__( branch: Branch | None = None, at: Timestamp | str | None = None, include_metadata: MetadataOptions = MetadataOptions.NONE, + requested_order: OrderModel | None = None, **kwargs, ) -> None: if not source and not source_ids: @@ -712,6 +715,7 @@ def __init__( self.rel_type = rel_type or self.rel.rel_type self.schema = schema or self.rel.schema self.include_metadata = include_metadata + self.requested_order = requested_order if not branch and inspect.isclass(rel) and not hasattr(rel, "branch"): raise ValueError("Either an instance of Relationship or a valid branch must be provided.") @@ -843,7 +847,45 @@ def _add_updated_metadata_to_query(self, branch_filter_str: str) -> None: """ % {"branch_filter": branch_filter_str, "time_details": time_details} self.add_to_query(last_updated_query) + def _query_time_order_overrides_schema(self) -> bool: + if self.requested_order is None: + return False + if self.requested_order.disable: + return True + if self.requested_order.node_metadata is not None: + return True + return False + + def _get_requested_metadata_order_fields(self) -> list[tuple[str, OrderDirection]]: + if not (self.requested_order and self.requested_order.node_metadata): + return [] + fields: list[tuple[str, OrderDirection]] = [] + nm = self.requested_order.node_metadata + if nm.created_at: + fields.append((METADATA_CREATED_AT, nm.created_at)) + if nm.updated_at: + fields.append((METADATA_UPDATED_AT, nm.updated_at)) + return fields + async def _add_peer_order_by(self, db: InfrahubDatabase, peer_schema: MainSchemaTypes, branch_filter: str) -> None: + if self.requested_order and self.requested_order.disable: + return + + if self._query_time_order_overrides_schema(): + for order_cnt, (metadata_field, direction) in enumerate(self._get_requested_metadata_order_fields(), start=1): + subquery, subquery_params, subquery_result_name = build_subquery_order_metadata( + metadata_field=metadata_field, + branch=self.branch, + branch_filter=branch_filter, + branch_agnostic=self.branch_agnostic, + node_alias="peer", + subquery_idx=order_cnt, + ) + self.order_by.append(f"{subquery_result_name} {direction.value}") + self.params.update(subquery_params) + self.add_subquery(subquery=subquery, node_alias="peer") + return + if not (hasattr(peer_schema, "order_by") and peer_schema.order_by): return @@ -876,11 +918,11 @@ async def _add_peer_order_by(self, db: InfrahubDatabase, peer_schema: MainSchema subquery, subquery_params, subquery_result_name = await build_subquery_order( db=db, field=field, - node_alias="peer", name=order_by_field_name, order_by=order_by_next_name, branch_filter=branch_filter, branch=self.branch, + node_alias="peer", subquery_idx=order_cnt, ) self.order_by.append(f"{subquery_result_name} {parsed.direction.value}") diff --git a/backend/infrahub/graphql/loaders/peers.py b/backend/infrahub/graphql/loaders/peers.py index 51cf38d8aef..86e78cf0a91 100644 --- a/backend/infrahub/graphql/loaders/peers.py +++ b/backend/infrahub/graphql/loaders/peers.py @@ -6,6 +6,7 @@ from infrahub.core.branch.models import Branch from infrahub.core.manager import NodeManager from infrahub.core.metadata.model import MetadataQueryOptions +from infrahub.core.order import OrderModel from infrahub.core.relationship.model import Relationship from infrahub.core.schema.relationship_schema import RelationshipSchema from infrahub.core.timestamp import Timestamp @@ -24,6 +25,7 @@ class QueryPeerParams: at: Timestamp | str | None = None branch_agnostic: bool = False include_metadata: MetadataQueryOptions = field(default_factory=MetadataQueryOptions) + order: OrderModel | None = None def __hash__(self) -> int: frozen_fields: frozenset | None = None @@ -42,6 +44,8 @@ def __hash__(self) -> int: str(self.source_kind), str(self.branch_agnostic), str(hash(self.include_metadata)), + # TODO: would it be safer to add a __hash__ method to OrderModel or is order guaranteed in model_dump_json? + self.order.model_dump_json() if self.order else "", ] ) return hash(hash_str) @@ -67,6 +71,7 @@ async def batch_load_fn(self, keys: list[Any]) -> list[list[Relationship]]: branch_agnostic=self.query_params.branch_agnostic, include_metadata=self.query_params.include_metadata, fetch_peers=True, + order=self.query_params.order, ) peer_rels_by_node_id: dict[str, list[Relationship]] = {} for rel in peer_rels: diff --git a/backend/infrahub/graphql/resolvers/many_relationship.py b/backend/infrahub/graphql/resolvers/many_relationship.py index 788c3d16f91..f7478b4bdb8 100644 --- a/backend/infrahub/graphql/resolvers/many_relationship.py +++ b/backend/infrahub/graphql/resolvers/many_relationship.py @@ -9,6 +9,7 @@ ) from infrahub.core.manager import NodeManager from infrahub.core.metadata.model import MetadataQueryOptions +from infrahub.core.order import OrderModel from infrahub.core.query.node import NodeGetHierarchyQuery from infrahub.core.relationship import Relationship from infrahub.core.schema.node_schema import NodeSchema @@ -19,6 +20,7 @@ from infrahub.graphql.metadata import build_metadata_query_options, get_metadata_options_from_fields from ..loaders.peers import PeerRelationshipsDataLoader, QueryPeerParams +from ..order import deserialize_order_input from ..types import RELATIONS_PROPERTY_MAP, RELATIONS_PROPERTY_MAP_REVERSED if TYPE_CHECKING: @@ -99,6 +101,7 @@ async def resolve( include_descendants: bool = False, offset: int | None = None, limit: int | None = None, + order: dict[str, Any] | None = None, **kwargs: Any, ) -> dict[str, Any]: """Resolver for relationships of cardinality=one for Edged responses. @@ -174,6 +177,8 @@ async def resolve( # Add relationship properties metadata to relationship_level include_metadata |= MetadataQueryOptions(relationship_level=get_metadata_options_from_fields(property_fields)) + order_model = deserialize_order_input(input_data=order) + if offset or limit: relationships = await self._get_entities_simple( db=graphql_context.db, @@ -187,6 +192,7 @@ async def resolve( include_metadata=include_metadata, offset=offset, limit=limit, + order=order_model, ) else: relationships = await self._get_entities_with_data_loader( @@ -199,6 +205,7 @@ async def resolve( filters=filters, node_fields=node_fields, include_metadata=include_metadata, + order=order_model, ) if not relationships: @@ -248,6 +255,7 @@ async def _get_entities_simple( include_metadata: MetadataQueryOptions, offset: int | None = None, limit: int | None = None, + order: OrderModel | None = None, ) -> list[Relationship] | None: async with db.start_session(read_only=True) as dbs: objs = await NodeManager.query_peers( @@ -264,6 +272,7 @@ async def _get_entities_simple( branch_agnostic=rel_schema.branch is BranchSupportType.AGNOSTIC, fetch_peers=True, include_metadata=include_metadata, + order=order, ) if not objs: return None @@ -280,6 +289,7 @@ async def _get_entities_with_data_loader( filters: dict[str, Any], node_fields: dict[str, Any], include_metadata: MetadataQueryOptions, + order: OrderModel | None = None, ) -> list[Relationship] | None: if node_fields and "hfid" in node_fields: node_fields["human_friendly_id"] = None @@ -293,6 +303,7 @@ async def _get_entities_with_data_loader( at=at, branch_agnostic=rel_schema.branch is BranchSupportType.AGNOSTIC, include_metadata=include_metadata, + order=order, ) if query_params in self._data_loader_instances: loader = self._data_loader_instances[query_params] diff --git a/backend/tests/component/graphql/metadata/test_graphql_query_metadata.py b/backend/tests/component/graphql/metadata/test_graphql_query_metadata.py index c7d068de25b..9713fc02d51 100644 --- a/backend/tests/component/graphql/metadata/test_graphql_query_metadata.py +++ b/backend/tests/component/graphql/metadata/test_graphql_query_metadata.py @@ -1586,3 +1586,52 @@ async def test_graphql_relationship_peer_schema_order_by_metadata_honored( cars_edges = result.data["TestPerson"]["edges"][0]["node"]["cars"]["edges"] names = [e["node"]["name"]["value"] for e in cars_edges] assert names == ["rel-peer-car-2", "rel-peer-car-1", "rel-peer-car-0"] + + +async def test_graphql_relationship_peer_query_order_replaces_schema_order( + db: InfrahubDatabase, default_branch: Branch, car_person_schema: SchemaBranch +) -> None: + car_schema = car_person_schema.get_node(name="TestCar", duplicate=False) + person_schema = car_person_schema.get_node(name="TestPerson", duplicate=False) + car_schema.order_by = ["node_metadata__created_at__desc"] + + owner = await Node.init(db=db, schema=person_schema) + await owner.new(db=db, name="rel-peer-override-owner") + await owner.save(db=db) + + cars: list[Node] = [] + for idx in range(3): + car = await Node.init(db=db, schema=car_schema) + await car.new(db=db, name=f"rel-peer-override-car-{idx}", nbr_seats=4, is_electric=False, owner=owner) + await car.save(db=db) + cars.append(car) + + default_branch.update_schema_hash() + + query = """ + query($owner_id: ID!) { + TestPerson(ids: [$owner_id]) { + edges { + node { + cars(order: {node_metadata: {created_at: ASC}}) { + edges { node { name { value } } } + } + } + } + } + } + """ + gql_params = await prepare_graphql_params(db=db, branch=default_branch) + result = await graphql( + schema=gql_params.schema, + source=query, + context_value=gql_params.context, + root_value=None, + variable_values={"owner_id": owner.id}, + ) + + assert result.errors is None + assert result.data + cars_edges = result.data["TestPerson"]["edges"][0]["node"]["cars"]["edges"] + names = [e["node"]["name"]["value"] for e in cars_edges] + assert names == ["rel-peer-override-car-0", "rel-peer-override-car-1", "rel-peer-override-car-2"] diff --git a/dev/specs/infp-530-order-by-metadata-direction/tasks.md b/dev/specs/infp-530-order-by-metadata-direction/tasks.md index 1063c70bce1..a5ef1b86bcb 100644 --- a/dev/specs/infp-530-order-by-metadata-direction/tasks.md +++ b/dev/specs/infp-530-order-by-metadata-direction/tasks.md @@ -66,7 +66,7 @@ description: "Task list for schema-level order_by for node metadata and directio - [X] T007 [P] [US1] Add to (or create) `backend/tests/component/core/test_relationship_get_list_query.py` a test `test_RelationshipGetListQuery_order_by_metadata_with_direction` exercising the same `order_by` values on the peer schema; assert peer UUID order and tiebreaker stability. - [X] T008 [P] [US1] Add to (or create) `backend/tests/component/core/test_node_get_hierarchy_query.py` a test `test_NodeGetHierarchyQuery_order_by_metadata_with_direction` exercising the same `order_by` values on a hierarchical schema; assert child UUID order and tiebreaker stability. - [X] T009 [P] [US1] Add to `backend/tests/component/core/schema_manager/test_manager_schema.py` parametrized success cases for `validate_order_by`: each of the six grammar shapes (attribute + direction, attribute without direction, relationship-attribute + direction, relationship-attribute without direction, metadata + direction, metadata without direction) loads cleanly. -- [ ] T010 [P] [US1] Add to `backend/tests/component/graphql/metadata/test_graphql_query_metadata.py` a test that the schema-level `order_by: ["node_metadata__created_at__desc"]` is honored by the GraphQL relationship-peer field when no query-time `order` argument is provided, and is ignored entirely when an `order` argument is provided (replace-not-stack precedence). **Partially covered:** `test_graphql_relationship_peer_schema_order_by_metadata_honored` covers the schema-honored half; the replace-not-stack half is blocked by T019. +- [X] T010 [P] [US1] Add to `backend/tests/component/graphql/metadata/test_graphql_query_metadata.py` a test that the schema-level `order_by: ["node_metadata__created_at__desc"]` is honored by the GraphQL relationship-peer field when no query-time `order` argument is provided, and is ignored entirely when an `order` argument is provided (replace-not-stack precedence). - [X] T011 [P] [US1] Add to `backend/tests/component/core/test_node_inheritance.py` (create if absent, otherwise the file holding `NodeInheritanceHandler` tests) a test that a generic declaring `order_by: ["node_metadata__created_at__desc"]` is inherited by a concrete kind that has not declared its own; and a test that renaming an attribute on the generic does not corrupt a metadata `order_by` entry. ### Implementation for User Story 1 @@ -78,7 +78,7 @@ description: "Task list for schema-level order_by for node metadata and directio - [X] T016 [US1] In `backend/infrahub/core/query/relationship.py`, `RelationshipGetListQuery` around lines 953–979: replace the inline split with `parse_order_by_entry(entry, peer_schema)`. For `ATTRIBUTE` and `RELATIONSHIP_ATTRIBUTE` entries, continue using `build_subquery_order` but append the result as `f"{subquery_result_name} {parsed.direction.value}"` (today the direction is dropped). For `METADATA` entries, emit a new subquery analogous to `_add_created_metadata_subquery` / `_add_updated_metadata_subquery` but with `node_alias="peer"`; append the resulting alias plus direction. Append `"peer.uuid ASC"` as a final tiebreaker whenever any entry was emitted (currently the `peer.uuid` line at 979 is only the no-order-by fallback). - [X] T017 [US1] In `backend/infrahub/core/query/node.py`, `NodeGetHierarchyQuery` around lines 2540–2565: apply the same changes as T016 — parser, per-entry direction in the outer `ORDER BY`, metadata subquery support for the `peer` alias, and `"peer.uuid ASC"` final tiebreaker whenever any entry was emitted. Extract the shared peer-metadata-subquery helper into a module-level function if both this and T016 would otherwise duplicate it (do not pre-extract; refactor after both call sites compile). - [X] T018 [US1] If T016/T017 introduced a shared peer-metadata helper, place it in `backend/infrahub/core/query/subquery.py` next to `build_subquery_order`. Name it `build_subquery_order_metadata(node_alias, metadata_field, ...)`. Keep its signature compatible with the existing `build_subquery_order` return tuple `(subquery, params, result_alias)`. -- [ ] T018a [US1] Wire query-time `order` through the relationship-peer GraphQL path so the replace-not-stack precedence (T015 for `NodeGetListQuery`) applies at the peer level too. Today the `order` argument is accepted by the GraphQL schema (`graphql/manager.py:1007`) but silently dropped: `graphql/resolvers/many_relationship.py::resolve` swallows it in `**kwargs` without forwarding, `NodeManager.query_peers` has no `order` parameter, and `RelationshipGetPeerQuery.__init__` has no `requested_order` either. Add a `requested_order: OrderModel | None` parameter to `RelationshipGetPeerQuery`, thread it through `query_peers` and the many-relationship resolver, and update `_add_peer_order_by` to drop the peer schema's `order_by` when `requested_order` carries non-default content (mirror `NodeGetListQuery._query_time_order_overrides_schema`). Once landed, finish T010 by adding the replace-not-stack assertion to `test_graphql_relationship_peer_schema_order_by_metadata_honored`. +- [X] T018a [US1] Wire query-time `order` through the relationship-peer GraphQL path so the replace-not-stack precedence (T015 for `NodeGetListQuery`) applies at the peer level too. Today the `order` argument is accepted by the GraphQL schema (`graphql/manager.py:1007`) but silently dropped: `graphql/resolvers/many_relationship.py::resolve` swallows it in `**kwargs` without forwarding, `NodeManager.query_peers` has no `order` parameter, and `RelationshipGetPeerQuery.__init__` has no `requested_order` either. Add a `requested_order: OrderModel | None` parameter to `RelationshipGetPeerQuery`, thread it through `query_peers` and the many-relationship resolver, and update `_add_peer_order_by` to drop the peer schema's `order_by` when `requested_order` carries non-default content (mirror `NodeGetListQuery._query_time_order_overrides_schema`). Once landed, finish T010 by adding the replace-not-stack assertion to `test_graphql_relationship_peer_schema_order_by_metadata_honored`. **Checkpoint**: All US1 tests (T006–T011) pass. Quickstart steps 1–6 and step 8 succeed manually. Customer's blocking pain (newest-first DocumentationNote on parent detail) is resolved. From de0df18190b4641d18caeabbd02ca660d926d075 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Fri, 29 May 2026 17:40:11 -0700 Subject: [PATCH 2/3] tests and fix for metadata/attribute combo ordering --- backend/infrahub/core/order.py | 3 + backend/infrahub/core/query/node.py | 239 +++++++++--------- backend/infrahub/core/query/relationship.py | 18 +- .../core/test_node_get_hierarchy_query.py | 129 ++++++++++ .../core/test_node_get_list_query.py | 86 +++++++ .../core/test_relationship_get_list_query.py | 119 +++++++++ .../tasks.md | 10 +- 7 files changed, 469 insertions(+), 135 deletions(-) diff --git a/backend/infrahub/core/order.py b/backend/infrahub/core/order.py index 926544b0456..a8e8ebd9791 100644 --- a/backend/infrahub/core/order.py +++ b/backend/infrahub/core/order.py @@ -26,6 +26,9 @@ class OrderModel(BaseModel): disable: bool | None = None node_metadata: NodeMetaOrder | None = None + def __bool__(self) -> bool: + return bool(self.disable) or bool(self.node_metadata) + @model_validator(mode="after") def validate_metadata(self) -> Self: if self.node_metadata and self.node_metadata.created_at and self.node_metadata.updated_at: diff --git a/backend/infrahub/core/query/node.py b/backend/infrahub/core/query/node.py index 48ea139c285..6fd5a9b7423 100644 --- a/backend/infrahub/core/query/node.py +++ b/backend/infrahub/core/query/node.py @@ -39,7 +39,9 @@ from infrahub.core.schema.attribute_schema import AttributeSchema from infrahub.core.schema.order_by import ( OrderByTargetKind, + ParsedAttributeOrderBy, ParsedOrderByEntry, + ParsedRelationshipAttributeOrderBy, parse_order_by_entry, ) from infrahub.core.timestamp import Timestamp @@ -1618,6 +1620,7 @@ class FieldAttributeRequirement: order_direction: OrderDirection | None = None # created_at, updated_at, created_by, updated_by is_metadata: bool = False + schema_order_position: int | None = None @property def is_attribute_value(self) -> bool: @@ -1704,39 +1707,14 @@ def has_filter_by_id(self) -> bool: return True return False - def _get_metadata_order_fields(self) -> list[tuple[str, OrderDirection]]: - """Return the metadata field and direction to order by, or None.""" - if self.requested_order and self.requested_order.node_metadata: - fields: list[tuple[str, OrderDirection]] = [] - nm = self.requested_order.node_metadata - if nm.created_at: - fields.append((METADATA_CREATED_AT, nm.created_at)) - if nm.updated_at: - fields.append((METADATA_UPDATED_AT, nm.updated_at)) - return fields - - return [ - (parsed.metadata_field.value, parsed.direction) - for parsed in self._get_parsed_schema_order_by() - if parsed.kind is OrderByTargetKind.METADATA - ] - def _get_parsed_schema_order_by(self) -> list[ParsedOrderByEntry]: - if self._query_time_order_overrides_schema(): + query_time_order_overrides_schema = bool(self.requested_order) + if query_time_order_overrides_schema: return [] if not self.schema.order_by: return [] return [parse_order_by_entry(entry=entry, node_schema=self.schema) for entry in self.schema.order_by] - def _query_time_order_overrides_schema(self) -> bool: - if self.requested_order is None: - return False - if self.requested_order.disable: - return True - if self.requested_order.node_metadata: - return True - return False - @property def _has_metadata_filters(self) -> bool: """Check if any metadata filters are requested.""" @@ -1888,10 +1866,6 @@ def _add_metadata_subqueries( self.add_to_query(f"AND {field} {far.comparison_operator} ${param_name}") self.params[param_name] = far.field_attr_value - if far.is_metadata_order: - direction = far.order_direction or OrderDirection.ASC - self.order_by.append(f"{field} {direction.value}") - async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa: ARG002 self.order_by = [] @@ -1934,8 +1908,8 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa return # Determine ordering behavior - disable_order = self.requested_order is not None and self.requested_order.disable - has_any_order = bool(self.schema.order_by) or self._get_metadata_order_fields() + disable_order = bool(self.requested_order and self.requested_order.disable) + has_any_order = bool(self.schema.order_by) or bool(self.requested_order and self.requested_order.node_metadata) # needs ordering or filter if... needs_order_or_filter = bool( @@ -1973,11 +1947,14 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa if not is_default_or_global: self._add_metadata_subqueries(field_requirements=field_attribute_requirements, branch_filter=branch_filter) - # Apply order subqueries for non-metadata attributes (metadata ordering handled by _add_metadata_subqueries) + # Apply order subqueries for non-metadata attributes. await self._add_node_order_attributes( db=db, field_requirements=field_attribute_requirements, branch_filter=branch_filter ) + # Emit the outer ORDER BY clauses in the schema-declared order. + self._emit_schema_order_by(field_attribute_requirements) + # Always order by uuid to guarantee pagination, see https://github.com/opsmill/infrahub/pull/4704. self.order_by.append("n.uuid") @@ -2043,23 +2020,20 @@ async def _add_node_order_attributes( ) -> None: """Add ordering subqueries for schema attributes. - Note: Metadata ordering (created_at, updated_at) is handled by _add_metadata_subqueries. + Outer ORDER BY clauses are emitted by _emit_schema_order_by() so mixed + metadata/attribute entries land in their schema-declared order. """ for far in field_requirements: # Skip metadata ordering - handled by _add_metadata_subqueries - if far.is_metadata: + if far.is_metadata or not far.is_order: continue - # Handle schema attribute ordering if far.field is None: continue - direction = (far.order_direction or OrderDirection.ASC).value - # If this field is also used for filtering, the filter subquery already - # extracted the value - just add it to order_by, don't create another subquery + # extracted the value - no dedicated order subquery needed. if far.is_filter: - self.order_by.append(f"{far.node_value_query_variable} {direction}") continue subquery, subquery_params, _ = await build_subquery_order( @@ -2077,7 +2051,14 @@ async def _add_node_order_attributes( self.params.update(subquery_params) self.add_to_query(["CALL (n) {", subquery, "}", f"WITH {with_str}"]) - self.order_by.append(f"{far.node_value_query_variable} {direction}") + + def _emit_schema_order_by(self, field_requirements: list[FieldAttributeRequirement]) -> None: + order_reqs = [far for far in field_requirements if far.is_order] + order_reqs.sort(key=lambda far: far.schema_order_position if far.schema_order_position is not None else 0) + for far in order_reqs: + direction = (far.order_direction or OrderDirection.ASC).value + variable = far.field_name if far.is_metadata else far.node_value_query_variable + self.order_by.append(f"{variable} {direction}") def _build_filter_where_clause(self, far: FieldAttributeRequirement) -> str | None: """Build a WHERE clause for a single filter requirement. @@ -2221,105 +2202,127 @@ def _get_filter_requirements(self, start_index: int) -> list[FieldAttributeRequi return requirements + def _upsert_metadata_order_requirement( + self, + filter_requirements: list[FieldAttributeRequirement], + metadata_field: str, + direction: OrderDirection, + position: int, + index: int, + ) -> FieldAttributeRequirement | None: + """Promote an existing filter FAR for the same metadata field, or build a new ORDER-only FAR. + + Metadata filter FARs use operator-specific `field_attr_name` ("before", "after", + "value", ...), so we match on `field_name` alone — any FAR for the same metadata + field is fine because the outer ORDER BY references the field name directly. + """ + for req in filter_requirements: + if req.field_name == metadata_field: + req.types.append(FieldAttributeRequirementType.ORDER) + req.order_direction = direction + req.schema_order_position = position + return None + return FieldAttributeRequirement( + field_name=metadata_field, + field=None, + field_attr_name=metadata_field, + field_attr_value=None, + index=index, + types=[FieldAttributeRequirementType.ORDER], + order_direction=direction, + is_metadata=True, + schema_order_position=position, + ) + + def _upsert_attribute_order_requirement( + self, + filter_requirements: list[FieldAttributeRequirement], + parsed: ParsedAttributeOrderBy | ParsedRelationshipAttributeOrderBy, + position: int, + index: int, + ) -> FieldAttributeRequirement | None: + if parsed.kind is OrderByTargetKind.ATTRIBUTE: + order_by_field_name = parsed.attribute_name + order_by_attr_property_name = parsed.property_name + else: + order_by_field_name = parsed.relationship_name + order_by_attr_property_name = f"{parsed.attribute_name}__{parsed.property_name}" + + for req in filter_requirements: + if req.field_name == order_by_field_name and req.field_attr_name == order_by_attr_property_name: + req.types.append(FieldAttributeRequirementType.ORDER) + req.order_direction = parsed.direction + req.schema_order_position = position + return None + + return FieldAttributeRequirement( + field_name=order_by_field_name, + field=self.schema.get_field(order_by_field_name), + field_attr_name=order_by_attr_property_name, + field_attr_value=None, + index=index, + types=[FieldAttributeRequirementType.ORDER], + order_direction=parsed.direction, + schema_order_position=position, + ) + def _get_order_requirements( self, filter_requirements: list[FieldAttributeRequirement], - start_index: int, ) -> list[FieldAttributeRequirement]: """Build ordering requirements. - Handles both metadata ordering and schema order_by. + Handles metadata-based and attribute-based ordering from query overrides or + the order_by defined on the schema. May modify existing requirements in filter_requirements to add ORDER type. Returns list of new FieldAttributeRequirement objects for order-only fields. + Each ORDER requirement is tagged with `schema_order_position` reflecting its + position in the originally declared ordering list, so the outer ORDER BY clause + can be emitted in the declared order regardless of which helper builds the subquery. """ - # Build nested lookup map: field_name -> {field_attr_name -> requirement} - requirements_map: dict[str | None, dict[str, FieldAttributeRequirement]] = {} - for req in filter_requirements: - if req.field_name not in requirements_map: - requirements_map[req.field_name] = {} - requirements_map[req.field_name][req.field_attr_name] = req - new_requirements: list[FieldAttributeRequirement] = [] - index = start_index - - # Add metadata ordering requirements first - for metadata_field, direction in self._get_metadata_order_fields(): - # Check if any filter exists for this metadata field - field_reqs = requirements_map.get(metadata_field) - existing_req = next(iter(field_reqs.values()), None) if field_reqs else None + base_index = len(filter_requirements) + 1 - if existing_req: - # Field already used for filtering, add ORDER type - existing_req.types.append(FieldAttributeRequirementType.ORDER) - existing_req.order_direction = direction - else: - new_requirements.append( - FieldAttributeRequirement( - field_name=metadata_field, - field=None, - field_attr_name=metadata_field, - field_attr_value=None, - index=index, - types=[FieldAttributeRequirementType.ORDER], - order_direction=direction, - is_metadata=True, - ) + # requested order only supports metadata ordering for now + if self.requested_order and self.requested_order.node_metadata: + nm = self.requested_order.node_metadata + pairs: list[tuple[str, OrderDirection]] = [] + if nm.created_at: + pairs.append((METADATA_CREATED_AT, nm.created_at)) + if nm.updated_at: + pairs.append((METADATA_UPDATED_AT, nm.updated_at)) + for position, (metadata_field, direction) in enumerate(pairs): + new_req = self._upsert_metadata_order_requirement( + filter_requirements, metadata_field, direction, position, base_index + position ) - index += 1 + if new_req is not None: + new_requirements.append(new_req) + return new_requirements - # Add schema order_by requirements - for parsed in self._get_parsed_schema_order_by(): + for position, parsed in enumerate(self._get_parsed_schema_order_by()): if parsed.kind is OrderByTargetKind.METADATA: - continue - - if parsed.kind is OrderByTargetKind.ATTRIBUTE: - order_by_field_name = parsed.attribute_name - order_by_attr_property_name = parsed.property_name - else: - order_by_field_name = parsed.relationship_name - order_by_attr_property_name = f"{parsed.attribute_name}__{parsed.property_name}" - - field = self.schema.get_field(order_by_field_name) - field_reqs = requirements_map.get(order_by_field_name) - existing_req = field_reqs.get(order_by_attr_property_name) if field_reqs else None - if existing_req: - # Field already used for filtering, add ORDER type - existing_req.types.append(FieldAttributeRequirementType.ORDER) - existing_req.order_direction = parsed.direction + new_req = self._upsert_metadata_order_requirement( + filter_requirements, + parsed.metadata_field.value, + parsed.direction, + position, + base_index + position, + ) else: - # New field requirement for ordering only - new_requirements.append( - FieldAttributeRequirement( - field_name=order_by_field_name, - field=field, - field_attr_name=order_by_attr_property_name, - field_attr_value=None, - index=index, - types=[FieldAttributeRequirementType.ORDER], - order_direction=parsed.direction, - ) + new_req = self._upsert_attribute_order_requirement( + filter_requirements, parsed, position, base_index + position ) - index += 1 + if new_req is not None: + new_requirements.append(new_req) return new_requirements def _get_field_requirements(self, disable_order: bool = False) -> list[FieldAttributeRequirement]: - """Build unified list of field requirements for filtering and ordering. - - Iterates through filters once, using _get_metadata_field_details to determine - whether each filter is metadata or attribute/relationship based. - """ - # Get filter requirements (single pass through self.filters) + """Build unified list of field requirements for filtering and ordering.""" filter_requirements = self._get_filter_requirements(start_index=1) - if disable_order: return filter_requirements - - # Get ordering requirements (may modify filter_requirements to add ORDER type) - next_index = len(filter_requirements) + 1 - order_requirements = self._get_order_requirements(filter_requirements, start_index=next_index) - - return filter_requirements + order_requirements + return filter_requirements + self._get_order_requirements(filter_requirements) def get_node_ids(self) -> list[str]: return [str(result.get("n.uuid")) for result in self.get_results()] diff --git a/backend/infrahub/core/query/relationship.py b/backend/infrahub/core/query/relationship.py index 10488150db4..80d8ce4ad53 100644 --- a/backend/infrahub/core/query/relationship.py +++ b/backend/infrahub/core/query/relationship.py @@ -14,7 +14,6 @@ ) from infrahub.core.constants import InfrahubKind, MetadataOptions, RelationshipDirection, RelationshipStatus from infrahub.core.constants.database import DatabaseEdgeType -from infrahub.constants.enums import OrderDirection from infrahub.core.order import METADATA_CREATED_AT, METADATA_UPDATED_AT, OrderModel from infrahub.core.query import Query, QueryResult, QueryType from infrahub.core.query.subquery import build_subquery_filter, build_subquery_order, build_subquery_order_metadata @@ -28,6 +27,7 @@ from neo4j.graph import Relationship as Neo4jRelationship + from infrahub.constants.enums import OrderDirection from infrahub.core.branch import Branch from infrahub.core.node import Node from infrahub.core.relationship import Relationship @@ -847,15 +847,6 @@ def _add_updated_metadata_to_query(self, branch_filter_str: str) -> None: """ % {"branch_filter": branch_filter_str, "time_details": time_details} self.add_to_query(last_updated_query) - def _query_time_order_overrides_schema(self) -> bool: - if self.requested_order is None: - return False - if self.requested_order.disable: - return True - if self.requested_order.node_metadata is not None: - return True - return False - def _get_requested_metadata_order_fields(self) -> list[tuple[str, OrderDirection]]: if not (self.requested_order and self.requested_order.node_metadata): return [] @@ -871,8 +862,11 @@ async def _add_peer_order_by(self, db: InfrahubDatabase, peer_schema: MainSchema if self.requested_order and self.requested_order.disable: return - if self._query_time_order_overrides_schema(): - for order_cnt, (metadata_field, direction) in enumerate(self._get_requested_metadata_order_fields(), start=1): + query_time_order_overrides_schema = bool(self.requested_order) + if query_time_order_overrides_schema: + for order_cnt, (metadata_field, direction) in enumerate( + self._get_requested_metadata_order_fields(), start=1 + ): subquery, subquery_params, subquery_result_name = build_subquery_order_metadata( metadata_field=metadata_field, branch=self.branch, diff --git a/backend/tests/component/core/test_node_get_hierarchy_query.py b/backend/tests/component/core/test_node_get_hierarchy_query.py index 2580d6a2364..488ce122443 100644 --- a/backend/tests/component/core/test_node_get_hierarchy_query.py +++ b/backend/tests/component/core/test_node_get_hierarchy_query.py @@ -123,3 +123,132 @@ async def test_NodeGetHierarchyQuery_order_by_uuid_tiebreaker( descendant_ids = list(query.get_peer_ids()) assert descendant_ids == sorted([paris_r1.id, paris_r2.id]) + + +async def test_NodeGetHierarchyQuery_order_by_multi_field_mixed_direction( + db: InfrahubDatabase, + hierarchical_location_data_simple: dict[str, Node], + default_branch: Branch, +) -> None: + location_generic = registry.schema.get(name="LocationGeneric", branch=default_branch, duplicate=False) + location_generic.order_by = ["status__value__desc", "name__value"] + + site_schema = registry.schema.get_node_schema(name="LocationSite", branch=default_branch, duplicate=False) + rack_schema = registry.schema.get_node_schema(name="LocationRack", branch=default_branch, duplicate=False) + + multi_region = await Node.init(db=db, branch=default_branch, schema="LocationRegion") + await multi_region.new(db=db, name="multi-region") + await multi_region.save(db=db) + + multi_site = await Node.init(db=db, branch=default_branch, schema="LocationSite") + await multi_site.new(db=db, name="multi-site", parent=multi_region.id) + await multi_site.save(db=db) + + specs = [ + ("multi-rack-alpha", "offline"), + ("multi-rack-bravo", "online"), + ("multi-rack-charlie", "offline"), + ("multi-rack-delta", "online"), + ] + racks_by_name: dict[str, Node] = {} + for rack_name, status_value in specs: + rack = await Node.init(db=db, branch=default_branch, schema=rack_schema) + await rack.new(db=db, name=rack_name, parent=multi_site.id, status=status_value) + await rack.save(db=db) + racks_by_name[rack_name] = rack + + query = await NodeGetHierarchyQuery.init( + db=db, + direction=RelationshipHierarchyDirection.DESCENDANTS, + node_id=multi_site.id, + node_schema=site_schema, + branch=default_branch, + ) + await query.execute(db=db) + + descendant_ids = list(query.get_peer_ids()) + assert descendant_ids == [ + racks_by_name["multi-rack-bravo"].id, + racks_by_name["multi-rack-delta"].id, + racks_by_name["multi-rack-alpha"].id, + racks_by_name["multi-rack-charlie"].id, + ] + + +@dataclass +class MixedDirectionWithMetadataCase: + name: str + order_by: list[str] + expected_indices: list[int] + + +# Creation order: 0=alpha(status=offline), 1=bravo(status=online), 2=charlie(status=offline), 3=delta(status=online). +# created_at strictly increases with index. +MIXED_DIRECTION_WITH_METADATA_CASES_HIERARCHY = [ + MixedDirectionWithMetadataCase( + name="status_desc_then_metadata_created_desc", + order_by=["status__value__desc", "node_metadata__created_at__desc"], + expected_indices=[3, 1, 2, 0], + ), + MixedDirectionWithMetadataCase( + name="status_desc_then_metadata_created_asc", + order_by=["status__value__desc", "node_metadata__created_at"], + expected_indices=[1, 3, 0, 2], + ), + MixedDirectionWithMetadataCase( + name="metadata_created_desc_then_status_asc", + order_by=["node_metadata__created_at__desc", "status__value"], + expected_indices=[3, 2, 1, 0], + ), + MixedDirectionWithMetadataCase( + name="metadata_created_asc_then_name_desc", + order_by=["node_metadata__created_at", "name__value__desc"], + expected_indices=[0, 1, 2, 3], + ), +] + + +async def test_NodeGetHierarchyQuery_order_by_multi_field_mixed_direction_with_metadata( + db: InfrahubDatabase, + hierarchical_location_data_simple: dict[str, Node], + default_branch: Branch, +) -> None: + location_generic = registry.schema.get(name="LocationGeneric", branch=default_branch, duplicate=False) + site_schema = registry.schema.get_node_schema(name="LocationSite", branch=default_branch, duplicate=False) + rack_schema = registry.schema.get_node_schema(name="LocationRack", branch=default_branch, duplicate=False) + + mfm_region = await Node.init(db=db, branch=default_branch, schema="LocationRegion") + await mfm_region.new(db=db, name="mfm-region") + await mfm_region.save(db=db) + + mfm_site = await Node.init(db=db, branch=default_branch, schema="LocationSite") + await mfm_site.new(db=db, name="mfm-site", parent=mfm_region.id) + await mfm_site.save(db=db) + + specs = [ + ("mfm-rack-alpha", "offline"), + ("mfm-rack-bravo", "online"), + ("mfm-rack-charlie", "offline"), + ("mfm-rack-delta", "online"), + ] + racks: list[Node] = [] + for rack_name, status_value in specs: + rack = await Node.init(db=db, branch=default_branch, schema=rack_schema) + await rack.new(db=db, name=rack_name, parent=mfm_site.id, status=status_value) + await rack.save(db=db) + racks.append(rack) + + for case in MIXED_DIRECTION_WITH_METADATA_CASES_HIERARCHY: + location_generic.order_by = case.order_by + query = await NodeGetHierarchyQuery.init( + db=db, + direction=RelationshipHierarchyDirection.DESCENDANTS, + node_id=mfm_site.id, + node_schema=site_schema, + branch=default_branch, + ) + await query.execute(db=db) + descendant_ids = list(query.get_peer_ids()) + assert descendant_ids == [racks[i].id for i in case.expected_indices], ( + f"order_by={case.order_by!r} produced wrong order" + ) diff --git a/backend/tests/component/core/test_node_get_list_query.py b/backend/tests/component/core/test_node_get_list_query.py index 709babfeab5..cc605fcc1f0 100644 --- a/backend/tests/component/core/test_node_get_list_query.py +++ b/backend/tests/component/core/test_node_get_list_query.py @@ -1386,3 +1386,89 @@ async def test_NodeGetListQuery_order_by_attribute_desc( query = await NodeGetListQuery.init(db=db, branch=branch, schema=criticality_schema) await query.execute(db=db) assert query.get_node_ids() == [nodes[2].id, nodes[1].id, nodes[0].id] + + +async def test_NodeGetListQuery_order_by_multi_field_mixed_direction( + db: InfrahubDatabase, criticality_schema: NodeSchema, branch: Branch +) -> None: + criticality_schema.order_by = ["level__value__desc", "name__value"] + + specs = [ + ("alpha-multi", 1), + ("bravo-multi", 2), + ("charlie-multi", 1), + ("delta-multi", 2), + ] + nodes_by_name: dict[str, Node] = {} + for name, level in specs: + node = await Node.init(db=db, branch=branch, schema=criticality_schema) + await node.new(db=db, name=name, level=level) + await node.save(db=db) + nodes_by_name[name] = node + + query = await NodeGetListQuery.init(db=db, branch=branch, schema=criticality_schema) + await query.execute(db=db) + assert query.get_node_ids() == [ + nodes_by_name["bravo-multi"].id, + nodes_by_name["delta-multi"].id, + nodes_by_name["alpha-multi"].id, + nodes_by_name["charlie-multi"].id, + ] + + +@dataclass +class MixedDirectionWithMetadataCase: + name: str + order_by: list[str] + expected_indices: list[int] + + +# Creation order: 0=alpha(level=1), 1=bravo(level=2), 2=charlie(level=1), 3=delta(level=2). +# created_at strictly increases with index. +MIXED_DIRECTION_WITH_METADATA_CASES_NODE = [ + MixedDirectionWithMetadataCase( + name="level_desc_then_metadata_created_desc", + order_by=["level__value__desc", "node_metadata__created_at__desc"], + expected_indices=[3, 1, 2, 0], + ), + MixedDirectionWithMetadataCase( + name="level_desc_then_metadata_created_asc", + order_by=["level__value__desc", "node_metadata__created_at"], + expected_indices=[1, 3, 0, 2], + ), + MixedDirectionWithMetadataCase( + name="metadata_created_desc_then_level_asc", + order_by=["node_metadata__created_at__desc", "level__value"], + expected_indices=[3, 2, 1, 0], + ), + MixedDirectionWithMetadataCase( + name="metadata_created_asc_then_name_desc", + order_by=["node_metadata__created_at", "name__value__desc"], + expected_indices=[0, 1, 2, 3], + ), +] + + +async def test_NodeGetListQuery_order_by_multi_field_mixed_direction_with_metadata( + db: InfrahubDatabase, criticality_schema: NodeSchema, branch: Branch +) -> None: + specs = [ + ("alpha-mfm", 1), + ("bravo-mfm", 2), + ("charlie-mfm", 1), + ("delta-mfm", 2), + ] + nodes: list[Node] = [] + for name, level in specs: + node = await Node.init(db=db, branch=branch, schema=criticality_schema) + await node.new(db=db, name=name, level=level) + await node.save(db=db) + nodes.append(node) + + for case in MIXED_DIRECTION_WITH_METADATA_CASES_NODE: + criticality_schema.order_by = case.order_by + query = await NodeGetListQuery.init(db=db, branch=branch, schema=criticality_schema) + await query.execute(db=db) + assert query.get_node_ids() == [nodes[i].id for i in case.expected_indices], ( + f"order_by={case.order_by!r} produced wrong order" + ) diff --git a/backend/tests/component/core/test_relationship_get_list_query.py b/backend/tests/component/core/test_relationship_get_list_query.py index a744ee12d84..d6102b6b8ab 100644 --- a/backend/tests/component/core/test_relationship_get_list_query.py +++ b/backend/tests/component/core/test_relationship_get_list_query.py @@ -183,3 +183,122 @@ async def test_RelationshipGetListQuery_order_by_uuid_tiebreaker( peer_ids = [peer.peer_id for peer in query.get_peers()] assert peer_ids == sorted(car.id for car in cars) + + +async def test_RelationshipGetListQuery_order_by_multi_field_mixed_direction( + db: InfrahubDatabase, car_person_schema: SchemaBranch, branch: Branch +) -> None: + car_schema = car_person_schema.get_node(name="TestCar", duplicate=False) + car_schema.order_by = ["nbr_seats__value__desc", "name__value"] + + person_schema = car_person_schema.get_node(name="TestPerson", duplicate=False) + rel_schema = person_schema.get_relationship("cars") + + owner = await Node.init(db=db, branch=branch, schema=person_schema) + await owner.new(db=db, name="multi-owner") + await owner.save(db=db) + + specs = [ + ("alpha-multi-car", 2), + ("bravo-multi-car", 4), + ("charlie-multi-car", 2), + ("delta-multi-car", 4), + ] + cars_by_name: dict[str, Node] = {} + for car_name, seats in specs: + car = await Node.init(db=db, branch=branch, schema=car_schema) + await car.new(db=db, name=car_name, nbr_seats=seats, is_electric=False, owner=owner) + await car.save(db=db) + cars_by_name[car_name] = car + + query = await RelationshipGetPeerQuery.init( + db=db, + branch=branch, + at=Timestamp(), + source=owner, + schema=rel_schema, + rel_type=DatabaseEdgeType.IS_RELATED.value, + ) + await query.execute(db=db) + + peer_ids = [peer.peer_id for peer in query.get_peers()] + assert peer_ids == [ + cars_by_name["bravo-multi-car"].id, + cars_by_name["delta-multi-car"].id, + cars_by_name["alpha-multi-car"].id, + cars_by_name["charlie-multi-car"].id, + ] + + +@dataclass +class MixedDirectionWithMetadataCase: + name: str + order_by: list[str] + expected_indices: list[int] + + +# Creation order: 0=alpha(seats=2), 1=bravo(seats=4), 2=charlie(seats=2), 3=delta(seats=4). +# created_at strictly increases with index. +MIXED_DIRECTION_WITH_METADATA_CASES_REL = [ + MixedDirectionWithMetadataCase( + name="nbr_seats_desc_then_metadata_created_desc", + order_by=["nbr_seats__value__desc", "node_metadata__created_at__desc"], + expected_indices=[3, 1, 2, 0], + ), + MixedDirectionWithMetadataCase( + name="nbr_seats_desc_then_metadata_created_asc", + order_by=["nbr_seats__value__desc", "node_metadata__created_at"], + expected_indices=[1, 3, 0, 2], + ), + MixedDirectionWithMetadataCase( + name="metadata_created_desc_then_nbr_seats_asc", + order_by=["node_metadata__created_at__desc", "nbr_seats__value"], + expected_indices=[3, 2, 1, 0], + ), + MixedDirectionWithMetadataCase( + name="metadata_created_asc_then_name_desc", + order_by=["node_metadata__created_at", "name__value__desc"], + expected_indices=[0, 1, 2, 3], + ), +] + + +async def test_RelationshipGetListQuery_order_by_multi_field_mixed_direction_with_metadata( + db: InfrahubDatabase, car_person_schema: SchemaBranch, branch: Branch +) -> None: + car_schema = car_person_schema.get_node(name="TestCar", duplicate=False) + person_schema = car_person_schema.get_node(name="TestPerson", duplicate=False) + rel_schema = person_schema.get_relationship("cars") + + owner = await Node.init(db=db, branch=branch, schema=person_schema) + await owner.new(db=db, name="mfm-owner") + await owner.save(db=db) + + specs = [ + ("alpha-mfm-car", 2), + ("bravo-mfm-car", 4), + ("charlie-mfm-car", 2), + ("delta-mfm-car", 4), + ] + cars: list[Node] = [] + for car_name, seats in specs: + car = await Node.init(db=db, branch=branch, schema=car_schema) + await car.new(db=db, name=car_name, nbr_seats=seats, is_electric=False, owner=owner) + await car.save(db=db) + cars.append(car) + + for case in MIXED_DIRECTION_WITH_METADATA_CASES_REL: + car_schema.order_by = case.order_by + query = await RelationshipGetPeerQuery.init( + db=db, + branch=branch, + at=Timestamp(), + source=owner, + schema=rel_schema, + rel_type=DatabaseEdgeType.IS_RELATED.value, + ) + await query.execute(db=db) + peer_ids = [peer.peer_id for peer in query.get_peers()] + assert peer_ids == [cars[i].id for i in case.expected_indices], ( + f"order_by={case.order_by!r} produced wrong order" + ) diff --git a/dev/specs/infp-530-order-by-metadata-direction/tasks.md b/dev/specs/infp-530-order-by-metadata-direction/tasks.md index a5ef1b86bcb..efd62d8343b 100644 --- a/dev/specs/infp-530-order-by-metadata-direction/tasks.md +++ b/dev/specs/infp-530-order-by-metadata-direction/tasks.md @@ -92,14 +92,14 @@ description: "Task list for schema-level order_by for node metadata and directio ### Tests for User Story 2 -- [ ] T019 [P] [US2] Add to `backend/tests/component/core/test_node_get_list_query.py` a test `test_NodeGetListQuery_order_by_attribute_desc` with three nodes named alpha / bravo / charlie and `order_by: ["name__value__desc"]`; assert charlie / bravo / alpha order. -- [ ] T020 [P] [US2] Add the equivalent test to `backend/tests/component/core/test_relationship_get_list_query.py`. -- [ ] T021 [P] [US2] Add the equivalent test to `backend/tests/component/core/test_node_get_hierarchy_query.py`. -- [ ] T022 [P] [US2] Add a multi-field mixed-direction test to each of the three component files: `order_by: ["status__value__desc", "name__value"]` on a schema with `status` and `name` attributes. Verify primary sort is descending status, secondary sort is ascending name, across two pairs of nodes that share status. +- [X] T019 [P] [US2] Add to `backend/tests/component/core/test_node_get_list_query.py` a test `test_NodeGetListQuery_order_by_attribute_desc` with three nodes named alpha / bravo / charlie and `order_by: ["name__value__desc"]`; assert charlie / bravo / alpha order. +- [X] T020 [P] [US2] Add the equivalent test to `backend/tests/component/core/test_relationship_get_list_query.py`. +- [X] T021 [P] [US2] Add the equivalent test to `backend/tests/component/core/test_node_get_hierarchy_query.py`. +- [X] T022 [P] [US2] Add a multi-field mixed-direction test to each of the three component files: `order_by: ["status__value__desc", "name__value"]` on a schema with `status` and `name` attributes. Verify primary sort is descending status, secondary sort is ascending name, across two pairs of nodes that share status. ### Implementation for User Story 2 -- [ ] T023 [US2] Run T019–T022. If any test fails, investigate whether the multi-field direction propagation in `_get_field_requirements` (`backend/infrahub/core/query/node.py`) preserves the parsed direction per entry. If not, fix the propagation point and re-run. Do not change behavior for ascending entries — they must remain byte-identical to today. +- [X] T023 [US2] Run T019–T022. If any test fails, investigate whether the multi-field direction propagation in `_get_field_requirements` (`backend/infrahub/core/query/node.py`) preserves the parsed direction per entry. If not, fix the propagation point and re-run. Do not change behavior for ascending entries — they must remain byte-identical to today. **Checkpoint**: All US2 tests pass; descending and mixed-direction multi-field `order_by` is honored across the three paths. Quickstart step 7 succeeds. From f0c35236c4a3f35c299941a2fc1b860a110db897 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Fri, 29 May 2026 18:00:26 -0700 Subject: [PATCH 3/3] hash OrderModel correctly --- backend/infrahub/core/order.py | 6 +++++- backend/infrahub/core/query/node.py | 7 +------ backend/infrahub/graphql/loaders/peers.py | 3 +-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/backend/infrahub/core/order.py b/backend/infrahub/core/order.py index a8e8ebd9791..9c28f29f694 100644 --- a/backend/infrahub/core/order.py +++ b/backend/infrahub/core/order.py @@ -2,7 +2,7 @@ from typing import Self -from pydantic import BaseModel, model_validator +from pydantic import BaseModel, ConfigDict, model_validator from infrahub.constants.enums import OrderDirection # noqa: TC001 from infrahub.exceptions import ValidationError @@ -15,6 +15,8 @@ class NodeMetaOrder(BaseModel): + model_config = ConfigDict(frozen=True) + created_at: OrderDirection | None = None updated_at: OrderDirection | None = None @@ -23,6 +25,8 @@ def __bool__(self) -> bool: class OrderModel(BaseModel): + model_config = ConfigDict(frozen=True) + disable: bool | None = None node_metadata: NodeMetaOrder | None = None diff --git a/backend/infrahub/core/query/node.py b/backend/infrahub/core/query/node.py index 6fd5a9b7423..60b06f2ab8d 100644 --- a/backend/infrahub/core/query/node.py +++ b/backend/infrahub/core/query/node.py @@ -2,7 +2,6 @@ import contextlib from collections import defaultdict -from copy import copy from dataclasses import dataclass from dataclasses import field as dataclass_field from datetime import datetime @@ -1685,11 +1684,7 @@ def __init__( # Force disabling order when `limit` is 1 as it simplifies the query a lot. if "limit" in kwargs and kwargs["limit"] == 1: - if order is None: - order = OrderModel(disable=True) - else: - order = copy(order) - order.disable = True + order = OrderModel(disable=True) if order is None else order.model_copy(update={"disable": True}) self.requested_order = order diff --git a/backend/infrahub/graphql/loaders/peers.py b/backend/infrahub/graphql/loaders/peers.py index 86e78cf0a91..c133ab316ba 100644 --- a/backend/infrahub/graphql/loaders/peers.py +++ b/backend/infrahub/graphql/loaders/peers.py @@ -44,8 +44,7 @@ def __hash__(self) -> int: str(self.source_kind), str(self.branch_agnostic), str(hash(self.include_metadata)), - # TODO: would it be safer to add a __hash__ method to OrderModel or is order guaranteed in model_dump_json? - self.order.model_dump_json() if self.order else "", + str(hash(self.order)) if self.order is not None else "", ] ) return hash(hash_str)