Skip to content

Commit

Permalink
Update GraphQL cursor encoding/decoding to use OpaqueCursor
Browse files Browse the repository at this point in the history
Close #355

The following sources are the main areas that have been modified.
The rest of the changes are fixes for Clippy errors caused by the Rust version update.

graphql.rs
graphql directory sub-source
  • Loading branch information
henry0715-dev committed Dec 4, 2024
1 parent dd41e12 commit 6c1a5fb
Show file tree
Hide file tree
Showing 26 changed files with 250 additions and 179 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl Config {

pub(crate) fn configure_reverse_proxies(
store: &Arc<RwLock<Store>>,
client: &Option<reqwest::Client>,
client: Option<&reqwest::Client>,

Check warning on line 51 in src/archive.rs

View check run for this annotation

Codecov / codecov/patch

src/archive.rs#L51

Added line #L51 was not covered by tests
reverse_proxies: &[Self],
) -> Vec<(ArchiveState, Router<ArchiveState>)> {
reverse_proxies
Expand All @@ -57,10 +57,10 @@ impl Config {
(
crate::archive::ArchiveState {
store: store.clone(),
client: client.clone(),
client: client.cloned(),

Check warning on line 60 in src/archive.rs

View check run for this annotation

Codecov / codecov/patch

src/archive.rs#L60

Added line #L60 was not covered by tests
config: rp.clone(),
},
crate::archive::reverse_proxy(store.clone(), client.clone(), rp.clone()),
crate::archive::reverse_proxy(store.clone(), client.cloned(), rp.clone()),

Check warning on line 63 in src/archive.rs

View check run for this annotation

Codecov / codecov/patch

src/archive.rs#L63

Added line #L63 was not covered by tests
)
})
.collect()
Expand Down
82 changes: 35 additions & 47 deletions src/graphql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand All @@ -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]>,
Expand All @@ -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()
Expand All @@ -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>>,
Expand All @@ -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 };
Expand All @@ -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)
}
Expand Down
33 changes: 23 additions & 10 deletions src/graphql/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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) =
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 6 additions & 4 deletions src/graphql/allow_network.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use async_graphql::connection::OpaqueCursor;
use async_graphql::{
connection::{Connection, EmptyFields},
Context, InputObject, Object, Result, ID,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 6 additions & 4 deletions src/graphql/block_network.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::sync::Arc;

use async_graphql::connection::OpaqueCursor;
use async_graphql::{
connection::{Connection, EmptyFields},
Context, InputObject, Object, Result, ID,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 5 additions & 4 deletions src/graphql/category.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::sync::Arc;

use async_graphql::connection::OpaqueCursor;
use async_graphql::{
connection::{Connection, EmptyFields},
types::ID,
Expand Down Expand Up @@ -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>> {

Check warning on line 33 in src/graphql/category.rs

View check run for this annotation

Codecov / codecov/patch

src/graphql/category.rs#L33

Added line #L33 was not covered by tests
query_with_constraints(
after,
before,
Expand Down Expand Up @@ -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>>>,

Check warning on line 113 in src/graphql/category.rs

View check run for this annotation

Codecov / codecov/patch

src/graphql/category.rs#L112-L113

Added lines #L112 - L113 were not covered by tests
first: Option<usize>,
last: Option<usize>,
) -> Result<Connection<String, Category, CategoryTotalCount, EmptyFields>> {
) -> Result<Connection<OpaqueCursor<Vec<u8>>, Category, CategoryTotalCount, EmptyFields>> {

Check warning on line 116 in src/graphql/category.rs

View check run for this annotation

Codecov / codecov/patch

src/graphql/category.rs#L116

Added line #L116 was not covered by tests
let store = super::get_store(ctx).await?;
let table = store.category_map();
super::load_edges(&table, after, before, first, last, CategoryTotalCount)
Expand Down
Loading

0 comments on commit 6c1a5fb

Please sign in to comment.