From 94b3af9ec36824ac0f13f3021f06e54bb6636dd6 Mon Sep 17 00:00:00 2001 From: Dushusir <1414556676@qq.com> Date: Tue, 26 Nov 2024 21:20:42 +0800 Subject: [PATCH 1/3] fix(formula): update formula data model --- packages/engine-formula/src/basics/common.ts | 10 + .../src/controller/calculate.controller.ts | 8 - .../__tests__/create-command-test-bed.ts | 4 - .../__tests__/create-function-test-bed.ts | 4 - .../__tests__/formula-data.model.spec.ts | 121 +++---- .../src/models/formula-data.model.ts | 327 ++++++++---------- .../src/models/utils/formula-data-util.ts | 14 +- .../src/apis/sheets/__tests__/f-range.spec.ts | 4 - .../__tests__/create-command-test-bed.ts | 1 - .../formula-clipboard.controller.ts | 6 +- .../formula-editor-show.controller.ts | 9 +- .../controllers/update-formula.controller.ts | 2 +- .../src/services/__test__/util.ts | 1 - 13 files changed, 210 insertions(+), 301 deletions(-) diff --git a/packages/engine-formula/src/basics/common.ts b/packages/engine-formula/src/basics/common.ts index 7b5578aa6530..45f404e8197c 100644 --- a/packages/engine-formula/src/basics/common.ts +++ b/packages/engine-formula/src/basics/common.ts @@ -125,6 +125,16 @@ export interface IFormulaData { [unitId: string]: Nullable<{ [sheetId: string]: Nullable>> }>; } +export interface IFormulaIdMap { + f: string; + r: number; + c: number; +} + +export interface IFormulaIdMapData { + [unitId: string]: Nullable<{ [subUnitId: string]: Nullable<{ [formulaId: string]: IFormulaIdMap }> }>; +} + export interface IOtherFormulaData { [unitId: string]: Nullable<{ [subUnitId: string]: Nullable<{ [formulaId: string]: IOtherFormulaDataItem }> }>; } diff --git a/packages/engine-formula/src/controller/calculate.controller.ts b/packages/engine-formula/src/controller/calculate.controller.ts index 940dcbc9ff1b..4fea92d06924 100644 --- a/packages/engine-formula/src/controller/calculate.controller.ts +++ b/packages/engine-formula/src/controller/calculate.controller.ts @@ -15,11 +15,9 @@ */ import type { ICommandInfo } from '@univerjs/core'; -import type { IFormulaData } from '../basics/common'; import type { ISetArrayFormulaDataMutationParams } from '../commands/mutations/set-array-formula-data.mutation'; import type { ISetFormulaCalculationStartMutation } from '../commands/mutations/set-formula-calculation.mutation'; -import type { ISetFormulaDataMutationParams } from '../commands/mutations/set-formula-data.mutation'; import type { IFormulaDirtyData } from '../services/current-data.service'; import { Disposable, ICommandService, Inject } from '@univerjs/core'; import { convertRuntimeToUnitData } from '../basics/runtime'; @@ -30,7 +28,6 @@ import { SetFormulaCalculationStartMutation, SetFormulaCalculationStopMutation, } from '../commands/mutations/set-formula-calculation.mutation'; -import { SetFormulaDataMutation } from '../commands/mutations/set-formula-data.mutation'; import { FormulaDataModel } from '../models/formula-data.model'; import { ICalculateFormulaService } from '../services/calculate-formula.service'; import { FormulaExecutedStateType, type IAllRuntimeData } from '../services/runtime.service'; @@ -56,11 +53,6 @@ export class CalculateController extends Disposable { this._commandService.onCommandExecuted((command: ICommandInfo) => { if (command.id === SetFormulaCalculationStopMutation.id) { this._calculateFormulaService.stopFormulaExecution(); - } else if (command.id === SetFormulaDataMutation.id) { - const formulaData = (command.params as ISetFormulaDataMutationParams).formulaData as IFormulaData; - - // formulaData is the incremental data sent from the main thread and needs to be merged into formulaDataModel - this._formulaDataModel.mergeFormulaData(formulaData); } else if (command.id === SetFormulaCalculationStartMutation.id) { const params = command.params as ISetFormulaCalculationStartMutation; diff --git a/packages/engine-formula/src/engine/analysis/__tests__/create-command-test-bed.ts b/packages/engine-formula/src/engine/analysis/__tests__/create-command-test-bed.ts index 1212aad82f66..ea2499dc6126 100644 --- a/packages/engine-formula/src/engine/analysis/__tests__/create-command-test-bed.ts +++ b/packages/engine-formula/src/engine/analysis/__tests__/create-command-test-bed.ts @@ -250,10 +250,6 @@ export function createCommandTestBed(workbookData?: IWorkbookData, dependencies? registerFormulaDependencies(this._injector); dependencies?.forEach((d) => this._injector.add(d)); } - - override onReady(): void { - this._formulaDataModel?.initFormulaData(); - } } univer.registerPlugin(TestPlugin); diff --git a/packages/engine-formula/src/functions/__tests__/create-function-test-bed.ts b/packages/engine-formula/src/functions/__tests__/create-function-test-bed.ts index f175f0fa8002..148b582914df 100644 --- a/packages/engine-formula/src/functions/__tests__/create-function-test-bed.ts +++ b/packages/engine-formula/src/functions/__tests__/create-function-test-bed.ts @@ -203,10 +203,6 @@ export function createFunctionTestBed(workbookData?: IWorkbookData, dependencies dependencies?.forEach((d) => injector.add(d)); } - - override onReady(): void { - this._formulaDataModel?.initFormulaData(); - } } univer.registerPlugin(TestPlugin); diff --git a/packages/engine-formula/src/models/__tests__/formula-data.model.spec.ts b/packages/engine-formula/src/models/__tests__/formula-data.model.spec.ts index 6fdcdad74d5a..be73ee21d017 100644 --- a/packages/engine-formula/src/models/__tests__/formula-data.model.spec.ts +++ b/packages/engine-formula/src/models/__tests__/formula-data.model.spec.ts @@ -97,8 +97,6 @@ describe('Test formula data model', () => { describe('updateFormulaData', () => { it('delete formula with id', () => { - formulaDataModel.initFormulaData(); - const unitId = 'test'; const sheetId = 'sheet1'; const cellValue = { @@ -113,41 +111,30 @@ describe('Test formula data model', () => { }; const result = { - [unitId]: { - [sheetId]: { - 0: { - 3: { - f: '=SUM(A1)', - si: '3e4r5t', - }, - }, - 2: { - 3: { - f: '=SUM(A3)', - si: 'OSPtzm', - }, - }, - 3: { - 3: { - f: '=SUM(A3)', - si: 'OSPtzm', - x: 0, - y: 1, - }, - }, + 1: { + 3: null, + }, + 2: { + 3: { + f: '=SUM(A3)', + si: 'OSPtzm', + }, + }, + 3: { + 3: { + f: '=SUM(A3)', + si: 'OSPtzm', + x: 0, + y: 1, }, }, }; - formulaDataModel.updateFormulaData(unitId, sheetId, cellValue); - - const formulaData = formulaDataModel.getFormulaData(); - expect(formulaData).toStrictEqual(result); + const newFormulaData = formulaDataModel.updateFormulaData(unitId, sheetId, cellValue); + expect(newFormulaData).toStrictEqual(result); }); it('delete formulas with ids and formulas with only ids', () => { - formulaDataModel.initFormulaData(); - const unitId = 'test'; const sheetId = 'sheet1'; const cellValue = { @@ -178,27 +165,28 @@ describe('Test formula data model', () => { }; const result = { - [unitId]: { - [sheetId]: { - 3: { - 3: { - f: '=SUM(A4)', - si: 'OSPtzm', - }, - }, + 0: { + 3: null, + }, + 1: { + 3: null, + }, + 2: { + 3: null, + }, + 3: { + 3: { + f: '=SUM(A4)', + si: 'OSPtzm', }, }, }; - formulaDataModel.updateFormulaData(unitId, sheetId, cellValue); - - const formulaData = formulaDataModel.getFormulaData(); - expect(formulaData).toStrictEqual(result); + const newFormulaData = formulaDataModel.updateFormulaData(unitId, sheetId, cellValue); + expect(newFormulaData).toStrictEqual(result); }); it('delete the formula with only id', () => { - formulaDataModel.initFormulaData(); - const unitId = 'test'; const sheetId = 'sheet1'; const cellValue = { @@ -213,36 +201,27 @@ describe('Test formula data model', () => { }; const result = { - [unitId]: { - [sheetId]: { - 0: { - 3: { - f: '=SUM(A1)', - si: '3e4r5t', - }, - }, - 1: { - 3: { - f: '=SUM(A2)', - si: 'OSPtzm', - }, - }, - 2: { - 3: { - f: '=SUM(A2)', - si: 'OSPtzm', - x: 0, - y: 1, - }, - }, + 1: { + 3: { + f: '=SUM(A2)', + si: 'OSPtzm', + }, + }, + 2: { + 3: { + f: '=SUM(A2)', + si: 'OSPtzm', + x: 0, + y: 1, }, }, + 3: { + 3: null, + }, }; - formulaDataModel.updateFormulaData(unitId, sheetId, cellValue); - - const formulaData = formulaDataModel.getFormulaData(); - expect(formulaData).toStrictEqual(result); + const newFormulaData = formulaDataModel.updateFormulaData(unitId, sheetId, cellValue); + expect(newFormulaData).toStrictEqual(result); }); }); @@ -352,8 +331,6 @@ describe('Test formula data model', () => { describe('getFormulaStringByCell', () => { it('get formula string by cell', () => { - formulaDataModel.initFormulaData(); - const unitId = 'test'; const sheetId = 'sheet1'; diff --git a/packages/engine-formula/src/models/formula-data.model.ts b/packages/engine-formula/src/models/formula-data.model.ts index 17b8b9785045..4bb46ad6f064 100644 --- a/packages/engine-formula/src/models/formula-data.model.ts +++ b/packages/engine-formula/src/models/formula-data.model.ts @@ -20,14 +20,14 @@ import type { IArrayFormulaUnitCellType, IFormulaData, IFormulaDataItem, + IFormulaIdMap, + IFormulaIdMapData, IRuntimeUnitDataType, ISheetData, IUnitData, IUnitSheetNameMap, } from '../basics/common'; -import type { IFormulaIdMap } from './utils/formula-data-util'; - import { Disposable, Inject, isFormulaId, isFormulaString, IUniverInstanceService, ObjectMatrix, RANGE_TYPE, UniverInstanceType } from '@univerjs/core'; import { LexerTreeBuilder } from '../engine/analysis/lexer-tree-builder'; import { clearArrayFormulaCellDataByCell, updateFormulaDataByCellValue } from './utils/formula-data-util'; @@ -38,7 +38,7 @@ export interface IRangeChange { } export class FormulaDataModel extends Disposable { - private _formulaData: IFormulaData = {}; + private _formulaIdMapData: IFormulaIdMapData = {}; private _arrayFormulaRange: IArrayFormulaRangeType = {}; @@ -49,15 +49,14 @@ export class FormulaDataModel extends Disposable { @Inject(LexerTreeBuilder) private readonly _lexerTreeBuilder: LexerTreeBuilder ) { super(); - - this.initFormulaData(); + this._initFormulaIdMap(); } override dispose() { super.dispose(); - this._formulaData = {}; this._arrayFormulaRange = {}; this._arrayFormulaCellData = {}; + this._formulaIdMapData = {}; } clearPreviousArrayFormulaCellData(clearArrayFormulaCellData: IRuntimeUnitDataType) { @@ -157,12 +156,48 @@ export class FormulaDataModel extends Disposable { }); } - getFormulaData() { - return this._formulaData; + getFormulaData(): IFormulaData { + const formulaData: IFormulaData = {}; + const allSheets = this._univerInstanceService.getAllUnitsForType(UniverInstanceType.UNIVER_SHEET); + if (allSheets.length === 0) { + return formulaData; + } + + allSheets.forEach((workbook) => { + const unitId = workbook.getUnitId(); + formulaData[unitId] = {}; + + const worksheets = workbook.getSheets(); + worksheets.forEach((worksheet) => { + const cellMatrix = worksheet.getCellMatrix(); + const sheetId = worksheet.getSheetId(); + + initSheetFormulaData(formulaData, unitId, sheetId, cellMatrix); + }); + }); + + return formulaData; } - setFormulaData(value: IFormulaData) { - this._formulaData = value; + getSheetFormulaData(unitId: string, sheetId: string) { + const formulaData: IFormulaData = {}; + const workbook = this._univerInstanceService.getUnit(unitId); + if (workbook == null) { + return {}; + } + + formulaData[unitId] = {}; + + const worksheet = workbook.getSheetBySheetId(sheetId); + if (worksheet == null) { + return {}; + } + + const cellMatrix = worksheet.getCellMatrix(); + + initSheetFormulaData(formulaData, unitId, sheetId, cellMatrix); + + return formulaData[unitId][sheetId]; } getArrayFormulaRange(): IArrayFormulaRangeType { @@ -208,53 +243,6 @@ export class FormulaDataModel extends Disposable { }); } - mergeFormulaData(formulaData: IFormulaData) { - Object.keys(formulaData).forEach((unitId) => { - const sheetData = formulaData[unitId]; - - if (sheetData === undefined) { - return; - } - - if (sheetData === null) { - delete this._formulaData[unitId]; - return; - } - - if (!this._formulaData[unitId]) { - this._formulaData[unitId] = {}; - } - - Object.keys(sheetData).forEach((sheetId) => { - const currentSheetData = sheetData[sheetId]; - - if (currentSheetData === undefined) { - return; - } - - if (currentSheetData === null) { - delete this._formulaData[unitId]?.[sheetId]; - return; - } - - if (!this._formulaData[unitId]?.[sheetId]) { - this._formulaData[unitId]![sheetId] = {}; - } - - const sheetFormula = new ObjectMatrix(currentSheetData); - const formulaMatrix = new ObjectMatrix(this._formulaData[unitId]![sheetId]); - - sheetFormula.forValue((r, c, v) => { - if (v == null) { - formulaMatrix.realDeleteValue(r, c); - } else { - formulaMatrix.setValue(r, c, v); - } - }); - }); - }); - } - deleteArrayFormulaRange(unitId: string, sheetId: string, row: number, column: number) { const cellMatrixData = this._arrayFormulaRange[unitId]?.[sheetId]; if (cellMatrixData == null) { @@ -270,32 +258,6 @@ export class FormulaDataModel extends Disposable { } } - /** - * Cache all formulas on the snapshot to the formula model - * @returns - */ - initFormulaData() { - // TODO@Dushusir: load doc/slide formula data - // Load formula data from workbook config data. - const allSheets = this._univerInstanceService.getAllUnitsForType(UniverInstanceType.UNIVER_SHEET); - if (allSheets.length === 0) { - return; - } - - // Since there is at least a sheet, there must be current univer sheet instance. - const workbook = this._univerInstanceService.getCurrentUnitForType(UniverInstanceType.UNIVER_SHEET)!; - const unitId = workbook.getUnitId(); - this._formulaData[unitId] = {}; - - const worksheets = workbook.getSheets(); - worksheets.forEach((worksheet) => { - const cellMatrix = worksheet.getCellMatrix(); - const sheetId = worksheet.getSheetId(); - - initSheetFormulaData(this._formulaData, unitId, sheetId, cellMatrix); - }); - } - getCalculateData() { const unitAllSheet = this._univerInstanceService.getAllUnitsForType(UniverInstanceType.UNIVER_SHEET); @@ -342,10 +304,19 @@ export class FormulaDataModel extends Disposable { updateFormulaData(unitId: string, sheetId: string, cellValue: IObjectMatrixPrimitiveType>) { const cellMatrix = new ObjectMatrix(cellValue); - const formulaIdMap = this.getFormulaIdMap(unitId, sheetId); // Connect the formula and ID + if (this._formulaIdMapData[unitId] == null) { + this._formulaIdMapData[unitId] = {}; + } + + if (this._formulaIdMapData[unitId][sheetId] == null) { + this._formulaIdMapData[unitId][sheetId] = {}; + } + + const formulaIdMap = this._formulaIdMapData[unitId][sheetId]; // Connect the formula and ID + const deleteFormulaIdMap = new Map(); - const formulaData = this._formulaData; + const formulaData = this.getFormulaData(); if (formulaData[unitId] == null) { formulaData[unitId] = {}; } @@ -368,7 +339,7 @@ export class FormulaDataModel extends Disposable { const formulaId = cell?.si || ''; if (isFormulaId(formulaId)) { - const formulaInfo = formulaIdMap.get(formulaId); + const formulaInfo = formulaIdMap?.[formulaId]; const deleteFormula = deleteFormulaIdMap.get(formulaId); if (formulaInfo && !isFormulaString(formulaString)) { @@ -383,11 +354,7 @@ export class FormulaDataModel extends Disposable { const y = cell?.y || 0; const offsetFormula = this._lexerTreeBuilder.moveFormulaRefOffset(deleteFormula, x, y); - deleteFormulaIdMap.set(formulaId, { - r, - c, - f: offsetFormula, - }); + deleteFormulaIdMap.set(formulaId, { r, c, f: offsetFormula }); sheetFormulaDataMatrix.setValue(r, c, { f: offsetFormula, si: formulaId }); newSheetFormulaDataMatrix.setValue(r, c, { f: offsetFormula, si: formulaId }); @@ -395,18 +362,8 @@ export class FormulaDataModel extends Disposable { const x = c - deleteFormula.c; const y = r - deleteFormula.r; - sheetFormulaDataMatrix.setValue(r, c, { - f: deleteFormula.f, - si: formulaId, - x, - y, - }); - newSheetFormulaDataMatrix.setValue(r, c, { - f: deleteFormula.f, - si: formulaId, - x, - y, - }); + sheetFormulaDataMatrix.setValue(r, c, { f: deleteFormula.f, si: formulaId, x, y }); + newSheetFormulaDataMatrix.setValue(r, c, { f: deleteFormula.f, si: formulaId, x, y }); } } }); @@ -459,108 +416,54 @@ export class FormulaDataModel extends Disposable { }); } - getFormulaItemBySId(sId: string, sheetId: string, unitId: string): Nullable { - const formulaData = this._formulaData; - if (formulaData[unitId] == null) { + getFormulaStringByCell(row: number, column: number, sheetId: string, unitId: string) { + const workbook = this._univerInstanceService.getUnit(unitId); + if (workbook == null) { return null; } - const workbookFormulaData = formulaData[unitId]; - if (workbookFormulaData?.[sheetId] == null) { + const worksheet = workbook.getSheetBySheetId(sheetId); + if (worksheet == null) { return null; } - const cellMatrix = new ObjectMatrix(workbookFormulaData[sheetId] || {}); - - let formulaDataItem: Nullable = null; + const cellMatrix = worksheet.getCellMatrix(); - cellMatrix.forValue((row, column, item) => { - if (item == null) { - return true; - } - const { f, si, x = 0, y = 0 } = item; + const cell = cellMatrix.getValue(row, column); - if (si === sId && f.length > 0 && x === 0 && y === 0) { - formulaDataItem = item; - return false; - } - }); - - return formulaDataItem; - } - - getFormulaDataItem(row: number, column: number, sheetId: string, unitId: string) { - return this._formulaData?.[unitId]?.[sheetId]?.[row]?.[column]; - } - - getFormulaIdMap(unitId: string, sheetId: string): Map { - const formulaIdMap = new Map(); // Connect the formula and ID - - const formulaData = this._formulaData; - if (formulaData[unitId] == null) { - return formulaIdMap; - } - const workbookFormulaData = formulaData[unitId]; - - if (workbookFormulaData?.[sheetId] == null) { - return formulaIdMap; + if (cell == null) { + return null; } - const sheetFormulaDataMatrix = new ObjectMatrix(workbookFormulaData[sheetId] || {}); + const { f, si } = cell; - sheetFormulaDataMatrix.forValue((r, c, cell) => { - const formulaString = cell?.f || ''; - const formulaId = cell?.si || ''; - const x = cell?.x || 0; - const y = cell?.y || 0; - - if (isFormulaString(formulaString) && isFormulaId(formulaId) && x === 0 && y === 0) { - formulaIdMap.set(formulaId, { f: formulaString, r, c }); - } - }); - - return formulaIdMap; - } - - getFormulaStringByCell(row: number, column: number, sheetId: string, unitId: string) { - const formulaDataItem = this.getFormulaDataItem(row, column, sheetId, unitId); - - if (formulaDataItem == null) { - return null; + if (isFormulaString(f)) { + return f; } - const { f, si, x = 0, y = 0 } = formulaDataItem; + if (isFormulaId(si)) { + let formulaString: Nullable = null; - // x and y support negative numbers. Negative numbers appear when the drop-down fill moves up or to the left. - if (si != null && (x !== 0 || y !== 0)) { - let formulaString = ''; - if (f.length > 0) { - formulaString = f; - } else { - const originItem = this.getFormulaItemBySId( - si, - sheetId, - unitId - ); - - if (originItem == null || originItem.f.length === 0) { - return null; + // Get the result in one traversal, pay attention to performance + cellMatrix.forValue((r, c, cell) => { + if (cell == null) { + return true; } - formulaString = originItem.f; - } + const { f, si: currentId } = cell; - formulaString = this._lexerTreeBuilder.moveFormulaRefOffset( - formulaString, - x, - y - ); + if (isFormulaString(f) && si === currentId) { + formulaString = this._lexerTreeBuilder.moveFormulaRefOffset( + f as string, + column - c, + row - r + ); - return formulaString; - } + return false; + } + }); - if (isFormulaString(f)) { - return f; + return formulaString; } return null; @@ -571,7 +474,7 @@ export class FormulaDataModel extends Disposable { * @returns */ getFormulaDirtyRanges(): IUnitRange[] { - const formulaData = this._formulaData; + const formulaData = this.getFormulaData(); const dirtyRanges: IUnitRange[] = []; @@ -647,6 +550,56 @@ export class FormulaDataModel extends Disposable { return dirtyRanges; } + + private _initFormulaIdMap() { + const allSheets = this._univerInstanceService.getAllUnitsForType(UniverInstanceType.UNIVER_SHEET); + if (allSheets.length === 0) { + return; + } + + allSheets.forEach((workbook) => { + const unitId = workbook.getUnitId(); + this._formulaIdMapData[unitId] = {}; + + const worksheets = workbook.getSheets(); + worksheets.forEach((worksheet) => { + const sheetId = worksheet.getSheetId(); + + if (this._formulaIdMapData[unitId]) { + this._formulaIdMapData[unitId][sheetId] = this._getSheetFormulaIdMap(unitId, sheetId); + } + }); + }); + } + + private _getSheetFormulaIdMap(unitId: string, sheetId: string) { + const formulaIdMap: Nullable<{ [formulaId: string]: IFormulaIdMap }> = {}; // Connect the formula and ID + + const workbook = this._univerInstanceService.getUnit(unitId); + if (workbook == null) { + return formulaIdMap; + } + + const worksheet = workbook.getSheetBySheetId(sheetId); + if (worksheet == null) { + return formulaIdMap; + } + + const cellMatrix = worksheet.getCellMatrix(); + cellMatrix.forValue((r, c, cell) => { + if (cell == null) { + return true; + } + + const { f, si } = cell; + + if (isFormulaString(f) && isFormulaId(si)) { + formulaIdMap[si as string] = { f: f as string, r, c }; + } + }); + + return formulaIdMap; + } } export function initSheetFormulaData( diff --git a/packages/engine-formula/src/models/utils/formula-data-util.ts b/packages/engine-formula/src/models/utils/formula-data-util.ts index 7173e7da42a7..8806bb8b8665 100644 --- a/packages/engine-formula/src/models/utils/formula-data-util.ts +++ b/packages/engine-formula/src/models/utils/formula-data-util.ts @@ -15,17 +15,11 @@ */ import type { ICellData, IRange, Nullable, ObjectMatrix } from '@univerjs/core'; -import type { IFormulaDataItem } from '../../basics/common'; +import type { IFormulaDataItem, IFormulaIdMap } from '../../basics/common'; import { cellToRange, isFormulaId, isFormulaString, Rectangle } from '@univerjs/core'; -export interface IFormulaIdMap { - f: string; - r: number; - c: number; -} - // eslint-disable-next-line complexity -export function updateFormulaDataByCellValue(sheetFormulaDataMatrix: ObjectMatrix>, newSheetFormulaDataMatrix: ObjectMatrix, formulaIdMap: Map, deleteFormulaIdMap: Map, r: number, c: number, cell: Nullable) { +export function updateFormulaDataByCellValue(sheetFormulaDataMatrix: ObjectMatrix>, newSheetFormulaDataMatrix: ObjectMatrix, formulaIdMap: { [formulaId: string]: IFormulaIdMap }, deleteFormulaIdMap: Map, r: number, c: number, cell: Nullable) { const formulaString = cell?.f || ''; const formulaId = cell?.si || ''; @@ -41,7 +35,7 @@ export function updateFormulaDataByCellValue(sheetFormulaDataMatrix: ObjectMatri // The id that needs to be offset // When the cell containing the formulas f and si is deleted, f and si lose their association, and f needs to be moved to the next cell containing the same si. if (isFormulaString(f) && isFormulaId(si)) { - const updatedFormula = formulaIdMap.get(si)?.f; + const updatedFormula = formulaIdMap?.[si]?.f; // The formula may have been updated. For example, when you delete a column referenced by a formula, it will become #REF and cannot take the original value. if (updatedFormula) { @@ -62,7 +56,7 @@ export function updateFormulaDataByCellValue(sheetFormulaDataMatrix: ObjectMatri si: formulaId, }); - formulaIdMap.set(formulaId, { f: formulaString, r, c }); + formulaIdMap[formulaId] = { f: formulaString, r, c }; newSheetFormulaDataMatrix.setValue(r, c, { f: formulaString, diff --git a/packages/facade/src/apis/sheets/__tests__/f-range.spec.ts b/packages/facade/src/apis/sheets/__tests__/f-range.spec.ts index 0147d62b4105..09b18862680e 100644 --- a/packages/facade/src/apis/sheets/__tests__/f-range.spec.ts +++ b/packages/facade/src/apis/sheets/__tests__/f-range.spec.ts @@ -18,7 +18,6 @@ import type { ICellData, Injector, IRange, IStyleData, Nullable } from '@univerjs/core'; import { DataValidationType, HorizontalAlign, ICommandService, IUniverInstanceService, VerticalAlign, WrapStrategy } from '@univerjs/core'; -import { FormulaDataModel } from '@univerjs/engine-formula'; import { AddWorksheetMergeCommand, SetHorizontalTextAlignCommand, SetRangeValuesCommand, SetRangeValuesMutation, SetStyleCommand, SetTextWrapCommand, SetVerticalTextAlignCommand } from '@univerjs/sheets'; import { AddSheetDataValidationCommand } from '@univerjs/sheets-data-validation'; import { ClearSheetsFilterCriteriaCommand, RemoveSheetFilterCommand, SetSheetFilterRangeCommand, SetSheetsFilterCriteriaCommand } from '@univerjs/sheets-filter'; @@ -313,9 +312,6 @@ describe('Test FRange', () => { }); it('Range getFormulas', () => { - const formulaDataModel = get(FormulaDataModel); - formulaDataModel.initFormulaData(); - const activeSheet = univerAPI.getActiveWorkbook()?.getActiveSheet(); const formulas = activeSheet?.getRange(0, 3, 5, 1)?.getFormulas(); expect(formulas).toStrictEqual([ diff --git a/packages/sheets-formula-ui/src/controllers/__tests__/create-command-test-bed.ts b/packages/sheets-formula-ui/src/controllers/__tests__/create-command-test-bed.ts index f465e7d85740..fec686f7f209 100644 --- a/packages/sheets-formula-ui/src/controllers/__tests__/create-command-test-bed.ts +++ b/packages/sheets-formula-ui/src/controllers/__tests__/create-command-test-bed.ts @@ -111,7 +111,6 @@ export function createCommandTestBed(workbookData?: IWorkbookData, dependencies? override onReady(): void { this._formulaDataModel = get(FormulaDataModel); - this._formulaDataModel.initFormulaData(); } } diff --git a/packages/sheets-formula-ui/src/controllers/formula-clipboard.controller.ts b/packages/sheets-formula-ui/src/controllers/formula-clipboard.controller.ts index 785c6fe3eaf3..438de8bf8d82 100644 --- a/packages/sheets-formula-ui/src/controllers/formula-clipboard.controller.ts +++ b/packages/sheets-formula-ui/src/controllers/formula-clipboard.controller.ts @@ -209,7 +209,7 @@ function getValueMatrixOfPasteFromIsNull( formulaDataModel: FormulaDataModel ): ObjectMatrix { const valueMatrix = new ObjectMatrix(); - const formulaData = formulaDataModel.getFormulaData()?.[unitId]?.[subUnitId]; + const formulaData = formulaDataModel.getSheetFormulaData(unitId, subUnitId); matrix.forValue((row, col, value) => { const toRow = range.rows[row]; @@ -248,7 +248,7 @@ function getSpecialPasteValueValueMatrix( ): ObjectMatrix { const valueMatrix = new ObjectMatrix(); const arrayFormulaCellData = formulaDataModel.getArrayFormulaCellData()?.[pasteFrom.unitId]?.[pasteFrom.subUnitId]; - const formulaData = formulaDataModel.getFormulaData()?.[unitId]?.[subUnitId]; + const formulaData = formulaDataModel.getSheetFormulaData(unitId, subUnitId); matrix.forValue((row, col, value) => { const fromRow = pasteFrom.range.rows[row % pasteFrom.range.rows.length]; @@ -393,7 +393,7 @@ function getDefaultPasteValueMatrix( ): ObjectMatrix { const valueMatrix = new ObjectMatrix(); const formulaIdMap = new Map(); - const formulaData = formulaDataModel.getFormulaData()?.[unitId]?.[subUnitId]; + const formulaData = formulaDataModel.getSheetFormulaData(unitId, subUnitId); matrix.forValue((row, col, value) => { const toRow = range.rows[row]; diff --git a/packages/sheets-formula-ui/src/controllers/formula-editor-show.controller.ts b/packages/sheets-formula-ui/src/controllers/formula-editor-show.controller.ts index de48b09e1e14..4a9dba533edf 100644 --- a/packages/sheets-formula-ui/src/controllers/formula-editor-show.controller.ts +++ b/packages/sheets-formula-ui/src/controllers/formula-editor-show.controller.ts @@ -175,6 +175,8 @@ export class FormulaEditorShowController extends Disposable implements IRenderMo } private _displayArrayFormulaRangeShape(matrixRange: IObjectMatrixPrimitiveType, row: number, col: number, unitId: string, subUnitId: string, worksheet: Worksheet, cellInfo: Nullable): Nullable { + const sheetFormulaData = this._formulaDataModel.getSheetFormulaData(unitId, subUnitId); + new ObjectMatrix(matrixRange).forValue((rowIndex, columnIndex, range) => { if (range == null) { return true; @@ -191,12 +193,7 @@ export class FormulaEditorShowController extends Disposable implements IRenderMo return; } - const formulaDataItem = this._formulaDataModel.getFormulaDataItem( - rowIndex, - columnIndex, - subUnitId, - unitId - ); + const formulaDataItem = sheetFormulaData?.[rowIndex]?.[columnIndex]; if (formulaDataItem == null || formulaDataItem.f == null) { return true; diff --git a/packages/sheets-formula/src/controllers/update-formula.controller.ts b/packages/sheets-formula/src/controllers/update-formula.controller.ts index 1185b571192a..2efe35bca66a 100644 --- a/packages/sheets-formula/src/controllers/update-formula.controller.ts +++ b/packages/sheets-formula/src/controllers/update-formula.controller.ts @@ -249,7 +249,7 @@ export class UpdateFormulaController extends Disposable { } private _handleWorkbookAdded(unit: Workbook) { - const formulaData = this._formulaDataModel.getFormulaData(); + const formulaData: IFormulaData = {}; const unitId = unit.getUnitId(); const newFormulaData: IFormulaData = { [unitId]: {} }; diff --git a/packages/sheets-formula/src/services/__test__/util.ts b/packages/sheets-formula/src/services/__test__/util.ts index ec7e45612b87..d3dc2a0eef89 100644 --- a/packages/sheets-formula/src/services/__test__/util.ts +++ b/packages/sheets-formula/src/services/__test__/util.ts @@ -115,7 +115,6 @@ export function createCommandTestBed(workbookData?: IWorkbookData, dependencies? override onReady(): void { this._formulaDataModel = get(FormulaDataModel); - this._formulaDataModel.initFormulaData(); } } From deb5200f615929e0cc1a9efe4b2324ac6a331909 Mon Sep 17 00:00:00 2001 From: Dushusir <1414556676@qq.com> Date: Wed, 27 Nov 2024 15:19:02 +0800 Subject: [PATCH 2/3] fix(formula): remove formula id map --- .../src/models/formula-data.model.ts | 38 ++----------------- .../controllers/update-formula.controller.ts | 19 +++++++--- 2 files changed, 16 insertions(+), 41 deletions(-) diff --git a/packages/engine-formula/src/models/formula-data.model.ts b/packages/engine-formula/src/models/formula-data.model.ts index 4bb46ad6f064..efc32f682c18 100644 --- a/packages/engine-formula/src/models/formula-data.model.ts +++ b/packages/engine-formula/src/models/formula-data.model.ts @@ -21,7 +21,6 @@ import type { IFormulaData, IFormulaDataItem, IFormulaIdMap, - IFormulaIdMapData, IRuntimeUnitDataType, ISheetData, IUnitData, @@ -38,8 +37,6 @@ export interface IRangeChange { } export class FormulaDataModel extends Disposable { - private _formulaIdMapData: IFormulaIdMapData = {}; - private _arrayFormulaRange: IArrayFormulaRangeType = {}; private _arrayFormulaCellData: IArrayFormulaUnitCellType = {}; @@ -49,14 +46,12 @@ export class FormulaDataModel extends Disposable { @Inject(LexerTreeBuilder) private readonly _lexerTreeBuilder: LexerTreeBuilder ) { super(); - this._initFormulaIdMap(); } override dispose() { super.dispose(); this._arrayFormulaRange = {}; this._arrayFormulaCellData = {}; - this._formulaIdMapData = {}; } clearPreviousArrayFormulaCellData(clearArrayFormulaCellData: IRuntimeUnitDataType) { @@ -304,22 +299,16 @@ export class FormulaDataModel extends Disposable { updateFormulaData(unitId: string, sheetId: string, cellValue: IObjectMatrixPrimitiveType>) { const cellMatrix = new ObjectMatrix(cellValue); - if (this._formulaIdMapData[unitId] == null) { - this._formulaIdMapData[unitId] = {}; - } - - if (this._formulaIdMapData[unitId][sheetId] == null) { - this._formulaIdMapData[unitId][sheetId] = {}; - } - - const formulaIdMap = this._formulaIdMapData[unitId][sheetId]; // Connect the formula and ID + const formulaIdMap = this._getSheetFormulaIdMap(unitId, sheetId); // Connect the formula and ID const deleteFormulaIdMap = new Map(); const formulaData = this.getFormulaData(); + if (formulaData[unitId] == null) { formulaData[unitId] = {}; } + const workbookFormulaData = formulaData[unitId]!; if (workbookFormulaData[sheetId] == null) { @@ -551,27 +540,6 @@ export class FormulaDataModel extends Disposable { return dirtyRanges; } - private _initFormulaIdMap() { - const allSheets = this._univerInstanceService.getAllUnitsForType(UniverInstanceType.UNIVER_SHEET); - if (allSheets.length === 0) { - return; - } - - allSheets.forEach((workbook) => { - const unitId = workbook.getUnitId(); - this._formulaIdMapData[unitId] = {}; - - const worksheets = workbook.getSheets(); - worksheets.forEach((worksheet) => { - const sheetId = worksheet.getSheetId(); - - if (this._formulaIdMapData[unitId]) { - this._formulaIdMapData[unitId][sheetId] = this._getSheetFormulaIdMap(unitId, sheetId); - } - }); - }); - } - private _getSheetFormulaIdMap(unitId: string, sheetId: string) { const formulaIdMap: Nullable<{ [formulaId: string]: IFormulaIdMap }> = {}; // Connect the formula and ID diff --git a/packages/sheets-formula/src/controllers/update-formula.controller.ts b/packages/sheets-formula/src/controllers/update-formula.controller.ts index 2efe35bca66a..ad8e02c4886b 100644 --- a/packages/sheets-formula/src/controllers/update-formula.controller.ts +++ b/packages/sheets-formula/src/controllers/update-formula.controller.ts @@ -106,9 +106,21 @@ export class UpdateFormulaController extends Disposable { })); this.disposeWithMe( - this._commandService.onCommandExecuted((command: ICommandInfo, options?: IExecutionOptions) => { + this._commandService.onCommandExecuted((command: ICommandInfo) => { if (!command.params) return; + if (command.id === RemoveSheetMutation.id) { + const { subUnitId: sheetId, unitId } = command.params as IRemoveSheetMutationParams; + this._handleWorkbookDisposed(unitId, sheetId); + } else if (command.id === InsertSheetMutation.id) { + this._handleInsertSheetMutation(command.params as IInsertSheetMutationParams); + } + }) + ); + + // Make sure to get the complete formula history data before updating, which contains the complete mapping of f and si + this.disposeWithMe( + this._commandService.beforeCommandExecuted((command: ICommandInfo, options?: IExecutionOptions) => { if (command.id === SetRangeValuesMutation.id) { const params = command.params as ISetRangeValuesMutationParams; @@ -121,11 +133,6 @@ export class UpdateFormulaController extends Disposable { return; } this._handleSetRangeValuesMutation(params as ISetRangeValuesMutationParams); - } else if (command.id === RemoveSheetMutation.id) { - const { subUnitId: sheetId, unitId } = command.params as IRemoveSheetMutationParams; - this._handleWorkbookDisposed(unitId, sheetId); - } else if (command.id === InsertSheetMutation.id) { - this._handleInsertSheetMutation(command.params as IInsertSheetMutationParams); } }) ); From 918e35b264286372f1634fd05d3e0ff474dac676 Mon Sep 17 00:00:00 2001 From: Dushusir <1414556676@qq.com> Date: Fri, 29 Nov 2024 17:10:30 +0800 Subject: [PATCH 3/3] fix(data-validation): fix workbook not found --- .../src/services/dv-validator-service.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/sheets-data-validation/src/services/dv-validator-service.ts b/packages/sheets-data-validation/src/services/dv-validator-service.ts index 7e815ee7dd78..43fbe55a91cc 100644 --- a/packages/sheets-data-validation/src/services/dv-validator-service.ts +++ b/packages/sheets-data-validation/src/services/dv-validator-service.ts @@ -108,6 +108,10 @@ export class SheetsDataValidationValidatorService extends Disposable { } validatorRanges(unitId: string, subUnitId: string, ranges: IRange[]) { + if (!ranges.length) { + return Promise.resolve([]); + } + const workbook = this._univerInstanceService.getUnit(unitId, UniverInstanceType.UNIVER_SHEET); if (!workbook) { throw new Error(`cannot find current workbook, unitId: ${unitId}`);