Skip to content

Commit f4b6fe2

Browse files
abhinavkrinMartinSchoeler
authored andcommitted
fix: export jobs getting stuck when userNameTable is null (#37387)
1 parent 3917587 commit f4b6fe2

File tree

6 files changed

+270
-17
lines changed

6 files changed

+270
-17
lines changed

.changeset/funny-rocks-admire.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@rocket.chat/meteor': patch
3+
---
4+
5+
Fixes an issue where user data exports requested would remain stuck and never complete.

apps/meteor/.mocharc.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ module.exports = {
1313
'lib/callbacks.spec.ts',
1414
'server/lib/ldap/*.spec.ts',
1515
'server/lib/ldap/**/*.spec.ts',
16+
'server/lib/dataExport/**/*.spec.ts',
1617
'server/ufs/*.spec.ts',
1718
'ee/server/lib/ldap/*.spec.ts',
1819
'ee/tests/**/*.tests.ts',

apps/meteor/server/lib/dataExport/exportRoomMessagesToFile.ts

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,23 @@
11
import { mkdir, writeFile } from 'fs/promises';
22

3-
import type { IMessage, IRoom, IUser, MessageAttachment, FileProp, RoomType } from '@rocket.chat/core-typings';
3+
import type { IMessage, IRoom, IUser, MessageAttachment, FileProp, RoomType, IExportOperation } from '@rocket.chat/core-typings';
44
import { Messages } from '@rocket.chat/models';
55

66
import { settings } from '../../../app/settings/server';
77
import { readSecondaryPreferred } from '../../database/readSecondaryPreferred';
88
import { joinPath } from '../fileUtils';
99
import { i18n } from '../i18n';
1010

11-
const hideUserName = (
12-
username: string,
13-
userData: Pick<IUser, 'username'> | undefined,
14-
usersMap: { userNameTable: Record<string, string> },
15-
) => {
16-
if (!usersMap.userNameTable) {
17-
usersMap.userNameTable = {};
18-
}
19-
20-
if (!usersMap.userNameTable[username]) {
11+
const hideUserName = (username: string, userData: Pick<IUser, 'username'> | undefined, usersMap: Record<string, string>) => {
12+
if (!usersMap[username]) {
2113
if (userData && username === userData.username) {
22-
usersMap.userNameTable[username] = username;
14+
usersMap[username] = username;
2315
} else {
24-
usersMap.userNameTable[username] = `User_${Object.keys(usersMap.userNameTable).length + 1}`;
16+
usersMap[username] = `User_${Object.keys(usersMap).length + 1}`;
2517
}
2618
}
2719

28-
return usersMap.userNameTable[username];
20+
return usersMap[username];
2921
};
3022

3123
const getAttachmentData = (attachment: MessageAttachment, message: IMessage) => {
@@ -66,7 +58,7 @@ export const getMessageData = (
6658
msg: IMessage,
6759
hideUsers: boolean,
6860
userData: Pick<IUser, 'username'> | undefined,
69-
usersMap: { userNameTable: Record<string, string> },
61+
usersMap: IExportOperation['userNameTable'],
7062
): MessageData => {
7163
const username = hideUsers ? hideUserName(msg.u.username || msg.u.name || '', userData, usersMap) : msg.u.username;
7264

@@ -202,7 +194,7 @@ export const exportRoomMessages = async (
202194
limit: number,
203195
userData: any,
204196
filter: any = {},
205-
usersMap: any = {},
197+
usersMap: IExportOperation['userNameTable'] = {},
206198
hideUsers = true,
207199
) => {
208200
const readPreference = readSecondaryPreferred();
@@ -257,7 +249,7 @@ export const exportRoomMessagesToFile = async function (
257249
)[],
258250
userData: IUser,
259251
messagesFilter = {},
260-
usersMap = {},
252+
usersMap: IExportOperation['userNameTable'] = {},
261253
hideUsers = true,
262254
) {
263255
await mkdir(exportPath, { recursive: true });
Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,251 @@
1+
import fs from 'fs';
2+
3+
import type { IExportOperation } from '@rocket.chat/core-typings';
4+
import { expect } from 'chai';
5+
import proxyquire from 'proxyquire';
6+
import Sinon from 'sinon';
7+
8+
let exportOperation: IExportOperation | null = null;
9+
10+
const modelsMock = {
11+
ExportOperations: {
12+
findLastOperationByUser: async (userId: string, fullExport = false) => {
13+
if (exportOperation?.userId === userId && exportOperation?.fullExport === fullExport) {
14+
return exportOperation;
15+
}
16+
},
17+
countAllPendingBeforeMyRequest: async (requestDay: Date) => {
18+
if (
19+
exportOperation &&
20+
exportOperation.createdAt < requestDay &&
21+
exportOperation.status !== 'completed' &&
22+
exportOperation.status !== 'skipped'
23+
) {
24+
return 1;
25+
}
26+
return 0;
27+
},
28+
create: async (data: any) => {
29+
exportOperation = {
30+
userNameTable: null, // need to keep this null for testing purposes
31+
...data,
32+
_id: 'exportOp1',
33+
createdAt: new Date(),
34+
};
35+
return exportOperation?._id as IExportOperation['_id'];
36+
},
37+
updateOperation: async (data: IExportOperation) => {
38+
if (exportOperation && exportOperation._id === data._id) {
39+
exportOperation = { ...exportOperation, ...data };
40+
}
41+
return { modifiedCount: 1 };
42+
},
43+
44+
findOnePending: async () => {
45+
if (exportOperation && exportOperation.status !== 'completed' && exportOperation.status !== 'skipped') {
46+
return exportOperation;
47+
}
48+
return null;
49+
},
50+
},
51+
UserDataFiles: {
52+
findOneById: async (fileId: string) => {
53+
if (exportOperation?.fileId === fileId) {
54+
return {
55+
_id: fileId,
56+
};
57+
}
58+
},
59+
findLastFileByUser: async (userId: string) => {
60+
if (exportOperation?.userId === userId && exportOperation.fileId) {
61+
return {
62+
_id: exportOperation.fileId,
63+
};
64+
}
65+
},
66+
},
67+
Avatars: {
68+
findOneByName: async (_name: string) => {
69+
return null;
70+
},
71+
},
72+
Subscriptions: {
73+
findByUserId: (_userId: string) => {
74+
return [
75+
{
76+
rid: 'general',
77+
},
78+
];
79+
},
80+
},
81+
Messages: {
82+
findPaginated: (_query: object, _options: object) => {
83+
return {
84+
cursor: {
85+
toArray: async () => [
86+
{
87+
_id: 'msg1',
88+
rid: 'general',
89+
ts: new Date(),
90+
msg: 'Hello World',
91+
u: { _id: 'user1', username: 'userone' },
92+
},
93+
{
94+
_id: 'msg2',
95+
rid: 'general',
96+
ts: new Date(),
97+
msg: 'Second message',
98+
u: { _id: 'user2', username: 'usertwo' },
99+
},
100+
],
101+
},
102+
totalCount: Promise.resolve(0),
103+
};
104+
},
105+
},
106+
};
107+
108+
const { exportRoomMessagesToFile } = proxyquire.noCallThru().load('./exportRoomMessagesToFile.ts', {
109+
'@rocket.chat/models': modelsMock,
110+
'../../../app/settings/server': {
111+
settings: {
112+
get: (_key: string) => {
113+
return undefined;
114+
},
115+
},
116+
},
117+
'../i18n': {
118+
i18n: {
119+
t: (key: string) => key,
120+
},
121+
},
122+
});
123+
124+
const { requestDataDownload } = proxyquire.noCallThru().load('../../methods/requestDataDownload.ts', {
125+
'@rocket.chat/models': modelsMock,
126+
'../../app/settings/server': {
127+
settings: {
128+
get: (_key: string) => {
129+
return undefined;
130+
},
131+
},
132+
},
133+
'../lib/dataExport': {
134+
getPath: (fileId: string) => `/data-download/${fileId}`,
135+
},
136+
'meteor/meteor': {
137+
Meteor: {
138+
methods: Sinon.stub(),
139+
},
140+
},
141+
}) as {
142+
requestDataDownload: (args: { userData: { _id: string }; fullExport?: boolean }) => Promise<{
143+
requested: boolean;
144+
exportOperation: IExportOperation;
145+
url: string | null;
146+
pendingOperationsBeforeMyRequest: number;
147+
}>;
148+
};
149+
150+
const { processDataDownloads } = proxyquire.noCallThru().load('./processDataDownloads.ts', {
151+
'@rocket.chat/models': modelsMock,
152+
'../../../app/file-upload/server': {
153+
FileUpload: {
154+
copy: async (fileId: string, _options: any) => {
155+
return `copied-${fileId}`;
156+
},
157+
},
158+
},
159+
'../../../app/settings/server': {
160+
settings: {
161+
get: (_key: string) => {
162+
return undefined;
163+
},
164+
},
165+
},
166+
'../../../app/utils/server/getURL': {
167+
getURL: (path: string) => `https://example.com${path}`,
168+
},
169+
'../i18n': {
170+
i18n: {
171+
t: (key: string) => key,
172+
},
173+
},
174+
'./copyFileUpload': {
175+
copyFileUpload: (_attachmentData: { _id: string; name: string }, _assetsPath: string) => {
176+
return Promise.resolve();
177+
},
178+
},
179+
'./exportRoomMessagesToFile': {
180+
exportRoomMessagesToFile,
181+
},
182+
'./getRoomData': {
183+
getRoomData: Sinon.stub().resolves({
184+
roomId: 'GENERAL',
185+
roomName: 'general',
186+
type: 'c',
187+
exportedCount: 0,
188+
status: 'pending',
189+
userId: 'user1',
190+
targetFile: 'general.json',
191+
}),
192+
},
193+
'./sendEmail': {
194+
sendEmail: Sinon.stub().resolves(),
195+
},
196+
'./uploadZipFile': {
197+
uploadZipFile: Sinon.stub().resolves({ _id: 'file1' }),
198+
},
199+
}) as {
200+
processDataDownloads: () => Promise<void>;
201+
};
202+
203+
const userData = { _id: 'user1', username: 'userone' };
204+
205+
describe('requestDataDownload', () => {
206+
beforeEach(() => {
207+
exportOperation = null;
208+
});
209+
210+
it('should create a new export operation if none exists', async () => {
211+
const result = await requestDataDownload({ userData, fullExport: false });
212+
213+
expect(result.requested).to.be.true;
214+
expect(result.exportOperation).to.exist;
215+
expect(result.exportOperation.userId).to.equal('user1');
216+
expect(result.exportOperation.fullExport).to.be.false;
217+
expect(result.url).to.be.null;
218+
expect(result.pendingOperationsBeforeMyRequest).to.equal(0);
219+
expect(result.exportOperation.status).to.equal('pending');
220+
});
221+
});
222+
223+
describe('export user data', async () => {
224+
beforeEach(() => {
225+
exportOperation = null;
226+
});
227+
it('should process data download for pending export operations', async () => {
228+
await requestDataDownload({ userData, fullExport: true });
229+
230+
expect(exportOperation).to.not.be.null;
231+
expect(exportOperation?.userId).to.equal('user1');
232+
expect(exportOperation?.fullExport).to.be.true;
233+
expect(exportOperation?.status).to.equal('pending');
234+
235+
await processDataDownloads();
236+
237+
expect(exportOperation?.status).to.equal('completed');
238+
expect(exportOperation?.fileId).to.equal('file1');
239+
expect(exportOperation?.generatedUserFile).to.be.true;
240+
expect(exportOperation?.roomList).to.have.lengthOf(1);
241+
expect(exportOperation?.roomList?.[0].roomId).to.equal('GENERAL');
242+
expect(exportOperation?.roomList?.[0].exportedCount).to.equal(2);
243+
expect(exportOperation?.exportPath).to.be.string;
244+
245+
expect(fs.readFileSync(`${exportOperation?.exportPath}/${exportOperation?.roomList?.[0].targetFile}`, 'utf-8')).to.contain(
246+
'Hello World',
247+
);
248+
expect(exportOperation?.generatedFile).to.be.string;
249+
expect(fs.existsSync(exportOperation?.generatedFile as string)).to.be.true;
250+
});
251+
});

apps/meteor/server/lib/dataExport/processDataDownloads.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,9 @@ const continueExportOperation = async function (exportOperation: IExportOperatio
178178

179179
// Run every room on every request, to avoid missing new messages on the rooms that finished first.
180180
if (exportOperation.status === 'exporting') {
181+
if (!exportOperation.userNameTable) {
182+
exportOperation.userNameTable = {};
183+
}
181184
const { fileList } = await exportRoomMessagesToFile(
182185
exportOperation.exportPath,
183186
exportOperation.assetsPath,

apps/meteor/server/methods/requestDataDownload.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export const requestDataDownload = async ({
8484
generatedFile: undefined,
8585
fullExport,
8686
userData: currentUserData,
87+
userNameTable: {},
8788
} as unknown as IExportOperation; // @todo yikes!
8889

8990
const id = await ExportOperations.create(exportOperation);

0 commit comments

Comments
 (0)