perf: Stop copying attachments when moving documents (#3251)
* perf: Stop copying attachments when moving documents * lint
This commit is contained in:
@@ -1,12 +1,9 @@
|
||||
import Attachment from "@server/models/Attachment";
|
||||
import {
|
||||
buildDocument,
|
||||
buildAttachment,
|
||||
buildCollection,
|
||||
buildUser,
|
||||
} from "@server/test/factories";
|
||||
import { flushdb, seed } from "@server/test/support";
|
||||
import parseAttachmentIds from "@server/utils/parseAttachmentIds";
|
||||
import documentMover from "./documentMover";
|
||||
|
||||
beforeEach(() => flushdb());
|
||||
@@ -110,45 +107,4 @@ describe("documentMover", () => {
|
||||
expect(response.documents[0].collection.id).toEqual(newCollection.id);
|
||||
expect(response.documents[1].collection.id).toEqual(newCollection.id);
|
||||
});
|
||||
|
||||
it("should move attachments in children to another collection", async () => {
|
||||
const { document, user, collection } = await seed();
|
||||
const newCollection = await buildCollection({
|
||||
teamId: collection.teamId,
|
||||
});
|
||||
const attachment = await buildAttachment({
|
||||
teamId: user.teamId,
|
||||
userId: user.id,
|
||||
});
|
||||
const newDocument = await buildDocument({
|
||||
parentDocumentId: document.id,
|
||||
collectionId: collection.id,
|
||||
teamId: collection.teamId,
|
||||
userId: collection.createdById,
|
||||
title: "Child document",
|
||||
text: `content `,
|
||||
});
|
||||
|
||||
await collection.addDocumentToStructure(newDocument);
|
||||
await documentMover({
|
||||
user,
|
||||
document,
|
||||
collectionId: newCollection.id,
|
||||
parentDocumentId: undefined,
|
||||
index: 0,
|
||||
ip,
|
||||
});
|
||||
|
||||
// check document ids where updated
|
||||
await newDocument.reload();
|
||||
expect(newDocument.collectionId).toBe(newCollection.id);
|
||||
|
||||
// check new attachment was created pointint to same key
|
||||
const attachmentIds = parseAttachmentIds(newDocument.text);
|
||||
const newAttachment = await Attachment.findByPk(attachmentIds[0]);
|
||||
expect(newAttachment?.documentId).toBe(newDocument.id);
|
||||
expect(newAttachment?.key).toBe(attachment.key);
|
||||
await document.reload();
|
||||
expect(document.collectionId).toBe(newCollection.id);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,52 +2,9 @@ import invariant from "invariant";
|
||||
import { Transaction } from "sequelize";
|
||||
import { sequelize } from "@server/database/sequelize";
|
||||
import { APM } from "@server/logging/tracing";
|
||||
import {
|
||||
User,
|
||||
Document,
|
||||
Attachment,
|
||||
Collection,
|
||||
Pin,
|
||||
Event,
|
||||
} from "@server/models";
|
||||
import parseAttachmentIds from "@server/utils/parseAttachmentIds";
|
||||
import { User, Document, Collection, Pin, Event } from "@server/models";
|
||||
import pinDestroyer from "./pinDestroyer";
|
||||
|
||||
async function copyAttachments(
|
||||
document: Document,
|
||||
options?: { transaction?: Transaction }
|
||||
) {
|
||||
let text = document.text;
|
||||
const documentId = document.id;
|
||||
// find any image attachments that are in this documents text
|
||||
const attachmentIds = parseAttachmentIds(text);
|
||||
|
||||
for (const id of attachmentIds) {
|
||||
const existing = await Attachment.findOne({
|
||||
where: {
|
||||
teamId: document.teamId,
|
||||
id,
|
||||
},
|
||||
});
|
||||
|
||||
// if the image attachment was originally uploaded to another document
|
||||
// (this can happen in various ways, copy/paste, or duplicate for example)
|
||||
// then create a new attachment pointed to this doc and update the reference
|
||||
// in the text so that it gets the moved documents permissions
|
||||
if (existing && existing.documentId !== documentId) {
|
||||
// @ts-expect-error dataValues exists
|
||||
const { id, ...rest } = existing.dataValues;
|
||||
const attachment = await Attachment.create(
|
||||
{ ...rest, documentId },
|
||||
options
|
||||
);
|
||||
text = text.replace(existing.redirectUrl, attachment.redirectUrl);
|
||||
}
|
||||
}
|
||||
|
||||
return text;
|
||||
}
|
||||
|
||||
type Props = {
|
||||
user: User;
|
||||
document: Document;
|
||||
@@ -127,9 +84,6 @@ async function documentMover({
|
||||
await collection?.save({
|
||||
transaction,
|
||||
});
|
||||
document.text = await copyAttachments(document, {
|
||||
transaction,
|
||||
});
|
||||
}
|
||||
|
||||
// add to new collection (may be the same)
|
||||
@@ -171,9 +125,6 @@ async function documentMover({
|
||||
await Promise.all(
|
||||
childDocuments.map(async (child) => {
|
||||
await loopChildren(child.id);
|
||||
child.text = await copyAttachments(child, {
|
||||
transaction,
|
||||
});
|
||||
child.collectionId = collectionId;
|
||||
await child.save();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user