From 112dcaab25f6f568735f38590ff6b26dcefd5682 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 6 Mar 2026 13:35:57 +0100 Subject: [PATCH 1/3] BE-451: Enforce configurable maximum `limit` for entity query endpoints Adds a server-configurable limit (default: 1000) for `queryEntities` and `queryEntitySubgraph`. Requests exceeding the limit are rejected with `InvalidArgument`; omitted limits default to the configured value. - Introduce `ApiConfig` in `hash-graph-api` with optional `clap` feature for CLI argument derivation (`--query-entity-limit` / env `HASH_GRAPH_QUERY_ENTITY_LIMIT`) - Change `QueryEntitiesParams.limit` from `Option` to `usize` - Move limit validation into `into_params` / `into_traversal_params`, returning `Report` with attached `StatusCode::InvalidArgument` --- Cargo.lock | 1 + apps/hash-graph/Cargo.toml | 2 +- apps/hash-graph/src/subcommand/server.rs | 6 +- libs/@local/graph/api/Cargo.toml | 6 ++ libs/@local/graph/api/src/rest/entity.rs | 27 +++++---- .../api/src/rest/entity_query_request.rs | 56 ++++++++++++++----- libs/@local/graph/api/src/rest/mod.rs | 19 ++++++- .../store/postgres/knowledge/entity/mod.rs | 6 +- libs/@local/graph/store/src/entity/store.rs | 2 +- .../graph/scenario/stages/entity_queries.rs | 7 ++- .../manual_queries/entity_queries/mod.rs | 17 +++++- .../read_scaling/knowledge/complete/entity.rs | 2 +- .../read_scaling/knowledge/linkless/entity.rs | 2 +- .../representative_read/knowledge/entity.rs | 6 +- .../postgres/email_filter_protection.rs | 4 +- tests/graph/integration/postgres/entity.rs | 10 ++-- tests/graph/integration/postgres/lib.rs | 12 ++-- tests/graph/integration/postgres/links.rs | 4 +- .../graph/integration/postgres/multi_type.rs | 12 ++-- .../integration/postgres/partial_updates.rs | 12 ++-- tests/graph/integration/postgres/sorting.rs | 2 +- 21 files changed, 143 insertions(+), 72 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 59fcffbd454..0319b329b43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3247,6 +3247,7 @@ dependencies = [ "axum", "axum-core", "bytes", + "clap", "derive-where", "derive_more", "error-stack", diff --git a/apps/hash-graph/Cargo.toml b/apps/hash-graph/Cargo.toml index 5c45453e6f3..cc76659c151 100644 --- a/apps/hash-graph/Cargo.toml +++ b/apps/hash-graph/Cargo.toml @@ -16,7 +16,7 @@ error-stack = { workspace = true } harpc-codec = { workspace = true, features = ["json"] } harpc-server = { workspace = true } hash-codec = { workspace = true } -hash-graph-api = { workspace = true } +hash-graph-api = { workspace = true, features = ["clap"] } hash-graph-authorization = { workspace = true } hash-graph-postgres-store = { workspace = true, features = ["clap"] } hash-graph-store = { workspace = true } diff --git a/apps/hash-graph/src/subcommand/server.rs b/apps/hash-graph/src/subcommand/server.rs index 83239ad5eaf..ddba73bdc80 100644 --- a/apps/hash-graph/src/subcommand/server.rs +++ b/apps/hash-graph/src/subcommand/server.rs @@ -14,7 +14,7 @@ use harpc_codec::json::JsonCodec; use harpc_server::Server; use hash_codec::bytes::JsonLinesEncoder; use hash_graph_api::{ - rest::{QueryLogger, RestApiStore, RestRouterDependencies, rest_api_router}, + rest::{ApiConfig, QueryLogger, RestApiStore, RestRouterDependencies, rest_api_router}, rpc::Dependencies, }; use hash_graph_authorization::policies::store::{PolicyStore, PrincipalStore}; @@ -164,6 +164,9 @@ pub struct ServerConfig { #[clap(long, env = "HASH_GRAPH_SKIP_FILTER_PROTECTION")] pub skip_filter_protection: bool, + #[clap(flatten)] + pub api_config: ApiConfig, + /// Outputs the queries made to the graph to the specified file. #[clap(long)] pub log_queries: Option, @@ -340,6 +343,7 @@ where domain_regex: DomainValidator::new(config.allowed_url_domain), temporal_client, query_logger, + api_config: config.api_config, }); start_rest_server(router, config.http_address, lifecycle); diff --git a/libs/@local/graph/api/Cargo.toml b/libs/@local/graph/api/Cargo.toml index c778a6178f8..9f0e1fe997a 100644 --- a/libs/@local/graph/api/Cargo.toml +++ b/libs/@local/graph/api/Cargo.toml @@ -49,6 +49,9 @@ hashql-hir = { workspace = true } hashql-syntax-jexpr = { workspace = true } type-system = { workspace = true, features = ["utoipa"] } +# Private third-party dependencies (optional) +clap = { workspace = true, optional = true, features = ["derive", "env"] } + # Private third-party dependencies bytes = { workspace = true } derive-where = { workspace = true } @@ -75,6 +78,9 @@ tracing-opentelemetry = { workspace = true } utoipa = { workspace = true } uuid = { workspace = true } +[features] +clap = ["dep:clap"] + [lints] workspace = true diff --git a/libs/@local/graph/api/src/rest/entity.rs b/libs/@local/graph/api/src/rest/entity.rs index ece300f0f9a..28b88f015db 100644 --- a/libs/@local/graph/api/src/rest/entity.rs +++ b/libs/@local/graph/api/src/rest/entity.rs @@ -75,13 +75,15 @@ use type_system::{ }; use utoipa::{OpenApi, ToSchema}; -use super::{InteractiveHeader, status::BoxedResponse}; pub use crate::rest::entity_query_request::{ EntityQuery, EntityQueryOptions, QueryEntitiesRequest, QueryEntitySubgraphRequest, }; use crate::rest::{ - AuthenticatedUserHeader, OpenApiQuery, QueryLogger, entity_query_request::CompilationOptions, - json::Json, status::report_to_response, utoipa_typedef::subgraph::Subgraph, + ApiConfig, AuthenticatedUserHeader, InteractiveHeader, OpenApiQuery, QueryLogger, + entity_query_request::CompilationOptions, + json::Json, + status::{BoxedResponse, report_to_response}, + utoipa_typedef::subgraph::Subgraph, }; #[derive(OpenApi)] @@ -428,6 +430,7 @@ async fn query_entities( InteractiveHeader(interactive): InteractiveHeader, store_pool: Extension>, temporal_client: Extension>>, + Extension(api_config): Extension, mut query_logger: Option>, Json(request): Json>, ) -> Result>, BoxedResponse> @@ -449,13 +452,6 @@ where let (query, options) = request.into_parts(); - if options.limit == Some(0) { - tracing::warn!( - %actor_id, - "The limit is set to zero, so no entities will be returned." - ); - } - // TODO: https://linear.app/hash/issue/H-5351/reuse-parts-between-compilation-units let mut heap = Heap::uninitialized(); @@ -469,7 +465,10 @@ where let filter = query.compile(&heap, CompilationOptions { interactive })?; - let params = options.into_params(filter); + let params = options + .into_params(filter, api_config) + .attach(hash_status::StatusCode::InvalidArgument) + .map_err(report_to_response)?; let response = store .query_entities(actor_id, params) @@ -545,6 +544,7 @@ async fn query_entity_subgraph( InteractiveHeader(interactive): InteractiveHeader, store_pool: Extension>, temporal_client: Extension>>, + Extension(api_config): Extension, mut query_logger: Option>, Json(request): Json, ) -> Result>, BoxedResponse> @@ -578,7 +578,10 @@ where let filter = query.compile(&heap, CompilationOptions { interactive })?; - let params = options.into_traversal_params(filter, traversal); + let params = options + .into_traversal_params(filter, traversal, api_config) + .attach(hash_status::StatusCode::InvalidArgument) + .map_err(report_to_response)?; let response = store .query_entity_subgraph(actor_id, params) diff --git a/libs/@local/graph/api/src/rest/entity_query_request.rs b/libs/@local/graph/api/src/rest/entity_query_request.rs index 6700a91ec19..fff6b55d505 100644 --- a/libs/@local/graph/api/src/rest/entity_query_request.rs +++ b/libs/@local/graph/api/src/rest/entity_query_request.rs @@ -21,6 +21,7 @@ use axum::{ Json, response::{Html, IntoResponse as _}, }; +use error_stack::Report; use hash_graph_store::{ entity::{ EntityQueryCursor, EntityQueryPath, EntityQuerySorting, EntityQuerySortingRecord, @@ -65,7 +66,7 @@ use serde_json::value::RawValue as RawJsonValue; use type_system::knowledge::Entity; use utoipa::ToSchema; -use super::status::BoxedResponse; +use super::{ApiConfig, status::BoxedResponse}; #[tracing::instrument(level = "info", skip_all)] fn generate_sorting_paths( @@ -485,7 +486,7 @@ impl<'q> EntityQuery<'q> { } #[derive(Debug, Copy, Clone, PartialEq, Eq, derive_more::Display)] -enum EntityQueryOptionsError { +pub enum EntityQueryOptionsError { #[display( "Field '{field}' is only valid in subgraph requests. Use the subgraph endpoint instead." )] @@ -495,6 +496,8 @@ enum EntityQueryOptionsError { instead." )] InvalidFieldForEntityOptions { field: &'static str }, + #[display("The requested limit ({requested}) exceeds the maximum allowed limit ({max}).")] + LimitExceeded { requested: usize, max: usize }, } impl core::error::Error for EntityQueryOptionsError {} @@ -597,12 +600,31 @@ impl<'q, 's, 'p> TryFrom> for EntityQue } impl<'p> EntityQueryOptions<'_, 'p> { - #[must_use] - pub fn into_params<'f>(self, filter: Filter<'f, Entity>) -> QueryEntitiesParams<'f> + /// # Errors + /// + /// Returns [`EntityQueryOptionsError::LimitExceeded`] if the requested limit exceeds the + /// configured maximum in [`ApiConfig::query_entity_limit`]. + pub fn into_params<'f>( + self, + filter: Filter<'f, Entity>, + config: ApiConfig, + ) -> Result, Report> where 'p: 'f, { - QueryEntitiesParams { + let max = config.query_entity_limit; + let limit = match self.limit { + Some(requested) if requested > max => { + return Err(Report::new(EntityQueryOptionsError::LimitExceeded { + requested, + max, + })); + } + Some(limit) => limit, + None => max, + }; + + Ok(QueryEntitiesParams { filter, sorting: generate_sorting_paths( self.sorting_paths, @@ -610,7 +632,7 @@ impl<'p> EntityQueryOptions<'_, 'p> { self.cursor, &self.temporal_axes, ), - limit: self.limit, + limit, conversions: self.conversions, include_drafts: self.include_drafts, include_count: self.include_count, @@ -622,33 +644,37 @@ impl<'p> EntityQueryOptions<'_, 'p> { include_type_ids: self.include_type_ids, include_type_titles: self.include_type_titles, include_permissions: self.include_permissions, - } + }) } - #[must_use] + /// # Errors + /// + /// Returns [`EntityQueryOptionsError::LimitExceeded`] if the requested limit exceeds the + /// configured maximum in [`ApiConfig::query_entity_limit`]. pub fn into_traversal_params<'q>( self, filter: Filter<'q, Entity>, traversal: SubgraphTraversalParams, - ) -> QueryEntitySubgraphParams<'q> + config: ApiConfig, + ) -> Result, Report> where 'p: 'q, { match traversal { SubgraphTraversalParams::Paths { traversal_paths } => { - QueryEntitySubgraphParams::Paths { + Ok(QueryEntitySubgraphParams::Paths { traversal_paths, - request: self.into_params(filter), - } + request: self.into_params(filter, config)?, + }) } SubgraphTraversalParams::ResolveDepths { traversal_paths, graph_resolve_depths, - } => QueryEntitySubgraphParams::ResolveDepths { + } => Ok(QueryEntitySubgraphParams::ResolveDepths { traversal_paths, graph_resolve_depths, - request: self.into_params(filter), - }, + request: self.into_params(filter, config)?, + }), } } } diff --git a/libs/@local/graph/api/src/rest/mod.rs b/libs/@local/graph/api/src/rest/mod.rs index d24385643be..815b128ce4a 100644 --- a/libs/@local/graph/api/src/rest/mod.rs +++ b/libs/@local/graph/api/src/rest/mod.rs @@ -323,6 +323,21 @@ pub enum OpenApiQuery<'a> { DiffEntity(&'a DiffEntityParams), } +/// Server-side configuration for the REST API, shared across handlers via an [`Extension`]. +#[derive(Debug, Clone, Copy)] +#[cfg_attr(feature = "clap", derive(clap::Parser))] +pub struct ApiConfig { + /// The default and maximum number of entities returned by a single query. + /// + /// When a request omits `limit`, this value is used. Requests that specify a `limit` larger + /// than this value are rejected. + #[cfg_attr( + feature = "clap", + clap(long, default_value_t = 1000, env = "HASH_GRAPH_QUERY_ENTITY_LIMIT") + )] + pub query_entity_limit: usize, +} + pub struct RestRouterDependencies where S: StorePool + Send + Sync + 'static, @@ -331,6 +346,7 @@ where pub temporal_client: Option>, pub domain_regex: DomainValidator, pub query_logger: Option, + pub api_config: ApiConfig, } /// A [`Router`] that only serves the `OpenAPI` specification (JSON, and necessary subschemas) for @@ -376,7 +392,8 @@ where .layer(http_tracing_layer::HttpTracingLayer) .layer(Extension(dependencies.store)) .layer(Extension(dependencies.temporal_client)) - .layer(Extension(dependencies.domain_regex)); + .layer(Extension(dependencies.domain_regex)) + .layer(Extension(dependencies.api_config)); if let Some(query_logger) = dependencies.query_logger { router = router.layer(Extension(query_logger)); diff --git a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs index 8d7870435c2..1a9e44593f5 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs @@ -700,9 +700,7 @@ where (None, None, None, None, None, None) }; - if let Some(limit) = params.limit { - compiler.set_limit(limit); - } + compiler.set_limit(params.limit); let cursor_parameters = params.sorting.encode().change_context(QueryError)?; let cursor_indices = params @@ -742,7 +740,7 @@ where .enumerate() .map(|(idx, row)| { let row = TypedRow::::from(row); - if idx == num_rows - 1 && params.limit == Some(num_rows) { + if idx == num_rows - 1 && params.limit == num_rows { cursor = Some(row.decode_cursor(&artifacts)); } row.decode_record(&artifacts) diff --git a/libs/@local/graph/store/src/entity/store.rs b/libs/@local/graph/store/src/entity/store.rs index 2d6f85d2c69..c9bcfe6ea91 100644 --- a/libs/@local/graph/store/src/entity/store.rs +++ b/libs/@local/graph/store/src/entity/store.rs @@ -195,7 +195,7 @@ pub struct QueryEntitiesParams<'a> { pub temporal_axes: QueryTemporalAxesUnresolved, pub sorting: EntityQuerySorting<'static>, pub conversions: Vec>, - pub limit: Option, + pub limit: usize, pub include_drafts: bool, pub include_count: bool, pub include_entity_types: Option, diff --git a/tests/graph/benches/graph/scenario/stages/entity_queries.rs b/tests/graph/benches/graph/scenario/stages/entity_queries.rs index 1d6059e60f7..3e83edf2816 100644 --- a/tests/graph/benches/graph/scenario/stages/entity_queries.rs +++ b/tests/graph/benches/graph/scenario/stages/entity_queries.rs @@ -26,12 +26,17 @@ pub enum QueryEntitiesError { impl Error for QueryEntitiesError {} +const fn default_limit() -> usize { + 1000 +} + #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct QueryEntitiesInputs { #[serde(default)] pub user_catalog: Option, - pub limit: Option, + #[serde(default = "default_limit")] + pub limit: usize, } #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] diff --git a/tests/graph/benches/manual_queries/entity_queries/mod.rs b/tests/graph/benches/manual_queries/entity_queries/mod.rs index 72aa91f2225..e4fc613cca1 100644 --- a/tests/graph/benches/manual_queries/entity_queries/mod.rs +++ b/tests/graph/benches/manual_queries/entity_queries/mod.rs @@ -6,7 +6,7 @@ use criterion_macro::criterion; use either::Either; use error_stack::Report; use hash_graph_api::rest::{ - self, + self, ApiConfig, entity::{EntityQueryOptions, QueryEntitiesRequest, QueryEntitySubgraphRequest}, }; use hash_graph_postgres_store::{ @@ -335,6 +335,10 @@ async fn run_benchmark<'q, 's, 'p: 'q, S>(store: &S, request: GraphQuery<'q, 's, where S: EntityStore + Sync, { + let config = ApiConfig { + query_entity_limit: 1000, + }; + match request { GraphQuery::QueryEntities(request) => { let (query, options) = request.request.into_parts(); @@ -343,7 +347,12 @@ where }; let _response = store - .query_entities(request.actor_id, options.into_params(filter)) + .query_entities( + request.actor_id, + options + .into_params(filter, config) + .expect("limit should not exceed configured maximum"), + ) .await .expect("failed to read entities from store"); } @@ -356,7 +365,9 @@ where let _response = store .query_entity_subgraph( request.actor_id, - options.into_traversal_params(filter, traversal), + options + .into_traversal_params(filter, traversal, config) + .expect("limit should not exceed configured maximum"), ) .await .expect("failed to read entity subgraph from store"); diff --git a/tests/graph/benches/read_scaling/knowledge/complete/entity.rs b/tests/graph/benches/read_scaling/knowledge/complete/entity.rs index 3102684afb3..94b00a52369 100644 --- a/tests/graph/benches/read_scaling/knowledge/complete/entity.rs +++ b/tests/graph/benches/read_scaling/knowledge/complete/entity.rs @@ -257,7 +257,7 @@ pub fn bench_get_entity_by_id( paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, diff --git a/tests/graph/benches/read_scaling/knowledge/linkless/entity.rs b/tests/graph/benches/read_scaling/knowledge/linkless/entity.rs index 4e9ebefab30..715a10c0bac 100644 --- a/tests/graph/benches/read_scaling/knowledge/linkless/entity.rs +++ b/tests/graph/benches/read_scaling/knowledge/linkless/entity.rs @@ -191,7 +191,7 @@ pub fn bench_get_entity_by_id( paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, diff --git a/tests/graph/benches/representative_read/knowledge/entity.rs b/tests/graph/benches/representative_read/knowledge/entity.rs index 1b92d2015f6..2b75253c6cf 100644 --- a/tests/graph/benches/representative_read/knowledge/entity.rs +++ b/tests/graph/benches/representative_read/knowledge/entity.rs @@ -59,7 +59,7 @@ pub fn bench_get_entity_by_id( paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, @@ -120,7 +120,7 @@ pub fn bench_query_entities_by_property( paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, @@ -186,7 +186,7 @@ pub fn bench_get_link_by_target_by_property( paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, diff --git a/tests/graph/integration/postgres/email_filter_protection.rs b/tests/graph/integration/postgres/email_filter_protection.rs index 76416338496..2d22cf1038f 100644 --- a/tests/graph/integration/postgres/email_filter_protection.rs +++ b/tests/graph/integration/postgres/email_filter_protection.rs @@ -392,7 +392,7 @@ impl DatabaseApi<'_> { filter, temporal_axes: standard_temporal_axes(), sorting, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, @@ -3161,7 +3161,7 @@ async fn subgraph_traversal_masks_linked_user_email() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, diff --git a/tests/graph/integration/postgres/entity.rs b/tests/graph/integration/postgres/entity.rs index ff5f5f18931..49aae06ccbe 100644 --- a/tests/graph/integration/postgres/entity.rs +++ b/tests/graph/integration/postgres/entity.rs @@ -103,7 +103,7 @@ async fn insert() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: true, include_entity_types: None, @@ -185,7 +185,7 @@ async fn query() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: true, include_entity_types: None, @@ -316,7 +316,7 @@ async fn update() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, @@ -355,7 +355,7 @@ async fn update() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: true, include_entity_types: None, @@ -391,7 +391,7 @@ async fn update() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: true, include_entity_types: None, diff --git a/tests/graph/integration/postgres/lib.rs b/tests/graph/integration/postgres/lib.rs index 7761b2bd88d..78aee083155 100644 --- a/tests/graph/integration/postgres/lib.rs +++ b/tests/graph/integration/postgres/lib.rs @@ -760,7 +760,7 @@ impl EntityStore for DatabaseApi<'_> { mut params: QueryEntitiesParams<'_>, ) -> Result, Report> { let include_count = params.include_count; - let has_limit = params.limit.is_some(); + let limit = params.limit; params.include_count = true; let count = self @@ -778,8 +778,8 @@ impl EntityStore for DatabaseApi<'_> { // We can ensure that `count_entities` and `get_entity` return the same count; assert_eq!(response.count, Some(count)); - // if the limit is not set, the count should be equal to the number of entities returned - if !has_limit { + // if the limit is large enough, the count should be equal to the number of entities + if count <= limit { assert_eq!(count, response.entities.len()); } @@ -797,7 +797,7 @@ impl EntityStore for DatabaseApi<'_> { let request = params.request_mut(); let include_count = request.include_count; - let has_limit = request.limit.is_some(); + let limit = request.limit; request.include_count = true; let count = self @@ -814,8 +814,8 @@ impl EntityStore for DatabaseApi<'_> { // We can ensure that `count_entities` and `get_entity` return the same count; assert_eq!(response.count, Some(count)); - // if the limit is not set, the count should be equal to the number of entities returned - if !has_limit { + // if the limit is large enough, the count should be equal to the number of entities + if count <= limit { assert_eq!(count, response.subgraph.roots.len()); } diff --git a/tests/graph/integration/postgres/links.rs b/tests/graph/integration/postgres/links.rs index 36d250d2601..d7910105466 100644 --- a/tests/graph/integration/postgres/links.rs +++ b/tests/graph/integration/postgres/links.rs @@ -236,7 +236,7 @@ async fn insert() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: true, include_entity_types: None, @@ -494,7 +494,7 @@ async fn get_entity_links() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, diff --git a/tests/graph/integration/postgres/multi_type.rs b/tests/graph/integration/postgres/multi_type.rs index 16f425cd9d4..5fa1271b24d 100644 --- a/tests/graph/integration/postgres/multi_type.rs +++ b/tests/graph/integration/postgres/multi_type.rs @@ -153,7 +153,7 @@ async fn initial_person() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: true, include_entity_types: None, @@ -218,7 +218,7 @@ async fn initial_person() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: true, include_entity_types: None, @@ -247,7 +247,7 @@ async fn initial_person() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: true, include_entity_types: None, @@ -330,7 +330,7 @@ async fn create_multi() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: true, include_entity_types: None, @@ -359,7 +359,7 @@ async fn create_multi() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: true, include_entity_types: None, @@ -419,7 +419,7 @@ async fn create_multi() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: true, include_entity_types: None, diff --git a/tests/graph/integration/postgres/partial_updates.rs b/tests/graph/integration/postgres/partial_updates.rs index 711aae56509..9278fff17d8 100644 --- a/tests/graph/integration/postgres/partial_updates.rs +++ b/tests/graph/integration/postgres/partial_updates.rs @@ -172,7 +172,7 @@ async fn properties_add() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, @@ -267,7 +267,7 @@ async fn properties_remove() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, @@ -364,7 +364,7 @@ async fn properties_replace() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, @@ -454,7 +454,7 @@ async fn type_ids() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, @@ -512,7 +512,7 @@ async fn type_ids() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, @@ -572,7 +572,7 @@ async fn type_ids() { paths: Vec::new(), cursor: None, }, - limit: None, + limit: 1000, conversions: Vec::new(), include_count: false, include_entity_types: None, diff --git a/tests/graph/integration/postgres/sorting.rs b/tests/graph/integration/postgres/sorting.rs index 20fadd6f28f..a3b022a52fe 100644 --- a/tests/graph/integration/postgres/sorting.rs +++ b/tests/graph/integration/postgres/sorting.rs @@ -92,7 +92,7 @@ async fn test_root_sorting( paths: sorting_paths.clone(), cursor: Option::take(&mut cursor), }, - limit: Some(chunk_size), + limit: chunk_size, conversions: Vec::new(), include_count: true, include_entity_types: None, From ccc0b67af2332dcea948aea568b1a1416eb6295e Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 6 Mar 2026 13:51:53 +0100 Subject: [PATCH 2/3] Remove `limit` and `cursor` parameters from `generate_sorting_paths` Since limit is now always resolved to a concrete value by `into_params`, sorting paths are always generated. The cursor was only passed through to the return struct without affecting path generation logic. The function now returns `Vec` and the caller assembles `EntityQuerySorting` with the cursor directly. --- .../api/src/rest/entity_query_request.rs | 61 ++++++++----------- 1 file changed, 24 insertions(+), 37 deletions(-) diff --git a/libs/@local/graph/api/src/rest/entity_query_request.rs b/libs/@local/graph/api/src/rest/entity_query_request.rs index fff6b55d505..4e37b6ace2b 100644 --- a/libs/@local/graph/api/src/rest/entity_query_request.rs +++ b/libs/@local/graph/api/src/rest/entity_query_request.rs @@ -71,39 +71,33 @@ use super::{ApiConfig, status::BoxedResponse}; #[tracing::instrument(level = "info", skip_all)] fn generate_sorting_paths( paths: Option>>, - limit: Option, - cursor: Option>, temporal_axes: &QueryTemporalAxesUnresolved, -) -> EntityQuerySorting<'static> { +) -> Vec> { let temporal_axes_sorting_path = match temporal_axes { QueryTemporalAxesUnresolved::TransactionTime { .. } => &EntityQueryPath::TransactionTime, QueryTemporalAxesUnresolved::DecisionTime { .. } => &EntityQueryPath::DecisionTime, }; - let sorting = paths + paths .map_or_else( || { - if limit.is_some() || cursor.is_some() { - vec![ - EntityQuerySortingRecord { - path: temporal_axes_sorting_path.clone(), - ordering: Ordering::Descending, - nulls: None, - }, - EntityQuerySortingRecord { - path: EntityQueryPath::Uuid, - ordering: Ordering::Ascending, - nulls: None, - }, - EntityQuerySortingRecord { - path: EntityQueryPath::WebId, - ordering: Ordering::Ascending, - nulls: None, - }, - ] - } else { - Vec::new() - } + vec![ + EntityQuerySortingRecord { + path: temporal_axes_sorting_path.clone(), + ordering: Ordering::Descending, + nulls: None, + }, + EntityQuerySortingRecord { + path: EntityQueryPath::Uuid, + ordering: Ordering::Ascending, + nulls: None, + }, + EntityQuerySortingRecord { + path: EntityQueryPath::WebId, + ordering: Ordering::Ascending, + nulls: None, + }, + ] }, |mut paths| { let mut has_temporal_axis = false; @@ -151,12 +145,7 @@ fn generate_sorting_paths( ) .into_iter() .map(EntityQuerySortingRecord::into_owned) - .collect(); - - EntityQuerySorting { - paths: sorting, - cursor: cursor.map(EntityQueryCursor::into_owned), - } + .collect() } /// Internal deserialization proxy for `QueryEntitiesRequest`. @@ -626,12 +615,10 @@ impl<'p> EntityQueryOptions<'_, 'p> { Ok(QueryEntitiesParams { filter, - sorting: generate_sorting_paths( - self.sorting_paths, - self.limit, - self.cursor, - &self.temporal_axes, - ), + sorting: EntityQuerySorting { + paths: generate_sorting_paths(self.sorting_paths, &self.temporal_axes), + cursor: self.cursor.map(EntityQueryCursor::into_owned), + }, limit, conversions: self.conversions, include_drafts: self.include_drafts, From cad3334dee8cd3ad5c1fe72881b1a0eb965ddb96 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 6 Mar 2026 15:56:21 +0100 Subject: [PATCH 3/3] Fix test assertions for paginated queries and resolve doc warnings The integration test assertions now correctly account for pagination by checking `cursor.is_none()` (first page) in addition to the limit check. Previously, the assertion fired incorrectly on follow-up pages where `count` (total) != `results.len()` (current page). Also fix rustdoc warnings for intra-doc links to private `EntityQueryOptionsError::LimitExceeded`. --- .../api/src/rest/entity_query_request.rs | 8 +-- tests/graph/integration/postgres/lib.rs | 60 +++++++++++-------- 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/libs/@local/graph/api/src/rest/entity_query_request.rs b/libs/@local/graph/api/src/rest/entity_query_request.rs index 4e37b6ace2b..44b3086df0e 100644 --- a/libs/@local/graph/api/src/rest/entity_query_request.rs +++ b/libs/@local/graph/api/src/rest/entity_query_request.rs @@ -591,8 +591,8 @@ impl<'q, 's, 'p> TryFrom> for EntityQue impl<'p> EntityQueryOptions<'_, 'p> { /// # Errors /// - /// Returns [`EntityQueryOptionsError::LimitExceeded`] if the requested limit exceeds the - /// configured maximum in [`ApiConfig::query_entity_limit`]. + /// Returns `LimitExceeded` if the requested limit exceeds the configured maximum in + /// [`ApiConfig::query_entity_limit`]. pub fn into_params<'f>( self, filter: Filter<'f, Entity>, @@ -636,8 +636,8 @@ impl<'p> EntityQueryOptions<'_, 'p> { /// # Errors /// - /// Returns [`EntityQueryOptionsError::LimitExceeded`] if the requested limit exceeds the - /// configured maximum in [`ApiConfig::query_entity_limit`]. + /// Returns `LimitExceeded` if the requested limit exceeds the configured maximum in + /// [`ApiConfig::query_entity_limit`]. pub fn into_traversal_params<'q>( self, filter: Filter<'q, Entity>, diff --git a/tests/graph/integration/postgres/lib.rs b/tests/graph/integration/postgres/lib.rs index 78aee083155..c8035271296 100644 --- a/tests/graph/integration/postgres/lib.rs +++ b/tests/graph/integration/postgres/lib.rs @@ -296,7 +296,8 @@ impl DataTypeStore for DatabaseApi<'_> { mut params: QueryDataTypesParams<'_>, ) -> Result> { let include_count = params.include_count; - let has_limit = params.limit.is_some(); + let is_first_page = params.after.is_none(); + let limit = params.limit; params.include_count = true; let count = self @@ -313,8 +314,9 @@ impl DataTypeStore for DatabaseApi<'_> { // We can ensure that `count_data_types` and `get_data_types` return the same count; assert_eq!(response.count, Some(count)); - // if the limit is not set, the count should be equal to the number of data types returned - if !has_limit { + // if this is the first page and the limit is large enough (or not set), all data types + // should be returned + if is_first_page && limit.is_none_or(|limit| count <= limit) { assert_eq!(count, response.data_types.len()); } @@ -331,7 +333,8 @@ impl DataTypeStore for DatabaseApi<'_> { ) -> Result> { let request = params.request_mut(); let include_count = request.include_count; - let has_limit = request.limit.is_some(); + let is_first_page = request.after.is_none(); + let limit = request.limit; request.include_count = true; let count = self @@ -351,8 +354,9 @@ impl DataTypeStore for DatabaseApi<'_> { // We can ensure that `count_data_types` and `get_data_type_subgraph` return the same count; assert_eq!(response.count, Some(count)); - // if the limit is not set, the count should be equal to the number of data types returned - if !has_limit { + // if this is the first page and the limit is large enough (or not set), all data types + // should be returned + if is_first_page && limit.is_none_or(|limit| count <= limit) { assert_eq!(count, response.subgraph.roots.len()); } @@ -450,7 +454,8 @@ impl PropertyTypeStore for DatabaseApi<'_> { mut params: QueryPropertyTypesParams<'_>, ) -> Result> { let include_count = params.include_count; - let has_limit = params.limit.is_some(); + let is_first_page = params.after.is_none(); + let limit = params.limit; params.include_count = true; let count = self @@ -467,9 +472,9 @@ impl PropertyTypeStore for DatabaseApi<'_> { // We can ensure that `count_property_types` and `get_property_types` return the same count; assert_eq!(response.count, Some(count)); - // if the limit is not set, the count should be equal to the number of property types - // returned - if !has_limit { + // if this is the first page and the limit is large enough (or not set), all property types + // should be returned + if is_first_page && limit.is_none_or(|limit| count <= limit) { assert_eq!(count, response.property_types.len()); } @@ -487,7 +492,8 @@ impl PropertyTypeStore for DatabaseApi<'_> { let request = params.request_mut(); let include_count = request.include_count; - let has_limit = request.limit.is_some(); + let is_first_page = request.after.is_none(); + let limit = request.limit; request.include_count = true; let count = self @@ -508,9 +514,9 @@ impl PropertyTypeStore for DatabaseApi<'_> { // We can ensure that `count_property_types` and `get_property_type_subgraph` return the // same count; assert_eq!(response.count, Some(count)); - // if the limit is not set, the count should be equal to the number of property types - // returned - if !has_limit { + // if this is the first page and the limit is large enough (or not set), all property types + // should be returned + if is_first_page && limit.is_none_or(|limit| count <= limit) { assert_eq!(count, response.subgraph.roots.len()); } @@ -596,7 +602,8 @@ impl EntityTypeStore for DatabaseApi<'_> { let request = &mut params.request; let include_count = request.include_count; - let has_limit = request.limit.is_some(); + let is_first_page = request.after.is_none(); + let limit = request.limit; request.include_count = true; let count = self @@ -613,8 +620,9 @@ impl EntityTypeStore for DatabaseApi<'_> { // We can ensure that `count_entity_types` and `get_entity_types` return the same count; assert_eq!(response.count, Some(count)); - // if the limit is not set, the count should be equal to the number of entity types returned - if !has_limit { + // if this is the first page and the limit is large enough (or not set), all entity types + // should be returned + if is_first_page && limit.is_none_or(|limit| count <= limit) { assert_eq!(count, response.entity_types.len()); } @@ -653,7 +661,8 @@ impl EntityTypeStore for DatabaseApi<'_> { let request = params.request_mut(); let include_count = request.include_count; - let has_limit = request.limit.is_some(); + let is_first_page = request.after.is_none(); + let limit = request.limit; request.include_count = true; let count = self @@ -674,8 +683,9 @@ impl EntityTypeStore for DatabaseApi<'_> { // We can ensure that `count_entity_types` and `get_entity_type_subgraph` return the same // count; assert_eq!(response.count, Some(count)); - // if the limit is not set, the count should be equal to the number of entity types returned - if !has_limit { + // if this is the first page and the limit is large enough (or not set), all entity types + // should be returned + if is_first_page && limit.is_none_or(|limit| count <= limit) { assert_eq!(count, response.subgraph.roots.len()); } @@ -760,6 +770,7 @@ impl EntityStore for DatabaseApi<'_> { mut params: QueryEntitiesParams<'_>, ) -> Result, Report> { let include_count = params.include_count; + let is_first_page = params.sorting.cursor.is_none(); let limit = params.limit; params.include_count = true; @@ -778,8 +789,8 @@ impl EntityStore for DatabaseApi<'_> { // We can ensure that `count_entities` and `get_entity` return the same count; assert_eq!(response.count, Some(count)); - // if the limit is large enough, the count should be equal to the number of entities - if count <= limit { + // if this is the first page and the limit is large enough, all entities should be returned + if is_first_page && count <= limit { assert_eq!(count, response.entities.len()); } @@ -797,6 +808,7 @@ impl EntityStore for DatabaseApi<'_> { let request = params.request_mut(); let include_count = request.include_count; + let is_first_page = request.sorting.cursor.is_none(); let limit = request.limit; request.include_count = true; @@ -814,8 +826,8 @@ impl EntityStore for DatabaseApi<'_> { // We can ensure that `count_entities` and `get_entity` return the same count; assert_eq!(response.count, Some(count)); - // if the limit is large enough, the count should be equal to the number of entities - if count <= limit { + // if this is the first page and the limit is large enough, all entities should be returned + if is_first_page && count <= limit { assert_eq!(count, response.subgraph.roots.len()); }