Implement free_effort datatype callback, Make aux_load cb ignore invo…

…cations, update documentation

Signed-off-by: Karthik Subbarao <[email protected]>

src/commands/bloom_data_type.rs

@@ -15,29 +15,38 @@ pub static BLOOM_FILTER_TYPE: ValkeyType = ValkeyType::new(
         version: raw::REDISMODULE_TYPE_METHOD_VERSION as u64,
         rdb_load: Some(bloom_callback::bloom_rdb_load),
         rdb_save: Some(bloom_callback::bloom_rdb_save),
+        // TODO: We need to decide if AOF Rewrite should be supported.
+        // The reason being we can create bloom obects through AOF Rewrite. However, it will not
+        // have any items "added" to it.
         aof_rewrite: None,
 
         mem_usage: Some(bloom_callback::bloom_mem_usage),
+        // TODO
         digest: None,
         free: Some(bloom_callback::bloom_free),
 
         aux_load: Some(bloom_callback::bloom_aux_load),
-        aux_save: Some(bloom_callback::bloom_aux_save),
+        // Callback not needed as there is no AUX (out of keyspace) data to be saved.
+        aux_save: None,
         aux_save2: None,
         aux_save_triggers: raw::Aux::Before as i32,
 
-        free_effort: None,
+        free_effort: Some(bloom_callback::bloom_free_effort),
+        // Callback not needed as it just notifies us when a bloom item is about to be freed.
         unlink: None,
         copy: Some(bloom_callback::bloom_copy),
+        // TODO
         defrag: None,
 
+        // The callbacks below are not needed since the version 1 variants are used when implemented.
         mem_usage2: None,
         free_effort2: None,
         unlink2: None,
         copy2: None,
     },
 );
 
+/// Callback to load and parse RDB data of a bloom item and create it.
 pub fn bloom_rdb_load_data_object(
     rdb: *mut raw::RedisModuleIO,
     encver: i32,
@@ -107,13 +116,8 @@ pub fn bloom_rdb_load_data_object(
     Some(item)
 }
 
-// Save the auxiliary data outside of the regular keyspace to the RDB file
-pub fn bloom_rdb_aux_save(_rdb: *mut raw::RedisModuleIO) {
-    logging::log_notice("NOOP for now");
-}
-
-// Load the auxiliary data outside of the regular keyspace from the RDB file
+/// Load the auxiliary data outside of the regular keyspace from the RDB file
 pub fn bloom_rdb_aux_load(_rdb: *mut raw::RedisModuleIO) -> c_int {
-    logging::log_notice("NOOP for now");
+    logging::log_notice("Ignoring AUX fields during RDB load.");
     raw::Status::Ok as i32
 }

src/commands/bloom_util.rs

@@ -46,6 +46,17 @@ impl BloomFilterType {
         mem
     }
 
+    /// Returns the Bloom object's free_effort.
+    /// We return 1 if there are no filters (BF.RESERVE) or if there is 1 filter.
+    /// Else, we return the number of filters as the free_effort.
+    /// This is similar to how the core handles aggregated objects.
+    pub fn free_effort(&self) -> usize {
+        if self.filters.is_empty() {
+            return 1;
+        }
+        self.filters.len()
+    }
+
     /// Check if item exists already.
     pub fn item_exists(&self, item: &[u8]) -> bool {
         for filter in &self.filters {

src/wrapper/bloom_callback.rs

@@ -49,12 +49,6 @@ pub unsafe extern "C" fn bloom_rdb_load(
     }
 }
 
-/// # Safety
-/// Save auxiliary data to RDB
-pub unsafe extern "C" fn bloom_aux_save(rdb: *mut raw::RedisModuleIO, _when: c_int) {
-    bloom_data_type::bloom_rdb_aux_save(rdb)
-}
-
 /// # Safety
 /// Load auxiliary data from RDB
 pub unsafe extern "C" fn bloom_aux_load(
@@ -86,8 +80,21 @@ pub unsafe extern "C" fn bloom_copy(
     _to_key: *mut RedisModuleString,
     value: *const c_void,
 ) -> *mut c_void {
-    let cur_item = &*value.cast::<BloomFilterType>();
-    let new_item = BloomFilterType::create_copy_from(cur_item);
+    let curr_item = &*value.cast::<BloomFilterType>();
+    let new_item = BloomFilterType::create_copy_from(curr_item);
     let bb = Box::new(new_item);
     Box::into_raw(bb).cast::<libc::c_void>()
 }
+
+/// # Safety
+/// Raw handler for the Bloom object's free_effort callback.
+/// We return 1 if there are no filters (BF.RESERVE) or if there is 1 filter.
+/// Else, we return the number of filters as the free_effort.
+/// This is similar to how the core handles aggregated objects.
+pub unsafe extern "C" fn bloom_free_effort(
+    _from_key: *mut RedisModuleString,
+    value: *const c_void,
+) -> usize {
+    let curr_item = &*value.cast::<BloomFilterType>();
+    curr_item.free_effort()
+}