diff --git a/apis/collection_test.go b/apis/collection_test.go index 1f75aacb..daa765e8 100644 --- a/apis/collection_test.go +++ b/apis/collection_test.go @@ -5,6 +5,8 @@ import ( "strings" "testing" + "github.com/labstack/echo/v5" + "github.com/pocketbase/pocketbase/models" "github.com/pocketbase/pocketbase/tests" ) @@ -473,19 +475,229 @@ func TestCollectionImport(t *testing.T) { `"data":{`, `"collections":{"code":"validation_required"`, }, + AfterFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { + collections := []*models.Collection{} + if err := app.Dao().CollectionQuery().All(&collections); err != nil { + t.Fatal(err) + } + if len(collections) != 5 { + t.Fatalf("Expected %d collections, got %d", 5, len(collections)) + } + }, }, { - Name: "authorized as admin + deleting collections", + Name: "authorized as admin + trying to delete system collections", Method: http.MethodPut, Url: "/api/collections/import", - Body: strings.NewReader(`{"collections":[]}`), + Body: strings.NewReader(`{"deleteMissing": true, "collections":[{"name": "test123"}]}`), RequestHeaders: map[string]string{ "Authorization": "Admin eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjJiNGE5N2NjLTNmODMtNGQwMS1hMjZiLTNkNzdiYzg0MmQzYyIsInR5cGUiOiJhZG1pbiIsImV4cCI6MTg3MzQ2Mjc5Mn0.AtRtXR6FHBrCUGkj5OffhmxLbSZaQ4L_Qgw4gfoHyfo", }, ExpectedStatus: 400, ExpectedContent: []string{ `"data":{`, - `"collections":{"code":"validation_required"`, + `"collections":{"code":"collections_import_failure"`, + }, + ExpectedEvents: map[string]int{ + "OnCollectionsBeforeImportRequest": 1, + }, + AfterFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { + collections := []*models.Collection{} + if err := app.Dao().CollectionQuery().All(&collections); err != nil { + t.Fatal(err) + } + if len(collections) != 5 { + t.Fatalf("Expected %d collections, got %d", 5, len(collections)) + } + }, + }, + { + Name: "authorized as admin + collections validator failure", + Method: http.MethodPut, + Url: "/api/collections/import", + Body: strings.NewReader(`{ + "collections":[ + { + "name": "import1", + "schema": [ + { + "id": "koih1lqx", + "name": "test", + "type": "text" + } + ] + }, + {"name": "import2"} + ] + }`), + RequestHeaders: map[string]string{ + "Authorization": "Admin eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjJiNGE5N2NjLTNmODMtNGQwMS1hMjZiLTNkNzdiYzg0MmQzYyIsInR5cGUiOiJhZG1pbiIsImV4cCI6MTg3MzQ2Mjc5Mn0.AtRtXR6FHBrCUGkj5OffhmxLbSZaQ4L_Qgw4gfoHyfo", + }, + ExpectedStatus: 400, + ExpectedContent: []string{ + `"data":{`, + `"collections":{"code":"collections_import_failure"`, + }, + ExpectedEvents: map[string]int{ + "OnCollectionsBeforeImportRequest": 1, + "OnModelBeforeCreate": 2, + "OnModelAfterCreate": 1, + }, + AfterFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { + collections := []*models.Collection{} + if err := app.Dao().CollectionQuery().All(&collections); err != nil { + t.Fatal(err) + } + if len(collections) != 5 { + t.Fatalf("Expected %d collections, got %d", 5, len(collections)) + } + }, + }, + { + Name: "authorized as admin + successfull collections save", + Method: http.MethodPut, + Url: "/api/collections/import", + Body: strings.NewReader(`{ + "collections":[ + { + "name": "import1", + "schema": [ + { + "id": "koih1lqx", + "name": "test", + "type": "text" + } + ] + }, + { + "name": "import2", + "schema": [ + { + "id": "koih1lqx", + "name": "test", + "type": "text" + } + ] + } + ] + }`), + RequestHeaders: map[string]string{ + "Authorization": "Admin eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjJiNGE5N2NjLTNmODMtNGQwMS1hMjZiLTNkNzdiYzg0MmQzYyIsInR5cGUiOiJhZG1pbiIsImV4cCI6MTg3MzQ2Mjc5Mn0.AtRtXR6FHBrCUGkj5OffhmxLbSZaQ4L_Qgw4gfoHyfo", + }, + ExpectedStatus: 204, + ExpectedEvents: map[string]int{ + "OnCollectionsBeforeImportRequest": 1, + "OnCollectionsAfterImportRequest": 1, + "OnModelBeforeCreate": 2, + "OnModelAfterCreate": 2, + }, + AfterFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { + collections := []*models.Collection{} + if err := app.Dao().CollectionQuery().All(&collections); err != nil { + t.Fatal(err) + } + if len(collections) != 7 { + t.Fatalf("Expected %d collections, got %d", 7, len(collections)) + } + }, + }, + { + Name: "authorized as admin + successfull collections save and old non-system collections deletion", + Method: http.MethodPut, + Url: "/api/collections/import", + Body: strings.NewReader(`{ + "deleteMissing": true, + "collections":[ + { + "id":"abe78266-fd4d-4aea-962d-8c0138ac522b", + "name":"profiles", + "system":true, + "listRule":"userId = @request.user.id", + "viewRule":"created > 'test_change'", + "createRule":"userId = @request.user.id", + "updateRule":"userId = @request.user.id", + "deleteRule":"userId = @request.user.id", + "schema":[ + { + "id":"koih1lqx", + "name":"userId", + "type":"user", + "system":true, + "required":true, + "unique":true, + "options":{ + "maxSelect":1, + "cascadeDelete":true + } + }, + { + "id":"69ycbg3q", + "name":"rel", + "type":"relation", + "system":false, + "required":false, + "unique":false, + "options":{ + "maxSelect":2, + "collectionId":"abe78266-fd4d-4aea-962d-8c0138ac522b", + "cascadeDelete":false + } + } + ] + }, + { + "id":"3f2888f8-075d-49fe-9d09-ea7e951000dc", + "name":"demo", + "schema":[ + { + "id":"_2hlxbmp", + "name":"title", + "type":"text", + "system":false, + "required":true, + "unique":false, + "options":{ + "min":3, + "max":null, + "pattern":"" + } + } + ] + }, + { + "name": "new_import", + "schema": [ + { + "id": "koih1lqx", + "name": "test", + "type": "text" + } + ] + } + ] + }`), + RequestHeaders: map[string]string{ + "Authorization": "Admin eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjJiNGE5N2NjLTNmODMtNGQwMS1hMjZiLTNkNzdiYzg0MmQzYyIsInR5cGUiOiJhZG1pbiIsImV4cCI6MTg3MzQ2Mjc5Mn0.AtRtXR6FHBrCUGkj5OffhmxLbSZaQ4L_Qgw4gfoHyfo", + }, + ExpectedStatus: 204, + ExpectedEvents: map[string]int{ + "OnCollectionsAfterImportRequest": 1, + "OnCollectionsBeforeImportRequest": 1, + "OnModelBeforeDelete": 3, + "OnModelAfterDelete": 3, + "OnModelBeforeUpdate": 2, + "OnModelAfterUpdate": 2, + "OnModelBeforeCreate": 1, + "OnModelAfterCreate": 1, + }, + AfterFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { + collections := []*models.Collection{} + if err := app.Dao().CollectionQuery().All(&collections); err != nil { + t.Fatal(err) + } + if len(collections) != 3 { + t.Fatalf("Expected %d collections, got %d", 3, len(collections)) + } }, }, } diff --git a/apis/record_test.go b/apis/record_test.go index 9929d60d..cea685e0 100644 --- a/apis/record_test.go +++ b/apis/record_test.go @@ -750,6 +750,62 @@ func TestRecordCreate(t *testing.T) { "OnModelAfterCreate": 1, }, }, + { + Name: "invalid custom insertion id (less than 15 chars)", + Method: http.MethodPost, + Url: "/api/collections/demo3/records", + Body: strings.NewReader(`{ + "id": "12345678901234", + "title": "test" + }`), + RequestHeaders: map[string]string{ + "Authorization": "Admin eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjJiNGE5N2NjLTNmODMtNGQwMS1hMjZiLTNkNzdiYzg0MmQzYyIsInR5cGUiOiJhZG1pbiIsImV4cCI6MTg3MzQ2Mjc5Mn0.AtRtXR6FHBrCUGkj5OffhmxLbSZaQ4L_Qgw4gfoHyfo", + }, + ExpectedStatus: 400, + ExpectedContent: []string{ + `"id":{"code":"validation_length_invalid"`, + }, + }, + { + Name: "invalid custom insertion id (more than 15 chars)", + Method: http.MethodPost, + Url: "/api/collections/demo3/records", + Body: strings.NewReader(`{ + "id": "1234567890123456", + "title": "test" + }`), + RequestHeaders: map[string]string{ + "Authorization": "Admin eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjJiNGE5N2NjLTNmODMtNGQwMS1hMjZiLTNkNzdiYzg0MmQzYyIsInR5cGUiOiJhZG1pbiIsImV4cCI6MTg3MzQ2Mjc5Mn0.AtRtXR6FHBrCUGkj5OffhmxLbSZaQ4L_Qgw4gfoHyfo", + }, + ExpectedStatus: 400, + ExpectedContent: []string{ + `"id":{"code":"validation_length_invalid"`, + }, + }, + { + Name: "valid custom insertion id (exactly 15 chars)", + Method: http.MethodPost, + Url: "/api/collections/demo3/records", + Body: strings.NewReader(`{ + "id": "123456789012345", + "title": "test" + }`), + RequestHeaders: map[string]string{ + "Authorization": "Admin eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjJiNGE5N2NjLTNmODMtNGQwMS1hMjZiLTNkNzdiYzg0MmQzYyIsInR5cGUiOiJhZG1pbiIsImV4cCI6MTg3MzQ2Mjc5Mn0.AtRtXR6FHBrCUGkj5OffhmxLbSZaQ4L_Qgw4gfoHyfo", + }, + ExpectedStatus: 200, + ExpectedContent: []string{ + `"id":"123456789012345"`, + `"title":"test"`, + }, + ExpectedEvents: map[string]int{ + "OnRecordBeforeCreateRequest": 1, + "OnRecordAfterCreateRequest": 1, + "OnModelBeforeCreate": 1, + "OnModelAfterCreate": 1, + }, + }, + { Name: "submit via multipart form data", Method: http.MethodPost, diff --git a/cmd/migrate.go b/cmd/migrate.go index dd35d453..a2d5c748 100644 --- a/cmd/migrate.go +++ b/cmd/migrate.go @@ -1,12 +1,21 @@ package cmd import ( + "encoding/json" + "fmt" "log" + "os" + "path" + "time" + "github.com/AlecAivazis/survey/v2" "github.com/pocketbase/dbx" "github.com/pocketbase/pocketbase/core" + "github.com/pocketbase/pocketbase/daos" "github.com/pocketbase/pocketbase/migrations" "github.com/pocketbase/pocketbase/migrations/logs" + "github.com/pocketbase/pocketbase/models" + "github.com/pocketbase/pocketbase/tools/inflector" "github.com/pocketbase/pocketbase/tools/migrate" "github.com/spf13/cobra" ) @@ -15,18 +24,40 @@ import ( func NewMigrateCommand(app core.App) *cobra.Command { desc := ` Supported arguments are: -- up - runs all available migrations. -- down [number] - reverts the last [number] applied migrations. -- create folder name - creates new migration template file. +- up - runs all available migrations. +- down [number] - reverts the last [number] applied migrations. +- create name [folder] - creates new migration template file. +- collections [folder] - creates new migration file with the current collections configuration. ` var databaseFlag string command := &cobra.Command{ Use: "migrate", Short: "Executes DB migration scripts", - ValidArgs: []string{"up", "down", "create"}, + ValidArgs: []string{"up", "down", "create", "collections"}, Long: desc, Run: func(command *cobra.Command, args []string) { + cmd := "" + if len(args) > 0 { + cmd = args[0] + } + + // additional commands + // --- + if cmd == "create" { + if err := migrateCreateHandler(defaultMigrateCreateTemplate, args[1:]); err != nil { + log.Fatal(err) + } + return + } + if cmd == "collections" { + if err := migrateCollectionsHandler(app, args[1:]); err != nil { + log.Fatal(err) + } + return + } + // --- + // normalize if databaseFlag != "logs" { databaseFlag = "db" @@ -75,3 +106,134 @@ func migrationsConnectionsMap(app core.App) map[string]migrationsConnection { }, } } + +// ------------------------------------------------------------------- +// migrate create +// ------------------------------------------------------------------- + +const defaultMigrateCreateTemplate = `package migrations + +import ( + "github.com/pocketbase/dbx" + m "github.com/pocketbase/pocketbase/migrations" +) + +func init() { + m.Register(func(db dbx.Builder) error { + // add up queries... + + return nil + }, func(db dbx.Builder) error { + // add down queries... + + return nil + }) +} +` + +func migrateCreateHandler(template string, args []string) error { + if len(args) < 1 { + return fmt.Errorf("Missing migration file name") + } + + name := args[0] + + var dir string + if len(args) == 2 { + dir = args[1] + } + if dir == "" { + // If not specified, auto point to the default migrations folder. + // + // NB! + // Since the create command makes sense only during development, + // it is expected the user to be in the app working directory + // and to be using `go run` + wd, err := os.Getwd() + if err != nil { + return err + } + dir = path.Join(wd, "migrations") + } + + resultFilePath := path.Join( + dir, + fmt.Sprintf("%d_%s.go", time.Now().Unix(), inflector.Snakecase(name)), + ) + + confirm := false + prompt := &survey.Confirm{ + Message: fmt.Sprintf("Do you really want to create migration %q?", resultFilePath), + } + survey.AskOne(prompt, &confirm) + if !confirm { + fmt.Println("The command has been cancelled") + return nil + } + + // ensure that migrations dir exist + if err := os.MkdirAll(dir, os.ModePerm); err != nil { + return err + } + + if err := os.WriteFile(resultFilePath, []byte(template), 0644); err != nil { + return fmt.Errorf("Failed to save migration file %q\n", resultFilePath) + } + + fmt.Printf("Successfully created file %q\n", resultFilePath) + return nil +} + +// ------------------------------------------------------------------- +// migrate collections +// ------------------------------------------------------------------- + +const collectionsMigrateCreateTemplate = `package migrations + +import ( + "encoding/json" + + "github.com/pocketbase/dbx" + "github.com/pocketbase/pocketbase/daos" + m "github.com/pocketbase/pocketbase/migrations" + "github.com/pocketbase/pocketbase/models" +) + +// Auto generated migration with the most recent collections configuration. +func init() { + m.Register(func(db dbx.Builder) error { + jsonData := ` + "`" + `%s` + "`" + ` + + collections := []*models.Collection{} + if err := json.Unmarshal([]byte(jsonData), &collections); err != nil { + return err + } + + return daos.New(db).ImportCollections(collections, true, nil) + }, func(db dbx.Builder) error { + return nil + }) +} +` + +func migrateCollectionsHandler(app core.App, args []string) error { + createArgs := []string{"collections_import"} + createArgs = append(createArgs, args...) + + dao := daos.New(app.DB()) + + collections := []*models.Collection{} + if err := dao.CollectionQuery().OrderBy("created ASC").All(&collections); err != nil { + return fmt.Errorf("Failed to fetch migrations list: %v", err) + } + + serialized, err := json.MarshalIndent(collections, "\t\t", "\t") + if err != nil { + return fmt.Errorf("Failed to serialize collections list: %v", err) + } + + return migrateCreateHandler( + fmt.Sprintf(collectionsMigrateCreateTemplate, string(serialized)), + createArgs, + ) +} diff --git a/daos/collection.go b/daos/collection.go index 1337bf5e..d50625bd 100644 --- a/daos/collection.go +++ b/daos/collection.go @@ -113,7 +113,7 @@ func (dao *Dao) FindCollectionReferences(collection *models.Collection, excludeI // - is referenced as part of a relation field in another collection func (dao *Dao) DeleteCollection(collection *models.Collection) error { if collection.System { - return errors.New("System collections cannot be deleted.") + return fmt.Errorf("System collection %q cannot be deleted.", collection.Name) } // ensure that there aren't any existing references. @@ -123,7 +123,7 @@ func (dao *Dao) DeleteCollection(collection *models.Collection) error { return err } if total := len(result); total > 0 { - return fmt.Errorf("The collection has external relation field references (%d).", total) + return fmt.Errorf("The collection %q has external relation field references (%d).", collection.Name, total) } return dao.RunInTransaction(func(txDao *Dao) error { @@ -161,3 +161,100 @@ func (dao *Dao) SaveCollection(collection *models.Collection) error { return txDao.SyncRecordTableSchema(collection, oldCollection) }) } + +// ImportCollections imports the provided collections list in a single transaction. +// +// If deleteMissing is set, all existing collections that are not present in the +// imported configuration will be deleted (including their related records table). +// +// NB! This method doesn't perform validations on the imported collections data! +// If you need validations, use [forms.CollectionsImport]. +func (dao *Dao) ImportCollections( + importedCollections []*models.Collection, + deleteMissing bool, + beforeRecordsSync func(txDao *Dao, mappedImported, mappedExisting map[string]*models.Collection) error, +) error { + if len(importedCollections) == 0 { + return errors.New("No collections to import") + } + + return dao.RunInTransaction(func(txDao *Dao) error { + existingCollections := []*models.Collection{} + if err := txDao.CollectionQuery().OrderBy("created ASC").All(&existingCollections); err != nil { + return err + } + mappedExisting := make(map[string]*models.Collection, len(existingCollections)) + for _, existing := range existingCollections { + mappedExisting[existing.GetId()] = existing + } + + mappedImported := make(map[string]*models.Collection, len(importedCollections)) + for _, imported := range importedCollections { + // normalize + if !imported.HasId() { + // generate id if not set + imported.MarkAsNew() + imported.RefreshId() + } else if _, ok := mappedExisting[imported.GetId()]; !ok { + imported.MarkAsNew() + } + + mappedImported[imported.GetId()] = imported + } + + // delete old collections not available in the new configuration + // (before saving the imports in case a deleted collection name is being reused) + if deleteMissing { + for _, existing := range existingCollections { + if mappedImported[existing.GetId()] != nil { + continue // exist + } + + if existing.System { + return fmt.Errorf("System collection %q cannot be deleted.", existing.Name) + } + + // delete the collection + if err := txDao.Delete(existing); err != nil { + return err + } + } + } + + // upsert imported collections + for _, imported := range importedCollections { + if err := txDao.Save(imported); err != nil { + return err + } + } + + if beforeRecordsSync != nil { + if err := beforeRecordsSync(txDao, mappedImported, mappedExisting); err != nil { + return err + } + } + + // delete the record tables of the deleted collections + if deleteMissing { + for _, existing := range existingCollections { + if mappedImported[existing.GetId()] != nil { + continue // exist + } + + if err := txDao.DeleteTable(existing.Name); err != nil { + return err + } + } + } + + // sync the upserted collections with the related records table + for _, imported := range importedCollections { + existing := mappedExisting[imported.GetId()] + if err := txDao.SyncRecordTableSchema(imported, existing); err != nil { + return err + } + } + + return nil + }) +} diff --git a/daos/collection_test.go b/daos/collection_test.go index 853d9365..76053d14 100644 --- a/daos/collection_test.go +++ b/daos/collection_test.go @@ -1,8 +1,11 @@ package daos_test import ( + "encoding/json" + "errors" "testing" + "github.com/pocketbase/pocketbase/daos" "github.com/pocketbase/pocketbase/models" "github.com/pocketbase/pocketbase/models/schema" "github.com/pocketbase/pocketbase/tests" @@ -250,3 +253,204 @@ func TestSaveCollectionUpdate(t *testing.T) { } } } + +func TestImportCollections(t *testing.T) { + scenarios := []struct { + name string + jsonData string + deleteMissing bool + beforeRecordsSync func(txDao *daos.Dao, mappedImported, mappedExisting map[string]*models.Collection) error + expectError bool + expectCollectionsCount int + }{ + { + name: "empty collections", + jsonData: `[]`, + expectError: true, + expectCollectionsCount: 5, + }, + { + name: "check db constraints", + jsonData: `[ + {"name": "import_test", "schema": []} + ]`, + deleteMissing: false, + expectError: true, + expectCollectionsCount: 5, + }, + { + name: "minimal collection import", + jsonData: `[ + {"name": "import_test", "schema": [{"name":"test", "type": "text"}]} + ]`, + deleteMissing: false, + expectError: false, + expectCollectionsCount: 6, + }, + { + name: "minimal collection import + failed beforeRecordsSync", + jsonData: `[ + {"name": "import_test", "schema": [{"name":"test", "type": "text"}]} + ]`, + beforeRecordsSync: func(txDao *daos.Dao, mappedImported, mappedExisting map[string]*models.Collection) error { + return errors.New("test_error") + }, + deleteMissing: false, + expectError: true, + expectCollectionsCount: 5, + }, + { + name: "minimal collection import + successful beforeRecordsSync", + jsonData: `[ + {"name": "import_test", "schema": [{"name":"test", "type": "text"}]} + ]`, + beforeRecordsSync: func(txDao *daos.Dao, mappedImported, mappedExisting map[string]*models.Collection) error { + return nil + }, + deleteMissing: false, + expectError: false, + expectCollectionsCount: 6, + }, + { + name: "new + update + delete system collection", + jsonData: `[ + { + "id":"3f2888f8-075d-49fe-9d09-ea7e951000dc", + "name":"demo", + "schema":[ + { + "id":"_2hlxbmp", + "name":"title", + "type":"text", + "system":false, + "required":true, + "unique":false, + "options":{ + "min":3, + "max":null, + "pattern":"" + } + } + ] + }, + { + "name": "import1", + "schema": [ + { + "name":"active", + "type":"bool" + } + ] + } + ]`, + deleteMissing: true, + expectError: true, + expectCollectionsCount: 5, + }, + { + name: "new + update + delete non-system collection", + jsonData: `[ + { + "id":"abe78266-fd4d-4aea-962d-8c0138ac522b", + "name":"profiles", + "system":true, + "listRule":"userId = @request.user.id", + "viewRule":"created > 'test_change'", + "createRule":"userId = @request.user.id", + "updateRule":"userId = @request.user.id", + "deleteRule":"userId = @request.user.id", + "schema":[ + { + "id":"koih1lqx", + "name":"userId", + "type":"user", + "system":true, + "required":true, + "unique":true, + "options":{ + "maxSelect":1, + "cascadeDelete":true + } + }, + { + "id":"69ycbg3q", + "name":"rel", + "type":"relation", + "system":false, + "required":false, + "unique":false, + "options":{ + "maxSelect":2, + "collectionId":"abe78266-fd4d-4aea-962d-8c0138ac522b", + "cascadeDelete":false + } + } + ] + }, + { + "id":"3f2888f8-075d-49fe-9d09-ea7e951000dc", + "name":"demo", + "schema":[ + { + "id":"_2hlxbmp", + "name":"title", + "type":"text", + "system":false, + "required":true, + "unique":false, + "options":{ + "min":3, + "max":null, + "pattern":"" + } + } + ] + }, + { + "id": "test_deleted_collection_name_reuse", + "name": "demo2", + "schema": [ + { + "id":"fz6iql2m", + "name":"active", + "type":"bool" + } + ] + } + ]`, + deleteMissing: true, + expectError: false, + expectCollectionsCount: 3, + }, + } + + for _, scenario := range scenarios { + testApp, _ := tests.NewTestApp() + defer testApp.Cleanup() + + importedCollections := []*models.Collection{} + + // load data + loadErr := json.Unmarshal([]byte(scenario.jsonData), &importedCollections) + if loadErr != nil { + t.Fatalf("[%s] Failed to load data: %v", scenario.name, loadErr) + continue + } + + err := testApp.Dao().ImportCollections(importedCollections, scenario.deleteMissing, scenario.beforeRecordsSync) + + hasErr := err != nil + if hasErr != scenario.expectError { + t.Errorf("[%s] Expected hasErr to be %v, got %v (%v)", scenario.name, scenario.expectError, hasErr, err) + } + + // check collections count + collections := []*models.Collection{} + if err := testApp.Dao().CollectionQuery().All(&collections); err != nil { + t.Fatal(err) + } + if len(collections) != scenario.expectCollectionsCount { + t.Errorf("[%s] Expected %d collections, got %d", scenario.name, scenario.expectCollectionsCount, len(collections)) + } + } +} diff --git a/forms/admin_login_test.go b/forms/admin_login_test.go index 9e5d09c8..5124bdc7 100644 --- a/forms/admin_login_test.go +++ b/forms/admin_login_test.go @@ -7,6 +7,16 @@ import ( "github.com/pocketbase/pocketbase/tests" ) +func TestAdminLoginPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewAdminLogin(nil) +} + func TestAdminLoginValidate(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() diff --git a/forms/admin_password_reset_confirm_test.go b/forms/admin_password_reset_confirm_test.go index 5f11c2ce..de894f57 100644 --- a/forms/admin_password_reset_confirm_test.go +++ b/forms/admin_password_reset_confirm_test.go @@ -8,6 +8,16 @@ import ( "github.com/pocketbase/pocketbase/tools/security" ) +func TestAdminPasswordResetPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewAdminPasswordResetConfirm(nil) +} + func TestAdminPasswordResetConfirmValidate(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() diff --git a/forms/admin_password_reset_request_test.go b/forms/admin_password_reset_request_test.go index 245d48b0..123804a2 100644 --- a/forms/admin_password_reset_request_test.go +++ b/forms/admin_password_reset_request_test.go @@ -7,6 +7,16 @@ import ( "github.com/pocketbase/pocketbase/tests" ) +func TestAdminPasswordResetRequestPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewAdminPasswordResetRequest(nil) +} + func TestAdminPasswordResetRequestValidate(t *testing.T) { testApp, _ := tests.NewTestApp() defer testApp.Cleanup() diff --git a/forms/admin_upsert_test.go b/forms/admin_upsert_test.go index 4fa44aab..f9ce1f8e 100644 --- a/forms/admin_upsert_test.go +++ b/forms/admin_upsert_test.go @@ -3,6 +3,7 @@ package forms_test import ( "encoding/json" "errors" + "fmt" "testing" validation "github.com/go-ozzo/ozzo-validation/v4" @@ -11,6 +12,29 @@ import ( "github.com/pocketbase/pocketbase/tests" ) +func TestAdminUpsertPanic1(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewAdminUpsert(nil, nil) +} + +func TestAdminUpsertPanic2(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewAdminUpsert(app, nil) +} + func TestNewAdminUpsert(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() @@ -347,3 +371,99 @@ func TestAdminUpsertSubmitInterceptors(t *testing.T) { t.Fatalf("Expected the form model to be filled before calling the interceptors") } } + +func TestAdminUpsertWithCustomId(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + existingAdmin, err := app.Dao().FindAdminByEmail("test@example.com") + if err != nil { + t.Fatal(err) + } + + scenarios := []struct { + name string + jsonData string + collection *models.Admin + expectError bool + }{ + { + "empty data", + "{}", + &models.Admin{}, + false, + }, + { + "empty id", + `{"id":""}`, + &models.Admin{}, + false, + }, + { + "id < 15 chars", + `{"id":"a23"}`, + &models.Admin{}, + true, + }, + { + "id > 15 chars", + `{"id":"a234567890123456"}`, + &models.Admin{}, + true, + }, + { + "id = 15 chars", + `{"id":"a23456789012345"}`, + &models.Admin{}, + false, + }, + { + "changing the id of an existing item", + `{"id":"b23456789012345"}`, + existingAdmin, + true, + }, + { + "using the same existing item id", + `{"id":"` + existingAdmin.Id + `"}`, + existingAdmin, + false, + }, + { + "skipping the id for existing item", + `{}`, + existingAdmin, + false, + }, + } + + for i, scenario := range scenarios { + form := forms.NewAdminUpsert(app, scenario.collection) + if form.Email == "" { + form.Email = fmt.Sprintf("test_id_%d@example.com", i) + } + form.Password = "1234567890" + form.PasswordConfirm = form.Password + + // load data + loadErr := json.Unmarshal([]byte(scenario.jsonData), form) + if loadErr != nil { + t.Errorf("[%s] Failed to load form data: %v", scenario.name, loadErr) + continue + } + + submitErr := form.Submit() + hasErr := submitErr != nil + + if hasErr != scenario.expectError { + t.Errorf("[%s] Expected hasErr to be %v, got %v (%v)", scenario.name, scenario.expectError, hasErr, submitErr) + } + + if !hasErr && form.Id != "" { + _, err := app.Dao().FindAdminById(form.Id) + if err != nil { + t.Errorf("[%s] Expected to find record with id %s, got %v", scenario.name, form.Id, err) + } + } + } +} diff --git a/forms/collection_upsert_test.go b/forms/collection_upsert_test.go index 69a261b2..3c30935e 100644 --- a/forms/collection_upsert_test.go +++ b/forms/collection_upsert_test.go @@ -10,9 +10,33 @@ import ( "github.com/pocketbase/pocketbase/models" "github.com/pocketbase/pocketbase/models/schema" "github.com/pocketbase/pocketbase/tests" + "github.com/pocketbase/pocketbase/tools/security" "github.com/spf13/cast" ) +func TestCollectionUpsertPanic1(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewCollectionUpsert(nil, nil) +} + +func TestCollectionUpsertPanic2(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewCollectionUpsert(app, nil) +} + func TestNewCollectionUpsert(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() @@ -518,3 +542,101 @@ func TestCollectionUpsertSubmitInterceptors(t *testing.T) { t.Fatalf("Expected the form model to be filled before calling the interceptors") } } + +func TestCollectionUpsertWithCustomId(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + existingCollection, err := app.Dao().FindCollectionByNameOrId("demo3") + if err != nil { + t.Fatal(err) + } + + newCollection := func() *models.Collection { + return &models.Collection{ + Name: "c_" + security.RandomString(4), + Schema: existingCollection.Schema, + } + } + + scenarios := []struct { + name string + jsonData string + collection *models.Collection + expectError bool + }{ + { + "empty data", + "{}", + newCollection(), + false, + }, + { + "empty id", + `{"id":""}`, + newCollection(), + false, + }, + { + "id < 15 chars", + `{"id":"a23"}`, + newCollection(), + true, + }, + { + "id > 15 chars", + `{"id":"a234567890123456"}`, + newCollection(), + true, + }, + { + "id = 15 chars", + `{"id":"a23456789012345"}`, + newCollection(), + false, + }, + { + "changing the id of an existing item", + `{"id":"b23456789012345"}`, + existingCollection, + true, + }, + { + "using the same existing item id", + `{"id":"` + existingCollection.Id + `"}`, + existingCollection, + false, + }, + { + "skipping the id for existing item", + `{}`, + existingCollection, + false, + }, + } + + for _, scenario := range scenarios { + form := forms.NewCollectionUpsert(app, scenario.collection) + + // load data + loadErr := json.Unmarshal([]byte(scenario.jsonData), form) + if loadErr != nil { + t.Errorf("[%s] Failed to load form data: %v", scenario.name, loadErr) + continue + } + + submitErr := form.Submit() + hasErr := submitErr != nil + + if hasErr != scenario.expectError { + t.Errorf("[%s] Expected hasErr to be %v, got %v (%v)", scenario.name, scenario.expectError, hasErr, submitErr) + } + + if !hasErr && form.Id != "" { + _, err := app.Dao().FindCollectionByNameOrId(form.Id) + if err != nil { + t.Errorf("[%s] Expected to find record with id %s, got %v", scenario.name, form.Id, err) + } + } + } +} diff --git a/forms/collections_import.go b/forms/collections_import.go index 195ee93e..3c31b8c5 100644 --- a/forms/collections_import.go +++ b/forms/collections_import.go @@ -16,8 +16,8 @@ import ( type CollectionsImport struct { config CollectionsImportConfig - Collections []*models.Collection `form:"collections" json:"collections"` - DeleteOthers bool `form:"deleteOthers" json:"deleteOthers"` + Collections []*models.Collection `form:"collections" json:"collections"` + DeleteMissing bool `form:"deleteMissing" json:"deleteMissing"` } // CollectionsImportConfig is the [CollectionsImport] factory initializer config. @@ -66,7 +66,7 @@ func (form *CollectionsImport) Validate() error { // - imports the form collections (create or replace) // - sync the collection changes with their related records table // - ensures the integrity of the imported structure (aka. run validations for each collection) -// - if [form.DeleteOthers] is set, deletes all local collections that are not found in the imports list +// - if [form.DeleteMissing] is set, deletes all local collections that are not found in the imports list // // All operations are wrapped in a single transaction that are // rollbacked on the first encountered error. @@ -78,116 +78,75 @@ func (form *CollectionsImport) Submit(interceptors ...InterceptorFunc) error { return err } - return runInterceptors(form.submit, interceptors...) + return runInterceptors(func() error { + return form.config.TxDao.RunInTransaction(func(txDao *daos.Dao) error { + importErr := txDao.ImportCollections( + form.Collections, + form.DeleteMissing, + form.beforeRecordsSync, + ) + if importErr == nil { + return nil + } + + // validation failure + if err, ok := importErr.(validation.Errors); ok { + return err + } + + // generic/db failure + if form.config.App.IsDebug() { + log.Println("Internal import failure:", importErr) + } + return validation.Errors{"collections": validation.NewError( + "collections_import_failure", + "Failed to import the collections configuration.", + )} + }) + }, interceptors...) } -func (form *CollectionsImport) submit() error { - return form.config.TxDao.RunInTransaction(func(txDao *daos.Dao) error { - oldCollections := []*models.Collection{} - if err := txDao.CollectionQuery().All(&oldCollections); err != nil { - return err +func (form *CollectionsImport) beforeRecordsSync(txDao *daos.Dao, mappedNew, mappedOld map[string]*models.Collection) error { + // refresh the actual persisted collections list + refreshedCollections := []*models.Collection{} + if err := txDao.CollectionQuery().OrderBy("created ASC").All(&refreshedCollections); err != nil { + return err + } + + // trigger the validator for each existing collection to + // ensure that the app is not left in a broken state + for _, collection := range refreshedCollections { + upsertModel := mappedOld[collection.GetId()] + if upsertModel == nil { + upsertModel = collection } - mappedOldCollections := make(map[string]*models.Collection, len(oldCollections)) - for _, old := range oldCollections { - mappedOldCollections[old.GetId()] = old + upsertForm := NewCollectionUpsertWithConfig(CollectionUpsertConfig{ + App: form.config.App, + TxDao: txDao, + }, upsertModel) + + // load form fields with the refreshed collection state + upsertForm.Id = collection.Id + upsertForm.Name = collection.Name + upsertForm.System = collection.System + upsertForm.ListRule = collection.ListRule + upsertForm.ViewRule = collection.ViewRule + upsertForm.CreateRule = collection.CreateRule + upsertForm.UpdateRule = collection.UpdateRule + upsertForm.DeleteRule = collection.DeleteRule + upsertForm.Schema = collection.Schema + + if err := upsertForm.Validate(); err != nil { + // serialize the validation error(s) + serializedErr, _ := json.Marshal(err) + + return validation.Errors{"collections": validation.NewError( + "collections_import_validate_failure", + fmt.Sprintf("Data validations failed for collection %q (%s): %s", collection.Name, collection.Id, serializedErr), + )} } + } - mappedFormCollections := make(map[string]*models.Collection, len(form.Collections)) - for _, collection := range form.Collections { - mappedFormCollections[collection.GetId()] = collection - } - - // delete all other collections not sent with the import - if form.DeleteOthers { - for _, old := range oldCollections { - if mappedFormCollections[old.GetId()] == nil { - // delete the collection - if err := txDao.DeleteCollection(old); err != nil { - if form.config.App.IsDebug() { - log.Println("[CollectionsImport] DeleteOthers failure", old.Name, err) - } - return validation.Errors{"collections": validation.NewError( - "collections_import_collection_delete_failure", - fmt.Sprintf("Failed to delete collection %q (%s). Make sure that the collection is not system or referenced by other collections.", old.Name, old.Id), - )} - } - - delete(mappedOldCollections, old.GetId()) - } - } - } - - // raw insert/replace (aka. without any validations) - // (required to make sure that all linked collections exists before running the validations) - for _, collection := range form.Collections { - if mappedOldCollections[collection.GetId()] == nil { - collection.MarkAsNew() - } - - if err := txDao.Save(collection); err != nil { - if form.config.App.IsDebug() { - log.Println("[CollectionsImport] Save failure", collection.Name, err) - } - return validation.Errors{"collections": validation.NewError( - "collections_import_save_failure", - fmt.Sprintf("Integrity constraints failed - the collection %q (%s) cannot be imported.", collection.Name, collection.Id), - )} - } - } - - // refresh the actual persisted collections list - refreshedCollections := []*models.Collection{} - if err := txDao.CollectionQuery().All(&refreshedCollections); err != nil { - return err - } - - // trigger the validator for each existing collection to - // ensure that the app is not left in a broken state - for _, collection := range refreshedCollections { - upsertModel := mappedOldCollections[collection.GetId()] - if upsertModel == nil { - upsertModel = collection - } - upsertForm := NewCollectionUpsertWithConfig(CollectionUpsertConfig{ - App: form.config.App, - TxDao: txDao, - }, upsertModel) - // load form fields with the refreshed collection state - upsertForm.Id = collection.Id - upsertForm.Name = collection.Name - upsertForm.System = collection.System - upsertForm.ListRule = collection.ListRule - upsertForm.ViewRule = collection.ViewRule - upsertForm.CreateRule = collection.CreateRule - upsertForm.UpdateRule = collection.UpdateRule - upsertForm.DeleteRule = collection.DeleteRule - upsertForm.Schema = collection.Schema - if err := upsertForm.Validate(); err != nil { - // serialize the validation error(s) - serializedErr, _ := json.Marshal(err) - - return validation.Errors{"collections": validation.NewError( - "collections_import_validate_failure", - fmt.Sprintf("Data validations failed for collection %q (%s): %s", collection.Name, collection.Id, serializedErr), - )} - } - } - - // sync the records table for each updated collection - for _, collection := range form.Collections { - oldCollection := mappedOldCollections[collection.GetId()] - if err := txDao.SyncRecordTableSchema(collection, oldCollection); err != nil { - if form.config.App.IsDebug() { - log.Println("[CollectionsImport] Records table sync failure", collection.Name, err) - } - return validation.Errors{"collections": validation.NewError( - "collections_import_records_table_sync_failure", - fmt.Sprintf("Failed to sync the records table changes for collection %q.", collection.Name), - )} - } - } - - return nil - }) + return nil } diff --git a/forms/collections_import_test.go b/forms/collections_import_test.go new file mode 100644 index 00000000..724e93d4 --- /dev/null +++ b/forms/collections_import_test.go @@ -0,0 +1,420 @@ +package forms_test + +import ( + "encoding/json" + "errors" + "testing" + + "github.com/pocketbase/pocketbase/forms" + "github.com/pocketbase/pocketbase/models" + "github.com/pocketbase/pocketbase/tests" +) + +func TestCollectionsImportPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewCollectionsImport(nil) +} + +func TestCollectionsImportValidate(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + form := forms.NewCollectionsImport(app) + + scenarios := []struct { + collections []*models.Collection + expectError bool + }{ + {nil, true}, + {[]*models.Collection{}, true}, + {[]*models.Collection{{}}, false}, + } + + for i, s := range scenarios { + form.Collections = s.collections + + err := form.Validate() + + hasErr := err != nil + if hasErr != s.expectError { + t.Errorf("(%d) Expected hasErr to be %v, got %v (%v)", i, s.expectError, hasErr, err) + } + } +} + +func TestCollectionsImportSubmit(t *testing.T) { + scenarios := []struct { + name string + jsonData string + expectError bool + expectCollectionsCount int + expectEvents map[string]int + }{ + { + name: "empty collections", + jsonData: `{ + "deleteMissing": true, + "collections": [] + }`, + expectError: true, + expectCollectionsCount: 5, + expectEvents: nil, + }, + { + name: "one of the collections has invalid data", + jsonData: `{ + "collections": [ + { + "name": "import1", + "schema": [ + { + "id":"fz6iql2m", + "name":"active", + "type":"bool" + } + ] + }, + { + "name": "import 2", + "schema": [ + { + "id":"fz6iql2m", + "name":"active", + "type":"bool" + } + ] + } + ] + }`, + expectError: true, + expectCollectionsCount: 5, + expectEvents: map[string]int{ + "OnModelBeforeCreate": 2, + "OnModelAfterCreate": 2, + }, + }, + { + name: "all imported collections has valid data", + jsonData: `{ + "collections": [ + { + "name": "import1", + "schema": [ + { + "id":"fz6iql2m", + "name":"active", + "type":"bool" + } + ] + }, + { + "name": "import2", + "schema": [ + { + "id":"fz6iql2m", + "name":"active", + "type":"bool" + } + ] + } + ] + }`, + expectError: false, + expectCollectionsCount: 7, + expectEvents: map[string]int{ + "OnModelBeforeCreate": 2, + "OnModelAfterCreate": 2, + }, + }, + { + name: "new collection with existing name", + jsonData: `{ + "collections": [ + { + "name": "demo2", + "schema": [ + { + "id":"fz6iql2m", + "name":"active", + "type":"bool" + } + ] + } + ] + }`, + expectError: true, + expectCollectionsCount: 5, + expectEvents: map[string]int{ + "OnModelBeforeCreate": 1, + }, + }, + { + name: "delete system + modified + new collection", + jsonData: `{ + "deleteMissing": true, + "collections": [ + { + "id":"3f2888f8-075d-49fe-9d09-ea7e951000dc", + "name":"demo", + "schema":[ + { + "id":"_2hlxbmp", + "name":"title", + "type":"text", + "system":false, + "required":true, + "unique":false, + "options":{ + "min":3, + "max":null, + "pattern":"" + } + } + ] + }, + { + "name": "import1", + "schema": [ + { + "id":"fz6iql2m", + "name":"active", + "type":"bool" + } + ] + } + ] + }`, + expectError: true, + expectCollectionsCount: 5, + }, + { + name: "modified + new collection", + jsonData: `{ + "collections": [ + { + "id":"3f2888f8-075d-49fe-9d09-ea7e951000dc", + "name":"demo", + "schema":[ + { + "id":"_2hlxbmp", + "name":"title", + "type":"text", + "system":false, + "required":true, + "unique":false, + "options":{ + "min":3, + "max":null, + "pattern":"" + } + } + ] + }, + { + "name": "import1", + "schema": [ + { + "id":"fz6iql2m", + "name":"active", + "type":"bool" + } + ] + }, + { + "name": "import2", + "schema": [ + { + "id":"fz6iql2m", + "name":"active", + "type":"bool" + } + ] + } + ] + }`, + expectError: false, + expectCollectionsCount: 7, + expectEvents: map[string]int{ + "OnModelBeforeUpdate": 1, + "OnModelAfterUpdate": 1, + "OnModelBeforeCreate": 2, + "OnModelAfterCreate": 2, + }, + }, + { + name: "delete non-system + modified + new collection", + jsonData: `{ + "deleteMissing": true, + "collections": [ + { + "id":"abe78266-fd4d-4aea-962d-8c0138ac522b", + "name":"profiles", + "system":true, + "listRule":"userId = @request.user.id", + "viewRule":"created > 'test_change'", + "createRule":"userId = @request.user.id", + "updateRule":"userId = @request.user.id", + "deleteRule":"userId = @request.user.id", + "schema":[ + { + "id":"koih1lqx", + "name":"userId", + "type":"user", + "system":true, + "required":true, + "unique":true, + "options":{ + "maxSelect":1, + "cascadeDelete":true + } + }, + { + "id":"69ycbg3q", + "name":"rel", + "type":"relation", + "system":false, + "required":false, + "unique":false, + "options":{ + "maxSelect":2, + "collectionId":"abe78266-fd4d-4aea-962d-8c0138ac522b", + "cascadeDelete":false + } + } + ] + }, + { + "id":"3f2888f8-075d-49fe-9d09-ea7e951000dc", + "name":"demo", + "schema":[ + { + "id":"_2hlxbmp", + "name":"title", + "type":"text", + "system":false, + "required":true, + "unique":false, + "options":{ + "min":3, + "max":null, + "pattern":"" + } + } + ] + }, + { + "id": "test_deleted_collection_name_reuse", + "name": "demo2", + "schema": [ + { + "id":"fz6iql2m", + "name":"active", + "type":"bool" + } + ] + } + ] + }`, + expectError: false, + expectCollectionsCount: 3, + expectEvents: map[string]int{ + "OnModelBeforeUpdate": 2, + "OnModelAfterUpdate": 2, + "OnModelBeforeCreate": 1, + "OnModelAfterCreate": 1, + "OnModelBeforeDelete": 3, + "OnModelAfterDelete": 3, + }, + }, + } + + for _, s := range scenarios { + testApp, _ := tests.NewTestApp() + defer testApp.Cleanup() + + form := forms.NewCollectionsImport(testApp) + + // load data + loadErr := json.Unmarshal([]byte(s.jsonData), form) + if loadErr != nil { + t.Errorf("[%s] Failed to load form data: %v", s.name, loadErr) + continue + } + + err := form.Submit() + + hasErr := err != nil + if hasErr != s.expectError { + t.Errorf("[%s] Expected hasErr to be %v, got %v (%v)", s.name, s.expectError, hasErr, err) + } + + // check collections count + collections := []*models.Collection{} + if err := testApp.Dao().CollectionQuery().All(&collections); err != nil { + t.Fatal(err) + } + if len(collections) != s.expectCollectionsCount { + t.Errorf("[%s] Expected %d collections, got %d", s.name, s.expectCollectionsCount, len(collections)) + } + + // check events + if len(testApp.EventCalls) > len(s.expectEvents) { + t.Errorf("[%s] Expected events %v, got %v", s.name, s.expectEvents, testApp.EventCalls) + } + for event, expectedCalls := range s.expectEvents { + actualCalls := testApp.EventCalls[event] + if actualCalls != expectedCalls { + t.Errorf("[%s] Expected event %s to be called %d, got %d", s.name, event, expectedCalls, actualCalls) + } + } + } +} + +func TestCollectionsImportSubmitInterceptors(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + collections := []*models.Collection{} + if err := app.Dao().CollectionQuery().All(&collections); err != nil { + t.Fatal(err) + } + + form := forms.NewCollectionsImport(app) + form.Collections = collections + + testErr := errors.New("test_error") + + interceptor1Called := false + interceptor1 := func(next forms.InterceptorNextFunc) forms.InterceptorNextFunc { + return func() error { + interceptor1Called = true + return next() + } + } + + interceptor2Called := false + interceptor2 := func(next forms.InterceptorNextFunc) forms.InterceptorNextFunc { + return func() error { + interceptor2Called = true + return testErr + } + } + + submitErr := form.Submit(interceptor1, interceptor2) + if submitErr != testErr { + t.Fatalf("Expected submitError %v, got %v", testErr, submitErr) + } + + if !interceptor1Called { + t.Fatalf("Expected interceptor1 to be called") + } + + if !interceptor2Called { + t.Fatalf("Expected interceptor2 to be called") + } +} diff --git a/forms/record_upsert.go b/forms/record_upsert.go index b864a7fc..52120c9c 100644 --- a/forms/record_upsert.go +++ b/forms/record_upsert.go @@ -43,7 +43,7 @@ type RecordUpsertConfig struct { // NewRecordUpsert creates a new [RecordUpsert] form with initializer // config created from the provided [core.App] and [models.Record] instances -// (for create you could pass a pointer to an empty Record - `&models.Record{}`). +// (for create you could pass a pointer to an empty Record - `models.NewRecord(collection)`). // // If you want to submit the form as part of another transaction, use // [NewRecordUpsertWithConfig] with explicitly set TxDao. @@ -55,7 +55,7 @@ func NewRecordUpsert(app core.App, record *models.Record) *RecordUpsert { // NewRecordUpsertWithConfig creates a new [RecordUpsert] form // with the provided config and [models.Record] instance or panics on invalid configuration -// (for create you could pass a pointer to an empty Record - `&models.Record{}`). +// (for create you could pass a pointer to an empty Record - `models.NewRecord(collection)`). func NewRecordUpsertWithConfig(config RecordUpsertConfig, record *models.Record) *RecordUpsert { form := &RecordUpsert{ config: config, @@ -302,8 +302,10 @@ func (form *RecordUpsert) DrySubmit(callback func(txDao *daos.Dao) error) error return err } + isNew := form.record.IsNew() + // custom insertion id can be set only on create - if form.record.IsNew() && form.Id != "" { + if isNew && form.Id != "" { form.record.MarkAsNew() form.record.SetId(form.Id) } @@ -319,6 +321,7 @@ func (form *RecordUpsert) DrySubmit(callback func(txDao *daos.Dao) error) error return errors.New("failed to get transaction db") } defer tx.Rollback() + txDao.BeforeCreateFunc = nil txDao.AfterCreateFunc = nil txDao.BeforeUpdateFunc = nil @@ -328,7 +331,16 @@ func (form *RecordUpsert) DrySubmit(callback func(txDao *daos.Dao) error) error return err } - return callback(txDao) + // restore record isNew state + if isNew { + form.record.MarkAsNew() + } + + if callback != nil { + return callback(txDao) + } + + return nil }) } diff --git a/forms/record_upsert_test.go b/forms/record_upsert_test.go index 9e8de527..a315e200 100644 --- a/forms/record_upsert_test.go +++ b/forms/record_upsert_test.go @@ -20,6 +20,29 @@ import ( "github.com/pocketbase/pocketbase/tools/list" ) +func TestRecordUpsertPanic1(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewRecordUpsert(nil, nil) +} + +func TestRecordUpsertPanic2(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewRecordUpsert(app, nil) +} + func TestNewRecordUpsert(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() @@ -32,7 +55,7 @@ func TestNewRecordUpsert(t *testing.T) { val := form.Data["title"] if val != "test_value" { - t.Errorf("Expected record data to be load, got %v", form.Data) + t.Errorf("Expected record data to be loaded, got %v", form.Data) } } @@ -68,6 +91,7 @@ func TestRecordUpsertLoadDataJson(t *testing.T) { } testData := map[string]any{ + "id": "test_id", "title": "test123", "unknown": "test456", // file fields unset/delete @@ -86,6 +110,10 @@ func TestRecordUpsertLoadDataJson(t *testing.T) { t.Fatal(loadErr) } + if form.Id != "test_id" { + t.Fatalf("Expect id field to be %q, got %q", "test_id", form.Id) + } + if v, ok := form.Data["title"]; !ok || v != "test123" { t.Fatalf("Expect title field to be %q, got %q", "test123", v) } @@ -134,6 +162,7 @@ func TestRecordUpsertLoadDataMultipart(t *testing.T) { } formData, mp, err := tests.MockMultipartData(map[string]string{ + "id": "test_id", "title": "test123", "unknown": "test456", // file fields unset/delete @@ -154,6 +183,10 @@ func TestRecordUpsertLoadDataMultipart(t *testing.T) { t.Fatal(loadErr) } + if form.Id != "test_id" { + t.Fatalf("Expect id field to be %q, got %q", "test_id", form.Id) + } + if v, ok := form.Data["title"]; !ok || v != "test123" { t.Fatalf("Expect title field to be %q, got %q", "test123", v) } @@ -202,6 +235,7 @@ func TestRecordUpsertValidateFailure(t *testing.T) { // try with invalid test data to check whether the RecordDataValidator is triggered formData, mp, err := tests.MockMultipartData(map[string]string{ + "id": "", "unknown": "test456", // should be ignored "title": "a", "onerel": "00000000-84ab-4057-a592-4604a731f78f", @@ -247,6 +281,7 @@ func TestRecordUpsertValidateSuccess(t *testing.T) { } formData, mp, err := tests.MockMultipartData(map[string]string{ + "id": record.Id, "unknown": "test456", // should be ignored "title": "abc", "onerel": "054f9f24-0a0a-4e09-87b1-bc7ff2b336a2", @@ -576,3 +611,103 @@ func hasRecordFile(app core.App, record *models.Record, filename string) bool { return exists } + +func TestRecordUpsertWithCustomId(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + collection, _ := app.Dao().FindCollectionByNameOrId("demo3") + existingRecord, err := app.Dao().FindFirstRecordByData(collection, "id", "2c542824-9de1-42fe-8924-e57c86267760") + if err != nil { + t.Fatal(err) + } + + scenarios := []struct { + name string + data map[string]string + record *models.Record + expectError bool + }{ + { + "empty data", + map[string]string{}, + models.NewRecord(collection), + false, + }, + { + "empty id", + map[string]string{"id": ""}, + models.NewRecord(collection), + false, + }, + { + "id < 15 chars", + map[string]string{"id": "a23"}, + models.NewRecord(collection), + true, + }, + { + "id > 15 chars", + map[string]string{"id": "a234567890123456"}, + models.NewRecord(collection), + true, + }, + { + "id = 15 chars", + map[string]string{"id": "a23456789012345"}, + models.NewRecord(collection), + false, + }, + { + "changing the id of an existing record", + map[string]string{"id": "b23456789012345"}, + existingRecord, + true, + }, + { + "using the same existing record id", + map[string]string{"id": existingRecord.Id}, + existingRecord, + false, + }, + { + "skipping the id for existing record", + map[string]string{}, + existingRecord, + false, + }, + } + + for _, scenario := range scenarios { + formData, mp, err := tests.MockMultipartData(scenario.data) + if err != nil { + t.Fatal(err) + } + + form := forms.NewRecordUpsert(app, scenario.record) + req := httptest.NewRequest(http.MethodGet, "/", formData) + req.Header.Set(echo.HeaderContentType, mp.FormDataContentType()) + form.LoadData(req) + + dryErr := form.DrySubmit(nil) + hasDryErr := dryErr != nil + + submitErr := form.Submit() + hasSubmitErr := submitErr != nil + + if hasDryErr != hasSubmitErr { + t.Errorf("[%s] Expected hasDryErr and hasSubmitErr to have the same value, got %v vs %v", scenario.name, hasDryErr, hasSubmitErr) + } + + if hasSubmitErr != scenario.expectError { + t.Errorf("[%s] Expected hasSubmitErr to be %v, got %v (%v)", scenario.name, scenario.expectError, hasSubmitErr, submitErr) + } + + if id, ok := scenario.data["id"]; ok && id != "" && !hasSubmitErr { + _, err := app.Dao().FindRecordById(collection, id, nil) + if err != nil { + t.Errorf("[%s] Expected to find record with id %s, got %v", scenario.name, id, err) + } + } + } +} diff --git a/forms/settings_upsert_test.go b/forms/settings_upsert_test.go index d0926052..1d3806a4 100644 --- a/forms/settings_upsert_test.go +++ b/forms/settings_upsert_test.go @@ -12,6 +12,16 @@ import ( "github.com/pocketbase/pocketbase/tools/security" ) +func TestSettingsUpsertPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewSettingsUpsert(nil) +} + func TestNewSettingsUpsert(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() diff --git a/forms/user_email_change_confirm_test.go b/forms/user_email_change_confirm_test.go index cc5d7502..d68f9ca7 100644 --- a/forms/user_email_change_confirm_test.go +++ b/forms/user_email_change_confirm_test.go @@ -10,6 +10,16 @@ import ( "github.com/pocketbase/pocketbase/tools/security" ) +func TestUserEmailChangeConfirmPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewUserEmailChangeConfirm(nil) +} + func TestUserEmailChangeConfirmValidateAndSubmit(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() diff --git a/forms/user_email_change_request.go b/forms/user_email_change_request.go index a1a2d32f..1b050aa2 100644 --- a/forms/user_email_change_request.go +++ b/forms/user_email_change_request.go @@ -44,8 +44,8 @@ func NewUserEmailChangeRequestWithConfig(config UserEmailChangeRequestConfig, us user: user, } - if form.config.App == nil { - panic("Missing required config.App instance.") + if form.config.App == nil || form.user == nil { + panic("Invalid initializer config or nil user model.") } if form.config.TxDao == nil { diff --git a/forms/user_email_change_request_test.go b/forms/user_email_change_request_test.go index e81df9e0..ed209be7 100644 --- a/forms/user_email_change_request_test.go +++ b/forms/user_email_change_request_test.go @@ -9,6 +9,29 @@ import ( "github.com/pocketbase/pocketbase/tests" ) +func TestUserEmailChangeRequestPanic1(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewUserEmailChangeRequest(nil, nil) +} + +func TestUserEmailChangeRequestPanic2(t *testing.T) { + testApp, _ := tests.NewTestApp() + defer testApp.Cleanup() + + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewUserEmailChangeRequest(testApp, nil) +} + func TestUserEmailChangeRequestValidateAndSubmit(t *testing.T) { testApp, _ := tests.NewTestApp() defer testApp.Cleanup() diff --git a/forms/user_email_login_test.go b/forms/user_email_login_test.go index 2d4b867d..1049f12a 100644 --- a/forms/user_email_login_test.go +++ b/forms/user_email_login_test.go @@ -9,6 +9,16 @@ import ( "github.com/pocketbase/pocketbase/tests" ) +func TestUserEmailLoginPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewUserEmailLogin(nil) +} + func TestUserEmailLoginValidate(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() diff --git a/forms/user_oauth2_login_test.go b/forms/user_oauth2_login_test.go index 13f3b0d0..e0a9674c 100644 --- a/forms/user_oauth2_login_test.go +++ b/forms/user_oauth2_login_test.go @@ -9,6 +9,16 @@ import ( "github.com/pocketbase/pocketbase/tests" ) +func TestUserOauth2LoginPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewUserOauth2Login(nil) +} + func TestUserOauth2LoginValidate(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() diff --git a/forms/user_password_reset_confirm_test.go b/forms/user_password_reset_confirm_test.go index 4cb8565f..f9437a3a 100644 --- a/forms/user_password_reset_confirm_test.go +++ b/forms/user_password_reset_confirm_test.go @@ -10,6 +10,16 @@ import ( "github.com/pocketbase/pocketbase/tools/security" ) +func TestUserPasswordResetConfirmPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewUserPasswordResetConfirm(nil) +} + func TestUserPasswordResetConfirmValidate(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() diff --git a/forms/user_password_reset_request_test.go b/forms/user_password_reset_request_test.go index cc4224bd..19e8d704 100644 --- a/forms/user_password_reset_request_test.go +++ b/forms/user_password_reset_request_test.go @@ -11,6 +11,16 @@ import ( "github.com/pocketbase/pocketbase/tools/types" ) +func TestUserPasswordResetRequestPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewUserPasswordResetRequest(nil) +} + func TestUserPasswordResetRequestValidate(t *testing.T) { testApp, _ := tests.NewTestApp() defer testApp.Cleanup() diff --git a/forms/user_upsert_test.go b/forms/user_upsert_test.go index 4451415c..64f81249 100644 --- a/forms/user_upsert_test.go +++ b/forms/user_upsert_test.go @@ -3,6 +3,7 @@ package forms_test import ( "encoding/json" "errors" + "fmt" "testing" validation "github.com/go-ozzo/ozzo-validation/v4" @@ -11,6 +12,29 @@ import ( "github.com/pocketbase/pocketbase/tests" ) +func TestUserUpsertPanic1(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewUserUpsert(nil, nil) +} + +func TestUserUpsertPanic2(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewUserUpsert(app, nil) +} + func TestNewUserUpsert(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() @@ -304,3 +328,99 @@ func TestUserUpsertSubmitInterceptors(t *testing.T) { t.Fatalf("Expected the form model to be filled before calling the interceptors") } } + +func TestUserUpsertWithCustomId(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + existingUser, err := app.Dao().FindUserByEmail("test@example.com") + if err != nil { + t.Fatal(err) + } + + scenarios := []struct { + name string + jsonData string + collection *models.User + expectError bool + }{ + { + "empty data", + "{}", + &models.User{}, + false, + }, + { + "empty id", + `{"id":""}`, + &models.User{}, + false, + }, + { + "id < 15 chars", + `{"id":"a23"}`, + &models.User{}, + true, + }, + { + "id > 15 chars", + `{"id":"a234567890123456"}`, + &models.User{}, + true, + }, + { + "id = 15 chars", + `{"id":"a23456789012345"}`, + &models.User{}, + false, + }, + { + "changing the id of an existing item", + `{"id":"b23456789012345"}`, + existingUser, + true, + }, + { + "using the same existing item id", + `{"id":"` + existingUser.Id + `"}`, + existingUser, + false, + }, + { + "skipping the id for existing item", + `{}`, + existingUser, + false, + }, + } + + for i, scenario := range scenarios { + form := forms.NewUserUpsert(app, scenario.collection) + if form.Email == "" { + form.Email = fmt.Sprintf("test_id_%d@example.com", i) + } + form.Password = "1234567890" + form.PasswordConfirm = form.Password + + // load data + loadErr := json.Unmarshal([]byte(scenario.jsonData), form) + if loadErr != nil { + t.Errorf("[%s] Failed to load form data: %v", scenario.name, loadErr) + continue + } + + submitErr := form.Submit() + hasErr := submitErr != nil + + if hasErr != scenario.expectError { + t.Errorf("[%s] Expected hasErr to be %v, got %v (%v)", scenario.name, scenario.expectError, hasErr, submitErr) + } + + if !hasErr && form.Id != "" { + _, err := app.Dao().FindUserById(form.Id) + if err != nil { + t.Errorf("[%s] Expected to find record with id %s, got %v", scenario.name, form.Id, err) + } + } + } +} diff --git a/forms/user_verification_confirm_test.go b/forms/user_verification_confirm_test.go index 14ff5a38..cff5c68e 100644 --- a/forms/user_verification_confirm_test.go +++ b/forms/user_verification_confirm_test.go @@ -10,6 +10,16 @@ import ( "github.com/pocketbase/pocketbase/tools/security" ) +func TestUserVerificationConfirmPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewUserVerificationConfirm(nil) +} + func TestUserVerificationConfirmValidate(t *testing.T) { app, _ := tests.NewTestApp() defer app.Cleanup() diff --git a/forms/user_verification_request_test.go b/forms/user_verification_request_test.go index 9ece9188..598d6d10 100644 --- a/forms/user_verification_request_test.go +++ b/forms/user_verification_request_test.go @@ -11,6 +11,16 @@ import ( "github.com/pocketbase/pocketbase/tools/types" ) +func TestUserVerificationRequestPanic(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("The form did not panic") + } + }() + + forms.NewUserVerificationRequest(nil) +} + func TestUserVerificationRequestValidate(t *testing.T) { testApp, _ := tests.NewTestApp() defer testApp.Cleanup() diff --git a/tools/migrate/runner.go b/tools/migrate/runner.go index b0a9aa6b..f9c6d87e 100644 --- a/tools/migrate/runner.go +++ b/tools/migrate/runner.go @@ -2,14 +2,11 @@ package migrate import ( "fmt" - "os" - "path" "time" "github.com/AlecAivazis/survey/v2" "github.com/fatih/color" "github.com/pocketbase/dbx" - "github.com/pocketbase/pocketbase/tools/inflector" "github.com/spf13/cast" ) @@ -100,57 +97,6 @@ func (r *Runner) Run(args ...string) error { } } - return nil - case "create": - if len(args) < 2 { - return fmt.Errorf("Missing migration file name") - } - - name := args[1] - - var dir string - if len(args) == 3 { - dir = args[2] - } - if dir == "" { - // If not specified, auto point to the default migrations folder. - // - // NB! - // Since the create command makes sense only during development, - // it is expected the user to be in the app working directory - // and to be using `go run ...` - wd, err := os.Getwd() - if err != nil { - return err - } - dir = path.Join(wd, "migrations") - } - - resultFilePath := path.Join( - dir, - fmt.Sprintf("%d_%s.go", time.Now().Unix(), inflector.Snakecase(name)), - ) - - confirm := false - prompt := &survey.Confirm{ - Message: fmt.Sprintf("Do you really want to create migration %q?", resultFilePath), - } - survey.AskOne(prompt, &confirm) - if !confirm { - fmt.Println("The command has been cancelled") - return nil - } - - // ensure that migrations dir exist - if err := os.MkdirAll(dir, os.ModePerm); err != nil { - return err - } - - if err := os.WriteFile(resultFilePath, []byte(createTemplateContent), 0644); err != nil { - return fmt.Errorf("Failed to save migration file %q\n", resultFilePath) - } - - fmt.Printf("Successfully created file %q\n", resultFilePath) return nil default: return fmt.Errorf("Unsupported command: %q\n", cmd) diff --git a/tools/migrate/template.go b/tools/migrate/template.go deleted file mode 100644 index 9a97458a..00000000 --- a/tools/migrate/template.go +++ /dev/null @@ -1,21 +0,0 @@ -package migrate - -const createTemplateContent = `package migrations - -import ( - "github.com/pocketbase/dbx" - m "github.com/pocketbase/pocketbase/migrations" -) - -func init() { - m.Register(func(db dbx.Builder) error { - // add up queries... - - return nil - }, func(db dbx.Builder) error { - // add down queries... - - return nil - }) -} -`