Skip to content

Commit 19640d7

Browse files
krassowskiandrii-i
authored andcommitted
Fix spurious "File Changed" dialog in JupyterLab 4.1+/Notebook 7.1+ (jupyterlab#337)
* Save current hash on the document * Use public API now that PR 262 in `jupyter_ydoc` is merged * Use a clean solution which requires a change in JupyterLab * `jupyter-server-ydoc`: require `jupyter-server` 2.11.1 or newer As only 2.11.1 added `require_hash` argument
1 parent b718ec2 commit 19640d7

File tree

4 files changed

+66
-6
lines changed

4 files changed

+66
-6
lines changed

packages/docprovider/src/ydrive.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import { PageConfig, URLExt } from '@jupyterlab/coreutils';
55
import { TranslationBundle } from '@jupyterlab/translation';
66
import { Contents, Drive, User } from '@jupyterlab/services';
7+
import { ISignal, Signal } from '@lumino/signaling';
78

89
import { DocumentChange, ISharedDocument, YDocument } from '@jupyter/ydoc';
910

@@ -45,6 +46,10 @@ export class YDrive extends Drive implements ICollaborativeDrive {
4546
this._providers = new Map<string, WebSocketProvider>();
4647

4748
this.sharedModelFactory = new SharedModelFactory(this._onCreate);
49+
super.fileChanged.connect((_, change) => {
50+
// pass through any events from the Drive superclass
51+
this._ydriveFileChanged.emit(change);
52+
});
4853
}
4954

5055
/**
@@ -84,7 +89,7 @@ export class YDrive extends Drive implements ICollaborativeDrive {
8489
const provider = this._providers.get(key);
8590

8691
if (provider) {
87-
// If the document does't exist, `super.get` will reject with an
92+
// If the document doesn't exist, `super.get` will reject with an
8893
// error and the provider will never be resolved.
8994
// Use `Promise.all` to reject as soon as possible. The Context will
9095
// show a dialog to the user.
@@ -132,6 +137,13 @@ export class YDrive extends Drive implements ICollaborativeDrive {
132137
return super.save(localPath, options);
133138
}
134139

140+
/**
141+
* A signal emitted when a file operation takes place.
142+
*/
143+
get fileChanged(): ISignal<this, Contents.IChangedArgs> {
144+
return this._ydriveFileChanged;
145+
}
146+
135147
private _onCreate = (
136148
options: Contents.ISharedFactoryOptions,
137149
sharedModel: YDocument<DocumentChange>
@@ -161,6 +173,37 @@ export class YDrive extends Drive implements ICollaborativeDrive {
161173
const key = `${options.format}:${options.contentType}:${options.path}`;
162174
this._providers.set(key, provider);
163175

176+
sharedModel.changed.connect(async (_, change) => {
177+
if (!change.stateChange) {
178+
return;
179+
}
180+
const hashChanges = change.stateChange.filter(
181+
change => change.name === 'hash'
182+
);
183+
if (hashChanges.length === 0) {
184+
return;
185+
}
186+
if (hashChanges.length > 1) {
187+
console.error(
188+
'Unexpected multiple changes to hash value in a single transaction'
189+
);
190+
}
191+
const hashChange = hashChanges[0];
192+
193+
// A change in hash signifies that a save occurred on the server-side
194+
// (e.g. a collaborator performed the save) - we want to notify the
195+
// observers about this change so that they can store the new hash value.
196+
const model = await this.get(options.path, { content: false });
197+
198+
this._ydriveFileChanged.emit({
199+
type: 'save',
200+
newValue: { ...model, hash: hashChange.newValue },
201+
// we do not have the old model because it was discarded when server made the change,
202+
// we only have the old hash here (which may be empty if the file was newly created!)
203+
oldValue: { hash: hashChange.oldValue }
204+
});
205+
});
206+
164207
sharedModel.disposed.connect(() => {
165208
const provider = this._providers.get(key);
166209
if (provider) {
@@ -190,6 +233,7 @@ export class YDrive extends Drive implements ICollaborativeDrive {
190233
private _trans: TranslationBundle;
191234
private _providers: Map<string, WebSocketProvider>;
192235
private _globalAwareness: Awareness | null;
236+
private _ydriveFileChanged = new Signal<this, Contents.IChangedArgs>(this);
193237
}
194238

195239
/**

projects/jupyter-server-ydoc/jupyter_server_ydoc/loaders.py

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ async def load_content(self, format: str, file_type: str) -> dict[str, Any]:
117117
self.last_modified = model["last_modified"]
118118
return model
119119

120-
async def maybe_save_content(self, model: dict[str, Any]) -> None:
120+
async def maybe_save_content(self, model: dict[str, Any]) -> dict[str, Any] | None:
121121
"""
122122
Save the content of the file.
123123
@@ -149,20 +149,34 @@ async def maybe_save_content(self, model: dict[str, Any]) -> None:
149149
# otherwise it could corrupt the file
150150
done_saving = asyncio.Event()
151151
task = asyncio.create_task(self._save_content(model, done_saving))
152+
saved_model = None
152153
try:
153-
await asyncio.shield(task)
154+
saved_model = await asyncio.shield(task)
154155
except asyncio.CancelledError:
155156
pass
156157
await done_saving.wait()
158+
return saved_model
157159
else:
158160
# file changed on disk, raise an error
159161
self.last_modified = m["last_modified"]
160162
raise OutOfBandChanges
161163

162-
async def _save_content(self, model: dict[str, Any], done_saving: asyncio.Event) -> None:
164+
async def _save_content(
165+
self, model: dict[str, Any], done_saving: asyncio.Event
166+
) -> dict[str, Any]:
163167
try:
164168
m = await ensure_async(self._contents_manager.save(model, self.path))
165169
self.last_modified = m["last_modified"]
170+
# TODO, get rid of the extra `get` here once upstream issue:
171+
# https://github.com/jupyter-server/jupyter_server/issues/1453 is resolved
172+
model_with_hash = await ensure_async(
173+
self._contents_manager.get(
174+
self.path,
175+
content=False,
176+
require_hash=True,
177+
)
178+
)
179+
return {**m, "hash": model_with_hash["hash"]}
166180
finally:
167181
done_saving.set()
168182

projects/jupyter-server-ydoc/jupyter_server_ydoc/rooms.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ async def _maybe_save_document(self, saving_document: asyncio.Task | None) -> No
273273
await asyncio.sleep(self._save_delay)
274274

275275
self.log.info("Saving the content from room %s", self._room_id)
276-
await self._file.maybe_save_content(
276+
saved_model = await self._file.maybe_save_content(
277277
{
278278
"format": self._file_format,
279279
"type": self._file_type,
@@ -291,6 +291,8 @@ async def _maybe_save_document(self, saving_document: asyncio.Task | None) -> No
291291
)
292292
self._last_modified = model["last_modified"]
293293
self._document.dirty = False
294+
if saved_model:
295+
self._document.hash = saved_model["hash"]
294296

295297
self._emit(LogLevel.INFO, "save", "Content saved.")
296298

projects/jupyter-server-ydoc/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ authors = [
2828
{ name = "Jupyter Development Team", email = "[email protected]" },
2929
]
3030
dependencies = [
31-
"jupyter_server>=2.4.0,<3.0.0",
31+
"jupyter_server>=2.11.1,<3.0.0",
3232
"jupyter_ydoc>=2.0.0,<4.0.0",
3333
"pycrdt",
3434
"pycrdt-websocket>=0.14.0,<0.15.0",

0 commit comments

Comments
 (0)