Fix race condition when starting proofreading with a split action (#8676)

When splitting an agglomerate in the proofreading mode while being in a
saved state and without any editable mapping yet being created, wk used
to crash.
What happened? The mapping name was updated in the store. This would
trigger the sagas that push update actions into the save queue. However,
this did not happen synchronously. Instead, the proofreading saga would
immediately call "ensureSavedState". As the save queue was not filled
yet, this function call would immediately return.

As a solution to this problem, this PR implements a ping-pong mechanism
in which "ensureSavedState" dispatches an action to which all diffing
sagas have to respond. By responding to the action, we know that the
diffing sagas have already run.

An alternative approach would have been to update some sort of version
counter every time the annotation in the store changes. Then, the
diffing sagas could store the newest version they have seen somewhere.
The downside of this approach is that we would need to store these
versions (and the "seen versions") per tracing. also, lots of store
updates for these counters would happen. All of this can be avoided with
the other approach described above.

Additionally, this PR changes 4 saga tests from a generator-style to an
integrated style where the saga middleware is used.

### URL of deployed dev instance (used for testing):
- https://fixawaitsaving.webknossos.xyz

### Steps to test:
- create a new annotation
- use the proofreading tool to split a segment (do this while no
editable mapping exists yet and while being in a saved state before
clicking "split")

### Issues:
- related to #8633 where the bug was initially observed

------
(Please delete unneeded items, merge only when none are left open)
- [x] Updated
[changelog](../blob/master/CHANGELOG.unreleased.md#unreleased)
- [ ] Updated [migration
guide](../blob/master/MIGRATIONS.unreleased.md#unreleased) if applicable
- [ ] Updated [documentation](../blob/master/docs) if applicable
- [ ] Adapted [wk-libs python
client](https://github.com/scalableminds/webknossos-libs/tree/master/webknossos/webknossos/client)
if relevant API parts change
- [ ] Removed dev-only changes like prints and application.conf edits
- [ ] Considered [common edge
cases](../blob/master/.github/common_edge_cases.md)
- [ ] Needs datastore update after deployment

---------

Co-authored-by: Daniel <daniel.werner@scalableminds.com>

Author: philippotto

frontend/javascripts/admin/rest_api.ts

@@ -1,6 +1,6 @@
 import dayjs from "dayjs";
 import { V3 } from "libs/mjs";
-import type { RequestOptions } from "libs/request";
+import type { RequestOptions, RequestOptionsWithData } from "libs/request";
 import Request from "libs/request";
 import type { Message } from "libs/toast";
 import Toast from "libs/toast";
@@ -90,6 +90,7 @@ import type {
   MappingType,
   NumberLike,
   PartialDatasetConfiguration,
+  SaveQueueEntry,
   StoreAnnotation,
   TraceOrViewCommand,
   UserConfiguration,
@@ -2398,3 +2399,12 @@ export function requestVerificationMail() {
     method: "POST",
   });
 }
+
+export function sendSaveRequestWithToken(
+  urlWithoutToken: string,
+  data: RequestOptionsWithData<Array<SaveQueueEntry>>,
+): Promise<void> {
+  // Ideally, this function should be refactored further so that it generates the
+  // correct urlWithoutToken itself.
+  return doWithToken((token) => Request.sendJSONReceiveJSON(`${urlWithoutToken}${token}`, data));
+}

frontend/javascripts/libs/utils.ts

@@ -1303,3 +1303,11 @@ export function getPhraseFromCamelCaseString(stringInCamelCase: string): string
     .map((word) => capitalize(word.replace(/(^|\s)td/, "$13D")))
     .join(" ");
 }
+
+export function areSetsEqual<T>(setA: Set<T>, setB: Set<T>) {
+  if (setA.size !== setB.size) return false;
+  for (const val of setA) {
+    if (!setB.has(val)) return false;
+  }
+  return true;
+}

frontend/javascripts/test/api/api_skeleton_latest.spec.ts

@@ -8,10 +8,9 @@ import { vi, describe, it, expect, beforeEach } from "vitest";
 import type { Vector3 } from "viewer/constants";
 import { enforceSkeletonTracing } from "viewer/model/accessors/skeletontracing_accessor";
 
-// All the mocking is done in the helpers file, so it can be reused for both skeleton and volume API
 describe("API Skeleton", () => {
   beforeEach<WebknossosTestContext>(async (context) => {
-    await setupWebknossosForTesting(context, "skeleton");
+    await setupWebknossosForTesting(context, "skeleton", { dontDispatchWkReady: true });
   });
 
   it<WebknossosTestContext>("getActiveNodeId should get the active node id", ({ api }) => {

frontend/javascripts/test/api/api_volume_latest.spec.ts

@@ -7,7 +7,6 @@ import {
 } from "../fixtures/volumetracing_server_objects";
 import { AnnotationTool } from "viewer/model/accessors/tool_accessor";
 
-// All the mocking is done in the helpers file, so it can be reused for both skeleton and volume API
 describe("API Volume", () => {
   beforeEach<WebknossosTestContext>(async (context) => {
     await setupWebknossosForTesting(context, "volume");

frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts

@@ -11,7 +11,7 @@ import { createTreeMapFromTreeArray } from "viewer/model/reducers/skeletontracin
 import { diffTrees } from "viewer/model/sagas/skeletontracing_saga";
 import { getNullableSkeletonTracing } from "viewer/model/accessors/skeletontracing_accessor";
 import { getServerVolumeTracings } from "viewer/model/accessors/volumetracing_accessor";
-import { sendRequestWithToken, addVersionNumbers } from "viewer/model/sagas/save_saga";
+import { addVersionNumbers } from "viewer/model/sagas/save_saga";
 import * as UpdateActions from "viewer/model/sagas/update_actions";
 import * as api from "admin/rest_api";
 import generateDummyTrees from "viewer/model/helpers/generate_dummy_trees";
@@ -174,7 +174,7 @@ describe("Annotation API (E2E)", () => {
   });
 
   async function sendUpdateActions(explorational: APIAnnotation, queue: SaveQueueEntry[]) {
-    return sendRequestWithToken(
+    return api.sendSaveRequestWithToken(
       `${explorational.tracingStore.url}/tracings/annotation/${explorational.id}/update?token=`,
       {
         method: "POST",

frontend/javascripts/test/fixtures/skeletontracing_server_objects.ts

@@ -5,11 +5,11 @@ import {
   type APITracingStoreAnnotation,
 } from "types/api_types";
 
-const TRACING_ID = "47e37793-d0be-4240-a371-87ce68561a13";
+const TRACING_ID = "skeletonTracingId-47e37793-d0be-4240-a371-87ce68561a13";
 
 export const tracing: ServerSkeletonTracing = {
   typ: AnnotationLayerEnum.Skeleton,
-  id: "47e37793-d0be-4240-a371-87ce68561a13",
+  id: TRACING_ID,
   trees: [
     {
       treeId: 2,

frontend/javascripts/test/fixtures/tasktracing_server_objects.ts

@@ -5,7 +5,7 @@ import {
   type APITracingStoreAnnotation,
 } from "types/api_types";
 
-const TRACING_ID = "e90133de-b2db-4912-8261-8b6f84f7edab";
+const TRACING_ID = "skeletonTracingId-e90133de-b2db-4912-8261-8b6f84f7edab";
 export const tracing: ServerSkeletonTracing = {
   typ: "Skeleton",
   trees: [
@@ -42,7 +42,7 @@ export const tracing: ServerSkeletonTracing = {
         b: 0,
         a: 1,
       },
-      name: "",
+      name: "", // there is a test that asserts that empty names will be renamed automatically
       isVisible: true,
       createdTimestamp: 1528811979356,
       metadata: [],
@@ -65,27 +65,27 @@ export const tracing: ServerSkeletonTracing = {
   },
   additionalAxes: [],
   zoomLevel: 2,
-  id: "e90133de-b2db-4912-8261-8b6f84f7edab",
+  id: TRACING_ID,
 };
 export const annotation: APIAnnotation = {
-  datasetId: "66f3c82966010034942e9740",
+  datasetId: "datasetId-66f3c82966010034942e9740",
   modified: 1529066010230,
   state: "Active",
-  id: "5b1fd1cf97000027049c67ee",
+  id: "annotationId-5b1fd1cf97000027049c67ee",
   name: "",
   description: "",
   stats: {},
   typ: "Task",
   task: {
-    id: "5b1fd1cb97000027049c67ec",
+    id: "taskId-5b1fd1cb97000027049c67ec",
     projectName: "sampleProject",
     projectId: "dummy-project-id",
     team: "Connectomics department",
     type: {
       id: "5b1e45faa000009d00abc2c6",
       summary: "sampleTaskType",
       description: "Description",
-      teamId: "5b1e45f9a00000a000abc2c3",
+      teamId: "teamId-5b1e45f9a00000a000abc2c3",
       teamName: "Connectomics department",
       settings: {
         allowedModes: ["orthogonal", "oblique", "flight"],
@@ -98,7 +98,7 @@ export const annotation: APIAnnotation = {
       recommendedConfiguration: null,
       tracingType: "skeleton",
     },
-    datasetId: "66f3c82966010034942e9740",
+    datasetId: "datasetId-66f3c82966010034942e9740",
     datasetName: "ROI2017_wkw",
     neededExperience: {
       domain: "oxalis",
@@ -156,7 +156,7 @@ export const annotation: APIAnnotation = {
   tracingTime: null,
   tags: ["ROI2017_wkw", "skeleton"],
   owner: {
-    id: "5b1e45faa00000a900abc2c5",
+    id: "userId-5b1e45faa00000a900abc2c5",
     email: "sample@scm.io",
     firstName: "Sample",
     lastName: "User",
@@ -165,7 +165,7 @@ export const annotation: APIAnnotation = {
     isDatasetManager: true,
     teams: [
       {
-        id: "5b1e45f9a00000a000abc2c3",
+        id: "teamId-5b1e45f9a00000a000abc2c3",
         name: "Connectomics department",
         isTeamManager: true,
       },
@@ -176,7 +176,7 @@ export const annotation: APIAnnotation = {
   isLockedByOwner: false,
   teams: [
     {
-      id: "5b1e45f9a00000a000abc2c3",
+      id: "teamId-5b1e45f9a00000a000abc2c3",
       name: "Connectomics department",
       organization: "Connectomics department",
     },

frontend/javascripts/test/fixtures/volumetracing_object.ts

@@ -3,14 +3,16 @@ import { AnnotationTool } from "viewer/model/accessors/tool_accessor";
 import Constants from "viewer/constants";
 import defaultState from "viewer/default_state";
 
+export const VOLUME_TRACING_ID = "volumeTracingId";
+
 const volumeTracing = {
   type: "volume",
   activeCellId: 0,
   activeTool: AnnotationTool.MOVE,
   largestSegmentId: 0,
   contourList: [],
   lastLabelActions: [],
-  tracingId: "tracingId",
+  tracingId: VOLUME_TRACING_ID,
 };
 const notEmptyViewportRect = {
   top: 0,

frontend/javascripts/test/fixtures/volumetracing_server_objects.ts

@@ -5,7 +5,7 @@ import {
   type APITracingStoreAnnotation,
 } from "types/api_types";
 
-const TRACING_ID = "tracingId-1234";
+const TRACING_ID = "volumeTracingId-1234";
 export const tracing: ServerVolumeTracing = {
   typ: "Volume",
   activeSegmentId: 10000,

frontend/javascripts/test/global_mocks.ts

@@ -112,3 +112,32 @@ vi.mock("viewer/model/helpers/shader_editor.ts", () => ({
     destroy: vi.fn(),
   },
 }));
+
+vi.mock("antd", () => {
+  return {
+    Button: {},
+    theme: {
+      getDesignToken: () => ({ colorPrimary: "white" }),
+      defaultAlgorithm: {},
+    },
+    Dropdown: {},
+    message: {
+      hide: vi.fn(),
+      // These return a "hide function"
+      show: vi.fn(() => () => {}),
+      loading: vi.fn(() => () => {}),
+      success: vi.fn(() => () => {}),
+    },
+    Modal: {
+      confirm: vi.fn(),
+    },
+    Select: {
+      Option: {},
+    },
+    Form: {
+      Item: {},
+    },
+  };
+});
+
+vi.mock("libs/render_independently", () => ({ default: vi.fn() }));

frontend/javascripts/test/helpers/apiHelpers.ts

@@ -29,14 +29,18 @@ import Model from "viewer/model";
 import UrlManager from "viewer/controller/url_manager";
 
 import WebknossosApi from "viewer/api/api_loader";
-import { default as Store, startSaga } from "viewer/store";
+import { type SaveQueueEntry, default as Store, startSaga } from "viewer/store";
 import rootSaga from "viewer/model/sagas/root_saga";
 import { setStore, setModel } from "viewer/singletons";
 import { setupApi } from "viewer/api/internal_api";
 import { setActiveOrganizationAction } from "viewer/model/actions/organization_actions";
 import Request, { type RequestOptions } from "libs/request";
 import { parseProtoAnnotation, parseProtoTracing } from "viewer/model/helpers/proto_helpers";
 import app from "app";
+import { sendSaveRequestWithToken } from "admin/rest_api";
+import { restartSagaAction, wkReadyAction } from "viewer/model/actions/actions";
+import { discardSaveQueuesAction } from "viewer/model/actions/save_actions";
+import { setActiveUserAction } from "viewer/model/actions/user_actions";
 
 const TOKEN = "secure-token";
 const ANNOTATION_TYPE = "annotationTypeValue";
@@ -51,6 +55,7 @@ export interface WebknossosTestContext extends BaseTestContext {
   setSlowCompression: (enabled: boolean) => void;
   api: ApiInterface;
   tearDownPullQueues: () => void;
+  receivedDataPerSaveRequest: Array<SaveQueueEntry[]>;
 }
 
 // Create mock objects
@@ -67,6 +72,21 @@ vi.mock("libs/request", () => ({
   },
 }));
 
+vi.mock("admin/rest_api.ts", async () => {
+  const actual = await vi.importActual<typeof import("admin/rest_api.ts")>("admin/rest_api.ts");
+
+  const receivedDataPerSaveRequest: Array<SaveQueueEntry[]> = [];
+  const mockedSendRequestWithToken = vi.fn((_, payload) => {
+    receivedDataPerSaveRequest.push(payload.data);
+    return Promise.resolve();
+  });
+  (mockedSendRequestWithToken as any).receivedDataPerSaveRequest = receivedDataPerSaveRequest;
+  return {
+    ...actual,
+    sendSaveRequestWithToken: mockedSendRequestWithToken,
+  };
+});
+
 vi.mock("libs/compute_bvh_async", () => ({
   computeBvhAsync: vi.fn().mockResolvedValue(undefined),
 }));
@@ -176,8 +196,15 @@ startSaga(rootSaga);
 export async function setupWebknossosForTesting(
   testContext: WebknossosTestContext,
   mode: keyof typeof modelData,
-  apiVersion?: number,
+  options?: { dontDispatchWkReady?: boolean },
 ): Promise<void> {
+  /*
+   * This will execute model.fetch(...) and initialize the store with the tracing, etc.
+   */
+  Store.dispatch(restartSagaAction());
+  Store.dispatch(discardSaveQueuesAction());
+  Store.dispatch(setActiveUserAction(dummyUser));
+
   Store.dispatch(setActiveOrganizationAction(dummyOrga));
   UrlManager.initialState = {
     position: [1, 2, 3],
@@ -192,6 +219,9 @@ export async function setupWebknossosForTesting(
     Model.getAllLayers().map((layer) => {
       layer.pullQueue.destroy();
     });
+  testContext.receivedDataPerSaveRequest = (
+    sendSaveRequestWithToken as any
+  ).receivedDataPerSaveRequest;
 
   const webknossos = new WebknossosApi(Model);
   const annotationFixture = modelData[mode].annotation;
@@ -221,8 +251,16 @@ export async function setupWebknossosForTesting(
     // Trigger the event ourselves, as the webKnossosController is not instantiated
     app.vent.emit("webknossos:ready");
 
-    const api = await webknossos.apiReady(apiVersion);
+    const api = await webknossos.apiReady();
     testContext.api = api;
+
+    // Ensure the slow compression is disabled by default. Tests may change
+    // this individually.
+    testContext.setSlowCompression(false);
+    if (!options?.dontDispatchWkReady) {
+      // Dispatch the wkReadyAction, so the sagas are started
+      Store.dispatch(wkReadyAction());
+    }
   } catch (error) {
     console.error("model.fetch() failed", error);
     if (error instanceof Error) {

frontend/javascripts/test/libs/__snapshots__/nml.spec.ts.snap

@@ -6,7 +6,7 @@ exports[`NML > NML serializer should escape special characters and multilines 1`
   <meta name="writerGitCommit" content="fc0ea6432ec7107e8f9b5b308ee0e90eae0e7b17" />
   <meta name="timestamp" content="1494695001688" />
   <meta name="annotationId" content="annotationId" />
-  <meta name="taskId" content="5b1fd1cb97000027049c67ec" />
+  <meta name="taskId" content="taskId-5b1fd1cb97000027049c67ec" />
   <parameters>
     <experiment datasetId="dummy-dataset-id" name="Loading" description="Multiline dataset&#xa;description&#xa;with special &amp;&apos;&lt;&gt;&quot; chars." organization="" wkUrl="http://localhost:9000" />
     <scale x="5" y="5" z="5" unit="nanometer" />
@@ -65,7 +65,7 @@ exports[`NML > NML serializer should produce correct NMLs 1`] = `
   <meta name="writerGitCommit" content="fc0ea6432ec7107e8f9b5b308ee0e90eae0e7b17" />
   <meta name="timestamp" content="1494695001688" />
   <meta name="annotationId" content="annotationId" />
-  <meta name="taskId" content="5b1fd1cb97000027049c67ec" />
+  <meta name="taskId" content="taskId-5b1fd1cb97000027049c67ec" />
   <parameters>
     <experiment datasetId="dummy-dataset-id" name="Loading" description="" organization="" wkUrl="http://localhost:9000" />
     <scale x="5" y="5" z="5" unit="nanometer" />
@@ -123,7 +123,7 @@ exports[`NML > NML serializer should produce correct NMLs with additional coordi
   <meta name="writerGitCommit" content="fc0ea6432ec7107e8f9b5b308ee0e90eae0e7b17" />
   <meta name="timestamp" content="1494695001688" />
   <meta name="annotationId" content="annotationId" />
-  <meta name="taskId" content="5b1fd1cb97000027049c67ec" />
+  <meta name="taskId" content="taskId-5b1fd1cb97000027049c67ec" />
   <parameters>
     <experiment datasetId="dummy-dataset-id" name="Loading" description="" organization="" wkUrl="http://localhost:9000" />
     <scale x="5" y="5" z="5" unit="nanometer" />

frontend/javascripts/test/libs/nml.spec.ts

@@ -555,7 +555,9 @@ describe("NML", () => {
   });
 
   it("Serialized nml should be correctly named", async () => {
-    expect(getNmlName(initialState)).toBe("Test Dataset__5b1fd1cb97000027049c67ec____tionId.nml");
+    expect(getNmlName(initialState)).toBe(
+      "Test Dataset__taskId-5b1fd1cb97000027049c67ec____tionId.nml",
+    );
 
     const stateWithoutTask = { ...initialState, task: null };