GH 624 - Simplify ConfigDao to reduce or eliminate SQLITE_BLOCKED errors (#157)

* Removed extract() from ConfigDao

* Seems like it's working

* Review issues

* Formatting

* Versions

* GH-624: Don't persist neighbors if they're the same as last time

* GH-624: Now warning and skipping past-neighbor storage that fails because of a locked database

* GH-627: Clippy should be happy again by now

* GH-627: one line was silly

* GH-627: starting ignoring the troublesome test again

* GH-627: there was a formatting issue

* handles_startup_and_shutdown_integration now doesn't run in Actions on Windows

* handles_startup_and_shutdown_integration now doesn't run in Actions on Windows

* GH-624: In order to circumvent a test, one must return from it

* GH-624: Fixed two squirrellinesses

Co-authored-by: Bert <[email protected]>

Author: dnwiebe

multinode_integration_tests/tests/blockchain_interaction_test.rs

@@ -74,12 +74,10 @@ fn debtors_are_credited_once_but_not_twice() {
         MASQRealNode::node_home_dir(&MASQNodeUtils::find_project_root(), &node_name);
     open_all_file_permissions(PathBuf::from(node_home_dir));
     {
-        let mut config_dao = config_dao(&node_name);
-        let mut config_xactn = config_dao.start_transaction().unwrap();
-        config_xactn
+        let config_dao = config_dao(&node_name);
+        config_dao
             .set("start_block", Some("1000".to_string()))
             .unwrap();
-        config_xactn.commit().unwrap();
     }
     {
         let receivable_dao = receivable_dao(&node_name);

node/src/blockchain/blockchain_interface.rs

@@ -1836,12 +1836,10 @@ mod tests {
     #[test]
     fn blockchain_interface_non_clandestine_can_fetch_transaction_receipt() {
         let port = find_free_port();
-        thread::spawn(move || {
-            Server::new(|_req, mut rsp| {
-                Ok(rsp.body(br#"{"jsonrpc":"2.0","id":2,"result":{"transactionHash":"0xa128f9ca1e705cc20a936a24a7fa1df73bad6e0aaf58e8e6ffcc154a7cff6e0e","blockHash":"0x6d0abccae617442c26104c2bc63d1bc05e1e002e555aec4ab62a46e826b18f18","blockNumber":"0xb0328d","contractAddress":null,"cumulativeGasUsed":"0x60ef","effectiveGasPrice":"0x22ecb25c00","from":"0x7424d05b59647119b01ff81e2d3987b6c358bf9c","gasUsed":"0x60ef","logs":[],"logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000","status":"0x0","to":"0x384dec25e03f94931767ce4c3556168468ba24c3","transactionIndex":"0x0","type":"0x0"}}"#.to_vec())?)
-            })
-                .listen(&Ipv4Addr::LOCALHOST.to_string(), &format!("{}", port));
-        });
+        let _test_server = TestServer::start (port, vec![
+            br#"{"jsonrpc":"2.0","id":2,"result":{"transactionHash":"0xa128f9ca1e705cc20a936a24a7fa1df73bad6e0aaf58e8e6ffcc154a7cff6e0e","blockHash":"0x6d0abccae617442c26104c2bc63d1bc05e1e002e555aec4ab62a46e826b18f18","blockNumber":"0xb0328d","contractAddress":null,"cumulativeGasUsed":"0x60ef","effectiveGasPrice":"0x22ecb25c00","from":"0x7424d05b59647119b01ff81e2d3987b6c358bf9c","gasUsed":"0x60ef","logs":[],"logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000","status":"0x0","to":"0x384dec25e03f94931767ce4c3556168468ba24c3","transactionIndex":"0x0","type":"0x0"}}"#
+                .to_vec()
+        ]);
         let (event_loop_handle, transport) = Http::with_max_parallel(
             &format!("http://{}:{}", &Ipv4Addr::LOCALHOST.to_string(), port),
             REQUESTS_IN_PARALLEL,

node/src/daemon/setup_reporter.rs

@@ -1036,7 +1036,7 @@ mod tests {
     use crate::daemon::setup_reporter;
     use crate::database::connection_wrapper::ConnectionWrapperReal;
     use crate::database::db_initializer::{DbInitializer, DbInitializerReal, DATABASE_FILE};
-    use crate::db_config::config_dao::{ConfigDaoRead, ConfigDaoReal};
+    use crate::db_config::config_dao::{ConfigDao, ConfigDaoReal};
     use crate::db_config::persistent_configuration::{
         PersistentConfigError, PersistentConfiguration, PersistentConfigurationReal,
     };

node/src/database/config_dumper.rs

@@ -7,7 +7,7 @@ use crate::database::db_initializer::{
     DbInitializer, DbInitializerReal, InitializationError, DATABASE_FILE,
 };
 use crate::database::db_migrations::MigratorConfig;
-use crate::db_config::config_dao::{ConfigDaoRead, ConfigDaoReal, ConfigDaoRecord};
+use crate::db_config::config_dao::{ConfigDao, ConfigDaoReal, ConfigDaoRecord};
 use crate::db_config::typed_config_layer::{decode_bytes, encode_bytes};
 use crate::node_configurator::DirsWrapperReal;
 use crate::node_configurator::{
@@ -156,6 +156,7 @@ mod tests {
     use crate::database::connection_wrapper::ConnectionWrapperReal;
     use crate::database::db_initializer::CURRENT_SCHEMA_VERSION;
     use crate::database::db_migrations::ExternalData;
+    use crate::db_config::config_dao::ConfigDao;
     use crate::db_config::persistent_configuration::{
         PersistentConfiguration, PersistentConfigurationReal,
     };

node/src/database/connection_wrapper.rs

@@ -3,17 +3,9 @@
 use rusqlite::{Connection, Error, Statement, Transaction};
 use std::fmt::Debug;
 
-#[cfg(test)]
-use std::any::Any;
-
 pub trait ConnectionWrapper: Debug + Send {
     fn prepare(&self, query: &str) -> Result<Statement, rusqlite::Error>;
     fn transaction<'a: 'b, 'b>(&'a mut self) -> Result<Transaction<'b>, rusqlite::Error>;
-
-    #[cfg(test)]
-    fn as_any(&self) -> &dyn Any {
-        intentionally_blank!()
-    }
 }
 
 #[derive(Debug)]
@@ -28,64 +20,10 @@ impl ConnectionWrapper for ConnectionWrapperReal {
     fn transaction<'a: 'b, 'b>(&'a mut self) -> Result<Transaction<'b>, Error> {
         self.conn.transaction()
     }
-
-    #[cfg(test)]
-    fn as_any(&self) -> &dyn Any {
-        self
-    }
 }
 
 impl ConnectionWrapperReal {
     pub fn new(conn: Connection) -> Self {
         Self { conn }
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use crate::database::db_initializer::{
-        DbInitializer, DbInitializerReal, CURRENT_SCHEMA_VERSION,
-    };
-    use crate::database::db_migrations::MigratorConfig;
-    use crate::db_config::config_dao::{ConfigDao, ConfigDaoRead, ConfigDaoReal};
-    use masq_lib::test_utils::utils::ensure_node_home_directory_exists;
-
-    #[test]
-    fn commit_works() {
-        let data_dir = ensure_node_home_directory_exists("connection_wrapper", "commit_works");
-        let conn = DbInitializerReal::default()
-            .initialize(&data_dir, true, MigratorConfig::test_default())
-            .unwrap();
-        let mut config_dao = ConfigDaoReal::new(conn);
-        {
-            let mut writer = config_dao.start_transaction().unwrap();
-            writer
-                .set("schema_version", Some("booga".to_string()))
-                .unwrap();
-            writer.commit().unwrap();
-        }
-
-        let result = config_dao.get("schema_version").unwrap().value_opt;
-
-        assert_eq!(result, Some("booga".to_string()));
-    }
-
-    #[test]
-    fn drop_works() {
-        let data_dir = ensure_node_home_directory_exists("connection_wrapper", "drop_works");
-        let conn = DbInitializerReal::default()
-            .initialize(&data_dir, true, MigratorConfig::test_default())
-            .unwrap();
-        let mut config_dao = ConfigDaoReal::new(conn);
-        {
-            let writer = config_dao.start_transaction().unwrap();
-            writer
-                .set("schema_version", Some("booga".to_string()))
-                .unwrap();
-        }
-
-        let result = config_dao.get("schema_version").unwrap().value_opt;
-
-        assert_eq!(result, Some(CURRENT_SCHEMA_VERSION.to_string()));
-    }
-}

node/src/database/db_initializer.rs

@@ -609,7 +609,7 @@ pub mod test_utils {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::db_config::config_dao::{ConfigDaoRead, ConfigDaoReal};
+    use crate::db_config::config_dao::{ConfigDao, ConfigDaoReal};
     use crate::test_utils::database_utils::{
         assert_create_table_statement_contains_all_important_parts,
         assert_index_statement_is_coupled_with_right_parameter, assert_no_index_exists_for_table,
@@ -1143,14 +1143,9 @@ mod tests {
             )
         );
         let mut migrate_database_params = migrate_database_params_arc.lock().unwrap();
-        let (mismatched_schema, target_version, connection_wrapper) =
-            migrate_database_params.remove(0);
+        let (mismatched_schema, target_version, _) = migrate_database_params.remove(0);
         assert_eq!(mismatched_schema, 0);
         assert_eq!(target_version, 5);
-        assert!(connection_wrapper
-            .as_any()
-            .downcast_ref::<ConnectionWrapperReal>()
-            .is_some());
         TestLogHandler::new().exists_log_containing(
             "WARN: DbInitializer: Database is incompatible and its updating is necessary",
         );