From 5344ec83fa65fb3594cf6f27ad192e275c6c2724 Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Mon, 6 Mar 2023 15:20:07 +0200 Subject: [PATCH] normalized values on maxSelect change --- daos/record_table_sync.go | 95 +++++++++++++ daos/record_table_sync_test.go | 133 ++++++++++++++++++ ...082970_normalize_single_multiple_values.go | 104 ++++++++++++++ models/schema/schema_field.go | 35 ++++- models/schema/schema_field_test.go | 67 +++++++++ tests/data/data.db | Bin 237568 -> 237568 bytes 6 files changed, 430 insertions(+), 4 deletions(-) create mode 100644 migrations/1678082970_normalize_single_multiple_values.go diff --git a/daos/record_table_sync.go b/daos/record_table_sync.go index 352fefe0..ffd9d9e0 100644 --- a/daos/record_table_sync.go +++ b/daos/record_table_sync.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/pocketbase/dbx" "github.com/pocketbase/pocketbase/models" "github.com/pocketbase/pocketbase/models/schema" "github.com/pocketbase/pocketbase/tools/list" @@ -149,10 +150,104 @@ func (dao *Dao) SyncRecordTableSchema(newCollection *models.Collection, oldColle } } + if err := txDao.normalizeSingleVsMultipleFieldChanges(newCollection, oldCollection); err != nil { + return err + } + return txDao.syncCollectionReferences(newCollection, renamedFieldNames, deletedFieldNames) }) } +func (dao *Dao) normalizeSingleVsMultipleFieldChanges(newCollection, oldCollection *models.Collection) error { + if newCollection.IsView() || oldCollection == nil { + return nil // view or not an update + } + + return dao.RunInTransaction(func(txDao *Dao) error { + for _, newField := range newCollection.Schema.Fields() { + oldField := oldCollection.Schema.GetFieldById(newField.Id) + if oldField == nil { + continue + } + + var isNewMultiple bool + if opt, ok := newField.Options.(schema.MultiValuer); ok { + isNewMultiple = opt.IsMultiple() + } + + var isOldMultiple bool + if opt, ok := oldField.Options.(schema.MultiValuer); ok { + isOldMultiple = opt.IsMultiple() + } + + if isOldMultiple == isNewMultiple { + continue // no change + } + + var updateQuery *dbx.Query + + if !isOldMultiple && isNewMultiple { + // single -> multiple (convert to array) + updateQuery = txDao.DB().NewQuery(fmt.Sprintf( + `UPDATE {{%s}} set [[%s]] = ( + CASE + WHEN COALESCE([[%s]], '') = '' + THEN '[]' + ELSE ( + CASE + WHEN json_valid([[%s]]) AND json_type([[%s]]) == 'array' + THEN [[%s]] + ELSE json_array([[%s]]) + END + ) + END + )`, + newCollection.Name, + newField.Name, + newField.Name, + newField.Name, + newField.Name, + newField.Name, + newField.Name, + )) + } else { + // multiple -> single (keep only the last element) + // + // note: for file fields the actual files are not deleted + // allowing additional custom handling via migration. + updateQuery = txDao.DB().NewQuery(fmt.Sprintf( + `UPDATE {{%s}} set [[%s]] = ( + CASE + WHEN COALESCE([[%s]], '[]') = '[]' + THEN '' + ELSE ( + CASE + WHEN json_valid([[%s]]) AND json_type([[%s]]) == 'array' + THEN COALESCE(json_extract([[%s]], '$[#-1]'), '') + ELSE [[%s]] + END + ) + END + )`, + newCollection.Name, + newField.Name, + newField.Name, + newField.Name, + newField.Name, + newField.Name, + newField.Name, + )) + } + + if _, err := updateQuery.Execute(); err != nil { + return err + } + } + + return nil + }) +} + func (dao *Dao) syncCollectionReferences(collection *models.Collection, renamedFieldNames map[string]string, deletedFieldNames []string) error { if len(renamedFieldNames) == 0 && len(deletedFieldNames) == 0 { return nil // nothing to sync diff --git a/daos/record_table_sync_test.go b/daos/record_table_sync_test.go index 0e25490d..7c3d485f 100644 --- a/daos/record_table_sync_test.go +++ b/daos/record_table_sync_test.go @@ -1,12 +1,16 @@ package daos_test import ( + "bytes" + "encoding/json" "testing" + "github.com/pocketbase/dbx" "github.com/pocketbase/pocketbase/models" "github.com/pocketbase/pocketbase/models/schema" "github.com/pocketbase/pocketbase/tests" "github.com/pocketbase/pocketbase/tools/list" + "github.com/pocketbase/pocketbase/tools/types" ) func TestSyncRecordTableSchema(t *testing.T) { @@ -117,3 +121,132 @@ func TestSyncRecordTableSchema(t *testing.T) { } } } + +func TestSingleVsMultipleValuesNormalization(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + collection, err := app.Dao().FindCollectionByNameOrId("demo1") + if err != nil { + t.Fatal(err) + } + + // mock field changes + { + selectOneField := collection.Schema.GetFieldByName("select_one") + opt := selectOneField.Options.(*schema.SelectOptions) + opt.MaxSelect = 2 + } + { + selectManyField := collection.Schema.GetFieldByName("select_many") + opt := selectManyField.Options.(*schema.SelectOptions) + opt.MaxSelect = 1 + } + { + + fileOneField := collection.Schema.GetFieldByName("file_one") + opt := fileOneField.Options.(*schema.FileOptions) + opt.MaxSelect = 2 + } + { + fileManyField := collection.Schema.GetFieldByName("file_many") + opt := fileManyField.Options.(*schema.FileOptions) + opt.MaxSelect = 1 + + } + { + relOneField := collection.Schema.GetFieldByName("rel_one") + opt := relOneField.Options.(*schema.RelationOptions) + opt.MaxSelect = types.Pointer(2) + } + { + relManyField := collection.Schema.GetFieldByName("rel_many") + opt := relManyField.Options.(*schema.RelationOptions) + opt.MaxSelect = types.Pointer(1) + } + + if err := app.Dao().SaveCollection(collection); err != nil { + t.Fatal(err) + } + + type expectation struct { + SelectOne string `db:"select_one"` + SelectMany string `db:"select_many"` + FileOne string `db:"file_one"` + FileMany string `db:"file_many"` + RelOne string `db:"rel_one"` + RelMany string `db:"rel_many"` + } + + scenarios := []struct { + recordId string + expected expectation + }{ + { + "imy661ixudk5izi", + expectation{ + SelectOne: `[]`, + SelectMany: ``, + FileOne: `[]`, + FileMany: ``, + RelOne: `[]`, + RelMany: ``, + }, + }, + { + "al1h9ijdeojtsjy", + expectation{ + SelectOne: `["optionB"]`, + SelectMany: `optionB`, + FileOne: `["300_Jsjq7RdBgA.png"]`, + FileMany: ``, + RelOne: `["84nmscqy84lsi1t"]`, + RelMany: `oap640cot4yru2s`, + }, + }, + { + "84nmscqy84lsi1t", + expectation{ + SelectOne: `["optionB"]`, + SelectMany: `optionC`, + FileOne: `["test_d61b33QdDU.txt"]`, + FileMany: `test_tC1Yc87DfC.txt`, + RelOne: `[]`, + RelMany: `oap640cot4yru2s`, + }, + }, + } + + for _, s := range scenarios { + result := new(expectation) + + err := app.Dao().DB().Select( + "select_one", + "select_many", + "file_one", + "file_many", + "rel_one", + "rel_many", + ).From(collection.Name).Where(dbx.HashExp{"id": s.recordId}).One(result) + if err != nil { + t.Errorf("[%s] Failed to load record: %v", s.recordId, err) + continue + } + + encodedResult, err := json.Marshal(result) + if err != nil { + t.Errorf("[%s] Failed to encode result: %v", s.recordId, err) + continue + } + + encodedExpectation, err := json.Marshal(s.expected) + if err != nil { + t.Errorf("[%s] Failed to encode expectation: %v", s.recordId, err) + continue + } + + if !bytes.EqualFold(encodedExpectation, encodedResult) { + t.Errorf("[%s] Expected \n%s, \ngot \n%s", s.recordId, encodedExpectation, encodedResult) + } + } +} diff --git a/migrations/1678082970_normalize_single_multiple_values.go b/migrations/1678082970_normalize_single_multiple_values.go new file mode 100644 index 00000000..0434ebf8 --- /dev/null +++ b/migrations/1678082970_normalize_single_multiple_values.go @@ -0,0 +1,104 @@ +package migrations + +import ( + "fmt" + + "github.com/pocketbase/dbx" + "github.com/pocketbase/pocketbase/daos" + "github.com/pocketbase/pocketbase/models" + "github.com/pocketbase/pocketbase/models/schema" +) + +// Normalizes old single and multiple values of MultiValuer fields (file, select, relation). +func init() { + AppMigrations.Register(func(db dbx.Builder) error { + dao := daos.New(db) + + collections := []*models.Collection{} + if err := dao.CollectionQuery().All(&collections); err != nil { + return err + } + + for _, c := range collections { + if c.IsView() { + // skip view collections + continue + } + + for _, f := range c.Schema.Fields() { + opt, ok := f.Options.(schema.MultiValuer) + if !ok { + continue + } + + var updateQuery *dbx.Query + + if opt.IsMultiple() { + updateQuery = dao.DB().NewQuery(fmt.Sprintf( + `UPDATE {{%s}} set [[%s]] = ( + CASE + WHEN COALESCE([[%s]], '') = '' + THEN '[]' + ELSE ( + CASE + WHEN json_valid([[%s]]) AND json_type([[%s]]) == 'array' + THEN [[%s]] + ELSE json_array([[%s]]) + END + ) + END + )`, + c.Name, + f.Name, + f.Name, + f.Name, + f.Name, + f.Name, + f.Name, + )) + } else { + updateQuery = dao.DB().NewQuery(fmt.Sprintf( + `UPDATE {{%s}} set [[%s]] = ( + CASE + WHEN COALESCE([[%s]], '[]') = '[]' + THEN '' + ELSE ( + CASE + WHEN json_valid([[%s]]) AND json_type([[%s]]) == 'array' + THEN COALESCE(json_extract([[%s]], '$[#-1]'), '') + ELSE [[%s]] + END + ) + END + )`, + c.Name, + f.Name, + f.Name, + f.Name, + f.Name, + f.Name, + f.Name, + )) + } + + if _, err := updateQuery.Execute(); err != nil { + return err + } + } + } + + // trigger view query update after the records normalization + // (ignore save error in case of invalid query to allow users to change it from the UI) + for _, c := range collections { + if !c.IsView() { + continue + } + + dao.SaveCollection(c) + } + + return nil + }, func(db dbx.Builder) error { + return nil + }) +} diff --git a/models/schema/schema_field.go b/models/schema/schema_field.go index 115e6265..ab065153 100644 --- a/models/schema/schema_field.go +++ b/models/schema/schema_field.go @@ -321,7 +321,7 @@ func (f *SchemaField) PrepareValue(value any) any { val := list.ToUniqueStringSlice(value) options, _ := f.Options.(*SelectOptions) - if options.MaxSelect <= 1 { + if !options.IsMultiple() { if len(val) > 0 { return val[len(val)-1] // the last selected } @@ -333,7 +333,7 @@ func (f *SchemaField) PrepareValue(value any) any { val := list.ToUniqueStringSlice(value) options, _ := f.Options.(*FileOptions) - if options.MaxSelect <= 1 { + if !options.IsMultiple() { if len(val) > 0 { return val[len(val)-1] // the last selected } @@ -345,7 +345,7 @@ func (f *SchemaField) PrepareValue(value any) any { ids := list.ToUniqueStringSlice(value) options, _ := f.Options.(*RelationOptions) - if options.MaxSelect != nil && *options.MaxSelect <= 1 { + if !options.IsMultiple() { if len(ids) > 0 { return ids[len(ids)-1] // the last selected } @@ -399,7 +399,12 @@ func (f *SchemaField) PrepareValueWithModifier(baseValue any, modifier string, m // ------------------------------------------------------------------- -// FieldOptions interfaces that defines common methods that every field options struct has. +// MultiValuer defines common interface methods that every multi-valued (eg. with MaxSelect) field option struct has. +type MultiValuer interface { + IsMultiple() bool +} + +// FieldOptions defines common interface methods that every field option struct has. type FieldOptions interface { Validate() error } @@ -564,6 +569,12 @@ func (o SelectOptions) Validate() error { ) } +// IsMultiple implements MultiValuer interface and checks whether the +// current field options support multiple values. +func (o SelectOptions) IsMultiple() bool { + return o.MaxSelect > 1 +} + // ------------------------------------------------------------------- type JsonOptions struct { @@ -575,6 +586,8 @@ func (o JsonOptions) Validate() error { // ------------------------------------------------------------------- +var _ MultiValuer = (*FileOptions)(nil) + type FileOptions struct { MaxSelect int `form:"maxSelect" json:"maxSelect"` MaxSize int `form:"maxSize" json:"maxSize"` // in bytes @@ -593,8 +606,16 @@ func (o FileOptions) Validate() error { ) } +// IsMultiple implements MultiValuer interface and checks whether the +// current field options support multiple values. +func (o FileOptions) IsMultiple() bool { + return o.MaxSelect > 1 +} + // ------------------------------------------------------------------- +var _ MultiValuer = (*RelationOptions)(nil) + type RelationOptions struct { // CollectionId is the id of the related collection. CollectionId string `form:"collectionId" json:"collectionId"` @@ -632,6 +653,12 @@ func (o RelationOptions) Validate() error { ) } +// IsMultiple implements MultiValuer interface and checks whether the +// current field options support multiple values. +func (o RelationOptions) IsMultiple() bool { + return o.MaxSelect == nil || *o.MaxSelect > 1 +} + // ------------------------------------------------------------------- // Deprecated: Will be removed in v0.9+ diff --git a/models/schema/schema_field_test.go b/models/schema/schema_field_test.go index d0b265ca..5af0dfcc 100644 --- a/models/schema/schema_field_test.go +++ b/models/schema/schema_field_test.go @@ -1983,6 +1983,28 @@ func TestSelectOptionsValidate(t *testing.T) { checkFieldOptionsScenarios(t, scenarios) } +func TestSelectOptionsIsMultiple(t *testing.T) { + scenarios := []struct { + maxSelect int + expect bool + }{ + {-1, false}, + {0, false}, + {1, false}, + {2, true}, + } + + for i, s := range scenarios { + opt := schema.SelectOptions{ + MaxSelect: s.maxSelect, + } + + if v := opt.IsMultiple(); v != s.expect { + t.Errorf("[%d] Expected %v, got %v", i, s.expect, v) + } + } +} + func TestJsonOptionsValidate(t *testing.T) { scenarios := []fieldOptionsScenario{ { @@ -2053,6 +2075,28 @@ func TestFileOptionsValidate(t *testing.T) { checkFieldOptionsScenarios(t, scenarios) } +func TestFileOptionsIsMultiple(t *testing.T) { + scenarios := []struct { + maxSelect int + expect bool + }{ + {-1, false}, + {0, false}, + {1, false}, + {2, true}, + } + + for i, s := range scenarios { + opt := schema.FileOptions{ + MaxSelect: s.maxSelect, + } + + if v := opt.IsMultiple(); v != s.expect { + t.Errorf("[%d] Expected %v, got %v", i, s.expect, v) + } + } +} + func TestRelationOptionsValidate(t *testing.T) { scenarios := []fieldOptionsScenario{ { @@ -2088,3 +2132,26 @@ func TestRelationOptionsValidate(t *testing.T) { checkFieldOptionsScenarios(t, scenarios) } + +func TestRelationOptionsIsMultiple(t *testing.T) { + scenarios := []struct { + maxSelect *int + expect bool + }{ + {nil, true}, + {types.Pointer(-1), false}, + {types.Pointer(0), false}, + {types.Pointer(1), false}, + {types.Pointer(2), true}, + } + + for i, s := range scenarios { + opt := schema.RelationOptions{ + MaxSelect: s.maxSelect, + } + + if v := opt.IsMultiple(); v != s.expect { + t.Errorf("[%d] Expected %v, got %v", i, s.expect, v) + } + } +} diff --git a/tests/data/data.db b/tests/data/data.db index ed9b50b18ca9f1b30c7de747685aef98f94a9542..99125146ef3e6f0bfebe0652d0c68a2eaff44409 100644 GIT binary patch delta 865 zcmb7>Ur19?9LLYOXPeXAd(I61g>y}XaA|9Qbh}&ZLDtAr%3A)p7Tztr>ZScl?{rrC4!^te+ z%o4s_U0MYI@clYay>nNZT-qvN8-9CtXr7p8Tn5M+qL7QEpSZ~m{1LC<1w4(9;IhS9 z<7Z~dOCwR|aC=xg>-2i;d^|B0i$ujqfgcy+BT<2mrJ|CUR7E-xO$p<+kp$3w@ojuq zd*Lu3ZWHx5TAXf{<;R40Bqs2Ylr$>DCGj*bCHRCmH2532vszbz0vZ)@yWK9A-OHzD zAKS>{E%Pa;R^Th+-pXsEYyiwqnKwjK{!dJYA9E#WPG*pfAS3-ol+A9^Sh zaE<;<735#&f*-cDhAg3={HX^GP*%JA>^U?kIS#c|7?@!|!8xF5hjZ$?W+8o{p~Tpz zFRz?QF^n=fhF(G}#zR6>7?kRK@l-S_A5Ej%Cf8oO+v0Hf?9INGW}DN__FvsuTma_| zD#i)4uG@}1U7`H5@w1$qc|Z}_CZUGy33aelSwDv;Sh0P4OFsbA4J9d|>(oL9S$+VH CaRs0N delta 566 zcmYL`OK1~O6o&7aJCmn#Z&FDok|ri8T_uf^iFq^-C{&@gD5b59iGsFCMI{wuqe<}9 zM2onyC|*I(;KGfol9r2zkcAX98(pd3#)S(-L|5(-CqZ!W|7Y?22hL-yMb=v6uFI|< zgp&URdfnUzMkED=a*Yq(h(=?4!h5R8O2-m}pNRx-AzR2<{!qyB}GrLX@chhH^w(^V%Fo?BP z!NyJv!4CWFhTs6Tp|+gbXAa1+Qj>})pP7X{uqCP9u$ts<4}5zsW)a=ymR}%{!%;OE zjfb?jn!Q-fSBhFbS1IMHkyv46@%*L5f~IQPXjmN$M|a_r^L63A@eoL2ZFbREo__t#7r}IFK$U~eGNh4!J~Kr zV4huAz>`c)W7D3R95*@?1!`9=pIt8Zr1cH4PYbx>oR0Nry-#`|+&9F|r13WM9K(&> zn$a;lY`TdJafALEglbbUHkZb2P}#eWFvMSE@Z_LX2ZC5YMuS+;N68Bo@nOK^P;SqS zjPU10{25sHjW8&#D;66+cMU%fEE{FBtrvNM`FsigFtElt9%HYrW6o!wc_6J TSaN?NsYODOgvPr!FnIq0KIWQ}