Include data_connector_type_mappings along with object_types. (#532)

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

## Description

`data_connector_type_mappings` is a map of data connector information
keyed by `CustomTypeName`. We create it when we resolve object types,
which are also keyed by `CustomTypeName`, however we pass them around
separately and it's not immediately obvious that they are related.

This PR stops passing `data_connector_type_mappings` around and instead
includes them with the object information.

Functional no-op.

V3_GIT_ORIGIN_REV_ID: 3ac1341907ea88c11fbc3639adf63140aa702262
This commit is contained in:
Daniel Harvey 2024-05-01 10:25:02 +01:00 committed by hasura-bot
parent 0323da8144
commit b9a29c3e1d
14 changed files with 78 additions and 107 deletions

View File

@ -179,7 +179,6 @@ pub(crate) fn resolve_value_expression_for_argument(
boolean_expressions::ObjectBooleanExpressionType,
>,
data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
) -> Result<ValueExpression, Error> {
match value_expression {
open_dds::permissions::ValueExpression::SessionVariable(session_variable) => {
@ -213,9 +212,9 @@ pub(crate) fn resolve_value_expression_for_argument(
// look up this type in the context of it's data connector
// so that we use the correct column names for the data source
let data_connector_field_mappings = data_connector_type_mappings
let data_connector_field_mappings = object_type_representation
.type_mappings
.get(
&boolean_expression_type.object_type,
&boolean_expression_type.data_connector_name,
&boolean_expression_type.data_connector_object_type,
)

View File

@ -50,7 +50,6 @@ pub enum TypeMappingCollectionError {
pub(crate) fn collect_type_mapping_for_source(
mapping_to_collect: &TypeMappingToCollect,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
data_connector_name: &Qualified<DataConnectorName>,
object_types: &HashMap<Qualified<CustomTypeName>, type_permissions::ObjectTypeWithPermissions>,
scalar_types: &HashMap<Qualified<CustomTypeName>, scalar_types::ScalarTypeRepresentation>,
@ -59,41 +58,38 @@ pub(crate) fn collect_type_mapping_for_source(
data_connector_type_mappings::TypeMapping,
>,
) -> Result<(), TypeMappingCollectionError> {
let type_mapping = data_connector_type_mappings
.get(
mapping_to_collect.type_name,
data_connector_name,
mapping_to_collect.ndc_object_type_name,
)
.ok_or_else(|| TypeMappingCollectionError::MappingNotDefined {
type_name: mapping_to_collect.type_name.clone(),
data_connector: data_connector_name.clone(),
ndc_type_name: mapping_to_collect.ndc_object_type_name.to_string(),
})?;
// If there is an existing mapping, make sure it maps to the same NDC object type.
if let Some(inserted_mapping) =
collected_mappings.insert(mapping_to_collect.type_name.clone(), type_mapping.clone())
{
let data_connector_type_mappings::TypeMapping::Object {
ndc_object_type_name,
..
} = inserted_mapping;
if ndc_object_type_name != mapping_to_collect.ndc_object_type_name {
return Err(
TypeMappingCollectionError::MappingToMultipleDataConnectorObjectType {
type_name: mapping_to_collect.type_name.clone(),
ndc_type_1: ndc_object_type_name,
ndc_type_2: mapping_to_collect.ndc_object_type_name.to_string(),
},
);
} else {
return Ok(());
}
}
match object_types.get(mapping_to_collect.type_name) {
Some(object_type_representation) => {
let type_mapping = object_type_representation
.type_mappings
.get(data_connector_name, mapping_to_collect.ndc_object_type_name)
.ok_or_else(|| TypeMappingCollectionError::MappingNotDefined {
type_name: mapping_to_collect.type_name.clone(),
data_connector: data_connector_name.clone(),
ndc_type_name: mapping_to_collect.ndc_object_type_name.to_string(),
})?;
// If there is an existing mapping, make sure it maps to the same NDC object type.
if let Some(inserted_mapping) = collected_mappings
.insert(mapping_to_collect.type_name.clone(), type_mapping.clone())
{
let data_connector_type_mappings::TypeMapping::Object {
ndc_object_type_name,
..
} = inserted_mapping;
if ndc_object_type_name != mapping_to_collect.ndc_object_type_name {
return Err(
TypeMappingCollectionError::MappingToMultipleDataConnectorObjectType {
type_name: mapping_to_collect.type_name.clone(),
ndc_type_1: ndc_object_type_name,
ndc_type_2: mapping_to_collect.ndc_object_type_name.to_string(),
},
);
} else {
return Ok(());
}
}
let data_connector_type_mappings::TypeMapping::Object { field_mappings, .. } =
type_mapping;
// For each field in the ObjectType, if that field is using an ObjectType in its type,
@ -122,7 +118,6 @@ pub(crate) fn collect_type_mapping_for_source(
collect_type_mapping_for_source(
&field_type_mapping_to_collect,
data_connector_type_mappings,
data_connector_name,
object_types,
scalar_types,

View File

@ -22,7 +22,6 @@ pub use types::{
pub fn resolve(
metadata_accessor: &open_dds::accessor::MetadataAccessor,
data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
object_types: &HashMap<Qualified<CustomTypeName>, type_permissions::ObjectTypeWithPermissions>,
scalar_types: &HashMap<Qualified<CustomTypeName>, scalar_types::ScalarTypeRepresentation>,
existing_graphql_types: &HashSet<ast::TypeName>,
@ -40,7 +39,6 @@ pub fn resolve(
boolean_expression_type,
subgraph,
data_connectors,
data_connector_type_mappings,
object_types,
scalar_types,
&mut graphql_types,
@ -68,7 +66,6 @@ pub(crate) fn resolve_boolean_expression_type(
object_boolean_expression: &open_dds::types::ObjectBooleanExpressionTypeV1,
subgraph: &str,
data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
object_types: &HashMap<Qualified<CustomTypeName>, type_permissions::ObjectTypeWithPermissions>,
scalar_types: &HashMap<Qualified<CustomTypeName>, scalar_types::ScalarTypeRepresentation>,
existing_graphql_types: &mut HashSet<ast::TypeName>,
@ -94,6 +91,7 @@ pub(crate) fn resolve_boolean_expression_type(
},
)
})?;
let qualified_data_connector_name = Qualified::new(
subgraph.to_string(),
object_boolean_expression.data_connector_name.to_owned(),
@ -130,9 +128,8 @@ pub(crate) fn resolve_boolean_expression_type(
));
}
data_connector_type_mappings
object_type_representation.type_mappings
.get(
&qualified_object_type_name,
&qualified_data_connector_name,
&object_boolean_expression.data_connector_object_type,
)
@ -207,7 +204,6 @@ pub(crate) fn resolve_boolean_expression_type(
};
type_mappings::collect_type_mapping_for_source(
&type_mapping_to_collect,
data_connector_type_mappings,
&qualified_data_connector_name,
object_types,
scalar_types,

View File

@ -7,8 +7,7 @@ use indexmap::IndexMap;
use open_dds::{commands::CommandName, types::CustomTypeName};
use crate::metadata::resolved::stages::{
boolean_expressions, commands, data_connector_scalar_types, data_connector_type_mappings,
relationships,
boolean_expressions, commands, data_connector_scalar_types, relationships,
};
use crate::metadata::resolved::types::error::Error;
use crate::metadata::resolved::types::permission::ValueExpression;
@ -45,7 +44,6 @@ pub fn resolve(
boolean_expressions::ObjectBooleanExpressionType,
>,
data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
) -> Result<IndexMap<Qualified<CommandName>, CommandWithPermissions>, Error> {
let mut commands_with_permissions: IndexMap<Qualified<CommandName>, CommandWithPermissions> =
commands
@ -79,7 +77,6 @@ pub fn resolve(
object_types,
boolean_expression_types,
data_connectors,
data_connector_type_mappings,
subgraph,
)?;
} else {
@ -100,7 +97,6 @@ pub fn resolve_command_permissions(
boolean_expressions::ObjectBooleanExpressionType,
>,
data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
subgraph: &str,
) -> Result<HashMap<Role, CommandPermission>, Error> {
let mut validated_permissions = HashMap::new();
@ -125,7 +121,6 @@ pub fn resolve_command_permissions(
object_types,
boolean_expression_types,
data_connectors,
data_connector_type_mappings,
)?;
// additionally typecheck literals

View File

@ -6,8 +6,8 @@ use crate::metadata::resolved::helpers::types::{
get_type_representation, mk_name, object_type_exists, unwrap_custom_type_name,
};
use crate::metadata::resolved::stages::{
boolean_expressions, data_connector_scalar_types, data_connector_type_mappings,
data_connectors, scalar_types, type_permissions,
boolean_expressions, data_connector_scalar_types, data_connectors, scalar_types,
type_permissions,
};
use crate::metadata::resolved::types::error::Error;
use crate::metadata::resolved::types::subgraph::{
@ -28,7 +28,6 @@ use crate::metadata::resolved::helpers::type_mappings;
pub fn resolve(
metadata_accessor: &open_dds::accessor::MetadataAccessor,
data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
object_types: &HashMap<Qualified<CustomTypeName>, type_permissions::ObjectTypeWithPermissions>,
scalar_types: &HashMap<Qualified<CustomTypeName>, scalar_types::ScalarTypeRepresentation>,
boolean_expression_types: &HashMap<
@ -55,7 +54,6 @@ pub fn resolve(
&mut resolved_command,
subgraph,
data_connectors,
data_connector_type_mappings,
object_types,
scalar_types,
boolean_expression_types,
@ -187,7 +185,6 @@ pub fn resolve_command_source(
command: &mut Command,
subgraph: &str,
data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
object_types: &HashMap<Qualified<CustomTypeName>, type_permissions::ObjectTypeWithPermissions>,
scalar_types: &HashMap<Qualified<CustomTypeName>, scalar_types::ScalarTypeRepresentation>,
boolean_expression_types: &HashMap<
@ -312,7 +309,6 @@ pub fn resolve_command_source(
{
type_mappings::collect_type_mapping_for_source(
type_mapping_to_collect,
data_connector_type_mappings,
&qualified_data_connector_name,
object_types,
scalar_types,

View File

@ -2,9 +2,9 @@ use std::collections::{BTreeMap, HashMap, HashSet};
pub mod types;
use open_dds::types::CustomTypeName;
pub use types::{
DataConnectorTypeMappings, DataConnectorTypeMappingsOutput, FieldDefinition, FieldMapping,
ObjectTypeRepresentation, ResolvedApolloFederationObjectKey,
ResolvedObjectApolloFederationConfig, TypeMapping,
DataConnectorTypeMappingsForObject, DataConnectorTypeMappingsOutput, FieldDefinition,
FieldMapping, ObjectTypeRepresentation, ObjectTypeWithTypeMappings,
ResolvedApolloFederationObjectKey, ResolvedObjectApolloFederationConfig, TypeMapping,
};
use crate::metadata::resolved::helpers::types::{mk_name, store_new_graphql_type};
@ -22,7 +22,6 @@ pub(crate) fn resolve(
metadata_accessor: &open_dds::accessor::MetadataAccessor,
data_connectors: &data_connectors::DataConnectors,
) -> Result<DataConnectorTypeMappingsOutput, Error> {
let mut data_connector_type_mappings = DataConnectorTypeMappings::new();
let mut object_types = HashMap::new();
let mut graphql_types = HashSet::new();
let mut global_id_enabled_types = HashMap::new();
@ -45,6 +44,8 @@ pub(crate) fn resolve(
&mut apollo_federation_entity_enabled_types,
)?;
let mut type_mappings = DataConnectorTypeMappingsForObject::new();
// resolve object types' type mappings
for dc_type_mapping in &object_type_definition.data_connector_type_mapping {
let qualified_data_connector_name = Qualified::new(
@ -64,16 +65,23 @@ pub(crate) fn resolve(
error: type_validation_error,
}
})?;
data_connector_type_mappings.insert(
&qualified_object_type_name,
type_mappings.insert(
&qualified_data_connector_name,
&dc_type_mapping.data_connector_object_type,
type_mapping,
)?;
}
let object_type_with_type_mappings = ObjectTypeWithTypeMappings {
object_type: resolved_object_type,
type_mappings,
};
if object_types
.insert(qualified_object_type_name.clone(), resolved_object_type)
.insert(
qualified_object_type_name.clone(),
object_type_with_type_mappings,
)
.is_some()
{
return Err(Error::DuplicateTypeDefinition {
@ -83,7 +91,6 @@ pub(crate) fn resolve(
}
Ok(DataConnectorTypeMappingsOutput {
data_connector_type_mappings,
object_types,
graphql_types,
global_id_enabled_types,

View File

@ -13,61 +13,48 @@ use crate::metadata::resolved::types::subgraph::Qualified;
use lang_graphql::ast::common as ast;
use open_dds::data_connector::DataConnectorName;
pub type DataConnectorTypeMappingsForObjectType =
HashMap<Qualified<DataConnectorName>, HashMap<String, TypeMapping>>;
#[derive(Debug)]
pub struct DataConnectorTypeMappings(
HashMap<Qualified<CustomTypeName>, DataConnectorTypeMappingsForObjectType>,
#[derive(Serialize, Deserialize, Debug, Clone, Eq, PartialEq)]
pub struct DataConnectorTypeMappingsForObject(
HashMap<Qualified<DataConnectorName>, HashMap<String, TypeMapping>>,
);
impl Default for DataConnectorTypeMappings {
impl Default for DataConnectorTypeMappingsForObject {
fn default() -> Self {
Self::new()
}
}
impl DataConnectorTypeMappings {
impl DataConnectorTypeMappingsForObject {
pub fn new() -> Self {
Self(HashMap::new())
}
pub fn get(
&self,
object_type_name: &Qualified<CustomTypeName>,
data_connector_name: &Qualified<DataConnectorName>,
data_connector_object_type: &str,
) -> Option<&TypeMapping> {
self.0
.get(object_type_name)
.and_then(|connectors| {
connectors
.get(data_connector_name)
.map(|data_connector_object_types| {
data_connector_object_types.get(data_connector_object_type)
})
.get(data_connector_name)
.and_then(|data_connector_object_types| {
data_connector_object_types.get(data_connector_object_type)
})
.flatten()
}
pub fn insert(
&mut self,
object_type_name: &Qualified<CustomTypeName>,
data_connector_name: &Qualified<DataConnectorName>,
data_connector_object_type: &str,
type_mapping: TypeMapping,
) -> Result<(), Error> {
if self
.0
.entry(object_type_name.clone())
.or_default()
.entry(data_connector_name.clone())
.or_default()
.insert(data_connector_object_type.to_string(), type_mapping)
.is_some()
{
return Err(Error::DuplicateDataConnectorTypeMapping {
type_name: object_type_name.clone(),
return Err(Error::DuplicateDataConnectorObjectTypeMapping {
data_connector: data_connector_name.clone(),
data_connector_object_type: data_connector_object_type.to_string(),
});
@ -82,8 +69,7 @@ pub struct DataConnectorTypeMappingsOutput {
pub global_id_enabled_types: HashMap<Qualified<CustomTypeName>, Vec<Qualified<ModelName>>>,
pub apollo_federation_entity_enabled_types:
HashMap<Qualified<CustomTypeName>, Option<Qualified<open_dds::models::ModelName>>>,
pub data_connector_type_mappings: DataConnectorTypeMappings,
pub object_types: HashMap<Qualified<CustomTypeName>, ObjectTypeRepresentation>,
pub object_types: HashMap<Qualified<CustomTypeName>, ObjectTypeWithTypeMappings>,
}
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, derive_more::Display)]
@ -98,6 +84,11 @@ pub struct ObjectTypeRepresentation {
// TODO: add graphql_output_type_kind if we support creating interfaces.
}
pub struct ObjectTypeWithTypeMappings {
pub object_type: ObjectTypeRepresentation,
pub type_mappings: DataConnectorTypeMappingsForObject,
}
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
pub struct FieldDefinition {
pub field_type: QualifiedTypeReference,

View File

@ -32,7 +32,6 @@ pub fn resolve(metadata: open_dds::Metadata) -> Result<Metadata, Error> {
let data_connectors = data_connectors::resolve(&metadata_accessor)?;
let data_connector_type_mappings::DataConnectorTypeMappingsOutput {
data_connector_type_mappings,
graphql_types,
global_id_enabled_types,
apollo_federation_entity_enabled_types,
@ -63,7 +62,6 @@ pub fn resolve(metadata: open_dds::Metadata) -> Result<Metadata, Error> {
} = boolean_expressions::resolve(
&metadata_accessor,
&data_connectors,
&data_connector_type_mappings,
&object_types_with_permissions,
&scalar_types,
&graphql_types,
@ -78,7 +76,6 @@ pub fn resolve(metadata: open_dds::Metadata) -> Result<Metadata, Error> {
} = models::resolve(
&metadata_accessor,
&data_connectors,
&data_connector_type_mappings,
&graphql_types,
&global_id_enabled_types,
&apollo_federation_entity_enabled_types,
@ -91,7 +88,6 @@ pub fn resolve(metadata: open_dds::Metadata) -> Result<Metadata, Error> {
let commands = commands::resolve(
&metadata_accessor,
&data_connectors,
&data_connector_type_mappings,
&object_types_with_permissions,
&scalar_types,
&boolean_expression_types,
@ -116,7 +112,6 @@ pub fn resolve(metadata: open_dds::Metadata) -> Result<Metadata, Error> {
&object_types_with_relationships,
&boolean_expression_types,
&data_connectors,
&data_connector_type_mappings,
)?;
let models_with_permissions = model_permissions::resolve(
@ -125,7 +120,6 @@ pub fn resolve(metadata: open_dds::Metadata) -> Result<Metadata, Error> {
&object_types_with_relationships,
&models,
&boolean_expression_types,
&data_connector_type_mappings,
)?;
let roles = roles::resolve(

View File

@ -39,7 +39,6 @@ pub fn resolve(
Qualified<CustomTypeName>,
boolean_expressions::ObjectBooleanExpressionType,
>,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
) -> Result<IndexMap<Qualified<ModelName>, ModelWithPermissions>, Error> {
let mut models_with_permissions: IndexMap<Qualified<ModelName>, ModelWithPermissions> = models
.iter()
@ -78,7 +77,6 @@ pub fn resolve(
object_types,
models, // This is required to get the model for the relationship target
boolean_expression_types,
data_connector_type_mappings,
)?);
model.select_permissions = select_permissions;
@ -506,7 +504,6 @@ pub fn resolve_model_select_permissions(
Qualified<CustomTypeName>,
boolean_expressions::ObjectBooleanExpressionType,
>,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
) -> Result<HashMap<Role, SelectPermission>, Error> {
let mut validated_permissions = HashMap::new();
for model_permission in &model_permissions.permissions {
@ -545,7 +542,6 @@ pub fn resolve_model_select_permissions(
object_types,
boolean_expression_types,
data_connectors,
data_connector_type_mappings,
)?;
// additionally typecheck literals

View File

@ -38,7 +38,6 @@ use std::iter;
pub fn resolve(
metadata_accessor: &open_dds::accessor::MetadataAccessor,
data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
existing_graphql_types: &HashSet<ast::TypeName>,
global_id_enabled_types: &HashMap<Qualified<CustomTypeName>, Vec<Qualified<ModelName>>>,
apollo_federation_entity_enabled_types: &HashMap<
@ -98,7 +97,6 @@ pub fn resolve(
data_connectors,
object_types,
scalar_types,
data_connector_type_mappings,
boolean_expression_types,
)?;
}
@ -574,7 +572,6 @@ fn resolve_model_source(
data_connectors: &data_connector_scalar_types::DataConnectorsWithScalars,
object_types: &HashMap<Qualified<CustomTypeName>, type_permissions::ObjectTypeWithPermissions>,
scalar_types: &HashMap<Qualified<CustomTypeName>, scalar_types::ScalarTypeRepresentation>,
data_connector_type_mappings: &data_connector_type_mappings::DataConnectorTypeMappings,
boolean_expression_types: &HashMap<
Qualified<CustomTypeName>,
boolean_expressions::ObjectBooleanExpressionType,
@ -636,7 +633,6 @@ fn resolve_model_source(
{
type_mappings::collect_type_mapping_for_source(
type_mapping_to_collect,
data_connector_type_mappings,
&qualified_data_connector_name,
object_types,
scalar_types,

View File

@ -43,6 +43,7 @@ pub fn resolve(
type_output_permissions,
type_input_permissions,
object_type,
type_mappings,
},
) in object_types_with_permissions
{
@ -53,6 +54,7 @@ pub fn resolve(
type_output_permissions: type_output_permissions.clone(),
type_input_permissions: type_input_permissions.clone(),
relationships: IndexMap::new(),
type_mappings: type_mappings.clone(),
},
);
}

View File

@ -25,6 +25,8 @@ pub struct ObjectTypeWithRelationships {
pub type_input_permissions: HashMap<Role, type_permissions::TypeInputPermission>,
/// any relationships defined on this object
pub relationships: IndexMap<ast::Name, Relationship>,
/// type mappings for each data connector
pub type_mappings: data_connector_type_mappings::DataConnectorTypeMappingsForObject,
}
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]

View File

@ -30,6 +30,8 @@ pub struct ObjectTypeWithPermissions {
/// permissions on this type, when it is used in an input context (e.g. in
/// an argument type of Model or Command)
pub type_input_permissions: HashMap<Role, TypeInputPermission>,
/// type mappings for each data connector
pub type_mappings: data_connector_type_mappings::DataConnectorTypeMappingsForObject,
}
/// resolve type permissions
@ -37,7 +39,7 @@ pub fn resolve(
metadata_accessor: &open_dds::accessor::MetadataAccessor,
object_types: &HashMap<
Qualified<CustomTypeName>,
data_connector_type_mappings::ObjectTypeRepresentation,
data_connector_type_mappings::ObjectTypeWithTypeMappings,
>,
) -> Result<HashMap<Qualified<CustomTypeName>, ObjectTypeWithPermissions>, Error> {
let mut object_types_with_permissions = HashMap::new();
@ -45,7 +47,8 @@ pub fn resolve(
object_types_with_permissions.insert(
object_type_name.clone(),
ObjectTypeWithPermissions {
object_type: object_type.clone(),
object_type: object_type.object_type.clone(),
type_mappings: object_type.type_mappings.clone(),
type_input_permissions: HashMap::new(),
type_output_permissions: HashMap::new(),
},

View File

@ -33,9 +33,8 @@ pub enum Error {
type_name: Qualified<CustomTypeName>,
error: TypeMappingValidationError,
},
#[error("Multiple mappings have been defined from type {type_name:} to object {data_connector_object_type:} of data connector {data_connector:}")]
DuplicateDataConnectorTypeMapping {
type_name: Qualified<CustomTypeName>,
#[error("Multiple mappings have been defined from object {data_connector_object_type:} of data connector {data_connector:}")]
DuplicateDataConnectorObjectTypeMapping {
data_connector: Qualified<DataConnectorName>,
data_connector_object_type: String,
},