From c25e67e13d345f94411e4ab137ec6feec93d9b40 Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Thu, 15 Dec 2022 23:20:23 +0200 Subject: [PATCH] [#1267] call app.Bootstrap() before cobra commands execution --- CHANGELOG.md | 17 +++++++- core/app.go | 8 +++- core/base.go | 10 ++++- core/base_test.go | 8 ++++ pocketbase.go | 68 ++++++++++++++++++++++++++++---- pocketbase_test.go | 98 ++++++++++++++++++++++++++++++++++++++++++---- 6 files changed, 189 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6764a78d..e6be4719 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,9 +14,22 @@ - Fixed `~` expressions backslash literal escaping ([#1231](https://github.com/pocketbase/pocketbase/discussions/1231)). -- Changed `core.NewBaseApp(dir, encryptionEnv, isDebug)` to `NewBaseApp(config *BaseAppConfig)` which allows to further configure the app instance. +- Refactored the `core.app.Bootstrap()` to be called before starting the cobra commands ([#1267](https://github.com/pocketbase/pocketbase/discussions/1267)). + +- ! Changed `pocketbase.NewWithConfig(config Config)` to `pocketbase.NewWithConfig(config *Config)` and added 4 new config settings: + ```go + DataMaxOpenConns int // default to core.DefaultDataMaxOpenConns + DataMaxIdleConns int // default to core.DefaultDataMaxIdleConns + LogsMaxOpenConns int // default to core.DefaultLogsMaxOpenConns + LogsMaxIdleConns int // default to core.DefaultLogsMaxIdleConns + ``` + +- Added new helper method `core.App.IsBootstrapped()` to check the current app bootstrap state. + +- ! Changed `core.NewBaseApp(dir, encryptionEnv, isDebug)` to `NewBaseApp(config *BaseAppConfig)`. + +- ! Removed `rest.UploadedFile` struct (see below `filesystem.File`). -- Removed `rest.UploadedFile` struct (see below `filesystem.File`). - Added generic file resource struct that allows loading and uploading file content from different sources (at the moment multipart/formdata requests and from the local filesystem). diff --git a/core/app.go b/core/app.go index 3700b1d9..c6e02878 100644 --- a/core/app.go +++ b/core/app.go @@ -78,8 +78,14 @@ type App interface { // RefreshSettings reinitializes and reloads the stored application settings. RefreshSettings() error + // IsBootstrapped checks if the application was initialized + // (aka. whether Bootstrap() was called). + IsBootstrapped() bool + // Bootstrap takes care for initializing the application - // (open db connections, load settings, etc.) + // (open db connections, load settings, etc.). + // + // It will call ResetBootstrapState() if the application was already bootstrapped. Bootstrap() error // ResetBootstrapState takes care for releasing initialized app resources diff --git a/core/base.go b/core/base.go index 7d50e3be..05ae117f 100644 --- a/core/base.go +++ b/core/base.go @@ -267,8 +267,16 @@ func NewBaseApp(config *BaseAppConfig) *BaseApp { return app } +// IsBootstrapped checks if the application was initialized +// (aka. whether Bootstrap() was called). +func (app *BaseApp) IsBootstrapped() bool { + return app.dao != nil && app.logsDao != nil && app.settings != nil +} + // Bootstrap initializes the application -// (aka. create data dir, open db connections, load settings, etc.) +// (aka. create data dir, open db connections, load settings, etc.). +// +// It will call ResetBootstrapState() if the application was already bootstrapped. func (app *BaseApp) Bootstrap() error { event := &BootstrapEvent{app} diff --git a/core/base_test.go b/core/base_test.go index 4a32c22e..ab5664dc 100644 --- a/core/base_test.go +++ b/core/base_test.go @@ -53,11 +53,19 @@ func TestBaseAppBootstrap(t *testing.T) { }) defer app.ResetBootstrapState() + if app.IsBootstrapped() { + t.Fatal("Didn't expect the application to be bootstrapped.") + } + // bootstrap if err := app.Bootstrap(); err != nil { t.Fatal(err) } + if !app.IsBootstrapped() { + t.Fatal("Expected the application to be bootstrapped.") + } + if stat, err := os.Stat(testDataDir); err != nil || !stat.IsDir() { t.Fatal("Expected test data directory to be created.") } diff --git a/pocketbase.go b/pocketbase.go index 8b176278..409b8da9 100644 --- a/pocketbase.go +++ b/pocketbase.go @@ -11,6 +11,7 @@ import ( "github.com/pocketbase/pocketbase/cmd" "github.com/pocketbase/pocketbase/core" + "github.com/pocketbase/pocketbase/tools/list" "github.com/spf13/cobra" ) @@ -68,7 +69,7 @@ type Config struct { func New() *PocketBase { _, isUsingGoRun := inspectRuntime() - return NewWithConfig(Config{ + return NewWithConfig(&Config{ DefaultDebug: isUsingGoRun, }) } @@ -80,7 +81,11 @@ func New() *PocketBase { // Everything will be initialized when [Start()] is executed. // If you want to initialize the application before calling [Start()], // then you'll have to manually call [Bootstrap()]. -func NewWithConfig(config Config) *PocketBase { +func NewWithConfig(config *Config) *PocketBase { + if config == nil { + panic("missing config") + } + // initialize a default data directory based on the executable baseDir if config.DefaultDataDir == "" { baseDir, _ := inspectRuntime() @@ -124,11 +129,6 @@ func NewWithConfig(config Config) *PocketBase { // hide the default help command (allow only `--help` flag) pb.RootCmd.SetHelpCommand(&cobra.Command{Hidden: true}) - // hook the bootstrap process - pb.RootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { - return pb.Bootstrap() - } - return pb } @@ -148,6 +148,18 @@ func (pb *PocketBase) Start() error { // This method differs from pb.Start() by not registering the default // system commands! func (pb *PocketBase) Execute() error { + // Run the bootstrap process if the command exist and it is not + // the default cobra version command to prevent creating + // unnecessary the initialization system files. + // + // https://github.com/pocketbase/pocketbase/issues/404 + // https://github.com/pocketbase/pocketbase/discussions/1267 + if !pb.skipBootstrap() { + if err := pb.Bootstrap(); err != nil { + return err + } + } + var wg sync.WaitGroup wg.Add(1) @@ -181,7 +193,7 @@ func (pb *PocketBase) onTerminate() error { // eagerParseFlags parses the global app flags before calling pb.RootCmd.Execute(). // so we can have all PocketBase flags ready for use on initialization. -func (pb *PocketBase) eagerParseFlags(config Config) error { +func (pb *PocketBase) eagerParseFlags(config *Config) error { pb.RootCmd.PersistentFlags().StringVar( &pb.dataDirFlag, "dir", @@ -206,6 +218,46 @@ func (pb *PocketBase) eagerParseFlags(config Config) error { return pb.RootCmd.ParseFlags(os.Args[1:]) } +// eager check if the app should skip the bootstap process: +// - already bootstrapped +// - is unknown command +// - is the default help command +// - is the default version command +func (pb *PocketBase) skipBootstrap() bool { + flags := []string{ + "-h", + "--help", + "-v", + "--version", + } + + if pb.IsBootstrapped() { + return true // already bootstrapped + } + + cmd, _, err := pb.RootCmd.Find(os.Args[1:]) + if err != nil { + return true // unknown command + } + + for _, arg := range os.Args { + if !list.ExistInSlice(arg, flags) { + continue + } + + // ensure that there is no user defined flag with the same name/shorthand + trimmed := strings.TrimLeft(arg, "-") + if len(trimmed) > 1 && cmd.Flags().Lookup(trimmed) == nil { + return true + } + if len(trimmed) == 1 && cmd.Flags().ShorthandLookup(trimmed) == nil { + return true + } + } + + return false +} + // tries to find the base executable directory and how it was run func inspectRuntime() (baseDir string, withGoRun bool) { if strings.HasPrefix(os.Args[0], os.TempDir()) { diff --git a/pocketbase_test.go b/pocketbase_test.go index 10ba6a54..d569d4d0 100644 --- a/pocketbase_test.go +++ b/pocketbase_test.go @@ -2,20 +2,23 @@ package pocketbase import ( "os" + "path/filepath" "testing" + + "github.com/spf13/cobra" ) func TestNew(t *testing.T) { // copy os.Args - originalArgs := []string{} + originalArgs := make([]string, len(os.Args)) copy(originalArgs, os.Args) defer func() { // restore os.Args - copy(os.Args, originalArgs) + os.Args = originalArgs }() // change os.Args - os.Args = os.Args[0:1] + os.Args = os.Args[:1] os.Args = append( os.Args, "--dir=test_dir", @@ -51,7 +54,7 @@ func TestNew(t *testing.T) { } func TestNewWithConfig(t *testing.T) { - app := NewWithConfig(Config{ + app := NewWithConfig(&Config{ DefaultDebug: true, DefaultDataDir: "test_dir", DefaultEncryptionEnv: "test_encryption_env", @@ -89,15 +92,15 @@ func TestNewWithConfig(t *testing.T) { func TestNewWithConfigAndFlags(t *testing.T) { // copy os.Args - originalArgs := []string{} + originalArgs := make([]string, len(os.Args)) copy(originalArgs, os.Args) defer func() { // restore os.Args - copy(os.Args, originalArgs) + os.Args = originalArgs }() // change os.Args - os.Args = os.Args[0:1] + os.Args = os.Args[:1] os.Args = append( os.Args, "--dir=test_dir_flag", @@ -105,7 +108,7 @@ func TestNewWithConfigAndFlags(t *testing.T) { "--debug=false", ) - app := NewWithConfig(Config{ + app := NewWithConfig(&Config{ DefaultDebug: true, DefaultDataDir: "test_dir", DefaultEncryptionEnv: "test_encryption_env", @@ -140,3 +143,82 @@ func TestNewWithConfigAndFlags(t *testing.T) { t.Fatal("Expected app.IsDebug() false, got true") } } + +func TestSkipBootstrap(t *testing.T) { + // copy os.Args + originalArgs := make([]string, len(os.Args)) + copy(originalArgs, os.Args) + defer func() { + // restore os.Args + os.Args = originalArgs + }() + + tempDir := filepath.Join(os.TempDir(), "temp_pb_data") + defer os.RemoveAll(tempDir) + + // already bootstrapped + app0 := NewWithConfig(&Config{DefaultDataDir: tempDir}) + app0.Bootstrap() + if v := app0.skipBootstrap(); !v { + t.Fatal("[bootstrapped] Expected true, got false") + } + + // unknown command + os.Args = os.Args[:1] + os.Args = append(os.Args, "demo") + app1 := NewWithConfig(&Config{DefaultDataDir: tempDir}) + app1.RootCmd.AddCommand(&cobra.Command{Use: "test"}) + if v := app1.skipBootstrap(); !v { + t.Fatal("[unknown] Expected true, got false") + } + + // default flags + flagScenarios := []struct { + name string + short string + }{ + {"help", "h"}, + {"version", "v"}, + } + + for _, s := range flagScenarios { + // base flag + os.Args = os.Args[:1] + os.Args = append(os.Args, "--"+s.name) + app1 := NewWithConfig(&Config{DefaultDataDir: tempDir}) + if v := app1.skipBootstrap(); !v { + t.Fatalf("[--%s] Expected true, got false", s.name) + } + + // short flag + os.Args = os.Args[:1] + os.Args = append(os.Args, "-"+s.short) + app2 := NewWithConfig(&Config{DefaultDataDir: tempDir}) + if v := app2.skipBootstrap(); !v { + t.Fatalf("[-%s] Expected true, got false", s.short) + } + + customCmd := &cobra.Command{Use: "custom"} + customCmd.PersistentFlags().BoolP(s.name, s.short, false, "") + + // base flag in custom command + os.Args = os.Args[:1] + os.Args = append(os.Args, "custom") + os.Args = append(os.Args, "--"+s.name) + app3 := NewWithConfig(&Config{DefaultDataDir: tempDir}) + app3.RootCmd.AddCommand(customCmd) + if v := app3.skipBootstrap(); v { + t.Fatalf("[--%s custom] Expected false, got true", s.name) + } + + // short flag in custom command + os.Args = os.Args[:1] + os.Args = append(os.Args, "custom") + os.Args = append(os.Args, "-"+s.short) + app4 := NewWithConfig(&Config{DefaultDataDir: tempDir}) + app4.RootCmd.AddCommand(customCmd) + if v := app4.skipBootstrap(); v { + t.Fatalf("[-%s custom] Expected false, got true", s.short) + } + } +}