(core) Adding schema validation for records endpoint

Summary:
Adding validation for api /records endpoint, that checks if the json payload is valid.
Modifying POST /records endpoint to allow creating blank or partial records.

Test Plan: Updated tests

Reviewers: alexmojaki

Reviewed By: alexmojaki

Differential Revision: https://phab.getgrist.com/D3061

app/plugin/CustomSectionAPI-ti.ts

@@ -1,3 +1,6 @@
+/**
+ * This module was automatically generated by `ts-interface-builder`
+ */
 import * as t from "ts-interface-checker";
 // tslint:disable:object-literal-key-quotes
 

app/plugin/FileParserAPI-ti.ts

@@ -1,3 +1,6 @@
+/**
+ * This module was automatically generated by `ts-interface-builder`
+ */
 import * as t from "ts-interface-checker";
 // tslint:disable:object-literal-key-quotes
 

app/plugin/GristData-ti.ts

@@ -0,0 +1,34 @@
+/**
+ * This module was automatically generated by `ts-interface-builder`
+ */
+import * as t from "ts-interface-checker";
+// tslint:disable:object-literal-key-quotes
+
+export const GristObjCode = t.enumtype({
+  "List": "L",
+  "Dict": "O",
+  "DateTime": "D",
+  "Date": "d",
+  "Skip": "S",
+  "Censored": "C",
+  "Reference": "R",
+  "ReferenceList": "r",
+  "Exception": "E",
+  "Pending": "P",
+  "Unmarshallable": "U",
+  "Versions": "V",
+});
+
+export const CellValue = t.union("number", "string", "boolean", "null", t.tuple("GristObjCode", t.rest(t.array("unknown"))));
+
+export const RowRecord = t.iface([], {
+  "id": "number",
+  [t.indexKey]: "CellValue",
+});
+
+const exportedTypeSuite: t.ITypeSuite = {
+  GristObjCode,
+  CellValue,
+  RowRecord,
+};
+export default exportedTypeSuite;

app/plugin/InternalImportSourceAPI-ti.ts

@@ -1,3 +1,6 @@
+/**
+ * This module was automatically generated by `ts-interface-builder`
+ */
 import * as t from "ts-interface-checker";
 // tslint:disable:object-literal-key-quotes
 

app/plugin/RenderOptions-ti.ts

@@ -1,3 +1,6 @@
+/**
+ * This module was automatically generated by `ts-interface-builder`
+ */
 import * as t from "ts-interface-checker";
 // tslint:disable:object-literal-key-quotes
 

app/plugin/StorageAPI-ti.ts

@@ -1,3 +1,6 @@
+/**
+ * This module was automatically generated by `ts-interface-builder`
+ */
 import * as t from "ts-interface-checker";
 // tslint:disable:object-literal-key-quotes
 

app/server/lib/DocApi.ts

@@ -2,12 +2,13 @@ import { createEmptyActionSummary } from "app/common/ActionSummary";
 import { ApiError } from 'app/common/ApiError';
 import { BrowserSettings } from "app/common/BrowserSettings";
 import {
-  CellValue, fromTableDataAction, TableColValues, TableRecordValue,
+  BulkColValues, CellValue, fromTableDataAction, TableColValues, TableRecordValue,
 } from 'app/common/DocActions';
 import {isRaisedException} from "app/common/gristTypes";
 import { arrayRepeat, isAffirmative } from "app/common/gutil";
 import { SortFunc } from 'app/common/SortFunc';
 import { DocReplacementOptions, DocState, DocStateComparison, DocStates, NEW_DOCUMENT_CODE} from 'app/common/UserAPI';
+import GristDataTI from 'app/plugin/GristData-ti';
 import { HomeDBManager, makeDocAuthResult } from 'app/gen-server/lib/HomeDBManager';
 import { concatenateSummaries, summarizeAction } from "app/server/lib/ActionSummary";
 import { ActiveDoc, tableIdToRef } from "app/server/lib/ActiveDoc";
@@ -34,12 +35,16 @@ import { SandboxError } from "app/server/lib/sandboxUtil";
 import {localeFromRequest} from "app/server/lib/ServerLocale";
 import {allowedEventTypes, isUrlAllowed, WebhookAction, WebHookSecret} from "app/server/lib/Triggers";
 import { handleOptionalUpload, handleUpload } from "app/server/lib/uploads";
+import DocApiTypesTI from "app/server/lib/DocApiTypes-ti";
+import * as Types from "app/server/lib/DocApiTypes";
 import * as contentDisposition from 'content-disposition';
 import { Application, NextFunction, Request, RequestHandler, Response } from "express";
 import * as _ from "lodash";
 import fetch from 'node-fetch';
 import * as path from 'path';
 import * as uuidv4 from "uuid/v4";
+import * as t from "ts-interface-checker";
+import { Checker } from "ts-interface-checker";
 
 // Cap on the number of requests that can be outstanding on a single document via the
 // rest doc api.  When this limit is exceeded, incoming requests receive an immediate
@@ -48,6 +53,29 @@ const MAX_PARALLEL_REQUESTS_PER_DOC = 10;
 
 type WithDocHandler = (activeDoc: ActiveDoc, req: RequestWithLogin, resp: Response) => Promise<void>;
 
+// Schema validators for api endpoints that creates or updates records.
+const {RecordsPatch, RecordsPost} = t.createCheckers(DocApiTypesTI, GristDataTI);
+RecordsPatch.setReportedPath("body");
+RecordsPost.setReportedPath("body");
+
+/**
+ * Middleware for validating request's body with a Checker instance.
+ */
+function validate(checker: Checker): RequestHandler {
+  return (req, res, next) => {
+    try {
+      checker.check(req.body);
+    } catch(err) {
+      res.status(400).json({
+        error : "Invalid payload",
+        details: String(err)
+      }).end();
+      return;
+    }
+    next();
+  };
+}
+
 /**
  * Middleware to track the number of requests outstanding on each document, and to
  * throw an exception when the maximum number of requests are already outstanding.
@@ -209,15 +237,17 @@ export class DocWorkerApi {
         .send(fileData);
     }));
 
+    /**
+     * Adds records to a table. If columnValues is an empty object (or not provided) it will create empty records.
+     * @param columnValues Optional values for fields (can be an empty object to add empty records)
+     * @param count Number of records to add
+     */
     async function addRecords(
-      req: RequestWithLogin, activeDoc: ActiveDoc, columnValues: {[colId: string]: CellValue[]}
+      req: RequestWithLogin, activeDoc: ActiveDoc, count: number, columnValues: BulkColValues
     ): Promise<number[]> {
       const tableId = req.params.tableId;
       const colNames = Object.keys(columnValues);
-      // user actions expect [null, ...] as row ids, first let's figure the number of items to add by
+      // user actions expect [null, ...] as row ids
-      // looking at the length of a column
-      const count = columnValues[colNames[0]].length;
-      // then, let's create [null, ...]
       const rowIds = arrayRepeat(count, null);
       const sandboxRes = await handleSandboxError(tableId, colNames, activeDoc.applyUserActions(
         docSessionFromRequest(req),
@@ -225,24 +255,44 @@ export class DocWorkerApi {
       return sandboxRes.retValues[0];
     }
 
-    function recordFieldsToColValues(fields: {[colId: string]: CellValue}[]): {[colId: string]: CellValue[]} {
+    function areSameFields(records: Array<Types.Record | Types.NewRecord>) {
-      return _.mapValues(fields[0], (_value, key) => _.map(fields, key));
+      const recordsFields = records.map(r => new Set(Object.keys(r.fields || {})));
+      const firstFields = recordsFields[0];
+      const allSame = recordsFields.every(s => _.isEqual(firstFields, s));
+      return allSame;
+    }
+
+    function convertToBulkColValues(records: Array<Types.Record | Types.NewRecord>): BulkColValues {
+      // User might want to create empty records, without providing a field name, for example for requests:
+      // { records: [{}] }; { records: [{fields:{}}] }
+      // Retrieve all field names from fields property.
+      const fieldNames = new Set<string>(_.flatMap(records, r => Object.keys(r.fields ?? {})));
+      const result: BulkColValues = {};
+      for (const fieldName of fieldNames) {
+        result[fieldName] = records.map(record => record.fields?.[fieldName] || null);
+      }
+      return result;
     }
 
     // Adds records given in a column oriented format,
     // returns an array of row IDs
     this._app.post('/api/docs/:docId/tables/:tableId/data', canEdit,
       withDoc(async (activeDoc, req, res) => {
-        const ids = await addRecords(req, activeDoc, req.body);
+        const colValues = req.body as BulkColValues;
+        const count = colValues[Object.keys(colValues)[0]].length;
+        const ids = await addRecords(req, activeDoc, count, colValues);
         res.json(ids);
       })
     );
 
     // Adds records given in a record oriented format,
     // returns in the same format as GET /records but without the fields object for now
-    this._app.post('/api/docs/:docId/tables/:tableId/records', canEdit,
+    this._app.post('/api/docs/:docId/tables/:tableId/records', canEdit, validate(RecordsPost),
       withDoc(async (activeDoc, req, res) => {
-        const ids = await addRecords(req, activeDoc, recordFieldsToColValues(_.map(req.body.records, 'fields')));
+        const body = req.body as Types.RecordsPost;
+        const postRecords = convertToBulkColValues(body.records);
+        // postRecords can be an empty object, in that case we will create empty records.
+        const ids = await addRecords(req, activeDoc, body.records.length, postRecords);
         const records = ids.map(id => ({id}));
         res.json({records});
       })
@@ -333,11 +383,18 @@ export class DocWorkerApi {
     );
 
     // Update records given in records format
-    this._app.patch('/api/docs/:docId/tables/:tableId/records', canEdit,
+    this._app.patch('/api/docs/:docId/tables/:tableId/records', canEdit, validate(RecordsPatch),
       withDoc(async (activeDoc, req, res) => {
-        const records = req.body.records;
+        const body = req.body as Types.RecordsPatch;
-        const rowIds = _.map(records, 'id');
+        const rowIds = _.map(body.records, r => r.id);
-        const columnValues = recordFieldsToColValues(_.map(records, 'fields'));
+        if (!areSameFields(body.records)) {
+          throw new ApiError("PATCH requires all records to have same fields", 400);
+        }
+        const columnValues = convertToBulkColValues(body.records);
+        if (!rowIds.length || !columnValues) {
+          // For patch method, we require at least one valid record.
+          throw new ApiError("PATCH requires a valid record object", 400);
+        }
         await updateRecords(req, activeDoc, columnValues, rowIds);
         res.json(null);
       })
@@ -479,7 +536,7 @@ export class DocWorkerApi {
       }
       if (req.body.select === 'unlisted') {
         // Remove any snapshots not listed in inventory.  Ideally, there should be no
-        // snapshots, and this undocument feature is just for fixing up problems.
+        // snapshots, and this undocumented feature is just for fixing up problems.
         const full = (await activeDoc.getSnapshots(true)).snapshots.map(s => s.snapshotId);
         const listed = new Set((await activeDoc.getSnapshots()).snapshots.map(s => s.snapshotId));
         const unlisted = full.filter(snapshotId => !listed.has(snapshotId));
@@ -525,7 +582,7 @@ export class DocWorkerApi {
       const activeDoc = await this._getActiveDoc(req);
       await activeDoc.flushDoc();
       // flushDoc terminates once there's no pending operation on the document.
-      // There could still be async operations in progess.  We mute their effect,
+      // There could still be async operations in progress.  We mute their effect,
       // as if they never happened.
       activeDoc.docClients.interruptAllClients();
       activeDoc.setMuted();

app/server/lib/DocApiTypes-ti.ts

@@ -0,0 +1,34 @@
+/**
+ * This module was automatically generated by `ts-interface-builder`
+ */
+import * as t from "ts-interface-checker";
+// tslint:disable:object-literal-key-quotes
+
+export const NewRecord = t.iface([], {
+  "fields": t.opt(t.iface([], {
+    [t.indexKey]: "CellValue",
+  })),
+});
+
+export const Record = t.iface([], {
+  "id": "number",
+  "fields": t.iface([], {
+    [t.indexKey]: "CellValue",
+  }),
+});
+
+export const RecordsPatch = t.iface([], {
+  "records": t.tuple("Record", t.rest(t.array("Record"))),
+});
+
+export const RecordsPost = t.iface([], {
+  "records": t.tuple("NewRecord", t.rest(t.array("NewRecord"))),
+});
+
+const exportedTypeSuite: t.ITypeSuite = {
+  NewRecord,
+  Record,
+  RecordsPatch,
+  RecordsPost,
+};
+export default exportedTypeSuite;

app/server/lib/DocApiTypes.ts

@@ -0,0 +1,30 @@
+import { CellValue } from "app/plugin/GristData";
+
+/**
+ * JSON schema for api /record endpoint. Used in POST method for adding new records.
+ */
+export interface NewRecord {
+  fields?: { [coldId: string]: CellValue }; // fields is optional, user can create blank records
+}
+
+/**
+ * JSON schema for api /record endpoint. Used in PATCH method for updating existing records.
+ */
+export interface Record {
+  id: number;
+  fields: { [coldId: string]: CellValue };
+}
+
+/**
+ * JSON schema for the body of api /record PATCH endpoint
+ */
+export interface RecordsPatch {
+  records: [Record, ...Record[]]; // at least one record is required
+}
+
+/**
+ * JSON schema for the body of api /record POST endpoint
+ */
+export interface RecordsPost {
+  records: [NewRecord, ...NewRecord[]]; // at least one record is required
+}

package.json

@@ -95,7 +95,7 @@
     "express": "4.16.4",
     "file-type": "14.1.4",
     "fs-extra": "7.0.0",
-    "grain-rpc": "0.1.6",
+    "grain-rpc": "0.1.7",
     "grainjs": "1.0.1",
     "highlight.js": "9.13.1",
     "i18n-iso-countries": "6.1.0",
@@ -121,7 +121,7 @@
     "saml2-js": "2.0.3",
     "short-uuid": "3.1.1",
     "tmp": "0.0.33",
-    "ts-interface-checker": "0.1.6",
+    "ts-interface-checker": "1.0.2",
     "typeorm": "0.2.18",
     "uuid": "3.3.2",
     "winston": "2.4.5",
@@ -129,6 +129,6 @@
   },
   "resolutions": {
     "jquery": "2.2.1",
-    "ts-interface-checker": "0.1.6"
+    "ts-interface-checker": "1.0.2"
   }
 }