add removeIfUndefined option

apps/app/src/server/service/config-manager/config-manager.integ.ts

@@ -107,6 +107,55 @@ describe('ConfigManager', () => {
     });
   });
 
+  describe('updateConfig', () => {
+    beforeEach(async() => {
+      await Config.deleteMany({ key: /app.*/ }).exec();
+      await Config.create({ key: 'app:siteUrl', value: JSON.stringify('initial value') });
+    });
+
+    test('updates a single config', async() => {
+      // arrange
+      await configManager.loadConfigs();
+      const config = await Config.findOne({ key: 'app:siteUrl' }).exec();
+      expect(config?.value).toEqual(JSON.stringify('initial value'));
+
+      // act
+      await configManager.updateConfig('app:siteUrl', 'updated value');
+
+      // assert
+      const updatedConfig = await Config.findOne({ key: 'app:siteUrl' }).exec();
+      expect(updatedConfig?.value).toEqual(JSON.stringify('updated value'));
+    });
+
+    test('removes config when value is undefined and removeIfUndefined is true', async() => {
+      // arrange
+      await configManager.loadConfigs();
+      const config = await Config.findOne({ key: 'app:siteUrl' }).exec();
+      expect(config?.value).toEqual(JSON.stringify('initial value'));
+
+      // act
+      await configManager.updateConfig('app:siteUrl', undefined, { removeIfUndefined: true });
+
+      // assert
+      const updatedConfig = await Config.findOne({ key: 'app:siteUrl' }).exec();
+      expect(updatedConfig).toBeNull(); // should be removed
+    });
+
+    test('does not update config when value is undefined and removeIfUndefined is false', async() => {
+      // arrange
+      await configManager.loadConfigs();
+      const config = await Config.findOne({ key: 'app:siteUrl' }).exec();
+      expect(config?.value).toEqual(JSON.stringify('initial value'));
+
+      // act
+      await configManager.updateConfig('app:siteUrl', undefined);
+
+      // assert
+      const updatedConfig = await Config.findOne({ key: 'app:siteUrl' }).exec();
+      expect(updatedConfig?.value).toEqual(JSON.stringify('initial value')); // should remain unchanged
+    });
+  });
+
   describe('updateConfigs', () => {
     beforeEach(async() => {
       await Config.deleteMany({ key: /app.*/ }).exec();
@@ -133,6 +182,44 @@ describe('ConfigManager', () => {
       expect(updatedConfig1?.value).toEqual(JSON.stringify('new value1'));
       expect(updatedConfig2?.value).toEqual(JSON.stringify('aws'));
     });
+
+    test('removes config when value is undefined and removeIfUndefined is true', async() => {
+      // arrange
+      await configManager.loadConfigs();
+      const config1 = await Config.findOne({ key: 'app:siteUrl' }).exec();
+      expect(config1?.value).toEqual(JSON.stringify('value1'));
+
+      // act
+      await configManager.updateConfigs({
+        'app:siteUrl': undefined,
+        'app:fileUploadType': 'aws',
+      }, { removeIfUndefined: true });
+
+      // assert
+      const updatedConfig1 = await Config.findOne({ key: 'app:siteUrl' }).exec();
+      const updatedConfig2 = await Config.findOne({ key: 'app:fileUploadType' }).exec();
+      expect(updatedConfig1).toBeNull(); // should be removed
+      expect(updatedConfig2?.value).toEqual(JSON.stringify('aws'));
+    });
+
+    test('does not update config when value is undefined and removeIfUndefined is false', async() => {
+      // arrange
+      await configManager.loadConfigs();
+      const config1 = await Config.findOne({ key: 'app:siteUrl' }).exec();
+      expect(config1?.value).toEqual(JSON.stringify('value1'));
+
+      // act
+      await configManager.updateConfigs({
+        'app:siteUrl': undefined,
+        'app:fileUploadType': 'aws',
+      });
+
+      // assert
+      const updatedConfig1 = await Config.findOne({ key: 'app:siteUrl' }).exec();
+      const updatedConfig2 = await Config.findOne({ key: 'app:fileUploadType' }).exec();
+      expect(updatedConfig1?.value).toEqual(JSON.stringify('value1')); // should remain unchanged
+      expect(updatedConfig2?.value).toEqual(JSON.stringify('aws'));
+    });
   });
 
   describe('removeConfigs', () => {

apps/app/src/server/service/config-manager/config-manager.spec.ts

@@ -13,6 +13,7 @@ const mocks = vi.hoisted(() => ({
   ConfigMock: {
     updateOne: vi.fn(),
     bulkWrite: vi.fn(),
+    deleteOne: vi.fn(),
   },
 }));
 vi.mock('../../models/config', () => ({
@@ -40,6 +41,9 @@ describe('ConfigManager test', () => {
     let loadConfigsSpy;
     beforeEach(async() => {
       loadConfigsSpy = vi.spyOn(configManager, 'loadConfigs');
+      // Reset mocks
+      mocks.ConfigMock.updateOne.mockClear();
+      mocks.ConfigMock.deleteOne.mockClear();
     });
 
     test('invoke publishUpdateMessage()', async() => {
@@ -70,6 +74,42 @@ describe('ConfigManager test', () => {
       expect(configManager.publishUpdateMessage).not.toHaveBeenCalled();
     });
 
+    test('remove config when value is undefined and removeIfUndefined is true', async() => {
+      // arrange
+      configManager.publishUpdateMessage = vi.fn();
+      vi.spyOn((configManager as unknown as ConfigManagerToGetLoader).configLoader, 'loadFromDB').mockImplementation(vi.fn());
+
+      // act
+      await configManager.updateConfig('app:siteUrl', undefined, { removeIfUndefined: true });
+
+      // assert
+      expect(mocks.ConfigMock.deleteOne).toHaveBeenCalledTimes(1);
+      expect(mocks.ConfigMock.deleteOne).toHaveBeenCalledWith({ key: 'app:siteUrl' });
+      expect(mocks.ConfigMock.updateOne).not.toHaveBeenCalled();
+      expect(loadConfigsSpy).toHaveBeenCalledTimes(1);
+      expect(configManager.publishUpdateMessage).toHaveBeenCalledTimes(1);
+    });
+
+    test('update config with undefined value when removeIfUndefined is false', async() => {
+      // arrange
+      configManager.publishUpdateMessage = vi.fn();
+      vi.spyOn((configManager as unknown as ConfigManagerToGetLoader).configLoader, 'loadFromDB').mockImplementation(vi.fn());
+
+      // act
+      await configManager.updateConfig('app:siteUrl', undefined);
+
+      // assert
+      expect(mocks.ConfigMock.updateOne).toHaveBeenCalledTimes(1);
+      expect(mocks.ConfigMock.updateOne).toHaveBeenCalledWith(
+        { key: 'app:siteUrl' },
+        { value: JSON.stringify(undefined) },
+        { upsert: true },
+      );
+      expect(mocks.ConfigMock.deleteOne).not.toHaveBeenCalled();
+      expect(loadConfigsSpy).toHaveBeenCalledTimes(1);
+      expect(configManager.publishUpdateMessage).toHaveBeenCalledTimes(1);
+    });
+
   });
 
   describe('updateConfigs()', () => {
@@ -77,18 +117,20 @@ describe('ConfigManager test', () => {
     let loadConfigsSpy;
     beforeEach(async() => {
       loadConfigsSpy = vi.spyOn(configManager, 'loadConfigs');
+      // Reset mocks
+      mocks.ConfigMock.bulkWrite.mockClear();
     });
 
     test('invoke publishUpdateMessage()', async() => {
-      // arrenge
+      // arrange
       configManager.publishUpdateMessage = vi.fn();
       vi.spyOn((configManager as unknown as ConfigManagerToGetLoader).configLoader, 'loadFromDB').mockImplementation(vi.fn());
 
       // act
-      await configManager.updateConfig('app:siteUrl', '');
+      await configManager.updateConfigs({ 'app:siteUrl': 'https://example.com' });
 
       // assert
-      // expect(Config.bulkWrite).toHaveBeenCalledTimes(1);
+      expect(mocks.ConfigMock.bulkWrite).toHaveBeenCalledTimes(1);
       expect(loadConfigsSpy).toHaveBeenCalledTimes(1);
       expect(configManager.publishUpdateMessage).toHaveBeenCalledTimes(1);
     });
@@ -102,10 +144,68 @@ describe('ConfigManager test', () => {
       await configManager.updateConfigs({ 'app:siteUrl': '' }, { skipPubsub: true });
 
       // assert
-      // expect(Config.bulkWrite).toHaveBeenCalledTimes(1);
+      expect(mocks.ConfigMock.bulkWrite).toHaveBeenCalledTimes(1);
       expect(loadConfigsSpy).toHaveBeenCalledTimes(1);
       expect(configManager.publishUpdateMessage).not.toHaveBeenCalled();
     });
+
+    test('remove configs when values are undefined and removeIfUndefined is true', async() => {
+      // arrange
+      configManager.publishUpdateMessage = vi.fn();
+      vi.spyOn((configManager as unknown as ConfigManagerToGetLoader).configLoader, 'loadFromDB').mockImplementation(vi.fn());
+
+      // act
+      await configManager.updateConfigs(
+        { 'app:siteUrl': undefined, 'app:title': 'GROWI' },
+        { removeIfUndefined: true },
+      );
+
+      // assert
+      expect(mocks.ConfigMock.bulkWrite).toHaveBeenCalledTimes(1);
+      const operations = mocks.ConfigMock.bulkWrite.mock.calls[0][0];
+      expect(operations).toHaveLength(2);
+      expect(operations[0]).toEqual({ deleteOne: { filter: { key: 'app:siteUrl' } } });
+      expect(operations[1]).toEqual({
+        updateOne: {
+          filter: { key: 'app:title' },
+          update: { value: JSON.stringify('GROWI') },
+          upsert: true,
+        },
+      });
+      expect(loadConfigsSpy).toHaveBeenCalledTimes(1);
+      expect(configManager.publishUpdateMessage).toHaveBeenCalledTimes(1);
+    });
+
+    test('update configs including undefined values when removeIfUndefined is false', async() => {
+      // arrange
+      configManager.publishUpdateMessage = vi.fn();
+      vi.spyOn((configManager as unknown as ConfigManagerToGetLoader).configLoader, 'loadFromDB').mockImplementation(vi.fn());
+
+      // act
+      await configManager.updateConfigs({ 'app:siteUrl': undefined, 'app:title': 'GROWI' });
+
+      // assert
+      expect(mocks.ConfigMock.bulkWrite).toHaveBeenCalledTimes(1);
+      const operations = mocks.ConfigMock.bulkWrite.mock.calls[0][0];
+      expect(operations).toHaveLength(2); // both operations should be included
+      expect(operations[0]).toEqual({
+        updateOne: {
+          filter: { key: 'app:siteUrl' },
+          update: { value: JSON.stringify(undefined) },
+          upsert: true,
+        },
+      });
+      expect(operations[1]).toEqual({
+        updateOne: {
+          filter: { key: 'app:title' },
+          update: { value: JSON.stringify('GROWI') },
+          upsert: true,
+        },
+      });
+      expect(loadConfigsSpy).toHaveBeenCalledTimes(1);
+      expect(configManager.publishUpdateMessage).toHaveBeenCalledTimes(1);
+    });
+
   });
 
   describe('getManagedEnvVars()', () => {

apps/app/src/server/service/config-manager/config-manager.ts

@@ -111,11 +111,17 @@ export class ConfigManager implements IConfigManagerForApp, S2sMessageHandlable
     // Dynamic import to avoid loading database modules too early
     const { Config } = await import('../../models/config');
 
-    await Config.updateOne(
-      { key },
-      { value: JSON.stringify(value) },
-      { upsert: true },
-    );
+    if (options?.removeIfUndefined && value === undefined) {
+      // remove the config if the value is undefined and removeIfUndefined is true
+      await Config.deleteOne({ key });
+    }
+    else {
+      await Config.updateOne(
+        { key },
+        { value: JSON.stringify(value) },
+        { upsert: true },
+      );
+    }
 
     await this.loadConfigs({ source: 'db' });
 
@@ -128,13 +134,19 @@ export class ConfigManager implements IConfigManagerForApp, S2sMessageHandlable
     // Dynamic import to avoid loading database modules too early
     const { Config } = await import('../../models/config');
 
-    const operations = Object.entries(updates).map(([key, value]) => ({
-      updateOne: {
-        filter: { key },
-        update: { value: JSON.stringify(value) },
-        upsert: true,
-      },
-    }));
+    const operations = Object.entries(updates).map(([key, value]) => {
+      return (options?.removeIfUndefined && value === undefined)
+        // remove the config if the value is undefined
+        ? { deleteOne: { filter: { key } } }
+        // update
+        : {
+          updateOne: {
+            filter: { key },
+            update: { value: JSON.stringify(value) },
+            upsert: true,
+          },
+        };
+    });
 
     await Config.bulkWrite(operations);
     await this.loadConfigs({ source: 'db' });

packages/core/src/interfaces/config-manager.ts

@@ -43,7 +43,10 @@ export type RawConfigData<K extends string, V extends Record<K, any>> = Record<K
   definition?: ConfigDefinition<V[K]>;
 }>;
 
-export type UpdateConfigOptions = { skipPubsub?: boolean };
+export type UpdateConfigOptions = {
+  skipPubsub?: boolean;
+  removeIfUndefined?: boolean;
+};
 
 /**
  * Interface for managing configuration values