From f3631f8e73014de0fa2b9d9a8f222be909aaa97b Mon Sep 17 00:00:00 2001 From: Leibale Date: Wed, 13 Dec 2023 12:27:29 -0500 Subject: [PATCH] improve decoder coverage + fix some bugs --- packages/client/lib/RESP/decoder.spec.ts | 122 +++++++++++++++++++++-- packages/client/lib/RESP/decoder.ts | 11 +- 2 files changed, 119 insertions(+), 14 deletions(-) diff --git a/packages/client/lib/RESP/decoder.spec.ts b/packages/client/lib/RESP/decoder.spec.ts index d587aea43c..37945ff699 100644 --- a/packages/client/lib/RESP/decoder.spec.ts +++ b/packages/client/lib/RESP/decoder.spec.ts @@ -1,8 +1,9 @@ import { strict as assert } from 'node:assert'; import { SinonSpy, spy } from 'sinon'; import { Decoder, RESP_TYPES } from './decoder'; -import { BlobError, ErrorReply, SimpleError } from '../errors'; +import { BlobError, SimpleError } from '../errors'; import { TypeMapping } from './types'; +import { VerbatimString } from './verbatim-string'; interface Test { toWrite: Buffer; @@ -107,6 +108,14 @@ describe('RESP Decoder', () => { toWrite: Buffer.from(':-1\r\n'), replies: [-1] }); + + test('1 as string', { + typeMapping: { + [RESP_TYPES.NUMBER]: String + }, + toWrite: Buffer.from(':1\r\n'), + replies: ['1'] + }); }); describe('BigNumber', () => { @@ -129,6 +138,14 @@ describe('RESP Decoder', () => { toWrite: Buffer.from('(-1\r\n'), replies: [-1n] }); + + test('1 as string', { + typeMapping: { + [RESP_TYPES.BIG_NUMBER]: String + }, + toWrite: Buffer.from('(1\r\n'), + replies: ['1'] + }); }); describe('Double', () => { @@ -182,15 +199,33 @@ describe('RESP Decoder', () => { replies: [1e1] }); - test('-1.1E1', { - toWrite: Buffer.from(',-1.1E1\r\n'), - replies: [-1.1E1] + test('-1.1E+1', { + toWrite: Buffer.from(',-1.1E+1\r\n'), + replies: [-1.1E+1] + }); + + test('1 as string', { + typeMapping: { + [RESP_TYPES.DOUBLE]: String + }, + toWrite: Buffer.from(',1\r\n'), + replies: ['1'] }); }); - test('SimpleString', { - toWrite: Buffer.from('+OK\r\n'), - replies: ['OK'] + describe('SimpleString', () => { + test("'OK'", { + toWrite: Buffer.from('+OK\r\n'), + replies: ['OK'] + }); + + test("'OK' as Buffer", { + typeMapping: { + [RESP_TYPES.SIMPLE_STRING]: Buffer + }, + toWrite: Buffer.from('+OK\r\n'), + replies: [Buffer.from('OK')] + }); }); describe('BlobString', () => { @@ -208,6 +243,14 @@ describe('RESP Decoder', () => { toWrite: Buffer.from('$-1\r\n'), replies: [null] }); + + test("'OK' as Buffer", { + typeMapping: { + [RESP_TYPES.BLOB_STRING]: Buffer + }, + toWrite: Buffer.from('$2\r\nOK\r\n'), + replies: [Buffer.from('OK')] + }); }); describe('VerbatimString', () => { @@ -220,6 +263,22 @@ describe('RESP Decoder', () => { toWrite: Buffer.from('=10\r\ntxt:123456\r\n'), replies: ['123456'] }); + + test("'OK' as VerbatimString", { + typeMapping: { + [RESP_TYPES.VERBATIM_STRING]: VerbatimString + }, + toWrite: Buffer.from('=6\r\ntxt:OK\r\n'), + replies: [new VerbatimString('txt', 'OK')] + }); + + test("'OK' as Buffer", { + typeMapping: { + [RESP_TYPES.VERBATIM_STRING]: Buffer + }, + toWrite: Buffer.from('=6\r\ntxt:OK\r\n'), + replies: [Buffer.from('OK')] + }); }); test('SimpleError', { @@ -256,9 +315,17 @@ describe('RESP Decoder', () => { }); test('of 0..9', { - toWrite: Buffer.from(`*10\r\n:0\r\n:1\r\n:2\r\n:3\r\n:4\r\n:5\r\n:6\r\n:7\r\n:8\r\n:9\r\n`), + toWrite: Buffer.from(`~10\r\n:0\r\n:1\r\n:2\r\n:3\r\n:4\r\n:5\r\n:6\r\n:7\r\n:8\r\n:9\r\n`), replies: [[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]] }); + + test('0..9 as Set', { + typeMapping: { + [RESP_TYPES.SET]: Set + }, + toWrite: Buffer.from(`~10\r\n:0\r\n:1\r\n:2\r\n:3\r\n:4\r\n:5\r\n:6\r\n:7\r\n:8\r\n:9\r\n`), + replies: [new Set([0, 1, 2, 3, 4, 5, 6, 7, 8, 9])] + }); }); describe('Map', () => { @@ -282,5 +349,44 @@ describe('RESP Decoder', () => { 9: { value: '9', enumerable: true } })] }); + + test("{ '0'..'9': } as Map", { + typeMapping: { + [RESP_TYPES.MAP]: Map + }, + toWrite: Buffer.from(`%10\r\n+0\r\n+0\r\n+1\r\n+1\r\n+2\r\n+2\r\n+3\r\n+3\r\n+4\r\n+4\r\n+5\r\n+5\r\n+6\r\n+6\r\n+7\r\n+7\r\n+8\r\n+8\r\n+9\r\n+9\r\n`), + replies: [new Map([ + ['0', '0'], + ['1', '1'], + ['2', '2'], + ['3', '3'], + ['4', '4'], + ['5', '5'], + ['6', '6'], + ['7', '7'], + ['8', '8'], + ['9', '9'] + ])] + }); + + test("{ '0'..'9': } as Array", { + typeMapping: { + [RESP_TYPES.MAP]: Array + }, + toWrite: Buffer.from(`%10\r\n+0\r\n+0\r\n+1\r\n+1\r\n+2\r\n+2\r\n+3\r\n+3\r\n+4\r\n+4\r\n+5\r\n+5\r\n+6\r\n+6\r\n+7\r\n+7\r\n+8\r\n+8\r\n+9\r\n+9\r\n`), + replies: [['0', '0', '1', '1', '2', '2', '3', '3', '4', '4', '5', '5', '6', '6', '7', '7', '8', '8', '9', '9']] + }); + }); + + describe('Push', () => { + test('[]', { + toWrite: Buffer.from('>0\r\n'), + pushReplies: [[]] + }); + + test('[0..9]', { + toWrite: Buffer.from(`>10\r\n:0\r\n:1\r\n:2\r\n:3\r\n:4\r\n:5\r\n:6\r\n:7\r\n:8\r\n:9\r\n`), + pushReplies: [[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]] + }); }); }); diff --git a/packages/client/lib/RESP/decoder.ts b/packages/client/lib/RESP/decoder.ts index 7c48f1f783..7ca80634eb 100644 --- a/packages/client/lib/RESP/decoder.ts +++ b/packages/client/lib/RESP/decoder.ts @@ -598,7 +598,7 @@ export class Decoder { private _continueDecodeVerbatimStringLength(lengthCb, type, chunk) { const length = lengthCb(chunk); - return typeof length === 'function'? + return typeof length === 'function' ? this._continueDecodeVerbatimStringLength.bind(this, length, type) : this._decodeVerbatimStringWithLength(length, type, chunk); } @@ -616,11 +616,10 @@ export class Decoder { } private _decodeVerbatimStringFormat(stringLength, chunk) { - return this._continueDecodeVerbatimStringFormat( - stringLength, - this._decodeStringWithLength.bind(this, 3, 1, String), - chunk - ); + const formatCb = this._decodeStringWithLength.bind(this, 3, 1, String); + return this._cursor >= chunk.length ? + this._continueDecodeVerbatimStringFormat.bind(this, stringLength, formatCb) : + this._continueDecodeVerbatimStringFormat(stringLength, formatCb, chunk); } private _continueDecodeVerbatimStringFormat(stringLength, formatCb, chunk) {