Skip to content

Commit b77bda0

Browse files
committed
refactor: simplify code added since v2.61.5
Extract duplicated logic into shared helpers: - resolveLabels() in gmail-client for label ID resolution - throwAsBoom() in routes-ui for export error handling - Shared jobOpts/removePolicy in webhooks pushToQueue - Shared retention variable in base-client queueMessage - Test helpers (createCallSimulator, atomicCheckAndAdd, etc.) Replace verbose patterns with idiomatic equivalents: - Boom.badRequest() instead of Boom.boomify(new Error(), {statusCode: 400}) - if/else chain instead of nested ternary in outlook-client $select - Consolidate redundant null checks in imap-client Remove redundant comments that restate what the code does, hoist top-level imports, and consolidate repetitive test cases.
1 parent 9f48f64 commit b77bda0

19 files changed

+327
-960
lines changed

lib/api-routes/export-routes.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const Joi = require('joi');
77
const { failAction } = require('../tools');
88
const { accountIdSchema, exportRequestSchema, exportStatusSchema, exportListSchema, exportIdSchema } = require('../schemas');
99
const getSecret = require('../get-secret');
10+
const { createDecryptStream } = require('../stream-encrypt');
1011

1112
function handleError(request, err) {
1213
request.logger.error({ msg: 'API request failed', err });
@@ -141,18 +142,15 @@ async function init(args) {
141142

142143
let stream = fs.createReadStream(fileInfo.filePath);
143144

144-
// Handle stream errors (e.g., file deleted mid-download)
145145
stream.on('error', err => {
146146
request.logger.error({ msg: 'Export download stream error', exportId, err });
147147
});
148148

149-
// Decrypt file if encrypted
150149
if (fileInfo.isEncrypted) {
151150
const secret = await getSecret();
152151
if (!secret) {
153152
throw Boom.serverUnavailable('Encryption secret not available for decryption');
154153
}
155-
const { createDecryptStream } = require('../stream-encrypt');
156154
const decryptStream = createDecryptStream(secret);
157155
decryptStream.on('error', err => {
158156
request.logger.error({ msg: 'Export decryption error', exportId, err });

lib/email-client/base-client.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2007,10 +2007,11 @@ class BaseClient {
20072007

20082008
// Configure queue job options
20092009
let queueName = 'queued';
2010+
let retention = typeof queueKeep === 'number' ? { age: 24 * 3600, count: queueKeep } : queueKeep;
20102011
let jobOpts = {
20112012
jobId: queueId,
2012-
removeOnComplete: typeof queueKeep === 'number' ? { age: 24 * 3600, count: queueKeep } : queueKeep,
2013-
removeOnFail: typeof queueKeep === 'number' ? { age: 24 * 3600, count: queueKeep } : queueKeep,
2013+
removeOnComplete: retention,
2014+
removeOnFail: retention,
20142015
attempts: typeof deliveryAttempts === 'number' ? deliveryAttempts : defaultDeliveryAttempts,
20152016
backoff: {
20162017
type: 'exponential',

lib/email-client/gmail-client.js

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,21 +1292,7 @@ class GmailClient extends BaseClient {
12921292
let formattedMessage = this.formatMessage(messageData, { extended: true, textType: options.textType });
12931293

12941294
// Resolve label IDs to human-readable names
1295-
if (formattedMessage.labels && Array.isArray(formattedMessage.labels)) {
1296-
const labelsResult = await this.getLabels();
1297-
const labelMap = new Map();
1298-
for (const label of labelsResult || []) {
1299-
labelMap.set(label.id, label.name);
1300-
}
1301-
formattedMessage.labels = formattedMessage.labels.map(label => {
1302-
// Skip special-use labels (already in correct format)
1303-
if (label.startsWith('\\')) {
1304-
return label;
1305-
}
1306-
// Resolve label ID to name
1307-
return labelMap.get(label) || label;
1308-
});
1309-
}
1295+
await this.resolveLabels(formattedMessage);
13101296

13111297
// Mark as seen if requested
13121298
if (options.markAsSeen && (!formattedMessage.flags || !formattedMessage.flags.includes('\\Seen'))) {
@@ -1395,17 +1381,7 @@ class GmailClient extends BaseClient {
13951381
const messageData = await this.request(`${GMAIL_API_BASE}/gmail/v1/users/me/messages/${emailId}`, 'get', requestQuery);
13961382
const formattedMessage = this.formatMessage(messageData, { extended: true, textType: options.textType });
13971383

1398-
// Resolve label IDs to human-readable names
1399-
if (formattedMessage.labels && Array.isArray(formattedMessage.labels)) {
1400-
formattedMessage.labels = formattedMessage.labels.map(label => {
1401-
// Skip special-use labels (already in correct format)
1402-
if (label.startsWith('\\')) {
1403-
return label;
1404-
}
1405-
// Resolve label ID to name
1406-
return labelMap.get(label) || label;
1407-
});
1408-
}
1384+
await this.resolveLabels(formattedMessage, labelMap);
14091385

14101386
return {
14111387
messageId: emailId,
@@ -2171,6 +2147,35 @@ class GmailClient extends BaseClient {
21712147
}
21722148
}
21732149

2150+
/**
2151+
* Resolves label IDs to human-readable names on a formatted message.
2152+
* Mutates formattedMessage.labels in place. Labels starting with '\\' are
2153+
* treated as special-use labels and left as-is.
2154+
*
2155+
* @param {Object} formattedMessage - Message with a .labels array
2156+
* @param {Map} [labelMap] - Optional pre-built id-to-name map (avoids an extra getLabels call)
2157+
*/
2158+
async resolveLabels(formattedMessage, labelMap) {
2159+
if (!Array.isArray(formattedMessage?.labels)) {
2160+
return;
2161+
}
2162+
2163+
if (!labelMap) {
2164+
const labelsResult = await this.getLabels();
2165+
labelMap = new Map();
2166+
for (const label of labelsResult || []) {
2167+
labelMap.set(label.id, label.name);
2168+
}
2169+
}
2170+
2171+
formattedMessage.labels = formattedMessage.labels.map(label => {
2172+
if (label.startsWith('\\')) {
2173+
return label;
2174+
}
2175+
return labelMap.get(label) || label;
2176+
});
2177+
}
2178+
21742179
/**
21752180
* Resolves a label by path or ID
21762181
* @param {string} path - Label path or ID

lib/email-client/imap-client.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -996,16 +996,10 @@ class IMAPClient extends BaseClient {
996996
this.state = 'connected';
997997
await this.setStateVal();
998998

999-
// Check if client disconnected during state update
1000-
if (!this.imapClient) {
1001-
this.logger.debug({ msg: 'IMAP client disconnected during state update' });
1002-
return;
1003-
}
1004-
1005999
// Capture local reference to avoid race conditions during async operations
10061000
const imapClient = this.imapClient;
10071001
if (!imapClient || !imapClient.usable) {
1008-
this.logger.debug({ msg: 'IMAP client not usable after state update' });
1002+
this.logger.debug({ msg: 'IMAP client disconnected or not usable after state update' });
10091003
return;
10101004
}
10111005

lib/email-client/outlook-client.js

Lines changed: 51 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -409,10 +409,8 @@ class OutlookClient extends BaseClient {
409409
*/
410410
async listMessages(query, options) {
411411
options = options || {};
412-
// Support metadataOnly passed via query (from export worker) or options
413-
if (query.metadataOnly && !options.metadataOnly) {
414-
options.metadataOnly = true;
415-
}
412+
// Support metadataOnly passed via query (from export worker)
413+
options.metadataOnly = options.metadataOnly || query.metadataOnly;
416414

417415
await this.prepare();
418416

@@ -465,56 +463,62 @@ class OutlookClient extends BaseClient {
465463
}
466464
}
467465

466+
// Build select fields based on detail level
467+
let selectFields;
468+
let expandFields;
469+
470+
if (options.minimalFields) {
471+
selectFields = ['id', 'conversationId', 'receivedDateTime'];
472+
} else if (options.metadataOnly) {
473+
selectFields = [
474+
'id',
475+
'conversationId',
476+
'receivedDateTime',
477+
'isRead',
478+
'isDraft',
479+
'flag',
480+
'body',
481+
'subject',
482+
'from',
483+
'replyTo',
484+
'sender',
485+
'internetMessageId'
486+
];
487+
} else {
488+
selectFields = [
489+
'id',
490+
'conversationId',
491+
'receivedDateTime',
492+
'isRead',
493+
'isDraft',
494+
'flag',
495+
'body',
496+
'subject',
497+
'from',
498+
'replyTo',
499+
'sender',
500+
'toRecipients',
501+
'ccRecipients',
502+
'bccRecipients',
503+
'internetMessageId',
504+
'bodyPreview'
505+
];
506+
expandFields = 'attachments($select=id,name,contentType,size,isInline,microsoft.graph.fileAttachment/contentId)';
507+
}
508+
509+
if (!folder) {
510+
selectFields.push('parentFolderId');
511+
}
512+
468513
// Build Graph API query parameters
469-
// minimalFields mode: only fetch essential fields for export indexing (id, conversationId, receivedDateTime)
470514
let requestQuery = {
471515
$count: true,
472516
$top: pageSize,
473517
$skip: page * pageSize,
474518
$skiptoken,
475519
$orderBy: 'receivedDateTime desc',
476-
$select: (options.minimalFields
477-
? ['id', 'conversationId', 'receivedDateTime']
478-
: options.metadataOnly
479-
? [
480-
'id',
481-
'conversationId',
482-
'receivedDateTime',
483-
'isRead',
484-
'isDraft',
485-
'flag',
486-
'body',
487-
'subject',
488-
'from',
489-
'replyTo',
490-
'sender',
491-
'internetMessageId'
492-
]
493-
: [
494-
'id',
495-
'conversationId',
496-
'receivedDateTime',
497-
'isRead',
498-
'isDraft',
499-
'flag',
500-
'body',
501-
'subject',
502-
'from',
503-
'replyTo',
504-
'sender',
505-
'toRecipients',
506-
'ccRecipients',
507-
'bccRecipients',
508-
'internetMessageId',
509-
'bodyPreview'
510-
]
511-
)
512-
.concat(!folder ? 'parentFolderId' : [])
513-
.join(','),
514-
$expand:
515-
options.minimalFields || options.metadataOnly
516-
? undefined
517-
: 'attachments($select=id,name,contentType,size,isInline,microsoft.graph.fileAttachment/contentId)'
520+
$select: selectFields.join(','),
521+
$expand: expandFields
518522
};
519523

520524
let useOutlookSearch = false;

0 commit comments

Comments
 (0)