Skip to content

Commit fa4422a

Browse files
Mark the callback as optional
Without callback, it is impossible to unit-test the backbeat routes, as we run async code and do not yet support promises. Issue: CLDSRV-591
1 parent a0f2c3f commit fa4422a

File tree

2 files changed

+23
-29
lines changed

2 files changed

+23
-29
lines changed

lib/routes/routeBackbeat.js

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,38 +1310,37 @@ const indexingSchema = joi.array().items(indexEntrySchema).min(1);
13101310

13111311
function respondToRequest(err, response, log, callback) {
13121312
responseJSONBody(err, null, response, log);
1313-
return callback(err);
1313+
// The callback is optional, as it is only used for testing purposes
1314+
// but value may be set to non-undefined or null due to the arsenal
1315+
// routes implementation
1316+
if (callback && typeof callback === 'function') {
1317+
return callback(err);
1318+
}
1319+
return undefined;
13141320
}
13151321

13161322
function routeIndexingAPIs(request, response, userInfo, log, callback) {
13171323
const route = backbeatRoutes[request.method][request.resourceType];
13181324

13191325
if (!['GET', 'POST'].includes(request.method)) {
1320-
responseJSONBody(errors.MethodNotAllowed, null, response, log);
1321-
return callback(errors.MethodNotAllowed);
1326+
return respondToRequest(errors.MethodNotAllowed, response, log, callback);
13221327
}
13231328

13241329
if (request.method === 'GET') {
1325-
return route(request, response, userInfo, log, err => {
1326-
if (err) {
1327-
responseJSONBody(err, null, response, log);
1328-
}
1329-
return callback(err);
1330-
});
1330+
return route(request, response, userInfo, log, err =>
1331+
respondToRequest(err, response, log, callback));
13311332
}
13321333

13331334
const op = request.query.operation;
13341335

13351336
if (!op || typeof route[op] !== 'function') {
13361337
log.error('Invalid operataion parameter', { operation: op });
1337-
responseJSONBody(errors.BadRequest, null, response, log);
1338-
return callback(errors.BadRequest);
1338+
return respondToRequest(errors.BadRequest, response, log, callback);
13391339
}
13401340

13411341
return _getRequestPayload(request, (err, payload) => {
13421342
if (err) {
1343-
responseJSONBody(err, null, response, log);
1344-
return callback(err);
1343+
return respondToRequest(err, response, log, callback);
13451344
}
13461345

13471346
let parsedIndex;
@@ -1350,16 +1349,11 @@ function routeIndexingAPIs(request, response, userInfo, log, callback) {
13501349
parsedIndex = joi.attempt(JSON.parse(payload), indexingSchema, 'invalid payload');
13511350
} catch (err) {
13521351
log.error('Unable to parse index request body', { error: err });
1353-
responseJSONBody(errors.BadRequest, null, response, log);
1354-
return callback(errors.BadRequest);
1352+
return respondToRequest(errors.BadRequest, response, log, callback);
13551353
}
13561354

1357-
return route[op](parsedIndex, request, response, userInfo, log, err => {
1358-
if (err) {
1359-
responseJSONBody(err, null, response, log);
1360-
}
1361-
return callback(err);
1362-
});
1355+
return route[op](parsedIndex, request, response, userInfo, log, err =>
1356+
respondToRequest(err, response, log, callback));
13631357
});
13641358
}
13651359

@@ -1477,10 +1471,11 @@ function routeBackbeat(clientIP, request, response, log, callback) {
14771471
resourceType: request.resourceType,
14781472
query: request.query,
14791473
});
1480-
responseJSONBody(errors.MethodNotAllowed, null, response, log);
1481-
return callback(errors.MethodNotAllowed);
1474+
return respondToRequest(errors.MethodNotAllowed, response, log, callback);
14821475
}
14831476

1477+
const isObjectRequest = _isObjectRequest(request);
1478+
14841479
return async.waterfall([
14851480
next => auth.server.doAuth(
14861481
request, log, (err, userInfo, authorizationResults, streamingV4Params, infos) => {
@@ -1501,7 +1496,7 @@ function routeBackbeat(clientIP, request, response, log, callback) {
15011496
next(err, userInfo)),
15021497
(userInfo, next) => {
15031498
// TODO: understand why non-object requests (batchdelete) were not authenticated
1504-
if (!_isObjectRequest(request)) {
1499+
if (!isObjectRequest) {
15051500
if (userInfo.getCanonicalID() === constants.publicId) {
15061501
log.debug(`unauthenticated access to backbeat ${request.resourceType} routes`, {
15071502
method: request.method,
@@ -1543,6 +1538,10 @@ function routeBackbeat(clientIP, request, response, log, callback) {
15431538
return standardMetadataValidateBucketAndObj(mdValParams, request.actionImplicitDenies, log, next);
15441539
},
15451540
(bucketInfo, objMd, next) => {
1541+
// Function was already called
1542+
if (!isObjectRequest) {
1543+
return next();
1544+
}
15461545
if (!useMultipleBackend) {
15471546
return backbeatRoutes[request.method][request.resourceType](
15481547
request, response, bucketInfo, objMd, log, next);

tests/unit/routeBackbeat.js

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ const testBucket = {
2323
namespace,
2424
headers: {
2525
'host': `${bucketName}.s3.amazonaws.com`,
26-
// set versioning
27-
2826
},
2927
url: `/${bucketName}`,
3028
actionImplicitDenies: false,
@@ -47,7 +45,6 @@ describe('routeBackbeat', () => {
4745
let response;
4846

4947
beforeEach(() => {
50-
// Mock backbeatRoutes
5148
sinon.stub(backbeatRoutes, 'PUT').returns({
5249
data: sinon.stub(),
5350
metadata: sinon.stub(),
@@ -86,7 +83,6 @@ describe('routeBackbeat', () => {
8683
index: sinon.stub(),
8784
});
8885

89-
// Mock request and response
9086
request = new DummyRequest(
9187
{
9288
method: 'GET',
@@ -131,7 +127,6 @@ describe('routeBackbeat', () => {
131127
target: `${bucketName}/${objectName}`,
132128
operation: null,
133129
versionId: false,
134-
// not sending any body here, so expect error
135130
expect: errors.MalformedPOSTRequest,
136131
},
137132
{

0 commit comments

Comments
 (0)