Skip to content

refactor(store): extract shared AbstractEventStore and registry utilities - #220

Open
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1784981094-shared-event-store-utils
Open

refactor(store): extract shared AbstractEventStore and registry utilities#220
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1784981094-shared-event-store-utils

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Description

The seven broker backed EventStore implementations each re-implemented the same boilerplate: nullable nodeId/eventStoreMode/prefix defaulting, SINGLE_CHANNEL channel-name resolution, mode validation, self-message filtering, the EventType -> Queue<registrationId> + registrationId -> handle bookkeeping with its identical unsubscribe loop, the Arrays.stream(EventType.values()).forEach(this::unsubscribe) shutdown, and the unchecked ListenerRegistration dispatch loop. This PR moves all of it into three shared types under store.event, so the concrete stores only contain broker-specific code. Net −540/+353 lines with no behavior change.

AbstractEventStore (base class for all remote stores):

protected AbstractEventStore(nodeId, eventStoreMode, defaultEventStoreMode, channelPrefix, defaultChannelPrefix)
protected EventType resolveType(EventType)   // ALL_SINGLE_CHANNEL in SINGLE_CHANNEL mode
protected String   channelName(EventType)    // channelPrefix + resolveType(type).name()
protected void     stampNodeId(EventMessage)
protected boolean  isRemote(EventMessage)    // !nodeId.equals(msg.getNodeId())
protected void     unsubscribeAll()
protected void     validateSubscribe(EventType)

SubscriptionRegistry<I, S> replaces the two ad-hoc maps in the pub/sub stores, so e.g. Redis pub/sub becomes:

subscriptions.add(type, regId, topic);
// unsubscribe0:
subscriptions.remove(type, (id, topic) -> topic.removeListener(id));

ListenerRegistry holds the local ListenerRegistration queues for the poll based stores (Kafka, Redis Stream) and owns the single remaining unchecked cast used to dispatch a message to typed listeners.

Per-store defaults are preserved by passing them to super(...): Redis pub/sub and NATS keep an empty prefix, Hazelcast (both) and Redis reliable/stream keep "SOCKETIO4J:", Kafka keeps "SOCKETIO4J-", and Redis Stream keeps SINGLE_CHANNEL as its default mode while everything else keeps MULTI_CHANNEL.

Type of Change

  • Code refactoring

Related Issue

N/A

Changes Made

  • Added store/event/AbstractEventStore.java, store/event/SubscriptionRegistry.java, store/event/ListenerRegistry.java
  • Reworked RedisPubSubEventStore, RedisPubSubReliableEventStore, RedisStreamEventStore, NatsEventStore, HazelcastPubSubEventStore, HazelcastPubSubRingBufferEventStore and KafkaEventStore to extend AbstractEventStore and use the shared registries
  • Removed the per-store duplicates of getStreamName/getSubjectName/getTopicName/topic, resolve, validateSubscribe, getEventStoreMode and the duplicated unsubscribe/shutdown/dispatch loops

Testing

  • Tests pass locally with mvn testmvn -pl netty-socketio-core verify (compile + checkstyle + license) passes; the store integration tests are Testcontainers based and are exercised by CI
  • New tests added for new functionality (pure refactor, no behavior change)

Checklist

  • Code follows project coding standards (checkstyle clean)
  • Self-review completed
  • Commit messages follow conventional format
  • No merge conflicts

Additional Notes

No public API changed: constructors, builders and the EventStore SPI keep their existing signatures.

Link to Devin session: https://app.devin.ai/sessions/78bf7322d0ff4d139f1f0c157954bfad
Requested by: @sanjomo

@sanjomo sanjomo self-assigned this Jul 25, 2026
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration

Copy link
Copy Markdown
Author

Test run: store integration tests (Devin)

Full netty-socketio-core suite on this branch with Docker/Testcontainers (Redis, Kafka, Hazelcast, NATS):

Tests run: 709, Failures: 0, Errors: 0, Skipped: 0, Flakes: 14
Channel-name / default-mode invariants (all 7 backends unchanged)

Temporary reflection test over AbstractEventStore#channelName with mocked clients:

RedisPubSubEventStore              MULTI_CHANNEL   CONNECT
NatsEventStore                     MULTI_CHANNEL   CONNECT
RedisPubSubReliableEventStore      MULTI_CHANNEL   SOCKETIO4J:CONNECT
RedisStreamEventStore              SINGLE_CHANNEL  SOCKETIO4J:ALL_SINGLE_CHANNEL
HazelcastPubSubEventStore          MULTI_CHANNEL   SOCKETIO4J:CONNECT
HazelcastPubSubRingBufferEventStore MULTI_CHANNEL  SOCKETIO4J:CONNECT
KafkaEventStore                    MULTI_CHANNEL   SOCKETIO4J-CONNECT
validateSubscribe rules OK

Matches pre-refactor naming/modes exactly. validateSubscribe still throws UnsupportedOperationException for the two illegal mode/type combinations.

Flakes (14) — likely environmental

All were "clients failed to join room within timeout" in Hazelcast/Kafka/NATS distributed tests and passed on retry. DistributedKafkaMultiChannelMemoryTest run in isolation: main 12/12 in 17.53s, branch 12/12 in 17.43s — identical, so contention during the 18-min full run is the likely cause. A full-suite run on main was not done for a like-for-like flake comparison.

Session: https://app.devin.ai/sessions/78bf7322d0ff4d139f1f0c157954bfad

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

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.

1 participant