Fixed error when relationships used in BooleanExpressionTypes were no…

…t handled correctly in partial supergraph mode (#1182)

### What
When the engine is run in `PARTIAL_SUPERGRAPH` mode, any relationship
that targets an unknown subgraph should be silently dropped. This was
being done when resolving relationship navigation fields on the
`ObjectType`, but not for `comparableRelationships` on
`BooleanExpressionType`. In that case we were erroring as we tried to
resolve the relationship and couldn't find the target subgraph.

### How
The `relationships` metadata resolve step has been enhanced to capture
if a relationship is targeting an unknown subgraph. The
`object_boolean_expressions` step already used this, so it was tweaked
to skip relationships targeting unknown subgraphs.

The `object_relationships` step, however, did not use `relationships`
and instead went back to metadata_accessor for relationships. It then
had special logic that skipped unknown subgraph relationships. The step
has now been refactored to use `relationships` instead and the special
skipping logic has been discarded, and it now just uses the unknown
subgraph information from `relationships`.

In addition, the `object_relationships` step now _consumes_ the output
of the `type_permissions` step, rather than cloning it. This reduces
unnecessary cloning and makes sense since `object_relationships` is
simply a further enriched version of `type_permissions`.

The test of whether the source object type of a relationship actually
exists has also been moved from `object_relationships` to
`relationships`, and the index built by `relationships` has been
reordered to group by type first and then name, since that is more
useful, especially in the `object_relationships` step.

V3_GIT_ORIGIN_REV_ID: e5d18343f5ce24532a3258e88751bc3183692c50

v3/changelog.md

@@ -47,6 +47,9 @@ the following customizations:
 ### Fixed
 
 - Fix poor performance of `process_response` for large and deeply-nested results
+- Fixed issue in partial supergraph builds where a `BooleanExpressionType` that
+  referenced a relationship that targeted an unknown subgraph would incorrectly
+  produce an error rather than ignoring the relationship
 
 ### Changed
 

v3/crates/metadata-resolve/src/helpers/mod.rs

@@ -3,6 +3,7 @@ pub mod argument;
 pub mod boolean_expression;
 pub mod http;
 pub mod ndc_validation;
+pub mod relationship;
 pub mod type_mappings;
 pub mod typecheck;
 pub mod types;

v3/crates/metadata-resolve/src/helpers/relationship.rs

@@ -0,0 +1,11 @@
+use open_dds::{
+    identifier::SubgraphName,
+    relationships::{RelationshipTarget, RelationshipV1},
+};
+
+pub fn get_target_subgraph(relationship: &RelationshipV1) -> Option<SubgraphName> {
+    match &relationship.target {
+        RelationshipTarget::Model(model_target) => model_target.subgraph(),
+        RelationshipTarget::Command(command_target) => command_target.subgraph(),
+    }
+}

v3/crates/metadata-resolve/src/stages/boolean_expressions/object.rs

@@ -121,46 +121,53 @@ fn resolve_comparable_relationships(
     let mut resolved_comparable_relationships = BTreeMap::new();
 
     for comparable_relationship in comparable_relationships {
-        // if the relationship has provided an optional boolean_expression_type, let's
-        // check it makes sense
-        let target_boolean_expression_type: Result<
-            Option<Qualified<CustomTypeName>>,
-            BooleanExpressionError,
-        > = match &comparable_relationship.boolean_expression_type {
-            Some(target_boolean_expression_type_name) => {
-                // we need to look up the target subgraph in the relationship
-                let relationship = relationships.get(
-                    underlying_object_type_name,
-                    &comparable_relationship.relationship_name,
-                )?;
-
-                // create target boolean expression name
-                let target_boolean_expression_type = Qualified::new(
-                    relationship
-                        .get_target_subgraph()
-                        .unwrap_or(subgraph.clone()),
-                    target_boolean_expression_type_name.clone(),
-                );
+        let relationship = relationships.get(
+            underlying_object_type_name,
+            &comparable_relationship.relationship_name,
+        )?;
+
+        match relationship {
+            relationships::Relationship::Relationship(relationship) => {
+                // if the relationship has provided an optional boolean_expression_type, let's
+                // check it makes sense
+                let target_boolean_expression_type = comparable_relationship
+                    .boolean_expression_type
+                    .as_ref()
+                    .map(
+                        |target_boolean_expression_type_name| -> Result<_, BooleanExpressionError> {
+                            // create target boolean expression name
+                            let target_boolean_expression_type = Qualified::new(
+                                crate::helpers::relationship::get_target_subgraph(relationship)
+                                    .unwrap_or(subgraph.clone()),
+                                target_boolean_expression_type_name.clone(),
+                            );
+
+                            // ...and ensure it exists
+                            let _raw_boolean_expression_type =
+                                helpers::lookup_raw_boolean_expression(
+                                    boolean_expression_type_name,
+                                    &target_boolean_expression_type,
+                                    raw_boolean_expression_types,
+                                )?;
 
-                // ...and ensure it exists
-                let _raw_boolean_expression_type = helpers::lookup_raw_boolean_expression(
-                    boolean_expression_type_name,
-                    &target_boolean_expression_type,
-                    raw_boolean_expression_types,
-                )?;
+                            Ok(target_boolean_expression_type)
+                        },
+                    )
+                    .transpose()?;
 
-                Ok(Some(target_boolean_expression_type))
+                resolved_comparable_relationships.insert(
+                    FieldName::new(comparable_relationship.relationship_name.inner().clone()),
+                    BooleanExpressionComparableRelationship {
+                        relationship_name: comparable_relationship.relationship_name.clone(),
+                        boolean_expression_type: target_boolean_expression_type,
+                    },
+                );
             }
-            None => Ok(None),
-        };
 
-        resolved_comparable_relationships.insert(
-            FieldName::new(comparable_relationship.relationship_name.inner().clone()),
-            BooleanExpressionComparableRelationship {
-                relationship_name: comparable_relationship.relationship_name.clone(),
-                boolean_expression_type: target_boolean_expression_type?,
-            },
-        );
+            // If the relationship is to an unknown subgraph, skip it because we're in
+            // allow unknown subgraphs mode
+            relationships::Relationship::RelationshipToUnknownSubgraph => {}
+        };
     }
 
     Ok(resolved_comparable_relationships)

v3/crates/metadata-resolve/src/stages/mod.rs

@@ -103,7 +103,12 @@ pub fn resolve(
         type_permissions::resolve(&metadata_accessor, object_types).map_err(Error::from)?;
 
     // collect raw relationships information
-    let relationships = relationships::resolve(&metadata_accessor).map_err(Error::from)?;
+    let relationships = relationships::resolve(
+        configuration,
+        &metadata_accessor,
+        &object_types_with_permissions,
+    )
+    .map_err(Error::from)?;
 
     // Resolve fancy new boolean expression types
     let boolean_expressions::BooleanExpressionsOutput {
@@ -208,11 +213,10 @@ pub fn resolve(
     relay::resolve(global_id_enabled_types).map_err(Error::from)?;
 
     let object_types_with_relationships = object_relationships::resolve(
-        &metadata_accessor,
-        configuration,
+        object_types_with_permissions,
+        &relationships,
         &data_connectors,
         &data_connector_scalars,
-        &object_types_with_permissions,
         &models,
         &commands,
         &aggregate_expressions,