diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 3fdfd5f122..39768c520d 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -1035,6 +1035,8 @@ export class ActiveDoc extends EventEmitter { unused: 0, }; + const sizesToUpdate = new Map(); + await unpackTarArchive(tarFile, async (file) => { try { const fileIdent = archiveFilePathToAttachmentIdent(file.path); @@ -1044,6 +1046,7 @@ export class ActiveDoc extends EventEmitter { fallbackStoreId, ); if (isAdded) { + sizesToUpdate.set(fileIdent, file.size); results.added += 1; } else { results.unused += 1; @@ -1057,6 +1060,11 @@ export class ActiveDoc extends EventEmitter { } }); + // This updates _grist_Attachments.fileSize with the size of the uploaded files. + // This prevents a loophole where a user has altered `fileSize`, imported the altered document, + // then restored the originals. + await this._updateAttachmentFileSizesUsingFileIdent(docSession, sizesToUpdate); + return results; } @@ -2590,6 +2598,33 @@ export class ActiveDoc extends EventEmitter { }]; } + private async _updateAttachmentFileSizesUsingFileIdent(docSession: OptDocSession, + newFileSizesByFileIdent: Map): Promise { + const rowIdsToUpdate: number[] = []; + const newFileSizesForRows: CellValue[] = []; + const attachments = this.docData?.getMetaTable("_grist_Attachments").getRecords(); + for (const attachmentRec of attachments ?? []) { + const newSize = newFileSizesByFileIdent.get(attachmentRec.fileIdent); + if (newSize && newSize !== attachmentRec.fileSize) { + rowIdsToUpdate.push(attachmentRec.id); + newFileSizesForRows.push(newSize); + } + } + + const action: BulkUpdateRecord = ['BulkUpdateRecord', '_grist_Attachments', rowIdsToUpdate, { + fileSize: newFileSizesForRows + }]; + + await this._applyUserActionsWithExtendedOptions( + docSession, + [action], + { attachment: true }, + ); + + // Updates doc's overall attachment usage to reflect any changes to file sizes. + await this._updateAttachmentsSize(); + } + /** * If the software is newer than the document, migrate the document by fetching all tables, and * giving them to the sandbox so that it can produce migration actions. diff --git a/app/server/lib/Archive.ts b/app/server/lib/Archive.ts index e18cda4b20..46c0751fd6 100644 --- a/app/server/lib/Archive.ts +++ b/app/server/lib/Archive.ts @@ -122,6 +122,7 @@ export function create_tar_archive( export interface UnpackedFile { path: string; data: stream.Readable; + size: number; } export async function unpackTarArchive( @@ -144,6 +145,8 @@ export async function unpackTarArchive( onFile({ path: header.name, data: contentStream, + // Realistically this should never be undefined - it's mandatory for files in a .tar archive + size: header.size ?? 0, }) // No sensible behaviour when an error is thrown by onFile - it's onFile's responsibility // to handle it. diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts index dfc794b28b..e05568b20d 100644 --- a/app/server/lib/DocStorage.ts +++ b/app/server/lib/DocStorage.ts @@ -1385,7 +1385,14 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { -- to make LENGTH() quickly read the stored length instead of actually reading the blob data. -- We use LENGTH() in the first place instead of _grist_Attachments.fileSize because the latter can -- be changed by users. - SELECT MAX(LENGTH(files.data)) AS len + -- External attachments have no internal blob data, so we need to use fileSize for those. + -- To avoid tampering with fileSize in offline records, we update it whenever files are + -- uploaded or retrieved. + SELECT + CASE WHEN files.storageId IS NOT NULL + THEN MAX(meta.fileSize) + ELSE MAX(LENGTH(files.data)) + END AS len FROM _gristsys_Files AS files JOIN _grist_Attachments AS meta ON meta.fileIdent = files.ident diff --git a/test/nbrowser/AttachmentsTransfer.ts b/test/nbrowser/AttachmentsTransfer.ts index 40e31b2754..b9f9074e6d 100644 --- a/test/nbrowser/AttachmentsTransfer.ts +++ b/test/nbrowser/AttachmentsTransfer.ts @@ -1,12 +1,10 @@ import {AttachmentsArchiveParams, DocAPI} from 'app/common/UserAPI'; import fs from 'fs'; import {assert, driver, Key, WebElementPromise} from 'mocha-webdriver'; -import os from 'os'; import path from 'path'; import * as gu from 'test/nbrowser/gristUtils'; import {fileDialogUpload, TestUser} from 'test/nbrowser/gristUtils'; -import {server, setupTestSuite} from 'test/nbrowser/testUtils'; -import * as testUtils from 'test/server/testUtils'; +import {setupTestSuite} from 'test/nbrowser/testUtils'; import {createTmpDir} from 'test/server/docTools'; import axios from 'axios'; import stream from 'node:stream'; @@ -17,26 +15,13 @@ describe("AttachmentsTransfer", function() { const cleanup = setupTestSuite(); let docId: string; let session: gu.Session; - let tmpAttachmentsFolder: string; let tmpDownloadsFolder: string; let reqHeaders: { Authorization: string } | undefined; let api: DocAPI; - let oldEnv: testUtils.EnvironmentSnapshot; - - /** Files will be stored in a folder inside the tmpFolder. Here is a helper that will get files names from it. */ - const files = () => { - const isDir = (f: string) => fs.statSync(path.join(tmpAttachmentsFolder, f)).isDirectory(); - const dirs = fs.readdirSync(tmpAttachmentsFolder).filter(isDir); - if (dirs.length === 0) { return []; } - if (dirs.length > 1) { throw new Error("Unexpected number of directories"); } - const innerFiles = fs.readdirSync(path.join(tmpAttachmentsFolder, dirs[0])); - return innerFiles; - }; + before(async function() { - tmpAttachmentsFolder = fs.mkdtempSync(path.join(os.tmpdir(), 'grist_attachments_')); tmpDownloadsFolder = await createTmpDir(); - oldEnv = new testUtils.EnvironmentSnapshot(); session = await gu.session().teamSite.login(); reqHeaders = { @@ -49,330 +34,334 @@ describe("AttachmentsTransfer", function() { await driver.sendKeys(Key.ESCAPE); }); - after(async function() { - oldEnv.restore(); - await server.restart(); - }); - - it('should show message that transfers are not configured', async function() { - docId = await session.tempNewDoc(cleanup); - api = session.createHomeApi().getDocAPI(docId); + describe("without external attachments enabled", () => { + it('should show message that transfers are not configured', async function() { + docId = await session.tempNewDoc(cleanup); + api = session.createHomeApi().getDocAPI(docId); - // Open document settings. - await gu.openDocumentSettings(); - - // We should see 1 message about no stores. - await gu.waitToPass(async () => assert.lengthOf(await messages(), 1)); - assert.isTrue(await noStoresWarning().isDisplayed()); - }); + // Open document settings. + await gu.openDocumentSettings(); - it('should hide section for non owner', async function() { - // Now login as editor and viewer, and make sure section is hidden. - const homeApi = session.createHomeApi(); - await homeApi.updateDocPermissions(docId, { - users: { - [gu.translateUser("user2").email]: 'viewers', - [gu.translateUser("user3").email]: 'editors' - } + // We should see 1 message about no stores. + await gu.waitToPass(async () => assert.lengthOf(await messages(), 1)); + assert.isTrue(await noStoresWarning().isDisplayed()); }); - async function checkFor(user: TestUser) { - const s = await gu.session().teamSite.user(user).login(); - await s.loadRelPath(`/doc/${docId}`); - await gu.openDocumentSettings(); - await driver.findWait('.test-admin-panel-item-timezone', 1000); - await waitForNotPresent(attachmentSection); - } - - await checkFor('user2'); - await checkFor('user3'); + it('should hide section for non owner', async function() { + // Now login as editor and viewer, and make sure section is hidden. + const homeApi = session.createHomeApi(); + await homeApi.updateDocPermissions(docId, { + users: { + [gu.translateUser("user2").email]: 'viewers', + [gu.translateUser("user3").email]: 'editors' + } + }); + + async function checkFor(user: TestUser) { + const s = await gu.session().teamSite.user(user).login(); + await s.loadRelPath(`/doc/${docId}`); + await gu.openDocumentSettings(); + await driver.findWait('.test-admin-panel-item-timezone', 1000); + await waitForNotPresent(attachmentSection); + } - await session.login(); - await session.loadRelPath(`/doc/${docId}`); - }); + await checkFor('user2'); + await checkFor('user3'); - it("should show transfer menu", async function() { - // Now restart the server. - Object.assign(process.env, { - GRIST_EXTERNAL_ATTACHMENTS_MODE: 'test', - GRIST_TEST_ATTACHMENTS_DIR: tmpAttachmentsFolder, - GRIST_TEST_TRANSFER_DELAY: '500' + await session.login(); + await session.loadRelPath(`/doc/${docId}`); }); - await server.restart(); - await session.loadRelPath(`/doc/${docId}`); + }); - // Open document settings. - await gu.openDocumentSettings(); + describe("with external attachments enabled", () => { + const externalAttachments = gu.enableExternalAttachments('500'); + + /** Files will be stored in a folder inside the tmpFolder. Here is a helper that will get files names from it. */ + const files = () => { + const tmpAttachmentsFolder = externalAttachments.getAttachmentsDir(); + const isDir = (f: string) => fs.statSync(path.join(tmpAttachmentsFolder, f)).isDirectory(); + const dirs = fs.readdirSync(tmpAttachmentsFolder).filter(isDir); + if (dirs.length === 0) { return []; } + if (dirs.length > 1) { throw new Error("Unexpected number of directories"); } + return fs.readdirSync(path.join(tmpAttachmentsFolder, dirs[0])); + }; - // Storage type should be set to Internal - assert.equal(await storageType.value(), 'Internal'); + it("should show transfer menu", async function () { + await session.loadRelPath(`/doc/${docId}`); - // We should see Internal and External options in the storage type dropdown. - assert.deepEqual(await storageType.options(), ['Internal', 'External']); + // Open document settings. + await gu.openDocumentSettings(); - // Now change to internal. - await storageType.select('External'); + // Storage type should be set to Internal + assert.equal(await storageType.value(), 'Internal'); - // The value now should be External. - assert.equal(await storageType.value(), 'External'); + // We should see Internal and External options in the storage type dropdown. + assert.deepEqual(await storageType.options(), ['Internal', 'External']); - // We shouldn't see any info as there are no attachments yet. - assert.lengthOf(await messages(), 0); + // Now change to internal. + await storageType.select('External'); - // Go back to internal. - await storageType.select('Internal'); - }); + // The value now should be External. + assert.equal(await storageType.value(), 'External'); - it("should show actions when some attachments are added", async function() { - // Upload four attachments. - await gu.openPage('Table1'); - await gu.selectColumn('A'); - await gu.setType('Attachment'); - await gu.toggleSidePanel('right', 'close'); - await addRow(); - const cell = await gu.getCell('A', 1); - await gu.openUploadDialog(cell); - await gu.uploadFiles('uploads/file1.mov', 'uploads/file2.mp3', 'uploads/file3.zip', 'uploads/simple_array.json'); - await gu.waitForAttachments(cell, 4); - - // Now switch to external to test the copy. - await gu.openDocumentSettings(); - await gu.toggleSidePanel('left', 'close'); - await storageType.select('External'); - - // We should see a message about attachments being still internal. - assert.lengthOf(await messages(), 1); - assert.isTrue(await internalCopy().isDisplayed()); - - // We should see start transfer button. - assert.isTrue(await startTransferButton().isDisplayed()); - - // When we switch back to internal, the message should be gone. - await storageType.select('Internal'); - assert.lengthOf(await messages(), 0); - assert.isFalse(await startTransferButton().isPresent()); - - // Now switch back to external. - await storageType.select('External'); - assert.lengthOf(await messages(), 1); - assert.isTrue(await internalCopy().isDisplayed()); - assert.isTrue(await startTransferButton().isDisplayed()); - }); + // We shouldn't see any info as there are no attachments yet. + assert.lengthOf(await messages(), 0); - it('should transfer files to external storage', async function() { - // First make sure that the tmp folder is empty. - assert.lengthOf(files(), 0); + // Go back to internal. + await storageType.select('Internal'); + }); - // Start transfer. - await startTransferButton().click(); - await gu.waitForServer(); + it("should show actions when some attachments are added", async function () { + // Upload four attachments. + await gu.openPage('Table1'); + await gu.selectColumn('A'); + await gu.setType('Attachment'); + await gu.toggleSidePanel('right', 'close'); + await addRow(); + const cell = await gu.getCell('A', 1); + await gu.openUploadDialog(cell); + await gu.uploadFiles('uploads/file1.mov', 'uploads/file2.mp3', 'uploads/file3.zip', 'uploads/simple_array.json'); + await gu.waitForAttachments(cell, 4); + + // Now switch to external to test the copy. + await gu.openDocumentSettings(); + await gu.toggleSidePanel('left', 'close'); + await storageType.select('External'); + + // We should see a message about attachments being still internal. + assert.lengthOf(await messages(), 1); + assert.isTrue(await internalCopy().isDisplayed()); + + // We should see start transfer button. + assert.isTrue(await startTransferButton().isDisplayed()); + + // When we switch back to internal, the message should be gone. + await storageType.select('Internal'); + assert.lengthOf(await messages(), 0); + assert.isFalse(await startTransferButton().isPresent()); + + // Now switch back to external. + await storageType.select('External'); + assert.lengthOf(await messages(), 1); + assert.isTrue(await internalCopy().isDisplayed()); + assert.isTrue(await startTransferButton().isDisplayed()); + }); - // We should see transfer spinner. - await waitForDisplay(transferSpinner); + it('should transfer files to external storage', async function () { + // First make sure that the tmp folder is empty. + assert.lengthOf(files(), 0); - // Wait for the spinner to disappear. - await waitForNotPresent(transferSpinner); + // Start transfer. + await startTransferButton().click(); + await gu.waitForServer(); - // We now should have those files transfer. - assert.lengthOf(files(), 4); + // We should see transfer spinner. + await waitForDisplay(transferSpinner); - // We are not testing here if transfer works or not, just the correct number of files is enough. + // Wait for the spinner to disappear. + await waitForNotPresent(transferSpinner); - // Make sure that transfer button is gone. - assert.isFalse(await startTransferButton().isPresent()); + // We now should have those files transfer. + assert.lengthOf(files(), 4); - // And we don't have any messages. - assert.lengthOf(await messages(), 0); - }); + // We are not testing here if transfer works or not, just the correct number of files is enough. - it('warns users downloading the doc that attachments are external', async function() { - try { - await driver.find('.test-tb-share').click(); - await driver.findContentWait('.test-tb-share-option', /Download document/, 5000).click(); - const attachmentsMsg = await driver.findWait('.test-external-attachments-info', 1000).getText(); - - assert.match(attachmentsMsg, /Attachments are external/, "should be informed attachments aren't included"); - - const downloadHref = await driver.find('.test-external-attachments-info a').getAttribute('href'); - const downloadUrl = new URL(downloadHref); - const idealUrl = new URL(api.getDownloadAttachmentsArchiveUrl({ format: 'tar' })); - assert.equal(downloadUrl.pathname, idealUrl.pathname, "wrong download link called"); - assert.equal(downloadUrl.search, idealUrl.search, "wrong search parameters in url"); - // Ensures the page isn't modified / navigated away from by the link, as subsequent tests will fail. - await driver.find('.test-external-attachments-info a').click(); - } finally { - // Try to close the modal to minimise the chances of other tests failing. - await driver.findContent("button", "Cancel").click(); - } - }); + // Make sure that transfer button is gone. + assert.isFalse(await startTransferButton().isPresent()); - it('can download attachments', async function() { - try { - await driver.find('.test-tb-share').click(); - await driver.findContentWait('.test-tb-share-option', /Download attachments/, 5000).click(); - const attachmentsStatus = await driver.findWait('.test-attachments-external-message', 1000).getText(); + // And we don't have any messages. + assert.lengthOf(await messages(), 0); + }); - assert.match(attachmentsStatus, /in the ".tar" format/, "users should be advised to use .tar"); + it('warns users downloading the doc that attachments are external', async function () { + try { + await driver.find('.test-tb-share').click(); + await driver.findContentWait('.test-tb-share-option', /Download document/, 5000).click(); + const attachmentsMsg = await driver.findWait('.test-external-attachments-info', 1000).getText(); - const selectFormat = async (formatRegex: RegExp) => { - await driver.findWait('.test-attachments-format-select', 500).click(); - await driver.findContentWait('.test-attachments-format-options .test-select-row', formatRegex, 500).click(); - }; + assert.match(attachmentsMsg, /Attachments are external/, "should be informed attachments aren't included"); - const testDownloadLink = async (params: AttachmentsArchiveParams) => { - const downloadUrl = new URL(await driver.find('.test-download-attachments-button-link').getAttribute('href')); - const idealUrl = new URL(api.getDownloadAttachmentsArchiveUrl(params)); + const downloadHref = await driver.find('.test-external-attachments-info a').getAttribute('href'); + const downloadUrl = new URL(downloadHref); + const idealUrl = new URL(api.getDownloadAttachmentsArchiveUrl({format: 'tar'})); assert.equal(downloadUrl.pathname, idealUrl.pathname, "wrong download link called"); assert.equal(downloadUrl.search, idealUrl.search, "wrong search parameters in url"); - const response = await axios.get(downloadUrl.toString(), { - headers: reqHeaders, - responseType: "stream" - }); - // Download the file so we've got a copy available for uploading. - const fileName = `attachments.${params.format}`; - assert.notInclude(await fse.readdir(tmpDownloadsFolder), fileName, "attachments file shouldn't exist yet"); - await stream.promises.pipeline( - response.data, - fs.createWriteStream(path.join(tmpDownloadsFolder, fileName)) - ); - assert.include(await fse.readdir(tmpDownloadsFolder), fileName, "attachments file wasn't downloaded"); - }; - - - await selectFormat(/.tar/); - await gu.waitToPass(() => testDownloadLink({format: 'tar'}), 500); - await selectFormat(/.zip/); - await gu.waitToPass(() => testDownloadLink({format: 'zip'}), 500); - } finally { - // Try to close the modal to minimise the chances of other tests failing. - await driver.findContent("button", "Cancel").click(); - } - }); + // Ensures the page isn't modified / navigated away from by the link, as subsequent tests will fail. + await driver.find('.test-external-attachments-info a').click(); + } finally { + // Try to close the modal to minimise the chances of other tests failing. + await driver.findContent("button", "Cancel").click(); + } + }); - it('can upload attachments', async function() { - const file = path.join(tmpDownloadsFolder, 'attachments.tar'); - await fileDialogUpload(file, async () => { - await driver.find('.test-settings-upload-attachment-archive').click(); + it('can download attachments', async function () { + try { + await driver.find('.test-tb-share').click(); + await driver.findContentWait('.test-tb-share-option', /Download attachments/, 5000).click(); + const attachmentsStatus = await driver.findWait('.test-attachments-external-message', 1000).getText(); + + assert.match(attachmentsStatus, /in the ".tar" format/, "users should be advised to use .tar"); + + const selectFormat = async (formatRegex: RegExp) => { + await driver.findWait('.test-attachments-format-select', 500).click(); + await driver.findContentWait('.test-attachments-format-options .test-select-row', formatRegex, 500).click(); + }; + + const testDownloadLink = async (params: AttachmentsArchiveParams) => { + const downloadUrl = new URL(await driver.find('.test-download-attachments-button-link').getAttribute('href')); + const idealUrl = new URL(api.getDownloadAttachmentsArchiveUrl(params)); + assert.equal(downloadUrl.pathname, idealUrl.pathname, "wrong download link called"); + assert.equal(downloadUrl.search, idealUrl.search, "wrong search parameters in url"); + const response = await axios.get(downloadUrl.toString(), { + headers: reqHeaders, + responseType: "stream" + }); + // Download the file so we've got a copy available for uploading. + const fileName = `attachments.${params.format}`; + assert.notInclude(await fse.readdir(tmpDownloadsFolder), fileName, "attachments file shouldn't exist yet"); + await stream.promises.pipeline( + response.data, + fs.createWriteStream(path.join(tmpDownloadsFolder, fileName)) + ); + assert.include(await fse.readdir(tmpDownloadsFolder), fileName, "attachments file wasn't downloaded"); + }; + + + await selectFormat(/.tar/); + await gu.waitToPass(() => testDownloadLink({format: 'tar'}), 500); + await selectFormat(/.zip/); + await gu.waitToPass(() => testDownloadLink({format: 'zip'}), 500); + } finally { + // Try to close the modal to minimise the chances of other tests failing. + await driver.findContent("button", "Cancel").click(); + } + }); + + it('can upload attachments', async function () { + const file = path.join(tmpDownloadsFolder, 'attachments.tar'); + await fileDialogUpload(file, async () => { + await driver.find('.test-settings-upload-attachment-archive').click(); + }); + assert.match( + await driver.findWait('.test-notifier-toast-message', 1000).getText(), + // Only care that the request is made, and that the UI behaves as expected. + // Don't care about checking attachments reconnect behaviour - API tests cover that. + /0 attachment files reconnected/ + ); + await driver.findWait('.test-notifier-toast-close', 2000).click(); }); - assert.match( - await driver.findWait('.test-notifier-toast-message', 1000).getText(), - // Only care that the request is made, and that the UI behaves as expected. - // Don't care about checking attachments reconnect behaviour - API tests cover that. - /0 attachment files reconnected/ - ); - await driver.findWait('.test-notifier-toast-close', 2000).click(); - }); - it('should transfer files to internal storage', async function() { - // Switch to internal. - await storageType.select('Internal'); + it('should transfer files to internal storage', async function () { + // Switch to internal. + await storageType.select('Internal'); - // We should see new copy and transfer button. - assert.lengthOf(await messages(), 1); - assert.isFalse(await internalCopy().isPresent()); - assert.isTrue(await externalCopy().isDisplayed()); - assert.isTrue(await startTransferButton().isDisplayed()); + // We should see new copy and transfer button. + assert.lengthOf(await messages(), 1); + assert.isFalse(await internalCopy().isPresent()); + assert.isTrue(await externalCopy().isDisplayed()); + assert.isTrue(await startTransferButton().isDisplayed()); - // Switching back hides everything. - await storageType.select('External'); - assert.lengthOf(await messages(), 0); - assert.isFalse(await startTransferButton().isPresent()); + // Switching back hides everything. + await storageType.select('External'); + assert.lengthOf(await messages(), 0); + assert.isFalse(await startTransferButton().isPresent()); - // Switch back to internal. - await storageType.select('Internal'); + // Switch back to internal. + await storageType.select('Internal'); - // Start transfer. - await startTransferButton().click(); + // Start transfer. + await startTransferButton().click(); - // We should see transfer spinner. - assert.isTrue(await transferSpinner(WAIT).isDisplayed()); + // We should see transfer spinner. + assert.isTrue(await transferSpinner(WAIT).isDisplayed()); - // Wait for the spinner to disappear. - await gu.waitToPass(async () => assert.isFalse(await transferSpinner().isPresent())); - await gu.waitForServer(); + // Wait for the spinner to disappear. + await gu.waitToPass(async () => assert.isFalse(await transferSpinner().isPresent())); + await gu.waitForServer(); - // We should see that internal storage is selected. - assert.equal(await storageType.value(), 'Internal'); + // We should see that internal storage is selected. + assert.equal(await storageType.value(), 'Internal'); - // And we don't have any messages here. - assert.lengthOf(await messages(), 0); + // And we don't have any messages here. + assert.lengthOf(await messages(), 0); - // Even after reload. - await driver.navigate().refresh(); - await storageType.waitForDisplay(); - assert.lengthOf(await messages(), 0); - assert.equal(await storageType.value(), 'Internal'); - }); + // Even after reload. + await driver.navigate().refresh(); + await storageType.waitForDisplay(); + assert.lengthOf(await messages(), 0); + assert.equal(await storageType.value(), 'Internal'); + }); - // Here we do the same stuff but with the API calls, and we expect that the UI will react to it. - it('user should be able to observe background actions', async function() { - // Sanity check. - assert.equal(await storageType.value(), 'Internal'); + // Here we do the same stuff but with the API calls, and we expect that the UI will react to it. + it('user should be able to observe background actions', async function () { + // Sanity check. + assert.equal(await storageType.value(), 'Internal'); - // Set to external. - await api.setAttachmentStore('external'); + // Set to external. + await api.setAttachmentStore('external'); - // The value should be changed. - await storageType.waitForValue('External'); + // The value should be changed. + await storageType.waitForValue('External'); - // We should see the message. - assert.lengthOf(await messages(), 1); - assert.isTrue(await internalCopy().isDisplayed()); - // And the button to start transfer. - assert.isTrue(await startTransferButton().isDisplayed()); + // We should see the message. + assert.lengthOf(await messages(), 1); + assert.isTrue(await internalCopy().isDisplayed()); + // And the button to start transfer. + assert.isTrue(await startTransferButton().isDisplayed()); - // Move back to internal and check that the message is gone. - await api.setAttachmentStore('internal'); - await storageType.waitForValue('Internal'); - assert.lengthOf(await messages(), 0); - assert.isFalse(await startTransferButton().isPresent()); + // Move back to internal and check that the message is gone. + await api.setAttachmentStore('internal'); + await storageType.waitForValue('Internal'); + assert.lengthOf(await messages(), 0); + assert.isFalse(await startTransferButton().isPresent()); - // Set to external again. - await api.setAttachmentStore('external'); - await storageType.waitForValue('External'); - await waitForDisplay(startTransferButton); + // Set to external again. + await api.setAttachmentStore('external'); + await storageType.waitForValue('External'); + await waitForDisplay(startTransferButton); - // We are seeing that some files are internal. - assert.isTrue(await internalCopy().isDisplayed()); + // We are seeing that some files are internal. + assert.isTrue(await internalCopy().isDisplayed()); - // The copy version is static. - assert.isTrue(await internalCopy().isStatic()); + // The copy version is static. + assert.isTrue(await internalCopy().isStatic()); - // And start transfer. - await api.transferAllAttachments(); + // And start transfer. + await api.transferAllAttachments(); - // Wait for the spinner to be shown. - await waitForDisplay(transferSpinner); + // Wait for the spinner to be shown. + await waitForDisplay(transferSpinner); - // The internal copy should be changed during the transfer. - assert.isTrue(await internalCopy().inProgress()); + // The internal copy should be changed during the transfer. + assert.isTrue(await internalCopy().inProgress()); - // Wait for the spinner to disappear. - await waitForNotPresent(transferSpinner); + // Wait for the spinner to disappear. + await waitForNotPresent(transferSpinner); - // Transfer button should also disappear - await waitForNotPresent(startTransferButton); + // Transfer button should also disappear + await waitForNotPresent(startTransferButton); - // And all messages should be gone. - assert.lengthOf(await messages(), 0); + // And all messages should be gone. + assert.lengthOf(await messages(), 0); - // Now go back to internal. - await api.setAttachmentStore('internal'); - await storageType.waitForValue('Internal'); - assert.lengthOf(await messages(), 1); - assert.isTrue(await externalCopy().isDisplayed()); - assert.isTrue(await externalCopy().isStatic()); - assert.isTrue(await startTransferButton().isDisplayed()); - assert.isFalse(await transferSpinner().isPresent()); + // Now go back to internal. + await api.setAttachmentStore('internal'); + await storageType.waitForValue('Internal'); + assert.lengthOf(await messages(), 1); + assert.isTrue(await externalCopy().isDisplayed()); + assert.isTrue(await externalCopy().isStatic()); + assert.isTrue(await startTransferButton().isDisplayed()); + assert.isFalse(await transferSpinner().isPresent()); - // Start transfer and check components. - await api.transferAllAttachments(); - await waitForDisplay(transferSpinner); - assert.isTrue(await externalCopy().inProgress()); - await waitForNotPresent(transferSpinner); - await waitForNotPresent(startTransferButton); - assert.lengthOf(await messages(), 0); + // Start transfer and check components. + await api.transferAllAttachments(); + await waitForDisplay(transferSpinner); + assert.isTrue(await externalCopy().inProgress()); + await waitForNotPresent(transferSpinner); + await waitForNotPresent(startTransferButton); + assert.lengthOf(await messages(), 0); + }); }); }); diff --git a/test/nbrowser/DocUsageTracking.ts b/test/nbrowser/DocUsageTracking.ts new file mode 100644 index 0000000000..2830835bc6 --- /dev/null +++ b/test/nbrowser/DocUsageTracking.ts @@ -0,0 +1,182 @@ +import {UserAPI} from 'app/common/UserAPI'; +import {assert, driver, Key} from 'mocha-webdriver'; +import fetch from 'node-fetch'; +import * as gu from 'test/nbrowser/gristUtils'; +import {server} from 'test/nbrowser/testServer'; +import {setupTestSuite} from 'test/nbrowser/testUtils'; + +describe('DocumentUsage', function() { + this.timeout(20000); + const cleanup = setupTestSuite(); + + const ownerUser = 'user1'; + let api: UserAPI; + let session: gu.Session; + + gu.enableExternalAttachments(); + + async function makeSessionAndLogin() { + // login() needs an options object passing to bypass an optimization that causes .login() + // to think we're already logged in when we're not after using `server.restart()`. + // Without this we end up with old credentials on the original session, or bad credentials on a new one. + session = await gu.session().user(ownerUser).login({ retainExistingLogin: false }); + api = session.createHomeApi(); + } + + before(async function () { + await makeSessionAndLogin(); + }); + + it('shows usage stats on the raw data page', async function() { + await session.tempNewDoc(cleanup, "EmptyUsageDoc"); + await testDocUsageStatsAreZero(); + }); + + function testAttachmentsUsage(getDocId: () => string) { + it('updates attachments size usage when uploading attachments', async function () { + const docId = getDocId(); + // Add a new 'Attachments' column of type Attachment to Table1. + await api.applyUserActions(docId, [['AddEmptyTable', "AttachmentsTable"]]); + await gu.getPageItem('AttachmentsTable').click(); + await gu.waitForServer(); + await addAttachmentColumn('Attachments'); + + // Upload some files into the first row. (We're putting Grist docs in a Grist doc!) + await driver.sendKeys(Key.ENTER); + await gu.fileDialogUpload( + 'docs/Covid-19.grist,docs/World-v0.grist,docs/World-v1.grist,docs/World-v3.grist,' + + 'docs/Landlord.grist,docs/ImportReferences.grist,docs/WorldUndo.grist,' + + 'docs/Ref-List-AC-Test.grist,docs/PasteParsing.grist', + () => driver.find('.test-pw-add').click() + ); + // Check all 9 attachments have uploaded. + await driver.findContentWait('.test-pw-counter', /of 9/, 4000); + await driver.find('.test-modal-dialog .test-pw-close').click(); + await gu.waitForServer(); + + // Navigate back to the raw data page, and check that attachments size updated. + await goToDocUsage(); + await assertDataSize('0.00'); + await assertAttachmentsSize('0.01'); + + // Delete the 'Attachments' column; usage should not immediately update. + await api.applyUserActions(docId, [['RemoveColumn', 'AttachmentsTable', 'Attachments']]); + await assertDataSize('0.00'); + await assertAttachmentsSize('0.01'); + + // Remove unused attachments via API and check that size automatically updates to 0. + await removeUnusedAttachments(api, docId); + await assertDataSize('0.00'); + await assertAttachmentsSize('0.00'); + }); + } + + describe('attachment usage without external attachments', function() { + let docId: string; + + before(async () => { + docId = await session.tempNewDoc(cleanup, `AttachmentUsageTestDoc - internal`); + }); + + testAttachmentsUsage(() => docId); + }); + + describe('attachment usage with external attachments', function() { + let docId: string; + + before(async () => { + docId = await session.tempNewDoc(cleanup, `AttachmentUsageTestDoc - internal`); + const docApi = api.getDocAPI(docId); + await docApi.setAttachmentStore("external"); + assert.equal((await docApi.getAttachmentStore()).type, "external"); + }); + + testAttachmentsUsage(() => docId); + }); +}); + +async function testDocUsageStatsAreZero() { + // Check that the Usage section exists. + await goToDocUsage(); + assert.equal(await driver.find('.test-doc-usage-heading').getText(), 'Usage'); + await assertUsageMessage(null); + + // Check that usage is at 0. + await assertRowCount('0'); + await assertDataSize('0.00'); + await assertAttachmentsSize('0.00'); + + // Check that banners aren't shown on the raw data page. + await gu.assertBannerText(null); +} + +async function goToDocUsage() { + await driver.findWait('.test-tools-raw', 2000).click(); + + // Check that the Usage section exists. + await waitForDocUsage(); +} + +async function assertUsageMessage(text: string | null) { + if (text === null) { + assert.isFalse(await driver.find('.test-doc-usage-message').isPresent()); + } else { + assert.equal(await driver.findWait('.test-doc-usage-message-text', 2000).getText(), text); + } +} + +async function assertRowCount(currentValue: string, maximumValue?: string) { + await gu.waitToPass(async () => { + const rowUsage = await driver.find('.test-doc-usage-rows .test-doc-usage-value').getText(); + const [, foundValue, foundMax] = rowUsage.match(/([0-9,]+) (?:of ([0-9,]+) )?rows/) || []; + assert.equal(foundValue, currentValue); + if (maximumValue) { + assert.equal(foundMax, maximumValue); + } + }); +} + +async function assertDataSize(currentValue: string, maximumValue?: string) { + await gu.waitToPass(async () => { + const dataUsage = await driver.find('.test-doc-usage-data-size .test-doc-usage-value').getText(); + const [, foundValue, foundMax] = dataUsage.match(/([0-9,.]+) (?:of ([0-9,.]+) )?MB/) || []; + assert.equal(foundValue, currentValue); + if (maximumValue) { + assert.equal(foundMax, maximumValue); + } + }); +} + +async function assertAttachmentsSize(currentValue: string, maximumValue?: string) { + await gu.waitToPass(async () => { + const attachmentUsage = await driver.find('.test-doc-usage-attachments-size .test-doc-usage-value').getText(); + const [, foundValue, foundMax] = attachmentUsage.match(/([0-9,.]+) (?:of ([0-9,.]+) )?GB/) || []; + assert.equal(foundValue, currentValue); + if (maximumValue) { + assert.equal(foundMax, maximumValue); + } + }); +} + +async function waitForDocUsage() { + await driver.findWait('.test-doc-usage-container', 8000); + await gu.waitToPass(async () => { + return assert.isFalse(await driver.find('.test-doc-usage-loading').isPresent()); + }); +} + +async function addAttachmentColumn(columnName: string) { + await gu.toggleSidePanel('right', 'open'); + await driver.find('.test-right-tab-field').click(); + await gu.addColumn(columnName); + await gu.setType(/Attachment/); +} + +async function removeUnusedAttachments(api: UserAPI, docId: string) { + const headers = {Authorization: `Bearer ${await api.fetchApiKey()}`}; + const url = server.getUrl('docs', `/api/docs/${docId}`); + await fetch(url + "/attachments/removeUnused?verifyfiles=0&expiredonly=0", { + headers, + method: "POST" + }); +} diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 8729c3716a..6dc07e6d17 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -38,6 +38,8 @@ import * as testUtils from 'test/server/testUtils'; import type { AssertionError } from 'assert'; import axios from 'axios'; import { lock } from 'proper-lockfile'; +import {createTmpDir} from 'test/server/docTools'; +import {mkdtemp} from 'fs-extra'; // tslint:disable:no-namespace // Wrap in a namespace so that we can apply stackWrapOwnMethods to all the exports together. @@ -3618,6 +3620,59 @@ export function withEnvironmentSnapshot(vars: Record) { }); } +export function enableExternalAttachments(transferDelay?: string, preserveEnvVars = true) { + const envVars: Record = { + GRIST_EXTERNAL_ATTACHMENTS_MODE: 'test', + GRIST_TEST_ATTACHMENTS_DIR: "", + }; + + if (transferDelay) { + envVars.GRIST_TEST_TRANSFER_DELAY = transferDelay; + } + + const originalEnv: Record = {}; + + before(async () => { + const tempFolder = await createTmpDir(); + envVars.GRIST_TEST_ATTACHMENTS_DIR = await mkdtemp(path.join(tempFolder, 'attachments')); + + if (preserveEnvVars) { + for (const envVar of Object.keys(envVars)) { + originalEnv[envVar] = process.env[envVar]; + } + } + + for (const [envVar, value] of Object.entries(envVars)) { + process.env[envVar] = value; + } + + await server.restart(); + }); + + after(async () => { + if (!preserveEnvVars) { return; } + + const originalEnvVars = Object.entries(originalEnv); + + for (const [envVar, value] of originalEnvVars) { + if (value === undefined) { + delete process.env[envVar]; + } else { + process.env[envVar] = value; + } + } + + await server.restart(); + }); + + return { + envVars, + getAttachmentsDir() { + return envVars.GRIST_TEST_ATTACHMENTS_DIR; + } + }; +} + /** * Helper to scroll creator panel top or bottom. By default bottom. */ diff --git a/test/server/lib/ActiveDoc.ts b/test/server/lib/ActiveDoc.ts index 52d9806e59..6189f404a0 100644 --- a/test/server/lib/ActiveDoc.ts +++ b/test/server/lib/ActiveDoc.ts @@ -8,7 +8,10 @@ import {TableData} from 'app/common/TableData'; import {GristObjCode} from 'app/plugin/GristData'; import {ActiveDoc} from 'app/server/lib/ActiveDoc'; import {getDocPoolIdFromDocInfo} from 'app/server/lib/AttachmentStore'; -import {AttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider'; +import { + AttachmentStoreProvider, + IAttachmentStoreProvider +} from 'app/server/lib/AttachmentStoreProvider'; import {DummyAuthorizer} from 'app/server/lib/Authorizer'; import {AuthSession} from 'app/server/lib/AuthSession'; import {Client} from 'app/server/lib/Client'; @@ -1228,32 +1231,76 @@ describe('ActiveDoc', async function() { } }); - it('can import missing attachments from an archive', async function() { - const docName = 'add-missing-attachments'; - const activeDoc = await docTools.createDoc(docName); + describe('restoring attachments', () => { + let activeDoc: ActiveDoc; + let provider: IAttachmentStoreProvider; + let externalStoreId: string; + + beforeEach(async function() { + activeDoc = await docTools.createDoc(this.currentTest?.title ?? 'restore-attachments'); + provider = docTools.getAttachmentStoreProvider(); + externalStoreId = provider.listAllStoreIds()[0]; + + await activeDoc.setAttachmentStore(fakeSession, externalStoreId); + + await uploadAttachments(activeDoc, testAttachments); + }); + + async function deleteAttachmentsFromStorage() { + const store = (await provider.getStore(externalStoreId))!; + // Purge any attachments related to this doc. + await store.removePool(getDocPoolIdFromDocInfo({ id: activeDoc.docName, trunkId: undefined })); + } + + async function downloadAttachmentsTarArchive() { + const attachmentsArchive = await activeDoc.getAttachmentsArchive(fakeSession, "tar"); + const attachmentsTarStream = new MemoryWritableStream(); + await attachmentsArchive.packInto(attachmentsTarStream); + return attachmentsTarStream.getBuffer(); + } + + it('can import missing attachments from an archive', async function() { + const attachmentsTar = await downloadAttachmentsTarArchive(); + await deleteAttachmentsFromStorage(); + + const result1 = await activeDoc.addMissingFilesFromArchive(fakeSession, stream.Readable.from(attachmentsTar)); + assert.equal(result1.added, testAttachments.length, "all attachments should be added"); + + const result2 = await activeDoc.addMissingFilesFromArchive(fakeSession, stream.Readable.from(attachmentsTar)); + assert.equal(result2.added, 0, "no attachments should be added"); + assert.equal(result2.unused, testAttachments.length, "all attachments should be unused"); + }); + + it('updates the document\'s attachment usage on .tar upload', async function() { + const systemSession = makeExceptionalDocSession('system'); - const provider = docTools.getAttachmentStoreProvider(); - const storeId = provider.listAllStoreIds()[0]; + const attachmentsTar = await downloadAttachmentsTarArchive(); + await deleteAttachmentsFromStorage(); - await activeDoc.setAttachmentStore(fakeSession, storeId); + const getAttachmentTableData = async () => + (await activeDoc.fetchTable(systemSession, '_grist_Attachments')).tableData; - await uploadAttachments(activeDoc, testAttachments); + const rowIds = (await getAttachmentTableData())[2]; - const attachmentsArchive = await activeDoc.getAttachmentsArchive(fakeSession, "tar"); - const attachmentsTarStream = new MemoryWritableStream(); - await attachmentsArchive.packInto(attachmentsTarStream); - const attachmentsTar = attachmentsTarStream.getBuffer(); + const getFileSizes = async () => + (await getAttachmentTableData())[3].fileSize; - const store = (await provider.getStore(storeId))!; - // Purge any attachments related to this doc. - await store.removePool(getDocPoolIdFromDocInfo({ id: activeDoc.docName, trunkId: undefined })); + const originalFileSizes = await getFileSizes(); + assert(originalFileSizes.every(size => size && size > 0), 'uploaded files should have non-zero sizes'); - const result1 = await activeDoc.addMissingFilesFromArchive(fakeSession, stream.Readable.from(attachmentsTar)); - assert.equal(result1.added, testAttachments.length, "all attachments should be added"); + // Sets all file sizes in _grist_Attachments to zero. + await activeDoc.applyUserActions( + systemSession, + [['BulkUpdateRecord', '_grist_Attachments', rowIds, { fileSize: rowIds.map(() => 0) }]] + ); - const result2 = await activeDoc.addMissingFilesFromArchive(fakeSession, stream.Readable.from(attachmentsTar)); - assert.equal(result2.added, 0, "no attachments should be added"); - assert.equal(result2.unused, testAttachments.length, "all attachments should be unused"); + const zeroedFileSizes = await getFileSizes(); + assert(zeroedFileSizes.every(size => size === 0), 'all file sizes should be 0'); + + await activeDoc.addMissingFilesFromArchive(fakeSession, stream.Readable.from(attachmentsTar)); + const restoredFileSizes = await getFileSizes(); + assert.deepEqual(restoredFileSizes, originalFileSizes, 'restored file sizes should match originals'); + }); }); /*