From c6aa9b65d4606d632ae8714e7ffba96fe896ba47 Mon Sep 17 00:00:00 2001
From: George Gevoian <george@gevoian.com>
Date: Thu, 18 Nov 2021 14:29:33 -0800
Subject: [PATCH] (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
---
 app/server/lib/ActiveDocImport.ts | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/app/server/lib/ActiveDocImport.ts b/app/server/lib/ActiveDocImport.ts
index 8f5dc5ae..a5bb423c 100644
--- a/app/server/lib/ActiveDocImport.ts
+++ b/app/server/lib/ActiveDocImport.ts
@@ -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});
   }
@@ -392,6 +392,9 @@ export class ActiveDocImport {
       // Transform rules for new tables don't have filled in destination column ids.
       const result = await this._activeDoc.applyUserActions(docSession, [['FillTransformRuleColIds', 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)) {
       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
-   * returns `parsedTable` and a list of descriptors of all columns of references.
+   * Changes every column of references into a column of integers in `parsedTables`. It
+   * returns a list of descriptors of all columns of references.
    */
   private _encodeReferenceAsInt(parsedTables: GristTable[]): ReferenceDescription[] {
     const references = [];
@@ -621,7 +624,6 @@ export class ActiveDocImport {
    * This function fix references that are broken by the change of table id.
    */
   private async _fixReferences(docSession: OptDocSession,
-                               parsedTables: GristTable[],
                                tables: ImportTableResult[],
                                fixedColumnIds: { [tableId: string]: string[]; },
                                references: ReferenceDescription[],
@@ -659,6 +661,23 @@ function isBlank(value: CellValue): boolean {
   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).
 function stripRulePrefixes({destCols}: TransformRule): void {
   for (const col of destCols) {