Skip to content

Commit

Permalink
fix(compiler): share the cached implementers map between different op…
Browse files Browse the repository at this point in the history
…erations in the same document
  • Loading branch information
goto-bus-stop committed Jun 5, 2024
1 parent a2b0ca9 commit ad30e71
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 68 deletions.
7 changes: 5 additions & 2 deletions crates/apollo-compiler/src/executable/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::validation::fragment::validate_fragment_used;
use crate::validation::operation::validate_operation_definitions;
use crate::validation::selection::FieldsInSetCanMerge;
use crate::validation::DiagnosticList;
use crate::validation::ExecutableValidationContext;
use crate::validation::Valid;
use crate::ExecutableDocument;
use crate::Schema;
Expand Down Expand Up @@ -40,7 +41,8 @@ pub(crate) fn validate_with_or_without_schema(
schema: Option<&Schema>,
document: &ExecutableDocument,
) {
validate_operation_definitions(errors, schema, document);
let context = ExecutableValidationContext::new(schema);
validate_operation_definitions(errors, document, &context);
for def in document.fragments.values() {
validate_fragment_used(errors, document, def);
}
Expand All @@ -52,11 +54,12 @@ pub(crate) fn validate_field_set(
field_set: &FieldSet,
) {
let document = &ExecutableDocument::new(); // No fragment definitions
let context = ExecutableValidationContext::new(Some(schema));
crate::validation::selection::validate_selection_set(
diagnostics,
document,
Some((schema, &field_set.selection_set.ty)),
&field_set.selection_set,
&crate::validation::operation::OperationValidationConfig::new(Some(schema), &[]),
context.operation_context(&[]),
)
}
6 changes: 3 additions & 3 deletions crates/apollo-compiler/src/validation/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ use crate::coordinate::{FieldArgumentCoordinate, TypeAttributeCoordinate};
use crate::validation::diagnostics::DiagnosticData;
use crate::{ast, executable, schema, ExecutableDocument, Node};

use super::operation::OperationValidationConfig;
use crate::ast::Name;
use crate::schema::Component;
use crate::validation::DiagnosticList;
use crate::validation::OperationValidationContext;
use indexmap::IndexMap;

pub(crate) fn validate_field(
Expand All @@ -14,13 +14,13 @@ pub(crate) fn validate_field(
// May be None if a parent selection was invalid
against_type: Option<(&crate::Schema, &ast::NamedType)>,
field: &Node<executable::Field>,
context: &OperationValidationConfig<'_>,
context: OperationValidationContext<'_>,
) {
// First do all the validation that we can without knowing the type of the field.

super::directive::validate_directives(
diagnostics,
context.schema,
context.schema(),
field.directives.iter(),
ast::DirectiveLocation::Field,
context.variables,
Expand Down
30 changes: 15 additions & 15 deletions crates/apollo-compiler/src/validation/fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use crate::executable;
use crate::schema;
use crate::schema::Implementers;
use crate::validation::diagnostics::DiagnosticData;
use crate::validation::operation::OperationValidationConfig;
use crate::validation::DiagnosticList;
use crate::validation::OperationValidationContext;
use crate::validation::{CycleError, NodeLocation, RecursionGuard, RecursionStack};
use crate::ExecutableDocument;
use crate::Node;
Expand Down Expand Up @@ -52,7 +52,7 @@ fn validate_fragment_spread_type(
against_type: &NamedType,
type_condition: &NamedType,
selection: &executable::Selection,
context: &OperationValidationConfig<'_>,
context: OperationValidationContext<'_>,
) {
// Another diagnostic will be raised if the type condition was wrong.
// We reduce noise by silencing other issues with the fragment.
Expand All @@ -66,8 +66,8 @@ fn validate_fragment_spread_type(
};

let implementers_map = context.implementers_map();
let concrete_parent_types = get_possible_types(against_type_definition, &implementers_map);
let concrete_condition_types = get_possible_types(type_condition_definition, &implementers_map);
let concrete_parent_types = get_possible_types(against_type_definition, implementers_map);
let concrete_condition_types = get_possible_types(type_condition_definition, implementers_map);

let mut applicable_types = concrete_parent_types.intersection(&concrete_condition_types);
if applicable_types.next().is_none() {
Expand Down Expand Up @@ -108,18 +108,18 @@ pub(crate) fn validate_inline_fragment(
document: &ExecutableDocument,
against_type: Option<(&crate::Schema, &ast::NamedType)>,
inline: &Node<executable::InlineFragment>,
context: &OperationValidationConfig<'_>,
context: OperationValidationContext<'_>,
) {
super::directive::validate_directives(
diagnostics,
context.schema,
context.schema(),
inline.directives.iter(),
ast::DirectiveLocation::InlineFragment,
context.variables,
);

let previous = diagnostics.len();
if let Some(schema) = context.schema {
if let Some(schema) = context.schema() {
if let Some(t) = &inline.type_condition {
validate_fragment_type_condition(diagnostics, schema, None, t, inline.location())
}
Expand All @@ -139,13 +139,13 @@ pub(crate) fn validate_inline_fragment(
against_type,
type_condition,
&executable::Selection::InlineFragment(inline.clone()),
&context,
context,
);
}
super::selection::validate_selection_set(
diagnostics,
document,
if let (Some(schema), Some(ty)) = (&context.schema, &inline.type_condition) {
if let (Some(schema), Some(ty)) = (&context.schema(), &inline.type_condition) {
Some((schema, ty))
} else {
against_type
Expand All @@ -161,11 +161,11 @@ pub(crate) fn validate_fragment_spread(
document: &ExecutableDocument,
against_type: Option<(&crate::Schema, &NamedType)>,
spread: &Node<executable::FragmentSpread>,
context: &OperationValidationConfig<'_>,
context: OperationValidationContext<'_>,
) {
super::directive::validate_directives(
diagnostics,
context.schema,
context.schema(),
spread.directives.iter(),
ast::DirectiveLocation::FragmentSpread,
context.variables,
Expand Down Expand Up @@ -201,18 +201,18 @@ pub(crate) fn validate_fragment_definition(
diagnostics: &mut DiagnosticList,
document: &ExecutableDocument,
fragment: &Node<executable::Fragment>,
context: &OperationValidationConfig<'_>,
context: OperationValidationContext<'_>,
) {
super::directive::validate_directives(
diagnostics,
context.schema,
context.schema(),
fragment.directives.iter(),
ast::DirectiveLocation::FragmentDefinition,
context.variables,
);

let previous = diagnostics.len();
if let Some(schema) = context.schema {
if let Some(schema) = context.schema() {
validate_fragment_type_condition(
diagnostics,
schema,
Expand All @@ -231,7 +231,7 @@ pub(crate) fn validate_fragment_definition(
// If the type does not exist, do not attempt to validate the selections against it;
// it has either already raised an error, or we are validating an executable without
// a schema.
let type_condition = context.schema.and_then(|schema| {
let type_condition = context.schema().and_then(|schema| {
schema
.types
.contains_key(fragment.type_condition())
Expand Down
72 changes: 70 additions & 2 deletions crates/apollo-compiler/src/validation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
use crate::coordinate::SchemaCoordinate;
#[cfg(doc)]
use crate::{ExecutableDocument, Schema};
use crate::ExecutableDocument;
use crate::Schema;

pub(crate) mod argument;
pub(crate) mod diagnostics;
Expand All @@ -25,13 +26,16 @@ pub(crate) mod variable;
use crate::ast::Name;
use crate::diagnostic::{CliReport, Diagnostic, ToCliReport};
use crate::executable::BuildError as ExecutableBuildError;
use crate::executable::VariableDefinition;
use crate::execution::{GraphQLError, Response};
use crate::schema::BuildError as SchemaBuildError;
use crate::schema::Implementers;
use crate::SourceMap;
use crate::{Node, NodeLocation};
use indexmap::IndexSet;
use std::collections::HashMap;
use std::fmt;
use std::sync::Arc;
use std::sync::{Arc, OnceLock};

pub(crate) use crate::node::FileId;

Expand Down Expand Up @@ -112,6 +116,70 @@ impl<T: fmt::Display> fmt::Display for Valid<T> {
}
}

/// Shared context with things that may be used throughout executable validation.
#[derive(Debug)]
pub(crate) struct ExecutableValidationContext<'a> {
/// When None, rules that require a schema to validate are disabled.
schema: Option<&'a Schema>,
/// `schema.implementers_map()` is expensive to compute. This caches it for reuse.
implementers_map: OnceLock<HashMap<Name, Implementers>>,
}

impl<'a> ExecutableValidationContext<'a> {
pub fn new(schema: Option<&'a Schema>) -> Self {
Self {
schema,
implementers_map: Default::default(),
}
}

/// Returns the schema to validate against, if any.
pub fn schema(&self) -> Option<&'a Schema> {
self.schema
}

/// Returns a cached reference to the implementers map.
pub fn implementers_map(&self) -> &HashMap<Name, Implementers> {
self.implementers_map.get_or_init(|| {
self.schema
.map(|schema| schema.implementers_map())
.unwrap_or_default()
})
}

/// Returns a context for operation validation.
pub fn operation_context<'o>(
&'o self,
variables: &'o [Node<VariableDefinition>],
) -> OperationValidationContext<'o> {
OperationValidationContext {
executable: self,
variables,
}
}
}

/// Shared context when validating things inside an operation.
#[derive(Debug, Clone, Copy)]
pub(crate) struct OperationValidationContext<'a> {
/// Parent context. Using a reference so the `OnceLock` is shared between all operation
/// contexts.
executable: &'a ExecutableValidationContext<'a>,
/// The variables defined for this operation.
pub variables: &'a [Node<VariableDefinition>],
}

impl<'a> OperationValidationContext<'a> {
pub fn schema(&self) -> Option<&'a Schema> {
self.executable.schema
}

/// Returns a cached reference to the implementers map.
pub fn implementers_map(&self) -> &HashMap<Name, Implementers> {
self.executable.implementers_map()
}
}

/// A conversion failed with some errors, but also resulted in a partial document.
///
/// The [`Debug`][fmt::Debug] trait is implemented by forwarding to [`Self::errors`] and
Expand Down
52 changes: 8 additions & 44 deletions crates/apollo-compiler/src/validation/operation.rs
Original file line number Diff line number Diff line change
@@ -1,42 +1,8 @@
use std::collections::HashMap;
use std::sync::OnceLock;

use crate::ast;
use crate::ast::Name;
use crate::executable;
use crate::schema::Implementers;
use crate::validation::DiagnosticList;
use crate::validation::ExecutableValidationContext;
use crate::ExecutableDocument;
use crate::Node;
use crate::Schema;

#[derive(Debug)]
pub(crate) struct OperationValidationConfig<'a> {
/// When None, rules that require a schema to validate are disabled.
pub schema: Option<&'a Schema>,
/// The variables defined for this operation.
pub variables: &'a [Node<ast::VariableDefinition>],
implementers_map: OnceLock<HashMap<Name, Implementers>>,
}

impl<'a> OperationValidationConfig<'a> {
pub fn new(schema: Option<&'a Schema>, variables: &'a [Node<ast::VariableDefinition>]) -> Self {
Self {
schema,
variables,
implementers_map: Default::default(),
}
}

/// Returns a cached reference to the implementers map.
pub fn implementers_map(&self) -> &HashMap<Name, Implementers> {
self.implementers_map.get_or_init(|| {
self.schema
.map(|schema| schema.implementers_map())
.unwrap_or_default()
})
}
}

pub(crate) fn validate_subscription(
document: &executable::ExecutableDocument,
Expand Down Expand Up @@ -85,13 +51,11 @@ pub(crate) fn validate_subscription(

pub(crate) fn validate_operation(
diagnostics: &mut DiagnosticList,
schema: Option<&Schema>,
document: &ExecutableDocument,
operation: &executable::Operation,
context: &ExecutableValidationContext<'_>,
) {
let config = OperationValidationConfig::new(schema, &operation.variables);

let against_type = if let Some(schema) = schema {
let against_type = if let Some(schema) = context.schema() {
schema
.root_operation(operation.operation_type)
.map(|ty| (schema, ty))
Expand All @@ -101,14 +65,14 @@ pub(crate) fn validate_operation(

super::directive::validate_directives(
diagnostics,
schema,
context.schema(),
operation.directives.iter(),
operation.operation_type.into(),
&operation.variables,
);
super::variable::validate_variable_definitions(
diagnostics,
config.schema,
context.schema(),
&operation.variables,
);

Expand All @@ -118,16 +82,16 @@ pub(crate) fn validate_operation(
document,
against_type,
&operation.selection_set,
&config,
context.operation_context(&operation.variables),
);
}

pub(crate) fn validate_operation_definitions(
diagnostics: &mut DiagnosticList,
schema: Option<&Schema>,
document: &ExecutableDocument,
context: &ExecutableValidationContext<'_>,
) {
for operation in document.all_operations() {
validate_operation(diagnostics, schema, document, operation);
validate_operation(diagnostics, document, operation, context);
}
}
4 changes: 2 additions & 2 deletions crates/apollo-compiler/src/validation/selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use crate::ast::NamedType;
use crate::coordinate::TypeAttributeCoordinate;
use crate::executable::BuildError;
use crate::executable::SelectionSet;
use crate::validation::operation::OperationValidationConfig;
use crate::validation::DiagnosticList;
use crate::validation::OperationValidationContext;
use crate::ExecutableDocument;
use crate::{ast, executable, schema, Node};
use apollo_parser::LimitTracker;
Expand Down Expand Up @@ -545,7 +545,7 @@ pub(crate) fn validate_selection_set(
document: &ExecutableDocument,
against_type: Option<(&crate::Schema, &NamedType)>,
selection_set: &SelectionSet,
context: &OperationValidationConfig<'_>,
context: OperationValidationContext<'_>,
) {
for selection in &selection_set.selections {
match selection {
Expand Down

0 comments on commit ad30e71

Please sign in to comment.