[#1177] Add AtomDB public_key parameter and ProtectedAtomDB auth wrapper - #1206
Closed
marcocapozzoli wants to merge 6 commits into
Closed
[#1177] Add AtomDB public_key parameter and ProtectedAtomDB auth wrapper#1206marcocapozzoli wants to merge 6 commits into
marcocapozzoli wants to merge 6 commits into
Conversation
Introduce mandatory public_key on AtomDB methods, is_protected() on backends, ProtectedAtomDB wrapper in AtomDBSingleton, and update tests and call sites.
Contributor
WalkthroughAtomDB APIs now carry ChangesPublic-key AtomDB protection
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
✅ Action performedFull review finished. |
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/atomdb/remotedb/RemoteAtomDBPeer.cc (1)
45-69: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftThe peer cache is shared across public keys but consulted as if it were key-scoped.
get_atomprobescache_withpublic_key, and on a miss populates it withcache_.add_atom(atom.get(), public_key). Butcache_is a singleInMemoryDBthat ignorespublic_keyentirely, andget_cached_atom/node/linkread it back with"". OnceProtectedAtomDB-style filtering is real, an atom fetched on behalf of key A becomes readable by key B (and by the key-lessget_cached_*path) straight out of this cache — the authorization check is bypassed by the cache layer.Worth deciding now whether the cache is per-key, keyed by
(handle, public_key), or explicitly documented as pre-authorization storage that callers must re-filter.Also applies to: 123-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/atomdb/remotedb/RemoteAtomDBPeer.cc` around lines 45 - 69, The cache path in RemoteAtomDBPeer::get_atom must not bypass public-key authorization: replace the shared cache lookup/population with key-scoped storage keyed by both handle and public_key, or explicitly reapply the same authorization filtering before returning cached atoms. Ensure get_cached_atom/node/link cannot read atoms fetched for another key through the shared cache, and keep cache behavior consistent across all related access paths.src/atomdb/redis_mongodb/RedisMongoDB.cc (2)
1066-1090: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winNewly added
public_keyparameters are discarded at internal call sites. Several methods now acceptpublic_keybut immediately hardcode""when calling back into the keyed API, so the key never reaches the cascading work — the migration is a no-op for those paths and becomes an authorization gap onceProtectedAtomDBfiltering is implemented.
src/atomdb/redis_mongodb/RedisMongoDB.cc#L1066-L1090: forwardpublic_keyfromdelete_atom/delete_node/delete_linkinto the nesteddelete_node,delete_link,query_for_incoming_set, and recursivedelete_atom(target_handle, ...)calls (and thread it throughdelete_document).src/atomdb/morkdb/MorkDB.cc#L316-L356: forwardre_index_patterns'public_keyintothis->add_links(links, public_key, false, true)instead of"".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/atomdb/redis_mongodb/RedisMongoDB.cc` around lines 1066 - 1090, Forward the existing public_key through RedisMongoDB::delete_atom, delete_node, and delete_link, including nested delete_node/delete_link calls, query_for_incoming_set, recursive delete_atom, and delete_document instead of hardcoding an empty key. In src/atomdb/redis_mongodb/RedisMongoDB.cc lines 1066-1090, update each affected call accordingly. In src/atomdb/morkdb/MorkDB.cc lines 316-356, update re_index_patterns to pass its public_key to this->add_links rather than an empty string.
100-142: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign
protectedwith theatomdb.protectedconfig key and make MongoDB config mutability explicit.
config/das.jsoncurrently setsatomdb.protected, butRedisMongoDB::mongodb_setup()only readsatomdb.mongodb.protected, so the shipped JSON leaves protected mode off. Also, this only insertsprotected=true; if the MongoDB config document exists withfalse, the flag never upgrades, whileis_protected()always delegates to that stored value. Sync the DB value with the configured flag, or use a config key path that actually reflectsatomdb.protected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/atomdb/redis_mongodb/RedisMongoDB.cc` around lines 100 - 142, Update RedisMongoDB::mongodb_setup() to read the configured protected flag from atomdb.protected rather than mongodb.protected. When initializing the MongoDB config document, synchronize its protected field with is_protected: insert it when absent and update it when the stored value differs from configuration, so is_protected() reflects the configured mutability state.src/atomdb/inmemorydb/InMemoryDB.cc (1)
369-388: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
add_nodesreturns each handle twice.
handlesis already populated at Lines 370-372 for the existence check, then Line 385 pushes every handle again — so the result has2 * nodes.size()entries, andadd_atomspropagates the duplicates to callers. Use a separate vector for the existence check (andreserve()while you're there).🐛 Proposed fix
- vector<string> handles; + vector<string> handles; + handles.reserve(nodes.size()); for (const auto& node : nodes) { handles.push_back(node->handle()); } if (throw_if_exists) { auto existing_handles = this->nodes_exist(handles, public_key); @@ for (const auto& node : nodes) { - handles.push_back(this->add_node(node, public_key, throw_if_exists)); + this->add_node(node, public_key, throw_if_exists); } return handles;This may predate the PR, but Line 385 is being touched here and a regression test in
src/tests/cpp/inmemorydb_test.ccassertingadd_nodes(...).size() == nodes.size()would be cheap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/atomdb/inmemorydb/InMemoryDB.cc` around lines 369 - 388, Update add_nodes to use a separate reserved vector for handles collected for nodes_exist, while keeping the returned handles vector reserved and initially empty; then append each result from add_node only once so add_nodes and add_atoms return exactly one handle per input node.
🧹 Nitpick comments (2)
src/tests/cpp/adapterdb_test.cc (1)
240-247: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo coverage for the new
is_protected()accessor.
AdapterDB::is_protected()is added in this PR (seesrc/atomdb/adapterdb/AdapterDB.h) but no test exercises it. A one-liner alongside the existingallow_nested_indexing("")check (e.g. asserting the adapter reports the backend's protection state) would cover the new path cheaply.As per coding guidelines, "Add or update C++ tests (*_test.cc under src/tests/cpp/) ... when production code under src/ ... changes behavior".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tests/cpp/adapterdb_test.cc` around lines 240 - 247, Add coverage for AdapterDB::is_protected() in the existing no-throw accessor test block alongside allow_nested_indexing(""). Exercise the accessor and assert that it matches the adapter’s backend protection state, using the available backend protection-status symbol.Source: Coding guidelines
src/atomdb/remotedb/RemoteAtomDBPeer.cc (1)
174-192: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant second cache insert per handle.
get_atom(handle, public_key)already inserts intocache_on every miss path (Lines 53, 62), so Line 190 clones and re-inserts each atom a second time. Dropping it saves N clones plus N trie inserts on the warm path.♻️ Proposed cleanup
string handle(handle_cstr); - auto atom = get_atom(handle, public_key); - if (atom) { - cache_.add_atom(atom.get(), public_key); - } + // get_atom() populates cache_ on every miss path. + get_atom(handle, public_key);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/atomdb/remotedb/RemoteAtomDBPeer.cc` around lines 174 - 192, Remove the redundant cache_.add_atom call from feed_cache_from_handle_set after get_atom(handle, public_key); retain the get_atom invocation so cache warming still occurs through its existing miss handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/das.json`:
- Line 4: Remove the redundant "protected" key from config/das.json so
protection has a single source of truth in the MongoDB configuration consumed by
RedisMongoDB::is_protected(). If the key must remain for compatibility, change
the shipped default to false and clearly flag this as a breaking configuration
change; ensure local and CI runs do not default to the stubbed ProtectedAtomDB
path.
In `@src/agents/atomdb_broker/AtomDBProxy.cc`:
- Line 170: Propagate each request’s authenticated public_key through all
protected AtomDB operations instead of hard-coding an empty key. Update
AtomDBProxy add/delete callbacks and queued batch inserts,
QueryEvolutionProcessor evolution reads/writes, CountLetterFunction, and the
EquivalenceProcessor, ImplicationProcessor, and LinkCreationService read/write
paths so every affected API passes the key consistently to the remote or local
AtomDB calls.
In `@src/atomdb/AtomDBSingleton.cc`:
- Around line 23-48: Add tests for the AtomDBSingleton initialization path
around the protected-backend wrapping branch: configure AtomDBMock so
is_protected() returns true and assert get_instance() returns a ProtectedAtomDB
that delegates to the backend, then cover the false case and assert the raw
instance is returned unchanged. Also verify provide() continues to bypass
wrapping, using the existing AtomDBMock setup and singleton symbols.
In `@src/atomdb/auth/ProtectedAtomDB.cc`:
- Around line 5-160: Update the unimplemented operation methods in
ProtectedAtomDB, including get_atom/get_node/get_link, query methods, existence
checks, add/delete methods, re_index_patterns, counts, and filter methods, to
fail explicitly with RAISE_ERROR(...) instead of returning nullptr, empty
values, false, or zero. Preserve the existing constructor and capability
methods, and add coverage verifying protected-mode operations fail loudly rather
than silently acting as no-ops.
In `@src/atomdb/inmemorydb/InMemoryDB.cc`:
- Around line 157-171: Update InMemoryDB::get_node and InMemoryDB::get_link to
validate the atom type before dereferencing the dynamic_cast, returning nullptr
when the handle resolves to the opposite atom type. Reuse the atom returned by
get_atom without creating an unnecessary second clone, while preserving cloning
for correctly typed results.
In `@src/atomdb/redis_mongodb/RedisMongoDB.cc`:
- Around line 59-70: Update RedisMongoDB::is_protected() to cache the protected
status after its first configuration lookup, returning the cached value on
subsequent calls. When reading the “protected” field, validate that the element
exists and has a boolean value; treat missing or malformed values as false
instead of calling get_bool() unconditionally.
In `@src/atomdb/remotedb/RemoteAtomDBPeer.cc`:
- Around line 419-483: The delete methods in RemoteAtomDBPeer currently report
success when local_persistence_ is absent and double-count batch deletions
across cache and local storage. Update delete_atom, delete_node, and delete_link
to return only successful deletion from attempted tiers, and adjust
delete_atoms, delete_nodes, and delete_links to avoid counting the same handle
twice when present in both tiers.
- Around line 492-517: Update RemoteAtomDBPeer::node_count(), link_count(), and
atom_count() to avoid unconditionally invoking unsupported InMemoryDB count
methods and to prevent double-counting records shared by cache_ and
local_persistence_. Route each count through a single AtomDB count interface or
the appropriate cached/local query, using the valid persistence or remote source
when available and preserving zero/unsupported behavior otherwise.
In `@src/atomdb/remotedb/RemoteAtomDBPeer.h`:
- Around line 39-43: Update the get_cached_atom, get_cached_node, and
get_cached_link APIs and implementations to accept and propagate public_key, and
use that key when accessing cache_ so cached results remain scoped to the
requesting key. Update all RemoteAtomDB facade callers accordingly, preserving
the cache-probe behavior without passing an empty key.
In `@src/tests/cpp/redis_mongodb_test.cc`:
- Around line 998-1025: Add protection-detection coverage in
src/tests/cpp/redis_mongodb_test.cc:998-1025 by asserting is_protected() is
false for a fresh database, true after writing the protection configuration
document, and remains true on a new RedisMongoDB instance using the same prefix;
also add a case documenting the ProtectedAtomDB wrapper’s current empty
public_key behavior. In src/tests/cpp/adapterdb_test.cc:240-247, assert
db->is_protected() matches the configured backend protection state beside the
existing allow_nested_indexing("") check.
---
Outside diff comments:
In `@src/atomdb/inmemorydb/InMemoryDB.cc`:
- Around line 369-388: Update add_nodes to use a separate reserved vector for
handles collected for nodes_exist, while keeping the returned handles vector
reserved and initially empty; then append each result from add_node only once so
add_nodes and add_atoms return exactly one handle per input node.
In `@src/atomdb/redis_mongodb/RedisMongoDB.cc`:
- Around line 1066-1090: Forward the existing public_key through
RedisMongoDB::delete_atom, delete_node, and delete_link, including nested
delete_node/delete_link calls, query_for_incoming_set, recursive delete_atom,
and delete_document instead of hardcoding an empty key. In
src/atomdb/redis_mongodb/RedisMongoDB.cc lines 1066-1090, update each affected
call accordingly. In src/atomdb/morkdb/MorkDB.cc lines 316-356, update
re_index_patterns to pass its public_key to this->add_links rather than an empty
string.
- Around line 100-142: Update RedisMongoDB::mongodb_setup() to read the
configured protected flag from atomdb.protected rather than mongodb.protected.
When initializing the MongoDB config document, synchronize its protected field
with is_protected: insert it when absent and update it when the stored value
differs from configuration, so is_protected() reflects the configured mutability
state.
In `@src/atomdb/remotedb/RemoteAtomDBPeer.cc`:
- Around line 45-69: The cache path in RemoteAtomDBPeer::get_atom must not
bypass public-key authorization: replace the shared cache lookup/population with
key-scoped storage keyed by both handle and public_key, or explicitly reapply
the same authorization filtering before returning cached atoms. Ensure
get_cached_atom/node/link cannot read atoms fetched for another key through the
shared cache, and keep cache behavior consistent across all related access
paths.
---
Nitpick comments:
In `@src/atomdb/remotedb/RemoteAtomDBPeer.cc`:
- Around line 174-192: Remove the redundant cache_.add_atom call from
feed_cache_from_handle_set after get_atom(handle, public_key); retain the
get_atom invocation so cache warming still occurs through its existing miss
handling.
In `@src/tests/cpp/adapterdb_test.cc`:
- Around line 240-247: Add coverage for AdapterDB::is_protected() in the
existing no-throw accessor test block alongside allow_nested_indexing("").
Exercise the accessor and assert that it matches the adapter’s backend
protection state, using the available backend protection-status symbol.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ba1db652-6608-43f1-aa4b-adada83775c7
📒 Files selected for processing (48)
config/das.jsonsrc/agents/atomdb_broker/AtomDBProxy.ccsrc/agents/evolution/QueryEvolutionProcessor.ccsrc/agents/evolution/fitness_functions/CountLetterFunction.ccsrc/agents/link_creation_agent/EquivalenceProcessor.ccsrc/agents/link_creation_agent/ImplicationProcessor.ccsrc/agents/link_creation_agent/LinkCreationService.ccsrc/agents/link_creation_agent/MettaTemplateProcessor.ccsrc/agents/query_engine/query_element/LinkTemplate.ccsrc/atomdb/AtomDB.hsrc/atomdb/AtomDBSingleton.ccsrc/atomdb/BUILDsrc/atomdb/adapterdb/AdapterDB.ccsrc/atomdb/adapterdb/AdapterDB.hsrc/atomdb/auth/BUILDsrc/atomdb/auth/ProtectedAtomDB.ccsrc/atomdb/auth/ProtectedAtomDB.hsrc/atomdb/inmemorydb/InMemoryDB.ccsrc/atomdb/inmemorydb/InMemoryDB.hsrc/atomdb/morkdb/MorkDB.ccsrc/atomdb/morkdb/MorkDB.hsrc/atomdb/redis_mongodb/RedisMongoDB.ccsrc/atomdb/redis_mongodb/RedisMongoDB.hsrc/atomdb/remotedb/RemoteAtomDB.ccsrc/atomdb/remotedb/RemoteAtomDB.hsrc/atomdb/remotedb/RemoteAtomDBPeer.ccsrc/atomdb/remotedb/RemoteAtomDBPeer.hsrc/db_adapter/AtomPersister.ccsrc/main/db_loader.ccsrc/tests/benchmark/atomdb/atomdb_operations.ccsrc/tests/cpp/adapterdb_test.ccsrc/tests/cpp/atomdb_broker_test.ccsrc/tests/cpp/atomdbutils_test.ccsrc/tests/cpp/chain_operator_test.ccsrc/tests/cpp/inmemorydb_test.ccsrc/tests/cpp/link_creation_agent_test.ccsrc/tests/cpp/morkdb_test.ccsrc/tests/cpp/pattern_matching_query_test.ccsrc/tests/cpp/redis_mongodb_test.ccsrc/tests/cpp/redis_mongodb_test_2.ccsrc/tests/cpp/remote_atomdb_test.ccsrc/tests/cpp/test_commons/MockAnimalsData.ccsrc/tests/cpp/test_commons/mocks/MockAtomDB.hsrc/tests/main/evaluation_evolution.ccsrc/tests/main/link_creation_engine_main.ccsrc/tests/main/word_query_evolution_main.ccsrc/tests/main/word_query_main.ccsrc/tests/regression/adapterdb_main.cc
… fixes, protection tests, and public_key API cleanup
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/atomdb/inmemorydb/InMemoryDB.cc (1)
533-588: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
node_count/link_countstill throw — blocksRemoteAtomDBPeeraggregation.Per the change summary, these still throw "not implemented yet" even though they now accept
public_key. This is the exact root cause behind the previously flagged (and left unresolved) issue onRemoteAtomDBPeer::node_count()/link_count(), wheremarcocapozzoliexplicitly said "Real fix = implement those stubs" and the follow-up remediation plan was to implement theseInMemoryDBcount stubs with tests. SinceRemoteAtomDBPeer's counts call straight intocache_.node_count()/cache_.link_count()(anInMemoryDB), anyRemoteAtomDB::node_count()/link_count()call still crashes today.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/atomdb/inmemorydb/InMemoryDB.cc` around lines 533 - 588, Implement InMemoryDB::node_count and InMemoryDB::link_count instead of raising errors, using the supplied public_key and existing stored atom data to return accurate counts matching the database semantics. Preserve atom_count behavior and ensure both methods support RemoteAtomDBPeer aggregation without throwing.src/atomdb/remotedb/RemoteAtomDBPeer.cc (1)
488-517: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
node_count/link_count/atom_countstill sum into unimplementedInMemoryDBstubs.These continue to call
cache_.node_count()/link_count(), which perInMemoryDB.ccstill throw "not implemented yet." This is the same crash path raised in the prior review round on this class (marcocapozzoli: "Real fix = implement those stubs"). See the companion comment onInMemoryDB.ccfor the root cause.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/atomdb/remotedb/RemoteAtomDBPeer.cc` around lines 488 - 517, Update RemoteAtomDBPeer::node_count, RemoteAtomDBPeer::link_count, and RemoteAtomDBPeer::atom_count to stop calling the unimplemented cache_ count methods. Return counts using only the supported local_persistence_ implementation, preserving the existing behavior when local persistence is unavailable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tests/cpp/atomdb_singleton_test.cc`:
- Around line 17-31: Replace the standalone wrap_if_protected test with coverage
of AtomDBSingleton::init() using a configuration or injectable backend whose
resulting AtomDB reports is_protected() == true. Assert directly on
AtomDBSingleton::get_instance() that the initialized instance is a
ProtectedAtomDB and remains protected, while preserving the singleton setup and
cleanup conventions used by the surrounding tests.
In `@src/tests/cpp/redis_mongodb_test.cc`:
- Around line 1048-1062: Make ProtectedAtomDBEmptyPublicKeyBehavior configure
protected mode explicitly for its own prot_wrap_ backend instead of relying on
shared state established by ProtectionDetection. Update the test’s config setup
to include the required seed_protected setting, preserving the existing
protection and empty-public-key assertions.
---
Outside diff comments:
In `@src/atomdb/inmemorydb/InMemoryDB.cc`:
- Around line 533-588: Implement InMemoryDB::node_count and
InMemoryDB::link_count instead of raising errors, using the supplied public_key
and existing stored atom data to return accurate counts matching the database
semantics. Preserve atom_count behavior and ensure both methods support
RemoteAtomDBPeer aggregation without throwing.
In `@src/atomdb/remotedb/RemoteAtomDBPeer.cc`:
- Around line 488-517: Update RemoteAtomDBPeer::node_count,
RemoteAtomDBPeer::link_count, and RemoteAtomDBPeer::atom_count to stop calling
the unimplemented cache_ count methods. Return counts using only the supported
local_persistence_ implementation, preserving the existing behavior when local
persistence is unavailable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 62bb916f-1024-4fc0-8223-244bf36f53f5
📒 Files selected for processing (13)
src/atomdb/inmemorydb/InMemoryDB.ccsrc/atomdb/morkdb/MorkDB.ccsrc/atomdb/redis_mongodb/RedisMongoDB.ccsrc/atomdb/remotedb/RemoteAtomDB.ccsrc/atomdb/remotedb/RemoteAtomDBPeer.ccsrc/atomdb/remotedb/RemoteAtomDBPeer.hsrc/tests/cpp/BUILDsrc/tests/cpp/adapterdb_test.ccsrc/tests/cpp/atomdb_singleton_test.ccsrc/tests/cpp/inmemorydb_test.ccsrc/tests/cpp/link_creation_agent_test.ccsrc/tests/cpp/redis_mongodb_test.ccsrc/tests/cpp/remote_atomdb_test.cc
🚧 Files skipped from review as they are similar to previous changes (3)
- src/atomdb/morkdb/MorkDB.cc
- src/tests/cpp/inmemorydb_test.cc
- src/tests/cpp/link_creation_agent_test.cc
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tests/cpp/atomdb_singleton_test.cc`:
- Line 32: Update the cleanup calls to reset both the singleton pointer and
initialization state, using a test-only reset API on AtomDBSingleton or
isolating these tests in separate processes. Ensure cleanup after both affected
test cases allows subsequent init() calls to succeed, rather than relying on
provide(nullptr), which leaves initialized true.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e3f75956-6821-44c2-a2f9-dfa0eb5ee8b7
📒 Files selected for processing (2)
src/tests/cpp/atomdb_singleton_test.ccsrc/tests/cpp/redis_mongodb_test.cc
Collaborator
Author
|
Outdated |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
public_keyto allAtomDBvirtual methods and updated parameter order for add/delete operationsis_protected()to AtomDB backends (RedisMongoDB reads/writes config document; RemoteAtomDB uses ANY peer)ProtectedAtomDBas a stub auth wrapper, wired inAtomDBSingletonwhen the backend is protected