[Runtime field editor] Error handling

packages/kbn-monaco/BUILD.bazel

@@ -36,12 +36,14 @@ RUNTIME_DEPS = [
   "@npm//monaco-editor",
   "@npm//raw-loader",
   "@npm//regenerator-runtime",
+  "@npm//rxjs",
 ]
 
 TYPES_DEPS = [
   "//packages/kbn-i18n",
   "@npm//antlr4ts",
   "@npm//monaco-editor",
+  "@npm//rxjs",
   "@npm//@types/jest",
   "@npm//@types/node",
 ]

packages/kbn-monaco/__jest__/jest.mocks.ts

@@ -0,0 +1,68 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { MockIModel } from './types';
+
+const createMockModel = (ID: string) => {
+  const model: MockIModel = {
+    uri: '',
+    id: 'mockModel',
+    value: '',
+    getModeId: () => ID,
+    changeContentListeners: [],
+    setValue(newValue) {
+      this.value = newValue;
+      this.changeContentListeners.forEach((listener) => listener());
+    },
+    getValue() {
+      return this.value;
+    },
+    onDidChangeContent(handler) {
+      this.changeContentListeners.push(handler);
+    },
+    onDidChangeLanguage: (handler) => {
+      handler({ newLanguage: ID });
+    },
+  };
+
+  return model;
+};
+
+jest.mock('../src/monaco_imports', () => {
+  const original = jest.requireActual('../src/monaco_imports');
+  const originalMonaco = original.monaco;
+  const originalEditor = original.monaco.editor;
+
+  return {
+    ...original,
+    monaco: {
+      ...originalMonaco,
+      editor: {
+        ...originalEditor,
+        model: null,
+        createModel(ID: string) {
+          this.model = createMockModel(ID);
+          return this.model;
+        },
+        onDidCreateModel(handler: (model: MockIModel) => void) {
+          if (!this.model) {
+            throw new Error(
+              `Model needs to be created by calling monaco.editor.createModel(ID) first.`
+            );
+          }
+          handler(this.model);
+        },
+        getModel() {
+          return this.model;
+        },
+        getModels: () => [],
+        setModelMarkers: () => undefined,
+      },
+    },
+  };
+});

packages/kbn-monaco/__jest__/types.ts

@@ -0,0 +1,19 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+export interface MockIModel {
+  uri: string;
+  id: string;
+  value: string;
+  changeContentListeners: Array<() => void>;
+  getModeId: () => string;
+  setValue: (value: string) => void;
+  getValue: () => string;
+  onDidChangeContent: (handler: () => void) => void;
+  onDidChangeLanguage: (handler: (options: { newLanguage: string }) => void) => void;
+}

packages/kbn-monaco/src/painless/diagnostics_adapter.test.ts

@@ -0,0 +1,147 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+import '../../__jest__/jest.mocks'; // Make sure this is the first import
+
+import { Subscription } from 'rxjs';
+
+import { MockIModel } from '../../__jest__/types';
+import { LangValidation } from '../types';
+import { monaco } from '../monaco_imports';
+import { ID } from './constants';
+
+import { DiagnosticsAdapter } from './diagnostics_adapter';
+
+const getSyntaxErrors = jest.fn(async (): Promise<string[] | undefined> => undefined);
+
+const getMockWorker = async () => {
+  return {
+    getSyntaxErrors,
+  } as any;
+};
+
+function flushPromises() {
+  return new Promise((resolve) => setImmediate(resolve));
+}
+
+describe('Painless DiagnosticAdapter', () => {
+  let diagnosticAdapter: DiagnosticsAdapter;
+  let subscription: Subscription;
+  let model: MockIModel;
+  let validation: LangValidation;
+
+  beforeAll(() => {
+    jest.useFakeTimers();
+  });
+
+  afterAll(() => {
+    jest.useRealTimers();
+  });
+
+  beforeEach(async () => {
+    model = monaco.editor.createModel(ID) as unknown as MockIModel;
+    diagnosticAdapter = new DiagnosticsAdapter(getMockWorker);
+
+    // validate() has a promise we need to wait for
+    // --> await worker.getSyntaxErrors()
+    await flushPromises();
+
+    subscription = diagnosticAdapter.validation$.subscribe((newValidation) => {
+      validation = newValidation;
+    });
+  });
+
+  afterEach(() => {
+    if (subscription) {
+      subscription.unsubscribe();
+    }
+  });
+
+  test('should validate when the content changes', async () => {
+    expect(validation!.isValidating).toBe(false);
+
+    model.setValue('new content');
+    await flushPromises();
+    expect(validation!.isValidating).toBe(true);
+
+    jest.advanceTimersByTime(500); // there is a 500ms debounce for the validate() to trigger
+    await flushPromises();
+
+    expect(validation!.isValidating).toBe(false);
+
+    model.setValue('changed');
+    // Flushing promise here is not actually required but adding it to make sure the test
+    // works as expected even when doing so.
+    await flushPromises();
+    expect(validation!.isValidating).toBe(true);
+
+    // when we clear the content we immediately set the
+    // "isValidating" to false and mark the content as valid.
+    // No need to wait for the setTimeout
+    model.setValue('');
+    await flushPromises();
+    expect(validation!.isValidating).toBe(false);
+    expect(validation!.isValid).toBe(true);
+  });
+
+  test('should prevent race condition of multiple content change and validation triggered', async () => {
+    const errors = ['Syntax error returned'];
+
+    getSyntaxErrors.mockResolvedValueOnce(errors);
+
+    expect(validation!.isValidating).toBe(false);
+
+    model.setValue('foo');
+    jest.advanceTimersByTime(300); // only 300ms out of the 500ms
+
+    model.setValue('bar'); // This will cancel the first setTimeout
+
+    jest.advanceTimersByTime(300); // Again, only 300ms out of the 500ms.
+    await flushPromises();
+
+    expect(validation!.isValidating).toBe(true); // we are still validating
+
+    jest.advanceTimersByTime(200); // rest of the 500ms
+    await flushPromises();
+
+    expect(validation!.isValidating).toBe(false);
+    expect(validation!.isValid).toBe(false);
+    expect(validation!.errors).toBe(errors);
+  });
+
+  test('should prevent race condition (2) of multiple content change and validation triggered', async () => {
+    const errors1 = ['First error returned'];
+    const errors2 = ['Second error returned'];
+
+    getSyntaxErrors
+      .mockResolvedValueOnce(errors1) // first call
+      .mockResolvedValueOnce(errors2); // second call
+
+    model.setValue('foo');
+    // By now we are waiting on the worker to await getSyntaxErrors()
+    // we won't flush the promise to not pass this point in time just yet
+    jest.advanceTimersByTime(700);
+
+    // We change the value at the same moment
+    model.setValue('bar');
+    // now we pass the await getSyntaxErrors() point but its result (errors1) should be stale and discarted
+    await flushPromises();
+
+    jest.advanceTimersByTime(300);
+    await flushPromises();
+
+    expect(validation!.isValidating).toBe(true); // we are still validating value "bar"
+
+    jest.advanceTimersByTime(200); // rest of the 500ms
+    await flushPromises();
+
+    expect(validation!.isValidating).toBe(false);
+    expect(validation!.isValid).toBe(false);
+    // We have the second error response, the first one has been discarted
+    expect(validation!.errors).toBe(errors2);
+  });
+});

packages/kbn-monaco/src/painless/diagnostics_adapter.ts

@@ -6,7 +6,10 @@
  * Side Public License, v 1.
  */
 
+import { BehaviorSubject } from 'rxjs';
+
 import { monaco } from '../monaco_imports';
+import { SyntaxErrors, LangValidation } from '../types';
 import { ID } from './constants';
 import { WorkerAccessor } from './language';
 import { PainlessError } from './worker';
@@ -18,11 +21,17 @@ const toDiagnostics = (error: PainlessError): monaco.editor.IMarkerData => {
   };
 };
 
-export interface SyntaxErrors {
-  [modelId: string]: PainlessError[];
-}
 export class DiagnosticsAdapter {
   private errors: SyntaxErrors = {};
+  private validation = new BehaviorSubject<LangValidation>({
+    isValid: true,
+    isValidating: false,
+    errors: [],
+  });
+  // To avoid stale validation data we keep track of the latest call to validate().
+  private validateIdx = 0;
+
+  public validation$ = this.validation.asObservable();
 
   constructor(private worker: WorkerAccessor) {
     const onModelAdd = (model: monaco.editor.IModel): void => {
@@ -35,14 +44,27 @@ export class DiagnosticsAdapter {
             return;
           }
 
+          const idx = ++this.validateIdx; // Disable any possible inflight validation
+          clearTimeout(handle);
+
           // Reset the model markers if an empty string is provided on change
           if (model.getValue().trim() === '') {
+            this.validation.next({
+              isValid: true,
+              isValidating: false,
+              errors: [],
+            });
             return monaco.editor.setModelMarkers(model, ID, []);
           }
 
+          this.validation.next({
+            ...this.validation.value,
+            isValidating: true,
+          });
           // Every time a new change is made, wait 500ms before validating
-          clearTimeout(handle);
-          handle = setTimeout(() => this.validate(model.uri), 500);
+          handle = setTimeout(() => {
+            this.validate(model.uri, idx);
+          }, 500);
         });
 
         model.onDidChangeLanguage(({ newLanguage }) => {
@@ -51,21 +73,33 @@ export class DiagnosticsAdapter {
           if (newLanguage !== ID) {
             return monaco.editor.setModelMarkers(model, ID, []);
           } else {
-            this.validate(model.uri);
+            this.validate(model.uri, ++this.validateIdx);
           }
         });
 
-        this.validate(model.uri);
+        this.validation.next({
+          ...this.validation.value,
+          isValidating: true,
+        });
+        this.validate(model.uri, ++this.validateIdx);
       }
     };
     monaco.editor.onDidCreateModel(onModelAdd);
     monaco.editor.getModels().forEach(onModelAdd);
   }
 
-  private async validate(resource: monaco.Uri): Promise<void> {
+  private async validate(resource: monaco.Uri, idx: number): Promise<void> {
+    if (idx !== this.validateIdx) {
+      return;
+    }
+
     const worker = await this.worker(resource);
     const errorMarkers = await worker.getSyntaxErrors(resource.toString());
 
+    if (idx !== this.validateIdx) {
+      return;
+    }
+
     if (errorMarkers) {
       const model = monaco.editor.getModel(resource);
       this.errors = {
@@ -75,6 +109,9 @@ export class DiagnosticsAdapter {
       // Set the error markers and underline them with "Error" severity
       monaco.editor.setModelMarkers(model!, ID, errorMarkers.map(toDiagnostics));
     }
+
+    const isValid = errorMarkers === undefined || errorMarkers.length === 0;
+    this.validation.next({ isValidating: false, isValid, errors: errorMarkers ?? [] });
   }
 
   public getSyntaxErrors() {

packages/kbn-monaco/src/painless/index.ts

@@ -8,7 +8,7 @@
 
 import { ID } from './constants';
 import { lexerRules, languageConfiguration } from './lexer_rules';
-import { getSuggestionProvider, getSyntaxErrors } from './language';
+import { getSuggestionProvider, getSyntaxErrors, validation$ } from './language';
 import { CompleteLangModuleType } from '../types';
 
 export const PainlessLang: CompleteLangModuleType = {
@@ -17,6 +17,7 @@ export const PainlessLang: CompleteLangModuleType = {
   lexerRules,
   languageConfiguration,
   getSyntaxErrors,
+  validation$,
 };
 
 export * from './types';