From 8abbfb3ca1eb7071ce69109ef9a675d79f3f67f6 Mon Sep 17 00:00:00 2001 From: jonathanl-bq <72158117+jonathanl-bq@users.noreply.github.com> Date: Wed, 28 Aug 2024 10:43:19 -0700 Subject: [PATCH] Node: Add binary support for hash commands (#2194) * Update CHANGELOG Signed-off-by: Jonathan Louie * Add binary variants for hash commands Signed-off-by: Jonathan Louie * Apply Prettier Signed-off-by: Jonathan Louie * Change args for hscan to be GlideString[] Signed-off-by: Jonathan Louie * Use GlideString for BaseScanOptions Signed-off-by: Jonathan Louie * Run Prettier Signed-off-by: Jonathan Louie * Exclude HSCAN command from changes for now Signed-off-by: Jonathan Louie * Revert change to return type of convertBaseScanOptionsToArgsArray Signed-off-by: Jonathan Louie * Revert accidental change of count to string for BaseScanOptions Signed-off-by: Jonathan Louie * Apply Prettier Signed-off-by: Jonathan Louie * Start changing hash commands to use DecoderOption Signed-off-by: Jonathan Louie * Add DecoderOption for hash commands Signed-off-by: Jonathan Louie * Update SharedTests Signed-off-by: Jonathan Louie * Shorten DecoderOption parameter descriptions Signed-off-by: Jonathan Louie * Apply Prettier Signed-off-by: Jonathan Louie * Try to fix test failures Signed-off-by: Jonathan Louie * Switch to toContainEqual for hrandfield binary test Signed-off-by: Jonathan Louie --------- Signed-off-by: Jonathan Louie Signed-off-by: jonathanl-bq <72158117+jonathanl-bq@users.noreply.github.com> --- CHANGELOG.md | 1 + node/src/BaseClient.ts | 83 ++++++++++++++++++++++------------ node/src/Commands.ts | 40 ++++++++--------- node/src/Transaction.ts | 32 +++++++------ node/tests/SharedTests.ts | 95 +++++++++++++++++++++++++++++---------- 5 files changed, 165 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7361c001b2..b7639dba5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ #### Changes +* Node: Added binary variant to HASH commands ([#2194](https://github.com/valkey-io/valkey-glide/pull/2194)) * Node: Added binary variant to server management commands ([#2179](https://github.com/valkey-io/valkey-glide/pull/2179)) * Node: Added/updated binary variant to connection management commands and WATCH/UNWATCH ([#2160](https://github.com/valkey-io/valkey-glide/pull/2160)) * Java: Fix docs for stream commands ([#2086](https://github.com/valkey-io/valkey-glide/pull/2086)) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index bda29ae255..8079df1e07 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -1651,7 +1651,7 @@ export class BaseClient { * ``` */ public async hset( - key: string, + key: GlideString, fieldValueMap: Record, ): Promise { return this.createWritePromise(createHSet(key, fieldValueMap)); @@ -1663,6 +1663,7 @@ export class BaseClient { * @see {@link https://valkey.io/commands/hkeys/|valkey.io} for details. * * @param key - The key of the hash. + * @param options - (Optional) See {@link DecoderOption}. * @returns A list of field names for the hash, or an empty list when the key does not exist. * * @example @@ -1673,8 +1674,12 @@ export class BaseClient { * console.log(result); // Output: ["field1", "field2", "field3"] - Returns all the field names stored in the hash "my_hash". * ``` */ - public async hkeys(key: string): Promise { - return this.createWritePromise(createHKeys(key)); + + public async hkeys( + key: GlideString, + options?: DecoderOption, + ): Promise { + return this.createWritePromise(createHKeys(key), options); } /** Sets `field` in the hash stored at `key` to `value`, only if `field` does not yet exist. @@ -1703,9 +1708,9 @@ export class BaseClient { * ``` */ public async hsetnx( - key: string, - field: string, - value: string, + key: GlideString, + field: GlideString, + value: GlideString, ): Promise { return this.createWritePromise(createHSetNX(key, field, value)); } @@ -1727,7 +1732,10 @@ export class BaseClient { * console.log(result); // Output: 2 - Indicates that two fields were successfully removed from the hash. * ``` */ - public async hdel(key: string, fields: string[]): Promise { + public async hdel( + key: GlideString, + fields: GlideString[], + ): Promise { return this.createWritePromise(createHDel(key, fields)); } @@ -1737,6 +1745,7 @@ export class BaseClient { * * @param key - The key of the hash. * @param fields - The fields in the hash stored at `key` to retrieve from the database. + * @param options - (Optional) See {@link DecoderOption}. * @returns a list of values associated with the given fields, in the same order as they are requested. * For every field that does not exist in the hash, a null value is returned. * If `key` does not exist, it is treated as an empty hash and it returns a list of null values. @@ -1749,10 +1758,11 @@ export class BaseClient { * ``` */ public async hmget( - key: string, - fields: string[], - ): Promise<(string | null)[]> { - return this.createWritePromise(createHMGet(key, fields)); + key: GlideString, + fields: GlideString[], + options?: DecoderOption, + ): Promise<(GlideString | null)[]> { + return this.createWritePromise(createHMGet(key, fields), options); } /** Returns if `field` is an existing field in the hash stored at `key`. @@ -1777,7 +1787,10 @@ export class BaseClient { * console.log(result); // Output: false * ``` */ - public async hexists(key: string, field: string): Promise { + public async hexists( + key: GlideString, + field: GlideString, + ): Promise { return this.createWritePromise(createHExists(key, field)); } @@ -1796,7 +1809,7 @@ export class BaseClient { * console.log(result); // Output: {"field1": "value1", "field2": "value2"} * ``` */ - public async hgetall(key: string): Promise> { + public async hgetall(key: GlideString): Promise> { return this.createWritePromise(createHGetAll(key)); } @@ -1819,8 +1832,8 @@ export class BaseClient { * ``` */ public async hincrBy( - key: string, - field: string, + key: GlideString, + field: GlideString, amount: number, ): Promise { return this.createWritePromise(createHIncrBy(key, field, amount)); @@ -1841,12 +1854,12 @@ export class BaseClient { * ```typescript * // Example usage of the hincrbyfloat method to increment the value of a floating point in a hash by a specified amount * const result = await client.hincrbyfloat("my_hash", "field1", 2.5); - * console.log(result); // Output: '2.5' + * console.log(result); // Output: 2.5 * ``` */ public async hincrByFloat( - key: string, - field: string, + key: GlideString, + field: GlideString, amount: number, ): Promise { return this.createWritePromise(createHIncrByFloat(key, field, amount)); @@ -1873,7 +1886,7 @@ export class BaseClient { * console.log(result); // Output: 0 * ``` */ - public async hlen(key: string): Promise { + public async hlen(key: GlideString): Promise { return this.createWritePromise(createHLen(key)); } @@ -1916,7 +1929,10 @@ export class BaseClient { * console.log(result); // Output: 5 * ``` */ - public async hstrlen(key: string, field: string): Promise { + public async hstrlen( + key: GlideString, + field: GlideString, + ): Promise { return this.createWritePromise(createHStrlen(key, field)); } @@ -1927,6 +1943,7 @@ export class BaseClient { * @remarks Since Valkey version 6.2.0. * * @param key - The key of the hash. + * @param options - (Optional) See {@link DecoderOption}. * @returns A random field name from the hash stored at `key`, or `null` when * the key does not exist. * @@ -1935,8 +1952,11 @@ export class BaseClient { * console.log(await client.hrandfield("myHash")); // Output: 'field' * ``` */ - public async hrandfield(key: string): Promise { - return this.createWritePromise(createHRandField(key)); + public async hrandfield( + key: GlideString, + options?: DecoderOption, + ): Promise { + return this.createWritePromise(createHRandField(key), options); } /** @@ -1992,6 +2012,7 @@ export class BaseClient { * * @param key - The key of the hash. * @param count - The number of field names to return. + * @param options - (Optional) See {@link DecoderOption}. * * If `count` is positive, returns unique elements. If negative, allows for duplicates. * @returns An `array` of random field names from the hash stored at `key`, @@ -2003,10 +2024,11 @@ export class BaseClient { * ``` */ public async hrandfieldCount( - key: string, + key: GlideString, count: number, - ): Promise { - return this.createWritePromise(createHRandField(key, count)); + options?: DecoderOption, + ): Promise { + return this.createWritePromise(createHRandField(key, count), options); } /** @@ -2018,6 +2040,7 @@ export class BaseClient { * * @param key - The key of the hash. * @param count - The number of field names to return. + * @param options - (Optional) See {@link DecoderOption}. * * If `count` is positive, returns unique elements. If negative, allows for duplicates. * @returns A 2D `array` of `[fieldName, value]` `arrays`, where `fieldName` is a random @@ -2031,10 +2054,14 @@ export class BaseClient { * ``` */ public async hrandfieldWithValues( - key: string, + key: GlideString, count: number, - ): Promise<[string, string][]> { - return this.createWritePromise(createHRandField(key, count, true)); + options?: DecoderOption, + ): Promise<[GlideString, GlideString][]> { + return this.createWritePromise( + createHRandField(key, count, true), + options, + ); } /** Inserts all the specified values at the head of the list stored at `key`. diff --git a/node/src/Commands.ts b/node/src/Commands.ts index 171b065138..22dd1f95ba 100644 --- a/node/src/Commands.ts +++ b/node/src/Commands.ts @@ -412,7 +412,7 @@ export function createHGet( * @internal */ export function createHSet( - key: string, + key: GlideString, fieldValueMap: Record, ): command_request.Command { return createCommand( @@ -424,7 +424,7 @@ export function createHSet( /** * @internal */ -export function createHKeys(key: string): command_request.Command { +export function createHKeys(key: GlideString): command_request.Command { return createCommand(RequestType.HKeys, [key]); } @@ -432,9 +432,9 @@ export function createHKeys(key: string): command_request.Command { * @internal */ export function createHSetNX( - key: string, - field: string, - value: string, + key: GlideString, + field: GlideString, + value: GlideString, ): command_request.Command { return createCommand(RequestType.HSetNX, [key, field, value]); } @@ -801,8 +801,8 @@ export function createBitField( * @internal */ export function createHDel( - key: string, - fields: string[], + key: GlideString, + fields: GlideString[], ): command_request.Command { return createCommand(RequestType.HDel, [key].concat(fields)); } @@ -811,8 +811,8 @@ export function createHDel( * @internal */ export function createHMGet( - key: string, - fields: string[], + key: GlideString, + fields: GlideString[], ): command_request.Command { return createCommand(RequestType.HMGet, [key].concat(fields)); } @@ -821,8 +821,8 @@ export function createHMGet( * @internal */ export function createHExists( - key: string, - field: string, + key: GlideString, + field: GlideString, ): command_request.Command { return createCommand(RequestType.HExists, [key, field]); } @@ -830,7 +830,7 @@ export function createHExists( /** * @internal */ -export function createHGetAll(key: string): command_request.Command { +export function createHGetAll(key: GlideString): command_request.Command { return createCommand(RequestType.HGetAll, [key]); } @@ -1193,8 +1193,8 @@ export function createCustomCommand(args: GlideString[]) { * @internal */ export function createHIncrBy( - key: string, - field: string, + key: GlideString, + field: GlideString, amount: number, ): command_request.Command { return createCommand(RequestType.HIncrBy, [key, field, amount.toString()]); @@ -1204,8 +1204,8 @@ export function createHIncrBy( * @internal */ export function createHIncrByFloat( - key: string, - field: string, + key: GlideString, + field: GlideString, amount: number, ): command_request.Command { return createCommand(RequestType.HIncrByFloat, [ @@ -1218,7 +1218,7 @@ export function createHIncrByFloat( /** * @internal */ -export function createHLen(key: string): command_request.Command { +export function createHLen(key: GlideString): command_request.Command { return createCommand(RequestType.HLen, [key]); } @@ -3618,15 +3618,15 @@ function createSortImpl( * @internal */ export function createHStrlen( - key: string, - field: string, + key: GlideString, + field: GlideString, ): command_request.Command { return createCommand(RequestType.HStrlen, [key, field]); } /** @internal */ export function createHRandField( - key: string, + key: GlideString, count?: number, withValues?: boolean, ): command_request.Command { diff --git a/node/src/Transaction.ts b/node/src/Transaction.ts index 05f0d229b3..3d9de03d50 100644 --- a/node/src/Transaction.ts +++ b/node/src/Transaction.ts @@ -809,7 +809,7 @@ export class BaseTransaction> { * * Command Response - The number of fields that were added. */ - public hset(key: string, fieldValueMap: Record): T { + public hset(key: GlideString, fieldValueMap: Record): T { return this.addAndReturn(createHSet(key, fieldValueMap)); } @@ -822,7 +822,7 @@ export class BaseTransaction> { * * Command Response - A list of field names for the hash, or an empty list when the key does not exist. */ - public hkeys(key: string): T { + public hkeys(key: GlideString): T { return this.addAndReturn(createHKeys(key)); } @@ -837,7 +837,7 @@ export class BaseTransaction> { * * Command Response - `true` if the field was set, `false` if the field already existed and was not set. */ - public hsetnx(key: string, field: string, value: string): T { + public hsetnx(key: GlideString, field: GlideString, value: GlideString): T { return this.addAndReturn(createHSetNX(key, field, value)); } @@ -851,7 +851,7 @@ export class BaseTransaction> { * Command Response - the number of fields that were removed from the hash, not including specified but non existing fields. * If `key` does not exist, it is treated as an empty hash and it returns 0. */ - public hdel(key: string, fields: string[]): T { + public hdel(key: GlideString, fields: GlideString[]): T { return this.addAndReturn(createHDel(key, fields)); } @@ -865,7 +865,7 @@ export class BaseTransaction> { * For every field that does not exist in the hash, a null value is returned. * If `key` does not exist, it is treated as an empty hash and it returns a list of null values. */ - public hmget(key: string, fields: string[]): T { + public hmget(key: GlideString, fields: GlideString[]): T { return this.addAndReturn(createHMGet(key, fields)); } @@ -878,7 +878,7 @@ export class BaseTransaction> { * Command Response - `true` if the hash contains `field`. If the hash does not contain `field`, or if `key` does not exist, * the command response will be `false`. */ - public hexists(key: string, field: string): T { + public hexists(key: GlideString, field: GlideString): T { return this.addAndReturn(createHExists(key, field)); } @@ -890,7 +890,7 @@ export class BaseTransaction> { * Command Response - a map of fields and their values stored in the hash. Every field name in the map is followed by its value. * If `key` does not exist, it returns an empty map. */ - public hgetall(key: string): T { + public hgetall(key: GlideString): T { return this.addAndReturn(createHGetAll(key)); } @@ -905,7 +905,7 @@ export class BaseTransaction> { * * Command Response - the value of `field` in the hash stored at `key` after the increment. */ - public hincrBy(key: string, field: string, amount: number): T { + public hincrBy(key: GlideString, field: GlideString, amount: number): T { return this.addAndReturn(createHIncrBy(key, field, amount)); } @@ -920,7 +920,11 @@ export class BaseTransaction> { * * Command Response - the value of `field` in the hash stored at `key` after the increment. */ - public hincrByFloat(key: string, field: string, amount: number): T { + public hincrByFloat( + key: GlideString, + field: GlideString, + amount: number, + ): T { return this.addAndReturn(createHIncrByFloat(key, field, amount)); } @@ -931,7 +935,7 @@ export class BaseTransaction> { * * Command Response - The number of fields in the hash, or 0 when the key does not exist. */ - public hlen(key: string): T { + public hlen(key: GlideString): T { return this.addAndReturn(createHLen(key)); } @@ -956,7 +960,7 @@ export class BaseTransaction> { * * Command Response - The string length or `0` if `field` or `key` does not exist. */ - public hstrlen(key: string, field: string): T { + public hstrlen(key: GlideString, field: GlideString): T { return this.addAndReturn(createHStrlen(key, field)); } @@ -971,7 +975,7 @@ export class BaseTransaction> { * Command Response - A random field name from the hash stored at `key`, or `null` when * the key does not exist. */ - public hrandfield(key: string): T { + public hrandfield(key: GlideString): T { return this.addAndReturn(createHRandField(key)); } @@ -1008,7 +1012,7 @@ export class BaseTransaction> { * Command Response - An `array` of random field names from the hash stored at `key`, * or an `empty array` when the key does not exist. */ - public hrandfieldCount(key: string, count: number): T { + public hrandfieldCount(key: GlideString, count: number): T { return this.addAndReturn(createHRandField(key, count)); } @@ -1028,7 +1032,7 @@ export class BaseTransaction> { * field name from the hash and `value` is the associated value of the field name. * If the hash does not exist or is empty, the response will be an empty `array`. */ - public hrandfieldWithValues(key: string, count: number): T { + public hrandfieldWithValues(key: GlideString, count: number): T { return this.addAndReturn(createHRandField(key, count, true)); } diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index df350b6344..c3b82e046b 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -1332,7 +1332,9 @@ export function runBaseTests(config: { const valueEncoded = Buffer.from(value); expect(await client.hset(key, fieldValueMap)).toEqual(2); - expect(await client.hget(key, field1)).toEqual(value); + expect( + await client.hget(Buffer.from(key), Buffer.from(field1)), + ).toEqual(value); expect(await client.hget(key, field2)).toEqual(value); expect(await client.hget(key, "nonExistingField")).toEqual( null, @@ -1367,6 +1369,7 @@ export function runBaseTests(config: { [field1]: value, [field2]: value2, }; + const field2Encoded = Buffer.from(field2); // set up hash with two keys/values expect(await client.hset(key, fieldValueMap)).toEqual(2); @@ -1374,7 +1377,11 @@ export function runBaseTests(config: { // remove one key expect(await client.hdel(key, [field1])).toEqual(1); - expect(await client.hkeys(key)).toEqual([field2]); + expect( + await client.hkeys(Buffer.from(key), { + decoder: Decoder.Bytes, + }), + ).toEqual([field2Encoded]); // non-existing key returns an empty list expect(await client.hkeys("nonExistingKey")).toEqual([]); @@ -1657,7 +1664,9 @@ export function runBaseTests(config: { }; expect(await client.hset(key, fieldValueMap)).toEqual(3); - expect(await client.hdel(key, [field1, field2])).toEqual(2); + expect( + await client.hdel(Buffer.from(key), [field1, field2]), + ).toEqual(2); expect(await client.hdel(key, ["nonExistingField"])).toEqual(0); expect(await client.hdel("nonExistingKey", [field3])).toEqual( 0, @@ -1688,7 +1697,11 @@ export function runBaseTests(config: { ]), ).toEqual([value, null, value]); expect( - await client.hmget("nonExistingKey", [field1, field2]), + await client.hmget( + Buffer.from("nonExistingKey"), + [field1, field2], + { decoder: Decoder.Bytes }, + ), ).toEqual([null, null]); }, protocol); }, @@ -1707,7 +1720,9 @@ export function runBaseTests(config: { [field2]: "value2", }; expect(await client.hset(key, fieldValueMap)).toEqual(2); - expect(await client.hexists(key, field1)).toEqual(true); + expect( + await client.hexists(Buffer.from(key), Buffer.from(field1)), + ).toEqual(true); expect(await client.hexists(key, "nonExistingField")).toEqual( false, ); @@ -1738,7 +1753,9 @@ export function runBaseTests(config: { [field2]: value, }); - expect(await client.hgetall("nonExistingKey")).toEqual({}); + expect( + await client.hgetall(Buffer.from("nonExistingKey")), + ).toEqual({}); }, protocol); }, config.timeout, @@ -1755,10 +1772,20 @@ export function runBaseTests(config: { }; expect(await client.hset(key, fieldValueMap)).toEqual(1); expect(await client.hincrBy(key, field, 1)).toEqual(11); - expect(await client.hincrBy(key, field, 4)).toEqual(15); - expect(await client.hincrByFloat(key, field, 1.5)).toEqual( - 16.5, - ); + expect( + await client.hincrBy( + Buffer.from(key), + Buffer.from(field), + 4, + ), + ).toEqual(15); + expect( + await client.hincrByFloat( + Buffer.from(key), + Buffer.from(field), + 1.5, + ), + ).toEqual(16.5); }, protocol); }, config.timeout, @@ -1838,7 +1865,7 @@ export function runBaseTests(config: { expect(await client.hset(key1, fieldValueMap)).toEqual(2); expect(await client.hlen(key1)).toEqual(2); expect(await client.hdel(key1, [field1])).toEqual(1); - expect(await client.hlen(key1)).toEqual(1); + expect(await client.hlen(Buffer.from(key1))).toEqual(1); expect(await client.hlen("nonExistingHash")).toEqual(0); }, protocol); }, @@ -1861,10 +1888,14 @@ export function runBaseTests(config: { const value1Encoded = Buffer.from("value1"); const value2Encoded = Buffer.from("value2"); - expect(await client.hset(key1, fieldValueMap)).toEqual(2); + expect( + await client.hset(Buffer.from(key1), fieldValueMap), + ).toEqual(2); expect(await client.hvals(key1)).toEqual(["value1", "value2"]); expect(await client.hdel(key1, [field1])).toEqual(1); - expect(await client.hvals(key1)).toEqual(["value2"]); + expect(await client.hvals(Buffer.from(key1))).toEqual([ + "value2", + ]); expect(await client.hvals("nonExistingHash")).toEqual([]); //hvals with binary buffers @@ -1891,9 +1922,13 @@ export function runBaseTests(config: { const field = uuidv4(); expect(await client.hsetnx(key1, field, "value")).toEqual(true); - expect(await client.hsetnx(key1, field, "newValue")).toEqual( - false, - ); + expect( + await client.hsetnx( + Buffer.from(key1), + Buffer.from(field), + "newValue", + ), + ).toEqual(false); expect(await client.hget(key1, field)).toEqual("value"); expect(await client.set(key2, "value")).toEqual("OK"); @@ -1924,9 +1959,9 @@ export function runBaseTests(config: { // key exists but holds non hash type value expect(await client.set(key2, "value")).toEqual("OK"); - await expect(client.hstrlen(key2, field)).rejects.toThrow( - RequestError, - ); + await expect( + client.hstrlen(Buffer.from(key2), Buffer.from(field)), + ).rejects.toThrow(RequestError); }, protocol); }, config.timeout, @@ -1944,13 +1979,19 @@ export function runBaseTests(config: { const key2 = uuidv4(); // key does not exist - expect(await client.hrandfield(key1)).toBeNull(); + expect( + await client.hrandfield(Buffer.from(key1), { + decoder: Decoder.Bytes, + }), + ).toBeNull(); expect(await client.hrandfieldCount(key1, 5)).toEqual([]); expect(await client.hrandfieldWithValues(key1, 5)).toEqual([]); const data = { "f 1": "v 1", "f 2": "v 2", "f 3": "v 3" }; const fields = Object.keys(data); const entries = Object.entries(data); + const encodedFields = fields.map(Buffer.from); + const encodedEntries = entries.map((e) => e.map(Buffer.from)); expect(await client.hset(key1, data)).toEqual(3); expect(fields).toContain(await client.hrandfield(key1)); @@ -1960,13 +2001,19 @@ export function runBaseTests(config: { expect(result).toEqual(fields); // With Count - negative count - result = await client.hrandfieldCount(key1, -5); + result = await client.hrandfieldCount(Buffer.from(key1), -5, { + decoder: Decoder.Bytes, + }); expect(result.length).toEqual(5); - result.map((r) => expect(fields).toContain(r)); + result.map((r) => expect(encodedFields).toContainEqual(r)); // With values - positive count - let result2 = await client.hrandfieldWithValues(key1, 5); - expect(result2).toEqual(entries); + let result2 = await client.hrandfieldWithValues( + Buffer.from(key1), + 5, + { decoder: Decoder.Bytes }, + ); + expect(result2).toEqual(encodedEntries); // With values - negative count result2 = await client.hrandfieldWithValues(key1, -5);