From 3f90ebee3cfa44e14cd541891c31db912bbb8d0c Mon Sep 17 00:00:00 2001 From: rmn-boiko Date: Thu, 2 Jan 2025 15:28:13 +0100 Subject: [PATCH] Add service tests --- .../domain/src/entities/dataset_env_var.rs | 4 +- .../src/services/dataset_env_var_service.rs | 1 + .../src/dataset_env_var_service_impl.rs | 1 + .../datasets/services/tests/tests/mod.rs | 1 + .../test_dataset_env_var_service_impl.rs | 172 ++++++++++++++++++ ...387c3d021b0248fd1a2fea65e4e76bc24d36.json} | 4 +- .../postgres_dataset_env_var_repository.rs | 2 +- .../dataset_env_var_repository_test_suite.rs | 2 +- ...ca0f03c0f7b5ffa11ba0236ab1b0a0324eef.json} | 4 +- ...723cbffa51b784536d2045db121a64d795e2e.json | 50 +++++ .../sqlite_dataset_env_var_repository.rs | 10 +- 11 files changed, 239 insertions(+), 12 deletions(-) create mode 100644 src/domain/datasets/services/tests/tests/test_dataset_env_var_service_impl.rs rename src/infra/datasets/postgres/.sqlx/{query-1f81f06928fca92051bcaa7aaf42c6c174cc3cbd5eaf76a865e8baa92ca999fb.json => query-0a636b42b4d1cd67772d8eb9d624387c3d021b0248fd1a2fea65e4e76bc24d36.json} (74%) rename src/infra/datasets/sqlite/.sqlx/{query-5e5b7fb1fefebae2c03722d043aec7ea26b2e7e8a920332e6676abff09192f87.json => query-0739a2ee5162394a888a644efd6fca0f03c0f7b5ffa11ba0236ab1b0a0324eef.json} (80%) create mode 100644 src/infra/datasets/sqlite/.sqlx/query-140e91ce459e7759cbd0f57dc8f723cbffa51b784536d2045db121a64d795e2e.json diff --git a/src/domain/datasets/domain/src/entities/dataset_env_var.rs b/src/domain/datasets/domain/src/entities/dataset_env_var.rs index edceabe2b..e77196ef0 100644 --- a/src/domain/datasets/domain/src/entities/dataset_env_var.rs +++ b/src/domain/datasets/domain/src/entities/dataset_env_var.rs @@ -97,7 +97,7 @@ impl DatasetEnvVar { ) -> Result { if let Some(secret_nonce) = self.secret_nonce.as_ref() { let cipher = Aes256Gcm::new(Key::::from_slice(encryption_key.as_bytes())); - let decypted_value = cipher + let decrypted_value = cipher .decrypt( GenericArray::from_slice(secret_nonce.as_slice()), self.value.as_ref(), @@ -105,7 +105,7 @@ impl DatasetEnvVar { .map_err(|err| DatasetEnvVarEncryptionError::InvalidCipherKeyError { source: Box::new(AesGcmError(err)), })?; - return Ok(std::str::from_utf8(decypted_value.as_slice()) + return Ok(std::str::from_utf8(decrypted_value.as_slice()) .map_err(|err| DatasetEnvVarEncryptionError::InternalError(err.int_err()))? .to_string()); } diff --git a/src/domain/datasets/domain/src/services/dataset_env_var_service.rs b/src/domain/datasets/domain/src/services/dataset_env_var_service.rs index 6c6b20984..b1cdb020c 100644 --- a/src/domain/datasets/domain/src/services/dataset_env_var_service.rs +++ b/src/domain/datasets/domain/src/services/dataset_env_var_service.rs @@ -60,6 +60,7 @@ pub struct DatasetEnvVarListing { pub total_count: usize, } +#[derive(Debug)] pub struct DatasetEnvVarUpsertResult { pub dataset_env_var: DatasetEnvVar, pub status: UpsertDatasetEnvVarStatus, diff --git a/src/domain/datasets/services/src/dataset_env_var_service_impl.rs b/src/domain/datasets/services/src/dataset_env_var_service_impl.rs index 14b277f82..ad33e1504 100644 --- a/src/domain/datasets/services/src/dataset_env_var_service_impl.rs +++ b/src/domain/datasets/services/src/dataset_env_var_service_impl.rs @@ -79,6 +79,7 @@ impl DatasetEnvVarService for DatasetEnvVarServiceImpl { self.dataset_env_var_encryption_key.expose_secret(), ) .int_err()?; + let upsert_result = self .dataset_env_var_repository .upsert_dataset_env_var(&dataset_env_var) diff --git a/src/domain/datasets/services/tests/tests/mod.rs b/src/domain/datasets/services/tests/tests/mod.rs index 4bd756220..efb384c40 100644 --- a/src/domain/datasets/services/tests/tests/mod.rs +++ b/src/domain/datasets/services/tests/tests/mod.rs @@ -8,4 +8,5 @@ // by the Apache License, Version 2.0. mod test_dataset_entry_service; +mod test_dataset_env_var_service_impl; mod test_dependency_graph_service_impl; diff --git a/src/domain/datasets/services/tests/tests/test_dataset_env_var_service_impl.rs b/src/domain/datasets/services/tests/tests/test_dataset_env_var_service_impl.rs new file mode 100644 index 000000000..a263d42a6 --- /dev/null +++ b/src/domain/datasets/services/tests/tests/test_dataset_env_var_service_impl.rs @@ -0,0 +1,172 @@ +// Copyright Kamu Data, Inc. and contributors. All rights reserved. +// +// Use of this software is governed by the Business Source License +// included in the LICENSE file. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0. + +use std::assert_matches::assert_matches; +use std::sync::Arc; + +use dill::{Catalog, CatalogBuilder}; +use kamu_datasets::{ + DatasetEnvVarRepository, + DatasetEnvVarService, + DatasetEnvVarUpsertResult, + DatasetEnvVarValue, + DatasetEnvVarsConfig, + UpsertDatasetEnvVarStatus, +}; +use kamu_datasets_inmem::InMemoryDatasetEnvVarRepository; +use kamu_datasets_services::DatasetEnvVarServiceImpl; +use opendatafabric::DatasetID; +use secrecy::SecretString; +use time_source::SystemTimeSourceDefault; + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +#[test_log::test(tokio::test)] +async fn test_upsert_dataset_env_var() { + let harness = DatasetEnvVarServiceHarness::new(); + + let create_result = harness + .dataset_env_var_service + .upsert_dataset_env_var( + "foo_key", + &DatasetEnvVarValue::Secret(SecretString::from("foo_value")), + &DatasetID::new_seeded_ed25519(b"foo"), + ) + .await; + + assert_matches!( + create_result, + Ok(DatasetEnvVarUpsertResult { + status: UpsertDatasetEnvVarStatus::Created, + .. + }) + ); + + // ToDo: currently we are not checking if the secret values are equal, so we + // are updating the same value in case they are both secret + // The blocker is postgres implementation which requires one more additional + // db request to fetch the value from the db and compare it with the new value + let update_up_to_date_result = harness + .dataset_env_var_service + .upsert_dataset_env_var( + "foo_key", + &DatasetEnvVarValue::Secret(SecretString::from("foo_value")), + &DatasetID::new_seeded_ed25519(b"foo"), + ) + .await; + + assert_matches!( + update_up_to_date_result, + Ok(DatasetEnvVarUpsertResult { + status: UpsertDatasetEnvVarStatus::Updated, + .. + }) + ); + + // Change visibility of env var from secret to regular + let update_modified_result = harness + .dataset_env_var_service + .upsert_dataset_env_var( + "foo_key", + &DatasetEnvVarValue::Regular("foo_value".to_owned()), + &DatasetID::new_seeded_ed25519(b"foo"), + ) + .await; + + assert_matches!( + update_modified_result, + Ok(DatasetEnvVarUpsertResult { + status: UpsertDatasetEnvVarStatus::Updated, + .. + }) + ); + + // Try to modify the regular value of the env var with the same value will + // return up to date + let update_modified_result = harness + .dataset_env_var_service + .upsert_dataset_env_var( + "foo_key", + &DatasetEnvVarValue::Regular("foo_value".to_owned()), + &DatasetID::new_seeded_ed25519(b"foo"), + ) + .await; + + assert_matches!( + update_modified_result, + Ok(DatasetEnvVarUpsertResult { + status: UpsertDatasetEnvVarStatus::UpToDate, + .. + }) + ); + + let update_modified_result = harness + .dataset_env_var_service + .upsert_dataset_env_var( + "foo_key", + &DatasetEnvVarValue::Regular("new_foo_value".to_owned()), + &DatasetID::new_seeded_ed25519(b"foo"), + ) + .await; + + assert_matches!( + update_modified_result, + Ok(DatasetEnvVarUpsertResult { + status: UpsertDatasetEnvVarStatus::Updated, + .. + }) + ); + + // Change visibility of env var back to secret + let update_modified_result = harness + .dataset_env_var_service + .upsert_dataset_env_var( + "foo_key", + &DatasetEnvVarValue::Secret(SecretString::from("new_foo_value")), + &DatasetID::new_seeded_ed25519(b"foo"), + ) + .await; + + assert_matches!( + update_modified_result, + Ok(DatasetEnvVarUpsertResult { + status: UpsertDatasetEnvVarStatus::Updated, + .. + }) + ); +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +struct DatasetEnvVarServiceHarness { + _catalog: Catalog, + dataset_env_var_service: Arc, +} + +impl DatasetEnvVarServiceHarness { + fn new() -> Self { + let catalog = { + let mut b = CatalogBuilder::new(); + + b.add::(); + b.add_value(InMemoryDatasetEnvVarRepository::new()); + b.bind::(); + + b.add::(); + b.add_value(DatasetEnvVarsConfig::sample()); + + b.build() + }; + + Self { + dataset_env_var_service: catalog.get_one().unwrap(), + _catalog: catalog, + } + } +} diff --git a/src/infra/datasets/postgres/.sqlx/query-1f81f06928fca92051bcaa7aaf42c6c174cc3cbd5eaf76a865e8baa92ca999fb.json b/src/infra/datasets/postgres/.sqlx/query-0a636b42b4d1cd67772d8eb9d624387c3d021b0248fd1a2fea65e4e76bc24d36.json similarity index 74% rename from src/infra/datasets/postgres/.sqlx/query-1f81f06928fca92051bcaa7aaf42c6c174cc3cbd5eaf76a865e8baa92ca999fb.json rename to src/infra/datasets/postgres/.sqlx/query-0a636b42b4d1cd67772d8eb9d624387c3d021b0248fd1a2fea65e4e76bc24d36.json index 5ecc2a383..57609d973 100644 --- a/src/infra/datasets/postgres/.sqlx/query-1f81f06928fca92051bcaa7aaf42c6c174cc3cbd5eaf76a865e8baa92ca999fb.json +++ b/src/infra/datasets/postgres/.sqlx/query-0a636b42b4d1cd67772d8eb9d624387c3d021b0248fd1a2fea65e4e76bc24d36.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n INSERT INTO dataset_env_vars (id, key, value, secret_nonce, created_at, dataset_id)\n VALUES ($1, $2, $3, $4, $5, $6)\n ON CONFLICT (key, dataset_id)\n DO UPDATE SET\n value = EXCLUDED.value,\n secret_nonce = CASE\n WHEN dataset_env_vars.secret_nonce IS NULL AND EXCLUDED.secret_nonce IS NOT NULL THEN EXCLUDED.secret_nonce\n WHEN dataset_env_vars.secret_nonce IS NOT NULL AND EXCLUDED.secret_nonce IS NULL THEN NULL\n ELSE dataset_env_vars.secret_nonce\n END\n RETURNING xmax = 0 AS is_inserted,\n id,\n (\n SELECT value FROM dataset_env_vars WHERE key = $2 and dataset_id = $6\n ) as \"value: Vec\";\n ", + "query": "\n INSERT INTO dataset_env_vars (id, key, value, secret_nonce, created_at, dataset_id)\n VALUES ($1, $2, $3, $4, $5, $6)\n ON CONFLICT (key, dataset_id)\n DO UPDATE SET\n value = EXCLUDED.value,\n secret_nonce = CASE\n WHEN dataset_env_vars.secret_nonce IS NULL AND EXCLUDED.secret_nonce IS NOT NULL THEN EXCLUDED.secret_nonce\n WHEN dataset_env_vars.secret_nonce IS NOT NULL AND EXCLUDED.secret_nonce IS NULL THEN NULL\n ELSE EXCLUDED.secret_nonce\n END\n RETURNING xmax = 0 AS is_inserted,\n id,\n (\n SELECT value FROM dataset_env_vars WHERE key = $2 and dataset_id = $6\n ) as \"value: Vec\";\n ", "describe": { "columns": [ { @@ -35,5 +35,5 @@ null ] }, - "hash": "1f81f06928fca92051bcaa7aaf42c6c174cc3cbd5eaf76a865e8baa92ca999fb" + "hash": "0a636b42b4d1cd67772d8eb9d624387c3d021b0248fd1a2fea65e4e76bc24d36" } diff --git a/src/infra/datasets/postgres/src/repos/postgres_dataset_env_var_repository.rs b/src/infra/datasets/postgres/src/repos/postgres_dataset_env_var_repository.rs index 4b78bb36f..9052da7c8 100644 --- a/src/infra/datasets/postgres/src/repos/postgres_dataset_env_var_repository.rs +++ b/src/infra/datasets/postgres/src/repos/postgres_dataset_env_var_repository.rs @@ -50,7 +50,7 @@ impl DatasetEnvVarRepository for PostgresDatasetEnvVarRepository { secret_nonce = CASE WHEN dataset_env_vars.secret_nonce IS NULL AND EXCLUDED.secret_nonce IS NOT NULL THEN EXCLUDED.secret_nonce WHEN dataset_env_vars.secret_nonce IS NOT NULL AND EXCLUDED.secret_nonce IS NULL THEN NULL - ELSE dataset_env_vars.secret_nonce + ELSE EXCLUDED.secret_nonce END RETURNING xmax = 0 AS is_inserted, id, diff --git a/src/infra/datasets/repo-tests/src/dataset_env_var_repository_test_suite.rs b/src/infra/datasets/repo-tests/src/dataset_env_var_repository_test_suite.rs index b05fbf19f..f06d80a2d 100644 --- a/src/infra/datasets/repo-tests/src/dataset_env_var_repository_test_suite.rs +++ b/src/infra/datasets/repo-tests/src/dataset_env_var_repository_test_suite.rs @@ -281,7 +281,7 @@ pub async fn test_upsert_dataset_env_vars(catalog: &Catalog) { let mut new_dataset_env_var = DatasetEnvVar::new( "foo", Utc::now().round_subsecs(6), - &DatasetEnvVarValue::Regular("foo".to_string()), + &DatasetEnvVarValue::Secret(SecretString::from("foo")), &entry_foo.id, SAMPLE_DATASET_ENV_VAR_ENCRYPTION_KEY, ) diff --git a/src/infra/datasets/sqlite/.sqlx/query-5e5b7fb1fefebae2c03722d043aec7ea26b2e7e8a920332e6676abff09192f87.json b/src/infra/datasets/sqlite/.sqlx/query-0739a2ee5162394a888a644efd6fca0f03c0f7b5ffa11ba0236ab1b0a0324eef.json similarity index 80% rename from src/infra/datasets/sqlite/.sqlx/query-5e5b7fb1fefebae2c03722d043aec7ea26b2e7e8a920332e6676abff09192f87.json rename to src/infra/datasets/sqlite/.sqlx/query-0739a2ee5162394a888a644efd6fca0f03c0f7b5ffa11ba0236ab1b0a0324eef.json index f2e2e1a93..4fba5658b 100644 --- a/src/infra/datasets/sqlite/.sqlx/query-5e5b7fb1fefebae2c03722d043aec7ea26b2e7e8a920332e6676abff09192f87.json +++ b/src/infra/datasets/sqlite/.sqlx/query-0739a2ee5162394a888a644efd6fca0f03c0f7b5ffa11ba0236ab1b0a0324eef.json @@ -1,6 +1,6 @@ { "db_name": "SQLite", - "query": "\n INSERT INTO dataset_env_vars (id, key, value, secret_nonce, created_at, dataset_id)\n VALUES ($1, $2, $3, $4, $5, $6)\n ON CONFLICT (key, dataset_id)\n DO UPDATE SET\n value = EXCLUDED.value,\n secret_nonce = CASE\n WHEN dataset_env_vars.secret_nonce IS NULL AND excluded.secret_nonce IS NOT NULL THEN excluded.secret_nonce\n WHEN dataset_env_vars.secret_nonce IS NOT NULL AND excluded.secret_nonce IS NULL THEN NULL\n ELSE dataset_env_vars.secret_nonce\n END\n ", + "query": "\n INSERT INTO dataset_env_vars (id, key, value, secret_nonce, created_at, dataset_id)\n VALUES ($1, $2, $3, $4, $5, $6)\n ON CONFLICT (key, dataset_id)\n DO UPDATE SET\n value = EXCLUDED.value,\n secret_nonce = CASE\n WHEN dataset_env_vars.secret_nonce IS NULL AND excluded.secret_nonce IS NOT NULL THEN excluded.secret_nonce\n WHEN dataset_env_vars.secret_nonce IS NOT NULL AND excluded.secret_nonce IS NULL THEN NULL\n ELSE excluded.secret_nonce\n END\n ", "describe": { "columns": [], "parameters": { @@ -8,5 +8,5 @@ }, "nullable": [] }, - "hash": "5e5b7fb1fefebae2c03722d043aec7ea26b2e7e8a920332e6676abff09192f87" + "hash": "0739a2ee5162394a888a644efd6fca0f03c0f7b5ffa11ba0236ab1b0a0324eef" } diff --git a/src/infra/datasets/sqlite/.sqlx/query-140e91ce459e7759cbd0f57dc8f723cbffa51b784536d2045db121a64d795e2e.json b/src/infra/datasets/sqlite/.sqlx/query-140e91ce459e7759cbd0f57dc8f723cbffa51b784536d2045db121a64d795e2e.json new file mode 100644 index 000000000..b6292b256 --- /dev/null +++ b/src/infra/datasets/sqlite/.sqlx/query-140e91ce459e7759cbd0f57dc8f723cbffa51b784536d2045db121a64d795e2e.json @@ -0,0 +1,50 @@ +{ + "db_name": "SQLite", + "query": "\n SELECT\n id as \"id: Uuid\",\n key,\n value as \"value: _\",\n secret_nonce as \"secret_nonce: _\",\n created_at as \"created_at: _\",\n dataset_id as \"dataset_id: _\"\n FROM dataset_env_vars\n WHERE key = $1 and dataset_id = $2\n ", + "describe": { + "columns": [ + { + "name": "id: Uuid", + "ordinal": 0, + "type_info": "Text" + }, + { + "name": "key", + "ordinal": 1, + "type_info": "Text" + }, + { + "name": "value: _", + "ordinal": 2, + "type_info": "Blob" + }, + { + "name": "secret_nonce: _", + "ordinal": 3, + "type_info": "Blob" + }, + { + "name": "created_at: _", + "ordinal": 4, + "type_info": "Null" + }, + { + "name": "dataset_id: _", + "ordinal": 5, + "type_info": "Text" + } + ], + "parameters": { + "Right": 2 + }, + "nullable": [ + false, + false, + false, + true, + false, + false + ] + }, + "hash": "140e91ce459e7759cbd0f57dc8f723cbffa51b784536d2045db121a64d795e2e" +} diff --git a/src/infra/datasets/sqlite/src/repos/sqlite_dataset_env_var_repository.rs b/src/infra/datasets/sqlite/src/repos/sqlite_dataset_env_var_repository.rs index 7001b1301..4df4013fe 100644 --- a/src/infra/datasets/sqlite/src/repos/sqlite_dataset_env_var_repository.rs +++ b/src/infra/datasets/sqlite/src/repos/sqlite_dataset_env_var_repository.rs @@ -40,6 +40,7 @@ impl DatasetEnvVarRepository for SqliteDatasetEnvVarRepository { let mut tr = self.transaction.lock().await; let connection_mut = tr.connection_mut().await?; + let dataset_env_var_dataset_id = dataset_env_var.dataset_id.to_string(); let old_record = sqlx::query_as!( DatasetEnvVarRowModel, r#" @@ -51,14 +52,16 @@ impl DatasetEnvVarRepository for SqliteDatasetEnvVarRepository { created_at as "created_at: _", dataset_id as "dataset_id: _" FROM dataset_env_vars - WHERE id = $1 + WHERE key = $1 and dataset_id = $2 "#, - dataset_env_var.id, + dataset_env_var.key, + dataset_env_var_dataset_id, ) .fetch_optional(&mut *connection_mut) .await .int_err()?; + // ToDo compare decrypted value once postgres implementation is done if let Some(record) = &old_record && dataset_env_var.value == record.value { @@ -68,7 +71,6 @@ impl DatasetEnvVarRepository for SqliteDatasetEnvVarRepository { }); } - let dataset_env_var_dataset_id = dataset_env_var.dataset_id.to_string(); sqlx::query!( r#" INSERT INTO dataset_env_vars (id, key, value, secret_nonce, created_at, dataset_id) @@ -79,7 +81,7 @@ impl DatasetEnvVarRepository for SqliteDatasetEnvVarRepository { secret_nonce = CASE WHEN dataset_env_vars.secret_nonce IS NULL AND excluded.secret_nonce IS NOT NULL THEN excluded.secret_nonce WHEN dataset_env_vars.secret_nonce IS NOT NULL AND excluded.secret_nonce IS NULL THEN NULL - ELSE dataset_env_vars.secret_nonce + ELSE excluded.secret_nonce END "#, dataset_env_var.id,