Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make schema validation report using diagnostics #4685

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions compiler/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 14 additions & 11 deletions compiler/crates/relay-compiler/src/build_project/build_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

use std::sync::Arc;

use common::Diagnostic;
use common::DiagnosticsResult;
use common::Location;
use common::PerfLogEvent;
use common::Span;
use fnv::FnvHashMap;
use relay_config::ProjectName;
use schema::SDLSchema;
Expand Down Expand Up @@ -74,22 +74,25 @@ pub fn build_schema(
.feature_flags
.enable_experimental_schema_validation
{
let validation_context = log_event.time("validate_composite_schema_time", || {
log_event.time("validate_composite_schema_time", || {
let default_location_key = schema_sources
.first()
.map(|(_, key)| key)
.or_else(|| extensions.first().map(|(_, key)| key));

let default_location = match default_location_key {
Some(location) => Location::new(*location, Span::empty()),
None => Location::generated(),
};

validate(
&schema,
default_location,
SchemaValidationOptions {
allow_introspection_names: true,
},
)
});
if !validation_context.errors.is_empty() {
// TODO: Before removing this feature flag, we should update schema validation
// to be able to return a list of Diagnostics with locations.
return Err(vec![Diagnostic::error(
validation_context.print_errors(),
Location::generated(),
)]);
}
})?;
}

Ok(Arc::new(schema))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
==================================== INPUT ====================================
//- PersonComponent.js
graphql`fragment PersonComponentFragment on IPerson {
name
}`

//- UserTypeResolvers.js
/**
* @RelayResolver User implements IPerson
*/

/**
* @RelayResolver User.name: String
*/

//- AdminTypeResolvers.js
/**
* @RelayResolver Admin implements IPerson
*/

# Admin should implement name, but does not!

//- relay.config.json
{
"language": "flow",
"jsModuleFormat": "haste",
"schema": "schema.graphql",
"schemaExtensions": [
"schema-extensions"
],
"featureFlags": {
"enable_relay_resolver_transform": true,
"enable_resolver_normalization_ast": true,
"relay_resolver_enable_interface_output_type": { "kind": "enabled" },
"enable_experimental_schema_validation": true
}
}

//- schema.graphql
type Query {
some_field: Boolean
}

//- schema-extensions/extension.graphql
interface IPerson {
id: ID!
name: String
}
==================================== OUTPUT ===================================
✖︎ Interface field 'IPerson.name' expected but 'Admin' does not provide it.

AdminTypeResolvers.js:2:19
1 │ *
2 │ * @RelayResolver Admin implements IPerson
│ ^^^^^
3 │

ℹ︎ The interface field is defined here:

schema-extensions/extension.graphql:3:3
2 │ id: ID!
3 │ name: String
│ ^^^^
4 │ }
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//- PersonComponent.js
graphql`fragment PersonComponentFragment on IPerson {
name
}`

//- UserTypeResolvers.js
/**
* @RelayResolver User implements IPerson
*/

/**
* @RelayResolver User.name: String
*/

//- AdminTypeResolvers.js
/**
* @RelayResolver Admin implements IPerson
*/

# Admin should implement name, but does not!

//- relay.config.json
{
"language": "flow",
"jsModuleFormat": "haste",
"schema": "schema.graphql",
"schemaExtensions": [
"schema-extensions"
],
"featureFlags": {
"enable_relay_resolver_transform": true,
"enable_resolver_normalization_ast": true,
"relay_resolver_enable_interface_output_type": { "kind": "enabled" },
"enable_experimental_schema_validation": true
}
}

//- schema.graphql
type Query {
some_field: Boolean
}

//- schema-extensions/extension.graphql
interface IPerson {
id: ID!
name: String
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<f5708275a9732c3842b1df840547eac9>>
* @generated SignedSource<<acfea8824e445c77070b52df2044f420>>
*/

mod relay_compiler_integration;
Expand Down Expand Up @@ -117,6 +117,13 @@ async fn resolver_on_interface() {
test_fixture(transform_fixture, file!(), "resolver_on_interface.input", "relay_compiler_integration/fixtures/resolver_on_interface.expected", input, expected).await;
}

#[tokio::test]
async fn resolver_on_interface_does_not_pass_schema_validation_invalid() {
let input = include_str!("relay_compiler_integration/fixtures/resolver_on_interface_does_not_pass_schema_validation.invalid.input");
let expected = include_str!("relay_compiler_integration/fixtures/resolver_on_interface_does_not_pass_schema_validation.invalid.expected");
test_fixture(transform_fixture, file!(), "resolver_on_interface_does_not_pass_schema_validation.invalid.input", "relay_compiler_integration/fixtures/resolver_on_interface_does_not_pass_schema_validation.invalid.expected", input, expected).await;
}

#[tokio::test]
async fn resolver_on_interface_of_all_strong_model_type() {
let input = include_str!("relay_compiler_integration/fixtures/resolver_on_interface_of_all_strong_model_type.input");
Expand Down
10 changes: 9 additions & 1 deletion compiler/crates/schema-validate/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,23 @@ name = "schema_validate_test"
path = "tests/validate_schema_test.rs"

[dependencies]
clap = { version = "3.2.25", features = ["derive", "env", "regex", "unicode", "wrap_help"] }
clap = { version = "3.2.25", features = [
"derive",
"env",
"regex",
"unicode",
"wrap_help",
] }
common = { path = "../common" }
fnv = "1.0"
intern = { path = "../intern" }
lazy_static = "1.4"
log = { version = "0.4.17", features = ["kv_unstable", "kv_unstable_std"] }
regex = "1.9.2"
schema = { path = "../schema" }
graphql-cli = { path = "../graphql-cli" }
schema-print = { path = "../schema-print" }
serde = { version = "1.0.185", features = ["derive", "rc"] }
thiserror = "1.0.49"

[dev-dependencies]
Expand Down
16 changes: 7 additions & 9 deletions compiler/crates/schema-validate/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,15 @@ use common::InterfaceName;
use common::ObjectName;
use common::UnionName;
use intern::string_key::StringKey;
use schema::Type;
use schema::TypeReference;
use thiserror::Error;

#[derive(Debug, Error)]
#[derive(Clone, Debug, Error, serde::Serialize)]
pub enum SchemaValidationError {
#[error("'{0}' root type must be provided.")]
MissingRootType(StringKey),

#[error("'{0}' root type must be Object type. Found '{1:?}'")]
InvalidRootType(StringKey, Type),
#[error("'{0}' root type must be Object type. Found {1}")]
InvalidRootType(StringKey, String),

#[error("Name '{0}' must not begin with '__', which is reserved by GraphQL introspection.")]
InvalidNamePrefix(String),
Expand All @@ -37,11 +35,11 @@ pub enum SchemaValidationError {
#[error("Type must define one or more fields.")]
TypeWithNoFields,

#[error("The type of '{0}.{1}' must be Output Type but got: '{2:?}'.")]
InvalidFieldType(StringKey, StringKey, TypeReference<Type>),
#[error("The type of '{0}.{1}' must be Output Type but got {2}.")]
InvalidFieldType(StringKey, StringKey, String),

#[error("The type of '{0}.{1}({2}:)' must be InputType but got: '{3:?}'.")]
InvalidArgumentType(StringKey, StringKey, ArgumentName, TypeReference<Type>),
#[error("The type of '{0}.{1}({2}:)' must be InputType but got: {3}.")]
InvalidArgumentType(StringKey, StringKey, ArgumentName, String),

#[error("Type '{0}' can only implement '{1}' once.")]
DuplicateInterfaceImplementation(StringKey, InterfaceName),
Expand Down