diff --git a/app/server/lib/ActionHistory.ts b/app/server/lib/ActionHistory.ts index 3af5467d..e01ad9e8 100644 --- a/app/server/lib/ActionHistory.ts +++ b/app/server/lib/ActionHistory.ts @@ -21,6 +21,18 @@ import {DocState} from 'app/common/UserAPI'; import toPairs = require('lodash/toPairs'); import {summarizeAction} from './ActionSummary'; +export interface ActionGroupOptions { + // If set, inspect the action in detail in order to include a summary of + // changes made within the action. Otherwise, the actionSummary returned is empty. + summarize?: boolean; + + // The client for which the action group is being prepared, if known. + client?: {clientId: string}|null; + + // Values returned by the action, if known. + retValues?: any[]; +} + export abstract class ActionHistory { /** * Initialize the ActionLog by reading the database. No other methods may be used until the @@ -85,9 +97,18 @@ export abstract class ActionHistory { * Get the most recent actions from the history. Results are ordered by * earliest actions first, later actions later. If `maxActions` is supplied, * at most that number of actions are returned. + * + * This method should be avoid in production, since it may convert and keep in memory many large + * actions. (It has in the past led to exhausting memory and crashing node.) */ public abstract getRecentActions(maxActions?: number): Promise; + /** + * Same as getRecentActions, but converts each to an ActionGroup using asActionGroup with the + * supplied options. + */ + public abstract getRecentActionGroups(maxActions: number, options: ActionGroupOptions): Promise; + /** * Get the most recent states from the history. States are just * actions without any content. Results are ordered by most recent @@ -194,18 +215,11 @@ export function humanDescription(actions: UserAction[]): string { * actions on the client. * @param history: interface to action history * @param act: action to convert - * @param options.summarize: if set, inspect the action in detail in order to include a summary of - * changes made within the action. Otherwise, the actionSummary returned is empty. - * @param options.client: the client for which the action group is being prepared, if known. - * @param options.retValues: values returned by the action, if known. + * @param options: options to construct the ActionGroup; see its documentation above. */ export function asActionGroup(history: ActionHistory, act: LocalActionBundle, - options: { - summarize?: boolean - client?: {clientId: string}|null, - retValues?: any[], - }): ActionGroup { + options: ActionGroupOptions): ActionGroup { const {summarize, client, retValues} = options; const info = act.info[1]; const fromSelf = (client && client.clientId && act.actionHash) ? diff --git a/app/server/lib/ActionHistoryImpl.ts b/app/server/lib/ActionHistoryImpl.ts index d4ddb365..56051992 100644 --- a/app/server/lib/ActionHistoryImpl.ts +++ b/app/server/lib/ActionHistoryImpl.ts @@ -2,12 +2,13 @@ * Minimal ActionHistory implementation */ import {LocalActionBundle} from 'app/common/ActionBundle'; +import {ActionGroup} from 'app/common/ActionGroup'; import * as marshaller from 'app/common/marshal'; import {DocState} from 'app/common/UserAPI'; import * as crypto from 'crypto'; import keyBy = require('lodash/keyBy'); import mapValues = require('lodash/mapValues'); -import {ActionHistory} from './ActionHistory'; +import {ActionGroupOptions, ActionHistory, asActionGroup} from './ActionHistory'; import {ISQLiteDB, ResultRow} from './SQLiteDB'; // History will from time to time be pruned back to within these limits @@ -370,16 +371,13 @@ export class ActionHistoryImpl implements ActionHistory { } public async getRecentActions(maxActions?: number): Promise { - const branches = await this._getBranches(); - const actions = await this._fetchParts(null, - branches.local_unsent, - "_gristsys_ActionHistory.id, actionNum, actionHash, body", - maxActions, - true); - const result = actions.map(decodeActionFromRow); - result.reverse(); // Implementation note: this could be optimized away when `maxActions` - // is not specified, by simply asking _fetchParts for ascending order. - return result; + const actions = await this._getRecentActionRows(maxActions); + return actions.map(decodeActionFromRow); + } + + public async getRecentActionGroups(maxActions: number, options: ActionGroupOptions): Promise { + const actions = await this._getRecentActionRows(maxActions); + return actions.map(row => asActionGroup(this, decodeActionFromRow(row), options)); } public async getRecentStates(maxStates?: number): Promise { @@ -438,6 +436,22 @@ export class ActionHistoryImpl implements ActionHistory { return this._actionClient.get(actionHash); } + /** + * Fetches the most recent action row from the history, orderd with earlier actions first. + * If `maxActions` is supplied, at most that number of actions are returned. + */ + private async _getRecentActionRows(maxActions?: number): Promise { + const branches = await this._getBranches(); + const result = await this._fetchParts(null, + branches.local_unsent, + "_gristsys_ActionHistory.id, actionNum, actionHash, body", + maxActions, + true); + result.reverse(); // Implementation note: this could be optimized away when `maxActions` + // is not specified, by simply asking _fetchParts for ascending order. + return result; + } + /** Check if we need to update the next shared actionNum */ private _noteSharedAction(actionNum: number): void { if (actionNum >= this._sharedActionNum) { diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index c2c4ea47..69d365d7 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -46,7 +46,7 @@ import * as log from 'app/server/lib/log'; import {shortDesc} from 'app/server/lib/shortDesc'; import {fetchURL, FileUploadInfo, globalUploadSet, UploadInfo} from 'app/server/lib/uploads'; -import {ActionHistory, asActionGroup} from './ActionHistory'; +import {ActionHistory} from './ActionHistory'; import {ActionHistoryImpl} from './ActionHistoryImpl'; import {ActiveDocImport} from './ActiveDocImport'; import {DocClients} from './DocClients'; @@ -174,6 +174,8 @@ export class ActiveDoc extends EventEmitter { return this._muted; } + // Note that this method is only used in tests, and should be avoided in production (see note + // in ActionHistory about getRecentActions). public getRecentActionsDirect(maxActions?: number): Promise { return this._actionHistory.getRecentActions(maxActions); } @@ -199,8 +201,8 @@ export class ActiveDoc extends EventEmitter { * action summaries are computed and included. */ public async getRecentActions(docSession: OptDocSession, summarize: boolean): Promise { - const actions = await this._actionHistory.getRecentActions(MAX_RECENT_ACTIONS); - const groups = actions.map(act => asActionGroup(this._actionHistory, act, {client: docSession.client, summarize})); + const groups = await this._actionHistory.getRecentActionGroups(MAX_RECENT_ACTIONS, + {client: docSession.client, summarize}); return groups.filter(actionGroup => this._granularAccess.allowActionGroup(docSession, actionGroup)); }