Add service tests

src/domain/datasets/domain/src/entities/dataset_env_var.rs

@@ -97,15 +97,15 @@ impl DatasetEnvVar {
     ) -> Result<String, DatasetEnvVarEncryptionError> {
         if let Some(secret_nonce) = self.secret_nonce.as_ref() {
             let cipher = Aes256Gcm::new(Key::<Aes256Gcm>::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(),
                 )
                 .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());
         }

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,

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)

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;

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<dyn DatasetEnvVarService>,
+}
+
+impl DatasetEnvVarServiceHarness {
+    fn new() -> Self {
+        let catalog = {
+            let mut b = CatalogBuilder::new();
+
+            b.add::<DatasetEnvVarServiceImpl>();
+            b.add_value(InMemoryDatasetEnvVarRepository::new());
+            b.bind::<dyn DatasetEnvVarRepository, InMemoryDatasetEnvVarRepository>();
+
+            b.add::<SystemTimeSourceDefault>();
+            b.add_value(DatasetEnvVarsConfig::sample());
+
+            b.build()
+        };
+
+        Self {
+            dataset_env_var_service: catalog.get_one().unwrap(),
+            _catalog: catalog,
+        }
+    }
+}

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,

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,
     )

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,