Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use content providers to remove RTC prefix #418

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 7 additions & 17 deletions packages/collaborative-drive/src/tokens.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
// Copyright (c) Jupyter Development Team.
// Distributed under the terms of the Modified BSD License.

import { DocumentChange, IAwareness, YDocument } from '@jupyter/ydoc';
import { Contents } from '@jupyterlab/services';
import { IAwareness } from '@jupyter/ydoc';
import { Contents, SharedDocumentFactory } from '@jupyterlab/services';
import { IDisposable } from '@lumino/disposable';

import { Token } from '@lumino/coreutils';

/**
* The collaborative drive.
*/
export const ICollaborativeDrive = new Token<ICollaborativeDrive>(
'@jupyter/collaboration-extension:ICollaborativeDrive'
);
export const ICollaborativeContentProvider =
new Token<ICollaborativeContentProvider>(
'@jupyter/collaboration-extension:ICollaborativeContentProvider'
);

/**
* The global awareness token.
Expand All @@ -21,18 +22,7 @@ export const IGlobalAwareness = new Token<IAwareness>(
'@jupyter/collaboration:IGlobalAwareness'
);

/**
* A document factory for registering shared models
*/
export type SharedDocumentFactory = (
options: Contents.ISharedFactoryOptions
) => YDocument<DocumentChange>;

/**
* A Collaborative implementation for an `IDrive`, talking to the
* server using the Jupyter REST API and a WebSocket connection.
*/
export interface ICollaborativeDrive extends Contents.IDrive {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we'll want to log that somewhere in the changelog as API breaking change, since other extensions may currently depend on ICollaborativeDrive?

For instance it's the case of JupyterCAD and JupyterGIS:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the transition will be smooth because:

  • both extensions used ICollaborativeDrive as an optional dependency (after all it was just added)
  • neither of the extensions actually relied on the Drive part of ICollaborativeDrive - the same properties that they needed will be available on ICollaborativeContentProvider

So when cutting the release we will just need to add a note that migration from ICollaborativeDrive to ICollaborativeContentProvider token for access to shared factory and forks will be required. I do not see a good way to do that right now as the changelog for next version is not there yet - is it ok if we add it during/just after cutting the release?

export interface ICollaborativeContentProvider {
/**
* SharedModel factory for the YDrive.
*/
Expand Down
233 changes: 78 additions & 155 deletions packages/docprovider-extension/src/filebrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,72 +4,88 @@
*/

import {
ILabShell,
IRouter,
JupyterFrontEnd,
JupyterFrontEndPlugin
} from '@jupyterlab/application';
import { Dialog, showDialog } from '@jupyterlab/apputils';
import { DocumentWidget, IDocumentWidget } from '@jupyterlab/docregistry';
import { Widget } from '@lumino/widgets';
import {
FileBrowser,
IDefaultFileBrowser,
IFileBrowserFactory
} from '@jupyterlab/filebrowser';

import { IStatusBar } from '@jupyterlab/statusbar';
import { ContentsManager } from '@jupyterlab/services';

import { IEditorTracker } from '@jupyterlab/fileeditor';
import {
IEditorTracker,
IEditorWidgetFactory,
FileEditorFactory
} from '@jupyterlab/fileeditor';
import { ILogger, ILoggerRegistry } from '@jupyterlab/logconsole';
import { INotebookTracker } from '@jupyterlab/notebook';
import {
INotebookTracker,
INotebookWidgetFactory,
NotebookWidgetFactory
} from '@jupyterlab/notebook';
import { ISettingRegistry } from '@jupyterlab/settingregistry';
import { ITranslator, nullTranslator } from '@jupyterlab/translation';

import { CommandRegistry } from '@lumino/commands';

import { YFile, YNotebook } from '@jupyter/ydoc';

import {
ICollaborativeDrive,
ICollaborativeContentProvider,
IGlobalAwareness
} from '@jupyter/collaborative-drive';
import { IForkProvider, TimelineWidget, YDrive } from '@jupyter/docprovider';
import {
IForkProvider,
TimelineWidget,
RtcContentProvider
} from '@jupyter/docprovider';
import { Awareness } from 'y-protocols/awareness';
import { URLExt } from '@jupyterlab/coreutils';

/**
* The command IDs used by the file browser plugin.
*/
namespace CommandIDs {
export const openPath = 'filebrowser:open-path';
}
const DOCUMENT_TIMELINE_URL = 'api/collaboration/timeline';

const TWO_SESSIONS_WARNING =
'The file %1 has been opened with two different views. ' +
'This is not supported. Please close this view; otherwise, ' +
'some of your edits may not be saved properly.';

/**
* The default collaborative drive provider.
*/
export const drive: JupyterFrontEndPlugin<ICollaborativeDrive> = {
id: '@jupyter/docprovider-extension:drive',
description: 'The default collaborative drive provider',
provides: ICollaborativeDrive,
requires: [ITranslator],
optional: [IGlobalAwareness],
activate: (
app: JupyterFrontEnd,
translator: ITranslator,
globalAwareness: Awareness | null
): ICollaborativeDrive => {
const trans = translator.load('jupyter_collaboration');
const drive = new YDrive(app.serviceManager.user, trans, globalAwareness);
app.serviceManager.contents.addDrive(drive);
return drive;
}
};
export const rtcContentProvider: JupyterFrontEndPlugin<ICollaborativeContentProvider> =
{
id: '@jupyter/docprovider-extension:content-provider',
description: 'The RTC content provider',
provides: ICollaborativeContentProvider,
requires: [ITranslator],
optional: [IGlobalAwareness],
activate: (
app: JupyterFrontEnd,
translator: ITranslator,
globalAwareness: Awareness | null
): ICollaborativeContentProvider => {
const trans = translator.load('jupyter_collaboration');
const defaultDrive = (app.serviceManager.contents as ContentsManager)
.defaultDrive;
if (!defaultDrive) {
throw Error(
'Cannot initialize content provider: default drive property not accessible on contents manager instance.'
);
}
const registry = defaultDrive.contentProviderRegistry;
if (!registry) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will that make jupyter-collaboration only compatible with JupyterLab 4.4 or higher?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I would propose that, the next version of jupyter-collaboration will require JupyterLab 4.4.

throw Error(
'Cannot initialize content provider: no content provider registry.'
);
}
const rtcContentProvider = new RtcContentProvider({
apiEndpoint: '/api/contents',
serverSettings: defaultDrive.serverSettings,
user: app.serviceManager.user,
trans,
globalAwareness
});
registry.register('rtc', rtcContentProvider);
return rtcContentProvider;
}
};

/**
* Plugin to register the shared model factory for the content type 'file'.
Expand All @@ -79,13 +95,20 @@ export const yfile: JupyterFrontEndPlugin<void> = {
description:
"Plugin to register the shared model factory for the content type 'file'",
autoStart: true,
requires: [ICollaborativeDrive],
optional: [],
activate: (app: JupyterFrontEnd, drive: ICollaborativeDrive): void => {
requires: [ICollaborativeContentProvider, IEditorWidgetFactory],
activate: (
app: JupyterFrontEnd,
contentProvider: ICollaborativeContentProvider,
editorFactory: FileEditorFactory.IFactory
): void => {
const yFileFactory = () => {
return new YFile();
};
drive.sharedModelFactory.registerDocumentFactory('file', yFileFactory);
contentProvider.sharedModelFactory.registerDocumentFactory(
'file',
yFileFactory
);
editorFactory.contentProviderId = 'rtc';
}
};

Expand All @@ -97,11 +120,12 @@ export const ynotebook: JupyterFrontEndPlugin<void> = {
description:
"Plugin to register the shared model factory for the content type 'notebook'",
autoStart: true,
requires: [ICollaborativeDrive],
requires: [ICollaborativeContentProvider, INotebookWidgetFactory],
optional: [ISettingRegistry],
activate: (
app: JupyterFrontEnd,
drive: YDrive,
contentProvider: ICollaborativeContentProvider,
notebookFactory: NotebookWidgetFactory.IFactory,
settingRegistry: ISettingRegistry | null
): void => {
let disableDocumentWideUndoRedo = true;
Expand Down Expand Up @@ -131,10 +155,11 @@ export const ynotebook: JupyterFrontEndPlugin<void> = {
disableDocumentWideUndoRedo
});
};
drive.sharedModelFactory.registerDocumentFactory(
contentProvider.sharedModelFactory.registerDocumentFactory(
'notebook',
yNotebookFactory
);
notebookFactory.contentProviderId = 'rtc';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to indicate that multiple extensions could try to set their content provider on the default notebook and editor factories?

What is the expected behavior if two extensions try to do this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the expected behavior if two extensions try to do this?

Currently the last one wins. Alternative, we could log a warning or reject a second use of setter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the last one wins. Alternative, we could log a warning or reject a second use of setter.

Maybe a hard failure would be preferable for end users, so it's easier to debug and report?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense. Done in jupyterlab/jupyterlab@1173ca7.

}
};
/**
Expand All @@ -144,11 +169,11 @@ export const statusBarTimeline: JupyterFrontEndPlugin<void> = {
id: '@jupyter/docprovider-extension:statusBarTimeline',
description: 'Plugin to add a timeline slider to the status bar',
autoStart: true,
requires: [IStatusBar, ICollaborativeDrive],
requires: [IStatusBar, ICollaborativeContentProvider],
activate: async (
app: JupyterFrontEnd,
statusBar: IStatusBar,
drive: ICollaborativeDrive
contentProvider: ICollaborativeContentProvider
): Promise<void> => {
try {
let sliderItem: Widget | null = null;
Expand All @@ -158,18 +183,17 @@ export const statusBarTimeline: JupyterFrontEndPlugin<void> = {
documentPath: string,
documentId: string
) => {
if (documentId && documentPath.split(':')[0] === 'RTC') {
if (drive) {
// Remove 'RTC:' from document path
documentPath = documentPath.slice(drive.name.length + 1);
// TODO: check if documentId uses RTC
if (documentId) {
if (contentProvider) {
// Dispose of the previous timelineWidget if it exists
if (timelineWidget) {
timelineWidget.dispose();
timelineWidget = null;
}

const [format, type] = documentId.split(':');
const provider = drive.providers.get(
const provider = contentProvider.providers.get(
`${format}:${type}:${documentPath}`
) as unknown as IForkProvider;
const fullPath = URLExt.join(
Expand Down Expand Up @@ -238,6 +262,7 @@ export const statusBarTimeline: JupyterFrontEndPlugin<void> = {
currentWidget.context.model.sharedModel.getState(
'document_id'
) as string;
// TODO
return !!documentId && documentPath.split(':')[0] === 'RTC';
}
return false;
Expand All @@ -251,52 +276,6 @@ export const statusBarTimeline: JupyterFrontEndPlugin<void> = {
}
};

/**
* The default file browser factory provider.
*/
export const defaultFileBrowser: JupyterFrontEndPlugin<IDefaultFileBrowser> = {
id: '@jupyter/docprovider-extension:defaultFileBrowser',
description: 'The default file browser factory provider',
provides: IDefaultFileBrowser,
requires: [ICollaborativeDrive, IFileBrowserFactory],
optional: [IRouter, JupyterFrontEnd.ITreeResolver, ILabShell, ITranslator],
activate: async (
app: JupyterFrontEnd,
drive: YDrive,
fileBrowserFactory: IFileBrowserFactory,
router: IRouter | null,
tree: JupyterFrontEnd.ITreeResolver | null,
labShell: ILabShell | null,
translator: ITranslator | null
): Promise<IDefaultFileBrowser> => {
const { commands } = app;
const trans = (translator ?? nullTranslator).load('jupyterlab');
app.serviceManager.contents.addDrive(drive);

// Manually restore and load the default file browser.
const defaultBrowser = fileBrowserFactory.createFileBrowser('filebrowser', {
auto: false,
restore: false,
driveName: drive.name
});
defaultBrowser.node.setAttribute('role', 'region');
defaultBrowser.node.setAttribute(
'aria-label',
trans.__('File Browser Section')
);

void Private.restoreBrowser(
defaultBrowser,
commands,
router,
tree,
labShell
);

return defaultBrowser;
}
};

/**
* The default collaborative drive provider.
*/
Expand Down Expand Up @@ -383,59 +362,3 @@ export const logger: JupyterFrontEndPlugin<void> = {
})();
}
};

namespace Private {
/**
* Restores file browser state and overrides state if tree resolver resolves.
*/
export async function restoreBrowser(
browser: FileBrowser,
commands: CommandRegistry,
router: IRouter | null,
tree: JupyterFrontEnd.ITreeResolver | null,
labShell: ILabShell | null
): Promise<void> {
const restoring = 'jp-mod-restoring';

browser.addClass(restoring);

if (!router) {
await browser.model.restore(browser.id);
await browser.model.refresh();
browser.removeClass(restoring);
return;
}

const listener = async () => {
router.routed.disconnect(listener);

const paths = await tree?.paths;

if (paths?.file || paths?.browser) {
// Restore the model without populating it.
await browser.model.restore(browser.id, false);
if (paths.file) {
await commands.execute(CommandIDs.openPath, {
path: paths.file,
dontShowBrowser: true
});
}
if (paths.browser) {
await commands.execute(CommandIDs.openPath, {
path: paths.browser,
dontShowBrowser: true
});
}
} else {
await browser.model.restore(browser.id);
await browser.model.refresh();
}
browser.removeClass(restoring);

if (labShell?.isEmpty('main')) {
void commands.execute('launcher:create');
}
};
router.routed.connect(listener);
}
}
Loading
Loading