Skip to content

Commit

Permalink
feat(formula): enable cancellation of ongoing formula calculations (#…
Browse files Browse the repository at this point in the history 
…2661)

Co-authored-by: 白熱 <sonne@asaki.me>
  • Loading branch information
@Dushusir @jikkai
Dushusir and jikkai authored Jul 9, 2024
1 parent 64264fc commit 5d80993
Show file tree
Hide file tree
Showing 8 changed files with 238 additions and 162 deletions.
25 changes: 14 additions & 11 deletions packages/engine-formula/src/engine/value-object/array-value-object.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ export class ArrayValueObject extends BaseValueObject {
return v;
}

getValueOrDefault(row: number, column: number) {
return this.get(row, column) || this._defaultValue;
}

set(row: number, column: number, value: Nullable<BaseValueObject>) {
if (row >= this._rowCount || column >= this._columnCount) {
throw new Error('Exceeding array bounds.');
Expand Down Expand Up @@ -1551,7 +1555,7 @@ export class ArrayValueObject extends BaseValueObject {
}

for (let r = 0; r < rowCount; r++) {
const currentValue = this._values?.[r]?.[column];
const currentValue = this.getValueOrDefault(r, column);
if (result[r] == null) {
result[r] = [];
}
Expand Down Expand Up @@ -1654,6 +1658,7 @@ export class ArrayValueObject extends BaseValueObject {
);
}

// eslint-disable-next-line max-lines-per-function, complexity
private _batchOperatorArray(
valueObject: BaseValueObject,
batchOperatorType: BatchOperatorType,
Expand All @@ -1673,8 +1678,6 @@ export class ArrayValueObject extends BaseValueObject {

const result: BaseValueObject[][] = [];

const valueObjectList = (valueObject as ArrayValueObject).getArrayValue();

const currentCalculateType = this._checkArrayCalculateType(this as ArrayValueObject);

const opCalculateType = this._checkArrayCalculateType(valueObject as ArrayValueObject);
Expand All @@ -1684,24 +1687,24 @@ export class ArrayValueObject extends BaseValueObject {
for (let c = 0; c < columnCount; c++) {
let currentValue: Nullable<BaseValueObject>;
if (currentCalculateType === ArrayCalculateType.SINGLE) {
currentValue = this._values?.[0]?.[0];
currentValue = this.getValueOrDefault(0, 0);
} else if (currentCalculateType === ArrayCalculateType.ROW) {
currentValue = this._values?.[0]?.[c];
currentValue = this.getValueOrDefault(0, c);
} else if (currentCalculateType === ArrayCalculateType.COLUMN) {
currentValue = this._values?.[r]?.[0];
currentValue = this.getValueOrDefault(r, 0);
} else {
currentValue = this._values?.[r]?.[c];
currentValue = this.getValueOrDefault(r, c);
}

let opValue: Nullable<BaseValueObject>;
if (opCalculateType === ArrayCalculateType.SINGLE) {
opValue = valueObjectList?.[0]?.[0];
opValue = (valueObject as ArrayValueObject).getValueOrDefault(0, 0);
} else if (opCalculateType === ArrayCalculateType.ROW) {
opValue = valueObjectList?.[0]?.[c];
opValue = (valueObject as ArrayValueObject).getValueOrDefault(0, c);
} else if (opCalculateType === ArrayCalculateType.COLUMN) {
opValue = valueObjectList?.[r]?.[0];
opValue = (valueObject as ArrayValueObject).getValueOrDefault(r, 0);
} else {
opValue = valueObjectList?.[r]?.[c];
opValue = (valueObject as ArrayValueObject).getValueOrDefault(r, c);
}

if (currentValue && opValue) {
Expand Down
144 changes: 144 additions & 0 deletions packages/sheets-formula/src/controllers/__tests__/update-formula.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,55 @@ const TEST_WORKBOOK_DATA_DEMO = (): IWorkbookData => ({
f: '=Sheet2!A1:B2',
},
},
18: {
0: {
v: 1,
},
1: {
f: '=SUM(A19)',
t: 2,
v: 1,
},
},
19: {
0: {
v: 2,
},
1: {
f: '=SUM(A20)',
si: 'id1',
t: 2,
v: 2,
},
},
20: {
0: {
v: 3,
},
1: {
si: 'id1',
t: 2,
v: 3,
},
},
21: {
2: {
f: '=OFFSET(A1,1,1)',
v: 0,
t: 2,
},
3: {
f: '=OFFSET(B1,1,1)',
si: 'id2',
v: 1,
t: 2,
},
4: {
si: 'id2',
v: 1,
t: 2,
},
},
},
name: 'Sheet1',
},
Expand Down Expand Up @@ -363,6 +412,37 @@ describe('Test insert function operation', () => {
expect(valuesRedo).toStrictEqual([[{}, { f: '=SUM(A1:B2)' }]]);
});

it('Move range, update reference with si ', async () => {
const params: IMoveRangeCommandParams = {
fromRange: {
startRow: 18,
startColumn: 1,
endRow: 20,
endColumn: 1,
rangeType: 0,
},
toRange: {
startRow: 18,
startColumn: 2,
endRow: 20,
endColumn: 2,
rangeType: 0,
},
};

expect(await commandService.executeCommand(MoveRangeCommand.id, params)).toBeTruthy();
const values = getValues(18, 1, 20, 2);
expect(values).toStrictEqual([[{}, { f: '=SUM(A19)', t: 2, v: 1 }], [{}, { f: '=SUM(A20)', si: 'id1', t: 2, v: 2 }], [{}, { si: 'id1', t: 2, v: 3 }]]);

expect(await commandService.executeCommand(UndoCommand.id)).toBeTruthy();
const valuesUndo = getValues(18, 1, 20, 2);
expect(valuesUndo).toStrictEqual([[{ f: '=SUM(A19)', t: 2, v: 1 }, {}], [{ f: '=SUM(A20)', si: 'id1', t: 2, v: 2 }, {}], [{ si: 'id1', t: 2, v: 3 }, {}]]);

expect(await commandService.executeCommand(RedoCommand.id)).toBeTruthy();
const valuesRedo = getValues(18, 1, 20, 2);
expect(valuesRedo).toStrictEqual([[{}, { f: '=SUM(A19)', t: 2, v: 1 }], [{}, { f: '=SUM(A20)', si: 'id1', t: 2, v: 2 }], [{}, { si: 'id1', t: 2, v: 3 }]]);
});

it('Move rows, update reference', async () => {
const selectionManager = get(SheetsSelectionsService);

Expand Down Expand Up @@ -848,6 +928,70 @@ describe('Test insert function operation', () => {
expect(valuesRedo3).toStrictEqual([[{ f: '=SUM(A8)' }, { f: '=SUM(#REF!)' }, { f: '=SUM(B8)' }, {}]]);
});

it('Remove column, update reference and position, contains #REF', async () => {
const params: IRemoveRowColCommandParams = {
range: {
startColumn: 0,
endColumn: 1,
startRow: 0,
endRow: 2,
},
};

expect(await commandService.executeCommand(RemoveColCommand.id, params)).toBeTruthy();
const values = getValues(21, 0, 21, 4);

// Ignore the calculation results and only verify the offset information
expect(values).toStrictEqual([[{
f: '=OFFSET(#REF!,1,1)',
v: 0,
t: 2,
}, {
f: '=OFFSET(#REF!,1,1)',
si: 'id2',
v: 1,
t: 2,
}, {
f: '=OFFSET(A1,1,1)',
v: 1,
t: 2,
}, {}, {}]]);

expect(await commandService.executeCommand(UndoCommand.id)).toBeTruthy();
const valuesUndo = getValues(21, 0, 21, 4);
expect(valuesUndo).toStrictEqual([[{ }, { }, {
f: '=OFFSET(A1,1,1)',
v: 0,
t: 2,
}, {
f: '=OFFSET(B1,1,1)',
si: 'id2',
v: 1,
t: 2,
}, {
si: 'id2',
v: 1,
t: 2,
}]]);

expect(await commandService.executeCommand(RedoCommand.id)).toBeTruthy();
const valuesRedo = getValues(21, 0, 21, 4);
expect(valuesRedo).toStrictEqual([[{
f: '=OFFSET(#REF!,1,1)',
v: 0,
t: 2,
}, {
f: '=OFFSET(#REF!,1,1)',
si: 'id2',
v: 1,
t: 2,
}, {
f: '=OFFSET(A1,1,1)',
v: 1,
t: 2,
}, {}, {}]]);
});

it('Remove column, removed column contains formula', async () => {
const params: IRemoveRowColCommandParams = {
range: {
Expand Down
13 changes: 13 additions & 0 deletions packages/sheets-formula/src/controllers/trigger-calculation.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ export class TriggerCalculationController extends Disposable {
this._initialExecuteFormulaProcessListener();

this._initialExecuteFormula();

this._initialProgressBar();
}

private _commandExecutedListener() {
Expand Down Expand Up @@ -448,6 +450,17 @@ export class TriggerCalculationController extends Disposable {
);
}

/**
* The user manually stops the progress bar
*/
private _initialProgressBar() {
this.disposeWithMe(this._progressService.progressVisible$.subscribe((isVisible) => {
if (!isVisible) {
this._commandService.executeCommand(SetFormulaCalculationStopMutation.id, {});
}
}));
}

/**
* Update progress by completed count
* @param completedCount
Expand Down
44 changes: 16 additions & 28 deletions packages/sheets-formula/src/controllers/update-formula.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ import { Inject, Injector } from '@wendellhu/redi';

import { filter, map, merge } from 'rxjs';
import { IEditorService } from '@univerjs/ui';
import type { IRefRangeWithPosition } from './utils/offset-formula-data';
import { removeFormulaData } from './utils/offset-formula-data';
import { getFormulaReferenceMoveUndoRedo } from './utils/ref-range-formula';

Expand Down Expand Up @@ -688,23 +687,21 @@ export class UpdateFormulaController extends Disposable {
};
}

// eslint-disable-next-line max-lines-per-function
private _getFormulaReferenceMoveInfo(
formulaData: IFormulaData,
unitSheetNameMap: IUnitSheetNameMap,
formulaReferenceMoveParam: IFormulaReferenceMoveParam
) {
if (!Tools.isDefine(formulaData)) return { newFormulaData: {}, oldFormulaData: {}, refRanges: [] };
if (!Tools.isDefine(formulaData)) return { newFormulaData: {}, oldFormulaData: {} };

const formulaDataKeys = Object.keys(formulaData);

if (formulaDataKeys.length === 0) return { newFormulaData: {}, oldFormulaData: {}, refRanges: [] };
if (formulaDataKeys.length === 0) return { newFormulaData: {}, oldFormulaData: {} };

const oldFormulaData: IFormulaData = {};
const newFormulaData: IFormulaData = {};

// Return all reference ranges. If the operation affects the reference range, you need to clear arrayFormulaRange and arrayFormulaCellData.
const refRanges: IRefRangeWithPosition[] = [];

for (const unitId of formulaDataKeys) {
const sheetData = formulaData[unitId];

Expand All @@ -725,9 +722,9 @@ export class UpdateFormulaController extends Disposable {
for (const sheetId of sheetDataKeys) {
const matrixData = new ObjectMatrix(sheetData[sheetId] || {});

const oldFormulaDataItem = new ObjectMatrix<IFormulaDataItem>();
const newFormulaDataItem = new ObjectMatrix<IFormulaDataItem>();

// eslint-disable-next-line max-lines-per-function, complexity
matrixData.forValue((row, column, formulaDataItem) => {
if (!formulaDataItem) return true;

Expand All @@ -752,9 +749,6 @@ export class UpdateFormulaController extends Disposable {

const { range, sheetName, unitId: sequenceUnitId } = sequenceGrid;

// store ref range
refRanges.push({ row, column, range });

const mapUnitId =
sequenceUnitId == null || sequenceUnitId.length === 0 ? unitId : sequenceUnitId;

Expand Down Expand Up @@ -849,32 +843,26 @@ export class UpdateFormulaController extends Disposable {

const newSequenceNodes = this._updateRefOffset(sequenceNodes, refChangeIds, x, y);

oldFormulaDataItem.setValue(row, column, {
f: formulaString,
x,
y,
si,
});

newFormulaDataItem.setValue(row, column, {
f: `=${generateStringWithSequence(newSequenceNodes)}`,
x,
y,
si,
});
// The formula f that originally depended on si has been actually calculated and does not require si and x,y.
if ((x !== undefined && (x > 0)) || (y !== undefined && (y > 0))) {
newFormulaDataItem.setValue(row, column, {
f: `=${generateStringWithSequence(newSequenceNodes)}`,
});
} else {
newFormulaDataItem.setValue(row, column, {
f: `=${generateStringWithSequence(newSequenceNodes)}`,
si,
});
}
});

if (oldFormulaData[unitId]) {
oldFormulaData[unitId]![sheetId] = oldFormulaDataItem.getData();
}

if (newFormulaData[unitId]) {
newFormulaData[unitId]![sheetId] = newFormulaDataItem.getData();
}
}
}

return { newFormulaData, oldFormulaData, refRanges };
return { newFormulaData };
}

private _getNewRangeByMoveParam(
Expand Down
Loading

0 comments on commit 5d80993

Please sign in to comment.