Code cleanup: removing unused dependencies in tests, removing digest (#…

…44)

file as it can be taken from a dependency and moving metric increments
to after creates

Signed-off-by: zackcam <[email protected]>

.github/workflows/ci.yml

@@ -22,7 +22,7 @@ jobs:
     - name: Run cargo and clippy format check
       run: |
         cargo fmt --check
-        # cargo clippy --profile release --all-targets -- -D clippy::all
+        cargo clippy --profile release --all-targets -- -D clippy::all
     - name: Release Build
       run: RUSTFLAGS="-D warnings" cargo build --all --all-targets  --release
     - name: Run unit tests
@@ -56,7 +56,7 @@ jobs:
     - name: Run cargo and clippy format check
       run: |
         cargo fmt --check
-        # cargo clippy --profile release --all-targets -- -D clippy::all
+        cargo clippy --profile release --all-targets -- -D clippy::all
     - name: Release Build
       run: RUSTFLAGS="-D warnings" cargo build --all --all-targets --release
     - name: Run unit tests
@@ -75,7 +75,7 @@ jobs:
     - name: Run cargo and clippy format check
       run: |
         cargo fmt --check
-        # cargo clippy --profile release --all-targets -- -D clippy::all
+        cargo clippy --profile release --all-targets -- -D clippy::all
     - name: Release Build
       run: RUSTFLAGS="-D warnings" cargo build --all --all-targets  --release
     - name: Run unit tests

src/bloom/data_type.rs

@@ -2,9 +2,9 @@ use crate::bloom::utils::BloomFilter;
 use crate::bloom::utils::BloomObject;
 use crate::configs;
 use crate::wrapper::bloom_callback;
-use crate::wrapper::digest::Digest;
 use crate::MODULE_NAME;
 use std::os::raw::c_int;
+use valkey_module::digest::Digest;
 use valkey_module::native_types::ValkeyType;
 use valkey_module::{logging, raw};
 

src/bloom/utils.rs

@@ -161,14 +161,15 @@ impl BloomObject {
             let new_filter = Box::new(BloomFilter::create_copy_from(filter));
             filters.push(new_filter);
         }
-        from_bf.bloom_object_incr_metrics_on_new_create();
-        BloomObject {
+        let new_copy = BloomObject {
             expansion: from_bf.expansion,
             fp_rate: from_bf.fp_rate,
             tightening_ratio: from_bf.tightening_ratio,
             is_seed_random: from_bf.is_seed_random,
             filters,
-        }
+        };
+        new_copy.bloom_object_incr_metrics_on_new_create();
+        new_copy
     }
 
     /// Return the total memory usage of the BloomObject and every allocation it contains.
@@ -328,10 +329,7 @@ impl BloomObject {
             // Scale out by adding a new filter with capacity bounded within the u32 range. false positive rate is also
             // bound within the range f64::MIN_POSITIVE <= x < 1.0.
             let new_fp_rate =
-                match Self::calculate_fp_rate(self.fp_rate, num_filters, self.tightening_ratio) {
-                    Ok(rate) => rate,
-                    Err(e) => return Err(e),
-                };
+                Self::calculate_fp_rate(self.fp_rate, num_filters, self.tightening_ratio)?;
             let new_capacity = match filter.capacity.checked_mul(self.expansion.into()) {
                 Some(new_capacity) => new_capacity,
                 None => {
@@ -467,13 +465,13 @@ impl BloomObject {
                     is_seed_random,
                     filters,
                 };
+                // Add overall bloom object metrics.
+                item.bloom_object_incr_metrics_on_new_create();
                 let bytes = item.memory_usage();
                 // Reject the request, if the operation will result in creation of a bloom object of size greater than what is allowed.
                 if validate_size_limit && !BloomObject::validate_size(bytes) {
                     return Err(BloomError::ExceedsMaxBloomSize);
                 }
-                // Add overall bloom object metrics.
-                item.bloom_object_incr_metrics_on_new_create();
                 Ok(item)
             }
             _ => Err(BloomError::DecodeUnsupportedVersion),
@@ -672,7 +670,7 @@ impl BloomFilter {
             + (self.bloom.len() / 8) as usize
     }
 
-    /// Caculates the number of bytes that the bloom filter will require to be allocated.
+    /// Calculates the number of bytes that the bloom filter will require to be allocated.
     pub fn compute_size(capacity: i64, fp_rate: f64) -> usize {
         std::mem::size_of::<BloomFilter>()
             + std::mem::size_of::<bloomfilter::Bloom<[u8]>>()

src/lib.rs

@@ -51,13 +51,13 @@ fn bloom_reserve_command(ctx: &Context, args: Vec<ValkeyString>) -> ValkeyResult
     command_handler::bloom_filter_reserve(ctx, &args)
 }
 
-/// Command handler for BF.INFO <key> [CAPACITY | SIZE | FILTERS | ITEMS | EXPANSION]
+/// Command handler for BF.INFO <key> [CAPACITY | SIZE | FILTERS | ITEMS | EXPANSION | ERROR | MAXSCALEDCAPACITY]
 fn bloom_info_command(ctx: &Context, args: Vec<ValkeyString>) -> ValkeyResult {
     command_handler::bloom_filter_info(ctx, &args)
 }
 
 /// Command handler for:
-/// BF.INSERT <key> [ERROR <fp_error>] [CAPACITY <capacity>] [EXPANSION <expansion>] [NOCREATE] [NONSCALING] ITEMS <item> [<item> ...]
+/// BF.INSERT <key> [ERROR <fp_error>] [CAPACITY <capacity>] [EXPANSION <expansion>] [NOCREATE] [NONSCALING] [VALIDATESCALETO <validatescaleto>] ITEMS <item> [<item> ...]
 fn bloom_insert_command(ctx: &Context, args: Vec<ValkeyString>) -> ValkeyResult {
     command_handler::bloom_filter_insert(ctx, &args)
 }

src/wrapper/bloom_callback.rs

@@ -4,7 +4,6 @@ use crate::bloom::utils::BloomFilter;
 use crate::bloom::utils::BloomObject;
 use crate::configs;
 use crate::metrics;
-use crate::wrapper::digest::Digest;
 use bloomfilter::Bloom;
 use lazy_static::lazy_static;
 use std::ffi::CString;
@@ -13,6 +12,7 @@ use std::os::raw::{c_char, c_int, c_void};
 use std::ptr::null_mut;
 use std::sync::atomic::Ordering;
 use std::sync::Mutex;
+use valkey_module::digest::Digest;
 use valkey_module::logging;
 use valkey_module::logging::{log_io_error, ValkeyLogLevel};
 use valkey_module::raw;

src/wrapper/mod.rs

@@ -1,3 +1,2 @@
 pub mod bloom_callback;
 pub mod defrag;
-pub mod digest;

tests/test_bloom_acl_category.py

@@ -1,4 +1,3 @@
-import pytest
 from valkeytests.conftest import resource_port_tracker
 from valkey_bloom_test_case import ValkeyBloomTestCaseBase
 from util.waiters import *

tests/test_bloom_basic.py

@@ -1,11 +1,8 @@
 import time
-import pytest
 from util.waiters import *
 from valkey import ResponseError
 from valkey_bloom_test_case import ValkeyBloomTestCaseBase
 from valkeytests.conftest import resource_port_tracker
-import logging
-import os
 
 class TestBloomBasic(ValkeyBloomTestCaseBase):
 

tests/test_bloom_command.py

@@ -1,8 +1,5 @@
-import pytest
 from valkey_bloom_test_case import ValkeyBloomTestCaseBase
 from valkeytests.conftest import resource_port_tracker
-import valkey
-import uuid
 
 class TestBloomCommand(ValkeyBloomTestCaseBase):
 

tests/test_bloom_correctness.py

@@ -1,4 +1,3 @@
-import pytest
 from valkeytests.conftest import resource_port_tracker
 from valkey_bloom_test_case import ValkeyBloomTestCaseBase
 

tests/test_bloom_defrag.py

@@ -1,5 +1,4 @@
 import time
-from valkeytests.valkey_test_case import ValkeyAction
 from valkey_bloom_test_case import ValkeyBloomTestCaseBase
 from valkeytests.conftest import resource_port_tracker
 from util.waiters import *

tests/test_bloom_keyspace.py

@@ -1,4 +1,4 @@
-import logging, time
+import time
 from valkey_bloom_test_case import ValkeyBloomTestCaseBase
 from valkeytests.conftest import resource_port_tracker
 

tests/test_bloom_metrics.py

@@ -1,5 +1,4 @@
-import pytest, time
-import os
+import time
 from valkey_bloom_test_case import ValkeyBloomTestCaseBase
 from valkeytests.conftest import resource_port_tracker
 from util.waiters import *

tests/test_bloom_replication.py

@@ -1,8 +1,7 @@
-import pytest
+import pytest, os
 from valkey import ResponseError
 from valkeytests.valkey_test_case import ReplicationTestCase
 from valkeytests.conftest import resource_port_tracker
-import os
 
 class TestBloomReplication(ReplicationTestCase):
 

tests/test_bloom_save_and_restore.py

@@ -1,4 +1,3 @@
-import pytest, time
 import os
 from valkey import ResponseError
 from valkey_bloom_test_case import ValkeyBloomTestCaseBase

tests/valkeytests/conftest.py

@@ -8,9 +8,6 @@
 import socket
 import os
 import tempfile
-import random
-import subprocess
-import threading
 from pathlib import Path
 
 class PortTracker(object):

tests/valkeytests/util/waiters.py

@@ -14,8 +14,6 @@
 timeout: The maximum time to wait for the function to return the expected value (default: TEST_MAX_WAIT_TIME_SECONDS)
 ignore_exception: The exception to ignore (default is None)
 """
-import contextlib
-import warnings
 from functools import wraps
 import operator
 import time