From 80f791c86b5917b9ed996505215765016bd3441b Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Thu, 3 Mar 2016 00:16:28 +0100 Subject: [PATCH] add tests, move CustomError to common, fix casting --- common/locales/en/api-v3.json | 10 +- common/script/api-v3/customError.js | 8 + common/script/api-v3/errors.js | 9 + common/script/content/spells.js | 29 +-- .../user/POST-user_class_cast_spellId.test.js | 172 ++++++++++++++++++ .../v3/unit/middlewares/errorHandler.test.js | 16 ++ website/src/controllers/api-v3/user.js | 27 +-- website/src/libs/api-v3/errors.js | 9 +- .../src/middlewares/api-v3/errorHandler.js | 10 +- website/src/models/group.js | 10 +- website/src/models/user.js | 2 +- 11 files changed, 260 insertions(+), 42 deletions(-) create mode 100644 common/script/api-v3/customError.js create mode 100644 common/script/api-v3/errors.js create mode 100644 test/api/v3/integration/user/POST-user_class_cast_spellId.test.js diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json index faa7ddc984..06acb4161d 100644 --- a/common/locales/en/api-v3.json +++ b/common/locales/en/api-v3.json @@ -73,7 +73,7 @@ "guildQuestsNotSupported": "Guilds cannot be invited on quests.", "questNotFound": "Quest \"<%= key %>\" not found.", "questNotOwned": "You don't own that quest scroll.", - "questLevelTooHigh": "You must be Level <%= level %> to begin this quest.", + "questLevelTooHigh": "You must be level <%= level %> to begin this quest.", "questAlreadyUnderway": "Your party is already on a quest. Try again when the current quest has ended.", "questAlreadyAccepted": "You already accepted the quest invitation.", "noActiveQuestToLeave": "No active quest to leave", @@ -90,5 +90,11 @@ "noAdminAccess": "You don't have admin access.", "pageMustBeNumber": "req.query.page must be a number", "missingUnsubscriptionCode": "Missing unsubscription code.", - "userNotFound": "User not Found" + "userNotFound": "User not found.", + "spellNotFound": "Spell \"<%= spellId %>\" not found.", + "partyNotFound": "Party not found", + "targetIdUUID": "\"targetId\" must be a valid UUID.", + "challengeTasksNoCast": "Casting a spell on challenge tasks is not supported.", + "spellNotOwned": "You don't own this spell.", + "spellLevelTooHigh": "You must be level <%= level %> to use this spell." } diff --git a/common/script/api-v3/customError.js b/common/script/api-v3/customError.js new file mode 100644 index 0000000000..3ebf5a316f --- /dev/null +++ b/common/script/api-v3/customError.js @@ -0,0 +1,8 @@ +// Base class for custom application errors +// It extends Error and capture the stack trace +export default class CustomError extends Error { + constructor () { + super(); + Error.captureStackTrace(this, this.constructor); + } +} diff --git a/common/script/api-v3/errors.js b/common/script/api-v3/errors.js new file mode 100644 index 0000000000..d4db1183da --- /dev/null +++ b/common/script/api-v3/errors.js @@ -0,0 +1,9 @@ +import CustomError from './customError'; + +export class NotAuthorized extends CustomError { + constructor (customMessage) { + super(); + this.name = this.constructor.name; + this.message = customMessage || 'Not authorized.'; + } +} diff --git a/common/script/content/spells.js b/common/script/content/spells.js index 7609bd844a..bf3149bf3a 100644 --- a/common/script/content/spells.js +++ b/common/script/content/spells.js @@ -1,6 +1,6 @@ import t from './translation'; import _ from 'lodash'; - +import { NotAuthorized } from '../api-v3/errors'; /* --------------------------------------------------------------- Spells @@ -40,14 +40,12 @@ spells.wizard = { lvl: 11, target: 'task', notes: t('spellWizardFireballNotes'), - cast (user, target) { + cast (user, target, req) { let bonus = user._statsComputed.int * user.fns.crit('per'); bonus *= Math.ceil((target.value < 0 ? 1 : target.value + 1) * 0.075); user.stats.exp += diminishingReturns(bonus, 75); if (!user.party.quest.progress) user.party.quest.progress = 0; user.party.quest.progress.up += Math.ceil(user._statsComputed.int * 9.1); - // TODO change, pass req to spell? - let req = {language: user.preferences.language}; user.fns.updateStats(user.stats, req); }, }, @@ -166,12 +164,11 @@ spells.rogue = { lvl: 12, target: 'task', notes: t('spellRogueBackStabNotes'), - cast (user, target) { + cast (user, target, req) { let _crit = user.fns.crit('str', 0.3); let bonus = calculateBonus(target.value, user._statsComputed.str, _crit); user.stats.exp += diminishingReturns(bonus, 75, 50); user.stats.gp += diminishingReturns(bonus, 18, 75); - let req = {language: user.preferences.language}; user.fns.updateStats(user.stats, req); }, }, @@ -262,9 +259,11 @@ spells.special = { text: t('spellSpecialSnowballAuraText'), mana: 0, value: 15, + previousPurchase: true, target: 'user', notes: t('spellSpecialSnowballAuraNotes'), - cast (user, target) { + cast (user, target, req) { + if (!user.items.special.snowball) throw new NotAuthorized(t('spellNotOwned')(req.language)); target.stats.buffs.snowball = true; target.stats.buffs.spookDust = false; target.stats.buffs.shinySeed = false; @@ -290,9 +289,11 @@ spells.special = { text: t('spellSpecialSpookDustText'), mana: 0, value: 15, + previousPurchase: true, target: 'user', notes: t('spellSpecialSpookDustNotes'), - cast (user, target) { + cast (user, target, req) { + if (!user.items.special.spookDust) throw new NotAuthorized(t('spellNotOwned')(req.language)); target.stats.buffs.snowball = false; target.stats.buffs.spookDust = true; target.stats.buffs.shinySeed = false; @@ -318,9 +319,11 @@ spells.special = { text: t('spellSpecialShinySeedText'), mana: 0, value: 15, + previousPurchase: true, target: 'user', notes: t('spellSpecialShinySeedNotes'), - cast (user, target) { + cast (user, target, req) { + if (!user.items.special.shinySeed) throw new NotAuthorized(t('spellNotOwned')(req.language)); target.stats.buffs.snowball = false; target.stats.buffs.spookDust = false; target.stats.buffs.shinySeed = true; @@ -346,9 +349,11 @@ spells.special = { text: t('spellSpecialSeafoamText'), mana: 0, value: 15, + previousPurchase: true, target: 'user', notes: t('spellSpecialSeafoamNotes'), - cast (user, target) { + cast (user, target, req) { + if (!user.items.special.seafoam) throw new NotAuthorized(t('spellNotOwned')(req.language)); target.stats.buffs.snowball = false; target.stats.buffs.spookDust = false; target.stats.buffs.shinySeed = false; @@ -499,8 +504,8 @@ _.each(spells, (spellClass) => { _.each(spellClass, (spell, key) => { spell.key = key; let _cast = spell.cast; - spell.cast = function castSpell (user, target) { - _cast(user, target); + spell.cast = function castSpell (user, target, req) { + _cast(user, target, req); user.stats.mp -= spell.mana; }; }); diff --git a/test/api/v3/integration/user/POST-user_class_cast_spellId.test.js b/test/api/v3/integration/user/POST-user_class_cast_spellId.test.js new file mode 100644 index 0000000000..169ad16dd2 --- /dev/null +++ b/test/api/v3/integration/user/POST-user_class_cast_spellId.test.js @@ -0,0 +1,172 @@ +import { + generateUser, + translate as t, + createAndPopulateGroup, + generateChallenge, + sleep, +} from '../../../../helpers/api-integration/v3'; + +import { v4 as generateUUID } from 'uuid'; + +describe('POST /user/class/cast/:spellId', () => { + let user; + + beforeEach(async () => { + user = await generateUser(); + }); + + it('returns an error if spell does not exist', async () => { + await user.update({'stats.class': 'rogue'}); + let spellId = 'invalidSpell'; + await expect(user.post(`/user/class/cast/${spellId}`)) + .to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('spellNotFound', {spellId}), + }); + }); + + it('returns an error if spell does not exist in user\'s class', async () => { + let spellId = 'pickPocket'; + await expect(user.post(`/user/class/cast/${spellId}`)) + .to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('spellNotFound', {spellId}), + }); + }); + + it('returns an error if spell.mana > user.mana', async () => { + await user.update({'stats.class': 'rogue'}); + await expect(user.post(`/user/class/cast/backStab`)) + .to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('notEnoughMana'), + }); + }); + + it('returns an error if spell.value > user.gold', async () => { + await expect(user.post(`/user/class/cast/birthday`)) + .to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('messageNotEnoughGold'), + }); + }); + + it('returns an error if spell.lvl > user.level', async () => { + await user.update({'stats.mp': 200, 'stats.class': 'wizard'}); + await expect(user.post(`/user/class/cast/earth`)) + .to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('spellLevelTooHigh', {level: 13}), + }); + }); + + it('returns an error if user doesn\'t own the spell', async () => { + await expect(user.post(`/user/class/cast/snowball`)) + .to.eventually.be.rejected.and.eql({ + code: 400, + error: 'NotAuthorized', + message: t('spellNotOwned'), + }); + }); + + it('returns an error if targetId is not an UUID', async () => { + await expect(user.post('/user/class/cast/spellId?targetId=notAnUUID')) + .to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: t('invalidReqParams'), + }); + }); + + it('returns an error if targetId is required but missing', async () => { + await user.update({'stats.class': 'rogue', 'stats.lvl': 11}); + await expect(user.post(`/user/class/cast/pickPocket`)) + .to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: t('targetIdUUID'), + }); + }); + + it('returns an error if targeted task doesn\'t exist', async () => { + await user.update({'stats.class': 'rogue', 'stats.lvl': 11}); + await expect(user.post(`/user/class/cast/pickPocket?targetId=${generateUUID()}`)) + .to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('taskNotFound'), + }); + }); + + it('returns an error if a challenge task was targeted', async () => { + let {group, groupLeader} = await createAndPopulateGroup(); + let challenge = await generateChallenge(groupLeader, group); + await groupLeader.post(`/tasks/challenge/${challenge._id}`, [ + {type: 'habit', text: 'task text'}, + ]); + await groupLeader.update({'stats.class': 'rogue', 'stats.lvl': 11}); + await sleep(0.5); + await groupLeader.sync(); + await expect(groupLeader.post(`/user/class/cast/pickPocket?targetId=${groupLeader.tasksOrder.habits[0]}`)) + .to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: t('challengeTasksNoCast'), + }); + }); + + it('returns an error if targeted party member doesn\'t exist', async () => { + let {groupLeader} = await createAndPopulateGroup({ + groupDetails: { type: 'party', privacy: 'private' }, + members: 1, + }); + await groupLeader.update({'items.special.snowball': 3}); + + let target = generateUUID(); + await expect(groupLeader.post(`/user/class/cast/snowball?targetId=${target}`)) + .to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('userWithIDNotFound', {userId: target}), + }); + }); + + it('returns an error if party does not exists', async () => { + await user.update({'items.special.snowball': 3}); + + await expect(user.post(`/user/class/cast/snowball?targetId=${generateUUID()}`)) + .to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('partyNotFound'), + }); + }); + + it('send message in party chat if party && !spell.silent', async () => { + let { group, groupLeader } = await createAndPopulateGroup({ + groupDetails: { type: 'party', privacy: 'private' }, + members: 1, + }); + await groupLeader.update({'stats.mp': 200, 'stats.class': 'wizard', 'stats.lvl': 13}); + await groupLeader.post(`/user/class/cast/earth`); + await sleep(1); + await group.sync(); + expect(group.chat[0]).to.exists; + expect(group.chat[0].uuid).to.equal('system'); + }); + + // TODO find a way to have sinon working in integration tests + // it doesn't work when tests are running separately from server + it('passes correct target to spell when targetType === \'task\''); + it('passes correct target to spell when targetType === \'tasks\''); + it('passes correct target to spell when targetType === \'self\''); + it('passes correct target to spell when targetType === \'party\''); + it('passes correct target to spell when targetType === \'user\''); + it('passes correct target to spell when targetType === \'party\' and user is not in a party'); + it('passes correct target to spell when targetType === \'user\' and user is not in a party'); +}); diff --git a/test/api/v3/unit/middlewares/errorHandler.test.js b/test/api/v3/unit/middlewares/errorHandler.test.js index 269390b35b..694a02a8fe 100644 --- a/test/api/v3/unit/middlewares/errorHandler.test.js +++ b/test/api/v3/unit/middlewares/errorHandler.test.js @@ -9,6 +9,7 @@ import responseMiddleware from '../../../../../website/src/middlewares/api-v3/re import getUserLanguage from '../../../../../website/src/middlewares/api-v3/getUserLanguage'; import { BadRequest } from '../../../../../website/src/libs/api-v3/errors'; +import { NotAuthorized as NotAuthorizedShared } from '../../../../../common/script/api-v3/errors'; import logger from '../../../../../website/src/libs/api-v3/logger'; describe('errorHandler', () => { @@ -86,6 +87,21 @@ describe('errorHandler', () => { }); }); + it('handle CustomError(s) from shared code', () => { + let error = new NotAuthorizedShared(); + + errorHandler(error, req, res, next); + + expect(res.status).to.be.calledOnce; + expect(res.json).to.be.calledOnce; + + expect(res.status).to.be.calledWith(400); + expect(res.json).to.be.calledWith({ + error: 'NotAuthorized', + message: 'Not authorized.', + }); + }); + it('handle http-errors errors', () => { let error = new Error('custom message'); error.statusCode = 422; diff --git a/website/src/controllers/api-v3/user.js b/website/src/controllers/api-v3/user.js index bcdad4d9f1..2925a828cc 100644 --- a/website/src/controllers/api-v3/user.js +++ b/website/src/controllers/api-v3/user.js @@ -4,6 +4,7 @@ import common from '../../../../common'; import { NotFound, BadRequest, + NotAuthorized, } from '../../libs/api-v3/errors'; import * as Tasks from '../../models/task'; import { model as Group } from '../../models/group'; @@ -43,7 +44,7 @@ api.getUser = { const partyMembersFields = 'profile.name stats achievements items.special'; /** - * @api {post} /user/class/cast/:spell Cast a spell on a target. + * @api {post} /user/class/cast/:spellId Cast a spell on a target. * @apiVersion 3.0.0 * @apiName UserCast * @apiGroup User @@ -56,7 +57,7 @@ const partyMembersFields = 'profile.name stats achievements items.special'; api.castSpell = { method: 'POST', middlewares: [authWithHeaders(), cron], - url: '/user/class/cast/:spell', + url: '/user/class/cast/:spellId', async handler (req, res) { let user = res.locals.user; let spellId = req.params.spellId; @@ -71,14 +72,16 @@ api.castSpell = { let klass = common.content.spells.special[spellId] ? 'special' : user.stats.class; let spell = common.content.spells[klass][spellId]; - if (!spell) throw new NotFound(res.t('spellNotFound', {spell: spellId})); - if (spell.mana > user.stats.mp) throw new BadRequest(res.t('notEnoughMana')); + if (!spell) throw new NotFound(res.t('spellNotFound', {spellId})); + if (spell.mana > user.stats.mp) throw new NotAuthorized(res.t('notEnoughMana')); + if (spell.value > user.stats.gp && !spell.previousPurchase) throw new NotAuthorized(res.t('messageNotEnoughGold')); + if (spell.lvl > user.stats.lvl) throw new NotAuthorized(res.t('spellLevelTooHigh', {level: spell.lvl})); + let targetType = spell.target; if (targetType === 'task') { if (!targetId) throw new BadRequest(res.t('targetIdUUID')); - // TODO what about challenge tasks? should casting be disabled on them? let task = await Tasks.Task.findOne({ _id: targetId, userId: user._id, @@ -86,11 +89,11 @@ api.castSpell = { if (!task) throw new NotFound(res.t('taskNotFound')); if (task.challenge.id) throw new BadRequest(res.t('challengeTasksNoCast')); - spell.cast(user, task); + spell.cast(user, task, req); await task.save(); res.respond(200, task); } else if (targetType === 'self') { - spell.cast(user); + spell.cast(user, null, req); await user.save(); res.respond(200, user); } else if (targetType === 'tasks') { // new target type when all the user's tasks are necessary @@ -103,7 +106,7 @@ api.castSpell = { ], }).exec(); - spell.cast(user, tasks); + spell.cast(user, tasks, req); let toSave = tasks.filter(t => t.isModified()); let isUserModified = user.isModified(); @@ -116,8 +119,7 @@ api.castSpell = { if (isUserModified) res.user = user; res.respond(200, response); } else if (targetType === 'party' || targetType === 'user') { - let party = await Group.getGroup({_id: 'party', user}); - + let party = await Group.getGroup({groupId: 'party', user}); // arrays of users when targetType is 'party' otherwise single users let partyMembers; @@ -128,18 +130,19 @@ api.castSpell = { partyMembers = await User.find({'party._id': party._id}).select(partyMembersFields).exec(); } - spell.cast(user, partyMembers); + spell.cast(user, partyMembers, req); await Q.all(partyMembers.map(m => m.save())); } else { if (!party && (!targetId || user._id === targetId)) { partyMembers = user; } else { if (!targetId) throw new BadRequest(res.t('targetIdUUID')); + if (!party) throw new NotFound(res.t('partyNotFound')); partyMembers = await User.findOne({_id: targetId, 'party._id': party._id}).select(partyMembersFields).exec(); } if (!partyMembers) throw new NotFound(res.t('userWithIDNotFound', {userId: targetId})); - spell.cast(user, partyMembers); + spell.cast(user, partyMembers, req); await partyMembers.save(); } res.respond(200, partyMembers); diff --git a/website/src/libs/api-v3/errors.js b/website/src/libs/api-v3/errors.js index cb93e7b4a2..e1218c805a 100644 --- a/website/src/libs/api-v3/errors.js +++ b/website/src/libs/api-v3/errors.js @@ -1,11 +1,4 @@ -// Base class for custom application errors -// It extends Error and capture the stack trace -export class CustomError extends Error { - constructor () { - super(); - Error.captureStackTrace(this, this.constructor); - } -} +import CustomError from '../../../../common/script/api-v3/customError'; /** * @apiDefine NotAuthorized diff --git a/website/src/middlewares/api-v3/errorHandler.js b/website/src/middlewares/api-v3/errorHandler.js index 0590ad9f55..0518962dff 100644 --- a/website/src/middlewares/api-v3/errorHandler.js +++ b/website/src/middlewares/api-v3/errorHandler.js @@ -1,8 +1,8 @@ // The error handler middleware that handles all errors // and respond to the client import logger from '../../libs/api-v3/logger'; +import CustomError from '../../../../common/script/api-v3/customError'; import { - CustomError, BadRequest, InternalServerError, } from '../../libs/api-v3/errors'; @@ -24,11 +24,17 @@ export default function errorHandler (err, req, res, next) { // eslint-disable-l // If we can't identify it, respond with a generic 500 error let responseErr = err instanceof CustomError ? err : null; + // TODO don't return always 400 for errors in common code + // If CustomError but without httpCode then they come from shared code, treat as 400s + if (err instanceof CustomError && !err.httpCode) { + err.httpCode = 400; + } + // Handle errors created with 'http-errors' or similar that have a status/statusCode property if (err.statusCode && typeof err.statusCode === 'number') { responseErr = new CustomError(); responseErr.httpCode = err.statusCode; - responseErr.error = err.name; + responseErr.name = err.name; responseErr.message = err.message; } diff --git a/website/src/models/group.js b/website/src/models/group.js index 1620ed020c..991a7c9621 100644 --- a/website/src/models/group.js +++ b/website/src/models/group.js @@ -125,20 +125,20 @@ schema.statics.getGroup = async function getGroup (options = {}) { let {user, groupId, fields, optionalMembership = false, populateLeader = false, requireMembership = false} = options; let query; - let isParty = groupId === 'party' || user.party._id === groupId; - let isGuild = user.guilds.indexOf(groupId) !== -1; + let isUserParty = groupId === 'party' || user.party._id === groupId; + let isUserGuild = user.guilds.indexOf(groupId) !== -1; // When requireMembership is true check that user is member even in public guild - if (requireMembership && !isParty && !isGuild) { + if (requireMembership && !isUserParty && !isUserGuild) { return null; } // When optionalMembership is true it's not required for the user to be a member of the group - if (isParty) { + if (isUserParty) { query = {type: 'party', _id: user.party._id}; } else if (optionalMembership === true) { query = {_id: groupId}; - } else if (isGuild) { + } else if (isUserGuild) { query = {type: 'guild', _id: groupId}; } else { query = {type: 'guild', privacy: 'public', _id: groupId}; diff --git a/website/src/models/user.js b/website/src/models/user.js index 7f6632f9da..242f8709bf 100644 --- a/website/src/models/user.js +++ b/website/src/models/user.js @@ -453,7 +453,7 @@ export let schema = new Schema({ lvl: {type: Number, default: 1}, // Class System - class: {type: String, enum: ['warrior', 'rogue', 'wizard', 'healer'], default: 'warrior'}, + class: {type: String, enum: ['warrior', 'rogue', 'wizard', 'healer'], default: 'warrior', required: true}, points: {type: Number, default: 0}, str: {type: Number, default: 0}, con: {type: Number, default: 0},