Skip to content

Commit e509d2c

Browse files
dvasilasdvasilas
committed
CLDSRV-815: Use nullish coalescing for null field omission
Replace omitNullFields function with nullish coalescing operator to leverage JSON.stringify's native undefined-field omission. This eliminates an object iteration per request while maintaining the same output behavior.
1 parent 1b925c2 commit e509d2c

File tree

2 files changed

+43
-126
lines changed

2 files changed

+43
-126
lines changed

‎lib/utilities/serverAccessLogger.js‎

Lines changed: 43 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -391,19 +391,6 @@ function timestampToDateTime643(unixMS) {
391391
return (unixMS / 1000).toFixed(3);
392392
}
393393

394-
/**
395-
* Omits null and undefined fields from an object.
396-
* Only processes own properties (not inherited).
397-
* Preserves all other falsy values (0, false, "", etc.)
398-
* @param {Object} obj - Object to filter
399-
* @returns {Object} - New object with null/undefined fields removed
400-
*/
401-
function omitNullFields(obj) {
402-
return Object.fromEntries(
403-
Object.entries(obj).filter(([, value]) => value !== null && value !== undefined)
404-
);
405-
}
406-
407394
function logServerAccess(req, res) {
408395
if (!req.serverAccessLog || !res.serverAccessLog || !serverAccessLogger) {
409396
return;
@@ -416,68 +403,68 @@ function logServerAccess(req, res) {
416403
const bytesSent = res.serverAccessLog.bytesSent;
417404
const authInfo = params.authInfo;
418405

419-
serverAccessLogger.write(`${JSON.stringify(omitNullFields({
406+
serverAccessLogger.write(`${JSON.stringify({
420407
// Werelog fields
421408
// logs.access_logs_ingest.timestamp in Clickhouse is of type DateTime so it expects seconds as an integer.
422409
time: Math.floor(Date.now() / 1000),
423410
hostname,
424411
pid: process.pid,
425412
426413
// Analytics
427-
action: params.analyticsAction === undefined ? null : params.analyticsAction,
428-
accountName: params.analyticsAccountName === undefined ? null : params.analyticsAccountName,
429-
userName: params.analyticsUserName === undefined ? null : params.analyticsUserName,
430-
httpMethod: req.method === undefined ? null : req.method,
431-
bytesDeleted: params.analyticsBytesDeleted === undefined ? null : params.analyticsBytesDeleted,
432-
bytesReceived: req.parsedContentLength === undefined ? null : req.parsedContentLength,
433-
bodyLength: req.headers['content-length'] === undefined ? null : parseInt(req.headers['content-length'], 10),
434-
contentLength: req.parsedContentLength === undefined ? null : req.parsedContentLength,
414+
action: params.analyticsAction ?? undefined,
415+
accountName: params.analyticsAccountName ?? undefined,
416+
userName: params.analyticsUserName ?? undefined,
417+
httpMethod: req.method ?? undefined,
418+
bytesDeleted: params.analyticsBytesDeleted ?? undefined,
419+
bytesReceived: req.parsedContentLength ?? undefined,
420+
bodyLength: req.headers['content-length'] !== undefined ? parseInt(req.headers['content-length'], 10) : undefined,
421+
contentLength: req.parsedContentLength ?? undefined,
435422
// eslint-disable-next-line camelcase
436-
elapsed_ms: calculateElapsedMS(params.startTime, params.onCloseEndTime),
423+
elapsed_ms: calculateElapsedMS(params.startTime, params.onCloseEndTime) ?? undefined,
437424
438425
// AWS access server logs fields https://docs.aws.amazon.com/AmazonS3/latest/userguide/LogFormat.html
439-
startTime: timestampToDateTime643(params.startTimeUnixMS), // AWS "Time" field
440-
requester: getRequester(authInfo),
426+
startTime: timestampToDateTime643(params.startTimeUnixMS) ?? undefined, // AWS "Time" field
427+
requester: getRequester(authInfo) ?? undefined,
441428
operation: getOperation(req),
442-
requestURI: getURI(req),
443-
errorCode: errorCode === undefined ? null : errorCode,
444-
objectSize: getObjectSize(req, res),
445-
totalTime: calculateTotalTime(params.startTime, params.onFinishEndTime),
446-
turnAroundTime: calculateTurnAroundTime(params.startTurnAroundTime, endTurnAroundTime),
447-
referer: req.headers.referer === undefined ? null : req.headers.referer,
448-
userAgent: req.headers['user-agent'] === undefined ? null : req.headers['user-agent'],
449-
versionID: !req.query ? null : req.query.versionId === undefined ? null : req.query.versionId,
450-
signatureVersion: authInfo ? authInfo.getAuthVersion() : null,
451-
cipherSuite: req.socket.encrypted ? req.socket.getCipher()['standardName'] : null,
452-
authenticationType: authInfo ? authInfo.getAuthType() : null,
453-
hostHeader: req.headers.host === undefined ? null : req.headers.host,
454-
tlsVersion: req.socket.encrypted ? req.socket.getCipher()['version'] : null,
455-
aclRequired: null, // TODO: CLDSRV-774
456-
// hostID: null, // NOT IMPLEMENTED
457-
// accessPointARN: null, // NOT IMPLEMENTED
429+
requestURI: getURI(req) ?? undefined,
430+
errorCode: errorCode ?? undefined,
431+
objectSize: getObjectSize(req, res) ?? undefined,
432+
totalTime: calculateTotalTime(params.startTime, params.onFinishEndTime) ?? undefined,
433+
turnAroundTime: calculateTurnAroundTime(params.startTurnAroundTime, endTurnAroundTime) ?? undefined,
434+
referer: req.headers.referer ?? undefined,
435+
userAgent: req.headers['user-agent'] ?? undefined,
436+
versionID: req.query?.versionId ?? undefined,
437+
signatureVersion: authInfo?.getAuthVersion() ?? undefined,
438+
cipherSuite: req.socket.encrypted ? req.socket.getCipher()['standardName'] : undefined,
439+
authenticationType: authInfo?.getAuthType() ?? undefined,
440+
hostHeader: req.headers.host ?? undefined,
441+
tlsVersion: req.socket.encrypted ? req.socket.getCipher()['version'] : undefined,
442+
aclRequired: undefined, // TODO: CLDSRV-774
443+
// hostID: undefined, // NOT IMPLEMENTED
444+
// accessPointARN: undefined, // NOT IMPLEMENTED
458445
459446
// Shared between AWS access server logs and Analytics logs
460-
bucketOwner: params.bucketOwner === undefined ? null : params.bucketOwner,
461-
bucketName: params.bucketName === undefined ? null : params.bucketName, // AWS "Bucket" field
447+
bucketOwner: params.bucketOwner ?? undefined,
448+
bucketName: params.bucketName ?? undefined, // AWS "Bucket" field
462449
// eslint-disable-next-line camelcase
463-
req_id: requestID === undefined ? null : requestID, // AWS "Request ID" field
464-
bytesSent: getBytesSent(res, bytesSent),
465-
clientIP: getRemoteIPFromRequest(req), // AWS 'Remote IP' field
466-
httpCode: res.statusCode === undefined ? null : res.statusCode, // AWS "HTTP Status" field
467-
objectKey: params.objectKey === undefined ? null : params.objectKey, // AWS "Key" field
450+
req_id: requestID ?? undefined, // AWS "Request ID" field
451+
bytesSent: getBytesSent(res, bytesSent) ?? undefined,
452+
clientIP: getRemoteIPFromRequest(req) ?? undefined, // AWS 'Remote IP' field
453+
httpCode: res.statusCode ?? undefined, // AWS "HTTP Status" field
454+
objectKey: params.objectKey ?? undefined, // AWS "Key" field
468455
469456
// Scality server access logs extra fields
470457
logFormatVersion: SERVER_ACCESS_LOG_FORMAT_VERSION,
471-
loggingEnabled: params.enabled,
472-
loggingTargetBucket: params.loggingEnabled ? params.loggingEnabled.TargetBucket : null,
473-
loggingTargetPrefix: params.loggingEnabled ? params.loggingEnabled.TargetPrefix : null,
474-
awsAccessKeyID: authInfo ? authInfo.getAccessKey() : null,
475-
raftSessionID: params.raftSessionID === undefined ? null : params.raftSessionID,
458+
loggingEnabled: params.enabled ?? undefined,
459+
loggingTargetBucket: params.loggingEnabled?.TargetBucket ?? undefined,
460+
loggingTargetPrefix: params.loggingEnabled?.TargetPrefix ?? undefined,
461+
awsAccessKeyID: authInfo?.getAccessKey() ?? undefined,
462+
raftSessionID: params.raftSessionID ?? undefined,
476463
477464
// Rate Limiting fields (only present when rate limited)
478-
rateLimited: params.rateLimited,
479-
rateLimitSource: params.rateLimitSource,
480-
}))}\n`);
465+
rateLimited: params.rateLimited ?? undefined,
466+
rateLimitSource: params.rateLimitSource ?? undefined,
467+
})}\n`);
481468
}
482469

483470
module.exports = {
@@ -486,7 +473,6 @@ module.exports = {
486473
ServerAccessLogger,
487474

488475
// Exported for testing.
489-
omitNullFields,
490476
getRemoteIPFromRequest,
491477
getOperation,
492478
getRequester,

‎tests/unit/utils/serverAccessLogger.js‎

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ const sinon = require('sinon');
33
const {
44
logServerAccess,
55
setServerAccessLogger,
6-
omitNullFields,
76
getRemoteIPFromRequest,
87
getOperation,
98
getRequester,
@@ -15,74 +14,6 @@ const {
1514
timestampToDateTime643,
1615
} = require('../../../lib/utilities/serverAccessLogger');
1716

18-
describe('omitNullFields', () => {
19-
it('should omit null fields', () => {
20-
const input = { a: 1, b: null, c: 3 };
21-
const result = omitNullFields(input);
22-
assert.deepStrictEqual(result, { a: 1, c: 3 });
23-
});
24-
25-
it('should omit undefined fields', () => {
26-
const input = { a: 1, b: undefined, c: 3 };
27-
const result = omitNullFields(input);
28-
assert.deepStrictEqual(result, { a: 1, c: 3 });
29-
});
30-
31-
it('should handle mixed null and undefined', () => {
32-
const input = { a: 1, b: null, c: undefined, d: 4 };
33-
const result = omitNullFields(input);
34-
assert.deepStrictEqual(result, { a: 1, d: 4 });
35-
});
36-
37-
it('should preserve zero values', () => {
38-
const input = { a: 0, b: null };
39-
const result = omitNullFields(input);
40-
assert.deepStrictEqual(result, { a: 0 });
41-
});
42-
43-
it('should preserve false values', () => {
44-
const input = { a: false, b: null };
45-
const result = omitNullFields(input);
46-
assert.deepStrictEqual(result, { a: false });
47-
});
48-
49-
it('should preserve empty string values', () => {
50-
const input = { a: '', b: null };
51-
const result = omitNullFields(input);
52-
assert.deepStrictEqual(result, { a: '' });
53-
});
54-
55-
it('should preserve empty array values', () => {
56-
const input = { a: [], b: null };
57-
const result = omitNullFields(input);
58-
assert.deepStrictEqual(result, { a: [] });
59-
});
60-
61-
it('should preserve empty object values', () => {
62-
const input = { a: {}, b: null };
63-
const result = omitNullFields(input);
64-
assert.deepStrictEqual(result, { a: {} });
65-
});
66-
67-
it('should handle objects with all null fields', () => {
68-
const input = { a: null, b: null, c: null };
69-
const result = omitNullFields(input);
70-
assert.deepStrictEqual(result, {});
71-
});
72-
73-
it('should handle objects with no null fields', () => {
74-
const input = { a: 1, b: 2, c: 3 };
75-
const result = omitNullFields(input);
76-
assert.deepStrictEqual(result, { a: 1, b: 2, c: 3 });
77-
});
78-
79-
it('should handle empty objects', () => {
80-
const input = {};
81-
const result = omitNullFields(input);
82-
assert.deepStrictEqual(result, {});
83-
});
84-
});
85-
8617
describe('serverAccessLogger utility functions', () => {
8718
describe('getRemoteIPFromRequest', () => {
8819
it('should return IP from x-forwarded-for header', () => {

0 commit comments

Comments
 (0)