From a2f48f4597cd43d79bfafbea3d501eeba7ccb8ea Mon Sep 17 00:00:00 2001 From: Boris Date: Tue, 25 Jun 2024 18:14:17 +0800 Subject: [PATCH] refactor: remove get table full (#673) * refactor: remove the functionality of fetching table content (view, field, record) from the API * chore: public convertProjection method in record.service * fix: multiple socket connections and resolve readonly user role confusion after snapshot changes --- .../src/features/record/record.service.ts | 2 +- .../open-api/table-open-api.controller.ts | 7 +-- .../table/open-api/table-open-api.service.ts | 7 +-- .../src/features/table/table.service.ts | 35 +----------- .../src/share-db/share-db.adapter.ts | 55 +++++++++++-------- .../test/table-import.e2e-spec.ts | 15 +---- apps/nestjs-backend/test/utils/init-app.ts | 24 +++++--- apps/nestjs-backend/test/view.e2e-spec.ts | 9 ++- .../src/backend/api/rest/table.ssr.ts | 35 ++++++++++-- .../inplace-panel/InplaceFieldConfigPanel.tsx | 2 +- packages/openapi/src/table/create.ts | 2 +- packages/openapi/src/table/get-list.ts | 24 +------- packages/openapi/src/table/get.ts | 11 +--- 13 files changed, 96 insertions(+), 132 deletions(-) diff --git a/apps/nestjs-backend/src/features/record/record.service.ts b/apps/nestjs-backend/src/features/record/record.service.ts index f628d4631..8bbee21a9 100644 --- a/apps/nestjs-backend/src/features/record/record.service.ts +++ b/apps/nestjs-backend/src/features/record/record.service.ts @@ -552,7 +552,7 @@ export class RecordService { return this.prismaService.txClient().$executeRawUnsafe(updateRecordSql); } - private convertProjection(fieldKeys?: string[]) { + convertProjection(fieldKeys?: string[]) { return fieldKeys?.reduce>((acc, cur) => { acc[cur] = true; return acc; diff --git a/apps/nestjs-backend/src/features/table/open-api/table-open-api.controller.ts b/apps/nestjs-backend/src/features/table/open-api/table-open-api.controller.ts index 15a0a20b0..aefaca994 100644 --- a/apps/nestjs-backend/src/features/table/open-api/table-open-api.controller.ts +++ b/apps/nestjs-backend/src/features/table/open-api/table-open-api.controller.ts @@ -2,8 +2,6 @@ import { Body, Controller, Delete, Get, Param, Post, Put, Query } from '@nestjs/common'; import type { ITableFullVo, ITableListVo, ITableVo } from '@teable/openapi'; import { - getTableQuerySchema, - IGetTableQuery, tableRoSchema, ICreateTableWithDefault, dbTableNameRoSchema, @@ -46,10 +44,9 @@ export class TableController { @Get(':tableId') async getTable( @Param('baseId') baseId: string, - @Param('tableId') tableId: string, - @Query(new ZodValidationPipe(getTableQuerySchema)) query: IGetTableQuery + @Param('tableId') tableId: string ): Promise { - return await this.tableOpenApiService.getTable(baseId, tableId, query); + return await this.tableOpenApiService.getTable(baseId, tableId); } @Permissions('table|read') diff --git a/apps/nestjs-backend/src/features/table/open-api/table-open-api.service.ts b/apps/nestjs-backend/src/features/table/open-api/table-open-api.service.ts index 3bcdf5e78..3d588a5e7 100644 --- a/apps/nestjs-backend/src/features/table/open-api/table-open-api.service.ts +++ b/apps/nestjs-backend/src/features/table/open-api/table-open-api.service.ts @@ -25,7 +25,6 @@ import type { ICreateRecordsRo, ICreateTableRo, ICreateTableWithDefault, - IGetTableQuery, ITableFullVo, ITablePermissionVo, ITableVo, @@ -161,11 +160,7 @@ export class TableOpenApiService { return await this.tableService.createTable(baseId, tableRo); } - async getTable(baseId: string, tableId: string, query: IGetTableQuery): Promise { - const { viewId, fieldKeyType, includeContent } = query; - if (includeContent) { - return await this.tableService.getFullTable(baseId, tableId, viewId, fieldKeyType); - } + async getTable(baseId: string, tableId: string): Promise { return await this.tableService.getTableMeta(baseId, tableId); } diff --git a/apps/nestjs-backend/src/features/table/table.service.ts b/apps/nestjs-backend/src/features/table/table.service.ts index 03f37bb47..e0cb50114 100644 --- a/apps/nestjs-backend/src/features/table/table.service.ts +++ b/apps/nestjs-backend/src/features/table/table.service.ts @@ -1,7 +1,6 @@ import { BadRequestException, Injectable, Logger, NotFoundException } from '@nestjs/common'; import type { IOtOperation, ISnapshotBase } from '@teable/core'; import { - FieldKeyType, generateTableId, getRandomString, getUniqName, @@ -10,7 +9,7 @@ import { } from '@teable/core'; import type { Prisma } from '@teable/db-main-prisma'; import { PrismaService } from '@teable/db-main-prisma'; -import type { ICreateTableRo, ITableFullVo, ITableVo } from '@teable/openapi'; +import type { ICreateTableRo, ITableVo } from '@teable/openapi'; import { Knex } from 'knex'; import { InjectModel } from 'nest-knexjs'; import { ClsService } from 'nestjs-cls'; @@ -22,9 +21,6 @@ import type { IClsStore } from '../../types/cls'; import { convertNameToValidCharacter } from '../../utils/name-conversion'; import { Timing } from '../../utils/timing'; import { BatchService } from '../calculation/batch.service'; -import { FieldService } from '../field/field.service'; -import { RecordService } from '../record/record.service'; -import { ViewService } from '../view/view.service'; @Injectable() export class TableService implements IReadonlyAdapterService { @@ -34,9 +30,6 @@ export class TableService implements IReadonlyAdapterService { private readonly cls: ClsService, private readonly prismaService: PrismaService, private readonly batchService: BatchService, - private readonly viewService: ViewService, - private readonly fieldService: FieldService, - private readonly recordService: RecordService, @InjectDbProvider() private readonly dbProvider: IDbProvider, @InjectModel('CUSTOM_KNEX') private readonly knex: Knex ) {} @@ -198,32 +191,6 @@ export class TableService implements IReadonlyAdapterService { }; } - async getFullTable( - baseId: string, - tableId: string, - viewId?: string, - fieldKeyType: FieldKeyType = FieldKeyType.Name - ): Promise { - const tableMeta = await this.getTableMeta(baseId, tableId); - const fields = await this.fieldService.getFieldsByQuery(tableId, { viewId }); - const views = await this.viewService.getViews(tableId); - const { records } = await this.recordService.getRecords(tableId, { - viewId, - skip: 0, - take: 50, - fieldKeyType, - }); - - return { - ...tableMeta, - description: tableMeta.description ?? undefined, - icon: tableMeta.icon ?? undefined, - fields, - views, - records, - }; - } - async getDefaultViewId(tableId: string) { const viewRaw = await this.prismaService.view.findFirst({ where: { tableId, deletedTime: null }, diff --git a/apps/nestjs-backend/src/share-db/share-db.adapter.ts b/apps/nestjs-backend/src/share-db/share-db.adapter.ts index db0d37b87..46bf5d090 100644 --- a/apps/nestjs-backend/src/share-db/share-db.adapter.ts +++ b/apps/nestjs-backend/src/share-db/share-db.adapter.ts @@ -102,17 +102,21 @@ export class ShareDbAdapter extends ShareDb.DB { callback: (error: any | null, ids: string[], extra?: any) => void ) { try { - await this.cls.runWith(this.cls.get(), async () => { - this.cls.set('cookie', options.cookie); - this.cls.set('shareViewId', options.shareId); - const [docType, collectionId] = collection.split('_'); - - const queryResult = await this.getReadonlyService(docType as IdPrefix).getDocIdsByQuery( - collectionId, - query - ); - callback(null, queryResult.ids, queryResult.extra); - }); + await this.cls.runWith( + { + ...this.cls.get(), + cookie: options.cookie, + shareViewId: options.shareId, + }, + async () => { + const [docType, collectionId] = collection.split('_'); + const queryResult = await this.getReadonlyService(docType as IdPrefix).getDocIdsByQuery( + collectionId, + query + ); + callback(null, queryResult.ids, queryResult.extra); + } + ); } catch (e) { this.logger.error(e); // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -201,18 +205,23 @@ export class ShareDbAdapter extends ShareDb.DB { options: any, callback: (err: unknown, data?: Snapshot) => void ) { - await this.cls.runWith(this.cls.get(), async () => { - this.cls.set('cookie', options.agentCustom.cookie); - this.cls.set('shareViewId', options.agentCustom.shareId); - this.getSnapshotBulk(collection, [id], projection, options, (err, data) => { - if (err) { - callback(err); - } else { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - callback(null, data![id]); - } - }); - }); + await this.cls.runWith( + { + ...this.cls.get(), + cookie: options.agentCustom.cookie, + shareViewId: options.agentCustom.shareId, + }, + async () => { + this.getSnapshotBulk(collection, [id], projection, options, (err, data) => { + if (err) { + callback(err); + } else { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + callback(null, data![id]); + } + }); + } + ); } // Get operations between [from, to) non-inclusively. (Ie, the range should diff --git a/apps/nestjs-backend/test/table-import.e2e-spec.ts b/apps/nestjs-backend/test/table-import.e2e-spec.ts index e35d5e2a3..d70c5f67f 100644 --- a/apps/nestjs-backend/test/table-import.e2e-spec.ts +++ b/apps/nestjs-backend/test/table-import.e2e-spec.ts @@ -10,7 +10,6 @@ import { notify as apiNotify, analyzeFile as apiAnalyzeFile, importTableFromFile as apiImportTableFromFile, - getTableById as apiGetTableById, createBase as apiCreateBase, createSpace as apiCreateSpace, deleteBase as apiDeleteBase, @@ -21,7 +20,7 @@ import { import * as XLSX from 'xlsx'; import { CsvImporter } from '../src/features/import/open-api/import.class'; -import { initApp, deleteTable } from './utils/init-app'; +import { initApp, deleteTable, getTable as apiGetTableById } from './utils/init-app'; enum TestFileFormat { 'CSV' = 'csv', @@ -265,17 +264,11 @@ describe('OpenAPI ImportController (e2e)', () => { name: field.name, })); - const res = await apiGetTableById(baseId, table.data[0].id, { - includeContent: true, - }); + await apiGetTableById(baseId, table.data[0].id); bases.push([baseId, id]); expect(createdFields).toEqual(assertHeaders); - expect(res).toMatchObject({ - status: 200, - statusText: 'OK', - }); } ); }); @@ -349,9 +342,7 @@ describe('OpenAPI ImportController (e2e)', () => { await delay(1000); - const { - data: { records }, - } = await apiGetTableById(baseId, tableId, { + const { records } = await apiGetTableById(baseId, tableId, { includeContent: true, }); diff --git a/apps/nestjs-backend/test/utils/init-app.ts b/apps/nestjs-backend/test/utils/init-app.ts index 580acc34a..24a4a99e6 100644 --- a/apps/nestjs-backend/test/utils/init-app.ts +++ b/apps/nestjs-backend/test/utils/init-app.ts @@ -25,8 +25,6 @@ import type { IRecordsVo, IUpdateRecordRo, ITableFullVo, - IGetTableQuery, - ITableVo, } from '@teable/openapi'; import { axios, @@ -163,17 +161,29 @@ export async function deleteTable(baseId: string, tableId: string, expectStatus? } } +type IMakeOptional = Omit & Partial>; + export async function getTable( baseId: string, tableId: string, - query: IGetTableQuery = {} -): Promise { - const result = await apiGetTableById(baseId, tableId, query); - + query?: { includeContent?: boolean; viewId?: string } +): Promise> { + const result = await apiGetTableById(baseId, tableId); + if (query?.includeContent) { + const { records } = await getRecords(tableId); + const fields = await getFields(tableId, query.viewId); + const views = await getViews(tableId); + return { + ...result.data, + records, + views, + fields, + }; + } return result.data; } -async function getCookie(email: string, password: string) { +export async function getCookie(email: string, password: string) { const sessionResponse = await apiSignin({ email, password }); return { access_token: sessionResponse.data, diff --git a/apps/nestjs-backend/test/view.e2e-spec.ts b/apps/nestjs-backend/test/view.e2e-spec.ts index 7ff2ec6b4..2e2b8efd0 100644 --- a/apps/nestjs-backend/test/view.e2e-spec.ts +++ b/apps/nestjs-backend/test/view.e2e-spec.ts @@ -2,12 +2,11 @@ import type { INestApplication } from '@nestjs/common'; import type { IColumn, IFieldRo, IFieldVo, IViewRo } from '@teable/core'; import { FieldType, Relationship, ViewType } from '@teable/core'; -import type { ICreateTableRo, ITableFullVo, ITableVo } from '@teable/openapi'; +import type { ICreateTableRo, ITableFullVo } from '@teable/openapi'; import { updateViewDescription, updateViewName, getViewFilterLinkRecords, - getTableById, updateViewShareMeta, enableShareView, } from '@teable/openapi'; @@ -21,6 +20,7 @@ import { createTable, getViews, getView, + getTable, } from './utils/init-app'; const defaultViews = [ @@ -140,7 +140,7 @@ describe('OpenAPI ViewController (e2e)', () => { }); describe('/api/table/{tableId}/view/:viewId/filter-link-records (GET)', () => { - let table: ITableVo; + let table: ITableFullVo; let linkTable1: ITableFullVo; let linkTable2: ITableFullVo; @@ -235,8 +235,7 @@ describe('OpenAPI ViewController (e2e)', () => { records: linkTable2RecordRo, }); - const tableData = await getTableById(baseId, fullTable.id, { includeContent: true }); - table = tableData.data; + table = (await getTable(baseId, fullTable.id, { includeContent: true })) as ITableFullVo; }); afterAll(async () => { diff --git a/apps/nextjs-app/src/backend/api/rest/table.ssr.ts b/apps/nextjs-app/src/backend/api/rest/table.ssr.ts index cceff73e7..251a23d8c 100644 --- a/apps/nextjs-app/src/backend/api/rest/table.ssr.ts +++ b/apps/nextjs-app/src/backend/api/rest/table.ssr.ts @@ -1,4 +1,4 @@ -import type { IFieldVo, IRecord } from '@teable/core'; +import type { IFieldVo, IGetFieldsQuery, IRecord, IViewVo } from '@teable/core'; import { FieldKeyType } from '@teable/core'; import type { AcceptInvitationLinkRo, @@ -11,6 +11,8 @@ import type { ShareViewGetVo, ITableFullVo, ITableListVo, + IRecordsVo, + ITableVo, } from '@teable/openapi'; import { ACCEPT_INVITATION_LINK, @@ -18,10 +20,12 @@ import { GET_BASE_ALL, GET_DEFAULT_VIEW_ID, GET_FIELD_LIST, + GET_RECORDS_URL, GET_RECORD_URL, GET_SPACE, GET_TABLE, GET_TABLE_LIST, + GET_VIEW_LIST, SHARE_VIEW_GET, SPACE_COLLABORATE_LIST, UPDATE_NOTIFICATION_STATUS, @@ -38,9 +42,22 @@ export class SsrApi { this.axios = getAxios(); } - async getTable(baseId: string, tableId: string, viewId?: string) { - return this.axios - .get(urlBuilder(GET_TABLE, { baseId, tableId }), { + async getTable(baseId: string, tableId: string, viewId?: string): Promise { + const { records } = await this.axios + .get(urlBuilder(GET_RECORDS_URL, { baseId, tableId }), { + params: { + viewId, + fieldKeyType: FieldKeyType.Id, + }, + }) + .then(({ data }) => data); + + const fields = await this.getFields(tableId, { viewId }); + const views = await this.axios + .get(urlBuilder(GET_VIEW_LIST, { tableId })) + .then(({ data }) => data); + const table = await this.axios + .get(urlBuilder(GET_TABLE, { baseId, tableId }), { params: { includeContent: true, viewId, @@ -48,11 +65,17 @@ export class SsrApi { }, }) .then(({ data }) => data); + return { + ...table, + records, + views, + fields, + }; } - async getFields(tableId: string) { + async getFields(tableId: string, query?: IGetFieldsQuery) { return this.axios - .get(urlBuilder(GET_FIELD_LIST, { tableId })) + .get(urlBuilder(GET_FIELD_LIST, { tableId }), { params: query }) .then(({ data }) => data); } diff --git a/apps/nextjs-app/src/features/app/blocks/import-table/field-config-panel/inplace-panel/InplaceFieldConfigPanel.tsx b/apps/nextjs-app/src/features/app/blocks/import-table/field-config-panel/inplace-panel/InplaceFieldConfigPanel.tsx index 84b84b0fc..d0c066f31 100644 --- a/apps/nextjs-app/src/features/app/blocks/import-table/field-config-panel/inplace-panel/InplaceFieldConfigPanel.tsx +++ b/apps/nextjs-app/src/features/app/blocks/import-table/field-config-panel/inplace-panel/InplaceFieldConfigPanel.tsx @@ -37,7 +37,7 @@ const InplaceFieldConfigPanel = (props: IInplaceFieldConfigPanel) => { const { data: table } = useQuery({ queryKey: ReactQueryKeys.tableInfo(base.id, tableId), - queryFn: () => apiGetTableById(base.id, tableId, {}), + queryFn: () => apiGetTableById(base.id, tableId), }); const { data: fields } = useQuery({ diff --git a/packages/openapi/src/table/create.ts b/packages/openapi/src/table/create.ts index fd1c52959..ca9baeb07 100644 --- a/packages/openapi/src/table/create.ts +++ b/packages/openapi/src/table/create.ts @@ -58,7 +58,7 @@ export const tableFullVoSchema = z export type ITableFullVo = z.infer; -export const tableVoSchema = tableFullVoSchema.partial({ +export const tableVoSchema = tableFullVoSchema.omit({ fields: true, views: true, records: true, diff --git a/packages/openapi/src/table/get-list.ts b/packages/openapi/src/table/get-list.ts index 449afa96a..3d8f5b806 100644 --- a/packages/openapi/src/table/get-list.ts +++ b/packages/openapi/src/table/get-list.ts @@ -1,31 +1,11 @@ import type { RouteConfig } from '@asteasolutions/zod-to-openapi'; -import { IdPrefix } from '@teable/core'; import { axios } from '../axios'; -import { fieldKeyTypeRoSchema } from '../record'; import { registerRoute, urlBuilder } from '../utils'; import { z } from '../zod'; import { tableListVoSchema } from './create'; export type ITableListVo = z.infer; -export const getTableQuerySchema = z.object({ - viewId: z.string().startsWith(IdPrefix.View).optional().openapi({ - description: 'Which view to get the data from.', - }), - includeContent: z - .string() - .or(z.boolean()) - .transform(Boolean) - .pipe(z.boolean()) - .optional() - .openapi({ - description: 'If true return table content. including fields, views, first 50 records.', - }), - fieldKeyType: fieldKeyTypeRoSchema, -}); - -export type IGetTableQuery = z.infer; - export const GET_TABLE_LIST = '/base/{baseId}/table'; export const GetTableListRoute: RouteConfig = registerRoute({ @@ -50,6 +30,6 @@ export const GetTableListRoute: RouteConfig = registerRoute({ tags: ['table'], }); -export const getTableList = async (baseId: string, params?: IGetTableQuery) => { - return axios.get(urlBuilder(GET_TABLE_LIST, { baseId }), { params }); +export const getTableList = async (baseId: string) => { + return axios.get(urlBuilder(GET_TABLE_LIST, { baseId })); }; diff --git a/packages/openapi/src/table/get.ts b/packages/openapi/src/table/get.ts index e90b182ac..d32fffa45 100644 --- a/packages/openapi/src/table/get.ts +++ b/packages/openapi/src/table/get.ts @@ -4,9 +4,6 @@ import { registerRoute, urlBuilder } from '../utils'; import { z } from '../zod'; import type { ITableVo } from './create'; import { tableVoSchema } from './create'; -import type { IGetTableQuery } from './get-list'; -import { getTableQuerySchema } from './get-list'; - export const GET_TABLE = '/base/{baseId}/table/{tableId}'; export const GetTableRoute: RouteConfig = registerRoute({ @@ -18,7 +15,6 @@ export const GetTableRoute: RouteConfig = registerRoute({ baseId: z.string(), tableId: z.string(), }), - query: getTableQuerySchema, }, responses: { 200: { @@ -33,14 +29,11 @@ export const GetTableRoute: RouteConfig = registerRoute({ tags: ['table'], }); -export const getTableById = async (baseId: string, tableId: string, query: IGetTableQuery) => { +export const getTableById = async (baseId: string, tableId: string) => { return axios.get( urlBuilder(GET_TABLE, { baseId, tableId, - }), - { - params: query, - } + }) ); };