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 21, 2022
1 parent c8f7657 commit c7f3223
Show file tree
Hide file tree
Showing 13 changed files with 122 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ impl TestApi {
} else {
&[]
},
&[],
)
}

Expand Down
25 changes: 24 additions & 1 deletion libs/test-setup/src/test_api_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,17 @@ impl TestApiArgs {
self.db.database_url.as_str()
}

pub fn datasource_block<'a>(&'a self, url: &'a str, params: &'a [(&'a str, &'a str)]) -> DatasourceBlock<'a> {
pub fn datasource_block<'a>(
&'a self,
url: &'a str,
params: &'a [(&'a str, &'a str)],
schemas: &'static [&'static str],
) -> DatasourceBlock<'a> {
DatasourceBlock {
provider: self.db.provider,
url,
params,
schemas,
preview_features: EMPTY_PREVIEW_FEATURES,
}
}
Expand All @@ -198,6 +204,7 @@ pub struct DatasourceBlock<'a> {
provider: &'a str,
url: &'a str,
params: &'a [(&'a str, &'a str)],
schemas: &'static [&'static str],
preview_features: &'static [&'static str],
}

Expand Down Expand Up @@ -244,6 +251,22 @@ impl Display for DatasourceBlock<'_> {
f.write_str("\n")?;
}

if !self.schemas.is_empty() {
let schemas = self
.schemas
.iter()
.map(|s| format!(r#""{s}""#))
.collect::<Vec<_>>()
.join(",");

let schemas = format!("[{schemas}]");

f.write_str(" ")?;
f.write_str("schemas = ")?;
f.write_str(schemas.as_str())?;
f.write_str("\n")?;
}

f.write_str("}")
}
}
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 @@ -105,7 +105,8 @@ impl TestApi {

/// Render a valid datasource block, including database URL.
pub fn datasource_block(&self) -> DatasourceBlock<'_> {
self.args.datasource_block(self.args.database_url(), &[])
self.args
.datasource_block(self.args.database_url(), &[], self.args.namespaces())
}

/// Returns true only when testing on MSSQL.
Expand Down Expand Up @@ -202,6 +203,7 @@ impl TestApi {
connector,
connection_info,
tags: self.args.tags(),
namespaces: self.args.namespaces(),
}
}

Expand All @@ -227,7 +229,13 @@ impl TestApi {

/// Render a valid datasource block, including database URL.
pub fn write_datasource_block(&self, out: &mut dyn std::fmt::Write) {
write!(out, "{}", self.args.datasource_block(self.args.database_url(), &[])).unwrap()
write!(
out,
"{}",
self.args
.datasource_block(self.args.database_url(), &[], self.args.namespaces())
)
.unwrap()
}

/// Currently enabled preview features.
Expand Down Expand Up @@ -265,12 +273,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
17 changes: 14 additions & 3 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 @@ -238,7 +245,9 @@ impl TestApi {
}

pub fn datasource_block_with<'a>(&'a self, params: &'a [(&'a str, &'a str)]) -> DatasourceBlock<'a> {
self.root.args.datasource_block(self.root.connection_string(), params)
self.root
.args
.datasource_block(self.root.connection_string(), params, self.root.args.namespaces())
}

/// Generate a migration script using `MigrationConnector::diff()`.
Expand Down Expand Up @@ -334,7 +343,9 @@ impl TestApi {
write!(
out,
"{}",
self.root.args.datasource_block(self.root.args.database_url(), &params)
self.root
.args
.datasource_block(self.root.args.database_url(), &params, self.root.args.namespaces())
)
.unwrap()
}
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,36 @@ 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 dm = api.datamodel_with_provider(indoc! {r#"
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

0 comments on commit c7f3223

Please sign in to comment.