diff --git a/flareon-cli/src/main.rs b/flareon-cli/src/main.rs index 02c6c03..3e1f8e6 100644 --- a/flareon-cli/src/main.rs +++ b/flareon-cli/src/main.rs @@ -1,5 +1,3 @@ -extern crate core; - mod migration_generator; mod utils; @@ -27,6 +25,9 @@ enum Commands { /// Path to the crate directory to generate migrations for (default: /// current directory) path: Option, + /// Name of the app to use in the migration (default: crate name) + #[arg(long)] + app_name: Option, /// Directory to write the migrations to (default: migrations/ directory /// in the crate's src/ directory) #[arg(long)] @@ -46,9 +47,16 @@ fn main() -> anyhow::Result<()> { .init(); match cli.command { - Commands::MakeMigrations { path, output_dir } => { + Commands::MakeMigrations { + path, + app_name, + output_dir, + } => { let path = path.unwrap_or_else(|| PathBuf::from(".")); - let options = MigrationGeneratorOptions { output_dir }; + let options = MigrationGeneratorOptions { + app_name, + output_dir, + }; make_migrations(&path, options).with_context(|| "unable to create migrations")?; } } diff --git a/flareon-cli/src/migration_generator.rs b/flareon-cli/src/migration_generator.rs index f4015b3..00b6820 100644 --- a/flareon-cli/src/migration_generator.rs +++ b/flareon-cli/src/migration_generator.rs @@ -48,6 +48,7 @@ pub fn make_migrations(path: &Path, options: MigrationGeneratorOptions) -> anyho #[derive(Debug, Clone, Default)] pub struct MigrationGeneratorOptions { + pub app_name: Option, pub output_dir: Option, } @@ -82,6 +83,7 @@ impl MigrationGenerator { Ok(()) } + /// Generate migrations as a ready-to-write source code. pub fn generate_migrations_to_write( &mut self, source_files: Vec, @@ -95,6 +97,8 @@ impl MigrationGenerator { } } + /// Generate migrations and return internal structures that can be used to + /// generate source code. pub fn generate_migrations( &mut self, source_files: Vec, @@ -319,7 +323,10 @@ impl MigrationGenerator { fn make_create_model_operation(app_model: &ModelInSource) -> DynOperation { DynOperation::CreateModel { table_name: app_model.model.table_name.clone(), - model_ty: app_model.model.resolved_ty.clone().expect("resolved_ty is expected to be present when parsing the entire file with symbol resolver"), + model_ty: app_model.model.resolved_ty.clone().expect( + "resolved_ty is expected to be present when \ + parsing the entire file with symbol resolver", + ), fields: app_model.model.fields.clone(), } } @@ -381,7 +388,10 @@ impl MigrationGenerator { fn make_add_field_operation(app_model: &ModelInSource, field: &Field) -> DynOperation { DynOperation::AddField { table_name: app_model.model.table_name.clone(), - model_ty: app_model.model.resolved_ty.clone().expect("resolved_ty is expected to be present when parsing the entire file with symbol resolver"), + model_ty: app_model.model.resolved_ty.clone().expect( + "resolved_ty is expected to be present \ + when parsing the entire file with symbol resolver", + ), field: field.clone(), } } @@ -426,7 +436,7 @@ impl MigrationGenerator { .map(|dependency| dependency.repr()) .collect(); - let app_name = &self.crate_name; + let app_name = self.options.app_name.as_ref().unwrap_or(&self.crate_name); let migration_name = &migration.migration_name; let migration_def = quote! { #[derive(Debug, Copy, Clone)] @@ -666,6 +676,8 @@ pub struct GeneratedMigration { } impl GeneratedMigration { + /// Get the list of [`DynDependency`] for all foreign keys that point + /// to models that are **not** created in this migration. fn get_foreign_key_dependencies(&self) -> Vec { let create_ops = self.get_create_ops_map(); let ops_adding_foreign_keys = self.get_ops_adding_foreign_keys(); @@ -682,6 +694,16 @@ impl GeneratedMigration { dependencies } + /// Removes dependency cycles by removing operations that create cycles. + /// + /// This method tries to minimize the number of operations added by + /// calculating the minimum feedback arc set of the dependency graph. + /// + /// This method modifies the `operations` field in place. + /// + /// # See also + /// + /// * [`Self::remove_dependency`] fn remove_cycles(&mut self) { let graph = self.construct_dependency_graph(); @@ -701,6 +723,11 @@ impl GeneratedMigration { } } + /// Remove a dependency between two operations. + /// + /// This is done by removing foreign keys from the `from` operation that + /// point to the model created by `to` operation, and creating a new + /// `AddField` operation for each removed foreign key. #[must_use] fn remove_dependency(from: &mut DynOperation, to: &DynOperation) -> Vec { match from { @@ -712,7 +739,10 @@ impl GeneratedMigration { let to_type = match to { DynOperation::CreateModel { model_ty, .. } => model_ty, DynOperation::AddField { .. } => { - unreachable!("AddField operation shouldn't be a dependency of CreateModel because it doesn't create a new model") + unreachable!( + "AddField operation shouldn't be a dependency of CreateModel \ + because it doesn't create a new model" + ) } }; trace!( @@ -745,6 +775,18 @@ impl GeneratedMigration { } } + /// Topologically sort operations in this migration. + /// + /// This is to ensure that operations will be applied in the correct order. + /// If there are no dependencies between operations, the order of operations + /// will not be modified. + /// + /// This method modifies the `operations` field in place. + /// + /// # Panics + /// + /// This method should be called after removing cycles; otherwise it will + /// panic. fn toposort_operations(&mut self) { let graph = self.construct_dependency_graph(); @@ -752,11 +794,17 @@ impl GeneratedMigration { .expect("cycles shouldn't exist after removing them"); let mut sorted = sorted .into_iter() - .map(petgraph::prelude::NodeIndex::index) + .map(petgraph::graph::NodeIndex::index) .collect::>(); flareon::__private::apply_permutation(&mut self.operations, &mut sorted); } + /// Construct a graph that represents reverse dependencies between + /// operations in this migration. + /// + /// The graph is directed and has an edge from operation A to operation B + /// if operation B creates a foreign key that points to a model created by + /// operation A. #[must_use] fn construct_dependency_graph(&mut self) -> DiGraph { let create_ops = self.get_create_ops_map(); @@ -769,7 +817,11 @@ impl GeneratedMigration { } for (i, dependency_ty) in &ops_adding_foreign_keys { if let Some(&dependency) = create_ops.get(dependency_ty) { - graph.add_edge(NodeIndex::new(dependency), NodeIndex::new(*i), ()); + graph.add_edge( + petgraph::graph::NodeIndex::new(dependency), + petgraph::graph::NodeIndex::new(*i), + (), + ); } } @@ -855,16 +907,19 @@ impl Repr for Field { let mut tokens = quote! { ::flareon::db::migrations::Field::new(::flareon::db::Identifier::new(#column_name), <#ty as ::flareon::db::DatabaseField>::TYPE) }; - if self - .auto_value - .expect("auto_value is expected to be present when parsing the entire file with symbol resolver") - { + if self.auto_value.expect( + "auto_value is expected to be present \ + when parsing the entire file with symbol resolver", + ) { tokens = quote! { #tokens.auto() } } if self.primary_key { tokens = quote! { #tokens.primary_key() } } - if let Some(fk_spec) = self.foreign_key.clone().expect("foreign_key is expected to be present when parsing the entire file with symbol resolver") { + if let Some(fk_spec) = self.foreign_key.clone().expect( + "foreign_key is expected to be present \ + when parsing the entire file with symbol resolver", + ) { let to_model = &fk_spec.to_model; tokens = quote! { @@ -966,7 +1021,8 @@ fn is_field_foreign_key_to(field: &Field, ty: &syn::Type) -> bool { /// Returns [`None`] if the field is not a foreign key. fn foreign_key_for_field(field: &Field) -> Option { match field.foreign_key.clone().expect( - "foreign_key is expected to be present when parsing the entire file with symbol resolver", + "foreign_key is expected to be present \ + when parsing the entire file with symbol resolver", ) { None => None, Some(foreign_key_spec) => Some(foreign_key_spec.to_model), @@ -1339,4 +1395,59 @@ mod tests { model_type: parse_quote!(crate::Table4), })); } + + #[test] + fn make_add_field_operation() { + let app_model = ModelInSource { + model_item: parse_quote! { + struct TestModel { + id: i32, + field1: i32, + } + }, + model: Model { + name: format_ident!("TestModel"), + original_name: "TestModel".to_string(), + resolved_ty: Some(parse_quote!(TestModel)), + model_type: Default::default(), + table_name: "test_model".to_string(), + pk_field: Field { + field_name: format_ident!("id"), + column_name: "id".to_string(), + ty: parse_quote!(i32), + auto_value: MaybeUnknown::Known(true), + primary_key: true, + unique: false, + foreign_key: MaybeUnknown::Known(None), + }, + fields: vec![], + }, + }; + + let field = Field { + field_name: format_ident!("new_field"), + column_name: "new_field".to_string(), + ty: parse_quote!(i32), + auto_value: MaybeUnknown::Known(false), + primary_key: false, + unique: false, + foreign_key: MaybeUnknown::Known(None), + }; + + let operation = MigrationGenerator::make_add_field_operation(&app_model, &field); + + match operation { + DynOperation::AddField { + table_name, + model_ty, + field: op_field, + } => { + assert_eq!(table_name, "test_model"); + assert_eq!(model_ty, parse_quote!(TestModel)); + assert_eq!(op_field.column_name, "new_field"); + assert_eq!(op_field.ty, parse_quote!(i32)); + } + _ => panic!("Expected AddField operation"), + } + } } diff --git a/flareon-cli/tests/migration_generator.rs b/flareon-cli/tests/migration_generator.rs index 43509e7..e5ba5a7 100644 --- a/flareon-cli/tests/migration_generator.rs +++ b/flareon-cli/tests/migration_generator.rs @@ -120,7 +120,7 @@ fn create_models_foreign_key_cycle() { } #[test] -fn create_models_foreign_two_migrations() { +fn create_models_foreign_key_two_migrations() { let mut generator = test_generator(); let src = include_str!("migration_generator/foreign_key_two_migrations/step_1.rs"); diff --git a/flareon/src/db.rs b/flareon/src/db.rs index 161085e..35ddca4 100644 --- a/flareon/src/db.rs +++ b/flareon/src/db.rs @@ -1163,4 +1163,44 @@ mod tests { LimitedString::<5>::new("test").unwrap(), ); } + + #[test] + fn db_field_value_is_auto() { + let auto_value = DbFieldValue::Auto; + assert!(auto_value.is_auto()); + assert!(!auto_value.is_value()); + } + + #[test] + fn db_field_value_is_value() { + let value = DbFieldValue::Value(42.into()); + assert!(value.is_value()); + assert!(!value.is_auto()); + } + + #[test] + fn db_field_value_unwrap() { + let value = DbFieldValue::Value(42.into()); + assert_eq!(value.unwrap_value(), 42.into()); + } + + #[test] + #[should_panic(expected = "called DbValue::unwrap_value() on a wrong DbValue variant")] + fn db_field_value_unwrap_panic() { + let auto_value = DbFieldValue::Auto; + let _ = auto_value.unwrap_value(); + } + + #[test] + fn db_field_value_expect() { + let value = DbFieldValue::Value(42.into()); + assert_eq!(value.expect_value("expected a value"), 42.into()); + } + + #[test] + #[should_panic(expected = "expected a value")] + fn db_field_value_expect_panic() { + let auto_value = DbFieldValue::Auto; + let _ = auto_value.expect_value("expected a value"); + } } diff --git a/flareon/src/db/migrations.rs b/flareon/src/db/migrations.rs index 0c3422c..f307dfd 100644 --- a/flareon/src/db/migrations.rs +++ b/flareon/src/db/migrations.rs @@ -451,7 +451,7 @@ impl Field { } } -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] struct ForeignKeyReference { model: Identifier, field: Identifier, @@ -848,7 +848,7 @@ mod tests { } #[test] - fn test_field_new() { + fn field_new() { let field = Field::new(Identifier::new("id"), ColumnType::Integer) .primary_key() .auto() @@ -861,6 +861,26 @@ mod tests { assert!(field.null); } + #[test] + fn field_foreign_key() { + let field = Field::new(Identifier::new("parent"), ColumnType::Integer).foreign_key( + Identifier::new("testapp__parent"), + Identifier::new("id"), + ForeignKeyOnDeletePolicy::Restrict, + ForeignKeyOnUpdatePolicy::Restrict, + ); + + assert_eq!( + field.foreign_key, + Some(ForeignKeyReference { + model: Identifier::new("testapp__parent"), + field: Identifier::new("id"), + on_delete: ForeignKeyOnDeletePolicy::Restrict, + on_update: ForeignKeyOnUpdatePolicy::Restrict, + }) + ); + } + #[test] fn test_migration_wrapper() { let migration = MigrationWrapper::new(TestMigration); diff --git a/flareon/src/db/relations.rs b/flareon/src/db/relations.rs index 95e9e3c..b634079 100644 --- a/flareon/src/db/relations.rs +++ b/flareon/src/db/relations.rs @@ -17,6 +17,7 @@ pub enum ForeignKey { } impl ForeignKey { + /// Returns the primary key of the referenced model. pub fn primary_key(&self) -> &T::PrimaryKey { match self { Self::PrimaryKey(pk) => pk, @@ -24,6 +25,8 @@ impl ForeignKey { } } + /// Returns the model, if it has been stored in this [`ForeignKey`] + /// instance, or [`None`] otherwise. pub fn model(&self) -> Option<&T> { match self { Self::Model(model) => Some(model), @@ -31,6 +34,11 @@ impl ForeignKey { } } + /// Unwrap the foreign key, returning the model. + /// + /// # Panics + /// + /// Panics if the model has not been stored in this [`ForeignKey`] instance. pub fn unwrap(self) -> T { match self { Self::Model(model) => *model, @@ -39,6 +47,11 @@ impl ForeignKey { } /// Retrieve the model from the database, if needed, and return it. + /// + /// If the model has already been retrieved, this method will return it. + /// + /// This method will replace the primary key with the model instance if + /// the primary key is stored in this [`ForeignKey`] instance. pub async fn get(&mut self, db: &DB) -> Result<&T> { match self { Self::Model(model) => Ok(model), @@ -131,3 +144,69 @@ impl From for sea_query::ForeignKeyAction { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::db::{model, Auto}; + + #[derive(Debug, Clone, PartialEq)] + #[model] + struct TestModel { + id: Auto, + } + + #[test] + fn test_primary_key() { + let fk = ForeignKey::::PrimaryKey(Auto::fixed(1)); + + assert_eq!(fk.primary_key(), &Auto::fixed(1)); + } + + #[test] + fn test_model() { + let model = TestModel { id: Auto::fixed(1) }; + let fk = ForeignKey::Model(Box::new(model.clone())); + + assert_eq!(fk.model().unwrap(), &model); + assert_eq!(fk.primary_key(), &Auto::fixed(1)); + } + + #[test] + fn test_unwrap_model() { + let model = TestModel { id: Auto::fixed(1) }; + let fk = ForeignKey::Model(Box::new(model.clone())); + + assert_eq!(fk.unwrap(), model); + } + + #[should_panic(expected = "object has not been retrieved from the database")] + fn test_unwrap_primary_key() { + let fk = ForeignKey::::PrimaryKey(Auto::fixed(1)); + fk.unwrap(); + } + + #[test] + fn test_partial_eq() { + let fk1 = ForeignKey::::PrimaryKey(Auto::fixed(1)); + let fk2 = ForeignKey::::PrimaryKey(Auto::fixed(1)); + + assert_eq!(fk1, fk2); + } + + #[test] + fn test_from_model() { + let model = TestModel { id: Auto::fixed(1) }; + let fk: ForeignKey = ForeignKey::from(model.clone()); + + assert_eq!(fk.model().unwrap(), &model); + } + + #[test] + fn test_from_model_ref() { + let model = TestModel { id: Auto::fixed(1) }; + let fk: ForeignKey = ForeignKey::from(&model); + + assert_eq!(fk.primary_key(), &Auto::fixed(1)); + } +}