feat: improve error msg for collections not found or empty (#698)

* feat: improve error msg for collections not found or empty

Signed-off-by: David Dal Busco <david.dalbusco@outlook.com>

* test: unknown and empty storage and datastore collection

Signed-off-by: David Dal Busco <david.dalbusco@outlook.com>

---------

Signed-off-by: David Dal Busco <david.dalbusco@outlook.com>

src/console/src/storage/state/heap.rs

@@ -1,5 +1,5 @@
 use crate::memory::STATE;
-use junobuild_collections::msg::COLLECTION_NOT_FOUND;
+use junobuild_collections::msg::msg_storage_collection_not_found;
 use junobuild_collections::types::core::CollectionKey;
 use junobuild_collections::types::rules::Rule;
 use junobuild_shared::types::core::DomainName;
@@ -58,7 +58,7 @@ pub fn get_rule(collection: &CollectionKey) -> Result<Rule, String> {
     });
 
     match rule {
-        None => Err([COLLECTION_NOT_FOUND, collection].join("")),
+        None => Err(msg_storage_collection_not_found(collection)),
         Some(rule) => Ok(rule),
     }
 }

src/libs/collections/src/msg.rs

@@ -1,3 +1,30 @@
-/// Rules
-pub const COLLECTION_NOT_FOUND: &str = "Collection not found: ";
+use crate::types::core::CollectionKey;
+
 pub const COLLECTION_NOT_EMPTY: &str = "Collection not empty: ";
+
+pub fn msg_db_collection_not_empty(collection: &CollectionKey) -> String {
+    msg_collection_not_empty(collection, &"Datastore".to_string())
+}
+
+pub fn msg_storage_collection_not_empty(collection: &CollectionKey) -> String {
+    msg_collection_not_empty(collection, &"Storage".to_string())
+}
+
+fn msg_collection_not_empty(collection: &CollectionKey, name: &String) -> String {
+    format!(
+        r#"The "{}" collection in {} is not empty."#,
+        collection, name
+    )
+}
+
+pub fn msg_db_collection_not_found(collection: &CollectionKey) -> String {
+    msg_collection_not_found(collection, &"Datastore".to_string())
+}
+
+pub fn msg_storage_collection_not_found(collection: &CollectionKey) -> String {
+    msg_collection_not_found(collection, &"Storage".to_string())
+}
+
+fn msg_collection_not_found(collection: &CollectionKey, name: &String) -> String {
+    format!(r#"Collection "{}" not found in {}."#, collection, name)
+}

src/libs/satellite/src/db/state.rs

@@ -1,7 +1,7 @@
 use crate::db::types::config::DbConfig;
 use crate::db::types::state::{Collection, DbHeap, DbHeapState, DbStable, Doc, StableKey};
 use crate::memory::STATE;
-use junobuild_collections::msg::COLLECTION_NOT_FOUND;
+use junobuild_collections::msg::msg_db_collection_not_found;
 use junobuild_collections::types::core::CollectionKey;
 use junobuild_collections::types::rules::{Memory, Rule};
 use junobuild_collections::utils::range_collection_end;
@@ -64,7 +64,7 @@ fn is_collection_empty_heap(collection: &CollectionKey, db: &DbHeap) -> Result<b
     let col = db.get(collection);
 
     match col {
-        None => Err([COLLECTION_NOT_FOUND, collection].join("")),
+        None => Err(msg_db_collection_not_found(collection)),
         Some(col) => Ok(col.is_empty()),
     }
 }
@@ -80,7 +80,7 @@ fn delete_collection_heap(collection: &CollectionKey, db: &mut DbHeap) -> Result
     let col = db.get(collection);
 
     match col {
-        None => Err([COLLECTION_NOT_FOUND, collection].join("")),
+        None => Err(msg_db_collection_not_found(collection)),
         Some(_) => {
             db.remove(collection);
 
@@ -163,7 +163,7 @@ fn get_doc_heap(collection: &CollectionKey, key: &Key, db: &DbHeap) -> Result<Op
     let col = db.get(collection);
 
     match col {
-        None => Err([COLLECTION_NOT_FOUND, collection].join("")),
+        None => Err(msg_db_collection_not_found(collection)),
         Some(col) => {
             let value = col.get(key);
 
@@ -193,7 +193,7 @@ pub fn get_docs_heap<'a>(
     let col = db.get(collection);
 
     match col {
-        None => Err([COLLECTION_NOT_FOUND, collection].join("")),
+        None => Err(msg_db_collection_not_found(collection)),
         Some(col) => {
             let items = col.iter().collect();
             Ok(items)
@@ -205,7 +205,7 @@ pub fn count_docs_heap(collection: &CollectionKey, db: &DbHeap) -> Result<usize,
     let col = db.get(collection);
 
     match col {
-        None => Err([COLLECTION_NOT_FOUND, collection].join("")),
+        None => Err(msg_db_collection_not_found(collection)),
         Some(col) => Ok(col.len()),
     }
 }
@@ -278,7 +278,7 @@ fn insert_doc_heap(
     let col = db.get_mut(collection);
 
     match col {
-        None => Err([COLLECTION_NOT_FOUND, collection].join("")),
+        None => Err(msg_db_collection_not_found(collection)),
         Some(col) => {
             let evicted_doc = limit_docs_heap_capacity(max_capacity, col);
 
@@ -318,7 +318,7 @@ fn delete_doc_heap(
     let col = db.get_mut(collection);
 
     match col {
-        None => Err([COLLECTION_NOT_FOUND, collection].join("")),
+        None => Err(msg_db_collection_not_found(collection)),
         Some(col) => {
             let deleted_doc = col.remove(key);
 
@@ -345,7 +345,7 @@ pub fn get_rule(collection: &CollectionKey) -> Result<Rule, String> {
     });
 
     match rule {
-        None => Err([COLLECTION_NOT_FOUND, collection].join("")),
+        None => Err(msg_db_collection_not_found(collection)),
         Some(rule) => Ok(rule),
     }
 }

src/libs/satellite/src/db/store.rs

@@ -18,7 +18,7 @@ use candid::Principal;
 use junobuild_collections::assert_stores::{
     assert_create_permission, assert_permission, public_permission,
 };
-use junobuild_collections::msg::COLLECTION_NOT_EMPTY;
+use junobuild_collections::msg::msg_db_collection_not_empty;
 use junobuild_collections::types::core::CollectionKey;
 use junobuild_collections::types::rules::{Memory, Permission, Rule};
 use junobuild_shared::assert::{assert_description_length, assert_max_memory_size, assert_version};
@@ -49,7 +49,7 @@ fn delete_collection_impl(
     let empty = is_state_collection_empty(collection, memory)?;
 
     if !empty {
-        return Err([COLLECTION_NOT_EMPTY, collection].join(""));
+        return Err(msg_db_collection_not_empty(collection));
     }
 
     delete_state_collection(collection, memory)?;

src/libs/satellite/src/storage/state.rs

@@ -3,7 +3,7 @@ use crate::storage::types::state::{
     AssetsStable, ContentChunksStable, StableEncodingChunkKey, StableKey,
 };
 use crate::types::state::StableState;
-use junobuild_collections::msg::COLLECTION_NOT_FOUND;
+use junobuild_collections::msg::msg_storage_collection_not_found;
 use junobuild_collections::types::core::CollectionKey;
 use junobuild_collections::types::rules::{Memory, Rule};
 use junobuild_collections::utils::range_collection_end;
@@ -270,7 +270,7 @@ pub fn get_rule(collection: &CollectionKey) -> Result<Rule, String> {
     });
 
     match rule {
-        None => Err([COLLECTION_NOT_FOUND, collection].join("")),
+        None => Err(msg_storage_collection_not_found(collection)),
         Some(rule) => Ok(rule),
     }
 }

src/libs/satellite/src/storage/store.rs

@@ -3,7 +3,7 @@ use crate::hooks::invoke_assert_delete_asset;
 use crate::memory::STATE;
 use candid::Principal;
 use junobuild_collections::assert_stores::{assert_permission, public_permission};
-use junobuild_collections::msg::COLLECTION_NOT_EMPTY;
+use junobuild_collections::msg::msg_storage_collection_not_empty;
 use junobuild_collections::types::core::CollectionKey;
 use junobuild_collections::types::rules::{Memory, Rule};
 use junobuild_shared::controllers::is_controller;
@@ -240,7 +240,7 @@ fn assert_assets_collection_empty_impl(
     let values = filter_collection_values(collection.clone(), assets);
 
     if !values.is_empty() {
-        return Err([COLLECTION_NOT_EMPTY, collection].join(""));
+        return Err(msg_storage_collection_not_empty(collection));
     }
 
     Ok(())

src/tests/satellite.datastore.spec.ts

@@ -485,6 +485,46 @@ describe.each([{ memory: { Heap: null } }, { memory: { Stable: null } }])(
 			});
 		});
 
+		describe('collection', () => {
+			beforeAll(async () => {
+				actor.setIdentity(controller);
+			});
+
+			it('should not delete not empty collection', async () => {
+				const { del_rule } = actor;
+
+				try {
+					await del_rule({ Db: null }, TEST_COLLECTION, { version: [1n] });
+
+					expect(true).toBe(false);
+				} catch (error: unknown) {
+					expect((error as Error).message).toContain(
+						`The "${TEST_COLLECTION}" collection in Datastore is not empty.`
+					);
+				}
+			});
+
+			it('should not set doc in unknown collection', async () => {
+				const { set_doc } = actor;
+
+				const collectionUnknown = 'unknown';
+
+				try {
+					await set_doc(collectionUnknown, nanoid(), {
+						data,
+						description: toNullable(),
+						version: toNullable()
+					});
+
+					expect(true).toBe(false);
+				} catch (error: unknown) {
+					expect((error as Error).message).toContain(
+						`Collection "${collectionUnknown}" not found in Datastore.`
+					);
+				}
+			});
+		});
+
 		describe('config', () => {
 			const setRule: SetRule = {
 				memory: toNullable(memory),

src/tests/satellite.storage.spec.ts

@@ -134,6 +134,45 @@ describe('Satellite storage', () => {
 		});
 	});
 
+	const HTML = '<html><body>Hello</body></html>';
+
+	const blob = new Blob([HTML], {
+		type: 'text/plain; charset=utf-8'
+	});
+
+	const upload = async ({
+		full_path,
+		name,
+		collection
+	}: {
+		full_path: string;
+		name: string;
+		collection: string;
+	}) => {
+		const { commit_asset_upload, upload_asset_chunk, init_asset_upload } = actor;
+
+		const file = await init_asset_upload({
+			collection,
+			description: toNullable(),
+			encoding_type: [],
+			full_path,
+			name,
+			token: toNullable()
+		});
+
+		const chunk = await upload_asset_chunk({
+			batch_id: file.batch_id,
+			content: arrayBufferToUint8Array(await blob.arrayBuffer()),
+			order_id: [0n]
+		});
+
+		await commit_asset_upload({
+			batch_id: file.batch_id,
+			chunk_ids: [chunk.chunk_id],
+			headers: []
+		});
+	};
+
 	describe('admin', () => {
 		beforeAll(() => {
 			actor.setIdentity(controller);
@@ -250,37 +289,6 @@ describe('Satellite storage', () => {
 			({ memory }) => {
 				const collection = `test_${'Heap' in memory ? 'heap' : 'stable'}`;
 
-				const HTML = '<html><body>Hello</body></html>';
-
-				const blob = new Blob([HTML], {
-					type: 'text/plain; charset=utf-8'
-				});
-
-				const upload = async ({ full_path, name }: { full_path: string; name: string }) => {
-					const { commit_asset_upload, upload_asset_chunk, init_asset_upload } = actor;
-
-					const file = await init_asset_upload({
-						collection,
-						description: toNullable(),
-						encoding_type: [],
-						full_path,
-						name,
-						token: toNullable()
-					});
-
-					const chunk = await upload_asset_chunk({
-						batch_id: file.batch_id,
-						content: arrayBufferToUint8Array(await blob.arrayBuffer()),
-						order_id: [0n]
-					});
-
-					await commit_asset_upload({
-						batch_id: file.batch_id,
-						chunk_ids: [chunk.chunk_id],
-						headers: []
-					});
-				};
-
 				it('should create a collection', async () => {
 					const { set_rule, list_rules } = actor;
 
@@ -336,7 +344,7 @@ describe('Satellite storage', () => {
 					const name = 'hello.html';
 					const full_path = `/${collection}/${name}`;
 
-					await upload({ full_path, name });
+					await upload({ full_path, name, collection });
 
 					const { headers, body } = await http_request({
 						body: [],
@@ -388,14 +396,14 @@ describe('Satellite storage', () => {
 					const name = 'update.html';
 					const full_path = `/${collection}/${name}`;
 
-					await upload({ full_path, name });
+					await upload({ full_path, name, collection });
 
 					const asset = fromNullable(await get_asset(collection, full_path));
 
 					expect(asset).not.toBeUndefined();
 					expect(fromNullable(asset!.version) ?? 0n).toEqual(1n);
 
-					await upload({ full_path, name });
+					await upload({ full_path, name, collection });
 
 					const updatedAsset = fromNullable(await get_asset(collection, full_path));
 
@@ -897,6 +905,48 @@ describe('Satellite storage', () => {
 		);
 	});
 
+	describe('collection', () => {
+		beforeAll(async () => {
+			actor.setIdentity(controller);
+		});
+
+		it('should not delete not empty collection', async () => {
+			const { del_rule } = actor;
+
+			const collection = 'images_heap';
+
+			try {
+				await del_rule({ Storage: null }, collection, { version: [1n] });
+
+				expect(true).toBe(false);
+			} catch (error: unknown) {
+				expect((error as Error).message).toContain(
+					`The "${collection}" collection in Storage is not empty.`
+				);
+			}
+		});
+
+		it('should not upload file in existing collection', async () => {
+			const { set_doc } = actor;
+
+			const collectionUnknown = 'unknown';
+
+			try {
+				await upload({
+					full_path: `/${collectionUnknown}/hello.html`,
+					name: 'hello.html',
+					collection: collectionUnknown
+				});
+
+				expect(true).toBe(false);
+			} catch (error: unknown) {
+				expect((error as Error).message).toContain(
+					`Collection "${collectionUnknown}" not found in Storage.`
+				);
+			}
+		});
+	});
+
 	describe('More configuration', () => {
 		const maxHeapMemorySize = 3_932_160n;
 		const maxStableMemorySize = 2_000_000n;