custom event store with memory store#94
Merged
NeatGuyCoding merged 2 commits intomainfrom Dec 21, 2025
Merged
Conversation
Contributor
WalkthroughMemoryStoreFactory now holds a generic Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (6)
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
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
netty-socketio-core/src/main/java/com/socketio4j/socketio/store/memory/MemoryStoreFactory.java (1)
35-37: Consider adding JavaDoc to document the custom EventStore constructor.The new constructor enables dependency injection of custom
EventStoreimplementations, which is a significant API enhancement. Adding JavaDoc would help users understand when and why to use this constructor versus the default one.💡 Example JavaDoc
+/** + * Creates a MemoryStoreFactory with a custom EventStore implementation. + * This constructor allows dependency injection of a custom event store while + * maintaining the in-memory session store behavior. + * + * @param eventStore the EventStore implementation to use (must not be null) + */ public MemoryStoreFactory(EventStore eventStore) { this.eventStore = eventStore; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
netty-socketio-core/src/main/java/com/socketio4j/socketio/store/memory/MemoryStoreFactory.java(1 hunks)
⏰ 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). (6)
- GitHub Check: build (21) / build
- GitHub Check: build (25) / build
- GitHub Check: build (17) / build
- GitHub Check: qodana
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (3)
netty-socketio-core/src/main/java/com/socketio4j/socketio/store/memory/MemoryStoreFactory.java (3)
29-29: LGTM! Good use of interface type for dependency injection.The field declaration correctly uses the
EventStoreinterface instead of the concrete implementation, enabling flexibility while thefinalmodifier ensures immutability.
31-33: LGTM! Backward compatibility maintained.The default constructor properly initializes the
EventStorewith aMemoryEventStoreinstance, ensuring existing code continues to work without modification.
44-45: LGTM! Method correctly returns the injected EventStore.The method now returns the generic
EventStoreinterface, which correctly supports both the defaultMemoryEventStoreand any custom implementations injected via the constructor.
Comment on lines
+31
to
+37
| public MemoryStoreFactory() { | ||
| this.eventStore = new MemoryEventStore(); | ||
| } | ||
|
|
||
| public MemoryStoreFactory(EventStore eventStore) { | ||
| this.eventStore = eventStore; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find test files referencing MemoryStoreFactory
find . -type f -name "*Test*.java" -o -name "*Tests*.java" | head -20Repository: socketio4j/netty-socketio
Length of output: 2002
🏁 Script executed:
#!/bin/bash
# Search for references to MemoryStoreFactory in test files
rg -t java "MemoryStoreFactory" --glob="*Test*.java" -lRepository: socketio4j/netty-socketio
Length of output: 160
🏁 Script executed:
#!/bin/bash
# Check if test files exist and show their content
find . -path "*test*" -name "*MemoryStore*" -type f 2>/dev/nullRepository: socketio4j/netty-socketio
Length of output: 249
🏁 Script executed:
#!/bin/bash
# Read the MemoryStoreFactoryTest.java file to check test coverage
cat -n ./netty-socketio-core/src/test/java/com/socketio4j/socketio/store/MemoryStoreFactoryTest.javaRepository: socketio4j/netty-socketio
Length of output: 6951
Add tests for the new parameterized constructor.
The test file covers the default constructor but lacks tests for the new dependency injection constructor. Add tests that:
- Create
MemoryStoreFactorywith a customEventStoreimplementation - Verify the custom
EventStoreis returned byeventStore() - Test null validation for the custom
EventStoreparameter (if null checks are implemented)
🤖 Prompt for AI Agents
In
netty-socketio-core/src/main/java/com/socketio4j/socketio/store/memory/MemoryStoreFactory.java
around lines 31 to 37, add unit tests that exercise the new parameterized
constructor: create a test that instantiates MemoryStoreFactory with a custom
EventStore implementation (e.g., an anonymous class or lightweight stub), assert
that factory.eventStore() returns the exact instance passed in, and add a test
for the null case (if the constructor should reject null) that verifies a
NullPointerException or IllegalArgumentException is thrown; if the constructor
currently allows null, instead assert behavior (e.g., returns null or uses
default) and update tests accordingly.
netty-socketio-core/src/main/java/com/socketio4j/socketio/store/memory/MemoryStoreFactory.java
Show resolved
Hide resolved
Qodana for JVM61 new problems were found
☁️ View the detailed Qodana report Detected 123 dependenciesThird-party software listThis page lists the third-party software dependencies used in project
Contact Qodana teamContact us at qodana-support@jetbrains.com
|
NeatGuyCoding
approved these changes
Dec 21, 2025
This was referenced Dec 21, 2025
18 tasks
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.
Description
Store - stores connection related details
Event store - events notification to other nodes
this option enables the users to use memory store and any other event store (redis, hazelcast, etc)
Type of Change
Related Issue
Closes #(issue number)
Changes Made
Testing
mvn testChecklist
Additional Notes
Any additional information, screenshots, or context that reviewers should know.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.