diff --git a/packages/crdt/src/crdt/timestamp.ts b/packages/crdt/src/crdt/timestamp.ts index 807b9b8045fe874ed6d00624d04f27fecc9452a1..6d431f4381dda43387ce431719d0ae7eefa92437 100644 --- a/packages/crdt/src/crdt/timestamp.ts +++ b/packages/crdt/src/crdt/timestamp.ts @@ -281,7 +281,7 @@ Timestamp.recv = function (msg) { * timestamp parsing * converts a fixed-length string timestamp to the structured value */ -Timestamp.parse = function (timestamp) { +Timestamp.parse = function (timestamp: string): Timestamp | null { if (typeof timestamp === 'string') { let parts = timestamp.split('-'); if (parts && parts.length === 5) { diff --git a/packages/loot-core/src/server/budget/actions.ts b/packages/loot-core/src/server/budget/actions.ts index e26536deb55a53fa06f0e2cf213b17a39ce304a1..2b0c527e487fbeacd6eedbc37cb6518e41d98f7a 100644 --- a/packages/loot-core/src/server/budget/actions.ts +++ b/packages/loot-core/src/server/budget/actions.ts @@ -122,7 +122,7 @@ export async function copyPreviousMonth({ month }) { let table = getBudgetTable(); let budgetData = await getBudgetData(table, prevMonth); - await batchMessages(() => { + await batchMessages(async () => { budgetData.forEach(prevBudget => { if (prevBudget.is_income === 1 && !isReflectBudget()) { return; @@ -141,7 +141,7 @@ export async function setZero({ month }) { 'SELECT * FROM v_categories WHERE tombstone = 0', ); - await batchMessages(() => { + await batchMessages(async () => { categories.forEach(cat => { if (cat.is_income === 1 && !isReflectBudget()) { return; @@ -266,7 +266,7 @@ export async function setCategoryCarryover({ startMonth, category, flag }) { let table = getBudgetTable(); let months = getAllMonths(startMonth); - await batchMessages(() => { + await batchMessages(async () => { for (let month of months) { setCarryover(table, category, dbMonth(month), flag); } diff --git a/packages/loot-core/src/server/db/index.ts b/packages/loot-core/src/server/db/index.ts index ce60678459facedc45a6733538d694e25354a6fe..ca27accf63b7fc275a207dee4b68cfcc283368d1 100644 --- a/packages/loot-core/src/server/db/index.ts +++ b/packages/loot-core/src/server/db/index.ts @@ -491,7 +491,7 @@ export async function mergePayees(target, ids) { }), ); - return Promise.all( + await Promise.all( ids.map(id => Promise.all([ update('payee_mapping', { id, targetId: target }), @@ -583,7 +583,7 @@ export async function moveAccount(id, targetId) { } const { updates, sort_order } = shoveSortOrders(accounts, targetId); - await batchMessages(() => { + await batchMessages(async () => { for (let info of updates) { update('accounts', info); } diff --git a/packages/loot-core/src/server/main.ts b/packages/loot-core/src/server/main.ts index 9777438cbc001771ff6b1d2503211c708732407c..cb8069813dba5077d513b04da22150600eb1b8f3 100644 --- a/packages/loot-core/src/server/main.ts +++ b/packages/loot-core/src/server/main.ts @@ -917,7 +917,7 @@ handlers['account-close'] = mutator(async function ({ [id], ); - await batchMessages(() => { + await batchMessages(async () => { // TODO: what this should really do is send a special message that // automatically marks the tombstone value for all transactions // within an account... or something? This is problematic diff --git a/packages/loot-core/src/server/mutators.ts b/packages/loot-core/src/server/mutators.ts index a50472c63d30d8859fcf465e7e646b8d9ff17c77..bd5035084c7d999788fbd68193a4fc56240d5153 100644 --- a/packages/loot-core/src/server/mutators.ts +++ b/packages/loot-core/src/server/mutators.ts @@ -84,11 +84,11 @@ export function disableGlobalMutations() { function _runMutator<T extends () => Promise<unknown>>( func: T, initialContext = {}, -) { +): Promise<Awaited<ReturnType<T>>> { currentContext = initialContext; return func().finally(() => { currentContext = null; - }) as ReturnType<T>; + }) as Promise<Awaited<ReturnType<T>>>; } // Type cast needed as TS looses types over nested generic returns export const runMutator = sequential(_runMutator) as typeof _runMutator; diff --git a/packages/loot-core/src/server/sync/encoder.ts b/packages/loot-core/src/server/sync/encoder.ts index e934719b3a1e505dc160fca72038eaf91c5c7a45..aa45156404904ed94db1db05c0c49cf64006f279 100644 --- a/packages/loot-core/src/server/sync/encoder.ts +++ b/packages/loot-core/src/server/sync/encoder.ts @@ -4,6 +4,8 @@ import * as encryption from '../encryption'; import { SyncError } from '../errors'; import * as prefs from '../prefs'; +import { Message } from './index'; + function coerceBuffer(value) { // The web encryption APIs give us back raw Uint8Array... but our // encryption code assumes we can work with it as a buffer. This is @@ -15,7 +17,12 @@ function coerceBuffer(value) { return value; } -export async function encode(groupId, fileId, since, messages) { +export async function encode( + groupId: string, + fileId: string, + since: string, + messages: Message[], +): Promise<Uint8Array> { let { encryptKeyId } = prefs.getPrefs(); let requestPb = new SyncProtoBuf.SyncRequest(); @@ -28,7 +35,7 @@ export async function encode(groupId, fileId, since, messages) { messagePb.setDataset(msg.dataset); messagePb.setRow(msg.row); messagePb.setColumn(msg.column); - messagePb.setValue(msg.value); + messagePb.setValue(msg.value as string); let binaryMsg = messagePb.serializeBinary(); if (encryptKeyId) { @@ -64,7 +71,9 @@ export async function encode(groupId, fileId, since, messages) { return requestPb.serializeBinary(); } -export async function decode(data) { +export async function decode( + data: Uint8Array, +): Promise<{ messages: Message[]; merkle: { hash: number } }> { let { encryptKeyId } = prefs.getPrefs(); let responsePb = SyncProtoBuf.SyncResponse.deserializeBinary(data); diff --git a/packages/loot-core/src/server/sync/index.ts b/packages/loot-core/src/server/sync/index.ts index d588900458709df764ddf8622ac197828ad38e16..4c017e14d6a5c0d91bff04c11e7a5ded9c4bee4b 100644 --- a/packages/loot-core/src/server/sync/index.ts +++ b/packages/loot-core/src/server/sync/index.ts @@ -25,6 +25,7 @@ import * as undo from '../undo'; import * as encoder from './encoder'; import { rebuildMerkleHash } from './repair'; +import { isError } from './utils'; export { default as makeTestMessage } from './make-test-message'; export { default as resetSync } from './reset'; @@ -32,8 +33,9 @@ export { default as repairSync } from './repair'; let FULL_SYNC_DELAY = 1000; let SYNCING_MODE = 'enabled'; +type SyncingMode = 'enabled' | 'offline' | 'disabled' | 'import'; -export function setSyncingMode(mode) { +export function setSyncingMode(mode: SyncingMode) { let prevMode = SYNCING_MODE; switch (mode) { case 'enabled': @@ -54,7 +56,7 @@ export function setSyncingMode(mode) { return prevMode; } -export function checkSyncingMode(mode) { +export function checkSyncingMode(mode: SyncingMode): boolean { switch (mode) { case 'enabled': return SYNCING_MODE === 'enabled' || SYNCING_MODE === 'offline'; @@ -69,7 +71,7 @@ export function checkSyncingMode(mode) { } } -function apply(msg, prev?: unknown) { +function apply(msg: Message, prev?: boolean) { let { dataset, row, column, value } = msg; if (dataset === 'prefs') { @@ -147,7 +149,7 @@ async function fetchAll(table, ids) { return results; } -export function serializeValue(value) { +export function serializeValue(value: string | number | null): string { if (value === null) { return '0:'; } else if (typeof value === 'number') { @@ -159,7 +161,7 @@ export function serializeValue(value) { throw new Error('Unserializable value type: ' + JSON.stringify(value)); } -export function deserializeValue(value) { +export function deserializeValue(value: string): string | number | null { const type = value[0]; switch (type) { case '0': @@ -174,9 +176,12 @@ export function deserializeValue(value) { throw new Error('Invalid type key for value: ' + value); } -let _syncListeners = []; +// TODO make this type stricter. +type DataMap = Map<string, unknown>; +type SyncListener = (oldData: DataMap, newData: DataMap) => unknown; +let _syncListeners: SyncListener[] = []; -export function addSyncListener(func) { +export function addSyncListener(func: SyncListener) { _syncListeners.push(func); return () => { @@ -184,7 +189,7 @@ export function addSyncListener(func) { }; } -async function compareMessages(messages) { +async function compareMessages(messages: Message[]): Promise<Message[]> { let newMessages = []; for (let i = 0; i < messages.length; i++) { @@ -218,7 +223,7 @@ async function compareMessages(messages) { // listeners importers should not rely on any functions that use any // projected state (like rules). We can't fire those because they // depend on having both old and new data which we don't quere here -function applyMessagesForImport(messages) { +function applyMessagesForImport(messages: Message[]): void { db.transaction(() => { for (let i = 0; i < messages.length; i++) { let msg = messages[i]; @@ -239,13 +244,13 @@ function applyMessagesForImport(messages) { }); } -type Message = { +export type Message = { column: string; dataset: string; old?: unknown; row: string; - timestamp: number; - value: unknown; + timestamp: string; + value: string | number | null; }; export const applyMessages = sequential(async (messages: Message[]) => { @@ -283,7 +288,7 @@ export const applyMessages = sequential(async (messages: Message[]) => { idsPerTable[msg.dataset].push(msg.row); }); - async function fetchData() { + async function fetchData(): Promise<DataMap> { let data = new Map(); for (let table of Object.keys(idsPerTable)) { @@ -415,7 +420,7 @@ export const applyMessages = sequential(async (messages: Message[]) => { return messages; }); -export function receiveMessages(messages: Message[]) { +export function receiveMessages(messages: Message[]): Promise<Message[]> { messages.forEach(msg => { Timestamp.recv(msg.timestamp); }); @@ -423,7 +428,7 @@ export function receiveMessages(messages: Message[]) { return runMutator(() => applyMessages(messages)); } -async function _sendMessages(messages) { +async function _sendMessages(messages: Message[]): Promise<void> { try { await applyMessages(messages); } catch (e) { @@ -450,15 +455,15 @@ async function _sendMessages(messages) { } let IS_BATCHING = false; -let _BATCHED = []; -export async function batchMessages(func) { +let _BATCHED: Message[] = []; +export async function batchMessages(func: () => Promise<void>): Promise<void> { if (IS_BATCHING) { await func(); return; } IS_BATCHING = true; - let batched = []; + let batched: Message[] = []; try { await func(); @@ -474,7 +479,7 @@ export async function batchMessages(func) { } } -export async function sendMessages(messages) { +export async function sendMessages(messages: Message[]) { if (IS_BATCHING) { _BATCHED = _BATCHED.concat(messages); } else { @@ -482,7 +487,7 @@ export async function sendMessages(messages) { } } -export function getMessagesSince(since) { +export function getMessagesSince(since: string) { return db.runQuery( 'SELECT timestamp, dataset, row, column, value FROM messages_crdt WHERE timestamp > ?', [since], @@ -490,19 +495,22 @@ export function getMessagesSince(since) { ); } -export async function syncAndReceiveMessages(messages, since) { +export async function syncAndReceiveMessages( + messages: Message[], + since: string, +): Promise<Message[]> { let localMessages = await getMessagesSince(since); await receiveMessages( messages.map(msg => ({ ...msg, - value: deserializeValue(msg.value), + value: deserializeValue(msg.value as string), timestamp: Timestamp.parse(msg.timestamp), })), ); return localMessages; } -export function clearFullSyncTimeout() { +export function clearFullSyncTimeout(): void { if (syncTimeout) { clearTimeout(syncTimeout); syncTimeout = null; @@ -510,13 +518,15 @@ export function clearFullSyncTimeout() { } let syncTimeout = null; -export function scheduleFullSync() { +export function scheduleFullSync(): Promise< + { messages: Message[] } | { error: unknown } +> { clearFullSyncTimeout(); if (checkSyncingMode('enabled') && !checkSyncingMode('offline')) { if (process.env.NODE_ENV === 'test') { return fullSync().then(res => { - if (res.error) { + if (isError(res)) { throw res.error; } return res; @@ -527,7 +537,7 @@ export function scheduleFullSync() { } } -function getTablesFromMessages(messages) { +function getTablesFromMessages(messages: Message[]): string[] { return messages.reduce((acc, message) => { let dataset = message.dataset === 'schedules_next_date' ? 'schedules' : message.dataset; @@ -543,15 +553,17 @@ function getTablesFromMessages(messages) { // spreadsheet to finish any processing. This is useful if we want to // perform a full sync and wait for everything to finish, usually if // you're doing an initial sync before working with a file. -export async function initialFullSync() { +export async function initialFullSync(): Promise<void> { let result = await fullSync(); - if (!result.error) { + if (isError(result)) { // Make sure to wait for anything in the spreadsheet to process await sheet.waitOnSpreadsheet(); } } -export const fullSync = once(async function () { +export const fullSync = once(async function (): Promise< + { messages: Message[] } | { error: unknown } +> { app.events.emit('sync', { type: 'start' }); let messages; @@ -621,7 +633,11 @@ export const fullSync = once(async function () { return { messages }; }); -async function _fullSync(sinceTimestamp, count, prevDiffTime) { +async function _fullSync( + sinceTimestamp: string, + count: number, + prevDiffTime: number, +): Promise<Message[]> { let { cloudFileId, groupId, lastSyncedTimestamp } = prefs.getPrefs() || {}; clearFullSyncTimeout(); @@ -678,7 +694,7 @@ async function _fullSync(sinceTimestamp, count, prevDiffTime) { receivedMessages = await receiveMessages( res.messages.map(msg => ({ ...msg, - value: deserializeValue(msg.value), + value: deserializeValue(msg.value as string), timestamp: Timestamp.parse(msg.timestamp), })), ); diff --git a/packages/loot-core/src/server/sync/migrate.test.ts b/packages/loot-core/src/server/sync/migrate.test.ts index 58a11738040679f7ec1aed63f45a9e93244a81a9..b2fd82fedfd494bc0564a758f03bfa25ccd757f0 100644 --- a/packages/loot-core/src/server/sync/migrate.test.ts +++ b/packages/loot-core/src/server/sync/migrate.test.ts @@ -7,7 +7,7 @@ import * as db from '../db'; import { listen, unlisten } from './migrate'; -import { addSyncListener, sendMessages } from './index'; +import { Message, addSyncListener, sendMessages } from './index'; beforeEach(() => { listen(); @@ -25,7 +25,7 @@ function toInternalField(publicField) { return schemaConfig.views.transactions.fields[publicField]; } -let messageArb = fc +let messageArb: fc.Arbitrary<Message> = fc .oneof(...fields.filter(f => f !== 'id').map(field => fc.constant(field))) .chain(field => { let value = arbs @@ -41,7 +41,7 @@ let messageArb = fc .noShrink() .map(date => date.toISOString() + '-0000-0123456789ABCDEF'); - return fc.record({ + return fc.record<Message>({ timestamp: timestamp, dataset: fc.constant('transactions'), column: fc.constant(toInternalField(field) || field), @@ -61,7 +61,11 @@ describe('sync migrations', () => { tracer.start(); let cleanup = addSyncListener((oldValues, newValues) => { - tracer.event('applied', [...newValues.get('transactions').keys()]); + let transactionsMap = newValues.get('transactions') as Map< + string, + unknown + >; + tracer.event('applied', [...transactionsMap.keys()]); }); await db.insert('transactions', { @@ -87,7 +91,10 @@ describe('sync migrations', () => { let tracer = execTracer(); tracer.start(); let cleanup = addSyncListener((oldValues, newValues) => { - let ts = newValues.get('transactions'); + let ts = newValues.get('transactions') as Map< + string, + { isChild: number; parent_id: string | null; id: string } + >; if ( ts && [...ts.values()].find( diff --git a/packages/loot-core/src/server/sync/sync.property.test.ts b/packages/loot-core/src/server/sync/sync.property.test.ts index 6a15cbc9d070963680104045af41c100c93f5a0f..f43f63b28e17f854f60816306fcb4256bf5d43aa 100644 --- a/packages/loot-core/src/server/sync/sync.property.test.ts +++ b/packages/loot-core/src/server/sync/sync.property.test.ts @@ -7,6 +7,7 @@ import * as sheet from '../sheet'; import * as mockSyncServer from '../tests/mockSyncServer'; import * as encoder from './encoder'; +import { isError } from './utils'; import * as sync from './index'; @@ -278,10 +279,10 @@ async function run(msgs) { ), ); - let { error } = await syncPromise; - if (error) { - console.log(error); - throw error; + let result = await syncPromise; + if (isError(result)) { + console.log(result.error); + throw result.error; } let serverMerkle = mockSyncServer.getClock().merkle; diff --git a/packages/loot-core/src/server/sync/sync.test.ts b/packages/loot-core/src/server/sync/sync.test.ts index 50f2de2d4080711bd8d0608f93fd8fefa65d507d..91cc537d9ed6e6a0b1fe60f3f3336108765852b2 100644 --- a/packages/loot-core/src/server/sync/sync.test.ts +++ b/packages/loot-core/src/server/sync/sync.test.ts @@ -6,6 +6,7 @@ import * as sheet from '../sheet'; import * as mockSyncServer from '../tests/mockSyncServer'; import * as encoder from './encoder'; +import { isError } from './utils'; import { setSyncingMode, sendMessages, applyMessages, fullSync } from './index'; @@ -86,9 +87,9 @@ describe('Sync', () => { expect(mockSyncServer.getMessages().length).toBe(0); - const { messages, error } = await fullSync(); - expect(error).toBeFalsy(); - expect(messages.length).toBe(0); + const result = await fullSync(); + if (isError(result)) throw result.error; + expect(result.messages.length).toBe(0); expect(mockSyncServer.getMessages().length).toBe(2); }); @@ -133,8 +134,9 @@ describe('Sync', () => { }, ]); - const { messages } = await fullSync(); - expect(messages.length).toBe(2); + const result = await fullSync(); + if (isError(result)) throw result.error; + expect(result.messages.length).toBe(2); expect(mockSyncServer.getMessages().length).toBe(3); }); }); diff --git a/packages/loot-core/src/server/sync/utils.ts b/packages/loot-core/src/server/sync/utils.ts new file mode 100644 index 0000000000000000000000000000000000000000..802dcc1896702b7d1bfc1921b14cc84829a35c34 --- /dev/null +++ b/packages/loot-core/src/server/sync/utils.ts @@ -0,0 +1,3 @@ +export function isError(value: unknown): value is { error: unknown } { + return (value as { error: unknown }).error !== undefined; +} diff --git a/packages/loot-core/src/shared/async.ts b/packages/loot-core/src/shared/async.ts index 3739a4e3d5fcbfc1172733876ee91255536f66cf..a534631cd99a875b90265466c717285ae2d30d27 100644 --- a/packages/loot-core/src/shared/async.ts +++ b/packages/loot-core/src/shared/async.ts @@ -1,6 +1,6 @@ export function sequential<T extends (...args: unknown[]) => unknown>( fn: T, -): (...args: Parameters<T>) => Promise<ReturnType<T>> { +): (...args: Parameters<T>) => Promise<Awaited<ReturnType<T>>> { let sequenceState = { running: null, queue: [], @@ -43,9 +43,11 @@ export function sequential<T extends (...args: unknown[]) => unknown>( }; } -export function once(fn) { +export function once<T extends (...args: unknown[]) => Promise<unknown>>( + fn: T, +): (...args: Parameters<T>) => Promise<Awaited<ReturnType<T>>> { let promise = null; - let onceFn = (...args) => { + let onceFn = (...args: Parameters<T>): Promise<Awaited<ReturnType<T>>> => { if (!promise) { promise = fn(...args).finally(() => { promise = null; diff --git a/packages/loot-core/src/shared/util.ts b/packages/loot-core/src/shared/util.ts index 3f726ea1d9897ae47adb3ab3629e5097561a2e94..f1dec4b3332b7f045c3ae5307a7bc9035f360878 100644 --- a/packages/loot-core/src/shared/util.ts +++ b/packages/loot-core/src/shared/util.ts @@ -136,18 +136,22 @@ export function groupById(data) { return res; } -export function setIn(map, keys, item) { +export function setIn( + map: Map<string, unknown>, + keys: string[], + item: unknown, +): void { for (let i = 0; i < keys.length; i++) { - let key = keys[i]; + const key = keys[i]; if (i === keys.length - 1) { map.set(key, item); } else { if (!map.has(key)) { - map.set(key, new Map()); + map.set(key, new Map<string, unknown>()); } - map = map.get(key); + map = map.get(key) as Map<string, unknown>; } } } diff --git a/upcoming-release-notes/1077.md b/upcoming-release-notes/1077.md new file mode 100644 index 0000000000000000000000000000000000000000..4bb8529f3ee7ad1f99e9939762bb848908b03295 --- /dev/null +++ b/upcoming-release-notes/1077.md @@ -0,0 +1,6 @@ +--- +category: Maintenance +authors: [TomAFrench] +--- + +Enforce proper types in server sync code