From 54344f2b9d383ac8c91eafa42f6421918fcbc772 Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Wed, 9 Oct 2024 17:11:14 +0300 Subject: [PATCH] fixed autogenerated go collection update migration template --- CHANGELOG.md | 47 +++++++++- core/fields_list.go | 120 +++++++++++++++++--------- core/fields_list_test.go | 100 +++++++++++++++++++++ plugins/migratecmd/automigrate.go | 17 +++- plugins/migratecmd/migratecmd_test.go | 16 ++-- plugins/migratecmd/templates.go | 8 +- ui/.env | 2 +- 7 files changed, 252 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 679fc474..e12abcd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,55 @@ -## v0.23.0-rc4 (WIP) +## v0.23.0-rc4 > [!CAUTION] > **This is a prerelease intended for test and experimental purposes only!** - Added more user friendly view collection truncate error message. +- Added `FieldsList.AddMarshaledJSON(json)` helper method to load a serialized json array of objects or a single json object into an existing collection fields list. + +- Added an extra suffix character to the name of autogenerated template migration file for `*test` suffixed collections to prevent acidentally resulting in `_test.go` migration files. + +- Fixed the autogenerated Go migration template when updating a collection ([#5631](https://github.com/pocketbase/pocketbase/discussions/5631)). + + ⚠️ If you have already used a previous prerelease and have autogenerated Go migration files, please check the migration files **`{timestamp}_updated_{collection}.go`** and change manually: + + + + + + + + + + +
Old (broken)New
+ + ```go + // add field / update field + if err := json.Unmarshal([]byte(`[{ + ... + }]`), &collection.Fields); err != nil { + return err + } + ``` + + + + ```go + // add field / update field + if err := collection.Fields.AddMarshaledJSON([]byte(`{ + ... + }`)); err != nil { + return err + } + ``` + +
+ To test that your Go migration files works correctly you can try to start PocketBase with a new temp pb_data, e.g.: + + ```go + go run . serve --dir="pb_data_temp" + ``` ## v0.23.0-rc3 diff --git a/core/fields_list.go b/core/fields_list.go index e5f09b86..c3be1bd7 100644 --- a/core/fields_list.go +++ b/core/fields_list.go @@ -10,21 +10,21 @@ import ( // NewFieldsList creates a new FieldsList instance with the provided fields. func NewFieldsList(fields ...Field) FieldsList { - s := make(FieldsList, 0, len(fields)) + l := make(FieldsList, 0, len(fields)) for _, f := range fields { - s.Add(f) + l.Add(f) } - return s + return l } // FieldsList defines a Collection slice of fields. type FieldsList []Field // Clone creates a deep clone of the current list. -func (s FieldsList) Clone() (FieldsList, error) { - copyRaw, err := json.Marshal(s) +func (l FieldsList) Clone() (FieldsList, error) { + copyRaw, err := json.Marshal(l) if err != nil { return nil, err } @@ -38,10 +38,10 @@ func (s FieldsList) Clone() (FieldsList, error) { } // FieldNames returns a slice with the name of all list fields. -func (s FieldsList) FieldNames() []string { - result := make([]string, len(s)) +func (l FieldsList) FieldNames() []string { + result := make([]string, len(l)) - for i, field := range s { + for i, field := range l { result[i] = field.GetName() } @@ -50,10 +50,10 @@ func (s FieldsList) FieldNames() []string { // AsMap returns a map with all registered list field. // The returned map is indexed with each field name. -func (s FieldsList) AsMap() map[string]Field { - result := make(map[string]Field, len(s)) +func (l FieldsList) AsMap() map[string]Field { + result := make(map[string]Field, len(l)) - for _, field := range s { + for _, field := range l { result[field.GetName()] = field } @@ -61,8 +61,8 @@ func (s FieldsList) AsMap() map[string]Field { } // GetById returns a single field by its id. -func (s FieldsList) GetById(fieldId string) Field { - for _, field := range s { +func (l FieldsList) GetById(fieldId string) Field { + for _, field := range l { if field.GetId() == fieldId { return field } @@ -71,8 +71,8 @@ func (s FieldsList) GetById(fieldId string) Field { } // GetByName returns a single field by its name. -func (s FieldsList) GetByName(fieldName string) Field { - for _, field := range s { +func (l FieldsList) GetByName(fieldName string) Field { + for _, field := range l { if field.GetName() == fieldName { return field } @@ -83,11 +83,11 @@ func (s FieldsList) GetByName(fieldName string) Field { // RemoveById removes a single field by its id. // // This method does nothing if field with the specified id doesn't exist. -func (s *FieldsList) RemoveById(fieldId string) { - fields := *s +func (l *FieldsList) RemoveById(fieldId string) { + fields := *l for i, field := range fields { if field.GetId() == fieldId { - *s = append(fields[:i], fields[i+1:]...) + *l = append(fields[:i], fields[i+1:]...) return } } @@ -96,11 +96,11 @@ func (s *FieldsList) RemoveById(fieldId string) { // RemoveByName removes a single field by its name. // // This method does nothing if field with the specified name doesn't exist. -func (s *FieldsList) RemoveByName(fieldName string) { - fields := *s +func (l *FieldsList) RemoveByName(fieldName string) { + fields := *l for i, field := range fields { if field.GetName() == fieldName { - *s = append(fields[:i], fields[i+1:]...) + *l = append(fields[:i], fields[i+1:]...) return } } @@ -115,13 +115,51 @@ func (s *FieldsList) RemoveByName(fieldName string) { // then the existing field is replaced with the new one. // // Otherwise the new field is appended after the other list fields. -func (s *FieldsList) Add(fields ...Field) { +func (l *FieldsList) Add(fields ...Field) { for _, f := range fields { - s.add(f) + l.add(f) } } -func (s *FieldsList) add(newField Field) { +// AddMarshaledJSON parses the provided raw json data and adds the +// found fields into the current list (following the same rule as the Add method). +// +// rawJSON could be either a serialized array of field objects or a single field object. +// +// Example: +// +// l.AddMarshaledJSON([]byte{`{"type":"text", name: "test"}`}) +// l.AddMarshaledJSON([]byte{`[{"type":"text", name: "test1"}, {"type":"text", name: "test2"}]`}) +func (l *FieldsList) AddMarshaledJSON(rawJSON []byte) error { + if len(rawJSON) == 0 { + return nil // nothing to add + } + + // try to unmarshal first into a new fieds list + // (assuming that rawJSON is array of objects) + extractedFields := FieldsList{} + err := json.Unmarshal(rawJSON, &extractedFields) + if err != nil { + // try again but wrap the rawJSON in [] + // (assuming that rawJSON is a single object) + wrapped := make([]byte, 0, len(rawJSON)+2) + wrapped = append(wrapped, '[') + wrapped = append(wrapped, rawJSON...) + wrapped = append(wrapped, ']') + err = json.Unmarshal(wrapped, &extractedFields) + if err != nil { + return fmt.Errorf("failed to unmarshal the provided JSON - expects array of objects or just single object: %w", err) + } + } + + for _, f := range extractedFields { + l.add(f) + } + + return nil +} + +func (l *FieldsList) add(newField Field) { newFieldId := newField.GetId() // set default id @@ -134,23 +172,23 @@ func (s *FieldsList) add(newField Field) { newField.SetId(newFieldId) } - fields := *s + fields := *l for i, field := range fields { // replace existing if newFieldId != "" && field.GetId() == newFieldId { - (*s)[i] = newField + (*l)[i] = newField return } } // add new field - *s = append(fields, newField) + *l = append(fields, newField) } // String returns the string representation of the current list. -func (s FieldsList) String() string { - v, _ := json.Marshal(s) +func (l FieldsList) String() string { + v, _ := json.Marshal(l) return string(v) } @@ -188,31 +226,31 @@ func (fwt *fieldWithType) UnmarshalJSON(data []byte) error { // UnmarshalJSON implements [json.Unmarshaler] and // loads the provided json data into the current FieldsList. -func (s *FieldsList) UnmarshalJSON(data []byte) error { +func (l *FieldsList) UnmarshalJSON(data []byte) error { fwts := []fieldWithType{} if err := json.Unmarshal(data, &fwts); err != nil { return err } - *s = []Field{} // reset + *l = []Field{} // reset for _, fwt := range fwts { - s.Add(fwt.Field) + l.Add(fwt.Field) } return nil } // MarshalJSON implements the [json.Marshaler] interface. -func (s FieldsList) MarshalJSON() ([]byte, error) { - if s == nil { - s = []Field{} // always init to ensure that it is serialized as empty array +func (l FieldsList) MarshalJSON() ([]byte, error) { + if l == nil { + l = []Field{} // always init to ensure that it is serialized as empty array } - wrapper := make([]map[string]any, 0, len(s)) + wrapper := make([]map[string]any, 0, len(l)) - for _, f := range s { + for _, f := range l { // precompute the json into a map so that we can append the type to a flatten object raw, err := json.Marshal(f) if err != nil { @@ -232,15 +270,15 @@ func (s FieldsList) MarshalJSON() ([]byte, error) { } // Value implements the [driver.Valuer] interface. -func (s FieldsList) Value() (driver.Value, error) { - data, err := json.Marshal(s) +func (l FieldsList) Value() (driver.Value, error) { + data, err := json.Marshal(l) return string(data), err } // Scan implements [sql.Scanner] interface to scan the provided value // into the current FieldsList instance. -func (s *FieldsList) Scan(value any) error { +func (l *FieldsList) Scan(value any) error { var data []byte switch v := value.(type) { case nil: @@ -257,5 +295,5 @@ func (s *FieldsList) Scan(value any) error { data = []byte("[]") } - return s.UnmarshalJSON(data) + return l.UnmarshalJSON(data) } diff --git a/core/fields_list_test.go b/core/fields_list_test.go index 08defbdd..d567bc76 100644 --- a/core/fields_list_test.go +++ b/core/fields_list_test.go @@ -201,6 +201,106 @@ func TestFieldsListAdd(t *testing.T) { } } +func TestFieldsListAddMarshaledJSON(t *testing.T) { + t.Parallel() + + scenarios := []struct { + name string + raw []byte + expectError bool + expectedFields map[string]string + }{ + { + "nil", + nil, + false, + map[string]string{"abc": core.FieldTypeNumber}, + }, + { + "empty array", + []byte(`[]`), + false, + map[string]string{"abc": core.FieldTypeNumber}, + }, + { + "empty object", + []byte(`{}`), + true, + map[string]string{"abc": core.FieldTypeNumber}, + }, + { + "array with empty object", + []byte(`[{}]`), + true, + map[string]string{"abc": core.FieldTypeNumber}, + }, + { + "single object with invalid type", + []byte(`{"type":"missing","name":"test"}`), + true, + map[string]string{"abc": core.FieldTypeNumber}, + }, + { + "single object with valid type", + []byte(`{"type":"text","name":"test"}`), + false, + map[string]string{ + "abc": core.FieldTypeNumber, + "test": core.FieldTypeText, + }, + }, + { + "array of object with valid types", + []byte(`[{"type":"text","name":"test1"},{"type":"url","name":"test2"}]`), + false, + map[string]string{ + "abc": core.FieldTypeNumber, + "test1": core.FieldTypeText, + "test2": core.FieldTypeURL, + }, + }, + { + "fields with duplicated ids should replace existing fields", + []byte(`[{"type":"text","name":"test1"},{"type":"url","name":"test2"},{"type":"text","name":"abc2", "id":"abc_id"}]`), + false, + map[string]string{ + "abc2": core.FieldTypeText, + "test1": core.FieldTypeText, + "test2": core.FieldTypeURL, + }, + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + testList := core.NewFieldsList(&core.NumberField{Name: "abc", Id: "abc_id"}) + err := testList.AddMarshaledJSON(s.raw) + + hasErr := err != nil + if hasErr != s.expectError { + t.Fatalf("Expected hasErr %v, got %v", s.expectError, hasErr) + } + + if len(s.expectedFields) != len(testList) { + t.Fatalf("Expected %d fields, got %d", len(s.expectedFields), len(testList)) + } + + for fieldName, typ := range s.expectedFields { + f := testList.GetByName(fieldName) + + if f == nil { + t.Errorf("Missing expected field %q", fieldName) + continue + } + + if f.Type() != typ { + t.Errorf("Expect field %q to has type %q, got %q", fieldName, typ, f.Type()) + } + } + }) + } +} + func TestFieldsListStringAndValue(t *testing.T) { t.Run("empty list", func(t *testing.T) { testFieldsList := core.NewFieldsList() diff --git a/plugins/migratecmd/automigrate.go b/plugins/migratecmd/automigrate.go index b94dabc0..a022a4e3 100644 --- a/plugins/migratecmd/automigrate.go +++ b/plugins/migratecmd/automigrate.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" "github.com/pocketbase/dbx" @@ -59,11 +60,11 @@ func (p *plugin) automigrateOnCollectionChange(e *core.CollectionRequestEvent) e var action string switch { case new == nil: - action = "deleted_" + old.Name + action = "deleted_" + normalizeCollectionName(old.Name) case old == nil: - action = "created_" + new.Name + action = "created_" + normalizeCollectionName(new.Name) default: - action = "updated_" + old.Name + action = "updated_" + normalizeCollectionName(old.Name) } name := fmt.Sprintf("%d_%s.%s", time.Now().Unix(), action, p.config.TemplateLang) @@ -93,3 +94,13 @@ func (p *plugin) automigrateOnCollectionChange(e *core.CollectionRequestEvent) e return nil }) } + +func normalizeCollectionName(name string) string { + // adds an extra "_" suffix to the name in case the collection ends + // with "test" to prevent accidentally resulting in "_test.go"/"_test.js" files + if strings.HasSuffix(strings.ToLower(name), "test") { + name += "_" + } + + return name +} diff --git a/plugins/migratecmd/migratecmd_test.go b/plugins/migratecmd/migratecmd_test.go index 043ca865..57412978 100644 --- a/plugins/migratecmd/migratecmd_test.go +++ b/plugins/migratecmd/migratecmd_test.go @@ -1036,7 +1036,7 @@ func init() { collection.Fields.RemoveById("f3_id") // add field - if err := json.Unmarshal([]byte(` + "`" + `[{ + if err := collection.Fields.AddMarshaledJSON([]byte(` + "`" + `{ "autogeneratePattern": "", "hidden": false, "id": "f4_id", @@ -1049,12 +1049,12 @@ func init() { "required": false, "system": false, "type": "text" - }]` + "`" + `), &collection.Fields); err != nil { + }` + "`" + `)); err != nil { return err } // update field - if err := json.Unmarshal([]byte(` + "`" + `[{ + if err := collection.Fields.AddMarshaledJSON([]byte(` + "`" + `{ "hidden": false, "id": "f2_id", "max": null, @@ -1065,7 +1065,7 @@ func init() { "required": false, "system": false, "type": "number" - }]` + "`" + `), &collection.Fields); err != nil { + }` + "`" + `)); err != nil { return err } @@ -1099,7 +1099,7 @@ func init() { } // add field - if err := json.Unmarshal([]byte(` + "`" + `[{ + if err := collection.Fields.AddMarshaledJSON([]byte(` + "`" + `{ "hidden": false, "id": "f3_id", "name": "f3_name", @@ -1107,7 +1107,7 @@ func init() { "required": false, "system": false, "type": "bool" - }]` + "`" + `), &collection.Fields); err != nil { + }` + "`" + `)); err != nil { return err } @@ -1115,7 +1115,7 @@ func init() { collection.Fields.RemoveById("f4_id") // update field - if err := json.Unmarshal([]byte(` + "`" + `[{ + if err := collection.Fields.AddMarshaledJSON([]byte(` + "`" + `{ "hidden": false, "id": "f2_id", "max": null, @@ -1126,7 +1126,7 @@ func init() { "required": false, "system": false, "type": "number" - }]` + "`" + `), &collection.Fields); err != nil { + }` + "`" + `)); err != nil { return err } diff --git a/plugins/migratecmd/templates.go b/plugins/migratecmd/templates.go index 3ce69a6e..0c01e6eb 100644 --- a/plugins/migratecmd/templates.go +++ b/plugins/migratecmd/templates.go @@ -546,7 +546,7 @@ func (p *plugin) goDiffTemplate(new *core.Collection, old *core.Collection) (str upParts = append(upParts, fmt.Sprintf("%s.Fields.RemoveById(%q)\n", varName, oldField.GetId())) downParts = append(downParts, "// add field") - downParts = append(downParts, goErrIf(fmt.Sprintf("json.Unmarshal([]byte(`[%s]`), &%s.Fields)", escapeBacktick(string(rawOldField)), varName))) + downParts = append(downParts, goErrIf(fmt.Sprintf("%s.Fields.AddMarshaledJSON([]byte(`%s`))", varName, escapeBacktick(string(rawOldField))))) } // created fields @@ -561,7 +561,7 @@ func (p *plugin) goDiffTemplate(new *core.Collection, old *core.Collection) (str } upParts = append(upParts, "// add field") - upParts = append(upParts, goErrIf(fmt.Sprintf("json.Unmarshal([]byte(`[%s]`), &%s.Fields)", escapeBacktick(string(rawNewField)), varName))) + upParts = append(upParts, goErrIf(fmt.Sprintf("%s.Fields.AddMarshaledJSON([]byte(`%s`))", varName, escapeBacktick(string(rawNewField))))) downParts = append(downParts, "// remove field") downParts = append(downParts, fmt.Sprintf("%s.Fields.RemoveById(%q)\n", varName, newField.GetId())) @@ -591,10 +591,10 @@ func (p *plugin) goDiffTemplate(new *core.Collection, old *core.Collection) (str } upParts = append(upParts, "// update field") - upParts = append(upParts, goErrIf(fmt.Sprintf("json.Unmarshal([]byte(`[%s]`), &%s.Fields)", escapeBacktick(string(rawNewField)), varName))) + upParts = append(upParts, goErrIf(fmt.Sprintf("%s.Fields.AddMarshaledJSON([]byte(`%s`))", varName, escapeBacktick(string(rawNewField))))) downParts = append(downParts, "// update field") - downParts = append(downParts, goErrIf(fmt.Sprintf("json.Unmarshal([]byte(`[%s]`), &%s.Fields)", escapeBacktick(string(rawOldField)), varName))) + downParts = append(downParts, goErrIf(fmt.Sprintf("%s.Fields.AddMarshaledJSON([]byte(`%s`))", varName, escapeBacktick(string(rawOldField))))) } // --------------------------------------------------------------- diff --git a/ui/.env b/ui/.env index 4ddd6566..591133d1 100644 --- a/ui/.env +++ b/ui/.env @@ -11,4 +11,4 @@ PB_DOCS_URL = "https://pocketbase.io/docs/" PB_JS_SDK_URL = "https://github.com/pocketbase/js-sdk" PB_DART_SDK_URL = "https://github.com/pocketbase/dart-sdk" PB_RELEASES = "https://github.com/pocketbase/pocketbase/releases" -PB_VERSION = "v0.23.0-rc3" +PB_VERSION = "v0.23.0-rc4"