Skip to content

List command refactor#301

Merged
alexr-bq merged 12 commits intomainfrom
alexr/list-command-refactor
Apr 15, 2026
Merged

List command refactor#301
alexr-bq merged 12 commits intomainfrom
alexr/list-command-refactor

Conversation

@alexr-bq
Copy link
Copy Markdown
Collaborator

@alexr-bq alexr-bq commented Apr 13, 2026 •
edited
Loading

Summary

Refactors List commands to seperate out non-SER commands and ensure full SER interface compatiblity.

Issue Link

This pull request is linked to issue: 270

Features and Behaviour Changes

This PR improves StackExchange.Redis (SER) API compatibility for list commands:

  1. Move blocking list commands to GLIDE-only interface - BLMOVE, BLMPOP, BLPOP, BRPOP are not supported by SER (they would block the multiplexer). These methods have been moved from IListBaseCommands to IBaseClient.ListCommands.cs.

  2. Add ListRightPopLeftPushAsync - SER-compatible wrapper for the deprecated RPOPLPUSH command. Internally delegates to ListMoveAsync(source, dest, ListSide.Right, ListSide.Left).

  3. Add CommandFlags-only overloads for push methods - Added overloads for ListLeftPushAsync and ListRightPushAsync that accept CommandFlags without requiring When parameter. This fixes compilation errors for code like ListRightPushAsync(key, values, flags).

  4. Remove blocking command wrappers from SER-compat interface - Removed ListBlockingLeftPopAsync, ListBlockingRightPopAsync, ListBlockingMoveAsync, and ListBlockingPopAsync with CommandFlags from IDatabaseAsync since SER doesn't have these methods.

Implementation

Key changes:

  • IBaseClient.ListCommands.cs (new) - Contains GLIDE-only blocking list commands
  • IListBaseCommands.cs - Shared interface for SER-compatible list commands only
  • IDatabaseAsync.ListCommands.cs - SER-compat wrappers with CommandFlags parameter
  • Database.ListCommands.cs - Implementation of SER-compat wrappers

The blocking commands remain fully functional via IBaseClient for GLIDE users who need them. They are simply not exposed through the SER-compatible IDatabaseAsync interface.

Limitations

  • Blocking list commands (BLPOP, BRPOP, BLMOVE, BLMPOP) are not available through the IDatabaseAsync interface. Users who need blocking operations should use the IBaseClient interface directly.
  • RPOPLPUSH is deprecated since Redis 6.2.0. The ListRightPopLeftPushAsync method is provided only for SER compatibility; new code should use ListMoveAsync.

Testing

  • Added integration tests for ListRightPopLeftPushAsync covering:
    • Basic element move between lists
    • Empty source list returns null
    • Same source and destination rotates the list
  • Added integration tests for CommandFlags-only overloads of ListLeftPushAsync and ListRightPushAsync
  • Build and lint pass

Checklist

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated and all checks pass.
  • CHANGELOG.md, README.md, DEVELOPER.md, and other documentation files are updated.
  • Destination branch is correct - main or release
  • Create merge commit if merging release branch into main, squash otherwise.

currantw
currantw reviewed Apr 14, 2026
View reviewed changes
Comment thread sources/Valkey.Glide/Commands/IListBaseCommands.cs Outdated
Comment thread sources/Valkey.Glide/Commands/IListBaseCommands.cs
Comment thread sources/Valkey.Glide/Commands/IListBaseCommands.cs
Comment thread sources/Valkey.Glide/Commands/IListBaseCommands.cs
Comment thread sources/Valkey.Glide/Commands/IListBaseCommands.cs Outdated
Comment thread sources/Valkey.Glide/Commands/IListBaseCommands.cs
Comment thread sources/Valkey.Glide/Commands/IListBaseCommands.cs Outdated
Comment thread sources/Valkey.Glide/Commands/IListBaseCommands.cs Outdated
Comment thread sources/Valkey.Glide/Client/IBaseClient.ListCommands.cs
Comment thread sources/Valkey.Glide/Client/IBaseClient.ListCommands.cs
@alexr-bq alexr-bq mentioned this pull request Apr 14, 2026
Merged
6 tasks
affonsov
affonsov approved these changes Apr 14, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@affonsov affonsov left a comment

Choose a reason for hiding this comment

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

I don't have any additional comment

currantw
currantw approved these changes Apr 15, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@currantw currantw left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks! ✅

alexr-bq and others added 11 commits April 15, 2026 10:19
@alexr-bq
refactor(list): move blocking list commands to IBaseClient
a4c24eb 
Move BLMOVE, BLMPOP, BLPOP, BRPOP from IListBaseCommands to
IBaseClient.ListCommands.cs since they are not supported by
StackExchange.Redis (blocking operations block the multiplexer).

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@alexr-bq
feat(list): add ListRightPopLeftPushAsync for SER compatibility
90cb437 
Add RPOPLPUSH wrapper method for StackExchange.Redis compatibility.
The method wraps ListMoveAsync with Right/Left sides.
RPOPLPUSH is deprecated since Redis 6.2.0 but needed for SER compat.

Includes integration tests for the new method.

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@alexr-bq
fix(list): add CommandFlags-only overloads for ListLeftPush/ListRight…
d7197eb 
…Push

Add overloads that accept CommandFlags without requiring When parameter.
This fixes SER compatibility for code calling ListRightPushAsync(key, values, flags).

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@alexr-bq
refactor(list): remove blocking command wrappers from SER-compat inte…
f29c42b 
…rface

Blocking list commands (BLPOP, BRPOP, BLMOVE, BLMPOP) are not supported
by StackExchange.Redis. Remove CommandFlags wrappers from IDatabaseAsync
and Database. These methods remain available via IBaseClient for GLIDE users.

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@alexr-bq
Remove comment
f16c309 
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@alexr-bq
refactor(list): add explicit LPUSHX/RPUSHX methods and move When para…
18788e4 
…meter to SER-specific interface

- Add ListLeftPushIfExistsAsync and ListRightPushIfExistsAsync to IBaseClient (GLIDE-style)
- Move ListLeftPushAsync/ListRightPushAsync with When parameter from IListBaseCommands to IDatabaseAsync (SER-specific)
- Keep simple ListLeftPushAsync/ListRightPushAsync (without When) in IListBaseCommands (shared)
- Add implementations in Database.ListCommands.cs for SER-specific methods
- Update XML documentation references across all affected files
- Add integration tests for both SER-specific (When parameter) and GLIDE-specific (IfExists) methods
- Update existing TestListPushX test to use GLIDE-style methods

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@alexr-bq
feat(list): split LINDEX/LSET between GLIDE and SER interfaces
698f314 
- Move ListGetByIndexAsync/ListSetByIndexAsync to IDatabaseAsync (SER-style)
- Move ListIndexAsync/ListSetAsync to IBaseClient (GLIDE-style)
- Add single-key convenience overloads for blocking list operations
- Rename GLIDE tests to use ListIndex/ListSet naming
- Add SER-compat tests for ListGetByIndexAsync/ListSetByIndexAsync
- Update batch interfaces to use GLIDE-style naming

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@jduo @alexr-bq
fix: memory leaks for commands and pubsub (#300)
f9566e9 
Fix native memory leak in free_response, fix managed memory leak from undisposed PubSub subscriptions in ConnectionMultiplexer, and clean up dead/duplicate code in the FFI layer.

This pull request is linked to issue: #298

* Fix critical native memory leak in free_response. Box::leak(Box::from_raw(ptr)).free_memory() reclaimed the heap-allocated ResponseValue via Box::from_raw, but immediately leaked it again with Box::leak. While free_memory() correctly freed nested contents (strings, arrays, maps), the top-level ResponseValue Box (~24 bytes) was never deallocated. This leaked on every single command response. Fixed by letting the Box drop naturally after free_memory() completes. free_memory(&self) only reads Copy-type fields (typ, val, size) to
reconstruct and drop inner allocations, so it's safe to drop the Box after the call returns.

* Fix managed memory leak in ConnectionMultiplexer.Dispose(). Dispose() was not calling RemoveAllSubscriptions(), leaving Subscription objects alive in the _subscriptions dictionary. Each Subscription holds delegate references (_handlers) and ChannelMessageQueue objects, which keep handler targets rooted. Added RemoveAllSubscriptions() before _db.Dispose() so subscriptions are cleaned up while the underlying client is still alive. The call is placed outside _lock since RemoveAllSubscriptions() has its own internal lock on _subscriptions.

* Remove duplicate MarshalPubSubMessage from BaseClient and unused FreeDropScriptError method declaration.

Signed-off-by: James Duong <duong.james@gmail.com>
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@alexr-bq @currantw @prateek-kumar-improving
Generic Command Interface Refactor (#292)
0d24eae 
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
Signed-off-by: Prateek Kumar <prateek.kumar@improving.com>
Co-authored-by: Taylor Curran <taylor.curran@improving.com>
Co-authored-by: prateek-kumar-improving <prateek.kumar@improving.com>
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@currantw @alexr-bq
fix(pubsub): implement SER pub/sub commands and make API binary-safe …
8719f0c 
…with ValkeyKey/ValkeyValue (#296)

Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@alexr-bq
refactor(generic): remove default params from SER Sort methods to avo…
193cc2a 
…id interface ambiguity

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@alexr-bq alexr-bq force-pushed the alexr/list-command-refactor branch from 8cfa48a to 193cc2a Compare April 15, 2026 17:19
@alexr-bq
Merge branch 'main' into alexr/list-command-refactor
e9f7213 
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@alexr-bq alexr-bq merged commit f46ceb8 into main Apr 15, 2026
13 of 14 checks passed
@alexr-bq alexr-bq deleted the alexr/list-command-refactor branch April 15, 2026 18:12
@yipin-chen yipin-chen mentioned this pull request Apr 16, 2026
Open
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@affonsov affonsov affonsov approved these changes

@currantw currantw currantw approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alexr-bq @affonsov @currantw @jduo