Skip to content

Commit

Permalink
feat(data-validation): code optimize for data-validation (#2815)
Browse files Browse the repository at this point in the history
  • Loading branch information
@weird94
weird94 authored Jul 20, 2024
1 parent ea92192 commit ab242a3
Show file tree
Hide file tree
Showing 15 changed files with 89 additions and 83 deletions.
5 changes: 3 additions & 2 deletions packages/data-validation/src/commands/commands/data-validation.command.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export interface IAddDataValidationCommandParams extends ISheetCommandSharedPara
export const AddDataValidationCommand: ICommand<IAddDataValidationCommandParams> = {
type: CommandType.COMMAND,
id: 'data-validation.command.addRule',
async handler(accessor, params) {
async handler(accessor, params) {
const logService = accessor.get(ILogService);
logService.warn('[Deprecated] AddDataValidationCommand is deprecated, please use AddSheetDataValidationCommand in @univerjs/sheets-data-validation instead!');
if (!params) {
Expand Down Expand Up @@ -83,7 +83,7 @@ export interface IRemoveDataValidationCommandParams extends ISheetCommandSharedP

export const removeDataValidationUndoFactory = (accessor: Injector, redoParams: IRemoveDataValidationMutationParams) => {
const dataValidationModel = accessor.get(DataValidationModel);
const { unitId, subUnitId, ruleId } = redoParams;
const { unitId, subUnitId, ruleId, source } = redoParams;
if (Array.isArray(ruleId)) {
const rules = ruleId.map((id) => dataValidationModel.getRuleById(unitId, subUnitId, id)).filter(Boolean) as ISheetDataValidationRule[];
return [{
Expand All @@ -92,6 +92,7 @@ export const removeDataValidationUndoFactory = (accessor: Injector, redoParams:
unitId,
subUnitId,
rule: rules,
source,
} as IAddDataValidationMutationParams,
}];
}
Expand Down
18 changes: 11 additions & 7 deletions packages/data-validation/src/commands/mutations/data-validation.mutation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ import { CommandType } from '@univerjs/core';
import type { ICommand, IDataValidationRule } from '@univerjs/core';
import type { ISheetCommandSharedParams } from '@univerjs/sheets';
import type { IUpdateRulePayload } from '../../types/interfaces/i-update-rule-payload';
import type { DataValidationChangeSource } from '../../models/data-validation-model';
import { DataValidationModel } from '../../models/data-validation-model';

export interface IAddDataValidationMutationParams extends ISheetCommandSharedParams {
rule: IDataValidationRule | IDataValidationRule[];
index?: number;
source?: DataValidationChangeSource;
}

export const AddDataValidationMutation: ICommand<IAddDataValidationMutationParams> = {
Expand All @@ -32,16 +34,17 @@ export const AddDataValidationMutation: ICommand<IAddDataValidationMutationParam
if (!params) {
return false;
}
const { unitId, subUnitId, rule, index } = params;
const { unitId, subUnitId, rule, index, source = 'command' } = params;
const dataValidationModel = accessor.get(DataValidationModel);
dataValidationModel.addRule(unitId, subUnitId, rule, index);
dataValidationModel.addRule(unitId, subUnitId, rule, source, index);

return true;
},
};

export interface IRemoveDataValidationMutationParams extends ISheetCommandSharedParams {
ruleId: string | string[];
source?: DataValidationChangeSource;
}

export const RemoveDataValidationMutation: ICommand<IRemoveDataValidationMutationParams> = {
Expand All @@ -52,14 +55,14 @@ export const RemoveDataValidationMutation: ICommand<IRemoveDataValidationMutatio
return false;
}

const { unitId, subUnitId, ruleId } = params;
const { unitId, subUnitId, ruleId, source = 'command' } = params;
const dataValidationModel = accessor.get(DataValidationModel);
if (Array.isArray(ruleId)) {
ruleId.forEach((item) => {
dataValidationModel.removeRule(unitId, subUnitId, item);
dataValidationModel.removeRule(unitId, subUnitId, item, source);
});
} else {
dataValidationModel.removeRule(unitId, subUnitId, ruleId);
dataValidationModel.removeRule(unitId, subUnitId, ruleId, source);
}

return true;
Expand All @@ -69,6 +72,7 @@ export const RemoveDataValidationMutation: ICommand<IRemoveDataValidationMutatio
export interface IUpdateDataValidationMutationParams extends ISheetCommandSharedParams {
payload: IUpdateRulePayload;
ruleId: string;
source?: DataValidationChangeSource;
}

export const UpdateDataValidationMutation: ICommand<IUpdateDataValidationMutationParams> = {
Expand All @@ -79,9 +83,9 @@ export const UpdateDataValidationMutation: ICommand<IUpdateDataValidationMutatio
return false;
}

const { unitId, subUnitId, ruleId, payload } = params;
const { unitId, subUnitId, ruleId, payload, source = 'command' } = params;
const dataValidationModel = accessor.get(DataValidationModel);
dataValidationModel.updateRule(unitId, subUnitId, ruleId, payload);
dataValidationModel.updateRule(unitId, subUnitId, ruleId, payload, source);
return true;
},
};
2 changes: 1 addition & 1 deletion packages/data-validation/src/controllers/dv-resource.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class DataValidationResourceController extends Disposable {
Object.keys(value).forEach((subunitId) => {
const ruleList = value[subunitId];
ruleList.forEach((rule) => {
this._dataValidationModel.addRule(unitID, subunitId, rule);
this._dataValidationModel.addRule(unitID, subunitId, rule, 'patched');
});
});
},
Expand Down
2 changes: 2 additions & 0 deletions packages/data-validation/src/controllers/dv-sheet.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ export class DataValidationSheetController extends Disposable {
unitId,
subUnitId,
ruleId: ids,
source: 'patched',
};
const undoParams: IAddDataValidationMutationParams = {
unitId,
subUnitId,
rule: rules,
source: 'patched',
};

return {
Expand Down
2 changes: 1 addition & 1 deletion packages/data-validation/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

export { UniverDataValidationPlugin } from './plugin';
export { DataValidatorRegistryService, DataValidatorRegistryScope } from './services/data-validator-registry.service';
export { DataValidationModel } from './models/data-validation-model';
export { DataValidationModel, type DataValidationChangeSource } from './models/data-validation-model';

export {
createDefaultNewRule,
Expand Down
20 changes: 4 additions & 16 deletions packages/data-validation/src/models/data-validation-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,29 +16,24 @@

import type { CellValue, IDataValidationRule, Nullable } from '@univerjs/core';
import { DataValidationStatus, Disposable } from '@univerjs/core';
import { Subject } from 'rxjs';
import { BehaviorSubject } from 'rxjs';
import type { IUpdateRulePayload } from '../types/interfaces/i-update-rule-payload';
import { UpdateRuleType } from '../types/enum/update-rule-type';
import { getRuleOptions, getRuleSetting } from '../common/util';

export class DataValidationManager<T extends IDataValidationRule> extends Disposable {
private _dataValidations: T[];
private _dataValidations: T[] = [];
private _dataValidationMap = new Map<string, T>();
private _dataValidations$ = new Subject<T[]>();
private _dataValidations$ = new BehaviorSubject<T[]>(this._dataValidations);

readonly unitId: string;
readonly subUnitId: string;
readonly dataValidations$ = this._dataValidations$.asObservable();

constructor(unitId: string, subUnitId: string, dataValidations: T[] | undefined) {
constructor(unitId: string, subUnitId: string) {
super();
this.unitId = unitId;
this.subUnitId = subUnitId;
if (!dataValidations) {
return;
}

this._insertRules(dataValidations);
this._notice();

this.disposeWithMe({
Expand All @@ -52,13 +47,6 @@ export class DataValidationManager<T extends IDataValidationRule> extends Dispos
this._dataValidations$.next(this._dataValidations);
}

private _insertRules(dataValidations: T[]) {
this._dataValidations = dataValidations;
dataValidations.forEach((validation) => {
this._dataValidationMap.set(validation.uid, validation);
});
}

getRuleById(id: string) {
return this._dataValidationMap.get(id);
}
Expand Down
22 changes: 14 additions & 8 deletions packages/data-validation/src/models/data-validation-model.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ import type { IUpdateRulePayload } from '../types/interfaces/i-update-rule-paylo
import { DataValidationManager } from './data-validation-manager';

type ManagerCreator<T extends IDataValidationRule> = (unitId: string, subUnitId: string) => DataValidationManager<T>;
type RuleChangeType = 'update' | 'add' | 'remove';
type DataValidationChangeType = 'update' | 'add' | 'remove';
export type DataValidationChangeSource = 'command' | 'patched';

export interface IRuleChange<T extends IDataValidationRule> {
rule?: T;
type: RuleChangeType;
type: DataValidationChangeType;
unitId: string;
subUnitId: string;
source: DataValidationChangeSource;
}

export interface IValidStatusChange {
Expand All @@ -38,7 +40,7 @@ export interface IValidStatusChange {

export class DataValidationModel<T extends IDataValidationRule = IDataValidationRule> extends Disposable {
private readonly _model = new Map<string, Map<string, DataValidationManager<T>>>();
private _managerCreator: ManagerCreator<T> = (unitId: string, subUnitId: string) => new DataValidationManager<T>(unitId, subUnitId, []);
private _managerCreator: ManagerCreator<T> = (unitId: string, subUnitId: string) => new DataValidationManager<T>(unitId, subUnitId);
private readonly _ruleChange$ = new Subject<IRuleChange<T>>();
private readonly _validStatusChange$ = new Subject<IValidStatusChange>();

Expand Down Expand Up @@ -75,10 +77,11 @@ export class DataValidationModel<T extends IDataValidationRule = IDataValidation

const manager = this._managerCreator(unitId, subUnitId);
unitMap.set(subUnitId, manager);
this.disposeWithMe(manager);
return manager;
}

private _addRuleSideEffect(unitId: string, subUnitId: string, rule: T) {
private _addRuleSideEffect(unitId: string, subUnitId: string, rule: T, source: DataValidationChangeSource) {
const manager = this.ensureManager(unitId, subUnitId);
const oldRule = manager.getRuleById(rule.uid);
if (oldRule) {
Expand All @@ -89,15 +92,16 @@ export class DataValidationModel<T extends IDataValidationRule = IDataValidation
type: 'add',
unitId,
subUnitId,
source,
});
}

addRule(unitId: string, subUnitId: string, rule: T | T[], index?: number) {
addRule(unitId: string, subUnitId: string, rule: T | T[], source: DataValidationChangeSource, index?: number) {
try {
const manager = this.ensureManager(unitId, subUnitId);
const rules = Array.isArray(rule) ? rule : [rule];
rules.forEach((item) => {
this._addRuleSideEffect(unitId, subUnitId, item);
this._addRuleSideEffect(unitId, subUnitId, item, source);
});

manager.addRule(rule, index);
Expand All @@ -106,7 +110,7 @@ export class DataValidationModel<T extends IDataValidationRule = IDataValidation
}
}

updateRule(unitId: string, subUnitId: string, ruleId: string, payload: IUpdateRulePayload) {
updateRule(unitId: string, subUnitId: string, ruleId: string, payload: IUpdateRulePayload, source: DataValidationChangeSource) {
try {
const manager = this.ensureManager(unitId, subUnitId);
const rule = manager.updateRule(ruleId, payload);
Expand All @@ -116,13 +120,14 @@ export class DataValidationModel<T extends IDataValidationRule = IDataValidation
type: 'update',
unitId,
subUnitId,
source,
});
} catch (error) {
this._logService.error(error);
}
}

removeRule(unitId: string, subUnitId: string, ruleId: string) {
removeRule(unitId: string, subUnitId: string, ruleId: string, source: DataValidationChangeSource) {
try {
const manager = this.ensureManager(unitId, subUnitId);
const oldRule = manager.getRuleById(ruleId);
Expand All @@ -133,6 +138,7 @@ export class DataValidationModel<T extends IDataValidationRule = IDataValidation
type: 'remove',
unitId,
subUnitId,
source,
});
}
} catch (error) {
Expand Down
11 changes: 9 additions & 2 deletions packages/sheets-data-validation/src/commands/commands/data-validation.command.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import { CommandType, DataValidationType, ICommandService, IUndoRedoService, IUniverInstanceService, ObjectMatrix, Range, sequenceExecute, sequenceExecuteAsync, Tools } from '@univerjs/core';
import type { CellValue, IAccessor, ICellData, ICommand, IDataValidationRuleBase, IDataValidationRuleOptions, IMutationInfo, IRange, ISheetDataValidationRule, Nullable } from '@univerjs/core';
import type { IAddDataValidationMutationParams, IUpdateDataValidationMutationParams } from '@univerjs/data-validation';
import type { DataValidationChangeSource, IAddDataValidationMutationParams, IUpdateDataValidationMutationParams } from '@univerjs/data-validation';
import { AddDataValidationMutation, createDefaultNewRule, DataValidationModel, DataValidatorRegistryService, getRuleOptions, getRuleSetting, RemoveDataValidationMutation, UpdateDataValidationMutation, UpdateRuleType } from '@univerjs/data-validation';
import type { ISetRangeValuesMutationParams, ISheetCommandSharedParams } from '@univerjs/sheets';
import { getSheetCommandTarget, SetRangeValuesMutation, SetRangeValuesUndoMutationFactory } from '@univerjs/sheets';
Expand Down Expand Up @@ -51,7 +51,8 @@ export function getDataValidationDiffMutations(
unitId: string,
subUnitId: string,
diffs: RangeMutation[],
accessor: IAccessor
accessor: IAccessor,
source: DataValidationChangeSource = 'command'
) {
const redoMutations: IMutationInfo[] = [];
const undoMutations: IMutationInfo[] = [];
Expand Down Expand Up @@ -93,6 +94,7 @@ export function getDataValidationDiffMutations(
unitId,
subUnitId,
ruleId: diff.rule.uid,
source,
},
});
undoMutations.unshift({
Expand All @@ -102,6 +104,7 @@ export function getDataValidationDiffMutations(
subUnitId,
rule: diff.rule,
index: diff.index,
source,
},
});
break;
Expand All @@ -116,6 +119,7 @@ export function getDataValidationDiffMutations(
type: UpdateRuleType.RANGE,
payload: diff.newRanges,
},
source,
} as IUpdateDataValidationMutationParams,
});
undoMutations.unshift({
Expand All @@ -128,6 +132,7 @@ export function getDataValidationDiffMutations(
type: UpdateRuleType.RANGE,
payload: diff.oldRanges,
},
source,
} as IUpdateDataValidationMutationParams,
});
const rule = manager.getRuleById(diff.ruleId);
Expand All @@ -145,6 +150,7 @@ export function getDataValidationDiffMutations(
unitId,
subUnitId,
rule: diff.rule,
source,
} as IAddDataValidationMutationParams,
});
undoMutations.unshift({
Expand All @@ -153,6 +159,7 @@ export function getDataValidationDiffMutations(
unitId,
subUnitId,
ruleId: diff.rule.uid,
source,
},
});
if (diff.rule.type === DataValidationType.CHECKBOX) {
Expand Down
2 changes: 1 addition & 1 deletion packages/sheets-data-validation/src/controllers/dv-auto-fill.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export class DataValidationAutoFillController extends Disposable {
});

const diffs = ruleMatrixCopy.diff(manager.getDataValidations());
const { redoMutations, undoMutations } = getDataValidationDiffMutations(unitId, subUnitId, diffs, this._injector);
const { redoMutations, undoMutations } = getDataValidationDiffMutations(unitId, subUnitId, diffs, this._injector, 'patched');
return {
undos: undoMutations,
redos: redoMutations,
Expand Down
6 changes: 4 additions & 2 deletions packages/sheets-data-validation/src/controllers/dv-copy-paste.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ export class DataValidationCopyPasteController extends Disposable {
copyInfo.unitId,
copyInfo.subUnitId,
ruleMatrix.diffWithAddition(manager.getDataValidations(), additionRules.values()),
this._injector
this._injector,
'patched'
);

return {
Expand Down Expand Up @@ -188,7 +189,8 @@ export class DataValidationCopyPasteController extends Disposable {
unitId,
subUnitId,
ruleMatrix.diff(manager.getDataValidations()),
this._injector
this._injector,
'patched'
);

return {
Expand Down
1 change: 0 additions & 1 deletion packages/sheets-data-validation/src/controllers/dv-model.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export class DataValidationModelController extends Disposable {
return new SheetDataValidationManager(
unitId,
subUnitId,
[],
this._injector
);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/sheets-data-validation/src/controllers/dv-ref-range.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ export class DataValidationRefRangeController extends Disposable {
type: UpdateRuleType.RANGE,
payload: resultRanges,
},
source: 'patched',
};
// in ref-range case, there won't be any overlap about rule ranges
const redos = [{ id: UpdateDataValidationMutation.id, params: redoParams }];
Expand All @@ -175,6 +176,7 @@ export class DataValidationRefRangeController extends Disposable {
type: UpdateRuleType.RANGE,
payload: oldRanges,
},
source: 'patched',
},
}];
return { redos, undos };
Expand Down
Loading

0 comments on commit ab242a3

Please sign in to comment.