diff --git a/v3/crates/graphql/schema/src/boolean_expression.rs b/v3/crates/graphql/schema/src/boolean_expression.rs index 9eef942be0327..e4a5ce0394839 100644 --- a/v3/crates/graphql/schema/src/boolean_expression.rs +++ b/v3/crates/graphql/schema/src/boolean_expression.rs @@ -346,40 +346,48 @@ fn build_new_comparable_relationships_schema( } })?; - // if we have specified a boolean expression type to use for this relationship, look it up - // (if none is specified, we use whichever is specified on the model) - let target_boolean_expression_graphql_type = - match &comparable_relationship.boolean_expression_type { - Some(target_boolean_expression_type_name) => { - let target_boolean_expression = gds + let target_boolean_expression_type_name = + &comparable_relationship.boolean_expression_type; + + let target_boolean_expression_graphql_type = { + let target_boolean_expression_graphql_type = match gds + .metadata + .boolean_expression_types + .objects + .get(target_boolean_expression_type_name) + { + Some(bool_exp) => { + Ok(bool_exp.graphql.as_ref().map(|graphql| &graphql.type_name)) + } + None => { + // perhaps it's referring to an old-style ObjectBooleanExpressionType + match gds .metadata - .boolean_expression_types - .objects + .object_boolean_expression_types .get(target_boolean_expression_type_name) - .ok_or_else(|| Error::InternalBooleanExpressionNotFound { + { + Some(object_bool_exp) => Ok(object_bool_exp + .graphql + .as_ref() + .map(|graphql| &graphql.type_name)), + None => Err(Error::InternalBooleanExpressionNotFound { type_name: target_boolean_expression_type_name.clone(), - })?; - - // if we find a type, make sure it's added to the schema - if let Some(graphql) = &target_boolean_expression.graphql { - let _registered_type_name = - builder.register_type(TypeId::InputObjectBooleanExpressionType { - graphql_type_name: graphql.type_name.clone(), - gds_type_name: target_boolean_expression_type_name.clone(), - }); + }), } - // return type name - target_boolean_expression - .graphql - .as_ref() - .map(|graphql| graphql.type_name.clone()) - } - None => { - // no specific type is provided by the relationship, so - // lookup filter expression graphql for target model - filter_graphql_type_for_model(target_model) } - }; + }?; + + // if we find a type, make sure it's added to the schema + if let Some(type_name) = target_boolean_expression_graphql_type { + let _registered_type_name = + builder.register_type(TypeId::InputObjectBooleanExpressionType { + graphql_type_name: type_name.clone(), + gds_type_name: target_boolean_expression_type_name.clone(), + }); + } + // return type name + target_boolean_expression_graphql_type + }; // lookup type underlying target model let target_object_type_representation = @@ -394,7 +402,7 @@ fn build_new_comparable_relationships_schema( let (name, schema) = build_model_relationship_schema( object_type_representation, target_object_type_representation, - &target_boolean_expression_graphql_type, + target_boolean_expression_graphql_type, target_model, target_source, relationship, @@ -412,26 +420,6 @@ fn build_new_comparable_relationships_schema( Ok(input_fields) } -fn filter_graphql_type_for_model( - model: &metadata_resolve::ModelWithArgumentPresets, -) -> Option { - match &model.filter_expression_type { - None => None, - Some(ModelExpressionType::BooleanExpressionType(target_boolean_expression_type)) => { - target_boolean_expression_type - .graphql - .as_ref() - .map(|graphql| graphql.type_name.clone()) - } - Some(ModelExpressionType::ObjectBooleanExpressionType(target_model_filter_expression)) => { - target_model_filter_expression - .graphql - .as_ref() - .map(|graphql| graphql.type_name.clone()) - } - } -} - // build comparable relationships input fields for the older `ObjectBooleanExpressionType` // TODO(naveen): Add support for command relationships fn build_comparable_relationships_schema( diff --git a/v3/crates/metadata-resolve/src/stages/boolean_expressions/error.rs b/v3/crates/metadata-resolve/src/stages/boolean_expressions/error.rs index f4397f8abe32a..ba514af393352 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/error.rs +++ b/v3/crates/metadata-resolve/src/stages/boolean_expressions/error.rs @@ -4,6 +4,7 @@ use crate::types::subgraph::{Qualified, QualifiedTypeName}; use open_dds::{ data_connector::{DataConnectorName, DataConnectorObjectType}, models::ModelName, + relationships::RelationshipName, types::{CustomTypeName, FieldName}, }; @@ -99,6 +100,14 @@ pub enum BooleanExpressionError { #[error("Field level comparison operator configuration is not fully supported yet. Please add all fields in filterable_fields.")] FieldLevelComparisonOperatorNeedsAllFields, + #[error( + "Target model {model_name:} not found, referenced in relationship {relationship_name:}" + )] + TargetModelNotFound { + relationship_name: RelationshipName, + model_name: Qualified, + }, + #[error("{0}")] GraphqlConfigError(#[from] graphql_config::GraphqlConfigError), diff --git a/v3/crates/metadata-resolve/src/stages/boolean_expressions/mod.rs b/v3/crates/metadata-resolve/src/stages/boolean_expressions/mod.rs index ac152c1fa8c5a..ff1f4bf559186 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/boolean_expressions/mod.rs @@ -46,6 +46,34 @@ pub fn resolve( ) -> Result { let mut raw_boolean_expression_types = BTreeMap::new(); let mut issues = Vec::new(); + let mut raw_models = BTreeMap::new(); + let mut object_boolean_expression_type_names = BTreeSet::new(); + + // collect raw models for use in resolving relationships + for open_dds::accessor::QualifiedObject { + path: _, + subgraph, + object: model, + } in &metadata_accessor.models + { + raw_models.insert( + Qualified::new(subgraph.clone(), model.name().clone()), + model, + ); + } + + // collect raw object_boolean_expression_type names for validation + for open_dds::accessor::QualifiedObject { + path: _, + subgraph, + object: object_boolean_expression_type, + } in &metadata_accessor.object_boolean_expression_types + { + object_boolean_expression_type_names.insert(Qualified::new( + subgraph.clone(), + object_boolean_expression_type.name.clone(), + )); + } // first we collect all the boolean_expression_types // so we have a full set to refer to when resolving them @@ -80,6 +108,8 @@ pub fn resolve( &boolean_expression_scalar_types, &raw_boolean_expression_types, relationships, + &raw_models, + &object_boolean_expression_type_names, graphql_config, &mut graphql_types, flags, diff --git a/v3/crates/metadata-resolve/src/stages/boolean_expressions/object.rs b/v3/crates/metadata-resolve/src/stages/boolean_expressions/object.rs index 2a3b1f02eff65..f9c6fa7047115 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/object.rs +++ b/v3/crates/metadata-resolve/src/stages/boolean_expressions/object.rs @@ -23,6 +23,7 @@ use open_dds::{ DataConnectorOperatorMapping, }, data_connector::DataConnectorName, + models::ModelName, types::{CustomTypeName, FieldName, TypeName}, }; @@ -50,6 +51,8 @@ pub(crate) fn resolve_object_boolean_expression_type( >, raw_boolean_expression_types: &RawBooleanExpressionTypes, relationships: &relationships::Relationships, + raw_models: &BTreeMap, &open_dds::models::Model>, + object_boolean_expression_type_names: &BTreeSet>, graphql_config: &graphql_config::GraphqlConfig, graphql_types: &mut BTreeSet, flags: &open_dds::flags::Flags, @@ -100,6 +103,8 @@ pub(crate) fn resolve_object_boolean_expression_type( relationships, subgraph, raw_boolean_expression_types, + raw_models, + object_boolean_expression_type_names, )?; // resolve graphql schema information @@ -145,6 +150,8 @@ fn resolve_comparable_relationships( relationships: &relationships::Relationships, subgraph: &SubgraphName, raw_boolean_expression_types: &RawBooleanExpressionTypes, + raw_models: &BTreeMap, &open_dds::models::Model>, + object_boolean_expression_type_names: &BTreeSet>, ) -> Result, BooleanExpressionError> { let mut resolved_comparable_relationships = BTreeMap::new(); @@ -156,40 +163,83 @@ fn resolve_comparable_relationships( 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, - )?; + let target_subgraph = + crate::helpers::relationship::get_target_subgraph(relationship) + .unwrap_or(subgraph.clone()); - Ok(target_boolean_expression_type) + let optional_target_boolean_expression_type_name = match &comparable_relationship + .boolean_expression_type + { + Some(target_boolean_expression_type_name) => { + Ok(Some(target_boolean_expression_type_name)) + } + None => { + // if nothing is defined we fall back to the boolean expression for the target + match &relationship.target { + open_dds::relationships::RelationshipTarget::Model(model_target) => { + let target_model_name = Qualified::new( + target_subgraph.clone(), + model_target.name.clone(), + ); + + match raw_models.get(&target_model_name) { + Some(raw_model) => { + Ok(raw_model.filter_expression_type().as_ref()) + } + None => Err(BooleanExpressionError::TargetModelNotFound { + relationship_name: comparable_relationship + .relationship_name + .clone(), + model_name: target_model_name, + }), + } + } + open_dds::relationships::RelationshipTarget::Command(_) => { + // command targets not currently supported in boolean expressions + // we just ignore these for now + Ok(None) + } + } + } + }?; + + // if the target is a Model, include it + if let Some(target_boolean_expression_type_name) = + optional_target_boolean_expression_type_name + { + // create target boolean expression name + let target_boolean_expression_type = Qualified::new( + target_subgraph, + target_boolean_expression_type_name.clone(), + ); + + // ...and ensure it exists + match helpers::lookup_raw_boolean_expression( + boolean_expression_type_name, + &target_boolean_expression_type, + raw_boolean_expression_types, + ) { + Ok(_) => Ok(()), + Err(e) => { + // it might be an old-style `ObjectBooleanExpressionType` + if object_boolean_expression_type_names + .contains(&target_boolean_expression_type) + { + Ok(()) + } else { + Err(e) + } + } + }?; + + 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, }, - ) - .transpose()?; - - 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 diff --git a/v3/crates/metadata-resolve/src/stages/boolean_expressions/types.rs b/v3/crates/metadata-resolve/src/stages/boolean_expressions/types.rs index 06a97c2bf2411..3975ce132e4db 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/types.rs +++ b/v3/crates/metadata-resolve/src/stages/boolean_expressions/types.rs @@ -181,7 +181,7 @@ pub struct BooleanExpressionComparableRelationship { /// The boolean expression type to use for comparison. This is optional for relationships to /// models, and defaults to the filterExpressionType of the model - pub boolean_expression_type: Option>, + pub boolean_expression_type: Qualified, } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] diff --git a/v3/crates/metadata-resolve/src/stages/model_permissions/model_permission.rs b/v3/crates/metadata-resolve/src/stages/model_permissions/model_permission.rs index bb3e50f6d96a0..2d8e9d34a3ca3 100644 --- a/v3/crates/metadata-resolve/src/stages/model_permissions/model_permission.rs +++ b/v3/crates/metadata-resolve/src/stages/model_permissions/model_permission.rs @@ -642,7 +642,6 @@ pub(crate) fn resolve_model_predicate_with_type( fields, type_name, &relationship.relationship_name, - target_model, boolean_expression_types, flags, ), @@ -762,7 +761,6 @@ fn lookup_relationship_in_boolean_expression( fields: &boolean_expressions::ResolvedObjectBooleanExpressionTypeFields, type_name: &Qualified, relationship_name: &open_dds::relationships::RelationshipName, - target_model: &models_graphql::ModelWithGraphql, boolean_expression_types: &boolean_expressions::BooleanExpressionTypes, flags: &open_dds::flags::Flags, ) -> Result, Error> { @@ -779,29 +777,16 @@ fn lookup_relationship_in_boolean_expression( })?; // lookup the boolean expression type for this comparable - // relationship - // if it is defined, we fetch it from metadata - match &comparable_relationship.boolean_expression_type { - Some(target_bool_exp_name) => boolean_expression_types - .objects - .get(target_bool_exp_name) - .map(|bool_exp| bool_exp.get_fields(flags).cloned()) - .ok_or_else(|| { - Error::from(TypePredicateError::BooleanExpressionNotFound { - boolean_expression_name: target_bool_exp_name.clone(), - }) - }), - None => { - // if it is not defined we fall back to the one defined on the model - // we ignore `ObjectBooleanExpressionType` as we should not be using a mixture - match &target_model.filter_expression_type { - Some(models_graphql::ModelExpressionType::BooleanExpressionType(bool_exp)) => { - Ok(bool_exp.get_fields(flags).cloned()) - } - _ => Ok(None), - } - } - } + // relationship and fetch it from metadata + boolean_expression_types + .objects + .get(&comparable_relationship.boolean_expression_type) + .map(|bool_exp| bool_exp.get_fields(flags).cloned()) + .ok_or_else(|| { + Error::from(TypePredicateError::BooleanExpressionNotFound { + boolean_expression_name: comparable_relationship.boolean_expression_type.clone(), + }) + }) } // this is only used for the older `ObjectBooleanExpressionType` where we