Skip to content

Conversation

@Havret
Copy link
Owner

@Havret Havret commented Jun 2, 2025

No description provided.

@Havret Havret force-pushed the feature/rocksdbstore-getallkeys-getallvalues-api branch from 81a3c07 to 4835388 Compare June 2, 2025 17:21
@Havret Havret requested a review from Copilot June 2, 2025 18:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces separate methods for retrieving all keys and all values from the store, deprecates the old GetAll method, and updates tests and examples to use the new API.

  • Add GetAllKeys and GetAllValues to RocksDbAccessor, IRocksDbAccessor, and RocksDbStore; mark GetAll as obsolete.
  • Update existing tests and add new tests for ProtoBuf‐Net serializer to cover key/value retrieval.
  • Update example controller to use GetAllValues.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/RocksDb.Extensions.Tests/RocksDbStoreWithProtobufSerializerTests.cs Updated test to call GetAllValues instead of GetAll
test/RocksDb.Extensions.Tests/RocksDbStoreWithProtoBufNetSerializerTests.cs Added tests for GetAllKeys and GetAllValues
src/RocksDb.Extensions/RocksDbAccessor.cs Implemented GetAllKeys, GetAllValues, and explicit ISpanDeserializer
src/RocksDb.Extensions/IRocksDbStore.cs Added new signatures and obsolete GetAll alias
src/RocksDb.Extensions/IRocksDbAccessor.cs Replaced GetAll with GetAllValues and GetAllKeys
examples/RocksDb.Extensions.Examples.AspNetCore/UsersController.cs Updated GetAll call to GetAllValues
Comments suppressed due to low confidence (3)

test/RocksDb.Extensions.Tests/RocksDbStoreWithProtobufSerializerTests.cs:113

  • [nitpick] You've updated the test to cover GetAllValues, but there's no test for GetAllKeys in this suite. Consider adding a corresponding keys test to ensure coverage for the new API.
var values = store.GetAllValues().ToList();

src/RocksDb.Extensions/RocksDbAccessor.cs:187

  • Moving Deserialize to an explicit interface implementation removes the public API surface. If any code calls Deserialize directly on RocksDbAccessor, consider adding a public forwarder or validate that all callers use the interface.
TValue ISpanDeserializer<TValue>.Deserialize(ReadOnlySpan<byte> buffer)

src/RocksDb.Extensions/IRocksDbAccessor.cs:21

  • You removed the original GetAll from the accessor interface. To avoid a breaking change for existing implementers, consider marking GetAll() as [Obsolete] first before removal, or provide a migration path.
IEnumerable<TValue> GetAllValues();

@Havret Havret merged commit a8dfa3d into main Jun 2, 2025
1 check passed
@Havret Havret deleted the feature/rocksdbstore-getallkeys-getallvalues-api branch June 2, 2025 19:03
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.

2 participants