(core) Fix bug preventing importing of nested json files

Summary:
BulkAddRecord when finishing imports of nested JSON was throwing
an error due to unchecked access of referencing tables. This adds
a guard to prepare_new_values to handle such cases.

Imports happened to cause this to occur because the order that
imported tables are created/populated isn't aware of references
between tables, so it's possible for a reference column to
exist (momentarily) without a valid reference to another table.
These references are currently fixed after all imported tables are
created/populated.

Test Plan: Browser test.

Reviewers: dsagal

Reviewed By: dsagal

Subscribers: dsagal

Differential Revision: https://phab.getgrist.com/D3144
This commit is contained in:
George Gevoian 2021-11-18 14:29:33 -08:00
parent 745ddc21ac
commit c6aa9b65d4

View File

@ -348,7 +348,7 @@ export class ActiveDocImport {
}); });
} }
await this._fixReferences(docSession, parsedTables, tables, fixedColumnIdsByTable, references, isHidden); await this._fixReferences(docSession, tables, fixedColumnIdsByTable, references, isHidden);
return ({options, tables}); return ({options, tables});
} }
@ -392,6 +392,9 @@ export class ActiveDocImport {
// Transform rules for new tables don't have filled in destination column ids. // Transform rules for new tables don't have filled in destination column ids.
const result = await this._activeDoc.applyUserActions(docSession, [['FillTransformRuleColIds', transformRule]]); const result = await this._activeDoc.applyUserActions(docSession, [['FillTransformRuleColIds', transformRule]]);
transformRule = result.retValues[0] as TransformRule; transformRule = result.retValues[0] as TransformRule;
// Encode Refs as Ints, to avoid table dependency issues. We'll convert back to Ref at the end.
encodeRuleReferences(transformRule);
} else if (transformRule.destCols.some(c => c.colId === null)) { } else if (transformRule.destCols.some(c => c.colId === null)) {
throw new Error('Column ids in transform rule must be filled when importing into an existing table'); throw new Error('Column ids in transform rule must be filled when importing into an existing table');
} }
@ -600,8 +603,8 @@ export class ActiveDocImport {
} }
/** /**
* The methods changes every column of references into a column of integers in `parsedTables`. It * Changes every column of references into a column of integers in `parsedTables`. It
* returns `parsedTable` and a list of descriptors of all columns of references. * returns a list of descriptors of all columns of references.
*/ */
private _encodeReferenceAsInt(parsedTables: GristTable[]): ReferenceDescription[] { private _encodeReferenceAsInt(parsedTables: GristTable[]): ReferenceDescription[] {
const references = []; const references = [];
@ -621,7 +624,6 @@ export class ActiveDocImport {
* This function fix references that are broken by the change of table id. * This function fix references that are broken by the change of table id.
*/ */
private async _fixReferences(docSession: OptDocSession, private async _fixReferences(docSession: OptDocSession,
parsedTables: GristTable[],
tables: ImportTableResult[], tables: ImportTableResult[],
fixedColumnIds: { [tableId: string]: string[]; }, fixedColumnIds: { [tableId: string]: string[]; },
references: ReferenceDescription[], references: ReferenceDescription[],
@ -659,6 +661,23 @@ function isBlank(value: CellValue): boolean {
return value === null || (typeof value === 'string' && value.trim().length === 0); return value === null || (typeof value === 'string' && value.trim().length === 0);
} }
/**
* Changes every Ref column to an Int column in `destCols`.
*
* Encoding references as ints can be useful when finishing imports to avoid
* issues such as importing linked tables in the wrong order. When encoding references,
* ActiveDocImport._fixReferences should be called at the end of importing to
* decode Ints back to Refs.
*/
function encodeRuleReferences({destCols}: TransformRule): void {
for (const col of destCols) {
const refTableId = gutil.removePrefix(col.type, "Ref:");
if (refTableId) {
col.type = 'Int';
}
}
}
// Helper function that strips import prefixes from columns in transform rules (if ids are present). // Helper function that strips import prefixes from columns in transform rules (if ids are present).
function stripRulePrefixes({destCols}: TransformRule): void { function stripRulePrefixes({destCols}: TransformRule): void {
for (const col of destCols) { for (const col of destCols) {