From fd62944efc75b35d180e7026c6dfe40c3d99bb3e Mon Sep 17 00:00:00 2001
From: Robert Habermeier <rphmeier@gmail.com>
Date: Tue, 5 Jul 2016 18:50:46 +0200
Subject: [PATCH] address comments

---
 parity/migration.rs         | 16 +++++++---------
 util/src/migration/mod.rs   |  5 ++++-
 util/src/migration/tests.rs |  5 +++--
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/parity/migration.rs b/parity/migration.rs
index b7cdecb16..ebe7aad03 100644
--- a/parity/migration.rs
+++ b/parity/migration.rs
@@ -20,7 +20,6 @@ use std::io::{Read, Write, Error as IoError, ErrorKind};
 use std::path::PathBuf;
 use std::fmt::{Display, Formatter, Error as FmtError};
 use util::migration::{Manager as MigrationManager, Config as MigrationConfig, Error as MigrationError};
-use util::kvdb::{Database, DatabaseConfig, CompactionProfile};
 use ethcore::migrations;
 
 /// Database is assumed to be at default version, when no version file is found.
@@ -148,27 +147,26 @@ fn extras_database_migrations() -> Result<MigrationManager, Error> {
 }
 
 /// Migrates database at given position with given migration rules.
-fn migrate_database(version: u32, path: PathBuf, migrations: MigrationManager) -> Result<(), Error> {
+fn migrate_database(version: u32, db_path: PathBuf, migrations: MigrationManager) -> Result<(), Error> {
 	// check if migration is needed
 	if !migrations.is_needed(version) {
 		return Ok(())
 	}
 
-	let backup_path = backup_database_path(&path);
-	// remove the dir if it exists
-	let _ = fs::remove_dir_all(&temp_path);
+	let backup_path = backup_database_path(&db_path);
+	// remove the backup dir if it exists
 	let _ = fs::remove_dir_all(&backup_path);
 
 	// migrate old database to the new one
-	let temp_path = try!(migrations.execute(version));
+	let temp_path = try!(migrations.execute(&db_path, version));
 
 	// create backup
-	try!(fs::rename(&path, &backup_path));
+	try!(fs::rename(&db_path, &backup_path));
 
 	// replace the old database with the new one
-	if let Err(err) = fs::rename(&temp_path, &path) {
+	if let Err(err) = fs::rename(&temp_path, &db_path) {
 		// if something went wrong, bring back backup
-		try!(fs::rename(&backup_path, path));
+		try!(fs::rename(&backup_path, &db_path));
 		return Err(err.into());
 	}
 
diff --git a/util/src/migration/mod.rs b/util/src/migration/mod.rs
index 80ca3b401..a964d35df 100644
--- a/util/src/migration/mod.rs
+++ b/util/src/migration/mod.rs
@@ -88,10 +88,13 @@ impl<T: SimpleMigration> Migration for T {
 
 			if batch.len() == config.batch_size {
 				try!(commit_batch(dest, &batch));
+				batch.clear();
 			}
 		}
 
-		try!(commit_batch(dest, &batch));
+		if batch.len() != 0 {
+			try!(commit_batch(dest, &batch));
+		}
 
 		Ok(())
 	}
diff --git a/util/src/migration/tests.rs b/util/src/migration/tests.rs
index e17a194b8..f441b31e4 100644
--- a/util/src/migration/tests.rs
+++ b/util/src/migration/tests.rs
@@ -15,7 +15,8 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 //! Tests for migrations.
-//! A random temp directory is created. A database
+//! A random temp directory is created. A database is created within it, and migrations
+//! are performed in temp sub-directories.
 
 use common::*;
 use migration::{Config, SimpleMigration, Manager};
@@ -43,7 +44,7 @@ fn make_db(path: &Path, pairs: BTreeMap<Vec<u8>, Vec<u8>>) {
 	}
 }
 
-// helper for verifying a a migrated database.
+// helper for verifying a migrated database.
 fn verify_migration(path: &Path, pairs: BTreeMap<Vec<u8>, Vec<u8>>) {
 	let db = Database::open_default(path.to_str().unwrap()).unwrap();