Skip to content

Commit 33de8ec

Browse files
committed
refactor mutex acquisition to use less state
1 parent 0f41c4b commit 33de8ec

File tree

1 file changed

+31
-35
lines changed

1 file changed

+31
-35
lines changed

frontend/javascripts/viewer/model/sagas/saving/save_mutex_saga.tsx

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,52 +25,53 @@ const DISABLE_EAGER_MUTEX_ACQUISITION = true;
2525

2626
type MutexLogicState = {
2727
isInitialRequest: boolean;
28-
doesHaveMutexFromBeginning: boolean;
29-
shallTryAcquireMutex: boolean;
3028
};
3129

3230
function* getDoesHaveMutex(): Saga<boolean> {
3331
return yield* select((state) => state.annotation.isMutexAcquired);
3432
}
3533

3634
export function* acquireAnnotationMutexMaybe(): Saga<void> {
37-
yield* call(ensureWkReady);
38-
yield* fork(watchMutexStateChanges);
3935
if (DISABLE_EAGER_MUTEX_ACQUISITION) {
4036
return;
4137
}
42-
const allowUpdate = yield* select((state) => state.annotation.restrictions.allowUpdate);
43-
if (!allowUpdate) {
38+
const initialAllowUpdate = yield* select(
39+
(state) => state.annotation.restrictions.initialAllowUpdate,
40+
);
41+
if (!initialAllowUpdate) {
42+
// We are in an read-only annotation.
4443
return;
4544
}
46-
const othersMayEdit = yield* select((state) => state.annotation.othersMayEdit);
47-
4845
const mutexLogicState: MutexLogicState = {
4946
isInitialRequest: true,
50-
doesHaveMutexFromBeginning: false,
51-
shallTryAcquireMutex: othersMayEdit,
5247
};
5348

49+
yield* call(ensureWkReady);
50+
yield* fork(watchMutexStateChangesForNotification, mutexLogicState);
51+
5452
let runningTryAcquireMutexContinuouslySaga = yield* fork(
5553
tryAcquireMutexContinuously,
5654
mutexLogicState,
5755
);
5856
function* reactToOthersMayEditChanges({
5957
othersMayEdit,
6058
}: SetOthersMayEditForAnnotationAction): Saga<void> {
61-
mutexLogicState.shallTryAcquireMutex = othersMayEdit;
62-
if (mutexLogicState.shallTryAcquireMutex) {
59+
if (othersMayEdit) {
6360
if (runningTryAcquireMutexContinuouslySaga != null) {
6461
yield* cancel(runningTryAcquireMutexContinuouslySaga);
6562
}
66-
mutexLogicState.isInitialRequest = true;
6763
runningTryAcquireMutexContinuouslySaga = yield* fork(
6864
tryAcquireMutexContinuously,
6965
mutexLogicState,
7066
);
7167
} else {
7268
// othersMayEdit was turned off. The user editing it should be able to edit the annotation.
73-
yield* put(setAnnotationAllowUpdateAction(true));
69+
// let's check that owner === activeUser, anyway.
70+
const owner = yield* select((storeState) => storeState.annotation.owner);
71+
const activeUser = yield* select((state) => state.activeUser);
72+
if (activeUser && owner?.id === activeUser?.id) {
73+
yield* put(setAnnotationAllowUpdateAction(true));
74+
}
7475
}
7576
}
7677
yield* takeEvery("SET_OTHERS_MAY_EDIT_FOR_ANNOTATION", reactToOthersMayEditChanges);
@@ -79,9 +80,15 @@ export function* acquireAnnotationMutexMaybe(): Saga<void> {
7980
function* tryAcquireMutexContinuously(mutexLogicState: MutexLogicState): Saga<void> {
8081
const annotationId = yield* select((storeState) => storeState.annotation.annotationId);
8182
const activeUser = yield* select((state) => state.activeUser);
83+
mutexLogicState.isInitialRequest = true;
8284

83-
while (mutexLogicState.shallTryAcquireMutex) {
84-
if (mutexLogicState.isInitialRequest) {
85+
// We can simply use an infinite loop here, because the saga will be cancelled by
86+
// reactToOthersMayEditChanges when othersMayEdit is set to false.
87+
while (true) {
88+
const blockedByUser = yield* select((state) => state.annotation.blockedByUser);
89+
if (blockedByUser == null || blockedByUser.id !== activeUser?.id) {
90+
// If the annotation is currently not blocked by the active user,
91+
// we immediately disallow updating the annotation.
8592
yield* put(setAnnotationAllowUpdateAction(false));
8693
}
8794
try {
@@ -91,20 +98,12 @@ function* tryAcquireMutexContinuously(mutexLogicState: MutexLogicState): Saga<vo
9198
acquireAnnotationMutex,
9299
annotationId,
93100
);
94-
if (mutexLogicState.isInitialRequest && canEdit) {
95-
mutexLogicState.doesHaveMutexFromBeginning = true;
96-
// Only set allow update to true in case the first try to get the mutex succeeded.
97-
yield* put(setAnnotationAllowUpdateAction(true));
98-
}
99-
if (!canEdit || !mutexLogicState.doesHaveMutexFromBeginning) {
100-
// If the mutex could not be acquired anymore or the user does not have it from the beginning, set allow update to false.
101-
mutexLogicState.doesHaveMutexFromBeginning = false;
102-
yield* put(setAnnotationAllowUpdateAction(false));
103-
}
104-
101+
yield* put(setAnnotationAllowUpdateAction(canEdit));
105102
yield* put(setBlockedByUserAction(canEdit ? activeUser : blockedByUser));
106103

107-
if (canEdit !== (yield* call(getDoesHaveMutex)) || mutexLogicState.isInitialRequest) {
104+
if (canEdit !== (yield* call(getDoesHaveMutex))) {
105+
// Only dispatch the action if it changes the store to avoid
106+
// unnecessary notifications.
108107
yield* put(setIsMutexAcquiredAction(canEdit));
109108
}
110109
} catch (error) {
@@ -120,8 +119,7 @@ function* tryAcquireMutexContinuously(mutexLogicState: MutexLogicState): Saga<vo
120119
console.error("Error while trying to acquire mutex.", error);
121120
yield* put(setBlockedByUserAction(undefined));
122121
yield* put(setAnnotationAllowUpdateAction(false));
123-
mutexLogicState.doesHaveMutexFromBeginning = false;
124-
if ((yield* call(getDoesHaveMutex)) || mutexLogicState.isInitialRequest) {
122+
if (yield* call(getDoesHaveMutex)) {
125123
yield* put(setIsMutexAcquiredAction(false));
126124
}
127125
}
@@ -131,15 +129,13 @@ function* tryAcquireMutexContinuously(mutexLogicState: MutexLogicState): Saga<vo
131129
}
132130
}
133131

134-
function* watchMutexStateChanges(): Saga<void> {
135-
// todop: wrong?
136-
let isInitialRequest = true;
132+
function* watchMutexStateChangesForNotification(mutexLogicState: MutexLogicState): Saga<void> {
137133
yield* takeEvery(
138134
"SET_IS_MUTEX_ACQUIRED",
139135
function* ({ isMutexAcquired }: SetIsMutexAcquiredAction) {
140136
if (isMutexAcquired) {
141137
Toast.close(MUTEX_NOT_ACQUIRED_KEY);
142-
if (!isInitialRequest) {
138+
if (!mutexLogicState.isInitialRequest) {
143139
const message = (
144140
<React.Fragment>
145141
{messages["annotation.acquiringMutexSucceeded"]}" "
@@ -159,7 +155,7 @@ function* watchMutexStateChanges(): Saga<void> {
159155
: messages["annotation.acquiringMutexFailed.noUser"];
160156
Toast.warning(message, { sticky: true, key: MUTEX_NOT_ACQUIRED_KEY });
161157
}
162-
isInitialRequest = false;
158+
mutexLogicState.isInitialRequest = false;
163159
},
164160
);
165161
}

0 commit comments

Comments
 (0)