Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-541 review 3 #250

Merged
merged 10 commits into from
Mar 27, 2023
14 changes: 7 additions & 7 deletions node/src/database/db_migrations/db_migrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::database::db_migrations::migrations::migration_4_to_5::Migrate_4_to_5
use crate::database::db_migrations::migrations::migration_5_to_6::Migrate_5_to_6;
use crate::database::db_migrations::migrations::migration_6_to_7::Migrate_6_to_7;
use crate::database::db_migrations::migrator_utils::{
DBMigDeclarationUtilities, DBMigrationUtilities, DBMigrationUtilitiesReal,
DBMigDeclarator, DBMigrationUtilities, DBMigrationUtilitiesReal,
DBMigratorInnerConfiguration,
};
use masq_lib::logger::Logger;
Expand Down Expand Up @@ -54,7 +54,7 @@ impl DbMigrator for DbMigratorReal {
pub trait DatabaseMigration {
fn migrate<'a>(
&self,
mig_declaration_utilities: Box<dyn DBMigDeclarationUtilities + 'a>,
mig_declaration_utilities: Box<dyn DBMigDeclarator + 'a>,
) -> rusqlite::Result<()>;
fn old_version(&self) -> usize;
}
Expand Down Expand Up @@ -189,7 +189,7 @@ mod tests {
};
use crate::database::db_migrations::migrations::migration_0_to_1::Migrate_0_to_1;
use crate::database::db_migrations::migrator_utils::{
DBMigDeclarationUtilities, DBMigrationUtilities, DBMigrationUtilitiesReal,
DBMigDeclarator, DBMigrationUtilities, DBMigrationUtilitiesReal,
DBMigratorInnerConfiguration,
};
use crate::database::db_migrations::test_utils::DBMigDeclaratorMock;
Expand All @@ -209,7 +209,7 @@ mod tests {
struct DBMigrationUtilitiesMock {
too_high_schema_panics_params: Arc<Mutex<Vec<usize>>>,
make_mig_declarator_params: Arc<Mutex<Vec<ExternalData>>>,
make_mig_declarator_results: RefCell<Vec<Box<dyn DBMigDeclarationUtilities>>>,
make_mig_declarator_results: RefCell<Vec<Box<dyn DBMigDeclarator>>>,
update_schema_version_params: Arc<Mutex<Vec<usize>>>,
update_schema_version_results: RefCell<Vec<rusqlite::Result<()>>>,
commit_results: RefCell<Vec<Result<(), String>>>,
Expand Down Expand Up @@ -241,7 +241,7 @@ mod tests {

pub fn make_mig_declarator_result(
self,
result: Box<dyn DBMigDeclarationUtilities>,
result: Box<dyn DBMigDeclarator>,
) -> Self {
self.make_mig_declarator_results.borrow_mut().push(result);
self
Expand All @@ -265,7 +265,7 @@ mod tests {
&'a self,
external: &'a ExternalData,
_logger: &'a Logger,
) -> Box<dyn DBMigDeclarationUtilities + 'a> {
) -> Box<dyn DBMigDeclarator + 'a> {
self.make_mig_declarator_params
.lock()
.unwrap()
Expand Down Expand Up @@ -319,7 +319,7 @@ mod tests {
impl DatabaseMigration for DatabaseMigrationMock {
fn migrate<'a>(
&self,
_migration_utilities: Box<dyn DBMigDeclarationUtilities + 'a>,
_migration_utilities: Box<dyn DBMigDeclarator + 'a>,
) -> rusqlite::Result<()> {
self.migrate_params.lock().unwrap().push(());
self.migrate_results.borrow_mut().remove(0)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
// Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved.

use crate::database::db_migrations::db_migrator::DatabaseMigration;
use crate::database::db_migrations::migrator_utils::DBMigDeclarationUtilities;
use crate::database::db_migrations::migrator_utils::DBMigDeclarator;

#[allow(non_camel_case_types)]
pub struct Migrate_0_to_1;

impl DatabaseMigration for Migrate_0_to_1 {
fn migrate<'a>(
&self,
declaration_utils: Box<dyn DBMigDeclarationUtilities + 'a>,
declaration_utils: Box<dyn DBMigDeclarator + 'a>,
) -> rusqlite::Result<()> {
declaration_utils.execute_upon_transaction(&[
&"INSERT INTO config (name, value, encrypted) VALUES ('mapping_protocol', null, 0)",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
// Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved.

use crate::database::db_migrations::db_migrator::DatabaseMigration;
use crate::database::db_migrations::migrator_utils::DBMigDeclarationUtilities;
use crate::database::db_migrations::migrator_utils::DBMigDeclarator;

#[allow(non_camel_case_types)]
pub struct Migrate_1_to_2;

impl DatabaseMigration for Migrate_1_to_2 {
fn migrate<'a>(
&self,
declaration_utils: Box<dyn DBMigDeclarationUtilities + 'a>,
declaration_utils: Box<dyn DBMigDeclarator + 'a>,
) -> rusqlite::Result<()> {
let statement = format!(
"INSERT INTO config (name, value, encrypted) VALUES ('chain_name', '{}', 0)",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
// Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved.

use crate::database::db_migrations::db_migrator::DatabaseMigration;
use crate::database::db_migrations::migrator_utils::DBMigDeclarationUtilities;
use crate::database::db_migrations::migrator_utils::DBMigDeclarator;

#[allow(non_camel_case_types)]
pub struct Migrate_2_to_3;

impl DatabaseMigration for Migrate_2_to_3 {
fn migrate<'a>(
&self,
declaration_utils: Box<dyn DBMigDeclarationUtilities + 'a>,
declaration_utils: Box<dyn DBMigDeclarator + 'a>,
) -> rusqlite::Result<()> {
let statement_1 =
"INSERT INTO config (name, value, encrypted) VALUES ('blockchain_service_url', null, 0)";
Expand Down
33 changes: 24 additions & 9 deletions node/src/database/db_migrations/migrations/migration_3_to_4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use crate::blockchain::bip39::Bip39;
use crate::database::db_migrations::db_migrator::DatabaseMigration;
use crate::database::db_migrations::migrator_utils::DBMigDeclarationUtilities;
use crate::database::db_migrations::migrator_utils::DBMigDeclarator;
use crate::db_config::db_encryption_layer::DbEncryptionLayer;
use crate::db_config::typed_config_layer::decode_bytes;
use crate::sub_lib::cryptde::PlainData;
Expand All @@ -17,7 +17,7 @@ impl Migrate_3_to_4 {
consuming_path_opt: Option<String>,
example_encrypted_opt: Option<String>,
seed_encrypted_opt: Option<String>,
utils: &dyn DBMigDeclarationUtilities,
utils: &dyn DBMigDeclarator,
) -> Option<String> {
match (
consuming_path_opt,
Expand Down Expand Up @@ -49,16 +49,17 @@ impl Migrate_3_to_4 {
)
}
(None, None, None) => None,
(consuming_path_opt, example_encrypted_opt, seed_encrypted_otp) => panic!(
(None, Some(_), None) => None,
(consuming_path_opt, example_encrypted_opt, seed_encrypted_opt) => panic!(
"these three options {:?}, {:?}, {:?} leave the database in an inconsistent state",
consuming_path_opt, example_encrypted_opt, seed_encrypted_otp
consuming_path_opt, example_encrypted_opt, seed_encrypted_opt
),
}
}
}

impl DatabaseMigration for Migrate_3_to_4 {
fn migrate<'a>(&self, utils: Box<dyn DBMigDeclarationUtilities + 'a>) -> rusqlite::Result<()> {
fn migrate<'a>(&self, utils: Box<dyn DBMigDeclarator + 'a>) -> rusqlite::Result<()> {
let transaction = utils.transaction();
let mut stmt = transaction
.prepare("select name, value from config where name in ('example_encrypted', 'seed', 'consuming_wallet_derivation_path') order by name")
Expand All @@ -85,13 +86,13 @@ impl DatabaseMigration for Migrate_3_to_4 {
let consuming_path_opt = rows[0].1.clone();
let example_encrypted_opt = rows[1].1.clone();
let seed_encrypted_opt = rows[2].1.clone();
let private_key_encoded = Self::maybe_exchange_seed_for_private_key(
let private_key_encoded_opt = Self::maybe_exchange_seed_for_private_key(
consuming_path_opt,
example_encrypted_opt,
seed_encrypted_opt,
utils.as_ref(),
);
let private_key_column = if let Some(private_key) = private_key_encoded {
let private_key_column = if let Some(private_key) = private_key_encoded_opt {
format!("'{}'", private_key)
} else {
"null".to_string()
Expand Down Expand Up @@ -272,6 +273,21 @@ mod tests {
);
}

#[test]
fn database_with_password_but_without_secrets_yet_still_accepted() {
let mig_declarator = DBMigDeclaratorMock::default();
let example_encrypted_opt = Some("Password".to_string());
Syther007 marked this conversation as resolved.
Show resolved Hide resolved

let result = Migrate_3_to_4::maybe_exchange_seed_for_private_key(
None,
example_encrypted_opt,
None,
&mig_declarator,
);

assert_eq!(result, None);
}

fn catch_panic_for_maybe_exchange_seed_for_private_key_with_corrupt_database(
consuming_path_opt: Option<&str>,
example_encrypted_opt: Option<&str>,
Expand All @@ -290,8 +306,7 @@ mod tests {
)
}))
.unwrap_err();
let panic_message = panic.downcast_ref::<String>().unwrap();
panic_message.to_owned()
panic.downcast_ref::<String>().unwrap().to_owned()
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

use crate::accountant::dao_utils::VigilantRusqliteFlatten;
use crate::database::db_migrations::db_migrator::DatabaseMigration;
use crate::database::db_migrations::migrator_utils::DBMigDeclarationUtilities;
use crate::database::db_migrations::migrator_utils::DBMigDeclarator;

#[allow(non_camel_case_types)]
pub struct Migrate_4_to_5;

impl DatabaseMigration for Migrate_4_to_5 {
fn migrate<'a>(&self, utils: Box<dyn DBMigDeclarationUtilities + 'a>) -> rusqlite::Result<()> {
fn migrate<'a>(&self, utils: Box<dyn DBMigDeclarator + 'a>) -> rusqlite::Result<()> {
let mut select_statement = utils
.transaction()
.prepare("select pending_payment_transaction from payable where pending_payment_transaction is not null")?;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved.

use crate::database::db_migrations::db_migrator::DatabaseMigration;
use crate::database::db_migrations::migrator_utils::DBMigDeclarationUtilities;
use crate::database::db_migrations::migrator_utils::DBMigDeclarator;
use crate::sub_lib::accountant::{DEFAULT_PAYMENT_THRESHOLDS, DEFAULT_SCAN_INTERVALS};
use crate::sub_lib::neighborhood::DEFAULT_RATE_PACK;

Expand All @@ -11,7 +11,7 @@ pub struct Migrate_5_to_6;
impl DatabaseMigration for Migrate_5_to_6 {
fn migrate<'a>(
&self,
declaration_utils: Box<dyn DBMigDeclarationUtilities + 'a>,
declaration_utils: Box<dyn DBMigDeclarator + 'a>,
) -> rusqlite::Result<()> {
let statement_1 = Self::make_initialization_statement(
"payment_thresholds",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::accountant::dao_utils::VigilantRusqliteFlatten;
use crate::accountant::gwei_to_wei;
use crate::database::db_migrations::db_migrator::DatabaseMigration;
use crate::database::db_migrations::migrator_utils::{
DBMigDeclarationUtilities, StatementObject, StatementWithRusqliteParams,
DBMigDeclarator, StatementObject, StatementWithRusqliteParams,
};
use crate::sub_lib::neighborhood::RatePack;
use itertools::Itertools;
Expand All @@ -15,7 +15,7 @@ use rusqlite::{Row, ToSql};
pub struct Migrate_6_to_7;

impl DatabaseMigration for Migrate_6_to_7 {
fn migrate<'a>(&self, utils: Box<dyn DBMigDeclarationUtilities + 'a>) -> rusqlite::Result<()> {
fn migrate<'a>(&self, utils: Box<dyn DBMigDeclarator + 'a>) -> rusqlite::Result<()> {
let mut migration_carrier = Migrate_6_to_7_carrier::new(utils.as_ref());
migration_carrier.retype_table(
"payable",
Expand Down Expand Up @@ -64,12 +64,12 @@ impl DatabaseMigration for Migrate_6_to_7 {

#[allow(non_camel_case_types)]
struct Migrate_6_to_7_carrier<'a> {
utils: &'a (dyn DBMigDeclarationUtilities + 'a),
utils: &'a (dyn DBMigDeclarator + 'a),
statements: Vec<Box<dyn StatementObject>>,
}

impl<'a> Migrate_6_to_7_carrier<'a> {
fn new(utils: &'a (dyn DBMigDeclarationUtilities + 'a)) -> Self {
fn new(utils: &'a (dyn DBMigDeclarator + 'a)) -> Self {
Self {
utils,
statements: vec![],
Expand Down
17 changes: 8 additions & 9 deletions node/src/database/db_migrations/migrator_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use masq_lib::utils::{ExpectValue, WrapResult};
use rusqlite::{params_from_iter, Error, ToSql, Transaction};
use std::fmt::{Display, Formatter};

pub trait DBMigDeclarationUtilities {
pub trait DBMigDeclarator {
fn db_password(&self) -> Option<String>;
fn transaction(&self) -> &Transaction;
fn execute_upon_transaction<'a>(
Expand All @@ -28,7 +28,7 @@ pub trait DBMigrationUtilities {
&'a self,
external: &'a ExternalData,
logger: &'a Logger,
) -> Box<dyn DBMigDeclarationUtilities + 'a>;
) -> Box<dyn DBMigDeclarator + 'a>;

fn too_high_schema_panics(&self, obsolete_schema: usize);
}
Expand Down Expand Up @@ -78,8 +78,8 @@ impl<'a> DBMigrationUtilities for DBMigrationUtilitiesReal<'a> {
&'b self,
external: &'b ExternalData,
logger: &'b Logger,
) -> Box<dyn DBMigDeclarationUtilities + 'b> {
Box::new(DBMigDeclarationUtilitiesReal::new(
) -> Box<dyn DBMigDeclarator + 'b> {
Box::new(DBMigDeclaratorReal::new(
self.root_transaction_ref(),
external,
logger,
Expand All @@ -97,13 +97,13 @@ impl<'a> DBMigrationUtilities for DBMigrationUtilitiesReal<'a> {
}
}

struct DBMigDeclarationUtilitiesReal<'a> {
struct DBMigDeclaratorReal<'a> {
root_transaction_ref: &'a Transaction<'a>,
external: &'a ExternalData,
logger: &'a Logger,
}

impl<'a> DBMigDeclarationUtilitiesReal<'a> {
impl<'a> DBMigDeclaratorReal<'a> {
fn new(
root_transaction_ref: &'a Transaction<'a>,
external: &'a ExternalData,
Expand All @@ -117,7 +117,7 @@ impl<'a> DBMigDeclarationUtilitiesReal<'a> {
}
}

impl DBMigDeclarationUtilities for DBMigDeclarationUtilitiesReal<'_> {
impl DBMigDeclarator for DBMigDeclaratorReal<'_> {
fn db_password(&self) -> Option<String> {
self.external.db_password_opt.clone()
}
Expand All @@ -132,7 +132,6 @@ impl DBMigDeclarationUtilities for DBMigDeclarationUtilitiesReal<'_> {
) -> rusqlite::Result<()> {
let transaction = self.root_transaction_ref;
sql_statements.iter().fold(Ok(()), |so_far, stm| {
println!("{}", stm);
if so_far.is_ok() {
match stm.execute(transaction) {
Ok(_) => Ok(()),
Expand Down Expand Up @@ -216,7 +215,7 @@ struct InterimMigrationPlaceholder(usize);
impl DatabaseMigration for InterimMigrationPlaceholder {
fn migrate<'a>(
&self,
_mig_declaration_utilities: Box<dyn DBMigDeclarationUtilities + 'a>,
_mig_declaration_utilities: Box<dyn DBMigDeclarator + 'a>,
) -> rusqlite::Result<()> {
Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions node/src/database/db_migrations/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#![cfg(test)]

use crate::database::db_initializer::ExternalData;
use crate::database::db_migrations::migrator_utils::{DBMigDeclarationUtilities, StatementObject};
use crate::database::db_migrations::migrator_utils::{DBMigDeclarator, StatementObject};
use masq_lib::logger::Logger;
use rusqlite::Transaction;
use std::cell::RefCell;
Expand Down Expand Up @@ -37,7 +37,7 @@ impl DBMigDeclaratorMock {
}
}

impl DBMigDeclarationUtilities for DBMigDeclaratorMock {
impl DBMigDeclarator for DBMigDeclaratorMock {
fn db_password(&self) -> Option<String> {
self.db_password_results.borrow_mut().remove(0)
}
Expand Down