Skip to content

Commit

Permalink
fix(account-listing): Show accounts as initializing if account has no…
Browse files Browse the repository at this point in the history
…t yet been processed after startup
  • Loading branch information
andris9 committed Mar 19, 2024
1 parent 311915e commit 0e70898
Show file tree
Hide file tree
Showing 12 changed files with 92 additions and 602 deletions.
19 changes: 16 additions & 3 deletions lib/account.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { normalizePath, formatAccountListingResponse, unpackUIDRangeForSearch, me
const crypto = require('crypto');
const { MessageChannel } = require('worker_threads');
const { MessagePortReadable } = require('./message-port-stream');
const { deepStrictEqual, strictEqual } = require('assert');
const { deepStrictEqual, strictEqual, ok: assertOk } = require('assert');

Check failure on line 10 in lib/account.js

View workflow job for this annotation

GitHub Actions / test (16.x, ubuntu-20.04)

'assertOk' is assigned a value but never used

Check failure on line 10 in lib/account.js

View workflow job for this annotation

GitHub Actions / test (18.x, ubuntu-20.04)

'assertOk' is assigned a value but never used

Check failure on line 10 in lib/account.js

View workflow job for this annotation

GitHub Actions / test (20.x, ubuntu-20.04)

'assertOk' is assigned a value but never used
const { encrypt, decrypt } = require('./encrypt');
const { oauth2Apps, LEGACY_KEYS } = require('./oauth2-apps');
const settings = require('./settings');
Expand Down Expand Up @@ -51,6 +51,10 @@ class Account {
page = Math.max(Number(page) || 0, 0);
let skip = page * limit;

const runIndex = await this.call({
cmd: 'runIndex'
});

let result = await this.redis.sListAccounts(`${REDIS_PREFIX}ia:accounts`, state || '*', skip, limit, `${REDIS_PREFIX}`, query);

let accounts = result[2].map(formatAccountListingResponse).map(this.unserializeAccountData.bind(this));
Expand Down Expand Up @@ -92,7 +96,7 @@ class Account {
accountData.oauth2 && accountData.oauth2.provider && accountData.oauth2.provider !== accountData.type
? accountData.oauth2.provider
: undefined,
state: accountData.state,
state: runIndex <= accountData.runIndex ? accountData.state : 'init',

webhooks: accountData.webhooks || undefined,
proxy: accountData.proxy || undefined,
Expand Down Expand Up @@ -260,6 +264,7 @@ class Account {
// number values
case 'connections':
case 'listRegistry':
case 'runIndex':
result[key] = Number(accountData[key]) || 0;
break;

Expand Down Expand Up @@ -425,7 +430,7 @@ class Account {
return result;
}

async loadAccountData(account, requireValid) {
async loadAccountData(account, requireValid, runIndex) {
if (!this.account || (account && account !== this.account)) {
let message = 'Invalid account ID';
let error = Boom.boomify(new Error(message), { statusCode: 400 });
Expand All @@ -441,6 +446,9 @@ class Account {
}

let accountData = this.unserializeAccountData(result);

accountData.state = runIndex <= accountData.runIndex ? accountData.state : 'init';

if (requireValid && !['connected', 'connecting', 'syncing'].includes(accountData.state)) {
let err;
switch (accountData.state) {
Expand Down Expand Up @@ -635,11 +643,16 @@ class Account {
}
}

const runIndex = await this.call({
cmd: 'runIndex'
});

let pipeline = this.redis
.multi()
.hgetall(this.getAccountKey())
.hmset(this.getAccountKey(), this.serializeAccountData(accountData))
.hsetnx(this.getAccountKey(), 'state', 'init')
.hsetnx(this.getAccountKey(), 'runIndex', runIndex.toString())
.hsetnx(this.getAccountKey(), `state:count:connected`, '0')
.sadd(`${REDIS_PREFIX}ia:accounts`, this.account);

Expand Down
3 changes: 3 additions & 0 deletions lib/api-client/base-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ const { REDIS_PREFIX } = require('../consts');
class BaseClient {
constructor(account, options) {
this.account = account;

this.options = options || {};

this.cid = this.getRandomId();

this.runIndex = this.options.runIndex;

this.accountObject = this.options.accountObject;
this.accountLogger = this.options.accountLogger;
this.redis = this.options.redis;
Expand Down
3 changes: 2 additions & 1 deletion lib/imap-connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,7 @@ class IMAPConnection extends BaseClient {
let [[e1], [e2, prevVal], [e3, incrVal], [e4, stateVal]] = await this.redis
.multi()
.hSetExists(this.getAccountKey(), 'state', this.state)
.hSetBigger(this.getAccountKey(), 'runIndex', this.runIndex.toString())
.hget(this.getAccountKey(), `state:count:${this.state}`)
.hIncrbyExists(this.getAccountKey(), `state:count:${this.state}`, 1)
.hget(this.getAccountKey(), 'state')
Expand Down Expand Up @@ -1819,7 +1820,7 @@ class IMAPConnection extends BaseClient {
deleted: false // set to true if mailbox is actually deleted
};
try {
let lock = await connectionClient.getMailboxLock(path);
let lock = await connectionClient.getMailboxLock(path, { description: `Delete mailbox ${path}` });

try {
await connectionClient.mailboxClose();
Expand Down
8 changes: 5 additions & 3 deletions lib/lua/h-set-bigger.lua
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ if redis.call("HEXISTS", hashKey, entryKey) == 1 then
local existing = redis.call("HGET", hashKey, entryKey);
if tonumber(value) > tonumber(existing) then
redis.call("HSET", hashKey, entryKey, value);
return 2;
return 3;
else
return 1;
end;
else
elseif redis.call("EXISTS", hashKey) == 1 then
redis.call("HSET", hashKey, entryKey, value);
return 1;
return 2;
end;

return 0;
30 changes: 15 additions & 15 deletions lib/mailbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ class Mailbox {
}

async select(skipIdle) {
let lock = await this.getMailboxLock();
let lock = await this.getMailboxLock(null, { description: `Select mailbox ${this.path}` });
// have to release the lock immediatelly, otherwise difficult to process 'exists' / 'expunge' events
lock.release();

Expand All @@ -363,14 +363,14 @@ class Mailbox {
}
}

async getMailboxLock(connectionClient) {
async getMailboxLock(connectionClient, options) {
connectionClient = connectionClient || this.connection.imapClient;

if (!connectionClient) {
throw new Error('IMAP connection not available');
}

let lock = await connectionClient.getMailboxLock(this.path, {});
let lock = await connectionClient.getMailboxLock(this.path, options || {});

if (connectionClient === this.connection.imapClient) {
clearTimeout(this.connection.completedTimer);
Expand Down Expand Up @@ -470,7 +470,7 @@ class Mailbox {
storedStatus = storedStatus || (await this.getStoredStatus());
let mailboxStatus = this.getMailboxStatus();

let lock = await this.getMailboxLock();
let lock = await this.getMailboxLock(null, { description: 'Partial sync' });
this.connection.syncing = true;
this.syncing = true;
try {
Expand Down Expand Up @@ -1663,7 +1663,7 @@ class Mailbox {
let fields = { uid: true, flags: true, modseq: true, emailId: true, labels: true, internalDate: true };
let opts = {};

let lock = await this.getMailboxLock();
let lock = await this.getMailboxLock(null, { description: 'Full sync' });
this.connection.syncing = true;
this.syncing = true;
try {
Expand Down Expand Up @@ -2075,7 +2075,7 @@ class Mailbox {

let lock;
if (!options.skipLock) {
lock = await this.getMailboxLock(connectionClient);
lock = await this.getMailboxLock(connectionClient, { description: `Get text ${message.uid}` });
}

try {
Expand Down Expand Up @@ -2133,7 +2133,7 @@ class Mailbox {
options = options || {};
const connectionClient = await this.connection.getImapConnection(allowSecondary);

let lock = await this.getMailboxLock(connectionClient);
let lock = await this.getMailboxLock(connectionClient, { description: `Get attachment ${message.uid}:${part}` });

let streaming = false;
let released = false;
Expand Down Expand Up @@ -2202,7 +2202,7 @@ class Mailbox {
try {
let lock;
if (!options.skipLock) {
lock = await this.getMailboxLock(connectionClient);
lock = await this.getMailboxLock(connectionClient, { description: `Get message ${message.uid}` });
}

try {
Expand Down Expand Up @@ -2365,7 +2365,7 @@ class Mailbox {

const connectionClient = await this.connection.getImapConnection(allowSecondary);

let lock = await this.getMailboxLock(connectionClient);
let lock = await this.getMailboxLock(connectionClient, { description: `Update message ${message.uid}` });

try {
let result = {};
Expand Down Expand Up @@ -2434,7 +2434,7 @@ class Mailbox {

const connectionClient = await this.connection.getImapConnection(allowSecondary);

let lock = await this.getMailboxLock(connectionClient);
let lock = await this.getMailboxLock(connectionClient, { description: `Update messages` });

try {
let result = {};
Expand Down Expand Up @@ -2503,7 +2503,7 @@ class Mailbox {

const connectionClient = await this.connection.getImapConnection(allowSecondary);

let lock = await this.getMailboxLock(connectionClient);
let lock = await this.getMailboxLock(connectionClient, { description: `Move message ${message.uid}` });

try {
let result = {};
Expand Down Expand Up @@ -2532,7 +2532,7 @@ class Mailbox {

const connectionClient = await this.connection.getImapConnection(allowSecondary);

let lock = await this.getMailboxLock(connectionClient);
let lock = await this.getMailboxLock(connectionClient, { description: `Move messages` });

try {
let result = {};
Expand Down Expand Up @@ -2561,7 +2561,7 @@ class Mailbox {
async deleteMessage(message, force, allowSecondary) {
const connectionClient = await this.connection.getImapConnection(allowSecondary);

let lock = await this.getMailboxLock(connectionClient);
let lock = await this.getMailboxLock(connectionClient, { description: `Delete message ${message.uid}` });

try {
let result = {};
Expand Down Expand Up @@ -2601,7 +2601,7 @@ class Mailbox {
async deleteMessages(search, force, allowSecondary) {
const connectionClient = await this.connection.getImapConnection(allowSecondary);

let lock = await this.getMailboxLock(connectionClient);
let lock = await this.getMailboxLock(connectionClient, { description: `Delete messages` });

try {
let result = {};
Expand Down Expand Up @@ -2649,7 +2649,7 @@ class Mailbox {

const connectionClient = await this.connection.getImapConnection(allowSecondary);

let lock = await this.getMailboxLock(connectionClient);
let lock = await this.getMailboxLock(connectionClient, { description: `List messages` });

try {
let mailboxStatus = this.getMailboxStatus(connectionClient);
Expand Down
14 changes: 11 additions & 3 deletions lib/routes-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -5979,7 +5979,11 @@ ${Buffer.from(data.content, 'base64url').toString('base64')}
method: 'GET',
path: '/admin/accounts',
async handler(request, h) {
let accountObject = new Account({ redis });
let accountObject = new Account({ redis, call });

const runIndex = await call({
cmd: 'runIndex'
});

const accounts = await accountObject.listAccounts(request.query.state, request.query.query, request.query.page - 1, request.query.pageSize);

Expand All @@ -5989,7 +5993,7 @@ ${Buffer.from(data.content, 'base64url').toString('base64')}

for (let account of accounts.accounts) {
let accountObject = new Account({ redis, account: account.account });
account.data = await accountObject.loadAccountData();
account.data = await accountObject.loadAccountData(null, null, runIndex);

if (account.data && account.data.oauth2 && account.data.oauth2.provider) {
let oauth2App = await oauth2Apps.get(account.data.oauth2.provider);
Expand Down Expand Up @@ -6789,9 +6793,13 @@ ${Buffer.from(data.content, 'base64url').toString('base64')}
let accountObject = new Account({ redis, account: request.params.account, call, secret: await getSecret() });
let accountData;

const runIndex = await call({
cmd: 'runIndex'
});

try {
// throws if account does not exist
accountData = await accountObject.loadAccountData();
accountData = await accountObject.loadAccountData(null, null, runIndex);
} catch (err) {
if (Boom.isBoom(err)) {
throw err;
Expand Down
9 changes: 4 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
"test": "grunt && node --test test/",
"swagger": "./getswagger.sh",
"build-source": "rm -rf node_modules && npm install && rm -rf node_modules && npm install --omit=dev && ./update-info.sh",
"build-dist": "npx pkg --compress Brotli package.json && npm install && node winconf.js",
"build-dist-fast": "npx pkg --debug package.json && npm install && node winconf.js",
"build-dist": "pkg --compress Brotli package.json && npm install && node winconf.js",
"build-dist-fast": "pkg --debug package.json && npm install && node winconf.js",
"licenses": "license-checker --excludePackages emailengine-app --json | node list-generate.js > static/licenses.html",
"gettext": "find ./views -name \"*.hbs\" -print0 | xargs -0 xgettext-template -L Handlebars -o translations/messages.pot --force-po && jsxgettext lib/routes-ui.js workers/api.js lib/tools.js -j -o translations/messages.pot",
"prepare-docker": "echo \"EE_DOCKER_LEGACY=$EE_DOCKER_LEGACY\" >> system.env && cat system.env",
Expand Down Expand Up @@ -59,7 +59,7 @@
"ace-builds": "1.32.7",
"base32.js": "0.1.0",
"bull-arena": "4.2.0",
"bullmq": "5.4.2",
"bullmq": "5.4.3",
"compare-versions": "6.1.0",
"dotenv": "16.4.5",
"encoding-japanese": "2.0.0",
Expand All @@ -75,7 +75,7 @@
"humanize": "0.0.9",
"ical.js": "1.5.0",
"iconv-lite": "0.6.3",
"imapflow": "1.0.156",
"imapflow": "1.0.157",
"ioredfour": "1.3.0-ioredis-07",
"ioredis": "5.3.2",
"ipaddr.js": "2.1.0",
Expand Down Expand Up @@ -121,7 +121,6 @@
"grunt-eslint": "24.3.0",
"jsxgettext": "0.11.0",
"pino-pretty": "10.3.1",
"pkg": "5.8.1",
"resedit": "2.0.0",
"xgettext-template": "5.0.0"
},
Expand Down
Loading

0 comments on commit 0e70898

Please sign in to comment.