Skip to content

Commit

Permalink
File handling overhaul (#42)
Browse files Browse the repository at this point in the history
* Enables Grist Desktop to open .grist files from anywhere on the system, without importing them to its own directory.
* Enables importing from the file menu
* Adds a WindowManager, which tracks open Grist documents and focuses windows instead of re-opening documents.
* Cleanup and improvements to various file-handling and Electron-specific methods.
* Makes "Recent Files" work correctly, but doesn't persist across restarts.

Filesystem changes
================

Grist Desktop used to rely on symlinks to comply with Grist Core's requirement that all documents be stored centrally in GRIST_DATA_DIR. This causes issues on Windows, and impacts the user experience since users cannot choose where to store their Grist documents.

Moreover, this contributed to a bad user experience as documents use docID as name, and GRIST_DATA_DIR defaults to the "Documents" folder. This means users will end up having a lot of <random string>.grist files in their "Documents" folder.

It has also led to complaints from macOS users, since they basically cannot use Grist Desktop without granting permissions to the "Documents" folder.

This enables Grist Desktop to open Grist documents anywhere on the local filesystem, without making symlinks. All modifications will directly write to the document.

Window manager
===============

Grist Desktop now knows all documents it has open, and would bring the open document to you if you open it twice. It can keep track of window content changes (i.e. when a user navigates away from / into a document).
  • Loading branch information
SleepyLeslie authored Sep 21, 2024
1 parent 0f60d1a commit 9566338
Show file tree
Hide file tree
Showing 30 changed files with 1,794 additions and 722 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
target:
- x64
node:
- 18
- 20
name: ${{ matrix.os }} (node=${{ matrix.node }}, host=${{ matrix.host }}, target=${{ matrix.target }})
steps:
- uses: actions/checkout@v3
Expand Down
9 changes: 5 additions & 4 deletions .github/workflows/package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,22 @@ jobs:
fail-fast: false
matrix:
os:
- macos-13
- macos-14
- ubuntu-22.04
- windows-2022
host:
- x64
target:
- x64
node:
- 18
- 20
include:
- os: macos-14
node: 18
node: 20
host: arm64
target: arm64
- os: windows-2022
node: 18
node: 20
host: x64
target: x86
name: ${{ matrix.os }} (node=${{ matrix.node }}, host=${{ matrix.host }}, target=${{ matrix.target }})
Expand Down Expand Up @@ -117,6 +117,7 @@ jobs:
DEBUG: electron-builder
APPLE_ID: ${{ secrets.APPLE_ID }}
APPLE_ID_PASSWORD: ${{ secrets.APPLE_ID_PASSWORD }}
APPLE_APP_SPECIFIC_PASSWORD: ${{ secrets.APPLE_APP_SPECIFIC_PASSWORD }}
APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }}
CSC_KEY_PASSWORD: ${{ secrets.CSC_KEY_PASSWORD }}
CSC_LINK: ${{ secrets.CSC_LINK }}
Expand Down
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[submodule "core"]
path = core
url = https://github.com/gristlabs/grist-core
branch = main
branch = nativefs
ignore = dirty
2 changes: 1 addition & 1 deletion core
Submodule core updated 256 files
31 changes: 31 additions & 0 deletions ext/app/client/electronAPI.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { App } from "app/client/ui/App";
import { HomeModel } from "app/client/models/HomeModel";

export type NewDocument = {
path: string,
id: string
}

/**
* Allows the Grist client to call into electron.
* See https://www.electronjs.org/docs/latest/tutorial/ipc
*/
interface IElectronAPI {

// The Grist client can use these interfaces to request the electron main process to perform
// certain tasks.
createDoc: () => Promise<NewDocument>,
importDoc: (uploadId: number) => Promise<NewDocument>,

// The Grist client needs to call these interfaces to register callback functions for certain
// events coming from the electron main process.
onMainProcessImportDoc: (callback: (fileContents: Buffer, fileName: string) => void) => void

}

declare global {
interface Window {
electronAPI: IElectronAPI,
gristApp: App,
}
}
7 changes: 7 additions & 0 deletions ext/app/client/electronOnly.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export function electronOnly() {
if (window.electronAPI === undefined) {
// User-facing error text, preventing a `window.electronAPI is undefined` error showing to users.
// TODO - Find a better way to trigger the display of this.
throw Error("Sorry, this must be done from within the app.");
}
}
50 changes: 50 additions & 0 deletions ext/app/client/ui/HomeImports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { AppModel, reportError } from 'app/client/models/AppModel';
import { IMPORTABLE_EXTENSIONS } from 'app/client/lib/uploads';
import { ImportProgress } from 'app/client/ui/ImportProgress';
import { byteString } from 'app/common/gutil';
import { openFilePicker } from 'app/client/ui/FileDialog';
import { uploadFiles } from 'app/client/lib/uploads';

/**
* Imports a document and returns its upload ID, or null if no files were selected.
*/
export async function docImport(app: AppModel): Promise<number|null> {
// We use openFilePicker() and uploadFiles() separately, rather than the selectFiles() helper,
// because we only want to connect to a docWorker if there are in fact any files to upload.

// Start selecting files. This needs to start synchronously to be seen as a user-initiated
// popup, or it would get blocked by default in a typical browser.
const files: File[] = await openFilePicker({
multiple: false,
accept: IMPORTABLE_EXTENSIONS.join(","),
});

if (!files.length) { return null; }

return await fileImport(files, app);
}

/**
* Imports one or more files and returns its upload ID, or null if no files were selected.
*/

export async function fileImport(
files: File[], app: AppModel): Promise<number | null> {
const progressUI = app.notifier.createProgressIndicator(files[0].name, byteString(files[0].size));
const progress = ImportProgress.create(progressUI, progressUI, files[0]);
try {
const docWorker = await app.api.getWorkerAPI('import');
const uploadResult = await uploadFiles(files, {docWorkerUrl: docWorker.url, sizeLimit: 'import'},
(p) => progress.setUploadProgress(p));

return uploadResult!.uploadId;
} catch (err) {
reportError(err);
return null;
} finally {
progress.finish();
progressUI.dispose();
}
}

export const homeImports = {docImport, fileImport};
50 changes: 50 additions & 0 deletions ext/app/client/ui/NewDocMethods.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { AppModel } from "app/client/models/AppModel";
import { HomeModel } from 'app/client/models/HomeModel';
import { electronOnly } from "app/client/electronOnly";
import { homeImports } from 'app/client/ui/HomeImports';
import { urlState } from 'app/client/models/gristUrlState';

async function createDocAndOpen() {
electronOnly();
const doc = await window.electronAPI.createDoc();
if (doc) {
window.location.assign("/o/docs/" + doc.id);
}
}

// Invoked by the "Import Document" button.
async function importDocAndOpen(home: HomeModel) {
electronOnly();
return _importUploadedFileAndOpen(
await homeImports.docImport(home.app)
);
}

// Internal implementation.
async function _importUploadedFileAndOpen(uploadId: number | null) {
if (uploadId === null) { return; }
const doc = await window.electronAPI.importDoc(uploadId);
if (doc) {
await urlState().pushUrl({doc: doc.id});
}
}

// Register the callback function for importing from the file menu.
// The ? is for external visitors over the network. electronAPI is set by electron's preload script
// and is undefined for non-electron visitors. An error here will make the entire page fail to load.
window.electronAPI?.onMainProcessImportDoc((fileContents: Buffer, fileName: string) => {
(async() => {
let appModel: AppModel | null = null;
// Loop to wait for initialisation of the app model.
// TODO It would be nicer to create a proper bridge which notifies the user when import isn't available.
while (!(appModel = window.gristApp?.topAppModel.appObs.get())) {
await new Promise(resolve => setTimeout(resolve, 100));
}
const uploadId = await homeImports.fileImport([new File([fileContents], fileName)], appModel);
await _importUploadedFileAndOpen(uploadId);
})();
});

// There _should_ also be an "importFromPluginAndOpen" here, but Grist Desktop will not have import
// plugins, so it is left out.
export const newDocMethods = { createDocAndOpen, importDocAndOpen };
21 changes: 18 additions & 3 deletions ext/app/electron/AppMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,33 @@ class AppMenu extends events.EventEmitter {
this.rebuildMenu();
}

addRecentItem(entry) {
this.recentItems.addItem(entry);
}

getMenu() { return this._menu; }

rebuildMenu() {
const isActive = electron.Menu.getApplicationMenu() === this.getMenu();
this._menu = electron.Menu.buildFromTemplate(this.buildTemplate());
// TODO: Electron does not yet support updating the menu except by reassigning the entire
// menu. There are proposals to allow menu templates include callbacks that
// are called on menu open. https://github.com/electron/electron/issues/528
if (isActive) {
this.setAsCurrentElectronMenu();
}
}

setAsCurrentElectronMenu() {
electron.Menu.setApplicationMenu(this.getMenu());
}

buildOpenRecentSubmenu() {
let subMenu = [];
this.recentItems.listItems().reverse().forEach(filePath => {
subMenu.push({
label: path.basename(filePath),
click: event => app.emit('open-file', event, filePath)
click: () => app.emit('open-file', undefined, filePath)
});
});
return subMenu;
Expand Down Expand Up @@ -61,11 +76,11 @@ class AppMenu extends events.EventEmitter {
submenu: [{
label: 'New',
accelerator: 'CmdOrCtrl+N',
click: () => this.emit('menu-file-new')
click: (item, focusedWindow) => this.emit('menu-file-new', focusedWindow)
}, {
label: 'Open...',
accelerator: 'CmdOrCtrl+O',
click: () => this.emit('menu-file-open')
click: (item, focusedWindow) => this.emit('menu-file-open', focusedWindow)
}, {
label: 'Open Recent',
submenu: this.buildOpenRecentSubmenu()
Expand Down
119 changes: 119 additions & 0 deletions ext/app/electron/DocRegistry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import * as log from "app/server/lib/log";
import * as path from "path";
import { HomeDBManager } from "app/gen-server/lib/homedb/HomeDBManager";
import { fileExists } from "app/electron/fileUtils";

/*
Doc registry is hard to remove.
In an ideal world, this cache would be maintained by Grist's internal machinery.
Potentially, the DesktopDocStorageManager
However, external code (i.e electron) needs to be able to add the paths for specific grist files to that file.
It should be possible to:
1. Add the path to the doc registry
2. Trigger an event, which triggers the electron browser to call the "new doc" API.
3. New doc is resolved to the right file when loaded.
The result should be we don't need to manually call into any addDocument shennanigans on the backend, which is a win.
However, anything short of this full refactor is probably not worth it? If I keep things as-is and just move them
to the storage manager, all I'm actually doing is moving things around, and not solving the problem of bypassing
Grist's main APIs/interfaces.
*/


export class DocRegistry {

private idToPathMap: Map<string, string>;
private pathToIdMap: Map<string, string>;
private db: HomeDBManager;

// Always use create() to create a new DocRegistry.
private constructor() {}

public static async create(dbManager: HomeDBManager) {
// Allocate space.
const dr = new DocRegistry();
dr.db = dbManager;
dr.idToPathMap = new Map<string, string>;
dr.pathToIdMap = new Map<string, string>;

// Go over all documents we know about.
for (const doc of await dr.db.getAllDocs()) {
// All documents are supposed to have externalId set.
const docPath = doc.options?.externalId;
if (docPath && fileExists(docPath)) {
// Cache the two-way mapping docID <-> path.
dr.idToPathMap.set(doc.id, docPath);
dr.pathToIdMap.set(docPath, doc.id);
} else {
// Remove this document - it should not appear in a DB for Grist Desktop.
await dr.db.deleteDocument({
userId: (await dr.getDefaultUser()).id,
urlId: doc.id
});
}
}
return dr;
}

public lookupById(docId: string): string | null {
return this.idToPathMap.get(docId) ?? null;
}

public lookupByPath(docPath: string): string | null {
return this.pathToIdMap.get(docPath) ?? null;
}

/**
* Look for the given path in the registry. If the path has already been assigned a doc ID beforehand, return
* this ID. Otherwise assign a new doc ID and return it.
* @param docPath Path to the document.
* @returns A Promise that resolves to either an existing doc ID or the newly assigned doc ID.
*/
public async lookupByPathOrCreate(docPath: string): Promise<string> {
let docId = this.lookupByPath(docPath);
if (docId === null) {
// Assign a doc ID if it does not already have one.
docId = await this.registerDoc(docPath);
log.debug(`Document ${docPath} not found in home DB, assigned doc ID ${docId}`);
} else {
log.debug(`Got known doc ID ${docId} for ${docPath}`);
}
return docId;
}

public async getDefaultUser() {
const user = await this.db.getUserByLogin(process.env.GRIST_DEFAULT_EMAIL as string);
if (!user) { throw new Error('cannot find default user'); }
return user;
}

/**
* Register a document in the home DB and DocRegistry cache, assigning it a new doc ID.
* @param docPath Path to the document to register. Must not correspond to a known document in the home DB.
* @returns A Promise that resolves to the newly assigned doc ID.
*/
public async registerDoc(docPath: string): Promise<string> {
const defaultUser = await this.getDefaultUser();
const wss = this.db.unwrapQueryResult(await this.db.getOrgWorkspaces({userId: defaultUser.id}, 0));
for (const doc of wss[0].docs) {
if (doc.options?.externalId === docPath) {
// We might be able to do better.
throw Error("DocRegistry cache incoherent. Please try restarting the app.");
}
}
// Create the entry in the home database.
const docId = this.db.unwrapQueryResult(await this.db.addDocument({
userId: defaultUser.id,
}, wss[0].id, {
name: path.basename(docPath, '.grist'),
options: { externalId: docPath },
}));
// Update the in-memory cache.
this.pathToIdMap.set(docPath, docId);
this.idToPathMap.set(docId, docPath);
return docId;
}

}
Loading

0 comments on commit 9566338

Please sign in to comment.