From 020a4d07aab66854d2c4fda7ae2b777e35f4cba9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=20Houl=C3=A9?= Date: Tue, 6 Dec 2022 13:10:11 +0100 Subject: [PATCH] me: pass namespaces to diff in devDiagnostic->diagnoseMigrationHistory The namespaces are from the schema the engine was started with This should address https://github.com/prisma/prisma/issues/16634 and https://github.com/prisma/prisma/issues/16585 --- libs/test-setup/src/lib.rs | 8 +- libs/test-setup/src/postgres.rs | 2 +- libs/test-setup/src/test_api_args.rs | 2 +- .../core/src/commands/dev_diagnostic.rs | 5 +- .../commands/diagnose_migration_history.rs | 7 +- migration-engine/core/src/state.rs | 42 ++++++----- .../src/commands/dev_diagnostic.rs | 1 + .../commands/diagnose_migration_history.rs | 1 + .../tests/migrations/dev_diagnostic_tests.rs | 74 +++++++++++++++++++ 9 files changed, 117 insertions(+), 25 deletions(-) diff --git a/libs/test-setup/src/lib.rs b/libs/test-setup/src/lib.rs index 1836da9a4711..a4261c38e317 100644 --- a/libs/test-setup/src/lib.rs +++ b/libs/test-setup/src/lib.rs @@ -5,6 +5,7 @@ //! engines. pub mod mysql; +pub mod postgres; /// Tokio test runtime utils. pub mod runtime; @@ -12,7 +13,6 @@ mod capabilities; mod diff; mod logging; mod mssql; -mod postgres; mod sqlite; mod tags; mod test_api_args; @@ -48,6 +48,12 @@ macro_rules! only { pub struct TestDb(&'static test_api_args::DbUnderTest); +impl TestDb { + pub fn url(&self) -> &'static str { + &self.0.database_url + } +} + #[doc(hidden)] #[inline(never)] pub fn only_impl(include_tagged: BitFlags, exclude_tags: BitFlags) -> (bool, TestDb) { diff --git a/libs/test-setup/src/postgres.rs b/libs/test-setup/src/postgres.rs index 3f14749829bf..ff86d11c59b4 100644 --- a/libs/test-setup/src/postgres.rs +++ b/libs/test-setup/src/postgres.rs @@ -44,7 +44,7 @@ pub(crate) fn get_postgres_tags(database_url: &str) -> Result, St tok(fut) } -pub(crate) async fn create_postgres_database(database_url: &str, db_name: &str) -> Result<(Quaint, String), AnyError> { +pub async fn create_postgres_database(database_url: &str, db_name: &str) -> Result<(Quaint, String), AnyError> { let mut url: Url = database_url.parse()?; let mut postgres_db_url = url.clone(); diff --git a/libs/test-setup/src/test_api_args.rs b/libs/test-setup/src/test_api_args.rs index 1070397baa93..3e83e3b5c7ff 100644 --- a/libs/test-setup/src/test_api_args.rs +++ b/libs/test-setup/src/test_api_args.rs @@ -7,7 +7,7 @@ use std::{fmt::Display, io::Write as _}; #[derive(Debug)] pub(crate) struct DbUnderTest { pub(crate) capabilities: BitFlags, - database_url: String, + pub(crate) database_url: String, shadow_database_url: Option, provider: &'static str, pub(crate) tags: BitFlags, diff --git a/migration-engine/core/src/commands/dev_diagnostic.rs b/migration-engine/core/src/commands/dev_diagnostic.rs index 0c69c8812319..60dfffa0c56e 100644 --- a/migration-engine/core/src/commands/dev_diagnostic.rs +++ b/migration-engine/core/src/commands/dev_diagnostic.rs @@ -3,12 +3,13 @@ use super::{ HistoryDiagnostic, }; use crate::json_rpc::types::{DevAction, DevActionReset, DevDiagnosticInput, DevDiagnosticOutput}; -use migration_connector::{migrations_directory, ConnectorResult, MigrationConnector}; +use migration_connector::{migrations_directory, ConnectorResult, MigrationConnector, Namespaces}; /// Method called at the beginning of `migrate dev` to decide the course of /// action based on the current state of the workspace. pub async fn dev_diagnostic( input: DevDiagnosticInput, + namespaces: Option, connector: &mut dyn MigrationConnector, ) -> ConnectorResult { migrations_directory::error_on_changed_provider(&input.migrations_directory_path, connector.connector_type())?; @@ -18,7 +19,7 @@ pub async fn dev_diagnostic( opt_in_to_shadow_database: true, }; - let diagnose_migration_history_output = diagnose_migration_history(diagnose_input, connector).await?; + let diagnose_migration_history_output = diagnose_migration_history(diagnose_input, namespaces, connector).await?; check_for_broken_migrations(&diagnose_migration_history_output)?; diff --git a/migration-engine/core/src/commands/diagnose_migration_history.rs b/migration-engine/core/src/commands/diagnose_migration_history.rs index 20e121d73fa8..415fea16a2bf 100644 --- a/migration-engine/core/src/commands/diagnose_migration_history.rs +++ b/migration-engine/core/src/commands/diagnose_migration_history.rs @@ -1,6 +1,6 @@ use crate::CoreResult; use migration_connector::{ - migrations_directory::*, ConnectorError, DiffTarget, MigrationConnector, MigrationRecord, + migrations_directory::*, ConnectorError, DiffTarget, MigrationConnector, MigrationRecord, Namespaces, PersistenceNotInitializedError, }; use serde::{Deserialize, Serialize}; @@ -65,6 +65,7 @@ impl DiagnoseMigrationHistoryOutput { /// reads, it does not write to the dev database nor the migrations directory. pub async fn diagnose_migration_history( input: DiagnoseMigrationHistoryInput, + namespaces: Option, connector: &mut dyn MigrationConnector, ) -> CoreResult { tracing::debug!("Diagnosing migration history"); @@ -132,10 +133,10 @@ pub async fn diagnose_migration_history( // TODO(MultiSchema): this should probably fill the following namespaces from the CLI since there is // no schema to grab the namespaces off, in the case of MultiSchema. let from = connector - .database_schema_from_diff_target(DiffTarget::Migrations(&applied_migrations), None, None) + .database_schema_from_diff_target(DiffTarget::Migrations(&applied_migrations), None, namespaces.clone()) .await; let to = connector - .database_schema_from_diff_target(DiffTarget::Database, None, None) + .database_schema_from_diff_target(DiffTarget::Database, None, namespaces) .await; let drift = match from.and_then(|from| to.map(|to| connector.diff(from, to))).map(|mig| { if connector.migration_is_empty(&mig) { diff --git a/migration-engine/core/src/state.rs b/migration-engine/core/src/state.rs index df59a300b3a4..2b9a6db01d02 100644 --- a/migration-engine/core/src/state.rs +++ b/migration-engine/core/src/state.rs @@ -52,6 +52,16 @@ impl EngineState { } } + fn namespaces(&self) -> Option { + self.initial_datamodel + .as_ref() + .and_then(|schema| schema.configuration.datasources.first()) + .and_then(|ds| { + let mut names = ds.namespaces.iter().map(|(ns, _)| ns.to_owned()).collect(); + Namespaces::from_vec(&mut names) + }) + } + async fn with_connector_from_schema_path( &self, path: &str, @@ -247,8 +257,13 @@ impl GenericApi for EngineState { } async fn dev_diagnostic(&self, input: DevDiagnosticInput) -> CoreResult { - self.with_default_connector(Box::new(|connector| { - Box::pin(commands::dev_diagnostic(input, connector).instrument(tracing::info_span!("DevDiagnostic"))) + let namespaces = self.namespaces(); + self.with_default_connector(Box::new(move |connector| { + Box::pin(async move { + commands::dev_diagnostic(input, namespaces, connector) + .instrument(tracing::info_span!("DevDiagnostic")) + .await + }) })) .await } @@ -266,11 +281,13 @@ impl GenericApi for EngineState { &self, input: commands::DiagnoseMigrationHistoryInput, ) -> CoreResult { - self.with_default_connector(Box::new(|connector| { - Box::pin( - commands::diagnose_migration_history(input, connector) - .instrument(tracing::info_span!("DiagnoseMigrationHistory")), - ) + let namespaces = self.namespaces(); + self.with_default_connector(Box::new(move |connector| { + Box::pin(async move { + commands::diagnose_migration_history(input, namespaces, connector) + .instrument(tracing::info_span!("DiagnoseMigrationHistory")) + .await + }) })) .await } @@ -368,16 +385,7 @@ impl GenericApi for EngineState { async fn reset(&self) -> CoreResult<()> { tracing::debug!("Resetting the database."); - - let namespaces: Option = self - .initial_datamodel - .as_ref() - .and_then(|schema| schema.configuration.datasources.first()) - .and_then(|ds| { - let mut names = ds.namespaces.iter().map(|(ns, _)| ns.to_owned()).collect(); - Namespaces::from_vec(&mut names) - }); - + let namespaces = self.namespaces(); self.with_default_connector(Box::new(move |connector| { Box::pin(MigrationConnector::reset(connector, false, namespaces).instrument(tracing::info_span!("Reset"))) })) diff --git a/migration-engine/migration-engine-tests/src/commands/dev_diagnostic.rs b/migration-engine/migration-engine-tests/src/commands/dev_diagnostic.rs index 34550137a529..cd83a7b41ae5 100644 --- a/migration-engine/migration-engine-tests/src/commands/dev_diagnostic.rs +++ b/migration-engine/migration-engine-tests/src/commands/dev_diagnostic.rs @@ -22,6 +22,7 @@ impl<'a> DevDiagnostic<'a> { DevDiagnosticInput { migrations_directory_path: self.migrations_directory.path().to_str().unwrap().to_owned(), }, + None, self.api, ); let output = test_setup::runtime::run_with_thread_local_runtime(fut)?; diff --git a/migration-engine/migration-engine-tests/src/commands/diagnose_migration_history.rs b/migration-engine/migration-engine-tests/src/commands/diagnose_migration_history.rs index 9e9e20e6ae24..c74b239ddb6a 100644 --- a/migration-engine/migration-engine-tests/src/commands/diagnose_migration_history.rs +++ b/migration-engine/migration-engine-tests/src/commands/diagnose_migration_history.rs @@ -34,6 +34,7 @@ impl<'a> DiagnoseMigrationHistory<'a> { migrations_directory_path: self.migrations_directory.path().to_str().unwrap().to_owned(), opt_in_to_shadow_database: self.opt_in_to_shadow_database, }, + None, self.api, ) .await?; diff --git a/migration-engine/migration-engine-tests/tests/migrations/dev_diagnostic_tests.rs b/migration-engine/migration-engine-tests/tests/migrations/dev_diagnostic_tests.rs index 7d640a0ce821..79f91ed81f00 100644 --- a/migration-engine/migration-engine-tests/tests/migrations/dev_diagnostic_tests.rs +++ b/migration-engine/migration-engine-tests/tests/migrations/dev_diagnostic_tests.rs @@ -768,3 +768,77 @@ fn dev_diagnostic_shadow_database_creation_error_is_special_cased_postgres(api: // // } + +#[test] +fn dev_diagnostic_multi_schema_does_not_panic() { + let db = test_setup::only!(Postgres); + let (_, url) = tok(test_setup::postgres::create_postgres_database( + db.url(), + "dev_diagnostic_multi_schema", + )) + .unwrap(); + + let schema = format! {r#" + datasource db {{ + provider = "postgresql" + url = "{url}" + schemas = ["prisma-tests", "auth"] + }} + + generator js {{ + provider = "prisma-client-js" + previewFeatures = ["multiSchema"] + }} + + model users {{ + id String @id @db.Uuid + profiles profiles? + + @@schema("auth") + }} + + model profiles {{ + id String @id @db.Uuid + users users @relation(fields: [id], references: [id], onDelete: NoAction, onUpdate: NoAction) + + @@schema("prisma-tests") + }} + "#}; + + let setup = r#" +-- ./sql/ddl.sql + +CREATE SCHEMA auth; + +-- auth.users definition +CREATE TABLE auth.users ( + id uuid NOT NULL, + CONSTRAINT users_pkey PRIMARY KEY (id) +); + +-- "prisma-tests".profiles definition +CREATE TABLE "prisma-tests".profiles ( + id uuid NOT NULL, + CONSTRAINT profiles_pkey PRIMARY KEY (id) +); + +-- "prisma-tests".profiles foreign keys +ALTER TABLE "prisma-tests".profiles ADD CONSTRAINT profiles_id_fkey FOREIGN KEY (id) REFERENCES auth.users(id); + "#; + + let tempdir = tempfile::tempdir().unwrap(); + std::fs::write(tempdir.path().join("schema.prisma"), &schema).unwrap(); + + let api = migration_core::migration_api(Some(schema), None).unwrap(); + + tok(api.db_execute(DbExecuteParams { + datasource_type: DbExecuteDatasourceType::Url(UrlContainer { url }), + script: setup.to_owned(), + })) + .unwrap(); + + tok(api.dev_diagnostic(DevDiagnosticInput { + migrations_directory_path: tempdir.path().join("migrations").to_string_lossy().into_owned(), + })) + .unwrap(); +}