Skip to content

Commit

Permalink
[graphql/rpc] easy: remove old limits logic (#15326)
Browse files Browse the repository at this point in the history
## Description 

Since we have custom `query_limits_checker` which has been shown to
work, we don't need the limit
[logic](https://async-graphql.github.io/async-graphql/en/depth_and_complexity.html)
provided by the library.
Removing it allows us make other changes such as making exceptions for
schema queries in limits checker.

## Test Plan 

Existing

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
oxade authored and gdanezis committed Dec 15, 2023
1 parent 4588446 commit 8b55b2d
Showing 1 changed file with 34 additions and 20 deletions.
54 changes: 34 additions & 20 deletions crates/sui-graphql-rpc/src/server/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,16 +76,6 @@ impl ServerBuilder {
format!("{}:{}", self.host, self.port)
}

pub fn max_query_depth(mut self, max_depth: u32) -> Self {
self.schema = self.schema.limit_depth(max_depth as usize);
self
}

pub fn max_query_nodes(mut self, max_nodes: u32) -> Self {
self.schema = self.schema.limit_complexity(max_nodes as usize);
self
}

pub fn context_data(mut self, context_data: impl Any + Send + Sync) -> Self {
self.schema = self.schema.data(context_data);
self
Expand Down Expand Up @@ -211,8 +201,6 @@ impl ServerBuilder {
let metrics = RequestMetrics::new(&registry);

builder = builder
.max_query_depth(config.service.limits.max_query_depth)
.max_query_nodes(config.service.limits.max_query_nodes)
.context_data(config.service.clone())
.context_data(pg_conn_pool)
.context_data(Resolver::new(package_cache))
Expand Down Expand Up @@ -416,9 +404,17 @@ pub mod tests {
let db_url: String = connection_config.db_url.clone();
let reader = PgManager::reader(db_url).expect("Failed to create pg connection pool");
let pg_conn_pool = PgManager::new(reader, Limits::default());
let server_config = ServiceConfig {
limits: Limits {
max_query_depth: depth,
..Default::default()
},
..Default::default()
};
let schema = ServerBuilder::new(8000, "127.0.0.1".to_string())
.context_data(pg_conn_pool)
.max_query_depth(depth)
.context_data(server_config)
.extension(QueryLimitsChecker::default())
.build_schema();
schema.execute(query).await
}
Expand All @@ -443,7 +439,10 @@ pub mod tests {
.map(|e| e.message)
.collect();

assert_eq!(errs, vec!["Query is nested too deep.".to_string()]);
assert_eq!(
errs,
vec!["Query has too many levels of nesting. The maximum allowed is 0".to_string()]
);
let errs: Vec<_> = exec_query_depth_limit(
2,
"{ chainIdentifier protocolConfig { configs { value key }} }",
Expand All @@ -455,7 +454,10 @@ pub mod tests {
.into_iter()
.map(|e| e.message)
.collect();
assert_eq!(errs, vec!["Query is nested too deep.".to_string()]);
assert_eq!(
errs,
vec!["Query has too many levels of nesting. The maximum allowed is 2".to_string()]
);
}

pub async fn test_query_node_limit_impl() {
Expand All @@ -469,9 +471,17 @@ pub mod tests {
let db_url: String = connection_config.db_url.clone();
let reader = PgManager::reader(db_url).expect("Failed to create pg connection pool");
let pg_conn_pool = PgManager::new(reader, Limits::default());
let server_config = ServiceConfig {
limits: Limits {
max_query_nodes: nodes,
..Default::default()
},
..Default::default()
};
let schema = ServerBuilder::new(8000, "127.0.0.1".to_string())
.context_data(pg_conn_pool)
.max_query_nodes(nodes)
.context_data(server_config)
.extension(QueryLimitsChecker::default())
.build_schema();
schema.execute(query).await
}
Expand All @@ -495,7 +505,10 @@ pub mod tests {
.into_iter()
.map(|e| e.message)
.collect();
assert_eq!(err, vec!["Query is too complex.".to_string()]);
assert_eq!(
err,
vec!["Query has too many nodes. The maximum allowed is 0".to_string()]
);

let err: Vec<_> = exec_query_node_limit(
4,
Expand All @@ -508,7 +521,10 @@ pub mod tests {
.into_iter()
.map(|e| e.message)
.collect();
assert_eq!(err, vec!["Query is too complex.".to_string()]);
assert_eq!(
err,
vec!["Query has too many nodes. The maximum allowed is 4".to_string()]
);
}

pub async fn test_query_default_page_limit_impl() {
Expand Down Expand Up @@ -611,8 +627,6 @@ pub mod tests {
let reader = PgManager::reader(db_url).expect("Failed to create pg connection pool");
let pg_conn_pool = PgManager::new(reader, service_config.limits);
let schema = ServerBuilder::new(8000, "127.0.0.1".to_string())
.max_query_depth(service_config.limits.max_query_depth)
.max_query_nodes(service_config.limits.max_query_nodes)
.context_data(service_config)
.context_data(pg_conn_pool)
.context_data(metrics)
Expand Down

0 comments on commit 8b55b2d

Please sign in to comment.