Skip to content

Commit 1717e9d

Browse files
danieljharveyhasura-bot
danieljharvey
authored and
hasura-bot
committed
Validate BooleanExpressionTypes when they are used by models (#730)
<!-- Thank you for submitting this PR! :) --> ## Description We cannot validate `BooleanExpressionType`s much until they are used. This adds checks for when they are used as model `where` clauses. We ensure the data connector in question is allowed to use nested filtering (if we try and do any), and ensure that every scalar field has mappings for the appropriate data connector. Hidden behind a feature flag, so technically a no-op. V3_GIT_ORIGIN_REV_ID: ab54f913157954e8197798febafc70012bfad9ad
1 parent eb86297 commit 1717e9d

File tree

15 files changed

+1265
-68
lines changed

15 files changed

+1265
-68
lines changed

‎v3/crates/execute/tests/generate_ir/field_argument/expected.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@
139139
"capabilities": {
140140
"supports_explaining_queries": true,
141141
"supports_explaining_mutations": false,
142+
"supports_nested_object_filtering": true,
142143
"supports_nested_object_aggregations": false
143144
}
144145
},

‎v3/crates/execute/tests/generate_ir/get_by_id/expected.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@
108108
"capabilities": {
109109
"supports_explaining_queries": true,
110110
"supports_explaining_mutations": false,
111+
"supports_nested_object_filtering": true,
111112
"supports_nested_object_aggregations": false
112113
}
113114
},

‎v3/crates/execute/tests/generate_ir/get_many/expected.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
"capabilities": {
9999
"supports_explaining_queries": true,
100100
"supports_explaining_mutations": false,
101+
"supports_nested_object_filtering": true,
101102
"supports_nested_object_aggregations": false
102103
}
103104
},

‎v3/crates/execute/tests/generate_ir/get_many_model_count/expected.json‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
"capabilities": {
4141
"supports_explaining_queries": true,
4242
"supports_explaining_mutations": false,
43+
"supports_nested_object_filtering": true,
4344
"supports_nested_object_aggregations": false
4445
}
4546
},
@@ -159,6 +160,7 @@
159160
"capabilities": {
160161
"supports_explaining_queries": true,
161162
"supports_explaining_mutations": false,
163+
"supports_nested_object_filtering": true,
162164
"supports_nested_object_aggregations": false
163165
}
164166
},
@@ -278,6 +280,7 @@
278280
"capabilities": {
279281
"supports_explaining_queries": true,
280282
"supports_explaining_mutations": false,
283+
"supports_nested_object_filtering": true,
281284
"supports_nested_object_aggregations": false
282285
}
283286
},
@@ -494,6 +497,7 @@
494497
"capabilities": {
495498
"supports_explaining_queries": true,
496499
"supports_explaining_mutations": false,
500+
"supports_nested_object_filtering": true,
497501
"supports_nested_object_aggregations": false
498502
}
499503
},
@@ -518,6 +522,7 @@
518522
"capabilities": {
519523
"supports_explaining_queries": true,
520524
"supports_explaining_mutations": false,
525+
"supports_nested_object_filtering": true,
521526
"supports_nested_object_aggregations": false
522527
}
523528
},
@@ -549,6 +554,7 @@
549554
"capabilities": {
550555
"supports_explaining_queries": true,
551556
"supports_explaining_mutations": false,
557+
"supports_nested_object_filtering": true,
552558
"supports_nested_object_aggregations": false
553559
}
554560
},
@@ -580,6 +586,7 @@
580586
"capabilities": {
581587
"supports_explaining_queries": true,
582588
"supports_explaining_mutations": false,
589+
"supports_nested_object_filtering": true,
583590
"supports_nested_object_aggregations": false
584591
}
585592
},
@@ -634,6 +641,7 @@
634641
"capabilities": {
635642
"supports_explaining_queries": true,
636643
"supports_explaining_mutations": false,
644+
"supports_nested_object_filtering": true,
637645
"supports_nested_object_aggregations": false
638646
}
639647
},
@@ -686,6 +694,7 @@
686694
"capabilities": {
687695
"supports_explaining_queries": true,
688696
"supports_explaining_mutations": false,
697+
"supports_nested_object_filtering": true,
689698
"supports_nested_object_aggregations": false
690699
}
691700
},
@@ -779,6 +788,7 @@
779788
"capabilities": {
780789
"supports_explaining_queries": true,
781790
"supports_explaining_mutations": false,
791+
"supports_nested_object_filtering": true,
782792
"supports_nested_object_aggregations": false
783793
}
784794
},
@@ -831,6 +841,7 @@
831841
"capabilities": {
832842
"supports_explaining_queries": true,
833843
"supports_explaining_mutations": false,
844+
"supports_nested_object_filtering": true,
834845
"supports_nested_object_aggregations": false
835846
}
836847
},
@@ -924,6 +935,7 @@
924935
"capabilities": {
925936
"supports_explaining_queries": true,
926937
"supports_explaining_mutations": false,
938+
"supports_nested_object_filtering": true,
927939
"supports_nested_object_aggregations": false
928940
}
929941
},
@@ -976,6 +988,7 @@
976988
"capabilities": {
977989
"supports_explaining_queries": true,
978990
"supports_explaining_mutations": false,
991+
"supports_nested_object_filtering": true,
979992
"supports_nested_object_aggregations": false
980993
}
981994
},

‎v3/crates/execute/tests/generate_ir/get_many_user_2/expected.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@
108108
"capabilities": {
109109
"supports_explaining_queries": true,
110110
"supports_explaining_mutations": false,
111+
"supports_nested_object_filtering": true,
111112
"supports_nested_object_aggregations": false
112113
}
113114
},

‎v3/crates/execute/tests/generate_ir/get_many_where/expected.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
"capabilities": {
9999
"supports_explaining_queries": true,
100100
"supports_explaining_mutations": false,
101+
"supports_nested_object_filtering": true,
101102
"supports_nested_object_aggregations": false
102103
}
103104
},
@@ -289,6 +290,7 @@
289290
"capabilities": {
290291
"supports_explaining_queries": true,
291292
"supports_explaining_mutations": false,
293+
"supports_nested_object_filtering": true,
292294
"supports_nested_object_aggregations": false
293295
}
294296
},

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

Lines changed: 2 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use open_dds::{
1717
data_connector::{DataConnectorName, DataConnectorOperatorName},
1818
types::{CustomTypeName, FieldName, OperatorName},
1919
};
20-
use std::collections::{BTreeMap, BTreeSet};
20+
use std::collections::BTreeMap;
2121

2222
/// Resolves a given object boolean expression type
2323
pub(crate) fn resolve_object_boolean_expression_type(
@@ -73,14 +73,6 @@ pub(crate) fn resolve_object_boolean_expression_type(
7373
raw_boolean_expression_types,
7474
)?;
7575

76-
// check that we have data connector information for each scalar type
77-
let _allowed_data_connectors = resolve_data_connector_types(
78-
boolean_expression_type_name,
79-
object_type_representation,
80-
scalar_boolean_expression_types,
81-
&comparable_fields,
82-
)?;
83-
8476
// resolve graphql schema information
8577
let resolved_graphql = graphql
8678
.as_ref()
@@ -106,61 +98,6 @@ pub(crate) fn resolve_object_boolean_expression_type(
10698
Ok(resolved_boolean_expression)
10799
}
108100

109-
// resolve data connector stuff
110-
// look at the object_type
111-
// for each data connector it mentions, check that every linked scalar type
112-
// has information for this data connector
113-
fn resolve_data_connector_types(
114-
boolean_expression_type_name: &Qualified<CustomTypeName>,
115-
object_type_representation: &type_permissions::ObjectTypeWithPermissions,
116-
scalar_boolean_expression_types: &BTreeMap<
117-
Qualified<CustomTypeName>,
118-
ResolvedScalarBooleanExpressionType,
119-
>,
120-
comparable_fields: &BTreeMap<FieldName, Qualified<CustomTypeName>>,
121-
) -> Result<BTreeSet<Qualified<DataConnectorName>>, Error> {
122-
let mut working_data_connectors = BTreeSet::new();
123-
124-
// for each data connector mentioned in `object_type`, check that every field is mapped
125-
// in our comparison fields
126-
// not sure what to do if this fails as we want schema to work without data connector info
127-
for data_connector_name in object_type_representation
128-
.type_mappings
129-
.data_connector_names()
130-
{
131-
for (comparable_field_name, comparable_field_type) in comparable_fields {
132-
// we don't mind if we can't find this, we've already checked validity so anything
133-
// else will be an object-flavoured boolean expression
134-
if let Some(scalar_boolean_expression) =
135-
scalar_boolean_expression_types.get(comparable_field_type)
136-
{
137-
// currently this throws an error if any data connector specified in the ObjectType
138-
// does not have matching data connector entries for each scalar boolean expression
139-
// type
140-
// Not sure if this is correct behaviour, or if we only want to make this a problem
141-
// when making these available in the GraphQL schema
142-
if !scalar_boolean_expression
143-
.data_connector_operator_mappings
144-
.contains_key(data_connector_name)
145-
{
146-
return Err(Error::BooleanExpressionError {
147-
boolean_expression_error:
148-
BooleanExpressionError::DataConnectorMappingMissingForField {
149-
field: comparable_field_name.clone(),
150-
boolean_expression_name: boolean_expression_type_name.clone(),
151-
data_connector_name: data_connector_name.clone(),
152-
},
153-
});
154-
};
155-
}
156-
}
157-
158-
working_data_connectors.insert(data_connector_name.clone());
159-
}
160-
161-
Ok(working_data_connectors)
162-
}
163-
164101
// validate graphql config
165102
// we use the raw boolean expression types for lookup
166103
fn resolve_object_boolean_graphql(
@@ -222,6 +159,7 @@ fn resolve_object_boolean_graphql(
222159
scalar_fields.insert(
223160
comparable_field_name.clone(),
224161
ComparisonExpressionInfo {
162+
object_type_name: Some(comparable_field_type_name.clone()),
225163
type_name: graphql_type_name.clone(),
226164
operators: operators.clone(),
227165
operator_mapping,

‎v3/crates/metadata-resolve/src/stages/boolean_expressions/types.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ pub struct ResolvedScalarBooleanExpressionType {
4949

5050
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
5151
pub struct ComparisonExpressionInfo {
52+
// we reuse this type for ObjectBooleanExpressionType and BooleanExpressionType
53+
// the former does not use this, hence partial
54+
// it will be good to get rid of `Option` in future
55+
pub object_type_name: Option<Qualified<CustomTypeName>>,
5256
pub type_name: ast::TypeName,
5357
pub operators: BTreeMap<OperatorName, QualifiedTypeReference>,
5458
pub operator_mapping:

‎v3/crates/metadata-resolve/src/stages/data_connectors/types.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,13 @@ impl DataConnectorLink {
279279
.mutation
280280
.explain
281281
.is_some(),
282+
supports_nested_object_filtering: info
283+
.capabilities
284+
.capabilities
285+
.query
286+
.nested_fields
287+
.filter_by
288+
.is_some(),
282289
supports_nested_object_aggregations: info
283290
.capabilities
284291
.capabilities
@@ -436,9 +443,11 @@ impl ResponseHeaders {
436443
}
437444

438445
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
446+
#[allow(clippy::struct_excessive_bools)]
439447
pub struct DataConnectorCapabilities {
440448
pub supports_explaining_queries: bool,
441449
pub supports_explaining_mutations: bool,
450+
pub supports_nested_object_filtering: bool,
442451
pub supports_nested_object_aggregations: bool,
443452
}
444453

0 commit comments

Comments
 (0)