From e32755c25fc2c2fb12c2d01bb7d93223e0d763e6 Mon Sep 17 00:00:00 2001 From: Monica Tang Date: Fri, 22 Mar 2024 10:14:25 -0700 Subject: [PATCH] Add helper functions on GraphQL schema Type implementation to check if it is a resolver type Summary: ## Context After this stack, we will have multiple files that need to identify legacy verbose resolver-defined models by checking that the `__relay_model_instance` field is missing. ## This diff This diff consolidates this logic into type checking methods on `Type` (along with `is_weak_resolver_object` / `is_resolver_object` checks). Also, refactored `client_edges.rs` to use the new helpers. Some tests were missing the `@__RelayResolverModel` annotations on the resolver objects, so I added them in. Reviewed By: captbaritone Differential Revision: D55155005 fbshipit-source-id: 1234699e08cae2c07fb30422eb04640c85b57f55 --- compiler/crates/relay-schema/Cargo.toml | 1 + .../relay-schema/src/definitions/mod.rs | 56 +++++++++++++++++++ compiler/crates/relay-schema/src/lib.rs | 1 + compiler/crates/relay-transforms/Cargo.toml | 1 + .../relay-transforms/src/client_edges.rs | 48 +++++++--------- .../client-edge-to-client-interface.expected | 2 +- .../client-edge-to-client-interface.graphql | 2 +- ...-edge-to-client-interface.invalid.expected | 2 +- ...t-edge-to-client-interface.invalid.graphql | 2 +- 9 files changed, 82 insertions(+), 33 deletions(-) create mode 100644 compiler/crates/relay-schema/src/definitions/mod.rs diff --git a/compiler/crates/relay-schema/Cargo.toml b/compiler/crates/relay-schema/Cargo.toml index a7a8d989f650c..eeda391d3ca2e 100644 --- a/compiler/crates/relay-schema/Cargo.toml +++ b/compiler/crates/relay-schema/Cargo.toml @@ -10,6 +10,7 @@ license = "MIT" [dependencies] common = { path = "../common" } +docblock-shared = { path = "../docblock-shared" } intern = { path = "../intern" } lazy_static = "1.4" schema = { path = "../schema" } diff --git a/compiler/crates/relay-schema/src/definitions/mod.rs b/compiler/crates/relay-schema/src/definitions/mod.rs new file mode 100644 index 0000000000000..0ff33634c7d78 --- /dev/null +++ b/compiler/crates/relay-schema/src/definitions/mod.rs @@ -0,0 +1,56 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +use common::NamedItem; +use docblock_shared::RELAY_RESOLVER_MODEL_DIRECTIVE_NAME; +use docblock_shared::RELAY_RESOLVER_MODEL_INSTANCE_FIELD; +use docblock_shared::RELAY_RESOLVER_WEAK_OBJECT_DIRECTIVE; +use schema::Schema; +use schema::Type; + +pub trait ResolverType { + fn is_resolver_object(&self, schema: &S) -> bool; + fn is_weak_resolver_object(&self, schema: &S) -> bool; + fn is_terse_resolver_object(&self, schema: &S) -> bool; +} + +impl ResolverType for Type { + fn is_resolver_object(&self, schema: &S) -> bool { + if let Type::Object(object_id) = self { + let object = schema.object(*object_id); + object + .directives + .named(*RELAY_RESOLVER_MODEL_DIRECTIVE_NAME) + .is_some() + } else { + false + } + } + + fn is_weak_resolver_object(&self, schema: &S) -> bool { + if let Type::Object(object_id) = self { + let object = schema.object(*object_id); + object + .directives + .named(*RELAY_RESOLVER_WEAK_OBJECT_DIRECTIVE) + .is_some() + } else { + false + } + } + + fn is_terse_resolver_object(&self, schema: &S) -> bool { + if let Type::Object(object_id) = self { + let object = schema.object(*object_id); + object.fields.iter().any(|field_id| { + schema.field(*field_id).name.item == *RELAY_RESOLVER_MODEL_INSTANCE_FIELD + }) + } else { + false + } + } +} diff --git a/compiler/crates/relay-schema/src/lib.rs b/compiler/crates/relay-schema/src/lib.rs index e48f5863f88d4..c1cf722ea6cae 100644 --- a/compiler/crates/relay-schema/src/lib.rs +++ b/compiler/crates/relay-schema/src/lib.rs @@ -11,6 +11,7 @@ #![deny(rust_2018_idioms)] #![deny(clippy::all)] +pub mod definitions; use std::iter::once; use ::intern::string_key::StringKey; diff --git a/compiler/crates/relay-transforms/Cargo.toml b/compiler/crates/relay-transforms/Cargo.toml index 0253f113af581..28a5daf41830e 100644 --- a/compiler/crates/relay-transforms/Cargo.toml +++ b/compiler/crates/relay-transforms/Cargo.toml @@ -130,6 +130,7 @@ once_cell = "1.12" parking_lot = { version = "0.12.1", features = ["send_guard"] } regex = "1.9.2" relay-config = { path = "../relay-config" } +relay-schema = { path = "../relay-schema" } rustc-hash = "1.1.0" schema = { path = "../schema" } serde = { version = "1.0.185", features = ["derive", "rc"] } diff --git a/compiler/crates/relay-transforms/src/client_edges.rs b/compiler/crates/relay-transforms/src/client_edges.rs index 0500187fdfe8e..fa4817b8922eb 100644 --- a/compiler/crates/relay-transforms/src/client_edges.rs +++ b/compiler/crates/relay-transforms/src/client_edges.rs @@ -19,7 +19,6 @@ use docblock_shared::HAS_OUTPUT_TYPE_ARGUMENT_NAME; use docblock_shared::LIVE_ARGUMENT_NAME; use docblock_shared::RELAY_RESOLVER_DIRECTIVE_NAME; use docblock_shared::RELAY_RESOLVER_MODEL_INSTANCE_FIELD; -use docblock_shared::RELAY_RESOLVER_WEAK_OBJECT_DIRECTIVE; use graphql_ir::associated_data_impl; use graphql_ir::Argument; use graphql_ir::ConstantValue; @@ -45,6 +44,7 @@ use intern::string_key::StringKeyMap; use intern::Lookup; use lazy_static::lazy_static; use relay_config::ProjectConfig; +use relay_schema::definitions::ResolverType; use schema::DirectiveValue; use schema::ObjectID; use schema::Schema; @@ -386,7 +386,9 @@ impl<'program, 'pc> ClientEdgesTransform<'program, 'pc> { .filter_map(|object_id| { let model_resolver = self.get_client_edge_model_resolver_for_object(*object_id); model_resolver.or_else(|| { - if !self.is_weak_type(*object_id) && self.is_resolver_type(*object_id) { + let object = Type::Object(*object_id); + let schema = self.program.schema.as_ref(); + if !object.is_weak_resolver_object(schema) && object.is_resolver_object(schema) { let model_name = self.program.schema.object(*object_id).name; self.errors.push(Diagnostic::error( ValidationMessage::ClientEdgeToClientInterfaceImplementingObjectMissingModelResolver { @@ -432,39 +434,24 @@ impl<'program, 'pc> ClientEdgesTransform<'program, 'pc> { } } - fn is_weak_type(&self, object_id: ObjectID) -> bool { - let object = self.program.schema.object(object_id); - object - .directives - .named(*RELAY_RESOLVER_WEAK_OBJECT_DIRECTIVE) - .is_some() - } - - fn is_resolver_type(&self, object_id: ObjectID) -> bool { - let object = self.program.schema.object(object_id); - object - .directives - .named(*RELAY_RESOLVER_DIRECTIVE_NAME) - .is_some() - } - fn get_client_edge_model_resolver_for_object( &mut self, object_id: ObjectID, ) -> Option { - let type_name = self.program.schema.object(object_id).name; - let model_type = self.program.schema.get_type(type_name.item.0).unwrap(); + let model = Type::Object(object_id); + let schema = self.program.schema.as_ref(); + if !model.is_resolver_object(schema) + || model.is_weak_resolver_object(schema) + || !model.is_terse_resolver_object(schema) + { + return None; + } + let object = self.program.schema.object(object_id); let model_field_id = self .program .schema - .named_field(model_type, *RELAY_RESOLVER_MODEL_INSTANCE_FIELD); - // Legacy models (using verbose resolver syntax) do not have a __relay_model_instance field - if self.is_weak_type(object_id) { - return None; - } - let id = model_field_id?; - // Note: is_live is only true if the __relay_model_instance field exists on the model field - let model_field = self.program.schema.field(id); + .named_field(model, *RELAY_RESOLVER_MODEL_INSTANCE_FIELD)?; + let model_field = self.program.schema.field(model_field_id); let resolver_directive = model_field.directives.named(*RELAY_RESOLVER_DIRECTIVE_NAME); let is_live = resolver_directive.map_or(false, |resolver_directive| { resolver_directive @@ -472,7 +459,10 @@ impl<'program, 'pc> ClientEdgesTransform<'program, 'pc> { .iter() .any(|arg| arg.name.0 == LIVE_ARGUMENT_NAME.0) }); - Some(ClientEdgeModelResolver { type_name, is_live }) + Some(ClientEdgeModelResolver { + type_name: object.name, + is_live, + }) } fn get_edge_to_server_object_metadata_directive( diff --git a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.expected b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.expected index 6cc84ba61f937..9839d77e55cb8 100644 --- a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.expected +++ b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.expected @@ -18,7 +18,7 @@ interface ClientOnlyInterface implements Node { } # Add a concrete type so that we don't trigger an unrelated compiler error. -type BestFriend implements ClientOnlyInterface { +type BestFriend implements ClientOnlyInterface @__RelayResolverModel { id: ID! __relay_model_instance: RelayResolverValue @relay_resolver(import_path: "BestFriendResolver" fragment_name: "BestFriend__id", generated_fragment: true, inject_fragment_data: "id", import_name: "BestFriend") } diff --git a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.graphql b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.graphql index cacc8c352852f..fb8bb04a41dee 100644 --- a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.graphql +++ b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.graphql @@ -17,7 +17,7 @@ interface ClientOnlyInterface implements Node { } # Add a concrete type so that we don't trigger an unrelated compiler error. -type BestFriend implements ClientOnlyInterface { +type BestFriend implements ClientOnlyInterface @__RelayResolverModel { id: ID! __relay_model_instance: RelayResolverValue @relay_resolver(import_path: "BestFriendResolver" fragment_name: "BestFriend__id", generated_fragment: true, inject_fragment_data: "id", import_name: "BestFriend") } diff --git a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.expected b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.expected index 37bc1c2576445..293bc39debbc3 100644 --- a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.expected +++ b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.expected @@ -17,7 +17,7 @@ interface ClientOnlyInterface implements Node { } # Add a concrete type so that we don't trigger an unrelated compiler error. -type BestFriend implements ClientOnlyInterface { +type BestFriend implements ClientOnlyInterface @__RelayResolverModel { id: ID! __relay_model_instance: RelayResolverValue @relay_resolver(import_path: "BestFriendResolver" fragment_name: "BestFriend__id", generated_fragment: true, inject_fragment_data: "id", import_name: "BestFriend") } diff --git a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.graphql b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.graphql index f4b746c0c9892..2c95d7aa987f2 100644 --- a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.graphql +++ b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.graphql @@ -16,7 +16,7 @@ interface ClientOnlyInterface implements Node { } # Add a concrete type so that we don't trigger an unrelated compiler error. -type BestFriend implements ClientOnlyInterface { +type BestFriend implements ClientOnlyInterface @__RelayResolverModel { id: ID! __relay_model_instance: RelayResolverValue @relay_resolver(import_path: "BestFriendResolver" fragment_name: "BestFriend__id", generated_fragment: true, inject_fragment_data: "id", import_name: "BestFriend") }