From 251d79704b2c1889251330f5c43464151c804d43 Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Fri, 1 Apr 2022 23:55:59 +0200 Subject: [PATCH] (core) Migrate Attachments columns from marshalled blobs to JSON Summary: Adds a migration in preparation for future work on tracking and deleting attachments. This includes a `_grist_Attachments.timeDeleted` column which isn't used yet, and changing the storage format of user columns of type `Attachments`. DocStorage now treats Attachments like RefList in general (since they use JSON), which also prompted a tiny bit of refactoring. Test Plan: Added a migration test case showing the change in format. Reviewers: dsagal Reviewed By: dsagal Differential Revision: https://phab.getgrist.com/D3352 --- app/common/ColumnFilterFunc.ts | 10 +++++----- app/common/gristTypes.ts | 6 +++++- app/common/schema.ts | 4 +++- app/server/lib/DocStorage.ts | 9 +++++---- app/server/lib/initialDocSql.ts | 8 ++++---- sandbox/grist/migrations.py | 18 ++++++++++++++++++ sandbox/grist/schema.py | 3 ++- test/fixtures/docs/Hello.grist | Bin 59392 -> 59392 bytes 8 files changed, 42 insertions(+), 16 deletions(-) diff --git a/app/common/ColumnFilterFunc.ts b/app/common/ColumnFilterFunc.ts index 5d486426..b4da6ca3 100644 --- a/app/common/ColumnFilterFunc.ts +++ b/app/common/ColumnFilterFunc.ts @@ -1,7 +1,7 @@ -import { CellValue } from "app/common/DocActions"; -import { FilterState, makeFilterState } from "app/common/FilterState"; -import { decodeObject } from "app/plugin/objtypes"; -import { isList, isRefListType } from "./gristTypes"; +import {CellValue} from "app/common/DocActions"; +import {FilterState, makeFilterState} from "app/common/FilterState"; +import {decodeObject} from "app/plugin/objtypes"; +import {isList, isListType} from "./gristTypes"; export type ColumnFilterFunc = (value: CellValue) => boolean; @@ -13,7 +13,7 @@ export function makeFilterFunc({ include, values }: FilterState, // For example, a TypeError in the formula column and the string '["E","TypeError"]' would be seen as the same. // TODO: This narrow corner case seems acceptable for now, but may be worth revisiting. return (val: CellValue) => { - if (isList(val) && (columnType === 'ChoiceList' || isRefListType(String(columnType)))) { + if (isList(val) && columnType && isListType(columnType)) { const list = decodeObject(val) as unknown[]; return list.some(item => values.has(item as any) === include); } diff --git a/app/common/gristTypes.ts b/app/common/gristTypes.ts index 79e48646..2e845575 100644 --- a/app/common/gristTypes.ts +++ b/app/common/gristTypes.ts @@ -330,7 +330,11 @@ export function getReferencedTableId(type: string) { } export function isRefListType(type: string) { - return type === "Attachments" || type.startsWith('RefList:'); + return type === "Attachments" || type?.startsWith('RefList:'); +} + +export function isListType(type: string) { + return type === "ChoiceList" || isRefListType(type); } export function isFullReferencingType(type: string) { diff --git a/app/common/schema.ts b/app/common/schema.ts index a3f5e111..44d71091 100644 --- a/app/common/schema.ts +++ b/app/common/schema.ts @@ -4,7 +4,7 @@ import { GristObjCode } from "app/plugin/GristData"; // tslint:disable:object-literal-key-quotes -export const SCHEMA_VERSION = 27; +export const SCHEMA_VERSION = 28; export const schema = { @@ -148,6 +148,7 @@ export const schema = { fileSize : "Int", imageHeight : "Int", imageWidth : "Int", + timeDeleted : "DateTime", timeUploaded : "DateTime", }, @@ -338,6 +339,7 @@ export interface SchemaTypes { fileSize: number; imageHeight: number; imageWidth: number; + timeDeleted: number; timeUploaded: number; }; diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts index 2daad15d..ad3203d0 100644 --- a/app/server/lib/DocStorage.ts +++ b/app/server/lib/DocStorage.ts @@ -11,7 +11,7 @@ import * as sqlite3 from '@gristlabs/sqlite3'; import {LocalActionBundle} from 'app/common/ActionBundle'; import {BulkColValues, DocAction, TableColValues, TableDataAction, toTableDataAction} from 'app/common/DocActions'; import * as gristTypes from 'app/common/gristTypes'; -import {isList} from 'app/common/gristTypes'; +import {isList, isListType, isRefListType} from 'app/common/gristTypes'; import * as marshal from 'app/common/marshal'; import * as schema from 'app/common/schema'; import {GristObjCode} from "app/plugin/GristData"; @@ -455,7 +455,7 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { if (isList(val) && val.every(tok => (typeof(tok) === 'string'))) { return JSON.stringify(val.slice(1)); } - } else if (gristType?.startsWith('RefList:')) { + } else if (isRefListType(gristType)) { if (isList(val) && val.slice(1).every((tok: any) => (typeof(tok) === 'number'))) { return JSON.stringify(val.slice(1)); } @@ -522,7 +522,7 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { return Boolean(val); } } - if (gristType === 'ChoiceList' || gristType?.startsWith('RefList:')) { + if (isListType(gristType)) { if (typeof val === 'string' && val.startsWith('[')) { try { return ['L', ...JSON.parse(val)]; @@ -566,6 +566,7 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { case 'ChoiceList': case 'RefList': case 'ReferenceList': + case 'Attachments': return 'TEXT'; // To be encoded as a JSON array of strings. case 'Date': return 'DATE'; @@ -1464,7 +1465,7 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { // For any marshalled objects, check if we can now unmarshall them if they are the // native type. - if (result.newGristType !== result.oldGristType) { + if (result.newGristType !== result.oldGristType || result.newSqlType !== result.oldSqlType) { const cells = await this.all(`SELECT id, ${q(colId)} as value FROM ${q(tableId)} ` + `WHERE typeof(${q(colId)}) = 'blob'`); const marshaller = new marshal.Marshaller({version: 2}); diff --git a/app/server/lib/initialDocSql.ts b/app/server/lib/initialDocSql.ts index 484223bd..89aa02f5 100644 --- a/app/server/lib/initialDocSql.ts +++ b/app/server/lib/initialDocSql.ts @@ -6,7 +6,7 @@ export const GRIST_DOC_SQL = ` PRAGMA foreign_keys=OFF; BEGIN TRANSACTION; CREATE TABLE IF NOT EXISTS "_grist_DocInfo" (id INTEGER PRIMARY KEY, "docId" TEXT DEFAULT '', "peers" TEXT DEFAULT '', "basketId" TEXT DEFAULT '', "schemaVersion" INTEGER DEFAULT 0, "timezone" TEXT DEFAULT '', "documentSettings" TEXT DEFAULT ''); -INSERT INTO _grist_DocInfo VALUES(1,'','','',27,'UTC','{"locale": "en-US"}'); +INSERT INTO _grist_DocInfo VALUES(1,'','','',28,'UTC','{"locale": "en-US"}'); CREATE TABLE IF NOT EXISTS "_grist_Tables" (id INTEGER PRIMARY KEY, "tableId" TEXT DEFAULT '', "primaryViewId" INTEGER DEFAULT 0, "summarySourceTable" INTEGER DEFAULT 0, "onDemand" BOOLEAN DEFAULT 0, "rawViewSectionRef" INTEGER DEFAULT 0); CREATE TABLE IF NOT EXISTS "_grist_Tables_column" (id INTEGER PRIMARY KEY, "parentId" INTEGER DEFAULT 0, "parentPos" NUMERIC DEFAULT 1e999, "colId" TEXT DEFAULT '', "type" TEXT DEFAULT '', "widgetOptions" TEXT DEFAULT '', "isFormula" BOOLEAN DEFAULT 0, "formula" TEXT DEFAULT '', "label" TEXT DEFAULT '', "untieColIdFromLabel" BOOLEAN DEFAULT 0, "summarySourceCol" INTEGER DEFAULT 0, "displayCol" INTEGER DEFAULT 0, "visibleCol" INTEGER DEFAULT 0, "rules" TEXT DEFAULT NULL, "recalcWhen" INTEGER DEFAULT 0, "recalcDeps" TEXT DEFAULT NULL); CREATE TABLE IF NOT EXISTS "_grist_Imports" (id INTEGER PRIMARY KEY, "tableRef" INTEGER DEFAULT 0, "origFileName" TEXT DEFAULT '', "parseFormula" TEXT DEFAULT '', "delimiter" TEXT DEFAULT '', "doublequote" BOOLEAN DEFAULT 0, "escapechar" TEXT DEFAULT '', "quotechar" TEXT DEFAULT '', "skipinitialspace" BOOLEAN DEFAULT 0, "encoding" TEXT DEFAULT '', "hasHeaders" BOOLEAN DEFAULT 0); @@ -21,7 +21,7 @@ CREATE TABLE IF NOT EXISTS "_grist_Views_section" (id INTEGER PRIMARY KEY, "tabl CREATE TABLE IF NOT EXISTS "_grist_Views_section_field" (id INTEGER PRIMARY KEY, "parentId" INTEGER DEFAULT 0, "parentPos" NUMERIC DEFAULT 1e999, "colRef" INTEGER DEFAULT 0, "width" INTEGER DEFAULT 0, "widgetOptions" TEXT DEFAULT '', "displayCol" INTEGER DEFAULT 0, "visibleCol" INTEGER DEFAULT 0, "filter" TEXT DEFAULT '', "rules" TEXT DEFAULT NULL); CREATE TABLE IF NOT EXISTS "_grist_Validations" (id INTEGER PRIMARY KEY, "formula" TEXT DEFAULT '', "name" TEXT DEFAULT '', "tableRef" INTEGER DEFAULT 0); CREATE TABLE IF NOT EXISTS "_grist_REPL_Hist" (id INTEGER PRIMARY KEY, "code" TEXT DEFAULT '', "outputText" TEXT DEFAULT '', "errorText" TEXT DEFAULT ''); -CREATE TABLE IF NOT EXISTS "_grist_Attachments" (id INTEGER PRIMARY KEY, "fileIdent" TEXT DEFAULT '', "fileName" TEXT DEFAULT '', "fileType" TEXT DEFAULT '', "fileSize" INTEGER DEFAULT 0, "imageHeight" INTEGER DEFAULT 0, "imageWidth" INTEGER DEFAULT 0, "timeUploaded" DATETIME DEFAULT NULL); +CREATE TABLE IF NOT EXISTS "_grist_Attachments" (id INTEGER PRIMARY KEY, "fileIdent" TEXT DEFAULT '', "fileName" TEXT DEFAULT '', "fileType" TEXT DEFAULT '', "fileSize" INTEGER DEFAULT 0, "imageHeight" INTEGER DEFAULT 0, "imageWidth" INTEGER DEFAULT 0, "timeDeleted" DATETIME DEFAULT NULL, "timeUploaded" DATETIME DEFAULT NULL); CREATE TABLE IF NOT EXISTS "_grist_Triggers" (id INTEGER PRIMARY KEY, "tableRef" INTEGER DEFAULT 0, "eventTypes" TEXT DEFAULT NULL, "isReadyColRef" INTEGER DEFAULT 0, "actions" TEXT DEFAULT ''); CREATE TABLE IF NOT EXISTS "_grist_ACLRules" (id INTEGER PRIMARY KEY, "resource" INTEGER DEFAULT 0, "permissions" INTEGER DEFAULT 0, "principals" TEXT DEFAULT '', "aclFormula" TEXT DEFAULT '', "aclColumn" INTEGER DEFAULT 0, "aclFormulaParsed" TEXT DEFAULT '', "permissionsText" TEXT DEFAULT '', "rulePos" NUMERIC DEFAULT 1e999, "userAttributes" TEXT DEFAULT ''); INSERT INTO _grist_ACLRules VALUES(1,1,63,'[1]','',0,'','',1e999,''); @@ -41,7 +41,7 @@ export const GRIST_DOC_WITH_TABLE1_SQL = ` PRAGMA foreign_keys=OFF; BEGIN TRANSACTION; CREATE TABLE IF NOT EXISTS "_grist_DocInfo" (id INTEGER PRIMARY KEY, "docId" TEXT DEFAULT '', "peers" TEXT DEFAULT '', "basketId" TEXT DEFAULT '', "schemaVersion" INTEGER DEFAULT 0, "timezone" TEXT DEFAULT '', "documentSettings" TEXT DEFAULT ''); -INSERT INTO _grist_DocInfo VALUES(1,'','','',27,'UTC','{"locale": "en-US"}'); +INSERT INTO _grist_DocInfo VALUES(1,'','','',28,'UTC','{"locale": "en-US"}'); CREATE TABLE IF NOT EXISTS "_grist_Tables" (id INTEGER PRIMARY KEY, "tableId" TEXT DEFAULT '', "primaryViewId" INTEGER DEFAULT 0, "summarySourceTable" INTEGER DEFAULT 0, "onDemand" BOOLEAN DEFAULT 0, "rawViewSectionRef" INTEGER DEFAULT 0); INSERT INTO _grist_Tables VALUES(1,'Table1',1,0,0,2); CREATE TABLE IF NOT EXISTS "_grist_Tables_column" (id INTEGER PRIMARY KEY, "parentId" INTEGER DEFAULT 0, "parentPos" NUMERIC DEFAULT 1e999, "colId" TEXT DEFAULT '', "type" TEXT DEFAULT '', "widgetOptions" TEXT DEFAULT '', "isFormula" BOOLEAN DEFAULT 0, "formula" TEXT DEFAULT '', "label" TEXT DEFAULT '', "untieColIdFromLabel" BOOLEAN DEFAULT 0, "summarySourceCol" INTEGER DEFAULT 0, "displayCol" INTEGER DEFAULT 0, "visibleCol" INTEGER DEFAULT 0, "rules" TEXT DEFAULT NULL, "recalcWhen" INTEGER DEFAULT 0, "recalcDeps" TEXT DEFAULT NULL); @@ -72,7 +72,7 @@ INSERT INTO _grist_Views_section_field VALUES(5,2,5,3,0,'',0,0,'',NULL); INSERT INTO _grist_Views_section_field VALUES(6,2,6,4,0,'',0,0,'',NULL); CREATE TABLE IF NOT EXISTS "_grist_Validations" (id INTEGER PRIMARY KEY, "formula" TEXT DEFAULT '', "name" TEXT DEFAULT '', "tableRef" INTEGER DEFAULT 0); CREATE TABLE IF NOT EXISTS "_grist_REPL_Hist" (id INTEGER PRIMARY KEY, "code" TEXT DEFAULT '', "outputText" TEXT DEFAULT '', "errorText" TEXT DEFAULT ''); -CREATE TABLE IF NOT EXISTS "_grist_Attachments" (id INTEGER PRIMARY KEY, "fileIdent" TEXT DEFAULT '', "fileName" TEXT DEFAULT '', "fileType" TEXT DEFAULT '', "fileSize" INTEGER DEFAULT 0, "imageHeight" INTEGER DEFAULT 0, "imageWidth" INTEGER DEFAULT 0, "timeUploaded" DATETIME DEFAULT NULL); +CREATE TABLE IF NOT EXISTS "_grist_Attachments" (id INTEGER PRIMARY KEY, "fileIdent" TEXT DEFAULT '', "fileName" TEXT DEFAULT '', "fileType" TEXT DEFAULT '', "fileSize" INTEGER DEFAULT 0, "imageHeight" INTEGER DEFAULT 0, "imageWidth" INTEGER DEFAULT 0, "timeDeleted" DATETIME DEFAULT NULL, "timeUploaded" DATETIME DEFAULT NULL); CREATE TABLE IF NOT EXISTS "_grist_Triggers" (id INTEGER PRIMARY KEY, "tableRef" INTEGER DEFAULT 0, "eventTypes" TEXT DEFAULT NULL, "isReadyColRef" INTEGER DEFAULT 0, "actions" TEXT DEFAULT ''); CREATE TABLE IF NOT EXISTS "_grist_ACLRules" (id INTEGER PRIMARY KEY, "resource" INTEGER DEFAULT 0, "permissions" INTEGER DEFAULT 0, "principals" TEXT DEFAULT '', "aclFormula" TEXT DEFAULT '', "aclColumn" INTEGER DEFAULT 0, "aclFormulaParsed" TEXT DEFAULT '', "permissionsText" TEXT DEFAULT '', "rulePos" NUMERIC DEFAULT 1e999, "userAttributes" TEXT DEFAULT ''); INSERT INTO _grist_ACLRules VALUES(1,1,63,'[1]','',0,'','',1e999,''); diff --git a/sandbox/grist/migrations.py b/sandbox/grist/migrations.py index b0c5b61f..e6d7a060 100644 --- a/sandbox/grist/migrations.py +++ b/sandbox/grist/migrations.py @@ -906,3 +906,21 @@ def migration27(tdset): add_column('_grist_Tables_column', 'rules', 'RefList:_grist_Tables_column'), add_column('_grist_Views_section_field', 'rules', 'RefList:_grist_Tables_column'), ]) + + +@migration(schema_version=28) +def migration28(tdset): + doc_actions = [add_column('_grist_Attachments', 'timeDeleted', 'DateTime')] + + tables = list(actions.transpose_bulk_action(tdset.all_tables["_grist_Tables"])) + columns = list(actions.transpose_bulk_action(tdset.all_tables["_grist_Tables_column"])) + + for table in tables: + for col in columns: + if table.id == col.parentId and col.type == "Attachments": + # This looks like it doesn't change anything, + # but it makes DocStorage realise that the sqlType has changed + # so it converts marshalled blobs to JSON + doc_actions.append(actions.ModifyColumn(table.tableId, col.colId, {"type": "Attachments"})) + + return tdset.apply_doc_actions(doc_actions) diff --git a/sandbox/grist/schema.py b/sandbox/grist/schema.py index 5aa08b00..619bf85e 100644 --- a/sandbox/grist/schema.py +++ b/sandbox/grist/schema.py @@ -15,7 +15,7 @@ import six import actions -SCHEMA_VERSION = 27 +SCHEMA_VERSION = 28 def make_column(col_id, col_type, formula='', isFormula=False): return { @@ -235,6 +235,7 @@ def schema_create_actions(): make_column("fileSize", "Int"), # The size in bytes make_column("imageHeight", "Int"), # height in pixels make_column("imageWidth", "Int"), # width in pixels + make_column("timeDeleted", "DateTime"), make_column("timeUploaded", "DateTime") ]), diff --git a/test/fixtures/docs/Hello.grist b/test/fixtures/docs/Hello.grist index e860434681572915e6e9db8f4c4afec6faccaf39..652be3452ffd18c7e51e1c951b5329b85c1750d9 100644 GIT binary patch delta 130 zcmZp;z}#?wd4jYcCj$e66%fM!-$Wf_M$U~1wc?C2o9~G$yYK^LSQr8sm|d7U83GwM zPMqk-)f6ktE^cVZ*pj;We4GKJnvQ}}NoH=UOKMJPNotCcf{SB_Ylx?>tAdNGn`5X? ah=O0JkB{c&ycmC`O$IC+n^`jc)B^xybRil5 delta 115 zcmZp;z}#?wd4jYc2Ll6x6%fM!??fGAMvjdMwc?D@o9~G$yC^a3Wnf{*V_;%rbYnUS yWXJ;{0x<1mRG+-H%xCkaSU)Bqpg71th9(PPc5y>P#un+#(J=;$oBzi%b^rj+Y8J`>