RI-8222: Add array append endpoint#6128
Open
pawelangelow wants to merge 1 commit into
Open
Conversation
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b644fec4c2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
Code Coverage - Integration Tests
|
pawelangelow
force-pushed
the
be/RI-8222/add-array-append-endpoint
branch
from
b644fec to
a347afe
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a347afe336
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Add POST /array/append: appends a value to the end of an array by reading the current length (ARLEN) and ARSETting at that index. Returns the written index. The key must already exist (404 otherwise); WrongType maps to 400 and an ACL denial to 403 via the standard catchAclError. The index is carried as a decimal string end-to-end, matching the unsigned 64-bit index contract, and is passed to ARSET as a string so the write stays precise. Above 2^53 the only imprecision is ioredis parsing the ARLEN reply as a JS number (the transport limitation tracked by RI-8296); the integration test for that case is skipped until ioredis returns 64-bit integers losslessly, at which point it flips green with no code change. Not wrapped in a transaction: the client has no MULTI/WATCH yet (sendMulti is a TODO), so two concurrent appends can race on the same length. Acceptable for the desktop GUI, and still fresher than a UI-cached length. An EVAL/Lua script was rejected because Lua numbers are doubles, which would round the index inside the script and corrupt the write above 2^53 — a loss the ioredis fix could not repair. ARINSERT is intentionally not used: its cursor starts at 0 on arrays built via ARSET/ARMSET or loaded fresh, so it would overwrite from the front rather than append to the end (the cursor needs ARSEEK, deferred to v2). Covered by service unit tests, Bruno, and an integration suite (append at end, consecutive appends, encoding=buffer, 404/400/403, plus a skipped >2^53 case). Verified against redis:8.8 (320 passing, 1 pending). References: #RI-8222 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pawelangelow
force-pushed
the
be/RI-8222/add-array-append-endpoint
branch
from
a347afe to
05cd432
Compare
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
1 participant
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.
What
Adds
POST /databases/:id/array/appendfor the array Modify vertical: appends avalue to the end of an array. The end index is computed server-side (current
ARLEN) — the client sends onlykeyName+value— and the written index isreturned. The key must already exist (404 otherwise);
WRONGTYPE→ 400, ACLdenial → 403.
Design notes
ARLENthenARSET key <len> value.An EVAL approach was rejected: Lua numbers are doubles, so it rounds the index
inside the script above 2⁵³ and corrupts the write — a loss the upcoming
ioredis fix could never repair.
passed to
ARSETas a string, so the write stays precise. Above 2⁵³ the onlyimprecision is ioredis parsing the
ARLENreply as a JS number — the transportlimitation tracked by RI-8296 (
stringNumbers). The integration test forthat case is
skipped and will flip green once ioredis is fixed, with no codechange.
MULTI/WATCHyet (sendMultiis aTODO), so two concurrent appends can race on the same length. Acceptable for
the desktop GUI, and still fresher than a UI-cached length.
ARINSERTis intentionally not used (its cursor starts at 0 on ARSET/ARMSET-builtarrays, so it overwrites from the front; cursor positioning needs
ARSEEK, v2).Testing
ARLEN→ARSETcall shape,404 / 400 / 403 paths.
consecutive appends,
encoding=buffer, non-array → 400, missing key/instance →404, ACL authorised +
-arset→ 403, and the skipped >2⁵³ precision case.Array/Append Element.Note
Medium Risk
New write path to live Redis data with a non-transactional ARLEN→ARSET sequence (concurrent appends can race); otherwise mirrors existing array modify patterns and error handling.
Overview
Adds
POST /databases/:id/array/appendso clients can append to an existing Redis array with onlykeyNameandvalue. The server readsARLEN, writes at that index viaARSET, and returns{ keyName, index }(index as a decimal string).Behavior and errors: Missing key → 404; wrong type → 400; ACL → 403. When length is
2^64-1(no valid append index), the API returns 400 withARRAY_IS_FULLinstead of hitting Redis with an invalid index.Tests & docs: Service unit tests, Redis 8.8 integration tests (including buffer encoding, full-array guard, ACL on
arset), a test factory, and a Bruno request under Array/Append Element.Reviewed by Cursor Bugbot for commit 05cd432. Bugbot is set up for automated code reviews on this repo. Configure here.