From 7b44bff1c15d8d37b29151005171e77c58a8029d Mon Sep 17 00:00:00 2001 From: George Manning Date: Tue, 1 Aug 2023 01:05:58 +0100 Subject: [PATCH 01/12] add failing pg test --- .../postgres/down.sql/expected.snap | 7 +++++++ .../diff_table_filter/postgres/initial_schema.sql | 10 ++++++++++ .../postgres/schema_out.rs/expected.snap | 13 +++++++++++++ .../diff_table_filter/postgres/up.sql/expected.snap | 7 +++++++ .../generate_migrations/diff_table_filter/schema.rs | 6 ++++++ diesel_cli/tests/migration_generate.rs | 4 ++-- 6 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/postgres/initial_schema.sql create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/postgres/schema_out.rs/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/postgres/up.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/schema.rs diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap new file mode 100644 index 000000000000..491c2a6dd900 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap @@ -0,0 +1,7 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +-- This file should undo anything in `up.sql` +ALTER TABLE "table_a" ADD COLUMN "code" VARCHAR NOT NULL; +ALTER TABLE "table_a" DROP COLUMN "script"; diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/initial_schema.sql new file mode 100644 index 000000000000..2e26de15026a --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/initial_schema.sql @@ -0,0 +1,10 @@ +CREATE TABLE table_a( + "id" serial8 NOT NULL PRIMARY KEY, + "code" VARCHAR NOT NULL +); +CREATE TABLE table_b( + "id" serial8 NOT NULL PRIMARY KEY +); +CREATE TABLE table_c( + "id" serial8 NOT NULL PRIMARY KEY +); diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/schema_out.rs/expected.snap new file mode 100644 index 000000000000..f3fd954401b6 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/schema_out.rs/expected.snap @@ -0,0 +1,13 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +// @generated automatically by Diesel CLI. + +diesel::table! { + table_a (id) { + id -> Int8, + script -> Varchar, + } +} + diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/up.sql/expected.snap new file mode 100644 index 000000000000..a6fe4bdc077e --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/up.sql/expected.snap @@ -0,0 +1,7 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +-- Your SQL goes here +ALTER TABLE "table_a" DROP COLUMN "code"; +ALTER TABLE "table_a" ADD COLUMN "script" VARCHAR NOT NULL; diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/schema.rs b/diesel_cli/tests/generate_migrations/diff_table_filter/schema.rs new file mode 100644 index 000000000000..75b76a494726 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/schema.rs @@ -0,0 +1,6 @@ +diesel::table! { + table_a (id) { + id -> Int8, + script -> Varchar, + } +} diff --git a/diesel_cli/tests/migration_generate.rs b/diesel_cli/tests/migration_generate.rs index c3402993dece..12dd98733dc4 100644 --- a/diesel_cli/tests/migration_generate.rs +++ b/diesel_cli/tests/migration_generate.rs @@ -256,8 +256,8 @@ fn migration_generate_from_diff_add_table_composite_key() { } #[test] -fn migration_generate_from_diff_drop_table_composite_key() { - test_generate_migration("diff_drop_table_composite_key", Vec::new()); +fn migration_generate_from_diff_filter() { + test_generate_migration("diff_table_filter", Vec::new()); } #[cfg(feature = "sqlite")] From 3921c23fcd2582731cb4fa27393270962459eab4 Mon Sep 17 00:00:00 2001 From: George Manning Date: Tue, 1 Aug 2023 02:29:09 +0100 Subject: [PATCH 02/12] rough fix needs work --- diesel_cli/src/config.rs | 91 ++++++++++++++++++- diesel_cli/src/main.rs | 1 + diesel_cli/src/migrations/diff_schema.rs | 8 +- diesel_cli/src/print_schema.rs | 86 +----------------- .../diff_table_filter/diesel.toml | 2 + .../postgres/down.sql/expected.snap | 4 +- diesel_cli/tests/migration_generate.rs | 36 +++++--- 7 files changed, 125 insertions(+), 103 deletions(-) create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/diesel.toml diff --git a/diesel_cli/src/config.rs b/diesel_cli/src/config.rs index 9a8206e4b7af..5e0635e38906 100644 --- a/diesel_cli/src/config.rs +++ b/diesel_cli/src/config.rs @@ -1,11 +1,14 @@ use clap::ArgMatches; -use serde::Deserialize; -use std::env; +use serde::de::{self, MapAccess, Visitor}; +use serde::{Deserialize, Deserializer}; +use serde_regex::Serde as RegexWrapper; use std::error::Error; use std::fs; use std::path::{Path, PathBuf}; +use std::{env, fmt}; use super::find_project_root; +use crate::infer_schema_internals::TableName; use crate::print_schema; use crate::print_schema::ColumnSorting; @@ -56,7 +59,7 @@ pub struct PrintSchema { #[serde(default)] pub with_docs: print_schema::DocConfig, #[serde(default)] - pub filter: print_schema::Filtering, + pub filter: Filtering, #[serde(default)] pub column_sorting: ColumnSorting, #[serde(default)] @@ -129,3 +132,85 @@ impl MigrationsDirectory { } } } + +type Regex = RegexWrapper<::regex::Regex>; + +pub enum Filtering { + OnlyTables(Vec), + ExceptTables(Vec), + None, +} + +#[allow(clippy::derivable_impls)] // that's not supported on rust 1.65 +impl Default for Filtering { + fn default() -> Self { + Filtering::None + } +} + +impl Filtering { + pub fn should_ignore_table(&self, name: &TableName) -> bool { + use self::Filtering::*; + + match *self { + OnlyTables(ref regexes) => !regexes.iter().any(|regex| regex.is_match(&name.sql_name)), + ExceptTables(ref regexes) => regexes.iter().any(|regex| regex.is_match(&name.sql_name)), + None => false, + } + } +} + +impl<'de> Deserialize<'de> for Filtering { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct FilteringVisitor; + + impl<'de> Visitor<'de> for FilteringVisitor { + type Value = Filtering; + + fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str("either only_tables or except_tables") + } + + fn visit_map(self, mut map: V) -> Result + where + V: MapAccess<'de>, + { + let mut only_tables = None::>; + let mut except_tables = None::>; + while let Some(key) = map.next_key::()? { + match &key as &str { + "only_tables" => { + if only_tables.is_some() { + return Err(de::Error::duplicate_field("only_tables")); + } + only_tables = Some(map.next_value()?); + } + "except_tables" => { + if except_tables.is_some() { + return Err(de::Error::duplicate_field("except_tables")); + } + except_tables = Some(map.next_value()?); + } + _ => { + return Err(de::Error::unknown_field( + &key, + &["only_tables", "except_tables"], + )) + } + } + } + match (only_tables, except_tables) { + (Some(t), None) => Ok(Filtering::OnlyTables(t)), + (None, Some(t)) => Ok(Filtering::ExceptTables(t)), + (None, None) => Ok(Filtering::None), + _ => Err(de::Error::duplicate_field("only_tables except_tables")), + } + } + } + + deserializer.deserialize_map(FilteringVisitor) + } +} diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index fc5e82fbdee2..771d7cb7a4de 100644 --- a/diesel_cli/src/main.rs +++ b/diesel_cli/src/main.rs @@ -39,6 +39,7 @@ use std::path::{Path, PathBuf}; use std::{env, fs}; use self::config::Config; +use self::config::Filtering; use self::database_error::{DatabaseError, DatabaseResult}; pub static TIMESTAMP_FORMAT: &str = "%Y-%m-%d-%H%M%S"; diff --git a/diesel_cli/src/migrations/diff_schema.rs b/diesel_cli/src/migrations/diff_schema.rs index c2945492ab68..a34c198ca154 100644 --- a/diesel_cli/src/migrations/diff_schema.rs +++ b/diesel_cli/src/migrations/diff_schema.rs @@ -27,7 +27,7 @@ fn compatible_type_list() -> HashMap<&'static str, Vec<&'static str>> { } pub fn generate_sql_based_on_diff_schema( - _config: Config, + config: Config, matches: &ArgMatches, schema_file_path: &Path, ) -> Result<(String, String), Box> { @@ -43,7 +43,11 @@ pub fn generate_sql_based_on_diff_schema( tables_from_schema.visit_file(&syn_file); let mut conn = InferConnection::from_matches(matches); - let tables_from_database = crate::infer_schema_internals::load_table_names(&mut conn, None)?; + let tables_from_database = crate::infer_schema_internals::load_table_names(&mut conn, None)? + .into_iter() + .filter(|t| !config.print_schema.filter.should_ignore_table(t)) + .collect::>(); + let foreign_keys = crate::infer_schema_internals::load_foreign_key_constraints(&mut conn, None)?; let foreign_key_map = foreign_keys.into_iter().fold(HashMap::new(), |mut acc, t| { diff --git a/diesel_cli/src/print_schema.rs b/diesel_cli/src/print_schema.rs index a64f75c8089b..4468c4dabb90 100644 --- a/diesel_cli/src/print_schema.rs +++ b/diesel_cli/src/print_schema.rs @@ -2,9 +2,7 @@ use crate::config; use crate::database::{Backend, InferConnection}; use crate::infer_schema_internals::*; -use serde::de::{self, MapAccess, Visitor}; -use serde::{Deserialize, Deserializer, Serialize}; -use serde_regex::Serde as RegexWrapper; +use serde::{Deserialize, Serialize}; use std::collections::HashSet; use std::error::Error; use std::fmt::{self, Display, Formatter, Write}; @@ -12,33 +10,6 @@ use std::io::Write as IoWrite; const SCHEMA_HEADER: &str = "// @generated automatically by Diesel CLI.\n"; -type Regex = RegexWrapper<::regex::Regex>; - -pub enum Filtering { - OnlyTables(Vec), - ExceptTables(Vec), - None, -} - -#[allow(clippy::derivable_impls)] // that's not supported on rust 1.65 -impl Default for Filtering { - fn default() -> Self { - Filtering::None - } -} - -impl Filtering { - pub fn should_ignore_table(&self, name: &TableName) -> bool { - use self::Filtering::*; - - match *self { - OnlyTables(ref regexes) => !regexes.iter().any(|regex| regex.is_match(&name.sql_name)), - ExceptTables(ref regexes) => regexes.iter().any(|regex| regex.is_match(&name.sql_name)), - None => false, - } - } -} - /// How to sort columns when querying the table schema. #[derive(Debug, Deserialize, Serialize)] pub enum ColumnSorting { @@ -772,61 +743,6 @@ impl<'a, 'b: 'a> Write for PadAdapter<'a, 'b> { } } -impl<'de> Deserialize<'de> for Filtering { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - struct FilteringVisitor; - - impl<'de> Visitor<'de> for FilteringVisitor { - type Value = Filtering; - - fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str("either only_tables or except_tables") - } - - fn visit_map(self, mut map: V) -> Result - where - V: MapAccess<'de>, - { - let mut only_tables = None::>; - let mut except_tables = None::>; - while let Some(key) = map.next_key::()? { - match &key as &str { - "only_tables" => { - if only_tables.is_some() { - return Err(de::Error::duplicate_field("only_tables")); - } - only_tables = Some(map.next_value()?); - } - "except_tables" => { - if except_tables.is_some() { - return Err(de::Error::duplicate_field("except_tables")); - } - except_tables = Some(map.next_value()?); - } - _ => { - return Err(de::Error::unknown_field( - &key, - &["only_tables", "except_tables"], - )) - } - } - } - match (only_tables, except_tables) { - (Some(t), None) => Ok(Filtering::OnlyTables(t)), - (None, Some(t)) => Ok(Filtering::ExceptTables(t)), - (None, None) => Ok(Filtering::None), - _ => Err(de::Error::duplicate_field("only_tables except_tables")), - } - } - } - - deserializer.deserialize_map(FilteringVisitor) - } -} - impl DocConfig { pub const VARIANTS_STR: &'static [&'static str] = &[ "database-comments-fallback-to-auto-generated-doc-comment", diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/diesel.toml b/diesel_cli/tests/generate_migrations/diff_table_filter/diesel.toml new file mode 100644 index 000000000000..66ee2f8dc43f --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/diesel.toml @@ -0,0 +1,2 @@ +[print_schema] +filter = { only_tables = ["table_a"] } diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap index 491c2a6dd900..d64657440ddf 100644 --- a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap @@ -3,5 +3,7 @@ source: diesel_cli/tests/migration_generate.rs description: "Test: diff_table_filter" --- -- This file should undo anything in `up.sql` -ALTER TABLE "table_a" ADD COLUMN "code" VARCHAR NOT NULL; ALTER TABLE "table_a" DROP COLUMN "script"; +ALTER TABLE "table_a" ADD COLUMN "code" VARCHAR NOT NULL; + + diff --git a/diesel_cli/tests/migration_generate.rs b/diesel_cli/tests/migration_generate.rs index 12dd98733dc4..abbda60fc8cb 100644 --- a/diesel_cli/tests/migration_generate.rs +++ b/diesel_cli/tests/migration_generate.rs @@ -212,52 +212,52 @@ Creating custom_migrations.12345_stuff.down.sql #[test] fn migration_generate_from_diff_drop_table() { - test_generate_migration("diff_drop_table", Vec::new()); + test_generate_migration("diff_drop_table", Vec::new(), false); } #[test] fn migration_generate_from_diff_add_table() { - test_generate_migration("diff_add_table", Vec::new()); + test_generate_migration("diff_add_table", Vec::new(), false); } #[test] fn migration_generate_from_diff_drop_alter_table_add_column() { - test_generate_migration("diff_alter_table_add_column", Vec::new()); + test_generate_migration("diff_alter_table_add_column", Vec::new(), false); } #[test] fn migration_generate_from_diff_alter_table_drop_column() { - test_generate_migration("diff_alter_table_drop_column", Vec::new()); + test_generate_migration("diff_alter_table_drop_column", Vec::new(), false); } #[test] fn migration_generate_from_diff_add_table_with_fk() { - test_generate_migration("diff_add_table_with_fk", Vec::new()); + test_generate_migration("diff_add_table_with_fk", Vec::new(), false); } #[test] fn migration_generate_from_diff_drop_table_with_fk() { - test_generate_migration("diff_drop_table_with_fk", Vec::new()); + test_generate_migration("diff_drop_table_with_fk", Vec::new(), false); } #[test] fn migration_generate_from_diff_drop_table_all_the_types() { - test_generate_migration("diff_drop_table_all_the_types", Vec::new()); + test_generate_migration("diff_drop_table_all_the_types", Vec::new(), false); } #[test] fn migration_generate_from_diff_add_table_all_the_types() { - test_generate_migration("diff_add_table_all_the_types", Vec::new()); + test_generate_migration("diff_add_table_all_the_types", Vec::new(), false); } #[test] fn migration_generate_from_diff_add_table_composite_key() { - test_generate_migration("diff_add_table_composite_key", Vec::new()); + test_generate_migration("diff_add_table_composite_key", Vec::new(), false); } #[test] fn migration_generate_from_diff_filter() { - test_generate_migration("diff_table_filter", Vec::new()); + test_generate_migration("diff_table_filter", Vec::new(), true); } #[cfg(feature = "sqlite")] @@ -276,8 +276,20 @@ fn backend_file_path(test_name: &str, file: &str) -> PathBuf { .join(file) } -fn test_generate_migration(test_name: &str, args: Vec<&str>) { - let p = project(test_name).build(); +fn test_generate_migration(test_name: &str, args: Vec<&str>, with_config: bool) { + let p = match with_config { + true => { + let test_path = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests") + .join("generate_migrations") + .join(test_name); + let config = read_file(&test_path.join("diesel.toml")); + + project(test_name).file("diesel.toml", &config).build() + } + false => project(test_name).build(), + }; + let db = crate::support::database(&p.database_url()); p.command("setup").run(); From 6d6f3c281b3443d7583b818881cbabbc552e336b Mon Sep 17 00:00:00 2001 From: George Manning Date: Wed, 2 Aug 2023 00:26:21 +0100 Subject: [PATCH 03/12] add cli args to migration generate command --- diesel_cli/src/cli.rs | 25 ++++++++++++++ diesel_cli/src/config.rs | 34 +++++++++++++++++++ diesel_cli/src/main.rs | 43 +++++++++--------------- diesel_cli/src/migrations/diff_schema.rs | 8 ++--- diesel_cli/src/migrations/mod.rs | 2 +- diesel_cli/src/print_schema.rs | 15 +++++---- diesel_cli/tests/migration_generate.rs | 40 ++++++++-------------- 7 files changed, 102 insertions(+), 65 deletions(-) diff --git a/diesel_cli/src/cli.rs b/diesel_cli/src/cli.rs index af9b63c7ec4f..6eee5d0cf68b 100644 --- a/diesel_cli/src/cli.rs +++ b/diesel_cli/src/cli.rs @@ -98,6 +98,8 @@ pub fn build_cli() -> Command { ) .arg( Arg::new("MIGRATION_NAME") + .index(1) + .num_args(1) .help("The name of the migration to create.") .required(true), ) @@ -145,6 +147,29 @@ pub fn build_cli() -> Command { .default_missing_value("NOT_SET") .num_args(0..=1) .require_equals(true), + ) + .arg( + Arg::new("table-name") + .index(2) + .num_args(1..) + .action(clap::ArgAction::Append) + .help("Table names to filter (default only-tables if not empty)."), + ) + .arg( + Arg::new("only-tables") + .short('o') + .long("only-tables") + .action(ArgAction::SetTrue) + .help("Only include tables from table-name that matches regexp.") + .conflicts_with("except-tables"), + ) + .arg( + Arg::new("except-tables") + .short('e') + .long("except-tables") + .action(ArgAction::SetTrue) + .help("Exclude tables from table-name that matches regex.") + .conflicts_with("only-tables"), ), ) .subcommand_required(true) diff --git a/diesel_cli/src/config.rs b/diesel_cli/src/config.rs index 5e0635e38906..ae1b373d9ba7 100644 --- a/diesel_cli/src/config.rs +++ b/diesel_cli/src/config.rs @@ -49,6 +49,26 @@ impl Config { migration.set_relative_path_base(base); } } + + pub fn set_filters( + mut self, + matches: &ArgMatches, + ) -> Result> { + let table_names = matches + .get_many::("table-name") + .unwrap_or_default() + .map(|table_name_regex| regex::Regex::new(table_name_regex).map(Into::into)) + .collect::, _>>() + .map_err(|e| format!("invalid argument for table filtering regex: {e}"))?; + + if matches.get_flag("only-tables") { + self.print_schema.filter = Filtering::OnlyTables(table_names) + } else if matches.get_flag("except-tables") { + self.print_schema.filter = Filtering::ExceptTables(table_names) + } + + Ok(self) + } } #[derive(Default, Deserialize)] @@ -135,6 +155,7 @@ impl MigrationsDirectory { type Regex = RegexWrapper<::regex::Regex>; +#[derive(Clone)] pub enum Filtering { OnlyTables(Vec), ExceptTables(Vec), @@ -160,6 +181,19 @@ impl Filtering { } } +pub trait FilteringT { + fn filter_table_names(self, config: &Config) -> Self; +} + +impl FilteringT for Vec { + fn filter_table_names(self, config: &Config) -> Self { + self + .into_iter() + .filter(|t| !config.print_schema.filter.should_ignore_table(t)) + .collect::() + } +} + impl<'de> Deserialize<'de> for Filtering { fn deserialize(deserializer: D) -> Result where diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index 771d7cb7a4de..ec2412fb8052 100644 --- a/diesel_cli/src/main.rs +++ b/diesel_cli/src/main.rs @@ -31,7 +31,6 @@ use database::InferConnection; use diesel::backend::Backend; use diesel::Connection; use diesel_migrations::{FileBasedMigrations, HarnessWithOutput, MigrationHarness}; -use regex::Regex; use std::error::Error; use std::fmt::Display; use std::io::stdout; @@ -39,7 +38,6 @@ use std::path::{Path, PathBuf}; use std::{env, fs}; use self::config::Config; -use self::config::Filtering; use self::database_error::{DatabaseError, DatabaseResult}; pub static TIMESTAMP_FORMAT: &str = "%Y-%m-%d-%H%M%S"; @@ -221,57 +219,48 @@ fn run_infer_schema(matches: &ArgMatches) -> Result<(), Box("schema") { - config.schema = Some(schema_name.clone()) - } - - let filter = matches - .get_many::("table-name") - .unwrap_or_default() - .map(|table_name_regex| Regex::new(table_name_regex).map(Into::into)) - .collect::>() - .map_err(|e| format!("invalid argument for table filtering regex: {e}")); - - if matches.get_flag("only-tables") { - config.filter = Filtering::OnlyTables(filter?) - } else if matches.get_flag("except-tables") { - config.filter = Filtering::ExceptTables(filter?) + config.print_schema.schema = Some(schema_name.clone()) } if matches.get_flag("with-docs") { - config.with_docs = DocConfig::DatabaseCommentsFallbackToAutoGeneratedDocComment; + config.print_schema.with_docs = + DocConfig::DatabaseCommentsFallbackToAutoGeneratedDocComment; } if let Some(sorting) = matches.get_one::("with-docs-config") { - config.with_docs = sorting.parse()?; + config.print_schema.with_docs = sorting.parse()?; } if let Some(sorting) = matches.get_one::("column-sorting") { match sorting as &str { - "ordinal_position" => config.column_sorting = ColumnSorting::OrdinalPosition, - "name" => config.column_sorting = ColumnSorting::Name, + "ordinal_position" => { + config.print_schema.column_sorting = ColumnSorting::OrdinalPosition + } + "name" => config.print_schema.column_sorting = ColumnSorting::Name, _ => return Err(format!("Invalid column sorting mode: {sorting}").into()), } } if let Some(path) = matches.get_one::("patch-file") { - config.patch_file = Some(path.clone()); + config.print_schema.patch_file = Some(path.clone()); } if let Some(types) = matches.get_many("import-types") { let types = types.cloned().collect(); - config.import_types = Some(types); + config.print_schema.import_types = Some(types); } if matches.get_flag("generate-custom-type-definitions") { - config.generate_missing_sql_type_definitions = Some(false); + config.print_schema.generate_missing_sql_type_definitions = Some(false); } if let Some(derives) = matches.get_many("custom-type-derives") { let derives = derives.cloned().collect(); - config.custom_type_derives = Some(derives); + config.print_schema.custom_type_derives = Some(derives); } run_print_schema(&mut conn, &config, &mut stdout())?; @@ -292,7 +281,7 @@ fn regenerate_schema_if_file_specified( if matches.get_flag("LOCKED_SCHEMA") { let mut buf = Vec::new(); - print_schema::run_print_schema(&mut connection, &config.print_schema, &mut buf)?; + print_schema::run_print_schema(&mut connection, &config, &mut buf)?; let mut old_buf = Vec::new(); let mut file = fs::File::open(path)?; @@ -310,7 +299,7 @@ fn regenerate_schema_if_file_specified( use std::io::Write; let mut file = fs::File::create(path)?; - let schema = print_schema::output_schema(&mut connection, &config.print_schema)?; + let schema = print_schema::output_schema(&mut connection, &config)?; file.write_all(schema.as_bytes())?; } } diff --git a/diesel_cli/src/migrations/diff_schema.rs b/diesel_cli/src/migrations/diff_schema.rs index a34c198ca154..51f356a27e1c 100644 --- a/diesel_cli/src/migrations/diff_schema.rs +++ b/diesel_cli/src/migrations/diff_schema.rs @@ -10,7 +10,7 @@ use std::io::Read; use std::path::Path; use syn::visit::Visit; -use crate::config::Config; +use crate::config::{Config, FilteringT}; use crate::database::InferConnection; use crate::infer_schema_internals::{ ColumnDefinition, ColumnType, ForeignKeyConstraint, TableData, TableName, @@ -31,6 +31,8 @@ pub fn generate_sql_based_on_diff_schema( matches: &ArgMatches, schema_file_path: &Path, ) -> Result<(String, String), Box> { + let config = config.set_filters(matches)?; + let project_root = crate::find_project_root()?; let schema_path = project_root.join(schema_file_path); @@ -44,9 +46,7 @@ pub fn generate_sql_based_on_diff_schema( tables_from_schema.visit_file(&syn_file); let mut conn = InferConnection::from_matches(matches); let tables_from_database = crate::infer_schema_internals::load_table_names(&mut conn, None)? - .into_iter() - .filter(|t| !config.print_schema.filter.should_ignore_table(t)) - .collect::>(); + .filter_table_names(&config); let foreign_keys = crate::infer_schema_internals::load_foreign_key_constraints(&mut conn, None)?; diff --git a/diesel_cli/src/migrations/mod.rs b/diesel_cli/src/migrations/mod.rs index b12e7308c104..bcbea6868a30 100644 --- a/diesel_cli/src/migrations/mod.rs +++ b/diesel_cli/src/migrations/mod.rs @@ -88,7 +88,7 @@ pub(super) fn run_migration_command( if let Some(diff_schema) = diff_schema { self::diff_schema::generate_sql_based_on_diff_schema( config, - matches, + args, &diff_schema, )? } else { diff --git a/diesel_cli/src/print_schema.rs b/diesel_cli/src/print_schema.rs index 4468c4dabb90..21c13686bc47 100644 --- a/diesel_cli/src/print_schema.rs +++ b/diesel_cli/src/print_schema.rs @@ -1,4 +1,4 @@ -use crate::config; +use crate::config::{Config, FilteringT}; use crate::database::{Backend, InferConnection}; use crate::infer_schema_internals::*; @@ -44,7 +44,7 @@ impl Default for DocConfig { pub fn run_print_schema( connection: &mut InferConnection, - config: &config::PrintSchema, + config: &Config, output: &mut W, ) -> Result<(), Box> { let schema = output_schema(connection, config)?; @@ -145,12 +145,13 @@ fn sqlite_diesel_types() -> HashSet<&'static str> { pub fn output_schema( connection: &mut InferConnection, - config: &config::PrintSchema, + config: &Config, ) -> Result> { - let table_names = load_table_names(connection, config.schema_name())? - .into_iter() - .filter(|t| !config.filter.should_ignore_table(t)) - .collect::>(); + let table_names = load_table_names(connection, config.print_schema.schema_name())? + .filter_table_names(config); + + let config = &config.print_schema; + let foreign_keys = load_foreign_key_constraints(connection, config.schema_name())?; let foreign_keys = remove_unsafe_foreign_keys_for_codegen(connection, &foreign_keys, &table_names); diff --git a/diesel_cli/tests/migration_generate.rs b/diesel_cli/tests/migration_generate.rs index abbda60fc8cb..357546239ae4 100644 --- a/diesel_cli/tests/migration_generate.rs +++ b/diesel_cli/tests/migration_generate.rs @@ -212,52 +212,52 @@ Creating custom_migrations.12345_stuff.down.sql #[test] fn migration_generate_from_diff_drop_table() { - test_generate_migration("diff_drop_table", Vec::new(), false); + test_generate_migration("diff_drop_table", Vec::new()); } #[test] fn migration_generate_from_diff_add_table() { - test_generate_migration("diff_add_table", Vec::new(), false); + test_generate_migration("diff_add_table", Vec::new()); } #[test] fn migration_generate_from_diff_drop_alter_table_add_column() { - test_generate_migration("diff_alter_table_add_column", Vec::new(), false); + test_generate_migration("diff_alter_table_add_column", Vec::new()); } #[test] fn migration_generate_from_diff_alter_table_drop_column() { - test_generate_migration("diff_alter_table_drop_column", Vec::new(), false); + test_generate_migration("diff_alter_table_drop_column", Vec::new()); } #[test] fn migration_generate_from_diff_add_table_with_fk() { - test_generate_migration("diff_add_table_with_fk", Vec::new(), false); + test_generate_migration("diff_add_table_with_fk", Vec::new()); } #[test] fn migration_generate_from_diff_drop_table_with_fk() { - test_generate_migration("diff_drop_table_with_fk", Vec::new(), false); + test_generate_migration("diff_drop_table_with_fk", Vec::new()); } #[test] fn migration_generate_from_diff_drop_table_all_the_types() { - test_generate_migration("diff_drop_table_all_the_types", Vec::new(), false); + test_generate_migration("diff_drop_table_all_the_types", Vec::new()); } #[test] fn migration_generate_from_diff_add_table_all_the_types() { - test_generate_migration("diff_add_table_all_the_types", Vec::new(), false); + test_generate_migration("diff_add_table_all_the_types", Vec::new()); } #[test] fn migration_generate_from_diff_add_table_composite_key() { - test_generate_migration("diff_add_table_composite_key", Vec::new(), false); + test_generate_migration("diff_add_table_composite_key", Vec::new()); } #[test] fn migration_generate_from_diff_filter() { - test_generate_migration("diff_table_filter", Vec::new(), true); + test_generate_migration("diff_table_filter", vec!["-o", "table_a"]); } #[cfg(feature = "sqlite")] @@ -276,20 +276,8 @@ fn backend_file_path(test_name: &str, file: &str) -> PathBuf { .join(file) } -fn test_generate_migration(test_name: &str, args: Vec<&str>, with_config: bool) { - let p = match with_config { - true => { - let test_path = Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests") - .join("generate_migrations") - .join(test_name); - let config = read_file(&test_path.join("diesel.toml")); - - project(test_name).file("diesel.toml", &config).build() - } - false => project(test_name).build(), - }; - +fn test_generate_migration(test_name: &str, args: Vec<&str>) { + let p = project(test_name).build(); let db = crate::support::database(&p.database_url()); p.command("setup").run(); @@ -318,7 +306,7 @@ fn test_generate_migration(test_name: &str, args: Vec<&str>, with_config: bool) "--diff-schema={schema_rs}", schema_rs = schema_rs.display() )) - .args(args) + .args(args.clone()) .run(); assert!(result.is_success(), "Result was unsuccessful {:?}", result); @@ -350,7 +338,7 @@ fn test_generate_migration(test_name: &str, args: Vec<&str>, with_config: bool) assert!(result.is_success(), "Result was unsuccessful {:?}", result); // check that we get back the expected schema - let result = p.command("print-schema").run(); + let result = p.command("print-schema").args(args).run(); assert!(result.is_success(), "Result was unsuccessful {:?}", result); let result = result.stdout().replace("\r\n", "\n"); From 13c7dcc973e9a064a396cefdc497a348503bfd5b Mon Sep 17 00:00:00 2001 From: George Manning Date: Wed, 2 Aug 2023 00:36:50 +0100 Subject: [PATCH 04/12] move filter table names to infer schema internals --- diesel_cli/src/config.rs | 13 ------------- diesel_cli/src/infer_schema_internals/inference.rs | 7 +++++++ diesel_cli/src/migrations/diff_schema.rs | 8 ++++---- diesel_cli/src/print_schema.rs | 8 +++++--- 4 files changed, 16 insertions(+), 20 deletions(-) diff --git a/diesel_cli/src/config.rs b/diesel_cli/src/config.rs index ae1b373d9ba7..b56c49815684 100644 --- a/diesel_cli/src/config.rs +++ b/diesel_cli/src/config.rs @@ -181,19 +181,6 @@ impl Filtering { } } -pub trait FilteringT { - fn filter_table_names(self, config: &Config) -> Self; -} - -impl FilteringT for Vec { - fn filter_table_names(self, config: &Config) -> Self { - self - .into_iter() - .filter(|t| !config.print_schema.filter.should_ignore_table(t)) - .collect::() - } -} - impl<'de> Deserialize<'de> for Filtering { fn deserialize(deserializer: D) -> Result where diff --git a/diesel_cli/src/infer_schema_internals/inference.rs b/diesel_cli/src/infer_schema_internals/inference.rs index 55327403fd67..7b9fbfa1bfde 100644 --- a/diesel_cli/src/infer_schema_internals/inference.rs +++ b/diesel_cli/src/infer_schema_internals/inference.rs @@ -124,6 +124,13 @@ pub fn load_table_names( } } +pub fn filter_table_names(table_names: Vec, config: &crate::Config) -> Vec { + table_names + .into_iter() + .filter(|t| !config.print_schema.filter.should_ignore_table(t)) + .collect::<_>() +} + fn get_table_comment( conn: &mut InferConnection, table: &TableName, diff --git a/diesel_cli/src/migrations/diff_schema.rs b/diesel_cli/src/migrations/diff_schema.rs index 51f356a27e1c..7438c9a56216 100644 --- a/diesel_cli/src/migrations/diff_schema.rs +++ b/diesel_cli/src/migrations/diff_schema.rs @@ -10,10 +10,11 @@ use std::io::Read; use std::path::Path; use syn::visit::Visit; -use crate::config::{Config, FilteringT}; +use crate::config::Config; use crate::database::InferConnection; use crate::infer_schema_internals::{ - ColumnDefinition, ColumnType, ForeignKeyConstraint, TableData, TableName, + filter_table_names, load_table_names, ColumnDefinition, ColumnType, ForeignKeyConstraint, + TableData, TableName, }; use crate::print_schema::DocConfig; @@ -45,8 +46,7 @@ pub fn generate_sql_based_on_diff_schema( tables_from_schema.visit_file(&syn_file); let mut conn = InferConnection::from_matches(matches); - let tables_from_database = crate::infer_schema_internals::load_table_names(&mut conn, None)? - .filter_table_names(&config); + let tables_from_database = filter_table_names(load_table_names(&mut conn, None)?, &config); let foreign_keys = crate::infer_schema_internals::load_foreign_key_constraints(&mut conn, None)?; diff --git a/diesel_cli/src/print_schema.rs b/diesel_cli/src/print_schema.rs index 21c13686bc47..cccb51b781c6 100644 --- a/diesel_cli/src/print_schema.rs +++ b/diesel_cli/src/print_schema.rs @@ -1,4 +1,4 @@ -use crate::config::{Config, FilteringT}; +use crate::config::Config; use crate::database::{Backend, InferConnection}; use crate::infer_schema_internals::*; @@ -147,8 +147,10 @@ pub fn output_schema( connection: &mut InferConnection, config: &Config, ) -> Result> { - let table_names = load_table_names(connection, config.print_schema.schema_name())? - .filter_table_names(config); + let table_names = filter_table_names( + load_table_names(connection, config.print_schema.schema_name())?, + config, + ); let config = &config.print_schema; From 8e7c563168e8f54429332b28790f8cc785070d2b Mon Sep 17 00:00:00 2001 From: George Manning Date: Wed, 2 Aug 2023 00:56:40 +0100 Subject: [PATCH 05/12] add matching snapshots for sqlite and mysql --- .../diff_table_filter/mysql/initial_schema.sql | 3 +++ .../diff_table_filter/postgres/initial_schema.sql | 13 +++---------- .../diff_table_filter/postgres/up.sql/expected.snap | 4 +++- .../generate_migrations/diff_table_filter/schema.rs | 4 ++-- .../diff_table_filter/sqlite/down.sql/expected.snap | 9 +++++++++ .../diff_table_filter/sqlite/initial_schema.sql | 3 +++ .../diff_table_filter/sqlite/up.sql/expected.snap | 9 +++++++++ 7 files changed, 32 insertions(+), 13 deletions(-) create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/mysql/initial_schema.sql create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/down.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/initial_schema.sql create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/up.sql/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/initial_schema.sql new file mode 100644 index 000000000000..aa1640d6ea45 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/initial_schema.sql @@ -0,0 +1,3 @@ +CREATE TABLE table_a(id INTEGER NOT NULL PRIMARY KEY, code TEXT NOT NULL); +CREATE TABLE table_b(id INTEGER NOT NULL PRIMARY KEY); +CREATE TABLE table_c(id INTEGER NOT NULL PRIMARY KEY); diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/initial_schema.sql index 2e26de15026a..aa1640d6ea45 100644 --- a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/initial_schema.sql +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/initial_schema.sql @@ -1,10 +1,3 @@ -CREATE TABLE table_a( - "id" serial8 NOT NULL PRIMARY KEY, - "code" VARCHAR NOT NULL -); -CREATE TABLE table_b( - "id" serial8 NOT NULL PRIMARY KEY -); -CREATE TABLE table_c( - "id" serial8 NOT NULL PRIMARY KEY -); +CREATE TABLE table_a(id INTEGER NOT NULL PRIMARY KEY, code TEXT NOT NULL); +CREATE TABLE table_b(id INTEGER NOT NULL PRIMARY KEY); +CREATE TABLE table_c(id INTEGER NOT NULL PRIMARY KEY); diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/up.sql/expected.snap index a6fe4bdc077e..021cc8163909 100644 --- a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/up.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/up.sql/expected.snap @@ -4,4 +4,6 @@ description: "Test: diff_table_filter" --- -- Your SQL goes here ALTER TABLE "table_a" DROP COLUMN "code"; -ALTER TABLE "table_a" ADD COLUMN "script" VARCHAR NOT NULL; +ALTER TABLE "table_a" ADD COLUMN "script" TEXT NOT NULL; + + diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/schema.rs b/diesel_cli/tests/generate_migrations/diff_table_filter/schema.rs index 75b76a494726..eea0858b09b3 100644 --- a/diesel_cli/tests/generate_migrations/diff_table_filter/schema.rs +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/schema.rs @@ -1,6 +1,6 @@ diesel::table! { table_a (id) { - id -> Int8, - script -> Varchar, + id -> Integer, + script -> Text, } } diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/down.sql/expected.snap new file mode 100644 index 000000000000..e981945c9b9a --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/down.sql/expected.snap @@ -0,0 +1,9 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +-- This file should undo anything in `up.sql` +ALTER TABLE `table_a` DROP COLUMN `script`; +ALTER TABLE `table_a` ADD COLUMN `code` TEXT NOT NULL; + + diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/initial_schema.sql new file mode 100644 index 000000000000..aa1640d6ea45 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/initial_schema.sql @@ -0,0 +1,3 @@ +CREATE TABLE table_a(id INTEGER NOT NULL PRIMARY KEY, code TEXT NOT NULL); +CREATE TABLE table_b(id INTEGER NOT NULL PRIMARY KEY); +CREATE TABLE table_c(id INTEGER NOT NULL PRIMARY KEY); diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/up.sql/expected.snap new file mode 100644 index 000000000000..7bd6ada40c31 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/up.sql/expected.snap @@ -0,0 +1,9 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +-- Your SQL goes here +ALTER TABLE `table_a` DROP COLUMN `code`; +ALTER TABLE `table_a` ADD COLUMN `script` TEXT NOT NULL; + + From d056ca57d4c078729adcc3f1a7bdcf658d63cb0e Mon Sep 17 00:00:00 2001 From: George Manning Date: Wed, 2 Aug 2023 02:38:03 +0100 Subject: [PATCH 06/12] add missing / updated test snapshots --- .../diff_table_filter/mysql/down.sql/expected.snap | 9 +++++++++ .../mysql/schema_out.rs/expected.snap | 13 +++++++++++++ .../diff_table_filter/mysql/up.sql/expected.snap | 9 +++++++++ .../postgres/down.sql/expected.snap | 2 +- .../postgres/schema_out.rs/expected.snap | 4 ++-- .../sqlite/schema_out.rs/expected.snap | 13 +++++++++++++ 6 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/mysql/down.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/mysql/schema_out.rs/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/mysql/up.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/schema_out.rs/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/down.sql/expected.snap new file mode 100644 index 000000000000..e981945c9b9a --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/down.sql/expected.snap @@ -0,0 +1,9 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +-- This file should undo anything in `up.sql` +ALTER TABLE `table_a` DROP COLUMN `script`; +ALTER TABLE `table_a` ADD COLUMN `code` TEXT NOT NULL; + + diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/schema_out.rs/expected.snap new file mode 100644 index 000000000000..c059cf16b2b5 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/schema_out.rs/expected.snap @@ -0,0 +1,13 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +// @generated automatically by Diesel CLI. + +diesel::table! { + table_a (id) { + id -> Integer, + script -> Text, + } +} + diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/up.sql/expected.snap new file mode 100644 index 000000000000..7bd6ada40c31 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/up.sql/expected.snap @@ -0,0 +1,9 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +-- Your SQL goes here +ALTER TABLE `table_a` DROP COLUMN `code`; +ALTER TABLE `table_a` ADD COLUMN `script` TEXT NOT NULL; + + diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap index d64657440ddf..8508be1f54ee 100644 --- a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap @@ -4,6 +4,6 @@ description: "Test: diff_table_filter" --- -- This file should undo anything in `up.sql` ALTER TABLE "table_a" DROP COLUMN "script"; -ALTER TABLE "table_a" ADD COLUMN "code" VARCHAR NOT NULL; +ALTER TABLE "table_a" ADD COLUMN "code" TEXT NOT NULL; diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/schema_out.rs/expected.snap index f3fd954401b6..19b73314e678 100644 --- a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/schema_out.rs/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/schema_out.rs/expected.snap @@ -6,8 +6,8 @@ description: "Test: diff_table_filter" diesel::table! { table_a (id) { - id -> Int8, - script -> Varchar, + id -> Int4, + script -> Text, } } diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/schema_out.rs/expected.snap new file mode 100644 index 000000000000..c059cf16b2b5 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/schema_out.rs/expected.snap @@ -0,0 +1,13 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +// @generated automatically by Diesel CLI. + +diesel::table! { + table_a (id) { + id -> Integer, + script -> Text, + } +} + From 8b207027b26e6883bf75e6658d7afeb8a26174d6 Mon Sep 17 00:00:00 2001 From: George Manning Date: Wed, 2 Aug 2023 23:28:58 +0100 Subject: [PATCH 07/12] cleanup changes to print_schema --- diesel_cli/src/config.rs | 2 +- .../src/infer_schema_internals/inference.rs | 7 +++-- diesel_cli/src/main.rs | 28 ++++++++----------- diesel_cli/src/migrations/diff_schema.rs | 7 +++-- diesel_cli/src/print_schema.rs | 12 ++++---- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/diesel_cli/src/config.rs b/diesel_cli/src/config.rs index b56c49815684..9486a55a0e8f 100644 --- a/diesel_cli/src/config.rs +++ b/diesel_cli/src/config.rs @@ -50,7 +50,7 @@ impl Config { } } - pub fn set_filters( + pub fn set_filter( mut self, matches: &ArgMatches, ) -> Result> { diff --git a/diesel_cli/src/infer_schema_internals/inference.rs b/diesel_cli/src/infer_schema_internals/inference.rs index 7b9fbfa1bfde..37d5094a2174 100644 --- a/diesel_cli/src/infer_schema_internals/inference.rs +++ b/diesel_cli/src/infer_schema_internals/inference.rs @@ -4,6 +4,9 @@ use diesel::result::Error::NotFound; use super::data_structures::*; use super::table_data::*; + +use crate::config::Filtering; + use crate::database::InferConnection; use crate::print_schema::{ColumnSorting, DocConfig}; @@ -124,10 +127,10 @@ pub fn load_table_names( } } -pub fn filter_table_names(table_names: Vec, config: &crate::Config) -> Vec { +pub fn filter_table_names(table_names: Vec, table_filter: &Filtering) -> Vec { table_names .into_iter() - .filter(|t| !config.print_schema.filter.should_ignore_table(t)) + .filter(|t| !table_filter.should_ignore_table(t)) .collect::<_>() } diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index ec2412fb8052..62f4fb844ea7 100644 --- a/diesel_cli/src/main.rs +++ b/diesel_cli/src/main.rs @@ -219,48 +219,44 @@ fn run_infer_schema(matches: &ArgMatches) -> Result<(), Box("schema") { - config.print_schema.schema = Some(schema_name.clone()) + config.schema = Some(schema_name.clone()) } if matches.get_flag("with-docs") { - config.print_schema.with_docs = - DocConfig::DatabaseCommentsFallbackToAutoGeneratedDocComment; + config.with_docs = DocConfig::DatabaseCommentsFallbackToAutoGeneratedDocComment; } if let Some(sorting) = matches.get_one::("with-docs-config") { - config.print_schema.with_docs = sorting.parse()?; + config.with_docs = sorting.parse()?; } if let Some(sorting) = matches.get_one::("column-sorting") { match sorting as &str { - "ordinal_position" => { - config.print_schema.column_sorting = ColumnSorting::OrdinalPosition - } - "name" => config.print_schema.column_sorting = ColumnSorting::Name, + "ordinal_position" => config.column_sorting = ColumnSorting::OrdinalPosition, + "name" => config.column_sorting = ColumnSorting::Name, _ => return Err(format!("Invalid column sorting mode: {sorting}").into()), } } if let Some(path) = matches.get_one::("patch-file") { - config.print_schema.patch_file = Some(path.clone()); + config.patch_file = Some(path.clone()); } if let Some(types) = matches.get_many("import-types") { let types = types.cloned().collect(); - config.print_schema.import_types = Some(types); + config.import_types = Some(types); } if matches.get_flag("generate-custom-type-definitions") { - config.print_schema.generate_missing_sql_type_definitions = Some(false); + config.generate_missing_sql_type_definitions = Some(false); } if let Some(derives) = matches.get_many("custom-type-derives") { let derives = derives.cloned().collect(); - config.print_schema.custom_type_derives = Some(derives); + config.custom_type_derives = Some(derives); } run_print_schema(&mut conn, &config, &mut stdout())?; @@ -272,8 +268,8 @@ fn regenerate_schema_if_file_specified( ) -> Result<(), Box> { use std::io::Read; - let config = Config::read(matches)?; - if let Some(ref path) = config.print_schema.file { + let config = Config::read(matches)?.print_schema; + if let Some(ref path) = config.file { let mut connection = InferConnection::from_matches(matches); if let Some(parent) = path.parent() { fs::create_dir_all(parent)?; diff --git a/diesel_cli/src/migrations/diff_schema.rs b/diesel_cli/src/migrations/diff_schema.rs index 7438c9a56216..36e381889a89 100644 --- a/diesel_cli/src/migrations/diff_schema.rs +++ b/diesel_cli/src/migrations/diff_schema.rs @@ -32,7 +32,7 @@ pub fn generate_sql_based_on_diff_schema( matches: &ArgMatches, schema_file_path: &Path, ) -> Result<(String, String), Box> { - let config = config.set_filters(matches)?; + let config = config.set_filter(matches)?; let project_root = crate::find_project_root()?; @@ -46,7 +46,10 @@ pub fn generate_sql_based_on_diff_schema( tables_from_schema.visit_file(&syn_file); let mut conn = InferConnection::from_matches(matches); - let tables_from_database = filter_table_names(load_table_names(&mut conn, None)?, &config); + let tables_from_database = filter_table_names( + load_table_names(&mut conn, None)?, + &config.print_schema.filter, + ); let foreign_keys = crate::infer_schema_internals::load_foreign_key_constraints(&mut conn, None)?; diff --git a/diesel_cli/src/print_schema.rs b/diesel_cli/src/print_schema.rs index cccb51b781c6..b11d1ee84846 100644 --- a/diesel_cli/src/print_schema.rs +++ b/diesel_cli/src/print_schema.rs @@ -1,4 +1,4 @@ -use crate::config::Config; +use crate::config; use crate::database::{Backend, InferConnection}; use crate::infer_schema_internals::*; @@ -44,7 +44,7 @@ impl Default for DocConfig { pub fn run_print_schema( connection: &mut InferConnection, - config: &Config, + config: &config::PrintSchema, output: &mut W, ) -> Result<(), Box> { let schema = output_schema(connection, config)?; @@ -145,15 +145,13 @@ fn sqlite_diesel_types() -> HashSet<&'static str> { pub fn output_schema( connection: &mut InferConnection, - config: &Config, + config: &config::PrintSchema, ) -> Result> { let table_names = filter_table_names( - load_table_names(connection, config.print_schema.schema_name())?, - config, + load_table_names(connection, config.schema_name())?, + &config.filter, ); - let config = &config.print_schema; - let foreign_keys = load_foreign_key_constraints(connection, config.schema_name())?; let foreign_keys = remove_unsafe_foreign_keys_for_codegen(connection, &foreign_keys, &table_names); From fe030a0fae53f18d087fa717b260d7ca75c51b27 Mon Sep 17 00:00:00 2001 From: George Manning Date: Thu, 3 Aug 2023 00:18:20 +0100 Subject: [PATCH 08/12] seperate tests for except / only --- .../diff_except_tables/diesel.toml | 2 ++ .../mysql/down.sql/expected.snap | 0 .../mysql/initial_schema.sql | 0 .../mysql/schema_out.rs/expected.snap | 0 .../mysql/up.sql/expected.snap | 0 .../postgres/down.sql/expected.snap | 0 .../postgres/initial_schema.sql | 0 .../postgres/schema_out.rs/expected.snap | 0 .../postgres/up.sql/expected.snap | 0 .../schema.rs | 0 .../sqlite/down.sql/expected.snap | 9 +++++++++ .../sqlite/initial_schema.sql | 0 .../sqlite/schema_out.rs/expected.snap | 0 .../diff_except_tables/sqlite/up.sql/expected.snap | 9 +++++++++ .../diesel.toml | 0 .../mysql}/down.sql/expected.snap | 0 .../diff_only_tables/mysql/initial_schema.sql | 3 +++ .../mysql/schema_out.rs/expected.snap | 13 +++++++++++++ .../mysql}/up.sql/expected.snap | 0 .../postgres/down.sql/expected.snap | 9 +++++++++ .../diff_only_tables/postgres/initial_schema.sql | 3 +++ .../postgres/schema_out.rs/expected.snap | 13 +++++++++++++ .../diff_only_tables/postgres/up.sql/expected.snap | 9 +++++++++ .../generate_migrations/diff_only_tables/schema.rs | 6 ++++++ .../diff_only_tables/sqlite/down.sql/expected.snap | 9 +++++++++ .../diff_only_tables/sqlite/initial_schema.sql | 3 +++ .../sqlite/schema_out.rs/expected.snap | 13 +++++++++++++ .../diff_only_tables/sqlite/up.sql/expected.snap | 9 +++++++++ diesel_cli/tests/migration_generate.rs | 9 +++++++-- 29 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 diesel_cli/tests/generate_migrations/diff_except_tables/diesel.toml rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_except_tables}/mysql/down.sql/expected.snap (100%) rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_except_tables}/mysql/initial_schema.sql (100%) rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_except_tables}/mysql/schema_out.rs/expected.snap (100%) rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_except_tables}/mysql/up.sql/expected.snap (100%) rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_except_tables}/postgres/down.sql/expected.snap (100%) rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_except_tables}/postgres/initial_schema.sql (100%) rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_except_tables}/postgres/schema_out.rs/expected.snap (100%) rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_except_tables}/postgres/up.sql/expected.snap (100%) rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_except_tables}/schema.rs (100%) create mode 100644 diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/down.sql/expected.snap rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_except_tables}/sqlite/initial_schema.sql (100%) rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_except_tables}/sqlite/schema_out.rs/expected.snap (100%) create mode 100644 diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/up.sql/expected.snap rename diesel_cli/tests/generate_migrations/{diff_table_filter => diff_only_tables}/diesel.toml (100%) rename diesel_cli/tests/generate_migrations/{diff_table_filter/sqlite => diff_only_tables/mysql}/down.sql/expected.snap (100%) create mode 100644 diesel_cli/tests/generate_migrations/diff_only_tables/mysql/initial_schema.sql create mode 100644 diesel_cli/tests/generate_migrations/diff_only_tables/mysql/schema_out.rs/expected.snap rename diesel_cli/tests/generate_migrations/{diff_table_filter/sqlite => diff_only_tables/mysql}/up.sql/expected.snap (100%) create mode 100644 diesel_cli/tests/generate_migrations/diff_only_tables/postgres/down.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_only_tables/postgres/initial_schema.sql create mode 100644 diesel_cli/tests/generate_migrations/diff_only_tables/postgres/schema_out.rs/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_only_tables/postgres/up.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_only_tables/schema.rs create mode 100644 diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/down.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/initial_schema.sql create mode 100644 diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/schema_out.rs/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/up.sql/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_except_tables/diesel.toml b/diesel_cli/tests/generate_migrations/diff_except_tables/diesel.toml new file mode 100644 index 000000000000..885aaed16632 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_except_tables/diesel.toml @@ -0,0 +1,2 @@ +[print_schema] +filter = { except_tables = ["table_b", "table_c"] } diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/down.sql/expected.snap similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/mysql/down.sql/expected.snap rename to diesel_cli/tests/generate_migrations/diff_except_tables/mysql/down.sql/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/initial_schema.sql similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/mysql/initial_schema.sql rename to diesel_cli/tests/generate_migrations/diff_except_tables/mysql/initial_schema.sql diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/schema_out.rs/expected.snap similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/mysql/schema_out.rs/expected.snap rename to diesel_cli/tests/generate_migrations/diff_except_tables/mysql/schema_out.rs/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/mysql/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/up.sql/expected.snap similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/mysql/up.sql/expected.snap rename to diesel_cli/tests/generate_migrations/diff_except_tables/mysql/up.sql/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/down.sql/expected.snap similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/postgres/down.sql/expected.snap rename to diesel_cli/tests/generate_migrations/diff_except_tables/postgres/down.sql/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/initial_schema.sql similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/postgres/initial_schema.sql rename to diesel_cli/tests/generate_migrations/diff_except_tables/postgres/initial_schema.sql diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/schema_out.rs/expected.snap similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/postgres/schema_out.rs/expected.snap rename to diesel_cli/tests/generate_migrations/diff_except_tables/postgres/schema_out.rs/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/postgres/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/up.sql/expected.snap similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/postgres/up.sql/expected.snap rename to diesel_cli/tests/generate_migrations/diff_except_tables/postgres/up.sql/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/schema.rs b/diesel_cli/tests/generate_migrations/diff_except_tables/schema.rs similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/schema.rs rename to diesel_cli/tests/generate_migrations/diff_except_tables/schema.rs diff --git a/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/down.sql/expected.snap new file mode 100644 index 000000000000..97ad299447dd --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/down.sql/expected.snap @@ -0,0 +1,9 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_except_tables" +--- +-- This file should undo anything in `up.sql` +ALTER TABLE `table_a` DROP COLUMN `script`; +ALTER TABLE `table_a` ADD COLUMN `code` TEXT NOT NULL; + + diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/initial_schema.sql similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/initial_schema.sql rename to diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/initial_schema.sql diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/schema_out.rs/expected.snap similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/schema_out.rs/expected.snap rename to diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/schema_out.rs/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/up.sql/expected.snap new file mode 100644 index 000000000000..0c3cb1b956a4 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/up.sql/expected.snap @@ -0,0 +1,9 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_except_tables" +--- +-- Your SQL goes here +ALTER TABLE `table_a` DROP COLUMN `code`; +ALTER TABLE `table_a` ADD COLUMN `script` TEXT NOT NULL; + + diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/diesel.toml b/diesel_cli/tests/generate_migrations/diff_only_tables/diesel.toml similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/diesel.toml rename to diesel_cli/tests/generate_migrations/diff_only_tables/diesel.toml diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/down.sql/expected.snap similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/down.sql/expected.snap rename to diesel_cli/tests/generate_migrations/diff_only_tables/mysql/down.sql/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/initial_schema.sql new file mode 100644 index 000000000000..aa1640d6ea45 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/initial_schema.sql @@ -0,0 +1,3 @@ +CREATE TABLE table_a(id INTEGER NOT NULL PRIMARY KEY, code TEXT NOT NULL); +CREATE TABLE table_b(id INTEGER NOT NULL PRIMARY KEY); +CREATE TABLE table_c(id INTEGER NOT NULL PRIMARY KEY); diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/schema_out.rs/expected.snap new file mode 100644 index 000000000000..c059cf16b2b5 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/schema_out.rs/expected.snap @@ -0,0 +1,13 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +// @generated automatically by Diesel CLI. + +diesel::table! { + table_a (id) { + id -> Integer, + script -> Text, + } +} + diff --git a/diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/up.sql/expected.snap similarity index 100% rename from diesel_cli/tests/generate_migrations/diff_table_filter/sqlite/up.sql/expected.snap rename to diesel_cli/tests/generate_migrations/diff_only_tables/mysql/up.sql/expected.snap diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/down.sql/expected.snap new file mode 100644 index 000000000000..8508be1f54ee --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/down.sql/expected.snap @@ -0,0 +1,9 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +-- This file should undo anything in `up.sql` +ALTER TABLE "table_a" DROP COLUMN "script"; +ALTER TABLE "table_a" ADD COLUMN "code" TEXT NOT NULL; + + diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/initial_schema.sql new file mode 100644 index 000000000000..aa1640d6ea45 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/initial_schema.sql @@ -0,0 +1,3 @@ +CREATE TABLE table_a(id INTEGER NOT NULL PRIMARY KEY, code TEXT NOT NULL); +CREATE TABLE table_b(id INTEGER NOT NULL PRIMARY KEY); +CREATE TABLE table_c(id INTEGER NOT NULL PRIMARY KEY); diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/schema_out.rs/expected.snap new file mode 100644 index 000000000000..19b73314e678 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/schema_out.rs/expected.snap @@ -0,0 +1,13 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +// @generated automatically by Diesel CLI. + +diesel::table! { + table_a (id) { + id -> Int4, + script -> Text, + } +} + diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/up.sql/expected.snap new file mode 100644 index 000000000000..021cc8163909 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/up.sql/expected.snap @@ -0,0 +1,9 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +-- Your SQL goes here +ALTER TABLE "table_a" DROP COLUMN "code"; +ALTER TABLE "table_a" ADD COLUMN "script" TEXT NOT NULL; + + diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/schema.rs b/diesel_cli/tests/generate_migrations/diff_only_tables/schema.rs new file mode 100644 index 000000000000..eea0858b09b3 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/schema.rs @@ -0,0 +1,6 @@ +diesel::table! { + table_a (id) { + id -> Integer, + script -> Text, + } +} diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/down.sql/expected.snap new file mode 100644 index 000000000000..e981945c9b9a --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/down.sql/expected.snap @@ -0,0 +1,9 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +-- This file should undo anything in `up.sql` +ALTER TABLE `table_a` DROP COLUMN `script`; +ALTER TABLE `table_a` ADD COLUMN `code` TEXT NOT NULL; + + diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/initial_schema.sql new file mode 100644 index 000000000000..aa1640d6ea45 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/initial_schema.sql @@ -0,0 +1,3 @@ +CREATE TABLE table_a(id INTEGER NOT NULL PRIMARY KEY, code TEXT NOT NULL); +CREATE TABLE table_b(id INTEGER NOT NULL PRIMARY KEY); +CREATE TABLE table_c(id INTEGER NOT NULL PRIMARY KEY); diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/schema_out.rs/expected.snap new file mode 100644 index 000000000000..c059cf16b2b5 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/schema_out.rs/expected.snap @@ -0,0 +1,13 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +// @generated automatically by Diesel CLI. + +diesel::table! { + table_a (id) { + id -> Integer, + script -> Text, + } +} + diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/up.sql/expected.snap new file mode 100644 index 000000000000..7bd6ada40c31 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/up.sql/expected.snap @@ -0,0 +1,9 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_table_filter" +--- +-- Your SQL goes here +ALTER TABLE `table_a` DROP COLUMN `code`; +ALTER TABLE `table_a` ADD COLUMN `script` TEXT NOT NULL; + + diff --git a/diesel_cli/tests/migration_generate.rs b/diesel_cli/tests/migration_generate.rs index 357546239ae4..fa7954a65a98 100644 --- a/diesel_cli/tests/migration_generate.rs +++ b/diesel_cli/tests/migration_generate.rs @@ -256,8 +256,13 @@ fn migration_generate_from_diff_add_table_composite_key() { } #[test] -fn migration_generate_from_diff_filter() { - test_generate_migration("diff_table_filter", vec!["-o", "table_a"]); +fn migration_generate_from_diff_only_tables() { + test_generate_migration("diff_only_tables", vec!["-o", "table_a"]); +} + +#[test] +fn migration_generate_from_diff_except_tables() { + test_generate_migration("diff_except_tables", vec!["-e", "table_b", "table_c"]); } #[cfg(feature = "sqlite")] From 20200487247d84b93cde3aea386d137eba642f4f Mon Sep 17 00:00:00 2001 From: George Manning Date: Thu, 3 Aug 2023 00:52:41 +0100 Subject: [PATCH 09/12] run migration generate tests with config file --- diesel_cli/tests/migration_generate.rs | 20 +++++++++++++++++++- diesel_cli/tests/support/mod.rs | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/diesel_cli/tests/migration_generate.rs b/diesel_cli/tests/migration_generate.rs index fa7954a65a98..877fae197c2c 100644 --- a/diesel_cli/tests/migration_generate.rs +++ b/diesel_cli/tests/migration_generate.rs @@ -4,7 +4,7 @@ use std::{fs::File, io::Read}; use chrono::prelude::*; use regex::Regex; -use crate::support::project; +use crate::support::{project, Project}; pub static TIMESTAMP_FORMAT: &str = "%Y-%m-%d-%H%M%S"; #[test] @@ -283,6 +283,24 @@ fn backend_file_path(test_name: &str, file: &str) -> PathBuf { fn test_generate_migration(test_name: &str, args: Vec<&str>) { let p = project(test_name).build(); + run_generate_migration_test(test_name, args, p); + + let config_path = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests") + .join("generate_migrations") + .join(test_name) + .join("diesel.toml"); + + if Path::new(&config_path).exists() { + let p = project(test_name) + .file("diesel.toml", &read_file(&config_path)) + .build(); + + run_generate_migration_test(test_name, Vec::new(), p); + } +} + +fn run_generate_migration_test(test_name: &str, args: Vec<&str>, p: Project) { let db = crate::support::database(&p.database_url()); p.command("setup").run(); diff --git a/diesel_cli/tests/support/mod.rs b/diesel_cli/tests/support/mod.rs index 56f407bf9cce..10e86f9b2ff2 100644 --- a/diesel_cli/tests/support/mod.rs +++ b/diesel_cli/tests/support/mod.rs @@ -30,7 +30,7 @@ mod postgres_database; #[cfg(rustfmt)] mod sqlite_database; -pub use self::project_builder::project; +pub use self::project_builder::{project, Project}; pub fn database(url: &str) -> database::Database { database::Database::new(url) From d234a8b76dd66b8d38ecc5da1ada67a903bd166c Mon Sep 17 00:00:00 2001 From: George Manning Date: Thu, 3 Aug 2023 00:57:19 +0100 Subject: [PATCH 10/12] revert accidental deleted test --- diesel_cli/tests/migration_generate.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/diesel_cli/tests/migration_generate.rs b/diesel_cli/tests/migration_generate.rs index 877fae197c2c..68ea79808485 100644 --- a/diesel_cli/tests/migration_generate.rs +++ b/diesel_cli/tests/migration_generate.rs @@ -255,6 +255,11 @@ fn migration_generate_from_diff_add_table_composite_key() { test_generate_migration("diff_add_table_composite_key", Vec::new()); } +#[test] +fn migration_generate_from_diff_drop_table_composite_key() { + test_generate_migration("diff_drop_table_composite_key", Vec::new()); +} + #[test] fn migration_generate_from_diff_only_tables() { test_generate_migration("diff_only_tables", vec!["-o", "table_a"]); From 9bb77c6bd72416b863a99c1ca8216202e451efa5 Mon Sep 17 00:00:00 2001 From: George Manning Date: Thu, 3 Aug 2023 01:06:01 +0100 Subject: [PATCH 11/12] refresh snapshots --- .../diff_except_tables/mysql/down.sql/expected.snap | 2 +- .../diff_except_tables/mysql/schema_out.rs/expected.snap | 2 +- .../diff_except_tables/mysql/up.sql/expected.snap | 2 +- .../diff_except_tables/postgres/down.sql/expected.snap | 2 +- .../diff_except_tables/postgres/schema_out.rs/expected.snap | 2 +- .../diff_except_tables/postgres/up.sql/expected.snap | 2 +- .../diff_except_tables/sqlite/schema_out.rs/expected.snap | 2 +- .../diff_only_tables/mysql/down.sql/expected.snap | 2 +- .../diff_only_tables/mysql/schema_out.rs/expected.snap | 2 +- .../diff_only_tables/mysql/up.sql/expected.snap | 2 +- .../diff_only_tables/postgres/down.sql/expected.snap | 2 +- .../diff_only_tables/postgres/schema_out.rs/expected.snap | 2 +- .../diff_only_tables/postgres/up.sql/expected.snap | 2 +- .../diff_only_tables/sqlite/down.sql/expected.snap | 2 +- .../diff_only_tables/sqlite/schema_out.rs/expected.snap | 2 +- .../diff_only_tables/sqlite/up.sql/expected.snap | 2 +- 16 files changed, 16 insertions(+), 16 deletions(-) diff --git a/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/down.sql/expected.snap index e981945c9b9a..97ad299447dd 100644 --- a/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/down.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/down.sql/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_except_tables" --- -- This file should undo anything in `up.sql` ALTER TABLE `table_a` DROP COLUMN `script`; diff --git a/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/schema_out.rs/expected.snap index c059cf16b2b5..6a964649cc86 100644 --- a/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/schema_out.rs/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/schema_out.rs/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_except_tables" --- // @generated automatically by Diesel CLI. diff --git a/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/up.sql/expected.snap index 7bd6ada40c31..0c3cb1b956a4 100644 --- a/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/up.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_except_tables/mysql/up.sql/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_except_tables" --- -- Your SQL goes here ALTER TABLE `table_a` DROP COLUMN `code`; diff --git a/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/down.sql/expected.snap index 8508be1f54ee..39788a83da29 100644 --- a/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/down.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/down.sql/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_except_tables" --- -- This file should undo anything in `up.sql` ALTER TABLE "table_a" DROP COLUMN "script"; diff --git a/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/schema_out.rs/expected.snap index 19b73314e678..a5c7110333b3 100644 --- a/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/schema_out.rs/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/schema_out.rs/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_except_tables" --- // @generated automatically by Diesel CLI. diff --git a/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/up.sql/expected.snap index 021cc8163909..4aee338f541d 100644 --- a/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/up.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_except_tables/postgres/up.sql/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_except_tables" --- -- Your SQL goes here ALTER TABLE "table_a" DROP COLUMN "code"; diff --git a/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/schema_out.rs/expected.snap index c059cf16b2b5..6a964649cc86 100644 --- a/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/schema_out.rs/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_except_tables/sqlite/schema_out.rs/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_except_tables" --- // @generated automatically by Diesel CLI. diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/down.sql/expected.snap index e981945c9b9a..6a5aef194160 100644 --- a/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/down.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/down.sql/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_only_tables" --- -- This file should undo anything in `up.sql` ALTER TABLE `table_a` DROP COLUMN `script`; diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/schema_out.rs/expected.snap index c059cf16b2b5..f9623dc70f1d 100644 --- a/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/schema_out.rs/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/schema_out.rs/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_only_tables" --- // @generated automatically by Diesel CLI. diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/up.sql/expected.snap index 7bd6ada40c31..5aa41c0e26a4 100644 --- a/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/up.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/mysql/up.sql/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_only_tables" --- -- Your SQL goes here ALTER TABLE `table_a` DROP COLUMN `code`; diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/down.sql/expected.snap index 8508be1f54ee..a9e89d29b22a 100644 --- a/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/down.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/down.sql/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_only_tables" --- -- This file should undo anything in `up.sql` ALTER TABLE "table_a" DROP COLUMN "script"; diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/schema_out.rs/expected.snap index 19b73314e678..1356f0468ec6 100644 --- a/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/schema_out.rs/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/schema_out.rs/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_only_tables" --- // @generated automatically by Diesel CLI. diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/up.sql/expected.snap index 021cc8163909..bac54f32c019 100644 --- a/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/up.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/postgres/up.sql/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_only_tables" --- -- Your SQL goes here ALTER TABLE "table_a" DROP COLUMN "code"; diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/down.sql/expected.snap index e981945c9b9a..6a5aef194160 100644 --- a/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/down.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/down.sql/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_only_tables" --- -- This file should undo anything in `up.sql` ALTER TABLE `table_a` DROP COLUMN `script`; diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/schema_out.rs/expected.snap index c059cf16b2b5..f9623dc70f1d 100644 --- a/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/schema_out.rs/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/schema_out.rs/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_only_tables" --- // @generated automatically by Diesel CLI. diff --git a/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/up.sql/expected.snap index 7bd6ada40c31..5aa41c0e26a4 100644 --- a/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/up.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_only_tables/sqlite/up.sql/expected.snap @@ -1,6 +1,6 @@ --- source: diesel_cli/tests/migration_generate.rs -description: "Test: diff_table_filter" +description: "Test: diff_only_tables" --- -- Your SQL goes here ALTER TABLE `table_a` DROP COLUMN `code`; From 0cae0f858c402955d5179fdcbff442e4090b6786 Mon Sep 17 00:00:00 2001 From: George Manning Date: Mon, 14 Aug 2023 20:38:28 +0100 Subject: [PATCH 12/12] update cli help for table-name --- diesel_cli/src/cli.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diesel_cli/src/cli.rs b/diesel_cli/src/cli.rs index 6eee5d0cf68b..25bf28de6028 100644 --- a/diesel_cli/src/cli.rs +++ b/diesel_cli/src/cli.rs @@ -153,7 +153,7 @@ pub fn build_cli() -> Command { .index(2) .num_args(1..) .action(clap::ArgAction::Append) - .help("Table names to filter (default only-tables if not empty)."), + .help("Table names to filter."), ) .arg( Arg::new("only-tables") @@ -223,7 +223,7 @@ pub fn build_cli() -> Command { .index(1) .num_args(1..) .action(clap::ArgAction::Append) - .help("Table names to filter (default only-tables if not empty)."), + .help("Table names to filter."), ) .arg( Arg::new("only-tables")