From f0ddbe3f482d88827e52b16ac7e55acd2593ab03 Mon Sep 17 00:00:00 2001 From: David Dal Busco Date: Sun, 3 Nov 2024 21:34:14 +0100 Subject: [PATCH] refactor: use store context for less methods parameters (#779) Signed-off-by: David Dal Busco --- src/libs/satellite/src/db/assert.rs | 18 ++-- src/libs/satellite/src/db/store.rs | 110 +++++++++--------------- src/libs/satellite/src/storage/store.rs | 109 ++++++++++++----------- src/libs/satellite/src/types.rs | 11 +++ 4 files changed, 123 insertions(+), 125 deletions(-) diff --git a/src/libs/satellite/src/db/assert.rs b/src/libs/satellite/src/db/assert.rs index 005a5b39a..7d6f4f410 100644 --- a/src/libs/satellite/src/db/assert.rs +++ b/src/libs/satellite/src/db/assert.rs @@ -3,22 +3,24 @@ use crate::db::types::config::DbConfig; use crate::db::types::state::{DocAssertDelete, DocAssertSet, DocContext}; use crate::hooks::{invoke_assert_delete_doc, invoke_assert_set_doc}; use crate::rules::assert_stores::assert_user_collection_caller_key; +use crate::types::store::StoreContext; use crate::{DelDoc, Doc, SetDoc}; use candid::Principal; use junobuild_collections::assert_stores::{ assert_create_permission, assert_permission, public_permission, }; -use junobuild_collections::types::core::CollectionKey; use junobuild_collections::types::rules::{Permission, Rule}; use junobuild_shared::assert::{assert_description_length, assert_max_memory_size, assert_version}; use junobuild_shared::types::core::Key; use junobuild_shared::types::state::{Controllers, Version}; pub fn assert_set_doc( - caller: Principal, - controllers: &Controllers, + &StoreContext { + caller, + controllers, + collection, + }: &StoreContext, config: &Option, - collection: &CollectionKey, key: &Key, value: &SetDoc, rule: &Rule, @@ -50,9 +52,11 @@ pub fn assert_set_doc( } pub fn assert_delete_doc( - caller: Principal, - controllers: &Controllers, - collection: &CollectionKey, + &StoreContext { + caller, + controllers, + collection, + }: &StoreContext, key: &Key, value: &DelDoc, rule: &Rule, diff --git a/src/libs/satellite/src/db/store.rs b/src/libs/satellite/src/db/store.rs index 1b1e9ff52..1adbee1d7 100644 --- a/src/libs/satellite/src/db/store.rs +++ b/src/libs/satellite/src/db/store.rs @@ -12,6 +12,7 @@ use crate::db::types::interface::{DelDoc, SetDoc}; use crate::db::types::state::{Doc, DocContext, DocUpsert}; use crate::db::utils::filter_values; use crate::memory::STATE; +use crate::types::store::StoreContext; use candid::Principal; use junobuild_collections::assert_stores::assert_permission; use junobuild_collections::msg::msg_db_collection_not_empty; @@ -79,32 +80,27 @@ pub fn get_doc_store( ) -> Result, String> { let controllers: Controllers = get_controllers(); - secure_get_doc(caller, &controllers, collection, key) + let context = StoreContext { + caller, + controllers: &controllers, + collection: &collection, + }; + + secure_get_doc(&context, key) } -fn secure_get_doc( - caller: Principal, - controllers: &Controllers, - collection: CollectionKey, - key: Key, -) -> Result, String> { - let rule = get_state_rule(&collection)?; - get_doc_impl(caller, controllers, collection, key, &rule) +fn secure_get_doc(context: &StoreContext, key: Key) -> Result, String> { + let rule = get_state_rule(context.collection)?; + get_doc_impl(context, key, &rule) } -fn get_doc_impl( - caller: Principal, - controllers: &Controllers, - collection: CollectionKey, - key: Key, - rule: &Rule, -) -> Result, String> { - let value = get_state_doc(&collection, &key, rule)?; +fn get_doc_impl(context: &StoreContext, key: Key, rule: &Rule) -> Result, String> { + let value = get_state_doc(context.collection, &key, rule)?; match value { None => Ok(None), Some(value) => { - if !assert_permission(&rule.read, value.owner, caller, controllers) { + if !assert_permission(&rule.read, value.owner, context.caller, context.controllers) { return Ok(None); } @@ -142,14 +138,13 @@ pub fn set_doc_store( let controllers: Controllers = get_controllers(); let config = get_config(); - let data = secure_set_doc( + let context = StoreContext { caller, - &controllers, - &config, - collection.clone(), - key.clone(), - value, - )?; + controllers: &controllers, + collection: &collection, + }; + + let data = secure_set_doc(&context, &config, key.clone(), value)?; Ok(DocContext { key, @@ -159,42 +154,29 @@ pub fn set_doc_store( } fn secure_set_doc( - caller: Principal, - controllers: &Controllers, + context: &StoreContext, config: &Option, - collection: CollectionKey, key: Key, value: SetDoc, ) -> Result { - let rule = get_state_rule(&collection)?; - set_doc_impl(caller, controllers, config, collection, key, value, &rule) + let rule = get_state_rule(context.collection)?; + set_doc_impl(context, config, key, value, &rule) } fn set_doc_impl( - caller: Principal, - controllers: &Controllers, + context: &StoreContext, config: &Option, - collection: CollectionKey, key: Key, value: SetDoc, rule: &Rule, ) -> Result { - let current_doc = get_state_doc(&collection, &key, rule)?; + let current_doc = get_state_doc(context.collection, &key, rule)?; - assert_set_doc( - caller, - controllers, - config, - &collection, - &key, - &value, - rule, - ¤t_doc, - )?; + assert_set_doc(context, config, &key, &value, rule, ¤t_doc)?; - let doc: Doc = Doc::prepare(caller, ¤t_doc, value); + let doc: Doc = Doc::prepare(context.caller, ¤t_doc, value); - let (_evicted_doc, after) = insert_state_doc(&collection, &key, &doc, rule)?; + let (_evicted_doc, after) = insert_state_doc(context.collection, &key, &doc, rule)?; Ok(DocUpsert { before: current_doc, @@ -325,7 +307,13 @@ pub fn delete_doc_store( ) -> Result>, String> { let controllers: Controllers = get_controllers(); - let doc = secure_delete_doc(caller, &controllers, collection.clone(), key.clone(), value)?; + let context = StoreContext { + caller, + controllers: &controllers, + collection: &collection, + }; + + let doc = secure_delete_doc(&context, key.clone(), value)?; Ok(DocContext { key, @@ -335,37 +323,25 @@ pub fn delete_doc_store( } fn secure_delete_doc( - caller: Principal, - controllers: &Controllers, - collection: CollectionKey, + context: &StoreContext, key: Key, value: DelDoc, ) -> Result, String> { - let rule = get_state_rule(&collection)?; - delete_doc_impl(caller, controllers, collection, key, value, &rule) + let rule = get_state_rule(context.collection)?; + delete_doc_impl(context, key, value, &rule) } fn delete_doc_impl( - caller: Principal, - controllers: &Controllers, - collection: CollectionKey, + context: &StoreContext, key: Key, value: DelDoc, rule: &Rule, ) -> Result, String> { - let current_doc = get_state_doc(&collection, &key, rule)?; + let current_doc = get_state_doc(context.collection, &key, rule)?; - assert_delete_doc( - caller, - controllers, - &collection, - &key, - &value, - rule, - ¤t_doc, - )?; - - delete_state_doc(&collection, &key, rule) + assert_delete_doc(context, &key, &value, rule, ¤t_doc)?; + + delete_state_doc(context.collection, &key, rule) } /// Delete multiple documents from a collection's store. diff --git a/src/libs/satellite/src/storage/store.rs b/src/libs/satellite/src/storage/store.rs index c0cf4c6c2..ce1413058 100644 --- a/src/libs/satellite/src/storage/store.rs +++ b/src/libs/satellite/src/storage/store.rs @@ -21,6 +21,7 @@ use crate::storage::state::{ insert_domain as insert_state_domain, }; use crate::storage::strategy_impls::{StorageAssertions, StorageState, StorageUpload}; +use crate::types::store::StoreContext; use junobuild_shared::types::core::{Blob, DomainName}; use junobuild_shared::types::domain::CustomDomains; use junobuild_shared::types::list::{ListParams, ListResults}; @@ -99,7 +100,13 @@ pub fn delete_asset_store( let controllers: Controllers = get_controllers(); let config = get_config_store(); - secure_delete_asset_impl(caller, &controllers, collection, full_path, &config) + let context = StoreContext { + caller, + controllers: &controllers, + collection, + }; + + secure_delete_asset_impl(&context, full_path, &config) } pub fn delete_assets_store(collection: &CollectionKey) -> Result<(), String> { @@ -150,7 +157,13 @@ pub fn list_assets_store( ) -> Result, String> { let controllers: Controllers = get_controllers(); - secure_list_assets_impl(caller, &controllers, collection, filters) + let context = StoreContext { + caller, + controllers: &controllers, + collection, + }; + + secure_list_assets_impl(&context, filters) } /// Count assets in a collection. @@ -206,33 +219,40 @@ pub fn get_asset_store( ) -> Result, String> { let controllers: Controllers = get_controllers(); - secure_get_asset_impl(caller, &controllers, collection, full_path) + let context = StoreContext { + caller, + controllers: &controllers, + collection, + }; + + secure_get_asset_impl(&context, full_path) } fn secure_get_asset_impl( - caller: Principal, - controllers: &Controllers, - collection: &CollectionKey, + context: &StoreContext, full_path: FullPath, ) -> Result, String> { - let rule = get_state_rule(collection)?; + let rule = get_state_rule(context.collection)?; - get_asset_impl(caller, controllers, full_path, collection, &rule) + get_asset_impl(context, full_path, &rule) } fn get_asset_impl( - caller: Principal, - controllers: &Controllers, + context: &StoreContext, full_path: FullPath, - collection: &CollectionKey, rule: &Rule, ) -> Result, String> { - let asset = get_state_asset(collection, &full_path, rule); + let asset = get_state_asset(context.collection, &full_path, rule); match asset { None => Ok(None), Some(asset) => { - if !assert_permission(&rule.read, asset.key.owner, caller, controllers) { + if !assert_permission( + &rule.read, + asset.key.owner, + context.caller, + context.controllers, + ) { return Ok(None); } @@ -293,49 +313,35 @@ fn assert_assets_collection_empty_impl( } fn secure_list_assets_impl( - caller: Principal, - controllers: &Controllers, - collection: &CollectionKey, + context: &StoreContext, filters: &ListParams, ) -> Result, String> { - let rule = get_state_rule(collection)?; + let rule = get_state_rule(context.collection)?; match rule.mem() { Memory::Heap => STATE.with(|state| { let state_ref = state.borrow(); - let assets = collect_assets_heap(collection, &state_ref.heap.storage.assets); - Ok(list_assets_impl( - &assets, - caller, - controllers, - collection, - &rule, - filters, - )) + let assets = collect_assets_heap(context.collection, &state_ref.heap.storage.assets); + Ok(list_assets_impl(&assets, context, &rule, filters)) }), Memory::Stable => STATE.with(|state| { - let stable = get_assets_stable(collection, &state.borrow().stable.assets); + let stable = get_assets_stable(context.collection, &state.borrow().stable.assets); let assets: Vec<(&FullPath, &Asset)> = stable .iter() .map(|(_, asset)| (&asset.key.full_path, asset)) .collect(); - Ok(list_assets_impl( - &assets, - caller, - controllers, - collection, - &rule, - filters, - )) + Ok(list_assets_impl(&assets, context, &rule, filters)) }), } } fn list_assets_impl( assets: &[(&FullPath, &Asset)], - caller: Principal, - controllers: &Controllers, - collection: &CollectionKey, + &StoreContext { + caller, + controllers, + collection, + }: &StoreContext, rule: &Rule, filters: &ListParams, ) -> ListResults { @@ -364,43 +370,44 @@ fn list_assets_impl( } fn secure_delete_asset_impl( - caller: Principal, - controllers: &Controllers, - collection: &CollectionKey, + context: &StoreContext, full_path: FullPath, config: &StorageConfig, ) -> Result, String> { - let rule = get_state_rule(collection)?; + let rule = get_state_rule(context.collection)?; - delete_asset_impl(caller, controllers, full_path, collection, &rule, config) + delete_asset_impl(context, full_path, &rule, config) } fn delete_asset_impl( - caller: Principal, - controllers: &Controllers, + context: &StoreContext, full_path: FullPath, - collection: &CollectionKey, rule: &Rule, config: &StorageConfig, ) -> Result, String> { - let asset = get_state_asset(collection, &full_path, rule); + let asset = get_state_asset(context.collection, &full_path, rule); match asset { None => Err(ERROR_ASSET_NOT_FOUND.to_string()), Some(asset) => { - if !assert_permission(&rule.write, asset.key.owner, caller, controllers) { + if !assert_permission( + &rule.write, + asset.key.owner, + context.caller, + context.controllers, + ) { return Err(ERROR_ASSET_NOT_FOUND.to_string()); } - invoke_assert_delete_asset(&caller, &asset)?; + invoke_assert_delete_asset(&context.caller, &asset)?; - let deleted = delete_state_asset(collection, &full_path, rule); + let deleted = delete_state_asset(context.collection, &full_path, rule); delete_runtime_certified_asset(&asset); // We just removed the rewrite for /404.html in the certification tree therefore if /index.html exists, we want to reintroduce it as rewrite if *full_path == *ROOT_404_HTML { if let Some(index_asset) = - get_state_asset(collection, &ROOT_INDEX_HTML.to_string(), rule) + get_state_asset(context.collection, &ROOT_INDEX_HTML.to_string(), rule) { update_runtime_certified_asset(&index_asset, config); } diff --git a/src/libs/satellite/src/types.rs b/src/libs/satellite/src/types.rs index 3d251815f..72c7e3e26 100644 --- a/src/libs/satellite/src/types.rs +++ b/src/libs/satellite/src/types.rs @@ -64,6 +64,17 @@ pub mod interface { } } +pub mod store { + use junobuild_collections::types::core::CollectionKey; + use junobuild_shared::types::state::{Controllers, UserId}; + + pub struct StoreContext<'a> { + pub caller: UserId, + pub controllers: &'a Controllers, + pub collection: &'a CollectionKey, + } +} + pub mod hooks { use crate::db::types::state::{DocAssertDelete, DocAssertSet, DocContext, DocUpsert}; use crate::Doc;