♻️ factorise logic in get and head lib/route/veeam

DarkIsDude · DarkIsDude · commit 48e4d3e0b027 · 2026-04-01T11:41:29.000+02:00
Issue: CLDSRV-878
diff --git a/lib/routes/veeam/get.js b/lib/routes/veeam/get.js
@@ -1,7 +1,5 @@
-const xml2js = require('xml2js');
 const { errors } = require('arsenal');
-const metadata = require('../../metadata/wrapper');
-const { respondWithData, buildHeadXML, getFileToBuild, isSystemXML, fetchCapacityMetrics } = require('./utils');
+const { respondWithData, buildHeadXML, buildVeeamFileData } = require('./utils');
 const { responseXMLBody } = require('arsenal/build/lib/s3routes/routesUtils');
 
 /**
@@ -17,51 +15,18 @@ function getVeeamFile(request, response, bucketMd, log) {
     if (!bucketMd) {
         return responseXMLBody(errors.NoSuchBucket, null, response, log);
     }
+
     if ('tagging' in request.query) {
         return respondWithData(request, response, log, bucketMd,
             buildHeadXML('<Tagging><TagSet></TagSet></Tagging>'));
     }
-    return metadata.getBucket(request.bucketName, log, (err, data) => {
+
+    return buildVeeamFileData(request, bucketMd, log, 'getVeeamFile', (err, result) => {
         if (err) {
-            return responseXMLBody(errors.InternalError, null, response, log);
+            return responseXMLBody(err, null, response, log);
         }
-        const finalizeRequest = bucketMetrics => {
-            const fileToBuild = getFileToBuild(request, data._capabilities?.VeeamSOSApi);
-            if (fileToBuild.error) {
-                return responseXMLBody(fileToBuild.error, null, response, log);
-            }
-
-            // Extract the last modified date, but do not include it when computing
-            // the file's ETag (md5)
-            const modified = bucketMetrics?.date || new Date();
-            delete fileToBuild.value.LastModified;
 
-            // The SOSAPI metrics are dynamically computed using the SUR backend.
-            if (bucketMetrics && !fileToBuild.value.CapacityInfo?.Used) {
-                fileToBuild.value.CapacityInfo.Used = Number(bucketMetrics.bytesTotal);
-                fileToBuild.value.CapacityInfo.Available =
-                    Number(fileToBuild.value.CapacityInfo.Capacity) - Number(bucketMetrics.bytesTotal);
-                // TODO CLDSRV-633 when SUR backend supports realtime metrics: it will
-                // report the real last cseq/date processed by SUR, instead of the current date,
-                // ensuring no issue in a SOSAPI context. We should use this information.
-            }
-
-            const builder = new xml2js.Builder({
-                headless: true,
-            });
-            return respondWithData(request, response, log, data,
-                buildHeadXML(builder.buildObject(fileToBuild.value)), modified);
-        };
-
-        if (!isSystemXML(request.objectKey)) {
-            return fetchCapacityMetrics(bucketMd, request, log, 'getVeeamFile', (err, bucketMetrics) => {
-                if (err) {
-                    return responseXMLBody(errors.InternalError, null, response, log);
-                }
-                return finalizeRequest(bucketMetrics);
-            });
-        }
-        return finalizeRequest();
+        return respondWithData(request, response, log, result.bucketData, result.xmlContent, result.modified);
     });
 }
 
diff --git a/lib/routes/veeam/head.js b/lib/routes/veeam/head.js
@@ -1,7 +1,5 @@
-const xml2js = require('xml2js');
 const { errors } = require('arsenal');
-const metadata = require('../../metadata/wrapper');
-const { getResponseHeader, buildHeadXML, getFileToBuild, isSystemXML, fetchCapacityMetrics } = require('./utils');
+const { getResponseHeader, buildVeeamFileData } = require('./utils');
 const { responseXMLBody, responseContentHeaders } = require('arsenal/build/lib/s3routes/routesUtils');
 
 /**
@@ -18,44 +16,18 @@ function headVeeamFile(request, response, bucketMd, log) {
         return responseXMLBody(errors.NoSuchBucket, null, response, log);
     }
 
-    return metadata.getBucket(request.bucketName, log, (err, data) => {
+    return buildVeeamFileData(request, bucketMd, log, 'headVeeamFile', (err, result) => {
         if (err) {
-            return responseXMLBody(errors.InternalError, null, response, log);
+            return responseXMLBody(err, null, response, log);
         }
 
-        const finalizeRequest = bucketMetrics => {
-            const fileToBuild = getFileToBuild(request, data._capabilities?.VeeamSOSApi);
-            if (fileToBuild.error) {
-                return responseXMLBody(fileToBuild.error, null, response, log);
-            }
-
-            // Extract the last modified date, but do not include it when computing the file's ETag (md5)
-            const modified = bucketMetrics?.date || new Date();
-            delete fileToBuild.value.LastModified;
-            const builder = new xml2js.Builder({
-                headless: true,
-            });
-            const dataBuffer = Buffer.from(buildHeadXML(builder.buildObject(fileToBuild)));
-
-            return responseContentHeaders(
-                null,
-                {},
-                getResponseHeader(request, data, dataBuffer, modified, log),
-                response,
-                log,
-            );
-        };
-
-        if (!isSystemXML(request.objectKey)) {
-            return fetchCapacityMetrics(bucketMd, request, log, 'headVeeamFile', (err, bucketMetrics) => {
-                if (err) {
-                    return responseXMLBody(errors.InternalError, null, response, log);
-                }
-                return finalizeRequest(bucketMetrics);
-            });
-        }
-
-        return finalizeRequest();
+        return responseContentHeaders(
+            null,
+            {},
+            getResponseHeader(request, result.bucketData, result.dataBuffer, result.modified, log),
+            response,
+            log,
+        );
     });
 }
 
diff --git a/lib/routes/veeam/list.js b/lib/routes/veeam/list.js
@@ -87,11 +87,13 @@ function listVeeamFiles(request, response, bucketMd, log) {
     if (!bucketMd) {
         return responseXMLBody(errors.NoSuchBucket, null, response, log);
     }
+
     // Only accept list-type query parameter
     if (!('list-type' in request.query) && !('versions' in request.query)) {
         return responseXMLBody(errorInstances.InvalidRequest
             .customizeDescription('The Veeam folder does not support this action.'), null, response, log);
     }
+
     return metadata.getBucket(request.bucketName, log, (err, data) => {
         if (err) {
             return responseXMLBody(errors.InternalError, null, response, log);
@@ -115,7 +117,7 @@ function listVeeamFiles(request, response, bucketMd, log) {
             fieldsToGenerate.forEach(file => {
                 const isCapacity = file.name.endsWith('capacity.xml');
                 const lastModified = isCapacity
-                    ? (bucketMetrics?.date || new Date())
+                    ? bucketMetrics.date
                     : file.LastModified;
                 // eslint-disable-next-line no-param-reassign
                 delete file.LastModified;
@@ -129,6 +131,7 @@ function listVeeamFiles(request, response, bucketMd, log) {
                     name: file.name,
                 });
             });
+
             // When `versions` is present, listing should return a versioned list
             return respondWithData(request, response, log, data,
                 buildXMLResponse(request, filesToBuild, 'versions' in request.query));
@@ -142,7 +145,8 @@ function listVeeamFiles(request, response, bucketMd, log) {
                 return buildAndRespond(bucketMetrics);
             });
         }
-        return buildAndRespond();
+
+        return buildAndRespond({ date: new Date() });
     });
 }
 
diff --git a/lib/routes/veeam/utils.js b/lib/routes/veeam/utils.js
@@ -1,10 +1,12 @@
+const xml2js = require('xml2js');
 const { errors, errorInstances, jsutil } = require('arsenal');
 const { Readable } = require('stream');
 const collectResponseHeaders = require('../../utilities/collectResponseHeaders');
 const collectCorsHeaders = require('../../utilities/collectCorsHeaders');
 const crypto = require('crypto');
 const { prepareStream } = require('arsenal/build/lib/s3middleware/prepareStream');
 const UtilizationService = require('../../utilization/instance');
+const metadata = require('../../metadata/wrapper');
 
 /**
  * Decodes an URI and return the result.
@@ -195,7 +197,6 @@ function getFileToBuild(request, data, inlineLastModified = false) {
         return {
             value: {
                 [fieldName]: fileToBuild,
-                LastModified: modified,
             },
             fieldName,
         };
@@ -204,14 +205,16 @@ function getFileToBuild(request, data, inlineLastModified = false) {
 
 /**
  * Fetches capacity metrics from UtilizationService for a bucket.
- * Handles 404 gracefully (no metrics available yet, e.g. post-install).
+ * Handles 404 gracefully (no metrics available yet, e.g. post-install),
+ * returning a default bucketMetrics with the current date so callers always
+ * receive a usable object.
  *
  * @param {object} bucketMd - bucket metadata
  * @param {object} request - incoming request
  * @param {object} log - logger object
  * @param {string} method - calling method name for log context
- * @param {function} callback - (err, bucketMetrics) where bucketMetrics
- *   is undefined when metrics are not available (404)
+ * @param {function} callback - (err, bucketMetrics) where bucketMetrics always
+ *   has at least a `date` field; on a real 404 the date defaults to new Date()
  * @returns {undefined}
  */
 function fetchCapacityMetrics(bucketMd, request, log, method, callback) {
@@ -225,7 +228,7 @@ function fetchCapacityMetrics(bucketMd, request, log, method, callback) {
                     bucket: request.bucketName,
                     error: err.message || err.code,
                 });
-                return callback(null);
+                return callback(null, { date: new Date() });
             }
             log.error('error fetching capacity metrics from UtilizationService', {
                 method,
@@ -239,6 +242,61 @@ function fetchCapacityMetrics(bucketMd, request, log, method, callback) {
     });
 }
 
+/**
+ * Builds Veeam file data (XML content + response metadata) for a given request.
+ *
+ * @param {object} request - incoming request
+ * @param {object} bucketMd - bucket metadata from the router
+ * @param {object} log - logger object
+ * @param {string} name - calling method name (for log context)
+ * @param {function} callback - (err, result) where result is { xmlContent, dataBuffer, modified, bucketData }
+ * @returns {undefined}
+ */
+function buildVeeamFileData(request, bucketMd, log, name, callback) {
+    return metadata.getBucket(request.bucketName, log, (err, data) => {
+        if (err) {
+            return callback(errors.InternalError);
+        }
+
+        const fileToBuild = getFileToBuild(request, data._capabilities?.VeeamSOSApi);
+
+        if (fileToBuild.error) {
+            return callback(fileToBuild.error);
+        }
+
+        const finalize = bucketMetrics => {
+            const modified = bucketMetrics.date;
+            if (
+                bucketMetrics.bytesTotal !== undefined
+                    && fileToBuild.value.CapacityInfo
+                    && !fileToBuild.value.CapacityInfo.Used
+            ) {
+                fileToBuild.value.CapacityInfo.Used = Number(bucketMetrics.bytesTotal);
+                fileToBuild.value.CapacityInfo.Available =
+                    Number(fileToBuild.value.CapacityInfo.Capacity) - Number(bucketMetrics.bytesTotal);
+                // TODO CLDSRV-633 when SUR backend supports realtime metrics: it will
+                // report the real last cseq/date processed by SUR, instead of the current date,
+                // ensuring no issue in a SOSAPI context. We should use this information.
+            }
+
+            const builder = new xml2js.Builder({ headless: true });
+            const xmlContent = buildHeadXML(builder.buildObject(fileToBuild.value));
+            const dataBuffer = Buffer.from(xmlContent);
+            return callback(null, { xmlContent, dataBuffer, modified, bucketData: data });
+        };
+
+        if (!isSystemXML(request.objectKey)) {
+            return fetchCapacityMetrics(bucketMd, request, log, name, (fetchErr, bucketMetrics) => {
+                if (fetchErr) {
+                    return callback(errors.InternalError);
+                }
+                return finalize(bucketMetrics);
+            });
+        }
+        return finalize({ date: new Date() });
+    });
+}
+
 module.exports = {
     _decodeURI,
     receiveData,
@@ -249,4 +307,5 @@ module.exports = {
     isSystemXML,
     getFileToBuild,
     fetchCapacityMetrics,
+    buildVeeamFileData,
 };
diff --git a/tests/unit/routes/veeam-utils.js b/tests/unit/routes/veeam-utils.js
@@ -57,14 +57,14 @@ describe('fetchCapacityMetrics', () => {
         });
     });
 
-    it('should call back with no error and no metrics on 404', done => {
+    it('should call back with no error and a default date on 404', done => {
         const error404 = new Error('Not Found');
         error404.response = { status: 404 };
         utilizationStub.callsArgWith(4, error404);
 
         fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => {
             assert.ifError(err);
-            assert.strictEqual(metrics, undefined);
+            assert(metrics && metrics.date instanceof Date, 'metrics should have a Date for date');
             assert(logWarnSpy.calledOnce);
             assert(logWarnSpy.getCall(0).args[0].includes('404'));
             assert.strictEqual(logWarnSpy.getCall(0).args[1].method, 'testMethod');
@@ -81,7 +81,7 @@ describe('fetchCapacityMetrics', () => {
 
         fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => {
             assert.ifError(err);
-            assert.strictEqual(metrics, undefined);
+            assert(metrics && metrics.date instanceof Date, 'metrics should have a Date for date');
             assert(logWarnSpy.calledOnce);
             done();
         });
