Skip to content

Commit 5d72827

Browse files
goto-bus-stopgoto-bus-stop
committed
Move operation limits enforcement into a layer
1 parent 0b9c439 commit 5d72827

File tree

11 files changed

+156
-95
lines changed

11 files changed

+156
-95
lines changed

‎apollo-router/src/error.rs

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ use crate::graphql::Response;
2222
use crate::json_ext::Path;
2323
use crate::json_ext::Value;
2424
use crate::spec::SpecError;
25-
use crate::spec::operation_limits::OperationLimits;
2625

2726
/// Return up to this many GraphQL parsing or validation errors.
2827
///
@@ -283,9 +282,6 @@ pub(crate) enum QueryPlannerError {
283282
/// spec error: {0}
284283
SpecError(SpecError),
285284

286-
/// complexity limit exceeded
287-
LimitExceeded(OperationLimits<bool>),
288-
289285
// Safe to cache because user scopes and policies are included in the cache key.
290286
/// Unauthorized field or type
291287
Unauthorized(Vec<Path>),
@@ -385,46 +381,6 @@ impl IntoGraphQLErrors for QueryPlannerError {
385381
QueryPlannerError::OperationValidationErrors(errs) => errs
386382
.into_graphql_errors()
387383
.map_err(QueryPlannerError::OperationValidationErrors),
388-
389-
QueryPlannerError::LimitExceeded(OperationLimits {
390-
depth,
391-
height,
392-
root_fields,
393-
aliases,
394-
}) => {
395-
let mut errors = Vec::new();
396-
let mut build = |exceeded, code, message| {
397-
if exceeded {
398-
errors.push(
399-
Error::builder()
400-
.message(message)
401-
.extension_code(code)
402-
.build(),
403-
)
404-
}
405-
};
406-
build(
407-
depth,
408-
"MAX_DEPTH_LIMIT",
409-
"Maximum depth limit exceeded in this operation",
410-
);
411-
build(
412-
height,
413-
"MAX_HEIGHT_LIMIT",
414-
"Maximum height (field count) limit exceeded in this operation",
415-
);
416-
build(
417-
root_fields,
418-
"MAX_ROOT_FIELDS_LIMIT",
419-
"Maximum root fields limit exceeded in this operation",
420-
);
421-
build(
422-
aliases,
423-
"MAX_ALIASES_LIMIT",
424-
"Maximum aliases limit exceeded in this operation",
425-
);
426-
Ok(errors)
427-
}
428384
QueryPlannerError::FederationError(err) => err
429385
.into_graphql_errors()
430386
.map_err(QueryPlannerError::FederationError),
@@ -466,11 +422,6 @@ impl From<ValidationErrors> for QueryPlannerError {
466422
QueryPlannerError::OperationValidationErrors(ValidationErrors { errors: err.errors })
467423
}
468424
}
469-
impl From<OperationLimits<bool>> for QueryPlannerError {
470-
fn from(error: OperationLimits<bool>) -> Self {
471-
QueryPlannerError::LimitExceeded(error)
472-
}
473-
}
474425

475426
impl From<QueryPlannerError> for Response {
476427
fn from(err: QueryPlannerError) -> Self {

‎apollo-router/src/plugins/file_uploads/rearrange_query_plan.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ pub(super) fn rearrange_query_plan(
4444
usage_reporting: query_plan.usage_reporting.clone(),
4545
formatted_query_plan: query_plan.formatted_query_plan.clone(),
4646
query: query_plan.query.clone(),
47-
query_metrics: query_plan.query_metrics,
4847
estimated_size: Default::default(),
4948
})
5049
}

‎apollo-router/src/plugins/limits/mod.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@ use crate::plugin::Plugin;
2020
use crate::plugin::PluginInit;
2121
use crate::plugins::limits::layer::BodyLimitError;
2222
use crate::plugins::limits::layer::RequestBodyLimitLayer;
23+
use crate::services::layers::enforce_operation_limits::EnforceOperationLimitsLayer;
2324
use crate::services::router;
24-
use crate::services::router::BoxService;
25+
use crate::services::supergraph;
2526

2627
/// Configuration for operation limits, parser limits, HTTP limits, etc.
2728
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)]
@@ -164,7 +165,7 @@ impl Plugin for LimitsPlugin {
164165
})
165166
}
166167

167-
fn router_service(&self, service: BoxService) -> BoxService {
168+
fn router_service(&self, service: router::BoxService) -> router::BoxService {
168169
ServiceBuilder::new()
169170
.map_future_with_request_data(
170171
|r: &router::Request| r.context.clone(),
@@ -181,6 +182,13 @@ impl Plugin for LimitsPlugin {
181182
.service(service)
182183
.boxed()
183184
}
185+
186+
fn supergraph_service(&self, service: supergraph::BoxService) -> supergraph::BoxService {
187+
ServiceBuilder::new()
188+
.layer(EnforceOperationLimitsLayer::new(&self.config))
189+
.service(service)
190+
.boxed()
191+
}
184192
}
185193

186194
impl LimitsPlugin {

‎apollo-router/src/query_planner/caching_query_planner.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -866,7 +866,6 @@ mod tests {
866866
usage_reporting: UsageReporting::Error("this is a test report key".to_string())
867867
.into(),
868868
query: Arc::new(Query::empty_for_tests()),
869-
query_metrics: Default::default(),
870869
estimated_size: Default::default(),
871870
};
872871
let qp_content = QueryPlannerContent::Plan {

‎apollo-router/src/query_planner/plan.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ use crate::query_planner::fetch::SubgraphSchemas;
2323
use crate::services::query_planner::PlanOptions;
2424
use crate::spec::Query;
2525
use crate::spec::QueryHash;
26-
use crate::spec::operation_limits::OperationLimits;
2726

2827
/// A planner key.
2928
///
@@ -45,7 +44,6 @@ pub struct QueryPlan {
4544
/// String representation of the query plan (not a json representation)
4645
pub(crate) formatted_query_plan: Option<Arc<String>>,
4746
pub(crate) query: Arc<Query>,
48-
pub(crate) query_metrics: OperationLimits<u32>,
4947

5048
/// The estimated size in bytes of the query plan
5149
#[serde(default)]
@@ -68,7 +66,6 @@ impl QueryPlan {
6866
root: Arc::new(root.unwrap_or_else(|| PlanNode::Sequence { nodes: Vec::new() })),
6967
formatted_query_plan: Default::default(),
7068
query: Arc::new(Query::empty_for_tests()),
71-
query_metrics: Default::default(),
7269
estimated_size: Default::default(),
7370
}
7471
}

‎apollo-router/src/query_planner/query_planner_service.rs

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ use crate::services::query_planner::PlanOptions;
5555
use crate::spec::Query;
5656
use crate::spec::Schema;
5757
use crate::spec::SpecError;
58-
use crate::spec::operation_limits::OperationLimits;
5958

6059
pub(crate) const RUST_QP_MODE: &str = "rust";
6160
const UNSUPPORTED_FED1: &str = "fed1";
@@ -257,16 +256,8 @@ impl QueryPlannerService {
257256
query: String,
258257
operation_name: Option<&str>,
259258
doc: &ParsedDocument,
260-
query_metrics_in: &mut OperationLimits<u32>,
261259
) -> Result<Query, QueryPlannerError> {
262260
let executable = &doc.executable;
263-
crate::spec::operation_limits::check(
264-
query_metrics_in,
265-
&self.configuration,
266-
&query,
267-
executable,
268-
operation_name,
269-
)?;
270261

271262
let (fragments, operation, defer_stats, schema_aware_hash) =
272263
Query::extract_query_information(&self.schema, &query, executable, operation_name)?;
@@ -304,7 +295,6 @@ impl QueryPlannerService {
304295
plan_options: PlanOptions,
305296
doc: &ParsedDocument,
306297
compute_job_type: ComputeJobType,
307-
query_metrics: OperationLimits<u32>,
308298
) -> Result<QueryPlannerContent, MaybeBackPressureError<QueryPlannerError>> {
309299
let plan_result = self
310300
.plan_inner(
@@ -366,7 +356,6 @@ impl QueryPlannerService {
366356
root: node,
367357
formatted_query_plan,
368358
query: Arc::new(selections),
369-
query_metrics,
370359
estimated_size: Default::default(),
371360
}),
372361
})
@@ -475,13 +464,11 @@ impl QueryPlannerService {
475464
mut doc: ParsedDocument,
476465
compute_job_type: ComputeJobType,
477466
) -> Result<QueryPlannerContent, MaybeBackPressureError<QueryPlannerError>> {
478-
let mut query_metrics = Default::default();
479467
let mut selections = self
480468
.parse_selections(
481469
key.original_query.clone(),
482470
key.operation_name.as_deref(),
483471
&doc,
484-
&mut query_metrics,
485472
)
486473
.await?;
487474

@@ -565,7 +552,6 @@ impl QueryPlannerService {
565552
key.filtered_query.clone(),
566553
key.operation_name.as_deref(),
567554
&doc,
568-
&mut query_metrics,
569555
)
570556
.await?;
571557
filtered.is_original = false;
@@ -581,7 +567,6 @@ impl QueryPlannerService {
581567
key.plan_options,
582568
&doc,
583569
compute_job_type,
584-
query_metrics,
585570
)
586571
.await
587572
}
@@ -713,9 +698,8 @@ mod tests {
713698

714699
let doc = Query::parse_document(query, None, &schema, &Configuration::default()).unwrap();
715700

716-
let mut query_metrics = Default::default();
717701
let selections = planner
718-
.parse_selections(query.to_string(), None, &doc, &mut query_metrics)
702+
.parse_selections(query.to_string(), None, &doc)
719703
.await
720704
.unwrap();
721705
let err =
@@ -732,7 +716,6 @@ mod tests {
732716
PlanOptions::default(),
733717
&doc,
734718
ComputeJobType::QueryPlanning,
735-
query_metrics
736719
)
737720
.await
738721
.unwrap_err();

‎apollo-router/src/query_planner/tests.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ async fn mock_subgraph_service_withf_panics_should_be_reported_as_service_closed
8383
root: serde_json::from_str(test_query_plan!()).unwrap(),
8484
formatted_query_plan: Default::default(),
8585
query: Arc::new(Query::empty_for_tests()),
86-
query_metrics: Default::default(),
8786
usage_reporting: UsageReporting::Error("this is a test report key".to_string()).into(),
8887
estimated_size: Default::default(),
8988
};
@@ -143,7 +142,6 @@ async fn fetch_includes_operation_name() {
143142
formatted_query_plan: Default::default(),
144143
usage_reporting: UsageReporting::Error("this is a test report key".to_string()).into(),
145144
query: Arc::new(Query::empty_for_tests()),
146-
query_metrics: Default::default(),
147145
estimated_size: Default::default(),
148146
};
149147

@@ -208,7 +206,6 @@ async fn fetch_makes_post_requests() {
208206
formatted_query_plan: Default::default(),
209207
usage_reporting: UsageReporting::Error("this is a test report key".to_string()).into(),
210208
query: Arc::new(Query::empty_for_tests()),
211-
query_metrics: Default::default(),
212209
estimated_size: Default::default(),
213210
};
214211

@@ -342,7 +339,6 @@ async fn defer() {
342339
}.into(),
343340
usage_reporting: UsageReporting::Error("this is a test report key".to_string()).into(),
344341
query: Arc::new(Query::empty_for_tests()),
345-
query_metrics: Default::default(),
346342
estimated_size: Default::default(),
347343
};
348344

@@ -476,7 +472,6 @@ async fn defer_if_condition() {
476472
.unwrap(),
477473
),
478474
formatted_query_plan: None,
479-
query_metrics: Default::default(),
480475
estimated_size: Default::default(),
481476
};
482477

@@ -636,7 +631,6 @@ async fn dependent_mutations() {
636631
.unwrap(),
637632
usage_reporting: UsageReporting::Error("this is a test report key".to_string()).into(),
638633
query: Arc::new(Query::empty_for_tests()),
639-
query_metrics: Default::default(),
640634
estimated_size: Default::default(),
641635
};
642636

@@ -1858,7 +1852,6 @@ fn broken_plan_does_not_panic() {
18581852
formatted_query_plan: Default::default(),
18591853
usage_reporting: UsageReporting::Error("this is a test report key".to_string()).into(),
18601854
query: Arc::new(Query::empty_for_tests()),
1861-
query_metrics: Default::default(),
18621855
estimated_size: Default::default(),
18631856
};
18641857
let subgraph_schema = apollo_compiler::Schema::parse_and_validate(subgraph_schema, "").unwrap();

0 commit comments

Comments
 (0)