Schema Composition: cleanup on initial PoC decisions

[wiedld]

Reconsider/address decisions made with initial PoC. Decide which is feasible, and the timeline:

• [ ] We need to add increased integration between the LSP layer and the UI code so that errors that occur can be communicated bi-directionally.
  • current behaviors which we ideally can fix with the middleware layer:
    • [ ] First time schema composition is turned on after page load occurs there is a hardcoded 3 second delay to prevent race conditions.
    • [ ] Race conditions can occur when a user clicks too many elements within Schema Browser too quickly.
    • [x] Any general error occurring within the LSP will desynchronize it from the UI.
    • [x] Page reload, and re-syncing, can cause reinitialize the composition block with many additional filters -- which requires coordination over many request/response loops.
      • https://github.com/influxdata/ui/issues/5622
• [x] When a composition block is many lines, and the user does something which changes the line size too much -- the replacement text (applyEdit from the LSP) is not the right size.
  • https://github.com/influxdata/flux-lsp/issues/574.
• [ ] “Flux Sync’ is a One Time One way process.
  • [ ] We want to add bi-directional capability. I.e. the ability to recreate the schema browser state from an existing query state.
  • [ ] We want to add support for multiple schema composition blocks with the caveat that only one block may be active in the sync functionality at one time.
• [x] We did not yet implement the default query text comment on page load which was included in design documentation. It is a small addition but it may interfere with default schema composition state so we wanted to be careful around testing it.

Small items that we can quickly resolve:

• [x] Flux Sync should be turned on by default.

[rockstar]

First time schema composition is turned on after page load occurs there is a hardcoded 3 second delay to prevent race conditions.

This hard-coded three seconds is mostly a guess, and on systems with slower cpus or slower connections, this is still likely to break. The solution could be to raise that limit to 5 seconds, but where does it end? 10 seconds? 30 seconds?

One of the things react et al gives us is otherwise a more straightforward API for managing state, and allowing components to update and change based on state (via javascript's event-based model). In this situation, what we're missing is a state that says "we're ready for composition to be turned on." Composition shouldn't be "turned on" until it can be turned on in the place where it matters.

Race conditions can occur when a user clicks too many elements within Schema Browser too quickly.

In this case, it's because we're again assuming that we don't have state/change events that allow us to see when changes are applied. It is the UI component's responsibility to handle input too quick (we debounce in text boxes, for instance). There are a number of ways we can hack around this, but the most straightforward way is likely to add a new composition criteria and not allow anymore composition criteria to be added until the query itself changes (likely by checking that the change gets updated in QueryContext, but I'm still trying to get my head around all the places where the query is stored.

Any general error occurring within the LSP will desynchronize it from the UI.

This happens as a side effect anywhere where the wasm is used. Its solutions are likely unrelated to composition, and shouldn't be addressed with the narrow view of composition. It warrants its own Epic for how we handle this going forward.

“Flux Sync’ is a One Time One way process.

Is this actually an issue, or is this a feature goal? If it's just an unimplemented feature, is it scope creep, or is it original functionality we're missing?

[rockstar]

“Flux Sync’ is a One Time One way process.

Per product, this is not something we need to worry about currently.

[wiedld]

Spoke with @rockstar about the general theme of wanting to move the editor-LSP wiring into an eventing based model. The crux of the issue is that we don't have control over when certain events occur in the monacoEditor-LSP communication, nor do we currently have transparency into when these events have completed.

A bit of background. The monacoEditor-LSP communication is a series of request/response messages, corresponding to events. Events must occur in sequence, e.g. (1) initialize request/response, (2) initialized request/response, (3) file didOpen request/response, and finally (4) any other messages (include schema composition messages). We have a series of async issues that arise due to need to have all these request/response messages occur in sequence, as well as (in some cases) having the monacoEditor fully "complete the event" triggered by the previous request/response message -- before the UI sends the next request.

We have been getting around this issue in the current implementation, by using message buffering and a facade design pattern. This adds code complexity which we would ideally like to remove prior to piling on more features. However, it does require that we add the missing event handlers to the monacoEditor API.

.

Here are the current async events, how we are currently handling them, and how we propose to handle them under the eventing-based model:

1. the LSP worker boot time is longer than the monacoEditor boot time.

   • problem:
     • the editor sends out all of the initialize sequence and didOpen sequence, which gets silently dropped/missed by the LSP. In which case, the LSP does not "boot up" properly.
   • current solution:
     • allow both to boot in parallel, but buffer the incoming jsonrpc messages on the LSP worker thread -- until the LSP has fully booted up.
   • proposed solution:
     • do these steps in series, not parallel. Boot up the LSP first, then trigger the monacoEditor.
     • gating decision: determine our current average boot times, so we know how long it takes in series.

2. the UI, outside the monacoEditor, can send request messages to the LSP. But these messages can only be acting upon (without error), when done in the proper async sequence.

   • challenge # 1:
     • problem:
       • the UI can issue composition requests, prior to the last request/response event being completed
       • the result is incorrect editor text being produced. (Not just missing parts, but half formed flux transformations -- invalid flux syntax).
     • current solution:
       • buffer all outgoing UI composition requests
       • listen for the previous composition request/response event being completed.
       • (we found an indirect listener, to determine applyEdit event completion.)
       • increment buffer for next request
     • proposed solution:
       • move the gating of UI events to the UI component itself.
       • e.g. checkboxes are not responsive, until you can issue the next event.
       • possible implementations:
         • (a) having the same listener (as above) be used on each UI element which triggers events?
         • (b) or just debouncing inputs from UI elements? and debouncing across UI elements affliiated with the composition?
   • challenge # 2:
     • problem:
       • on page reload, the persisted session has a schema selection. The editor text (schema composition block) gets restored to match the schema selection in the session.
       • therefore, the UI needs to send composition request messages to the LSP.
       • but we don't currently have a way to see that the boot sequence (initialize, initialized, didOpen) has completed.
     • current solution:
       • a crude timeout, with documented tech debt to allow us to build a middleware, that listens to the jsonrpc response/request messages. That way we know when the didOpen event has completed.
     • proposed solution:
       • have the monacoEditor own all events.
       • this means extending the monacoEditor API to have a didOpen() event listener, after which we can send out the composition requests.

3. Have all events only be triggered through the monaco-editor.

   • problem:
     • The monacoEditor lib is currently designed where we only can control the initial instantiation/creation of the editor...and then all the downstream jsonrpc messages/events are "owned" by the monacoEditor itself.
     • monacoEditor does not provide the API we need, to send UI-triggered messages for schema composition. (Although the LSP spec does include this ability.)
   • current solution:
     • we construct and inject the UI-triggered jsonrpc messages ourselves, directly into the connection to the LSP (skipping the monacoEditor).
     • LSP response is handled by the monacoEditor, as per usual.
   • proposed solution:
     • extended the monacoEditor API to support these commandExecute messages, per the LSP spec.

4. Handling of LSP error messages:

   • problem:
     • The monacoEditor lib owns all incoming messages. It does not have event handlers for when certain errors occur in the LSP.
   • current proposed solution:
     • have middleware listener also handle
   • solution under eventing-based model:
     • TBD. have ideas.