Skip to content

Commit

Permalink
refactor: consolidate multiple plan error to single error variant
Browse files Browse the repository at this point in the history
Signed-off-by: Saurav Sharma <[email protected]>
  • Loading branch information
iamsauravsharma committed Aug 13, 2024
1 parent 230742c commit 1abb204
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 59 deletions.
51 changes: 7 additions & 44 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,62 +14,25 @@ pub enum Error {
#[cfg(feature = "cli")]
#[error(transparent)]
StdIo(#[from] std::io::Error),
/// Error generated when no migration are added
#[error("no migration are added to migration list")]
NoMigrationAdded,
/// Error generated if virtual migration is present while running generate
/// migration plan
#[error("virtual migrations which is not replaced is present in migration plan")]
VirtualMigrationPresent,
/// Error for failed to create migrations plan
#[error("failed to create migrations plan")]
FailedToCreateMigrationPlan,
/// Parent is not applied
#[error("children is applied before parent")]
ParentIsNotApplied,
/// Error when one migration is replaced by multiple times which is not
/// supported
#[error("migration is replaced multiple times")]
MigrationReplacedMultipleTimes,
/// Error when migration plan has applied replaces migrations as well as
/// current migration
#[error("both replace migrations and current migration are applied")]
BothMigrationTypeApplied,
/// Error generated during planning state
#[error("planning error: {message}")]
PlanError {
/// Message for error
message: String,
},
/// Error for irreversible operation
#[error("operation is irreversible")]
IrreversibleOperation,
/// Error for pending migration present
#[cfg(feature = "cli")]
#[error("pending migration present")]
PendingMigrationPresent,
/// Error when provided app name doesn't exists
#[error("provided app {app} doesn't exists")]
AppNameNotExists {
/// Name of app
app: String,
},
/// Error when provided migration name doesn't exists for app
#[error("provided migration {migration} doesn't exists for app {app}")]
MigrationNameNotExists {
/// Name of app
app: String,
/// Name of migration
migration: String,
},
/// Error when applied migrations exists
#[cfg(feature = "cli")]
#[error("applied migrations exists. Revert all using revert subcommand")]
AppliedMigrationExists,
/// Error when count of migrations is big than total number of migration
#[error("count of migrations is big only {actual_len} present passed {count}")]
CountGreater {
/// Actual length of migration
actual_len: usize,
/// Count passed in option
count: usize,
},
/// Error when unsupported database is used as any database
#[error("database not supported for any migrator")]
#[error("database not supported")]
UnsupportedDatabase,
/// Error when passed prefix is not alpha numeric
#[error("prefix can only be ascii alphanumeric and underscore character")]
Expand Down
65 changes: 50 additions & 15 deletions src/migrator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,15 @@ fn populate_recursive<'populate, DB, State>(
) -> Result<(), Error> {
// protect against a case where two migration depends upon each other
if key == value {
return Err(Error::FailedToCreateMigrationPlan);
return Err(Error::PlanError {
message: format!(
"migration {}:{} and migration {}:{} depends with each other",
key.app(),
key.name(),
value.app(),
value.name()
),
});
}
let populate_hash_map_vec = populate_hash_map.entry(key).or_default();
if !populate_hash_map_vec.contains(&value) {
Expand Down Expand Up @@ -474,13 +482,12 @@ where
.iter()
.any(|migration| migration.app() == app)
{
return Err(Error::MigrationNameNotExists {
app: app.to_string(),
migration: name.to_string(),
return Err(Error::PlanError {
message: format!("migration {app}:{name} doesn't exists for app"),
});
}
return Err(Error::AppNameNotExists {
app: app.to_string(),
return Err(Error::PlanError {
message: format!("app {app} doesn't exists"),
});
};
pos
Expand All @@ -489,8 +496,8 @@ where
.iter()
.rposition(|migration| migration.app() == app)
else {
return Err(Error::AppNameNotExists {
app: app.to_string(),
return Err(Error::PlanError {
message: format!("app {app} doesn't exists"),
});
};
pos
Expand All @@ -509,7 +516,11 @@ where
} else if let Some(count) = plan.count {
let actual_len = migration_list.len();
if count > actual_len {
return Err(Error::CountGreater { actual_len, count });
return Err(Error::PlanError {
message: format!(
"passed count value is larger than migration length: {actual_len}"
),
});
}
migration_list.truncate(count);
}
Expand Down Expand Up @@ -547,14 +558,18 @@ where
plan: Option<&Plan>,
) -> MigrationVecResult<DB, State> {
if self.migrations().is_empty() {
return Err(Error::NoMigrationAdded);
return Err(Error::PlanError {
message: "no migration are added to migration list".to_string(),
});
}
if self
.migrations()
.iter()
.any(|migration| migration.is_virtual())
{
return Err(Error::VirtualMigrationPresent);
return Err(Error::PlanError {
message: "virtual migrations which is not replaced is present".to_string(),
});
}

tracing::debug!("generating {:?} migration plan", plan);
Expand All @@ -565,11 +580,14 @@ where

for parent_migration in self.migrations() {
for child_migration in parent_migration.replaces() {
let child_name = format!("{}:{}", child_migration.app(), child_migration.name());
if parent_due_to_replaces
.insert(child_migration, parent_migration)
.is_some()
{
return Err(Error::MigrationReplacedMultipleTimes);
return Err(Error::PlanError {
message: format!("migration {child_name} replaced multiple times",),
});
}
}
}
Expand Down Expand Up @@ -650,7 +668,9 @@ where
// can arise due to looping in migration plan i.e If there is two migration A
// and B, than when B is ancestor of A as well as descendants of A
if loop_initial_migration_list_length == migration_list.len() {
return Err(Error::FailedToCreateMigrationPlan);
return Err(Error::PlanError {
message: "reached deadlock stage during plan generation".to_string(),
});
}
}

Expand Down Expand Up @@ -696,7 +716,16 @@ where
.iter()
.any(|applied| recursive_vec.contains(applied))
{
return Err(Error::ParentIsNotApplied);
return Err(Error::PlanError {
message: format!(
"children migration {}:{} applied before its parent migration \
{}:{}",
migration.app(),
migration.name(),
parent.app(),
parent.name()
),
});
}
}
}
Expand All @@ -715,7 +744,13 @@ where
if replaces_applied {
// Error if current migration as well as replace migration both are applied
if applied_migrations.contains(&migration) {
return Err(Error::BothMigrationTypeApplied);
return Err(Error::PlanError {
message: format!(
"migration {}:{} and its replaces are applied together",
migration.app(),
migration.name(),
),
});
}
migration_list.retain(|&plan_migration| migration != plan_migration);
} else {
Expand Down

0 comments on commit 1abb204

Please sign in to comment.