handle timezone

vatesfr · Nov 8, 2024 · e9e8ece · e9e8ece
1 parent 82e946e
commit e9e8ece
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 36 deletions.
diff --git a/@xen-orchestra/backups/_getOldEntries.mjs b/@xen-orchestra/backups/_getOldEntries.mjs
@@ -1,36 +1,50 @@
+import moment from 'moment-timezone'
+import assert from 'node:assert'
+
+function instantiateTimezonedDateCreator(timezone) {
+  return date => (timezone ? moment.tz(date, timezone) : moment(date))
+}
+
 const LTR_DEFINITIONS = {
   daily: {
-    makeDateFormatter: ({ firstHourOfTheDay = 0 } = {}) => {
+    makeDateFormatter: ({ firstHourOfTheDay = 0, dateCreator } = {}) => {
       return date => {
-        const copy = new Date(date)
-        copy.setHours(copy.getHours() - firstHourOfTheDay)
-        return `${copy.getFullYear()}-${copy.getMonth()}-${copy.getDate()}`
+        const copy = dateCreator(date)
+        copy.hour(copy.hour() - firstHourOfTheDay)
+        return copy.format('YYYY-MM-DD')
       }
     },
   },
   weekly: {
-    makeDateFormatter: ({ firstDayOfWeek = 1 /* sunday is 0 , let's use monday as default instead */ } = {}) => {
+    makeDateFormatter: ({ firstDayOfWeek = 0 /* relative to timezone week start */, dateCreator } = {}) => {
       return date => {
-        const copy = new Date(date)
-        copy.setDate(date.getDate() - ((date.getDay() + 7 - firstDayOfWeek) % 7))
-        return `${copy.getFullYear()}-${copy.getMonth()}-${copy.getDate()}`
+        const copy = dateCreator(date)
+
+        copy.date(date.date() - firstDayOfWeek)
+        // warning, the year in term of week may different from YYYY
+        // since the computation of the first week of a year is timezone dependant
+        return copy.format('gggg-WW')
       }
     },
     ancestor: 'daily',
   },
   monthly: {
-    makeDateFormatter: ({ firstDayOfMonth = 0 } = {}) => {
+    makeDateFormatter: ({ firstDayOfMonth = 0, dateCreator } = {}) => {
       return date => {
-        const copy = new Date(date)
-        copy.setDate(copy.getDate() - firstDayOfMonth)
-        return `${copy.getFullYear()}-${copy.getMonth()}`
+        const copy = dateCreator(date)
+        copy.date(copy.date() - firstDayOfMonth)
+        return copy.format('YYYY-MM')
       }
     },
     ancestor: 'weekly',
   },
   yearly: {
-    makeDateFormatter: () => {
-      return date => `${date.getFullYear()}`
+    makeDateFormatter: ({ firstDayOfYear = 0, dateCreator } = {}) => {
+      return date => {
+        const copy = dateCreator(date)
+        copy.date(copy.date() - firstDayOfYear)
+        return copy.format('YYYY')
+      }
     },
     ancestor: 'monthly',
   },
@@ -44,31 +58,40 @@ const LTR_DEFINITIONS = {
  *  if a bucket is cmpletly empty : it does not count as one, thus it may extend the retention
  * @returns Array<Backup>
  */
-export function getOldEntries(minRetentionCount, entries, { longTermRetention = {} } = {}) {
+export function getOldEntries(minRetentionCount, entries, { longTermRetention = {}, timezone } = {}) {
   const dateBuckets = {}
+  const dateCreator = instantiateTimezonedDateCreator(timezone)
+  // only check buckets that have a retention set
   for (const [duration, { retention, settings }] of Object.entries(longTermRetention)) {
     if (LTR_DEFINITIONS[duration] === undefined) {
       throw new Error(`Retention of type ${retention} is invalid`)
     }
     dateBuckets[duration] = {
       remaining: retention,
       lastMatchingBucket: null,
-      formatter: LTR_DEFINITIONS[duration].makeDateFormatter(settings),
+      formatter: LTR_DEFINITIONS[duration].makeDateFormatter({ ...settings, dateCreator }),
     }
   }
   const nb = entries.length
   const oldEntries = []
 
   for (let i = nb - 1; i >= 0; i--) {
     const entry = entries[i]
-    const entryDate = new Date(entry.timestamp)
+    const entryDate = dateCreator(entry.timestamp)
     let shouldBeKept = false
     for (const [duration, { remaining, lastMatchingBucket, formatter }] of Object.entries(dateBuckets)) {
       if (remaining === 0) {
         continue
       }
       const bucket = formatter(entryDate)
       if (lastMatchingBucket !== bucket) {
+        if (lastMatchingBucket !== null) {
+          assert.strictEqual(
+            lastMatchingBucket > bucket,
+            true,
+            `entries must be sorted in asc order ${lastMatchingBucket} ${bucket}`
+          )
+        }
         shouldBeKept = true
         dateBuckets[duration].remaining -= 1
         dateBuckets[duration].lastMatchingBucket = bucket

diff --git a/@xen-orchestra/backups/_getOldEntries.test.mjs b/@xen-orchestra/backups/_getOldEntries.test.mjs
@@ -58,6 +58,7 @@ describe('_getOldEntries() should succeed', () => {
           longTermRetention: {
             weekly: { retention: 3 },
           },
+          timezone: 'Europe/Paris',
         },
       ],
       expectedIds: [1, 2, 3, 4, 5, 6, 8],
@@ -78,11 +79,11 @@ describe('_getOldEntries() should succeed', () => {
         ],
         {
           longTermRetention: {
-            weekly: { retention: 3 },
+            monthly: { retention: 3 },
           },
         },
       ],
-      expectedIds: [1, 2, 3, 4, 6],
+      expectedIds: [1, 3, 4, 6, 7],
       testLabel: 'should  handle month based retention',
     },
     {
@@ -91,38 +92,69 @@ describe('_getOldEntries() should succeed', () => {
         [
           { timestamp: +new Date('2023-05-18 00:09:00'), id: 1 }, // too old
           { timestamp: +new Date('2024-06-15 00:09:00'), id: 2 }, // too old in same year
-          { timestamp: +new Date('2024-07-04 00:09:00'), id: 3 },
-          { timestamp: +new Date('2024-08-12 00:09:00'), id: 4 },
-          { timestamp: +new Date('2024-09-05 00:09:00'), id: 5 },
-          { timestamp: +new Date('2024-10-02 00:09:00'), id: 6 },
-          { timestamp: +new Date('2024-11-01 00:09:00'), id: 7 },
-          { timestamp: +new Date('2024-12-17 00:09:00'), id: 8 },
-          { timestamp: +new Date('2024-12-24 00:09:00'), id: 10 },
-          { timestamp: +new Date('2025-12-31 00:09:00'), id: 11 }, // same day/week/month/year
-          { timestamp: +new Date('2025-12-31 00:09:00'), id: 12 }, // new month /year
-          { timestamp: +new Date('2025-01-01 00:09:00'), id: 13 }, // same day/week/month/year
-          { timestamp: +new Date('2025-01-01 00:10:00'), id: 14 }, //  new year /
+          { timestamp: +new Date('2024-07-04 00:09:00'), id: 3 }, // too old
+          { timestamp: +new Date('2024-08-12 00:09:00'), id: 4 }, // too old
+          { timestamp: +new Date('2024-09-05 00:09:00'), id: 5 }, // too old
+          { timestamp: +new Date('2024-10-02 00:09:00'), id: 6 }, // new month,
+          { timestamp: +new Date('2024-11-01 00:09:00'), id: 7 }, // new month , week reached retention
+          { timestamp: +new Date('2024-12-17 00:09:00'), id: 8 }, // new week
+          { timestamp: +new Date('2024-12-24 00:09:00'), id: 9 }, // new week/month / year  daily reach retention
+          { timestamp: +new Date('2025-01-01 00:09:00'), id: 10 }, // same day/week/month/year
+          { timestamp: +new Date('2025-01-01 00:10:00'), id: 11 }, //  new  day / week / month
+          { timestamp: +new Date('2025-12-31 00:09:00'), id: 12 }, // same day/week/month/year
+          { timestamp: +new Date('2025-12-31 00:09:00'), id: 13 }, // new month /year
         ],
         {
           longTermRetention: {
             daily: { retention: 2 },
             weekly: { retention: 4 },
-            monthly: { retention: 8 },
+            monthly: { retention: 5 },
             yearly: { retention: 2 },
           },
+          timezone: 'Europe/Paris', // use a time zone here because week definition is timezone bound
         },
       ],
-      expectedIds: [1, 2, 11, 13],
+      expectedIds: [1, 2, 3, 4, 5, 10, 12],
       testLabel: 'complete test  ',
     },
+    {
+      args: [
+        0,
+        [
+          { timestamp: +new Date('2024-09-01 00:01:00'), id: 1 }, // thrid day too old
+          { timestamp: +new Date('2024-09-01 00:10:00'), id: 2 }, // thrid day
+          { timestamp: +new Date('2024-09-01 00:20:00'), id: 3 }, // second day, too old
+          { timestamp: +new Date('2024-09-02 00:22:00'), id: 4 }, // second day too old
+          { timestamp: +new Date('2024-09-03 00:20:00'), id: 5 }, // second day in NZ
+          { timestamp: +new Date('2024-09-04 00:09:00'), id: 6 }, // same day in NZ
+          { timestamp: +new Date('2024-09-04 00:10:00'), id: 7 }, // most recent
+        ],
+        {
+          longTermRetention: {
+            daily: { retention: 3 },
+          },
+          timezone: 'Pacific/Auckland', // GMT +6
+        },
+      ],
+      expectedIds: [1, 2, 3, 6],
+      testLabel: 'should  handle timezone ',
+    },
   ]
 
   for (const { args, expectedIds, testLabel } of tests) {
     it(testLabel, () => {
       const oldEntries = getOldEntries.apply(null, args)
-      assert.strictEqual(oldEntries.length, expectedIds.length, 'different length')
+      assert.strictEqual(
+        oldEntries.length,
+        expectedIds.length,
+        `different length , ${JSON.stringify({ oldEntries, expectedIds })}`
+      )
       for (let i = 0; i < expectedIds.length; i++) {
-        assert.strictEqual(oldEntries[i].id, expectedIds[i])
+        assert.strictEqual(
+          oldEntries[i].id,
+          expectedIds[i],
+          `different id , ${JSON.stringify({ i, expectedIds, oldEntries })}`
+        )
       }
     })
   }

diff --git a/@xen-orchestra/backups/_runners/_writers/FullRemoteWriter.mjs b/@xen-orchestra/backups/_runners/_writers/FullRemoteWriter.mjs
@@ -39,7 +39,7 @@ export class FullRemoteWriter extends MixinRemoteWriter(AbstractFullWriter) {
     const oldBackups = getOldEntries(
       settings.exportRetention - 1,
       await adapter.listVmBackups(vm.uuid, _ => _.mode === 'full' && _.scheduleId === scheduleId),
-      { longTermRetention: settings.longTermRetention }
+      { longTermRetention: settings.longTermRetention, timezone: settings.timezone }
     )
     const deleteOldBackups = () => adapter.deleteFullVmBackups(oldBackups)
 

diff --git a/@xen-orchestra/backups/_runners/_writers/IncrementalRemoteWriter.mjs b/@xen-orchestra/backups/_runners/_writers/IncrementalRemoteWriter.mjs
@@ -96,7 +96,7 @@ export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrement
     const oldEntries = getOldEntries(
       settings.exportRetention - 1,
       await adapter.listVmBackups(vmUuid, _ => _.mode === 'delta' && _.scheduleId === scheduleId),
-      { longTermRetention: settings.longTermRetention }
+      { longTermRetention: settings.longTermRetention, timezone: settings.timezone }
     )
     this._oldEntries = oldEntries
 

diff --git a/@xen-orchestra/backups/package.json b/@xen-orchestra/backups/package.json
@@ -39,6 +39,7 @@
     "human-format": "^1.2.0",
     "limit-concurrency-decorator": "^0.6.0",
     "lodash": "^4.17.20",
+    "moment-timezone": "^0.5.46",
     "ms": "^2.1.3",
     "node-zone": "^0.4.0",
     "parse-pairs": "^2.0.0",