Skip to content

Commit

Permalink
[FIX] UUID: Reduce uuid size everywhere except revisions
Browse files Browse the repository at this point in the history
Long uuids are not necessary for standard identifiers (like sheet,cf,
figure) as they are only meaningful when two users will try to create
them at the exact same time. A string with 8 random alphanumeric values
gives have 36^8 to 1 chance to collide, which is clearly enough.
The strong uuids are still necessary in the case of revisions as they
can come way more often.

On a spreadsheet 112mb of revisions, it reduces de size of all the
revisions by 15mb, -> 13.4% size gained.

closes #5691

Task: 4532659
X-original-commit: e1fc375
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
Signed-off-by: Rémi Rahir (rar) <[email protected]>
Co-authored-by: Vincent Schippefilt <[email protected]>
  • Loading branch information
rrahir and VincentSchippefilt committed Feb 12, 2025
1 parent 58e1e90 commit 9f5457b
Show file tree
Hide file tree
Showing 17 changed files with 65 additions and 28 deletions.
6 changes: 3 additions & 3 deletions src/actions/insert_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export const insertCheckbox: ActionSpec = {
ranges,
sheetId,
rule: {
id: env.model.uuidGenerator.uuidv4(),
id: env.model.uuidGenerator.smallUuid(),
criterion: {
type: "isBoolean",
values: [],
Expand All @@ -295,7 +295,7 @@ export const insertDropdown: ActionSpec = {
const zones = env.model.getters.getSelectedZones();
const sheetId = env.model.getters.getActiveSheetId();
const ranges = zones.map((zone) => env.model.getters.getRangeDataFromZone(sheetId, zone));
const ruleID = env.model.uuidGenerator.uuidv4();
const ruleID = env.model.uuidGenerator.smallUuid();
env.model.dispatch("ADD_DATA_VALIDATION_RULE", {
ranges,
sheetId,
Expand Down Expand Up @@ -327,7 +327,7 @@ export const insertSheet: ActionSpec = {
execute: (env) => {
const activeSheetId = env.model.getters.getActiveSheetId();
const position = env.model.getters.getSheetIds().indexOf(activeSheetId) + 1;
const sheetId = env.model.uuidGenerator.uuidv4();
const sheetId = env.model.uuidGenerator.smallUuid();
env.model.dispatch("CREATE_SHEET", { sheetId, position });
env.model.dispatch("ACTIVATE_SHEET", { sheetIdFrom: activeSheetId, sheetIdTo: sheetId });
},
Expand Down
8 changes: 4 additions & 4 deletions src/actions/menu_items_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ export const HIDE_ROWS_NAME = (env: SpreadsheetChildEnv) => {

export const CREATE_CHART = (env: SpreadsheetChildEnv) => {
const getters = env.model.getters;
const id = env.model.uuidGenerator.uuidv4();
const id = env.model.uuidGenerator.smallUuid();
const sheetId = getters.getActiveSheetId();

if (getZoneArea(env.model.getters.getSelectedZone()) === 1) {
Expand Down Expand Up @@ -409,8 +409,8 @@ export const CREATE_CHART = (env: SpreadsheetChildEnv) => {
//------------------------------------------------------------------------------

export const CREATE_PIVOT = (env: SpreadsheetChildEnv) => {
const pivotId = env.model.uuidGenerator.uuidv4();
const newSheetId = env.model.uuidGenerator.uuidv4();
const pivotId = env.model.uuidGenerator.smallUuid();
const newSheetId = env.model.uuidGenerator.smallUuid();
const result = env.model.dispatch("INSERT_NEW_PIVOT", { pivotId, newSheetId });
if (result.isSuccessful) {
env.openSidePanel("PivotSidePanel", { pivotId });
Expand Down Expand Up @@ -474,7 +474,7 @@ async function requestImage(env: SpreadsheetChildEnv): Promise<Image | undefined
export const CREATE_IMAGE = async (env: SpreadsheetChildEnv) => {
if (env.imageProvider) {
const sheetId = env.model.getters.getActiveSheetId();
const figureId = env.model.uuidGenerator.uuidv4();
const figureId = env.model.uuidGenerator.smallUuid();
const image = await requestImage(env);
if (!image) {
throw new Error("No image provider was given to the environment");
Expand Down
2 changes: 1 addition & 1 deletion src/actions/sheet_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const duplicateSheet: ActionSpec = {
name: _t("Duplicate"),
execute: (env) => {
const sheetIdFrom = env.model.getters.getActiveSheetId();
const sheetIdTo = env.model.uuidGenerator.uuidv4();
const sheetIdTo = env.model.uuidGenerator.smallUuid();
env.model.dispatch("DUPLICATE_SHEET", {
sheetId: sheetIdFrom,
sheetIdTo,
Expand Down
2 changes: 1 addition & 1 deletion src/clipboard_handlers/chart_clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class ChartClipboardHandler extends AbstractFigureClipboardHandler<Clipbo
content: ClipboardContent,
options?: ClipboardOptions
): ClipboardPasteTarget {
const newId = new UuidGenerator().uuidv4();
const newId = new UuidGenerator().smallUuid();
return { zones: [], figureId: newId, sheetId };
}

Expand Down
2 changes: 1 addition & 1 deletion src/clipboard_handlers/conditional_format_clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,6 @@ export class ConditionalFormatClipboardHandler extends AbstractCellClipboardHand
)?.cf;
}

return targetCF || { ...originCF, id: this.uuidGenerator.uuidv4(), ranges: [] };
return targetCF || { ...originCF, id: this.uuidGenerator.smallUuid(), ranges: [] };
}
}
2 changes: 1 addition & 1 deletion src/clipboard_handlers/data_validation_clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export class DataValidationClipboardHandler extends AbstractCellClipboardHandler
return (
targetRule || {
...originRule,
id: newId ? this.uuidGenerator.uuidv4() : originRule.id,
id: newId ? this.uuidGenerator.smallUuid() : originRule.id,
ranges: [],
}
);
Expand Down
2 changes: 1 addition & 1 deletion src/clipboard_handlers/image_clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class ImageClipboardHandler extends AbstractFigureClipboardHandler<Clipbo
content: ClipboardContent,
options?: ClipboardOptions
): ClipboardPasteTarget {
const newId = new UuidGenerator().uuidv4();
const newId = new UuidGenerator().smallUuid();
return { sheetId, zones: [], figureId: newId };
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/bottom_bar/bottom_bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export class BottomBar extends Component<Props, SpreadsheetChildEnv> {
const activeSheetId = this.env.model.getters.getActiveSheetId();
const position =
this.env.model.getters.getSheetIds().findIndex((sheetId) => sheetId === activeSheetId) + 1;
const sheetId = this.env.model.uuidGenerator.uuidv4();
const sheetId = this.env.model.uuidGenerator.smallUuid();
const name = this.env.model.getters.getNextSheetName(_t("Sheet"));
this.env.model.dispatch("CREATE_SHEET", { sheetId, position, name });
this.env.model.dispatch("ACTIVATE_SHEET", { sheetIdFrom: activeSheetId, sheetIdTo: sheetId });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export class ConditionalFormattingEditor extends Component<Props, SpreadsheetChi

setup() {
const cf = this.props.editedCf || {
id: this.env.model.uuidGenerator.uuidv4(),
id: this.env.model.uuidGenerator.smallUuid(),
ranges: this.env.model.getters
.getSelectedZones()
.map((zone) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export class DataValidationEditor extends Component<Props, SpreadsheetChildEnv>
.getSelectedZones()
.map((zone) => zoneToXc(this.env.model.getters.getUnboundedZone(sheetId, zone)));
return {
id: this.env.model.uuidGenerator.uuidv4(),
id: this.env.model.uuidGenerator.smallUuid(),
criterion: { type: "textContains", values: [""] },
ranges,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ export class PivotTitleSection extends Component<Props, SpreadsheetChildEnv> {
}

duplicatePivot() {
const newPivotId = this.env.model.uuidGenerator.uuidv4();
const newSheetId = this.env.model.uuidGenerator.uuidv4();
const newPivotId = this.env.model.uuidGenerator.smallUuid();
const newSheetId = this.env.model.uuidGenerator.smallUuid();
const result = this.env.model.dispatch("DUPLICATE_PIVOT_IN_NEW_SHEET", {
pivotId: this.props.pivotId,
newPivotId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export class TableStyleEditorPanel extends Component<
}

onConfirm() {
const tableStyleId = this.props.styleId || this.env.model.uuidGenerator.uuidv4();
const tableStyleId = this.props.styleId || this.env.model.uuidGenerator.smallUuid();
this.env.model.dispatch("CREATE_TABLE_STYLE", {
tableStyleId,
tableStyleName: this.state.styleName,
Expand Down
35 changes: 34 additions & 1 deletion src/helpers/uuid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,39 @@ export class UuidGenerator {
this.isFastIdStrategy = isFast;
}

/**
* Generates a custom UUID using a simple 36^12 method (8-character alphanumeric string with lowercase letters)
* This has a higher chance of collision than a UUIDv4, but not only faster to generate than an UUIDV4,
* it also has a smaller size, which is preferable to alleviate the overall data size.
*
* This method is preferable when generating uuids for the core data (sheetId, figureId, etc)
* as they will appear several times in the revisions and local history.
*
*/
smallUuid(): string {
if (this.isFastIdStrategy) {
this.fastIdStart++;
return String(this.fastIdStart);
//@ts-ignore
} else if (window.crypto && window.crypto.getRandomValues) {
//@ts-ignore
return ([1e7] + -1e3).replace(/[018]/g, (c) =>
(c ^ (crypto.getRandomValues(new Uint8Array(1))[0] & (15 >> (c / 4)))).toString(16)
);
} else {
// mainly for jest and other browsers that do not have the crypto functionality
return "xxxxxxxx-xxxx".replace(/[xy]/g, function (c) {
var r = (Math.random() * 16) | 0,
v = c == "x" ? r : (r & 0x3) | 0x8;
return v.toString(16);
});
}
}

/**
* Generates an UUIDV4, has astronomically low chance of collision, but is larger in size than the smallUuid.
* This method should be used when you need to avoid collisions at all costs, like the id of a revision.
*/
uuidv4(): string {
if (this.isFastIdStrategy) {
this.fastIdStart++;
Expand All @@ -25,7 +58,7 @@ export class UuidGenerator {
// mainly for jest and other browsers that do not have the crypto functionality
return "xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx".replace(/[xy]/g, function (c) {
var r = (Math.random() * 16) | 0,
v = c === "x" ? r : (r & 0x3) | 0x8;
v = c == "x" ? r : (r & 0x3) | 0x8;
return v.toString(16);
});
}
Expand Down
8 changes: 4 additions & 4 deletions src/history/repeat_commands/repeat_commands_specific.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export function repeatCreateChartCommand(
): CreateChartCommand {
return {
...repeatSheetDependantCommand(getters, cmd),
id: uuidGenerator.uuidv4(),
id: uuidGenerator.smallUuid(),
};
}

Expand All @@ -39,7 +39,7 @@ export function repeatCreateImageCommand(
): CreateImageOverCommand {
return {
...repeatSheetDependantCommand(getters, cmd),
figureId: uuidGenerator.uuidv4(),
figureId: uuidGenerator.smallUuid(),
};
}

Expand All @@ -48,7 +48,7 @@ export function repeatCreateFigureCommand(
cmd: CreateFigureCommand
): CreateFigureCommand {
const newCmd = repeatSheetDependantCommand(getters, cmd);
newCmd.figure.id = uuidGenerator.uuidv4();
newCmd.figure.id = uuidGenerator.smallUuid();
return newCmd;
}

Expand All @@ -57,7 +57,7 @@ export function repeatCreateSheetCommand(
cmd: CreateSheetCommand
): CreateSheetCommand {
const newCmd = deepCopy(cmd);
newCmd.sheetId = uuidGenerator.uuidv4();
newCmd.sheetId = uuidGenerator.smallUuid();

const sheetName = cmd.name || getters.getSheet(getters.getActiveSheetId()).name;
// Extract the prefix of the sheet name (everything before the number at the end of the name)
Expand Down
2 changes: 1 addition & 1 deletion src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {

private setupConfig(config: Partial<ModelConfig>): ModelConfig {
const client = config.client || {
id: this.uuidGenerator.uuidv4(),
id: this.uuidGenerator.smallUuid(),
name: _t("Anonymous").toString(),
};
const transportService = config.transportService || new LocalTransportService();
Expand Down
8 changes: 4 additions & 4 deletions src/plugins/core/tables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export class TablePlugin extends CorePlugin<TableState> implements TableState {
const mergesInTarget = this.getters.getMergesInZone(cmd.sheetId, union.zone);
this.dispatch("REMOVE_MERGE", { sheetId: cmd.sheetId, target: mergesInTarget });

const id = this.uuidGenerator.uuidv4();
const id = this.uuidGenerator.smallUuid();
const config = cmd.config || DEFAULT_TABLE_CONFIG;
const newTable =
cmd.tableType === "dynamic"
Expand Down Expand Up @@ -310,7 +310,7 @@ export class TablePlugin extends CorePlugin<TableState> implements TableState {
filters = [];
for (const i of range(zone.left, zone.right + 1)) {
const filterZone = { ...zone, left: i, right: i };
const uid = this.uuidGenerator.uuidv4();
const uid = this.uuidGenerator.smallUuid();
filters.push(this.createFilterFromZone(uid, tableRange.sheetId, filterZone, config));
}
}
Expand Down Expand Up @@ -391,7 +391,7 @@ export class TablePlugin extends CorePlugin<TableState> implements TableState {
? table.filters.find((f) => f.col === i)
: undefined;
const filterZone = { ...tableZone, left: i, right: i };
const filterId = oldFilter?.id || this.uuidGenerator.uuidv4();
const filterId = oldFilter?.id || this.uuidGenerator.smallUuid();
filters.push(this.createFilterFromZone(filterId, tableRange.sheetId, filterZone, config));
}
}
Expand Down Expand Up @@ -514,7 +514,7 @@ export class TablePlugin extends CorePlugin<TableState> implements TableState {
if (filters.length < zoneToDimension(tableZone).numberOfCols) {
for (let col = tableZone.left; col <= tableZone.right; col++) {
if (!filters.find((filter) => filter.col === col)) {
const uid = this.uuidGenerator.uuidv4();
const uid = this.uuidGenerator.smallUuid();
const filterZone = { ...tableZone, left: col, right: col };
filters.push(this.createFilterFromZone(uid, sheetId, filterZone, table.config));
}
Expand Down
4 changes: 4 additions & 0 deletions tests/__mocks__/uuid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ export class UuidGenerator {
return String(this.nextId++);
}

smallUuid(): string {
return String(this.nextId++);
}

setNextId(i: number) {
this.nextId = i;
}
Expand Down

0 comments on commit 9f5457b

Please sign in to comment.