Bug fixes #91
Bug fixes #91
Conversation
Introduces RocksDBOptions to encapsulate configuration settings, including the data path and serializer. Adds extension methods to streamline RocksDB integration and JSON serialization configuration, allowing users to customize JSON serialization behavior through options. Updates RocksDBStreamStoreFactory to use the configured serializer for data storage.
WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant KafkaBuilder
participant RocksDBOptions
participant RocksDBStreamStoreFactory
User->>KafkaBuilder: UseRocksDB(options => { ... })
KafkaBuilder->>RocksDBOptions: Configure DataPath, Serializer, etc.
KafkaBuilder->>RocksDBStreamStoreFactory: Initialize with RocksDBOptions
RocksDBStreamStoreFactory->>RocksDBOptions: Access DataPath and Serializer
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/MinimalKafka.RocksDB/RocksDBStoreManager.cs (1)
1-1: Remove unused using statements.The
using Confluent.Kafka;andusing System.Text.Json;directives don't appear to be used in this file. Consider removing them to keep the imports clean.-using Confluent.Kafka; using RocksDbSharp; using System.Collections.Concurrent; -using System.Text.Json;Also applies to: 4-4
src/MinimalKafka.RocksDB/KafkaBuilderExtensions.cs (1)
32-51: Consider improving the UseJsonSerializer method documentation.The XML documentation could be more specific about what happens when the options delegate is null and provide a clearer example of usage.
/// <summary> /// Configures the specified <see cref="RocksDBOptions"/> to use a JSON-based serializer. /// </summary> -/// <remarks>This method sets the <see cref="RocksDBOptions.Serializer"/> property to a serializer that -/// uses JSON serialization. Use the <paramref name="options"/> parameter to customize the behavior of the JSON -/// serializer, such as formatting or type handling.</remarks> +/// <remarks> +/// This method sets the <see cref="RocksDBOptions.Serializer"/> property to a <see cref="ByteSerializer"/> +/// that uses JSON serialization with customizable options. If no options delegate is provided, +/// default JSON serialization settings are used. +/// </remarks> /// <param name="rockDBOptions">The <see cref="RocksDBOptions"/> instance to configure.</param> -/// <param name="options">An optional delegate to configure the <see cref="JsonSerializerOptions"/> used by the serializer. If not -/// provided, default options are used.</param> +/// <param name="options">An optional delegate to configure the <see cref="JsonSerializerOptions"/>. +/// If null, default JSON serialization options are used.</param> /// <returns>The configured <see cref="RocksDBOptions"/> instance.</returns>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/MinimalKafka.RocksDB/ByteSerializer.cs(1 hunks)src/MinimalKafka.RocksDB/KafkaBuilderExtensions.cs(2 hunks)src/MinimalKafka.RocksDB/RocksDBOptions.cs(1 hunks)src/MinimalKafka.RocksDB/RocksDBStoreManager.cs(4 hunks)src/MinimalKafka/KafkaProducerFactory.cs(2 hunks)src/MinimalKafka/Stream/Storage/InMemoryStore.cs(1 hunks)test/MinimalKafka.RocksDB.Tests/StreamStore_Tests.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze
🔇 Additional comments (9)
src/MinimalKafka.RocksDB/ByteSerializer.cs (2)
5-5: Clean implementation with primary constructor.The use of primary constructor syntax for accepting
JsonSerializerOptionsis clean and modern. Good improvement for configurability.
12-12: Proper usage of injected serialization options.The implementation correctly uses the provided
JsonSerializerOptionsin both serialization and deserialization methods, enabling customizable JSON behavior.Also applies to: 20-20
test/MinimalKafka.RocksDB.Tests/StreamStore_Tests.cs (1)
20-23: Test updated to match new API pattern.The test configuration correctly adapts to the new
RocksDBOptionsdelegate pattern while maintaining the same test logic.src/MinimalKafka.RocksDB/RocksDBOptions.cs (1)
21-21: Good default serializer configuration.The default serializer initialization with
JsonSerializerOptions.Defaultprovides a sensible baseline that can be customized as needed.src/MinimalKafka/KafkaProducerFactory.cs (3)
19-19: Good addition of metadata transparency.Exposing the metadata through a public readonly property increases transparency and allows consumers to inspect the configuration metadata.
25-25: Clean separation of concerns.Delegating configuration building to a separate method improves code organization and maintainability.
41-52: Well-structured configuration building method.The
BuildConfig()method properly encapsulates the configuration logic, handling both existing configurations and applying metadata-driven settings. The implementation is clear and follows good separation of concerns.src/MinimalKafka.RocksDB/RocksDBStoreManager.cs (1)
24-24: LGTM: Configuration refactoring is well-implemented.The changes correctly replace the hardcoded string path and serializer with configurable options from the
RocksDBOptionsobject. This provides better flexibility and follows the dependency injection pattern.Also applies to: 38-38, 65-65
src/MinimalKafka.RocksDB/KafkaBuilderExtensions.cs (1)
21-30: LGTM: Method refactoring improves configurability.The refactoring from a simple string path to a configurable options delegate provides better flexibility and extensibility. The implementation correctly creates and configures the options object.
Updates RocksDB integration to improve configuration and ensure proper initialization. - Uses a new JsonSerializerOptions instance instead of the default one to allow for customized JSON serialization. - Corrects default DataPath for RocksDB. - Ensures RocksDBStreamStoreFactory is initialized correctly by validating the configuration and passing it consistently. - Switches to ConcurrentBag for in-memory store to allow multiple threads to add stores.
Adds a lock to the `GetStreamStore` method to prevent race conditions when creating and accessing stores from multiple threads. This ensures that only one store of a given type is created.
Summary by CodeRabbit
New Features
Refactor
Tests