diff --git a/cmd/admin.go b/cmd/admin.go index c2c9956f..fd76a9bd 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -1,7 +1,8 @@ package cmd import ( - "os" + "errors" + "fmt" "github.com/fatih/color" "github.com/go-ozzo/ozzo-validation/v4/is" @@ -30,20 +31,20 @@ func adminCreateCommand(app core.App) *cobra.Command { Use: "create", Example: "admin create test@example.com 1234567890", Short: "Creates a new admin account", - Run: func(command *cobra.Command, args []string) { + // prevents printing the error log twice + SilenceErrors: true, + SilenceUsage: true, + RunE: func(command *cobra.Command, args []string) error { if len(args) != 2 { - color.Red("Missing email and password arguments.") - os.Exit(1) + return errors.New("Missing email and password arguments.") } - if is.EmailFormat.Validate(args[0]) != nil { - color.Red("Invalid email address.") - os.Exit(1) + if args[0] == "" || is.EmailFormat.Validate(args[0]) != nil { + return errors.New("Missing or invalid email address.") } if len(args[1]) < 8 { - color.Red("The password must be at least 8 chars long.") - os.Exit(1) + return errors.New("The password must be at least 8 chars long.") } admin := &models.Admin{} @@ -51,11 +52,11 @@ func adminCreateCommand(app core.App) *cobra.Command { admin.SetPassword(args[1]) if err := app.Dao().SaveAdmin(admin); err != nil { - color.Red("Failed to create new admin account: %v", err) - os.Exit(1) + return fmt.Errorf("Failed to create new admin account: %v", err) } color.Green("Successfully created new admin %s!", admin.Email) + return nil }, } @@ -67,36 +68,35 @@ func adminUpdateCommand(app core.App) *cobra.Command { Use: "update", Example: "admin update test@example.com 1234567890", Short: "Changes the password of a single admin account", - Run: func(command *cobra.Command, args []string) { + // prevents printing the error log twice + SilenceErrors: true, + SilenceUsage: true, + RunE: func(command *cobra.Command, args []string) error { if len(args) != 2 { - color.Red("Missing email and password arguments.") - os.Exit(1) + return errors.New("Missing email and password arguments.") } - if is.EmailFormat.Validate(args[0]) != nil { - color.Red("Invalid email address.") - os.Exit(1) + if args[0] == "" || is.EmailFormat.Validate(args[0]) != nil { + return errors.New("Missing or invalid email address.") } if len(args[1]) < 8 { - color.Red("The new password must be at least 8 chars long.") - os.Exit(1) + return errors.New("The new password must be at least 8 chars long.") } admin, err := app.Dao().FindAdminByEmail(args[0]) if err != nil { - color.Red("Admin with email %s doesn't exist.", args[0]) - os.Exit(1) + return fmt.Errorf("Admin with email %s doesn't exist.", args[0]) } admin.SetPassword(args[1]) if err := app.Dao().SaveAdmin(admin); err != nil { - color.Red("Failed to change admin %s password: %v", admin.Email, err) - os.Exit(1) + return fmt.Errorf("Failed to change admin %s password: %v", admin.Email, err) } color.Green("Successfully changed admin %s password!", admin.Email) + return nil }, } @@ -108,24 +108,26 @@ func adminDeleteCommand(app core.App) *cobra.Command { Use: "delete", Example: "admin delete test@example.com", Short: "Deletes an existing admin account", - Run: func(command *cobra.Command, args []string) { - if len(args) == 0 || is.EmailFormat.Validate(args[0]) != nil { - color.Red("Invalid or missing email address.") - os.Exit(1) + // prevents printing the error log twice + SilenceErrors: true, + SilenceUsage: true, + RunE: func(command *cobra.Command, args []string) error { + if len(args) == 0 || args[0] == "" || is.EmailFormat.Validate(args[0]) != nil { + return errors.New("Invalid or missing email address.") } admin, err := app.Dao().FindAdminByEmail(args[0]) if err != nil { color.Yellow("Admin %s is already deleted.", args[0]) - return + return nil } if err := app.Dao().DeleteAdmin(admin); err != nil { - color.Red("Failed to delete admin %s: %v", admin.Email, err) - os.Exit(1) + return fmt.Errorf("Failed to delete admin %s: %v", admin.Email, err) } color.Green("Successfully deleted admin %s!", admin.Email) + return nil }, } diff --git a/cmd/admin_test.go b/cmd/admin_test.go new file mode 100644 index 00000000..c3fbbd70 --- /dev/null +++ b/cmd/admin_test.go @@ -0,0 +1,219 @@ +package cmd_test + +import ( + "testing" + + "github.com/pocketbase/pocketbase/cmd" + "github.com/pocketbase/pocketbase/tests" +) + +func TestAdminCreateCommand(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + scenarios := []struct { + name string + email string + password string + expectError bool + }{ + { + "empty email and password", + "", + "", + true, + }, + { + "empty email", + "", + "1234567890", + true, + }, + { + "invalid email", + "invalid", + "1234567890", + true, + }, + { + "duplicated email", + "test@example.com", + "1234567890", + true, + }, + { + "empty password", + "test@example.com", + "", + true, + }, + { + "short password", + "test_new@example.com", + "1234567", + true, + }, + { + "valid email and password", + "test_new@example.com", + "12345678", + false, + }, + } + + for _, s := range scenarios { + command := cmd.NewAdminCommand(app) + command.SetArgs([]string{"create", s.email, s.password}) + + err := command.Execute() + + hasErr := err != nil + if s.expectError != hasErr { + t.Errorf("[%s] Expected hasErr %v, got %v (%v)", s.name, s.expectError, hasErr, err) + } + + if hasErr { + continue + } + + // check whether the admin account was actually created + admin, err := app.Dao().FindAdminByEmail(s.email) + if err != nil { + t.Errorf("[%s] Failed to fetch created admin %s: %v", s.name, s.email, err) + } else if !admin.ValidatePassword(s.password) { + t.Errorf("[%s] Expected the admin password to match", s.name) + } + } +} + +func TestAdminUpdateCommand(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + scenarios := []struct { + name string + email string + password string + expectError bool + }{ + { + "empty email and password", + "", + "", + true, + }, + { + "empty email", + "", + "1234567890", + true, + }, + { + "invalid email", + "invalid", + "1234567890", + true, + }, + { + "nonexisting admin", + "test_missing@example.com", + "1234567890", + true, + }, + { + "empty password", + "test@example.com", + "", + true, + }, + { + "short password", + "test_new@example.com", + "1234567", + true, + }, + { + "valid email and password", + "test@example.com", + "12345678", + false, + }, + } + + for _, s := range scenarios { + command := cmd.NewAdminCommand(app) + command.SetArgs([]string{"update", s.email, s.password}) + + err := command.Execute() + + hasErr := err != nil + if s.expectError != hasErr { + t.Errorf("[%s] Expected hasErr %v, got %v (%v)", s.name, s.expectError, hasErr, err) + } + + if hasErr { + continue + } + + // check whether the admin password was actually changed + admin, err := app.Dao().FindAdminByEmail(s.email) + if err != nil { + t.Errorf("[%s] Failed to fetch admin %s: %v", s.name, s.email, err) + } else if !admin.ValidatePassword(s.password) { + t.Errorf("[%s] Expected the admin password to match", s.name) + } + } +} + +func TestAdminDeleteCommand(t *testing.T) { + app, _ := tests.NewTestApp() + defer app.Cleanup() + + scenarios := []struct { + name string + email string + expectError bool + }{ + { + "empty email", + "", + true, + }, + { + "invalid email", + "invalid", + true, + }, + { + "nonexisting admin", + "test_missing@example.com", + false, + }, + { + "existing admin", + "test@example.com", + false, + }, + } + + for _, s := range scenarios { + command := cmd.NewAdminCommand(app) + command.SetArgs([]string{"delete", s.email}) + + err := command.Execute() + + hasErr := err != nil + if s.expectError != hasErr { + t.Errorf("[%s] Expected hasErr %v, got %v (%v)", s.name, s.expectError, hasErr, err) + } + + if hasErr { + continue + } + + // check whether the admin account was actually deleted + if _, err := app.Dao().FindAdminByEmail(s.email); err == nil { + t.Errorf("[%s] Expected the admin account to be deleted", s.name) + } + } +} diff --git a/plugins/migratecmd/migratecmd.go b/plugins/migratecmd/migratecmd.go index 41e7ad0a..363cd3f3 100644 --- a/plugins/migratecmd/migratecmd.go +++ b/plugins/migratecmd/migratecmd.go @@ -17,7 +17,6 @@ package migratecmd import ( "fmt" - "log" "os" "path" "path/filepath" @@ -129,9 +128,12 @@ func (p *plugin) createCommand() *cobra.Command { command := &cobra.Command{ Use: "migrate", Short: "Executes app DB migration scripts", - ValidArgs: []string{"up", "down", "create", "collections"}, Long: cmdDesc, - Run: func(command *cobra.Command, args []string) { + ValidArgs: []string{"up", "down", "create", "collections"}, + // prevents printing the error log twice + SilenceErrors: true, + SilenceUsage: true, + RunE: func(command *cobra.Command, args []string) error { cmd := "" if len(args) > 0 { cmd = args[0] @@ -140,22 +142,24 @@ func (p *plugin) createCommand() *cobra.Command { switch cmd { case "create": if err := p.migrateCreateHandler("", args[1:], true); err != nil { - log.Fatal(err) + return err } case "collections": if err := p.migrateCollectionsHandler(args[1:], true); err != nil { - log.Fatal(err) + return err } default: runner, err := migrate.NewRunner(p.app.DB(), migrations.AppMigrations) if err != nil { - log.Fatal(err) + return err } if err := runner.Run(args...); err != nil { - log.Fatal(err) + return err } } + + return nil }, } diff --git a/pocketbase.go b/pocketbase.go index 58669feb..19dcb917 100644 --- a/pocketbase.go +++ b/pocketbase.go @@ -1,7 +1,6 @@ package pocketbase import ( - "log" "os" "os/signal" "path/filepath" @@ -9,6 +8,7 @@ import ( "sync" "syscall" + "github.com/fatih/color" "github.com/pocketbase/pocketbase/cmd" "github.com/pocketbase/pocketbase/core" "github.com/pocketbase/pocketbase/tools/list" @@ -171,7 +171,9 @@ func (pb *PocketBase) Execute() error { go func() { defer wg.Done() if err := pb.RootCmd.Execute(); err != nil { - log.Println(err) + // @todo replace with db log once generalized logs are added + // (note may need to update the existing commands to not silence errors) + color.Red(err.Error()) } }() diff --git a/tools/migrate/runner.go b/tools/migrate/runner.go index ee504f68..952b7ee0 100644 --- a/tools/migrate/runner.go +++ b/tools/migrate/runner.go @@ -50,7 +50,6 @@ func (r *Runner) Run(args ...string) error { case "up": applied, err := r.Up() if err != nil { - color.Red(err.Error()) return err } @@ -75,7 +74,6 @@ func (r *Runner) Run(args ...string) error { names, err := r.lastAppliedMigrations(toRevertCount) if err != nil { - color.Red(err.Error()) return err } @@ -95,7 +93,6 @@ func (r *Runner) Run(args ...string) error { reverted, err := r.Down(toRevertCount) if err != nil { - color.Red(err.Error()) return err } @@ -110,7 +107,6 @@ func (r *Runner) Run(args ...string) error { return nil case "history-sync": if err := r.removeMissingAppliedMigrations(); err != nil { - color.Red(err.Error()) return err }