mirror of
https://github.com/gristlabs/grist-core.git
synced 2024-10-27 20:44:07 +00:00
(core) Parse string cell values in Doc API and Imports
Summary: - Adds a function `parseUserAction` for parsing strings in UserActions to `ValueParser.ts` - Adds a boolean option `parseStrings` to use `parseUserAction` in `ActiveDoc.applyUserActions`, off by default. - Uses `parseStrings` by default in DocApi (set `?noparse=true` in a request to disable) when adding/updating records through the `/data` or `/records` endpoints or in general with the `/apply` endpoint. - Uses `parseStrings` for various actions in `ActiveDocImport`. Since most types are parsed in Python before these actions are constructed, this only affects references, which still look like errors in the import preview. Importing references can also easily still run into more complicated problems discussed in https://grist.slack.com/archives/C0234CPPXPA/p1639514844028200 Test Plan: - Added tests to DocApi to compare behaviour with and without string parsing. - Added a new browser test, fixture doc, and fixture CSV to test importing a file containing references. Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3183
This commit is contained in:
parent
9d62e67369
commit
d1a848b44a
@ -11,6 +11,7 @@ export interface ApplyUAOptions {
|
|||||||
otherId?: number; // For undo/redo; the actionNum of the original action to which it applies.
|
otherId?: number; // For undo/redo; the actionNum of the original action to which it applies.
|
||||||
linkId?: number; // For bundled actions, actionNum of the previous action in the bundle.
|
linkId?: number; // For bundled actions, actionNum of the previous action in the bundle.
|
||||||
bestEffort?: boolean; // If set, action may be applied in part if it cannot be applied completely.
|
bestEffort?: boolean; // If set, action may be applied in part if it cannot be applied completely.
|
||||||
|
parseStrings?: boolean; // If true, parses string values in some actions based on the column
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface ApplyUAResult {
|
export interface ApplyUAResult {
|
||||||
|
@ -1,8 +1,9 @@
|
|||||||
import {csvDecodeRow} from 'app/common/csvFormat';
|
import {csvDecodeRow} from 'app/common/csvFormat';
|
||||||
|
import {BulkColValues, CellValue, ColValues, UserAction} from 'app/common/DocActions';
|
||||||
import {DocData} from 'app/common/DocData';
|
import {DocData} from 'app/common/DocData';
|
||||||
import {DocumentSettings} from 'app/common/DocumentSettings';
|
import {DocumentSettings} from 'app/common/DocumentSettings';
|
||||||
import {getReferencedTableId, isFullReferencingType} from 'app/common/gristTypes';
|
|
||||||
import * as gristTypes from 'app/common/gristTypes';
|
import * as gristTypes from 'app/common/gristTypes';
|
||||||
|
import {getReferencedTableId, isFullReferencingType} from 'app/common/gristTypes';
|
||||||
import * as gutil from 'app/common/gutil';
|
import * as gutil from 'app/common/gutil';
|
||||||
import {safeJsonParse} from 'app/common/gutil';
|
import {safeJsonParse} from 'app/common/gutil';
|
||||||
import {getCurrency, NumberFormatOptions} from 'app/common/NumberFormat';
|
import {getCurrency, NumberFormatOptions} from 'app/common/NumberFormat';
|
||||||
@ -11,6 +12,7 @@ import {parseDateStrict, parseDateTime} from 'app/common/parseDate';
|
|||||||
import {MetaRowRecord, TableData} from 'app/common/TableData';
|
import {MetaRowRecord, TableData} from 'app/common/TableData';
|
||||||
import {DateFormatOptions, DateTimeFormatOptions, formatDecoded, FormatOptions} from 'app/common/ValueFormatter';
|
import {DateFormatOptions, DateTimeFormatOptions, formatDecoded, FormatOptions} from 'app/common/ValueFormatter';
|
||||||
import flatMap = require('lodash/flatMap');
|
import flatMap = require('lodash/flatMap');
|
||||||
|
import mapValues = require('lodash/mapValues');
|
||||||
|
|
||||||
|
|
||||||
export class ValueParser {
|
export class ValueParser {
|
||||||
@ -213,6 +215,7 @@ export const valueParserClasses: { [type: string]: typeof ValueParser } = {
|
|||||||
RefList: ReferenceListParser,
|
RefList: ReferenceListParser,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const identity = (value: string) => value;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns a function which can parse strings into values appropriate for
|
* Returns a function which can parse strings into values appropriate for
|
||||||
@ -228,7 +231,7 @@ export function createParserRaw(
|
|||||||
const parser = new cls(type, widgetOpts, docSettings);
|
const parser = new cls(type, widgetOpts, docSettings);
|
||||||
return parser.cleanParse.bind(parser);
|
return parser.cleanParse.bind(parser);
|
||||||
}
|
}
|
||||||
return value => value;
|
return identity;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -270,3 +273,68 @@ export function createParser(
|
|||||||
|
|
||||||
return createParserRaw(type, widgetOpts, docSettings);
|
return createParserRaw(type, widgetOpts, docSettings);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns a copy of `colValues` with string values parsed according to the type and options of each column.
|
||||||
|
* `bulk` should be `true` if `colValues` is of type `BulkColValues`.
|
||||||
|
*/
|
||||||
|
function parseColValues<T extends ColValues | BulkColValues>(
|
||||||
|
tableId: string, colValues: T, docData: DocData, bulk: boolean
|
||||||
|
): T {
|
||||||
|
const columnsTable = docData.getMetaTable('_grist_Tables_column');
|
||||||
|
const tablesTable = docData.getMetaTable('_grist_Tables');
|
||||||
|
const tableRef = tablesTable.findRow('tableId', tableId);
|
||||||
|
if (!tableRef) {
|
||||||
|
return colValues;
|
||||||
|
}
|
||||||
|
|
||||||
|
return mapValues(colValues, (values, colId) => {
|
||||||
|
const colRef = columnsTable.findMatchingRowId({colId, parentId: tableRef});
|
||||||
|
if (!colRef) {
|
||||||
|
// Column not found - let something else deal with that
|
||||||
|
return values;
|
||||||
|
}
|
||||||
|
|
||||||
|
const parser = createParser(docData, colRef);
|
||||||
|
|
||||||
|
// Optimisation: If there's no special parser for this column type, do nothing
|
||||||
|
if (parser === identity) {
|
||||||
|
return values;
|
||||||
|
}
|
||||||
|
|
||||||
|
function parseIfString(val: any) {
|
||||||
|
return typeof val === "string" ? parser(val) : val;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (bulk) {
|
||||||
|
if (!Array.isArray(values)) { // in case of bad input
|
||||||
|
return values;
|
||||||
|
}
|
||||||
|
// `colValues` is of type `BulkColValues`
|
||||||
|
return (values as CellValue[]).map(parseIfString);
|
||||||
|
} else {
|
||||||
|
// `colValues` is of type `ColValues`, `values` is just one value
|
||||||
|
return parseIfString(values);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
export function parseUserAction(ua: UserAction, docData: DocData): UserAction {
|
||||||
|
const actionType = ua[0] as string;
|
||||||
|
let parseBulk: boolean;
|
||||||
|
|
||||||
|
if (['AddRecord', 'UpdateRecord'].includes(actionType)) {
|
||||||
|
parseBulk = false;
|
||||||
|
} else if (['BulkAddRecord', 'BulkUpdateRecord', 'ReplaceTableData'].includes(actionType)) {
|
||||||
|
parseBulk = true;
|
||||||
|
} else {
|
||||||
|
return ua;
|
||||||
|
}
|
||||||
|
|
||||||
|
ua = ua.slice();
|
||||||
|
const tableId = ua[1] as string;
|
||||||
|
const lastIndex = ua.length - 1;
|
||||||
|
const colValues = ua[lastIndex] as ColValues | BulkColValues;
|
||||||
|
ua[lastIndex] = parseColValues(tableId, colValues, docData, parseBulk);
|
||||||
|
return ua;
|
||||||
|
}
|
||||||
|
@ -41,6 +41,7 @@ import {schema, SCHEMA_VERSION} from 'app/common/schema';
|
|||||||
import {MetaRowRecord} from 'app/common/TableData';
|
import {MetaRowRecord} from 'app/common/TableData';
|
||||||
import {FetchUrlOptions, UploadResult} from 'app/common/uploads';
|
import {FetchUrlOptions, UploadResult} from 'app/common/uploads';
|
||||||
import {DocReplacementOptions, DocState, DocStateComparison} from 'app/common/UserAPI';
|
import {DocReplacementOptions, DocState, DocStateComparison} from 'app/common/UserAPI';
|
||||||
|
import {parseUserAction} from 'app/common/ValueParser';
|
||||||
import {ParseOptions} from 'app/plugin/FileParserAPI';
|
import {ParseOptions} from 'app/plugin/FileParserAPI';
|
||||||
import {GristDocAPI} from 'app/plugin/GristAPI';
|
import {GristDocAPI} from 'app/plugin/GristAPI';
|
||||||
import {compileAclFormula} from 'app/server/lib/ACLFormula';
|
import {compileAclFormula} from 'app/server/lib/ACLFormula';
|
||||||
@ -1373,6 +1374,11 @@ export class ActiveDoc extends EventEmitter {
|
|||||||
this._log.debug(docSession, "_applyUserActions(%s, %s)", client, shortDesc(actions));
|
this._log.debug(docSession, "_applyUserActions(%s, %s)", client, shortDesc(actions));
|
||||||
this._inactivityTimer.ping(); // The doc is in active use; ping it to stay open longer.
|
this._inactivityTimer.ping(); // The doc is in active use; ping it to stay open longer.
|
||||||
|
|
||||||
|
if (options.parseStrings) {
|
||||||
|
actions = actions.map(ua => parseUserAction(ua, this.docData!));
|
||||||
|
this._log.debug(docSession, "_applyUserActions(%s, %s) (after parsing)", client, shortDesc(actions));
|
||||||
|
}
|
||||||
|
|
||||||
if (options?.bestEffort) {
|
if (options?.bestEffort) {
|
||||||
actions = await this._granularAccess.prefilterUserActions(docSession, actions);
|
actions = await this._granularAccess.prefilterUserActions(docSession, actions);
|
||||||
}
|
}
|
||||||
|
@ -9,7 +9,7 @@ import {ApplyUAResult, DataSourceTransformed, ImportOptions, ImportResult, Impor
|
|||||||
TransformRule,
|
TransformRule,
|
||||||
TransformRuleMap} from 'app/common/ActiveDocAPI';
|
TransformRuleMap} from 'app/common/ActiveDocAPI';
|
||||||
import {ApiError} from 'app/common/ApiError';
|
import {ApiError} from 'app/common/ApiError';
|
||||||
import {BulkColValues, CellValue, fromTableDataAction, TableRecordValue} from 'app/common/DocActions';
|
import {BulkColValues, CellValue, fromTableDataAction, TableRecordValue, UserAction} from 'app/common/DocActions';
|
||||||
import * as gutil from 'app/common/gutil';
|
import * as gutil from 'app/common/gutil';
|
||||||
import {DocStateComparison} from 'app/common/UserAPI';
|
import {DocStateComparison} from 'app/common/UserAPI';
|
||||||
import {ParseFileResult, ParseOptions} from 'app/plugin/FileParserAPI';
|
import {ParseFileResult, ParseOptions} from 'app/plugin/FileParserAPI';
|
||||||
@ -312,7 +312,7 @@ export class ActiveDocImport {
|
|||||||
const ruleCanBeApplied = (transformRule != null) &&
|
const ruleCanBeApplied = (transformRule != null) &&
|
||||||
_.difference(transformRule.sourceCols, hiddenTableColIds).length === 0;
|
_.difference(transformRule.sourceCols, hiddenTableColIds).length === 0;
|
||||||
await this._activeDoc.applyUserActions(docSession,
|
await this._activeDoc.applyUserActions(docSession,
|
||||||
[["ReplaceTableData", hiddenTableId, rowIdColumn, columnValues]]);
|
[["ReplaceTableData", hiddenTableId, rowIdColumn, columnValues]], {parseStrings: true});
|
||||||
|
|
||||||
// data parsed and put into hiddenTableId
|
// data parsed and put into hiddenTableId
|
||||||
// For preview_table (isHidden) do GenImporterView to make views and formulas and cols
|
// For preview_table (isHidden) do GenImporterView to make views and formulas and cols
|
||||||
@ -438,7 +438,8 @@ export class ActiveDocImport {
|
|||||||
}
|
}
|
||||||
|
|
||||||
await this._activeDoc.applyUserActions(docSession,
|
await this._activeDoc.applyUserActions(docSession,
|
||||||
[['BulkAddRecord', destTableId, gutil.arrayRepeat(hiddenTableData.id.length, null), columnData]]);
|
[['BulkAddRecord', destTableId, gutil.arrayRepeat(hiddenTableData.id.length, null), columnData]],
|
||||||
|
{parseStrings: true});
|
||||||
|
|
||||||
return destTableId;
|
return destTableId;
|
||||||
}
|
}
|
||||||
@ -518,17 +519,17 @@ export class ActiveDocImport {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// We no longer need the temporary import table, so remove it.
|
// We no longer need the temporary import table, so remove it.
|
||||||
await this._activeDoc.applyUserActions(docSession, [['RemoveTable', hiddenTableId]]);
|
const actions: UserAction[] = [['RemoveTable', hiddenTableId]];
|
||||||
|
|
||||||
if (updatedRecordIds.length > 0) {
|
if (updatedRecordIds.length > 0) {
|
||||||
await this._activeDoc.applyUserActions(docSession,
|
actions.push(['BulkUpdateRecord', destTableId, updatedRecordIds, updatedRecords]);
|
||||||
[['BulkUpdateRecord', destTableId, updatedRecordIds, updatedRecords]]);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (numNewRecords > 0) {
|
if (numNewRecords > 0) {
|
||||||
await this._activeDoc.applyUserActions(docSession,
|
actions.push(['BulkAddRecord', destTableId, gutil.arrayRepeat(numNewRecords, null), newRecords]);
|
||||||
[['BulkAddRecord', destTableId, gutil.arrayRepeat(numNewRecords, null), newRecords]]);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
await this._activeDoc.applyUserActions(docSession, actions, {parseStrings: true});
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -2,7 +2,7 @@ import { createEmptyActionSummary } from "app/common/ActionSummary";
|
|||||||
import { ApiError } from 'app/common/ApiError';
|
import { ApiError } from 'app/common/ApiError';
|
||||||
import { BrowserSettings } from "app/common/BrowserSettings";
|
import { BrowserSettings } from "app/common/BrowserSettings";
|
||||||
import {
|
import {
|
||||||
BulkColValues, CellValue, ColValues, fromTableDataAction, TableColValues, TableRecordValue,
|
BulkColValues, ColValues, fromTableDataAction, TableColValues, TableRecordValue,
|
||||||
} from 'app/common/DocActions';
|
} from 'app/common/DocActions';
|
||||||
import {isRaisedException} from "app/common/gristTypes";
|
import {isRaisedException} from "app/common/gristTypes";
|
||||||
import { arrayRepeat, isAffirmative } from "app/common/gutil";
|
import { arrayRepeat, isAffirmative } from "app/common/gutil";
|
||||||
@ -142,7 +142,8 @@ export class DocWorkerApi {
|
|||||||
|
|
||||||
// Apply user actions to a document.
|
// Apply user actions to a document.
|
||||||
this._app.post('/api/docs/:docId/apply', canEdit, withDoc(async (activeDoc, req, res) => {
|
this._app.post('/api/docs/:docId/apply', canEdit, withDoc(async (activeDoc, req, res) => {
|
||||||
res.json(await activeDoc.applyUserActions(docSessionFromRequest(req), req.body));
|
const parseStrings = !isAffirmative(req.query.noparse);
|
||||||
|
res.json(await activeDoc.applyUserActions(docSessionFromRequest(req), req.body, {parseStrings}));
|
||||||
}));
|
}));
|
||||||
|
|
||||||
async function getTableData(activeDoc: ActiveDoc, req: RequestWithLogin) {
|
async function getTableData(activeDoc: ActiveDoc, req: RequestWithLogin) {
|
||||||
@ -252,14 +253,9 @@ export class DocWorkerApi {
|
|||||||
async function addRecords(
|
async function addRecords(
|
||||||
req: RequestWithLogin, activeDoc: ActiveDoc, count: number, columnValues: BulkColValues
|
req: RequestWithLogin, activeDoc: ActiveDoc, count: number, columnValues: BulkColValues
|
||||||
): Promise<number[]> {
|
): Promise<number[]> {
|
||||||
const tableId = req.params.tableId;
|
|
||||||
const colNames = Object.keys(columnValues);
|
|
||||||
// user actions expect [null, ...] as row ids
|
// user actions expect [null, ...] as row ids
|
||||||
const rowIds = arrayRepeat(count, null);
|
const rowIds = arrayRepeat(count, null);
|
||||||
const sandboxRes = await handleSandboxError(tableId, colNames, activeDoc.applyUserActions(
|
return addOrUpdateRecords(req, activeDoc, columnValues, rowIds, 'BulkAddRecord');
|
||||||
docSessionFromRequest(req),
|
|
||||||
[['BulkAddRecord', tableId, rowIds, columnValues]]));
|
|
||||||
return sandboxRes.retValues[0];
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function areSameFields(records: Array<Types.Record | Types.NewRecord>) {
|
function areSameFields(records: Array<Types.Record | Types.NewRecord>) {
|
||||||
@ -367,13 +363,24 @@ export class DocWorkerApi {
|
|||||||
// Update records identified by rowIds. Any invalid id fails
|
// Update records identified by rowIds. Any invalid id fails
|
||||||
// the request and returns a 400 error code.
|
// the request and returns a 400 error code.
|
||||||
async function updateRecords(
|
async function updateRecords(
|
||||||
req: RequestWithLogin, activeDoc: ActiveDoc, columnValues: {[colId: string]: CellValue[]}, rowIds: number[]
|
req: RequestWithLogin, activeDoc: ActiveDoc, columnValues: BulkColValues, rowIds: number[]
|
||||||
|
) {
|
||||||
|
await addOrUpdateRecords(req, activeDoc, columnValues, rowIds, 'BulkUpdateRecord');
|
||||||
|
}
|
||||||
|
|
||||||
|
async function addOrUpdateRecords(
|
||||||
|
req: RequestWithLogin, activeDoc: ActiveDoc,
|
||||||
|
columnValues: BulkColValues, rowIds: (number | null)[],
|
||||||
|
actionType: 'BulkUpdateRecord' | 'BulkAddRecord'
|
||||||
) {
|
) {
|
||||||
const tableId = req.params.tableId;
|
const tableId = req.params.tableId;
|
||||||
const colNames = Object.keys(columnValues);
|
const colNames = Object.keys(columnValues);
|
||||||
await handleSandboxError(tableId, colNames, activeDoc.applyUserActions(
|
const sandboxRes = await handleSandboxError(tableId, colNames, activeDoc.applyUserActions(
|
||||||
docSessionFromRequest(req),
|
docSessionFromRequest(req),
|
||||||
[['BulkUpdateRecord', tableId, rowIds, columnValues]]));
|
[[actionType, tableId, rowIds, columnValues]],
|
||||||
|
{parseStrings: !isAffirmative(req.query.noparse)},
|
||||||
|
));
|
||||||
|
return sandboxRes.retValues[0];
|
||||||
}
|
}
|
||||||
|
|
||||||
// Update records given in column format
|
// Update records given in column format
|
||||||
|
Loading…
Reference in New Issue
Block a user