[FIX] model: prevent UI plugins to refuse core commands

Currently it's possible for UI plugins to handle and refuse core commands
in the allowDispatch. This is a problem because this could lead to
de-synchronized states in multi-user.

This commit fixes this by making sure that the core commands don't go
through the allowDispatch of the UI plugins.

Odoo task 3209463

closes #2120

Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>

src/model.ts

@@ -46,6 +46,7 @@ import {
   GridRenderingContext,
   isCoreCommand,
   LAYERS,
+  LocalCommand,
   UID,
 } from "./types/index";
 import { NotifyUIEvent } from "./types/ui";
@@ -267,7 +268,11 @@ export class Model extends EventBus<any> implements CommandDispatcher {
   }
 
   get handlers(): CommandHandler<Command>[] {
-    return [this.range, ...this.corePlugins, ...this.allUIPlugins, this.history];
+    return [...this.coreHandlers, ...this.allUIPlugins, this.history];
+  }
+
+  get coreHandlers(): CommandHandler<Command>[] {
+    return [this.range, ...this.corePlugins];
   }
 
   get allUIPlugins(): UIPlugin[] {
@@ -419,6 +424,18 @@ export class Model extends EventBus<any> implements CommandDispatcher {
    * Check if the given command is allowed by all the plugins and the history.
    */
   private checkDispatchAllowed(command: Command): DispatchResult {
+    if (isCoreCommand(command)) {
+      return this.checkDispatchAllowedCoreCommand(command);
+    }
+    return this.checkDispatchAllowedLocalCommand(command);
+  }
+
+  private checkDispatchAllowedCoreCommand(command: CoreCommand): DispatchResult {
+    const results = this.coreHandlers.map((handler) => handler.allowDispatch(command));
+    return new DispatchResult(results.flat());
+  }
+
+  private checkDispatchAllowedLocalCommand(command: LocalCommand): DispatchResult {
     const results = this.handlers.map((handler) => handler.allowDispatch(command));
     return new DispatchResult(results.flat());
   }

src/plugins/ui_core_views/filter_evaluation.ts

@@ -14,7 +14,6 @@ import {
   Border,
   CellPosition,
   CellValueType,
-  Command,
   CommandResult,
   ExcelFilterData,
   ExcelWorkbookData,
@@ -24,7 +23,7 @@ import {
   Zone,
 } from "../../types";
 import { UIPlugin } from "../ui_plugin";
-import { UpdateFilterCommand } from "./../../types/commands";
+import { Command, LocalCommand, UpdateFilterCommand } from "./../../types/commands";
 
 export class FilterEvaluationPlugin extends UIPlugin {
   static getters = [
@@ -41,7 +40,7 @@ export class FilterEvaluationPlugin extends UIPlugin {
   hiddenRows: Set<number> = new Set();
   isEvaluationDirty = false;
 
-  allowDispatch(cmd: Command) {
+  allowDispatch(cmd: LocalCommand): CommandResult {
     switch (cmd.type) {
       case "UPDATE_FILTER":
         if (!this.getters.getFilterId(cmd)) {

src/plugins/ui_core_views/sheetview.ts

@@ -13,6 +13,7 @@ import {
   EdgeScrollInfo,
   Figure,
   HeaderIndex,
+  LocalCommand,
   Pixel,
   Position,
   Rect,
@@ -117,7 +118,7 @@ export class SheetViewPlugin extends UIPlugin {
   // Command Handling
   // ---------------------------------------------------------------------------
 
-  allowDispatch(cmd: Command) {
+  allowDispatch(cmd: LocalCommand): CommandResult | CommandResult[] {
     switch (cmd.type) {
       case "SET_VIEWPORT_OFFSET":
         return this.checkScrollingDirection(cmd);

src/plugins/ui_feature/autofill.ts

@@ -14,6 +14,7 @@ import {
   GridRenderingContext,
   HeaderIndex,
   LAYERS,
+  LocalCommand,
   Tooltip,
   Zone,
 } from "../../types/index";
@@ -94,7 +95,7 @@ export class AutofillPlugin extends UIPlugin {
   // Command Handling
   // ---------------------------------------------------------------------------
 
-  allowDispatch(cmd: Command): CommandResult {
+  allowDispatch(cmd: LocalCommand): CommandResult {
     switch (cmd.type) {
       case "AUTOFILL_SELECT":
         const sheetId = this.getters.getActiveSheetId();

src/plugins/ui_feature/cell_popovers.ts

@@ -1,6 +1,6 @@
 import { positionToZone } from "../../helpers";
 import { cellPopoverRegistry } from "../../registries/cell_popovers_registry";
-import { CellPosition, Command, CommandResult, Position, Rect } from "../../types";
+import { CellPosition, Command, CommandResult, LocalCommand, Position, Rect } from "../../types";
 import {
   CellPopoverType,
   ClosedCellPopover,
@@ -20,7 +20,7 @@ export class CellPopoverPlugin extends UIPlugin {
 
   private persistentPopover?: CellPosition & { type: CellPopoverType };
 
-  allowDispatch(cmd: Command) {
+  allowDispatch(cmd: LocalCommand): CommandResult {
     switch (cmd.type) {
       case "OPEN_CELL_POPOVER":
         try {

src/plugins/ui_feature/selection_input.ts

@@ -7,7 +7,7 @@ import {
 } from "../../helpers/index";
 import { StreamCallbacks } from "../../selection_stream/event_stream";
 import { SelectionEvent } from "../../types/event_stream";
-import { Command, CommandResult, Highlight, LAYERS, UID } from "../../types/index";
+import { Command, CommandResult, Highlight, LAYERS, LocalCommand, UID } from "../../types/index";
 import { UIPlugin, UIPluginConfig } from "../ui_plugin";
 
 export interface RangeInputValue {
@@ -50,7 +50,7 @@ export class SelectionInputPlugin extends UIPlugin implements StreamCallbacks<Se
   // Command Handling
   // ---------------------------------------------------------------------------
 
-  allowDispatch(cmd: Command): CommandResult {
+  allowDispatch(cmd: LocalCommand): CommandResult {
     switch (cmd.type) {
       case "ADD_EMPTY_RANGE":
         if (this.inputHasSingleRange && this.ranges.length === 1) {

src/plugins/ui_feature/selection_inputs_manager.ts

@@ -1,5 +1,5 @@
 import { positionToZone, rangeReference, splitReference } from "../../helpers/index";
-import { Command, CommandResult, Highlight, LAYERS, UID } from "../../types/index";
+import { Command, CommandResult, Highlight, LAYERS, LocalCommand, UID } from "../../types/index";
 import { UIPlugin, UIPluginConfig } from "../ui_plugin";
 import { RangeInputValue, SelectionInputPlugin } from "./selection_input";
 
@@ -33,7 +33,7 @@ export class SelectionInputsManagerPlugin extends UIPlugin {
   // Command Handling
   // ---------------------------------------------------------------------------
 
-  allowDispatch(cmd: Command): CommandResult {
+  allowDispatch(cmd: LocalCommand): CommandResult {
     switch (cmd.type) {
       case "FOCUS_RANGE":
         const index = this.currentInput?.getIndex(cmd.rangeId);

src/plugins/ui_feature/sort.ts

@@ -8,6 +8,7 @@ import {
   CommandResult,
   EvaluatedCell,
   HeaderIndex,
+  LocalCommand,
   Position,
   SortCommand,
   SortDirection,
@@ -21,7 +22,7 @@ import { UIPlugin } from "../ui_plugin";
 export class SortPlugin extends UIPlugin {
   static getters = ["getContiguousZone"] as const;
 
-  allowDispatch(cmd: Command) {
+  allowDispatch(cmd: LocalCommand): CommandResult | CommandResult[] {
     switch (cmd.type) {
       case "SORT_CELLS":
         if (!isInside(cmd.col, cmd.row, cmd.zone)) {

src/plugins/ui_feature/ui_sheet.ts

@@ -4,7 +4,7 @@ import {
   PADDING_AUTORESIZE_HORIZONTAL,
 } from "../../constants";
 import { computeIconWidth, computeTextWidth, positions } from "../../helpers/index";
-import { Command, CommandResult, UID } from "../../types";
+import { Command, CommandResult, LocalCommand, UID } from "../../types";
 import {
   CellPosition,
   Dimension,
@@ -32,7 +32,7 @@ export class SheetUIPlugin extends UIPlugin {
   // Command Handling
   // ---------------------------------------------------------------------------
 
-  allowDispatch(cmd: Command): CommandResult {
+  allowDispatch(cmd: LocalCommand): CommandResult {
     switch (cmd.type) {
       case "AUTORESIZE_ROWS":
       case "AUTORESIZE_COLUMNS":

src/plugins/ui_stateful/clipboard.ts

@@ -15,6 +15,7 @@ import {
   GridRenderingContext,
   isCoreCommand,
   LAYERS,
+  LocalCommand,
   Zone,
 } from "../../types/index";
 import { UIPlugin } from "../ui_plugin";
@@ -47,7 +48,7 @@ export class ClipboardPlugin extends UIPlugin {
   // Command Handling
   // ---------------------------------------------------------------------------
 
-  allowDispatch(cmd: Command): CommandResult {
+  allowDispatch(cmd: LocalCommand): CommandResult {
     switch (cmd.type) {
       case "CUT":
         const zones = cmd.target || this.getters.getSelectedZones();

src/plugins/ui_stateful/edition.ts

@@ -18,23 +18,22 @@ import {
 import { loopThroughReferenceType } from "../../helpers/reference_type";
 import { _lt } from "../../translation";
 import {
+  AddColumnsRowsCommand,
   CellPosition,
   CellValueType,
+  Command,
+  CommandResult,
   Format,
+  HeaderIndex,
   Highlight,
+  LocalCommand,
   Range,
   RangePart,
+  RemoveColumnsRowsCommand,
   UID,
   Zone,
 } from "../../types";
 import { SelectionEvent } from "../../types/event_stream";
-import {
-  AddColumnsRowsCommand,
-  Command,
-  CommandResult,
-  HeaderIndex,
-  RemoveColumnsRowsCommand,
-} from "../../types/index";
 import { UIPlugin } from "../ui_plugin";
 
 type EditionMode =
@@ -80,7 +79,7 @@ export class EditionPlugin extends UIPlugin {
   // Command Handling
   // ---------------------------------------------------------------------------
 
-  allowDispatch(cmd: Command): CommandResult {
+  allowDispatch(cmd: LocalCommand): CommandResult {
     switch (cmd.type) {
       case "CHANGE_COMPOSER_CURSOR_SELECTION":
         return this.validateSelection(this.currentContent.length, cmd.start, cmd.end);

src/plugins/ui_stateful/selection.ts

@@ -27,6 +27,7 @@ import {
   GridRenderingContext,
   HeaderIndex,
   LAYERS,
+  LocalCommand,
   MoveColumnsRowsCommand,
   RemoveColumnsRowsCommand,
   Selection,
@@ -132,7 +133,7 @@ export class GridSelectionPlugin extends UIPlugin {
   // Command Handling
   // ---------------------------------------------------------------------------
 
-  allowDispatch(cmd: Command): CommandResult {
+  allowDispatch(cmd: LocalCommand): CommandResult {
     switch (cmd.type) {
       case "ACTIVATE_SHEET":
         try {

tests/collaborative/collaborative.test.ts

@@ -1,8 +1,9 @@
-import { Model } from "../../src";
+import { Model, UIPlugin } from "../../src";
 import { DEFAULT_REVISION_ID, MESSAGE_VERSION } from "../../src/constants";
 import { functionRegistry } from "../../src/functions";
 import { getDefaultCellHeight, range, toCartesian, toZone } from "../../src/helpers";
-import { CommandResult, CoreCommand } from "../../src/types";
+import { featurePluginRegistry } from "../../src/plugins";
+import { Command, CommandResult, CoreCommand } from "../../src/types";
 import { CollaborationMessage } from "../../src/types/collaborative/transport_service";
 import {
   activateSheet,
@@ -1010,3 +1011,22 @@ describe("Multi users synchronisation", () => {
     );
   });
 });
+
+test("UI plugins cannot refuse core command and de-synchronize the users", () => {
+  class MyUIPlugin extends UIPlugin {
+    allowDispatch(cmd: Command) {
+      if (cmd.type === "UPDATE_CELL") {
+        return this.getters.getClient().name === "Alice"
+          ? CommandResult.Success
+          : CommandResult.CancelledForUnknownReason;
+      }
+      return CommandResult.Success;
+    }
+  }
+  featurePluginRegistry.add("myUIPlugin", MyUIPlugin);
+  const { alice, bob } = setupCollaborativeEnv();
+
+  setCellContent(alice, "A1", "hello");
+  expect([alice, bob]).toHaveSynchronizedValue((user) => getCellContent(user, "A1"), "hello");
+  featurePluginRegistry.remove("myUIPlugin");
+});

tests/model.test.ts

@@ -94,6 +94,23 @@ describe("Model", () => {
     featurePluginRegistry.remove("myUIPlugin");
   });
 
+  test("UI plugins cannot refuse core commands", () => {
+    class MyUIPlugin extends UIPlugin {
+      allowDispatch(cmd: Command) {
+        if (cmd.type === "UPDATE_CELL") {
+          return CommandResult.CancelledForUnknownReason;
+        }
+        return CommandResult.Success;
+      }
+    }
+    featurePluginRegistry.add("myUIPlugin", MyUIPlugin);
+    const model = new Model();
+
+    setCellContent(model, "A1", "hello");
+    expect(getCellContent(model, "A1")).toBe("hello");
+    featurePluginRegistry.remove("myUIPlugin");
+  });
+
   test("Can open a model in readonly mode", () => {
     const model = new Model({}, { mode: "readonly" });
     expect(model.getters.isReadonly()).toBe(true);