Feat/prepare findex server v7 #93

HatemMn · 2024-11-27T17:52:41Z

Previous related PR : #92 , closed for convenience

What's new :

generic tests
redis store
- New feature : storing the script in the db cache
exporting necessary primitives for findex-server

feat: fix the bugs

test:add generic memory ADT tests test:add ccr test (fails)

fix:fix import of test fix:re-order tests fix:notyes feat: fix the bugs lol fix: fix the code fix: fix clippy ci fix: vscode folder fix: rip first test test: change async tests fix: ci

fix: review fixes monday2 fix: review fixes monday3 fix: clippy fix: clippy2 fix: clippy3

fix: last fixes test:change concurrency test fix:newline issues chore:Update src/in_memory_store.rs Co-authored-by: Théophile BRÉZOT <[email protected]> fix:review fixes fix:review fixes2 fix:review fixes3 fix:review fixes4 fix:review fixes5

feat:clean deps and adapt word typing to byte sequence test:change concurrent test signatures

tbrezot

Functional modifications should not include formatting modifications, this makes proofreading tedious and even less bulletproof.

Overall okay, but many details to improve: please read your diffs before publishing a MR.

Try removing the clones in your guarded_write. I would be surprise they are necessary.

tbrezot · 2024-11-29T17:09:54Z

.rustfmt.toml

-version = "Two"
+style_edition = "2024"


Why did you change that ?

Deprecated.

Also I suggest we simply commit a rust-toolchain file like done in findex server. I committed one, make me notice if you disagree with the idea to delete it

Update after discussion with @Manuthor

We thought of setting the rust toolchain to a stable release, notably :

[toolchain] channel = "1.83.0" components = ["rustfmt"]

And to format, we use cargo +nightly fmt --all
what do you think ?

Cargo.toml

tbrezot · 2024-11-29T17:11:43Z

Cargo.toml

@@ -14,12 +16,26 @@ rand_core = "0.6.4"
 tiny-keccak = { version = "2.0.2", features = ["sha3"] }
 xts-mode = "0.5.1"
 zeroize = { version = "1.8.1", features = ["derive"] }
+redis = { version = "0.27.5", features = [
+    "aio",
+    "ahash",


Why do you need that?

"aio", : for async operations involving connection manager

ahash : for hashing that write script and keeping it on cache

tbrezot · 2024-11-29T17:18:13Z

src/findex.rs

+
+    #[tokio::test]
+    #[cfg(feature = "redis-store")]
+    async fn test_redis_insert_search_delete_search() {


If correctly implemented, changing the memory should not influence the correctness of Findex. I agree though that this is in theory and testing is better than trusting. However, this test re-implements the exact same logic as the previous one : why not making the test generic instead?

Moreover, this is findex.rs, I would rather like not seeing any memory related code there. Move this test to src/tests/findex-redis.rs.

I agree with your first paragraph. Findex should work all ways so instead of moving this I will generize it later after we fix what we already have. I delete it for now

src/findex.rs

tbrezot · 2024-11-29T17:49:18Z

src/memory/in_memory_store.rs

@@ -31,6 +31,7 @@ impl<Address: Hash + Eq + Debug, Value: Clone + Eq + Debug> Default for InMemory
    }
 }

+#[cfg(any(test, feature = "bench"))]


why do you need this ?

compiler keeps nagging if taken off

method `clear` is never used `#[warn(dead_code)]` on by defaultrustc

can't delete test, it's needed in ovec.rs

src/test/memory.rs

tbrezot · 2024-11-29T17:51:02Z

src/test/memory.rs

+#[cfg(feature = "test-utils")]
+use crate::MemoryADT;
+
+// TODO: should return a result type and proper errors


Should this be solved ?

I kept that for this review to ask a question : are the tests good as they are or shall I create some " tesResult " type with proper error type ? I just want to make sure it's good this way

src/test/memory.rs

tbrezot · 2024-11-30T16:25:33Z

Tests do not pass on my machine (cargo test --all-features):

thread 'findex::tests::test_redis_insert_search_delete_search' panicked at src/findex.rs:299:65:
called `Result::unwrap()` on an `Err` value: MissingValue(Address([214, 22, 96, 125, 62, 75, 169, 106, 116, 243, 35, 207, 252, 95, 32, 163]), 0)

tbrezot · 2024-11-30T16:29:06Z

Does not compile on my machine (cargo clippy --all-targets --no-default-features --features bench):

error[E0432]: unresolved imports `findex::dummy_decode`, `findex::dummy_encode`
  --> benches/benches.rs:5:69
   |
5  |     Findex, InMemory, IndexADT, MemoryADT, Op, Secret, WORD_LENGTH, dummy_decode, dummy_encode,
   |                                                                     ^^^^^^^^^^^^  ^^^^^^^^^^^^ no `dummy_encode` in the root
   |                                                                     |
   |                                                                     no `dummy_decode` in the root

HatemMn

The tests do pass now, please notify me if they still flacky

HatemMn · 2024-12-02T10:33:03Z

Cargo.toml

@@ -14,12 +16,26 @@ rand_core = "0.6.4"
 tiny-keccak = { version = "2.0.2", features = ["sha3"] }
 xts-mode = "0.5.1"
 zeroize = { version = "1.8.1", features = ["derive"] }
+redis = { version = "0.27.5", features = [
+    "aio",
+    "ahash",


"aio", : for async operations involving connection manager

ahash : for hashing that write script and keeping it on cache

HatemMn · 2024-12-02T10:34:34Z

src/findex.rs

+
+    #[tokio::test]
+    #[cfg(feature = "redis-store")]
+    async fn test_redis_insert_search_delete_search() {


I agree with your first paragraph. Findex should work all ways so instead of moving this I will generize it later after we fix what we already have. I delete it for now

HatemMn · 2024-12-02T13:07:54Z

.rustfmt.toml

-version = "Two"
+style_edition = "2024"


Update after discussion with @Manuthor

We thought of setting the rust toolchain to a stable release, notably :

[toolchain] channel = "1.83.0" components = ["rustfmt"]

And to format, we use cargo +nightly fmt --all
what do you think ?

HatemMn · 2024-12-02T13:48:55Z

src/lib.rs

 pub use findex::Findex;
+#[cfg(feature = "redis-store")]
+pub use memory::db_stores::redis_store::RedisStore;


done, I fixed the error enum type

src/memory/db_stores/error.rs

src/memory/db_stores/redis_store.rs

HatemMn · 2024-12-02T15:27:56Z

src/memory/in_memory_store.rs

@@ -31,6 +31,7 @@ impl<Address: Hash + Eq + Debug, Value: Clone + Eq + Debug> Default for InMemory
    }
 }

+#[cfg(any(test, feature = "bench"))]


compiler keeps nagging if taken off

method `clear` is never used `#[warn(dead_code)]` on by defaultrustc

HatemMn · 2024-12-02T15:28:45Z

src/memory/in_memory_store.rs

@@ -31,6 +31,7 @@ impl<Address: Hash + Eq + Debug, Value: Clone + Eq + Debug> Default for InMemory
    }
 }

+#[cfg(any(test, feature = "bench"))]


can't delete test, it's needed in ovec.rs

HatemMn · 2024-12-02T15:33:08Z

src/test/memory.rs

+#[cfg(feature = "test-utils")]
+use crate::MemoryADT;
+
+// TODO: should return a result type and proper errors


I kept that for this review to ask a question : are the tests good as they are or shall I create some " tesResult " type with proper error type ? I just want to make sure it's good this way

HatemMn · 2024-12-02T16:02:55Z

src/memory/db_stores/redis_store.rs

+
+    #[tokio::test]
+    #[serial]
+    async fn test_db_flush() -> Result<(), RedisStoreError> {


performed a coverage report ;

with this functions :

Without it :

The gain is indeed negligible, proceeding to delete it

* fix: cleanup deny.toml and delete docker-compose.yml * fix: remove .rustfmt.toml

HatemMn added 5 commits November 6, 2024 14:10

feat: expose dummy encode decode + add toRedisArg

90474b5

feat: fix the bugs

feat: no more sync futures in memory ADT

6e0ab96

test:add generic memory ADT tests + delete deprecated vscode config

7f0acb2

test:add generic memory ADT tests test:add ccr test (fails)

test:add ccr test (works) + expose tests to consumers on test cfg

4cb5af7

fix:fix import of test fix:re-order tests fix:notyes feat: fix the bugs lol fix: fix the code fix: fix clippy ci fix: vscode folder fix: rip first test test: change async tests fix: ci

fix: create test submodule + clean tokio dependency

516ed3b

fix: review fixes monday2 fix: review fixes monday3 fix: clippy fix: clippy2 fix: clippy3

HatemMn mentioned this pull request Nov 27, 2024

test/test_memory #92

Closed

HatemMn added 2 commits November 27, 2024 18:53

fix: review fixes

06be981

fix: last fixes test:change concurrency test fix:newline issues chore:Update src/in_memory_store.rs Co-authored-by: Théophile BRÉZOT <[email protected]> fix:review fixes fix:review fixes2 fix:review fixes3 fix:review fixes4 fix:review fixes5

feat: implement from/to [u8;LEN] for address type, update tests

33506ee

feat:clean deps and adapt word typing to byte sequence test:change concurrent test signatures

HatemMn force-pushed the feat/prepare_findex_server_v7 branch from 0f589c4 to 33506ee Compare November 27, 2024 17:54

HatemMn self-assigned this Nov 27, 2024

HatemMn added 6 commits November 27, 2024 19:19

feat: expore findex core error type

4b4f144

feat: add redis store

d06592b

feat: redis store implemented, working tested ok

3884ee9

feat: connection manager OK

f6ac736

chore: run cargo fmt to pass the lovely CI

0791844

fix: deny rsa bug

c2a24e2

HatemMn requested review from tbrezot and Manuthor November 29, 2024 16:22

tbrezot reviewed Nov 29, 2024

View reviewed changes

tbrezot force-pushed the feat/prepare_findex_server_v7 branch from 049dd01 to c2a24e2 Compare November 30, 2024 16:42

Manuthor and others added 6 commits December 2, 2024 10:55

fix: cargo deny + lints

cd2d6ea

style: rename redis-store to redis-mem

dc3a1f9

feat: put alltogether memories

af8d9e0

chore: merge

05a19bd

fix : errors + review fixes

5554076

fix more fixes

5a83afc

HatemMn force-pushed the feat/prepare_findex_server_v7 branch from b5621d8 to 5a83afc Compare December 2, 2024 16:17

style: nightly formatting

a2bb2b4

HatemMn commented Dec 2, 2024

View reviewed changes

chore: delete and rename various files

25ea96b

HatemMn assigned Manuthor and tbrezot and unassigned HatemMn Dec 2, 2024

tbrezot and others added 5 commits December 3, 2024 10:52

review

dc9469b

unmodify lib.rs

87bf53a

rename without _store

3199015

remove test_utils feature

2cded1e

fix: cleanup deny.toml and delete docker-compose.yml (#97)

2ee118d

* fix: cleanup deny.toml and delete docker-compose.yml * fix: remove .rustfmt.toml

Base automatically changed from eprint to develop January 15, 2025 09:29

tbrezot closed this Jan 15, 2025

tbrezot deleted the feat/prepare_findex_server_v7 branch January 15, 2025 09:35

tbrezot restored the feat/prepare_findex_server_v7 branch January 15, 2025 09:49

HatemMn mentioned this pull request Jan 15, 2025

Feat/findex server v7 #112

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/prepare findex server v7 #93

Feat/prepare findex server v7 #93

HatemMn commented Nov 27, 2024 •

edited

Loading

tbrezot left a comment

tbrezot Nov 29, 2024

HatemMn Dec 2, 2024

HatemMn Dec 2, 2024

tbrezot Nov 29, 2024

HatemMn Dec 2, 2024

tbrezot Nov 29, 2024

HatemMn Dec 2, 2024

tbrezot Nov 29, 2024

HatemMn Dec 2, 2024

HatemMn Dec 2, 2024

tbrezot Nov 29, 2024 •

edited

Loading

HatemMn Dec 2, 2024

tbrezot commented Nov 30, 2024

tbrezot commented Nov 30, 2024

HatemMn left a comment

HatemMn Dec 2, 2024

HatemMn Dec 2, 2024

HatemMn Dec 2, 2024

HatemMn Dec 2, 2024

HatemMn Dec 2, 2024

HatemMn Dec 2, 2024

HatemMn Dec 2, 2024

HatemMn Dec 2, 2024

Feat/prepare findex server v7 #93

Feat/prepare findex server v7 #93

Conversation

HatemMn commented Nov 27, 2024 • edited Loading

tbrezot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbrezot Nov 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbrezot commented Nov 30, 2024

tbrezot commented Nov 30, 2024

HatemMn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HatemMn commented Nov 27, 2024 •

edited

Loading

tbrezot Nov 29, 2024 •

edited

Loading