Skip to content

Commit

Permalink
fix(MultiplyingArchitecture): Avoid setting stale edits
Browse files Browse the repository at this point in the history
Currently, an MA edit lifecycle looks like the following:

1. The Kaoto editor makes an edit and notifies the MA.
2. After a quiet period (debounce), MA process the edit and sends it
   back to the Kaoto editor.
3. If the content is the same, this last call is ignored by Kaoto, as
   the editor content is exactly the same, so there's no need to
recreate the UI. If the contrary, the current state gets discarded and
the UI is regenerated with the incoming content.

This process opens the case for an edit happening a few milliseconds
before the MA sending the content back, meaning that at this stage, the
Kaoto content has changed but is receiving a stale content, causing for
the UI to be regenerated with the stale content.

The fix for this situation is to implement a hash for each content and
validate whether the incoming content is stale or not.

fix: KaotoIO#1376
fix: https://issues.redhat.com/projects/KTO/issues/KTO-452
  • Loading branch information
lordrip committed Sep 10, 2024
1 parent 6916d00 commit e8354d0
Show file tree
Hide file tree
Showing 6 changed files with 381 additions and 27 deletions.
12 changes: 9 additions & 3 deletions packages/ui/jest-setup.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import '@testing-library/jest-dom';
import { FilterDOMPropsKeys, filterDOMProps } from 'uniforms';
import { setupJestCanvasMock } from 'jest-canvas-mock';
// import '@testing-library/jest-dom/extend-expect'
import { subtle } from 'node:crypto';
import { TextDecoder, TextEncoder } from 'node:util';
import { FilterDOMPropsKeys, filterDOMProps } from 'uniforms';

Object.defineProperties(global, {
TextDecoder: { value: TextDecoder },
TextEncoder: { value: TextEncoder },
});

filterDOMProps.register('inputRef' as FilterDOMPropsKeys, 'placeholder' as FilterDOMPropsKeys);
enableSVGElementMocks();
Expand All @@ -13,7 +19,7 @@ Object.defineProperty(window, 'fetch', {

jest
.spyOn(global, 'crypto', 'get')
.mockImplementation(() => ({ getRandomValues: () => [12345678] }) as unknown as Crypto);
.mockImplementation(() => ({ getRandomValues: () => [12345678], subtle }) as unknown as Crypto);

jest.spyOn(console, 'warn').mockImplementation((...args) => {
if (
Expand Down
81 changes: 81 additions & 0 deletions packages/ui/src/multiplying-architecture/EditService.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { EditService } from './EditService';

describe('EditService', () => {
const text = 'Hello, World!';
let service: EditService;

beforeEach(() => {
jest.resetModules();
service = EditService.getInstance();
service.clearEdits();
});

afterEach(() => {
service.clearEdits();
});

it('should return the same instance', () => {
const anotherEditService = EditService.getInstance();

expect(service).toBe(anotherEditService);
});

it('should not throw when registering an edit', async () => {
expect(async () => {
await service.registerEdit(text);
}).not.toThrow();
});

it('should allow consumers registering an edit', async () => {
const serviceTest = new EditServiceTest();
await serviceTest.registerEdit(text);

expect(serviceTest.getHashes()).toHaveLength(1);
});

describe('isStaleEdit', () => {
it('should return false if there is no other edit', async () => {
const isStale = await service.isStaleEdit(text);

expect(isStale).toBe(false);
});

it('should return false if the edit matches the last one', async () => {
await service.registerEdit(text);
const isStale = await service.isStaleEdit(text);

expect(isStale).toBe(false);
});

it('should return false if the content hash is not registered already', async () => {
await service.registerEdit(text);
const isStale = await service.isStaleEdit('Hello, World!');

expect(isStale).toBe(false);
});

it('should return true if the content hash is already registered but not the last one', async () => {
await service.registerEdit(text);
await service.registerEdit('This is a new edit');
const isStale = await service.isStaleEdit(text);

expect(isStale).toBe(true);
});
});

it('should clear all edits', async () => {
const serviceTest = new EditServiceTest();
await serviceTest.registerEdit(text);
await serviceTest.registerEdit(text);

serviceTest.clearEdits();

expect(serviceTest.getHashes()).toHaveLength(0);
});
});

class EditServiceTest extends EditService {
getHashes() {
return this.hashes;
}
}
37 changes: 37 additions & 0 deletions packages/ui/src/multiplying-architecture/EditService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
export class EditService {
protected static instance: EditService | undefined;
protected hashes: string[] = [];

static getInstance(): EditService {
if (!this.instance) {
this.instance = new EditService();
}

return this.instance;
}

async registerEdit(content: string) {
const hash = await this.hash(content);
this.hashes.push(hash);
}

async isStaleEdit(content: string) {
const hash = await this.hash(content);
const doesExists = this.hashes.includes(hash);
const isLastEdit = this.hashes[this.hashes.length - 1] === hash;

return doesExists && !isLastEdit;
}

clearEdits() {
this.hashes = [];
}

protected async hash(message: string) {
const msgUint8 = new TextEncoder().encode(message);
const hashBuffer = await window.crypto.subtle.digest('SHA-256', msgUint8);
const hashArray = Array.from(new Uint8Array(hashBuffer));

return hashArray.map((b) => b.toString(16).padStart(2, '0')).join('');
}
}
7 changes: 3 additions & 4 deletions packages/ui/src/multiplying-architecture/KaotoBridge.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { ChannelType, EditorApi, StateControlCommand } from '@kie-tools-core/editor/dist/api';
import { Notification } from '@kie-tools-core/notifications/dist/api';
import { WorkspaceEdit } from '@kie-tools-core/workspace/dist/api';
import { PropsWithChildren, forwardRef, useCallback, useContext, useEffect, useImperativeHandle, useRef } from 'react';
import { useReload } from '../hooks/reload.hook';
import { CatalogTilesProvider } from '../providers/catalog-tiles.provider';
Expand Down Expand Up @@ -30,7 +29,7 @@ interface KaotoBridgeProps {
* that a change has taken place.
* @param edit An object representing the unique change.
*/
onNewEdit: (edit: WorkspaceEdit) => void;
onNewEdit: (edit: string) => Promise<void>;

/**
* Delegation for NotificationsChannelApi.kogigotNotifications_setNotifications(path, notifications) to report all validation
Expand Down Expand Up @@ -89,14 +88,14 @@ export const KaotoBridge = forwardRef<EditorApi, PropsWithChildren<KaotoBridgePr
*/
useEffect(() => {
const unsubscribeFromEntities = eventNotifier.subscribe('entities:updated', (newContent: string) => {
props.onNewEdit(new WorkspaceEdit(newContent));
props.onNewEdit(newContent);
sourceCodeRef.current = newContent;
});

const unsubscribeFromSourceCode = eventNotifier.subscribe('code:updated', (newContent: string) => {
/** Ignore the first change, from an empty string to the file content */
if (sourceCodeRef.current !== '') {
props.onNewEdit(new WorkspaceEdit(newContent));
props.onNewEdit(newContent);
}
sourceCodeRef.current = newContent;
});
Expand Down
209 changes: 209 additions & 0 deletions packages/ui/src/multiplying-architecture/KaotoEditorApp.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
jest.mock('react-router-dom');
import {
ChannelType,
EditorApi,
EditorInitArgs,
EditorTheme,
KogitoEditorEnvelopeContextType,
StateControlCommand,
} from '@kie-tools-core/editor/dist/api';
import { I18nService } from '@kie-tools-core/i18n/dist/envelope/I18nService';
import { KeyboardShortcutsService } from '@kie-tools-core/keyboard-shortcuts/dist/envelope/KeyboardShortcutsService';
import { OperatingSystem } from '@kie-tools-core/operating-system/dist/OperatingSystem';
import { RefObject } from 'react';
import { AbstractSettingsAdapter, DefaultSettingsAdapter } from '../models/settings';
import { EditService } from './EditService';
import { KaotoEditorApp } from './KaotoEditorApp';
import { KaotoEditorChannelApi } from './KaotoEditorChannelApi';

describe('KaotoEditorApp', () => {
let kaotoEditorApp: KaotoEditorAppTest;
let editService: EditService;
let editorRef: RefObject<EditorApi>;
let envelopeContext: KogitoEditorEnvelopeContextType<KaotoEditorChannelApi>;
let initArgs: EditorInitArgs;
let settingsAdapter: AbstractSettingsAdapter;

beforeEach(() => {
jest.resetModules();
editService = EditService.getInstance();
editorRef = {
current: {
setContent: jest.fn(),
getContent: jest.fn(),
getPreview: jest.fn(),
undo: jest.fn(),
redo: jest.fn(),
setTheme: jest.fn(),
validate: jest.fn(),
},
};

envelopeContext = {
channelApi: {
notifications: {
kogitoEditor_ready: getNotificationMock(),
kogitoEditor_setContentError: getNotificationMock(),
kogitoEditor_stateControlCommandUpdate: getNotificationMock(),
kogitoNotifications_createNotification: getNotificationMock(),
kogitoNotifications_removeNotifications: getNotificationMock(),
kogitoNotifications_setNotifications: getNotificationMock(),
kogitoWorkspace_newEdit: getNotificationMock(),
kogitoWorkspace_openFile: getNotificationMock(),
},
// eslint-disable-next-line @typescript-eslint/no-explicit-any
requests: {} as any,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
shared: {} as any,
},
operatingSystem: OperatingSystem.LINUX,
services: {
keyboardShortcuts: {} as KeyboardShortcutsService,
i18n: {} as I18nService,
},
};

initArgs = {
resourcesPathPrefix: 'route.camel',
fileExtension: 'yaml',
initialLocale: 'en-us',
isReadOnly: false,
channel: ChannelType.VSCODE_DESKTOP,
};

settingsAdapter = new DefaultSettingsAdapter();

kaotoEditorApp = new KaotoEditorAppTest(envelopeContext, initArgs, settingsAdapter);
kaotoEditorApp.setEditorRef(editorRef);
});

afterEach(() => {
editService.clearEdits();
});

describe('setContent', () => {
it('should check if the edit is stale', async () => {
const isStaleEditSpy = jest.spyOn(editService, 'isStaleEdit').mockResolvedValueOnce(true);

await kaotoEditorApp.setContent('path', 'content');

expect(isStaleEditSpy).toHaveBeenCalledWith('content');
});

it('should not do anything if the edit is stale', async () => {
jest.spyOn(editService, 'isStaleEdit').mockResolvedValueOnce(true);

await kaotoEditorApp.setContent('path', 'content');

expect(editorRef.current!.setContent).not.toHaveBeenCalled();
});

it('should clear the hashes when the edit is not stale', async () => {
jest.spyOn(editService, 'isStaleEdit').mockResolvedValueOnce(false);
const clearHashesSpy = jest.spyOn(editService, 'clearEdits');

await kaotoEditorApp.setContent('path', 'content');

expect(clearHashesSpy).toHaveBeenCalled();
});

it('should delegate to the channelApi if the edit is not stale', async () => {
jest.spyOn(editService, 'isStaleEdit').mockResolvedValueOnce(false);

await kaotoEditorApp.setContent('path', 'content');

expect(editorRef.current!.setContent).toHaveBeenCalledWith('path', 'content');
});
});

it('getContent', async () => {
(editorRef.current!.getContent as jest.Mock).mockResolvedValue('content');

const content = await kaotoEditorApp.getContent();

expect(content).toBe('content');
});

it('getPreview', async () => {
(editorRef.current!.getPreview as jest.Mock).mockResolvedValue('preview');

const preview = await kaotoEditorApp.getPreview();

expect(preview).toBe('preview');
});

it('undo', async () => {
await kaotoEditorApp.undo();

expect(editorRef.current!.undo).toHaveBeenCalled();
});

it('redo', async () => {
await kaotoEditorApp.redo();

expect(editorRef.current!.redo).toHaveBeenCalled();
});

it('validate', async () => {
(editorRef.current!.validate as jest.Mock).mockResolvedValue([]);

const notifications = await kaotoEditorApp.validate();

expect(notifications).toEqual([]);
});

it('setTheme', async () => {
await kaotoEditorApp.setTheme(EditorTheme.DARK);

expect(editorRef.current!.setTheme).toHaveBeenCalledWith(EditorTheme.DARK);
});

it('sendReady', () => {
kaotoEditorApp.sendReady();

expect(envelopeContext.channelApi.notifications.kogitoEditor_ready.send).toHaveBeenCalled();
});

describe('sendNewEdit', () => {
it('should register the content with the EditService', async () => {
const registerSpy = jest.spyOn(editService, 'registerEdit');
await kaotoEditorApp.sendNewEdit('content');

expect(registerSpy).toHaveBeenCalledWith('content');
});

it('should delegate to the channelApi', async () => {
await kaotoEditorApp.sendNewEdit('content');

expect(envelopeContext.channelApi.notifications.kogitoWorkspace_newEdit.send).toHaveBeenCalledWith(
expect.objectContaining({ id: 'content' }),
);
});
});

it('sendNotifications', () => {
kaotoEditorApp.sendNotifications('path', []);

expect(envelopeContext.channelApi.notifications.kogitoNotifications_setNotifications.send).toHaveBeenCalled();
});

it('sendStateControlCommand', () => {
kaotoEditorApp.sendStateControlCommand(StateControlCommand.REDO);

expect(envelopeContext.channelApi.notifications.kogitoEditor_stateControlCommandUpdate.send).toHaveBeenCalledWith(
StateControlCommand.REDO,
);
});
});

const getNotificationMock = () => ({
subscribe: jest.fn(),
unsubscribe: jest.fn(),
send: jest.fn(),
});

class KaotoEditorAppTest extends KaotoEditorApp {
setEditorRef(editorRef: RefObject<EditorApi>) {
this.editorRef = editorRef;
}
}
Loading

0 comments on commit e8354d0

Please sign in to comment.