Skip to content

Commit

Permalink
fix: make hscan return flat list of entries (#1300)
Browse files Browse the repository at this point in the history
  • Loading branch information
silverwind authored Aug 13, 2023
1 parent eb043e3 commit 6f440f0
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 32 deletions.
17 changes: 10 additions & 7 deletions src/commands-utils/scan-command.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,30 +42,33 @@ function getCountAndMatch(args) {
return [count, matchPattern]
}

export function scanHelper(allKeys, size, cursorStart, ...args) {
// allKeysOrEntries and keysOrEntries are either:
// - an array of keys: ['key1', 'key2']
// - an array of entries: [['key1', 'value1'], ['key2', 'value2']]
export function scanHelper(allKeysOrEntries, size, cursorStart, ...args) {
const cursor = parseInt(cursorStart, 10)
if (Number.isNaN(cursor)) {
throw new Error('Cursor must be integer')
}
const [count, matchPattern] = getCountAndMatch(args)
let nextCursor = cursor + count
const keys = allKeys.slice(cursor, nextCursor)
const keysOrEntries = allKeysOrEntries.slice(cursor, nextCursor)

// Apply MATCH filtering _after_ getting number of keys
if (matchPattern) {
let i = 0
while (i < keys.length)
if (!matchPattern(keys[i])) {
keys.splice(i, size)
while (i < keysOrEntries.length)
if (!matchPattern(keysOrEntries[i])) {
keysOrEntries.splice(i, size)
} else {
i += size
}
}

// Return 0 when iteration is complete.
if (nextCursor >= allKeys.length) {
if (nextCursor >= allKeysOrEntries.length) {
nextCursor = 0
}

return [String(nextCursor), keys]
return [String(nextCursor), keysOrEntries]
}
5 changes: 3 additions & 2 deletions src/commands/hscan.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ export function hscan(key, cursor, ...args) {
if (!this.data.has(key)) {
return ['0', []]
}
const hKeys = Object.keys(this.data.get(key))
return scanHelper(hKeys, 1, cursor, ...args)
const entries = Object.entries(this.data.get(key))
const [cur, scannedEntries] = scanHelper(entries, 1, cursor, ...args)
return [cur, scannedEntries.flat()]
}

export function hscanBuffer(...args) {
Expand Down
24 changes: 8 additions & 16 deletions test/integration/commands/hscan.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,13 @@
import Chance from 'chance'
import Redis from 'ioredis'

const chance = new Chance()

describe('hscan', () => {
const redis = new Redis()
afterAll(() => {
redis.disconnect()
})

function createHashSet(keys) {
return keys.reduce((obj, key) => {
const res = obj
res[key] = chance.cc_type({ raw: true })
return res
}, {})
}
const keysToFlatEntries = keys => keys.flatMap(key => [key, `${key}v`])
const createHashSet = keys => Object.fromEntries(keys.map(key => [key, `${key}v`]))

it('should return null array if hset does not exist', () => {
return redis.hscan('key', 0).then(result => {
Expand All @@ -34,7 +26,7 @@ describe('hscan', () => {

return redis.hscan('hset', 0).then(result => {
expect(result[0]).toBe('0')
expect(result[1]).toEqual(['foo', 'bar', 'baz'])
expect(result[1]).toEqual(keysToFlatEntries(['foo', 'bar', 'baz']))
})
})

Expand All @@ -50,7 +42,7 @@ describe('hscan', () => {

return redis.hscan('hset', 0, 'MATCH', 'foo*').then(result => {
expect(result[0]).toBe('0')
expect(result[1]).toEqual(['foo0', 'foo1', 'foo2'])
expect(result[1]).toEqual(keysToFlatEntries(['foo0', 'foo1', 'foo2']))
})
}
)
Expand All @@ -69,12 +61,12 @@ describe('hscan', () => {
.hscan('hset', 0, 'MATCH', 'foo*', 'COUNT', 1)
.then(result => {
expect(result[0]).toBe('1') // more elements left, this is why cursor is not 0
expect(result[1]).toEqual(['foo0'])
expect(result[1]).toEqual(keysToFlatEntries(['foo0']))
return redis.hscan('hset', result[0], 'MATCH', 'foo*', 'COUNT', 10)
})
.then(result2 => {
expect(result2[0]).toBe('0')
expect(result2[1]).toEqual(['foo1', 'foo2'])
expect(result2[1]).toEqual(keysToFlatEntries(['foo1', 'foo2']))
})
}
)
Expand All @@ -93,12 +85,12 @@ describe('hscan', () => {
.hscan('hset', 0, 'COUNT', 3)
.then(result => {
expect(result[0]).toBe('3')
expect(result[1]).toEqual(['foo0', 'foo1', 'bar0'])
expect(result[1]).toEqual(keysToFlatEntries(['foo0', 'foo1', 'bar0']))
return redis.hscan('hset', result[0], 'COUNT', 3)
})
.then(result2 => {
expect(result2[0]).toBe('0')
expect(result2[1]).toEqual(['bar1'])
expect(result2[1]).toEqual(keysToFlatEntries(['bar1']))
})
}
)
Expand Down
14 changes: 7 additions & 7 deletions test/integration/commands/hscanStream.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import Chance from 'chance'
import Redis from 'ioredis'
import zipObject from 'lodash.zipobject'
import { ObjectWritableMock } from 'stream-mock'

const chance = new Chance()

// @TODO Rewrite test suite so it runs on a real Redis instance
;(process.env.IS_E2E ? describe.skip : describe)('hscanStream', () => {
let writable
const randomCCType = () => chance.cc_type({ raw: true })
const createHashSet = keys => zipObject(keys, keys.map(randomCCType))

const keysToFlatEntries = keys => keys.flatMap(key => [key, `${key}v`])
const createHashSet = keys => Object.fromEntries(keys.map(key => [key, `${key}v`]))

beforeEach(() => {
writable = new ObjectWritableMock()
Expand Down Expand Up @@ -39,7 +39,7 @@ const chance = new Chance()
stream.pipe(writable)
writable.on('finish', () => {
// Then
expect([].concat(...writable.data)).toEqual(['foo', 'bar'])
expect([].concat(...writable.data)).toEqual(keysToFlatEntries(['foo', 'bar']))
done()
})
})
Expand All @@ -55,7 +55,7 @@ const chance = new Chance()
writable.on('finish', () => {
// Then
expect(writable.data.length).toEqual(Math.ceil(keys.length / count))
expect([].concat(...writable.data)).toEqual(keys)
expect([].concat(...writable.data)).toEqual(keysToFlatEntries(keys))
done()
})
})
Expand All @@ -72,7 +72,7 @@ const chance = new Chance()
stream.pipe(writable)
writable.on('finish', () => {
// Then
expect([].concat(...writable.data)).toEqual(['foo0', 'foo1', 'foo2'])
expect([].concat(...writable.data)).toEqual(keysToFlatEntries(['foo0', 'foo1', 'foo2']))
done()
})
})
Expand All @@ -90,7 +90,7 @@ const chance = new Chance()
writable.on('finish', () => {
// Then
expect(writable.data.length).toEqual(Math.ceil(3))
expect([].concat(...writable.data)).toEqual(['foo0', 'foo1', 'foo2'])
expect([].concat(...writable.data)).toEqual(keysToFlatEntries(['foo0', 'foo1', 'foo2']))
done()
})
})
Expand Down

0 comments on commit 6f440f0

Please sign in to comment.