Skip to content

LPush command - #40

Merged
unikdahal merged 7 commits into
mainfrom
feature/LPushCommand
Jan 25, 2026
Merged

LPush command#40
unikdahal merged 7 commits into
mainfrom
feature/LPushCommand

Conversation

@unikdahal

@unikdahal unikdahal commented Jan 25, 2026

Copy link
Copy Markdown
Owner

LPush command

Summary by CodeRabbit

  • New Features

    • LPUSH command added for prepending elements to lists with RESP size replies.
  • Improvements

    • List push operations are now atomic and safer under concurrent access; related push behavior unified.
  • Bug Fixes

    • Correct WRONGTYPE responses when operating on non-list keys.
  • Tests

    • Comprehensive unit tests for list push behavior, ordering, sizes, and error cases.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 25, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds an LPUSH command implementation and registers it; refactors RPUSH to use an atomic compute API; introduces RedisDatabase.compute for atomic read-modify-write with expiry semantics; and adds unit tests for LPUSH behavior and interactions.

Changes

Cohort / File(s) Summary
LPUSH Command
src/main/java/com/redis/commands/LPushCommand.java
New ICommand implementation for LPUSH: validates args, atomically prepends elements via RedisDatabase.compute, handles WRONGTYPE, and returns new list size.
Command Registry
src/main/java/com/redis/commands/CommandRegistry.java
Registers LPushCommand among built-in commands during initialization.
RPUSH Refactor
src/main/java/com/redis/commands/RPushCommand.java
Reworked to use RedisDatabase.compute for atomic append; consolidates logic into execute and removes prior helper.
Database API
src/main/java/com/redis/storage/RedisDatabase.java
Added compute(String, Function<RedisValue, RedisValue>) providing atomic read-modify-write with expiry preservation and null-removal semantics.
Tests
src/test/java/com/redis/commands/LPushCommandTest.java
New comprehensive unit tests for LPUSH: arity, single/multi-element prepends, ordering, WRONGTYPE behavior, and interactions with RPUSH.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client
    participant Registry as CommandRegistry
    participant Cmd as LPushCommand
    participant DB as RedisDatabase
    participant Store as Storage

    Client->>Registry: "LPUSH key v1 v2"
    Registry->>Cmd: dispatch LPUSH
    Cmd->>DB: compute(key, remappingFunction)
    DB->>Store: ConcurrentHashMap.compute callback (read current value)
    Store-->>DB: existing RedisValue or null
    DB->>Store: write updated RedisValue (new LinkedList with prepended items)
    Store-->>DB: ack
    DB-->>Cmd: new size / updated value
    Cmd-->>Client: RESP integer reply with new size
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

Poem

🐰 I hopped in code with whiskers bright,
I pushed new items leftward, tight,
compute atomically hugs each list,
Tests nod — the order's kissed,
Carrots for passing CI tonight! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'LPush command' directly and clearly describes the main change: the introduction of a new LPUSH command implementation with supporting infrastructure.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai Bot and others added 3 commits January 26, 2026 01:09
Docstrings generation was requested by @unikdahal.

* #37 (comment)

The following files were modified:

* `src/main/java/com/redis/commands/CommandRegistry.java`
* `src/main/java/com/redis/commands/RPushCommand.java`
* `src/main/java/com/redis/server/RedisCommandHandler.java`
# Conflicts:
#	src/main/java/com/redis/commands/RPushCommand.java
# Conflicts:
#	src/main/java/com/redis/commands/CommandRegistry.java

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/main/java/com/redis/storage/RedisDatabase.java`:
- Around line 206-227: The compute method currently drops TTL whenever
remappingFunction returns a new RedisValue instance; change the expiry logic in
compute (inside map.compute lambda) to preserve the existingEntry.expiryMillis
if the key existed and was not expired (use isExpired(existingEntry) check) so
updates like LPUSH/RPUSH that return new RedisValue objects keep the TTL; only
set Long.MAX_VALUE for non-existent or expired entries (and still return null
when remappingFunction returns null to remove the key). Reference: compute,
map.compute, existingEntry, isExpired(existingEntry), expiryMillis, ValueEntry,
remappingFunction, RedisValue.
🧹 Nitpick comments (1)
src/test/java/com/redis/commands/LPushCommandTest.java (1)

132-140: Avoid tying tests to internal list implementation

testLPush_UsesLinkedList locks behavior to LinkedList, which is an internal detail and makes refactors harder without improving correctness coverage. Consider removing this test or asserting behavior only.

♻️ Possible adjustment
-    `@Test`
-    void testLPush_UsesLinkedList() {
-        db.remove("linkedtest");
-
-        command.execute(List.of("linkedtest", "element"), ctx);
-
-        RedisValue value = db.getValue("linkedtest");
-        assertTrue(value.getData() instanceof LinkedList<?>);
-
-        db.remove("linkedtest");
-    }
+    // Consider removing this test or validating only ordering/size behavior.

Comment thread src/main/java/com/redis/storage/RedisDatabase.java

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/com/redis/commands/CommandRegistry.java (1)

22-26: Stale Javadoc - missing LRangeCommand and LPushCommand.

The constructor Javadoc lists only "SetCommand, GetCommand, DelCommand, and RPushCommand" but the registry now also includes LRangeCommand and LPushCommand.

📝 Suggested documentation update
     /**
      * Initializes the singleton CommandRegistry and registers the built-in commands.
      *
-     * Registers the default command implementations: SetCommand, GetCommand, DelCommand, and RPushCommand.
+     * Registers the default command implementations: SetCommand, GetCommand, DelCommand, RPushCommand, LRangeCommand, and LPushCommand.
      */

@unikdahal
unikdahal merged commit e4fbaad into main Jan 25, 2026
2 checks passed
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