[squash] CLDSRV-815: Replace omitNullFields with JSON.stringify

dvasilas · dvasilas · commit 8b04327477dd · 2026-01-05T17:16:26.000+02:00
Replace omitNullFields function with nullish coalescing operator
to leverage JSON.stringify's omission of undefined fields.
diff --git a/lib/utilities/serverAccessLogger.js b/lib/utilities/serverAccessLogger.js
@@ -391,19 +391,6 @@ function timestampToDateTime643(unixMS) {
     return (unixMS / 1000).toFixed(3);
 }
 
-/**
- * Omits null and undefined fields from an object.
- * Only processes own properties (not inherited).
- * Preserves all other falsy values (0, false, "", etc.)
- * @param {Object} obj - Object to filter
- * @returns {Object} - New object with null/undefined fields removed
- */
-function omitNullFields(obj) {
-    return Object.fromEntries(
-        Object.entries(obj).filter(([, value]) => value !== null && value !== undefined)
-    );
-}
-
 function logServerAccess(req, res) {
     if (!req.serverAccessLog || !res.serverAccessLog || !serverAccessLogger) {
         return;
@@ -416,68 +403,70 @@ function logServerAccess(req, res) {
     const bytesSent = res.serverAccessLog.bytesSent;
     const authInfo = params.authInfo;
 
-    serverAccessLogger.write(`${JSON.stringify(omitNullFields({
+    serverAccessLogger.write(`${JSON.stringify({
         // Werelog fields
         // logs.access_logs_ingest.timestamp in Clickhouse is of type DateTime so it expects seconds as an integer.
         time: Math.floor(Date.now() / 1000),
         hostname,
         pid: process.pid,
 
         // Analytics
-        action: params.analyticsAction === undefined ? null : params.analyticsAction,
-        accountName: params.analyticsAccountName === undefined ? null : params.analyticsAccountName,
-        userName: params.analyticsUserName === undefined ? null : params.analyticsUserName,
-        httpMethod: req.method === undefined ? null : req.method,
-        bytesDeleted: params.analyticsBytesDeleted === undefined ? null : params.analyticsBytesDeleted,
-        bytesReceived: req.parsedContentLength === undefined ? null : req.parsedContentLength,
-        bodyLength: req.headers['content-length'] === undefined ? null : parseInt(req.headers['content-length'], 10),
-        contentLength: req.parsedContentLength === undefined ? null : req.parsedContentLength,
+        action: params.analyticsAction ?? undefined,
+        accountName: params.analyticsAccountName ?? undefined,
+        userName: params.analyticsUserName ?? undefined,
+        httpMethod: req.method ?? undefined,
+        bytesDeleted: params.analyticsBytesDeleted ?? undefined,
+        bytesReceived: req.parsedContentLength ?? undefined,
+        bodyLength: req.headers['content-length'] !== undefined
+            ? parseInt(req.headers['content-length'], 10)
+            : undefined,
+        contentLength: req.parsedContentLength ?? undefined,
         // eslint-disable-next-line camelcase
-        elapsed_ms: calculateElapsedMS(params.startTime, params.onCloseEndTime),
+        elapsed_ms: calculateElapsedMS(params.startTime, params.onCloseEndTime) ?? undefined,
 
         // AWS access server logs fields https://docs.aws.amazon.com/AmazonS3/latest/userguide/LogFormat.html
-        startTime: timestampToDateTime643(params.startTimeUnixMS), // AWS "Time" field
-        requester: getRequester(authInfo),
+        startTime: timestampToDateTime643(params.startTimeUnixMS) ?? undefined, // AWS "Time" field
+        requester: getRequester(authInfo) ?? undefined,
         operation: getOperation(req),
-        requestURI: getURI(req),
-        errorCode: errorCode === undefined ? null : errorCode,
-        objectSize: getObjectSize(req, res),
-        totalTime: calculateTotalTime(params.startTime, params.onFinishEndTime),
-        turnAroundTime: calculateTurnAroundTime(params.startTurnAroundTime, endTurnAroundTime),
-        referer: req.headers.referer === undefined ? null : req.headers.referer,
-        userAgent: req.headers['user-agent'] === undefined ? null : req.headers['user-agent'],
-        versionID: !req.query ? null : req.query.versionId === undefined ? null : req.query.versionId,
-        signatureVersion: authInfo ? authInfo.getAuthVersion() : null,
-        cipherSuite: req.socket.encrypted ? req.socket.getCipher()['standardName'] : null,
-        authenticationType: authInfo ? authInfo.getAuthType() : null,
-        hostHeader: req.headers.host === undefined ? null : req.headers.host,
-        tlsVersion: req.socket.encrypted ? req.socket.getCipher()['version'] : null,
-        aclRequired: null,       // TODO: CLDSRV-774
-        // hostID: null,         // NOT IMPLEMENTED
-        // accessPointARN: null, // NOT IMPLEMENTED
+        requestURI: getURI(req) ?? undefined,
+        errorCode: errorCode ?? undefined,
+        objectSize: getObjectSize(req, res) ?? undefined,
+        totalTime: calculateTotalTime(params.startTime, params.onFinishEndTime) ?? undefined,
+        turnAroundTime: calculateTurnAroundTime(params.startTurnAroundTime, endTurnAroundTime) ?? undefined,
+        referer: req.headers.referer ?? undefined,
+        userAgent: req.headers['user-agent'] ?? undefined,
+        versionID: req.query?.versionId ?? undefined,
+        signatureVersion: authInfo?.getAuthVersion() ?? undefined,
+        cipherSuite: req.socket.encrypted ? req.socket.getCipher()['standardName'] : undefined,
+        authenticationType: authInfo?.getAuthType() ?? undefined,
+        hostHeader: req.headers.host ?? undefined,
+        tlsVersion: req.socket.encrypted ? req.socket.getCipher()['version'] : undefined,
+        aclRequired: undefined,  // TODO: CLDSRV-774
+        // hostID: undefined,         // NOT IMPLEMENTED
+        // accessPointARN: undefined, // NOT IMPLEMENTED
 
         // Shared between AWS access server logs and Analytics logs
-        bucketOwner: params.bucketOwner === undefined ? null : params.bucketOwner,
-        bucketName: params.bucketName === undefined ? null : params.bucketName, // AWS "Bucket" field
+        bucketOwner: params.bucketOwner ?? undefined,
+        bucketName: params.bucketName ?? undefined, // AWS "Bucket" field
         // eslint-disable-next-line camelcase
-        req_id: requestID === undefined ? null : requestID, // AWS "Request ID" field
-        bytesSent: getBytesSent(res, bytesSent),
-        clientIP: getRemoteIPFromRequest(req),  // AWS 'Remote IP' field
-        httpCode: res.statusCode === undefined ? null : res.statusCode, // AWS "HTTP Status" field
-        objectKey: params.objectKey === undefined ? null : params.objectKey, // AWS "Key" field
+        req_id: requestID ?? undefined, // AWS "Request ID" field
+        bytesSent: getBytesSent(res, bytesSent) ?? undefined,
+        clientIP: getRemoteIPFromRequest(req) ?? undefined,  // AWS 'Remote IP' field
+        httpCode: res.statusCode ?? undefined, // AWS "HTTP Status" field
+        objectKey: params.objectKey ?? undefined, // AWS "Key" field
 
         // Scality server access logs extra fields
         logFormatVersion: SERVER_ACCESS_LOG_FORMAT_VERSION,
-        loggingEnabled: params.enabled,
-        loggingTargetBucket: params.loggingEnabled ? params.loggingEnabled.TargetBucket : null,
-        loggingTargetPrefix: params.loggingEnabled ? params.loggingEnabled.TargetPrefix : null,
-        awsAccessKeyID: authInfo ? authInfo.getAccessKey() : null,
-        raftSessionID: params.raftSessionID === undefined ? null : params.raftSessionID,
+        loggingEnabled: params.enabled ?? undefined,
+        loggingTargetBucket: params.loggingEnabled?.TargetBucket ?? undefined,
+        loggingTargetPrefix: params.loggingEnabled?.TargetPrefix ?? undefined,
+        awsAccessKeyID: authInfo?.getAccessKey() ?? undefined,
+        raftSessionID: params.raftSessionID ?? undefined,
 
         // Rate Limiting fields (only present when rate limited)
-        rateLimited: params.rateLimited,
-        rateLimitSource: params.rateLimitSource,
-    }))}\n`);
+        rateLimited: params.rateLimited ?? undefined,
+        rateLimitSource: params.rateLimitSource ?? undefined,
+    })}\n`);
 }
 
 module.exports = {
@@ -486,7 +475,6 @@ module.exports = {
     ServerAccessLogger,
 
     // Exported for testing.
-    omitNullFields,
     getRemoteIPFromRequest,
     getOperation,
     getRequester,
diff --git a/tests/unit/utils/serverAccessLogger.js b/tests/unit/utils/serverAccessLogger.js
@@ -3,7 +3,6 @@ const sinon = require('sinon');
 const {
     logServerAccess,
     setServerAccessLogger,
-    omitNullFields,
     getRemoteIPFromRequest,
     getOperation,
     getRequester,
@@ -15,74 +14,6 @@ const {
     timestampToDateTime643,
 } = require('../../../lib/utilities/serverAccessLogger');
 
-describe('omitNullFields', () => {
-    it('should omit null fields', () => {
-        const input = { a: 1, b: null, c: 3 };
-        const result = omitNullFields(input);
-        assert.deepStrictEqual(result, { a: 1, c: 3 });
-    });
-
-    it('should omit undefined fields', () => {
-        const input = { a: 1, b: undefined, c: 3 };
-        const result = omitNullFields(input);
-        assert.deepStrictEqual(result, { a: 1, c: 3 });
-    });
-
-    it('should handle mixed null and undefined', () => {
-        const input = { a: 1, b: null, c: undefined, d: 4 };
-        const result = omitNullFields(input);
-        assert.deepStrictEqual(result, { a: 1, d: 4 });
-    });
-
-    it('should preserve zero values', () => {
-        const input = { a: 0, b: null };
-        const result = omitNullFields(input);
-        assert.deepStrictEqual(result, { a: 0 });
-    });
-
-    it('should preserve false values', () => {
-        const input = { a: false, b: null };
-        const result = omitNullFields(input);
-        assert.deepStrictEqual(result, { a: false });
-    });
-
-    it('should preserve empty string values', () => {
-        const input = { a: '', b: null };
-        const result = omitNullFields(input);
-        assert.deepStrictEqual(result, { a: '' });
-    });
-
-    it('should preserve empty array values', () => {
-        const input = { a: [], b: null };
-        const result = omitNullFields(input);
-        assert.deepStrictEqual(result, { a: [] });
-    });
-
-    it('should preserve empty object values', () => {
-        const input = { a: {}, b: null };
-        const result = omitNullFields(input);
-        assert.deepStrictEqual(result, { a: {} });
-    });
-
-    it('should handle objects with all null fields', () => {
-        const input = { a: null, b: null, c: null };
-        const result = omitNullFields(input);
-        assert.deepStrictEqual(result, {});
-    });
-
-    it('should handle objects with no null fields', () => {
-        const input = { a: 1, b: 2, c: 3 };
-        const result = omitNullFields(input);
-        assert.deepStrictEqual(result, { a: 1, b: 2, c: 3 });
-    });
-
-    it('should handle empty objects', () => {
-        const input = {};
-        const result = omitNullFields(input);
-        assert.deepStrictEqual(result, {});
-    });
-});
-
 describe('serverAccessLogger utility functions', () => {
     describe('getRemoteIPFromRequest', () => {
         it('should return IP from x-forwarded-for header', () => {
