diff --git a/lib/storage/providers/SQLiteProvider.ts b/lib/storage/providers/SQLiteProvider.ts index d3b6fee67..7b12b7f3f 100644 --- a/lib/storage/providers/SQLiteProvider.ts +++ b/lib/storage/providers/SQLiteProvider.ts @@ -2,7 +2,7 @@ * The SQLiteStorage provider stores everything in a key/value store by * converting the value to a JSON string */ -import type {BatchQueryCommand, NitroSQLiteConnection} from 'react-native-nitro-sqlite'; +import type {BatchQueryCommand, NitroSQLiteConnection, QueryResult} from 'react-native-nitro-sqlite'; import {open} from 'react-native-nitro-sqlite'; import {getFreeDiskStorage} from 'react-native-device-info'; import type {FastMergeReplaceNullPatch} from '../../utils'; @@ -10,6 +10,13 @@ import utils from '../../utils'; import type StorageProvider from './types'; import type {StorageKeyList, StorageKeyValuePair} from './types'; +/** + * The result of the `PRAGMA compile_options`, which lists SQLite compile-time options + */ +type CompileOptionsResult = { + compile_options: string; +}; + /** * The type of the key-value pair stored in the SQLite database * @property record_key - the key of the record @@ -35,6 +42,40 @@ type PageCountResult = { }; const DB_NAME = 'OnyxDB'; +const SQLITE_MAX_VARIABLE_NUMBER = 32766; +const COMPILE_OPTIONS = { + MAX_VARIABLE_NUMBER: 'MAX_VARIABLE_NUMBER', +} as const; + +/** SQLite's maximum number of bound parameters per statement, read once from PRAGMA compile_options in init(). */ +let sqliteMaxVariableNumber = SQLITE_MAX_VARIABLE_NUMBER; + +/** + * Returns the value of a compile option from the rows returned by `PRAGMA compile_options`. + * For flag-only options (e.g. `ENABLE_FTS3`), returns an empty string when the option is present. + */ +function getCompileOptionValue(compileOptionsResult: QueryResult, optionName: string): string | undefined { + const optionPrefix = `${optionName}=`; + const rowCount = compileOptionsResult.rows?.length ?? 0; + + for (let index = 0; index < rowCount; index++) { + const compileOption = compileOptionsResult.rows?.item(index)?.compile_options; + + if (!compileOption) { + continue; + } + + if (compileOption === optionName) { + return ''; + } + + if (compileOption.startsWith(optionPrefix)) { + return compileOption.slice(optionPrefix.length); + } + } + + return undefined; +} /** * Prevents the stringifying of the object markers. @@ -76,6 +117,13 @@ const provider: StorageProvider = { provider.store.execute('PRAGMA CACHE_SIZE=-20000;'); provider.store.execute('PRAGMA synchronous=NORMAL;'); provider.store.execute('PRAGMA journal_mode=WAL;'); + + const compileOptionsResult = provider.store.execute('PRAGMA compile_options;'); + + // Get the value of MAX_VARIABLE_NUMBER from the compile options and + // stores it in a global variable, that is going to be used during runtime. + const maxVariableNumber = Number(getCompileOptionValue(compileOptionsResult, COMPILE_OPTIONS.MAX_VARIABLE_NUMBER)); + sqliteMaxVariableNumber = maxVariableNumber > 0 ? maxVariableNumber : SQLITE_MAX_VARIABLE_NUMBER; }, getItem(key) { if (!provider.store) { @@ -100,12 +148,29 @@ const provider: StorageProvider = { throw new Error('Store is not initialized!'); } - const placeholders = keys.map(() => '?').join(','); - const command = `SELECT record_key, valueJSON FROM keyvaluepairs WHERE record_key IN (${placeholders});`; - return provider.store.executeAsync(command, keys).then(({rows}) => { - // eslint-disable-next-line no-underscore-dangle - const result = rows?._array.map((row) => [row.record_key, JSON.parse(row.valueJSON)]); - return (result ?? []) as StorageKeyValuePair[]; + if (keys.length === 0) { + return Promise.resolve([]); + } + + const keyChunks = utils.chunkArray(keys, sqliteMaxVariableNumber); + + return Promise.all( + keyChunks.map((keyChunk) => { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + + const placeholders = keyChunk.map(() => '?').join(','); + const command = `SELECT record_key, valueJSON FROM keyvaluepairs WHERE record_key IN (${placeholders});`; + return provider.store.executeAsync(command, keyChunk); + }), + ).then((results) => { + const result = results.flatMap( + ({rows}) => + // eslint-disable-next-line no-underscore-dangle + rows?._array.map((row) => [row.record_key, JSON.parse(row.valueJSON)]) ?? [], + ); + return result as StorageKeyValuePair[]; }); }, setItem(key, value) { @@ -121,7 +186,7 @@ const provider: StorageProvider = { } const query = 'REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, ?);'; - const params = pairs.map((pair) => [pair[0], JSON.stringify(pair[1] === undefined ? null : pair[1])]); + const params = pairs.map(([key, value]) => [key, JSON.stringify(value === undefined ? null : value)]); if (utils.isEmptyObject(params)) { return Promise.resolve(); } @@ -214,9 +279,28 @@ const provider: StorageProvider = { throw new Error('Store is not initialized!'); } - const placeholders = keys.map(() => '?').join(','); - const query = `DELETE FROM keyvaluepairs WHERE record_key IN (${placeholders});`; - return provider.store.executeAsync(query, keys).then(() => undefined); + if (keys.length === 0) { + return Promise.resolve(); + } + + const keyChunks = utils.chunkArray(keys, sqliteMaxVariableNumber); + + const buildDeleteQuery = (keyChunk: readonly string[]) => { + const placeholders = keyChunk.map(() => '?').join(','); + return `DELETE FROM keyvaluepairs WHERE record_key IN (${placeholders});`; + }; + + if (keyChunks.length === 1) { + const keyChunk = keyChunks[0]; + return provider.store.executeAsync(buildDeleteQuery(keyChunk), keyChunk).then(() => undefined); + } + + const commands: BatchQueryCommand[] = keyChunks.map((keyChunk) => ({ + query: buildDeleteQuery(keyChunk), + params: keyChunk, + })); + + return provider.store.executeBatchAsync(commands).then(() => undefined); }, clear() { if (!provider.store) { diff --git a/lib/utils.ts b/lib/utils.ts index b469c1acd..287f4cf26 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -322,6 +322,25 @@ function omit(obj: Record, condition: string | string[] return filterObject(obj, condition, false); } +/** + * Splits an array into chunks no larger than maxChunkSize. + */ +function chunkArray(items: readonly T[], maxChunkSize: number): T[][] { + if (items.length === 0) { + return []; + } + + if (items.length <= maxChunkSize) { + return [items as T[]]; + } + + const chunks: T[][] = []; + for (let i = 0; i < items.length; i += maxChunkSize) { + chunks.push(items.slice(i, i + maxChunkSize)); + } + return chunks; +} + export default { fastMerge, isEmptyObject, @@ -330,6 +349,7 @@ export default { checkCompatibilityWithExistingValue, pick, omit, + chunkArray, ONYX_INTERNALS__REPLACE_OBJECT_MARK, }; export type {FastMergeResult, FastMergeReplaceNullPatch, FastMergeOptions}; diff --git a/tests/unit/mocks/sqliteMock.ts b/tests/unit/mocks/sqliteMock.ts index 8b5cd1d45..d7282e80b 100644 --- a/tests/unit/mocks/sqliteMock.ts +++ b/tests/unit/mocks/sqliteMock.ts @@ -7,13 +7,13 @@ * - open({name}) * - connection.execute(sql) * - connection.executeAsync(sql, params?) - * - connection.executeBatchAsync([{query, params}]) + * - connection.executeBatchAsync([{query, params}, ...]) * * Result rows are shaped to match Nitro: `{rows: {_array, item, length}}`. */ import BetterSqlite3 from 'better-sqlite3'; import type {Database} from 'better-sqlite3'; -import type {NitroSQLiteConnection, NitroSQLiteQueryResultRows, QueryResult, QueryResultRow, SQLiteQueryParams} from 'react-native-nitro-sqlite'; +import type {BatchQueryCommand, NitroSQLiteConnection, NitroSQLiteQueryResultRows, QueryResult, QueryResultRow, SQLiteQueryParams} from 'react-native-nitro-sqlite'; // `better-sqlite3` is declared as `export = Database` (CommonJS), so the type is // derived from the default import's namespace rather than via a named type import. @@ -54,7 +54,7 @@ function wrapRows(rowsArray: TRow[]): NitroSQLiteQu }; } -function prepareAndBind(database: Database, sql: string, parameters?: SQLiteQueryParams) { +function prepareAndBind(database: Database, sql: BatchQueryCommand['query'], parameters?: BatchQueryCommand['params']) { const namedOrder = extractNamedParameterOrder(sql); if (namedOrder) { // Map positional parameters array to named bindings object — NitroSQLite's @@ -71,6 +71,35 @@ function prepareAndBind(database: Database, sql: string, parameters?: SQLiteQuer return {statement: database.prepare(sql), boundArguments: parameters ?? []}; } +/** + * Expands batch commands the same way NitroSQLite does in `batchParamsToCommands`: + * `params` is either one binding set for the query, or an array of binding sets + * (same query executed once per row). + */ +function batchParamsToCommands(commands: BatchQueryCommand[]): BatchQueryCommand[] { + const expanded: BatchQueryCommand[] = []; + + for (const command of commands) { + const {query, params} = command; + + if (!params) { + expanded.push({query}); + continue; + } + + if (Array.isArray(params[0])) { + for (const rowParams of params as SQLiteQueryParams[]) { + expanded.push({query, params: rowParams}); + } + continue; + } + + expanded.push({query, params: params as SQLiteQueryParams}); + } + + return expanded; +} + function runOne(database: Database, sql: string, parameters?: SQLiteQueryParams): QueryResult { // Multi-statement (CREATE TABLE; SELECT ...; etc.) — better-sqlite3 cannot // prepare more than one statement at a time. SQLiteProvider's init() issues @@ -120,29 +149,13 @@ function makeConnection(name: string): Pick { - for (const command of commands) { - const namedOrder = extractNamedParameterOrder(command.query); - const statement = connection.prepare(command.query); - const parameterRows = (command.params ?? []) as SQLiteQueryParams[]; - if (parameterRows.length === 0) { - const info = statement.run(); - total += info.changes; - continue; - } - for (const row of parameterRows) { - if (namedOrder) { - const bindings: Record = {}; - for (let index = 0; index < namedOrder.length; index++) { - bindings[namedOrder[index]] = row[index]; - } - const info = statement.run(bindings); - total += info.changes; - } else { - const info = statement.run(...row); - total += info.changes; - } - } + for (const command of expandedCommands) { + const {statement, boundArguments} = prepareAndBind(connection, command.query, command.params); + const info = statement.run(...(boundArguments as unknown[])); + total += info.changes; } })(); return Promise.resolve({rowsAffected: total}); diff --git a/tests/unit/storage/providers/SQLiteProviderTest.ts b/tests/unit/storage/providers/SQLiteProviderTest.ts index bc0cac00a..321ccf0dd 100644 --- a/tests/unit/storage/providers/SQLiteProviderTest.ts +++ b/tests/unit/storage/providers/SQLiteProviderTest.ts @@ -312,6 +312,99 @@ describe('SQLiteProvider', () => { }); }); + describe('query splitting', () => { + const CHUNK_SIZE = 2; + const originalChunkArray = utils.chunkArray; + + const createKeyValueEntries = (count: number): Array<[string, unknown]> => Array.from({length: count}, (_, index) => [`chunk_key_${index}`, index]); + + beforeEach(() => { + resetAllDatabases(); + SQLiteProvider.init(); + + jest.spyOn(utils, 'chunkArray').mockImplementation((items, _maxChunkSize) => originalChunkArray(items, CHUNK_SIZE)); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('multiGet', () => { + it('should return all values when keys exceed MAX_VARIABLE_NUMBER', async () => { + const entries = createKeyValueEntries(5); + await SQLiteProvider.multiSet(entries); + + const keys = entries.map(([key]) => key); + const result = await SQLiteProvider.multiGet(keys); + + expect(result).toEqual(expect.arrayContaining(entries)); + expect(result).toHaveLength(entries.length); + }); + + it('should issue one IN query per chunk', async () => { + const entries = createKeyValueEntries(5); + await SQLiteProvider.multiSet(entries); + + const executeAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeAsync'); + executeAsyncSpy.mockClear(); + + const keys = entries.map(([key]) => key); + await SQLiteProvider.multiGet(keys); + + const inQueries = executeAsyncSpy.mock.calls.filter(([sql]) => typeof sql === 'string' && sql.includes('WHERE record_key IN')); + expect(inQueries).toHaveLength(3); + }); + }); + + describe('removeItems', () => { + it('should remove all keys when keys exceed MAX_VARIABLE_NUMBER', async () => { + const entries = createKeyValueEntries(5); + await SQLiteProvider.multiSet(entries); + + const keys = entries.map(([key]) => key); + await SQLiteProvider.removeItems(keys); + + expect(await SQLiteProvider.getAllKeys()).toEqual([]); + }); + + it('should use executeAsync when keys fit in a single chunk', async () => { + const entries = createKeyValueEntries(2); + await SQLiteProvider.multiSet(entries); + + const executeAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeAsync'); + const executeBatchAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeBatchAsync'); + executeAsyncSpy.mockClear(); + executeBatchAsyncSpy.mockClear(); + + const keys = entries.map(([key]) => key); + await SQLiteProvider.removeItems(keys); + + expect(executeAsyncSpy).toHaveBeenCalledTimes(1); + expect(executeBatchAsyncSpy).not.toHaveBeenCalled(); + }); + + it('should use executeBatchAsync when keys span multiple chunks', async () => { + const entries = createKeyValueEntries(5); + await SQLiteProvider.multiSet(entries); + + const executeAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeAsync'); + const executeBatchAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeBatchAsync'); + executeAsyncSpy.mockClear(); + executeBatchAsyncSpy.mockClear(); + + const keys = entries.map(([key]) => key); + await SQLiteProvider.removeItems(keys); + + expect(executeAsyncSpy).not.toHaveBeenCalled(); + expect(executeBatchAsyncSpy).toHaveBeenCalledTimes(1); + + const batchCommands = executeBatchAsyncSpy.mock.calls[0][0]; + expect(batchCommands).toHaveLength(3); + expect(batchCommands.every((command) => command.query.includes('DELETE FROM keyvaluepairs WHERE record_key IN'))).toBe(true); + }); + }); + }); + // SQLite-specific: the IN-list is parameterised, so a key containing SQL // fragments must be treated as a literal record_key. describe('SQL-injection safety', () => { diff --git a/tests/unit/utilsTest.ts b/tests/unit/utilsTest.ts index 48e51d598..6d25dabaf 100644 --- a/tests/unit/utilsTest.ts +++ b/tests/unit/utilsTest.ts @@ -390,6 +390,53 @@ describe('utils', () => { }); }); + describe('chunkArray', () => { + it('should return an empty array when given an empty array', () => { + expect(utils.chunkArray([], 3)).toEqual([]); + }); + + it('should return a single chunk when the array length is less than maxChunkSize', () => { + const items = [1, 2]; + const result = utils.chunkArray(items, 3); + + expect(result).toEqual([[1, 2]]); + expect(result[0]).toBe(items); + }); + + it('should return a single chunk when the array length equals maxChunkSize', () => { + const items = [1, 2, 3]; + const result = utils.chunkArray(items, 3); + + expect(result).toEqual([[1, 2, 3]]); + expect(result[0]).toBe(items); + }); + + it('should split the array into evenly sized chunks', () => { + expect(utils.chunkArray([1, 2, 3, 4, 5, 6], 3)).toEqual([ + [1, 2, 3], + [4, 5, 6], + ]); + }); + + it('should include a smaller final chunk when the array length is not divisible by maxChunkSize', () => { + expect(utils.chunkArray([1, 2, 3, 4, 5], 2)).toEqual([[1, 2], [3, 4], [5]]); + }); + + it('should create one item per chunk when maxChunkSize is 1', () => { + expect(utils.chunkArray(['a', 'b', 'c'], 1)).toEqual([['a'], ['b'], ['c']]); + }); + + it('should work with readonly arrays and preserve element types', () => { + const items = Object.freeze(['x', 'y', 'z', 'w']) as readonly string[]; + const result = utils.chunkArray(items, 2); + + expect(result).toEqual([ + ['x', 'y'], + ['z', 'w'], + ]); + }); + }); + describe('isEmptyObject', () => { it('should return true for an empty object', () => { expect(utils.isEmptyObject({})).toBe(true);