From 438f2596877c3da6211174ae283417c827f8a602 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Tue, 12 Jan 2021 10:48:40 -0500 Subject: [PATCH] (core) start reconciling forking with granular access Summary: This allows a fork to be made by a user if: * That user is an owner of the document being forked, or * That user has full read access to the document being forked. The bulk of the diff is reorganization of how forking is done. ActiveDoc.fork is now responsible for creating a fork, not just a docId/urlId for the fork. Since fork creation should not be limited to the doc worker hosting the trunk, a helper endpoint is added for placing the fork. The change required sanitizing worker allocation a bit, and allowed session knowledge to be removed from HostedStorageManager. Test Plan: Added test; existing tests pass. Reviewers: dsagal Reviewed By: dsagal Differential Revision: https://phab.getgrist.com/D2700 --- app/common/KeyedOps.ts | 1 + app/gen-server/lib/DocApiForwarder.ts | 2 + app/gen-server/lib/DocWorkerMap.ts | 21 +++++++++ app/server/lib/ActiveDoc.ts | 53 ++++++++++++++++------ app/server/lib/Client.ts | 24 +++++++--- app/server/lib/DocApi.ts | 18 +++++++- app/server/lib/DocManager.ts | 19 ++++++-- app/server/lib/DocSnapshots.ts | 7 +++ app/server/lib/DocStorageManager.ts | 7 ++- app/server/lib/FlexServer.ts | 6 +++ app/server/lib/GranularAccess.ts | 23 +++++----- app/server/lib/GristServer.ts | 2 + app/server/lib/HostedStorageManager.ts | 62 +++++++++++++++++--------- app/server/lib/IDocStorageManager.ts | 4 +- app/server/lib/Permit.ts | 1 + app/server/lib/idUtils.ts | 12 ++--- 16 files changed, 193 insertions(+), 69 deletions(-) diff --git a/app/common/KeyedOps.ts b/app/common/KeyedOps.ts index f3d1ca6e..58fe6e12 100644 --- a/app/common/KeyedOps.ts +++ b/app/common/KeyedOps.ts @@ -99,6 +99,7 @@ export class KeyedOps { await status.promise; return; } + if (!this._changed.has(key)) { return; } const callback = new Promise((resolve) => { status.callbacks.push(resolve); }); diff --git a/app/gen-server/lib/DocApiForwarder.ts b/app/gen-server/lib/DocApiForwarder.ts index 92ab61a4..d5b68286 100644 --- a/app/gen-server/lib/DocApiForwarder.ts +++ b/app/gen-server/lib/DocApiForwarder.ts @@ -42,6 +42,8 @@ export class DocApiForwarder { app.use('/api/docs/:docId/remove', withDoc); app.delete('/api/docs/:docId', withDoc); app.use('/api/docs/:docId/download', withDoc); + app.use('/api/docs/:docId/fork', withDoc); + app.use('/api/docs/:docId/create-fork', withDoc); app.use('/api/docs/:docId/apply', withDoc); app.use('/api/docs/:docId/attachments', withDoc); app.use('/api/docs/:docId/snapshots', withDoc); diff --git a/app/gen-server/lib/DocWorkerMap.ts b/app/gen-server/lib/DocWorkerMap.ts index ffacdb76..c3fb6f23 100644 --- a/app/gen-server/lib/DocWorkerMap.ts +++ b/app/gen-server/lib/DocWorkerMap.ts @@ -328,8 +328,29 @@ export class DocWorkerMap implements IDocWorkerMap { * A preferred doc worker can be specified, which will be assigned * if no assignment is already made. * + * For the special docId "import", return a potential assignment. + * It will be up to the doc worker to assign the eventually + * created document, if desired. + * */ public async assignDocWorker(docId: string, workerId?: string): Promise { + if (docId === 'import') { + const lock = await this._redlock.lock(`workers-lock`, LOCK_TIMEOUT); + try { + const workerId = await this._client.srandmemberAsync(`workers-available-default`); + if (!workerId) { throw new Error('no doc worker available'); } + const docWorker = await this._client.hgetallAsync(`worker-${workerId}`) as DocWorkerInfo|null; + if (!docWorker) { throw new Error('no doc worker contact info available'); } + return { + docMD5: null, + docWorker, + isActive: false + }; + } finally { + await lock.unlock(); + } + } + // Check if a DocWorker is already assigned; if so return result immediately // without locking. let docStatus = await this.getDocWorker(docId); diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index a9c41f0c..3a9e4f90 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -52,13 +52,14 @@ import {ActionHistoryImpl} from './ActionHistoryImpl'; import {ActiveDocImport} from './ActiveDocImport'; import {DocClients} from './DocClients'; import {DocPluginManager} from './DocPluginManager'; -import {DocSession, getDocSessionAccess, getDocSessionUserId, makeExceptionalDocSession, - OptDocSession} from './DocSession'; +import {DocSession, getDocSessionAccess, getDocSessionUser, getDocSessionUserId, + makeExceptionalDocSession, OptDocSession} from './DocSession'; import {DocStorage} from './DocStorage'; import {expandQuery} from './ExpandedQuery'; import {GranularAccess} from './GranularAccess'; import {OnDemandActions} from './OnDemandActions'; import {findOrAddAllEnvelope, Sharing} from './Sharing'; +import fetch from 'node-fetch'; bluebird.promisifyAll(tmp); @@ -328,8 +329,7 @@ export class ActiveDoc extends EventEmitter { const startTime = Date.now(); this.logDebug(docSession, "loadDoc"); try { - const isNew: boolean = await this._docManager.storageManager.prepareLocalDoc(this.docName, - docSession); + const isNew: boolean = await this._docManager.storageManager.prepareLocalDoc(this.docName); if (isNew) { await this.createDoc(docSession); await this.addInitialTable(docSession); @@ -571,8 +571,7 @@ export class ActiveDoc extends EventEmitter { // Check if user has rights to download this doc. public async canDownload(docSession: OptDocSession) { - return this._granularAccess.hasViewAccess(docSession) && - await this._granularAccess.canReadEverything(docSession); + return this._granularAccess.canCopyEverything(docSession); } /** @@ -885,19 +884,47 @@ export class ActiveDoc extends EventEmitter { * Fork the current document. In fact, all that requires is calculating a good * ID for the fork. TODO: reconcile the two ways there are now of preparing a fork. */ - public async fork(docSession: DocSession): Promise { - if (!await this._granularAccess.canReadEverything(docSession)) { - throw new Error('cannot confirm authority to copy document'); + public async fork(docSession: OptDocSession): Promise { + const user = getDocSessionUser(docSession); + // For now, fork only if user can read everything (or is owner). + // TODO: allow forks with partial content. + if (!user || !await this._granularAccess.canCopyEverything(docSession)) { + throw new ApiError('Insufficient access to document to copy it entirely', 403); } - const userId = docSession.client.getCachedUserId(); - const isAnonymous = docSession.client.isAnonymous(); + const userId = user.id; + const isAnonymous = this._docManager.isAnonymous(userId); // Get fresh document metadata (the cached metadata doesn't include the urlId). - const doc = await docSession.authorizer.getDoc(); + const doc = await docSession.authorizer?.getDoc(); if (!doc) { throw new Error('document id not known'); } const trunkDocId = doc.id; const trunkUrlId = doc.urlId || doc.id; await this.flushDoc(); // Make sure fork won't be too out of date. - return makeForkIds({userId, isAnonymous, trunkDocId, trunkUrlId}); + const forkIds = makeForkIds({userId, isAnonymous, trunkDocId, trunkUrlId}); + + // To actually create the fork, we call an endpoint. This is so the fork + // can be associated with an arbitrary doc worker, rather than tied to the + // same worker as the trunk. We use a Permit for authorization. + const permitStore = this._docManager.gristServer.getPermitStore(); + const permitKey = await permitStore.setPermit({docId: forkIds.docId, + otherDocId: this.docName}); + try { + const url = await this._docManager.gristServer.getHomeUrlByDocId(forkIds.docId, `/api/docs/${forkIds.docId}/create-fork`); + const resp = await fetch(url, { + method: 'POST', + body: JSON.stringify({ srcDocId: this.docName }), + headers: { + Permit: permitKey, + 'Content-Type': 'application/json', + }, + }); + if (resp.status !== 200) { + throw new ApiError(resp.statusText, resp.status); + } + } finally { + await permitStore.removePermit(permitKey); + } + + return forkIds; } /** diff --git a/app/server/lib/Client.ts b/app/server/lib/Client.ts index 692e39fc..cfdce688 100644 --- a/app/server/lib/Client.ts +++ b/app/server/lib/Client.ts @@ -3,6 +3,7 @@ import {BrowserSettings} from 'app/common/BrowserSettings'; import {ErrorWithCode} from 'app/common/ErrorWithCode'; import {UserProfile} from 'app/common/LoginSessionAPI'; import {getLoginState, LoginState} from 'app/common/LoginState'; +import {ANONYMOUS_USER_EMAIL} from 'app/common/UserAPI'; import {User} from 'app/gen-server/entity/User'; import {HomeDBManager} from 'app/gen-server/lib/HomeDBManager'; import {ActiveDoc} from 'app/server/lib/ActiveDoc'; @@ -297,10 +298,17 @@ export class Client { // practice via a change to profile, but let's not make any assumptions here.) this._userId = null; this._firstLoginAt = null; - this._isAnonymous = false; + this._isAnonymous = !profile; } public getProfile(): UserProfile|null { + if (this._isAnonymous) { + return { + name: 'Anonymous', + email: ANONYMOUS_USER_EMAIL, + anonymous: true, + }; + } return this._profile; } @@ -315,10 +323,16 @@ export class Client { // Returns the userId for profile.email, or null when profile is not set; with caching. public async getUserId(dbManager: HomeDBManager): Promise { if (!this._userId) { - const user = await this._fetchUser(dbManager); - this._userId = (user && user.id) || null; - this._isAnonymous = this._userId && dbManager.getAnonymousUserId() === this._userId || false; - this._firstLoginAt = (user && user.firstLoginAt) || null; + if (this._profile) { + const user = await this._fetchUser(dbManager); + this._userId = (user && user.id) || null; + this._isAnonymous = this._userId && dbManager.getAnonymousUserId() === this._userId || false; + this._firstLoginAt = (user && user.firstLoginAt) || null; + } else { + this._userId = dbManager.getAnonymousUserId(); + this._isAnonymous = true; + this._firstLoginAt = null; + } } return this._userId; } diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index 15039597..05a34031 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -207,6 +207,22 @@ export class DocWorkerApi { } })); + // Fork the specified document. + this._app.post('/api/docs/:docId/fork', canView, withDoc(async (activeDoc, req, res) => { + const result = await activeDoc.fork(docSessionFromRequest(req)); + res.json(result); + })); + + // Initiate a fork. Used internally to implement ActiveDoc.fork. Only usable via a Permit. + this._app.post('/api/docs/:docId/create-fork', canEdit, throttled(async (req, res) => { + const mreq = req as RequestWithLogin; + const docId = stringParam(req.params.docId); + const srcDocId = stringParam(req.body.srcDocId); + if (srcDocId !== mreq.specialPermit?.otherDocId) { throw new Error('access denied'); } + await this._docManager.storageManager.prepareFork(srcDocId, docId); + res.json({srcDocId, docId}); + })); + // Update records. The records to update are identified by their id column. Any invalid id fails // the request and returns a 400 error code. this._app.patch('/api/docs/:docId/tables/:tableId/data', canEdit, withDoc(async (activeDoc, req, res) => { @@ -458,7 +474,7 @@ export class DocWorkerApi { const isAnonymous = isAnonymousUser(req); const {docId} = makeForkIds({userId, isAnonymous, trunkDocId: NEW_DOCUMENT_CODE, trunkUrlId: NEW_DOCUMENT_CODE}); - await this._docManager.fetchDoc(makeExceptionalDocSession('nascent', { + await this._docManager.createNamedDoc(makeExceptionalDocSession('nascent', { req: req as RequestWithLogin, browserSettings }), docId); diff --git a/app/server/lib/DocManager.ts b/app/server/lib/DocManager.ts index bcdedef2..ec1b21c5 100644 --- a/app/server/lib/DocManager.ts +++ b/app/server/lib/DocManager.ts @@ -109,7 +109,11 @@ export class DocManager extends EventEmitter { public async createNewDoc(client: Client): Promise { log.debug('DocManager.createNewDoc'); const docSession = makeExceptionalDocSession('nascent', {client}); - const activeDoc: ActiveDoc = await this.createNewEmptyDoc(docSession, 'Untitled'); + return this.createNamedDoc(docSession, 'Untitled'); + } + + public async createNamedDoc(docSession: OptDocSession, docId: string): Promise { + const activeDoc: ActiveDoc = await this.createNewEmptyDoc(docSession, docId); await activeDoc.addInitialTable(docSession); return activeDoc.docName; } @@ -188,9 +192,10 @@ export class DocManager extends EventEmitter { } } - // Ship the import to S3, since it isn't associated with any particular worker at this time. - // We could associate it with the current worker, but that is not necessarily desirable. - await this.storageManager.addToStorage(result.id); + // The imported document is associated with the worker that did the import. + // We could break that association (see /api/docs/:docId/assign for how) if + // we start using dedicated import workers. + return result; } @@ -397,6 +402,11 @@ export class DocManager extends EventEmitter { return makeAccessId(this.gristServer, userId); } + public isAnonymous(userId: number): boolean { + if (!this._homeDbManager) { throw new Error("HomeDbManager not available"); } + return userId === this._homeDbManager.getAnonymousUserId(); + } + /** * Helper function for creating a new shared document given the doc snapshot bundles received * from the sharing hub. @@ -458,6 +468,7 @@ export class DocManager extends EventEmitter { const docName = await this._createNewDoc(id); const docPath = await this.storageManager.getPath(docName); await docUtils.copyFile(uploadInfo.files[0].absPath, docPath); + await this.storageManager.addToStorage(docName); return {title: basename, id: docName}; } else { const doc = await this.createNewEmptyDoc(docSession, id); diff --git a/app/server/lib/DocSnapshots.ts b/app/server/lib/DocSnapshots.ts index 634e9dc7..7d4f515e 100644 --- a/app/server/lib/DocSnapshots.ts +++ b/app/server/lib/DocSnapshots.ts @@ -121,6 +121,13 @@ export class DocSnapshotInventory implements IInventory { }); } + /** + * Return true if document inventory does not need to be saved and is not in flux. + */ + public isSaved(key: string) { + return !this._needFlush.has(key) && !this._mutex.isLocked(key); + } + /** * Add a new snapshot of a document to the existing inventory. A prevSnapshotId may * be supplied as a cross-check. It will be matched against the most recent diff --git a/app/server/lib/DocStorageManager.ts b/app/server/lib/DocStorageManager.ts index a4b44246..d070f5e2 100644 --- a/app/server/lib/DocStorageManager.ts +++ b/app/server/lib/DocStorageManager.ts @@ -8,7 +8,6 @@ import {DocEntry, DocEntryTag} from 'app/common/DocListAPI'; import {DocSnapshots} from 'app/common/DocSnapshot'; import * as gutil from 'app/common/gutil'; import * as Comm from 'app/server/lib/Comm'; -import {OptDocSession} from 'app/server/lib/DocSession'; import * as docUtils from 'app/server/lib/docUtils'; import {GristServer} from 'app/server/lib/GristServer'; import {IDocStorageManager} from 'app/server/lib/IDocStorageManager'; @@ -85,12 +84,16 @@ export class DocStorageManager implements IDocStorageManager { * Prepares a document for use locally. Returns whether the document is new (needs to be * created). This is a no-op in the local DocStorageManager case. */ - public async prepareLocalDoc(docName: string, docSession: OptDocSession): Promise { return false; } + public async prepareLocalDoc(docName: string): Promise { return false; } public async prepareToCreateDoc(docName: string): Promise { // nothing to do } + public async prepareFork(srcDocName: string, destDocName: string): Promise { + // nothing to do + } + /** * Returns a promise for the list of docNames to show in the doc list. For the file-based * storage, this will include all .grist files under the docsRoot. diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 5061e655..4083098a 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -35,6 +35,7 @@ import {IDocStorageManager} from 'app/server/lib/IDocStorageManager'; import {INotifier} from 'app/server/lib/INotifier'; import * as log from 'app/server/lib/log'; import {getLoginMiddleware} from 'app/server/lib/logins'; +import {IPermitStore} from 'app/server/lib/Permit'; import {getAppPathTo, getAppRoot, getUnpackedAppRoot} from 'app/server/lib/places'; import {addPluginEndpoints, limitToPlugins} from 'app/server/lib/PluginEndpoint'; import {PluginManager} from 'app/server/lib/PluginManager'; @@ -221,6 +222,11 @@ export class FlexServer implements GristServer { return this.server ? (this.server.address() as AddressInfo).port : this.port; } + public getPermitStore(): IPermitStore { + if (!this._docWorkerMap) { throw new Error('no permit store available'); } + return this._docWorkerMap; + } + public addLogging() { if (this._check('logging')) { return; } if (process.env.GRIST_LOG_SKIP_HTTP) { return; } diff --git a/app/server/lib/GranularAccess.ts b/app/server/lib/GranularAccess.ts index 5af51032..8e459dbb 100644 --- a/app/server/lib/GranularAccess.ts +++ b/app/server/lib/GranularAccess.ts @@ -324,13 +324,24 @@ export class GranularAccess { } /** - * Check whether user can read everything in document. + * Check whether user can read everything in document. Checks both home-level and doc-level + * permissions. */ public async canReadEverything(docSession: OptDocSession): Promise { + const access = getDocSessionAccess(docSession); + if (!canView(access)) { return false; } const permInfo = await this._getAccess(docSession); return permInfo.getFullAccess().read === 'allow'; } + /** + * Check whether user can copy everything in document. Owners can always copy + * everything, even if there are rules that specify they cannot. + */ + public async canCopyEverything(docSession: OptDocSession): Promise { + return this.isOwner(docSession) || this.canReadEverything(docSession); + } + /** * Check whether user has full access to the document. Currently that is interpreted * as equivalent owner-level access to the document. @@ -349,16 +360,6 @@ export class GranularAccess { return access === 'owners'; } - /** - * Check for view access to the document. For most code paths, a request or message - * won't even be considered if there isn't view access, but there's no harm in double - * checking. - */ - public hasViewAccess(docSession: OptDocSession): boolean { - const access = getDocSessionAccess(docSession); - return canView(access); - } - /** * * If the user does not have access to the full document, we need to filter out diff --git a/app/server/lib/GristServer.ts b/app/server/lib/GristServer.ts index a022369c..1c8af911 100644 --- a/app/server/lib/GristServer.ts +++ b/app/server/lib/GristServer.ts @@ -2,6 +2,7 @@ import {SessionUserObj} from 'app/server/lib/BrowserSession'; import * as Comm from 'app/server/lib/Comm'; import {Hosts} from 'app/server/lib/extractOrg'; import {ICreate} from 'app/server/lib/ICreate'; +import {IPermitStore} from 'app/server/lib/Permit'; import {Sessions} from 'app/server/lib/Sessions'; import * as express from 'express'; @@ -14,6 +15,7 @@ export interface GristServer { getHost(): string; getHomeUrl(req: express.Request, relPath?: string): string; getHomeUrlByDocId(docId: string, relPath?: string): Promise; + getPermitStore(): IPermitStore; } export interface GristLoginMiddleware { diff --git a/app/server/lib/HostedStorageManager.ts b/app/server/lib/HostedStorageManager.ts index a62d77c5..cf4b6aad 100644 --- a/app/server/lib/HostedStorageManager.ts +++ b/app/server/lib/HostedStorageManager.ts @@ -7,9 +7,7 @@ import {buildUrlId, parseUrlId} from 'app/common/gristUrls'; import {KeyedOps} from 'app/common/KeyedOps'; import {DocReplacementOptions, NEW_DOCUMENT_CODE} from 'app/common/UserAPI'; import {HomeDBManager} from 'app/gen-server/lib/HomeDBManager'; -import {getUserId} from 'app/server/lib/Authorizer'; import {checksumFile} from 'app/server/lib/checksumFile'; -import {OptDocSession} from 'app/server/lib/DocSession'; import {DocSnapshotInventory, DocSnapshotPruner} from 'app/server/lib/DocSnapshots'; import {IDocWorkerMap} from 'app/server/lib/DocWorkerMap'; import {ChecksummedExternalStorage, DELETED_TOKEN, ExternalStorage} from 'app/server/lib/ExternalStorage'; @@ -216,8 +214,10 @@ export class HostedStorageManager implements IDocStorageManager { * Returns whether the document is new (needs to be created). * Calling this method multiple times in parallel for the same document is treated as a sign * of a bug. + * + * The optional srcDocName parameter is set when preparing a fork. */ - public async prepareLocalDoc(docName: string, docSession: OptDocSession): Promise { + public async prepareLocalDoc(docName: string, srcDocName?: string): Promise { // We could be reopening a document that is still closing down. // Wait for that to happen. TODO: we could also try to interrupt the closing-down process. await this.closeDocument(docName); @@ -228,7 +228,7 @@ export class HostedStorageManager implements IDocStorageManager { try { this._prepareFiles.add(docName); - const isNew = !(await this._ensureDocumentIsPresent(docName, docSession)); + const isNew = !(await this._claimDocument(docName, srcDocName)); return isNew; } finally { this._prepareFiles.delete(docName); @@ -236,17 +236,27 @@ export class HostedStorageManager implements IDocStorageManager { } public async prepareToCreateDoc(docName: string): Promise { + await this.prepareLocalDoc(docName, 'new'); if (this._inventory) { await this._inventory.create(docName); this._onInventoryChange(docName); } + this.markAsChanged(docName); + } + + /** + * Initialize one document from another, associating the result with the current + * worker. + */ + public async prepareFork(srcDocName: string, destDocName: string): Promise { + await this.prepareLocalDoc(destDocName, srcDocName); } // Gets a copy of the document, eg. for downloading. Returns full file path. // Copy won't change if edits are made to the document. It is caller's responsibility // to delete the result. public async getCopy(docName: string): Promise { - const present = await this._ensureDocumentIsPresent(docName, {client: null}); + const present = await this._claimDocument(docName); if (!present) { throw new Error('cannot copy document that does not exist yet'); } @@ -395,9 +405,10 @@ export class HostedStorageManager implements IDocStorageManager { return this._baseStore; } - // return true if document is backed up to s3. - public isSaved(docName: string): boolean { - return !this._uploads.hasPendingOperation(docName); + // return true if document and inventory is backed up to external store (if attached). + public isAllSaved(docName: string): boolean { + return !this._uploads.hasPendingOperation(docName) && + (this._inventory ? this._inventory.isSaved(docName) : true); } // pick up the pace of pushing to s3, from leisurely to urgent. @@ -423,10 +434,11 @@ export class HostedStorageManager implements IDocStorageManager { * Make sure document is backed up to s3. */ public async flushDoc(docName: string): Promise { - while (!this.isSaved(docName)) { + while (!this.isAllSaved(docName)) { log.info('HostedStorageManager: waiting for document to finish: %s', docName); await this._uploads.expediteOperationAndWait(docName); - if (!this.isSaved(docName)) { + await this._inventory?.flush(docName); + if (!this.isAllSaved(docName)) { // Throttle slightly in case this operation ends up looping excessively. await delay(1000); } @@ -502,25 +514,28 @@ export class HostedStorageManager implements IDocStorageManager { } /** - * Makes sure a document is present locally, fetching it from S3 if necessary. - * Returns true on success, false if document not found. It is safe to call - * this method multiple times in parallel. + * Makes sure a document is assigned to this worker, adding an + * assignment if it has none. If the document is present in + * external storage, fetch it. Return true if the document was + * fetched. + * + * The document can optionally be copied from an alternative + * source (srcDocName). This is useful for forking. + * + * If srcDocName is 'new', checks for the document in external storage + * are skipped. */ - private async _ensureDocumentIsPresent(docName: string, - docSession: OptDocSession): Promise { + private async _claimDocument(docName: string, + srcDocName?: string): Promise { // AsyncCreate.mapGetOrSet ensures we don't start multiple promises to talk to S3/Redis // and that we clean up the failed key in case of failure. return mapGetOrSet(this._localFiles, docName, async () => { if (this._closed) { throw new Error("HostedStorageManager._ensureDocumentIsPresent called after closing"); } checkValidDocId(docName); - const {trunkId, forkId, forkUserId, snapshotId} = parseUrlId(docName); + const {trunkId, forkId, snapshotId} = parseUrlId(docName); - // If forkUserId is set to a valid user id, we can only create a fork if we know the - // requesting user and their id matches the forkUserId. - const userId = (docSession.client && docSession.client.getCachedUserId()) || - (docSession.req && getUserId(docSession.req)); - const canCreateFork = forkUserId ? (forkUserId === userId) : true; + const canCreateFork = Boolean(srcDocName); const docStatus = await this._docWorkerMap.getDocWorkerOrAssign(docName, this._docWorkerId); if (!docStatus.isActive) { throw new Error(`Doc is not active on a DocWorker: ${docName}`); } @@ -528,10 +543,12 @@ export class HostedStorageManager implements IDocStorageManager { throw new Error(`Doc belongs to a different DocWorker (${docStatus.docWorker.id}): ${docName}`); } + if (srcDocName === 'new') { return false; } + if (this._disableS3) { // skip S3, just use file system let present: boolean = await fse.pathExists(this.getPath(docName)); - if (forkId && !present) { + if ((forkId || snapshotId) && !present) { if (!canCreateFork) { throw new Error(`Cannot create fork`); } if (snapshotId && snapshotId !== 'current') { throw new Error(`cannot find snapshot ${snapshotId} of ${docName}`); @@ -569,6 +586,7 @@ export class HostedStorageManager implements IDocStorageManager { } } return this._fetchFromS3(docName, { + sourceDocId: srcDocName, trunkId: forkId ? trunkId : undefined, snapshotId, canCreateFork }); }); diff --git a/app/server/lib/IDocStorageManager.ts b/app/server/lib/IDocStorageManager.ts index a1dbcca4..87f22674 100644 --- a/app/server/lib/IDocStorageManager.ts +++ b/app/server/lib/IDocStorageManager.ts @@ -1,7 +1,6 @@ import {DocEntry} from 'app/common/DocListAPI'; import {DocSnapshots} from 'app/common/DocSnapshot'; import {DocReplacementOptions} from 'app/common/UserAPI'; -import {OptDocSession} from 'app/server/lib/DocSession'; export interface IDocStorageManager { getPath(docName: string): string; @@ -11,8 +10,9 @@ export interface IDocStorageManager { // This method must not be called for the same docName twice in parallel. // In the current implementation, it is called in the context of an // AsyncCreate[docName]. - prepareLocalDoc(docName: string, docSession: OptDocSession): Promise; + prepareLocalDoc(docName: string): Promise; prepareToCreateDoc(docName: string): Promise; + prepareFork(srcDocName: string, destDocName: string): Promise; listDocs(): Promise; deleteDoc(docName: string, deletePermanently?: boolean): Promise; diff --git a/app/server/lib/Permit.ts b/app/server/lib/Permit.ts index 585ea34b..3c5d4168 100644 --- a/app/server/lib/Permit.ts +++ b/app/server/lib/Permit.ts @@ -28,6 +28,7 @@ export interface Permit { docId?: string; workspaceId?: number; org?: string|number; + otherDocId?: string; // For operations involving two documents. } /* A store of permits */ diff --git a/app/server/lib/idUtils.ts b/app/server/lib/idUtils.ts index a700ba1c..23227f37 100644 --- a/app/server/lib/idUtils.ts +++ b/app/server/lib/idUtils.ts @@ -35,14 +35,8 @@ export function makeForkIds(options: { userId: number|null, isAnonymous: boolean }; } -// For importing, we can assign any worker to the job. As a hack, we reuse the document -// assignment mechanism. To spread the work around a bit if we have several doc workers, -// we use a fake document id between import0 and import9. -// This method takes a DocWorkerMap to allow for something smarter in future. +// This used to do a hack for importing, but now does nothing. +// Instead, the server will interpret the special docId "import". export function getAssignmentId(docWorkerMap: IDocWorkerMap, docId: string): string { - let assignmentId = docId; - if (assignmentId === 'import') { - assignmentId = `import${Math.round(Math.random() * 10)}`; - } - return assignmentId; + return docId; }