From 7c3e13c8c4f3446c6e555af730a57f806dd31952 Mon Sep 17 00:00:00 2001 From: JunichiSugiura Date: Sun, 19 Nov 2023 14:24:49 +0100 Subject: [PATCH] Return page info even for limit/offset based pagination --- .../graphql/src/object/connection/mod.rs | 4 +- .../src/object/connection/page_info.rs | 41 ++++--- crates/torii/graphql/src/object/entity.rs | 1 + crates/torii/graphql/src/object/event.rs | 1 + .../torii/graphql/src/object/metadata/mod.rs | 17 ++- crates/torii/graphql/src/object/mod.rs | 1 + crates/torii/graphql/src/object/model_data.rs | 1 + crates/torii/graphql/src/query/data.rs | 107 ++++++++++-------- .../torii/graphql/src/tests/entities_test.rs | 63 ++++++----- .../torii/graphql/src/tests/metadata_test.rs | 6 + crates/torii/graphql/src/tests/mod.rs | 3 +- crates/torii/graphql/src/tests/models_test.rs | 6 + 12 files changed, 142 insertions(+), 109 deletions(-) diff --git a/crates/torii/graphql/src/object/connection/mod.rs b/crates/torii/graphql/src/object/connection/mod.rs index 9a571eb8bd..63b796e31b 100644 --- a/crates/torii/graphql/src/object/connection/mod.rs +++ b/crates/torii/graphql/src/object/connection/mod.rs @@ -43,7 +43,7 @@ impl ConnectionObject { (Name::new("total_count"), TypeData::Simple(TypeRef::named_nn(TypeRef::INT))), ( Name::new("page_info"), - TypeData::Nested((TypeRef::named(PAGE_INFO_TYPE_NAME), IndexMap::new())), + TypeData::Nested((TypeRef::named_nn(PAGE_INFO_TYPE_NAME), IndexMap::new())), ), ]); @@ -117,7 +117,7 @@ pub fn connection_output( id_column: &str, total_count: i64, is_external: bool, - page_info: Option, + page_info: PageInfo, ) -> sqlx::Result { let model_edges = data .iter() diff --git a/crates/torii/graphql/src/object/connection/page_info.rs b/crates/torii/graphql/src/object/connection/page_info.rs index 4b8a7b4e69..126f00f7aa 100644 --- a/crates/torii/graphql/src/object/connection/page_info.rs +++ b/crates/torii/graphql/src/object/connection/page_info.rs @@ -31,27 +31,24 @@ impl ObjectTrait for PageInfoObject { } impl PageInfoObject { - pub fn value(page_info: Option) -> Value { - match page_info { - Some(page_info) => Value::Object(IndexMap::from([ - (Name::new("has_previous_page"), Value::from(page_info.has_previous_page)), - (Name::new("has_next_page"), Value::from(page_info.has_next_page)), - ( - Name::new("start_cursor"), - match page_info.start_cursor { - Some(val) => Value::from(val), - None => Value::Null, - }, - ), - ( - Name::new("end_cursor"), - match page_info.end_cursor { - Some(val) => Value::from(val), - None => Value::Null, - }, - ), - ])), - None => Value::Null, - } + pub fn value(page_info: PageInfo) -> Value { + Value::Object(IndexMap::from([ + (Name::new("has_previous_page"), Value::from(page_info.has_previous_page)), + (Name::new("has_next_page"), Value::from(page_info.has_next_page)), + ( + Name::new("start_cursor"), + match page_info.start_cursor { + Some(val) => Value::from(val), + None => Value::Null, + }, + ), + ( + Name::new("end_cursor"), + match page_info.end_cursor { + Some(val) => Value::from(val), + None => Value::Null, + }, + ), + ])) } } diff --git a/crates/torii/graphql/src/object/entity.rs b/crates/torii/graphql/src/object/entity.rs index 884efffb8e..4d9751573e 100644 --- a/crates/torii/graphql/src/object/entity.rs +++ b/crates/torii/graphql/src/object/entity.rs @@ -60,6 +60,7 @@ impl ObjectTrait for EntityObject { &None, &None, &connection, + total_count, ) .await?; let results = connection_output( diff --git a/crates/torii/graphql/src/object/event.rs b/crates/torii/graphql/src/object/event.rs index 11d7c1a3e6..31b660c2b9 100644 --- a/crates/torii/graphql/src/object/event.rs +++ b/crates/torii/graphql/src/object/event.rs @@ -57,6 +57,7 @@ impl ObjectTrait for EventObject { &None, &None, &connection, + total_count, ) .await?; let results = connection_output( diff --git a/crates/torii/graphql/src/object/metadata/mod.rs b/crates/torii/graphql/src/object/metadata/mod.rs index 3614227567..4a00b2dd43 100644 --- a/crates/torii/graphql/src/object/metadata/mod.rs +++ b/crates/torii/graphql/src/object/metadata/mod.rs @@ -1,8 +1,10 @@ +use async_graphql::connection::PageInfo; use async_graphql::dynamic::{Field, FieldFuture, TypeRef}; use async_graphql::{Name, Value}; use sqlx::sqlite::SqliteRow; use sqlx::{Pool, Row, Sqlite}; +use super::connection::page_info::PageInfoObject; use super::connection::{connection_arguments, cursor, parse_connection_arguments}; use super::ObjectTrait; use crate::constants::{ @@ -54,7 +56,7 @@ impl ObjectTrait for MetadataObject { let mut conn = ctx.data::>()?.acquire().await?; let connection = parse_connection_arguments(&ctx)?; let total_count = count_rows(&mut conn, &table_name, &None, &None).await?; - let (data, _page_info) = fetch_multiple_rows( + let (data, page_info) = fetch_multiple_rows( &mut conn, &table_name, ID_COLUMN, @@ -62,11 +64,13 @@ impl ObjectTrait for MetadataObject { &None, &None, &connection, + total_count, ) .await?; // convert json field to value_mapping expected by content object - let results = metadata_connection_output(&data, &type_mapping, total_count)?; + let results = + metadata_connection_output(&data, &type_mapping, total_count, page_info)?; Ok(Some(Value::Object(results))) }) @@ -85,6 +89,7 @@ fn metadata_connection_output( data: &[SqliteRow], types: &TypeMapping, total_count: i64, + page_info: PageInfo, ) -> sqlx::Result { let edges = data .iter() @@ -107,9 +112,10 @@ fn metadata_connection_output( value_mapping.insert(Name::new("content"), Value::Object(content)); - let mut edge = ValueMapping::new(); - edge.insert(Name::new("node"), Value::Object(value_mapping)); - edge.insert(Name::new("cursor"), Value::String(cursor)); + let edge = ValueMapping::from([ + (Name::new("node"), Value::Object(value_mapping)), + (Name::new("cursor"), Value::String(cursor)), + ]); Ok(Value::Object(edge)) }) @@ -118,6 +124,7 @@ fn metadata_connection_output( Ok(ValueMapping::from([ (Name::new("total_count"), Value::from(total_count)), (Name::new("edges"), Value::List(edges?)), + (Name::new("page_info"), PageInfoObject::value(page_info)), ])) } diff --git a/crates/torii/graphql/src/object/mod.rs b/crates/torii/graphql/src/object/mod.rs index 9884d3070f..494449654f 100644 --- a/crates/torii/graphql/src/object/mod.rs +++ b/crates/torii/graphql/src/object/mod.rs @@ -104,6 +104,7 @@ pub trait ObjectTrait: Send + Sync { &None, &None, &connection, + total_count, ) .await?; let results = connection_output( diff --git a/crates/torii/graphql/src/object/model_data.rs b/crates/torii/graphql/src/object/model_data.rs index f04f0e3f49..aadd7ec784 100644 --- a/crates/torii/graphql/src/object/model_data.rs +++ b/crates/torii/graphql/src/object/model_data.rs @@ -99,6 +99,7 @@ impl ObjectTrait for ModelDataObject { &order, &filters, &connection, + total_count, ) .await?; let connection = connection_output( diff --git a/crates/torii/graphql/src/query/data.rs b/crates/torii/graphql/src/query/data.rs index 6228b6e038..de04e12e49 100644 --- a/crates/torii/graphql/src/query/data.rs +++ b/crates/torii/graphql/src/query/data.rs @@ -35,6 +35,7 @@ pub async fn fetch_single_row( sqlx::query(&query).fetch_one(conn).await } +#[allow(clippy::too_many_arguments)] pub async fn fetch_multiple_rows( conn: &mut PoolConnection, table_name: &str, @@ -43,7 +44,8 @@ pub async fn fetch_multiple_rows( order: &Option, filters: &Option>, connection: &ConnectionArguments, -) -> Result<(Vec, Option)> { + total_count: i64, +) -> Result<(Vec, PageInfo)> { let mut conditions = build_conditions(keys, filters); let mut cursor_param = &connection.after; @@ -104,65 +106,70 @@ pub async fn fetch_multiple_rows( } let mut data = sqlx::query(&query).fetch_all(conn).await?; + let mut page_info = PageInfo { + has_previous_page: false, + has_next_page: false, + start_cursor: None, + end_cursor: None, + }; - if !is_cursor_based { - Ok((data, None)) - } else { - let mut page_info = PageInfo { - has_previous_page: false, - has_next_page: false, - start_cursor: None, - end_cursor: None, + if data.is_empty() { + Ok((data, page_info)) + } else if is_cursor_based { + let order_field = match order { + Some(order) => format!("external_{}", order.field), + None => id_column.to_string(), }; - if data.is_empty() { - Ok((data, Some(page_info))) - } else { - let order_field = match order { - Some(order) => format!("external_{}", order.field), - None => id_column.to_string(), - }; + match cursor_param { + Some(cursor_query) => { + let first_cursor = cursor::encode( + &data[0].try_get::(id_column)?, + &data[0].try_get_unchecked::(&order_field)?, + ); - match cursor_param { - Some(cursor_query) => { - let first_cursor = cursor::encode( - &data[0].try_get::(id_column)?, - &data[0].try_get_unchecked::(&order_field)?, - ); - - if &first_cursor == cursor_query && data.len() != 1 { - data.remove(0); - page_info.has_previous_page = true; - } else { - data.pop(); - } - - if data.len() as u64 == limit - 1 { - page_info.has_next_page = true; - data.pop(); - } + if &first_cursor == cursor_query && data.len() != 1 { + data.remove(0); + page_info.has_previous_page = true; + } else { + data.pop(); } - None => { - if data.len() as u64 == limit { - page_info.has_next_page = true; - data.pop(); - } + + if data.len() as u64 == limit - 1 { + page_info.has_next_page = true; + data.pop(); } } - - if !data.is_empty() { - page_info.start_cursor = Some(cursor::encode( - &data[0].try_get::(id_column)?, - &data[0].try_get_unchecked::(&order_field)?, - )); - page_info.end_cursor = Some(cursor::encode( - &data[data.len() - 1].try_get::(id_column)?, - &data[data.len() - 1].try_get_unchecked::(&order_field)?, - )); + None => { + if data.len() as u64 == limit { + page_info.has_next_page = true; + data.pop(); + } } + } - Ok((data, Some(page_info))) + if !data.is_empty() { + page_info.start_cursor = Some(cursor::encode( + &data[0].try_get::(id_column)?, + &data[0].try_get_unchecked::(&order_field)?, + )); + page_info.end_cursor = Some(cursor::encode( + &data[data.len() - 1].try_get::(id_column)?, + &data[data.len() - 1].try_get_unchecked::(&order_field)?, + )); } + + Ok((data, page_info)) + } else { + let offset = connection.offset.unwrap_or(0); + if 1 < offset && offset < total_count as u64 { + page_info.has_previous_page = true; + } + if limit + offset < total_count as u64 { + page_info.has_next_page = true; + } + + Ok((data, page_info)) } } diff --git a/crates/torii/graphql/src/tests/entities_test.rs b/crates/torii/graphql/src/tests/entities_test.rs index 1e3d1138a5..cf2850268e 100644 --- a/crates/torii/graphql/src/tests/entities_test.rs +++ b/crates/torii/graphql/src/tests/entities_test.rs @@ -137,11 +137,10 @@ mod tests { assert_eq!(connection.edges.first().unwrap(), three); assert_eq!(connection.edges.last().unwrap(), four); - let page_info = connection.page_info.take().unwrap(); - assert_eq!(page_info.has_previous_page, true); - assert_eq!(page_info.has_next_page, true); - assert_eq!(page_info.start_cursor.unwrap(), three.cursor); - assert_eq!(page_info.end_cursor.unwrap(), four.cursor); + assert!(connection.page_info.has_previous_page); + assert!(connection.page_info.has_next_page); + assert_eq!(connection.page_info.start_cursor.unwrap(), three.cursor); + assert_eq!(connection.page_info.end_cursor.unwrap(), four.cursor); let entities = entities_query(&schema, &format!("(first: 3, after: \"{}\")", three.cursor)).await; @@ -150,11 +149,10 @@ mod tests { assert_eq!(connection.edges.first().unwrap(), four); assert_eq!(connection.edges.last().unwrap(), six); - let page_info = connection.page_info.take().unwrap(); - assert_eq!(page_info.has_previous_page, true); - assert_eq!(page_info.has_next_page, true); - assert_eq!(page_info.start_cursor.unwrap(), four.cursor); - assert_eq!(page_info.end_cursor.unwrap(), six.cursor); + assert!(connection.page_info.has_previous_page); + assert!(connection.page_info.has_next_page); + assert_eq!(connection.page_info.start_cursor.unwrap(), four.cursor); + assert_eq!(connection.page_info.end_cursor.unwrap(), six.cursor); // cursor based backward pagination let entities = @@ -164,11 +162,10 @@ mod tests { assert_eq!(connection.edges.first().unwrap(), six); assert_eq!(connection.edges.last().unwrap(), five); - let page_info = connection.page_info.take().unwrap(); - assert_eq!(page_info.has_previous_page, true); - assert_eq!(page_info.has_next_page, true); - assert_eq!(page_info.start_cursor.unwrap(), six.cursor); - assert_eq!(page_info.end_cursor.unwrap(), five.cursor); + assert!(connection.page_info.has_previous_page); + assert!(connection.page_info.has_next_page); + assert_eq!(connection.page_info.start_cursor.unwrap(), six.cursor); + assert_eq!(connection.page_info.end_cursor.unwrap(), five.cursor); let entities = entities_query(&schema, &format!("(last: 3, before: \"{}\")", six.cursor)).await; @@ -177,11 +174,10 @@ mod tests { assert_eq!(connection.edges.first().unwrap(), five); assert_eq!(connection.edges.last().unwrap(), three); - let page_info = connection.page_info.take().unwrap(); - assert_eq!(page_info.has_previous_page, true); - assert_eq!(page_info.has_next_page, true); - assert_eq!(page_info.start_cursor.unwrap(), five.cursor); - assert_eq!(page_info.end_cursor.unwrap(), three.cursor); + assert!(connection.page_info.has_previous_page); + assert!(connection.page_info.has_next_page); + assert_eq!(connection.page_info.start_cursor.unwrap(), five.cursor); + assert_eq!(connection.page_info.end_cursor.unwrap(), three.cursor); let empty_entities = entities_query( &schema, @@ -194,11 +190,10 @@ mod tests { let connection: Connection = serde_json::from_value(empty_entities).unwrap(); assert_eq!(connection.edges.len(), 0); - let page_info = connection.page_info.take().unwrap(); - assert_eq!(page_info.has_previous_page, false); - assert_eq!(page_info.has_next_page, false); - assert_eq!(page_info.start_cursor, None); - assert_eq!(page_info.end_cursor, None); + assert!(!connection.page_info.has_previous_page); + assert!(!connection.page_info.has_next_page); + assert_eq!(connection.page_info.start_cursor, None); + assert_eq!(connection.page_info.end_cursor, None); // offset/limit based pagination let entities = entities_query(&schema, "(limit: 2)").await; @@ -206,19 +201,31 @@ mod tests { assert_eq!(connection.edges.len(), 2); assert_eq!(connection.edges.first().unwrap(), one); assert_eq!(connection.edges.last().unwrap(), two); - assert!(connection.page_info.is_null()); + + assert!(!connection.page_info.has_previous_page); + assert!(connection.page_info.has_next_page); + assert_eq!(connection.page_info.start_cursor, None); + assert_eq!(connection.page_info.end_cursor, None); let entities = entities_query(&schema, "(limit: 3, offset: 2)").await; let connection: Connection = serde_json::from_value(entities).unwrap(); assert_eq!(connection.edges.len(), 3); assert_eq!(connection.edges.first().unwrap(), three); assert_eq!(connection.edges.last().unwrap(), five); - assert!(connection.page_info.is_null()); + + assert!(connection.page_info.has_previous_page); + assert!(connection.page_info.has_next_page); + assert_eq!(connection.page_info.start_cursor, None); + assert_eq!(connection.page_info.end_cursor, None); let empty_entities = entities_query(&schema, "(limit: 1, offset: 20)").await; let connection: Connection = serde_json::from_value(empty_entities).unwrap(); assert_eq!(connection.edges.len(), 0); - assert!(connection.page_info.is_null()); + + assert!(!connection.page_info.has_previous_page); + assert!(!connection.page_info.has_next_page); + assert_eq!(connection.page_info.start_cursor, None); + assert_eq!(connection.page_info.end_cursor, None); // entity model union let id = poseidon_hash_many(&[FieldElement::ZERO]); diff --git a/crates/torii/graphql/src/tests/metadata_test.rs b/crates/torii/graphql/src/tests/metadata_test.rs index 63285035b9..d6490421ff 100644 --- a/crates/torii/graphql/src/tests/metadata_test.rs +++ b/crates/torii/graphql/src/tests/metadata_test.rs @@ -33,6 +33,12 @@ mod tests { } } } + page_info { + has_previous_page + has_next_page + start_cursor + end_cursor + } } } "#; diff --git a/crates/torii/graphql/src/tests/mod.rs b/crates/torii/graphql/src/tests/mod.rs index 0175a4080f..83c7a3839c 100644 --- a/crates/torii/graphql/src/tests/mod.rs +++ b/crates/torii/graphql/src/tests/mod.rs @@ -2,7 +2,6 @@ use std::str::FromStr; use anyhow::Result; use async_graphql::dynamic::Schema; -use async_graphql::MaybeUndefined; use dojo_test_utils::compiler::build_test_config; use dojo_test_utils::migration::prepare_migration; use dojo_test_utils::sequencer::{ @@ -41,7 +40,7 @@ use crate::schema::build_schema; pub struct Connection { pub total_count: i64, pub edges: Vec>, - pub page_info: MaybeUndefined, + pub page_info: PageInfo, } #[derive(Deserialize, Debug, PartialEq)] diff --git a/crates/torii/graphql/src/tests/models_test.rs b/crates/torii/graphql/src/tests/models_test.rs index 0c6e3ca181..a27b97e314 100644 --- a/crates/torii/graphql/src/tests/models_test.rs +++ b/crates/torii/graphql/src/tests/models_test.rs @@ -58,6 +58,12 @@ mod tests { }} }} }} + page_info {{ + has_previous_page + has_next_page + start_cursor + end_cursor + }} }} }} "#,