From 1df84a8e28646688d9d68352b71870e2cfebd047 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= <tomusdrw@users.noreply.github.com>
Date: Tue, 18 Oct 2016 16:34:58 +0200
Subject: [PATCH] fix migration system, better errors (#2661)

---
 parity/migration.rs       | 11 +++++++----
 util/src/migration/mod.rs | 27 ++++++++++++++++++---------
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/parity/migration.rs b/parity/migration.rs
index 2a6a949a6..7ae631c91 100644
--- a/parity/migration.rs
+++ b/parity/migration.rs
@@ -43,13 +43,15 @@ pub enum Error {
 	/// Returned when current version cannot be read or guessed.
 	UnknownDatabaseVersion,
 	/// Migration does not support existing pruning algorithm.
-	UnsuportedPruningMethod,
+	UnsupportedPruningMethod,
 	/// Existing DB is newer than the known one.
 	FutureDBVersion,
 	/// Migration is not possible.
 	MigrationImpossible,
 	/// Migration unexpectadly failed.
 	MigrationFailed,
+	/// Internal migration error.
+	Internal(MigrationError),
 	/// Migration was completed succesfully,
 	/// but there was a problem with io.
 	Io(IoError),
@@ -59,10 +61,11 @@ impl Display for Error {
 	fn fmt(&self, f: &mut Formatter) -> Result<(), FmtError> {
 		let out = match *self {
 			Error::UnknownDatabaseVersion => "Current database version cannot be read".into(),
-			Error::UnsuportedPruningMethod => "Unsupported pruning method for database migration. Delete DB and resync.".into(),
+			Error::UnsupportedPruningMethod => "Unsupported pruning method for database migration. Delete DB and resync.".into(),
 			Error::FutureDBVersion => "Database was created with newer client version. Upgrade your client or delete DB and resync.".into(),
 			Error::MigrationImpossible => format!("Database migration to version {} is not possible.", CURRENT_VERSION),
 			Error::MigrationFailed => "Database migration unexpectedly failed".into(),
+			Error::Internal(ref err) => format!("{}", err),
 			Error::Io(ref err) => format!("Unexpected io error on DB migration: {}.", err),
 		};
 
@@ -80,7 +83,7 @@ impl From<MigrationError> for Error {
 	fn from(err: MigrationError) -> Self {
 		match err {
 			MigrationError::Io(e) => Error::Io(e),
-			_ => Error::MigrationFailed,
+			_ => Error::Internal(err),
 		}
 	}
 }
@@ -336,7 +339,7 @@ mod legacy {
 		let res = match pruning {
 			Algorithm::Archive => manager.add_migration(migrations::state::ArchiveV7::default()),
 			Algorithm::OverlayRecent => manager.add_migration(migrations::state::OverlayRecentV7::default()),
-			_ => return Err(Error::UnsuportedPruningMethod),
+			_ => return Err(Error::UnsupportedPruningMethod),
 		};
 
 		try!(res.map_err(|_| Error::MigrationImpossible));
diff --git a/util/src/migration/mod.rs b/util/src/migration/mod.rs
index b15af50b4..5f89169d8 100644
--- a/util/src/migration/mod.rs
+++ b/util/src/migration/mod.rs
@@ -20,6 +20,7 @@ mod tests;
 
 use std::collections::BTreeMap;
 use std::fs;
+use std::fmt;
 use std::path::{Path, PathBuf};
 
 use ::kvdb::{CompactionProfile, Database, DatabaseConfig, DBTransaction};
@@ -96,6 +97,17 @@ pub enum Error {
 	Custom(String),
 }
 
+impl fmt::Display for Error {
+	fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
+		match *self {
+			Error::CannotAddMigration => write!(f, "Cannot add migration"),
+			Error::MigrationImpossible => write!(f, "Migration impossible"),
+			Error::Io(ref err) => write!(f, "{}", err),
+			Error::Custom(ref err) => write!(f, "{}", err),
+		}
+	}
+}
+
 impl From<::std::io::Error> for Error {
 	fn from(e: ::std::io::Error) -> Self {
 		Error::Io(e)
@@ -110,6 +122,8 @@ impl From<String> for Error {
 
 /// A generalized migration from the given db to a destination db.
 pub trait Migration: 'static {
+	/// Number of columns in the database before the migration.
+	fn pre_columns(&self) -> Option<u32> { self.columns() }
 	/// Number of columns in database after the migration.
 	fn columns(&self) -> Option<u32>;
 	/// Version of the database after the migration.
@@ -201,6 +215,7 @@ impl Manager {
 			Some(last) => migration.version() > last.version(),
 			None => true,
 		};
+
 		match is_new {
 			true => Ok(self.migrations.push(Box::new(migration))),
 			false => Err(Error::CannotAddMigration),
@@ -211,9 +226,11 @@ impl Manager {
 	/// and producing a path where the final migration lives.
 	pub fn execute(&mut self, old_path: &Path, version: u32) -> Result<PathBuf, Error> {
 		let config = self.config.clone();
-		let columns = self.no_of_columns_at(version);
 		let migrations = self.migrations_from(version);
 		if migrations.is_empty() { return Err(Error::MigrationImpossible) };
+
+		let columns = migrations.iter().find(|m| m.version() == version).and_then(|m| m.pre_columns());
+
 		let mut db_config = DatabaseConfig {
 			max_open_files: 64,
 			cache_size: None,
@@ -273,14 +290,6 @@ impl Manager {
 	fn migrations_from(&mut self, version: u32) -> Vec<&mut Box<Migration>> {
 		self.migrations.iter_mut().filter(|m| m.version() > version).collect()
 	}
-
-	fn no_of_columns_at(&self, version: u32) -> Option<u32> {
-		let migration = self.migrations.iter().find(|m| m.version() == version);
-		match migration {
-			Some(m) => m.columns(),
-			None => None
-		}
-	}
 }
 
 /// Prints a dot every `max` ticks