From df233c95a9b846259c8122d981a3812fb71d8cea Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Thu, 18 Feb 2021 22:36:07 -0800 Subject: [PATCH] refactor: Upload file to storage, and then pass attachmentId to collections.import This avoids having large file uploads going directly to the server and allows us to fetch it async into a worker process --- app/scenes/Settings/ImportExport.js | 27 +++++++++--------- app/stores/CollectionsStore.js | 18 ++++-------- app/utils/uploadFile.js | 4 ++- server/api/attachments.js | 3 +- server/api/collections.js | 33 +++++++++++++--------- server/api/collections.test.js | 2 +- server/commands/collectionImporter.js | 13 +++++++-- server/models/Attachment.js | 5 +++- server/policies/attachment.js | 2 +- server/utils/s3.js | 7 +++-- server/utils/zip.js | 4 +-- shared/i18n/locales/en_US/translation.json | 6 ++-- 12 files changed, 70 insertions(+), 54 deletions(-) diff --git a/app/scenes/Settings/ImportExport.js b/app/scenes/Settings/ImportExport.js index 78a17e0fc..9ae3c5949 100644 --- a/app/scenes/Settings/ImportExport.js +++ b/app/scenes/Settings/ImportExport.js @@ -1,4 +1,5 @@ // @flow +import invariant from "invariant"; import { observer } from "mobx-react"; import { CollectionIcon } from "outline-icons"; import * as React from "react"; @@ -14,6 +15,7 @@ import PageTitle from "components/PageTitle"; import useCurrentUser from "hooks/useCurrentUser"; import useStores from "hooks/useStores"; import getDataTransferFiles from "utils/getDataTransferFiles"; +import { uploadFile } from "utils/uploadFile"; function ImportExport() { const { t } = useTranslation(); @@ -23,7 +25,7 @@ function ImportExport() { const { showToast } = ui; const [isLoading, setLoading] = React.useState(false); const [isImporting, setImporting] = React.useState(false); - const [importedDetails, setImported] = React.useState(false); + const [isImported, setImported] = React.useState(false); const [isExporting, setExporting] = React.useState(false); const [file, setFile] = React.useState(); const [importDetails, setImportDetails] = React.useState(); @@ -34,11 +36,13 @@ function ImportExport() { setImporting(true); try { - const { documentCount, collectionCount } = await collections.import( - file - ); - showToast(t("Import completed")); - setImported({ documentCount, collectionCount }); + invariant(file, "File must exist to upload"); + const attachment = await uploadFile(file, { + name: file.name, + }); + await collections.import(attachment.id); + showToast(t("Import started")); + setImported(true); } catch (err) { showToast(err.message); } finally { @@ -121,14 +125,11 @@ function ImportExport() { accept="application/zip" /> - {importedDetails && ( + {isImported && ( - - Import successful, {{ count: importedDetails.documentCount }}{" "} - documents were imported to your knowledge base. + + Your file has been uploaded and the import is being processed, you + can safely leave this page. )} diff --git a/app/stores/CollectionsStore.js b/app/stores/CollectionsStore.js index c2920dc58..6f9c786a5 100644 --- a/app/stores/CollectionsStore.js +++ b/app/stores/CollectionsStore.js @@ -1,5 +1,4 @@ // @flow -import invariant from "invariant"; import { concat, filter, last } from "lodash"; import { computed, action } from "mobx"; import naturalSort from "shared/utils/naturalSort"; @@ -89,18 +88,11 @@ export default class CollectionsStore extends BaseStore { } @action - import = async (file: File) => { - const formData = new FormData(); - formData.append("type", "outline"); - formData.append("file", file); - - const res = await client.post("/collections.import", formData); - invariant(res && res.data, "Data should be available"); - - this.addPolicies(res.policies); - res.data.collections.forEach(this.add); - - return res.data; + import = async (attachmentId: string) => { + await client.post("/collections.import", { + type: "outline", + attachmentId, + }); }; async update(params: Object): Promise { diff --git a/app/utils/uploadFile.js b/app/utils/uploadFile.js index 09ea37567..8cd7384e2 100644 --- a/app/utils/uploadFile.js +++ b/app/utils/uploadFile.js @@ -39,11 +39,13 @@ export const uploadFile = async ( formData.append("file", file); } - await fetch(data.uploadUrl, { + const uploadResponse = await fetch(data.uploadUrl, { method: "post", body: formData, }); + invariant(uploadResponse.ok, "Upload failed, try again?"); + return attachment; }; diff --git a/server/api/attachments.js b/server/api/attachments.js index a2d34d547..0cc3b3770 100644 --- a/server/api/attachments.js +++ b/server/api/attachments.js @@ -38,7 +38,7 @@ router.post("attachments.create", auth(), async (ctx) => { const key = `${bucket}/${user.id}/${s3Key}/${name}`; const credential = makeCredential(); const longDate = format(new Date(), "YYYYMMDDTHHmmss\\Z"); - const policy = makePolicy(credential, longDate, acl); + const policy = makePolicy(credential, longDate, acl, contentType); const endpoint = publicS3Endpoint(); const url = `${endpoint}/${key}`; @@ -85,6 +85,7 @@ router.post("attachments.create", auth(), async (ctx) => { documentId, contentType, name, + id: attachment.id, url: attachment.redirectUrl, size, }, diff --git a/server/api/collections.js b/server/api/collections.js index 50476786b..121ad8595 100644 --- a/server/api/collections.js +++ b/server/api/collections.js @@ -1,8 +1,10 @@ // @flow import fs from "fs"; +import os from "os"; +import File from "formidable/lib/file"; import Router from "koa-router"; import collectionImporter from "../commands/collectionImporter"; -import { ValidationError, InvalidRequestError } from "../errors"; +import { ValidationError } from "../errors"; import { exportCollections } from "../logistics"; import auth from "../middlewares/authentication"; import { @@ -13,6 +15,7 @@ import { Event, User, Group, + Attachment, } from "../models"; import policy from "../policies"; import { @@ -100,23 +103,27 @@ router.post("collections.info", auth(), async (ctx) => { }); router.post("collections.import", auth(), async (ctx) => { - const { type } = ctx.body; + const { type, attachmentId } = ctx.body; ctx.assertIn(type, ["outline"], "type must be one of 'outline'"); - - if (!ctx.is("multipart/form-data")) { - throw new InvalidRequestError("Request type must be multipart/form-data"); - } - - const file: any = Object.values(ctx.request.files)[0]; - ctx.assertPresent(file, "file is required"); - - if (file.type !== "application/zip") { - throw new InvalidRequestError("File type must be a zip"); - } + ctx.assertUuid(attachmentId, "attachmentId is required"); const user = ctx.state.user; authorize(user, "import", Collection); + const attachment = await Attachment.findByPk(attachmentId); + authorize(user, "read", attachment); + + const buffer = await attachment.buffer; + const tmpDir = os.tmpdir(); + const tmpFilePath = `${tmpDir}/upload-${attachmentId}`; + + await fs.promises.writeFile(tmpFilePath, buffer); + const file = new File({ + name: attachment.name, + type: attachment.type, + path: tmpFilePath, + }); + const { documents, attachments, collections } = await collectionImporter({ file, user, diff --git a/server/api/collections.test.js b/server/api/collections.test.js index d58e201ad..ce4cd927c 100644 --- a/server/api/collections.test.js +++ b/server/api/collections.test.js @@ -111,7 +111,7 @@ describe("#collections.list", () => { }); describe("#collections.import", () => { - it("should error if no file is passed", async () => { + it("should error if no attachmentId is passed", async () => { const user = await buildUser(); const res = await server.post("/api/collections.import", { body: { diff --git a/server/commands/collectionImporter.js b/server/commands/collectionImporter.js index a5be2fecb..58e6e1dab 100644 --- a/server/commands/collectionImporter.js +++ b/server/commands/collectionImporter.js @@ -9,7 +9,7 @@ import { values, keys } from "lodash"; import uuid from "uuid"; import { parseOutlineExport } from "../../shared/utils/zip"; import { FileImportError } from "../errors"; -import { Attachment, Document, Collection, User } from "../models"; +import { Attachment, Event, Document, Collection, User } from "../models"; import attachmentCreator from "./attachmentCreator"; import documentCreator from "./documentCreator"; import documentImporter from "./documentImporter"; @@ -66,12 +66,21 @@ export default async function collectionImporter({ // there is also a "Name (Imported)" but this is a case not worth dealing // with right now if (!isCreated) { + const name = `${item.name} (Imported)`; collection = await Collection.create({ teamId: user.teamId, creatorId: user.id, - name: `${item.name} (Imported)`, + name, private: false, }); + await Event.create({ + name: "collections.create", + collectionId: collection.id, + teamId: collection.teamId, + actorId: user.id, + data: { name }, + ip, + }); } collections[item.path] = collection; diff --git a/server/models/Attachment.js b/server/models/Attachment.js index e5792ece3..fdb3d779e 100644 --- a/server/models/Attachment.js +++ b/server/models/Attachment.js @@ -1,7 +1,7 @@ // @flow import path from "path"; import { DataTypes, sequelize } from "../sequelize"; -import { deleteFromS3 } from "../utils/s3"; +import { deleteFromS3, getFileByKey } from "../utils/s3"; const Attachment = sequelize.define( "attachment", @@ -47,6 +47,9 @@ const Attachment = sequelize.define( isPrivate: function () { return this.acl === "private"; }, + buffer: function () { + return getFileByKey(this.key); + }, }, } ); diff --git a/server/policies/attachment.js b/server/policies/attachment.js index d7105402d..28fdb8fe5 100644 --- a/server/policies/attachment.js +++ b/server/policies/attachment.js @@ -6,7 +6,7 @@ const { allow } = policy; allow(User, "create", Attachment); -allow(User, "delete", Attachment, (actor, attachment) => { +allow(User, ["read", "delete"], Attachment, (actor, attachment) => { if (!attachment || attachment.teamId !== actor.teamId) return false; if (actor.isAdmin) return true; if (actor.id === attachment.userId) return true; diff --git a/server/utils/s3.js b/server/utils/s3.js index 7cb294a01..ece56262d 100644 --- a/server/utils/s3.js +++ b/server/utils/s3.js @@ -46,7 +46,8 @@ export const makeCredential = () => { export const makePolicy = ( credential: string, longDate: string, - acl: string + acl: string, + contentType: string = "image" ) => { const tomorrow = addHours(new Date(), 24); const policy = { @@ -55,7 +56,7 @@ export const makePolicy = ( ["starts-with", "$key", ""], { acl }, ["content-length-range", 0, +process.env.AWS_S3_UPLOAD_MAX_SIZE], - ["starts-with", "$Content-Type", "image"], + ["starts-with", "$Content-Type", contentType], ["starts-with", "$Cache-Control", ""], { "x-amz-algorithm": "AWS4-HMAC-SHA256" }, { "x-amz-credential": credential }, @@ -177,7 +178,7 @@ export const getSignedImageUrl = async (key: string) => { : s3.getSignedUrl("getObject", params); }; -export const getImageByKey = async (key: string) => { +export const getFileByKey = async (key: string) => { const params = { Bucket: AWS_S3_UPLOAD_BUCKET_NAME, Key: key, diff --git a/server/utils/zip.js b/server/utils/zip.js index aa1205fdd..4aaf13669 100644 --- a/server/utils/zip.js +++ b/server/utils/zip.js @@ -4,7 +4,7 @@ import * as Sentry from "@sentry/node"; import JSZip from "jszip"; import tmp from "tmp"; import { Attachment, Collection, Document } from "../models"; -import { getImageByKey } from "./s3"; +import { getFileByKey } from "./s3"; async function addToArchive(zip, documents) { for (const doc of documents) { @@ -38,7 +38,7 @@ async function addToArchive(zip, documents) { async function addImageToArchive(zip, key) { try { - const img = await getImageByKey(key); + const img = await getFileByKey(key); zip.file(key, img, { createFolders: true }); } catch (err) { if (process.env.SENTRY_DSN) { diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json index 0412b90f8..f11324f6b 100644 --- a/shared/i18n/locales/en_US/translation.json +++ b/shared/i18n/locales/en_US/translation.json @@ -312,12 +312,12 @@ "It is possible to import a zip file of folders and Markdown files previously exported from an Outline instance. Support will soon be added for importing from other services.": "It is possible to import a zip file of folders and Markdown files previously exported from an Outline instance. Support will soon be added for importing from other services.", "importSuccessful": "Import successful, {{ count }} document was imported into your knowledge base.", "importSuccessful_plural": "Import successful, {{ count }} documents were imported into your knowledge base.", - "Sorry, the file <1>{{fileName}} is missing valid collections or documents.": "Sorry, the file <1>{{fileName}} is missing valid collections or documents.", - "<0>{{fileName}} looks good, the following collections and their documents will be imported:": "<0>{{fileName}} looks good, the following collections and their documents will be imported:", + "Sorry, the file {{ fileName }} is missing valid collections or documents.": "Sorry, the file {{ fileName }} is missing valid collections or documents.", + "{{ fileName }} looks good, the following collections and their documents will be imported:": "{{ fileName }} looks good, the following collections and their documents will be imported:", "Importing": "Importing", "Confirm & Import": "Confirm & Import", "Choose File…": "Choose File…", - "A full export might take some time, consider exporting a single document or collection if possible. We’ll put together a zip of all your documents in Markdown format and email it to <2>{{userEmail}}.": "A full export might take some time, consider exporting a single document or collection if possible. We’ll put together a zip of all your documents in Markdown format and email it to <2>{{userEmail}}.", + "A full export might take some time, consider exporting a single document or collection if possible. We’ll put together a zip of all your documents in Markdown format and email it to {{ userEmail }}.": "A full export might take some time, consider exporting a single document or collection if possible. We’ll put together a zip of all your documents in Markdown format and email it to {{ userEmail }}.", "Export Requested": "Export Requested", "Requesting Export": "Requesting Export", "Export Data": "Export Data",