diff --git a/v3/crates/engine/tests/execute/commands/functions/boolean_expression_command_argument/object_boolean_expression_type/metadata.json b/v3/crates/engine/tests/execute/commands/functions/boolean_expression_command_argument/object_boolean_expression_type/metadata.json index 94ebf185598..f2e5f32dde5 100644 --- a/v3/crates/engine/tests/execute/commands/functions/boolean_expression_command_argument/object_boolean_expression_type/metadata.json +++ b/v3/crates/engine/tests/execute/commands/functions/boolean_expression_command_argument/object_boolean_expression_type/metadata.json @@ -4,6 +4,28 @@ { "name": "default", "objects": [ + { + "kind": "DataConnectorScalarRepresentation", + "version": "v1", + "definition": { + "dataConnectorName": "custom", + "dataConnectorScalarType": "String", + "representation": "String", + "graphql": { + "comparisonExpressionTypeName": "String_Comparison_Exp" + } + } + }, + { + "kind": "DataConnectorScalarRepresentation", + "version": "v1", + "definition": { + "dataConnectorName": "custom", + "dataConnectorScalarType": "Int", + "representation": "Int" + } + }, + { "kind": "TypePermissions", "version": "v1", diff --git a/v3/crates/engine/tests/execute/common_metadata/command_metadata.json b/v3/crates/engine/tests/execute/common_metadata/command_metadata.json index 1a0a3d3d287..8d88b5cf81e 100644 --- a/v3/crates/engine/tests/execute/common_metadata/command_metadata.json +++ b/v3/crates/engine/tests/execute/common_metadata/command_metadata.json @@ -4,27 +4,6 @@ { "name": "default", "objects": [ - { - "kind": "DataConnectorScalarRepresentation", - "version": "v1", - "definition": { - "dataConnectorName": "custom", - "dataConnectorScalarType": "String", - "representation": "String", - "graphql": { - "comparisonExpressionTypeName": "String_Comparison_Exp" - } - } - }, - { - "kind": "DataConnectorScalarRepresentation", - "version": "v1", - "definition": { - "dataConnectorName": "custom", - "dataConnectorScalarType": "Int", - "representation": "Int" - } - }, { "kind": "ObjectType", "version": "v1", diff --git a/v3/crates/engine/tests/execute/models/select_many/relationship_predicates/common_metadata.json b/v3/crates/engine/tests/execute/models/select_many/relationship_predicates/common_metadata.json index a236e4bce33..859aba36550 100644 --- a/v3/crates/engine/tests/execute/models/select_many/relationship_predicates/common_metadata.json +++ b/v3/crates/engine/tests/execute/models/select_many/relationship_predicates/common_metadata.json @@ -52,33 +52,36 @@ "kind": "ObjectType" }, { - "kind": "ObjectBooleanExpressionType", + "kind": "BooleanExpressionType", "version": "v1", "definition": { "name": "album_bool_exp", - "objectType": "Album", - "dataConnectorName": "db", - "dataConnectorObjectType": "Album", - "comparableFields": [ - { - "fieldName": "AlbumId", - "operators": { - "enableAll": true - } - }, - { - "fieldName": "ArtistId", - "operators": { - "enableAll": true - } - }, - { - "fieldName": "Title", - "operators": { - "enableAll": true - } + "operand": { + "object": { + "type": "Album", + "comparableFields": [ + { + "fieldName": "AlbumId", + "booleanExpressionType": "int_bool_exp" + }, + { + "fieldName": "ArtistId", + "booleanExpressionType": "int_bool_exp" + }, + { + "fieldName": "Title", + "booleanExpressionType": "string_bool_exp" + } + ], + "comparableRelationships": [ + { "relationshipName": "Tracks" }, + { "relationshipName": "Artist" } + ] } - ], + }, + "logicalOperators": { "enable": true }, + "isNull": { "enable": true }, + "graphql": { "typeName": "Album_Where_Exp" } @@ -141,39 +144,37 @@ "kind": "ObjectType" }, { - "kind": "ObjectBooleanExpressionType", + "kind": "BooleanExpressionType", "version": "v1", "definition": { "name": "track_bool_exp", - "objectType": "Track", - "dataConnectorName": "db", - "dataConnectorObjectType": "Track", - "comparableFields": [ - { - "fieldName": "AlbumId", - "operators": { - "enableAll": true - } - }, - { - "fieldName": "TrackId", - "operators": { - "enableAll": true - } - }, - { - "fieldName": "Name", - "operators": { - "enableAll": true - } - }, - { - "fieldName": "GenreId", - "operators": { - "enableAll": true - } + "operand": { + "object": { + "type": "Track", + "comparableFields": [ + { + "fieldName": "AlbumId", + "booleanExpressionType": "int_bool_exp" + }, + { + "fieldName": "TrackId", + "booleanExpressionType": "int_bool_exp" + }, + { + "fieldName": "Name", + "booleanExpressionType": "string_bool_exp" + }, + { + "fieldName": "GenreId", + "booleanExpressionType": "int_bool_exp" + } + ], + "comparableRelationships": [{ "relationshipName": "Album" }] } - ], + }, + "logicalOperators": { "enable": true }, + "isNull": { "enable": true }, + "graphql": { "typeName": "Track_Where_Exp" } @@ -218,44 +219,34 @@ "kind": "ObjectType" }, { - "kind": "ObjectBooleanExpressionType", + "kind": "BooleanExpressionType", "version": "v1", "definition": { "name": "genre_bool_exp", - "objectType": "Genre", - "dataConnectorName": "db", - "dataConnectorObjectType": "Genre", - "comparableFields": [ - { - "fieldName": "Name", - "operators": { - "enableAll": true - } - }, - { - "fieldName": "GenreId", - "operators": { - "enableAll": true - } + "operand": { + "object": { + "type": "Genre", + "comparableFields": [ + { + "fieldName": "Name", + "booleanExpressionType": "string_bool_exp" + }, + { + "fieldName": "GenreId", + "booleanExpressionType": "int_bool_exp" + } + ], + "comparableRelationships": [] } - ], + }, + "logicalOperators": { "enable": true }, + "isNull": { "enable": true }, + "graphql": { "typeName": "Genre_Where_Exp" } } }, - { - "kind": "DataConnectorScalarRepresentation", - "version": "v1", - "definition": { - "dataConnectorName": "db", - "dataConnectorScalarType": "String", - "representation": "String", - "graphql": { - "comparisonExpressionTypeName": "String_Comparison_Exp" - } - } - }, { "kind": "ScalarType", "version": "v1", @@ -630,28 +621,86 @@ "version": "v1", "kind": "ObjectType" }, + { - "kind": "ObjectBooleanExpressionType", + "kind": "BooleanExpressionType", + "version": "v1", + "definition": { + "name": "string_bool_exp", + "operand": { + "scalar": { + "type": "String", + "comparisonOperators": [ + { "name": "_eq", "argumentType": "String!" } + ], + "dataConnectorOperatorMapping": [ + { + "dataConnectorName": "db", + "dataConnectorScalarType": "String", + "operatorMapping": {} + } + ] + } + }, + "logicalOperators": { "enable": true }, + "isNull": { "enable": true }, + + "graphql": { + "typeName": "String_Comparison_Exp" + } + } + }, + { + "kind": "BooleanExpressionType", + "version": "v1", + "definition": { + "name": "int_bool_exp", + "operand": { + "scalar": { + "type": "Int", + "comparisonOperators": [ + { "name": "_eq", "argumentType": "Int!" } + ], + "dataConnectorOperatorMapping": [ + { + "dataConnectorName": "db", + "dataConnectorScalarType": "Int", + "operatorMapping": {} + } + ] + } + }, + "logicalOperators": { "enable": true }, + "isNull": { "enable": true }, + "graphql": { + "typeName": "Int_Comparison_Exp" + } + } + }, + + { + "kind": "BooleanExpressionType", "version": "v1", "definition": { "name": "artist_bool_exp", - "objectType": "Artist", - "dataConnectorName": "db", - "dataConnectorObjectType": "Artist", - "comparableFields": [ - { - "fieldName": "ArtistId", - "operators": { - "enableAll": true - } - }, - { - "fieldName": "Name", - "operators": { - "enableAll": true - } + "operand": { + "object": { + "type": "Artist", + "comparableFields": [ + { + "fieldName": "ArtistId", + "booleanExpressionType": "int_bool_exp" + }, + { + "fieldName": "Name", + "booleanExpressionType": "string_bool_exp" + } + ], + "comparableRelationships": [] } - ], + }, + "logicalOperators": { "enable": true }, + "isNull": { "enable": true }, "graphql": { "typeName": "Artist_Where_Exp" } @@ -780,18 +829,6 @@ }, "version": "v1", "kind": "Relationship" - }, - { - "kind": "DataConnectorScalarRepresentation", - "version": "v1", - "definition": { - "dataConnectorName": "db", - "dataConnectorScalarType": "Int", - "representation": "Int", - "graphql": { - "comparisonExpressionTypeName": "Int_comparison" - } - } } ] } diff --git a/v3/crates/engine/tests/execute/models/select_many/type_permission/where/metadata.json b/v3/crates/engine/tests/execute/models/select_many/type_permission/where/metadata.json index f92f19f1f89..a8a1337ad55 100644 --- a/v3/crates/engine/tests/execute/models/select_many/type_permission/where/metadata.json +++ b/v3/crates/engine/tests/execute/models/select_many/type_permission/where/metadata.json @@ -5,26 +5,58 @@ "name": "default", "objects": [ { - "kind": "DataConnectorScalarRepresentation", + "kind": "BooleanExpressionType", "version": "v1", "definition": { - "dataConnectorName": "db", - "dataConnectorScalarType": "String", - "representation": "String", + "name": "string_bool_exp", + "operand": { + "scalar": { + "type": "String", + "comparisonOperators": [ + { "name": "_eq", "argumentType": "String!" } + ], + "dataConnectorOperatorMapping": [ + { + "dataConnectorName": "db", + "dataConnectorScalarType": "String", + "operatorMapping": {} + } + ] + } + }, + "logicalOperators": { "enable": true }, + "isNull": { "enable": true }, + "graphql": { - "comparisonExpressionTypeName": "String_Comparison_Exp" + "typeName": "String_Comparison_Exp" } } }, { - "kind": "DataConnectorScalarRepresentation", + "kind": "BooleanExpressionType", "version": "v1", "definition": { - "dataConnectorName": "db", - "dataConnectorScalarType": "Int", - "representation": "Int" + "name": "int_bool_exp", + "operand": { + "scalar": { + "type": "Int", + "comparisonOperators": [ + { "name": "_eq", "argumentType": "Int!" } + ], + "dataConnectorOperatorMapping": [ + { + "dataConnectorName": "db", + "dataConnectorScalarType": "Int", + "operatorMapping": {} + } + ] + } + }, + "logicalOperators": { "enable": true }, + "isNull": { "enable": true } } }, + { "kind": "ObjectType", "version": "v1", @@ -73,33 +105,32 @@ } }, { - "kind": "ObjectBooleanExpressionType", + "kind": "BooleanExpressionType", "version": "v1", "definition": { "name": "author_bool_exp", - "objectType": "author", - "dataConnectorName": "db", - "dataConnectorObjectType": "author", - "comparableFields": [ - { - "fieldName": "author_id", - "operators": { - "enableAll": true - } - }, - { - "fieldName": "first_name", - "operators": { - "enableAll": true - } - }, - { - "fieldName": "last_name", - "operators": { - "enableAll": true - } + "operand": { + "object": { + "type": "author", + "comparableFields": [ + { + "fieldName": "author_id", + "booleanExpressionType": "int_bool_exp" + }, + { + "fieldName": "first_name", + "booleanExpressionType": "string_bool_exp" + }, + { + "fieldName": "last_name", + "booleanExpressionType": "string_bool_exp" + } + ], + "comparableRelationships": [] } - ], + }, + "logicalOperators": { "enable": true }, + "isNull": { "enable": true }, "graphql": { "typeName": "Author_Filter" } diff --git a/v3/crates/metadata-resolve/src/helpers/boolean_expression.rs b/v3/crates/metadata-resolve/src/helpers/boolean_expression.rs index 41476fd66fe..2f783c588fc 100644 --- a/v3/crates/metadata-resolve/src/helpers/boolean_expression.rs +++ b/v3/crates/metadata-resolve/src/helpers/boolean_expression.rs @@ -1,6 +1,8 @@ use crate::types::error::{BooleanExpressionError, Error, TypePredicateError}; -use crate::stages::{boolean_expressions, data_connectors, models, relationships}; +use crate::stages::{ + boolean_expressions, data_connectors, models, relationships, scalar_boolean_expressions, +}; use crate::types::subgraph::Qualified; use indexmap::IndexMap; use open_dds::{ @@ -170,7 +172,7 @@ fn validate_data_connector_with_comparable_relationship( // check that a scalar BooleanExpressionType has info for whichever data connector we are using fn validate_data_connector_with_scalar_boolean_expression_type( - scalar_boolean_expression_type: &boolean_expressions::ResolvedScalarBooleanExpressionType, + scalar_boolean_expression_type: &scalar_boolean_expressions::ResolvedScalarBooleanExpressionType, parent_boolean_expression_type_name: &Qualified, data_connector: &data_connectors::DataConnectorLink, field_name: &FieldName, diff --git a/v3/crates/metadata-resolve/src/helpers/types.rs b/v3/crates/metadata-resolve/src/helpers/types.rs index 55e74f26d04..48ac10b3582 100644 --- a/v3/crates/metadata-resolve/src/helpers/types.rs +++ b/v3/crates/metadata-resolve/src/helpers/types.rs @@ -1,4 +1,7 @@ -use crate::stages::{boolean_expressions, object_boolean_expressions, relationships, scalar_types}; +use crate::stages::{ + boolean_expressions, object_boolean_expressions, relationships, scalar_boolean_expressions, + scalar_types, +}; use crate::types::error::{BooleanExpressionError, Error}; use crate::types::subgraph::{ @@ -45,7 +48,7 @@ pub enum TypeRepresentation<'a, ObjectType> { /// New object boolean expression type BooleanExpressionObject(&'a boolean_expressions::ResolvedObjectBooleanExpressionType), /// New scalar boolean expression type - BooleanExpressionScalar(&'a boolean_expressions::ResolvedScalarBooleanExpressionType), + BooleanExpressionScalar(&'a scalar_boolean_expressions::ResolvedScalarBooleanExpressionType), } /// validate whether a given CustomTypeName exists within `object_types`, `scalar_types` or diff --git a/v3/crates/metadata-resolve/src/lib.rs b/v3/crates/metadata-resolve/src/lib.rs index 8dcadb31ba1..007706ad657 100644 --- a/v3/crates/metadata-resolve/src/lib.rs +++ b/v3/crates/metadata-resolve/src/lib.rs @@ -24,7 +24,7 @@ pub use stages::aggregates::{ pub use stages::boolean_expressions::{ BooleanExpressionComparableRelationship, BooleanExpressionGraphqlConfig, ComparisonExpressionInfo, IncludeLogicalOperators, ObjectComparisonExpressionInfo, - ResolvedObjectBooleanExpressionType, ResolvedScalarBooleanExpressionType, + ResolvedObjectBooleanExpressionType, }; pub use stages::command_permissions::CommandWithPermissions; pub use stages::commands::Command; @@ -34,6 +34,7 @@ pub use stages::model_permissions::{ FilterPermission, ModelPredicate, ModelTargetSource, ModelWithPermissions, SelectPermission, }; pub use stages::models::{ConnectorArgumentName, Model, ModelSource}; +pub use stages::scalar_boolean_expressions::ResolvedScalarBooleanExpressionType; pub use stages::models_graphql::{ ModelExpressionType, ModelOrderByExpression, SelectAggregateGraphQlDefinition, diff --git a/v3/crates/metadata-resolve/src/stages/boolean_expressions/graphql.rs b/v3/crates/metadata-resolve/src/stages/boolean_expressions/graphql.rs index 4372f50731c..188865c645c 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/graphql.rs +++ b/v3/crates/metadata-resolve/src/stages/boolean_expressions/graphql.rs @@ -1,11 +1,10 @@ use super::helpers; pub use super::{ BooleanExpressionComparableRelationship, BooleanExpressionGraphqlConfig, - BooleanExpressionGraphqlFieldConfig, ComparisonExpressionInfo, IncludeIsNull, - ObjectComparisonExpressionInfo, ResolvedScalarBooleanExpressionType, + BooleanExpressionGraphqlFieldConfig, ComparisonExpressionInfo, ObjectComparisonExpressionInfo, }; use crate::helpers::types::mk_name; -use crate::stages::graphql_config; +use crate::stages::{graphql_config, scalar_boolean_expressions}; use crate::types::error::{Error, GraphqlConfigError}; use crate::types::subgraph::mk_qualified_type_reference; use crate::Qualified; @@ -29,7 +28,7 @@ pub(crate) fn resolve_object_boolean_graphql( comparable_relationships: &BTreeMap, scalar_boolean_expression_types: &BTreeMap< Qualified, - ResolvedScalarBooleanExpressionType, + scalar_boolean_expressions::ResolvedScalarBooleanExpressionType, >, raw_boolean_expression_types: &super::object::RawBooleanExpressionTypes, subgraph: &str, @@ -72,7 +71,8 @@ pub(crate) fn resolve_object_boolean_graphql( // Register scalar comparison field only if it contains non-zero operators. if !operators.is_empty() - || scalar_boolean_expression_type.include_is_null == IncludeIsNull::Yes + || scalar_boolean_expression_type.include_is_null + == scalar_boolean_expressions::IncludeIsNull::Yes { scalar_fields.insert( comparable_field_name.clone(), @@ -84,10 +84,10 @@ pub(crate) fn resolve_object_boolean_graphql( is_null_operator_name: match scalar_boolean_expression_type .include_is_null { - IncludeIsNull::Yes => { + scalar_boolean_expressions::IncludeIsNull::Yes => { Some(filter_graphql_config.operator_names.is_null.clone()) } - IncludeIsNull::No => None, + scalar_boolean_expressions::IncludeIsNull::No => None, }, }, ); diff --git a/v3/crates/metadata-resolve/src/stages/boolean_expressions/helpers.rs b/v3/crates/metadata-resolve/src/stages/boolean_expressions/helpers.rs index b00ec19d1d5..49b85bbfad3 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/helpers.rs +++ b/v3/crates/metadata-resolve/src/stages/boolean_expressions/helpers.rs @@ -1,12 +1,9 @@ -use open_dds::{ - boolean_expression::{BooleanExpressionIsNull, BooleanExpressionLogicalOperators}, - types::CustomTypeName, -}; +use open_dds::{boolean_expression::BooleanExpressionLogicalOperators, types::CustomTypeName}; use crate::types::error::{BooleanExpressionError, Error}; use crate::Qualified; -use super::types::{IncludeIsNull, IncludeLogicalOperators}; +use super::types::IncludeLogicalOperators; pub(crate) fn lookup_raw_boolean_expression<'a>( parent_boolean_expression_name: &Qualified, @@ -30,15 +27,7 @@ pub(crate) fn lookup_raw_boolean_expression<'a>( }) } -pub(crate) fn resolve_is_null(is_null: &BooleanExpressionIsNull) -> IncludeIsNull { - if is_null.enable { - IncludeIsNull::Yes - } else { - IncludeIsNull::No - } -} - -pub(crate) fn resolve_logical_operators( +pub fn resolve_logical_operators( logical_operators: &BooleanExpressionLogicalOperators, ) -> IncludeLogicalOperators { if logical_operators.enable { 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 11abfdfb029..d4546f4486d 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/boolean_expressions/mod.rs @@ -1,7 +1,6 @@ mod graphql; mod helpers; mod object; -mod scalar; mod types; use std::collections::{BTreeMap, BTreeSet}; @@ -9,7 +8,7 @@ use std::collections::{BTreeMap, BTreeSet}; use lang_graphql::ast::common as ast; use open_dds::{boolean_expression::BooleanExpressionOperand, types::CustomTypeName}; -use crate::stages::{data_connectors, graphql_config, type_permissions}; +use crate::stages::{graphql_config, scalar_boolean_expressions, type_permissions}; use crate::types::configuration::Configuration; use crate::types::error::{BooleanExpressionError, Error}; use crate::Qualified; @@ -17,17 +16,19 @@ use crate::Qualified; pub use types::{ BooleanExpressionComparableRelationship, BooleanExpressionGraphqlConfig, BooleanExpressionGraphqlFieldConfig, BooleanExpressionTypes, BooleanExpressionsOutput, - ComparisonExpressionInfo, IncludeIsNull, IncludeLogicalOperators, - ObjectComparisonExpressionInfo, ResolvedObjectBooleanExpressionType, - ResolvedScalarBooleanExpressionType, + ComparisonExpressionInfo, IncludeLogicalOperators, ObjectComparisonExpressionInfo, + ResolvedObjectBooleanExpressionType, }; pub fn resolve( metadata_accessor: &open_dds::accessor::MetadataAccessor, configuration: Configuration, + boolean_expression_scalar_types: &BTreeMap< + Qualified, + scalar_boolean_expressions::ResolvedScalarBooleanExpressionType, + >, existing_graphql_types: &BTreeSet, graphql_config: &graphql_config::GraphqlConfig, - data_connectors: &data_connectors::DataConnectors, object_types: &BTreeMap, type_permissions::ObjectTypeWithPermissions>, ) -> Result { if !configuration @@ -58,32 +59,6 @@ pub fn resolve( ); } - // let's resolve scalar types first - - let mut boolean_expression_scalar_types = BTreeMap::new(); - - for (boolean_expression_type_name, (subgraph, boolean_expression_type)) in - &raw_boolean_expression_types - { - if let BooleanExpressionOperand::Scalar(boolean_expression_scalar_operand) = - &boolean_expression_type.operand - { - let scalar_boolean_expression_type = scalar::resolve_scalar_boolean_expression_type( - boolean_expression_type_name, - boolean_expression_scalar_operand, - &boolean_expression_type.is_null, - subgraph, - data_connectors, - &boolean_expression_type.graphql, - )?; - - boolean_expression_scalar_types.insert( - boolean_expression_type_name.clone(), - scalar_boolean_expression_type, - ); - } - } - let mut boolean_expression_object_types = BTreeMap::new(); for (boolean_expression_type_name, (subgraph, boolean_expression_type)) in @@ -99,7 +74,7 @@ pub fn resolve( subgraph, &boolean_expression_type.graphql, object_types, - &boolean_expression_scalar_types, + boolean_expression_scalar_types, &raw_boolean_expression_types, graphql_config, )?; @@ -114,7 +89,7 @@ pub fn resolve( Ok(BooleanExpressionsOutput { boolean_expression_types: BooleanExpressionTypes { objects: boolean_expression_object_types, - scalars: boolean_expression_scalar_types, + scalars: boolean_expression_scalar_types.clone(), }, graphql_types, }) 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 f18b7903cf6..9151d35f5a0 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/object.rs +++ b/v3/crates/metadata-resolve/src/stages/boolean_expressions/object.rs @@ -1,11 +1,8 @@ use super::graphql; use super::helpers; -pub use super::{ - BooleanExpressionComparableRelationship, ResolvedObjectBooleanExpressionType, - ResolvedScalarBooleanExpressionType, -}; +pub use super::{BooleanExpressionComparableRelationship, ResolvedObjectBooleanExpressionType}; use crate::helpers::ndc_validation::get_underlying_type_name; -use crate::stages::{graphql_config, object_types, type_permissions}; +use crate::stages::{graphql_config, object_types, scalar_boolean_expressions, type_permissions}; use crate::types::error::{BooleanExpressionError, Error}; use crate::types::subgraph::mk_qualified_type_name; use crate::Qualified; @@ -37,7 +34,7 @@ pub(crate) fn resolve_object_boolean_expression_type( object_types: &BTreeMap, type_permissions::ObjectTypeWithPermissions>, scalar_boolean_expression_types: &BTreeMap< Qualified, - ResolvedScalarBooleanExpressionType, + scalar_boolean_expressions::ResolvedScalarBooleanExpressionType, >, raw_boolean_expression_types: &RawBooleanExpressionTypes, graphql_config: &graphql_config::GraphqlConfig, 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 d8d697fbf28..4ba28865e7a 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/types.rs +++ b/v3/crates/metadata-resolve/src/stages/boolean_expressions/types.rs @@ -1,18 +1,21 @@ +use crate::stages::scalar_boolean_expressions; use crate::types::subgraph::{Qualified, QualifiedTypeReference}; +use lang_graphql::ast::common as ast; use open_dds::{ data_connector::{DataConnectorName, DataConnectorOperatorName}, relationships::RelationshipName, - types::{CustomTypeName, FieldName, GraphQlTypeName, OperatorName, TypeReference}, + types::{CustomTypeName, FieldName, OperatorName}, }; use serde::{Deserialize, Serialize}; use std::collections::{BTreeMap, BTreeSet}; -use lang_graphql::ast::common as ast; - #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] pub struct BooleanExpressionTypes { pub objects: BTreeMap, ResolvedObjectBooleanExpressionType>, - pub scalars: BTreeMap, ResolvedScalarBooleanExpressionType>, + pub scalars: BTreeMap< + Qualified, + scalar_boolean_expressions::ResolvedScalarBooleanExpressionType, + >, } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] @@ -31,33 +34,6 @@ pub struct ResolvedObjectBooleanExpressionType { pub include_logical_operators: IncludeLogicalOperators, } -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] -pub struct ResolvedScalarBooleanExpressionType { - pub name: Qualified, - - /// The list of comparison operators that can used on this scalar type - pub comparison_operators: BTreeMap, - - /// The list of mappings between OpenDD operator names and the names used in the data - /// connector schema - pub data_connector_operator_mappings: BTreeMap< - Qualified, - open_dds::boolean_expression::DataConnectorOperatorMapping, - >, - - // optional name for exposing this in the GraphQL schema - pub graphql_name: Option, - - // do we allow _is_null comparisons for this type? - pub include_is_null: IncludeIsNull, -} - -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] -pub enum IncludeIsNull { - Yes, - No, -} - #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] pub enum IncludeLogicalOperators { Yes, diff --git a/v3/crates/metadata-resolve/src/stages/data_connector_scalar_types/mod.rs b/v3/crates/metadata-resolve/src/stages/data_connector_scalar_types/mod.rs index 9a3be4f1446..90b4e1aae7a 100644 --- a/v3/crates/metadata-resolve/src/stages/data_connector_scalar_types/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/data_connector_scalar_types/mod.rs @@ -15,7 +15,7 @@ use crate::helpers::types::mk_name; use crate::types::error::Error; use crate::types::subgraph::Qualified; -use crate::stages::{data_connectors, scalar_types}; +use crate::stages::{data_connectors, scalar_boolean_expressions, scalar_types}; pub struct DataConnectorWithScalarsOutput<'a> { pub data_connector_scalars: @@ -24,10 +24,15 @@ pub struct DataConnectorWithScalarsOutput<'a> { } /// resolve data connector scalar representations +/// also use scalar `BooleanExpressionType`s pub fn resolve<'a>( metadata_accessor: &'a open_dds::accessor::MetadataAccessor, data_connectors: &'a data_connectors::DataConnectors, scalar_types: &'a BTreeMap, scalar_types::ScalarTypeRepresentation>, + scalar_boolean_expression_types: &BTreeMap< + Qualified, + scalar_boolean_expressions::ResolvedScalarBooleanExpressionType, + >, existing_graphql_types: &'a BTreeSet, ) -> Result, Error> { let mut graphql_types = existing_graphql_types.clone(); @@ -65,20 +70,13 @@ pub fn resolve<'a>( })?; if scalar_type.representation.is_none() { - match &scalar_type_representation.representation { - TypeName::Inbuilt(_) => {} // TODO: Validate Nullable and Array types in Inbuilt - TypeName::Custom(type_name) => { - let qualified_type_name = - Qualified::new(subgraph.to_string(), type_name.to_owned()); - let _representation = - scalar_types.get(&qualified_type_name).ok_or_else(|| { - Error::ScalarTypeUnknownRepresentation { - scalar_type: scalar_type_name.clone(), - type_name: qualified_type_name, - } - })?; - } - } + validate_type_name( + &scalar_type_representation.representation, + subgraph, + scalar_types, + scalar_type_name, + )?; + scalar_type.representation = Some(scalar_type_representation.representation.clone()); } else { return Err(Error::DuplicateDataConnectorScalarRepresentation { @@ -105,12 +103,75 @@ pub fn resolve<'a>( }; } + for scalar_boolean_expression in scalar_boolean_expression_types.values() { + for (data_connector_name, operator_mapping) in + &scalar_boolean_expression.data_connector_operator_mappings + { + let scalar_type_name = &operator_mapping.data_connector_scalar_type; + + let scalars = data_connector_scalars + .get_mut(data_connector_name) + .ok_or_else(|| Error::ScalarTypeFromUnknownDataConnector { + scalar_type: scalar_type_name.clone(), + data_connector: data_connector_name.clone(), + })?; + + let scalar_type = scalars.0.get_mut(scalar_type_name).ok_or_else(|| { + Error::UnknownScalarTypeInDataConnector { + scalar_type: scalar_type_name.clone(), + data_connector: data_connector_name.clone(), + } + })?; + + validate_type_name( + &scalar_boolean_expression.representation, + &scalar_boolean_expression.name.subgraph, + scalar_types, + scalar_type_name, + )?; + + // we may have multiple `BooleanExpressionType` for the same type, + // we allow it but check their OpenDD types don't conflict + if let Some(existing_representation) = &scalar_type.representation { + if *existing_representation != scalar_boolean_expression.representation { + return Err(Error::DataConnectorScalarRepresentationMismatch { + data_connector: data_connector_name.clone(), + old_representation: existing_representation.clone(), + new_representation: scalar_boolean_expression.representation.clone(), + }); + } + } + scalar_type.representation = Some(scalar_boolean_expression.representation.clone()); + } + } + Ok(DataConnectorWithScalarsOutput { data_connector_scalars, graphql_types, }) } +fn validate_type_name( + type_name: &TypeName, + subgraph: &str, + scalar_types: &BTreeMap, scalar_types::ScalarTypeRepresentation>, + scalar_type_name: &DataConnectorScalarType, +) -> Result<(), Error> { + match type_name { + TypeName::Inbuilt(_) => {} // TODO: Validate Nullable and Array types in Inbuilt + TypeName::Custom(type_name) => { + let qualified_type_name = Qualified::new(subgraph.to_string(), type_name.to_owned()); + let _representation = scalar_types.get(&qualified_type_name).ok_or_else(|| { + Error::ScalarTypeUnknownRepresentation { + scalar_type: scalar_type_name.clone(), + type_name: qualified_type_name, + } + })?; + } + } + Ok(()) +} + // convert from types in previous stage to this stage fn convert_data_connectors_contexts<'a>( old_data_connectors: &'a data_connectors::DataConnectors<'a>, diff --git a/v3/crates/metadata-resolve/src/stages/mod.rs b/v3/crates/metadata-resolve/src/stages/mod.rs index 4868784c96a..6ca585a1282 100644 --- a/v3/crates/metadata-resolve/src/stages/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/mod.rs @@ -14,6 +14,7 @@ pub mod object_boolean_expressions; pub mod object_types; pub mod relationships; pub mod roles; +pub mod scalar_boolean_expressions; pub mod scalar_types; pub mod type_permissions; mod types; @@ -53,6 +54,17 @@ pub fn resolve( graphql_types, } = scalar_types::resolve(&metadata_accessor, &graphql_types)?; + // Validate scalar `BooleanExpressionType`s + let scalar_boolean_expressions::ScalarBooleanExpressionsOutput { + graphql_types, + boolean_expression_scalar_types, + } = scalar_boolean_expressions::resolve( + &metadata_accessor, + configuration, + &graphql_types, + &data_connectors, + )?; + // Validate `DataConnectorScalarType` metadata. This will soon be deprecated and subsumed by // `BooleanExpressionType` let data_connector_scalar_types::DataConnectorWithScalarsOutput { @@ -62,6 +74,7 @@ pub fn resolve( &metadata_accessor, &data_connectors, &scalar_types, + &boolean_expression_scalar_types, &graphql_types, )?; @@ -69,6 +82,19 @@ pub fn resolve( let object_types_with_permissions = type_permissions::resolve(&metadata_accessor, &object_types)?; + // Resolve fancy new boolean expression types + let boolean_expressions::BooleanExpressionsOutput { + boolean_expression_types, + graphql_types, + } = boolean_expressions::resolve( + &metadata_accessor, + configuration, + &boolean_expression_scalar_types, + &graphql_types, + &graphql_config, + &object_types_with_permissions, + )?; + // Check aggregate expressions let aggregates::AggregateExpressionsOutput { aggregate_expressions, @@ -97,19 +123,6 @@ pub fn resolve( &graphql_config, )?; - // Resolve fancy new boolean expression types - let boolean_expressions::BooleanExpressionsOutput { - boolean_expression_types, - graphql_types, - } = boolean_expressions::resolve( - &metadata_accessor, - configuration, - &graphql_types, - &graphql_config, - &data_connectors, - &object_types_with_permissions, - )?; - // Resolve models and their sources let models::ModelsOutput { models, diff --git a/v3/crates/metadata-resolve/src/stages/scalar_boolean_expressions/mod.rs b/v3/crates/metadata-resolve/src/stages/scalar_boolean_expressions/mod.rs new file mode 100644 index 00000000000..e976b618238 --- /dev/null +++ b/v3/crates/metadata-resolve/src/stages/scalar_boolean_expressions/mod.rs @@ -0,0 +1,82 @@ +mod scalar; +mod types; + +use std::collections::{BTreeMap, BTreeSet}; + +use lang_graphql::ast::common as ast; +use open_dds::boolean_expression::BooleanExpressionOperand; + +use crate::stages::data_connectors; +use crate::types::configuration::Configuration; +use crate::types::error::{BooleanExpressionError, Error}; +use crate::Qualified; + +pub use types::{ + IncludeIsNull, ResolvedScalarBooleanExpressionType, ScalarBooleanExpressionsOutput, +}; + +pub fn resolve( + metadata_accessor: &open_dds::accessor::MetadataAccessor, + configuration: Configuration, + existing_graphql_types: &BTreeSet, + data_connectors: &data_connectors::DataConnectors, +) -> Result { + if !configuration + .unstable_features + .enable_boolean_expression_types + && !metadata_accessor.boolean_expression_types.is_empty() + { + return Err(Error::BooleanExpressionError { + boolean_expression_error: BooleanExpressionError::NewBooleanExpressionTypesAreDisabled, + }); + }; + + let mut raw_boolean_expression_types = BTreeMap::new(); + + // TODO: make sure we are adding new types here, we are almost certainly not doing this atm + let graphql_types = existing_graphql_types.clone(); + + // first we collect all the boolean_expression_types + // so we have a full set to refer to when resolving them + for open_dds::accessor::QualifiedObject { + subgraph, + object: boolean_expression_type, + } in &metadata_accessor.boolean_expression_types + { + raw_boolean_expression_types.insert( + Qualified::new(subgraph.to_string(), boolean_expression_type.name.clone()), + (subgraph, boolean_expression_type), + ); + } + + // let's resolve scalar types first + + let mut boolean_expression_scalar_types = BTreeMap::new(); + + for (boolean_expression_type_name, (subgraph, boolean_expression_type)) in + &raw_boolean_expression_types + { + if let BooleanExpressionOperand::Scalar(boolean_expression_scalar_operand) = + &boolean_expression_type.operand + { + let scalar_boolean_expression_type = scalar::resolve_scalar_boolean_expression_type( + boolean_expression_type_name, + boolean_expression_scalar_operand, + &boolean_expression_type.is_null, + subgraph, + data_connectors, + &boolean_expression_type.graphql, + )?; + + boolean_expression_scalar_types.insert( + boolean_expression_type_name.clone(), + scalar_boolean_expression_type, + ); + } + } + + Ok(ScalarBooleanExpressionsOutput { + boolean_expression_scalar_types, + graphql_types, + }) +} diff --git a/v3/crates/metadata-resolve/src/stages/boolean_expressions/scalar.rs b/v3/crates/metadata-resolve/src/stages/scalar_boolean_expressions/scalar.rs similarity index 89% rename from v3/crates/metadata-resolve/src/stages/boolean_expressions/scalar.rs rename to v3/crates/metadata-resolve/src/stages/scalar_boolean_expressions/scalar.rs index 18b629542cd..01ef6a9332a 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/scalar.rs +++ b/v3/crates/metadata-resolve/src/stages/scalar_boolean_expressions/scalar.rs @@ -1,5 +1,4 @@ -use super::helpers; -use super::types::ResolvedScalarBooleanExpressionType; +use super::types::{IncludeIsNull, ResolvedScalarBooleanExpressionType}; use crate::stages::data_connectors; use crate::types::error::Error; use crate::Qualified; @@ -74,8 +73,17 @@ pub(crate) fn resolve_scalar_boolean_expression_type( Ok(ResolvedScalarBooleanExpressionType { name: boolean_expression_type_name.clone(), comparison_operators: resolved_comparison_operators, + representation: scalar_boolean_expression_operand.r#type.clone(), data_connector_operator_mappings, - include_is_null: helpers::resolve_is_null(is_null), + include_is_null: resolve_is_null(is_null), graphql_name, }) } + +pub fn resolve_is_null(is_null: &BooleanExpressionIsNull) -> IncludeIsNull { + if is_null.enable { + IncludeIsNull::Yes + } else { + IncludeIsNull::No + } +} diff --git a/v3/crates/metadata-resolve/src/stages/scalar_boolean_expressions/types.rs b/v3/crates/metadata-resolve/src/stages/scalar_boolean_expressions/types.rs new file mode 100644 index 00000000000..6888463c361 --- /dev/null +++ b/v3/crates/metadata-resolve/src/stages/scalar_boolean_expressions/types.rs @@ -0,0 +1,43 @@ +use crate::types::subgraph::Qualified; +use open_dds::types::{CustomTypeName, GraphQlTypeName, OperatorName, TypeName, TypeReference}; +use serde::{Deserialize, Serialize}; +use std::collections::{BTreeMap, BTreeSet}; + +use lang_graphql::ast::common as ast; + +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] +pub struct ScalarBooleanExpressionsOutput { + pub boolean_expression_scalar_types: + BTreeMap, ResolvedScalarBooleanExpressionType>, + pub graphql_types: BTreeSet, +} + +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] +pub struct ResolvedScalarBooleanExpressionType { + pub name: Qualified, + + /// The OpenDD type this scalar refers to + pub representation: TypeName, + + /// The list of comparison operators that can used on this scalar type + pub comparison_operators: BTreeMap, + + /// The list of mappings between OpenDD operator names and the names used in the data + /// connector schema + pub data_connector_operator_mappings: BTreeMap< + Qualified, + open_dds::boolean_expression::DataConnectorOperatorMapping, + >, + + // optional name for exposing this in the GraphQL schema + pub graphql_name: Option, + + // do we allow _is_null comparisons for this type? + pub include_is_null: IncludeIsNull, +} + +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] +pub enum IncludeIsNull { + Yes, + No, +} diff --git a/v3/crates/metadata-resolve/src/types/error.rs b/v3/crates/metadata-resolve/src/types/error.rs index 0648405dbd7..f588101f36b 100644 --- a/v3/crates/metadata-resolve/src/types/error.rs +++ b/v3/crates/metadata-resolve/src/types/error.rs @@ -12,7 +12,7 @@ use open_dds::{ data_connector::{DataConnectorName, DataConnectorScalarType}, models::ModelName, relationships::RelationshipName, - types::{CustomTypeName, FieldName, OperatorName, TypeReference}, + types::{CustomTypeName, FieldName, OperatorName, TypeName, TypeReference}, }; use crate::helpers::{ @@ -452,6 +452,12 @@ pub enum Error { data_connector: Qualified, scalar_type: DataConnectorScalarType, }, + #[error("conflicting type representations found for data connector {data_connector:}: {old_representation:} and {new_representation:}")] + DataConnectorScalarRepresentationMismatch { + data_connector: Qualified, + old_representation: TypeName, + new_representation: TypeName, + }, #[error( "scalar type representation required for type {scalar_type:} in data connector {data_connector:}" )] diff --git a/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/basic/resolved.snap b/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/basic/resolved.snap index 00a73e0b415..66560919f0e 100644 --- a/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/basic/resolved.snap +++ b/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/basic/resolved.snap @@ -475,6 +475,9 @@ Metadata { ), ), }, + representation: Inbuilt( + Int, + ), comparison_operators: { OperatorName( "_in", @@ -551,6 +554,9 @@ Metadata { ), ), }, + representation: Inbuilt( + String, + ), comparison_operators: { OperatorName( "_in", diff --git a/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/nested_object/resolved.snap b/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/nested_object/resolved.snap index ec1e589b3f6..2e1765c8d0e 100644 --- a/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/nested_object/resolved.snap +++ b/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/nested_object/resolved.snap @@ -1434,6 +1434,9 @@ Metadata { ), ), }, + representation: Inbuilt( + Int, + ), comparison_operators: {}, data_connector_operator_mappings: { Qualified { @@ -1478,6 +1481,9 @@ Metadata { ), ), }, + representation: Inbuilt( + String, + ), comparison_operators: {}, data_connector_operator_mappings: { Qualified { diff --git a/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/no_graphql/resolved.snap b/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/no_graphql/resolved.snap index 543e33b770e..c437b6d6fde 100644 --- a/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/no_graphql/resolved.snap +++ b/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/no_graphql/resolved.snap @@ -242,6 +242,9 @@ Metadata { ), ), }, + representation: Inbuilt( + Int, + ), comparison_operators: { OperatorName( "_in", @@ -314,6 +317,9 @@ Metadata { ), ), }, + representation: Inbuilt( + String, + ), comparison_operators: { OperatorName( "_in", diff --git a/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/range/resolved.snap b/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/range/resolved.snap index b0a63a72382..7db246add74 100644 --- a/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/range/resolved.snap +++ b/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/range/resolved.snap @@ -746,6 +746,9 @@ Metadata { ), ), }, + representation: Inbuilt( + Int, + ), comparison_operators: {}, data_connector_operator_mappings: { Qualified { @@ -786,6 +789,9 @@ Metadata { ), ), }, + representation: Inbuilt( + Int, + ), comparison_operators: { OperatorName( "within", @@ -851,6 +857,9 @@ Metadata { ), ), }, + representation: Inbuilt( + String, + ), comparison_operators: {}, data_connector_operator_mappings: { Qualified { diff --git a/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/two_data_sources/resolved.snap b/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/two_data_sources/resolved.snap index b92a8bbc1c0..0a1fd90d03a 100644 --- a/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/two_data_sources/resolved.snap +++ b/v3/crates/metadata-resolve/tests/passing/boolean_expression_type/two_data_sources/resolved.snap @@ -242,6 +242,9 @@ Metadata { ), ), }, + representation: Inbuilt( + Int, + ), comparison_operators: { OperatorName( "_in", @@ -338,6 +341,9 @@ Metadata { ), ), }, + representation: Inbuilt( + String, + ), comparison_operators: { OperatorName( "_in",