From b8f8dad2d4aa0f4a0da2f4a323b384e0055ed26b Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 18 Nov 2024 09:35:22 +0100 Subject: [PATCH] Remove all references in `signer` to the `mithril_persistence::adapter` module --- .../src/sqlite/connection_builder.rs | 2 +- mithril-signer/src/database/migration.rs | 4 +- .../protocol_initializer_repository.rs | 32 +++++------- .../database/repository/stake_pool_store.rs | 48 +++++------------ mithril-signer/src/database/test_helper.rs | 51 +++++++++++++++++++ mithril-signer/src/services/epoch_service.rs | 6 ++- .../test_extensions/state_machine_tester.rs | 16 +++--- 7 files changed, 89 insertions(+), 70 deletions(-) diff --git a/internal/mithril-persistence/src/sqlite/connection_builder.rs b/internal/mithril-persistence/src/sqlite/connection_builder.rs index 06ee22fa386..705cd845360 100644 --- a/internal/mithril-persistence/src/sqlite/connection_builder.rs +++ b/internal/mithril-persistence/src/sqlite/connection_builder.rs @@ -124,7 +124,7 @@ impl ConnectionBuilder { // Check database migrations debug!(logger, "Applying database migrations"); let mut db_checker = - DatabaseVersionChecker::new(self.base_logger, self.node_type, &connection); + DatabaseVersionChecker::new(self.base_logger, self.node_type, connection); for migration in sql_migrations { db_checker.add_migration(migration); diff --git a/mithril-signer/src/database/migration.rs b/mithril-signer/src/database/migration.rs index 7f2af1a46c3..bf26d72c65d 100644 --- a/mithril-signer/src/database/migration.rs +++ b/mithril-signer/src/database/migration.rs @@ -95,11 +95,11 @@ insert into stake_pool (epoch, stake_pool_id, stake, created_at) drop table stake; "#, ), - // Migration 5 + // Migration 6 // Add the `protocol_initializer` table and migration data from the previous // `protocol_initializer` JSON format. SqlMigration::new( - 5, + 6, r#" create table new_protocol_initializer ( epoch integer not null, diff --git a/mithril-signer/src/database/repository/protocol_initializer_repository.rs b/mithril-signer/src/database/repository/protocol_initializer_repository.rs index 3fb456a3c79..1dcb0e9603c 100644 --- a/mithril-signer/src/database/repository/protocol_initializer_repository.rs +++ b/mithril-signer/src/database/repository/protocol_initializer_repository.rs @@ -13,7 +13,7 @@ use crate::{ }; use mithril_common::{crypto_helper::ProtocolInitializer, entities::Epoch, StdResult}; use mithril_persistence::sqlite::ConnectionExtensions; -use mithril_persistence::{sqlite::SqliteConnection, store::adapter::StoreAdapter}; +use mithril_persistence::{sqlite::SqliteConnection /*store::adapter::StoreAdapter*/}; /// Implementation of the ProtocolInitializerStorer pub struct ProtocolInitializerRepository { @@ -95,13 +95,9 @@ impl ProtocolInitializerStorer for ProtocolInitializerRepository { #[cfg(test)] mod tests { + use crate::database::test_helper::{main_db_connection, FakeStoreAdapter}; use mithril_common::test_utils::fake_data; - use mithril_persistence::{ - sqlite::{ConnectionBuilder, ConnectionOptions}, - store::adapter::SQLiteAdapter, - }; - - use crate::database::test_helper::main_db_connection; + use mithril_persistence::sqlite::{ConnectionBuilder, ConnectionOptions}; use super::*; @@ -263,29 +259,25 @@ mod tests { .with_options(&[ConnectionOptions::ForceDisableForeignKeys]) } let connection = Arc::new(create_connection_builder().build().unwrap()); - + let protocol_initializer_adapter = + FakeStoreAdapter::new(connection.clone(), "protocol_initializer"); // The adapter will create the table. - let mut adapter = SQLiteAdapter::::new( - "protocol_initializer", - connection.clone(), - ) - .unwrap(); + protocol_initializer_adapter.create_table(); assert!(connection .prepare("select key_hash from protocol_initializer;") .is_ok()); - assert!(connection.prepare("select * from db_version;").is_err()); // Here we can add some data with the old schema. let (_, protocol_initializer_to_retrieve) = &setup_protocol_initializers(1)[0]; - // If we don't want to use the adapter anymore, we can execute request directly. - assert!(adapter.get_record(&Epoch(5)).await.unwrap().is_none()); - adapter - .store_record(&Epoch(5), &protocol_initializer_to_retrieve) - .await + assert!(!protocol_initializer_adapter.is_key_hash_exist("HashEpoch5")); + + protocol_initializer_adapter + .store_record("HashEpoch5", &Epoch(5), protocol_initializer_to_retrieve) .unwrap(); - assert!(adapter.get_record(&Epoch(5)).await.unwrap().is_some()); + + assert!(protocol_initializer_adapter.is_key_hash_exist("HashEpoch5")); // We finish the migration create_connection_builder() diff --git a/mithril-signer/src/database/repository/stake_pool_store.rs b/mithril-signer/src/database/repository/stake_pool_store.rs index 195e574ab3a..5ada1cbc9a2 100644 --- a/mithril-signer/src/database/repository/stake_pool_store.rs +++ b/mithril-signer/src/database/repository/stake_pool_store.rs @@ -8,7 +8,6 @@ use mithril_common::entities::{Epoch, StakeDistribution}; use mithril_common::signable_builder::StakeDistributionRetriever; use mithril_common::StdResult; use mithril_persistence::sqlite::{ConnectionExtensions, SqliteConnection}; -use mithril_persistence::store::adapter::AdapterError; use mithril_persistence::store::StakeStorer; use crate::database::query::{ @@ -51,8 +50,7 @@ impl StakeStorer for StakePoolStore { .map(|(pool_id, stake)| (pool_id, epoch, stake)) .collect(), )) - .with_context(|| format!("persist stakes failure, epoch: {epoch}")) - .map_err(AdapterError::GeneralError)?; + .with_context(|| format!("persist stakes failure, epoch: {epoch}"))?; Ok(Some(StakeDistribution::from_iter( pools.into_iter().map(|p| (p.stake_pool_id, p.stake)), @@ -63,8 +61,7 @@ impl StakeStorer for StakePoolStore { let cursor = self .connection .fetch(GetStakePoolQuery::by_epoch(epoch)?) - .with_context(|| format!("get stakes failure, epoch: {epoch}")) - .map_err(AdapterError::GeneralError)?; + .with_context(|| format!("get stakes failure, epoch: {epoch}"))?; let mut stake_distribution = StakeDistribution::new(); for stake_pool in cursor { @@ -96,8 +93,7 @@ impl EpochPruningTask for StakePoolStore { self.connection .apply(DeleteStakePoolQuery::below_epoch_threshold( epoch - threshold, - )) - .map_err(AdapterError::QueryError)?; + ))?; } Ok(()) } @@ -105,15 +101,10 @@ impl EpochPruningTask for StakePoolStore { #[cfg(test)] mod tests { - use mithril_persistence::{ - database::SqlMigration, - sqlite::{ConnectionBuilder, ConnectionOptions}, - store::adapter::SQLiteAdapter, - }; + use mithril_persistence::sqlite::{ConnectionBuilder, ConnectionOptions}; use super::*; - use crate::database::test_helper::{insert_stake_pool, main_db_connection}; - use mithril_persistence::store::adapter::StoreAdapter; + use crate::database::test_helper::{insert_stake_pool, main_db_connection, FakeStoreAdapter}; #[tokio::test] async fn prune_epoch_settings_older_than_threshold() { @@ -200,39 +191,24 @@ mod tests { let connection = Arc::new(create_connection_builder().build().unwrap()); // The adapter will create the table. - let mut adapter = - SQLiteAdapter::::new("stake", connection.clone()).unwrap(); + let stake_adapter = FakeStoreAdapter::new(connection.clone(), "stake"); + // The adapter will create the table. + stake_adapter.create_table(); assert!(connection.prepare("select * from stake;").is_ok()); assert!(connection.prepare("select * from db_version;").is_err()); assert!(connection.prepare("select * from stake_pool;").is_err()); - // TODO: We recreate a Builder (because it was consume by the build) to have same option but we don't use a new connection. - // builder is needed for options, base_logger and node_type. - create_connection_builder() - .apply_migrations( - &connection.clone(), - migrations - .clone() - .into_iter() - .filter(|migration| migration.version <= 1) - .collect::>(), - ) - .unwrap(); - assert!(connection.prepare("select * from db_version;").is_ok()); - assert!(connection.prepare("select * from stake_pool;").is_err()); - // Here we can add some data with the old schema. let stake_distribution_to_retrieve = StakeDistribution::from([("pool-123".to_string(), 123)]); // If we don't want to use the adapter anymore, we can execute request directly. - assert!(adapter.get_record(&Epoch(5)).await.unwrap().is_none()); - adapter - .store_record(&Epoch(5), &stake_distribution_to_retrieve) - .await + assert!(!stake_adapter.is_key_hash_exist("HashEpoch5")); + stake_adapter + .store_record("HashEpoch5", &Epoch(5), &stake_distribution_to_retrieve) .unwrap(); - assert!(adapter.get_record(&Epoch(5)).await.unwrap().is_some()); + assert!(stake_adapter.is_key_hash_exist("HashEpoch5")); // We finish the migration create_connection_builder() diff --git a/mithril-signer/src/database/test_helper.rs b/mithril-signer/src/database/test_helper.rs index 0dbdf18b578..0bef3b049fc 100644 --- a/mithril-signer/src/database/test_helper.rs +++ b/mithril-signer/src/database/test_helper.rs @@ -1,4 +1,5 @@ use std::path::Path; +use std::sync::Arc; use chrono::Utc; use mithril_common::entities::Epoch; @@ -6,6 +7,7 @@ use mithril_common::StdResult; use mithril_persistence::sqlite::{ ConnectionBuilder, ConnectionExtensions, ConnectionOptions, Query, SqliteConnection, }; +use serde::Serialize; use sqlite::Value; use crate::database::query::{InsertOrReplaceStakePoolQuery, InsertSignedBeaconRecordQuery}; @@ -102,3 +104,52 @@ pub fn insert_stake_pool( Ok(()) } + +/// A simple struct that help to initialize database with the old adapter behavior for testing purposes. +pub struct FakeStoreAdapter { + connection: Arc, + table: &'static str, +} + +impl FakeStoreAdapter { + pub fn new(connection: Arc, table: &'static str) -> Self { + Self { connection, table } + } + + pub fn create_table(&self) { + let sql = format!( + "create table {} (key_hash text primary key, key json not null, value json not null)", + self.table + ); + self.connection.execute(sql).unwrap(); + } + + pub fn is_key_hash_exist(&self, key_hash: &str) -> bool { + let sql = format!( + "select exists(select 1 from {} where key_hash = ?1) as record_exists", + self.table + ); + let parameters = [Value::String(key_hash.to_string())]; + let result: i64 = self.connection.query_single_cell(sql, ¶meters).unwrap(); + result == 1 + } + + pub fn store_record( + &self, + key_hash: &str, + key: &K, + record: &V, + ) -> StdResult<()> { + let sql = format!( + "insert into {} (key_hash, key, value) values (?1, ?2, ?3) on conflict (key_hash) do update set value = excluded.value", + self.table + ); + let mut statement = self.connection.prepare(sql)?; + statement.bind((1, key_hash))?; + statement.bind((2, serde_json::to_string(&key)?.as_str()))?; + statement.bind((3, serde_json::to_string(record)?.as_str()))?; + let _ = statement.next()?; + + Ok(()) + } +} diff --git a/mithril-signer/src/services/epoch_service.rs b/mithril-signer/src/services/epoch_service.rs index 23f59c86d1a..68aba151483 100644 --- a/mithril-signer/src/services/epoch_service.rs +++ b/mithril-signer/src/services/epoch_service.rs @@ -5,7 +5,6 @@ use std::collections::BTreeSet; use std::sync::Arc; use thiserror::Error; -use crate::database::repository::ProtocolInitializerRepository; use crate::dependency_injection::EpochServiceWrapper; use crate::entities::SignerEpochSettings; use crate::services::SignedEntityConfigProvider; @@ -317,6 +316,9 @@ impl SignedEntityConfigProvider for SignerSignedEntityConfigProvider { } } +#[cfg(test)] +use crate::database::repository::ProtocolInitializerRepository; + #[cfg(test)] impl MithrilEpochService { /// `TEST ONLY` - Create a new instance of the service using dumb dependencies. @@ -430,7 +432,7 @@ mod tests { use mithril_common::entities::{Epoch, StakeDistribution}; use mithril_common::test_utils::{fake_data, MithrilFixtureBuilder}; - use crate::database::repository::StakePoolStore; + use crate::database::repository::{ProtocolInitializerRepository, StakePoolStore}; use crate::database::test_helper::main_db_connection; use crate::entities::SignerEpochSettings; use crate::services::MithrilProtocolInitializerBuilder; diff --git a/mithril-signer/tests/test_extensions/state_machine_tester.rs b/mithril-signer/tests/test_extensions/state_machine_tester.rs index f5d4a100295..812e1c36d36 100644 --- a/mithril-signer/tests/test_extensions/state_machine_tester.rs +++ b/mithril-signer/tests/test_extensions/state_machine_tester.rs @@ -37,18 +37,18 @@ use mithril_common::{ }; use mithril_persistence::{ database::repository::CardanoTransactionRepository, sqlite::SqliteConnectionPool, - store::adapter::SQLiteAdapter, store::StakeStorer, + store::StakeStorer, }; use mithril_signer::{ - database::repository::{SignedBeaconRepository, StakePoolStore}, + database::repository::{ProtocolInitializerRepository, SignedBeaconRepository, StakePoolStore}, dependency_injection::{DependenciesBuilder, SignerDependencyContainer}, services::{ AggregatorClient, CardanoTransactionsImporter, MithrilEpochService, MithrilSingleSigner, SignerCertifierService, SignerSignableSeedBuilder, SignerSignedEntityConfigProvider, SignerUpkeepService, }, - store::{MKTreeStoreSqlite, ProtocolInitializerStore, ProtocolInitializerStorer}, + store::{MKTreeStoreSqlite, ProtocolInitializerStorer}, Configuration, MetricsService, RuntimeError, SignerRunner, SignerState, StateMachine, }; @@ -77,7 +77,7 @@ pub struct StateMachineTester { immutable_observer: Arc, chain_observer: Arc, certificate_handler: Arc, - protocol_initializer_store: Arc, + protocol_initializer_store: Arc, stake_store: Arc, era_checker: Arc, era_reader_adapter: Arc, @@ -158,11 +158,9 @@ impl StateMachineTester { ticker_service.clone(), )); let digester = Arc::new(DumbImmutableDigester::new("DIGEST", true)); - let protocol_initializer_store = Arc::new(ProtocolInitializerStore::new( - Box::new( - SQLiteAdapter::new("protocol_initializer", sqlite_connection.clone()).unwrap(), - ), - config.store_retention_limit, + let protocol_initializer_store = Arc::new(ProtocolInitializerRepository::new( + sqlite_connection.clone(), + config.store_retention_limit.map(|limit| limit as u64), )); let stake_store = Arc::new(StakePoolStore::new( sqlite_connection.clone(),