kv-explorer/adapt to new apis

[pdesoyres-cc]

What this PR do ?

There are various enhancements:

• first commit is about adapting kv client to some APIs modification
• commits (2 to 13) are minor refactoring after @hsablonniere review
• commits (14 to 16) is about clarifying separation between view and model by implementing three controllers
• Then, there are three commits about refining state with a new filtering type
• Then, there is a commit that avoids every race condition during key loading/scanning by using a shared Semaphore

How to review

• Check the code commit by commit, or it will be difficult to understand anything!
• Check the new stories related to the filtering state type
• Play with staging-five console which is plugged to the preprod redis-http APIs

[github-actions]

🔎 A preview has been automatically published : https://clever-components-preview.cellar-c2.services.clever-cloud.com/kv-explorer/adapt-to-new-apis/index.html.

This preview will be deleted once this PR is closed.

[roberttran-cc]

LGTM!

The new code feels a lot better (I'll admit I did not check everything thoroughly).

[florian-sanders-cc]

I did not review thoroughly most of the refactoring commits but I have a small question about the delete button tabindex and some nitpicks.

Apart from that, LGTM, gg @pdesoyres-cc!

[hsablonniere]

👏 Wow, impressive work @pdesoyres-cc !!

General

• a few comments here and there, nothing serious
• praise: the way the code is structured in the smart + the 3 Kv*Ctrl makes the code easier to read and hopefully to maintain later
• praise: the abort controller usage with the semaphore is nice
  • issue: the way you share just one kvSharedSemaphore means having 2 KV explorers on the same page could be a problem. The instance of the semaphore should be unique per instance of the component but we won't really face this problem so not sure if we should "fix" it and when. I'll let you decided (maybe a small comment if we don't act).
  • note: I haven't tested but I was wondering about a given situation (see "Race condition investiation" below)

Race condition investiation

• If we click on hash A in the list, the detail view uses the loadMore of the KvScanner with the fetch of the KvHashElementsScanner and ensures we use the AbortSignal via the semaphore
• Then if we click on hash B in the list, if the A hash scan async call was still ongoing it gets cancelled before we try to scan B, no risk for race conditions here.
• I was wondering what happens if we try to loadMore but on the same hash (or list, or set...)?
  • It would mean "page 1" loading would get cancelled when we try to load "page 2".
  • I think the UI and/or smart prevents this situation but I'm not sure
  • Can you confirm this?

Loading error

I just realized this now but if the component fails to load the list on the first display of the page, we get a toast and the component stays in skeleton/loading state. We don't have any way to click on reload keys. I think I expected an "error" state where I can click on reload and no toast. This is fine for now as this won't happen that much and the user can do a full page reload.

We have a similar situation when the call to reload the keys fails. We get the toast, the list goes to skeleton/waiting and then empty with the No results matching the filter message and the number of keys still defined with the last value. Again here, I expected an "error" state where I can click on reload and no toast. Same as above, not a very important problem to tackle and it was already present before this PR so it can be tackled later.

[pdesoyres-cc]

Thanks @hsablonniere for this deep review.

• About race condition
  • I can confirm that the UI prevents from scanning page 2 while still scanning page 1.
• About loading errors
  • I have added the right state and logic in a dedicated commit.
• About the shared Semaphore.
  • the smart component maintains a Map of Semaphore (by component)
  • when context is updated, a new Semaphore is created and added to the Map
  • when signal is aborted, the Semaphore is removed from the Map.
  • I needed to pass along the semaphore to controllers and to scanners (no more singleton easy to get)
  • I tried to play with 2 components in the same page and it works like a charm (just by duplicating the component in the sandbox).
  • Can you take a look and verify it is ok?

[hsablonniere]

Well done @pdesoyres-cc !!!

• some adjustments on what we discussed IRL
• a small comment

[github-actions]

🔎 The preview has been automatically deleted.