From 62792329c3363ecc86be44efea945cd294dca51b Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Thu, 20 Oct 2022 21:24:14 +0200 Subject: [PATCH] (core) DocApi meta endpoints: GET /tables and POST/PATCH /tables and /columns Summary: Adds new API endpoints to list tables in a document and create or modify tables and columns. The request and response formats are designed to mirror the style of the existing `GET /columns` and `GET/POST/PATCH /records` endpoints. Discussion: https://grist.slack.com/archives/C0234CPPXPA/p1665139807125649?thread_ts=1628957179.010500&cid=C0234CPPXPA Test Plan: DocApi test Reviewers: jarek Reviewed By: jarek Subscribers: paulfitz Differential Revision: https://phab.getgrist.com/D3667 --- app/plugin/DocApiTypes-ti.ts | 41 ++++++ app/plugin/DocApiTypes.ts | 38 +++++ app/plugin/TableOperationsImpl.ts | 4 +- app/server/lib/DocApi.ts | 110 +++++++++++++- test/server/lib/DocApi.ts | 232 ++++++++++++++++++++++++++++++ 5 files changed, 417 insertions(+), 8 deletions(-) diff --git a/app/plugin/DocApiTypes-ti.ts b/app/plugin/DocApiTypes-ti.ts index 70ab13e6..0f6c26dc 100644 --- a/app/plugin/DocApiTypes-ti.ts +++ b/app/plugin/DocApiTypes-ti.ts @@ -10,6 +10,13 @@ export const NewRecord = t.iface([], { })), }); +export const NewRecordWithStringId = t.iface([], { + "id": t.opt("string"), + "fields": t.opt(t.iface([], { + [t.indexKey]: "CellValue", + })), +}); + export const Record = t.iface([], { "id": "number", "fields": t.iface([], { @@ -17,6 +24,13 @@ export const Record = t.iface([], { }), }); +export const RecordWithStringId = t.iface([], { + "id": "string", + "fields": t.iface([], { + [t.indexKey]: "CellValue", + }), +}); + export const AddOrUpdateRecord = t.iface([], { "require": t.intersection(t.iface([], { [t.indexKey]: "CellValue", @@ -46,14 +60,41 @@ export const MinimalRecord = t.iface([], { "id": "number", }); +export const ColumnsPost = t.iface([], { + "columns": t.tuple("NewRecordWithStringId", t.rest(t.array("NewRecordWithStringId"))), +}); + +export const ColumnsPatch = t.iface([], { + "columns": t.tuple("RecordWithStringId", t.rest(t.array("RecordWithStringId"))), +}); + +export const TablePost = t.iface(["ColumnsPost"], { + "id": t.opt("string"), +}); + +export const TablesPost = t.iface([], { + "tables": t.tuple("TablePost", t.rest(t.array("TablePost"))), +}); + +export const TablesPatch = t.iface([], { + "tables": t.tuple("RecordWithStringId", t.rest(t.array("RecordWithStringId"))), +}); + const exportedTypeSuite: t.ITypeSuite = { NewRecord, + NewRecordWithStringId, Record, + RecordWithStringId, AddOrUpdateRecord, RecordsPatch, RecordsPost, RecordsPut, RecordId, MinimalRecord, + ColumnsPost, + ColumnsPatch, + TablePost, + TablesPost, + TablesPatch, }; export default exportedTypeSuite; diff --git a/app/plugin/DocApiTypes.ts b/app/plugin/DocApiTypes.ts index 97e832b5..ea9b8cb7 100644 --- a/app/plugin/DocApiTypes.ts +++ b/app/plugin/DocApiTypes.ts @@ -11,6 +11,15 @@ export interface NewRecord { fields?: { [coldId: string]: CellValue }; } +export interface NewRecordWithStringId { + id?: string; // tableId or colId + /** + * Initial values of cells in record. Optional, if not set cells are left + * blank. + */ + fields?: { [coldId: string]: CellValue }; +} + /** * JSON schema for api /record endpoint. Used in PATCH method for updating existing records. */ @@ -19,6 +28,11 @@ export interface Record { fields: { [coldId: string]: CellValue }; } +export interface RecordWithStringId { + id: string; // tableId or colId + fields: { [coldId: string]: CellValue }; +} + /** * JSON schema for api /record endpoint. Used in PUT method for adding or updating records. */ @@ -65,3 +79,27 @@ export type RecordId = number; export interface MinimalRecord { id: number } + +export interface ColumnsPost { + columns: [NewRecordWithStringId, ...NewRecordWithStringId[]]; // at least one column is required +} + +export interface ColumnsPatch { + columns: [RecordWithStringId, ...RecordWithStringId[]]; // at least one column is required +} + +/** + * Creating tables requires a list of columns. + * `fields` is not accepted because it's not generally sensible to set the metadata fields on new tables. + */ +export interface TablePost extends ColumnsPost { + id?: string; +} + +export interface TablesPost { + tables: [TablePost, ...TablePost[]]; // at least one table is required +} + +export interface TablesPatch { + tables: [RecordWithStringId, ...RecordWithStringId[]]; // at least one table is required +} diff --git a/app/plugin/TableOperationsImpl.ts b/app/plugin/TableOperationsImpl.ts index 183e0f64..22af81d8 100644 --- a/app/plugin/TableOperationsImpl.ts +++ b/app/plugin/TableOperationsImpl.ts @@ -199,7 +199,9 @@ export async function handleSandboxErrorOnPlatform( if (match) { platform.throwError('', `Invalid row id ${match[1]}`, 400); } - match = message.match(/\[Sandbox] KeyError u?'(?:Table \w+ has no column )?(\w+)'/); + match = message.match( + /\[Sandbox] (?:KeyError u?'(?:Table \w+ has no column )?|ValueError No such table: )(\w+)/ + ); if (match) { if (match[1] === tableId) { platform.throwError('', `Table not found "${tableId}"`, 404); diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index 36de1773..eff8cc4e 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -32,7 +32,7 @@ import {DocManager} from "app/server/lib/DocManager"; import {docSessionFromRequest, makeExceptionalDocSession, OptDocSession} from "app/server/lib/DocSession"; import {DocWorker} from "app/server/lib/DocWorker"; import {IDocWorkerMap} from "app/server/lib/DocWorkerMap"; -import {parseExportParameters, DownloadOptions} from "app/server/lib/Export"; +import {DownloadOptions, parseExportParameters} from "app/server/lib/Export"; import {downloadCSV} from "app/server/lib/ExportCSV"; import {downloadXLSX} from "app/server/lib/ExportXLSX"; import {expressWrap} from 'app/server/lib/expressWrap'; @@ -84,10 +84,15 @@ const MAX_ACTIVE_DOCS_USAGE_CACHE = 1000; type WithDocHandler = (activeDoc: ActiveDoc, req: RequestWithLogin, resp: Response) => Promise; // Schema validators for api endpoints that creates or updates records. -const {RecordsPatch, RecordsPost, RecordsPut} = t.createCheckers(DocApiTypesTI, GristDataTI); -RecordsPatch.setReportedPath("body"); -RecordsPost.setReportedPath("body"); -RecordsPut.setReportedPath("body"); +const { + RecordsPatch, RecordsPost, RecordsPut, + ColumnsPost, ColumnsPatch, + TablesPost, TablesPatch, +} = t.createCheckers(DocApiTypesTI, GristDataTI); + +for (const checker of [RecordsPatch, RecordsPost, RecordsPut, ColumnsPost, ColumnsPatch, TablesPost, TablesPatch]) { + checker.setReportedPath("body"); +} /** * Middleware for validating request's body with a Checker instance. @@ -223,6 +228,21 @@ export class DocWorkerApi { }) ); + // Get the tables of the specified document in recordish format + this._app.get('/api/docs/:docId/tables', canView, + withDoc(async (activeDoc, req, res) => { + const records = await getTableRecords(activeDoc, req, "_grist_Tables"); + const tables = records.map((record) => ({ + id: record.fields.tableId, + fields: { + ..._.omit(record.fields, "tableId"), + tableRef: record.id, + } + })).filter(({id}) => id); + res.json({tables}); + }) + ); + // The upload should be a multipart post with an 'upload' field containing one or more files. // Returns the list of rowIds for the rows created in the _grist_Attachments table. this._app.post('/api/docs/:docId/attachments', canEdit, withDoc(async (activeDoc, req, res) => { @@ -328,6 +348,40 @@ export class DocWorkerApi { }) ); + // Create columns in a table, given as records of the _grist_Tables_column metatable. + this._app.post('/api/docs/:docId/tables/:tableId/columns', canEdit, validate(ColumnsPost), + withDoc(async (activeDoc, req, res) => { + const body = req.body as Types.ColumnsPost; + const {tableId} = req.params; + const actions = body.columns.map(({fields, id: colId}) => + // AddVisibleColumn adds the column to all widgets of the table. + // This isn't necessarily what the user wants, but it seems like a good default. + // Maybe there should be a query param to control this? + ["AddVisibleColumn", tableId, colId, fields || {}] + ); + const {retValues} = await handleSandboxError(tableId, [], + activeDoc.applyUserActions(docSessionFromRequest(req), actions) + ); + const columns = retValues.map(({colId}) => ({id: colId})); + res.json({columns}); + }) + ); + + // Create new tables in a doc. Unlike POST /records or /columns, each 'record' (table) should have a `columns` + // property in the same format as POST /columns above, and no `fields` property. + this._app.post('/api/docs/:docId/tables', canEdit, validate(TablesPost), + withDoc(async (activeDoc, req, res) => { + const body = req.body as Types.TablesPost; + const actions = body.tables.map(({columns, id}) => { + const colInfos = columns.map(({fields, id: colId}) => ({...fields, id: colId})); + return ["AddTable", id, colInfos]; + }); + const {retValues} = await activeDoc.applyUserActions(docSessionFromRequest(req), actions); + const tables = retValues.map(({table_id}) => ({id: table_id})); + res.json({tables}); + }) + ); + this._app.post('/api/docs/:docId/tables/:tableId/data/delete', canEdit, withDoc(async (activeDoc, req, res) => { const rowIds = req.body; const op = getTableOperations(req, activeDoc); @@ -409,6 +463,48 @@ export class DocWorkerApi { }) ); + // Update columns given in records format + this._app.patch('/api/docs/:docId/tables/:tableId/columns', canEdit, validate(ColumnsPatch), + withDoc(async (activeDoc, req, res) => { + const tablesTable = activeDoc.docData!.getMetaTable("_grist_Tables"); + const columnsTable = activeDoc.docData!.getMetaTable("_grist_Tables_column"); + const {tableId} = req.params; + const tableRef = tablesTable.findMatchingRowId({tableId}); + if (!tableRef) { + throw new ApiError(`Table not found "${tableId}"`, 404); + } + const body = req.body as Types.ColumnsPatch; + const columns: Types.Record[] = body.columns.map((col) => { + const id = columnsTable.findMatchingRowId({parentId: tableRef, colId: col.id}); + if (!id) { + throw new ApiError(`Column not found "${col.id}"`, 404); + } + return {...col, id}; + }); + const ops = getTableOperations(req, activeDoc, "_grist_Tables_column"); + await ops.update(columns); + res.json(null); + }) + ); + + // Update tables given in records format + this._app.patch('/api/docs/:docId/tables', canEdit, validate(TablesPatch), + withDoc(async (activeDoc, req, res) => { + const tablesTable = activeDoc.docData!.getMetaTable("_grist_Tables"); + const body = req.body as Types.TablesPatch; + const tables: Types.Record[] = body.tables.map((table) => { + const id = tablesTable.findMatchingRowId({tableId: table.id}); + if (!id) { + throw new ApiError(`Table not found "${table.id}"`, 404); + } + return {...table, id}; + }); + const ops = getTableOperations(req, activeDoc, "_grist_Tables"); + await ops.update(tables); + res.json(null); + }) + ); + // Add or update records given in records format this._app.put('/api/docs/:docId/tables/:tableId/records', canEdit, validate(RecordsPut), withDoc(async (activeDoc, req, res) => { @@ -1198,12 +1294,12 @@ function getErrorPlatform(tableId: string): TableOperationsPlatform { }; } -function getTableOperations(req: RequestWithLogin, activeDoc: ActiveDoc): TableOperationsImpl { +function getTableOperations(req: RequestWithLogin, activeDoc: ActiveDoc, tableId?: string): TableOperationsImpl { const options: OpOptions = { parseStrings: !isAffirmative(req.query.noparse) }; const platform: TableOperationsPlatform = { - ...getErrorPlatform(req.params.tableId), + ...getErrorPlatform(tableId ?? req.params.tableId), applyUserActions(actions, opts) { if (!activeDoc) { throw new Error('no document'); } return activeDoc.applyUserActions( diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index f1f1d71a..9e6af838 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -518,6 +518,238 @@ function testDocApi() { ); }); + it("GET/POST/PATCH /docs/{did}/tables and /columns", async function() { + // POST /tables: Create new tables + let resp = await axios.post(`${serverUrl}/api/docs/${docIds.Timesheets}/tables`, { + tables: [ + {columns: [{}]}, // The minimal allowed request + {id: "", columns: [{id: ""}]}, + {id: "NewTable1", columns: [{id: "NewCol1", fields: {}}]}, + { + id: "NewTable2", + columns: [ + {id: "NewCol2", fields: {label: "Label2"}}, + {id: "NewCol3", fields: {label: "Label3"}}, + {id: "NewCol3", fields: {label: "Label3"}}, // Duplicate column id + ] + }, + { + id: "NewTable2", // Create a table with duplicate tableId + columns: [ + {id: "NewCol2", fields: {label: "Label2"}}, + {id: "NewCol3", fields: {label: "Label3"}}, + ] + }, + ] + }, chimpy); + assert.equal(resp.status, 200); + assert.deepEqual(resp.data, { + tables: [ + {id: "Table2"}, + {id: "Table3"}, + {id: "NewTable1"}, + {id: "NewTable2"}, + {id: "NewTable2_2"}, // duplicated tableId ends with _2 + ] + }); + + // POST /columns: Create new columns + resp = await axios.post(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/NewTable2/columns`, { + columns: [ + {}, + {id: ""}, + {id: "NewCol4", fields: {}}, + {id: "NewCol4", fields: {}}, // Create a column with duplicate colId + {id: "NewCol5", fields: {label: "Label5"}}, + ], + }, chimpy); + assert.equal(resp.status, 200); + assert.deepEqual(resp.data, { + columns: [ + {id: "A"}, + {id: "B"}, + {id: "NewCol4"}, + {id: "NewCol4_2"}, // duplicated colId ends with _2 + {id: "NewCol5"}, + ] + }); + + // POST /columns to invalid table ID + resp = await axios.post(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/NoSuchTable/columns`, + {columns: [{}]}, chimpy); + assert.equal(resp.status, 404); + assert.deepEqual(resp.data, {error: 'Table not found "NoSuchTable"'}); + + // PATCH /tables: Modify a table. This is pretty much only good for renaming tables. + resp = await axios.patch(`${serverUrl}/api/docs/${docIds.Timesheets}/tables`, { + tables: [ + {id: "Table3", fields: {tableId: "Table3_Renamed"}}, + ] + }, chimpy); + assert.equal(resp.status, 200); + + // Repeat the same operation to check that it gives 404 if the table doesn't exist. + resp = await axios.patch(`${serverUrl}/api/docs/${docIds.Timesheets}/tables`, { + tables: [ + {id: "Table3", fields: {tableId: "Table3_Renamed"}}, + ] + }, chimpy); + assert.equal(resp.status, 404); + assert.deepEqual(resp.data, {error: 'Table not found "Table3"'}); + + // PATCH /columns: Modify a column. + resp = await axios.patch(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table2/columns`, { + columns: [ + {id: "A", fields: {colId: "A_Renamed"}}, + ] + }, chimpy); + assert.equal(resp.status, 200); + + // Repeat the same operation to check that it gives 404 if the column doesn't exist. + resp = await axios.patch(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table2/columns`, { + columns: [ + {id: "A", fields: {colId: "A_Renamed"}}, + ] + }, chimpy); + assert.equal(resp.status, 404); + assert.deepEqual(resp.data, {error: 'Column not found "A"'}); + + // Repeat the same operation to check that it gives 404 if the table doesn't exist. + resp = await axios.patch(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table222/columns`, { + columns: [ + {id: "A", fields: {colId: "A_Renamed"}}, + ] + }, chimpy); + assert.equal(resp.status, 404); + assert.deepEqual(resp.data, {error: 'Table not found "Table222"'}); + + // Rename NewTable2.A -> B to test the name conflict resolution. + resp = await axios.patch(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/NewTable2/columns`, { + columns: [ + {id: "A", fields: {colId: "B"}}, + ] + }, chimpy); + assert.equal(resp.status, 200); + + // Hide NewTable2.NewCol5 and NewTable2_2 with ACL + resp = await axios.post(`${serverUrl}/api/docs/${docIds.Timesheets}/apply`, [ + ['AddRecord', '_grist_ACLResources', -1, {tableId: 'NewTable2', colIds: 'NewCol5'}], + ['AddRecord', '_grist_ACLResources', -2, {tableId: 'NewTable2_2', colIds: '*'}], + ['AddRecord', '_grist_ACLRules', null, { + resource: -1, aclFormula: '', permissionsText: '-R', + }], + ['AddRecord', '_grist_ACLRules', null, { + // Don't use permissionsText: 'none' here because we need S permission to delete the table at the end. + resource: -2, aclFormula: '', permissionsText: '-R', + }], + ], chimpy); + assert.equal(resp.status, 200); + + // GET /tables: Check that the tables were created and renamed. + resp = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables`, chimpy); + assert.equal(resp.status, 200); + assert.deepEqual(resp.data, + { + "tables": [ + { + "id": "Table1", + "fields": { + "rawViewSectionRef": 2, + "primaryViewId": 1, + "onDemand": false, + "summarySourceTable": 0, + "tableRef": 1 + } + }, + // New tables start here + { + "id": "Table2", + "fields": { + "rawViewSectionRef": 4, + "primaryViewId": 2, + "onDemand": false, + "summarySourceTable": 0, + "tableRef": 2 + } + }, + { + "id": "Table3_Renamed", + "fields": { + "rawViewSectionRef": 6, + "primaryViewId": 3, + "onDemand": false, + "summarySourceTable": 0, + "tableRef": 3 + } + }, + { + "id": "NewTable1", + "fields": { + "rawViewSectionRef": 8, + "primaryViewId": 4, + "onDemand": false, + "summarySourceTable": 0, + "tableRef": 4 + } + }, + { + "id": "NewTable2", + "fields": { + "rawViewSectionRef": 10, + "primaryViewId": 5, + "onDemand": false, + "summarySourceTable": 0, + "tableRef": 5 + } + }, + // NewTable2_2 is hidden by ACL + ] + } + ); + + // Check the created columns. + // TODO these columns should probably be included in the GET /tables response. + async function checkColumns(tableId: string, expected: { colId: string, label: string }[]) { + const colsResp = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/${tableId}/columns`, chimpy); + assert.equal(colsResp.status, 200); + const actual = colsResp.data.columns.map((c: any) => ({ + colId: c.id, + label: c.fields.label, + })); + assert.deepEqual(actual, expected); + } + + await checkColumns("Table2", [ + {colId: "A_Renamed", label: 'A'}, + ]); + await checkColumns("Table3_Renamed", [ + {colId: "A", label: 'A'}, + ]); + await checkColumns("NewTable1", [ + {colId: "NewCol1", label: 'NewCol1'}, + ]); + await checkColumns("NewTable2", [ + {colId: "NewCol2", label: 'Label2'}, + {colId: "NewCol3", label: 'Label3'}, + {colId: "NewCol3_2", label: 'Label3'}, + {colId: "B2", label: 'A'}, // Result of renaming A -> B + {colId: "B", label: 'B'}, + {colId: "NewCol4", label: 'NewCol4'}, + {colId: "NewCol4_2", label: 'NewCol4_2'}, + // NewCol5 is hidden by ACL + ]); + + resp = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/NewTable2_2/columns`, chimpy); + assert.equal(resp.status, 404); + assert.deepEqual(resp.data, {error: 'Table not found "NewTable2_2"'}); // hidden by ACL + + // Clean up the created tables for other tests + // TODO add a DELETE endpoint for /tables and /columns. Probably best to do alongside DELETE /records. + resp = await axios.post(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/_grist_Tables/data/delete`, + [2, 3, 4, 5, 6], chimpy); + assert.equal(resp.status, 200); + }); + it("GET /docs/{did}/tables/{tid}/data returns 404 for non-existent doc", async function() { const resp = await axios.get(`${serverUrl}/api/docs/typotypotypo/tables/Table1/data`, chimpy); assert.equal(resp.status, 404);