I have a Controller
class for which my delete
function is out of control. I've come to this conclusion because my tests are complicated, the function is greater than 30 lines, I have nested if/then
statements, and multiple things are happening inside the function.
Deciding to break up the delete
function, I am unclear on the best approach. Oh, and unless I'm confused, I want to transition the code from imperative to more declarative.
Here is the method as it stands today:
static async delete(req: express.Request, res: express.Response) {
try {
// extract and validate apiAccess token
const { apiAccessToken } = req.body;
if (!apiAccessToken || typeof apiAccessToken !== 'string') {
res.status(400).json({ error: 'Bad password format, expected string.' });
return;
}
// extract jwt from headers
const userJwt = req.get('Authorization').slice('Bearer '.length);
// generate a Registrant object representing the claimed user
const userClaimResult = Registrant.decoded(userJwt);
if (userClaimResult.err) {
const { message } = userClaimResult.val;
res.status(401).json({ message });
return;
}
const userClaim = userClaimResult.val;
// attempt to get registrant record from database, and validate
const getRegistrantResult = await RegistrantsDAO.getRegistrant(userClaim.email);
if (getRegistrantResult.err) {
const { message, httpCode } = getRegistrantResult.val;
res.status(httpCode).json({ message });
} else {
const registrant = new Registrant(getRegistrantResult.val);
const validateUserClaimResult = await registrant.compareAPIAccessToken(apiAccessToken);
if (validateUserClaimResult.err) {
const { message, httpCode } = validateUserClaimResult.val;
res.status(httpCode).json({ message });
} else {
const deleteResult = await RegistrantsDAO.deleteRegistrant(userClaim.email);
if (deleteResult.err) {
const { message, httpCode } = deleteResult.val;
res.status(httpCode).json({ message })
} else res.json(deleteResult.val);
}
}
} catch (e) {
res.status(500).json(e);
}
}
}
The method performs four tasks as I see it:
-
Extract and validate the API access token. This code may be handled by the
express-validator
lib, and so I'm not too worried about it right now. -
Extract JWT and create a
Registrant
object as the user claim. TheRegistrant
object (returned fromdecoded
) representing the claim is what I want for the next stage... -
Authenticate the
Registrant
(i.e., user). This involves use of the dependent DAO to retrieve and validate the user's claim. Or, it could be argued, the authenticated user is what I really want, right? Or is this more accurately referred to as a validated user claim? - Finally, delete the user.
Testing this function requires mocking (inside isolateModules
because my DAO is a singleton) the Registrant
constructor and two of its functions, the RegistrantDAO
and req/res
. Ideally, breaking this up should simplify the test.
It does seem there are a couple of approaches I could consider:
- Write a separate private function that provides the user claim (or error) back to this
delete
function, followed by a separate private function to validate the claim against the database. - Write a single private function that is passed the extracted JWT which then returns
true/false
if theRegistrant
and claim are both valid.
I'm leaning towards the second option, but suspect this just shifts the difficult testing to a different code block since both the Registrant
and RegistrantDAO
classes would be used in this new method. If I split generateUserClaim
from validateUser
, then it might simplify testing a little bit.
Am I on track or should I be considering a different approach?
Top comments (1)
I received a tip on Discord to explore custom middleware as a way to break this function up. That is the approach Iām now researching.