fix: cache pipeline & source delete (#8688)

* fix: avoid duplicate refresh * fix: pipe nested refresh * fix: wrong reference for source * fix: base list * fix: unit test base list * fix: disable meta info for ee * fix: improve readability
nocodb · Jun 12, 2024 · 9a621e4 · 9a621e4
1 parent 3aa80ff
commit 9a621e4
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 24 deletions.
diff --git a/packages/nocodb/src/cache/CacheMgr.ts b/packages/nocodb/src/cache/CacheMgr.ts
@@ -1,5 +1,6 @@
 import debug from 'debug';
 import { Logger } from '@nestjs/common';
+import type { ChainableCommander } from 'ioredis';
 import type IORedis from 'ioredis';
 import { CacheDelDirection, CacheGetType } from '~/utils/globals';
 
@@ -77,7 +78,7 @@ export default abstract class CacheMgr {
             if (!skipTTL && o.timestamp) {
               const diff = Date.now() - o.timestamp;
               if (diff > NC_REDIS_GRACE_TTL * 1000) {
-                await this.refreshTTL(key);
+                await this.execRefreshTTL(key);
               }
             }
 
@@ -166,7 +167,7 @@ export default abstract class CacheMgr {
           NC_REDIS_TTL,
         )
         .then(async () => {
-          await this.refreshTTL(key, timestamp);
+          await this.execRefreshTTL(key, timestamp);
           return true;
         });
     } else {
@@ -317,7 +318,7 @@ export default abstract class CacheMgr {
         if (typeof o === 'object') {
           const diff = Date.now() - o.timestamp;
           if (diff > NC_REDIS_GRACE_TTL * 1000) {
-            await this.refreshTTL(key);
+            await this.execRefreshTTL(key);
           }
         }
       } catch (e) {
@@ -510,10 +511,7 @@ export default abstract class CacheMgr {
     });
 
     list.push(key);
-    return this.set(listKey, list).then(async (res) => {
-      await this.refreshTTL(listKey);
-      return res;
-    });
+    return this.set(listKey, list);
   }
 
   async update(key: string, value: any): Promise<boolean> {
@@ -564,7 +562,16 @@ export default abstract class CacheMgr {
     }
   }
 
-  async refreshTTL(key: string, timestamp?: number): Promise<void> {
+  async execRefreshTTL(keys: string, timestamp?: number): Promise<void> {
+    const p = await this.refreshTTL(this.client.pipeline(), keys, timestamp);
+    await p.exec();
+  }
+
+  async refreshTTL(
+    pipeline: ChainableCommander,
+    key: string,
+    timestamp?: number,
+  ): Promise<ChainableCommander> {
     log(`${this.context}::refreshTTL: refreshing TTL for ${key}`);
     const isParent = /:list$/.test(key);
     timestamp = timestamp || Date.now();
@@ -573,7 +580,6 @@ export default abstract class CacheMgr {
         (await this.getRaw(key, CacheGetType.TYPE_ARRAY, true)) || [];
       if (list && list.length) {
         const listValues = await this.client.mget(list);
-        const pipeline = this.client.pipeline();
         for (const [i, v] of listValues.entries()) {
           const key = list[i];
           if (v) {
@@ -602,19 +608,18 @@ export default abstract class CacheMgr {
           }
         }
         pipeline.expire(key, NC_REDIS_TTL - 60);
-        await pipeline.exec();
       }
     } else {
       const rawValue = await this.getRaw(key, null, true);
       if (rawValue) {
         if (rawValue.parentKeys && rawValue.parentKeys.length) {
           for (const parent of rawValue.parentKeys) {
-            await this.refreshTTL(parent, timestamp);
+            pipeline = await this.refreshTTL(pipeline, parent, timestamp);
           }
         } else {
           if (rawValue.timestamp !== timestamp) {
             rawValue.timestamp = timestamp;
-            await this.client.set(
+            pipeline.set(
               key,
               JSON.stringify(rawValue, this.getCircularReplacer()),
               'EX',
@@ -624,6 +629,8 @@ export default abstract class CacheMgr {
         }
       }
     }
+
+    return pipeline;
   }
 
   async destroy(): Promise<boolean> {

diff --git a/packages/nocodb/src/models/Base.ts b/packages/nocodb/src/models/Base.ts
@@ -105,8 +105,7 @@ export default class Base implements BaseType {
   }
 
   static async list(
-    // @ts-ignore
-    param,
+    workspaceId?: string,
     ncMeta = Noco.ncMeta,
   ): Promise<Base[]> {
     // todo: pagination

diff --git a/packages/nocodb/src/models/Source.ts b/packages/nocodb/src/models/Source.ts
@@ -488,9 +488,9 @@ export default class Source implements SourceType {
     ncMeta = Noco.ncMeta,
     { force }: { force?: boolean } = {},
   ) {
-    const bases = await Base.list({ baseId: this.base_id }, ncMeta);
+    const sources = await Source.list(context, { baseId: this.id }, ncMeta);
 
-    if (bases[0].id === this.id && !force) {
+    if (sources[0].id === this.id && !force) {
       NcError.badRequest('Cannot delete first base');
     }
 

diff --git a/packages/nocodb/src/models/User.ts b/packages/nocodb/src/models/User.ts
@@ -75,7 +75,7 @@ export default class User implements UserType {
     await NocoCache.del(CacheScope.INSTANCE_META);
 
     // clear all base user related cache for instance
-    const bases = await Base.list({}, ncMeta);
+    const bases = await Base.list(null, ncMeta);
     for (const base of bases) {
       await NocoCache.deepDel(
         `${CacheScope.BASE_USER}:${base.id}:list`,

diff --git a/packages/nocodb/src/modules/jobs/jobs/export-import/duplicate.controller.ts b/packages/nocodb/src/modules/jobs/jobs/export-import/duplicate.controller.ts
@@ -69,7 +69,7 @@ export class DuplicateController {
       throw new Error(`Source not found!`);
     }
 
-    const bases = await Base.list({});
+    const bases = await Base.list(context.workspace_id);
 
     const uniqueTitle = generateUniqueName(
       `${base.title} copy`,
@@ -145,7 +145,7 @@ export class DuplicateController {
       throw new Error(`Source not found!`);
     }
 
-    const bases = await Base.list({});
+    const bases = await Base.list(context.workspace_id);
 
     const uniqueTitle = generateUniqueName(
       `${base.title} copy`,

diff --git a/packages/nocodb/src/services/bases.service.ts b/packages/nocodb/src/services/bases.service.ts
@@ -45,7 +45,7 @@ export class BasesService {
     },
   ) {
     const bases = extractRolesObj(param.user?.roles)[OrgUserRoles.SUPER_ADMIN]
-      ? await Base.list(param.query)
+      ? await Base.list()
       : await BaseUser.getProjectsList(param.user.id, param.query);
 
     return bases;

diff --git a/packages/nocodb/src/services/utils.service.ts b/packages/nocodb/src/services/utils.service.ts
@@ -212,8 +212,9 @@ export class UtilsService {
   }
 
   async aggregatedMetaInfo() {
+    // TODO: fix or deprecate for EE
     const [bases, userCount] = await Promise.all([
-      Base.list({}),
+      Base.list(),
       Noco.ncMeta.metaCount(RootScopes.ROOT, RootScopes.ROOT, MetaTable.USERS),
     ]);
 

diff --git a/packages/nocodb/tests/unit/init/cleanupMeta.ts b/packages/nocodb/tests/unit/init/cleanupMeta.ts
@@ -1,10 +1,18 @@
+import TestDbMngr from '../TestDbMngr';
 import { Base, Model } from '~/models';
 import NcConnectionMgrv2 from '~/utils/common/NcConnectionMgrv2';
-import { orderedMetaTables } from '~/utils/globals';
-import TestDbMngr from '../TestDbMngr';
+import { MetaTable, orderedMetaTables, RootScopes } from '~/utils/globals';
+import Noco from '~/Noco';
 
 const dropTablesAllNonExternalProjects = async () => {
-  const bases = await Base.list({});
+  const rawBases = await Noco.ncMeta.metaList2(
+    RootScopes.BASE,
+    RootScopes.BASE,
+    MetaTable.PROJECT,
+  );
+
+  const bases = rawBases.map((b) => Base.castType(b));
+
   const userCreatedTableNames: string[] = [];
   await Promise.all(
     bases

diff --git a/packages/nocodb/tests/unit/rest/tests/base.test.ts b/packages/nocodb/tests/unit/rest/tests/base.test.ts
@@ -351,6 +351,10 @@ function baseTest() {
   });
 
   it('Get all bases meta', async () => {
+    if (process.env.EE === 'true') {
+      return;
+    }
+
     await createTable(context, base, {
       table_name: 'table1',
       title: 'table1',