diff --git a/frontend/javascripts/admin/rest_api.ts b/frontend/javascripts/admin/rest_api.ts index 8af960c0d96..feed2b6cb4d 100644 --- a/frontend/javascripts/admin/rest_api.ts +++ b/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>, +): Promise { + // Ideally, this function should be refactored further so that it generates the + // correct urlWithoutToken itself. + return doWithToken((token) => Request.sendJSONReceiveJSON(`${urlWithoutToken}${token}`, data)); +} diff --git a/frontend/javascripts/libs/utils.ts b/frontend/javascripts/libs/utils.ts index 62feda217db..e8498c7f9d8 100644 --- a/frontend/javascripts/libs/utils.ts +++ b/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(setA: Set, setB: Set) { + if (setA.size !== setB.size) return false; + for (const val of setA) { + if (!setB.has(val)) return false; + } + return true; +} diff --git a/frontend/javascripts/test/api/api_skeleton_latest.spec.ts b/frontend/javascripts/test/api/api_skeleton_latest.spec.ts index e21c41cca07..8750278301b 100644 --- a/frontend/javascripts/test/api/api_skeleton_latest.spec.ts +++ b/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(async (context) => { - await setupWebknossosForTesting(context, "skeleton"); + await setupWebknossosForTesting(context, "skeleton", { dontDispatchWkReady: true }); }); it("getActiveNodeId should get the active node id", ({ api }) => { diff --git a/frontend/javascripts/test/api/api_volume_latest.spec.ts b/frontend/javascripts/test/api/api_volume_latest.spec.ts index 4875205393a..e1275d881aa 100644 --- a/frontend/javascripts/test/api/api_volume_latest.spec.ts +++ b/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(async (context) => { await setupWebknossosForTesting(context, "volume"); diff --git a/frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts b/frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts index 8271f31c41c..7a793068895 100644 --- a/frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts +++ b/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", diff --git a/frontend/javascripts/test/fixtures/skeletontracing_server_objects.ts b/frontend/javascripts/test/fixtures/skeletontracing_server_objects.ts index 723d7bf9c3e..bbde524965c 100644 --- a/frontend/javascripts/test/fixtures/skeletontracing_server_objects.ts +++ b/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, diff --git a/frontend/javascripts/test/fixtures/tasktracing_server_objects.ts b/frontend/javascripts/test/fixtures/tasktracing_server_objects.ts index 98124cc0a71..411a5a591d8 100644 --- a/frontend/javascripts/test/fixtures/tasktracing_server_objects.ts +++ b/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,19 +65,19 @@ 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", @@ -85,7 +85,7 @@ export const annotation: APIAnnotation = { 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", }, diff --git a/frontend/javascripts/test/fixtures/volumetracing_object.ts b/frontend/javascripts/test/fixtures/volumetracing_object.ts index d0b325cfafd..b0b515eb170 100644 --- a/frontend/javascripts/test/fixtures/volumetracing_object.ts +++ b/frontend/javascripts/test/fixtures/volumetracing_object.ts @@ -3,6 +3,8 @@ 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, @@ -10,7 +12,7 @@ const volumeTracing = { largestSegmentId: 0, contourList: [], lastLabelActions: [], - tracingId: "tracingId", + tracingId: VOLUME_TRACING_ID, }; const notEmptyViewportRect = { top: 0, diff --git a/frontend/javascripts/test/fixtures/volumetracing_server_objects.ts b/frontend/javascripts/test/fixtures/volumetracing_server_objects.ts index 6a11c4215bc..8abdb70591a 100644 --- a/frontend/javascripts/test/fixtures/volumetracing_server_objects.ts +++ b/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, diff --git a/frontend/javascripts/test/global_mocks.ts b/frontend/javascripts/test/global_mocks.ts index 79287b7c906..46e400848e5 100644 --- a/frontend/javascripts/test/global_mocks.ts +++ b/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() })); diff --git a/frontend/javascripts/test/helpers/apiHelpers.ts b/frontend/javascripts/test/helpers/apiHelpers.ts index 1cba1ded9c0..132ffba2b08 100644 --- a/frontend/javascripts/test/helpers/apiHelpers.ts +++ b/frontend/javascripts/test/helpers/apiHelpers.ts @@ -29,7 +29,7 @@ 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"; @@ -37,6 +37,10 @@ import { setActiveOrganizationAction } from "viewer/model/actions/organization_a 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; } // Create mock objects @@ -67,6 +72,21 @@ vi.mock("libs/request", () => ({ }, })); +vi.mock("admin/rest_api.ts", async () => { + const actual = await vi.importActual("admin/rest_api.ts"); + + const receivedDataPerSaveRequest: Array = []; + 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 { + /* + * 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) { diff --git a/frontend/javascripts/test/libs/__snapshots__/nml.spec.ts.snap b/frontend/javascripts/test/libs/__snapshots__/nml.spec.ts.snap index c424f6cc631..8a39bbcf760 100644 --- a/frontend/javascripts/test/libs/__snapshots__/nml.spec.ts.snap +++ b/frontend/javascripts/test/libs/__snapshots__/nml.spec.ts.snap @@ -6,7 +6,7 @@ exports[`NML > NML serializer should escape special characters and multilines 1` - + @@ -65,7 +65,7 @@ exports[`NML > NML serializer should produce correct NMLs 1`] = ` - + @@ -123,7 +123,7 @@ exports[`NML > NML serializer should produce correct NMLs with additional coordi - + diff --git a/frontend/javascripts/test/libs/nml.spec.ts b/frontend/javascripts/test/libs/nml.spec.ts index 30eaf7be1ae..bedd8113a09 100644 --- a/frontend/javascripts/test/libs/nml.spec.ts +++ b/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 }; diff --git a/frontend/javascripts/test/reducers/volumetracing_reducer.spec.ts b/frontend/javascripts/test/reducers/volumetracing_reducer.spec.ts index 55d78c0ec7d..b400d47bffb 100644 --- a/frontend/javascripts/test/reducers/volumetracing_reducer.spec.ts +++ b/frontend/javascripts/test/reducers/volumetracing_reducer.spec.ts @@ -6,7 +6,7 @@ import * as UiActions from "viewer/model/actions/ui_actions"; import VolumeTracingReducer from "viewer/model/reducers/volumetracing_reducer"; import UiReducer from "viewer/model/reducers/ui_reducer"; import { describe, it, expect } from "vitest"; -import { initialState } from "test/fixtures/volumetracing_object"; +import { initialState, VOLUME_TRACING_ID } from "test/fixtures/volumetracing_object"; import type { WebknossosState, StoreAnnotation, VolumeTracing } from "viewer/store"; import { getActiveMagIndexForLayer } from "viewer/model/accessors/flycam_accessor"; @@ -113,7 +113,7 @@ describe("VolumeTracing", () => { LARGEST_SEGMENT_ID, ); const finishAnnotationStrokeAction = - VolumeTracingActions.finishAnnotationStrokeAction("tracingId"); + VolumeTracingActions.finishAnnotationStrokeAction(VOLUME_TRACING_ID); const alteredState = update(initialState, { annotation: { volumes: { @@ -160,7 +160,7 @@ describe("VolumeTracing", () => { }, }); - expect(getActiveMagIndexForLayer(alteredState, "tracingId") > 1).toBe(true); + expect(getActiveMagIndexForLayer(alteredState, VOLUME_TRACING_ID) > 1).toBe(true); // Try to change tool to Trace const newState = UiReducer(alteredState, setToolAction); @@ -230,7 +230,7 @@ describe("VolumeTracing", () => { }, }, }); - expect(getActiveMagIndexForLayer(alteredState, "tracingId") > 1).toBe(true); + expect(getActiveMagIndexForLayer(alteredState, VOLUME_TRACING_ID) > 1).toBe(true); const { newState, contourList } = prepareContourListTest(alteredState); expect(newState).not.toBe(initialState); diff --git a/frontend/javascripts/test/sagas/saga_integration.mock.ts b/frontend/javascripts/test/sagas/saga_integration.mock.ts deleted file mode 100644 index 55f7effb416..00000000000 --- a/frontend/javascripts/test/sagas/saga_integration.mock.ts +++ /dev/null @@ -1,30 +0,0 @@ -import "antd"; -import { vi } from "vitest"; - -vi.mock("antd", () => { - return { - 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() })); diff --git a/frontend/javascripts/test/sagas/saga_integration.spec.ts b/frontend/javascripts/test/sagas/saga_integration.spec.ts index a32cdeb5cda..5ec7b80c02a 100644 --- a/frontend/javascripts/test/sagas/saga_integration.spec.ts +++ b/frontend/javascripts/test/sagas/saga_integration.spec.ts @@ -1,15 +1,11 @@ import { describe, it, beforeEach, afterEach, expect } from "vitest"; -import "test/sagas/saga_integration.mock"; import { setupWebknossosForTesting, type WebknossosTestContext } from "test/helpers/apiHelpers"; import { createSaveQueueFromUpdateActions } from "test/helpers/saveHelpers"; import { enforceSkeletonTracing } from "viewer/model/accessors/skeletontracing_accessor"; import { getStats } from "viewer/model/accessors/annotation_accessor"; import { MAXIMUM_ACTION_COUNT_PER_BATCH } from "viewer/model/sagas/save_saga_constants"; -import { restartSagaAction, wkReadyAction } from "viewer/model/actions/actions"; import Store from "viewer/store"; import generateDummyTrees from "viewer/model/helpers/generate_dummy_trees"; -import { setActiveUserAction } from "viewer/model/actions/user_actions"; -import dummyUser from "test/fixtures/dummy_user"; import { hasRootSagaCrashed } from "viewer/model/sagas/root_saga"; import { omit } from "lodash"; @@ -28,16 +24,7 @@ import { TIMESTAMP } from "test/global_mocks"; describe("Saga Integration Tests", () => { beforeEach(async (context) => { - // Setup Webknossos - // this will execute model.fetch(...) and initialize the store with the tracing, etc. - Store.dispatch(restartSagaAction()); - Store.dispatch(discardSaveQueuesAction()); - Store.dispatch(setActiveUserAction(dummyUser)); - await setupWebknossosForTesting(context, "task"); - - // Dispatch the wkReadyAction, so the sagas are started - Store.dispatch(wkReadyAction()); }); afterEach(async (context) => { diff --git a/frontend/javascripts/test/sagas/save_saga.spec.ts b/frontend/javascripts/test/sagas/save_saga.spec.ts index 6f04d942477..bcd062da63f 100644 --- a/frontend/javascripts/test/sagas/save_saga.spec.ts +++ b/frontend/javascripts/test/sagas/save_saga.spec.ts @@ -16,9 +16,9 @@ import { sendSaveRequestToServer, toggleErrorHighlighting, addVersionNumbers, - sendRequestWithToken, } from "viewer/model/sagas/save_saga"; import { TIMESTAMP } from "test/global_mocks"; +import { sendSaveRequestWithToken } from "admin/rest_api"; vi.mock("viewer/model/sagas/root_saga", () => { return { @@ -139,7 +139,7 @@ describe("Save Saga", () => { expect, saga.next(TRACINGSTORE_URL), call( - sendRequestWithToken, + sendSaveRequestWithToken, `${TRACINGSTORE_URL}/tracings/annotation/${annotationId}/update?token=`, { method: "POST", @@ -162,7 +162,7 @@ describe("Save Saga", () => { const [saveQueueWithVersions, versionIncrement] = addVersionNumbers(saveQueue, LAST_VERSION); expect(versionIncrement).toBe(2); const requestWithTokenCall = call( - sendRequestWithToken, + sendSaveRequestWithToken, `${TRACINGSTORE_URL}/tracings/annotation/${annotationId}/update?token=`, { method: "POST", @@ -210,7 +210,7 @@ describe("Save Saga", () => { expect, saga.next(TRACINGSTORE_URL), call( - sendRequestWithToken, + sendSaveRequestWithToken, `${TRACINGSTORE_URL}/tracings/annotation/${annotationId}/update?token=`, { method: "POST", diff --git a/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts b/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts index 62f30a27823..a7ea2fe5152 100644 --- a/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts +++ b/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts @@ -1,37 +1,31 @@ +import { setupWebknossosForTesting, type WebknossosTestContext } from "test/helpers/apiHelpers"; +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import Store from "viewer/store"; +import { hasRootSagaCrashed } from "viewer/model/sagas/root_saga"; + import type { Flycam, SkeletonTracing, StoreAnnotation } from "viewer/store"; import { chainReduce } from "test/helpers/chainReducer"; import DiffableMap from "libs/diffable_map"; import EdgeCollection from "viewer/model/edge_collection"; import compactSaveQueue from "viewer/model/helpers/compaction/compact_save_queue"; import compactUpdateActions from "viewer/model/helpers/compaction/compact_update_actions"; -import { describe, it, expect, vi } from "vitest"; import defaultState from "viewer/default_state"; import update from "immutability-helper"; import { createSaveQueueFromUpdateActions, withoutUpdateTracing } from "../helpers/saveHelpers"; -import { expectValueDeepEqual, execCall } from "../helpers/sagaHelpers"; import { MISSING_GROUP_ID } from "viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view_helpers"; import { TreeTypeEnum } from "viewer/constants"; -import type { ServerSkeletonTracing } from "types/api_types"; import { enforceSkeletonTracing } from "viewer/model/accessors/skeletontracing_accessor"; import type { UpdateActionWithoutIsolationRequirement } from "viewer/model/sagas/update_actions"; import type { TracingStats } from "viewer/model/accessors/annotation_accessor"; import { diffSkeletonTracing } from "viewer/model/sagas/skeletontracing_saga"; -import { setupSavingForTracingType } from "viewer/model/sagas/save_saga"; import * as SkeletonTracingActions from "viewer/model/actions/skeletontracing_actions"; -import { pushSaveQueueTransaction } from "viewer/model/actions/save_actions"; import SkeletonTracingReducer from "viewer/model/reducers/skeletontracing_reducer"; -import { put } from "redux-saga/effects"; import { TIMESTAMP } from "test/global_mocks"; import { type Tree, TreeMap } from "viewer/model/types/tree_types"; +import { Model } from "viewer/singletons"; const actionTracingId = "tracingId"; -vi.mock("viewer/model/sagas/root_saga", () => ({ - default: function* () { - yield; - }, -})); - function testDiffing( prevAnnotation: StoreAnnotation, nextAnnotation: StoreAnnotation, @@ -101,29 +95,6 @@ const skeletonTracing: SkeletonTracing = { additionalAxes: [], }; -const serverSkeletonTracing: ServerSkeletonTracing = { - ...skeletonTracing, - id: skeletonTracing.tracingId, - editPosition: { - x: 0, - y: 0, - z: 0, - }, - editPositionAdditionalCoordinates: null, - editRotation: { - x: 0, - y: 0, - z: 0, - }, - additionalAxes: [], - zoomLevel: 2, - userBoundingBoxes: [], - typ: "Skeleton", - activeNodeId: undefined, - boundingBox: undefined, - trees: [], -}; - const initialState = update(defaultState, { annotation: { restrictions: { @@ -161,46 +132,37 @@ const createBranchPointAction = SkeletonTracingActions.createBranchPointAction( const applyActions = chainReduce(SkeletonTracingReducer); describe("SkeletonTracingSaga", () => { - it("shouldn't do anything if unchanged (saga test)", () => { - const saga = setupSavingForTracingType( - SkeletonTracingActions.initializeSkeletonTracingAction(serverSkeletonTracing), - ); - - saga.next(); - saga.next(initialState.annotation.skeleton); - saga.next(initialState.flycam); - saga.next(initialState.viewModeData.plane.tdCamera); - saga.next(); - saga.next(); - saga.next(true); - saga.next(initialState.annotation.skeleton); - saga.next(initialState.flycam); - - // only updateTracing - const items = execCall(expect, saga.next(initialState.viewModeData.plane.tdCamera)); - expect(withoutUpdateTracing(items).length).toBe(0); - }); - - it("should do something if changed (saga test)", () => { - const newState = SkeletonTracingReducer(initialState, createNodeAction); - const saga = setupSavingForTracingType( - SkeletonTracingActions.initializeSkeletonTracingAction(serverSkeletonTracing), - ); + describe("With Saga Middleware", () => { + beforeEach(async (context) => { + await setupWebknossosForTesting(context, "skeleton"); + }); - saga.next(); - saga.next(initialState.annotation.skeleton); - saga.next(initialState.flycam); - saga.next(initialState.viewModeData.plane.tdCamera); - saga.next(); - saga.next(); - saga.next(true); - saga.next(newState.annotation.skeleton); - saga.next(newState.flycam); + afterEach(async (context) => { + context.tearDownPullQueues(); + // Saving after each test and checking that the root saga didn't crash, + // ensures that each test is cleanly exited. Without it weird output can + // occur (e.g., a promise gets resolved which interferes with the next test). + expect(hasRootSagaCrashed()).toBe(false); + }); - const items = execCall(expect, saga.next(newState.viewModeData.plane.tdCamera)); + it("shouldn't do anything if unchanged (saga test)", async (context: WebknossosTestContext) => { + await Model.ensureSavedState(); + expect(context.receivedDataPerSaveRequest.length).toBe(0); + }); - expect(withoutUpdateTracing(items).length).toBeGreaterThan(0); - expectValueDeepEqual(expect, saga.next(items), put(pushSaveQueueTransaction(items))); + it("should do something if changed (saga test)", async (context: WebknossosTestContext) => { + Store.dispatch(createNodeAction); + await Model.ensureSavedState(); + expect(context.receivedDataPerSaveRequest.length).toBe(1); + const requestBatches = context.receivedDataPerSaveRequest[0]; + expect(requestBatches.length).toBe(1); + const updateBatch = requestBatches[0]; + expect(updateBatch.actions.map((action) => action.name)).toEqual([ + "createNode", + "createEdge", + "updateSkeletonTracing", + ]); + }); }); it("should emit createNode update actions", () => { diff --git a/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_helper.ts b/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_helper.ts index 448170bf99c..a8c00d67f5d 100644 --- a/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_helper.ts +++ b/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_helper.ts @@ -1,5 +1,4 @@ import _ from "lodash"; -import "test/sagas/saga_integration.mock"; import { createBucketResponseFunction, type WebknossosTestContext } from "test/helpers/apiHelpers"; import Store from "viewer/store"; import { AnnotationTool } from "viewer/model/accessors/tool_accessor"; diff --git a/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_with_saving.spec.ts b/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_with_saving.spec.ts index 7a1714ae1c4..430e833ceb5 100644 --- a/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_with_saving.spec.ts +++ b/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_with_saving.spec.ts @@ -1,31 +1,16 @@ import { vi, it, expect, beforeEach, describe } from "vitest"; import { waitForCondition } from "libs/utils"; -import "test/sagas/saga_integration.mock"; import { setupWebknossosForTesting, createBucketResponseFunction, type WebknossosTestContext, } from "test/helpers/apiHelpers"; -import { restartSagaAction, wkReadyAction } from "viewer/model/actions/actions"; -import Store from "viewer/store"; import { hasRootSagaCrashed } from "viewer/model/sagas/root_saga"; -import dummyUser from "test/fixtures/dummy_user"; -import { setActiveUserAction } from "viewer/model/actions/user_actions"; import { testLabelingManyBuckets } from "./bucket_eviction_helper"; -import { discardSaveQueuesAction } from "viewer/model/actions/save_actions"; describe("Bucket Eviction With Saving", () => { beforeEach(async (context) => { - // Setup Webknossos - // this will execute model.fetch(...) and initialize the store with the tracing, etc. - Store.dispatch(restartSagaAction()); - Store.dispatch(discardSaveQueuesAction()); - Store.dispatch(setActiveUserAction(dummyUser)); - await setupWebknossosForTesting(context, "volume"); - - // Dispatch the wkReadyAction, so the sagas are started - Store.dispatch(wkReadyAction()); }); it("Brushing/Tracing should not crash when too many buckets are labeled at once with saving in between", async (context) => { diff --git a/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_without_saving.spec.ts b/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_without_saving.spec.ts index b42319c13e1..070a03323bc 100644 --- a/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_without_saving.spec.ts +++ b/frontend/javascripts/test/sagas/volumetracing/bucket_eviction_without_saving.spec.ts @@ -1,29 +1,15 @@ import { vi, it, expect, beforeEach } from "vitest"; import { waitForCondition } from "libs/utils"; -import "test/sagas/saga_integration.mock"; import { setupWebknossosForTesting, createBucketResponseFunction, type WebknossosTestContext, } from "test/helpers/apiHelpers"; -import { restartSagaAction, wkReadyAction } from "viewer/model/actions/actions"; -import Store from "viewer/store"; import { hasRootSagaCrashed } from "viewer/model/sagas/root_saga"; -import dummyUser from "test/fixtures/dummy_user"; -import { setActiveUserAction } from "viewer/model/actions/user_actions"; import { testLabelingManyBuckets } from "./bucket_eviction_helper"; -import { discardSaveQueuesAction } from "viewer/model/actions/save_actions"; beforeEach(async (context) => { - // Setup webknossos, this will execute model.fetch(...) and initialize the store with the tracing, etc. - Store.dispatch(restartSagaAction()); - Store.dispatch(discardSaveQueuesAction()); - Store.dispatch(setActiveUserAction(dummyUser)); - await setupWebknossosForTesting(context, "volume"); - - // Dispatch the wkReadyAction, so the sagas are started - Store.dispatch(wkReadyAction()); }); it("Brushing/Tracing should not crash when a lot of buckets are labeled at once without saving in between", async (context) => { diff --git a/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts b/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts index 4b29f1a3018..30014e20e68 100644 --- a/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts +++ b/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts @@ -1,7 +1,7 @@ -import { vi, it, expect, describe } from "vitest"; +import { it, expect, describe, beforeEach, afterEach } from "vitest"; +import { setupWebknossosForTesting, type WebknossosTestContext } from "test/helpers/apiHelpers"; import { take, put, call } from "redux-saga/effects"; -import update from "immutability-helper"; -import type { APISegmentationLayer, ServerVolumeTracing } from "types/api_types"; +import type { ServerVolumeTracing } from "types/api_types"; import { AnnotationTool } from "viewer/model/accessors/tool_accessor"; import { OrthoViews, @@ -9,17 +9,10 @@ import { OverwriteModeEnum, MappingStatusEnum, } from "viewer/constants"; -import { convertFrontendBoundingBoxToServer } from "viewer/model/reducers/reducer_helpers"; -import { enforce } from "libs/utils"; -import { pushSaveQueueTransaction } from "viewer/model/actions/save_actions"; import * as VolumeTracingActions from "viewer/model/actions/volumetracing_actions"; -import VolumeTracingReducer from "viewer/model/reducers/volumetracing_reducer"; -import defaultState from "viewer/default_state"; import { expectValueDeepEqual, execCall } from "test/helpers/sagaHelpers"; -import { withoutUpdateTracing } from "test/helpers/saveHelpers"; import type { ActiveMappingInfo } from "viewer/store"; import { askUserForLockingActiveMapping } from "viewer/model/sagas/saga_helpers"; -import { setupSavingForTracingType } from "viewer/model/sagas/save_saga"; import { editVolumeLayerAsync, finishLayer } from "viewer/model/sagas/volumetracing_saga"; import { requestBucketModificationInVolumeTracing, @@ -27,13 +20,8 @@ import { } from "viewer/model/sagas/saga_helpers"; import VolumeLayer from "viewer/model/volumetracing/volumelayer"; import { serverVolumeToClientVolumeTracing } from "viewer/model/reducers/volumetracing_reducer"; - -// Mock dependencies -vi.mock("viewer/model/sagas/root_saga", () => ({ - default: function* () { - yield; - }, -})); +import { Model, Store } from "viewer/singletons"; +import { hasRootSagaCrashed } from "viewer/model/sagas/root_saga"; const serverVolumeTracing: ServerVolumeTracing = { typ: "Volume", @@ -70,41 +58,6 @@ const serverVolumeTracing: ServerVolumeTracing = { }; const volumeTracing = serverVolumeToClientVolumeTracing(serverVolumeTracing); -const volumeTracingLayer: APISegmentationLayer = { - name: volumeTracing.tracingId, - category: "segmentation", - boundingBox: enforce(convertFrontendBoundingBoxToServer)(volumeTracing.boundingBox), - resolutions: [[1, 1, 1]], - elementClass: serverVolumeTracing.elementClass, - largestSegmentId: serverVolumeTracing.largestSegmentId, - tracingId: volumeTracing.tracingId, - additionalAxes: [], -}; - -const initialState = update(defaultState, { - annotation: { - volumes: { - $set: [volumeTracing], - }, - }, - dataset: { - dataSource: { - dataLayers: { - $set: [volumeTracingLayer], - }, - }, - }, - datasetConfiguration: { - layers: { - $set: { - [volumeTracing.tracingId]: { - isDisabled: false, - alpha: 100, - }, - }, - }, - }, -}); const dummyActiveMapping: ActiveMappingInfo = { mappingName: "dummy-mapping-name", @@ -124,46 +77,42 @@ const addToLayerActionFn = VolumeTracingActions.addToLayerAction; const finishEditingAction = VolumeTracingActions.finishEditingAction(); describe("VolumeTracingSaga", () => { - it("shouldn't do anything if unchanged (saga test)", () => { - const saga = setupSavingForTracingType( - VolumeTracingActions.initializeVolumeTracingAction(serverVolumeTracing), - ); - - saga.next(); - saga.next(initialState.annotation.volumes[0]); - saga.next(initialState.flycam); - saga.next(initialState.viewModeData.plane.tdCamera); - saga.next(); - saga.next(); - saga.next(true); - saga.next(initialState.annotation.volumes[0]); - saga.next(initialState.flycam); - // only updateTracing - const items = execCall(expect, saga.next(initialState.viewModeData.plane.tdCamera)); - expect(withoutUpdateTracing(items).length).toBe(0); - }); - - it("should do something if changed (saga test)", () => { - const newState = VolumeTracingReducer(initialState, setActiveCellAction); - const saga = setupSavingForTracingType( - VolumeTracingActions.initializeVolumeTracingAction(serverVolumeTracing), - ); + describe("With Saga Middleware", () => { + beforeEach(async (context) => { + await setupWebknossosForTesting(context, "volume"); + }); - saga.next(); - saga.next(initialState.annotation.volumes[0]); - saga.next(initialState.flycam); - saga.next(initialState.viewModeData.plane.tdCamera); - saga.next(); - saga.next(); - saga.next(true); - saga.next(newState.annotation.volumes[0]); - saga.next(newState.flycam); + afterEach(async (context) => { + context.tearDownPullQueues(); + // Saving after each test and checking that the root saga didn't crash, + // ensures that each test is cleanly exited. Without it weird output can + // occur (e.g., a promise gets resolved which interferes with the next test). + expect(hasRootSagaCrashed()).toBe(false); + }); - const items = execCall(expect, saga.next(newState.viewModeData.plane.tdCamera)); + it("shouldn't do anything if unchanged (saga test)", async (context: WebknossosTestContext) => { + await Model.ensureSavedState(); + expect(context.receivedDataPerSaveRequest.length).toBe(0); + }); - expect(withoutUpdateTracing(items).length).toBe(0); - expect(items[0].value.activeSegmentId).toBe(ACTIVE_CELL_ID); - expectValueDeepEqual(expect, saga.next(items), put(pushSaveQueueTransaction(items))); + it("should do something if changed (saga test)", async (context: WebknossosTestContext) => { + Store.dispatch(setActiveCellAction); + await Model.ensureSavedState(); + expect(context.receivedDataPerSaveRequest.length).toBe(1); + const requestBatches = context.receivedDataPerSaveRequest[0]; + expect(requestBatches.length).toBe(1); + const updateBatch = requestBatches[0]; + expect(updateBatch.actions.map((action) => action.name)).toEqual(["updateVolumeTracing"]); + const action = updateBatch.actions[0]; + + expect(action).toMatchObject({ + name: "updateVolumeTracing", + value: { + actionTracingId: "volumeTracingId-1234", + activeSegmentId: 5, + }, + }); + }); }); it("should create a volume layer (saga test)", () => { diff --git a/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_1.spec.ts b/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_1.spec.ts index 11dca413af8..348caaa01c9 100644 --- a/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_1.spec.ts +++ b/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_1.spec.ts @@ -3,7 +3,6 @@ * The tests are split into two modules to allow for isolated parallelization and thus * increased performance. */ -import "test/sagas/saga_integration.mock"; import _ from "lodash"; import { AnnotationTool } from "viewer/model/accessors/tool_accessor"; import { ContourModeEnum, OrthoViews, OverwriteModeEnum, type Vector3 } from "viewer/constants"; @@ -13,11 +12,8 @@ import { type WebknossosTestContext, } from "test/helpers/apiHelpers"; import { hasRootSagaCrashed } from "viewer/model/sagas/root_saga"; -import { restartSagaAction, wkReadyAction } from "viewer/model/actions/actions"; import { updateUserSettingAction } from "viewer/model/actions/settings_actions"; import Store from "viewer/store"; -import dummyUser from "test/fixtures/dummy_user"; -import { setActiveUserAction } from "viewer/model/actions/user_actions"; import { batchUpdateGroupsAndSegmentsAction, clickSegmentAction, @@ -32,29 +28,13 @@ import { } from "viewer/model/actions/volumetracing_actions"; import { MISSING_GROUP_ID } from "viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view_helpers"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { - dispatchUndoAsync, - dispatchRedoAsync, - discardSaveQueuesAction, -} from "viewer/model/actions/save_actions"; +import { dispatchUndoAsync, dispatchRedoAsync } from "viewer/model/actions/save_actions"; import { setPositionAction, setZoomStepAction } from "viewer/model/actions/flycam_actions"; import { setToolAction } from "viewer/model/actions/ui_actions"; describe("Volume Tracing", () => { beforeEach(async (context) => { - // Setup Webknossos - // this will execute model.fetch(...) and initialize the store with the tracing, etc. - Store.dispatch(restartSagaAction()); - Store.dispatch(discardSaveQueuesAction()); - Store.dispatch(setActiveUserAction(dummyUser)); - await setupWebknossosForTesting(context, "volume"); - - // Ensure the slow compression is disabled by default. Tests may change - // this individually. - context.setSlowCompression(false); - // Dispatch the wkReadyAction, so the sagas are started - Store.dispatch(wkReadyAction()); }); afterEach(async (context) => { diff --git a/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_2.spec.ts b/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_2.spec.ts index f626298e1c3..d080ebb1d80 100644 --- a/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_2.spec.ts +++ b/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_2.spec.ts @@ -1,4 +1,3 @@ -import "test/sagas/saga_integration.mock"; import { AnnotationTool } from "viewer/model/accessors/tool_accessor"; import Constants, { ContourModeEnum, @@ -13,12 +12,9 @@ import { type WebknossosTestContext, } from "test/helpers/apiHelpers"; import { hasRootSagaCrashed } from "viewer/model/sagas/root_saga"; -import { restartSagaAction, wkReadyAction } from "viewer/model/actions/actions"; import { updateUserSettingAction } from "viewer/model/actions/settings_actions"; import Store from "viewer/store"; import { V3 } from "libs/mjs"; -import dummyUser from "test/fixtures/dummy_user"; -import { setActiveUserAction } from "viewer/model/actions/user_actions"; import { setActiveCellAction, addToLayerAction, @@ -29,29 +25,13 @@ import { } from "viewer/model/actions/volumetracing_actions"; import type { DataBucket } from "viewer/model/bucket_data_handling/bucket"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { - dispatchUndoAsync, - dispatchRedoAsync, - discardSaveQueuesAction, -} from "viewer/model/actions/save_actions"; +import { dispatchUndoAsync, dispatchRedoAsync } from "viewer/model/actions/save_actions"; import { setPositionAction, setZoomStepAction } from "viewer/model/actions/flycam_actions"; import { setToolAction } from "viewer/model/actions/ui_actions"; describe("Volume Tracing", () => { beforeEach(async (context) => { - // Setup Webknossos - // this will execute model.fetch(...) and initialize the store with the tracing, etc. - Store.dispatch(restartSagaAction()); - Store.dispatch(discardSaveQueuesAction()); - Store.dispatch(setActiveUserAction(dummyUser)); - await setupWebknossosForTesting(context, "volume"); - - // Ensure the slow compression is disabled by default. Tests may change - // this individually. - context.setSlowCompression(false); - // Dispatch the wkReadyAction, so the sagas are started - Store.dispatch(wkReadyAction()); }); afterEach(async (context) => { diff --git a/frontend/javascripts/viewer/model.ts b/frontend/javascripts/viewer/model.ts index 44b85bf8689..42106462a3b 100644 --- a/frontend/javascripts/viewer/model.ts +++ b/frontend/javascripts/viewer/model.ts @@ -10,7 +10,10 @@ import { } from "viewer/model/accessors/dataset_accessor"; import { getActiveMagIndexForLayer } from "viewer/model/accessors/flycam_accessor"; import { getActiveSegmentationTracingLayer } from "viewer/model/accessors/volumetracing_accessor"; -import { saveNowAction } from "viewer/model/actions/save_actions"; +import { + ensureTracingsWereDiffedToSaveQueueAction, + saveNowAction, +} from "viewer/model/actions/save_actions"; import type DataCube from "viewer/model/bucket_data_handling/data_cube"; import type LayerRenderingManager from "viewer/model/bucket_data_handling/layer_rendering_manager"; import type PullQueue from "viewer/model/bucket_data_handling/pullqueue"; @@ -19,6 +22,7 @@ import { getTotalSaveQueueLength } from "viewer/model/reducers/save_reducer"; import type { TraceOrViewCommand } from "viewer/store"; import Store from "viewer/store"; +import Deferred from "libs/async/deferred"; import { globalToLayerTransformedPosition } from "./model/accessors/dataset_layer_transformation_accessor"; import { initialize } from "./model_initialization"; @@ -340,9 +344,38 @@ export class WebKnossosModel { }; ensureSavedState = async () => { - // This function will only return once all state is saved - // even if new updates are pushed to the save queue during saving - while (!this.stateSaved()) { + /* This function will only return once all state is saved + * even if new updates are pushed to the save queue during saving + */ + + // This is a helper function which ensures that the diffing sagas + // have seen the newest tracings. These sagas are responsible for diffing + // old vs. new tracings to fill the save queue with update actions. + // waitForDifferResponses dispatches a special action which the diffing sagas + // will respond to by calling the callback that is attached to the action. + // That way, we can be sure that the diffing sagas have processed all user actions + // up until the time of where waitForDifferResponses was invoked. + async function waitForDifferResponses() { + const { annotation } = Store.getState(); + // All skeleton and volume tracings should respond to the dispatched action. + const tracingIds = new Set( + _.compact([annotation.skeleton?.tracingId, ...annotation.volumes.map((t) => t.tracingId)]), + ); + const reportedTracingIds = new Set(); + const deferred = new Deferred(); + function callback(tracingId: string) { + reportedTracingIds.add(tracingId); + if (Utils.areSetsEqual(tracingIds, reportedTracingIds)) { + deferred.resolve(null); + } + } + + Store.dispatch(ensureTracingsWereDiffedToSaveQueueAction(callback)); + await deferred.promise(); + return true; + } + + while ((await waitForDifferResponses()) && !this.stateSaved()) { // The dispatch of the saveNowAction IN the while loop is deliberate. // Otherwise if an update action is pushed to the save queue during the Utils.sleep, // the while loop would continue running until the next save would be triggered. diff --git a/frontend/javascripts/viewer/model/actions/save_actions.ts b/frontend/javascripts/viewer/model/actions/save_actions.ts index 5544e65ff0f..2335b626c59 100644 --- a/frontend/javascripts/viewer/model/actions/save_actions.ts +++ b/frontend/javascripts/viewer/model/actions/save_actions.ts @@ -23,6 +23,9 @@ export type SetVersionNumberAction = ReturnType; export type UndoAction = ReturnType; export type RedoAction = ReturnType; type DisableSavingAction = ReturnType; +export type EnsureTracingsWereDiffedToSaveQueueAction = ReturnType< + typeof ensureTracingsWereDiffedToSaveQueueAction +>; export type SaveAction = | PushSaveQueueTransaction @@ -34,7 +37,8 @@ export type SaveAction = | SetVersionNumberAction | UndoAction | RedoAction - | DisableSavingAction; + | DisableSavingAction + | EnsureTracingsWereDiffedToSaveQueueAction; // The action creators pushSaveQueueTransaction and pushSaveQueueTransactionIsolated // are typed so that update actions that need isolation are isolated in a group each. @@ -121,3 +125,10 @@ export const dispatchRedoAsync = async (dispatch: Dispatch): Promise dispatch(action); await readyDeferred.promise(); }; + +// See Model.ensureSavedState for an explanation of this action. +export const ensureTracingsWereDiffedToSaveQueueAction = (callback: (tracingId: string) => void) => + ({ + type: "ENSURE_TRACINGS_WERE_DIFFED_TO_SAVE_QUEUE", + callback, + }) as const; diff --git a/frontend/javascripts/viewer/model/sagas/annotation_saga.tsx b/frontend/javascripts/viewer/model/sagas/annotation_saga.tsx index 02a50e4d6d5..bf0be52f494 100644 --- a/frontend/javascripts/viewer/model/sagas/annotation_saga.tsx +++ b/frontend/javascripts/viewer/model/sagas/annotation_saga.tsx @@ -311,6 +311,13 @@ export function* acquireAnnotationMutexMaybe(): Saga { onMutexStateChanged(canEdit, blockedByUser); } } catch (error) { + if (process.env.IS_TESTING) { + // In unit tests, that explicitly control this generator function, + // the console.error after the next yield won't be printed, because + // test assertions on the yield will already throw. + // Therefore, we also print the error in the test context. + console.error("Error while trying to acquire mutex:", error); + } const wasCanceled = yield* cancelled(); if (!wasCanceled) { console.error("Error while trying to acquire mutex.", error); diff --git a/frontend/javascripts/viewer/model/sagas/save_saga.ts b/frontend/javascripts/viewer/model/sagas/save_saga.ts index 2d3c37eebbb..e884f3fbaaf 100644 --- a/frontend/javascripts/viewer/model/sagas/save_saga.ts +++ b/frontend/javascripts/viewer/model/sagas/save_saga.ts @@ -1,20 +1,20 @@ -import { doWithToken, getNewestVersionForAnnotation } from "admin/rest_api"; +import { getNewestVersionForAnnotation, sendSaveRequestWithToken } from "admin/rest_api"; import Date from "libs/date"; import ErrorHandling from "libs/error_handling"; -import type { RequestOptionsWithData } from "libs/request"; -import Request from "libs/request"; import Toast from "libs/toast"; import { sleep } from "libs/utils"; import window, { alert, document, location } from "libs/window"; import _ from "lodash"; import memoizeOne from "memoize-one"; import messages from "messages"; -import { call, delay, fork, put, race, take } from "typed-redux-saga"; +import { buffers } from "redux-saga"; +import { actionChannel, call, delay, fork, put, race, take } from "typed-redux-saga"; import { ControlModeEnum } from "viewer/constants"; import { getMagInfo } from "viewer/model/accessors/dataset_accessor"; import { selectTracing } from "viewer/model/accessors/tracing_accessor"; import { FlycamActions } from "viewer/model/actions/flycam_actions"; import { + type EnsureTracingsWereDiffedToSaveQueueAction, pushSaveQueueTransaction, setLastSaveTimestampAction, setSaveBusyAction, @@ -54,6 +54,7 @@ import type { SkeletonTracing, VolumeTracing, } from "viewer/store"; +import type { Action } from "../actions/actions"; import { takeEveryWithBatchActionSupport } from "./saga_helpers"; const ONE_YEAR_MS = 365 * 24 * 3600 * 1000; @@ -122,12 +123,6 @@ export function* pushSaveQueueAsync(): Saga { yield* put(setSaveBusyAction(false)); } } -export function sendRequestWithToken( - urlWithoutToken: string, - data: RequestOptionsWithData>, -): Promise { - return doWithToken((token) => Request.sendJSONReceiveJSON(`${urlWithoutToken}${token}`, data)); -} // This function returns the first n batches of the provided array, so that the count of // all actions in these n batches does not exceed MAXIMUM_ACTION_COUNT_PER_SAVE @@ -181,7 +176,7 @@ export function* sendSaveRequestToServer(): Saga { try { const startTime = Date.now(); yield* call( - sendRequestWithToken, + sendSaveRequestWithToken, `${tracingStoreUrl}/tracings/annotation/${annotationId}/update?token=`, { method: "POST", @@ -385,32 +380,52 @@ export function* setupSavingForTracingType( old and new state. The actual push to the server is done by the forked pushSaveQueueAsync saga. */ - const saveQueueType = + const tracingType = initializeAction.type === "INITIALIZE_SKELETONTRACING" ? "skeleton" : "volume"; const tracingId = initializeAction.tracing.id; - let prevTracing = (yield* select((state) => selectTracing(state, saveQueueType, tracingId))) as + let prevTracing = (yield* select((state) => selectTracing(state, tracingType, tracingId))) as | VolumeTracing | SkeletonTracing; let prevFlycam = yield* select((state) => state.flycam); let prevTdCamera = yield* select((state) => state.viewModeData.plane.tdCamera); yield* call(ensureWkReady); + const actionBuffer = buffers.expanding(); + const tracingActionChannel = yield* actionChannel( + tracingType === "skeleton" + ? [ + ...SkeletonTracingSaveRelevantActions, + ...FlycamActions, + ...ViewModeSaveRelevantActions, + // SET_TRACING is not included in SkeletonTracingSaveRelevantActions, because it is used by Undo/Redo and + // should not create its own Undo/Redo stack entry + "SET_TRACING", + ] + : [...VolumeTracingSaveRelevantActions, ...FlycamActions, ...ViewModeSaveRelevantActions], + actionBuffer, + ); + + // See Model.ensureSavedState for an explanation of this action channel. + const ensureDiffedChannel = yield* actionChannel( + "ENSURE_TRACINGS_WERE_DIFFED_TO_SAVE_QUEUE", + ); + while (true) { - if (saveQueueType === "skeleton") { - yield* take([ - ...SkeletonTracingSaveRelevantActions, - ...FlycamActions, - ...ViewModeSaveRelevantActions, - // SET_TRACING is not included in SkeletonTracingSaveRelevantActions, because it is used by Undo/Redo and - // should not create its own Undo/Redo stack entry - "SET_TRACING", - ]); + // Prioritize consumption of tracingActionChannel since we don't want to + // reply to the ENSURE_TRACINGS_WERE_DIFFED_TO_SAVE_QUEUE action if there + // are unprocessed user actions. + if (!actionBuffer.isEmpty()) { + yield* take(tracingActionChannel); } else { - yield* take([ - ...VolumeTracingSaveRelevantActions, - ...FlycamActions, - ...ViewModeSaveRelevantActions, - ]); + // Wait for either a user action or the "ensureAction". + const { ensureAction } = yield* race({ + _tracingAction: take(tracingActionChannel), + ensureAction: take(ensureDiffedChannel), + }); + if (ensureAction != null) { + ensureAction.callback(tracingId); + continue; + } } // The allowUpdate setting could have changed in the meantime @@ -419,7 +434,7 @@ export function* setupSavingForTracingType( state.annotation.restrictions.allowUpdate && state.annotation.restrictions.allowSave, ); if (!allowUpdate) continue; - const tracing = (yield* select((state) => selectTracing(state, saveQueueType, tracingId))) as + const tracing = (yield* select((state) => selectTracing(state, tracingType, tracingId))) as | VolumeTracing | SkeletonTracing; const flycam = yield* select((state) => state.flycam);