Skip to content

Commit

Permalink
me: Fix schema handling in migration persistence
Browse files Browse the repository at this point in the history
  • Loading branch information
Julius de Bruijn committed Dec 22, 2022
1 parent c8f7657 commit a11d4ff
Show file tree
Hide file tree
Showing 13 changed files with 93 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{checksum, BoxFuture, ConnectorError, ConnectorResult};
use crate::{checksum, BoxFuture, ConnectorError, ConnectorResult, Namespaces};

/// A timestamp.
pub type Timestamp = chrono::DateTime<chrono::Utc>;
Expand All @@ -14,7 +14,7 @@ pub trait MigrationPersistence: Send + Sync {
/// If the migration persistence is not present in the target database,
/// check whether the database schema is empty. If it is, initialize the
/// migration persistence. If not, return a DatabaseSchemaNotEmpty error.
fn initialize(&mut self) -> BoxFuture<'_, ConnectorResult<()>>;
fn initialize(&mut self, namespaces: Option<Namespaces>) -> BoxFuture<'_, ConnectorResult<()>>;

/// Implementation in the connector for the core's MarkMigrationApplied
/// command. See the docs there. Note that the started_at and finished_at
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::MongoDbMigrationConnector;
use migration_connector::{BoxFuture, ConnectorResult, MigrationPersistence};
use migration_connector::{BoxFuture, ConnectorResult, MigrationPersistence, Namespaces};

impl MigrationPersistence for MongoDbMigrationConnector {
fn baseline_initialize(&mut self) -> migration_connector::BoxFuture<'_, ConnectorResult<()>> {
unsupported_command_error()
}

fn initialize(&mut self) -> BoxFuture<'_, ConnectorResult<()>> {
fn initialize(&mut self, _namespaces: Option<Namespaces>) -> BoxFuture<'_, ConnectorResult<()>> {
unsupported_command_error()
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::SqlMigrationConnector;
use migration_connector::{
BoxFuture, ConnectorError, ConnectorResult, MigrationPersistence, MigrationRecord, PersistenceNotInitializedError,
BoxFuture, ConnectorError, ConnectorResult, MigrationPersistence, MigrationRecord, Namespaces,
PersistenceNotInitializedError,
};
use quaint::ast::*;
use uuid::Uuid;
Expand All @@ -10,10 +11,9 @@ impl MigrationPersistence for SqlMigrationConnector {
self.flavour.create_migrations_table()
}

fn initialize(&mut self) -> BoxFuture<'_, ConnectorResult<()>> {
fn initialize(&mut self, namespaces: Option<Namespaces>) -> BoxFuture<'_, ConnectorResult<()>> {
Box::pin(async move {
// TODO(MultiSchema): We may need to change this too.
let schema = self.flavour.describe_schema(None).await?;
let schema = self.flavour.describe_schema(namespaces).await?;

if schema
.table_walkers()
Expand Down
6 changes: 3 additions & 3 deletions migration-engine/core/src/commands/apply_migrations.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{json_rpc::types::*, CoreError, CoreResult};
use migration_connector::{
migrations_directory::{error_on_changed_provider, list_migrations, MigrationDirectory},
ConnectorError, MigrationConnector, MigrationRecord, PersistenceNotInitializedError,
ConnectorError, MigrationConnector, MigrationRecord, Namespaces, PersistenceNotInitializedError,
};
use std::{path::Path, time::Instant};
use tracing::Instrument;
Expand All @@ -10,14 +10,14 @@ use user_facing_errors::migration_engine::FoundFailedMigrations;
pub async fn apply_migrations(
input: ApplyMigrationsInput,
connector: &mut dyn MigrationConnector,
namespaces: Option<Namespaces>,
) -> CoreResult<ApplyMigrationsOutput> {
let start = Instant::now();

error_on_changed_provider(&input.migrations_directory_path, connector.connector_type())?;

connector.acquire_lock().await?;

connector.migration_persistence().initialize().await?;
connector.migration_persistence().initialize(namespaces).await?;

let migrations_from_filesystem = list_migrations(Path::new(&input.migrations_directory_path))?;
let migrations_from_database = connector
Expand Down
6 changes: 5 additions & 1 deletion migration-engine/core/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,12 @@ impl GenericApi for EngineState {
}

async fn apply_migrations(&self, input: ApplyMigrationsInput) -> CoreResult<ApplyMigrationsOutput> {
let namespaces = self.namespaces();
self.with_default_connector(Box::new(move |connector| {
Box::pin(commands::apply_migrations(input, connector).instrument(tracing::info_span!("ApplyMigrations")))
Box::pin(
commands::apply_migrations(input, connector, namespaces)
.instrument(tracing::info_span!("ApplyMigrations")),
)
}))
.await
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,30 @@
use migration_core::{
commands::apply_migrations, json_rpc::types::*, migration_connector::MigrationConnector, CoreError, CoreResult,
commands::apply_migrations,
json_rpc::types::*,
migration_connector::{MigrationConnector, Namespaces},
CoreError, CoreResult,
};
use tempfile::TempDir;

#[must_use = "This struct does nothing on its own. See ApplyMigrations::send()"]
pub struct ApplyMigrations<'a> {
api: &'a mut dyn MigrationConnector,
migrations_directory: &'a TempDir,
namespaces: Option<Namespaces>,
}

impl<'a> ApplyMigrations<'a> {
pub fn new(api: &'a mut dyn MigrationConnector, migrations_directory: &'a TempDir) -> Self {
pub fn new(
api: &'a mut dyn MigrationConnector,
migrations_directory: &'a TempDir,
mut namespaces: Vec<String>,
) -> Self {
let namespaces = Namespaces::from_vec(&mut namespaces);

ApplyMigrations {
api,
migrations_directory,
namespaces,
}
}

Expand All @@ -23,6 +34,7 @@ impl<'a> ApplyMigrations<'a> {
migrations_directory_path: self.migrations_directory.path().to_str().unwrap().to_owned(),
},
self.api,
self.namespaces,
)
.await?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ impl TestApi {
connector,
connection_info,
tags: self.args.tags(),
namespaces: self.args.namespaces(),
}
}

Expand Down Expand Up @@ -265,12 +266,19 @@ pub struct EngineTestApi {
pub(crate) connector: SqlMigrationConnector,
connection_info: ConnectionInfo,
tags: BitFlags<Tags>,
namespaces: &'static [&'static str],
}

impl EngineTestApi {
/// Plan an `applyMigrations` command
pub fn apply_migrations<'a>(&'a mut self, migrations_directory: &'a TempDir) -> ApplyMigrations<'a> {
ApplyMigrations::new(&mut self.connector, migrations_directory)
let mut namespaces = vec![self.connection_info.schema_name().to_string()];

for namespace in self.namespaces {
namespaces.push(namespace.to_string());
}

ApplyMigrations::new(&mut self.connector, migrations_directory, namespaces)
}

/// Plan a `createMigration` command
Expand Down
11 changes: 9 additions & 2 deletions migration-engine/migration-engine-tests/src/test_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,14 @@ impl TestApi {

/// Plan an `applyMigrations` command
pub fn apply_migrations<'a>(&'a mut self, migrations_directory: &'a TempDir) -> ApplyMigrations<'a> {
ApplyMigrations::new(&mut self.connector, migrations_directory)
let search_path = self.root.admin_conn.connection_info().schema_name();
let mut namespaces = vec![search_path.to_string()];

for namespace in self.root.args.namespaces() {
namespaces.push(namespace.to_string());
}

ApplyMigrations::new(&mut self.connector, migrations_directory, namespaces)
}

pub fn connection_string(&self) -> &str {
Expand Down Expand Up @@ -339,7 +346,7 @@ impl TestApi {
.unwrap()
}

fn generator_block(&self) -> String {
pub fn generator_block(&self) -> String {
let preview_feature_string = if self.root.preview_features().is_empty() {
"".to_string()
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn mark_migration_rolled_back_on_an_empty_database_errors(api: TestApi) {

#[test_connector]
fn mark_migration_rolled_back_on_a_database_with_migrations_table_errors(api: TestApi) {
tok(api.migration_persistence().initialize()).unwrap();
tok(api.migration_persistence().initialize(None)).unwrap();

let err = api
.mark_migration_rolled_back("anything")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use pretty_assertions::assert_eq;
fn starting_a_migration_works(api: TestApi) {
let persistence = api.migration_persistence();

tok(persistence.initialize()).unwrap();
tok(persistence.initialize(None)).unwrap();

let script = "CREATE ENUM MyBoolean ( \"TRUE\", \"FALSE\" )";

Expand Down Expand Up @@ -39,7 +39,7 @@ fn starting_a_migration_works(api: TestApi) {
fn finishing_a_migration_works(api: TestApi) {
let persistence = api.migration_persistence();

tok(persistence.initialize()).unwrap();
tok(persistence.initialize(None)).unwrap();

let script = "CREATE ENUM MyBoolean ( \"TRUE\", \"FALSE\" )";

Expand Down Expand Up @@ -76,7 +76,7 @@ fn finishing_a_migration_works(api: TestApi) {
fn updating_then_finishing_a_migration_works(api: TestApi) {
let persistence = api.migration_persistence();

tok(persistence.initialize()).unwrap();
tok(persistence.initialize(None)).unwrap();

let script = "CREATE ENUM MyBoolean ( \"TRUE\", \"FALSE\" )";

Expand Down Expand Up @@ -114,7 +114,7 @@ fn updating_then_finishing_a_migration_works(api: TestApi) {
fn multiple_successive_migrations_work(api: TestApi) {
let persistence = api.migration_persistence();

tok(persistence.initialize()).unwrap();
tok(persistence.initialize(None)).unwrap();

let script_1 = "CREATE ENUM MyBoolean ( \"TRUE\", \"FALSE\" )";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,43 @@ use crate::migrations::multi_schema::*;
use migration_engine_tests::test_api::*;
use sql_schema_describer::DefaultValue;

#[test_connector(tags(Postgres), preview_features("multiSchema"), namespaces("one", "prisma-tests"))]
fn apply_migrations_with_multiple_schemas_where_one_is_search_path_with_a_foreign_key(api: TestApi) {
let datasource = api.datasource_block_with(&[("schemas", r#"["one", "prisma-tests"]"#)]);
let generator = api.generator_block();

let dm = indoc::formatdoc! {r#"
{datasource}
{generator}
model A {{
id Int @id
bs B[]
@@schema("one")
}}
model B {{
id Int @id
aId Int
a A @relation(fields: [aId], references: [id])
@@schema("prisma-tests")
}}
"#};

let dir = api.create_migrations_directory();

api.create_migration("init", &dm, &dir).send_sync();

api.apply_migrations(&dir)
.send_sync()
.assert_applied_migrations(&["init"]);

api.apply_migrations(&dir).send_sync().assert_applied_migrations(&[]);
}

// This is the only "top" level test in this module. It defines a list of tests and executes them.
// If you want to look at the tests, see the `tests` variable below.
#[test_connector(tags(Postgres), preview_features("multiSchema"), namespaces("one", "two"))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,10 @@ fn multi_schema_reset(mut api: TestApi) {
}}
"#, api.datasource_block_with(&[("schemas", r#"["felines", "rodents"]"#)])
};
api.schema_push(&prisma_schema)
.send()
.assert_green()
.assert_has_executed_steps();

let migrations_dir = api.create_migrations_directory();
api.create_migration("0-init", &prisma_schema, &migrations_dir)
.send_sync();
api.apply_migrations(&migrations_dir).send_sync();
api.raw_cmd("CREATE TABLE randomTable (id INTEGER PRIMARY KEY);");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,6 @@ model m1 {
let expected = expect_test::expect![[r#"
The `mysql` database is a system database, it should not be altered with prisma migrate. Please connect to another database.
0: migration_core::state::SchemaPush
at migration-engine/core/src/state.rs:402"#]];
at migration-engine/core/src/state.rs:406"#]];
expected.assert_eq(&err.to_string());
}

0 comments on commit a11d4ff

Please sign in to comment.