Skip to content

Commit

Permalink
chore(tests): Zms 90 (#503)
Browse files Browse the repository at this point in the history 
* mailbox-handler create() returns errors, code and http status codes correctly

* mailboxes handler now correctly return errors, error codes and appropriate http status codes, fixed tests

* mailbox-handler.js codes fix + tests fix in mailboxes-test.js

* mailbox-handler.js correct error handling for update and rename

* fix mailboxes.js http status part for PUT request, fix appropriate tests

* update responseWrapper, remove unnecessary try catches, fix mailbox-handler error codes and http status response codes, fix tests

* fix imap-core create

* reverted 405 errors to 400

* fix tests
  • Loading branch information
@NickOvt
NickOvt authored Sep 28, 2023
1 parent 1c1b414 commit e43ba60
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 109 deletions.
2 changes: 1 addition & 1 deletion imap-core/lib/commands/create.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ module.exports = {
// do not return actual error to user
return callback(null, {
response: 'NO',
code: 'TEMPFAIL'
code: err.code || 'TEMPFAIL'
});
}

Expand Down
59 changes: 6 additions & 53 deletions lib/api/mailboxes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,24 +296,10 @@ module.exports = (db, server, mailboxHandler) => {
}

let status, id;
try {
let data = await createMailbox(user, path, opts);
status = data.status;
id = data.id;
} catch (err) {
res.status(500); // TODO: use response code specific status
return res.json({
error: err.message,
code: err.code
});
}

if (typeof status === 'string') {
res.status(500);
return res.json({
error: 'Mailbox creation failed with code ' + status
});
}
let data = await createMailbox(user, path, opts);
status = data.status;
id = data.id;

return res.json({
success: !!status,
Expand All @@ -326,7 +312,7 @@ module.exports = (db, server, mailboxHandler) => {
'/users/:user/mailboxes/:mailbox',
tools.responseWrapper(async (req, res) => {
res.charSet('utf-8');
const schema = Joi.object().keys({
const schema = Joi.object({
user: Joi.string().hex().lowercase().length(24).required(),
mailbox: Joi.string().hex().lowercase().length(24).allow('resolve').required(),
path: Joi.string().regex(/\/{2,}|\/$/, { invert: true }),
Expand Down Expand Up @@ -506,23 +492,7 @@ module.exports = (db, server, mailboxHandler) => {
});
}

let status;
try {
status = await updateMailbox(user, mailbox, updates);
} catch (err) {
res.status(500); // TODO: use response code specific status
return res.json({
error: err.message,
code: err.code
});
}

if (typeof status === 'string') {
res.status(500);
return res.json({
error: 'Mailbox update failed with code ' + status
});
}
await updateMailbox(user, mailbox, updates);

return res.json({
success: true
Expand Down Expand Up @@ -566,24 +536,7 @@ module.exports = (db, server, mailboxHandler) => {
let user = new ObjectId(result.value.user);
let mailbox = new ObjectId(result.value.mailbox);

let status;
try {
status = await deleteMailbox(user, mailbox);
} catch (err) {
res.status(500); // TODO: use response code specific status
return res.json({
error: err.message,
code: err.code
});
}

if (typeof status === 'string') {
res.status(500); // TODO: use response code specific status
return res.json({
error: 'Mailbox deletion failed with code ' + status,
code: status
});
}
await deleteMailbox(user, mailbox);

return res.json({
success: true
Expand Down
55 changes: 44 additions & 11 deletions lib/mailbox-handler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ class MailboxHandler {
return callback(err);
}
if (mailboxData) {
return callback(null, 'ALREADYEXISTS');
const err = new Error('Mailbox creation failed with code MailboxAlreadyExists');
err.code = 'ALREADYEXISTS';
err.responseCode = 400;
return callback(err, 'ALREADYEXISTS');
}

this.users.collection('users').findOne(
Expand All @@ -50,7 +53,10 @@ class MailboxHandler {
}

if (!userData) {
return callback(new Error('User not found'));
const err = new Error('This user does not exist');
err.code = 'UserNotFound';
err.responseCode = 404;
return callback(err, 'UserNotFound');
}

mailboxData = {
Expand All @@ -74,7 +80,10 @@ class MailboxHandler {
this.database.collection('mailboxes').insertOne(mailboxData, { writeConcern: 'majority' }, (err, r) => {
if (err) {
if (err.code === 11000) {
return callback(null, 'ALREADYEXISTS');
const err = new Error('Mailbox creation failed with code MailboxAlreadyExists');
err.code = 'ALREADYEXISTS';
err.responseCode = 400;
return callback(err, 'ALREADYEXISTS');
}
return callback(err);
}
Expand Down Expand Up @@ -116,10 +125,16 @@ class MailboxHandler {
return callback(err);
}
if (!mailboxData) {
return callback(null, 'NONEXISTENT');
const err = new Error('Mailbox update failed with code NoSuchMailbox');
err.code = 'NONEXISTENT';
err.responseCode = 404;
return callback(err, 'NONEXISTENT');
}
if (mailboxData.path === 'INBOX' || mailboxData.hidden) {
return callback(null, 'CANNOT');
const err = new Error('Mailbox update failed with code DisallowedMailboxMethod');
err.code = 'CANNOT';
err.responseCode = 400;
return callback(err, 'CANNOT');
}
this.database.collection('mailboxes').findOne(
{
Expand All @@ -131,7 +146,10 @@ class MailboxHandler {
return callback(err);
}
if (existing) {
return callback(null, 'ALREADYEXISTS');
const err = new Error('Mailbox rename failed with code MailboxAlreadyExists');
err.code = 'ALREADYEXISTS';
err.responseCode = 400;
return callback(err, 'ALREADYEXISTS');
}

let $set = { path: newname };
Expand All @@ -157,7 +175,10 @@ class MailboxHandler {

if (!item || !item.value) {
// was not able to acquire a lock
return callback(null, 'NONEXISTENT');
const err = new Error('Mailbox update failed with code NoSuchMailbox');
err.code = 'NONEXISTENT';
err.responseCode = 404;
return callback(err, 'NONEXISTENT');
}

publish(this.redis, {
Expand Down Expand Up @@ -201,10 +222,16 @@ class MailboxHandler {
return callback(err);
}
if (!mailboxData) {
return callback(null, 'NONEXISTENT');
const err = new Error('Mailbox deletion failed with code NoSuchMailbox');
err.code = 'NONEXISTENT';
err.responseCode = 404;
return callback(err, 'NONEXISTENT');
}
if (mailboxData.specialUse || mailboxData.path === 'INBOX' || mailboxData.hidden) {
return callback(null, 'CANNOT');
const err = new Error('Mailbox deletion failed with code DisallowedMailboxMethod');
err.code = 'CANNOT';
err.responseCode = 400;
return callback(err, 'CANNOT');
}

this.database.collection('mailboxes').deleteOne(
Expand Down Expand Up @@ -338,7 +365,10 @@ class MailboxHandler {
return callback(err);
}
if (!mailboxData) {
return callback(null, 'NONEXISTENT');
const err = new Error('Mailbox update failed with code NoSuchMailbox');
err.code = 'NONEXISTENT';
err.responseCode = 404;
return callback(err, 'NONEXISTENT');
}

if (updates.path && updates.path !== mailboxData.path) {
Expand Down Expand Up @@ -373,7 +403,10 @@ class MailboxHandler {
}

if (!item || !item.value) {
return callback(null, 'NONEXISTENT');
const err = new Error('Mailbox update failed with code NoSuchMailbox');
err.code = 'NONEXISTENT';
err.responseCode = 404;
return callback(err, 'NONEXISTENT');
}

return callback(null, true);
Expand Down
4 changes: 4 additions & 0 deletions lib/tools.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,10 @@ module.exports = {
err.responseCode = err.responseCode || 404;
err.code = 'NoSuchMailbox';
break;
case 'CANNOT':
err.responseCode = err.responseCode || 400;
err.code = 'DisallowedMailboxMethod';
break;
}

if (err.responseCode) {
Expand Down
68 changes: 24 additions & 44 deletions test/api/mailboxes-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ describe('Mailboxes tests', function () {
expect(response.body.error).to.not.be.empty;
});

// TODO: rewrite when mailboxes.js gets updated status codes
it('should POST /users/{user}/mailboxes expect failure / user not found', async () => {
const response = await server
.post(`/users/${'0'.repeat(24)}/mailboxes`)
.send({ path: '/cool2/path2/whatisthis346/cool-drink', hidden: false, retention: 0 })
.expect(500);
.expect(404);

expect(response.body.error).to.be.equal('User not found');
expect(response.body.error).to.be.equal('This user does not exist');
expect(response.body.code).to.be.equal('UserNotFound');
});

it('should POST /users/{user}/mailboxes expect failure / path is missing', async () => {
Expand Down Expand Up @@ -228,30 +228,21 @@ describe('Mailboxes tests', function () {
expect(response3.body.code).to.be.equal('InputValidationError');
expect(response3.body.error).to.be.not.empty;

const response4 = await server
.get(`/users/${user}/mailboxes/${'-'.repeat(24)}`)

.expect(400);
const response4 = await server.get(`/users/${user}/mailboxes/${'-'.repeat(24)}`).expect(400);

expect(response4.body.code).to.be.equal('InputValidationError');
expect(response4.body.error).to.be.not.empty;
});

it('should GET /users/{user}/mailboxes/{mailbox} expect failure / mailbox not found', async () => {
const response = await server
.get(`/users/${user}/mailboxes/${'0'.repeat(24)}`)

.expect(404);
const response = await server.get(`/users/${user}/mailboxes/${'0'.repeat(24)}`).expect(404);

expect(response.body.code).to.be.equal('NoSuchMailbox');
expect(response.body.error).to.be.equal('This mailbox does not exist');
});

it('should GET /users/{user}/mailboxes/{mailbox} expect failure / user not found', async () => {
const response = await server
.get(`/users/${'0'.repeat(24)}/mailboxes`)

.expect(404);
const response = await server.get(`/users/${'0'.repeat(24)}/mailboxes`).expect(404);

expect(response.body.error).to.be.equal('This user does not exist');
expect(response.body.code).to.be.equal('UserNotFound');
Expand Down Expand Up @@ -335,9 +326,9 @@ describe('Mailboxes tests', function () {
const response = await server
.put(`/users/${user}/mailboxes/${'0'.repeat(24)}`)
.send({ path: 'newPath' })
.expect(500);
.expect(404);

expect(response.body.error).to.be.equal('Mailbox update failed with code NONEXISTENT');
expect(response.body.error).to.be.equal('Mailbox update failed with code NoSuchMailbox');
});

it('should PUT /users/{user}/mailboxes/{mailbox} expect failure / user not found', async () => {
Expand All @@ -346,18 +337,15 @@ describe('Mailboxes tests', function () {
const response = await server
.put(`/users/${'0'.repeat(24)}/mailboxes/${mailboxes.body.results.at(-1).id}`)
.send({ path: 'newPath' })
.expect(500);
.expect(404);

expect(response.body.error).to.be.equal('Mailbox update failed with code NONEXISTENT');
expect(response.body.error).to.be.equal('Mailbox update failed with code NoSuchMailbox');
});

it('should PUT /users/{user}/mailboxes/{mailbox} expect failure / nothing was changed', async () => {
const mailboxes = await server.get(`/users/${user}/mailboxes`).expect(200);

const response = await server
.put(`/users/${user}/mailboxes/${mailboxes.body.results.at(-1).id}`)

.expect(400);
const response = await server.put(`/users/${user}/mailboxes/${mailboxes.body.results.at(-1).id}`).expect(400);

expect(response.body.error).to.be.equal('Nothing was changed');
});
Expand All @@ -367,9 +355,9 @@ describe('Mailboxes tests', function () {

const inboxMailbox = mailboxes.body.results.find(el => el.path === 'INBOX');

const response = await server.put(`/users/${user}/mailboxes/${inboxMailbox.id}`).send({ path: 'newPath/folder123' }).expect(500);
const response = await server.put(`/users/${user}/mailboxes/${inboxMailbox.id}`).send({ path: 'newPath/folder123' }).expect(400);

expect(response.body.error).to.be.equal('Mailbox update failed with code CANNOT');
expect(response.body.error).to.be.equal('Mailbox update failed with code DisallowedMailboxMethod');
});

it('should DELETE /users/{user}/mailboxes/{mailbox} expect success', async () => {
Expand All @@ -385,10 +373,10 @@ describe('Mailboxes tests', function () {
it('should DELETE /users/{user}/mailboxes/{mailbox} expect failure / protected path', async () => {
const mailboxes = await server.get(`/users/${user}/mailboxes`).expect(200);

const response = await server.del(`/users/${user}/mailboxes/${mailboxes.body.results[0].id}`).expect(500);
const response = await server.del(`/users/${user}/mailboxes/${mailboxes.body.results[0].id}`).expect(400);

expect(response.body.error).to.be.equal('Mailbox deletion failed with code CANNOT');
expect(response.body.code).to.be.equal('CANNOT');
expect(response.body.error).to.be.equal('Mailbox deletion failed with code DisallowedMailboxMethod');
expect(response.body.code).to.be.equal('DisallowedMailboxMethod');
});

it('should DELETE /users/{user}/mailboxes/{mailbox} expect failure / incorrect params', async () => {
Expand Down Expand Up @@ -422,36 +410,28 @@ describe('Mailboxes tests', function () {
});

it('should DELETE /users/{user}/mailboxes/{mailbox} expect failure / mailbox not found', async () => {
const response = await server
.del(`/users/${user}/mailboxes/${'0'.repeat(24)}`)

.expect(500);

console.log(response.body);
const response = await server.del(`/users/${user}/mailboxes/${'0'.repeat(24)}`).expect(404);

expect(response.body.error).to.be.equal('Mailbox deletion failed with code NONEXISTENT');
expect(response.body.code).to.be.equal('NONEXISTENT');
expect(response.body.error).to.be.equal('Mailbox deletion failed with code NoSuchMailbox');
expect(response.body.code).to.be.equal('NoSuchMailbox');
});

it('should DELETE /users/{user}/mailboxes/{mailbox} expect failure / user not found', async () => {
const mailboxes = await server.get(`/users/${user}/mailboxes`).expect(200);

const response = await server
.del(`/users/${'0'.repeat(24)}/mailboxes/${mailboxes.body.results[0].id}`)

.expect(500);
const response = await server.del(`/users/${'0'.repeat(24)}/mailboxes/${mailboxes.body.results[0].id}`).expect(404);

expect(response.body.error).to.be.equal('Mailbox deletion failed with code NONEXISTENT');
expect(response.body.error).to.be.equal('Mailbox deletion failed with code NoSuchMailbox');
});

it('should DELETE /users/{user}/mailboxes/{mailbox} expect failure / cannot delete inbox', async () => {
const mailboxes = await server.get(`/users/${user}/mailboxes`).expect(200);

const inboxMailbox = mailboxes.body.results.find(el => el.path === 'INBOX');

const response = await server.del(`/users/${user}/mailboxes/${inboxMailbox.id}`).expect(500);
const response = await server.del(`/users/${user}/mailboxes/${inboxMailbox.id}`).expect(400);

expect(response.body.error).to.be.equal('Mailbox deletion failed with code CANNOT');
expect(response.body.code).to.be.equal('CANNOT');
expect(response.body.error).to.be.equal('Mailbox deletion failed with code DisallowedMailboxMethod');
expect(response.body.code).to.be.equal('DisallowedMailboxMethod');
});
});

0 comments on commit e43ba60

Please sign in to comment.