Skip to content

Commit d09cbdf

Browse files
authored
Add a RedisClient#getBuffer method (oven-sh#19567)
1 parent 15a5e3a commit d09cbdf

9 files changed

Lines changed: 188 additions & 14 deletions

File tree

.vscode/settings.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@
4343
"editor.tabSize": 4,
4444
"editor.useTabStops": false,
4545
"editor.defaultFormatter": "ziglang.vscode-zig",
46+
"editor.codeActionsOnSave": {
47+
"source.organizeImports": "never",
48+
},
4649
},
4750

4851
// lldb

packages/bun-types/redis.d.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,17 @@ declare module "bun" {
110110
/**
111111
* Get the value of a key
112112
* @param key The key to get
113-
* @returns Promise that resolves with the key's value, or null if the key doesn't exist
113+
* @returns Promise that resolves with the key's value as a string, or null if the key doesn't exist
114114
*/
115115
get(key: string | ArrayBufferView | Blob): Promise<string | null>;
116116

117+
/**
118+
* Get the value of a key as a Uint8Array
119+
* @param key The key to get
120+
* @returns Promise that resolves with the key's value as a Uint8Array, or null if the key doesn't exist
121+
*/
122+
getBuffer(key: string | ArrayBufferView | Blob): Promise<Uint8Array<ArrayBuffer> | null>;
123+
117124
/**
118125
* Set key to hold the string value
119126
* @param key The key to set

src/bun.js/api/valkey.classes.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ export default [
3131
fn: "get",
3232
length: 1,
3333
},
34+
getBuffer: {
35+
fn: "getBuffer",
36+
length: 1,
37+
},
3438
set: {
3539
fn: "set",
3640
length: 2,

src/valkey/ValkeyCommand.zig

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ pub fn deinit(_: *Command) void {
8181
pub const Meta = packed struct(u8) {
8282
return_as_bool: bool = false,
8383
supports_auto_pipelining: bool = true,
84-
_padding: u6 = 0,
84+
return_as_buffer: bool = false,
85+
_padding: u5 = 0,
8586

8687
const not_allowed_autopipeline_commands = bun.ComptimeStringMap(void, .{
8788
.{"AUTH"},
@@ -123,7 +124,11 @@ pub const Promise = struct {
123124
}
124125

125126
pub fn resolve(self: *Promise, globalObject: *JSC.JSGlobalObject, value: *protocol.RESPValue) void {
126-
const js_value = value.toJS(globalObject) catch |err| {
127+
const options = protocol.RESPValue.ToJSOptions{
128+
.return_as_buffer = self.meta.return_as_buffer,
129+
};
130+
131+
const js_value = value.toJSWithOptions(globalObject, options) catch |err| {
127132
self.reject(globalObject, globalObject.takeError(err));
128133
return;
129134
};

src/valkey/js_valkey.zig

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,7 @@ pub const JSValkeyClient = struct {
648648
pub const expire = fns.expire;
649649
pub const expiretime = fns.expiretime;
650650
pub const get = fns.get;
651+
pub const getBuffer = fns.getBuffer;
651652
pub const getdel = fns.getdel;
652653
pub const getex = fns.getex;
653654
pub const getset = fns.getset;

src/valkey/js_valkey_functions.zig

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,26 @@ pub fn get(this: *JSValkeyClient, globalObject: *JSC.JSGlobalObject, callframe:
6060
return promise.asValue(globalObject);
6161
}
6262

63+
pub fn getBuffer(this: *JSValkeyClient, globalObject: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSValue {
64+
const key = (try fromJS(globalObject, callframe.argument(0))) orelse {
65+
return globalObject.throwInvalidArgumentType("getBuffer", "key", "string or buffer");
66+
};
67+
defer key.deinit();
68+
69+
const promise = this.send(
70+
globalObject,
71+
callframe.this(),
72+
&.{
73+
.command = "GET",
74+
.args = .{ .args = &.{key} },
75+
.meta = .{ .return_as_buffer = true },
76+
},
77+
) catch |err| {
78+
return protocol.valkeyErrorToJS(globalObject, "Failed to send GET command", err);
79+
};
80+
return promise.asValue(globalObject);
81+
}
82+
6383
pub fn set(this: *JSValkeyClient, globalObject: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSValue {
6484
const args_view = callframe.arguments();
6585
var stack_fallback = std.heap.stackFallback(512, bun.default_allocator);

src/valkey/valkey_protocol.zig

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
const std = @import("std");
2+
23
const bun = @import("bun");
34
const JSC = bun.JSC;
45
const String = bun.String;
6+
57
const debug = bun.Output.scoped(.Redis, false);
68

79
pub const RedisError = error{
@@ -244,23 +246,40 @@ pub const RESPValue = union(RESPType) {
244246
}
245247
}
246248

247-
// Convert RESPValue to JSValue
248249
pub fn toJS(self: *RESPValue, globalObject: *JSC.JSGlobalObject) bun.JSError!JSC.JSValue {
250+
return self.toJSWithOptions(globalObject, .{});
251+
}
252+
253+
pub const ToJSOptions = struct {
254+
return_as_buffer: bool = false,
255+
};
256+
257+
fn valkeyStrToJSValue(globalObject: *JSC.JSGlobalObject, str: []const u8, options: *const ToJSOptions) bun.JSError!JSC.JSValue {
258+
if (options.return_as_buffer) {
259+
// TODO: handle values > 4.7 GB
260+
const buf = JSC.ArrayBuffer.createBuffer(globalObject, str);
261+
return buf.toJS(globalObject);
262+
} else {
263+
return bun.String.createUTF8ForJS(globalObject, str);
264+
}
265+
}
266+
267+
pub fn toJSWithOptions(self: *RESPValue, globalObject: *JSC.JSGlobalObject, options: ToJSOptions) bun.JSError!JSC.JSValue {
249268
switch (self.*) {
250-
.SimpleString => |str| return bun.String.createUTF8ForJS(globalObject, str),
269+
.SimpleString => |str| return valkeyStrToJSValue(globalObject, str, &options),
251270
.Error => |str| return valkeyErrorToJS(globalObject, str, RedisError.InvalidResponse),
252271
.Integer => |int| return JSC.JSValue.jsNumber(int),
253272
.BulkString => |maybe_str| {
254273
if (maybe_str) |str| {
255-
return bun.String.createUTF8ForJS(globalObject, str);
274+
return valkeyStrToJSValue(globalObject, str, &options);
256275
} else {
257276
return JSC.JSValue.jsNull();
258277
}
259278
},
260279
.Array => |array| {
261280
var js_array = JSC.JSValue.createEmptyArray(globalObject, array.len);
262281
for (array, 0..) |*item, i| {
263-
const js_item = try item.toJS(globalObject);
282+
const js_item = try item.toJSWithOptions(globalObject, options);
264283
js_array.putIndex(globalObject, @intCast(i), js_item);
265284
}
266285
return js_array;
@@ -269,14 +288,14 @@ pub const RESPValue = union(RESPType) {
269288
.Double => |d| return JSC.JSValue.jsNumber(d),
270289
.Boolean => |b| return JSC.JSValue.jsBoolean(b),
271290
.BlobError => |str| return valkeyErrorToJS(globalObject, str, RedisError.InvalidBlobError),
272-
.VerbatimString => |verbatim| return bun.String.createUTF8ForJS(globalObject, verbatim.content),
291+
.VerbatimString => |verbatim| return valkeyStrToJSValue(globalObject, verbatim.content, &options),
273292
.Map => |entries| {
274293
var js_obj = JSC.JSValue.createEmptyObjectWithNullPrototype(globalObject);
275294
for (entries) |*entry| {
276-
const js_key = try entry.key.toJS(globalObject);
295+
const js_key = try entry.key.toJSWithOptions(globalObject, .{});
277296
var key_str = try js_key.toBunString(globalObject);
278297
defer key_str.deref();
279-
const js_value = try entry.value.toJS(globalObject);
298+
const js_value = try entry.value.toJSWithOptions(globalObject, options);
280299

281300
js_obj.putMayBeIndex(globalObject, &key_str, js_value);
282301
}
@@ -285,15 +304,15 @@ pub const RESPValue = union(RESPType) {
285304
.Set => |set| {
286305
var js_array = JSC.JSValue.createEmptyArray(globalObject, set.len);
287306
for (set, 0..) |*item, i| {
288-
const js_item = try item.toJS(globalObject);
307+
const js_item = try item.toJSWithOptions(globalObject, options);
289308
js_array.putIndex(globalObject, @intCast(i), js_item);
290309
}
291310
return js_array;
292311
},
293312
.Attribute => |attribute| {
294313
// For now, we just return the value and ignore attributes
295314
// In the future, we could attach the attributes as a hidden property
296-
return try attribute.value.toJS(globalObject);
315+
return try attribute.value.toJSWithOptions(globalObject, options);
297316
},
298317
.Push => |push| {
299318
var js_obj = JSC.JSValue.createEmptyObjectWithNullPrototype(globalObject);
@@ -305,7 +324,7 @@ pub const RESPValue = union(RESPType) {
305324
// Add the data as an array
306325
var data_array = JSC.JSValue.createEmptyArray(globalObject, push.data.len);
307326
for (push.data, 0..) |*item, i| {
308-
const js_item = try item.toJS(globalObject);
327+
const js_item = try item.toJSWithOptions(globalObject, options);
309328
data_array.putIndex(globalObject, @intCast(i), js_item);
310329
}
311330
js_obj.put(globalObject, "data", data_array);

test/js/node/no-addons.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { test, expect } from "bun:test";
21
import { spawnSync } from "bun";
2+
import { expect, test } from "bun:test";
33
import { bunExe, bunEnv as env } from "harness";
44

55
test("--no-addons throws an error on process.dlopen", () => {
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
import { beforeEach, describe, expect, test } from "bun:test";
2+
import { ConnectionType, createClient, ctx, isEnabled } from "../test-utils";
3+
4+
describe.skipIf(!isEnabled)("Valkey: Buffer Operations", () => {
5+
beforeEach(() => {
6+
if (ctx.redis?.connected) {
7+
ctx.redis.close?.();
8+
}
9+
ctx.redis = createClient(ConnectionType.TCP);
10+
});
11+
12+
test("getBuffer returns binary data as Uint8Array", async () => {
13+
const key = ctx.generateKey("buffer-test");
14+
15+
const binaryData = new Uint8Array([0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64]);
16+
await ctx.redis.set(key, binaryData);
17+
18+
const asString = await ctx.redis.get(key);
19+
const asBuffer = await ctx.redis.getBuffer(key);
20+
21+
expectAssert(asString);
22+
expectAssert(asBuffer);
23+
24+
expect(asBuffer.buffer).toBeInstanceOf(ArrayBuffer);
25+
expect(asBuffer).toBeInstanceOf(Uint8Array);
26+
expect(asBuffer.length).toBe(binaryData.length);
27+
expect(asBuffer).toStrictEqual(binaryData);
28+
29+
for (let i = 0; i < binaryData.length; i++) {
30+
expect(asBuffer[i]).toBe(binaryData[i]);
31+
}
32+
33+
const stringBuffer = Buffer.from(asString);
34+
expect(stringBuffer.length).toBe(binaryData.length);
35+
});
36+
37+
test("getBuffer for non-existent key returns null", async () => {
38+
const key = ctx.generateKey("non-existent");
39+
const result = await ctx.redis.getBuffer(key);
40+
expect(result).toBeNull();
41+
});
42+
43+
test("Really long buffer", async () => {
44+
const key = ctx.generateKey("long-buffer");
45+
const binaryData = new Uint8Array(1000000);
46+
await ctx.redis.set(key, binaryData);
47+
const result = await ctx.redis.getBuffer(key);
48+
expect(result).toBeInstanceOf(Uint8Array);
49+
});
50+
51+
test("Buffer with no bytes", async () => {
52+
const key = ctx.generateKey("empty-buffer");
53+
const binaryData = new Uint8Array(0);
54+
await ctx.redis.set(key, binaryData);
55+
const result = await ctx.redis.getBuffer(key);
56+
expectAssert(result);
57+
expect(result).toBeInstanceOf(Uint8Array);
58+
expect(result.length).toBe(0);
59+
});
60+
61+
test("Buffer with null bytes", async () => {
62+
const key = ctx.generateKey("null-bytes");
63+
const binaryData = new Uint8Array([0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09]);
64+
await ctx.redis.set(key, binaryData);
65+
const result = await ctx.redis.getBuffer(key);
66+
expectAssert(result);
67+
expect(result).toBeInstanceOf(Uint8Array);
68+
expect(result.length).toBe(binaryData.length);
69+
for (let i = 0; i < binaryData.length; i++) {
70+
expect(result[i]).toBe(binaryData[i]);
71+
}
72+
});
73+
74+
test("concurrent getBuffer against large blob", async () => {
75+
const key = ctx.generateKey("concurrent");
76+
const big = new Uint8Array(500_000).map((_, i) => i % 256);
77+
await ctx.redis.set(key, big);
78+
const readers = Array.from({ length: 20 }, () => ctx.redis.getBuffer(key));
79+
const results = await Promise.all(readers);
80+
for (const r of results) expect(r).toStrictEqual(big);
81+
});
82+
83+
test("set and getBuffer with ArrayBufferView key", async () => {
84+
const keyBytes = new Uint8Array([0x6b, 0x65, 0x79, 0x21]); // "key!"
85+
const value = new Uint8Array([0x01, 0x02, 0x03]);
86+
await ctx.redis.set(keyBytes, value);
87+
const out = await ctx.redis.getBuffer(keyBytes);
88+
expect(out).toBeInstanceOf(Uint8Array);
89+
expect(out).toStrictEqual(value);
90+
});
91+
92+
test("set and getBuffer with ArrayBuffer key", async () => {
93+
const keyBuffer = new Uint8Array([0x62, 0x75, 0x6e, 0x21]).buffer; // "bun!"
94+
expect(keyBuffer).toBeInstanceOf(ArrayBuffer);
95+
const value = new Uint8Array([0x0a, 0x0b]);
96+
await ctx.redis.set(keyBuffer, value);
97+
const out = await ctx.redis.getBuffer(keyBuffer);
98+
expect(out).toBeInstanceOf(Uint8Array);
99+
expect(out).toStrictEqual(value);
100+
});
101+
102+
test("set and getBuffer with Blob key", async () => {
103+
const keyBytes = new Uint8Array([0x74, 0x65, 0x73, 0x74]); // "test"
104+
const keyBlob = new Blob([keyBytes]);
105+
const value = new Uint8Array([0xff, 0xee, 0xdd]);
106+
await ctx.redis.set(keyBlob, value);
107+
const out = await ctx.redis.getBuffer(keyBlob);
108+
expect(out).toBeInstanceOf(Uint8Array);
109+
expect(out).toStrictEqual(value);
110+
});
111+
});
112+
113+
function expectAssert(value: unknown): asserts value {
114+
expect(value).toBeTruthy();
115+
}

0 commit comments

Comments
 (0)