Separate model_permissions resolve step (#526)

<!-- Thank you for submitting this PR! :) -->

## Description

Following work in hasura/v3-engine#523 and
friends, this resolves `model_permissions` in a fresh step, creating a
new `ModelWithPermissions` type to differentiate from the regular
`Model` which no longer has any notion of permissions.

Functional no-op.

V3_GIT_ORIGIN_REV_ID: fdf928ae12159a6c4ef87effc4704193c5306e46

v3/crates/engine/src/metadata/resolved.rs

@@ -18,9 +18,10 @@ pub use stages::boolean_expressions::{
 pub use stages::command_permissions::CommandWithPermissions;
 pub use stages::commands::Command;
 pub use stages::data_connector_type_mappings::{FieldMapping, TypeMapping};
+pub use stages::model_permissions::{FilterPermission, ModelPredicate, ModelWithPermissions};
 pub use stages::models::{
-    FilterPermission, Model, ModelOrderByExpression, ModelPredicate, ModelSource,
-    SelectManyGraphQlDefinition, SelectUniqueGraphQlDefinition,
+    Model, ModelOrderByExpression, ModelSource, SelectManyGraphQlDefinition,
+    SelectUniqueGraphQlDefinition,
 };
 /// we seem to be exporting functions. perhaps these would be better served as methods on the data
 /// types we export?

v3/crates/engine/src/metadata/resolved/argument.rs

@@ -5,8 +5,8 @@ use crate::metadata::resolved::model::resolve_ndc_type;
 use crate::metadata::resolved::ndc_validation;
 use crate::metadata::resolved::permission::ValueExpression;
 use crate::metadata::resolved::stages::{
-    boolean_expressions, data_connector_scalar_types, data_connector_type_mappings, models,
-    relationships, scalar_types, type_permissions,
+    boolean_expressions, data_connector_scalar_types, data_connector_type_mappings,
+    model_permissions, relationships, scalar_types, type_permissions,
 };
 use crate::metadata::resolved::subgraph::{ArgumentInfo, Qualified};
 use crate::metadata::resolved::subgraph::{QualifiedBaseType, QualifiedTypeReference};
@@ -256,7 +256,7 @@ pub(crate) fn resolve_model_predicate_with_type(
     subgraph: &str,
     data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
     fields: &IndexMap<FieldName, data_connector_type_mappings::FieldDefinition>,
-) -> Result<models::ModelPredicate, Error> {
+) -> Result<model_permissions::ModelPredicate, Error> {
     match model_predicate {
         permissions::ModelPredicate::FieldComparison(permissions::FieldComparisonPredicate {
             field,
@@ -328,7 +328,7 @@ pub(crate) fn resolve_model_predicate_with_type(
                 }),
             }?;
 
-            Ok(models::ModelPredicate::BinaryFieldComparison {
+            Ok(model_permissions::ModelPredicate::BinaryFieldComparison {
                 field: field.clone(),
                 ndc_column: field_mapping.column.clone(),
                 operator: resolved_operator,
@@ -347,7 +347,7 @@ pub(crate) fn resolve_model_predicate_with_type(
                 }
             })?;
 
-            Ok(models::ModelPredicate::UnaryFieldComparison {
+            Ok(model_permissions::ModelPredicate::UnaryFieldComparison {
                 field: field.clone(),
                 ndc_column: field_mapping.column.clone(),
                 operator: ndc_models::UnaryComparisonOperator::IsNull,
@@ -368,7 +368,9 @@ pub(crate) fn resolve_model_predicate_with_type(
                 data_connectors,
                 fields,
             )?;
-            Ok(models::ModelPredicate::Not(Box::new(resolved_predicate)))
+            Ok(model_permissions::ModelPredicate::Not(Box::new(
+                resolved_predicate,
+            )))
         }
         permissions::ModelPredicate::And(predicates) => {
             let mut resolved_predicates = Vec::new();
@@ -383,7 +385,7 @@ pub(crate) fn resolve_model_predicate_with_type(
                     fields,
                 )?);
             }
-            Ok(models::ModelPredicate::And(resolved_predicates))
+            Ok(model_permissions::ModelPredicate::And(resolved_predicates))
         }
         permissions::ModelPredicate::Or(predicates) => {
             let mut resolved_predicates = Vec::new();
@@ -398,7 +400,7 @@ pub(crate) fn resolve_model_predicate_with_type(
                     fields,
                 )?);
             }
-            Ok(models::ModelPredicate::Or(resolved_predicates))
+            Ok(model_permissions::ModelPredicate::Or(resolved_predicates))
         }
     }
 }

v3/crates/engine/src/metadata/resolved/metadata.rs

@@ -7,12 +7,11 @@ use serde::{Deserialize, Serialize};
 use open_dds::{commands::CommandName, models::ModelName, types::CustomTypeName};
 
 use crate::metadata::resolved::error::Error;
-use crate::metadata::resolved::model::resolve_model_select_permissions;
 use crate::metadata::resolved::subgraph::Qualified;
 
 use crate::metadata::resolved::stages::{
-    boolean_expressions, command_permissions, data_connector_scalar_types,
-    data_connector_type_mappings, graphql_config, models, relationships, roles, scalar_types,
+    boolean_expressions, command_permissions, graphql_config, model_permissions, relationships,
+    roles, scalar_types,
 };
 
 /// Resolved and validated metadata for a project. Used internally in the v3 server.
@@ -21,7 +20,7 @@ pub struct Metadata {
     pub object_types:
         HashMap<Qualified<CustomTypeName>, relationships::ObjectTypeWithRelationships>,
     pub scalar_types: HashMap<Qualified<CustomTypeName>, scalar_types::ScalarTypeRepresentation>,
-    pub models: IndexMap<Qualified<ModelName>, models::Model>,
+    pub models: IndexMap<Qualified<ModelName>, model_permissions::ModelWithPermissions>,
     pub commands: IndexMap<Qualified<CommandName>, command_permissions::CommandWithPermissions>,
     pub boolean_expression_types:
         HashMap<Qualified<CustomTypeName>, boolean_expressions::ObjectBooleanExpressionType>,
@@ -33,33 +32,16 @@ pub struct Metadata {
     Functions to validate and resolve OpenDD spec to internal metadata
 *******************/
 pub fn resolve_metadata(
-    metadata_accessor: &open_dds::accessor::MetadataAccessor,
     graphql_config: &graphql_config::GraphqlConfig,
-    data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
     object_types: HashMap<Qualified<CustomTypeName>, relationships::ObjectTypeWithRelationships>,
     scalar_types: &HashMap<Qualified<CustomTypeName>, scalar_types::ScalarTypeRepresentation>,
     boolean_expression_types: &HashMap<
         Qualified<CustomTypeName>,
         boolean_expressions::ObjectBooleanExpressionType,
     >,
-    data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
-    mut models: IndexMap<Qualified<ModelName>, models::Model>,
+    models: IndexMap<Qualified<ModelName>, model_permissions::ModelWithPermissions>,
     commands: IndexMap<Qualified<CommandName>, command_permissions::CommandWithPermissions>,
 ) -> Result<Metadata, Error> {
-    // resolve model permissions
-    // Note: Model permissions's predicate can include the relationship field,
-    // hence Model permissions should be resolved after the relationships of a
-    // model is resolved.
-    // TODO: make this return values rather than blindly mutating it's inputs
-    resolve_model_permissions(
-        metadata_accessor,
-        data_connectors,
-        &object_types,
-        &mut models,
-        boolean_expression_types,
-        data_connector_type_mappings,
-    )?;
-
     let roles = roles::resolve(&object_types, &models, &commands);
 
     Ok(Metadata {
@@ -72,60 +54,3 @@ pub fn resolve_metadata(
         roles,
     })
 }
-
-/// resolve model permissions
-/// this currently works by mutating `models`, let's change it to
-/// return new values instead where possible
-fn resolve_model_permissions(
-    metadata_accessor: &open_dds::accessor::MetadataAccessor,
-    data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
-    object_types: &HashMap<Qualified<CustomTypeName>, relationships::ObjectTypeWithRelationships>,
-    models: &mut IndexMap<Qualified<ModelName>, models::Model>,
-    boolean_expression_types: &HashMap<
-        Qualified<CustomTypeName>,
-        boolean_expressions::ObjectBooleanExpressionType,
-    >,
-    data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
-) -> Result<(), Error> {
-    // Note: Model permissions's predicate can include the relationship field,
-    // hence Model permissions should be resolved after the relationships of a
-    // model is resolved.
-    for open_dds::accessor::QualifiedObject {
-        subgraph,
-        object: permissions,
-    } in &metadata_accessor.model_permissions
-    {
-        let model_name = Qualified::new(subgraph.to_string(), permissions.model_name.clone());
-        let model =
-            models
-                .get(&model_name)
-                .ok_or_else(|| Error::UnknownModelInModelSelectPermissions {
-                    model_name: model_name.clone(),
-                })?;
-
-        if model.select_permissions.is_none() {
-            let select_permissions = Some(resolve_model_select_permissions(
-                model,
-                subgraph,
-                permissions,
-                data_connectors,
-                object_types,
-                models, // This is required to get the model for the relationship target
-                boolean_expression_types,
-                data_connector_type_mappings,
-            )?);
-
-            let model = models.get_mut(&model_name).ok_or_else(|| {
-                Error::UnknownModelInModelSelectPermissions {
-                    model_name: model_name.clone(),
-                }
-            })?;
-            model.select_permissions = select_permissions;
-        } else {
-            return Err(Error::DuplicateModelSelectPermission {
-                model_name: model_name.clone(),
-            });
-        }
-    }
-    Ok(())
-}