Add helper functions on GraphQL schema Type implementation to check i…

…f 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
sakib-xeon · Mar 22, 2024 · e32755c · e32755c
1 parent 2e62de4
commit e32755c
Show file tree

Hide file tree

Showing 9 changed files with 82 additions and 33 deletions.
diff --git 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
@@ -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<S: Schema>(&self, schema: &S) -> bool;
+    fn is_weak_resolver_object<S: Schema>(&self, schema: &S) -> bool;
+    fn is_terse_resolver_object<S: Schema>(&self, schema: &S) -> bool;
+}
+
+impl ResolverType for Type {
+    fn is_resolver_object<S: Schema>(&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<S: Schema>(&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<S: Schema>(&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
@@ -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
@@ -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
@@ -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,47 +434,35 @@ 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<ClientEdgeModelResolver> {
-        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
                 .arguments
                 .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/...tes/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.expected b/...tes/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/...ates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.graphql b/...ates/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/...y-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.expected b/...y-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/...ay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.graphql b/...ay-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")
 }