From dc46e9c689b2571a51200964ffc927b883a081eb Mon Sep 17 00:00:00 2001 From: Larko <59736843+Larkooo@users.noreply.github.com> Date: Fri, 9 Aug 2024 10:10:17 -0400 Subject: [PATCH] refactor(torii-grpc): correct pagination logic for grpc query member (#2256) * refactor(torii-grpc): correct pagination logic for grpc query member * fix: sql uqery * fix: order by * refactor: order by event id * fmt * fix: tests --- crates/torii/core/src/model.rs | 35 +++++++++++++++---- crates/torii/grpc/src/server/mod.rs | 28 +++++++++------ .../grpc/src/server/subscriptions/entity.rs | 4 ++- .../src/server/subscriptions/event_message.rs | 4 ++- 4 files changed, 52 insertions(+), 19 deletions(-) diff --git a/crates/torii/core/src/model.rs b/crates/torii/core/src/model.rs index d991d86fab..b753685142 100644 --- a/crates/torii/core/src/model.rs +++ b/crates/torii/core/src/model.rs @@ -231,7 +231,9 @@ pub fn build_sql_query( entity_relation_column: &str, where_clause: Option<&str>, where_clause_arrays: Option<&str>, -) -> Result<(String, HashMap), Error> { + limit: Option, + offset: Option, +) -> Result<(String, HashMap, String), Error> { fn parse_ty( path: &str, name: &str, @@ -398,9 +400,21 @@ pub fn build_sql_query( "SELECT {entities_table}.id, {entities_table}.keys, {selections_clause} FROM \ {entities_table}{join_clause}" ); + let mut count_query = + format!("SELECT COUNT({entities_table}.id) FROM {entities_table}{join_clause}",); if let Some(where_clause) = where_clause { - query = format!("{} WHERE {}", query, where_clause); + query += &format!(" WHERE {}", where_clause); + count_query += &format!(" WHERE {}", where_clause); + } + query += &format!(" ORDER BY {entities_table}.event_id DESC"); + + if let Some(limit) = limit { + query += &format!(" LIMIT {}", limit); + } + + if let Some(offset) = offset { + query += &format!(" OFFSET {}", offset); } if let Some(where_clause_arrays) = where_clause_arrays { @@ -409,7 +423,7 @@ pub fn build_sql_query( } } - Ok((query, formatted_arrays_queries)) + Ok((query, formatted_arrays_queries, count_query)) } /// Populate the values of a Ty (schema) from SQLite row. @@ -992,9 +1006,16 @@ mod tests { ], }); - let query = - build_sql_query(&vec![position, player_config], "entities", "entity_id", None, None) - .unwrap(); + let query = build_sql_query( + &vec![position, player_config], + "entities", + "entity_id", + None, + None, + None, + None, + ) + .unwrap(); let expected_query = "SELECT entities.id, entities.keys, [Test-Position].external_player AS \ @@ -1006,7 +1027,7 @@ mod tests { entities.id = [Test-Position$vec].entity_id JOIN [Test-Position] ON entities.id = \ [Test-Position].entity_id JOIN [Test-PlayerConfig$favorite_item] ON entities.id = \ [Test-PlayerConfig$favorite_item].entity_id JOIN [Test-PlayerConfig] ON entities.id \ - = [Test-PlayerConfig].entity_id"; + = [Test-PlayerConfig].entity_id ORDER BY entities.event_id DESC"; // todo: completely tests arrays assert_eq!(query.0, expected_query); } diff --git a/crates/torii/grpc/src/server/mod.rs b/crates/torii/grpc/src/server/mod.rs index e00b3347e2..14c30f3f30 100644 --- a/crates/torii/grpc/src/server/mod.rs +++ b/crates/torii/grpc/src/server/mod.rs @@ -294,12 +294,14 @@ impl DojoWorld { .map_err(ParseError::FromStr)?; let schemas = self.model_cache.schemas(&model_ids).await?; - let (entity_query, arrays_queries) = build_sql_query( + let (entity_query, arrays_queries, _) = build_sql_query( &schemas, table, entity_relation_column, Some(&format!("{table}.id = ?")), Some(&format!("{table}.id = ?")), + None, + None, )?; let row = @@ -433,12 +435,14 @@ impl DojoWorld { .map_err(ParseError::FromStr)?; let schemas = self.model_cache.schemas(&model_ids).await?; - let (entity_query, arrays_queries) = build_sql_query( + let (entity_query, arrays_queries, _) = build_sql_query( &schemas, table, entity_relation_column, Some(&format!("{table}.id = ?")), Some(&format!("{table}.id = ?")), + None, + None, )?; let row = sqlx::query(&entity_query).bind(entity_id).fetch_one(&self.pool).await?; @@ -525,17 +529,21 @@ impl DojoWorld { let table_name = member_clause.model; let column_name = format!("external_{}", member_clause.member); - let (entity_query, arrays_queries) = build_sql_query( + let (entity_query, arrays_queries, count_query) = build_sql_query( &schemas, table, entity_relation_column, - Some(&format!( - "[{table_name}].{column_name} {comparison_operator} ? ORDER BY {table}.event_id \ - DESC LIMIT ? OFFSET ?" - )), + Some(&format!("[{table_name}].{column_name} {comparison_operator} ?")), None, + limit, + offset, )?; + let total_count = sqlx::query_scalar(&count_query) + .bind(comparison_value.clone()) + .fetch_one(&self.pool) + .await?; + let db_entities = sqlx::query(&entity_query) .bind(comparison_value.clone()) .bind(limit) @@ -553,8 +561,6 @@ impl DojoWorld { .iter() .map(|row| map_row_to_entity(row, &arrays_rows, schemas.clone())) .collect::, Error>>()?; - // Since there is not limit and offset, total_count is same as number of entities - let total_count = entities_collection.len() as u32; Ok((entities_collection, total_count)) } @@ -698,12 +704,14 @@ impl DojoWorld { .map_err(ParseError::FromStr)?; let schemas = self.model_cache.schemas(&model_ids).await?; - let (entity_query, arrays_queries) = build_sql_query( + let (entity_query, arrays_queries, _) = build_sql_query( &schemas, table, entity_relation_column, Some(&format!("[{table}].id = ?")), Some(&format!("[{table}].id = ?")), + None, + None, )?; let row = sqlx::query(&entity_query).bind(entity_id).fetch_one(&self.pool).await?; diff --git a/crates/torii/grpc/src/server/subscriptions/entity.rs b/crates/torii/grpc/src/server/subscriptions/entity.rs index ee6a44275d..bdcf5a15b8 100644 --- a/crates/torii/grpc/src/server/subscriptions/entity.rs +++ b/crates/torii/grpc/src/server/subscriptions/entity.rs @@ -220,12 +220,14 @@ impl Service { .map_err(ParseError::FromStr)?; let schemas = cache.schemas(&model_ids).await?; - let (entity_query, arrays_queries) = build_sql_query( + let (entity_query, arrays_queries, _) = build_sql_query( &schemas, "entities", "entity_id", Some("entities.id = ?"), Some("entities.id = ?"), + None, + None, )?; let row = sqlx::query(&entity_query).bind(&entity.id).fetch_one(&pool).await?; diff --git a/crates/torii/grpc/src/server/subscriptions/event_message.rs b/crates/torii/grpc/src/server/subscriptions/event_message.rs index 47792418dd..71a708b49d 100644 --- a/crates/torii/grpc/src/server/subscriptions/event_message.rs +++ b/crates/torii/grpc/src/server/subscriptions/event_message.rs @@ -199,12 +199,14 @@ impl Service { .map_err(ParseError::FromStr)?; let schemas = cache.schemas(&model_ids).await?; - let (entity_query, arrays_queries) = build_sql_query( + let (entity_query, arrays_queries, _) = build_sql_query( &schemas, "event_messages", "event_message_id", Some("event_messages.id = ?"), Some("event_messages.id = ?"), + None, + None, )?; let row = sqlx::query(&entity_query).bind(&entity.id).fetch_one(&pool).await?;