From e797fca15224094134a350600b80a5f865b2534d Mon Sep 17 00:00:00 2001 From: Tathagat Sharma Date: Thu, 20 Nov 2025 15:09:55 +0530 Subject: [PATCH 1/7] feat: implement strict 500 version limit for configurations --- .../configuration/configuration.collection.js | 52 +++- .../configuration.collection.test.js | 246 +++++++++++++++++- 2 files changed, 296 insertions(+), 2 deletions(-) diff --git a/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js b/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js index 4070c89de..29e8614ba 100755 --- a/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js +++ b/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js @@ -22,13 +22,63 @@ import BaseCollection from '../base/base.collection.js'; * @extends BaseCollection */ class ConfigurationCollection extends BaseCollection { + static MAX_VERSIONS = 500; + + static BATCH_SIZE = 25; + async create(data) { const latestConfiguration = await this.findLatest(); const version = latestConfiguration ? incrementVersion(latestConfiguration.getVersion()) : 1; const sanitizedData = sanitizeIdAndAuditFields('Organization', data); sanitizedData.version = version; - return super.create(sanitizedData); + const newConfig = await super.create(sanitizedData); + + // Enforce version limit asynchronously (fire-and-forget) + setImmediate(() => { + this.#enforceVersionLimit().catch((error) => { + this.log.error('Failed to enforce configuration version limit', error); + }); + }); + + return newConfig; + } + + async #enforceVersionLimit() { + const { MAX_VERSIONS, BATCH_SIZE } = ConfigurationCollection; + + try { + // Get all configuration versions, sorted newest first + const allConfigs = await this.all({}, { order: 'desc' }); + + // If within limit, nothing to do + if (allConfigs.length <= MAX_VERSIONS) { + this.log.debug(`Configuration version count within limit: ${allConfigs.length}/${MAX_VERSIONS}`); + return; + } + + // Calculate versions to delete (keep newest 500, delete the rest) + const versionsToDelete = allConfigs.slice(MAX_VERSIONS); + const deleteCount = versionsToDelete.length; + + this.log.info(`Enforcing configuration version limit: deleting ${deleteCount} old versions (current: ${allConfigs.length}, target: ${MAX_VERSIONS})`); + + // Delete in batches (DynamoDB batch write limit is 25) + for (let i = 0; i < versionsToDelete.length; i += BATCH_SIZE) { + const batch = versionsToDelete.slice(i, i + BATCH_SIZE); + const ids = batch.map((config) => config.getId()); + + // eslint-disable-next-line no-await-in-loop + await this.removeByIds(ids); + + this.log.debug(`Deleted batch ${Math.floor(i / BATCH_SIZE) + 1}/${Math.ceil(deleteCount / BATCH_SIZE)}: ${ids.length} versions`); + } + + this.log.info(`Configuration version limit enforced successfully. Deleted ${deleteCount} versions. Remaining: ${MAX_VERSIONS}`); + } catch (error) { + this.log.error('Configuration version limit enforcement failed', error); + // Don't throw - we don't want to fail the main operation + } } async findByVersion(version) { diff --git a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js index 8ebff370d..d5b9be5e6 100755 --- a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js +++ b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js @@ -14,7 +14,7 @@ import { expect, use as chaiUse } from 'chai'; import chaiAsPromised from 'chai-as-promised'; -import { stub } from 'sinon'; +import sinon, { stub } from 'sinon'; import sinonChai from 'sinon-chai'; import Configuration from '../../../../src/models/configuration/configuration.model.js'; @@ -117,4 +117,248 @@ describe('ConfigurationCollection', () => { expect(instance.findByAll).to.have.been.calledWithExactly({}, { order: 'desc' }); }); }); + + describe('version cleanup', () => { + describe('create with version limit enforcement', () => { + it('does not trigger cleanup when version count is within limit', async () => { + const latestConfiguration = { + getId: () => 's12345', + getVersion: () => 450, + }; + + instance.findLatest = stub().resolves(latestConfiguration); + instance.all = stub().resolves(new Array(451).fill(null).map((_, i) => ({ + getId: () => `config-${i}`, + getVersion: () => 451 - i, + }))); + instance.removeByIds = stub().resolves(); + + const result = await instance.create(mockRecord); + + expect(result).to.be.an('object'); + expect(result.getId()).to.equal(mockRecord.configurationId); + + // Wait a bit for async cleanup to potentially run + await new Promise((resolve) => { + setTimeout(resolve, 100); + }); + + // all should be called for cleanup check + expect(instance.all).to.have.been.called; + // removeByIds should NOT be called (within limit) + expect(instance.removeByIds).to.not.have.been.called; + }); + + it('triggers cleanup when version count is exactly 500', async () => { + const latestConfiguration = { + getId: () => 's12345', + getVersion: () => 499, + }; + + instance.findLatest = stub().resolves(latestConfiguration); + instance.all = stub().resolves(new Array(500).fill(null).map((_, i) => ({ + getId: () => `config-${500 - i}`, + getVersion: () => 500 - i, + }))); + instance.removeByIds = stub().resolves(); + + await instance.create(mockRecord); + + // Wait for async cleanup + await new Promise((resolve) => { + setTimeout(resolve, 100); + }); + + expect(instance.all).to.have.been.called; + // Should not delete anything (exactly at limit) + expect(instance.removeByIds).to.not.have.been.called; + }); + + it('triggers cleanup and deletes 1 version when count is 501', async () => { + const latestConfiguration = { + getId: () => 's12345', + getVersion: () => 500, + }; + + const mockConfigs = new Array(501).fill(null).map((_, i) => ({ + getId: () => `config-${501 - i}`, + getVersion: () => 501 - i, + })); + + instance.findLatest = stub().resolves(latestConfiguration); + instance.all = stub().resolves(mockConfigs); + instance.removeByIds = stub().resolves(); + + await instance.create(mockRecord); + + // Wait for async cleanup + await new Promise((resolve) => { + setTimeout(resolve, 100); + }); + + expect(instance.all).to.have.been.called; + expect(instance.removeByIds).to.have.been.calledOnce; + expect(instance.removeByIds).to.have.been.calledWith(['config-1']); + }); + + it('triggers cleanup and deletes multiple versions in batches', async () => { + const latestConfiguration = { + getId: () => 's12345', + getVersion: () => 549, + }; + + // Create 550 mock configurations + const mockConfigs = new Array(550).fill(null).map((_, i) => ({ + getId: () => `config-${550 - i}`, + getVersion: () => 550 - i, + })); + + instance.findLatest = stub().resolves(latestConfiguration); + instance.all = stub().resolves(mockConfigs); + instance.removeByIds = stub().resolves(); + + await instance.create(mockRecord); + + // Wait for async cleanup + await new Promise((resolve) => { + setTimeout(resolve, 100); + }); + + expect(instance.all).to.have.been.called; + // Should delete 50 versions (550 - 500 = 50) + // 50 versions / 25 batch size = 2 batches + expect(instance.removeByIds).to.have.been.calledTwice; + + // First batch: 25 items + const firstBatchCall = instance.removeByIds.getCall(0); + expect(firstBatchCall.args[0]).to.have.lengthOf(25); + + // Second batch: 25 items + const secondBatchCall = instance.removeByIds.getCall(1); + expect(secondBatchCall.args[0]).to.have.lengthOf(25); + }); + + it('handles large cleanup (delete 100 versions in 4 batches)', async () => { + const latestConfiguration = { + getId: () => 's12345', + getVersion: () => 599, + }; + + // Create 600 mock configurations + const mockConfigs = new Array(600).fill(null).map((_, i) => ({ + getId: () => `config-${600 - i}`, + getVersion: () => 600 - i, + })); + + instance.findLatest = stub().resolves(latestConfiguration); + instance.all = stub().resolves(mockConfigs); + instance.removeByIds = stub().resolves(); + + await instance.create(mockRecord); + + // Wait for async cleanup + await new Promise((resolve) => { + setTimeout(resolve, 150); + }); + + expect(instance.all).to.have.been.called; + // Should delete 100 versions (600 - 500 = 100) + // 100 versions / 25 batch size = 4 batches + expect(instance.removeByIds).to.have.callCount(4); + }); + + it('does not fail create operation if cleanup fails', async () => { + const latestConfiguration = { + getId: () => 's12345', + getVersion: () => 500, + }; + + const mockConfigs = new Array(501).fill(null).map((_, i) => ({ + getId: () => `config-${501 - i}`, + getVersion: () => 501 - i, + })); + + instance.findLatest = stub().resolves(latestConfiguration); + instance.all = stub().resolves(mockConfigs); + instance.removeByIds = stub().rejects(new Error('DynamoDB error')); + + // Create should succeed even if cleanup fails + const result = await instance.create(mockRecord); + + expect(result).to.be.an('object'); + expect(result.getId()).to.equal(mockRecord.configurationId); + + // Wait for async cleanup to fail + await new Promise((resolve) => { + setTimeout(resolve, 100); + }); + + expect(mockLogger.error).to.have.been.called; + }); + + it('handles all failure gracefully', async () => { + const latestConfiguration = { + getId: () => 's12345', + getVersion: () => 500, + }; + + instance.findLatest = stub().resolves(latestConfiguration); + instance.all = stub().rejects(new Error('Query failed')); + + // Create should succeed even if cleanup query fails + const result = await instance.create(mockRecord); + + expect(result).to.be.an('object'); + expect(result.getId()).to.equal(mockRecord.configurationId); + + // Wait for async cleanup to fail + await new Promise((resolve) => { + setTimeout(resolve, 100); + }); + + expect(mockLogger.error).to.have.been.called; + }); + + it('handles errors in cleanup error handler (outer catch)', async () => { + const latestConfiguration = { + getId: () => 's12345', + getVersion: () => 500, + }; + + instance.findLatest = stub().resolves(latestConfiguration); + instance.all = stub().rejects(new Error('Query failed')); + + // Make the logger.error inside #enforceVersionLimit throw + // This will cause the method to reject, triggering the outer catch + let errorCallCount = 0; + mockLogger.error = stub().callsFake(() => { + errorCallCount += 1; + if (errorCallCount === 1) { + // First call is inside #enforceVersionLimit (line 78) + // Throw to make the method reject + throw new Error('Logger error'); + } + // Second call would be the outer catch (line 40) - let it succeed + }); + + // Create should succeed even if cleanup's error handler fails + const result = await instance.create(mockRecord); + + expect(result).to.be.an('object'); + expect(result.getId()).to.equal(mockRecord.configurationId); + + // Wait for async cleanup to fail + await new Promise((resolve) => { + setTimeout(resolve, 150); + }); + + // Verify both error handlers were called + expect(mockLogger.error).to.have.been.calledTwice; + expect(mockLogger.error.secondCall).to.have.been.calledWith( + 'Failed to enforce configuration version limit', + sinon.match.instanceOf(Error), + ); + }); + }); + }); }); From 9d9a6955d2487f4419e47e09c631b724dd3970c9 Mon Sep 17 00:00:00 2001 From: Tathagat Sharma Date: Thu, 20 Nov 2025 15:19:05 +0530 Subject: [PATCH 2/7] feat: improve version cleanup logging and code readability --- .../configuration/configuration.collection.js | 4 --- .../configuration.collection.test.js | 28 ------------------- 2 files changed, 32 deletions(-) diff --git a/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js b/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js index 29e8614ba..6339bdb69 100755 --- a/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js +++ b/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js @@ -34,7 +34,6 @@ class ConfigurationCollection extends BaseCollection { const newConfig = await super.create(sanitizedData); - // Enforce version limit asynchronously (fire-and-forget) setImmediate(() => { this.#enforceVersionLimit().catch((error) => { this.log.error('Failed to enforce configuration version limit', error); @@ -48,10 +47,8 @@ class ConfigurationCollection extends BaseCollection { const { MAX_VERSIONS, BATCH_SIZE } = ConfigurationCollection; try { - // Get all configuration versions, sorted newest first const allConfigs = await this.all({}, { order: 'desc' }); - // If within limit, nothing to do if (allConfigs.length <= MAX_VERSIONS) { this.log.debug(`Configuration version count within limit: ${allConfigs.length}/${MAX_VERSIONS}`); return; @@ -77,7 +74,6 @@ class ConfigurationCollection extends BaseCollection { this.log.info(`Configuration version limit enforced successfully. Deleted ${deleteCount} versions. Remaining: ${MAX_VERSIONS}`); } catch (error) { this.log.error('Configuration version limit enforcement failed', error); - // Don't throw - we don't want to fail the main operation } } diff --git a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js index d5b9be5e6..e2b964316 100755 --- a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js +++ b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js @@ -138,14 +138,11 @@ describe('ConfigurationCollection', () => { expect(result).to.be.an('object'); expect(result.getId()).to.equal(mockRecord.configurationId); - // Wait a bit for async cleanup to potentially run await new Promise((resolve) => { setTimeout(resolve, 100); }); - // all should be called for cleanup check expect(instance.all).to.have.been.called; - // removeByIds should NOT be called (within limit) expect(instance.removeByIds).to.not.have.been.called; }); @@ -164,13 +161,11 @@ describe('ConfigurationCollection', () => { await instance.create(mockRecord); - // Wait for async cleanup await new Promise((resolve) => { setTimeout(resolve, 100); }); expect(instance.all).to.have.been.called; - // Should not delete anything (exactly at limit) expect(instance.removeByIds).to.not.have.been.called; }); @@ -191,7 +186,6 @@ describe('ConfigurationCollection', () => { await instance.create(mockRecord); - // Wait for async cleanup await new Promise((resolve) => { setTimeout(resolve, 100); }); @@ -207,7 +201,6 @@ describe('ConfigurationCollection', () => { getVersion: () => 549, }; - // Create 550 mock configurations const mockConfigs = new Array(550).fill(null).map((_, i) => ({ getId: () => `config-${550 - i}`, getVersion: () => 550 - i, @@ -219,21 +212,16 @@ describe('ConfigurationCollection', () => { await instance.create(mockRecord); - // Wait for async cleanup await new Promise((resolve) => { setTimeout(resolve, 100); }); expect(instance.all).to.have.been.called; - // Should delete 50 versions (550 - 500 = 50) - // 50 versions / 25 batch size = 2 batches expect(instance.removeByIds).to.have.been.calledTwice; - // First batch: 25 items const firstBatchCall = instance.removeByIds.getCall(0); expect(firstBatchCall.args[0]).to.have.lengthOf(25); - // Second batch: 25 items const secondBatchCall = instance.removeByIds.getCall(1); expect(secondBatchCall.args[0]).to.have.lengthOf(25); }); @@ -244,7 +232,6 @@ describe('ConfigurationCollection', () => { getVersion: () => 599, }; - // Create 600 mock configurations const mockConfigs = new Array(600).fill(null).map((_, i) => ({ getId: () => `config-${600 - i}`, getVersion: () => 600 - i, @@ -256,14 +243,11 @@ describe('ConfigurationCollection', () => { await instance.create(mockRecord); - // Wait for async cleanup await new Promise((resolve) => { setTimeout(resolve, 150); }); expect(instance.all).to.have.been.called; - // Should delete 100 versions (600 - 500 = 100) - // 100 versions / 25 batch size = 4 batches expect(instance.removeByIds).to.have.callCount(4); }); @@ -282,13 +266,11 @@ describe('ConfigurationCollection', () => { instance.all = stub().resolves(mockConfigs); instance.removeByIds = stub().rejects(new Error('DynamoDB error')); - // Create should succeed even if cleanup fails const result = await instance.create(mockRecord); expect(result).to.be.an('object'); expect(result.getId()).to.equal(mockRecord.configurationId); - // Wait for async cleanup to fail await new Promise((resolve) => { setTimeout(resolve, 100); }); @@ -305,13 +287,11 @@ describe('ConfigurationCollection', () => { instance.findLatest = stub().resolves(latestConfiguration); instance.all = stub().rejects(new Error('Query failed')); - // Create should succeed even if cleanup query fails const result = await instance.create(mockRecord); expect(result).to.be.an('object'); expect(result.getId()).to.equal(mockRecord.configurationId); - // Wait for async cleanup to fail await new Promise((resolve) => { setTimeout(resolve, 100); }); @@ -328,31 +308,23 @@ describe('ConfigurationCollection', () => { instance.findLatest = stub().resolves(latestConfiguration); instance.all = stub().rejects(new Error('Query failed')); - // Make the logger.error inside #enforceVersionLimit throw - // This will cause the method to reject, triggering the outer catch let errorCallCount = 0; mockLogger.error = stub().callsFake(() => { errorCallCount += 1; if (errorCallCount === 1) { - // First call is inside #enforceVersionLimit (line 78) - // Throw to make the method reject throw new Error('Logger error'); } - // Second call would be the outer catch (line 40) - let it succeed }); - // Create should succeed even if cleanup's error handler fails const result = await instance.create(mockRecord); expect(result).to.be.an('object'); expect(result.getId()).to.equal(mockRecord.configurationId); - // Wait for async cleanup to fail await new Promise((resolve) => { setTimeout(resolve, 150); }); - // Verify both error handlers were called expect(mockLogger.error).to.have.been.calledTwice; expect(mockLogger.error.secondCall).to.have.been.calledWith( 'Failed to enforce configuration version limit', From 9e6dcb1890fc934291327a53f1456fc3a1f38dc8 Mon Sep 17 00:00:00 2001 From: Tathagat Sharma Date: Wed, 26 Nov 2025 16:26:31 +0530 Subject: [PATCH 3/7] feat: simplify version cleanup and add one-time cleanup script --- .../cleanup-old-configuration-versions.js | 174 ++++++++++++++++++ .../configuration/configuration.collection.js | 24 +-- .../configuration.collection.test.js | 45 +---- 3 files changed, 185 insertions(+), 58 deletions(-) create mode 100644 packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js diff --git a/packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js b/packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js new file mode 100644 index 000000000..0e7ba0132 --- /dev/null +++ b/packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js @@ -0,0 +1,174 @@ +#!/usr/bin/env node + +/* + * Copyright 2024 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +/* eslint-disable no-console, import/no-named-as-default, max-len */ + +/** + * ONE-TIME CLEANUP SCRIPT + * + * This script deletes old configuration versions to bring the total count down to 500. + * It keeps the newest 500 versions and deletes all older versions. + * + * ⚠️ WARNING: This script is for ONE-TIME use only and should NEVER be merged to main. + * Run this script in production BEFORE deploying the automatic version limit feature. + * + * Usage: + * DRY_RUN=true node scripts/cleanup-old-configuration-versions.js + * node scripts/cleanup-old-configuration-versions.js + * + * Environment Variables Required: + * - DYNAMO_TABLE_NAME: The DynamoDB table name + * - AWS_REGION: AWS region (default: us-east-1) + * - DRY_RUN: Set to 'true' to preview deletions without executing (optional) + */ + +import createDataAccess from '../src/index.js'; + +const MAX_VERSIONS = 500; +const BATCH_SIZE = 25; // DynamoDB batch write limit + +/** + * Main cleanup function + */ +async function cleanupOldConfigVersions() { + const isDryRun = process.env.DRY_RUN === 'true'; + + console.log('='.repeat(80)); + console.log('Configuration Version Cleanup Script'); + console.log('='.repeat(80)); + console.log(`Mode: ${isDryRun ? 'DRY RUN (preview only)' : 'LIVE DELETION'}`); + console.log(`Target: Keep ${MAX_VERSIONS} newest versions, delete the rest`); + console.log(`Table: ${process.env.DYNAMO_TABLE_NAME || 'NOT SET'}`); + console.log('='.repeat(80)); + console.log(''); + + if (!process.env.DYNAMO_TABLE_NAME) { + console.error('ERROR: DYNAMO_TABLE_NAME environment variable is required'); + process.exit(1); + } + + let dataAccess; + + try { + // Initialize data access + console.log('Initializing data access...'); + dataAccess = createDataAccess({ + log: console, + }); + + const { Configuration } = dataAccess; + + // Query all configuration versions + console.log('Querying all configuration versions...'); + const allConfigs = await Configuration.all({}, { order: 'desc' }); + console.log(`Found ${allConfigs.length} total configuration versions`); + console.log(''); + + // Check if cleanup is needed + if (allConfigs.length <= MAX_VERSIONS) { + console.log(`No cleanup needed. Current count (${allConfigs.length}) is within limit (${MAX_VERSIONS})`); + process.exit(0); + } + + // Calculate versions to delete + const versionsToDelete = allConfigs.slice(MAX_VERSIONS); + const deleteCount = versionsToDelete.length; + const batchCount = Math.ceil(deleteCount / BATCH_SIZE); + + console.log('Cleanup Summary:'); + console.log(` Current versions: ${allConfigs.length}`); + console.log(` Target versions: ${MAX_VERSIONS}`); + console.log(` To delete: ${deleteCount}`); + console.log(` Batch size: ${BATCH_SIZE}`); + console.log(` Total batches: ${batchCount}`); + console.log(''); + + // Show sample of versions to be deleted + console.log('Sample of versions to be deleted (oldest first):'); + const sampleCount = Math.min(10, versionsToDelete.length); + versionsToDelete.slice(-sampleCount).reverse().forEach((config, index) => { + console.log(` ${index + 1}. Version ${config.getVersion()} - Created: ${config.getCreatedAt()}`); + }); + if (versionsToDelete.length > sampleCount) { + console.log(` ... and ${versionsToDelete.length - sampleCount} more versions`); + } + console.log(''); + + // Show sample of versions to be kept + console.log('Sample of versions to be kept (newest first):'); + const keepSample = Math.min(5, MAX_VERSIONS); + allConfigs.slice(0, keepSample).forEach((config, index) => { + console.log(` ${index + 1}. Version ${config.getVersion()} - Created: ${config.getCreatedAt()}`); + }); + console.log(` ... and ${MAX_VERSIONS - keepSample} more versions`); + console.log(''); + + if (isDryRun) { + console.log('🔍 DRY RUN MODE: No deletions will be performed'); + console.log(' Set DRY_RUN=false or remove it to actually delete versions'); + process.exit(0); + } + + // Confirm before deletion + console.log('WARNING: About to delete configuration versions!'); + console.log(' This operation cannot be undone.'); + console.log(' Press Ctrl+C within 10 seconds to cancel...'); + console.log(''); + + await new Promise((resolve) => { + setTimeout(resolve, 10000); + }); + + // Delete in batches + console.log('Starting deletion process...'); + console.log(''); + + for (let i = 0; i < versionsToDelete.length; i += BATCH_SIZE) { + const batch = versionsToDelete.slice(i, i + BATCH_SIZE); + const batchNumber = Math.floor(i / BATCH_SIZE) + 1; + const ids = batch.map((config) => config.getId()); + + console.log(` Batch ${batchNumber}/${batchCount}: Deleting ${ids.length} versions...`); + + try { + // eslint-disable-next-line no-await-in-loop + await Configuration.removeByIds(ids); + console.log(` Batch ${batchNumber} deleted successfully`); + } catch (error) { + console.error(` Batch ${batchNumber} failed:`, error.message); + throw error; // Stop on first failure + } + } + + console.log(''); + console.log('='.repeat(80)); + console.log('CLEANUP COMPLETED SUCCESSFULLY'); + console.log('='.repeat(80)); + console.log(` Deleted: ${deleteCount} versions`); + console.log(` Remaining: ${MAX_VERSIONS} versions`); + console.log(''); + } catch (error) { + console.error(''); + console.error('='.repeat(80)); + console.error('CLEANUP FAILED'); + console.error('='.repeat(80)); + console.error('Error:', error.message); + console.error('Stack:', error.stack); + console.error(''); + process.exit(1); + } +} + +// Run the cleanup +cleanupOldConfigVersions(); diff --git a/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js b/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js index 6339bdb69..7267fe15a 100755 --- a/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js +++ b/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js @@ -24,8 +24,6 @@ import BaseCollection from '../base/base.collection.js'; class ConfigurationCollection extends BaseCollection { static MAX_VERSIONS = 500; - static BATCH_SIZE = 25; - async create(data) { const latestConfiguration = await this.findLatest(); const version = latestConfiguration ? incrementVersion(latestConfiguration.getVersion()) : 1; @@ -44,34 +42,18 @@ class ConfigurationCollection extends BaseCollection { } async #enforceVersionLimit() { - const { MAX_VERSIONS, BATCH_SIZE } = ConfigurationCollection; + const { MAX_VERSIONS } = ConfigurationCollection; try { const allConfigs = await this.all({}, { order: 'desc' }); if (allConfigs.length <= MAX_VERSIONS) { - this.log.debug(`Configuration version count within limit: ${allConfigs.length}/${MAX_VERSIONS}`); return; } - // Calculate versions to delete (keep newest 500, delete the rest) const versionsToDelete = allConfigs.slice(MAX_VERSIONS); - const deleteCount = versionsToDelete.length; - - this.log.info(`Enforcing configuration version limit: deleting ${deleteCount} old versions (current: ${allConfigs.length}, target: ${MAX_VERSIONS})`); - - // Delete in batches (DynamoDB batch write limit is 25) - for (let i = 0; i < versionsToDelete.length; i += BATCH_SIZE) { - const batch = versionsToDelete.slice(i, i + BATCH_SIZE); - const ids = batch.map((config) => config.getId()); - - // eslint-disable-next-line no-await-in-loop - await this.removeByIds(ids); - - this.log.debug(`Deleted batch ${Math.floor(i / BATCH_SIZE) + 1}/${Math.ceil(deleteCount / BATCH_SIZE)}: ${ids.length} versions`); - } - - this.log.info(`Configuration version limit enforced successfully. Deleted ${deleteCount} versions. Remaining: ${MAX_VERSIONS}`); + const ids = versionsToDelete.map((config) => config.getId()); + await this.removeByIds(ids); } catch (error) { this.log.error('Configuration version limit enforcement failed', error); } diff --git a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js index e2b964316..e602b2ed0 100755 --- a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js +++ b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js @@ -195,15 +195,15 @@ describe('ConfigurationCollection', () => { expect(instance.removeByIds).to.have.been.calledWith(['config-1']); }); - it('triggers cleanup and deletes multiple versions in batches', async () => { + it('triggers cleanup and deletes 2 versions when count is 502', async () => { const latestConfiguration = { getId: () => 's12345', - getVersion: () => 549, + getVersion: () => 501, }; - const mockConfigs = new Array(550).fill(null).map((_, i) => ({ - getId: () => `config-${550 - i}`, - getVersion: () => 550 - i, + const mockConfigs = new Array(502).fill(null).map((_, i) => ({ + getId: () => `config-${502 - i}`, + getVersion: () => 502 - i, })); instance.findLatest = stub().resolves(latestConfiguration); @@ -217,38 +217,9 @@ describe('ConfigurationCollection', () => { }); expect(instance.all).to.have.been.called; - expect(instance.removeByIds).to.have.been.calledTwice; - - const firstBatchCall = instance.removeByIds.getCall(0); - expect(firstBatchCall.args[0]).to.have.lengthOf(25); - - const secondBatchCall = instance.removeByIds.getCall(1); - expect(secondBatchCall.args[0]).to.have.lengthOf(25); - }); - - it('handles large cleanup (delete 100 versions in 4 batches)', async () => { - const latestConfiguration = { - getId: () => 's12345', - getVersion: () => 599, - }; - - const mockConfigs = new Array(600).fill(null).map((_, i) => ({ - getId: () => `config-${600 - i}`, - getVersion: () => 600 - i, - })); - - instance.findLatest = stub().resolves(latestConfiguration); - instance.all = stub().resolves(mockConfigs); - instance.removeByIds = stub().resolves(); - - await instance.create(mockRecord); - - await new Promise((resolve) => { - setTimeout(resolve, 150); - }); - - expect(instance.all).to.have.been.called; - expect(instance.removeByIds).to.have.callCount(4); + expect(instance.removeByIds).to.have.been.calledOnce; + const actualIds = instance.removeByIds.getCall(0).args[0]; + expect(actualIds.sort()).to.deep.equal(['config-1', 'config-2'].sort()); }); it('does not fail create operation if cleanup fails', async () => { From b468a81b6027c774f9b611e10f1f3b3deea736f2 Mon Sep 17 00:00:00 2001 From: Tathagat Sharma Date: Tue, 2 Dec 2025 14:37:42 +0530 Subject: [PATCH 4/7] feat: improve error handling in cleanup script --- .../cleanup-old-configuration-versions.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js b/packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js index 0e7ba0132..9d768f17b 100644 --- a/packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js +++ b/packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js @@ -63,10 +63,27 @@ async function cleanupOldConfigVersions() { try { // Initialize data access console.log('Initializing data access...'); + + // Create data access (reads from environment variables) dataAccess = createDataAccess({ log: console, }); + // Verify Configuration entity is available + if (!dataAccess || !dataAccess.Configuration) { + console.error('ERROR: Configuration entity is not available'); + console.error(' Troubleshooting:'); + console.error(' 1. Ensure DYNAMO_TABLE_NAME is set correctly'); + console.error(' 2. Ensure AWS credentials are configured'); + console.error(' 3. Try running: npm install (to ensure all dependencies are installed)'); + console.error(''); + console.error(' Current environment:'); + console.error(` - DYNAMO_TABLE_NAME: ${process.env.DYNAMO_TABLE_NAME || 'NOT SET'}`); + console.error(` - AWS_REGION: ${process.env.AWS_REGION || 'NOT SET'}`); + console.error(` - AWS_ACCESS_KEY_ID: ${process.env.AWS_ACCESS_KEY_ID ? 'SET' : 'NOT SET'}`); + process.exit(1); + } + const { Configuration } = dataAccess; // Query all configuration versions From f1a7cd2000c735d0b49a58e3586ebcd8d785643e Mon Sep 17 00:00:00 2001 From: Tathagat Sharma Date: Tue, 2 Dec 2025 16:17:37 +0530 Subject: [PATCH 5/7] changed max version value to retain. --- .../src/models/configuration/configuration.collection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js b/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js index c2016cb26..eead686ae 100755 --- a/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js +++ b/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js @@ -24,7 +24,7 @@ import BaseCollection from '../base/base.collection.js'; class ConfigurationCollection extends BaseCollection { static COLLECTION_NAME = 'ConfigurationCollection'; - static MAX_VERSIONS = 500; + static MAX_VERSIONS = 499; async create(data) { const latestConfiguration = await this.findLatest(); From b401b4245a2ccbbefce9f46696e64d08b1bb378e Mon Sep 17 00:00:00 2001 From: Tathagat Sharma Date: Tue, 2 Dec 2025 16:26:28 +0530 Subject: [PATCH 6/7] chore: update MAX_VERSIONS to 499 and fix unit tests --- .../models/configuration/configuration.collection.test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js index e602b2ed0..862f2ed94 100755 --- a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js +++ b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js @@ -166,7 +166,8 @@ describe('ConfigurationCollection', () => { }); expect(instance.all).to.have.been.called; - expect(instance.removeByIds).to.not.have.been.called; + expect(instance.removeByIds).to.have.been.calledOnce; + expect(instance.removeByIds).to.have.been.calledWith(['config-1']); }); it('triggers cleanup and deletes 1 version when count is 501', async () => { @@ -192,7 +193,8 @@ describe('ConfigurationCollection', () => { expect(instance.all).to.have.been.called; expect(instance.removeByIds).to.have.been.calledOnce; - expect(instance.removeByIds).to.have.been.calledWith(['config-1']); + const actualIds = instance.removeByIds.getCall(0).args[0]; + expect(actualIds.sort()).to.deep.equal(['config-1', 'config-2'].sort()); }); it('triggers cleanup and deletes 2 versions when count is 502', async () => { @@ -219,7 +221,7 @@ describe('ConfigurationCollection', () => { expect(instance.all).to.have.been.called; expect(instance.removeByIds).to.have.been.calledOnce; const actualIds = instance.removeByIds.getCall(0).args[0]; - expect(actualIds.sort()).to.deep.equal(['config-1', 'config-2'].sort()); + expect(actualIds.sort()).to.deep.equal(['config-1', 'config-2', 'config-3'].sort()); }); it('does not fail create operation if cleanup fails', async () => { From 62685b1e450840c6ab6d928154f0be019cf5c0ea Mon Sep 17 00:00:00 2001 From: Tathagat Sharma Date: Thu, 4 Dec 2025 14:26:10 +0530 Subject: [PATCH 7/7] kept max version to retain at 500. --- .../cleanup-old-configuration-versions.js | 191 ------------------ .../configuration/configuration.collection.js | 2 +- .../configuration.collection.test.js | 9 +- 3 files changed, 5 insertions(+), 197 deletions(-) delete mode 100644 packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js diff --git a/packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js b/packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js deleted file mode 100644 index 9d768f17b..000000000 --- a/packages/spacecat-shared-data-access/scripts/cleanup-old-configuration-versions.js +++ /dev/null @@ -1,191 +0,0 @@ -#!/usr/bin/env node - -/* - * Copyright 2024 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ - -/* eslint-disable no-console, import/no-named-as-default, max-len */ - -/** - * ONE-TIME CLEANUP SCRIPT - * - * This script deletes old configuration versions to bring the total count down to 500. - * It keeps the newest 500 versions and deletes all older versions. - * - * ⚠️ WARNING: This script is for ONE-TIME use only and should NEVER be merged to main. - * Run this script in production BEFORE deploying the automatic version limit feature. - * - * Usage: - * DRY_RUN=true node scripts/cleanup-old-configuration-versions.js - * node scripts/cleanup-old-configuration-versions.js - * - * Environment Variables Required: - * - DYNAMO_TABLE_NAME: The DynamoDB table name - * - AWS_REGION: AWS region (default: us-east-1) - * - DRY_RUN: Set to 'true' to preview deletions without executing (optional) - */ - -import createDataAccess from '../src/index.js'; - -const MAX_VERSIONS = 500; -const BATCH_SIZE = 25; // DynamoDB batch write limit - -/** - * Main cleanup function - */ -async function cleanupOldConfigVersions() { - const isDryRun = process.env.DRY_RUN === 'true'; - - console.log('='.repeat(80)); - console.log('Configuration Version Cleanup Script'); - console.log('='.repeat(80)); - console.log(`Mode: ${isDryRun ? 'DRY RUN (preview only)' : 'LIVE DELETION'}`); - console.log(`Target: Keep ${MAX_VERSIONS} newest versions, delete the rest`); - console.log(`Table: ${process.env.DYNAMO_TABLE_NAME || 'NOT SET'}`); - console.log('='.repeat(80)); - console.log(''); - - if (!process.env.DYNAMO_TABLE_NAME) { - console.error('ERROR: DYNAMO_TABLE_NAME environment variable is required'); - process.exit(1); - } - - let dataAccess; - - try { - // Initialize data access - console.log('Initializing data access...'); - - // Create data access (reads from environment variables) - dataAccess = createDataAccess({ - log: console, - }); - - // Verify Configuration entity is available - if (!dataAccess || !dataAccess.Configuration) { - console.error('ERROR: Configuration entity is not available'); - console.error(' Troubleshooting:'); - console.error(' 1. Ensure DYNAMO_TABLE_NAME is set correctly'); - console.error(' 2. Ensure AWS credentials are configured'); - console.error(' 3. Try running: npm install (to ensure all dependencies are installed)'); - console.error(''); - console.error(' Current environment:'); - console.error(` - DYNAMO_TABLE_NAME: ${process.env.DYNAMO_TABLE_NAME || 'NOT SET'}`); - console.error(` - AWS_REGION: ${process.env.AWS_REGION || 'NOT SET'}`); - console.error(` - AWS_ACCESS_KEY_ID: ${process.env.AWS_ACCESS_KEY_ID ? 'SET' : 'NOT SET'}`); - process.exit(1); - } - - const { Configuration } = dataAccess; - - // Query all configuration versions - console.log('Querying all configuration versions...'); - const allConfigs = await Configuration.all({}, { order: 'desc' }); - console.log(`Found ${allConfigs.length} total configuration versions`); - console.log(''); - - // Check if cleanup is needed - if (allConfigs.length <= MAX_VERSIONS) { - console.log(`No cleanup needed. Current count (${allConfigs.length}) is within limit (${MAX_VERSIONS})`); - process.exit(0); - } - - // Calculate versions to delete - const versionsToDelete = allConfigs.slice(MAX_VERSIONS); - const deleteCount = versionsToDelete.length; - const batchCount = Math.ceil(deleteCount / BATCH_SIZE); - - console.log('Cleanup Summary:'); - console.log(` Current versions: ${allConfigs.length}`); - console.log(` Target versions: ${MAX_VERSIONS}`); - console.log(` To delete: ${deleteCount}`); - console.log(` Batch size: ${BATCH_SIZE}`); - console.log(` Total batches: ${batchCount}`); - console.log(''); - - // Show sample of versions to be deleted - console.log('Sample of versions to be deleted (oldest first):'); - const sampleCount = Math.min(10, versionsToDelete.length); - versionsToDelete.slice(-sampleCount).reverse().forEach((config, index) => { - console.log(` ${index + 1}. Version ${config.getVersion()} - Created: ${config.getCreatedAt()}`); - }); - if (versionsToDelete.length > sampleCount) { - console.log(` ... and ${versionsToDelete.length - sampleCount} more versions`); - } - console.log(''); - - // Show sample of versions to be kept - console.log('Sample of versions to be kept (newest first):'); - const keepSample = Math.min(5, MAX_VERSIONS); - allConfigs.slice(0, keepSample).forEach((config, index) => { - console.log(` ${index + 1}. Version ${config.getVersion()} - Created: ${config.getCreatedAt()}`); - }); - console.log(` ... and ${MAX_VERSIONS - keepSample} more versions`); - console.log(''); - - if (isDryRun) { - console.log('🔍 DRY RUN MODE: No deletions will be performed'); - console.log(' Set DRY_RUN=false or remove it to actually delete versions'); - process.exit(0); - } - - // Confirm before deletion - console.log('WARNING: About to delete configuration versions!'); - console.log(' This operation cannot be undone.'); - console.log(' Press Ctrl+C within 10 seconds to cancel...'); - console.log(''); - - await new Promise((resolve) => { - setTimeout(resolve, 10000); - }); - - // Delete in batches - console.log('Starting deletion process...'); - console.log(''); - - for (let i = 0; i < versionsToDelete.length; i += BATCH_SIZE) { - const batch = versionsToDelete.slice(i, i + BATCH_SIZE); - const batchNumber = Math.floor(i / BATCH_SIZE) + 1; - const ids = batch.map((config) => config.getId()); - - console.log(` Batch ${batchNumber}/${batchCount}: Deleting ${ids.length} versions...`); - - try { - // eslint-disable-next-line no-await-in-loop - await Configuration.removeByIds(ids); - console.log(` Batch ${batchNumber} deleted successfully`); - } catch (error) { - console.error(` Batch ${batchNumber} failed:`, error.message); - throw error; // Stop on first failure - } - } - - console.log(''); - console.log('='.repeat(80)); - console.log('CLEANUP COMPLETED SUCCESSFULLY'); - console.log('='.repeat(80)); - console.log(` Deleted: ${deleteCount} versions`); - console.log(` Remaining: ${MAX_VERSIONS} versions`); - console.log(''); - } catch (error) { - console.error(''); - console.error('='.repeat(80)); - console.error('CLEANUP FAILED'); - console.error('='.repeat(80)); - console.error('Error:', error.message); - console.error('Stack:', error.stack); - console.error(''); - process.exit(1); - } -} - -// Run the cleanup -cleanupOldConfigVersions(); diff --git a/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js b/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js index eead686ae..c2016cb26 100755 --- a/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js +++ b/packages/spacecat-shared-data-access/src/models/configuration/configuration.collection.js @@ -24,7 +24,7 @@ import BaseCollection from '../base/base.collection.js'; class ConfigurationCollection extends BaseCollection { static COLLECTION_NAME = 'ConfigurationCollection'; - static MAX_VERSIONS = 499; + static MAX_VERSIONS = 500; async create(data) { const latestConfiguration = await this.findLatest(); diff --git a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js index 862f2ed94..713f5aed1 100755 --- a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js +++ b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.collection.test.js @@ -146,7 +146,7 @@ describe('ConfigurationCollection', () => { expect(instance.removeByIds).to.not.have.been.called; }); - it('triggers cleanup when version count is exactly 500', async () => { + it('does not trigger cleanup when version count is exactly 500', async () => { const latestConfiguration = { getId: () => 's12345', getVersion: () => 499, @@ -166,8 +166,7 @@ describe('ConfigurationCollection', () => { }); expect(instance.all).to.have.been.called; - expect(instance.removeByIds).to.have.been.calledOnce; - expect(instance.removeByIds).to.have.been.calledWith(['config-1']); + expect(instance.removeByIds).to.not.have.been.called; }); it('triggers cleanup and deletes 1 version when count is 501', async () => { @@ -194,7 +193,7 @@ describe('ConfigurationCollection', () => { expect(instance.all).to.have.been.called; expect(instance.removeByIds).to.have.been.calledOnce; const actualIds = instance.removeByIds.getCall(0).args[0]; - expect(actualIds.sort()).to.deep.equal(['config-1', 'config-2'].sort()); + expect(actualIds.sort()).to.deep.equal(['config-1'].sort()); }); it('triggers cleanup and deletes 2 versions when count is 502', async () => { @@ -221,7 +220,7 @@ describe('ConfigurationCollection', () => { expect(instance.all).to.have.been.called; expect(instance.removeByIds).to.have.been.calledOnce; const actualIds = instance.removeByIds.getCall(0).args[0]; - expect(actualIds.sort()).to.deep.equal(['config-1', 'config-2', 'config-3'].sort()); + expect(actualIds.sort()).to.deep.equal(['config-1', 'config-2'].sort()); }); it('does not fail create operation if cleanup fails', async () => {