Update GraphQL cursor to use OpaqueCursor

Close #355 The following sources are the main areas that have been modified. graphql.rs graphql directory sub-source The rest of the changes are fixes for Clippy errors caused by the Rust version update.
aicers · Dec 5, 2024 · 238fb77 · 238fb77
1 parent dd41e12
commit 238fb77
Show file tree

Hide file tree

Showing 26 changed files with 250 additions and 179 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,12 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+### Changed
+
+- Updated the encoding and decoding method for GraphQL cursors by removing
+  `graphql::encode_cursor` and `graphql::decode_cursor` methods and replacing
+  them with the encoding and decoding methods of `OpaqueCursor`.
+
 ### Fixed
 
 - Resolved an issue in the `applyNode` GraphQL API, where configuration values

diff --git a/src/archive.rs b/src/archive.rs
@@ -48,7 +48,7 @@ impl Config {
 
     pub(crate) fn configure_reverse_proxies(
         store: &Arc<RwLock<Store>>,
-        client: &Option<reqwest::Client>,
+        client: Option<&reqwest::Client>,
         reverse_proxies: &[Self],
     ) -> Vec<(ArchiveState, Router<ArchiveState>)> {
         reverse_proxies
@@ -57,10 +57,10 @@ impl Config {
                 (
                     crate::archive::ArchiveState {
                         store: store.clone(),
-                        client: client.clone(),
+                        client: client.cloned(),
                         config: rp.clone(),
                     },
-                    crate::archive::reverse_proxy(store.clone(), client.clone(), rp.clone()),
+                    crate::archive::reverse_proxy(store.clone(), client.cloned(), rp.clone()),
                 )
             })
             .collect()

diff --git a/src/graphql.rs b/src/graphql.rs
@@ -40,13 +40,12 @@ use std::future::Future;
 use std::net::SocketAddr;
 use std::sync::{Arc, Mutex};
 
-use async_graphql::connection::{ConnectionNameType, CursorType, EdgeNameType};
+use async_graphql::connection::{ConnectionNameType, CursorType, EdgeNameType, OpaqueCursor};
 use async_graphql::{
     connection::{Connection, Edge, EmptyFields},
     Context, Guard, MergedObject, MergedSubscription, ObjectType, OutputType, Result,
 };
 use chrono::TimeDelta;
-use data_encoding::BASE64;
 use num_traits::ToPrimitive;
 #[cfg(test)]
 use review_database::HostNetworkGroup;
@@ -236,13 +235,23 @@ async fn query_with_constraints<Node, ConnectionFields, Name, F, R, E>(
     first: Option<i32>,
     last: Option<i32>,
     f: F,
-) -> Result<Connection<String, Node, ConnectionFields, EmptyFields, Name>>
+) -> Result<Connection<OpaqueCursor<Vec<u8>>, Node, ConnectionFields, EmptyFields, Name>>
 where
     Node: OutputType,
     ConnectionFields: ObjectType,
     Name: ConnectionNameType,
-    F: FnOnce(Option<String>, Option<String>, Option<usize>, Option<usize>) -> R,
-    R: Future<Output = Result<Connection<String, Node, ConnectionFields, EmptyFields, Name>, E>>,
+    F: FnOnce(
+        Option<OpaqueCursor<Vec<u8>>>,
+        Option<OpaqueCursor<Vec<u8>>>,
+        Option<usize>,
+        Option<usize>,
+    ) -> R,
+    R: Future<
+        Output = Result<
+            Connection<OpaqueCursor<Vec<u8>>, Node, ConnectionFields, EmptyFields, Name>,
+            E,
+        >,
+    >,
     E: Into<async_graphql::Error>,
 {
     extra_validate_pagination_params(
@@ -322,49 +331,27 @@ async fn get_store<'a>(ctx: &Context<'a>) -> Result<tokio::sync::RwLockReadGuard
     Ok(ctx.data::<Arc<RwLock<Store>>>()?.read().await)
 }
 
-/// Decodes a cursor used in pagination.
-fn decode_cursor(cursor: &str) -> Option<Vec<u8>> {
-    BASE64.decode(cursor.as_bytes()).ok()
-}
-
-/// Encodes a cursor used in pagination.
-fn encode_cursor(cursor: &[u8]) -> String {
-    BASE64.encode(cursor)
-}
-
-#[allow(clippy::type_complexity)]
-fn decode_cursor_pair(
-    after: Option<String>,
-    before: Option<String>,
-) -> Result<(Option<Vec<u8>>, Option<Vec<u8>>)> {
-    let after = if let Some(after) = after {
-        Some(decode_cursor(&after).ok_or("invalid cursor `after`")?)
-    } else {
-        None
-    };
-    let before = if let Some(before) = before {
-        Some(decode_cursor(&before).ok_or("invalid cursor `before`")?)
-    } else {
-        None
-    };
-    Ok((after, before))
-}
-
 #[allow(clippy::type_complexity)]
 fn process_load_edges<'a, T, I, R>(
     table: &'a T,
-    after: Option<String>,
-    before: Option<String>,
+    after: Option<OpaqueCursor<Vec<u8>>>,
+    before: Option<OpaqueCursor<Vec<u8>>>,
     first: Option<usize>,
     last: Option<usize>,
     prefix: Option<&[u8]>,
-) -> Result<(Vec<Result<R, anyhow::Error>>, bool, bool)>
+) -> (
+    std::vec::Vec<std::result::Result<R, anyhow::Error>>,
+    bool,
+    bool,
+)
 where
     T: database::Iterable<'a, I>,
     I: std::iter::Iterator<Item = anyhow::Result<R>>,
     R: database::UniqueKey,
 {
-    let (after, before) = decode_cursor_pair(after, before)?;
+    let after = after.map(|cursor| cursor.0);
+    let before = before.map(|cursor| cursor.0);
+
     let (nodes, has_previous, has_next) = if let Some(first) = first {
         let (nodes, has_more) =
             collect_edges(table, Direction::Forward, after, before, prefix, first);
@@ -377,13 +364,13 @@ where
         (nodes, has_more, false)
     };
 
-    Ok((nodes, has_previous, has_next))
+    (nodes, has_previous, has_next)
 }
 
 fn load_edges_interim<'a, T, I, R>(
     table: &'a T,
-    after: Option<String>,
-    before: Option<String>,
+    after: Option<OpaqueCursor<Vec<u8>>>,
+    before: Option<OpaqueCursor<Vec<u8>>>,
     first: Option<usize>,
     last: Option<usize>,
     prefix: Option<&[u8]>,
@@ -394,7 +381,7 @@ where
     R: database::UniqueKey,
 {
     let (nodes, has_previous, has_next) =
-        process_load_edges(table, after, before, first, last, prefix)?;
+        process_load_edges(table, after, before, first, last, prefix);
 
     let nodes = nodes
         .into_iter()
@@ -403,14 +390,15 @@ where
     Ok((nodes, has_previous, has_next))
 }
 
+#[allow(clippy::type_complexity)]
 fn load_edges<'a, T, I, R, N, A, NodesField>(
     table: &'a T,
-    after: Option<String>,
-    before: Option<String>,
+    after: Option<OpaqueCursor<Vec<u8>>>,
+    before: Option<OpaqueCursor<Vec<u8>>>,
     first: Option<usize>,
     last: Option<usize>,
     additional_fields: A,
-) -> Result<Connection<String, N, A, EmptyFields, NodesField>>
+) -> Result<Connection<OpaqueCursor<Vec<u8>>, N, A, EmptyFields, NodesField>>
 where
     T: database::Iterable<'a, I>,
     I: std::iter::Iterator<Item = anyhow::Result<R>>,
@@ -420,7 +408,7 @@ where
     NodesField: ConnectionNameType,
 {
     let (nodes, has_previous, has_next) =
-        process_load_edges(table, after, before, first, last, None)?;
+        process_load_edges(table, after, before, first, last, None);
 
     for node in &nodes {
         let Err(e) = node else { continue };
@@ -432,8 +420,8 @@ where
         Connection::with_additional_fields(has_previous, has_next, additional_fields);
     connection.edges.extend(nodes.into_iter().map(|node| {
         let Ok(node) = node else { unreachable!() };
-        let encoded = encode_cursor(node.unique_key().as_ref());
-        Edge::new(encoded, node.into())
+        let key = node.unique_key().as_ref().to_vec();
+        Edge::new(OpaqueCursor(key), node.into())
     }));
     Ok(connection)
 }

diff --git a/src/graphql/account.rs b/src/graphql/account.rs
@@ -4,6 +4,7 @@ use std::{
 };
 
 use anyhow::anyhow;
+use async_graphql::connection::OpaqueCursor;
 use async_graphql::{
     connection::{Connection, EmptyFields},
     Context, Enum, InputObject, Object, Result, SimpleObject, StringNumber,
@@ -58,7 +59,7 @@ impl AccountQuery {
         before: Option<String>,
         first: Option<i32>,
         last: Option<i32>,
-    ) -> Result<Connection<String, Account, AccountTotalCount, EmptyFields>> {
+    ) -> Result<Connection<OpaqueCursor<Vec<u8>>, Account, AccountTotalCount, EmptyFields>> {
         query_with_constraints(
             after,
             before,
@@ -325,10 +326,16 @@ impl AccountMutation {
         if let Some(mut account) = account_map.get(&username)? {
             validate_password(&account, &username, &password)?;
             validate_last_signin_time(&account, &username)?;
-            validate_allow_access_from(&account, &client_ip, &username)?;
+            validate_allow_access_from(&account, client_ip.as_ref(), &username)?;
             validate_max_parallel_sessions(&account, &store, &username)?;
 
-            sign_in_actions(&mut account, &store, &account_map, &client_ip, &username)
+            sign_in_actions(
+                &mut account,
+                &store,
+                &account_map,
+                client_ip.as_ref(),
+                &username,
+            )
         } else {
             info!("{username} is not a valid username");
             Err("incorrect username or password".into())
@@ -355,13 +362,19 @@ impl AccountMutation {
 
         if let Some(mut account) = account_map.get(&username)? {
             validate_password(&account, &username, &password)?;
-            validate_allow_access_from(&account, &client_ip, &username)?;
+            validate_allow_access_from(&account, client_ip.as_ref(), &username)?;
             validate_max_parallel_sessions(&account, &store, &username)?;
             validate_update_new_password(&password, &new_password, &username)?;
 
             account.update_password(&new_password)?;
 
-            sign_in_actions(&mut account, &store, &account_map, &client_ip, &username)
+            sign_in_actions(
+                &mut account,
+                &store,
+                &account_map,
+                client_ip.as_ref(),
+                &username,
+            )
         } else {
             info!("{username} is not a valid username");
             Err("incorrect username or password".into())
@@ -474,7 +487,7 @@ fn validate_last_signin_time(account: &types::Account, username: &str) -> Result
 
 fn validate_allow_access_from(
     account: &types::Account,
-    client_ip: &Option<SocketAddr>,
+    client_ip: Option<&SocketAddr>,
     username: &str,
 ) -> Result<()> {
     if let Some(allow_access_from) = account.allow_access_from.as_ref() {
@@ -533,7 +546,7 @@ fn sign_in_actions(
     account: &mut types::Account,
     store: &Store,
     account_map: &Table<types::Account>,
-    client_ip: &Option<SocketAddr>,
+    client_ip: Option<&SocketAddr>,
     username: &str,
 ) -> Result<AuthPayload> {
     let (token, expiration_time) =
@@ -720,11 +733,11 @@ impl AccountTotalCount {
 
 async fn load(
     ctx: &Context<'_>,
-    after: Option<String>,
-    before: Option<String>,
+    after: Option<OpaqueCursor<Vec<u8>>>,
+    before: Option<OpaqueCursor<Vec<u8>>>,
     first: Option<usize>,
     last: Option<usize>,
-) -> Result<Connection<String, Account, AccountTotalCount, EmptyFields>> {
+) -> Result<Connection<OpaqueCursor<Vec<u8>>, Account, AccountTotalCount, EmptyFields>> {
     let store = crate::graphql::get_store(ctx).await?;
     let table = store.account_map();
     super::load_edges(&table, after, before, first, last, AccountTotalCount)

diff --git a/src/graphql/allow_network.rs b/src/graphql/allow_network.rs
@@ -1,3 +1,4 @@
+use async_graphql::connection::OpaqueCursor;
 use async_graphql::{
     connection::{Connection, EmptyFields},
     Context, InputObject, Object, Result, ID,
@@ -29,7 +30,8 @@ impl AllowNetworkQuery {
         before: Option<String>,
         first: Option<i32>,
         last: Option<i32>,
-    ) -> Result<Connection<String, AllowNetwork, AllowNetworkTotalCount, EmptyFields>> {
+    ) -> Result<Connection<OpaqueCursor<Vec<u8>>, AllowNetwork, AllowNetworkTotalCount, EmptyFields>>
+    {
         query_with_constraints(
             after,
             before,
@@ -197,11 +199,11 @@ impl AllowNetworkTotalCount {
 
 async fn load(
     ctx: &Context<'_>,
-    after: Option<String>,
-    before: Option<String>,
+    after: Option<OpaqueCursor<Vec<u8>>>,
+    before: Option<OpaqueCursor<Vec<u8>>>,
     first: Option<usize>,
     last: Option<usize>,
-) -> Result<Connection<String, AllowNetwork, AllowNetworkTotalCount, EmptyFields>> {
+) -> Result<Connection<OpaqueCursor<Vec<u8>>, AllowNetwork, AllowNetworkTotalCount, EmptyFields>> {
     let store = crate::graphql::get_store(ctx).await?;
     let map = store.allow_network_map();
     super::load_edges(&map, after, before, first, last, AllowNetworkTotalCount)

diff --git a/src/graphql/block_network.rs b/src/graphql/block_network.rs
@@ -1,5 +1,6 @@
 use std::sync::Arc;
 
+use async_graphql::connection::OpaqueCursor;
 use async_graphql::{
     connection::{Connection, EmptyFields},
     Context, InputObject, Object, Result, ID,
@@ -32,7 +33,8 @@ impl BlockNetworkQuery {
         before: Option<String>,
         first: Option<i32>,
         last: Option<i32>,
-    ) -> Result<Connection<String, BlockNetwork, BlockNetworkTotalCount, EmptyFields>> {
+    ) -> Result<Connection<OpaqueCursor<Vec<u8>>, BlockNetwork, BlockNetworkTotalCount, EmptyFields>>
+    {
         query_with_constraints(
             after,
             before,
@@ -198,11 +200,11 @@ impl BlockNetworkTotalCount {
 
 async fn load(
     ctx: &Context<'_>,
-    after: Option<String>,
-    before: Option<String>,
+    after: Option<OpaqueCursor<Vec<u8>>>,
+    before: Option<OpaqueCursor<Vec<u8>>>,
     first: Option<usize>,
     last: Option<usize>,
-) -> Result<Connection<String, BlockNetwork, BlockNetworkTotalCount, EmptyFields>> {
+) -> Result<Connection<OpaqueCursor<Vec<u8>>, BlockNetwork, BlockNetworkTotalCount, EmptyFields>> {
     let db = super::get_store(ctx).await?;
     let map = db.block_network_map();
     super::load_edges(&map, after, before, first, last, BlockNetworkTotalCount)

diff --git a/src/graphql/category.rs b/src/graphql/category.rs
@@ -1,5 +1,6 @@
 use std::sync::Arc;
 
+use async_graphql::connection::OpaqueCursor;
 use async_graphql::{
     connection::{Connection, EmptyFields},
     types::ID,
@@ -29,7 +30,7 @@ impl CategoryQuery {
         before: Option<String>,
         first: Option<i32>,
         last: Option<i32>,
-    ) -> Result<Connection<String, Category, CategoryTotalCount, EmptyFields>> {
+    ) -> Result<Connection<OpaqueCursor<Vec<u8>>, Category, CategoryTotalCount, EmptyFields>> {
         query_with_constraints(
             after,
             before,
@@ -108,11 +109,11 @@ impl CategoryTotalCount {
 
 async fn load(
     ctx: &Context<'_>,
-    after: Option<String>,
-    before: Option<String>,
+    after: Option<OpaqueCursor<Vec<u8>>>,
+    before: Option<OpaqueCursor<Vec<u8>>>,
     first: Option<usize>,
     last: Option<usize>,
-) -> Result<Connection<String, Category, CategoryTotalCount, EmptyFields>> {
+) -> Result<Connection<OpaqueCursor<Vec<u8>>, Category, CategoryTotalCount, EmptyFields>> {
     let store = super::get_store(ctx).await?;
     let table = store.category_map();
     super::load_edges(&table, after, before, first, last, CategoryTotalCount)