Merge pull request #4987 from nocodb/fix/attachment

fix: broken attachments

packages/nc-gui/components/smartsheet/Cell.vue

@@ -107,7 +107,6 @@ const vModel = computed({
         syncValue()
       } else if (!isManualSaved(column.value)) {
         emit('save')
-        currentRow.value.rowMeta.changed = true
       }
     }
   },

packages/nc-gui/components/smartsheet/Grid.vue

@@ -558,11 +558,14 @@ const saveOrUpdateRecords = async (args: { metaValue?: TableType; viewMetaValue?
       currentRow.rowMeta.changed = false
       continue
     }
+
     /** if existing row check updated cell and invoke update method */
     if (currentRow.rowMeta.changed) {
       currentRow.rowMeta.changed = false
       for (const field of (args.metaValue || meta.value)?.columns ?? []) {
-        if (isVirtualCol(field)) continue
+        // `url` would be enriched in attachment during listing
+        // hence it would consider as a change while it is not necessary to update
+        if (isVirtualCol(field) || field.uidt === UITypes.Attachment) continue
         if (field.title! in currentRow.row && currentRow.row[field.title!] !== currentRow.oldRow[field.title!]) {
           await updateOrSaveRow(currentRow, field.title!, {}, args)
         }

packages/nocodb/src/lib/Noco.ts

@@ -104,7 +104,7 @@ export default class Noco {
   constructor() {
     process.env.PORT = process.env.PORT || '8080';
     // todo: move
-    process.env.NC_VERSION = '0101002';
+    process.env.NC_VERSION = '0104002';
 
     // if env variable NC_MINIMAL_DBS is set, then disable project creation with external sources
     if (process.env.NC_MINIMAL_DBS) {

packages/nocodb/src/lib/models/Model.ts

@@ -441,11 +441,14 @@ export default class Model implements TableType {
     const insertObj = {};
     for (const col of await this.getColumns()) {
       if (isVirtualCol(col)) continue;
-      const val =
+      let val =
         data?.[col.column_name] !== undefined
           ? data?.[col.column_name]
           : data?.[col.title];
       if (val !== undefined) {
+        if (col.uidt === UITypes.Attachment && typeof val !== 'string') {
+          val = JSON.stringify(val);
+        }
         insertObj[sanitize(col.column_name)] = val;
       }
     }

packages/nocodb/src/lib/version-upgrader/NcUpgrader.ts

@@ -10,6 +10,7 @@ import ncDataTypesUpgrader from './ncDataTypesUpgrader';
 import ncProjectRolesUpgrader from './ncProjectRolesUpgrader';
 import ncFilterUpgrader from './ncFilterUpgrader';
 import ncAttachmentUpgrader from './ncAttachmentUpgrader';
+import ncAttachmentUpgrader_0104002 from './ncAttachmentUpgrader_0104002';
 
 const log = debug('nc:version-upgrader');
 import boxen from 'boxen';
@@ -39,6 +40,7 @@ export default class NcUpgrader {
         { name: '0098005', handler: ncProjectRolesUpgrader },
         { name: '0100002', handler: ncFilterUpgrader },
         { name: '0101002', handler: ncAttachmentUpgrader },
+        { name: '0104002', handler: ncAttachmentUpgrader_0104002 },
       ];
       if (!(await ctx.ncMeta.knexConnection?.schema?.hasTable?.('nc_store'))) {
         return;

packages/nocodb/src/lib/version-upgrader/ncAttachmentUpgrader.ts

@@ -47,7 +47,7 @@ export default async function ({ ncMeta }: NcUpgraderCtx) {
   for (const _base of bases) {
     const base = new Base(_base);
 
-    // skip if the prodect_id is missing
+    // skip if the project_id is missing
     if (!base.project_id) {
       continue;
     }

packages/nocodb/src/lib/version-upgrader/ncAttachmentUpgrader_0104002.ts

@@ -0,0 +1,159 @@
+import { Knex } from 'knex';
+import { NcUpgraderCtx } from './NcUpgrader';
+import { MetaTable } from '../utils/globals';
+import Base from '../models/Base';
+import Model from '../models/Model';
+import { XKnex } from '../db/sql-data-mapper/index';
+import NcConnectionMgrv2 from '../utils/common/NcConnectionMgrv2';
+import { BaseType, UITypes } from 'nocodb-sdk';
+
+// after 0101002 upgrader, the attachment object would become broken when
+// (1) switching views after updating a singleSelect field
+// since `url` will be enriched the attachment cell, and `saveOrUpdateRecords` in Grid.vue will be triggered
+// in this way, the attachment value will be corrupted like
+// {"{\"path\":\"download/noco/xcdb2/attachment2/a/JRubxMQgPlcumdm3jL.jpeg\",\"title\":\"haha.jpeg\",\"mimetype\":\"image/jpeg\",\"size\":6494,\"url\":\"http://localhost:8080/download/noco/xcdb2/attachment2/a/JRubxMQgPlcumdm3jL.jpeg\"}"}
+// while the expected one is
+// [{"path":"download/noco/xcdb2/attachment2/a/JRubxMQgPlcumdm3jL.jpeg","title":"haha.jpeg","mimetype":"image/jpeg","size":6494}]
+// (2) or reordering attachments
+// since the incoming value is not string, the value will be broken
+// hence, this upgrader is to revert back these corrupted values
+
+function getTnPath(knex: XKnex, tb: Model) {
+  const schema = (knex as any).searchPath?.();
+  const clientType = knex.clientType();
+  if (clientType === 'mssql' && schema) {
+    return knex.raw('??.??', [schema, tb.table_name]).toQuery();
+  } else if (clientType === 'snowflake') {
+    return [
+      knex.client.config.connection.database,
+      knex.client.config.connection.schema,
+      tb.table_name,
+    ].join('.');
+  } else {
+    return tb.table_name;
+  }
+}
+
+export default async function ({ ncMeta }: NcUpgraderCtx) {
+  const bases: BaseType[] = await ncMeta.metaList2(null, null, MetaTable.BASES);
+  for (const _base of bases) {
+    const base = new Base(_base);
+
+    // skip if the project_id is missing
+    if (!base.project_id) {
+      continue;
+    }
+
+    const project = await ncMeta.metaGet2(null, null, MetaTable.PROJECT, {
+      id: base.project_id,
+    });
+
+    // skip if the project is missing
+    if (!project) {
+      continue;
+    }
+
+    const isProjectDeleted = project.deleted;
+
+    const knex: Knex = base.is_meta
+      ? ncMeta.knexConnection
+      : NcConnectionMgrv2.get(base);
+    const models = await base.getModels(ncMeta);
+
+    for (const model of models) {
+      try {
+        // if the table is missing in database, skip
+        if (!(await knex.schema.hasTable(getTnPath(knex, model)))) {
+          continue;
+        }
+
+        const updateRecords = [];
+
+        // get all attachment & primary key columns
+        // and filter out the columns that are missing in database
+        const columns = await (await Model.get(model.id, ncMeta))
+          .getColumns(ncMeta)
+          .then(async (columns) => {
+            const filteredColumns = [];
+
+            for (const column of columns) {
+              if (column.uidt !== UITypes.Attachment && !column.pk) continue;
+              if (
+                !(await knex.schema.hasColumn(
+                  getTnPath(knex, model),
+                  column.column_name
+                ))
+              )
+                continue;
+              filteredColumns.push(column);
+            }
+
+            return filteredColumns;
+          });
+
+        const attachmentColumns = columns
+          .filter((c) => c.uidt === UITypes.Attachment)
+          .map((c) => c.column_name);
+        if (attachmentColumns.length === 0) {
+          continue;
+        }
+        const primaryKeys = columns
+          .filter((c) => c.pk)
+          .map((c) => c.column_name);
+
+        const records = await knex(getTnPath(knex, model)).select();
+
+        for (const record of records) {
+          const where = primaryKeys
+            .map((key) => {
+              return { [key]: record[key] };
+            })
+            .reduce((acc, val) => Object.assign(acc, val), {});
+          for (const attachmentColumn of attachmentColumns) {
+            if (typeof record[attachmentColumn] === 'string') {
+              // potentially corrupted
+              try {
+                JSON.parse(record[attachmentColumn]);
+                // it works fine - skip
+                continue;
+              } catch {
+                try {
+                  // corrupted
+                  let corruptedAttachment = record[attachmentColumn];
+                  // replace the first and last character with `[` and `]`
+                  // and parse it again
+                  corruptedAttachment = JSON.parse(
+                    `[${corruptedAttachment.slice(
+                      1,
+                      corruptedAttachment.length - 1
+                    )}]`
+                  );
+                  let newAttachmentMeta = [];
+                  for (const attachment of corruptedAttachment) {
+                    newAttachmentMeta.push(JSON.parse(attachment));
+                  }
+                  updateRecords.push(
+                    await knex(getTnPath(knex, model))
+                      .update({
+                        [attachmentColumn]: JSON.stringify(newAttachmentMeta),
+                      })
+                      .where(where)
+                  );
+                } catch {
+                  // if parsing failed ignore the cell
+                  continue;
+                }
+              }
+            }
+          }
+        }
+        await Promise.all(updateRecords);
+      } catch (e) {
+        // ignore the error related to deleted project
+        if (!isProjectDeleted) {
+          throw e;
+        }
+      }
+    }
+  }
+}