Skip to content

Conversation

@strawgate
Copy link
Owner

@strawgate strawgate commented Nov 8, 2025

Summary

Removes the native_storage parameter from MongoDBStore and simplifies the codebase. The store now exclusively uses native BSON storage for better query support and performance, while maintaining backward compatibility for reading legacy JSON string data.

Changes

  • Removed native_storage parameter from MongoDBStore init
  • Simplified MongoDBSerializationAdapter to always use BSON dict storage
  • Maintained legacy JSON string read support for backward compatibility
  • Consolidated tests into single TestMongoDBStore class
  • Removed non-native mode test class and related tests

Backward Compatibility

✅ Legacy JSON string data can still be read via prepare_load()

Fixes #212


Generated with Claude Code) • Branch • [View job run](https://github.com/strawgate/py-key-value/actions/runs/19187564782

Summary by CodeRabbit

  • Breaking Changes

    • Removed the native_storage parameter from MongoDB store/adapter constructors; storage is now BSON-only.
  • Behavior Changes

    • Values are consistently serialized as BSON dicts under a single object path; legacy string/JSON read support removed.
  • Tests

    • Test suites consolidated and renamed to reflect BSON-only storage; legacy/non-native-mode tests removed.
  • Documentation

    • Docs and docstrings updated to reflect BSON-native storage behavior.

Remove the `native_storage` parameter from MongoDBStore and related code.
The store now exclusively uses native BSON storage for better query support
and performance. Legacy JSON string data can still be read for backward
compatibility.

Changes:
- Removed `native_storage` parameter from MongoDBStore __init__
- Simplified MongoDBSerializationAdapter to always use BSON dict storage
- Maintained legacy JSON string read support in prepare_load()
- Updated tests to remove non-native mode test class
- Consolidated tests into single TestMongoDBStore class
- Updated test names to reflect current functionality

Fixes #212

Co-authored-by: William Easton <strawgate@users.noreply.github.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 8, 2025

📝 Walkthrough

Walkthrough

Removes the native_storage toggle across async and sync MongoDB stores/adapters, makes BSON dict storage the sole format (value stored under data["value"]["object"]), removes legacy JSON/string read support, and updates tests/fixtures to reflect BSON-only behavior.

Changes

Cohort / File(s) Summary
Async MongoDB store & adapter
key-value/key-value-aio/src/key_value/aio/stores/mongodb/store.py
Removed native_storage parameter/flag; MongoDBSerializationAdapter ctor is now no-arg and forces BSON dict format (value always stored as data["value"] = {"object": value}). Legacy string/JSON read-path and backward-compat handling removed; docstrings/comments updated.
Async MongoDB tests
key-value/key-value-aio/tests/stores/mongodb/test_mongodb.py
Consolidated/renamed tests (e.g., TestMongoDBStoreNativeModeTestMongoDBStore, test_managed_entry_document_conversion_native_modetest_managed_entry_document_conversion); removed non-native/legacy-mode tests and fixture native_storage usage; assertions updated for BSON dict storage and db_name adjustments.
Sync MongoDB store & adapter
key-value/key-value-sync/src/key_value/sync/code_gen/stores/mongodb/store.py
Same changes as async: adapter ctor now no-arg and _value_format forced to dict; MongoDBStore overloads/constructors no longer accept native_storage; legacy string fallback removed; docstrings updated.
Sync MongoDB tests
key-value/key-value-sync/tests/code_gen/stores/mongodb/test_mongodb.py
Tests renamed/consolidated to reflect BSON-only storage; removed legacy/non-native tests; adapter/store fixture calls drop native_storage; expectations adjusted to BSON dict storage and db_name changes.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant MongoDBStore
    participant MongoDBSerializationAdapter
    participant MongoDB

    Note over MongoDBSerializationAdapter: BSON dict storage only

    rect rgb(240,248,255)
    Note over Client,MongoDB: Store (write)
    Client->>MongoDBStore: set(key, value)
    MongoDBStore->>MongoDBSerializationAdapter: prepare_dump(value)
    MongoDBSerializationAdapter->>MongoDBSerializationAdapter: data["value"] = {"object": value}
    MongoDBSerializationAdapter-->>MongoDBStore: BSON document
    MongoDBStore->>MongoDB: insert document
    end

    rect rgb(240,255,240)
    Note over Client,MongoDB: Load (read)
    Client->>MongoDBStore: get(key)
    MongoDB->>MongoDBStore: BSON document
    MongoDBStore->>MongoDBSerializationAdapter: prepare_load(document)
    MongoDBSerializationAdapter->>MongoDBSerializationAdapter: read data["value"]["object"] (error if missing)
    MongoDBSerializationAdapter-->>MongoDBStore: deserialized value
    MongoDBStore-->>Client: value
    end
Loading

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removing the non-native MongoDB storage option by eliminating the native_storage parameter and consolidating to BSON-only storage.
Linked Issues check ✅ Passed The PR successfully addresses issue #212 by removing the native_storage parameter from MongoDBStore and MongoDBSerializationAdapter, consolidating to exclusive BSON dict storage as required.
Out of Scope Changes check ✅ Passed All changes are directly scoped to removing non-native storage: parameter removal from constructors, test consolidation, and docstring updates. No unrelated changes detected.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch claude/issue-212-20251108-0350

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 34dbc97 and 60253e2.

📒 Files selected for processing (1)
  • key-value/key-value-aio/tests/stores/mongodb/test_mongodb.py (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
key-value/key-value-aio/tests/stores/mongodb/test_mongodb.py (1)
key-value/key-value-aio/src/key_value/aio/stores/mongodb/store.py (2)
  • MongoDBSerializationAdapter (40-79)
  • MongoDBStore (91-339)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: test_quick (3.10, macos-latest, key-value/key-value-sync)
  • GitHub Check: test_quick (3.10, windows-latest, key-value/key-value-aio)
  • GitHub Check: test_quick (3.10, windows-latest, key-value/key-value-sync)
  • GitHub Check: test_quick (3.10, windows-2022, key-value/key-value-aio)
  • GitHub Check: test_quick (3.10, ubuntu-22.04, key-value/key-value-aio)
  • GitHub Check: test_quick (3.10, windows-2022, key-value/key-value-sync)
  • GitHub Check: test_quick (3.10, macos-14, key-value/key-value-sync)
  • GitHub Check: test_quick (3.10, macos-latest, key-value/key-value-aio)
  • GitHub Check: test_quick (3.10, macos-14, key-value/key-value-aio)
  • GitHub Check: test_quick (3.10, ubuntu-latest, key-value/key-value-sync)
  • GitHub Check: test_quick (3.10, ubuntu-latest, key-value/key-value-aio)
  • GitHub Check: test_quick (3.10, ubuntu-22.04, key-value/key-value-sync)
🔇 Additional comments (4)
key-value/key-value-aio/tests/stores/mongodb/test_mongodb.py (4)

51-76: LGTM: Test correctly verifies BSON-only storage.

The test properly validates that MongoDBSerializationAdapter stores values as BSON dicts in the format {"object": {...}} and correctly handles round-trip serialization.


130-136: LGTM: Class and fixture correctly updated for BSON-only mode.

The class rename and removal of the native_storage parameter from the store initialization correctly reflect the simplified API.


143-152: LGTM: Database naming simplified appropriately.

Removing the "native-" prefix from the database name makes sense now that BSON dict storage is the only option.


154-173: LGTM: Direct verification of storage format is valuable.

This test provides important validation by checking the raw MongoDB document structure, ensuring values are actually stored as BSON dicts at the database level.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@strawgate
Copy link
Owner Author

@claude this is an unstable store, remove the backwards compat please

@claude
Copy link

claude bot commented Nov 8, 2025

Claude finished @strawgate's task —— View job


Removed backward compatibility from MongoDB store ✓

Successfully removed the backward compatibility code for legacy JSON string storage from the MongoDB store.

Changes Made

  • ✓ Removed legacy JSON string read support from prepare_load() method (lines 64-66)
  • ✓ Removed backward compatibility note from __init__ docstring (line 163)
  • ✓ Removed test_legacy_document_compatibility unit test
  • ✓ Removed test_migration_from_legacy_mode integration test
  • ✓ Ran make codegen to update sync library
  • ✓ Verified MongoDB tests pass
  • ✓ Committed and pushed changes

Summary

The MongoDB store now exclusively uses native BSON dictionary storage with no support for reading legacy JSON string data. This is a breaking change for any existing deployments with legacy data, but is acceptable since MongoDB is marked as an unstable store.

The prepare_load() method now only checks for the "object" key in the value field and raises a DeserializationError if not found, simplifying the code and removing the dual-format support.


Branch

github-actions bot and others added 3 commits November 8, 2025 16:47
…e in MongoDB

Remove the ability to read legacy JSON string data format in MongoDBStore.
Since this is an unstable store, breaking changes are acceptable.

Changes:
- Remove legacy JSON string read support from prepare_load()
- Remove backward compatibility documentation
- Remove test_legacy_document_compatibility test
- Remove test_migration_from_legacy_mode integration test
- Update sync library via codegen

Co-authored-by: William Easton <strawgate@users.noreply.github.com>
@strawgate strawgate merged commit 432e9e4 into main Nov 9, 2025
5 checks passed
@strawgate strawgate deleted the claude/issue-212-20251108-0350 branch November 9, 2025 16:59
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
50.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove non-native MongoDB Storage

2 participants