From 9a621e49583ec3a969f91bf199871fc99d2f9e17 Mon Sep 17 00:00:00 2001 From: Mert E Date: Wed, 12 Jun 2024 12:58:06 +0300 Subject: [PATCH] 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 --- packages/nocodb/src/cache/CacheMgr.ts | 31 ++++++++++++------- packages/nocodb/src/models/Base.ts | 3 +- packages/nocodb/src/models/Source.ts | 4 +-- packages/nocodb/src/models/User.ts | 2 +- .../export-import/duplicate.controller.ts | 4 +-- packages/nocodb/src/services/bases.service.ts | 2 +- packages/nocodb/src/services/utils.service.ts | 3 +- .../nocodb/tests/unit/init/cleanupMeta.ts | 14 +++++++-- .../nocodb/tests/unit/rest/tests/base.test.ts | 4 +++ 9 files changed, 43 insertions(+), 24 deletions(-) diff --git a/packages/nocodb/src/cache/CacheMgr.ts b/packages/nocodb/src/cache/CacheMgr.ts index 6f77ed319e..d6df1bb83f 100644 --- 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 { @@ -564,7 +562,16 @@ export default abstract class CacheMgr { } } - async refreshTTL(key: string, timestamp?: number): Promise { + async execRefreshTTL(keys: string, timestamp?: number): Promise { + const p = await this.refreshTTL(this.client.pipeline(), keys, timestamp); + await p.exec(); + } + + async refreshTTL( + pipeline: ChainableCommander, + key: string, + timestamp?: number, + ): Promise { 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 { diff --git a/packages/nocodb/src/models/Base.ts b/packages/nocodb/src/models/Base.ts index 55cf2f419a..f046f528f8 100644 --- 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 { // todo: pagination diff --git a/packages/nocodb/src/models/Source.ts b/packages/nocodb/src/models/Source.ts index 22a58937e8..f5acae9011 100644 --- 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 index 1183662626..6b3b0e8544 100644 --- 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 index 594df86b47..c288f1d110 100644 --- 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 index 1e4a30fab9..1b83e1a516 100644 --- 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 index 823d306486..d705c7e516 100644 --- 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 index 2ba0fd5601..a168e9bd70 100644 --- 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 index 89a785adcd..1df32d000f 100644 --- 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',