Tighten Version Check Part 2 (#795)

introspection-engine/connectors/introspection-connector/src/error.rs

@@ -52,6 +52,12 @@ pub enum ErrorKind {
     #[error("Could not create the database. {}", explanation)]
     DatabaseCreationFailed { explanation: String },
 
+    #[error(
+        "Could not introspect the database since the schema was inconsistent. {}",
+        explanation
+    )]
+    DatabaseSchemaInconsistent { explanation: String },
+
     #[error("Authentication failed for user '{}'", user)]
     AuthenticationFailed { user: String },
 

introspection-engine/connectors/sql-introspection-connector/Cargo.toml

@@ -32,4 +32,4 @@ features = ["single"]
 barrel = { version = "0.6.5-alpha.0", features = ["sqlite3", "mysql", "pg"] }
 test-macros = { path = "../../../libs/test-macros" }
 test-setup = { path = "../../../libs/test-setup" }
-pretty_assertions = "0.6.1"
+pretty_assertions = "0.6.1"

introspection-engine/connectors/sql-introspection-connector/src/calculate_datamodel.rs

@@ -29,7 +29,7 @@ pub fn calculate_datamodel(schema: &SqlSchema, family: &SqlFamily) -> SqlIntrosp
         let mut model = Model::new(table.name.clone(), None);
 
         for column in &table.columns {
-            version_check.uses_non_prisma_type(&column.tpe);
+            version_check.check_column_for_type_and_default_value(&column);
             let field = calculate_scalar_field(&table, &column);
             model.add_field(field);
         }
@@ -45,7 +45,7 @@ pub fn calculate_datamodel(schema: &SqlSchema, family: &SqlFamily) -> SqlIntrosp
         }) {
             version_check.has_inline_relations(table);
             version_check.uses_on_delete(foreign_key, table);
-            model.add_field(calculate_relation_field(schema, table, foreign_key));
+            model.add_field(calculate_relation_field(schema, table, foreign_key)?);
         }
 
         for index in table
@@ -91,7 +91,8 @@ pub fn calculate_datamodel(schema: &SqlSchema, family: &SqlFamily) -> SqlIntrosp
                     .is_none()
                 {
                     let other_model = data_model.find_model(relation_info.to.as_str()).unwrap();
-                    let field = calculate_backrelation_field(schema, model, other_model, relation_field, relation_info);
+                    let field =
+                        calculate_backrelation_field(schema, model, other_model, relation_field, relation_info)?;
 
                     fields_to_be_added.push((other_model.name.clone(), field));
                 }
@@ -133,7 +134,7 @@ pub fn calculate_datamodel(schema: &SqlSchema, family: &SqlFamily) -> SqlIntrosp
 
     deduplicate_field_names(&mut data_model);
 
-    let version = version_check.version(&warnings);
+    let version = version_check.version(&warnings, &data_model);
 
     add_prisma_1_id_defaults(family, &version, &mut data_model, schema, &mut warnings);
 

introspection-engine/connectors/sql-introspection-connector/src/error.rs

@@ -1,7 +1,8 @@
 use introspection_connector::{ConnectorError, ErrorKind};
 use quaint::error::{Error as QuaintError, ErrorKind as QuaintKind};
 use thiserror::Error;
-use user_facing_errors::{quaint::render_quaint_error, query_engine::DatabaseConstraint};
+use user_facing_errors::introspection_engine::DatabaseSchemaInconsistent;
+use user_facing_errors::{quaint::render_quaint_error, query_engine::DatabaseConstraint, KnownError};
 
 pub type SqlResult<T> = Result<T, SqlError>;
 
@@ -65,6 +66,9 @@ pub enum SqlError {
         #[source]
         cause: QuaintKind,
     },
+
+    #[error("An Error occurred because the schema was inconsistent: '{}'", explanation)]
+    SchemaInconsistent { explanation: String },
 }
 
 impl SqlError {
@@ -126,6 +130,13 @@ impl SqlError {
                     },
                 }
             }
+            SqlError::SchemaInconsistent { explanation } => ConnectorError {
+                user_facing_error: KnownError::new(DatabaseSchemaInconsistent {
+                    explanation: explanation.to_owned(),
+                })
+                .ok(),
+                kind: ErrorKind::DatabaseSchemaInconsistent { explanation },
+            },
             error => ConnectorError::from_kind(ErrorKind::QueryError(error.into())),
         }
     }

introspection-engine/connectors/sql-introspection-connector/src/lib.rs

@@ -89,7 +89,9 @@ impl IntrospectionConnector for SqlIntrospectionConnector {
 
         let family = self.connection_info.sql_family();
 
-        let introspection_result = calculate_datamodel::calculate_datamodel(&sql_schema, &family).unwrap();
+        let introspection_result = calculate_datamodel::calculate_datamodel(&sql_schema, &family)
+            .map_err(|sql_introspection_error| sql_introspection_error.into_connector_error(&self.connection_info))?;
+
         tracing::debug!("Calculating datamodel is done: {:?}", sql_schema);
         Ok(introspection_result)
     }

introspection-engine/connectors/sql-introspection-connector/src/misc_helpers.rs

@@ -1,3 +1,4 @@
+use crate::SqlError;
 use datamodel::{
     Datamodel, DefaultValue as DMLDef, Field, FieldArity, FieldType, IndexDefinition, Model, OnDeleteStrategy,
     RelationInfo, ScalarType, ValueGenerator as VG,
@@ -160,11 +161,15 @@ pub(crate) fn calculate_scalar_field(table: &Table, column: &Column) -> Field {
     }
 }
 
-pub(crate) fn calculate_relation_field(schema: &SqlSchema, table: &Table, foreign_key: &ForeignKey) -> Field {
+pub(crate) fn calculate_relation_field(
+    schema: &SqlSchema,
+    table: &Table,
+    foreign_key: &ForeignKey,
+) -> Result<Field, SqlError> {
     debug!("Handling foreign key  {:?}", foreign_key);
 
     let field_type = FieldType::Relation(RelationInfo {
-        name: calculate_relation_name(schema, foreign_key, table),
+        name: calculate_relation_name(schema, foreign_key, table)?,
         fields: foreign_key.columns.clone(),
         to: foreign_key.referenced_table.clone(),
         to_fields: foreign_key.referenced_columns.clone(),
@@ -183,7 +188,7 @@ pub(crate) fn calculate_relation_field(schema: &SqlSchema, table: &Table, foreig
     };
 
     // todo Should this be an extra type? It uses just a small subset of the features of a scalar field
-    Field {
+    Ok(Field {
         name: foreign_key.referenced_table.clone(),
         arity,
         field_type, // todo we could remove relation out of the type and make relationinfo part of RelationField
@@ -195,7 +200,7 @@ pub(crate) fn calculate_relation_field(schema: &SqlSchema, table: &Table, foreig
         is_generated: false,
         is_updated_at: false,
         is_commented_out: false,
-    }
+    })
 }
 
 pub(crate) fn calculate_backrelation_field(
@@ -204,53 +209,58 @@ pub(crate) fn calculate_backrelation_field(
     other_model: &Model,
     relation_field: &Field,
     relation_info: &RelationInfo,
-) -> Field {
-    let table = schema.table_bang(&model.name);
-
-    let field_type = FieldType::Relation(RelationInfo {
-        name: relation_info.name.clone(),
-        to: model.name.clone(),
-        fields: vec![],
-        to_fields: vec![],
-        on_delete: OnDeleteStrategy::None,
-    });
-
-    let other_is_unique = || match &relation_info.fields.len() {
-        1 => {
-            let column_name = &relation_info.fields.first().unwrap();
-            table.is_column_unique(column_name)
-        }
-        _ => table
-            .indices
-            .iter()
-            .any(|i| columns_match(&i.columns, &relation_info.fields) && i.tpe == IndexType::Unique),
-    };
+) -> Result<Field, SqlError> {
+    match schema.table(&model.name) {
+        Err(table_name) => Err(SqlError::SchemaInconsistent {
+            explanation: format!("Table {} not found.", table_name),
+        }),
+        Ok(table) => {
+            let field_type = FieldType::Relation(RelationInfo {
+                name: relation_info.name.clone(),
+                to: model.name.clone(),
+                fields: vec![],
+                to_fields: vec![],
+                on_delete: OnDeleteStrategy::None,
+            });
+
+            let other_is_unique = || match &relation_info.fields.len() {
+                1 => {
+                    let column_name = &relation_info.fields.first().unwrap();
+                    table.is_column_unique(column_name)
+                }
+                _ => table
+                    .indices
+                    .iter()
+                    .any(|i| columns_match(&i.columns, &relation_info.fields) && i.tpe == IndexType::Unique),
+            };
 
-    let arity = match relation_field.arity {
-        FieldArity::Required | FieldArity::Optional if other_is_unique() => FieldArity::Optional,
-        FieldArity::Required | FieldArity::Optional => FieldArity::List,
-        FieldArity::List => FieldArity::Optional,
-    };
+            let arity = match relation_field.arity {
+                FieldArity::Required | FieldArity::Optional if other_is_unique() => FieldArity::Optional,
+                FieldArity::Required | FieldArity::Optional => FieldArity::List,
+                FieldArity::List => FieldArity::Optional,
+            };
 
-    //if the backrelation name would be duplicate, probably due to being a selfrelation
-    let name = if model.name == other_model.name && relation_field.name == model.name {
-        format!("other_{}", model.name.clone())
-    } else {
-        model.name.clone()
-    };
+            //if the backrelation name would be duplicate, probably due to being a selfrelation
+            let name = if model.name == other_model.name && relation_field.name == model.name {
+                format!("other_{}", model.name.clone())
+            } else {
+                model.name.clone()
+            };
 
-    Field {
-        name,
-        arity,
-        field_type,
-        database_name: None,
-        default_value: None,
-        is_unique: false,
-        is_id: false,
-        documentation: None,
-        is_generated: false,
-        is_updated_at: false,
-        is_commented_out: false,
+            Ok(Field {
+                name,
+                arity,
+                field_type,
+                database_name: None,
+                default_value: None,
+                is_unique: false,
+                is_id: false,
+                documentation: None,
+                is_generated: false,
+                is_updated_at: false,
+                is_commented_out: false,
+            })
+        }
     }
 }
 
@@ -283,7 +293,7 @@ pub(crate) fn is_sequence(column: &Column, table: &Table) -> bool {
         .unwrap_or(false)
 }
 
-pub(crate) fn calculate_relation_name(schema: &SqlSchema, fk: &ForeignKey, table: &Table) -> String {
+pub(crate) fn calculate_relation_name(schema: &SqlSchema, fk: &ForeignKey, table: &Table) -> Result<String, SqlError> {
     //this is not called for prisma many to many relations. for them the name is just the name of the join table.
     let referenced_model = &fk.referenced_table;
     let model_with_fk = &table.name;
@@ -295,26 +305,34 @@ pub(crate) fn calculate_relation_name(schema: &SqlSchema, fk: &ForeignKey, table
         .filter(|fk| fk.referenced_table == referenced_model.clone())
         .collect();
 
-    let fk_from_other_model_to_this: Vec<&ForeignKey> = schema
-        .table_bang(referenced_model)
-        .foreign_keys
-        .iter()
-        .filter(|fk| fk.referenced_table == model_with_fk.clone())
-        .collect();
+    match schema.table(referenced_model) {
+        Err(table_name) => Err(SqlError::SchemaInconsistent {
+            explanation: format!("Table {} not found.", table_name),
+        }),
+        Ok(table) => {
+            let fk_from_other_model_to_this: Vec<&ForeignKey> = table
+                .foreign_keys
+                .iter()
+                .filter(|fk| fk.referenced_table == model_with_fk.clone())
+                .collect();
+
+            //unambiguous
+            let name = if fk_to_same_model.len() < 2 && fk_from_other_model_to_this.len() == 0 {
+                if model_with_fk < referenced_model {
+                    format!("{}To{}", model_with_fk, referenced_model)
+                } else {
+                    format!("{}To{}", referenced_model, model_with_fk)
+                }
+            } else {
+                //ambiguous
+                if model_with_fk < referenced_model {
+                    format!("{}_{}To{}", model_with_fk, fk_column_name, referenced_model)
+                } else {
+                    format!("{}To{}_{}", referenced_model, model_with_fk, fk_column_name)
+                }
+            };
 
-    //unambiguous
-    if fk_to_same_model.len() < 2 && fk_from_other_model_to_this.len() == 0 {
-        if model_with_fk < referenced_model {
-            format!("{}To{}", model_with_fk, referenced_model)
-        } else {
-            format!("{}To{}", referenced_model, model_with_fk)
-        }
-    } else {
-        //ambiguous
-        if model_with_fk < referenced_model {
-            format!("{}_{}To{}", model_with_fk, fk_column_name, referenced_model)
-        } else {
-            format!("{}To{}_{}", referenced_model, model_with_fk, fk_column_name)
+            Ok(name)
         }
     }
 }