From 2b6ab1e6ab89c21f1f583cf812a86396bc7a0cad Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 18 Nov 2019 17:05:49 -0600 Subject: [PATCH 01/64] graph: Add UnvalidatedSubgraphManifest --- graph/src/data/schema.rs | 13 ++++++++++++ graph/src/data/subgraph/mod.rs | 36 ++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 327e68d0b7a..098d9dddbed 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -39,6 +39,19 @@ impl Schema { } } + /// Creates a new schema from a parsed and validated GraphQL schema document. + pub fn initialize(id: SubgraphDeploymentId, document: schema::Document) -> Result { + let (interfaces_for_type, types_for_interface) = Self::collect_interfaces(&document)?; + let mut schema = Self { + id: id.clone(), + document, + interfaces_for_type, + types_for_interface, + }; + schema.add_subgraph_id_directives(id); + Ok(schema) + } + pub fn collect_interfaces( document: &schema::Document, ) -> Result< diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index 8ab0dd08290..6035d3d27d1 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -367,11 +367,11 @@ impl From for Link { } #[derive(Clone, Debug, Hash, Eq, PartialEq, Deserialize)] -pub struct SchemaData { +pub struct UnresolvedSchema { pub file: Link, } -impl SchemaData { +impl UnresolvedSchema { pub fn resolve( self, id: SubgraphDeploymentId, @@ -812,10 +812,29 @@ impl PartialEq for BaseSubgraphManifest { } } -pub type UnresolvedSubgraphManifest = - BaseSubgraphManifest; +/// SubgraphManifest with IPFS links unresolved +type UnresolvedSubgraphManifest = + BaseSubgraphManifest; + +/// SubgraphManifest validated with IPFS links resolved pub type SubgraphManifest = BaseSubgraphManifest; +/// Unvalidated SubgraphManifest +pub struct UnvalidatedSubgraphManifest(SubgraphManifest); + +impl UnvalidatedSubgraphManifest { + /// Entry point for resolving a subgraph definition. + /// Right now the only supported links are of the form: + /// `/ipfs/QmUmg7BZC1YP1ca66rRtWKxpXp77WgVHrnv263JtDuvs2k` + pub fn resolve( + link: Link, + resolver: Arc, + logger: Logger, + ) -> impl Future + Send { + SubgraphManifest::resolve(link, resolver, logger).map(|manifest| Self(manifest)) + } +} + impl SubgraphManifest { /// Entry point for resolving a subgraph definition. /// Right now the only supported links are of the form: @@ -892,6 +911,15 @@ impl SubgraphManifest { } } +impl UnvalidatedSubgraphManifest { + pub fn validate( + &self, + _logger: Logger, + ) -> impl Future> { + return future::ok(()); + } +} + impl UnresolvedSubgraphManifest { pub fn resolve( self, From 719bba7c0c694c4c7b5af48abd64516f44ef66bc Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 18 Nov 2019 17:47:12 -0600 Subject: [PATCH 02/64] core, graph: Update subgraph manifest resolution --- core/src/subgraph/registrar.rs | 107 +++++++++++++++++---------------- graph/src/data/subgraph/mod.rs | 3 +- graph/src/lib.rs | 2 +- 3 files changed, 57 insertions(+), 55 deletions(-) diff --git a/core/src/subgraph/registrar.rs b/core/src/subgraph/registrar.rs index 59c50a72557..a5bbdde6959 100644 --- a/core/src/subgraph/registrar.rs +++ b/core/src/subgraph/registrar.rs @@ -307,59 +307,60 @@ where let name_inner = name.clone(); Box::new( - SubgraphManifest::resolve(hash.to_ipfs_link(), self.resolver.clone(), logger.clone()) - .map_err(SubgraphRegistrarError::ResolveError) - .and_then(validation::validate_manifest) - .and_then(move |manifest| { - manifest - .network_name() - .map_err(|e| SubgraphRegistrarError::ManifestValidationError(vec![e])) - .and_then(move |network_name| { - chain_stores - .clone() - .get(&network_name) - .ok_or(SubgraphRegistrarError::NetworkNotSupported( - network_name.clone(), - )) - .and_then(move |chain_store| { - ethereum_adapters - .get(&network_name) - .ok_or(SubgraphRegistrarError::NetworkNotSupported( - network_name.clone(), - )) - .map(move |ethereum_adapter| { - ( - manifest, - ethereum_adapter.clone(), - chain_store.clone(), - ) - }) - }) - }) - }) - .and_then(move |(manifest, ethereum_adapter, chain_store)| { - let manifest_id = manifest.id.clone(); - create_subgraph_version( - &logger2, - store, - chain_store.clone(), - ethereum_adapter.clone(), - name, - manifest, - node_id, - version_switching_mode, - ) - .map(|_| manifest_id) - }) - .and_then(move |manifest_id| { - debug!( - logger3, - "Wrote new subgraph version to store"; - "subgraph_name" => name_inner.to_string(), - "subgraph_hash" => manifest_id.to_string(), - ); - Ok(()) - }), + UnvalidatedSubgraphManifest::resolve( + hash.to_ipfs_link(), + self.resolver.clone(), + logger.clone(), + ) + .map_err(SubgraphRegistrarError::ResolveError) + // .and_then(validation::validate_manifest) + .and_then(move |manifest| { + let manifest = manifest.0; + manifest + .network_name() + .map_err(|e| SubgraphRegistrarError::ManifestValidationError(vec![e])) + .and_then(move |network_name| { + chain_stores + .clone() + .get(&network_name) + .ok_or(SubgraphRegistrarError::NetworkNotSupported( + network_name.clone(), + )) + .and_then(move |chain_store| { + ethereum_adapters + .get(&network_name) + .ok_or(SubgraphRegistrarError::NetworkNotSupported( + network_name.clone(), + )) + .map(move |ethereum_adapter| { + (manifest, ethereum_adapter.clone(), chain_store.clone()) + }) + }) + }) + }) + .and_then(move |(manifest, ethereum_adapter, chain_store)| { + let manifest_id = manifest.id.clone(); + create_subgraph_version( + &logger2, + store, + chain_store.clone(), + ethereum_adapter.clone(), + name, + manifest, + node_id, + version_switching_mode, + ) + .map(|_| manifest_id) + }) + .and_then(move |manifest_id| { + debug!( + logger3, + "Wrote new subgraph version to store"; + "subgraph_name" => name_inner.to_string(), + "subgraph_hash" => manifest_id.to_string(), + ); + Ok(()) + }), ) } diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index 6035d3d27d1..6277e0a25a3 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -820,7 +820,8 @@ type UnresolvedSubgraphManifest = pub type SubgraphManifest = BaseSubgraphManifest; /// Unvalidated SubgraphManifest -pub struct UnvalidatedSubgraphManifest(SubgraphManifest); +// TODO: Make the tuple fields private +pub struct UnvalidatedSubgraphManifest(pub SubgraphManifest); impl UnvalidatedSubgraphManifest { /// Entry point for resolving a subgraph definition. diff --git a/graph/src/lib.rs b/graph/src/lib.rs index 92c31e83093..e1ac2c09a4f 100644 --- a/graph/src/lib.rs +++ b/graph/src/lib.rs @@ -100,7 +100,7 @@ pub mod prelude { MappingBlockHandler, MappingCallHandler, MappingEventHandler, SubgraphAssignmentProviderError, SubgraphAssignmentProviderEvent, SubgraphDeploymentId, SubgraphManifest, SubgraphManifestResolveError, SubgraphManifestValidationError, - SubgraphName, SubgraphRegistrarError, + SubgraphName, SubgraphRegistrarError, UnvalidatedSubgraphManifest, }; pub use crate::data::subscription::{ QueryResultStream, Subscription, SubscriptionError, SubscriptionResult, From cfa183ac6995898823bc9d13a7cb820388e7b471 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 18 Nov 2019 17:59:37 -0600 Subject: [PATCH 03/64] graph: Add SchemaReference enum and remove Schema method --- graph/src/data/schema.rs | 18 +++++------------- graph/src/data/subgraph/mod.rs | 16 +++++++--------- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 098d9dddbed..44314852922 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -13,6 +13,11 @@ use graphql_parser::{ use std::collections::BTreeMap; use std::iter::FromIterator; +pub enum SchemaReference { + ByName(String), + ByHash(String), +} + /// A validated and preprocessed GraphQL schema for a subgraph. #[derive(Clone, Debug, PartialEq)] pub struct Schema { @@ -39,19 +44,6 @@ impl Schema { } } - /// Creates a new schema from a parsed and validated GraphQL schema document. - pub fn initialize(id: SubgraphDeploymentId, document: schema::Document) -> Result { - let (interfaces_for_type, types_for_interface) = Self::collect_interfaces(&document)?; - let mut schema = Self { - id: id.clone(), - document, - interfaces_for_type, - types_for_interface, - }; - schema.add_subgraph_id_directives(id); - Ok(schema) - } - pub fn collect_interfaces( document: &schema::Document, ) -> Result< diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index 6277e0a25a3..d804dda92f5 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -834,6 +834,13 @@ impl UnvalidatedSubgraphManifest { ) -> impl Future + Send { SubgraphManifest::resolve(link, resolver, logger).map(|manifest| Self(manifest)) } + + pub fn validate( + &self, + _logger: Logger, + ) -> impl Future> { + return future::ok(()); + } } impl SubgraphManifest { @@ -912,15 +919,6 @@ impl SubgraphManifest { } } -impl UnvalidatedSubgraphManifest { - pub fn validate( - &self, - _logger: Logger, - ) -> impl Future> { - return future::ok(()); - } -} - impl UnresolvedSubgraphManifest { pub fn resolve( self, From ebae6f0aa84bac40bdf558b8ae498ab7e4402315 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 18 Nov 2019 23:11:33 -0600 Subject: [PATCH 04/64] schema: Add imported_schemas method to Schema --- graph/src/data/schema.rs | 58 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 2 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 44314852922..4eceadcd7b6 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -7,15 +7,17 @@ use failure::Error; use graphql_parser; use graphql_parser::{ query::Name, - schema::{self, InterfaceType, ObjectType, TypeDefinition}, + schema::{self, InterfaceType, ObjectType, TypeDefinition, Value}, Pos, }; use std::collections::BTreeMap; use std::iter::FromIterator; +pub const SUBGRAPH_SCHEMA_TYPE_NAME: &str = "_SubgraphSchema_"; + pub enum SchemaReference { ByName(String), - ByHash(String), + ById(String), } /// A validated and preprocessed GraphQL schema for a subgraph. @@ -114,6 +116,42 @@ impl Schema { Ok(schema) } + pub fn imported_schemas(&self) -> Vec { + self.subgraph_schema_object_type().map_or(vec![], |object| { + object + .directives + .iter() + .filter_map(|directive| directive.arguments.iter().find(|(name, _)| name == "from")) + .filter_map(|(_, value)| match value { + Value::Object(map) => { + let id = map + .get("id") + .filter(|id| match id { + Value::String(_) => true, + _ => false, + }) + .map(|id| match id { + Value::String(i) => SchemaReference::ById(i.to_string()), + _ => unreachable!(), + }); + let name = map + .get("name") + .filter(|name| match name { + Value::String(_) => true, + _ => false, + }) + .map(|name| match name { + Value::String(n) => SchemaReference::ByName(n.to_string()), + _ => unreachable!(), + }); + id.or(name) + } + _ => None, + }) + .collect() + }) + } + /// Returned map has one an entry for each interface in the schema. pub fn types_for_interface(&self) -> &BTreeMap> { &self.types_for_interface @@ -160,6 +198,22 @@ impl Schema { }; } } + + fn subgraph_schema_object_type(&self) -> Option<&ObjectType> { + self.document.definitions.iter().find_map(|def| match def { + schema::Definition::TypeDefinition(type_def) => match type_def { + schema::TypeDefinition::Object(object_type) => { + if object_type.name == SUBGRAPH_SCHEMA_TYPE_NAME { + Some(object_type) + } else { + None + } + } + _ => None, + }, + _ => None, + }) + } } #[test] From f70804a67d07c6a6f694e2436edeea188fd7ec1b Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Thu, 21 Nov 2019 17:36:39 -0600 Subject: [PATCH 05/64] core, graph: SchemaReferences are being resolved --- core/src/subgraph/registrar.rs | 77 +++++++++++++++++++++++++--- graph/src/data/graphql/validation.rs | 1 + graph/src/data/schema.rs | 77 +++++++++++++++++++++++++++- graph/src/data/subgraph/mod.rs | 16 +++++- 4 files changed, 161 insertions(+), 10 deletions(-) diff --git a/core/src/subgraph/registrar.rs b/core/src/subgraph/registrar.rs index a5bbdde6959..bff7cd7b3ad 100644 --- a/core/src/subgraph/registrar.rs +++ b/core/src/subgraph/registrar.rs @@ -14,6 +14,7 @@ lazy_static! { } use super::validation; +use graph::data::schema::{SchemaImportError, SchemaReference}; use graph::data::subgraph::schema::{ generate_entity_id, SubgraphDeploymentAssignmentEntity, SubgraphDeploymentEntity, SubgraphEntity, SubgraphVersionEntity, TypedEntity, @@ -40,7 +41,7 @@ impl SubgraphRegistrar where L: LinkResolver + Clone, P: SubgraphAssignmentProviderTrait, - S: Store, + S: Store + SubgraphDeploymentStore, CS: ChainStore, { pub fn new( @@ -275,7 +276,7 @@ impl SubgraphRegistrarTrait for SubgraphRegistrar where L: LinkResolver, P: SubgraphAssignmentProviderTrait, - S: Store, + S: Store + SubgraphDeploymentStore, CS: ChainStore, { fn create_subgraph( @@ -313,10 +314,9 @@ where logger.clone(), ) .map_err(SubgraphRegistrarError::ResolveError) - // .and_then(validation::validate_manifest) - .and_then(move |manifest| { - let manifest = manifest.0; - manifest + .and_then(move |unvalidated| { + unvalidated + .0 .network_name() .map_err(|e| SubgraphRegistrarError::ManifestValidationError(vec![e])) .and_then(move |network_name| { @@ -333,12 +333,47 @@ where network_name.clone(), )) .map(move |ethereum_adapter| { - (manifest, ethereum_adapter.clone(), chain_store.clone()) + (unvalidated, ethereum_adapter.clone(), chain_store.clone()) }) }) }) }) - .and_then(move |(manifest, ethereum_adapter, chain_store)| { + .and_then(move |(unvalidated, ethereum_adapter, chain_store)| { + // Get the correct store from the `chain_stores` + future::ok(resolve_schema_references( + &unvalidated.0.schema, + store.clone(), + )) + .and_then(|(schemas, import_errors)| { + // Separate errors and warning from SchemaImportError(s) + let failable_schema_errors: Vec = import_errors + .iter() + .filter(|err| SchemaImportError::is_failure(err)) + .map(|err| err.clone()) + .collect(); + let schema_import_warnings: Vec = import_errors + .iter() + .filter(|err| !SchemaImportError::is_failure(err)) + .map(|err| err.clone()) + .collect(); + + // Validate the unvalidated manifest + + future::ok((schemas, import_errors)) + }) + .map(move |(schemas, import_errors)| { + // Call unvalidate.validate(schemas) + (unvalidated.0, ethereum_adapter, chain_store, store) + }) + + // QUESTION: What should be done with the errors here? + // + + // Validate the UnvalidatedSubgraphManifest + + // future::ok((unvalidated.0, ethereum_adapter, chain_store, store)) + }) + .and_then(move |(manifest, ethereum_adapter, chain_store, store)| { let manifest_id = manifest.id.clone(); create_subgraph_version( &logger2, @@ -388,6 +423,32 @@ where } } +fn resolve_schema_references( + schema: &Schema, + store: Arc, +) -> ( + HashMap>, + Vec, +) { + schema.imported_schemas().into_iter().fold( + (HashMap::new(), vec![]), + |(mut schemas, mut errors), schema_ref| { + match schema_ref.clone().resolve(store.clone()) { + Ok(schema) => { + let (s, e) = resolve_schema_references(&schema, store.clone()); + schemas.insert(schema_ref, schema); + schemas.extend(s); + errors.extend(e); + } + Err(err) => { + errors.push(err); + } + } + (schemas, errors) + }, + ) +} + fn handle_assignment_event

( event: AssignmentEvent, provider: Arc

, diff --git a/graph/src/data/graphql/validation.rs b/graph/src/data/graphql/validation.rs index 710c51e36e0..83ab28b5678 100644 --- a/graph/src/data/graphql/validation.rs +++ b/graph/src/data/graphql/validation.rs @@ -1,3 +1,4 @@ +use crate::data::subgraph::{SubgraphDeploymentId, SubgraphName}; use crate::prelude::Fail; use graphql_parser::schema::*; use serde::{Deserialize, Serialize}; diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 4eceadcd7b6..4c775c0e513 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -1,8 +1,11 @@ +use crate::components::store::{Store, SubgraphDeploymentStore}; use crate::data::graphql::validation::{ get_object_type_definitions, validate_interface_implementation, validate_schema, SchemaValidationError, }; -use crate::data::subgraph::SubgraphDeploymentId; +use crate::data::subgraph::{SubgraphDeploymentId, SubgraphName}; +use crate::prelude::future::{self, *}; +use crate::prelude::Fail; use failure::Error; use graphql_parser; use graphql_parser::{ @@ -10,16 +13,88 @@ use graphql_parser::{ schema::{self, InterfaceType, ObjectType, TypeDefinition, Value}, Pos, }; + use std::collections::BTreeMap; +use std::fmt; +use std::hash::{Hash, Hasher}; use std::iter::FromIterator; +use std::sync::Arc; pub const SUBGRAPH_SCHEMA_TYPE_NAME: &str = "_SubgraphSchema_"; +#[derive(Debug, Fail, PartialEq, Eq, Clone)] +pub enum SchemaImportError { + #[fail(display = "Schema for imported subgraph `{}` was not found", _0)] + ImportedSchemaNotFound(SchemaReference), + #[fail(display = "Subgraph for imported schema `{}` is not deployed", _0)] + ImportedSubgraphNotFound(SchemaReference), + #[fail(display = "Name for imported subgraph `{}` is invalid", _0)] + ImportedSubgraphNameInvalid(String), + #[fail(display = "Id for imported subgraph `{}` is invalid", _0)] + ImportedSubgraphIdInvalid(String), +} + +impl SchemaImportError { + pub fn is_failure(error: &Self) -> bool { + match error { + SchemaImportError::ImportedSubgraphNameInvalid(_) + | SchemaImportError::ImportedSubgraphIdInvalid(_) => true, + _ => false, + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] pub enum SchemaReference { ByName(String), ById(String), } +impl Hash for SchemaReference { + fn hash(&self, state: &mut H) { + match self { + Self::ById(id) => id.hash(state), + Self::ByName(name) => name.hash(state), + }; + } +} + +impl fmt::Display for SchemaReference { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + match self { + SchemaReference::ByName(name) => write!(f, "{}", name), + SchemaReference::ById(id) => write!(f, "{}", id), + } + } +} + +impl SchemaReference { + pub fn resolve( + self, + store: Arc, + ) -> Result, SchemaImportError> { + let subgraph_id = match &self { + SchemaReference::ByName(name) => { + let subgraph_name = SubgraphName::new(name.clone()) + .map_err(|err| SchemaImportError::ImportedSubgraphNameInvalid(name.clone()))?; + store + .resolve_subgraph_name_to_id(subgraph_name.clone()) + .map_err(|_| SchemaImportError::ImportedSubgraphNotFound(self.clone())) + .and_then(|subgraph_id_opt| { + subgraph_id_opt + .ok_or(SchemaImportError::ImportedSubgraphNotFound(self.clone())) + })? + } + SchemaReference::ById(id) => SubgraphDeploymentId::new(id.clone()) + .map_err(|err| SchemaImportError::ImportedSubgraphIdInvalid(id.clone()))?, + }; + + store + .input_schema(&subgraph_id) + .map_err(|err| SchemaImportError::ImportedSchemaNotFound(self.clone())) + } +} + /// A validated and preprocessed GraphQL schema for a subgraph. #[derive(Clone, Debug, PartialEq)] pub struct Schema { diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index d804dda92f5..f9b08f0b1c3 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -18,7 +18,7 @@ use web3::types::{Address, H256}; use crate::components::link_resolver::LinkResolver; use crate::components::store::StoreError; use crate::data::query::QueryExecutionError; -use crate::data::schema::Schema; +use crate::data::schema::{Schema, SchemaImportError, SchemaReference}; use crate::data::subgraph::schema::{ EthereumBlockHandlerEntity, EthereumCallHandlerEntity, EthereumContractAbiEntity, EthereumContractDataSourceEntity, EthereumContractDataSourceTemplateEntity, @@ -333,6 +333,8 @@ pub enum SubgraphManifestValidationError { DataSourceBlockHandlerLimitExceeded, #[fail(display = "the specified block must exist on the Ethereum network")] BlockNotFound(String), + #[fail(display = "imported schemas are invalid")] + SchemaImportErrors(Vec), } #[derive(Fail, Debug)] @@ -839,8 +841,20 @@ impl UnvalidatedSubgraphManifest { &self, _logger: Logger, ) -> impl Future> { + // Implement UnvalidatedSubgraphManifest::validate method HashMap> -> Result<(SubgraphManifest, Vec), Vec> + // Should include all logic in core/src/subgraph/validation.rs + // Should include all logic in graph/src/data/graphql/validation.rs + // Should validate that all types in the Subgraph referenced from other subgraphs exist + // If the referenced subgraph is not provided as an argument, do not validate those types + // Should validate that import directives are properly formed + // Should that import directives only exist on the _SubgraphSchema_ type + // _SubgraphSchema_ type should not have fields return future::ok(()); } + + pub fn imported_schemas(&self) -> Vec { + self.0.schema.imported_schemas() + } } impl SubgraphManifest { From 01f55e1eae1bdcedac2b0c8ecee147da58536a0a Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 25 Nov 2019 15:40:12 -0600 Subject: [PATCH 06/64] core, graph: Validation of imported subgraphs --- core/src/subgraph/mod.rs | 1 - core/src/subgraph/registrar.rs | 16 +- core/src/subgraph/validation.rs | 59 ---- graph/src/data/graphql/mod.rs | 2 +- graph/src/data/graphql/traversal.rs | 127 +++++++ graph/src/data/graphql/validation.rs | 349 ------------------- graph/src/data/schema.rs | 504 ++++++++++++++++++++++++--- graph/src/data/subgraph/mod.rs | 98 ++++-- graphql/src/execution/execution.rs | 6 +- 9 files changed, 667 insertions(+), 495 deletions(-) delete mode 100644 core/src/subgraph/validation.rs create mode 100644 graph/src/data/graphql/traversal.rs delete mode 100644 graph/src/data/graphql/validation.rs diff --git a/core/src/subgraph/mod.rs b/core/src/subgraph/mod.rs index 0c595863a8a..25a214a136a 100644 --- a/core/src/subgraph/mod.rs +++ b/core/src/subgraph/mod.rs @@ -3,7 +3,6 @@ mod instance_manager; mod loader; mod provider; mod registrar; -mod validation; pub use self::instance::SubgraphInstance; pub use self::instance_manager::SubgraphInstanceManager; diff --git a/core/src/subgraph/registrar.rs b/core/src/subgraph/registrar.rs index bff7cd7b3ad..7ae21c4cf40 100644 --- a/core/src/subgraph/registrar.rs +++ b/core/src/subgraph/registrar.rs @@ -13,7 +13,6 @@ lazy_static! { ); } -use super::validation; use graph::data::schema::{SchemaImportError, SchemaReference}; use graph::data::subgraph::schema::{ generate_entity_id, SubgraphDeploymentAssignmentEntity, SubgraphDeploymentEntity, @@ -339,7 +338,6 @@ where }) }) .and_then(move |(unvalidated, ethereum_adapter, chain_store)| { - // Get the correct store from the `chain_stores` future::ok(resolve_schema_references( &unvalidated.0.schema, store.clone(), @@ -352,26 +350,18 @@ where .map(|err| err.clone()) .collect(); let schema_import_warnings: Vec = import_errors - .iter() + .into_iter() .filter(|err| !SchemaImportError::is_failure(err)) - .map(|err| err.clone()) .collect(); // Validate the unvalidated manifest - - future::ok((schemas, import_errors)) + // unvalidated.validate(schemas) + future::ok((schemas, schema_import_warnings)) }) .map(move |(schemas, import_errors)| { // Call unvalidate.validate(schemas) (unvalidated.0, ethereum_adapter, chain_store, store) }) - - // QUESTION: What should be done with the errors here? - // - - // Validate the UnvalidatedSubgraphManifest - - // future::ok((unvalidated.0, ethereum_adapter, chain_store, store)) }) .and_then(move |(manifest, ethereum_adapter, chain_store, store)| { let manifest_id = manifest.id.clone(); diff --git a/core/src/subgraph/validation.rs b/core/src/subgraph/validation.rs deleted file mode 100644 index 1fd013da160..00000000000 --- a/core/src/subgraph/validation.rs +++ /dev/null @@ -1,59 +0,0 @@ -use graph::prelude::*; - -pub fn validate_manifest( - manifest: SubgraphManifest, -) -> Result { - let mut errors: Vec = Vec::new(); - - // Validate that the manifest has at least one data source - if manifest.data_sources.is_empty() { - errors.push(SubgraphManifestValidationError::NoDataSources); - } - - // Validate that the manifest has a `source` address in each data source - // which has call or block handlers - let has_invalid_data_source = manifest.data_sources.iter().any(|data_source| { - let no_source_address = data_source.source.address.is_none(); - let has_call_handlers = !data_source.mapping.call_handlers.is_empty(); - let has_block_handlers = !data_source.mapping.block_handlers.is_empty(); - - no_source_address && (has_call_handlers || has_block_handlers) - }); - - if has_invalid_data_source { - errors.push(SubgraphManifestValidationError::SourceAddressRequired) - } - - // Validate that there are no more than one of each type of - // block_handler in each data source. - let has_too_many_block_handlers = manifest.data_sources.iter().any(|data_source| { - if data_source.mapping.block_handlers.is_empty() { - return false; - } - - let mut non_filtered_block_handler_count = 0; - let mut call_filtered_block_handler_count = 0; - data_source - .mapping - .block_handlers - .iter() - .for_each(|block_handler| { - if block_handler.filter.is_none() { - non_filtered_block_handler_count += 1 - } else { - call_filtered_block_handler_count += 1 - } - }); - return non_filtered_block_handler_count > 1 || call_filtered_block_handler_count > 1; - }); - - if has_too_many_block_handlers { - errors.push(SubgraphManifestValidationError::DataSourceBlockHandlerLimitExceeded) - } - - if errors.is_empty() { - return Ok(manifest); - } - - return Err(SubgraphRegistrarError::ManifestValidationError(errors)); -} diff --git a/graph/src/data/graphql/mod.rs b/graph/src/data/graphql/mod.rs index a94969c935e..72d2d26e2ba 100644 --- a/graph/src/data/graphql/mod.rs +++ b/graph/src/data/graphql/mod.rs @@ -1,7 +1,7 @@ mod serialization; /// Utilities for validating GraphQL schemas. -pub mod validation; +pub mod traversal; /// Utilities for working with GraphQL values. mod values; diff --git a/graph/src/data/graphql/traversal.rs b/graph/src/data/graphql/traversal.rs new file mode 100644 index 00000000000..60f4492054f --- /dev/null +++ b/graph/src/data/graphql/traversal.rs @@ -0,0 +1,127 @@ +use crate::data::subgraph::{SubgraphDeploymentId, SubgraphName}; +use crate::prelude::Fail; +use graphql_parser::schema::*; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use std::fmt; + +/// Returns all object type definitions in the schema. +pub fn get_object_type_definitions(schema: &Document) -> Vec<&ObjectType> { + schema + .definitions + .iter() + .filter_map(|d| match d { + Definition::TypeDefinition(TypeDefinition::Object(t)) => Some(t), + _ => None, + }) + .collect() +} + +/// Returns all object and interface type definitions in the schema. +pub fn get_object_and_interface_type_fields(schema: &Document) -> HashMap<&Name, &Vec> { + schema + .definitions + .iter() + .filter_map(|d| match d { + Definition::TypeDefinition(TypeDefinition::Object(t)) => Some((&t.name, &t.fields)), + Definition::TypeDefinition(TypeDefinition::Interface(t)) => Some((&t.name, &t.fields)), + _ => None, + }) + .collect() +} + +/// Looks up a directive in a object type, if it is provided. +pub fn get_object_type_directive(object_type: &ObjectType, name: Name) -> Option<&Directive> { + object_type + .directives + .iter() + .find(|directive| directive.name == name) +} + +/// Returns the underlying type for a GraphQL field type +pub fn get_base_type(field_type: &Type) -> &Name { + match field_type { + Type::NamedType(name) => name, + Type::NonNullType(inner) => get_base_type(&inner), + Type::ListType(inner) => get_base_type(&inner), + } +} + +pub fn find_interface<'a>(schema: &'a Document, name: &str) -> Option<&'a InterfaceType> { + schema.definitions.iter().find_map(|d| match d { + Definition::TypeDefinition(TypeDefinition::Interface(t)) if t.name == name => Some(t), + _ => None, + }) +} + +pub fn find_derived_from<'a>(field: &'a Field) -> Option<&'a Directive> { + field + .directives + .iter() + .find(|dir| dir.name == "derivedFrom") +} + +// #[test] +// fn test_derived_from_validation() { +// const OTHER_TYPES: &str = " +// type B @entity { id: ID! } +// type C @entity { id: ID! } +// type D @entity { id: ID! } +// type E @entity { id: ID! } +// type F @entity { id: ID! } +// type G @entity { id: ID! a: BigInt } +// type H @entity { id: ID! a: A! } +// # This sets up a situation where we need to allow `Transaction.from` to +// # point to an interface because of `Account.txn` +// type Transaction @entity { from: Address! } +// interface Address { txn: Transaction! @derivedFrom(field: \"from\") } +// type Account implements Address @entity { id: ID!, txn: Transaction! @derivedFrom(field: \"from\") }"; + +// fn validate(field: &str, errmsg: &str) { +// let raw = format!("type A @entity {{ id: ID!\n {} }}\n{}", field, OTHER_TYPES); + +// let document = graphql_parser::parse_schema(&raw).expect("Failed to parse raw schema"); +// match validate_derived_from(&document) { +// Err(ref e) => match e { +// SchemaValidationError::DerivedFromInvalid(_, _, msg) => assert_eq!(errmsg, msg), +// _ => panic!("expected variant SchemaValidationError::DerivedFromInvalid"), +// }, +// Ok(_) => { +// if errmsg != "ok" { +// panic!("expected validation for `{}` to fail", field) +// } +// } +// } +// } + +// validate( +// "b: B @derivedFrom(field: \"a\")", +// "field `a` does not exist on type `B`", +// ); +// validate( +// "c: [C!]! @derivedFrom(field: \"a\")", +// "field `a` does not exist on type `C`", +// ); +// validate( +// "d: D @derivedFrom", +// "the @derivedFrom directive must have a `field` argument", +// ); +// validate( +// "e: E @derivedFrom(attr: \"a\")", +// "the @derivedFrom directive must have a `field` argument", +// ); +// validate( +// "f: F @derivedFrom(field: 123)", +// "the value of the @derivedFrom `field` argument must be a string", +// ); +// validate( +// "g: G @derivedFrom(field: \"a\")", +// "field `a` on type `G` must have one of the following types: A, A!, [A!], [A!]!", +// ); +// validate("h: H @derivedFrom(field: \"a\")", "ok"); +// validate( +// "i: NotAType @derivedFrom(field: \"a\")", +// "the type of the field must be an existing entity or interface type", +// ); +// validate("j: B @derivedFrom(field: \"id\")", "ok"); +// } diff --git a/graph/src/data/graphql/validation.rs b/graph/src/data/graphql/validation.rs deleted file mode 100644 index 83ab28b5678..00000000000 --- a/graph/src/data/graphql/validation.rs +++ /dev/null @@ -1,349 +0,0 @@ -use crate::data::subgraph::{SubgraphDeploymentId, SubgraphName}; -use crate::prelude::Fail; -use graphql_parser::schema::*; -use serde::{Deserialize, Serialize}; -use std::collections::HashMap; -use std::fmt; - -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub struct Strings(Vec); - -impl fmt::Display for Strings { - fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { - let s = (&self.0).join(", "); - write!(f, "{}", s) - } -} - -#[derive(Debug, Fail, PartialEq, Eq)] -pub enum SchemaValidationError { - #[fail(display = "Interface {} not defined", _0)] - UndefinedInterface(String), - - #[fail(display = "@entity directive missing on the following types: {}", _0)] - EntityDirectivesMissing(Strings), - - #[fail( - display = "Entity type `{}` cannot implement `{}` because it is missing \ - the required fields: {}", - _0, _1, _2 - )] - CannotImplement(String, String, Strings), // (type, interface, missing_fields) - #[fail( - display = "Field `{}` in type `{}` has invalid @derivedFrom: {}", - _1, _0, _2 - )] - DerivedFromInvalid(String, String, String), // (type, field, reason) -} - -/// Validates whether a GraphQL schema is compatible with The Graph. -pub(crate) fn validate_schema(schema: &Document) -> Result<(), SchemaValidationError> { - validate_schema_types(schema)?; - validate_derived_from(schema) -} - -/// Validates whether all object types in the schema are declared with an @entity directive. -fn validate_schema_types(schema: &Document) -> Result<(), SchemaValidationError> { - use self::SchemaValidationError::*; - - let types_without_entity_directive = get_object_type_definitions(schema) - .iter() - .filter(|t| get_object_type_directive(t, String::from("entity")).is_none()) - .map(|t| t.name.to_owned()) - .collect::>(); - - if types_without_entity_directive.is_empty() { - Ok(()) - } else { - Err(EntityDirectivesMissing(Strings( - types_without_entity_directive, - ))) - } -} - -/// Validate `interfaceethat `object` implements `interface`. -pub(crate) fn validate_interface_implementation( - object: &ObjectType, - interface: &InterfaceType, -) -> Result<(), SchemaValidationError> { - // Check that all fields in the interface exist in the object with same name and type. - let mut missing_fields = vec![]; - for i in &interface.fields { - if object - .fields - .iter() - .find(|o| o.name == i.name && o.field_type == i.field_type) - .is_none() - { - missing_fields.push(i.to_string().trim().to_owned()); - } - } - if !missing_fields.is_empty() { - Err(SchemaValidationError::CannotImplement( - object.name.clone(), - interface.name.clone(), - Strings(missing_fields), - )) - } else { - Ok(()) - } -} - -/// Returns all object type definitions in the schema. -pub fn get_object_type_definitions(schema: &Document) -> Vec<&ObjectType> { - schema - .definitions - .iter() - .filter_map(|d| match d { - Definition::TypeDefinition(TypeDefinition::Object(t)) => Some(t), - _ => None, - }) - .collect() -} - -/// Returns all object and interface type definitions in the schema. -pub fn get_object_and_interface_type_fields(schema: &Document) -> HashMap<&Name, &Vec> { - schema - .definitions - .iter() - .filter_map(|d| match d { - Definition::TypeDefinition(TypeDefinition::Object(t)) => Some((&t.name, &t.fields)), - Definition::TypeDefinition(TypeDefinition::Interface(t)) => Some((&t.name, &t.fields)), - _ => None, - }) - .collect() -} - -/// Looks up a directive in a object type, if it is provided. -pub fn get_object_type_directive(object_type: &ObjectType, name: Name) -> Option<&Directive> { - object_type - .directives - .iter() - .find(|directive| directive.name == name) -} - -/// Returns the underlying type for a GraphQL field type -pub fn get_base_type(field_type: &Type) -> &Name { - match field_type { - Type::NamedType(name) => name, - Type::NonNullType(inner) => get_base_type(&inner), - Type::ListType(inner) => get_base_type(&inner), - } -} - -fn find_interface<'a>(schema: &'a Document, name: &str) -> Option<&'a InterfaceType> { - schema.definitions.iter().find_map(|d| match d { - Definition::TypeDefinition(TypeDefinition::Interface(t)) if t.name == name => Some(t), - _ => None, - }) -} - -fn find_derived_from<'a>(field: &'a Field) -> Option<&'a Directive> { - field - .directives - .iter() - .find(|dir| dir.name == "derivedFrom") -} - -/// Check `@derivedFrom` annotations for various problems. This follows the -/// corresponding checks in graph-cli -fn validate_derived_from(schema: &Document) -> Result<(), SchemaValidationError> { - // Helper to construct a DerivedFromInvalid - fn invalid(object_type: &ObjectType, field_name: &str, reason: &str) -> SchemaValidationError { - SchemaValidationError::DerivedFromInvalid( - object_type.name.to_owned(), - field_name.to_owned(), - reason.to_owned(), - ) - } - - let type_definitions = get_object_type_definitions(schema); - let object_and_interface_type_fields = get_object_and_interface_type_fields(schema); - - // Iterate over all derived fields in all entity types; include the - // interface types that the entity with the `@derivedFrom` implements - // and the `field` argument of @derivedFrom directive - for (object_type, interface_types, field, target_field) in type_definitions - .clone() - .iter() - .flat_map(|object_type| { - object_type - .fields - .iter() - .map(move |field| (object_type, field)) - }) - .filter_map(|(object_type, field)| { - find_derived_from(field).map(|directive| { - ( - object_type, - object_type - .implements_interfaces - .iter() - .filter(|iface| { - // Any interface that has `field` can be used - // as the type of the field - find_interface(schema, iface) - .map(|iface| { - iface.fields.iter().any(|ifield| ifield.name == field.name) - }) - .unwrap_or(false) - }) - .collect::>(), - field, - directive - .arguments - .iter() - .find(|(name, _)| name == "field") - .map(|(_, value)| value), - ) - }) - }) - { - // Turn `target_field` into the string name of the field - let target_field = target_field.ok_or_else(|| { - invalid( - object_type, - &field.name, - "the @derivedFrom directive must have a `field` argument", - ) - })?; - let target_field = match target_field { - Value::String(s) => s, - _ => { - return Err(invalid( - object_type, - &field.name, - "the value of the @derivedFrom `field` argument must be a string", - )) - } - }; - - // Check that the type we are deriving from exists - let target_type_name = get_base_type(&field.field_type); - let target_fields = object_and_interface_type_fields - .get(target_type_name) - .ok_or_else(|| { - invalid( - object_type, - &field.name, - "the type of the field must be an existing entity or interface type", - ) - })?; - - // Check that the type we are deriving from has a field with the - // right name and type - let target_field = target_fields - .iter() - .find(|field| &field.name == target_field) - .ok_or_else(|| { - let msg = format!( - "field `{}` does not exist on type `{}`", - target_field, target_type_name - ); - invalid(object_type, &field.name, &msg) - })?; - - // The field we are deriving from has to point back to us; as an - // exception, we allow deriving from the `id` of another type. - // For that, we will wind up comparing the `id`s of the two types - // when we query, and just assume that that's ok. - let target_field_type = get_base_type(&target_field.field_type); - if target_field_type != &object_type.name - && target_field_type != "ID" - && !interface_types - .iter() - .any(|iface| &target_field_type == iface) - { - fn type_signatures(name: &String) -> Vec { - vec![ - format!("{}", name), - format!("{}!", name), - format!("[{}!]", name), - format!("[{}!]!", name), - ] - }; - - let mut valid_types = type_signatures(&object_type.name); - valid_types.extend( - interface_types - .iter() - .flat_map(|iface| type_signatures(iface)), - ); - let valid_types = valid_types.join(", "); - - let msg = format!( - "field `{tf}` on type `{tt}` must have one of the following types: {valid_types}", - tf = target_field.name, - tt = target_type_name, - valid_types = valid_types, - ); - return Err(invalid(object_type, &field.name, &msg)); - } - } - Ok(()) -} - -#[test] -fn test_derived_from_validation() { - const OTHER_TYPES: &str = " -type B @entity { id: ID! } -type C @entity { id: ID! } -type D @entity { id: ID! } -type E @entity { id: ID! } -type F @entity { id: ID! } -type G @entity { id: ID! a: BigInt } -type H @entity { id: ID! a: A! } -# This sets up a situation where we need to allow `Transaction.from` to -# point to an interface because of `Account.txn` -type Transaction @entity { from: Address! } -interface Address { txn: Transaction! @derivedFrom(field: \"from\") } -type Account implements Address @entity { id: ID!, txn: Transaction! @derivedFrom(field: \"from\") }"; - - fn validate(field: &str, errmsg: &str) { - let raw = format!("type A @entity {{ id: ID!\n {} }}\n{}", field, OTHER_TYPES); - - let document = graphql_parser::parse_schema(&raw).expect("Failed to parse raw schema"); - match validate_derived_from(&document) { - Err(ref e) => match e { - SchemaValidationError::DerivedFromInvalid(_, _, msg) => assert_eq!(errmsg, msg), - _ => panic!("expected variant SchemaValidationError::DerivedFromInvalid"), - }, - Ok(_) => { - if errmsg != "ok" { - panic!("expected validation for `{}` to fail", field) - } - } - } - } - - validate( - "b: B @derivedFrom(field: \"a\")", - "field `a` does not exist on type `B`", - ); - validate( - "c: [C!]! @derivedFrom(field: \"a\")", - "field `a` does not exist on type `C`", - ); - validate( - "d: D @derivedFrom", - "the @derivedFrom directive must have a `field` argument", - ); - validate( - "e: E @derivedFrom(attr: \"a\")", - "the @derivedFrom directive must have a `field` argument", - ); - validate( - "f: F @derivedFrom(field: 123)", - "the value of the @derivedFrom `field` argument must be a string", - ); - validate( - "g: G @derivedFrom(field: \"a\")", - "field `a` on type `G` must have one of the following types: A, A!, [A!], [A!]!", - ); - validate("h: H @derivedFrom(field: \"a\")", "ok"); - validate( - "i: NotAType @derivedFrom(field: \"a\")", - "the type of the field must be an existing entity or interface type", - ); - validate("j: B @derivedFrom(field: \"id\")", "ok"); -} diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 4c775c0e513..04a7679d283 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -1,20 +1,19 @@ use crate::components::store::{Store, SubgraphDeploymentStore}; -use crate::data::graphql::validation::{ - get_object_type_definitions, validate_interface_implementation, validate_schema, - SchemaValidationError, -}; +use crate::data::graphql::traversal; use crate::data::subgraph::{SubgraphDeploymentId, SubgraphName}; use crate::prelude::future::{self, *}; use crate::prelude::Fail; + use failure::Error; use graphql_parser; use graphql_parser::{ - query::Name, - schema::{self, InterfaceType, ObjectType, TypeDefinition, Value}, + query::{Name, Value}, + schema::{self, InterfaceType, ObjectType, TypeDefinition, *}, Pos, }; +use serde::{Deserialize, Serialize}; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use std::fmt; use std::hash::{Hash, Hasher}; use std::iter::FromIterator; @@ -22,6 +21,43 @@ use std::sync::Arc; pub const SUBGRAPH_SCHEMA_TYPE_NAME: &str = "_SubgraphSchema_"; +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct Strings(Vec); + +impl fmt::Display for Strings { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + let s = (&self.0).join(", "); + write!(f, "{}", s) + } +} + +#[derive(Debug, Fail, PartialEq, Eq)] +pub enum SchemaValidationError { + #[fail(display = "Interface {} not defined", _0)] + UndefinedInterface(String), + + #[fail(display = "@entity directive missing on the following type: {}", _0)] + EntityDirectivesMissing(Strings), + + #[fail( + display = "Entity type `{}` cannot implement `{}` because it is missing \ + the required fields: {}", + _0, _1, _2 + )] + CannotImplement(String, String, Strings), // (type, interface, missing_fields) + #[fail( + display = "Field `{}` in type `{}` has invalid @derivedFrom: {}", + _1, _0, _2 + )] + DerivedFromInvalid(String, String, String), // (type, field, reason) + #[fail(display = "_SubgraphSchema_ type is solely for imports and should have no fields")] + SubgraphSchemaTypeFieldsInvalid, + #[fail(display = "_SubgraphSchema_ type only allows @import directives")] + SubgraphSchemaDirectivesInvalid, + #[fail(display = "@import defined incorrectly")] + ImportDirectiveInvalid, +} + #[derive(Debug, Fail, PartialEq, Eq, Clone)] pub enum SchemaImportError { #[fail(display = "Schema for imported subgraph `{}` was not found", _0)] @@ -44,6 +80,33 @@ impl SchemaImportError { } } +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ImportedType { + Name(String), + NameAs(String, String), +} + +impl Hash for ImportedType { + fn hash(&self, state: &mut H) { + match self { + Self::Name(name) => name.hash(state), + Self::NameAs(name, az) => { + name.hash(state); + az.hash(state); + } + }; + } +} + +impl fmt::Display for ImportedType { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + match self { + Self::Name(name) => write!(f, "{}", name), + Self::NameAs(name, az) => write!(f, "name: {}, as: {}", name, az), + } + } +} + #[derive(Clone, Debug, PartialEq, Eq)] pub enum SchemaReference { ByName(String), @@ -141,7 +204,7 @@ impl Schema { })); let mut interfaces_for_type = BTreeMap::<_, Vec<_>>::new(); - for object_type in get_object_type_definitions(&document) { + for object_type in traversal::get_object_type_definitions(&document) { for implemented_interface in object_type.implements_interfaces.clone() { let interface_type = document .definitions @@ -158,7 +221,7 @@ impl Schema { SchemaValidationError::UndefinedInterface(implemented_interface.clone()) })?; - validate_interface_implementation(object_type, &interface_type)?; + Self::validate_interface_implementation(object_type, &interface_type)?; interfaces_for_type .entry(object_type.name.clone()) @@ -176,7 +239,8 @@ impl Schema { pub fn parse(raw: &str, id: SubgraphDeploymentId) -> Result { let document = graphql_parser::parse_schema(&raw)?; - validate_schema(&document)?; + // TODO: Decide if we want to keep this here + // validate_schema(&document)?; let (interfaces_for_type, types_for_interface) = Self::collect_interfaces(&document)?; @@ -191,42 +255,124 @@ impl Schema { Ok(schema) } + pub fn imported_types(&self) -> HashMap { + self.subgraph_schema_object_type() + .map_or(HashMap::new(), |object| { + object + .directives + .iter() + .filter(|directive| directive.name == "imports") + .map(|imports| { + imports + .arguments + .iter() + .find(|(name, _)| name == "from") + .map_or(vec![], |from| { + self.schema_reference_from_directive_argument(from).map_or( + vec![], + |schema_ref| { + self.imported_types_from_import_directive(imports) + .iter() + .map(|imported_type| { + (imported_type.clone(), schema_ref.clone()) + }) + .collect() + }, + ) + }) + }) + .flatten() + .collect::>() + }) + } + pub fn imported_schemas(&self) -> Vec { self.subgraph_schema_object_type().map_or(vec![], |object| { object .directives .iter() + .filter(|directive| directive.name == "imports") .filter_map(|directive| directive.arguments.iter().find(|(name, _)| name == "from")) - .filter_map(|(_, value)| match value { - Value::Object(map) => { - let id = map - .get("id") - .filter(|id| match id { - Value::String(_) => true, - _ => false, - }) - .map(|id| match id { - Value::String(i) => SchemaReference::ById(i.to_string()), - _ => unreachable!(), - }); - let name = map - .get("name") - .filter(|name| match name { - Value::String(_) => true, - _ => false, - }) - .map(|name| match name { - Value::String(n) => SchemaReference::ByName(n.to_string()), - _ => unreachable!(), - }); - id.or(name) - } - _ => None, - }) + .filter_map(|from| self.schema_reference_from_directive_argument(from)) .collect() }) } + fn imported_types_from_import_directive(&self, imports: &Directive) -> Vec { + imports + .arguments + .iter() + .find(|(name, _)| name == "types") + .filter(|(_, value)| match value { + Value::List(_) => true, + _ => false, + }) + .map(|(_, value)| match value { + Value::List(types) => types + .iter() + .filter_map(|import_type| match import_type { + Value::String(type_name) => Some(ImportedType::Name(type_name.to_string())), + Value::Object(type_name_as) => { + let name = + type_name_as + .get("name") + .and_then(|name_value| match name_value { + Value::String(name) => Some(name.to_string()), + _ => None, + }); + let az = type_name_as.get("as").and_then(|as_value| match as_value { + Value::String(az) => Some(az.to_string()), + _ => None, + }); + match (name, az) { + (Some(name), Some(az)) => Some(ImportedType::NameAs(name, az)), + _ => None, + } + } + _ => None, + }) + .collect(), + _ => unreachable!(), + }) + .unwrap_or(vec![]) + } + + fn schema_reference_from_directive_argument( + &self, + from: &(Name, Value), + ) -> Option { + let (name, value) = from; + if name != "from" { + return None; + } + match value { + Value::Object(map) => { + let id = map + .get("id") + .filter(|id| match id { + Value::String(_) => true, + _ => false, + }) + .map(|id| match id { + Value::String(i) => SchemaReference::ById(i.to_string()), + _ => unreachable!(), + }); + let name = map + .get("name") + .filter(|name| match name { + Value::String(_) => true, + _ => false, + }) + .map(|name| match name { + Value::String(n) => SchemaReference::ByName(n.to_string()), + _ => unreachable!(), + }); + id.or(name) + } + _ => None, + } + } + /// Returned map has one an entry for each interface in the schema. pub fn types_for_interface(&self) -> &BTreeMap> { &self.types_for_interface @@ -274,20 +420,280 @@ impl Schema { } } - fn subgraph_schema_object_type(&self) -> Option<&ObjectType> { - self.document.definitions.iter().find_map(|def| match def { - schema::Definition::TypeDefinition(type_def) => match type_def { - schema::TypeDefinition::Object(object_type) => { - if object_type.name == SUBGRAPH_SCHEMA_TYPE_NAME { - Some(object_type) - } else { - None - } + pub fn validate( + &self, + schemas: &HashMap>, + ) -> Result<(), Vec> { + let mut errors = vec![]; + // [X] Should include all logic in graph/src/data/graphql/validation.rs + self.validate_schema_types() + .unwrap_or_else(|err| errors.push(err)); + self.validate_derived_from() + .unwrap_or_else(|err| errors.push(err)); + // _SubgraphSchema_ type should not have fields + self.validate_subgraph_schema_has_no_fields() + .unwrap_or_else(|err| errors.push(err)); + // Should validate that import directives are properly formed + // Should that import directives only exist on the _SubgraphSchema_ type + self.validate_import_directives() + .unwrap_or_else(|err| errors.push(err)); + // Should validate that all types in the Subgraph referenced from other subgraphs exist + // If the referenced subgraph is not provided as an argument, do not validate those types + self.validate_imported_types(schemas) + .unwrap_or_else(|err| errors.push(err)); + + if errors.is_empty() { + Ok(()) + } else { + Err(errors) + } + } + + fn validate_subgraph_schema_has_no_fields(&self) -> Result<(), SchemaValidationError> { + match self + .subgraph_schema_object_type() + .and_then(|subgraph_schema_type| { + if !subgraph_schema_type.fields.is_empty() { + Some(SchemaValidationError::SubgraphSchemaTypeFieldsInvalid) + } else { + None } - _ => None, - }, - _ => None, - }) + }) { + Some(err) => Err(err), + None => Ok(()), + } + } + + fn validate_import_directives(&self) -> Result<(), SchemaValidationError> { + match self + .subgraph_schema_object_type() + .and_then(|subgraph_schema_type| { + if !subgraph_schema_type + .directives + .iter() + .filter(|directive| directive.name != "imports") + .collect::>() + .is_empty() + { + Some(SchemaValidationError::SubgraphSchemaDirectivesInvalid) + } else { + subgraph_schema_type + .directives + .iter() + .filter(|directive| directive.name == "imports") + // TODO: Fix + .find(|directive| true) + .map(|_| SchemaValidationError::ImportDirectiveInvalid) + } + }) { + Some(err) => Err(err), + None => Ok(()), + } + } + + fn validate_imported_types( + &self, + schemas: &HashMap>, + ) -> Result<(), SchemaValidationError> { + // Look up of all types in schema + let root_schema = traversal::get_object_and_interface_type_fields(&self.document); + + // Look up of ImportedType to SchemaReference + let imported_types = self.imported_types(); + + // Look up of SchemaReference to Option of all types in schema + + Ok(()) + } + + fn validate_schema_types(&self) -> Result<(), SchemaValidationError> { + let types_without_entity_directive = traversal::get_object_type_definitions(&self.document) + .iter() + .filter(|t| traversal::get_object_type_directive(t, String::from("entity")).is_none()) + .map(|t| t.name.to_owned()) + .collect::>(); + if types_without_entity_directive.is_empty() { + Ok(()) + } else { + Err(SchemaValidationError::EntityDirectivesMissing(Strings( + types_without_entity_directive, + ))) + } + } + + fn validate_derived_from(&self) -> Result<(), SchemaValidationError> { + // Helper to construct a DerivedFromInvalid + fn invalid( + object_type: &ObjectType, + field_name: &str, + reason: &str, + ) -> SchemaValidationError { + SchemaValidationError::DerivedFromInvalid( + object_type.name.to_owned(), + field_name.to_owned(), + reason.to_owned(), + ) + } + + let type_definitions = traversal::get_object_type_definitions(&self.document); + let object_and_interface_type_fields = + traversal::get_object_and_interface_type_fields(&self.document); + + // Iterate over all derived fields in all entity types; include the + // interface types that the entity with the `@derivedFrom` implements + // and the `field` argument of @derivedFrom directive + for (object_type, interface_types, field, target_field) in type_definitions + .clone() + .iter() + .flat_map(|object_type| { + object_type + .fields + .iter() + .map(move |field| (object_type, field)) + }) + .filter_map(|(object_type, field)| { + traversal::find_derived_from(field).map(|directive| { + ( + object_type, + object_type + .implements_interfaces + .iter() + .filter(|iface| { + // Any interface that has `field` can be used + // as the type of the field + traversal::find_interface(&self.document, iface) + .map(|iface| { + iface.fields.iter().any(|ifield| ifield.name == field.name) + }) + .unwrap_or(false) + }) + .collect::>(), + field, + directive + .arguments + .iter() + .find(|(name, _)| name == "field") + .map(|(_, value)| value), + ) + }) + }) + { + // Turn `target_field` into the string name of the field + let target_field = target_field.ok_or_else(|| { + invalid( + object_type, + &field.name, + "the @derivedFrom directive must have a `field` argument", + ) + })?; + let target_field = match target_field { + Value::String(s) => s, + _ => { + return Err(invalid( + object_type, + &field.name, + "the value of the @derivedFrom `field` argument must be a string", + )) + } + }; + + // Check that the type we are deriving from exists + let target_type_name = traversal::get_base_type(&field.field_type); + let target_fields = object_and_interface_type_fields + .get(target_type_name) + .ok_or_else(|| { + invalid( + object_type, + &field.name, + "the type of the field must be an existing entity or interface type", + ) + })?; + + // Check that the type we are deriving from has a field with the + // right name and type + let target_field = target_fields + .iter() + .find(|field| &field.name == target_field) + .ok_or_else(|| { + let msg = format!( + "field `{}` does not exist on type `{}`", + target_field, target_type_name + ); + invalid(object_type, &field.name, &msg) + })?; + + // The field we are deriving from has to point back to us; as an + // exception, we allow deriving from the `id` of another type. + // For that, we will wind up comparing the `id`s of the two types + // when we query, and just assume that that's ok. + let target_field_type = traversal::get_base_type(&target_field.field_type); + if target_field_type != &object_type.name + && target_field_type != "ID" + && !interface_types + .iter() + .any(|iface| &target_field_type == iface) + { + fn type_signatures(name: &String) -> Vec { + vec![ + format!("{}", name), + format!("{}!", name), + format!("[{}!]", name), + format!("[{}!]!", name), + ] + } + + let mut valid_types = type_signatures(&object_type.name); + valid_types.extend( + interface_types + .iter() + .flat_map(|iface| type_signatures(iface)), + ); + let valid_types = valid_types.join(", "); + + let msg = format!( + "field `{tf}` on type `{tt}` must have one of the following type: {valid_types}", + tf = target_field.name, + tt = target_type_name, + valid_types = valid_types, + ); + return Err(invalid(object_type, &field.name, &msg)); + } + } + Ok(()) + } + + /// Validate `interfaceethat `object` implements `interface`. + fn validate_interface_implementation( + object: &ObjectType, + interface: &InterfaceType, + ) -> Result<(), SchemaValidationError> { + // Check that all fields in the interface exist in the object with same name and type. + let mut missing_fields = vec![]; + for i in &interface.fields { + if object + .fields + .iter() + .find(|o| o.name == i.name && o.field_type == i.field_type) + .is_none() + { + missing_fields.push(i.to_string().trim().to_owned()); + } + } + if !missing_fields.is_empty() { + Err(SchemaValidationError::CannotImplement( + object.name.clone(), + interface.name.clone(), + Strings(missing_fields), + )) + } else { + Ok(()) + } + } + + fn subgraph_schema_object_type(&self) -> Option<&ObjectType> { + traversal::get_object_type_definitions(&self.document) + .into_iter() + .find(|object_type| object_type.name == SUBGRAPH_SCHEMA_TYPE_NAME) } } diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index f9b08f0b1c3..6bdc49d4b01 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -8,17 +8,13 @@ use serde::de; use serde::ser; use serde_yaml; use slog::{info, Logger}; -use std::fmt; -use std::ops::Deref; -use std::str::FromStr; -use std::sync::Arc; use tokio::prelude::*; use web3::types::{Address, H256}; use crate::components::link_resolver::LinkResolver; use crate::components::store::StoreError; use crate::data::query::QueryExecutionError; -use crate::data::schema::{Schema, SchemaImportError, SchemaReference}; +use crate::data::schema::{Schema, SchemaImportError, SchemaReference, SchemaValidationError}; use crate::data::subgraph::schema::{ EthereumBlockHandlerEntity, EthereumCallHandlerEntity, EthereumContractAbiEntity, EthereumContractDataSourceEntity, EthereumContractDataSourceTemplateEntity, @@ -28,6 +24,12 @@ use crate::data::subgraph::schema::{ use crate::prelude::{format_err, Deserialize, Fail, Serialize}; use crate::util::ethereum::string_to_h256; +use std::collections::HashMap; +use std::fmt; +use std::ops::Deref; +use std::str::FromStr; +use std::sync::Arc; + /// Rust representation of the GraphQL schema for a `SubgraphManifest`. pub mod schema; @@ -333,8 +335,12 @@ pub enum SubgraphManifestValidationError { DataSourceBlockHandlerLimitExceeded, #[fail(display = "the specified block must exist on the Ethereum network")] BlockNotFound(String), - #[fail(display = "imported schemas are invalid")] - SchemaImportErrors(Vec), + // TODO: Figure out how to get these error to properly show up + #[fail(display = "imported schema(s) are invalid: {:?}", _0)] + SchemaImportError(Vec), + // TODO: Figure out how to get these error to properly show up + #[fail(display = "schema validation failed: {:?}", _0)] + SchemaValidationError(Vec), } #[derive(Fail, Debug)] @@ -837,23 +843,75 @@ impl UnvalidatedSubgraphManifest { SubgraphManifest::resolve(link, resolver, logger).map(|manifest| Self(manifest)) } + pub fn imported_schemas(&self) -> Vec { + self.0.schema.imported_schemas() + } + pub fn validate( &self, _logger: Logger, - ) -> impl Future> { - // Implement UnvalidatedSubgraphManifest::validate method HashMap> -> Result<(SubgraphManifest, Vec), Vec> - // Should include all logic in core/src/subgraph/validation.rs - // Should include all logic in graph/src/data/graphql/validation.rs - // Should validate that all types in the Subgraph referenced from other subgraphs exist - // If the referenced subgraph is not provided as an argument, do not validate those types - // Should validate that import directives are properly formed - // Should that import directives only exist on the _SubgraphSchema_ type - // _SubgraphSchema_ type should not have fields - return future::ok(()); - } + schemas: HashMap>, + ) -> Result> { + let manifest = &self.0; - pub fn imported_schemas(&self) -> Vec { - self.0.schema.imported_schemas() + let mut errors: Vec = vec![]; + + // Validate that the manifest has at least one data source + if manifest.data_sources.is_empty() { + errors.push(SubgraphManifestValidationError::NoDataSources); + } + + // Validate that the manifest has a `source` address in each data source + // which has call or block handlers + if manifest.data_sources.iter().any(|data_source| { + let no_source_address = data_source.source.address.is_none(); + let has_call_handlers = !data_source.mapping.call_handlers.is_empty(); + let has_block_handlers = !data_source.mapping.block_handlers.is_empty(); + + no_source_address && (has_call_handlers || has_block_handlers) + }) { + errors.push(SubgraphManifestValidationError::SourceAddressRequired) + }; + + // Validate that there are no more than one of each type of + // block_handler in each data source. + let has_too_many_block_handlers = manifest.data_sources.iter().any(|data_source| { + if data_source.mapping.block_handlers.is_empty() { + return false; + } + + let mut non_filtered_block_handler_count = 0; + let mut call_filtered_block_handler_count = 0; + data_source + .mapping + .block_handlers + .iter() + .for_each(|block_handler| { + if block_handler.filter.is_none() { + non_filtered_block_handler_count += 1 + } else { + call_filtered_block_handler_count += 1 + } + }); + return non_filtered_block_handler_count > 1 || call_filtered_block_handler_count > 1; + }); + + if has_too_many_block_handlers { + errors.push(SubgraphManifestValidationError::DataSourceBlockHandlerLimitExceeded) + } + + manifest + .schema + .validate(&schemas) + .err() + .into_iter() + .for_each(|schema_errors| { + errors.push(SubgraphManifestValidationError::SchemaValidationError( + schema_errors, + )); + }); + + return Err(errors); } } diff --git a/graphql/src/execution/execution.rs b/graphql/src/execution/execution.rs index 906a3544ad8..e0b00b34a29 100644 --- a/graphql/src/execution/execution.rs +++ b/graphql/src/execution/execution.rs @@ -7,7 +7,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::ops::Deref; use std::time::Instant; -use graph::data::graphql::validation::get_base_type; +use graph::data::graphql::traversal; use graph::prelude::*; use crate::introspection::INTROSPECTION_DOCUMENT; @@ -187,7 +187,7 @@ where .ok_or(Invalid)?; let field_complexity = self.query_complexity( - &get_named_type(schema, get_base_type(&s_field.field_type)) + &get_named_type(schema, traversal::get_base_type(&s_field.field_type)) .ok_or(Invalid)?, &field.selection_set, max_depth, @@ -263,7 +263,7 @@ where match s_field { Some(s_field) => { - let base_type = get_base_type(&s_field.field_type); + let base_type = traversal::get_base_type(&s_field.field_type); match get_named_type(schema, base_type) { Some(ty) => errors.extend(self.validate_fields( base_type, From 408cfd6298febcf41f73990b2ec57a293ad0b945 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 26 Nov 2019 18:27:59 -0600 Subject: [PATCH 07/64] graph: Validate Schema fields --- graph/src/data/graphql/mod.rs | 3 ++ graph/src/data/graphql/scalar.rs | 28 +++++++++++++++ graph/src/data/schema.rs | 61 ++++++++++++++++++++++++++------ 3 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 graph/src/data/graphql/scalar.rs diff --git a/graph/src/data/graphql/mod.rs b/graph/src/data/graphql/mod.rs index 72d2d26e2ba..71dba22e665 100644 --- a/graph/src/data/graphql/mod.rs +++ b/graph/src/data/graphql/mod.rs @@ -3,6 +3,9 @@ mod serialization; /// Utilities for validating GraphQL schemas. pub mod traversal; +/// Types to represent built in scalar values in GraphQL documents +pub mod scalar; + /// Utilities for working with GraphQL values. mod values; diff --git a/graph/src/data/graphql/scalar.rs b/graph/src/data/graphql/scalar.rs new file mode 100644 index 00000000000..4806be19987 --- /dev/null +++ b/graph/src/data/graphql/scalar.rs @@ -0,0 +1,28 @@ +use std::convert::TryFrom; + +pub enum BuiltInScalarType { + Boolean, + Int, + BigDecimal, + String, + BigInt, + Bytes, + ID, +} + +impl TryFrom<&String> for BuiltInScalarType { + type Error = (); + + fn try_from(value: &String) -> Result { + match value.as_ref() { + "Boolean" => Ok(BuiltInScalarType::Boolean), + "Int" => Ok(BuiltInScalarType::Int), + "BigDecimal" => Ok(BuiltInScalarType::BigDecimal), + "String" => Ok(BuiltInScalarType::String), + "BigInt" => Ok(BuiltInScalarType::BigInt), + "Bytes" => Ok(BuiltInScalarType::Bytes), + "ID" => Ok(BuiltInScalarType::ID), + _ => Err(()), + } + } +} diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 04a7679d283..c95f5baa845 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -1,4 +1,5 @@ use crate::components::store::{Store, SubgraphDeploymentStore}; +use crate::data::graphql::scalar::BuiltInScalarType; use crate::data::graphql::traversal; use crate::data::subgraph::{SubgraphDeploymentId, SubgraphName}; use crate::prelude::future::{self, *}; @@ -14,6 +15,7 @@ use graphql_parser::{ use serde::{Deserialize, Serialize}; use std::collections::{BTreeMap, HashMap}; +use std::convert::TryFrom; use std::fmt; use std::hash::{Hash, Hasher}; use std::iter::FromIterator; @@ -56,6 +58,11 @@ pub enum SchemaValidationError { SubgraphSchemaDirectivesInvalid, #[fail(display = "@import defined incorrectly")] ImportDirectiveInvalid, + #[fail( + display = "GraphQL type `{}` has field `{}` with type `{}` which is not defined or imported", + _0, _1, _2 + )] + GraphQLTypeFieldInvalid(String, String, String), // (type_name, field_name, field_type) } #[derive(Debug, Fail, PartialEq, Eq, Clone)] @@ -439,8 +446,8 @@ impl Schema { .unwrap_or_else(|err| errors.push(err)); // Should validate that all types in the Subgraph referenced from other subgraphs exist // If the referenced subgraph is not provided as an argument, do not validate those types - self.validate_imported_types(schemas) - .unwrap_or_else(|err| errors.push(err)); + self.validate_fields() + .unwrap_or_else(|mut err| errors.append(&mut err)); if errors.is_empty() { Ok(()) @@ -491,19 +498,51 @@ impl Schema { } } - fn validate_imported_types( - &self, - schemas: &HashMap>, - ) -> Result<(), SchemaValidationError> { - // Look up of all types in schema + fn validate_fields(&self) -> Result<(), Vec> { + // Native types let root_schema = traversal::get_object_and_interface_type_fields(&self.document); - - // Look up of ImportedType to SchemaReference + // Imported types let imported_types = self.imported_types(); - // Look up of SchemaReference to Option of all types in schema + // For each field in the root_schema, verify that the field + // is either a [BuiltInScalar, Native, Imported] type + let errors = root_schema + .iter() + .fold(vec![], |errors, (type_name, fields)| { + fields.iter().fold(errors, |mut errors, field| { + let base = traversal::get_base_type(&field.field_type); + BuiltInScalarType::try_from(base) + .map(|_| ()) + .or_else(|_| match root_schema.contains_key(base) { + true => Ok(()), + false => Err(()), + }) + .or_else(|_| { + // Check imported types and the corresponding schema + imported_types + .iter() + .find(|(imported_type, schema_reference)| match imported_type { + ImportedType::Name(name) if name == base => true, + ImportedType::NameAs(_, az) if az == base => true, + _ => false, + }) + .map_or(Err(()), |_| Ok(())) + }) + .map_err(|_| { + errors.push(SchemaValidationError::GraphQLTypeFieldInvalid( + type_name.to_string(), + field.name.to_string(), + base.to_string(), + )) + }); + errors + }) + }); - Ok(()) + match errors.is_empty() { + false => Err(errors), + true => Ok(()), + } } fn validate_schema_types(&self) -> Result<(), SchemaValidationError> { From 1eace3927338c59564075fbc9e403f9f821ae5ea Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Wed, 27 Nov 2019 11:24:36 -0600 Subject: [PATCH 08/64] core, graph: Address warnings --- core/src/subgraph/registrar.rs | 7 ++++- graph/src/data/graphql/traversal.rs | 4 --- graph/src/data/schema.rs | 47 ++++++++++++++--------------- 3 files changed, 28 insertions(+), 30 deletions(-) diff --git a/core/src/subgraph/registrar.rs b/core/src/subgraph/registrar.rs index 7ae21c4cf40..8c695d22810 100644 --- a/core/src/subgraph/registrar.rs +++ b/core/src/subgraph/registrar.rs @@ -356,10 +356,15 @@ where // Validate the unvalidated manifest // unvalidated.validate(schemas) + + // Take the errors from the validation function and combine them + // with the `failable_schema_errors`, to get a single Vector of errors + // If the vector is not emmpty, return an error + + // Log the import warnings and continue with the validated SubgraphManifest future::ok((schemas, schema_import_warnings)) }) .map(move |(schemas, import_errors)| { - // Call unvalidate.validate(schemas) (unvalidated.0, ethereum_adapter, chain_store, store) }) }) diff --git a/graph/src/data/graphql/traversal.rs b/graph/src/data/graphql/traversal.rs index 60f4492054f..9b46a06cb48 100644 --- a/graph/src/data/graphql/traversal.rs +++ b/graph/src/data/graphql/traversal.rs @@ -1,9 +1,5 @@ -use crate::data::subgraph::{SubgraphDeploymentId, SubgraphName}; -use crate::prelude::Fail; use graphql_parser::schema::*; -use serde::{Deserialize, Serialize}; use std::collections::HashMap; -use std::fmt; /// Returns all object type definitions in the schema. pub fn get_object_type_definitions(schema: &Document) -> Vec<&ObjectType> { diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index c95f5baa845..534d818f6c1 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -2,7 +2,6 @@ use crate::components::store::{Store, SubgraphDeploymentStore}; use crate::data::graphql::scalar::BuiltInScalarType; use crate::data::graphql::traversal; use crate::data::subgraph::{SubgraphDeploymentId, SubgraphName}; -use crate::prelude::future::{self, *}; use crate::prelude::Fail; use failure::Error; @@ -146,7 +145,7 @@ impl SchemaReference { let subgraph_id = match &self { SchemaReference::ByName(name) => { let subgraph_name = SubgraphName::new(name.clone()) - .map_err(|err| SchemaImportError::ImportedSubgraphNameInvalid(name.clone()))?; + .map_err(|_| SchemaImportError::ImportedSubgraphNameInvalid(name.clone()))?; store .resolve_subgraph_name_to_id(subgraph_name.clone()) .map_err(|_| SchemaImportError::ImportedSubgraphNotFound(self.clone())) @@ -156,12 +155,12 @@ impl SchemaReference { })? } SchemaReference::ById(id) => SubgraphDeploymentId::new(id.clone()) - .map_err(|err| SchemaImportError::ImportedSubgraphIdInvalid(id.clone()))?, + .map_err(|_| SchemaImportError::ImportedSubgraphIdInvalid(id.clone()))?, }; store .input_schema(&subgraph_id) - .map_err(|err| SchemaImportError::ImportedSchemaNotFound(self.clone())) + .map_err(|_| SchemaImportError::ImportedSchemaNotFound(self.clone())) } } @@ -246,8 +245,6 @@ impl Schema { pub fn parse(raw: &str, id: SubgraphDeploymentId) -> Result { let document = graphql_parser::parse_schema(&raw)?; - // TODO: Decide if we want to keep this here - // validate_schema(&document)?; let (interfaces_for_type, types_for_interface) = Self::collect_interfaces(&document)?; @@ -429,7 +426,7 @@ impl Schema { pub fn validate( &self, - schemas: &HashMap>, + _schemas: &HashMap>, ) -> Result<(), Vec> { let mut errors = vec![]; // [X] Should include all logic in graph/src/data/graphql/validation.rs @@ -488,8 +485,8 @@ impl Schema { .directives .iter() .filter(|directive| directive.name == "imports") - // TODO: Fix - .find(|directive| true) + // TODO: Finish verifying import directive + .find(|_directive| true) .map(|_| SchemaValidationError::ImportDirectiveInvalid) } }) { @@ -498,22 +495,22 @@ impl Schema { } } + // fn validate_imported_types(&self) + fn validate_fields(&self) -> Result<(), Vec> { - // Native types - let root_schema = traversal::get_object_and_interface_type_fields(&self.document); - // Imported types + let native_types = traversal::get_object_and_interface_type_fields(&self.document); let imported_types = self.imported_types(); // For each field in the root_schema, verify that the field - // is either a [BuiltInScalar, Native, Imported] type - let errors = root_schema + // is either a: [BuiltInScalar, Native, Imported] type + let errors = native_types .iter() .fold(vec![], |errors, (type_name, fields)| { fields.iter().fold(errors, |mut errors, field| { let base = traversal::get_base_type(&field.field_type); - BuiltInScalarType::try_from(base) + match BuiltInScalarType::try_from(base) .map(|_| ()) - .or_else(|_| match root_schema.contains_key(base) { + .or_else(|_| match native_types.contains_key(base) { true => Ok(()), false => Err(()), }) @@ -521,20 +518,20 @@ impl Schema { // Check imported types and the corresponding schema imported_types .iter() - .find(|(imported_type, schema_reference)| match imported_type { + .find(|(imported_type, _)| match imported_type { ImportedType::Name(name) if name == base => true, ImportedType::NameAs(_, az) if az == base => true, _ => false, }) .map_or(Err(()), |_| Ok(())) - }) - .map_err(|_| { - errors.push(SchemaValidationError::GraphQLTypeFieldInvalid( - type_name.to_string(), - field.name.to_string(), - base.to_string(), - )) - }); + }) { + Err(_) => errors.push(SchemaValidationError::GraphQLTypeFieldInvalid( + type_name.to_string(), + field.name.to_string(), + base.to_string(), + )), + Ok(_) => (), + }; errors }) }); From a88e04f14e322cab0d4729c64f7e2490dd3643d6 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 2 Dec 2019 17:57:43 -0600 Subject: [PATCH 09/64] core, graph: Move resolve_schema_references to method on Schema --- core/src/subgraph/registrar.rs | 99 ++++++++++------------------------ graph/src/data/schema.rs | 27 ++++++++++ graph/src/data/subgraph/mod.rs | 35 +++++++----- 3 files changed, 79 insertions(+), 82 deletions(-) diff --git a/core/src/subgraph/registrar.rs b/core/src/subgraph/registrar.rs index 8c695d22810..9eeef4a5ed7 100644 --- a/core/src/subgraph/registrar.rs +++ b/core/src/subgraph/registrar.rs @@ -13,7 +13,6 @@ lazy_static! { ); } -use graph::data::schema::{SchemaImportError, SchemaReference}; use graph::data::subgraph::schema::{ generate_entity_id, SubgraphDeploymentAssignmentEntity, SubgraphDeploymentEntity, SubgraphEntity, SubgraphVersionEntity, TypedEntity, @@ -338,50 +337,36 @@ where }) }) .and_then(move |(unvalidated, ethereum_adapter, chain_store)| { - future::ok(resolve_schema_references( - &unvalidated.0.schema, - store.clone(), - )) - .and_then(|(schemas, import_errors)| { - // Separate errors and warning from SchemaImportError(s) - let failable_schema_errors: Vec = import_errors - .iter() - .filter(|err| SchemaImportError::is_failure(err)) - .map(|err| err.clone()) - .collect(); - let schema_import_warnings: Vec = import_errors - .into_iter() - .filter(|err| !SchemaImportError::is_failure(err)) - .collect(); - - // Validate the unvalidated manifest - // unvalidated.validate(schemas) - - // Take the errors from the validation function and combine them - // with the `failable_schema_errors`, to get a single Vector of errors - // If the vector is not emmpty, return an error - - // Log the import warnings and continue with the validated SubgraphManifest - future::ok((schemas, schema_import_warnings)) - }) - .map(move |(schemas, import_errors)| { - (unvalidated.0, ethereum_adapter, chain_store, store) - }) - }) - .and_then(move |(manifest, ethereum_adapter, chain_store, store)| { - let manifest_id = manifest.id.clone(); - create_subgraph_version( - &logger2, - store, - chain_store.clone(), - ethereum_adapter.clone(), - name, - manifest, - node_id, - version_switching_mode, - ) - .map(|_| manifest_id) + future::result(unvalidated.validate(store.clone())) + .map_err(|validation_errors| { + SubgraphRegistrarError::ManifestValidationError(validation_errors) + }) + .map(move |(manifest, validation_warnings)| { + ( + manifest, + ethereum_adapter, + chain_store, + store, + validation_warnings, + ) + }) }) + .and_then( + move |(manifest, ethereum_adapter, chain_store, store, _validation_warnings)| { + let manifest_id = manifest.id.clone(); + create_subgraph_version( + &logger2, + store, + chain_store.clone(), + ethereum_adapter.clone(), + name, + manifest, + node_id, + version_switching_mode, + ) + .map(|_| manifest_id) + }, + ) .and_then(move |manifest_id| { debug!( logger3, @@ -418,32 +403,6 @@ where } } -fn resolve_schema_references( - schema: &Schema, - store: Arc, -) -> ( - HashMap>, - Vec, -) { - schema.imported_schemas().into_iter().fold( - (HashMap::new(), vec![]), - |(mut schemas, mut errors), schema_ref| { - match schema_ref.clone().resolve(store.clone()) { - Ok(schema) => { - let (s, e) = resolve_schema_references(&schema, store.clone()); - schemas.insert(schema_ref, schema); - schemas.extend(s); - errors.extend(e); - } - Err(err) => { - errors.push(err); - } - } - (schemas, errors) - }, - ) -} - fn handle_assignment_event

( event: AssignmentEvent, provider: Arc

, diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 534d818f6c1..5e554bff467 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -190,6 +190,33 @@ impl Schema { } } + pub fn resolve_schema_references( + &self, + store: Arc, + ) -> ( + HashMap>, + Vec, + ) { + // TODO: Handle circular dependencies with a memo + self.imported_schemas().into_iter().fold( + (HashMap::new(), vec![]), + |(mut schemas, mut errors), schema_ref| { + match schema_ref.clone().resolve(store.clone()) { + Ok(schema) => { + let (s, e) = schema.resolve_schema_references(store.clone()); + schemas.insert(schema_ref, schema); + schemas.extend(s); + errors.extend(e); + } + Err(err) => { + errors.push(err); + } + } + (schemas, errors) + }, + ) + } + pub fn collect_interfaces( document: &schema::Document, ) -> Result< diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index 6bdc49d4b01..2e65d9964e4 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -12,7 +12,7 @@ use tokio::prelude::*; use web3::types::{Address, H256}; use crate::components::link_resolver::LinkResolver; -use crate::components::store::StoreError; +use crate::components::store::{Store, StoreError, SubgraphDeploymentStore}; use crate::data::query::QueryExecutionError; use crate::data::schema::{Schema, SchemaImportError, SchemaReference, SchemaValidationError}; use crate::data::subgraph::schema::{ @@ -321,6 +321,12 @@ pub enum SubgraphAssignmentProviderEvent { SubgraphStop(SubgraphDeploymentId), } +#[derive(Fail, Debug)] +pub enum SubgraphManifestValidationWarning { + #[fail(display = "schema validation produced warnings: {:?}", _0)] + SchemaValidationWarning(Vec), +} + #[derive(Fail, Debug)] pub enum SubgraphManifestValidationError { #[fail(display = "subgraph has no data sources")] @@ -847,23 +853,25 @@ impl UnvalidatedSubgraphManifest { self.0.schema.imported_schemas() } - pub fn validate( - &self, - _logger: Logger, - schemas: HashMap>, - ) -> Result> { - let manifest = &self.0; + pub fn validate( + self, + store: Arc, + ) -> Result< + (SubgraphManifest, Vec), + Vec, + > { + let (schemas, import_errors) = self.0.schema.resolve_schema_references(store); let mut errors: Vec = vec![]; // Validate that the manifest has at least one data source - if manifest.data_sources.is_empty() { + if self.0.data_sources.is_empty() { errors.push(SubgraphManifestValidationError::NoDataSources); } // Validate that the manifest has a `source` address in each data source // which has call or block handlers - if manifest.data_sources.iter().any(|data_source| { + if self.0.data_sources.iter().any(|data_source| { let no_source_address = data_source.source.address.is_none(); let has_call_handlers = !data_source.mapping.call_handlers.is_empty(); let has_block_handlers = !data_source.mapping.block_handlers.is_empty(); @@ -875,7 +883,7 @@ impl UnvalidatedSubgraphManifest { // Validate that there are no more than one of each type of // block_handler in each data source. - let has_too_many_block_handlers = manifest.data_sources.iter().any(|data_source| { + let has_too_many_block_handlers = self.0.data_sources.iter().any(|data_source| { if data_source.mapping.block_handlers.is_empty() { return false; } @@ -900,7 +908,7 @@ impl UnvalidatedSubgraphManifest { errors.push(SubgraphManifestValidationError::DataSourceBlockHandlerLimitExceeded) } - manifest + self.0 .schema .validate(&schemas) .err() @@ -911,7 +919,10 @@ impl UnvalidatedSubgraphManifest { )); }); - return Err(errors); + match errors.is_empty() { + true => Ok((self.0, vec![])), + false => Err(errors), + } } } From 82e734e1d2522b05892878536b9e8ca9b43c7fa0 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 2 Dec 2019 18:15:08 -0600 Subject: [PATCH 10/64] core: Update combinator order subgraph deploy --- core/src/subgraph/registrar.rs | 37 +++++++++++++++------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/core/src/subgraph/registrar.rs b/core/src/subgraph/registrar.rs index 9eeef4a5ed7..9bbda76f235 100644 --- a/core/src/subgraph/registrar.rs +++ b/core/src/subgraph/registrar.rs @@ -295,7 +295,8 @@ where hash: SubgraphDeploymentId, node_id: NodeId, ) -> Box + Send + 'static> { - let store = self.store.clone(); + let store_1 = self.store.clone(); + let store_2 = self.store.clone(); let chain_stores = self.chain_stores.clone(); let ethereum_adapters = self.ethereum_adapters.clone(); let version_switching_mode = self.version_switching_mode; @@ -313,8 +314,12 @@ where ) .map_err(SubgraphRegistrarError::ResolveError) .and_then(move |unvalidated| { - unvalidated - .0 + future::result(unvalidated.validate(store_1)).map_err(|validation_errors| { + SubgraphRegistrarError::ManifestValidationError(validation_errors) + }) + }) + .and_then(move |(manifest, validation_warnings)| { + manifest .network_name() .map_err(|e| SubgraphRegistrarError::ManifestValidationError(vec![e])) .and_then(move |network_name| { @@ -331,32 +336,22 @@ where network_name.clone(), )) .map(move |ethereum_adapter| { - (unvalidated, ethereum_adapter.clone(), chain_store.clone()) + ( + manifest, + ethereum_adapter.clone(), + chain_store.clone(), + validation_warnings, + ) }) }) }) }) - .and_then(move |(unvalidated, ethereum_adapter, chain_store)| { - future::result(unvalidated.validate(store.clone())) - .map_err(|validation_errors| { - SubgraphRegistrarError::ManifestValidationError(validation_errors) - }) - .map(move |(manifest, validation_warnings)| { - ( - manifest, - ethereum_adapter, - chain_store, - store, - validation_warnings, - ) - }) - }) .and_then( - move |(manifest, ethereum_adapter, chain_store, store, _validation_warnings)| { + move |(manifest, ethereum_adapter, chain_store, _validation_warnings)| { let manifest_id = manifest.id.clone(); create_subgraph_version( &logger2, - store, + store_2, chain_store.clone(), ethereum_adapter.clone(), name, From 5acf36ea221c755dde7c4afc88b5d37720e1a5ef Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 2 Dec 2019 18:20:57 -0600 Subject: [PATCH 11/64] graph: Update schema type name const --- graph/src/data/schema.rs | 4 ++-- graph/src/data/subgraph/mod.rs | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 5e554bff467..e0ec4462d04 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -20,7 +20,7 @@ use std::hash::{Hash, Hasher}; use std::iter::FromIterator; use std::sync::Arc; -pub const SUBGRAPH_SCHEMA_TYPE_NAME: &str = "_SubgraphSchema_"; +pub const SCHEMA_TYPE_NAME: &str = "_schema_"; #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct Strings(Vec); @@ -756,7 +756,7 @@ impl Schema { fn subgraph_schema_object_type(&self) -> Option<&ObjectType> { traversal::get_object_type_definitions(&self.document) .into_iter() - .find(|object_type| object_type.name == SUBGRAPH_SCHEMA_TYPE_NAME) + .find(|object_type| object_type.name == SCHEMA_TYPE_NAME) } } diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index 2e65d9964e4..f2ba191b438 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -24,7 +24,6 @@ use crate::data::subgraph::schema::{ use crate::prelude::{format_err, Deserialize, Fail, Serialize}; use crate::util::ethereum::string_to_h256; -use std::collections::HashMap; use std::fmt; use std::ops::Deref; use std::str::FromStr; From 8c43beb2d6f7c299086a99a6970cca5b425e420b Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 2 Dec 2019 18:33:43 -0600 Subject: [PATCH 12/64] graph: Simplify combinators --- graph/src/data/schema.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index e0ec4462d04..3c34f2b7703 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -334,11 +334,7 @@ impl Schema { .arguments .iter() .find(|(name, _)| name == "types") - .filter(|(_, value)| match value { - Value::List(_) => true, - _ => false, - }) - .map(|(_, value)| match value { + .map_or(vec![], |(_, value)| match value { Value::List(types) => types .iter() .filter_map(|import_type| match import_type { @@ -363,9 +359,8 @@ impl Schema { _ => None, }) .collect(), - _ => unreachable!(), + _ => vec![], }) - .unwrap_or(vec![]) } fn schema_reference_from_directive_argument( From 7ceffc2a2f72548e8ab206e50af7b6801e19da20 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 2 Dec 2019 19:00:15 -0600 Subject: [PATCH 13/64] graph: Update comments --- graph/src/data/schema.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 3c34f2b7703..17cb60079df 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -451,16 +451,14 @@ impl Schema { _schemas: &HashMap>, ) -> Result<(), Vec> { let mut errors = vec![]; - // [X] Should include all logic in graph/src/data/graphql/validation.rs self.validate_schema_types() .unwrap_or_else(|err| errors.push(err)); self.validate_derived_from() .unwrap_or_else(|err| errors.push(err)); - // _SubgraphSchema_ type should not have fields self.validate_subgraph_schema_has_no_fields() .unwrap_or_else(|err| errors.push(err)); // Should validate that import directives are properly formed - // Should that import directives only exist on the _SubgraphSchema_ type + // Should verify that import directives only exist on the _Schema_ type self.validate_import_directives() .unwrap_or_else(|err| errors.push(err)); // Should validate that all types in the Subgraph referenced from other subgraphs exist @@ -508,7 +506,9 @@ impl Schema { .iter() .filter(|directive| directive.name == "imports") // TODO: Finish verifying import directive - .find(|_directive| true) + // Each import directive must have a valid `from` argument + // Each import directive must have a valid 'types` argument + .find(|_directive| false) .map(|_| SchemaValidationError::ImportDirectiveInvalid) } }) { From add1bd7a07b9f290bd0929f8d5aeb0480beaa3f2 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 2 Dec 2019 19:21:59 -0600 Subject: [PATCH 14/64] graph: Remove comment and add placeholder method --- graph/src/data/schema.rs | 7 ++++++- graph/src/data/subgraph/mod.rs | 2 -- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 17cb60079df..11c1e9c0a7f 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -517,7 +517,12 @@ impl Schema { } } - // fn validate_imported_types(&self) + fn validate_imported_types( + &self, + schemas: &HashMap>, + ) -> Result<(), Vec> { + Ok(()) + } fn validate_fields(&self) -> Result<(), Vec> { let native_types = traversal::get_object_and_interface_type_fields(&self.document); diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index f2ba191b438..07fc4231ba9 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -340,10 +340,8 @@ pub enum SubgraphManifestValidationError { DataSourceBlockHandlerLimitExceeded, #[fail(display = "the specified block must exist on the Ethereum network")] BlockNotFound(String), - // TODO: Figure out how to get these error to properly show up #[fail(display = "imported schema(s) are invalid: {:?}", _0)] SchemaImportError(Vec), - // TODO: Figure out how to get these error to properly show up #[fail(display = "schema validation failed: {:?}", _0)] SchemaValidationError(Vec), } From f3cf58529726663d654d53523e4485a4a56ef195 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 2 Dec 2019 19:23:23 -0600 Subject: [PATCH 15/64] graph: Remove extra method on UnvalidateSubgraphManifest --- graph/src/data/subgraph/mod.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index 07fc4231ba9..1b2df1a404a 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -846,10 +846,6 @@ impl UnvalidatedSubgraphManifest { SubgraphManifest::resolve(link, resolver, logger).map(|manifest| Self(manifest)) } - pub fn imported_schemas(&self) -> Vec { - self.0.schema.imported_schemas() - } - pub fn validate( self, store: Arc, From 5d7dfa34536c475d6123639c542c39a18a5c4df1 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 3 Dec 2019 13:26:36 -0600 Subject: [PATCH 16/64] graph: Detect cycles in graph when resolving imports --- graph/src/data/schema.rs | 46 +++++++++++++++++++++++----------- graph/src/data/subgraph/mod.rs | 4 +-- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 11c1e9c0a7f..50034ac409e 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -141,7 +141,7 @@ impl SchemaReference { pub fn resolve( self, store: Arc, - ) -> Result, SchemaImportError> { + ) -> Result<(Arc, SubgraphDeploymentId), SchemaImportError> { let subgraph_id = match &self { SchemaReference::ByName(name) => { let subgraph_name = SubgraphName::new(name.clone()) @@ -161,6 +161,7 @@ impl SchemaReference { store .input_schema(&subgraph_id) .map_err(|_| SchemaImportError::ImportedSchemaNotFound(self.clone())) + .map(|schema| (schema, subgraph_id)) } } @@ -197,24 +198,41 @@ impl Schema { HashMap>, Vec, ) { - // TODO: Handle circular dependencies with a memo - self.imported_schemas().into_iter().fold( - (HashMap::new(), vec![]), - |(mut schemas, mut errors), schema_ref| { + let mut schemas = HashMap::new(); + let mut visit_log = HashMap::new(); + let import_errors = self.resolve_import_graph(store, &mut schemas, &mut visit_log); + (schemas, import_errors) + } + + fn resolve_import_graph( + &self, + store: Arc, + schemas: &mut HashMap>, + visit_log: &mut HashMap>, + ) -> Vec { + // Use the visit log to detect cycles in the import graph + self.imported_schemas() + .into_iter() + .fold(vec![], |mut errors, schema_ref| { match schema_ref.clone().resolve(store.clone()) { - Ok(schema) => { - let (s, e) = schema.resolve_schema_references(store.clone()); - schemas.insert(schema_ref, schema); - schemas.extend(s); - errors.extend(e); + Ok((schema, subgraph_id)) => { + schemas.insert(schema_ref, schema.clone()); + // If this node in the graph has already been visited stop traversing + if !visit_log.contains_key(&subgraph_id) { + visit_log.insert(subgraph_id, schema.clone()); + errors.extend(schema.resolve_import_graph( + store.clone(), + schemas, + visit_log, + )); + } } Err(err) => { errors.push(err); } } - (schemas, errors) - }, - ) + errors + }) } pub fn collect_interfaces( @@ -519,7 +537,7 @@ impl Schema { fn validate_imported_types( &self, - schemas: &HashMap>, + _schemas: &HashMap>, ) -> Result<(), Vec> { Ok(()) } diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index 1b2df1a404a..ed1a2ccc73b 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -14,7 +14,7 @@ use web3::types::{Address, H256}; use crate::components::link_resolver::LinkResolver; use crate::components::store::{Store, StoreError, SubgraphDeploymentStore}; use crate::data::query::QueryExecutionError; -use crate::data::schema::{Schema, SchemaImportError, SchemaReference, SchemaValidationError}; +use crate::data::schema::{Schema, SchemaImportError, SchemaValidationError}; use crate::data::subgraph::schema::{ EthereumBlockHandlerEntity, EthereumCallHandlerEntity, EthereumContractAbiEntity, EthereumContractDataSourceEntity, EthereumContractDataSourceTemplateEntity, @@ -853,7 +853,7 @@ impl UnvalidatedSubgraphManifest { (SubgraphManifest, Vec), Vec, > { - let (schemas, import_errors) = self.0.schema.resolve_schema_references(store); + let (schemas, _import_errors) = self.0.schema.resolve_schema_references(store); let mut errors: Vec = vec![]; From 8b53793ea9ad67370e9a9bbfbaaf73bac1d1bdaa Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Wed, 4 Dec 2019 12:18:16 -0600 Subject: [PATCH 17/64] schema: Better validation for import directives --- graph/src/data/schema.rs | 119 +++++++++++++++++++++++++++++++++------ 1 file changed, 103 insertions(+), 16 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 50034ac409e..7641de4ec8b 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -51,11 +51,14 @@ pub enum SchemaValidationError { _1, _0, _2 )] DerivedFromInvalid(String, String, String), // (type, field, reason) - #[fail(display = "_SubgraphSchema_ type is solely for imports and should have no fields")] + #[fail(display = "_schema_ type is solely for imports and should have no fields")] SubgraphSchemaTypeFieldsInvalid, - #[fail(display = "_SubgraphSchema_ type only allows @import directives")] + #[fail(display = "_schema_ type only allows @import directives")] SubgraphSchemaDirectivesInvalid, - #[fail(display = "@import defined incorrectly")] + #[fail(display = r#" +@imports directives must be defined in one of the following forms: \ +@imports(types: ["A", {{ name: "B", as: "C"}}], from: {{ name: "org/subgraph"}}) \ +@imports(types: ["A", {{ name: "B", as: "C"}}], from: {{ id: "Qm..."}})")]"#)] ImportDirectiveInvalid, #[fail( display = "GraphQL type `{}` has field `{}` with type `{}` which is not defined or imported", @@ -475,8 +478,10 @@ impl Schema { .unwrap_or_else(|err| errors.push(err)); self.validate_subgraph_schema_has_no_fields() .unwrap_or_else(|err| errors.push(err)); - // Should validate that import directives are properly formed - // Should verify that import directives only exist on the _Schema_ type + // Should verify that only import directives exist on the _schema_ type + self.validate_only_import_directives_on_reserved_type() + .unwrap_or_else(|err| errors.push(err)); + // Should validate that import directives on the _schema_ type are properly formed self.validate_import_directives() .unwrap_or_else(|err| errors.push(err)); // Should validate that all types in the Subgraph referenced from other subgraphs exist @@ -506,29 +511,111 @@ impl Schema { } } - fn validate_import_directives(&self) -> Result<(), SchemaValidationError> { + fn validate_only_import_directives_on_reserved_type( + &self, + ) -> Result<(), SchemaValidationError> { match self .subgraph_schema_object_type() .and_then(|subgraph_schema_type| { if !subgraph_schema_type .directives .iter() - .filter(|directive| directive.name != "imports") + .filter(|directive| !directive.name.eq("imports")) .collect::>() .is_empty() { Some(SchemaValidationError::SubgraphSchemaDirectivesInvalid) } else { - subgraph_schema_type - .directives - .iter() - .filter(|directive| directive.name == "imports") - // TODO: Finish verifying import directive - // Each import directive must have a valid `from` argument - // Each import directive must have a valid 'types` argument - .find(|_directive| false) - .map(|_| SchemaValidationError::ImportDirectiveInvalid) + None + } + }) { + Some(err) => Err(err), + None => Ok(()), + } + } + + fn import_directive_has_valid_types(directive: &Directive) -> bool { + directive + .arguments + .iter() + .find(|(name, value)| { + if !name.eq("types") { + return false; + } + match value { + Value::List(values) => { + // Each value must be a String or an Object with String:String key value + // pairs for `name` and `as` + // Search for an invalid type in the list of imported types + values + .iter() + .find(|value| match value { + Value::String(_) => false, + Value::Object(obj) => { + let has_invalid_name = + obj.get("name").map_or(true, |value| match value { + Value::String(_) => false, + _ => true, + }); + let has_invalid_as = + obj.get("as").map_or(true, |value| match value { + Value::String(_) => false, + _ => true, + }); + has_invalid_name || has_invalid_as + } + _ => true, + }) + .map_or(true, |_| false) + } + _ => return false, } + }) + .map_or(false, |_| true) + } + + fn import_directive_has_valid_from(directive: &Directive) -> bool { + directive + .arguments + .iter() + // Look for a valid `from` argument + .find(|(name, value)| { + if !name.eq("from") { + return false; + } + match value { + Value::Object(obj) => { + let has_id = obj.get("id").map_or(false, |value| match value { + Value::String(_) => true, + _ => false, + }); + let has_name = obj.get("name").map_or(false, |value| match value { + Value::String(_) => true, + _ => false, + }); + has_id ^ has_name + } + _ => return false, + } + }) + .map_or(false, |_| true) + } + + fn validate_import_directives(&self) -> Result<(), SchemaValidationError> { + match self + .subgraph_schema_object_type() + .and_then(|subgraph_schema_type| { + subgraph_schema_type + .directives + .iter() + .filter(|directive| directive.name == "imports") + // Look for an invalid directive + .find(|directive| { + let has_valid_types = Self::import_directive_has_valid_types(directive); + let has_valid_from = Self::import_directive_has_valid_from(directive); + !has_valid_types || !has_valid_from + }) + .map(|_| SchemaValidationError::ImportDirectiveInvalid) }) { Some(err) => Err(err), None => Ok(()), From 6d0fe095bd3dbc77c9b93c71a0b97de202320531 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Wed, 4 Dec 2019 16:29:27 -0600 Subject: [PATCH 18/64] graph: Validate imported types exists in schema --- graph/src/data/schema.rs | 68 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 5 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 7641de4ec8b..85f7de162ee 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -65,6 +65,11 @@ pub enum SchemaValidationError { _0, _1, _2 )] GraphQLTypeFieldInvalid(String, String, String), // (type_name, field_name, field_type) + #[fail( + display = "Imported type `{}` does not exist in the `{}` schema", + _0, _1 + )] + ImportedTypeDNE(String, String), } #[derive(Debug, Fail, PartialEq, Eq, Clone)] @@ -469,13 +474,16 @@ impl Schema { pub fn validate( &self, - _schemas: &HashMap>, + schemas: &HashMap>, ) -> Result<(), Vec> { let mut errors = vec![]; self.validate_schema_types() .unwrap_or_else(|err| errors.push(err)); self.validate_derived_from() .unwrap_or_else(|err| errors.push(err)); + self.validate_fields() + .unwrap_or_else(|mut err| errors.append(&mut err)); + self.validate_subgraph_schema_has_no_fields() .unwrap_or_else(|err| errors.push(err)); // Should verify that only import directives exist on the _schema_ type @@ -486,8 +494,8 @@ impl Schema { .unwrap_or_else(|err| errors.push(err)); // Should validate that all types in the Subgraph referenced from other subgraphs exist // If the referenced subgraph is not provided as an argument, do not validate those types - self.validate_fields() - .unwrap_or_else(|mut err| errors.append(&mut err)); + self.validate_imported_types(schemas) + .unwrap_or_else(|errs| errors.extend(errs)); if errors.is_empty() { Ok(()) @@ -624,9 +632,59 @@ impl Schema { fn validate_imported_types( &self, - _schemas: &HashMap>, + schemas: &HashMap>, ) -> Result<(), Vec> { - Ok(()) + let errors = + self.imported_types() + .iter() + .fold(vec![], |mut errors, (imported_type, schema_ref)| { + // See if `schemas` has the schema associated with `schema_ref` + schemas + .get(schema_ref) + .and_then(|schema| { + // Get the defined types in the schema and the imported types + let native_types = + traversal::get_object_type_definitions(&schema.document); + let imported_types = schema.imported_types(); + + // Ensure that the imported type is in one of those two sets + let schema_handle = match schema_ref { + SchemaReference::ById(id) => id, + SchemaReference::ByName(name) => name, + }; + let name = match imported_type { + ImportedType::Name(name) => name, + ImportedType::NameAs(name, _) => name, + }; + let is_native = native_types + .iter() + .find(|object| object.name.eq(name)) + .map_or(false, |_| true); + let is_imported = imported_types + .iter() + .find(|(import, _)| match import { + ImportedType::Name(n) => name.eq(n), + ImportedType::NameAs(_, az) => name.eq(az), + }) + .map_or(false, |_| true); + if !is_native || !is_imported { + Some(SchemaValidationError::ImportedTypeDNE( + name.to_string(), + schema_handle.to_string(), + )) + } else { + None + } + }) + .into_iter() + .for_each(|err| errors.push(err)); + errors + }); + + match errors.is_empty() { + true => Ok(()), + false => Err(errors), + } } fn validate_fields(&self) -> Result<(), Vec> { From 8eb4abe64884c3b439512cebaa96a1d932ac47ba Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Thu, 5 Dec 2019 19:06:37 -0600 Subject: [PATCH 19/64] graph: Update SchemaReference type --- graph/src/data/schema.rs | 80 +++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 47 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 85f7de162ee..c266f51c979 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -53,6 +53,10 @@ pub enum SchemaValidationError { DerivedFromInvalid(String, String, String), // (type, field, reason) #[fail(display = "_schema_ type is solely for imports and should have no fields")] SubgraphSchemaTypeFieldsInvalid, + #[fail(display = "Name for imported subgraph `{}` is invalid", _0)] + ImportedSubgraphNameInvalid(String), + #[fail(display = "Id for imported subgraph `{}` is invalid", _0)] + ImportedSubgraphIdInvalid(String), #[fail(display = "_schema_ type only allows @import directives")] SubgraphSchemaDirectivesInvalid, #[fail(display = r#" @@ -78,20 +82,6 @@ pub enum SchemaImportError { ImportedSchemaNotFound(SchemaReference), #[fail(display = "Subgraph for imported schema `{}` is not deployed", _0)] ImportedSubgraphNotFound(SchemaReference), - #[fail(display = "Name for imported subgraph `{}` is invalid", _0)] - ImportedSubgraphNameInvalid(String), - #[fail(display = "Id for imported subgraph `{}` is invalid", _0)] - ImportedSubgraphIdInvalid(String), -} - -impl SchemaImportError { - pub fn is_failure(error: &Self) -> bool { - match error { - SchemaImportError::ImportedSubgraphNameInvalid(_) - | SchemaImportError::ImportedSubgraphIdInvalid(_) => true, - _ => false, - } - } } #[derive(Clone, Debug, PartialEq, Eq)] @@ -123,8 +113,8 @@ impl fmt::Display for ImportedType { #[derive(Clone, Debug, PartialEq, Eq)] pub enum SchemaReference { - ByName(String), - ById(String), + ByName(SubgraphName), + ById(SubgraphDeploymentId), } impl Hash for SchemaReference { @@ -147,23 +137,17 @@ impl fmt::Display for SchemaReference { impl SchemaReference { pub fn resolve( - self, + &self, store: Arc, ) -> Result<(Arc, SubgraphDeploymentId), SchemaImportError> { - let subgraph_id = match &self { - SchemaReference::ByName(name) => { - let subgraph_name = SubgraphName::new(name.clone()) - .map_err(|_| SchemaImportError::ImportedSubgraphNameInvalid(name.clone()))?; - store - .resolve_subgraph_name_to_id(subgraph_name.clone()) - .map_err(|_| SchemaImportError::ImportedSubgraphNotFound(self.clone())) - .and_then(|subgraph_id_opt| { - subgraph_id_opt - .ok_or(SchemaImportError::ImportedSubgraphNotFound(self.clone())) - })? - } - SchemaReference::ById(id) => SubgraphDeploymentId::new(id.clone()) - .map_err(|_| SchemaImportError::ImportedSubgraphIdInvalid(id.clone()))?, + let subgraph_id = match self { + SchemaReference::ByName(name) => store + .resolve_subgraph_name_to_id(name.clone()) + .map_err(|_| SchemaImportError::ImportedSubgraphNotFound(self.clone())) + .and_then(|subgraph_id_opt| { + subgraph_id_opt.ok_or(SchemaImportError::ImportedSubgraphNotFound(self.clone())) + })?, + SchemaReference::ById(id) => id.clone(), }; store @@ -401,24 +385,26 @@ impl Schema { Value::Object(map) => { let id = map .get("id") - .filter(|id| match id { - Value::String(_) => true, - _ => false, + .into_iter() + .filter_map(|id| match id { + Value::String(i) => match SubgraphDeploymentId::new(i) { + Ok(sid) => Some(SchemaReference::ById(sid)), + _ => None, + }, + _ => None, }) - .map(|id| match id { - Value::String(i) => SchemaReference::ById(i.to_string()), - _ => unreachable!(), - }); + .next(); let name = map .get("name") - .filter(|name| match name { - Value::String(_) => true, - _ => false, + .into_iter() + .filter_map(|name| match name { + Value::String(n) => match SubgraphName::new(n) { + Ok(sn) => Some(SchemaReference::ByName(sn)), + _ => None, + }, + _ => None, }) - .map(|name| match name { - Value::String(n) => SchemaReference::ByName(n.to_string()), - _ => unreachable!(), - }); + .next(); id.or(name) } _ => None, @@ -649,8 +635,8 @@ impl Schema { // Ensure that the imported type is in one of those two sets let schema_handle = match schema_ref { - SchemaReference::ById(id) => id, - SchemaReference::ByName(name) => name, + SchemaReference::ById(id) => id.to_string(), + SchemaReference::ByName(name) => name.to_string(), }; let name = match imported_type { ImportedType::Name(name) => name, From 8f75eac4996d01bceabbaf7f4d377d0bb64ff5fb Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 6 Dec 2019 12:52:21 -0600 Subject: [PATCH 20/64] graph: Validated subgraph name and id in imports directives --- graph/src/data/schema.rs | 102 ++++++++++++++++++++++++++++----------- 1 file changed, 73 insertions(+), 29 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index c266f51c979..805fc231e40 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -254,7 +254,7 @@ impl Schema { .iter() .find_map(|def| match def { schema::Definition::TypeDefinition(TypeDefinition::Interface(i)) - if i.name == implemented_interface => + if i.name.eq(&implemented_interface) => { Some(i.clone()) } @@ -302,12 +302,12 @@ impl Schema { object .directives .iter() - .filter(|directive| directive.name == "imports") + .filter(|directive| directive.name.eq("imports")) .map(|imports| { imports .arguments .iter() - .find(|(name, _)| name == "from") + .find(|(name, _)| name.eq("from")) .map_or(vec![], |from| { self.schema_reference_from_directive_argument(from).map_or( vec![], @@ -332,8 +332,10 @@ impl Schema { object .directives .iter() - .filter(|directive| directive.name == "imports") - .filter_map(|directive| directive.arguments.iter().find(|(name, _)| name == "from")) + .filter(|directive| directive.name.eq("imports")) + .filter_map(|directive| { + directive.arguments.iter().find(|(name, _)| name.eq("from")) + }) .filter_map(|from| self.schema_reference_from_directive_argument(from)) .collect() }) @@ -343,7 +345,7 @@ impl Schema { imports .arguments .iter() - .find(|(name, _)| name == "types") + .find(|(name, _)| name.eq("types")) .map_or(vec![], |(_, value)| match value { Value::List(types) => types .iter() @@ -449,7 +451,7 @@ impl Schema { if directives .iter() - .find(|directive| directive.name == "subgraphId") + .find(|directive| directive.name.eq("subgraphId")) .is_none() { directives.push(subgraph_id_directive); @@ -477,7 +479,7 @@ impl Schema { .unwrap_or_else(|err| errors.push(err)); // Should validate that import directives on the _schema_ type are properly formed self.validate_import_directives() - .unwrap_or_else(|err| errors.push(err)); + .unwrap_or_else(|mut err| errors.append(&mut err)); // Should validate that all types in the Subgraph referenced from other subgraphs exist // If the referenced subgraph is not provided as an argument, do not validate those types self.validate_imported_types(schemas) @@ -595,24 +597,63 @@ impl Schema { .map_or(false, |_| true) } - fn validate_import_directives(&self) -> Result<(), SchemaValidationError> { - match self + fn validate_import_directives(&self) -> Result<(), Vec> { + let errors = self .subgraph_schema_object_type() - .and_then(|subgraph_schema_type| { + .map_or(vec![], |subgraph_schema_type| { subgraph_schema_type .directives .iter() - .filter(|directive| directive.name == "imports") - // Look for an invalid directive - .find(|directive| { - let has_valid_types = Self::import_directive_has_valid_types(directive); - let has_valid_from = Self::import_directive_has_valid_from(directive); - !has_valid_types || !has_valid_from + .filter(|directive| directive.name.eq("imports")) + .fold(vec![], |mut errors, imports| { + // Check for badly formed import directives + let has_valid_types = Self::import_directive_has_valid_types(imports); + let has_valid_from = Self::import_directive_has_valid_from(imports); + if !has_valid_types || !has_valid_from { + errors.push(SchemaValidationError::ImportDirectiveInvalid) + } + + // Check for a badly formed subgraph id or name + imports + .arguments + .iter() + .find(|(name, _)| name.eq("from")) + .iter() + .for_each(|(_, from)| match from { + Value::Object(obj) => { + obj.get("id").iter().for_each(|id| match id { + Value::String(i) => match SubgraphDeploymentId::new(i) { + Err(_) => errors.push( + SchemaValidationError::ImportedSubgraphIdInvalid( + i.clone(), + ), + ), + _ => (), + }, + _ => (), + }); + obj.get("name").iter().for_each(|name| match name { + Value::String(n) => match SubgraphName::new(n) { + Err(_) => errors.push( + SchemaValidationError::ImportedSubgraphNameInvalid( + n.clone(), + ), + ), + _ => (), + }, + _ => (), + }); + } + _ => (), + }); + + errors }) - .map(|_| SchemaValidationError::ImportDirectiveInvalid) - }) { - Some(err) => Err(err), - None => Ok(()), + }); + + match errors.is_empty() { + true => Ok(()), + false => Err(errors), } } @@ -695,8 +736,8 @@ impl Schema { imported_types .iter() .find(|(imported_type, _)| match imported_type { - ImportedType::Name(name) if name == base => true, - ImportedType::NameAs(_, az) if az == base => true, + ImportedType::Name(name) if name.eq(base) => true, + ImportedType::NameAs(_, az) if az.eq(base) => true, _ => false, }) .map_or(Err(()), |_| Ok(())) @@ -775,7 +816,10 @@ impl Schema { // as the type of the field traversal::find_interface(&self.document, iface) .map(|iface| { - iface.fields.iter().any(|ifield| ifield.name == field.name) + iface + .fields + .iter() + .any(|ifield| ifield.name.eq(&field.name)) }) .unwrap_or(false) }) @@ -784,7 +828,7 @@ impl Schema { directive .arguments .iter() - .find(|(name, _)| name == "field") + .find(|(name, _)| name.eq("field")) .map(|(_, value)| value), ) }) @@ -825,7 +869,7 @@ impl Schema { // right name and type let target_field = target_fields .iter() - .find(|field| &field.name == target_field) + .find(|field| field.name.eq(target_field)) .ok_or_else(|| { let msg = format!( "field `{}` does not exist on type `{}`", @@ -843,7 +887,7 @@ impl Schema { && target_field_type != "ID" && !interface_types .iter() - .any(|iface| &target_field_type == iface) + .any(|iface| target_field_type.eq(iface.clone())) { fn type_signatures(name: &String) -> Vec { vec![ @@ -885,7 +929,7 @@ impl Schema { if object .fields .iter() - .find(|o| o.name == i.name && o.field_type == i.field_type) + .find(|o| o.name.eq(&i.name) && o.field_type.eq(&i.field_type)) .is_none() { missing_fields.push(i.to_string().trim().to_owned()); @@ -905,7 +949,7 @@ impl Schema { fn subgraph_schema_object_type(&self) -> Option<&ObjectType> { traversal::get_object_type_definitions(&self.document) .into_iter() - .find(|object_type| object_type.name == SCHEMA_TYPE_NAME) + .find(|object_type| object_type.name.eq(SCHEMA_TYPE_NAME)) } } From be3491aac56a306c96c2ffbe492575373b19aa52 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 6 Dec 2019 17:39:58 -0600 Subject: [PATCH 21/64] graph: Migrate the tests to schema --- graph/src/data/graphql/traversal.rs | 65 ------------------------ graph/src/data/schema.rs | 76 ++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 66 deletions(-) diff --git a/graph/src/data/graphql/traversal.rs b/graph/src/data/graphql/traversal.rs index 9b46a06cb48..ec1f0dc24fa 100644 --- a/graph/src/data/graphql/traversal.rs +++ b/graph/src/data/graphql/traversal.rs @@ -56,68 +56,3 @@ pub fn find_derived_from<'a>(field: &'a Field) -> Option<&'a Directive> { .iter() .find(|dir| dir.name == "derivedFrom") } - -// #[test] -// fn test_derived_from_validation() { -// const OTHER_TYPES: &str = " -// type B @entity { id: ID! } -// type C @entity { id: ID! } -// type D @entity { id: ID! } -// type E @entity { id: ID! } -// type F @entity { id: ID! } -// type G @entity { id: ID! a: BigInt } -// type H @entity { id: ID! a: A! } -// # This sets up a situation where we need to allow `Transaction.from` to -// # point to an interface because of `Account.txn` -// type Transaction @entity { from: Address! } -// interface Address { txn: Transaction! @derivedFrom(field: \"from\") } -// type Account implements Address @entity { id: ID!, txn: Transaction! @derivedFrom(field: \"from\") }"; - -// fn validate(field: &str, errmsg: &str) { -// let raw = format!("type A @entity {{ id: ID!\n {} }}\n{}", field, OTHER_TYPES); - -// let document = graphql_parser::parse_schema(&raw).expect("Failed to parse raw schema"); -// match validate_derived_from(&document) { -// Err(ref e) => match e { -// SchemaValidationError::DerivedFromInvalid(_, _, msg) => assert_eq!(errmsg, msg), -// _ => panic!("expected variant SchemaValidationError::DerivedFromInvalid"), -// }, -// Ok(_) => { -// if errmsg != "ok" { -// panic!("expected validation for `{}` to fail", field) -// } -// } -// } -// } - -// validate( -// "b: B @derivedFrom(field: \"a\")", -// "field `a` does not exist on type `B`", -// ); -// validate( -// "c: [C!]! @derivedFrom(field: \"a\")", -// "field `a` does not exist on type `C`", -// ); -// validate( -// "d: D @derivedFrom", -// "the @derivedFrom directive must have a `field` argument", -// ); -// validate( -// "e: E @derivedFrom(attr: \"a\")", -// "the @derivedFrom directive must have a `field` argument", -// ); -// validate( -// "f: F @derivedFrom(field: 123)", -// "the value of the @derivedFrom `field` argument must be a string", -// ); -// validate( -// "g: G @derivedFrom(field: \"a\")", -// "field `a` on type `G` must have one of the following types: A, A!, [A!], [A!]!", -// ); -// validate("h: H @derivedFrom(field: \"a\")", "ok"); -// validate( -// "i: NotAType @derivedFrom(field: \"a\")", -// "the type of the field must be an existing entity or interface type", -// ); -// validate("j: B @derivedFrom(field: \"id\")", "ok"); -// } diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 805fc231e40..b22769215f5 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -907,7 +907,7 @@ impl Schema { let valid_types = valid_types.join(", "); let msg = format!( - "field `{tf}` on type `{tt}` must have one of the following type: {valid_types}", + "field `{tf}` on type `{tt}` must have one of the following types: {valid_types}", tf = target_field.name, tt = target_type_name, valid_types = valid_types, @@ -986,3 +986,77 @@ fn invalid_interface_implementation() { required fields: x: Int, y: Int" ); } + +#[test] +fn test_derived_from_validation() { + const OTHER_TYPES: &str = " +type B @entity { id: ID! } +type C @entity { id: ID! } +type D @entity { id: ID! } +type E @entity { id: ID! } +type F @entity { id: ID! } +type G @entity { id: ID! a: BigInt } +type H @entity { id: ID! a: A! } +# This sets up a situation where we need to allow `Transaction.from` to +# point to an interface because of `Account.txn` +type Transaction @entity { from: Address! } +interface Address { txn: Transaction! @derivedFrom(field: \"from\") } +type Account implements Address @entity { id: ID!, txn: Transaction! @derivedFrom(field: \"from\") }"; + + fn validate(field: &str, errmsg: &str) { + let raw = format!("type A @entity {{ id: ID!\n {} }}\n{}", field, OTHER_TYPES); + + let document = graphql_parser::parse_schema(&raw).expect("Failed to parse raw schema"); + let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); + match schema.validate(&HashMap::new()) { + Err(ref errors) => { + errors + .iter() + .find(|e| match e { + SchemaValidationError::DerivedFromInvalid(_, _, msg) => { + assert_eq!(errmsg, msg); + true + } + _ => false, + }) + .expect("expected variant SchemaValidationError::DerivedFromInvalid"); + } + Ok(_) => { + if !errmsg.eq("ok") { + panic!("expected validation for `{}` to fail", field); + } + } + }; + } + + validate( + "b: B @derivedFrom(field: \"a\")", + "field `a` does not exist on type `B`", + ); + validate( + "c: [C!]! @derivedFrom(field: \"a\")", + "field `a` does not exist on type `C`", + ); + validate( + "d: D @derivedFrom", + "the @derivedFrom directive must have a `field` argument", + ); + validate( + "e: E @derivedFrom(attr: \"a\")", + "the @derivedFrom directive must have a `field` argument", + ); + validate( + "f: F @derivedFrom(field: 123)", + "the value of the @derivedFrom `field` argument must be a string", + ); + validate( + "g: G @derivedFrom(field: \"a\")", + "field `a` on type `G` must have one of the following types: A, A!, [A!], [A!]!", + ); + validate("h: H @derivedFrom(field: \"a\")", "ok"); + validate( + "i: NotAType @derivedFrom(field: \"a\")", + "the type of the field must be an existing entity or interface type", + ); + validate("j: B @derivedFrom(field: \"id\")", "ok"); +} From 3b3bcdab001a69e7ad2bd394df627144c33b27d7 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 6 Dec 2019 17:48:32 -0600 Subject: [PATCH 22/64] graph: Update assertions on test --- graph/src/data/schema.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index b22769215f5..aa53d1449ea 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -1008,25 +1008,17 @@ type Account implements Address @entity { id: ID!, txn: Transaction! @derivedFro let document = graphql_parser::parse_schema(&raw).expect("Failed to parse raw schema"); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); - match schema.validate(&HashMap::new()) { - Err(ref errors) => { - errors - .iter() - .find(|e| match e { - SchemaValidationError::DerivedFromInvalid(_, _, msg) => { - assert_eq!(errmsg, msg); - true - } - _ => false, - }) - .expect("expected variant SchemaValidationError::DerivedFromInvalid"); - } + match schema.validate_derived_from() { + Err(ref e) => match e { + SchemaValidationError::DerivedFromInvalid(_, _, msg) => assert_eq!(errmsg, msg), + _ => panic!("expected variant SchemaValidationError::DerivedFromInvalid"), + }, Ok(_) => { - if !errmsg.eq("ok") { - panic!("expected validation for `{}` to fail", field); + if errmsg != "ok" { + panic!("expected validation for `{}` to fail", field) } } - }; + } } validate( From ba3a328081830fd56d53500644e5029bbf4c9ce6 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 6 Dec 2019 18:15:53 -0600 Subject: [PATCH 23/64] graph: Update fail string --- graph/src/data/schema.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index aa53d1449ea..661c85f2d12 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -59,10 +59,9 @@ pub enum SchemaValidationError { ImportedSubgraphIdInvalid(String), #[fail(display = "_schema_ type only allows @import directives")] SubgraphSchemaDirectivesInvalid, - #[fail(display = r#" -@imports directives must be defined in one of the following forms: \ -@imports(types: ["A", {{ name: "B", as: "C"}}], from: {{ name: "org/subgraph"}}) \ -@imports(types: ["A", {{ name: "B", as: "C"}}], from: {{ id: "Qm..."}})")]"#)] + #[fail( + display = r#"@imports directives must be defined in one of the following forms: @imports(types: ["A", {{ name: "B", as: "C"}}], from: {{ name: "org/subgraph"}}) @imports(types: ["A", {{ name: "B", as: "C"}}], from: {{ id: "Qm..."}})")]"# + )] ImportDirectiveInvalid, #[fail( display = "GraphQL type `{}` has field `{}` with type `{}` which is not defined or imported", From 1453a896cf19f4577259bbc73e21401d458a3d55 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 6 Dec 2019 18:19:26 -0600 Subject: [PATCH 24/64] graph: Edit fail string --- graph/src/data/schema.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 661c85f2d12..ce9f0dc9079 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -60,7 +60,7 @@ pub enum SchemaValidationError { #[fail(display = "_schema_ type only allows @import directives")] SubgraphSchemaDirectivesInvalid, #[fail( - display = r#"@imports directives must be defined in one of the following forms: @imports(types: ["A", {{ name: "B", as: "C"}}], from: {{ name: "org/subgraph"}}) @imports(types: ["A", {{ name: "B", as: "C"}}], from: {{ id: "Qm..."}})")]"# + display = "@imports directives must be defined in one of the following forms: @imports(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ name: 'org/subgraph'}}) @imports(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ id: 'Qm...'}})" )] ImportDirectiveInvalid, #[fail( From 59ba60b8722ae583feee2f3297f4f2bbad62ffd7 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 6 Dec 2019 18:45:19 -0600 Subject: [PATCH 25/64] graph: Remove superfluous comments --- graph/src/data/schema.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index ce9f0dc9079..9fb3b3aa72b 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -470,20 +470,14 @@ impl Schema { .unwrap_or_else(|err| errors.push(err)); self.validate_fields() .unwrap_or_else(|mut err| errors.append(&mut err)); - self.validate_subgraph_schema_has_no_fields() .unwrap_or_else(|err| errors.push(err)); - // Should verify that only import directives exist on the _schema_ type self.validate_only_import_directives_on_reserved_type() .unwrap_or_else(|err| errors.push(err)); - // Should validate that import directives on the _schema_ type are properly formed self.validate_import_directives() .unwrap_or_else(|mut err| errors.append(&mut err)); - // Should validate that all types in the Subgraph referenced from other subgraphs exist - // If the referenced subgraph is not provided as an argument, do not validate those types self.validate_imported_types(schemas) .unwrap_or_else(|errs| errors.extend(errs)); - if errors.is_empty() { Ok(()) } else { From 7d1025e7134f3b5e3fa26afb8f4b941564b17c3b Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 6 Dec 2019 18:49:11 -0600 Subject: [PATCH 26/64] graph: Update comments --- graph/src/data/schema.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 9fb3b3aa72b..63a85d997c9 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -658,16 +658,17 @@ impl Schema { self.imported_types() .iter() .fold(vec![], |mut errors, (imported_type, schema_ref)| { - // See if `schemas` has the schema associated with `schema_ref` schemas .get(schema_ref) .and_then(|schema| { - // Get the defined types in the schema and the imported types let native_types = traversal::get_object_type_definitions(&schema.document); let imported_types = schema.imported_types(); - // Ensure that the imported type is in one of those two sets + // Ensure that the imported type is either native to + // the respective schema or is itself imported + // If the imported type is itself imported, do not + // recursively check the schema let schema_handle = match schema_ref { SchemaReference::ById(id) => id.to_string(), SchemaReference::ByName(name) => name.to_string(), From 3eb173139ba9ef4f040b352e3f0fb25eee015a0d Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Sun, 8 Dec 2019 12:03:24 -0600 Subject: [PATCH 27/64] graph: Add validation tests for imported schemas --- graph/src/data/schema.rs | 156 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 149 insertions(+), 7 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 63a85d997c9..4921a3339c3 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -52,13 +52,13 @@ pub enum SchemaValidationError { )] DerivedFromInvalid(String, String, String), // (type, field, reason) #[fail(display = "_schema_ type is solely for imports and should have no fields")] - SubgraphSchemaTypeFieldsInvalid, + ReservedTypeFieldsInvalid, #[fail(display = "Name for imported subgraph `{}` is invalid", _0)] ImportedSubgraphNameInvalid(String), #[fail(display = "Id for imported subgraph `{}` is invalid", _0)] ImportedSubgraphIdInvalid(String), #[fail(display = "_schema_ type only allows @import directives")] - SubgraphSchemaDirectivesInvalid, + ReservedTypeDirectivesInvalid, #[fail( display = "@imports directives must be defined in one of the following forms: @imports(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ name: 'org/subgraph'}}) @imports(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ id: 'Qm...'}})" )] @@ -72,7 +72,7 @@ pub enum SchemaValidationError { display = "Imported type `{}` does not exist in the `{}` schema", _0, _1 )] - ImportedTypeDNE(String, String), + ImportedTypeDNE(String, String), // (type_name, schema) } #[derive(Debug, Fail, PartialEq, Eq, Clone)] @@ -470,7 +470,7 @@ impl Schema { .unwrap_or_else(|err| errors.push(err)); self.validate_fields() .unwrap_or_else(|mut err| errors.append(&mut err)); - self.validate_subgraph_schema_has_no_fields() + self.validate_reserved_type_has_no_fields() .unwrap_or_else(|err| errors.push(err)); self.validate_only_import_directives_on_reserved_type() .unwrap_or_else(|err| errors.push(err)); @@ -485,12 +485,12 @@ impl Schema { } } - fn validate_subgraph_schema_has_no_fields(&self) -> Result<(), SchemaValidationError> { + fn validate_reserved_type_has_no_fields(&self) -> Result<(), SchemaValidationError> { match self .subgraph_schema_object_type() .and_then(|subgraph_schema_type| { if !subgraph_schema_type.fields.is_empty() { - Some(SchemaValidationError::SubgraphSchemaTypeFieldsInvalid) + Some(SchemaValidationError::ReservedTypeFieldsInvalid) } else { None } @@ -513,7 +513,7 @@ impl Schema { .collect::>() .is_empty() { - Some(SchemaValidationError::SubgraphSchemaDirectivesInvalid) + Some(SchemaValidationError::ReservedTypeDirectivesInvalid) } else { None } @@ -1046,3 +1046,145 @@ type Account implements Address @entity { id: ID!, txn: Transaction! @derivedFro ); validate("j: B @derivedFrom(field: \"id\")", "ok"); } + +#[test] +fn test_reserved_type_with_fields() { + const ROOT_SCHEMA: &str = " +type _schema_ { id: ID! }"; + + let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); + let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); + match schema.validate_reserved_type_has_no_fields() { + Err(e) => assert_eq!(e, SchemaValidationError::ReservedTypeFieldsInvalid), + Ok(_) => panic!( + "Expected validation for `{}` to fail due to fields defined on the reserved type", + ROOT_SCHEMA, + ), + } +} + +#[test] +fn test_reserved_type_directives() { + const ROOT_SCHEMA: &str = " +type _schema_ @illegal"; + + let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); + let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); + match schema.validate_only_import_directives_on_reserved_type() { + Err(e) => assert_eq!(e, SchemaValidationError::ReservedTypeDirectivesInvalid), + Ok(_) => panic!( + "Expected validation for `{}` to fail due to extra imports defined on the reserved type", + ROOT_SCHEMA, + ), + } +} + +#[test] +fn test_imports_directive_from_argument() { + const ROOT_SCHEMA: &str = r#" +type _schema_ @imports(types: ["T", "A", "C"])"#; + + let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); + let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); + match schema.validate_import_directives() { + Err(errors) => match errors.into_iter().find(|err| *err == SchemaValidationError::ImportDirectiveInvalid) { + None => panic!( + "Expected validation for `{}` to fail due to an @imports directive without a `from` argument", + ROOT_SCHEMA, + ), + _ => (), + }, + Ok(_) => panic!( + "Expected validation for `{}` to fail due to an @imports directive without a `from` argument", + ROOT_SCHEMA, + ), + } +} + +#[test] +fn test_recursively_imported_type_validates() { + const ROOT_SCHEMA: &str = r#" +type _schema_ @imports(types: ["T"], from: { name: "child1/subgraph" })"#; + const CHILD_1_SCHEMA: &str = r#" +type _schema_ @imports(types: ["T"], from: { name: "child2/subgraph" })"#; + const CHILD_2_SCHEMA: &str = r#" +type T @entity { id: ID! } +"#; + + let root_document = + graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); + let child_1_document = + graphql_parser::parse_schema(CHILD_1_SCHEMA).expect("Failed to parse child 1 schema"); + let child_2_document = + graphql_parser::parse_schema(CHILD_2_SCHEMA).expect("Failed to parse child 2 schema"); + + let root_schema = Schema::new(SubgraphDeploymentId::new("rid").unwrap(), root_document); + let child_1_schema = Schema::new(SubgraphDeploymentId::new("c1id").unwrap(), child_1_document); + let child_2_schema = Schema::new(SubgraphDeploymentId::new("c2id").unwrap(), child_2_document); + + let mut schemas = HashMap::new(); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("childone/subgraph").unwrap()), + Arc::new(child_1_schema), + ); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("childtwo/subgraph").unwrap()), + Arc::new(child_2_schema), + ); + + match root_schema.validate_imported_types(&schemas) { + Err(errors) => panic!( + "Expected imported types validation for `{}` to suceed", + ROOT_SCHEMA, + ), + Ok(_) => (), + } +} + +fn test_recursively_imported_type_which_dne_fails_validation() { + const ROOT_SCHEMA: &str = r#" +type _schema_ @imports(types: ["T"], from: { name: "child1/subgraph" })"#; + const CHILD_1_SCHEMA: &str = r#" +type _schema_ @imports(types: [{name: "T", as: "A"}], from: { name: "child2/subgraph" })"#; + const CHILD_2_SCHEMA: &str = r#" +type T @entity { id: ID! } +"#; + + let root_document = + graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); + let child_1_document = + graphql_parser::parse_schema(CHILD_1_SCHEMA).expect("Failed to parse child 1 schema"); + let child_2_document = + graphql_parser::parse_schema(CHILD_2_SCHEMA).expect("Failed to parse child 2 schema"); + + let root_schema = Schema::new(SubgraphDeploymentId::new("rid").unwrap(), root_document); + let child_1_schema = Schema::new(SubgraphDeploymentId::new("c1id").unwrap(), child_1_document); + let child_2_schema = Schema::new(SubgraphDeploymentId::new("c2id").unwrap(), child_2_document); + + let mut schemas = HashMap::new(); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("childone/subgraph").unwrap()), + Arc::new(child_1_schema), + ); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("childtwo/subgraph").unwrap()), + Arc::new(child_2_schema), + ); + + match root_schema.validate_imported_types(&schemas) { + Err(errors) => match errors.into_iter().find(|err| match err { + SchemaValidationError::ImportedTypeDNE(_, _) => true, + _ => false, + }) { + None => panic!( + "Expected imported types validation for `{}` to fail because an imported type was missing in the target schema", + ROOT_SCHEMA, + ), + _ => (), + }, + Ok(_) => panic!( + "Expected imported types validation for `{}` to fail because an imported type was missing in the target schema", + ROOT_SCHEMA, + ), + } +} From ea5f178e0f1d1df99e404cfdc0d6d20646aacb5f Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Sun, 8 Dec 2019 15:11:23 -0600 Subject: [PATCH 28/64] graph: Update tests for validating imported types --- graph/src/data/schema.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 4921a3339c3..9650f16691d 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -677,6 +677,7 @@ impl Schema { ImportedType::Name(name) => name, ImportedType::NameAs(name, _) => name, }; + let is_native = native_types .iter() .find(|object| object.name.eq(name)) @@ -1133,7 +1134,7 @@ type T @entity { id: ID! } ); match root_schema.validate_imported_types(&schemas) { - Err(errors) => panic!( + Err(_) => panic!( "Expected imported types validation for `{}` to suceed", ROOT_SCHEMA, ), @@ -1141,11 +1142,12 @@ type T @entity { id: ID! } } } +#[test] fn test_recursively_imported_type_which_dne_fails_validation() { const ROOT_SCHEMA: &str = r#" -type _schema_ @imports(types: ["T"], from: { name: "child1/subgraph" })"#; +type _schema_ @imports(types: ["T"], from: { name: "childone/subgraph" })"#; const CHILD_1_SCHEMA: &str = r#" -type _schema_ @imports(types: [{name: "T", as: "A"}], from: { name: "child2/subgraph" })"#; +type _schema_ @imports(types: [{name: "T", as: "A"}], from: { name: "childtwo/subgraph" })"#; const CHILD_2_SCHEMA: &str = r#" type T @entity { id: ID! } "#; @@ -1177,14 +1179,12 @@ type T @entity { id: ID! } _ => false, }) { None => panic!( - "Expected imported types validation for `{}` to fail because an imported type was missing in the target schema", - ROOT_SCHEMA, + "Expected imported types validation to fail because an imported type was missing in the target schema", ), _ => (), }, Ok(_) => panic!( - "Expected imported types validation for `{}` to fail because an imported type was missing in the target schema", - ROOT_SCHEMA, + "Expected imported types validation to fail because an imported type was missing in the target schema", ), } } From 22198522e50b8e5cf2dcede8b52a79b7824fe974 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Sun, 8 Dec 2019 17:29:44 -0600 Subject: [PATCH 29/64] graph: Rename method on Schema --- graph/src/data/schema.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 9650f16691d..a94acb2987e 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -311,7 +311,7 @@ impl Schema { self.schema_reference_from_directive_argument(from).map_or( vec![], |schema_ref| { - self.imported_types_from_import_directive(imports) + self.imported_types_from_imports_directive(imports) .iter() .map(|imported_type| { (imported_type.clone(), schema_ref.clone()) @@ -340,7 +340,7 @@ impl Schema { }) } - fn imported_types_from_import_directive(&self, imports: &Directive) -> Vec { + fn imported_types_from_imports_directive(&self, imports: &Directive) -> Vec { imports .arguments .iter() From 509f225483d1617c5c0d3d72a4cd4e3d50ae6434 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Sun, 8 Dec 2019 21:53:22 -0600 Subject: [PATCH 30/64] graph: Shorten validation with if let --- graph/src/data/schema.rs | 47 ++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index a94acb2987e..2791a5e613f 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -612,32 +612,31 @@ impl Schema { .iter() .find(|(name, _)| name.eq("from")) .iter() - .for_each(|(_, from)| match from { - Value::Object(obj) => { - obj.get("id").iter().for_each(|id| match id { - Value::String(i) => match SubgraphDeploymentId::new(i) { - Err(_) => errors.push( - SchemaValidationError::ImportedSubgraphIdInvalid( - i.clone(), - ), - ), - _ => (), - }, - _ => (), - }); - obj.get("name").iter().for_each(|name| match name { - Value::String(n) => match SubgraphName::new(n) { - Err(_) => errors.push( - SchemaValidationError::ImportedSubgraphNameInvalid( - n.clone(), - ), - ), - _ => (), - }, - _ => (), + .for_each(|(_, from)| { + if let Value::Object(obj) = from { + obj.get("id").iter().for_each(|id| { + if let Value::String(i) = id { + if let Err(_) = SubgraphDeploymentId::new(i) { + errors.push( + SchemaValidationError::ImportedSubgraphIdInvalid( + i.clone(), + ), + ) + } + } }); + obj.get("name").iter().for_each(|name| { + if let Value::String(n) = name { + if let Err(_) = SubgraphName::new(n) { + errors.push( + SchemaValidationError::ImportedSubgraphNameInvalid( + n.clone(), + ), + ); + } + } + }) } - _ => (), }); errors From 52db556ea35149d2cf02f16acf2577b8290dc8e4 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Sun, 8 Dec 2019 22:12:14 -0600 Subject: [PATCH 31/64] graph: Remove extra whitespace --- graph/src/data/schema.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 2791a5e613f..92b07924460 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -605,7 +605,6 @@ impl Schema { if !has_valid_types || !has_valid_from { errors.push(SchemaValidationError::ImportDirectiveInvalid) } - // Check for a badly formed subgraph id or name imports .arguments @@ -621,7 +620,7 @@ impl Schema { SchemaValidationError::ImportedSubgraphIdInvalid( i.clone(), ), - ) + ); } } }); @@ -632,7 +631,7 @@ impl Schema { SchemaValidationError::ImportedSubgraphNameInvalid( n.clone(), ), - ); + ); } } }) From 1401b9f2a7693eea4b637809582b5c1f696345cc Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Sun, 8 Dec 2019 22:16:45 -0600 Subject: [PATCH 32/64] graph: Propogate warnings --- graph/src/data/subgraph/mod.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index ed1a2ccc73b..a5b2657d862 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -323,7 +323,7 @@ pub enum SubgraphAssignmentProviderEvent { #[derive(Fail, Debug)] pub enum SubgraphManifestValidationWarning { #[fail(display = "schema validation produced warnings: {:?}", _0)] - SchemaValidationWarning(Vec), + SchemaValidationWarning(SchemaImportError), } #[derive(Fail, Debug)] @@ -853,7 +853,11 @@ impl UnvalidatedSubgraphManifest { (SubgraphManifest, Vec), Vec, > { - let (schemas, _import_errors) = self.0.schema.resolve_schema_references(store); + let (schemas, import_errors) = self.0.schema.resolve_schema_references(store); + let validation_warnings = import_errors + .into_iter() + .map(|err| SubgraphManifestValidationWarning::SchemaValidationWarning(err)) + .collect(); let mut errors: Vec = vec![]; @@ -913,7 +917,7 @@ impl UnvalidatedSubgraphManifest { }); match errors.is_empty() { - true => Ok((self.0, vec![])), + true => Ok((self.0, validation_warnings)), false => Err(errors), } } From 650d9385575d8c21398ccb07a921db545f2a79cc Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 13 Dec 2019 12:28:15 -0600 Subject: [PATCH 33/64] graph: _schema -> _Schema_ --- graph/src/data/schema.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 92b07924460..f3f9c5714b0 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -20,7 +20,7 @@ use std::hash::{Hash, Hasher}; use std::iter::FromIterator; use std::sync::Arc; -pub const SCHEMA_TYPE_NAME: &str = "_schema_"; +pub const SCHEMA_TYPE_NAME: &str = "_Schema_"; #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct Strings(Vec); @@ -51,13 +51,13 @@ pub enum SchemaValidationError { _1, _0, _2 )] DerivedFromInvalid(String, String, String), // (type, field, reason) - #[fail(display = "_schema_ type is solely for imports and should have no fields")] + #[fail(display = "_Schema_ type is solely for imports and should have no fields")] ReservedTypeFieldsInvalid, #[fail(display = "Name for imported subgraph `{}` is invalid", _0)] ImportedSubgraphNameInvalid(String), #[fail(display = "Id for imported subgraph `{}` is invalid", _0)] ImportedSubgraphIdInvalid(String), - #[fail(display = "_schema_ type only allows @import directives")] + #[fail(display = "_Schema_ type only allows @import directives")] ReservedTypeDirectivesInvalid, #[fail( display = "@imports directives must be defined in one of the following forms: @imports(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ name: 'org/subgraph'}}) @imports(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ id: 'Qm...'}})" @@ -1049,7 +1049,7 @@ type Account implements Address @entity { id: ID!, txn: Transaction! @derivedFro #[test] fn test_reserved_type_with_fields() { const ROOT_SCHEMA: &str = " -type _schema_ { id: ID! }"; +type _Schema_ { id: ID! }"; let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); @@ -1065,7 +1065,7 @@ type _schema_ { id: ID! }"; #[test] fn test_reserved_type_directives() { const ROOT_SCHEMA: &str = " -type _schema_ @illegal"; +type _Schema_ @illegal"; let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); @@ -1081,7 +1081,7 @@ type _schema_ @illegal"; #[test] fn test_imports_directive_from_argument() { const ROOT_SCHEMA: &str = r#" -type _schema_ @imports(types: ["T", "A", "C"])"#; +type _Schema_ @imports(types: ["T", "A", "C"])"#; let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); @@ -1103,9 +1103,9 @@ type _schema_ @imports(types: ["T", "A", "C"])"#; #[test] fn test_recursively_imported_type_validates() { const ROOT_SCHEMA: &str = r#" -type _schema_ @imports(types: ["T"], from: { name: "child1/subgraph" })"#; +type _Schema_ @imports(types: ["T"], from: { name: "child1/subgraph" })"#; const CHILD_1_SCHEMA: &str = r#" -type _schema_ @imports(types: ["T"], from: { name: "child2/subgraph" })"#; +type _Schema_ @imports(types: ["T"], from: { name: "child2/subgraph" })"#; const CHILD_2_SCHEMA: &str = r#" type T @entity { id: ID! } "#; @@ -1143,9 +1143,9 @@ type T @entity { id: ID! } #[test] fn test_recursively_imported_type_which_dne_fails_validation() { const ROOT_SCHEMA: &str = r#" -type _schema_ @imports(types: ["T"], from: { name: "childone/subgraph" })"#; +type _Schema_ @imports(types: ["T"], from: { name: "childone/subgraph" })"#; const CHILD_1_SCHEMA: &str = r#" -type _schema_ @imports(types: [{name: "T", as: "A"}], from: { name: "childtwo/subgraph" })"#; +type _Schema_ @imports(types: [{name: "T", as: "A"}], from: { name: "childtwo/subgraph" })"#; const CHILD_2_SCHEMA: &str = r#" type T @entity { id: ID! } "#; From 42302177b1a37ea7dda43a8aba0936f8a96e5394 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 17 Dec 2019 13:40:38 -0600 Subject: [PATCH 34/64] core: Rename cloned loggers and stores --- core/src/subgraph/registrar.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/core/src/subgraph/registrar.rs b/core/src/subgraph/registrar.rs index 9bbda76f235..55e0281b163 100644 --- a/core/src/subgraph/registrar.rs +++ b/core/src/subgraph/registrar.rs @@ -295,28 +295,30 @@ where hash: SubgraphDeploymentId, node_id: NodeId, ) -> Box + Send + 'static> { - let store_1 = self.store.clone(); - let store_2 = self.store.clone(); + let store_for_validation = self.store.clone(); + let store_for_subgraph_version = self.store.clone(); let chain_stores = self.chain_stores.clone(); let ethereum_adapters = self.ethereum_adapters.clone(); let version_switching_mode = self.version_switching_mode; let logger = self.logger_factory.subgraph_logger(&hash); - let logger2 = logger.clone(); - let logger3 = logger.clone(); + let logger_for_subgraph_version = logger.clone(); + let logger_for_debug = logger.clone(); let name_inner = name.clone(); Box::new( UnvalidatedSubgraphManifest::resolve( hash.to_ipfs_link(), self.resolver.clone(), - logger.clone(), + logger, ) .map_err(SubgraphRegistrarError::ResolveError) .and_then(move |unvalidated| { - future::result(unvalidated.validate(store_1)).map_err(|validation_errors| { - SubgraphRegistrarError::ManifestValidationError(validation_errors) - }) + future::result(unvalidated.validate(store_for_validation)).map_err( + |validation_errors| { + SubgraphRegistrarError::ManifestValidationError(validation_errors) + }, + ) }) .and_then(move |(manifest, validation_warnings)| { manifest @@ -350,8 +352,8 @@ where move |(manifest, ethereum_adapter, chain_store, _validation_warnings)| { let manifest_id = manifest.id.clone(); create_subgraph_version( - &logger2, - store_2, + &logger_for_subgraph_version, + store_for_subgraph_version, chain_store.clone(), ethereum_adapter.clone(), name, @@ -364,7 +366,7 @@ where ) .and_then(move |manifest_id| { debug!( - logger3, + logger_for_debug, "Wrote new subgraph version to store"; "subgraph_name" => name_inner.to_string(), "subgraph_hash" => manifest_id.to_string(), From 886819f572122967e7bdb66233c3ce583481c8db Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 17 Dec 2019 16:26:19 -0600 Subject: [PATCH 35/64] core, graph: Validate network names in UnvalidatedSubgraphManifest --- core/src/subgraph/instance.rs | 2 +- core/src/subgraph/instance_manager.rs | 78 ++++++++++++--------------- core/src/subgraph/registrar.rs | 36 ++++++------- graph/src/data/subgraph/mod.rs | 38 +++++++------ 4 files changed, 73 insertions(+), 81 deletions(-) diff --git a/core/src/subgraph/instance.rs b/core/src/subgraph/instance.rs index a829ba1fc85..c42a9c00049 100644 --- a/core/src/subgraph/instance.rs +++ b/core/src/subgraph/instance.rs @@ -41,7 +41,7 @@ where host_metrics: Arc, ) -> Result { let subgraph_id = manifest.id.clone(); - let network = manifest.network_name()?; + let network = manifest.network_name(); let templates = manifest.templates; let mut this = SubgraphInstance { diff --git a/core/src/subgraph/instance_manager.rs b/core/src/subgraph/instance_manager.rs index 4ac4e1dc3e2..2eb514a8a7c 100644 --- a/core/src/subgraph/instance_manager.rs +++ b/core/src/subgraph/instance_manager.rs @@ -247,52 +247,42 @@ impl SubgraphInstanceManager { "Start subgraph"; "data_sources" => manifest.data_sources.len() ); - - match manifest.network_name() { - Ok(n) => { - Self::start_subgraph( - logger.clone(), - instances.clone(), - host_builder.clone(), - block_stream_builder.clone(), - stores - .get(&n) - .expect(&format!( - "expected store that matches subgraph network: {}", - &n - )) - .clone(), - eth_adapters - .get(&n) - .expect(&format!( - "expected eth adapter that matches subgraph network: {}", - &n - )) - .clone(), - manifest, - metrics_registry_for_subgraph.clone(), - ) - .map_err(|err| { - error!( - logger, - "Failed to start subgraph"; - "error" => format!("{}", err), - "code" => LogCode::SubgraphStartFailure - ) - }) - .and_then(|_| { - manager_metrics.subgraph_count.inc(); - Ok(()) - }) - .ok(); - } - Err(err) => error!( + let network = manifest.network_name(); + Self::start_subgraph( + logger.clone(), + instances.clone(), + host_builder.clone(), + block_stream_builder.clone(), + stores + .get(&network) + .expect(&format!( + "expected store that matches subgraph network: {}", + &network + )) + .clone(), + eth_adapters + .get(&network) + .expect(&format!( + "expected eth adapter that matches subgraph network: {}", + &network + )) + .clone(), + manifest, + metrics_registry_for_subgraph.clone(), + ) + .map_err(|err| { + error!( logger, "Failed to start subgraph"; - "error" => format!("{}", err), + "error" => format!("{}", err), "code" => LogCode::SubgraphStartFailure - ), - }; + ) + }) + .and_then(|_| { + manager_metrics.subgraph_count.inc(); + Ok(()) + }) + .ok(); } SubgraphStop(id) => { let logger = logger_factory.subgraph_logger(&id); @@ -338,7 +328,7 @@ impl SubgraphInstanceManager { // Clone the deployment ID for later let deployment_id = manifest.id.clone(); - let network_name = manifest.network_name()?; + let network_name = manifest.network_name(); // Obtain filters from the manifest let log_filter = EthereumLogFilter::from_data_sources(&manifest.data_sources); diff --git a/core/src/subgraph/registrar.rs b/core/src/subgraph/registrar.rs index 55e0281b163..7d9bf2177b1 100644 --- a/core/src/subgraph/registrar.rs +++ b/core/src/subgraph/registrar.rs @@ -321,30 +321,26 @@ where ) }) .and_then(move |(manifest, validation_warnings)| { - manifest - .network_name() - .map_err(|e| SubgraphRegistrarError::ManifestValidationError(vec![e])) - .and_then(move |network_name| { - chain_stores - .clone() + let network_name = manifest.network_name(); + chain_stores + .clone() + .get(&network_name) + .ok_or(SubgraphRegistrarError::NetworkNotSupported( + network_name.clone(), + )) + .and_then(move |chain_store| { + ethereum_adapters .get(&network_name) .ok_or(SubgraphRegistrarError::NetworkNotSupported( network_name.clone(), )) - .and_then(move |chain_store| { - ethereum_adapters - .get(&network_name) - .ok_or(SubgraphRegistrarError::NetworkNotSupported( - network_name.clone(), - )) - .map(move |ethereum_adapter| { - ( - manifest, - ethereum_adapter.clone(), - chain_store.clone(), - validation_warnings, - ) - }) + .map(move |ethereum_adapter| { + ( + manifest, + ethereum_adapter.clone(), + chain_store.clone(), + validation_warnings, + ) }) }) }) diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index a5b2657d862..5daffd8d5d0 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -900,11 +900,26 @@ impl UnvalidatedSubgraphManifest { }); return non_filtered_block_handler_count > 1 || call_filtered_block_handler_count > 1; }); - if has_too_many_block_handlers { errors.push(SubgraphManifestValidationError::DataSourceBlockHandlerLimitExceeded) } + let mut networks = self + .0 + .data_sources + .iter() + .cloned() + .filter(|d| d.kind.eq("ethereum/contract")) + .filter_map(|d| d.network) + .collect::>(); + networks.sort(); + networks.dedup(); + match networks.len() { + 0 => errors.push(SubgraphManifestValidationError::EthereumNetworkRequired), + 1 => (), + _ => errors.push(SubgraphManifestValidationError::MultipleEthereumNetworks), + } + self.0 .schema .validate(&schemas) @@ -971,24 +986,15 @@ impl SubgraphManifest { }) } - pub fn network_name(&self) -> Result { - let mut ethereum_networks: Vec> = self - .data_sources + pub fn network_name(&self) -> String { + // Assume the manifest has been validated, ensuring network names are homogenous + self.data_sources .iter() .cloned() .filter(|d| d.kind == "ethereum/contract".to_string()) - .map(|d| d.network) - .collect(); - ethereum_networks.sort(); - ethereum_networks.dedup(); - match ethereum_networks.len() { - 0 => Err(SubgraphManifestValidationError::EthereumNetworkRequired), - 1 => match ethereum_networks.first().and_then(|n| n.clone()) { - Some(n) => Ok(n), - None => Err(SubgraphManifestValidationError::EthereumNetworkRequired), - }, - _ => Err(SubgraphManifestValidationError::MultipleEthereumNetworks), - } + .filter_map(|d| d.network) + .next() + .expect("Validated manifest does not have a network defined on any datasource") } pub fn start_blocks(&self) -> Vec { From b4a1abbd88bca7981c73455d9b2fe0ebe3e88b04 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 17 Dec 2019 16:50:15 -0600 Subject: [PATCH 36/64] core: Log the validation warnings --- core/src/subgraph/registrar.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/src/subgraph/registrar.rs b/core/src/subgraph/registrar.rs index 7d9bf2177b1..e436795c521 100644 --- a/core/src/subgraph/registrar.rs +++ b/core/src/subgraph/registrar.rs @@ -345,7 +345,7 @@ where }) }) .and_then( - move |(manifest, ethereum_adapter, chain_store, _validation_warnings)| { + move |(manifest, ethereum_adapter, chain_store, validation_warnings)| { let manifest_id = manifest.id.clone(); create_subgraph_version( &logger_for_subgraph_version, @@ -357,15 +357,16 @@ where node_id, version_switching_mode, ) - .map(|_| manifest_id) + .map(|_| (manifest_id, validation_warnings)) }, ) - .and_then(move |manifest_id| { + .and_then(move |(manifest_id, validation_warnings)| { debug!( logger_for_debug, "Wrote new subgraph version to store"; "subgraph_name" => name_inner.to_string(), "subgraph_hash" => manifest_id.to_string(), + "validation_warnings" => format!("{:?}", validation_warnings), ); Ok(()) }), From 2950821c0673872666a1416cdf6184d8c839ca2e Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 17 Dec 2019 16:52:18 -0600 Subject: [PATCH 37/64] graph: Implement TryFrom<&str> for BuiltInScalarType --- graph/src/data/graphql/scalar.rs | 4 ++-- graph/src/data/schema.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/graph/src/data/graphql/scalar.rs b/graph/src/data/graphql/scalar.rs index 4806be19987..e85607977ca 100644 --- a/graph/src/data/graphql/scalar.rs +++ b/graph/src/data/graphql/scalar.rs @@ -10,10 +10,10 @@ pub enum BuiltInScalarType { ID, } -impl TryFrom<&String> for BuiltInScalarType { +impl TryFrom<&str> for BuiltInScalarType { type Error = (); - fn try_from(value: &String) -> Result { + fn try_from(value: &str) -> Result { match value.as_ref() { "Boolean" => Ok(BuiltInScalarType::Boolean), "Int" => Ok(BuiltInScalarType::Int), diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index f3f9c5714b0..b2a2798853b 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -718,7 +718,7 @@ impl Schema { .fold(vec![], |errors, (type_name, fields)| { fields.iter().fold(errors, |mut errors, field| { let base = traversal::get_base_type(&field.field_type); - match BuiltInScalarType::try_from(base) + match BuiltInScalarType::try_from(base.as_ref()) .map(|_| ()) .or_else(|_| match native_types.contains_key(base) { true => Ok(()), From 2121cfff539129eb58dafffe95f80ad910c684ed Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 17 Dec 2019 20:01:39 -0600 Subject: [PATCH 38/64] graph, graphql: Move graphql AST extension traits to graph crate --- graph/src/data/graphql/ext.rs | 95 +++++++++++++++++++++++++++++ graph/src/data/graphql/mod.rs | 6 +- graph/src/data/graphql/traversal.rs | 58 ------------------ graph/src/data/schema.rs | 86 +++++++++++++------------- graphql/src/execution/execution.rs | 6 +- graphql/src/schema/ext.rs | 17 ------ graphql/src/schema/mod.rs | 2 - graphql/src/store/prefetch.rs | 2 +- 8 files changed, 147 insertions(+), 125 deletions(-) create mode 100644 graph/src/data/graphql/ext.rs delete mode 100644 graph/src/data/graphql/traversal.rs delete mode 100644 graphql/src/schema/ext.rs diff --git a/graph/src/data/graphql/ext.rs b/graph/src/data/graphql/ext.rs new file mode 100644 index 00000000000..80417ec965b --- /dev/null +++ b/graph/src/data/graphql/ext.rs @@ -0,0 +1,95 @@ +use graphql_parser::schema::{ + Definition, Directive, Document, Field, InterfaceType, Name, ObjectType, Type, TypeDefinition, +}; + +use std::collections::HashMap; + +pub trait ObjectTypeExt { + fn field(&self, name: &Name) -> Option<&Field>; +} + +impl ObjectTypeExt for ObjectType { + fn field(&self, name: &Name) -> Option<&Field> { + self.fields.iter().find(|field| &field.name == name) + } +} + +impl ObjectTypeExt for InterfaceType { + fn field(&self, name: &Name) -> Option<&Field> { + self.fields.iter().find(|field| &field.name == name) + } +} + +pub trait DocumentExt { + fn get_object_type_definitions(&self) -> Vec<&ObjectType>; + + fn get_object_and_interface_type_fields(&self) -> HashMap<&Name, &Vec>; + + fn find_interface(&self, name: &str) -> Option<&InterfaceType>; +} + +impl DocumentExt for Document { + fn get_object_type_definitions(&self) -> Vec<&ObjectType> { + self.definitions + .iter() + .filter_map(|d| match d { + Definition::TypeDefinition(TypeDefinition::Object(t)) => Some(t), + _ => None, + }) + .collect() + } + + fn get_object_and_interface_type_fields(&self) -> HashMap<&Name, &Vec> { + self.definitions + .iter() + .filter_map(|d| match d { + Definition::TypeDefinition(TypeDefinition::Object(t)) => Some((&t.name, &t.fields)), + Definition::TypeDefinition(TypeDefinition::Interface(t)) => { + Some((&t.name, &t.fields)) + } + _ => None, + }) + .collect() + } + + fn find_interface(&self, name: &str) -> Option<&InterfaceType> { + self.definitions.iter().find_map(|d| match d { + Definition::TypeDefinition(TypeDefinition::Interface(t)) if t.name == name => Some(t), + _ => None, + }) + } +} + +pub trait TypeExt { + fn get_base_type(&self) -> &Name; +} + +impl TypeExt for Type { + fn get_base_type(&self) -> &Name { + match self { + Type::NamedType(name) => name, + Type::NonNullType(inner) => Self::get_base_type(&inner), + Type::ListType(inner) => Self::get_base_type(&inner), + } + } +} + +pub trait DirectiveFinder { + fn find_directive(&self, name: Name) -> Option<&Directive>; +} + +impl DirectiveFinder for ObjectType { + fn find_directive(&self, name: Name) -> Option<&Directive> { + self.directives + .iter() + .find(|directive| directive.name.eq(&name)) + } +} + +impl DirectiveFinder for Field { + fn find_directive(&self, name: Name) -> Option<&Directive> { + self.directives + .iter() + .find(|directive| directive.name.eq(&name)) + } +} diff --git a/graph/src/data/graphql/mod.rs b/graph/src/data/graphql/mod.rs index 71dba22e665..939df147547 100644 --- a/graph/src/data/graphql/mod.rs +++ b/graph/src/data/graphql/mod.rs @@ -1,11 +1,11 @@ mod serialization; -/// Utilities for validating GraphQL schemas. -pub mod traversal; - /// Types to represent built in scalar values in GraphQL documents pub mod scalar; +/// Traits to navigate the GraphQL AST +pub mod ext; + /// Utilities for working with GraphQL values. mod values; diff --git a/graph/src/data/graphql/traversal.rs b/graph/src/data/graphql/traversal.rs deleted file mode 100644 index ec1f0dc24fa..00000000000 --- a/graph/src/data/graphql/traversal.rs +++ /dev/null @@ -1,58 +0,0 @@ -use graphql_parser::schema::*; -use std::collections::HashMap; - -/// Returns all object type definitions in the schema. -pub fn get_object_type_definitions(schema: &Document) -> Vec<&ObjectType> { - schema - .definitions - .iter() - .filter_map(|d| match d { - Definition::TypeDefinition(TypeDefinition::Object(t)) => Some(t), - _ => None, - }) - .collect() -} - -/// Returns all object and interface type definitions in the schema. -pub fn get_object_and_interface_type_fields(schema: &Document) -> HashMap<&Name, &Vec> { - schema - .definitions - .iter() - .filter_map(|d| match d { - Definition::TypeDefinition(TypeDefinition::Object(t)) => Some((&t.name, &t.fields)), - Definition::TypeDefinition(TypeDefinition::Interface(t)) => Some((&t.name, &t.fields)), - _ => None, - }) - .collect() -} - -/// Looks up a directive in a object type, if it is provided. -pub fn get_object_type_directive(object_type: &ObjectType, name: Name) -> Option<&Directive> { - object_type - .directives - .iter() - .find(|directive| directive.name == name) -} - -/// Returns the underlying type for a GraphQL field type -pub fn get_base_type(field_type: &Type) -> &Name { - match field_type { - Type::NamedType(name) => name, - Type::NonNullType(inner) => get_base_type(&inner), - Type::ListType(inner) => get_base_type(&inner), - } -} - -pub fn find_interface<'a>(schema: &'a Document, name: &str) -> Option<&'a InterfaceType> { - schema.definitions.iter().find_map(|d| match d { - Definition::TypeDefinition(TypeDefinition::Interface(t)) if t.name == name => Some(t), - _ => None, - }) -} - -pub fn find_derived_from<'a>(field: &'a Field) -> Option<&'a Directive> { - field - .directives - .iter() - .find(|dir| dir.name == "derivedFrom") -} diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index b2a2798853b..22b495476f7 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -1,6 +1,6 @@ use crate::components::store::{Store, SubgraphDeploymentStore}; +use crate::data::graphql::ext::{DirectiveFinder, DocumentExt, ObjectTypeExt, TypeExt}; use crate::data::graphql::scalar::BuiltInScalarType; -use crate::data::graphql::traversal; use crate::data::subgraph::{SubgraphDeploymentId, SubgraphName}; use crate::prelude::Fail; @@ -246,7 +246,7 @@ impl Schema { })); let mut interfaces_for_type = BTreeMap::<_, Vec<_>>::new(); - for object_type in traversal::get_object_type_definitions(&document) { + for object_type in document.get_object_type_definitions() { for implemented_interface in object_type.implements_interfaces.clone() { let interface_type = document .definitions @@ -659,8 +659,7 @@ impl Schema { schemas .get(schema_ref) .and_then(|schema| { - let native_types = - traversal::get_object_type_definitions(&schema.document); + let native_types = schema.document.get_object_type_definitions(); let imported_types = schema.imported_types(); // Ensure that the imported type is either native to @@ -708,7 +707,7 @@ impl Schema { } fn validate_fields(&self) -> Result<(), Vec> { - let native_types = traversal::get_object_and_interface_type_fields(&self.document); + let native_types = self.document.get_object_and_interface_type_fields(); let imported_types = self.imported_types(); // For each field in the root_schema, verify that the field @@ -717,7 +716,7 @@ impl Schema { .iter() .fold(vec![], |errors, (type_name, fields)| { fields.iter().fold(errors, |mut errors, field| { - let base = traversal::get_base_type(&field.field_type); + let base = field.field_type.get_base_type(); match BuiltInScalarType::try_from(base.as_ref()) .map(|_| ()) .or_else(|_| match native_types.contains_key(base) { @@ -753,9 +752,11 @@ impl Schema { } fn validate_schema_types(&self) -> Result<(), SchemaValidationError> { - let types_without_entity_directive = traversal::get_object_type_definitions(&self.document) + let types_without_entity_directive = self + .document + .get_object_type_definitions() .iter() - .filter(|t| traversal::get_object_type_directive(t, String::from("entity")).is_none()) + .filter(|t| t.find_directive(String::from("entity")).is_none()) .map(|t| t.name.to_owned()) .collect::>(); if types_without_entity_directive.is_empty() { @@ -781,9 +782,8 @@ impl Schema { ) } - let type_definitions = traversal::get_object_type_definitions(&self.document); - let object_and_interface_type_fields = - traversal::get_object_and_interface_type_fields(&self.document); + let type_definitions = self.document.get_object_type_definitions(); + let object_and_interface_type_fields = self.document.get_object_and_interface_type_fields(); // Iterate over all derived fields in all entity types; include the // interface types that the entity with the `@derivedFrom` implements @@ -798,33 +798,36 @@ impl Schema { .map(move |field| (object_type, field)) }) .filter_map(|(object_type, field)| { - traversal::find_derived_from(field).map(|directive| { - ( - object_type, - object_type - .implements_interfaces - .iter() - .filter(|iface| { - // Any interface that has `field` can be used - // as the type of the field - traversal::find_interface(&self.document, iface) - .map(|iface| { - iface - .fields - .iter() - .any(|ifield| ifield.name.eq(&field.name)) - }) - .unwrap_or(false) - }) - .collect::>(), - field, - directive - .arguments - .iter() - .find(|(name, _)| name.eq("field")) - .map(|(_, value)| value), - ) - }) + field + .find_directive(String::from("derivedFrom")) + .map(|directive| { + ( + object_type, + object_type + .implements_interfaces + .iter() + .filter(|iface| { + // Any interface that has `field` can be used + // as the type of the field + self.document + .find_interface(iface) + .map(|iface| { + iface + .fields + .iter() + .any(|ifield| ifield.name.eq(&field.name)) + }) + .unwrap_or(false) + }) + .collect::>(), + field, + directive + .arguments + .iter() + .find(|(name, _)| name.eq("field")) + .map(|(_, value)| value), + ) + }) }) { // Turn `target_field` into the string name of the field @@ -847,7 +850,7 @@ impl Schema { }; // Check that the type we are deriving from exists - let target_type_name = traversal::get_base_type(&field.field_type); + let target_type_name = field.field_type.get_base_type(); let target_fields = object_and_interface_type_fields .get(target_type_name) .ok_or_else(|| { @@ -875,7 +878,7 @@ impl Schema { // exception, we allow deriving from the `id` of another type. // For that, we will wind up comparing the `id`s of the two types // when we query, and just assume that that's ok. - let target_field_type = traversal::get_base_type(&target_field.field_type); + let target_field_type = target_field.field_type.get_base_type(); if target_field_type != &object_type.name && target_field_type != "ID" && !interface_types @@ -940,7 +943,8 @@ impl Schema { } fn subgraph_schema_object_type(&self) -> Option<&ObjectType> { - traversal::get_object_type_definitions(&self.document) + self.document + .get_object_type_definitions() .into_iter() .find(|object_type| object_type.name.eq(SCHEMA_TYPE_NAME)) } diff --git a/graphql/src/execution/execution.rs b/graphql/src/execution/execution.rs index e0b00b34a29..dd099a326ee 100644 --- a/graphql/src/execution/execution.rs +++ b/graphql/src/execution/execution.rs @@ -7,7 +7,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::ops::Deref; use std::time::Instant; -use graph::data::graphql::traversal; +use graph::data::graphql::ext::TypeExt; use graph::prelude::*; use crate::introspection::INTROSPECTION_DOCUMENT; @@ -187,7 +187,7 @@ where .ok_or(Invalid)?; let field_complexity = self.query_complexity( - &get_named_type(schema, traversal::get_base_type(&s_field.field_type)) + &get_named_type(schema, s_field.field_type.get_base_type()) .ok_or(Invalid)?, &field.selection_set, max_depth, @@ -263,7 +263,7 @@ where match s_field { Some(s_field) => { - let base_type = traversal::get_base_type(&s_field.field_type); + let base_type = s_field.field_type.get_base_type(); match get_named_type(schema, base_type) { Some(ty) => errors.extend(self.validate_fields( base_type, diff --git a/graphql/src/schema/ext.rs b/graphql/src/schema/ext.rs deleted file mode 100644 index 3f1381cb7a1..00000000000 --- a/graphql/src/schema/ext.rs +++ /dev/null @@ -1,17 +0,0 @@ -use graphql_parser::schema as s; - -pub trait ObjectTypeExt { - fn field(&self, name: &s::Name) -> Option<&s::Field>; -} - -impl ObjectTypeExt for s::ObjectType { - fn field(&self, name: &s::Name) -> Option<&s::Field> { - self.fields.iter().find(|field| &field.name == name) - } -} - -impl ObjectTypeExt for s::InterfaceType { - fn field(&self, name: &s::Name) -> Option<&s::Field> { - self.fields.iter().find(|field| &field.name == name) - } -} diff --git a/graphql/src/schema/mod.rs b/graphql/src/schema/mod.rs index ccb7c191a70..6df51907471 100644 --- a/graphql/src/schema/mod.rs +++ b/graphql/src/schema/mod.rs @@ -5,5 +5,3 @@ pub mod api; pub mod ast; pub use self::api::{api_schema, APISchemaError}; - -pub mod ext; diff --git a/graphql/src/store/prefetch.rs b/graphql/src/store/prefetch.rs index b87955e3fec..f0bc0101ce4 100644 --- a/graphql/src/store/prefetch.rs +++ b/graphql/src/store/prefetch.rs @@ -10,6 +10,7 @@ use std::rc::Rc; use std::sync::Arc; use std::time::Instant; +use graph::data::graphql::ext::ObjectTypeExt; use graph::prelude::{ BlockNumber, Entity, EntityCollection, EntityFilter, EntityLink, EntityWindow, ParentLink, QueryExecutionError, Schema, Store, Value as StoreValue, WindowAttribute, @@ -18,7 +19,6 @@ use graph::prelude::{ use crate::execution::{ExecutionContext, ObjectOrInterface, Resolver}; use crate::query::ast as qast; use crate::schema::ast as sast; -use crate::schema::ext::ObjectTypeExt; use crate::store::build_query; lazy_static! { From a3db6119ffc5485f13c6e663e0ca3336ca138fb1 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 17 Dec 2019 20:06:31 -0600 Subject: [PATCH 39/64] schema: Add backticks in error msg around type name --- graph/src/data/schema.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 22b495476f7..962fa90c8fb 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -34,10 +34,10 @@ impl fmt::Display for Strings { #[derive(Debug, Fail, PartialEq, Eq)] pub enum SchemaValidationError { - #[fail(display = "Interface {} not defined", _0)] + #[fail(display = "Interface `{}` not defined", _0)] UndefinedInterface(String), - #[fail(display = "@entity directive missing on the following type: {}", _0)] + #[fail(display = "@entity directive missing on the following type: `{}`", _0)] EntityDirectivesMissing(Strings), #[fail( From 7a80b690b087d4e8bf7ac75d344a287e45dcdc91 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 17 Dec 2019 20:29:23 -0600 Subject: [PATCH 40/64] graph: Reanme SchemaValidationError enum variants --- graph/src/data/schema.rs | 42 ++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 962fa90c8fb..a47c4eaa381 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -35,30 +35,30 @@ impl fmt::Display for Strings { #[derive(Debug, Fail, PartialEq, Eq)] pub enum SchemaValidationError { #[fail(display = "Interface `{}` not defined", _0)] - UndefinedInterface(String), + InterfaceUndefined(String), - #[fail(display = "@entity directive missing on the following type: `{}`", _0)] + #[fail(display = "@entity directive missing on the following types: `{}`", _0)] EntityDirectivesMissing(Strings), #[fail( - display = "Entity type `{}` cannot implement `{}` because it is missing \ - the required fields: {}", + display = "Entity type `{}` does not satisfy interface `{}` because it is missing \ + the following fields: {}", _0, _1, _2 )] - CannotImplement(String, String, Strings), // (type, interface, missing_fields) + InterfaceFieldsMissing(String, String, Strings), // (type, interface, missing_fields) #[fail( display = "Field `{}` in type `{}` has invalid @derivedFrom: {}", _1, _0, _2 )] - DerivedFromInvalid(String, String, String), // (type, field, reason) - #[fail(display = "_Schema_ type is solely for imports and should have no fields")] - ReservedTypeFieldsInvalid, - #[fail(display = "Name for imported subgraph `{}` is invalid", _0)] + InvalidDerivedFrom(String, String, String), // (type, field, reason) + #[fail(display = "_Schema_ type is only for @imports and must not have any fields")] + SchemaTypeWithFields, + #[fail(display = "Imported subgraph name `{}` is invalid", _0)] ImportedSubgraphNameInvalid(String), - #[fail(display = "Id for imported subgraph `{}` is invalid", _0)] + #[fail(display = "Imported subgraph id `{}` is invalid", _0)] ImportedSubgraphIdInvalid(String), - #[fail(display = "_Schema_ type only allows @import directives")] - ReservedTypeDirectivesInvalid, + #[fail(display = "The _Schema_ type only allows @import directives")] + InvalidSchemaTypeDirectives, #[fail( display = "@imports directives must be defined in one of the following forms: @imports(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ name: 'org/subgraph'}}) @imports(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ id: 'Qm...'}})" )] @@ -260,7 +260,7 @@ impl Schema { _ => None, }) .ok_or_else(|| { - SchemaValidationError::UndefinedInterface(implemented_interface.clone()) + SchemaValidationError::InterfaceUndefined(implemented_interface.clone()) })?; Self::validate_interface_implementation(object_type, &interface_type)?; @@ -490,7 +490,7 @@ impl Schema { .subgraph_schema_object_type() .and_then(|subgraph_schema_type| { if !subgraph_schema_type.fields.is_empty() { - Some(SchemaValidationError::ReservedTypeFieldsInvalid) + Some(SchemaValidationError::SchemaTypeWithFields) } else { None } @@ -513,7 +513,7 @@ impl Schema { .collect::>() .is_empty() { - Some(SchemaValidationError::ReservedTypeDirectivesInvalid) + Some(SchemaValidationError::InvalidSchemaTypeDirectives) } else { None } @@ -775,7 +775,7 @@ impl Schema { field_name: &str, reason: &str, ) -> SchemaValidationError { - SchemaValidationError::DerivedFromInvalid( + SchemaValidationError::InvalidDerivedFrom( object_type.name.to_owned(), field_name.to_owned(), reason.to_owned(), @@ -932,7 +932,7 @@ impl Schema { } } if !missing_fields.is_empty() { - Err(SchemaValidationError::CannotImplement( + Err(SchemaValidationError::InterfaceFieldsMissing( object.name.clone(), interface.name.clone(), Strings(missing_fields), @@ -960,7 +960,7 @@ fn non_existing_interface() { .unwrap(); assert_eq!( error, - SchemaValidationError::UndefinedInterface("Bar".to_owned()) + SchemaValidationError::InterfaceUndefined("Bar".to_owned()) ); } @@ -1007,7 +1007,7 @@ type Account implements Address @entity { id: ID!, txn: Transaction! @derivedFro let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); match schema.validate_derived_from() { Err(ref e) => match e { - SchemaValidationError::DerivedFromInvalid(_, _, msg) => assert_eq!(errmsg, msg), + SchemaValidationError::InvalidDerivedFrom(_, _, msg) => assert_eq!(errmsg, msg), _ => panic!("expected variant SchemaValidationError::DerivedFromInvalid"), }, Ok(_) => { @@ -1058,7 +1058,7 @@ type _Schema_ { id: ID! }"; let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); match schema.validate_reserved_type_has_no_fields() { - Err(e) => assert_eq!(e, SchemaValidationError::ReservedTypeFieldsInvalid), + Err(e) => assert_eq!(e, SchemaValidationError::SchemaTypeWithFields), Ok(_) => panic!( "Expected validation for `{}` to fail due to fields defined on the reserved type", ROOT_SCHEMA, @@ -1074,7 +1074,7 @@ type _Schema_ @illegal"; let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); match schema.validate_only_import_directives_on_reserved_type() { - Err(e) => assert_eq!(e, SchemaValidationError::ReservedTypeDirectivesInvalid), + Err(e) => assert_eq!(e, SchemaValidationError::InvalidSchemaTypeDirectives), Ok(_) => panic!( "Expected validation for `{}` to fail due to extra imports defined on the reserved type", ROOT_SCHEMA, From 8fcfabb976258fa105e0292eb2599b55c9c80ef4 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 17 Dec 2019 20:37:48 -0600 Subject: [PATCH 41/64] graph: Address naming and msg feedback --- graph/src/data/schema.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index a47c4eaa381..7071e3064a5 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -72,7 +72,7 @@ pub enum SchemaValidationError { display = "Imported type `{}` does not exist in the `{}` schema", _0, _1 )] - ImportedTypeDNE(String, String), // (type_name, schema) + ImportedTypeUndefined(String, String), // (type_name, schema) } #[derive(Debug, Fail, PartialEq, Eq, Clone)] @@ -331,7 +331,7 @@ impl Schema { object .directives .iter() - .filter(|directive| directive.name.eq("imports")) + .filter(|directive| directive.name.eq("import")) .filter_map(|directive| { directive.arguments.iter().find(|(name, _)| name.eq("from")) }) @@ -348,7 +348,7 @@ impl Schema { .map_or(vec![], |(_, value)| match value { Value::List(types) => types .iter() - .filter_map(|import_type| match import_type { + .filter_map(|type_import| match type_import { Value::String(type_name) => Some(ImportedType::Name(type_name.to_string())), Value::Object(type_name_as) => { let name = @@ -509,7 +509,7 @@ impl Schema { if !subgraph_schema_type .directives .iter() - .filter(|directive| !directive.name.eq("imports")) + .filter(|directive| !directive.name.eq("import")) .collect::>() .is_empty() { @@ -597,7 +597,7 @@ impl Schema { subgraph_schema_type .directives .iter() - .filter(|directive| directive.name.eq("imports")) + .filter(|directive| directive.name.eq("import")) .fold(vec![], |mut errors, imports| { // Check for badly formed import directives let has_valid_types = Self::import_directive_has_valid_types(imports); @@ -687,7 +687,7 @@ impl Schema { }) .map_or(false, |_| true); if !is_native || !is_imported { - Some(SchemaValidationError::ImportedTypeDNE( + Some(SchemaValidationError::ImportedTypeUndefined( name.to_string(), schema_handle.to_string(), )) @@ -857,7 +857,7 @@ impl Schema { invalid( object_type, &field.name, - "the type of the field must be an existing entity or interface type", + "type must be an existing entity or interface", ) })?; From e4f75dabd0bc88060161d535c992d7353652426a Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 17 Dec 2019 20:40:09 -0600 Subject: [PATCH 42/64] graph: Remove TODO and make tuple values private --- graph/src/data/schema.rs | 2 +- graph/src/data/subgraph/mod.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 7071e3064a5..86334674955 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -1,5 +1,5 @@ use crate::components::store::{Store, SubgraphDeploymentStore}; -use crate::data::graphql::ext::{DirectiveFinder, DocumentExt, ObjectTypeExt, TypeExt}; +use crate::data::graphql::ext::{DirectiveFinder, DocumentExt, TypeExt}; use crate::data::graphql::scalar::BuiltInScalarType; use crate::data::subgraph::{SubgraphDeploymentId, SubgraphName}; use crate::prelude::Fail; diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index 5daffd8d5d0..e0868c8cbb6 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -831,8 +831,7 @@ type UnresolvedSubgraphManifest = pub type SubgraphManifest = BaseSubgraphManifest; /// Unvalidated SubgraphManifest -// TODO: Make the tuple fields private -pub struct UnvalidatedSubgraphManifest(pub SubgraphManifest); +pub struct UnvalidatedSubgraphManifest(SubgraphManifest); impl UnvalidatedSubgraphManifest { /// Entry point for resolving a subgraph definition. From 8d02fb3c28e53623fa48bf429f749562b774a185 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Wed, 18 Dec 2019 15:48:26 -0600 Subject: [PATCH 43/64] graph: Address comments about errors and hashing --- graph/src/data/schema.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 86334674955..de5012c3144 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -13,7 +13,7 @@ use graphql_parser::{ }; use serde::{Deserialize, Serialize}; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::convert::TryFrom; use std::fmt; use std::hash::{Hash, Hasher}; @@ -60,14 +60,14 @@ pub enum SchemaValidationError { #[fail(display = "The _Schema_ type only allows @import directives")] InvalidSchemaTypeDirectives, #[fail( - display = "@imports directives must be defined in one of the following forms: @imports(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ name: 'org/subgraph'}}) @imports(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ id: 'Qm...'}})" + display = "@import directives must have the form @import(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ name: 'org/subgraph'}}) or @import(types: ['A', {{ name: 'B', as: 'C'}}], from: {{ id: 'Qm...'}})" )] ImportDirectiveInvalid, #[fail( - display = "GraphQL type `{}` has field `{}` with type `{}` which is not defined or imported", + display = "Type `{}`, field `{}`, type `{}` is neither defined or imported", _0, _1, _2 )] - GraphQLTypeFieldInvalid(String, String, String), // (type_name, field_name, field_type) + FieldTypeUnknown(String, String, String), // (type_name, field_name, field_type) #[fail( display = "Imported type `{}` does not exist in the `{}` schema", _0, _1 @@ -95,6 +95,7 @@ impl Hash for ImportedType { Self::Name(name) => name.hash(state), Self::NameAs(name, az) => { name.hash(state); + String::from(" as ").hash(state); az.hash(state); } }; @@ -190,7 +191,7 @@ impl Schema { Vec, ) { let mut schemas = HashMap::new(); - let mut visit_log = HashMap::new(); + let mut visit_log = HashSet::new(); let import_errors = self.resolve_import_graph(store, &mut schemas, &mut visit_log); (schemas, import_errors) } @@ -199,7 +200,7 @@ impl Schema { &self, store: Arc, schemas: &mut HashMap>, - visit_log: &mut HashMap>, + visit_log: &mut HashSet, ) -> Vec { // Use the visit log to detect cycles in the import graph self.imported_schemas() @@ -209,8 +210,8 @@ impl Schema { Ok((schema, subgraph_id)) => { schemas.insert(schema_ref, schema.clone()); // If this node in the graph has already been visited stop traversing - if !visit_log.contains_key(&subgraph_id) { - visit_log.insert(subgraph_id, schema.clone()); + if !visit_log.contains(&subgraph_id) { + visit_log.insert(subgraph_id); errors.extend(schema.resolve_import_graph( store.clone(), schemas, @@ -734,7 +735,7 @@ impl Schema { }) .map_or(Err(()), |_| Ok(())) }) { - Err(_) => errors.push(SchemaValidationError::GraphQLTypeFieldInvalid( + Err(_) => errors.push(SchemaValidationError::FieldTypeUnknown( type_name.to_string(), field.name.to_string(), base.to_string(), From 9fb0afdf04ce8b2b13468d9c9bf209d6f91b2142 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Wed, 18 Dec 2019 16:37:06 -0600 Subject: [PATCH 44/64] graph: Address schema.rs comment from review --- graph/src/data/schema.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index de5012c3144..9d9d8a852fb 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -470,15 +470,15 @@ impl Schema { self.validate_derived_from() .unwrap_or_else(|err| errors.push(err)); self.validate_fields() - .unwrap_or_else(|mut err| errors.append(&mut err)); - self.validate_reserved_type_has_no_fields() + .unwrap_or_else(|mut errs| errors.append(&mut errs)); + self.validate_schema_type_has_no_fields() .unwrap_or_else(|err| errors.push(err)); - self.validate_only_import_directives_on_reserved_type() + self.validate_only_import_directives_on_schema_type() .unwrap_or_else(|err| errors.push(err)); self.validate_import_directives() - .unwrap_or_else(|mut err| errors.append(&mut err)); + .unwrap_or_else(|mut errs| errors.append(&mut errs)); self.validate_imported_types(schemas) - .unwrap_or_else(|errs| errors.extend(errs)); + .unwrap_or_else(|mut errs| errors.append(&mut errs)); if errors.is_empty() { Ok(()) } else { @@ -486,7 +486,7 @@ impl Schema { } } - fn validate_reserved_type_has_no_fields(&self) -> Result<(), SchemaValidationError> { + fn validate_schema_type_has_no_fields(&self) -> Result<(), SchemaValidationError> { match self .subgraph_schema_object_type() .and_then(|subgraph_schema_type| { @@ -501,9 +501,7 @@ impl Schema { } } - fn validate_only_import_directives_on_reserved_type( - &self, - ) -> Result<(), SchemaValidationError> { + fn validate_only_import_directives_on_schema_type(&self) -> Result<(), SchemaValidationError> { match self .subgraph_schema_object_type() .and_then(|subgraph_schema_type| { @@ -845,7 +843,7 @@ impl Schema { return Err(invalid( object_type, &field.name, - "the value of the @derivedFrom `field` argument must be a string", + "the @derivedFrom `field` argument must be a string", )) } }; From 72552c31ae9a05c59ec1bd05e3a0e505cccca449 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Wed, 18 Dec 2019 18:09:21 -0600 Subject: [PATCH 45/64] graph: Overload variable names --- graph/src/data/schema.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 9d9d8a852fb..9c2ce8dddf9 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -312,7 +312,7 @@ impl Schema { self.schema_reference_from_directive_argument(from).map_or( vec![], |schema_ref| { - self.imported_types_from_imports_directive(imports) + self.imported_types_from_import_directive(imports) .iter() .map(|imported_type| { (imported_type.clone(), schema_ref.clone()) @@ -341,7 +341,7 @@ impl Schema { }) } - fn imported_types_from_imports_directive(&self, imports: &Directive) -> Vec { + fn imported_types_from_import_directive(&self, imports: &Directive) -> Vec { imports .arguments .iter() @@ -389,8 +389,8 @@ impl Schema { .get("id") .into_iter() .filter_map(|id| match id { - Value::String(i) => match SubgraphDeploymentId::new(i) { - Ok(sid) => Some(SchemaReference::ById(sid)), + Value::String(id) => match SubgraphDeploymentId::new(id) { + Ok(id) => Some(SchemaReference::ById(id)), _ => None, }, _ => None, @@ -400,8 +400,8 @@ impl Schema { .get("name") .into_iter() .filter_map(|name| match name { - Value::String(n) => match SubgraphName::new(n) { - Ok(sn) => Some(SchemaReference::ByName(sn)), + Value::String(name) => match SubgraphName::new(name) { + Ok(name) => Some(SchemaReference::ByName(name)), _ => None, }, _ => None, @@ -526,10 +526,8 @@ impl Schema { directive .arguments .iter() - .find(|(name, value)| { - if !name.eq("types") { - return false; - } + .filter(|(name, _)| name.eq("types")) + .find(|(_, value)| { match value { Value::List(values) => { // Each value must be a String or an Object with String:String key value From 24a24833c6e0a733a25af666c5ce85a2933f3f4c Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Thu, 19 Dec 2019 16:01:29 -0600 Subject: [PATCH 46/64] graph: Address validation comments --- graph/src/data/schema.rs | 202 +++++++++++++++++++-------------------- 1 file changed, 96 insertions(+), 106 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 9c2ce8dddf9..66584e728a4 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -475,8 +475,7 @@ impl Schema { .unwrap_or_else(|err| errors.push(err)); self.validate_only_import_directives_on_schema_type() .unwrap_or_else(|err| errors.push(err)); - self.validate_import_directives() - .unwrap_or_else(|mut errs| errors.append(&mut errs)); + errors.append(&mut self.validate_import_directives()); self.validate_imported_types(schemas) .unwrap_or_else(|mut errs| errors.append(&mut errs)); if errors.is_empty() { @@ -522,127 +521,118 @@ impl Schema { } } - fn import_directive_has_valid_types(directive: &Directive) -> bool { - directive + fn validate_import_type(typ: &Value) -> Result<(), ()> { + match typ { + Value::String(_) => Ok(()), + Value::Object(typ) => match (typ.get("name"), typ.get("as")) { + (Some(Value::String(_)), Some(Value::String(_))) => Ok(()), + _ => Err(()), + }, + _ => Err(()), + } + } + + fn is_import_directive_argument_types_valid(types: &Value) -> bool { + // All of the elements in the `types` field are valid: either a string or an object with keys `name` and `as` which are strings' + if let Value::List(types) = types { + types + .iter() + .try_for_each(Self::validate_import_type) + .err() + .is_none() + } else { + false + } + } + + fn is_import_directive_argument_from_valid(from: &Value) -> bool { + if let Value::Object(from) = from { + let has_id = match from.get("id") { + Some(Value::String(_)) => true, + _ => false, + }; + let has_name = match from.get("name") { + Some(Value::String(_)) => true, + _ => false, + }; + has_id ^ has_name + } else { + false + } + } + + fn validate_import_directive_arguments(directive: &Directive) -> Option { + let from_is_valid = directive .arguments .iter() - .filter(|(name, _)| name.eq("types")) - .find(|(_, value)| { - match value { - Value::List(values) => { - // Each value must be a String or an Object with String:String key value - // pairs for `name` and `as` - // Search for an invalid type in the list of imported types - values - .iter() - .find(|value| match value { - Value::String(_) => false, - Value::Object(obj) => { - let has_invalid_name = - obj.get("name").map_or(true, |value| match value { - Value::String(_) => false, - _ => true, - }); - let has_invalid_as = - obj.get("as").map_or(true, |value| match value { - Value::String(_) => false, - _ => true, - }); - has_invalid_name || has_invalid_as - } - _ => true, - }) - .map_or(true, |_| false) - } - _ => return false, - } - }) - .map_or(false, |_| true) + .find(|(name, _)| name.eq("from")) + .map_or(false, |(_, from)| { + Self::is_import_directive_argument_from_valid(from) + }); + let types_are_valid = directive + .arguments + .iter() + .find(|(name, _)| name.eq("types")) + .map_or(false, |(_, types)| { + Self::is_import_directive_argument_types_valid(types) + }); + if from_is_valid && types_are_valid { + None + } else { + Some(SchemaValidationError::ImportDirectiveInvalid) + } } - fn import_directive_has_valid_from(directive: &Directive) -> bool { + fn validate_import_directive_schema_reference_parses( + directive: &Directive, + ) -> Option { directive .arguments .iter() - // Look for a valid `from` argument - .find(|(name, value)| { - if !name.eq("from") { - return false; - } - match value { - Value::Object(obj) => { - let has_id = obj.get("id").map_or(false, |value| match value { - Value::String(_) => true, - _ => false, - }); - let has_name = obj.get("name").map_or(false, |value| match value { - Value::String(_) => true, - _ => false, - }); - has_id ^ has_name - } - _ => return false, + .find(|(name, _)| name.eq("from")) + .and_then(|(_, from)| match from { + Value::Object(from) => { + let id_parse_error = match from.get("id") { + Some(Value::String(id)) => match SubgraphDeploymentId::new(id) { + Err(_) => { + Some(SchemaValidationError::ImportedSubgraphIdInvalid(id.clone())) + } + _ => None, + }, + _ => None, + }; + let name_parse_error = match from.get("name") { + Some(Value::String(name)) => match SubgraphName::new(name) { + Err(_) => Some(SchemaValidationError::ImportedSubgraphNameInvalid( + name.clone(), + )), + _ => None, + }, + _ => None, + }; + id_parse_error.or(name_parse_error) } + _ => None, }) - .map_or(false, |_| true) } - fn validate_import_directives(&self) -> Result<(), Vec> { - let errors = self - .subgraph_schema_object_type() + fn validate_import_directives(&self) -> Vec { + self.subgraph_schema_object_type() .map_or(vec![], |subgraph_schema_type| { subgraph_schema_type .directives .iter() - .filter(|directive| directive.name.eq("import")) - .fold(vec![], |mut errors, imports| { - // Check for badly formed import directives - let has_valid_types = Self::import_directive_has_valid_types(imports); - let has_valid_from = Self::import_directive_has_valid_from(imports); - if !has_valid_types || !has_valid_from { - errors.push(SchemaValidationError::ImportDirectiveInvalid) - } - // Check for a badly formed subgraph id or name - imports - .arguments - .iter() - .find(|(name, _)| name.eq("from")) - .iter() - .for_each(|(_, from)| { - if let Value::Object(obj) = from { - obj.get("id").iter().for_each(|id| { - if let Value::String(i) = id { - if let Err(_) = SubgraphDeploymentId::new(i) { - errors.push( - SchemaValidationError::ImportedSubgraphIdInvalid( - i.clone(), - ), - ); - } - } - }); - obj.get("name").iter().for_each(|name| { - if let Value::String(n) = name { - if let Err(_) = SubgraphName::new(n) { - errors.push( - SchemaValidationError::ImportedSubgraphNameInvalid( - n.clone(), - ), - ); - } - } - }) - } - }); - + .filter(|directives| directives.name.eq("import")) + .fold(vec![], |mut errors, import| { + Self::validate_import_directive_arguments(import) + .into_iter() + .for_each(|err| errors.push(err)); + Self::validate_import_directive_schema_reference_parses(import) + .into_iter() + .for_each(|err| errors.push(err)); errors }) - }); - - match errors.is_empty() { - true => Ok(()), - false => Err(errors), - } + }) } fn validate_imported_types( From 67f5c562140d74d31def92067f93630085b0adb0 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Thu, 19 Dec 2019 17:04:52 -0600 Subject: [PATCH 47/64] graph: Better use of combinators --- graph/src/data/schema.rs | 60 ++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 66584e728a4..33c41586679 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -662,17 +662,12 @@ impl Schema { ImportedType::NameAs(name, _) => name, }; - let is_native = native_types - .iter() - .find(|object| object.name.eq(name)) - .map_or(false, |_| true); - let is_imported = imported_types - .iter() - .find(|(import, _)| match import { + let is_native = native_types.iter().any(|object| object.name.eq(name)); + let is_imported = + imported_types.iter().any(|(import, _)| match import { ImportedType::Name(n) => name.eq(n), ImportedType::NameAs(_, az) => name.eq(az), - }) - .map_or(false, |_| true); + }); if !is_native || !is_imported { Some(SchemaValidationError::ImportedTypeUndefined( name.to_string(), @@ -696,42 +691,35 @@ impl Schema { fn validate_fields(&self) -> Result<(), Vec> { let native_types = self.document.get_object_and_interface_type_fields(); let imported_types = self.imported_types(); - - // For each field in the root_schema, verify that the field - // is either a: [BuiltInScalar, Native, Imported] type let errors = native_types .iter() .fold(vec![], |errors, (type_name, fields)| { fields.iter().fold(errors, |mut errors, field| { let base = field.field_type.get_base_type(); - match BuiltInScalarType::try_from(base.as_ref()) - .map(|_| ()) - .or_else(|_| match native_types.contains_key(base) { - true => Ok(()), - false => Err(()), + if let Ok(_) = BuiltInScalarType::try_from(base.as_ref()) { + return errors; + } + if native_types.contains_key(base) { + return errors; + } + if imported_types + .iter() + .any(|(imported_type, _)| match imported_type { + ImportedType::Name(name) if name.eq(base) => true, + ImportedType::NameAs(_, az) if az.eq(base) => true, + _ => false, }) - .or_else(|_| { - // Check imported types and the corresponding schema - imported_types - .iter() - .find(|(imported_type, _)| match imported_type { - ImportedType::Name(name) if name.eq(base) => true, - ImportedType::NameAs(_, az) if az.eq(base) => true, - _ => false, - }) - .map_or(Err(()), |_| Ok(())) - }) { - Err(_) => errors.push(SchemaValidationError::FieldTypeUnknown( - type_name.to_string(), - field.name.to_string(), - base.to_string(), - )), - Ok(_) => (), - }; + { + return errors; + } + errors.push(SchemaValidationError::FieldTypeUnknown( + type_name.to_string(), + field.name.to_string(), + base.to_string(), + )); errors }) }); - match errors.is_empty() { false => Err(errors), true => Ok(()), From 8713b29d54b089d96e3f2b9f3389e8cab4d8f8c9 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Thu, 19 Dec 2019 17:18:15 -0600 Subject: [PATCH 48/64] graph: Leaner pattern matching --- graph/src/data/schema.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 33c41586679..410d435028a 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -352,19 +352,10 @@ impl Schema { .filter_map(|type_import| match type_import { Value::String(type_name) => Some(ImportedType::Name(type_name.to_string())), Value::Object(type_name_as) => { - let name = - type_name_as - .get("name") - .and_then(|name_value| match name_value { - Value::String(name) => Some(name.to_string()), - _ => None, - }); - let az = type_name_as.get("as").and_then(|as_value| match as_value { - Value::String(az) => Some(az.to_string()), - _ => None, - }); - match (name, az) { - (Some(name), Some(az)) => Some(ImportedType::NameAs(name, az)), + match (type_name_as.get("name"), type_name_as.get("as")) { + (Some(name), Some(az)) => { + Some(ImportedType::NameAs(name.to_string(), az.to_string())) + } _ => None, } } From 214d44b0cc9be3966ac5c9abd462f23357da9cf0 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Thu, 19 Dec 2019 17:29:03 -0600 Subject: [PATCH 49/64] graph: Better pattern matching --- graph/src/data/schema.rs | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 410d435028a..adc10ee2513 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -376,28 +376,20 @@ impl Schema { } match value { Value::Object(map) => { - let id = map + let id = match map .get("id") - .into_iter() - .filter_map(|id| match id { - Value::String(id) => match SubgraphDeploymentId::new(id) { - Ok(id) => Some(SchemaReference::ById(id)), - _ => None, - }, - _ => None, - }) - .next(); - let name = map + .map(|id| SubgraphDeploymentId::new(id.to_string())) + { + Some(Ok(id)) => Some(SchemaReference::ById(id)), + _ => None, + }; + let name = match map .get("name") - .into_iter() - .filter_map(|name| match name { - Value::String(name) => match SubgraphName::new(name) { - Ok(name) => Some(SchemaReference::ByName(name)), - _ => None, - }, - _ => None, - }) - .next(); + .map(|name| SubgraphName::new(name.to_string())) + { + Some(Ok(name)) => Some(SchemaReference::ByName(name)), + _ => None, + }; id.or(name) } _ => None, From a30807766890bf8c6fe2bd97545f2e31583654c1 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Thu, 19 Dec 2019 17:44:45 -0600 Subject: [PATCH 50/64] graph: Address feedback on collecting errors --- graph/src/data/schema.rs | 105 +++++++++++++++++---------------------- 1 file changed, 46 insertions(+), 59 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index adc10ee2513..d7abc11bd95 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -452,15 +452,13 @@ impl Schema { .unwrap_or_else(|err| errors.push(err)); self.validate_derived_from() .unwrap_or_else(|err| errors.push(err)); - self.validate_fields() - .unwrap_or_else(|mut errs| errors.append(&mut errs)); self.validate_schema_type_has_no_fields() .unwrap_or_else(|err| errors.push(err)); self.validate_only_import_directives_on_schema_type() .unwrap_or_else(|err| errors.push(err)); + errors.append(&mut self.validate_fields()); errors.append(&mut self.validate_import_directives()); - self.validate_imported_types(schemas) - .unwrap_or_else(|mut errs| errors.append(&mut errs)); + errors.append(&mut self.validate_imported_types(schemas)); if errors.is_empty() { Ok(()) } else { @@ -621,60 +619,53 @@ impl Schema { fn validate_imported_types( &self, schemas: &HashMap>, - ) -> Result<(), Vec> { - let errors = - self.imported_types() - .iter() - .fold(vec![], |mut errors, (imported_type, schema_ref)| { - schemas - .get(schema_ref) - .and_then(|schema| { - let native_types = schema.document.get_object_type_definitions(); - let imported_types = schema.imported_types(); - - // Ensure that the imported type is either native to - // the respective schema or is itself imported - // If the imported type is itself imported, do not - // recursively check the schema - let schema_handle = match schema_ref { - SchemaReference::ById(id) => id.to_string(), - SchemaReference::ByName(name) => name.to_string(), - }; - let name = match imported_type { - ImportedType::Name(name) => name, - ImportedType::NameAs(name, _) => name, - }; - - let is_native = native_types.iter().any(|object| object.name.eq(name)); - let is_imported = - imported_types.iter().any(|(import, _)| match import { - ImportedType::Name(n) => name.eq(n), - ImportedType::NameAs(_, az) => name.eq(az), - }); - if !is_native || !is_imported { - Some(SchemaValidationError::ImportedTypeUndefined( - name.to_string(), - schema_handle.to_string(), - )) - } else { - None - } - }) - .into_iter() - .for_each(|err| errors.push(err)); - errors - }); - - match errors.is_empty() { - true => Ok(()), - false => Err(errors), - } + ) -> Vec { + self.imported_types() + .iter() + .fold(vec![], |mut errors, (imported_type, schema_ref)| { + schemas + .get(schema_ref) + .and_then(|schema| { + let native_types = schema.document.get_object_type_definitions(); + let imported_types = schema.imported_types(); + + // Ensure that the imported type is either native to + // the respective schema or is itself imported + // If the imported type is itself imported, do not + // recursively check the schema + let schema_handle = match schema_ref { + SchemaReference::ById(id) => id.to_string(), + SchemaReference::ByName(name) => name.to_string(), + }; + let name = match imported_type { + ImportedType::Name(name) => name, + ImportedType::NameAs(name, _) => name, + }; + + let is_native = native_types.iter().any(|object| object.name.eq(name)); + let is_imported = imported_types.iter().any(|(import, _)| match import { + ImportedType::Name(n) => name.eq(n), + ImportedType::NameAs(_, az) => name.eq(az), + }); + if !is_native || !is_imported { + Some(SchemaValidationError::ImportedTypeUndefined( + name.to_string(), + schema_handle.to_string(), + )) + } else { + None + } + }) + .into_iter() + .for_each(|err| errors.push(err)); + errors + }) } - fn validate_fields(&self) -> Result<(), Vec> { + fn validate_fields(&self) -> Vec { let native_types = self.document.get_object_and_interface_type_fields(); let imported_types = self.imported_types(); - let errors = native_types + native_types .iter() .fold(vec![], |errors, (type_name, fields)| { fields.iter().fold(errors, |mut errors, field| { @@ -702,11 +693,7 @@ impl Schema { )); errors }) - }); - match errors.is_empty() { - false => Err(errors), - true => Ok(()), - } + }) } fn validate_schema_types(&self) -> Result<(), SchemaValidationError> { From 17f5d842540fbd9d65e5071e7de6187a998cf8e5 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Thu, 19 Dec 2019 20:22:44 -0600 Subject: [PATCH 51/64] graph: Tests passing --- graph/src/data/schema.rs | 86 ++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 48 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index d7abc11bd95..ef9d68b0721 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -302,7 +302,7 @@ impl Schema { object .directives .iter() - .filter(|directive| directive.name.eq("imports")) + .filter(|directive| directive.name.eq("import")) .map(|imports| { imports .arguments @@ -341,8 +341,8 @@ impl Schema { }) } - fn imported_types_from_import_directive(&self, imports: &Directive) -> Vec { - imports + fn imported_types_from_import_directive(&self, import: &Directive) -> Vec { + import .arguments .iter() .find(|(name, _)| name.eq("types")) @@ -371,23 +371,23 @@ impl Schema { from: &(Name, Value), ) -> Option { let (name, value) = from; - if name != "from" { + if !name.eq("from") { return None; } match value { Value::Object(map) => { - let id = match map - .get("id") - .map(|id| SubgraphDeploymentId::new(id.to_string())) - { - Some(Ok(id)) => Some(SchemaReference::ById(id)), + let id = match map.get("id") { + Some(Value::String(id)) => match SubgraphDeploymentId::new(id) { + Ok(id) => Some(SchemaReference::ById(id)), + _ => None, + }, _ => None, }; - let name = match map - .get("name") - .map(|name| SubgraphName::new(name.to_string())) - { - Some(Ok(name)) => Some(SchemaReference::ByName(name)), + let name = match map.get("name") { + Some(Value::String(name)) => match SubgraphName::new(name) { + Ok(name) => Some(SchemaReference::ByName(name)), + _ => None, + }, _ => None, }; id.or(name) @@ -647,7 +647,7 @@ impl Schema { ImportedType::Name(n) => name.eq(n), ImportedType::NameAs(_, az) => name.eq(az), }); - if !is_native || !is_imported { + if !is_native && !is_imported { Some(SchemaValidationError::ImportedTypeUndefined( name.to_string(), schema_handle.to_string(), @@ -924,8 +924,7 @@ fn invalid_interface_implementation() { let res = Schema::parse(schema, SubgraphDeploymentId::new("dummy").unwrap()); assert_eq!( res.unwrap_err().to_string(), - "Entity type `Bar` cannot implement `Foo` because it is missing the \ - required fields: x: Int, y: Int" + "Entity type `Bar` does not satisfy interface `Foo` because it is missing the following fields: x: Int, y: Int", ); } @@ -981,7 +980,7 @@ type Account implements Address @entity { id: ID!, txn: Transaction! @derivedFro ); validate( "f: F @derivedFrom(field: 123)", - "the value of the @derivedFrom `field` argument must be a string", + "the @derivedFrom `field` argument must be a string", ); validate( "g: G @derivedFrom(field: \"a\")", @@ -990,7 +989,7 @@ type Account implements Address @entity { id: ID!, txn: Transaction! @derivedFro validate("h: H @derivedFrom(field: \"a\")", "ok"); validate( "i: NotAType @derivedFrom(field: \"a\")", - "the type of the field must be an existing entity or interface type", + "type must be an existing entity or interface", ); validate("j: B @derivedFrom(field: \"id\")", "ok"); } @@ -1002,7 +1001,7 @@ type _Schema_ { id: ID! }"; let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); - match schema.validate_reserved_type_has_no_fields() { + match schema.validate_schema_type_has_no_fields() { Err(e) => assert_eq!(e, SchemaValidationError::SchemaTypeWithFields), Ok(_) => panic!( "Expected validation for `{}` to fail due to fields defined on the reserved type", @@ -1018,7 +1017,7 @@ type _Schema_ @illegal"; let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); - match schema.validate_only_import_directives_on_reserved_type() { + match schema.validate_only_import_directives_on_schema_type() { Err(e) => assert_eq!(e, SchemaValidationError::InvalidSchemaTypeDirectives), Ok(_) => panic!( "Expected validation for `{}` to fail due to extra imports defined on the reserved type", @@ -1030,31 +1029,28 @@ type _Schema_ @illegal"; #[test] fn test_imports_directive_from_argument() { const ROOT_SCHEMA: &str = r#" -type _Schema_ @imports(types: ["T", "A", "C"])"#; +type _Schema_ @import(types: ["T", "A", "C"])"#; let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); - match schema.validate_import_directives() { - Err(errors) => match errors.into_iter().find(|err| *err == SchemaValidationError::ImportDirectiveInvalid) { + match schema + .validate_import_directives() + .into_iter() + .find(|err| *err == SchemaValidationError::ImportDirectiveInvalid) { None => panic!( "Expected validation for `{}` to fail due to an @imports directive without a `from` argument", ROOT_SCHEMA, ), _ => (), - }, - Ok(_) => panic!( - "Expected validation for `{}` to fail due to an @imports directive without a `from` argument", - ROOT_SCHEMA, - ), } } #[test] fn test_recursively_imported_type_validates() { const ROOT_SCHEMA: &str = r#" -type _Schema_ @imports(types: ["T"], from: { name: "child1/subgraph" })"#; +type _Schema_ @import(types: ["T"], from: { name: "child1/subgraph" })"#; const CHILD_1_SCHEMA: &str = r#" -type _Schema_ @imports(types: ["T"], from: { name: "child2/subgraph" })"#; +type _Schema_ @import(types: ["T"], from: { name: "child2/subgraph" })"#; const CHILD_2_SCHEMA: &str = r#" type T @entity { id: ID! } "#; @@ -1080,25 +1076,24 @@ type T @entity { id: ID! } Arc::new(child_2_schema), ); - match root_schema.validate_imported_types(&schemas) { - Err(_) => panic!( + match root_schema.validate_imported_types(&schemas).is_empty() { + false => panic!( "Expected imported types validation for `{}` to suceed", ROOT_SCHEMA, ), - Ok(_) => (), + true => (), } } #[test] fn test_recursively_imported_type_which_dne_fails_validation() { const ROOT_SCHEMA: &str = r#" -type _Schema_ @imports(types: ["T"], from: { name: "childone/subgraph" })"#; +type _Schema_ @import(types: ["T"], from: { name:"childone/subgraph"})"#; const CHILD_1_SCHEMA: &str = r#" -type _Schema_ @imports(types: [{name: "T", as: "A"}], from: { name: "childtwo/subgraph" })"#; +type _Schema_ @import(types: [{name: "T", as: "A"}], from: { name:"childtwo/subgraph"})"#; const CHILD_2_SCHEMA: &str = r#" type T @entity { id: ID! } "#; - let root_document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); let child_1_document = @@ -1120,18 +1115,13 @@ type T @entity { id: ID! } Arc::new(child_2_schema), ); - match root_schema.validate_imported_types(&schemas) { - Err(errors) => match errors.into_iter().find(|err| match err { - SchemaValidationError::ImportedTypeDNE(_, _) => true, - _ => false, - }) { - None => panic!( - "Expected imported types validation to fail because an imported type was missing in the target schema", - ), - _ => (), - }, - Ok(_) => panic!( + match root_schema.validate_imported_types(&schemas).into_iter().find(|err| match err { + SchemaValidationError::ImportedTypeUndefined(_, _) => true, + _ => false, + }) { + None => panic!( "Expected imported types validation to fail because an imported type was missing in the target schema", ), + _ => (), } } From 8976ca0ae175a75010cbb356bd45224e7d1fe1cc Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Sat, 21 Dec 2019 23:18:33 -0600 Subject: [PATCH 52/64] graphql: Initial implementation of `merged_schema` --- graphql/src/schema/merge.rs | 171 ++++++++++++++++++++++++++++++++++++ graphql/src/schema/mod.rs | 3 + 2 files changed, 174 insertions(+) create mode 100644 graphql/src/schema/merge.rs diff --git a/graphql/src/schema/merge.rs b/graphql/src/schema/merge.rs new file mode 100644 index 00000000000..40fc09a36bf --- /dev/null +++ b/graphql/src/schema/merge.rs @@ -0,0 +1,171 @@ +use graphql_parser::{ + schema::{Definition, Directive, Field, ObjectType, Type, TypeDefinition, Value}, + Pos, +}; + +use crate::schema::ast; + +use graph::data::graphql::ext; +use graph::data::schema::{ImportedType, SchemaReference}; +use graph::prelude::*; + +use std::collections::HashMap; + +/// Optimistically merges a subgraph schema with all of its imports. +pub fn merged_schema( + root_schema: &Schema, + schemas: HashMap>, +) -> Schema { + // Create a Vec<(ImportedType, SchemaReference)> from the imported types in the root schema + // + // Iterate over the Vec<(ImportedType, SchemaReference)> and in each iteration, look up the schema + // which corresponds to the current element in the Vector. + // + // If the schema is not available, then add a placeholder type to the root schema. + // + // If the schema is available, copy the schema over. + // Check each field in the copied type and for non scalar fields, produce an (ImportedType, SchemaRefernce) + // tuple; the new vector element will either be for the same schema or for an imported schema. + // + // Copying a type: + // 1. Clone the type + // 2. Add a subgraph id directive + // 3. If the type is imported with { name : "...", as: "..." }, change the name and + // add an @originalName(name: "...") directive + // 4. Push it onto the schema.document.definitions + // + // QUESTION: How should naming conflicts be handled? + // A subgraph developer will probably ensure that an imported type does not conflict with local subgraph types. + // However, the non scalar fields of an imported type are also imported and those types might overlap with local + // subgraph types. What should we do in this case? + // Presumably overlapping type names will not be accepted by GraphQL clients. + let mut merged = root_schema.clone(); + let mut imports: Vec<(_, _)> = merged + .imported_types() + .iter() + .map(|(t, sr)| (t.clone(), sr.clone())) + .collect(); + + while let Some((import, schema_reference)) = imports.pop() { + match schemas.get(&schema_reference) { + Some(schema) => { + let subgraph_id = schema.id.clone(); + let (original_name, new_name) = match import { + ImportedType::Name(name) => (name.clone(), name), + ImportedType::NameAs(name, az) => (name, az), + }; + // Find the type + let local_type = schema + .document + .definitions + .iter() + .find(|definition| { + if let Definition::TypeDefinition(TypeDefinition::Object(obj)) = definition + { + obj.name.eq(&original_name) + } else { + false + } + }) + .map(|definition| match definition { + Definition::TypeDefinition(TypeDefinition::Object(obj)) => obj, + _ => unreachable!(), + }); + if let Some(obj) = local_type { + // 1. Clone the type + let mut new_obj = obj.clone(); + + // 2. Add a subgraph id directive + new_obj.directives.push(Directive { + position: Pos::default(), + name: String::from("subgraphId"), + arguments: vec![( + String::from("id"), + Value::String(subgraph_id.to_string()), + )], + }); + + // 3. If the type is imported with { name : "...", as: "..." }, change the name and + // add an @originalName(name: "...") directive + if !original_name.eq(&new_name) { + new_obj.name = new_name.clone(); + } + + // 4. Push it onto the schema.document.definitions + merged + .document + .definitions + .push(Definition::TypeDefinition(TypeDefinition::Object(new_obj))); + } else { + // Determine if the type is imported + // If it is imported, push a tuple onto the `imports` vector + if let Some((import, schema_reference)) = + schema + .imported_types() + .iter() + .find(|(import, schema_reference)| match import { + ImportedType::Name(name) if name.eq(&original_name) => true, + ImportedType::NameAs(_, az) if az.eq(&original_name) => true, + _ => false, + }) + { + let import = match import { + ImportedType::Name(name) if new_name.eq(name) => import.clone(), + ImportedType::Name(name) => { + ImportedType::NameAs(name.clone(), new_name) + } + ImportedType::NameAs(name, _) => { + ImportedType::NameAs(name.clone(), new_name) + } + }; + imports.push((import, schema_reference.clone())); + continue; + } + + // If it is not imported, then add a placeholder type + merged.document.definitions.push(placeholder_type( + new_name.clone(), + match new_name.eq(&original_name) { + true => None, + false => Some(original_name), + }, + )); + } + } + None => { + // Add a placeholder type to the root schema + } + } + } + + merged +} + +fn placeholder_type(name: String, original_name: Option) -> Definition { + let mut obj = ObjectType::new(name); + + // Add id field + obj.fields.push(Field { + position: Pos::default(), + description: None, + name: String::from("id"), + arguments: vec![], + field_type: Type::NonNullType(Box::new(Type::NamedType(String::from("ID")))), + directives: vec![], + }); + + // Add entity directive + obj.directives.push(Directive { + position: Pos::default(), + name: String::from("entity"), + arguments: vec![], + }); + if let Some(original_name) = original_name { + obj.directives.push(Directive { + position: Pos::default(), + name: String::from("originalName"), + arguments: vec![(String::from("name"), Value::String(original_name))], + }); + } + Definition::TypeDefinition(TypeDefinition::Object(obj)) +} diff --git a/graphql/src/schema/mod.rs b/graphql/src/schema/mod.rs index 6df51907471..8acfa42fa7b 100644 --- a/graphql/src/schema/mod.rs +++ b/graphql/src/schema/mod.rs @@ -1,3 +1,6 @@ +/// Generate a merged schema from a schema and a set of imported schemas +pub mod merge; + /// Generate full-fledged API schemas from existing GraphQL schemas. pub mod api; From c612581dadd6de65f8533d106f6809c351795e89 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Sun, 22 Dec 2019 12:57:42 -0600 Subject: [PATCH 53/64] graphql: Add placeholder type in case where schema is not found --- graphql/src/schema/merge.rs | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/graphql/src/schema/merge.rs b/graphql/src/schema/merge.rs index 40fc09a36bf..f6472542d25 100644 --- a/graphql/src/schema/merge.rs +++ b/graphql/src/schema/merge.rs @@ -47,13 +47,15 @@ pub fn merged_schema( .collect(); while let Some((import, schema_reference)) = imports.pop() { + let (original_name, new_name) = match import { + ImportedType::Name(name) => (name.clone(), name), + ImportedType::NameAs(name, az) => (name, az), + }; + match schemas.get(&schema_reference) { Some(schema) => { let subgraph_id = schema.id.clone(); - let (original_name, new_name) = match import { - ImportedType::Name(name) => (name.clone(), name), - ImportedType::NameAs(name, az) => (name, az), - }; + // Find the type let local_type = schema .document @@ -120,20 +122,27 @@ pub fn merged_schema( }; imports.push((import, schema_reference.clone())); continue; + } else { + // If it is not imported, then add a placeholder type + merged.document.definitions.push(placeholder_type( + new_name.clone(), + match new_name.eq(&original_name) { + true => None, + false => Some(original_name), + }, + )); } - - // If it is not imported, then add a placeholder type - merged.document.definitions.push(placeholder_type( - new_name.clone(), - match new_name.eq(&original_name) { - true => None, - false => Some(original_name), - }, - )); } } None => { // Add a placeholder type to the root schema + merged.document.definitions.push(placeholder_type( + new_name.clone(), + match new_name.eq(&original_name) { + true => None, + false => Some(original_name), + }, + )); } } } From e62eafde1a48e7692c7f194cd32474227e6a3367 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 23 Dec 2019 09:26:02 -0600 Subject: [PATCH 54/64] graphql: Import nonscalar fields of imported types, add tests cases --- graphql/src/schema/merge.rs | 47 +++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/graphql/src/schema/merge.rs b/graphql/src/schema/merge.rs index f6472542d25..dc1f9a2b2b6 100644 --- a/graphql/src/schema/merge.rs +++ b/graphql/src/schema/merge.rs @@ -5,11 +5,13 @@ use graphql_parser::{ use crate::schema::ast; -use graph::data::graphql::ext; +use graph::data::graphql::ext::*; +use graph::data::graphql::scalar::BuiltInScalarType; use graph::data::schema::{ImportedType, SchemaReference}; use graph::prelude::*; use std::collections::HashMap; +use std::convert::TryFrom; /// Optimistically merges a subgraph schema with all of its imports. pub fn merged_schema( @@ -74,10 +76,10 @@ pub fn merged_schema( _ => unreachable!(), }); if let Some(obj) = local_type { - // 1. Clone the type + // Clone the type let mut new_obj = obj.clone(); - // 2. Add a subgraph id directive + // Add a subgraph id directive new_obj.directives.push(Directive { position: Pos::default(), name: String::from("subgraphId"), @@ -87,17 +89,34 @@ pub fn merged_schema( )], }); - // 3. If the type is imported with { name : "...", as: "..." }, change the name and + // If the type is imported with { name : "...", as: "..." }, change the name and // add an @originalName(name: "...") directive if !original_name.eq(&new_name) { new_obj.name = new_name.clone(); } - // 4. Push it onto the schema.document.definitions + // Push it onto the schema.document.definitions merged .document .definitions .push(Definition::TypeDefinition(TypeDefinition::Object(new_obj))); + + // Import each none scalar field + obj.fields + .iter() + .filter_map(|field| { + let base_type = field.field_type.get_base_type(); + match BuiltInScalarType::try_from(base_type.as_ref()) { + Ok(_) => None, + Err(_) => Some(base_type), + } + }) + .for_each(|base_type| { + imports.push(( + ImportedType::Name(base_type.to_string()), + schema_reference.clone(), + )); + }); } else { // Determine if the type is imported // If it is imported, push a tuple onto the `imports` vector @@ -178,3 +197,21 @@ fn placeholder_type(name: String, original_name: Option) -> Definition { } Definition::TypeDefinition(TypeDefinition::Object(obj)) } + +#[test] +fn test_recursive_import() {} + +#[test] +fn test_placeholder_for_missing_schema() {} + +#[test] +fn test_placeholder_for_missing_type() {} + +#[test] +fn test_original_name_directive() {} + +#[test] +fn test_subgraph_id_directive_added_correctly() {} + +#[test] +fn test_import_of_non_scalar_fields_for_imported_type() {} From cd83cd86f17332a30e2398ccf234c04f6e4d57f2 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 23 Dec 2019 10:53:59 -0600 Subject: [PATCH 55/64] graphql: Remove unused import --- graphql/src/schema/merge.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/graphql/src/schema/merge.rs b/graphql/src/schema/merge.rs index dc1f9a2b2b6..babed15697e 100644 --- a/graphql/src/schema/merge.rs +++ b/graphql/src/schema/merge.rs @@ -3,8 +3,6 @@ use graphql_parser::{ Pos, }; -use crate::schema::ast; - use graph::data::graphql::ext::*; use graph::data::graphql::scalar::BuiltInScalarType; use graph::data::schema::{ImportedType, SchemaReference}; @@ -199,7 +197,12 @@ fn placeholder_type(name: String, original_name: Option) -> Definition { } #[test] -fn test_recursive_import() {} +fn test_recursive_import() { + // Generate a root schema + // Generate the schema lookup for the import graph + // Call merged_schema + // Verify the output schema is correct +} #[test] fn test_placeholder_for_missing_schema() {} From fb4a3020b019f5c8182b84c2d5e708416c719a04 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Mon, 30 Dec 2019 11:14:09 -0600 Subject: [PATCH 56/64] graph, graphql: Add tests for merged_schema --- graph/src/data/schema.rs | 28 +- graphql/src/schema/merge.rs | 502 ++++++++++++++++++++++++++++++++++-- 2 files changed, 489 insertions(+), 41 deletions(-) diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index ef9d68b0721..cd8bb49536c 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -353,7 +353,7 @@ impl Schema { Value::String(type_name) => Some(ImportedType::Name(type_name.to_string())), Value::Object(type_name_as) => { match (type_name_as.get("name"), type_name_as.get("as")) { - (Some(name), Some(az)) => { + (Some(Value::String(name)), Some(Value::String(az))) => { Some(ImportedType::NameAs(name.to_string(), az.to_string())) } _ => None, @@ -999,7 +999,7 @@ fn test_reserved_type_with_fields() { const ROOT_SCHEMA: &str = " type _Schema_ { id: ID! }"; - let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); + let document = graphql_parser::parse_schema(ROOT_SCHEMA).unwrap(); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); match schema.validate_schema_type_has_no_fields() { Err(e) => assert_eq!(e, SchemaValidationError::SchemaTypeWithFields), @@ -1015,7 +1015,7 @@ fn test_reserved_type_directives() { const ROOT_SCHEMA: &str = " type _Schema_ @illegal"; - let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); + let document = graphql_parser::parse_schema(ROOT_SCHEMA).unwrap(); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); match schema.validate_only_import_directives_on_schema_type() { Err(e) => assert_eq!(e, SchemaValidationError::InvalidSchemaTypeDirectives), @@ -1028,10 +1028,9 @@ type _Schema_ @illegal"; #[test] fn test_imports_directive_from_argument() { - const ROOT_SCHEMA: &str = r#" -type _Schema_ @import(types: ["T", "A", "C"])"#; + const ROOT_SCHEMA: &str = "type _Schema_ @import(types: [\"T\", \"A\", \"C\"])"; - let document = graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); + let document = graphql_parser::parse_schema(ROOT_SCHEMA).unwrap(); let schema = Schema::new(SubgraphDeploymentId::new("id").unwrap(), document); match schema .validate_import_directives() @@ -1088,18 +1087,15 @@ type T @entity { id: ID! } #[test] fn test_recursively_imported_type_which_dne_fails_validation() { const ROOT_SCHEMA: &str = r#" -type _Schema_ @import(types: ["T"], from: { name:"childone/subgraph"})"#; +type _Schema_ @import(types: ["T"], from: { name: "childone/subgraph" })"#; const CHILD_1_SCHEMA: &str = r#" -type _Schema_ @import(types: [{name: "T", as: "A"}], from: { name:"childtwo/subgraph"})"#; +type _Schema_ @import(types: [{name: "T", as: "A"}], from: { name: "childtwo/subgraph" })"#; const CHILD_2_SCHEMA: &str = r#" -type T @entity { id: ID! } -"#; - let root_document = - graphql_parser::parse_schema(ROOT_SCHEMA).expect("Failed to parse root schema"); - let child_1_document = - graphql_parser::parse_schema(CHILD_1_SCHEMA).expect("Failed to parse child 1 schema"); - let child_2_document = - graphql_parser::parse_schema(CHILD_2_SCHEMA).expect("Failed to parse child 2 schema"); +type T @entity { id: ID! }"#; + + let root_document = graphql_parser::parse_schema(ROOT_SCHEMA).unwrap(); + let child_1_document = graphql_parser::parse_schema(CHILD_1_SCHEMA).unwrap(); + let child_2_document = graphql_parser::parse_schema(CHILD_2_SCHEMA).unwrap(); let root_schema = Schema::new(SubgraphDeploymentId::new("rid").unwrap(), root_document); let child_1_schema = Schema::new(SubgraphDeploymentId::new("c1id").unwrap(), child_1_document); diff --git a/graphql/src/schema/merge.rs b/graphql/src/schema/merge.rs index babed15697e..916b7c9bdab 100644 --- a/graphql/src/schema/merge.rs +++ b/graphql/src/schema/merge.rs @@ -5,7 +5,7 @@ use graphql_parser::{ use graph::data::graphql::ext::*; use graph::data::graphql::scalar::BuiltInScalarType; -use graph::data::schema::{ImportedType, SchemaReference}; +use graph::data::schema::{ImportedType, SchemaReference, SCHEMA_TYPE_NAME}; use graph::prelude::*; use std::collections::HashMap; @@ -57,7 +57,7 @@ pub fn merged_schema( let subgraph_id = schema.id.clone(); // Find the type - let local_type = schema + if let Some(obj) = schema .document .definitions .iter() @@ -72,8 +72,8 @@ pub fn merged_schema( .map(|definition| match definition { Definition::TypeDefinition(TypeDefinition::Object(obj)) => obj, _ => unreachable!(), - }); - if let Some(obj) = local_type { + }) + { // Clone the type let mut new_obj = obj.clone(); @@ -88,9 +88,17 @@ pub fn merged_schema( }); // If the type is imported with { name : "...", as: "..." }, change the name and - // add an @originalName(name: "...") directive + // add an @originalName(name: "...") directive if !original_name.eq(&new_name) { new_obj.name = new_name.clone(); + new_obj.directives.push(Directive { + position: Pos::default(), + name: String::from("originalName"), + arguments: vec![( + String::from("name"), + Value::String(original_name.to_string()), + )], + }); } // Push it onto the schema.document.definitions @@ -123,9 +131,8 @@ pub fn merged_schema( .imported_types() .iter() .find(|(import, schema_reference)| match import { - ImportedType::Name(name) if name.eq(&original_name) => true, - ImportedType::NameAs(_, az) if az.eq(&original_name) => true, - _ => false, + ImportedType::Name(name) => name.eq(&original_name), + ImportedType::NameAs(_, az) => az.eq(&original_name), }) { let import = match import { @@ -164,6 +171,23 @@ pub fn merged_schema( } } + // Remove the _Schema_ type + if let Some((idx, _)) = + merged + .document + .definitions + .iter() + .enumerate() + .find(|(_, definition)| match definition { + Definition::TypeDefinition(TypeDefinition::Object(obj)) => { + obj.name.eq(SCHEMA_TYPE_NAME) + } + _ => false, + }) + { + merged.document.definitions.remove(idx); + }; + merged } @@ -186,6 +210,11 @@ fn placeholder_type(name: String, original_name: Option) -> Definition { name: String::from("entity"), arguments: vec![], }); + obj.directives.push(Directive { + position: Pos::default(), + name: String::from("placeholder"), + arguments: vec![], + }); if let Some(original_name) = original_name { obj.directives.push(Directive { position: Pos::default(), @@ -196,25 +225,448 @@ fn placeholder_type(name: String, original_name: Option) -> Definition { Definition::TypeDefinition(TypeDefinition::Object(obj)) } -#[test] -fn test_recursive_import() { - // Generate a root schema - // Generate the schema lookup for the import graph - // Call merged_schema - // Verify the output schema is correct -} +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use super::merged_schema; + + use graph::data::graphql::ext::*; + use graph::data::schema::{ImportedType, SchemaReference, SCHEMA_TYPE_NAME}; + use graph::prelude::*; + + use graphql_parser::schema::Value; + + fn schema_with_import( + subgraph_id: SubgraphDeploymentId, + type_name: String, + subgraph_name: String, + ) -> Schema { + let schema = format!( + r#"type _Schema_ @import(types: ["{}"], from: {{ name: "{}" }})"#, + type_name, subgraph_name, + ); + let document = graphql_parser::parse_schema(&schema).unwrap(); + Schema::new(subgraph_id, document) + } + + fn schema_with_name_as_import( + subgraph_id: SubgraphDeploymentId, + type_name: String, + type_as: String, + subgraph_name: String, + ) -> Schema { + let schema = format!( + r#"type _Schema_ @import(types: [{{ name: "{}", as: "{}" }}] from: {{ name: "{}" }})"#, + type_name, type_as, subgraph_name, + ); + let document = graphql_parser::parse_schema(&schema).unwrap(); + Schema::new(subgraph_id, document) + } + + fn schema_with_type(subgraph_id: SubgraphDeploymentId, type_name: String) -> Schema { + let schema = format!( + r#" +type {} @entity {{ + id: ID! + foo: String +}} +"#, + type_name, + ); + let document = graphql_parser::parse_schema(&schema).unwrap(); + Schema::new(subgraph_id, document) + } + + fn schema_with_type_with_nonscalar_field( + subgraph_id: SubgraphDeploymentId, + type_name: String, + field_type_name: String, + ) -> Schema { + let schema = format!( + r#" +type {} @entity {{ + id: ID! + foo: {} +}} + +type {} @entity {{ + id: ID! + bar: String +}} +"#, + type_name, field_type_name, field_type_name, + ); + let document = graphql_parser::parse_schema(&schema).unwrap(); + Schema::new(subgraph_id, document) + } + + #[test] + fn test_recursive_import() { + let root_schema = schema_with_import( + SubgraphDeploymentId::new("root").unwrap(), + String::from("A"), + String::from("c1/subgraph"), + ); + let child_1_schema = schema_with_name_as_import( + SubgraphDeploymentId::new("childone").unwrap(), + String::from("T"), + String::from("A"), + String::from("c2/subgraph"), + ); + let child_2_schema = schema_with_type( + SubgraphDeploymentId::new("childtwo").unwrap(), + String::from("T"), + ); + + let mut schemas = HashMap::new(); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("c1/subgraph").unwrap()), + Arc::new(child_1_schema), + ); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("c2/subgraph").unwrap()), + Arc::new(child_2_schema), + ); + + // Call merged_schema + let merged = merged_schema(&root_schema, schemas); + + // Verify the output schema is correctl + match merged + .document + .get_object_type_definitions() + .into_iter() + .find(|object_type| object_type.name.eq("A")) + { + None => panic!("Failed to import type `A`"), + Some(type_a) => { + // Type A is imported with all the correct fields + let id_field = type_a.fields.iter().find(|field| field.name.eq("id")); + let foo_field = type_a.fields.iter().find(|field| field.name.eq("foo")); + + match (id_field, foo_field) { + (Some(_), Some(_)) => (), + _ => panic!("Imported type `A` does not have the correct fields"), + }; + + // Type A has a @subgraphId directive with the correct id + match type_a + .directives + .iter() + .find(|directive| directive.name.eq("subgraphId")) + { + Some(directive) => { + // Ensure the id argument on the directive is correct + match directive.arguments.iter().find(|(name, _)| name.eq("id")) { + Some((_, Value::String(id))) if id.eq("childtwo") => (), + _ => { + panic!( + "Imported type `A` needs a @subgraphId directive: @subgraphId(id: \"childtwo\")" + ); + } + } + } + None => panic!("Imported type `A` does not have a `@subgraphId` directive"), + }; + + // Type A has an @originalName directive with the correct name + match type_a + .directives + .iter() + .find(|directive| directive.name.eq("originalName")) + { + Some(directive) => { + // Ensure the original name argument on the directive is correct + match directive.arguments.iter().find(|(name, _)| name.eq("name")) { + Some((_, Value::String(name))) if name.eq("T") => (), + _ => { + panic!( + "Imported type `A` needs an originalName directive: @originalName(name: \"T\")" + ); + } + }; + } + None => panic!("Imported type `A` does not have an `originalName` directive"), + } + } + } + } + + #[test] + fn test_placeholder_for_missing_type() { + let root_schema = schema_with_import( + SubgraphDeploymentId::new("root").unwrap(), + String::from("A"), + String::from("c1/subgraph"), + ); + let child_1_schema = schema_with_name_as_import( + SubgraphDeploymentId::new("childone").unwrap(), + String::from("T"), + String::from("A"), + String::from("c2/subgraph"), + ); + let child_2_schema = schema_with_type( + SubgraphDeploymentId::new("childtwo").unwrap(), + String::from("B"), + ); + + let mut schemas = HashMap::new(); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("c1/subgraph").unwrap()), + Arc::new(child_1_schema), + ); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("c2/subgraph").unwrap()), + Arc::new(child_2_schema), + ); + + // Call merged_schema + let merged = merged_schema(&root_schema, schemas); + + match merged.document.get_object_type_definitions().iter().next() { + None => panic!("Failed to import placeholder for type `A`"), + Some(type_a) => { + // Has an id field + match type_a.fields.iter().find(|field| field.name.eq("id")) { + Some(_) => (), + _ => panic!("Placeholder for imported type does not have the correct fields"), + }; + + // Has a placeholder directive + match type_a + .directives + .iter() + .find(|directive| directive.name.eq("placeholder")) + { + Some(_) => (), + _ => { + panic!("Imported type `A` does not have a `@placeholder` directive"); + } + } + } + }; + } + + #[test] + fn test_placeholder_for_missing_schema() { + let root_schema = schema_with_import( + SubgraphDeploymentId::new("root").unwrap(), + String::from("A"), + String::from("c1/subgraph"), + ); + let child_1_schema = schema_with_name_as_import( + SubgraphDeploymentId::new("childone").unwrap(), + String::from("T"), + String::from("A"), + String::from("c2/subgraph"), + ); + + let mut schemas = HashMap::new(); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("c1/subgraph").unwrap()), + Arc::new(child_1_schema), + ); + // Call merged_schema + let merged = merged_schema(&root_schema, schemas); + + match merged.document.get_object_type_definitions().iter().next() { + None => panic!("Failed to import placeholder for type `A`"), + Some(type_a) => { + // Has an id field + match type_a.fields.iter().find(|field| field.name.eq("id")) { + Some(_) => (), + _ => panic!("Placeholder for imported type does not have the correct fields"), + }; + + // Has a placeholder directive + match type_a + .directives + .iter() + .find(|directive| directive.name.eq("placeholder")) + { + Some(_) => (), + _ => { + panic!("Imported type `A` does not have a `@placeholder` directive"); + } + } + } + }; + } + + #[test] + fn test_import_of_non_scalar_fields_for_imported_type() { + let root_schema = schema_with_import( + SubgraphDeploymentId::new("root").unwrap(), + String::from("A"), + String::from("c1/subgraph"), + ); + let child_1_schema = schema_with_name_as_import( + SubgraphDeploymentId::new("childone").unwrap(), + String::from("T"), + String::from("A"), + String::from("c2/subgraph"), + ); + let child_2_schema = schema_with_type_with_nonscalar_field( + SubgraphDeploymentId::new("childtwo").unwrap(), + String::from("T"), + String::from("B"), + ); + + let mut schemas = HashMap::new(); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("c1/subgraph").unwrap()), + Arc::new(child_1_schema), + ); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("c2/subgraph").unwrap()), + Arc::new(child_2_schema), + ); + + // Call merged_schema + let merged = merged_schema(&root_schema, schemas); + + // Verify the output schema is correct + match merged + .document + .get_object_type_definitions() + .into_iter() + .find(|object_type| object_type.name.eq("A")) + { + None => panic!("Failed to import type `A`"), + Some(type_a) => { + // Type A is imported with all the correct fields + let id_field = type_a.fields.iter().find(|field| field.name.eq("id")); + let foo_field = type_a.fields.iter().find(|field| field.name.eq("foo")); + + match (id_field, foo_field) { + (Some(_), Some(_)) => (), + _ => panic!("Imported type `A` does not have the correct fields"), + }; + + // Type A has a @subgraphId directive with the correct id + match type_a + .directives + .iter() + .find(|directive| directive.name.eq("subgraphId")) + { + Some(directive) => { + // Ensure the id argument on the directive is correct + match directive.arguments.iter().find(|(name, _)| name.eq("id")) { + Some((_, Value::String(id))) if id.eq("childtwo") => (), + _ => { + panic!( + "Imported type `A` needs a @subgraphId directive: @subgraphId(id: \"c2id\")" + ); + } + } + } + None => panic!("Imported type `A` does not have a `@subgraphId` directive"), + }; -#[test] -fn test_placeholder_for_missing_schema() {} + // Type A has an @originalName directive with the correct name + match type_a + .directives + .iter() + .find(|directive| directive.name.eq("originalName")) + { + Some(directive) => { + // Ensure the original name argument on the directive is correct + match directive.arguments.iter().find(|(name, _)| name.eq("name")) { + Some((_, Value::String(name))) if name.eq("T") => (), + _ => { + panic!( + "Imported type `A` needs an originalName directive: @originalName(name: \"T\")" + ); + } + }; + } + None => panic!("Imported type `A` does not have an `originalName` directive"), + } + } + } + // Verify the output schema is correct + match merged + .document + .get_object_type_definitions() + .into_iter() + .find(|object_type| object_type.name.eq("B")) + { + None => panic!("Failed to import type `B`"), + Some(type_b) => { + // Type A is imported with all the correct fields + let id_field = type_b.fields.iter().find(|field| field.name.eq("id")); + let foo_field = type_b.fields.iter().find(|field| field.name.eq("bar")); -#[test] -fn test_placeholder_for_missing_type() {} + match (id_field, foo_field) { + (Some(_), Some(_)) => (), + _ => panic!("Imported type `B` does not have the correct fields"), + }; -#[test] -fn test_original_name_directive() {} + // Type A has a @subgraphId directive with the correct id + match type_b + .directives + .iter() + .find(|directive| directive.name.eq("subgraphId")) + { + Some(directive) => { + // Ensure the id argument on the directive is correct + match directive.arguments.iter().find(|(name, _)| name.eq("id")) { + Some((_, Value::String(id))) if id.eq("childtwo") => (), + _ => { + panic!( + "Imported type `B` needs a @subgraphId directive: @subgraphId(id: \"c2id\")" + ); + } + } + } + None => panic!("Imported type `B` does not have a `@subgraphId` directive"), + }; + } + } + } + + #[test] + fn test_schema_type_definition_removed() { + let root_schema = schema_with_import( + SubgraphDeploymentId::new("root").unwrap(), + String::from("A"), + String::from("c1/subgraph"), + ); + let child_1_schema = schema_with_name_as_import( + SubgraphDeploymentId::new("childone").unwrap(), + String::from("T"), + String::from("A"), + String::from("c2/subgraph"), + ); + let child_2_schema = schema_with_type( + SubgraphDeploymentId::new("childtwo").unwrap(), + String::from("T"), + ); -#[test] -fn test_subgraph_id_directive_added_correctly() {} + let mut schemas = HashMap::new(); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("childone/subgraph").unwrap()), + Arc::new(child_1_schema), + ); + schemas.insert( + SchemaReference::ByName(SubgraphName::new("childtwo/subgraph").unwrap()), + Arc::new(child_2_schema), + ); -#[test] -fn test_import_of_non_scalar_fields_for_imported_type() {} + // Call merged_schema + let merged = merged_schema(&root_schema, schemas); + + match merged + .document + .get_object_type_definitions() + .into_iter() + .find(|object_type| object_type.name.eq(SCHEMA_TYPE_NAME)) + { + None => (), + Some(_) => { + panic!("_Schema_ type should be removed from the merged schema"); + } + }; + } +} From 5950601b2e722d3b8e4f2f18c2cf5d5414731970 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 3 Jan 2020 09:55:30 -0600 Subject: [PATCH 57/64] graphql: Define @originalName directive, update comments --- graphql/src/schema/api.rs | 16 ++++++++++++++++ graphql/src/schema/merge.rs | 16 +++++----------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/graphql/src/schema/api.rs b/graphql/src/schema/api.rs index 89a08bd926a..1895327c1fd 100644 --- a/graphql/src/schema/api.rs +++ b/graphql/src/schema/api.rs @@ -109,9 +109,25 @@ fn add_directives(schema: &mut Document) { locations: vec![DirectiveLocation::Object], }); + let original_name = Definition::DirectiveDefinition(DirectiveDefinition { + position: Pos::default(), + description: None, + name: "originalName".to_owned(), + arguments: vec![InputValue { + position: Pos::default(), + description: None, + name: "name".to_owned(), + value_type: Type::NamedType("String".to_owned()), + default_value: None, + directives: vec![], + }], + locations: vec![DirectiveLocation::Object], + }); + schema.definitions.push(entity); schema.definitions.push(derived_from); schema.definitions.push(subgraph_id); + schema.definitions.push(original_name); } /// Adds a global `OrderDirection` type to the schema. diff --git a/graphql/src/schema/merge.rs b/graphql/src/schema/merge.rs index 916b7c9bdab..b406f7238c8 100644 --- a/graphql/src/schema/merge.rs +++ b/graphql/src/schema/merge.rs @@ -23,27 +23,21 @@ pub fn merged_schema( // // If the schema is not available, then add a placeholder type to the root schema. // - // If the schema is available, copy the schema over. + // If the schema is available, copy the type over. // Check each field in the copied type and for non scalar fields, produce an (ImportedType, SchemaRefernce) - // tuple; the new vector element will either be for the same schema or for an imported schema. + // tuple. // // Copying a type: // 1. Clone the type // 2. Add a subgraph id directive // 3. If the type is imported with { name : "...", as: "..." }, change the name and // add an @originalName(name: "...") directive - // 4. Push it onto the schema.document.definitions - // - // QUESTION: How should naming conflicts be handled? - // A subgraph developer will probably ensure that an imported type does not conflict with local subgraph types. - // However, the non scalar fields of an imported type are also imported and those types might overlap with local - // subgraph types. What should we do in this case? - // Presumably overlapping type names will not be accepted by GraphQL clients. + let mut merged = root_schema.clone(); let mut imports: Vec<(_, _)> = merged .imported_types() .iter() - .map(|(t, sr)| (t.clone(), sr.clone())) + .map(|(import, schema_reference)| (import.clone(), schema_reference.clone())) .collect(); while let Some((import, schema_reference)) = imports.pop() { @@ -130,7 +124,7 @@ pub fn merged_schema( schema .imported_types() .iter() - .find(|(import, schema_reference)| match import { + .find(|(import, _)| match import { ImportedType::Name(name) => name.eq(&original_name), ImportedType::NameAs(_, az) => az.eq(&original_name), }) From d6f08a0dd049d9e6f20a9d383293f7e59bf03865 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 3 Jan 2020 14:48:54 -0600 Subject: [PATCH 58/64] graph, graphql, mock, store: Integrate merged_schema with Store --- graph/src/components/store.rs | 11 +++++ graph/src/data/schema.rs | 89 +++++++++------------------------- graph/src/data/subgraph/mod.rs | 6 +-- graphql/src/lib.rs | 4 +- mock/src/store.rs | 13 ++++- store/postgres/src/store.rs | 56 +++++++++++++++++++-- 6 files changed, 103 insertions(+), 76 deletions(-) diff --git a/graph/src/components/store.rs b/graph/src/components/store.rs index 8b8baa1e708..8d624c69db8 100644 --- a/graph/src/components/store.rs +++ b/graph/src/components/store.rs @@ -14,6 +14,7 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use web3::types::H256; +use crate::data::schema::SchemaReference; use crate::data::store::*; use crate::data::subgraph::schema::*; use crate::prelude::*; @@ -1187,6 +1188,16 @@ pub trait SubgraphDeploymentStore: Send + Sync + 'static { /// store internals that should really be hidden and should be used /// sparingly and only when absolutely needed fn uses_relational_schema(&self, subgraph_id: &SubgraphDeploymentId) -> Result; + + fn resolve_schema_reference( + &self, + schema_reference: &SchemaReference, + ) -> Result, Error>; + + fn resolve_import_graph( + &self, + schema: &Schema, + ) -> (HashMap>, Vec); } /// Common trait for blockchain store implementations. diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index cd8bb49536c..33d39393c1e 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -1,4 +1,3 @@ -use crate::components::store::{Store, SubgraphDeploymentStore}; use crate::data::graphql::ext::{DirectiveFinder, DocumentExt, TypeExt}; use crate::data::graphql::scalar::BuiltInScalarType; use crate::data::subgraph::{SubgraphDeploymentId, SubgraphName}; @@ -13,7 +12,7 @@ use graphql_parser::{ }; use serde::{Deserialize, Serialize}; -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap}; use std::convert::TryFrom; use std::fmt; use std::hash::{Hash, Hasher}; @@ -135,27 +134,27 @@ impl fmt::Display for SchemaReference { } } -impl SchemaReference { - pub fn resolve( - &self, - store: Arc, - ) -> Result<(Arc, SubgraphDeploymentId), SchemaImportError> { - let subgraph_id = match self { - SchemaReference::ByName(name) => store - .resolve_subgraph_name_to_id(name.clone()) - .map_err(|_| SchemaImportError::ImportedSubgraphNotFound(self.clone())) - .and_then(|subgraph_id_opt| { - subgraph_id_opt.ok_or(SchemaImportError::ImportedSubgraphNotFound(self.clone())) - })?, - SchemaReference::ById(id) => id.clone(), - }; - - store - .input_schema(&subgraph_id) - .map_err(|_| SchemaImportError::ImportedSchemaNotFound(self.clone())) - .map(|schema| (schema, subgraph_id)) - } -} +// impl SchemaReference { +// pub fn resolve( +// &self, +// store: Arc, +// ) -> Result<(Arc, SubgraphDeploymentId), SchemaImportError> { +// let subgraph_id = match self { +// SchemaReference::ByName(name) => store +// .resolve_subgraph_name_to_id(name.clone()) +// .map_err(|_| SchemaImportError::ImportedSubgraphNotFound(self.clone())) +// .and_then(|subgraph_id_opt| { +// subgraph_id_opt.ok_or(SchemaImportError::ImportedSubgraphNotFound(self.clone())) +// })?, +// SchemaReference::ById(id) => id.clone(), +// }; + +// store +// .input_schema(&subgraph_id) +// .map_err(|_| SchemaImportError::ImportedSchemaNotFound(self.clone())) +// .map(|schema| (schema, subgraph_id)) +// } +// } /// A validated and preprocessed GraphQL schema for a subgraph. #[derive(Clone, Debug, PartialEq)] @@ -183,50 +182,6 @@ impl Schema { } } - pub fn resolve_schema_references( - &self, - store: Arc, - ) -> ( - HashMap>, - Vec, - ) { - let mut schemas = HashMap::new(); - let mut visit_log = HashSet::new(); - let import_errors = self.resolve_import_graph(store, &mut schemas, &mut visit_log); - (schemas, import_errors) - } - - fn resolve_import_graph( - &self, - store: Arc, - schemas: &mut HashMap>, - visit_log: &mut HashSet, - ) -> Vec { - // Use the visit log to detect cycles in the import graph - self.imported_schemas() - .into_iter() - .fold(vec![], |mut errors, schema_ref| { - match schema_ref.clone().resolve(store.clone()) { - Ok((schema, subgraph_id)) => { - schemas.insert(schema_ref, schema.clone()); - // If this node in the graph has already been visited stop traversing - if !visit_log.contains(&subgraph_id) { - visit_log.insert(subgraph_id); - errors.extend(schema.resolve_import_graph( - store.clone(), - schemas, - visit_log, - )); - } - } - Err(err) => { - errors.push(err); - } - } - errors - }) - } - pub fn collect_interfaces( document: &schema::Document, ) -> Result< diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index e0868c8cbb6..8600770d0e6 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -323,7 +323,7 @@ pub enum SubgraphAssignmentProviderEvent { #[derive(Fail, Debug)] pub enum SubgraphManifestValidationWarning { #[fail(display = "schema validation produced warnings: {:?}", _0)] - SchemaValidationWarning(SchemaImportError), + SchemaImportError(Error), } #[derive(Fail, Debug)] @@ -852,10 +852,10 @@ impl UnvalidatedSubgraphManifest { (SubgraphManifest, Vec), Vec, > { - let (schemas, import_errors) = self.0.schema.resolve_schema_references(store); + let (schemas, import_errors) = store.resolve_import_graph(&self.0.schema); let validation_warnings = import_errors .into_iter() - .map(|err| SubgraphManifestValidationWarning::SchemaValidationWarning(err)) + .map(|err| SubgraphManifestValidationWarning::SchemaImportError(err)) .collect(); let mut errors: Vec = vec![]; diff --git a/graphql/src/lib.rs b/graphql/src/lib.rs index 4f0d4588d27..6e37212046a 100644 --- a/graphql/src/lib.rs +++ b/graphql/src/lib.rs @@ -30,7 +30,9 @@ pub mod prelude { pub use super::query::{ execute_query, ext::BlockConstraint, ext::BlockLocator, QueryExecutionOptions, }; - pub use super::schema::{api_schema, ast::validate_entity, APISchemaError}; + pub use super::schema::{ + api_schema, ast::validate_entity, merge::merged_schema, APISchemaError, + }; pub use super::store::{build_query, StoreResolver}; pub use super::subscription::{execute_subscription, SubscriptionExecutionOptions}; pub use super::values::{object_value, MaybeCoercible}; diff --git a/mock/src/store.rs b/mock/src/store.rs index 99a1ad7825e..49f2d456494 100644 --- a/mock/src/store.rs +++ b/mock/src/store.rs @@ -1,8 +1,9 @@ use mockall::predicate::*; use mockall::*; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use graph::components::store::*; +use graph::data::schema::SchemaReference; use graph::data::subgraph::schema::*; use graph::prelude::*; use graph_graphql::prelude::api_schema; @@ -91,6 +92,16 @@ mock! { fn api_schema(&self, subgraph_id: &SubgraphDeploymentId) -> Result, Error>; fn uses_relational_schema(&self, subgraph_id: &SubgraphDeploymentId) -> Result; + + fn resolve_schema_reference( + &self, + _schema_reference: &SchemaReference, + ) -> Result, Error>; + + fn resolve_import_graph( + &self, + _schema: &Schema, + ) -> (HashMap>, Vec); } trait ChainStore: Send + Sync + 'static { diff --git a/store/postgres/src/store.rs b/store/postgres/src/store.rs index dfb4388d261..90c900c57ad 100644 --- a/store/postgres/src/store.rs +++ b/store/postgres/src/store.rs @@ -5,7 +5,7 @@ use diesel::r2d2::{ConnectionManager, Pool, PooledConnection}; use diesel::{insert_into, select, update}; use futures::sync::mpsc::{channel, Sender}; use lru_time_cache::LruCache; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::convert::{TryFrom, TryInto}; use std::iter::FromIterator; use std::sync::{Arc, Mutex, RwLock}; @@ -13,6 +13,7 @@ use std::time::{Duration, Instant}; use uuid::Uuid; use graph::components::store::Store as StoreTrait; +use graph::data::schema::SchemaReference; use graph::data::subgraph::schema::{ SubgraphDeploymentEntity, SubgraphManifestEntity, TypedEntity as _, SUBGRAPHS_ID, }; @@ -28,7 +29,7 @@ use graph::prelude::{ SubgraphEntityPair, TransactionAbortError, Value, BLOCK_NUMBER_MAX, }; use graph_chain_ethereum::BlockIngestorMetrics; -use graph_graphql::prelude::api_schema; +use graph_graphql::prelude::{api_schema, merged_schema}; use tokio::timer::Interval; use web3::types::H256; @@ -727,12 +728,13 @@ impl Store { } }; - // Parse the schema and add @subgraphId directives + // Parse the schema let input_schema = Schema::parse(&input_schema, subgraph_id.clone())?; - let mut schema = input_schema.clone(); // Generate an API schema for the subgraph and make sure all types in the // API schema have a @subgraphId directive as well + let (imported_schemas, _) = self.resolve_import_graph(&input_schema); + let mut schema = merged_schema(&input_schema, imported_schemas); schema.document = api_schema(&schema.document)?; schema.add_subgraph_id_directives(subgraph_id.clone()); @@ -1109,6 +1111,52 @@ impl SubgraphDeploymentStore for Store { self.get_entity_conn(subgraph) .map(|econn| econn.uses_relational_schema()) } + + fn resolve_schema_reference( + &self, + schema_reference: &SchemaReference, + ) -> Result, Error> { + let subgraph_id = match schema_reference { + SchemaReference::ByName(name) => self + .resolve_subgraph_name_to_id(name.clone()) + .and_then(|subgraph_id_opt| { + subgraph_id_opt + .ok_or(format_err!("Subgraph name `{}` not found ", name.clone())) + })?, + SchemaReference::ById(id) => id.clone(), + }; + self.input_schema(&subgraph_id) + } + + fn resolve_import_graph( + &self, + schema: &Schema, + ) -> (HashMap>, Vec) { + let mut imports = schema.imported_schemas(); + let mut visited = HashSet::new(); + let mut schemas = HashMap::new(); + let mut errors = vec![]; + + while let Some(schema_ref) = imports.pop() { + match self.resolve_schema_reference(&schema_ref) { + Ok(schema) => { + schemas.insert(schema_ref, schema.clone()); + if !visited.contains(&schema.id) { + visited.insert(schema.id.clone()); + schema + .imported_schemas() + .into_iter() + .for_each(|schema_reference| imports.push(schema_reference)); + } + } + Err(e) => { + errors.push(e); + } + } + } + + (schemas, errors) + } } impl ChainStore for Store { From 47efb3a2d4d0ad4c234eba5542fe4c77f311a762 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 3 Jan 2020 15:19:45 -0600 Subject: [PATCH 59/64] graphql: Remove extra import --- graphql/src/schema/merge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphql/src/schema/merge.rs b/graphql/src/schema/merge.rs index b406f7238c8..327cb089ce4 100644 --- a/graphql/src/schema/merge.rs +++ b/graphql/src/schema/merge.rs @@ -226,7 +226,7 @@ mod tests { use super::merged_schema; use graph::data::graphql::ext::*; - use graph::data::schema::{ImportedType, SchemaReference, SCHEMA_TYPE_NAME}; + use graph::data::schema::{SchemaReference, SCHEMA_TYPE_NAME}; use graph::prelude::*; use graphql_parser::schema::Value; From 88e0f4c259ade67710ae9f3f04156c4464159d5b Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 3 Jan 2020 17:21:33 -0600 Subject: [PATCH 60/64] graph, mock, store: Use SchemaImportError --- graph/src/components/store.rs | 9 ++++++--- graph/src/data/schema.rs | 22 ---------------------- graph/src/data/subgraph/mod.rs | 2 +- mock/src/store.rs | 2 +- store/postgres/src/store.rs | 16 +++++++++++----- 5 files changed, 19 insertions(+), 32 deletions(-) diff --git a/graph/src/components/store.rs b/graph/src/components/store.rs index 8d624c69db8..66734b02675 100644 --- a/graph/src/components/store.rs +++ b/graph/src/components/store.rs @@ -14,7 +14,7 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use web3::types::H256; -use crate::data::schema::SchemaReference; +use crate::data::schema::{SchemaImportError, SchemaReference}; use crate::data::store::*; use crate::data::subgraph::schema::*; use crate::prelude::*; @@ -1192,12 +1192,15 @@ pub trait SubgraphDeploymentStore: Send + Sync + 'static { fn resolve_schema_reference( &self, schema_reference: &SchemaReference, - ) -> Result, Error>; + ) -> Result, SchemaImportError>; fn resolve_import_graph( &self, schema: &Schema, - ) -> (HashMap>, Vec); + ) -> ( + HashMap>, + Vec, + ); } /// Common trait for blockchain store implementations. diff --git a/graph/src/data/schema.rs b/graph/src/data/schema.rs index 33d39393c1e..de159341034 100644 --- a/graph/src/data/schema.rs +++ b/graph/src/data/schema.rs @@ -134,28 +134,6 @@ impl fmt::Display for SchemaReference { } } -// impl SchemaReference { -// pub fn resolve( -// &self, -// store: Arc, -// ) -> Result<(Arc, SubgraphDeploymentId), SchemaImportError> { -// let subgraph_id = match self { -// SchemaReference::ByName(name) => store -// .resolve_subgraph_name_to_id(name.clone()) -// .map_err(|_| SchemaImportError::ImportedSubgraphNotFound(self.clone())) -// .and_then(|subgraph_id_opt| { -// subgraph_id_opt.ok_or(SchemaImportError::ImportedSubgraphNotFound(self.clone())) -// })?, -// SchemaReference::ById(id) => id.clone(), -// }; - -// store -// .input_schema(&subgraph_id) -// .map_err(|_| SchemaImportError::ImportedSchemaNotFound(self.clone())) -// .map(|schema| (schema, subgraph_id)) -// } -// } - /// A validated and preprocessed GraphQL schema for a subgraph. #[derive(Clone, Debug, PartialEq)] pub struct Schema { diff --git a/graph/src/data/subgraph/mod.rs b/graph/src/data/subgraph/mod.rs index 8600770d0e6..b35265100fa 100644 --- a/graph/src/data/subgraph/mod.rs +++ b/graph/src/data/subgraph/mod.rs @@ -323,7 +323,7 @@ pub enum SubgraphAssignmentProviderEvent { #[derive(Fail, Debug)] pub enum SubgraphManifestValidationWarning { #[fail(display = "schema validation produced warnings: {:?}", _0)] - SchemaImportError(Error), + SchemaImportError(SchemaImportError), } #[derive(Fail, Debug)] diff --git a/mock/src/store.rs b/mock/src/store.rs index 49f2d456494..f2c1d9034af 100644 --- a/mock/src/store.rs +++ b/mock/src/store.rs @@ -3,7 +3,7 @@ use mockall::*; use std::collections::{BTreeMap, HashMap}; use graph::components::store::*; -use graph::data::schema::SchemaReference; +use graph::data::schema::{SchemaImportError, SchemaReference}; use graph::data::subgraph::schema::*; use graph::prelude::*; use graph_graphql::prelude::api_schema; diff --git a/store/postgres/src/store.rs b/store/postgres/src/store.rs index 90c900c57ad..0d87b1d45ec 100644 --- a/store/postgres/src/store.rs +++ b/store/postgres/src/store.rs @@ -13,7 +13,7 @@ use std::time::{Duration, Instant}; use uuid::Uuid; use graph::components::store::Store as StoreTrait; -use graph::data::schema::SchemaReference; +use graph::data::schema::{SchemaImportError, SchemaReference}; use graph::data::subgraph::schema::{ SubgraphDeploymentEntity, SubgraphManifestEntity, TypedEntity as _, SUBGRAPHS_ID, }; @@ -1115,23 +1115,29 @@ impl SubgraphDeploymentStore for Store { fn resolve_schema_reference( &self, schema_reference: &SchemaReference, - ) -> Result, Error> { + ) -> Result, SchemaImportError> { let subgraph_id = match schema_reference { SchemaReference::ByName(name) => self .resolve_subgraph_name_to_id(name.clone()) + .map_err(|_| SchemaImportError::ImportedSubgraphNotFound(schema_reference.clone())) .and_then(|subgraph_id_opt| { - subgraph_id_opt - .ok_or(format_err!("Subgraph name `{}` not found ", name.clone())) + subgraph_id_opt.ok_or(SchemaImportError::ImportedSubgraphNotFound( + schema_reference.clone(), + )) })?, SchemaReference::ById(id) => id.clone(), }; self.input_schema(&subgraph_id) + .map_err(|_| SchemaImportError::ImportedSchemaNotFound(schema_reference.clone())) } fn resolve_import_graph( &self, schema: &Schema, - ) -> (HashMap>, Vec) { + ) -> ( + HashMap>, + Vec, + ) { let mut imports = schema.imported_schemas(); let mut visited = HashSet::new(); let mut schemas = HashMap::new(); From d24ab6ad544e4804879184f4530d9077c3df3986 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Sun, 5 Jan 2020 13:40:55 -0600 Subject: [PATCH 61/64] store: Implement cache invalidation --- store/postgres/src/store.rs | 60 ++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 7 deletions(-) diff --git a/store/postgres/src/store.rs b/store/postgres/src/store.rs index 0d87b1d45ec..d655b640d88 100644 --- a/store/postgres/src/store.rs +++ b/store/postgres/src/store.rs @@ -118,11 +118,15 @@ pub struct StoreConfig { } #[derive(Clone)] -struct SchemaPair { +struct SchemaCacheEntry { /// The schema as supplied by the user input: Arc, /// The schema we derive from `input` with `graphql::schema::api::api_schema` api: Arc, + /// The imported schemas which are referenced by name or were not available to merge + unstable_imports: Vec<(SchemaReference, Option)>, + /// Timestamp for the last merge + last_refresh: Instant, } /// A Store based on Diesel and Postgres. @@ -136,7 +140,7 @@ pub struct Store { network_name: String, genesis_block_ptr: EthereumBlockPointer, conn: Pool>, - schema_cache: Mutex>, + schema_cache: Mutex>, /// A cache for the storage metadata for subgraphs. The Store just /// hosts this because it lives long enough, but it is managed from @@ -699,9 +703,26 @@ impl Store { Ok(storage.clone()) } - fn cached_schema(&self, subgraph_id: &SubgraphDeploymentId) -> Result { - if let Some(pair) = self.schema_cache.lock().unwrap().get(&subgraph_id) { - return Ok(pair.clone()); + fn cached_schema(&self, subgraph_id: &SubgraphDeploymentId) -> Result { + if let Some(entry) = self.schema_cache.lock().unwrap().get(&subgraph_id) { + // Cache entry is stale and an unstable import has changed or is now available + // TODO: Is it okay to ignore entries unstable imports which are no longer available? + // TODO: Make this threshold an envvar? + let requires_refresh = entry.last_refresh.elapsed().as_secs() >= 120 + && entry + .unstable_imports + .iter() + .any(|(schema_reference, subgraph_id_opt)| { + self.resolve_schema_reference(schema_reference) + .map(|schema| match subgraph_id_opt { + Some(subgraph_id) => schema.id.eq(subgraph_id), + None => true, + }) + .unwrap_or(false) + }); + if !requires_refresh { + return Ok(entry.clone()); + } } trace!(self.logger, "schema cache miss"; "id" => subgraph_id.to_string()); @@ -733,14 +754,39 @@ impl Store { // Generate an API schema for the subgraph and make sure all types in the // API schema have a @subgraphId directive as well - let (imported_schemas, _) = self.resolve_import_graph(&input_schema); + let (imported_schemas, schema_import_errors) = self.resolve_import_graph(&input_schema); + let mut unstable_schemas = imported_schemas + .iter() + .filter_map(|(schema_reference, schema)| match schema_reference { + SchemaReference::ByName(_) => { + Some((schema_reference.clone(), Some(schema.id.clone()))) + } + _ => None, + }) + .collect::)>>(); + let mut missing_schemas = schema_import_errors + .iter() + .map(|e| match e { + SchemaImportError::ImportedSchemaNotFound(schema_reference) => { + (schema_reference.clone(), None) + } + SchemaImportError::ImportedSubgraphNotFound(schema_reference) => { + (schema_reference.clone(), None) + } + }) + .collect::>(); + unstable_schemas.append(&mut missing_schemas); + + // Merge the schemas and create the api schema let mut schema = merged_schema(&input_schema, imported_schemas); schema.document = api_schema(&schema.document)?; schema.add_subgraph_id_directives(subgraph_id.clone()); - let pair = SchemaPair { + let pair = SchemaCacheEntry { input: Arc::new(input_schema), api: Arc::new(schema), + unstable_imports: unstable_schemas, + last_refresh: Instant::now(), }; // Insert the schema into the cache. From 94a9577e3477d2cdc12e2427376ff8e39dad1c2f Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Sun, 5 Jan 2020 14:03:58 -0600 Subject: [PATCH 62/64] store: Update comment --- store/postgres/src/store.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/store/postgres/src/store.rs b/store/postgres/src/store.rs index d655b640d88..d1034d30d25 100644 --- a/store/postgres/src/store.rs +++ b/store/postgres/src/store.rs @@ -706,7 +706,7 @@ impl Store { fn cached_schema(&self, subgraph_id: &SubgraphDeploymentId) -> Result { if let Some(entry) = self.schema_cache.lock().unwrap().get(&subgraph_id) { // Cache entry is stale and an unstable import has changed or is now available - // TODO: Is it okay to ignore entries unstable imports which are no longer available? + // TODO: Is it okay to ignore unstable imports which are no longer available? // TODO: Make this threshold an envvar? let requires_refresh = entry.last_refresh.elapsed().as_secs() >= 120 && entry From 4f0436b8c07f7eb0c1712d0060616e467e1d1294 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Fri, 10 Jan 2020 14:18:19 -0600 Subject: [PATCH 63/64] graphql, server: Remove redundant code --- graphql/src/execution/execution.rs | 2 ++ server/http/src/service.rs | 46 ++++++++++++------------------ 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/graphql/src/execution/execution.rs b/graphql/src/execution/execution.rs index dd099a326ee..027143046ae 100644 --- a/graphql/src/execution/execution.rs +++ b/graphql/src/execution/execution.rs @@ -359,6 +359,8 @@ where // See if this is an introspection or data field. We don't worry about // nonexistant fields; those will cause an error later when we execute // the data_set SelectionSet + // TODO: Does anything guarantee that the introspection and subgraph query fields + // do not overlap? if sast::get_field(introspection_query_type, &name).is_some() { intro_set.items.extend(selections) } else { diff --git a/server/http/src/service.rs b/server/http/src/service.rs index e2980bfeb68..d9147f34247 100644 --- a/server/http/src/service.rs +++ b/server/http/src/service.rs @@ -287,34 +287,26 @@ where .then( move |result: Result| -> GraphQLResponse { let elapsed = start.elapsed(); + service_metrics.observe_query_execution_time( + elapsed.as_secs_f64(), + sd_id.deref().to_string(), + ); match result { - Ok(_) => { - service_metrics.observe_query_execution_time( - elapsed.as_secs_f64(), - sd_id.deref().to_string(), - ); - info!( - logger, - "GraphQL query served"; - "subgraph_deployment" => sd_id.deref(), - "query_time_ms" => elapsed.as_millis(), - "code" => LogCode::GraphQlQuerySuccess, - ) - } - Err(ref e) => { - service_metrics.observe_query_execution_time( - elapsed.as_secs_f64(), - sd_id.deref().to_string(), - ); - error!( - logger, - "GraphQL query failed"; - "subgraph_deployment" => sd_id.deref(), - "error" => e.to_string(), - "query_time_ms" => elapsed.as_millis(), - "code" => LogCode::GraphQlQueryFailure, - ) - } + Ok(_) => info!( + logger, + "GraphQL query served"; + "subgraph_deployment" => sd_id.deref(), + "query_time_ms" => elapsed.as_millis(), + "code" => LogCode::GraphQlQuerySuccess, + ), + Err(ref e) => error!( + logger, + "GraphQL query failed"; + "subgraph_deployment" => sd_id.deref(), + "error" => e.to_string(), + "query_time_ms" => elapsed.as_millis(), + "code" => LogCode::GraphQlQueryFailure, + ), } GraphQLResponse::new(result) }, From 2b12994142d9a082977e0ee485278a36d58dfb19 Mon Sep 17 00:00:00 2001 From: Jorge Olivero Date: Tue, 14 Jan 2020 11:44:32 -0600 Subject: [PATCH 64/64] mock: Ensure Store is correctly mocked --- mock/src/store.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mock/src/store.rs b/mock/src/store.rs index f2c1d9034af..1c9138413a7 100644 --- a/mock/src/store.rs +++ b/mock/src/store.rs @@ -96,12 +96,12 @@ mock! { fn resolve_schema_reference( &self, _schema_reference: &SchemaReference, - ) -> Result, Error>; + ) -> Result, SchemaImportError>; fn resolve_import_graph( &self, _schema: &Schema, - ) -> (HashMap>, Vec); + ) -> (HashMap>, Vec); } trait ChainStore: Send + Sync + 'static {