From 346bd9afb1c83eb31282c84f65470ae974cbb936 Mon Sep 17 00:00:00 2001 From: Sung <8265228+sungwoncho@users.noreply.github.com> Date: Sun, 12 Oct 2025 15:08:11 -0700 Subject: [PATCH] Add dbPath flag and update apiEndpoint flag (#692) * Allow to specify CLI db path as a flag * Make API endpoint flag per command and change case --- .github/workflows/release-cli.yml | 19 --------- pkg/cli/cmd/login/login.go | 8 +++- pkg/cli/cmd/logout/logout.go | 10 +++++ pkg/cli/cmd/root/root.go | 10 ++--- pkg/cli/cmd/sync/sync.go | 7 ++++ pkg/cli/infra/init.go | 48 +++++++++++++--------- pkg/cli/infra/init_test.go | 29 ++++--------- pkg/cli/main.go | 12 ++---- pkg/cli/main_test.go | 68 +++++++++++++++++++++++++++++++ pkg/e2e/sync_test.go | 8 ++-- 10 files changed, 141 insertions(+), 78 deletions(-) diff --git a/.github/workflows/release-cli.yml b/.github/workflows/release-cli.yml index 723c2d1a..81a2ea5e 100644 --- a/.github/workflows/release-cli.yml +++ b/.github/workflows/release-cli.yml @@ -75,22 +75,3 @@ jobs: --title="$TAG" \ --notes-file=/tmp/changelog.txt \ --draft - - - name: Bump Homebrew formula - env: - HOMEBREW_GITHUB_API_TOKEN: ${{ secrets.HOMEBREW_RELEASE_TOKEN }} - run: | - VERSION="${{ steps.version.outputs.version }}" - TAG="cli-v${VERSION}" - - # Only bump Homebrew for stable releases (not prereleases) - if [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then - brew update-reset - brew bump-formula-pr \ - --no-browse \ - --tag="$TAG" \ - --revision="${{ github.sha }}" \ - dnote - else - echo "Skipping Homebrew update for prerelease version: $VERSION" - fi diff --git a/pkg/cli/cmd/login/login.go b/pkg/cli/cmd/login/login.go index 1e667382..4d8ee392 100644 --- a/pkg/cli/cmd/login/login.go +++ b/pkg/cli/cmd/login/login.go @@ -37,7 +37,7 @@ import ( var example = ` dnote login` -var usernameFlag, passwordFlag string +var usernameFlag, passwordFlag, apiEndpointFlag string // NewCmd returns a new login command func NewCmd(ctx context.DnoteCtx) *cobra.Command { @@ -51,6 +51,7 @@ func NewCmd(ctx context.DnoteCtx) *cobra.Command { f := cmd.Flags() f.StringVarP(&usernameFlag, "username", "u", "", "email address for authentication") f.StringVarP(&passwordFlag, "password", "p", "", "password for authentication") + f.StringVar(&apiEndpointFlag, "apiEndpoint", "", "API endpoint to connect to (defaults to value in config)") return cmd } @@ -147,6 +148,11 @@ func getGreeting(ctx context.DnoteCtx) string { func newRun(ctx context.DnoteCtx) infra.RunEFunc { return func(cmd *cobra.Command, args []string) error { + // Override APIEndpoint if flag was provided + if apiEndpointFlag != "" { + ctx.APIEndpoint = apiEndpointFlag + } + greeting := getGreeting(ctx) log.Plain(greeting) diff --git a/pkg/cli/cmd/logout/logout.go b/pkg/cli/cmd/logout/logout.go index 0137e078..008c9652 100644 --- a/pkg/cli/cmd/logout/logout.go +++ b/pkg/cli/cmd/logout/logout.go @@ -37,6 +37,8 @@ var ErrNotLoggedIn = errors.New("not logged in") var example = ` dnote logout` +var apiEndpointFlag string + // NewCmd returns a new logout command func NewCmd(ctx context.DnoteCtx) *cobra.Command { cmd := &cobra.Command{ @@ -46,6 +48,9 @@ func NewCmd(ctx context.DnoteCtx) *cobra.Command { RunE: newRun(ctx), } + f := cmd.Flags() + f.StringVar(&apiEndpointFlag, "apiEndpoint", "", "API endpoint to connect to (defaults to value in config)") + return cmd } @@ -84,6 +89,11 @@ func Do(ctx context.DnoteCtx) error { func newRun(ctx context.DnoteCtx) infra.RunEFunc { return func(cmd *cobra.Command, args []string) error { + // Override APIEndpoint if flag was provided + if apiEndpointFlag != "" { + ctx.APIEndpoint = apiEndpointFlag + } + err := Do(ctx) if err == ErrNotLoggedIn { log.Error("not logged in\n") diff --git a/pkg/cli/cmd/root/root.go b/pkg/cli/cmd/root/root.go index 750629e4..604f7682 100644 --- a/pkg/cli/cmd/root/root.go +++ b/pkg/cli/cmd/root/root.go @@ -22,7 +22,7 @@ import ( "github.com/spf13/cobra" ) -var apiEndpointFlag string +var dbPathFlag string var root = &cobra.Command{ Use: "dnote", @@ -35,7 +35,7 @@ var root = &cobra.Command{ } func init() { - root.PersistentFlags().StringVar(&apiEndpointFlag, "api-endpoint", "", "the API endpoint to connect to (defaults to value in config)") + root.PersistentFlags().StringVar(&dbPathFlag, "dbPath", "", "the path to the database file (defaults to standard location)") } // GetRoot returns the root command @@ -43,9 +43,9 @@ func GetRoot() *cobra.Command { return root } -// GetAPIEndpointFlag returns the value of the --api-endpoint flag -func GetAPIEndpointFlag() string { - return apiEndpointFlag +// GetDBPathFlag returns the value of the --dbPath flag +func GetDBPathFlag() string { + return dbPathFlag } // Register adds a new command diff --git a/pkg/cli/cmd/sync/sync.go b/pkg/cli/cmd/sync/sync.go index ff8e8a5f..a1fea893 100644 --- a/pkg/cli/cmd/sync/sync.go +++ b/pkg/cli/cmd/sync/sync.go @@ -44,6 +44,7 @@ var example = ` dnote sync` var isFullSync bool +var apiEndpointFlag string // NewCmd returns a new sync command func NewCmd(ctx context.DnoteCtx) *cobra.Command { @@ -57,6 +58,7 @@ func NewCmd(ctx context.DnoteCtx) *cobra.Command { f := cmd.Flags() f.BoolVarP(&isFullSync, "full", "f", false, "perform a full sync instead of incrementally syncing only the changed data.") + f.StringVar(&apiEndpointFlag, "apiEndpoint", "", "API endpoint to connect to (defaults to value in config)") return cmd } @@ -931,6 +933,11 @@ func prepareEmptyServerSync(tx *database.DB) error { func newRun(ctx context.DnoteCtx) infra.RunEFunc { return func(cmd *cobra.Command, args []string) error { + // Override APIEndpoint if flag was provided + if apiEndpointFlag != "" { + ctx.APIEndpoint = apiEndpointFlag + } + if ctx.SessionKey == "" { return errors.New("not logged in") } diff --git a/pkg/cli/infra/init.go b/pkg/cli/infra/init.go index 532f1f1e..0d2d767a 100644 --- a/pkg/cli/infra/init.go +++ b/pkg/cli/infra/init.go @@ -42,6 +42,11 @@ import ( "github.com/spf13/cobra" ) +const ( + // DefaultAPIEndpoint is the default API endpoint used when none is configured + DefaultAPIEndpoint = "http://localhost:3001/api" +) + // RunEFunc is a function type of dnote commands type RunEFunc func(*cobra.Command, []string) error @@ -59,7 +64,12 @@ func checkLegacyDBPath() (string, bool) { return "", false } -func getDBPath(paths context.Paths) string { +func getDBPath(paths context.Paths, customPath string) string { + // If custom path is provided, use it + if customPath != "" { + return customPath + } + legacyDnoteDir, ok := checkLegacyDBPath() if ok { return fmt.Sprintf("%s/%s", legacyDnoteDir, consts.DnoteDBFileName) @@ -71,7 +81,7 @@ func getDBPath(paths context.Paths) string { // newBaseCtx creates a minimal context with paths and database connection. // This base context is used for file and database initialization before // being enriched with config values by setupCtx. -func newBaseCtx(versionTag string) (context.DnoteCtx, error) { +func newBaseCtx(versionTag, customDBPath string) (context.DnoteCtx, error) { dnoteDir := getLegacyDnotePath(dirs.Home) paths := context.Paths{ Home: dirs.Home, @@ -81,7 +91,7 @@ func newBaseCtx(versionTag string) (context.DnoteCtx, error) { LegacyDnote: dnoteDir, } - dbPath := getDBPath(paths) + dbPath := getDBPath(paths, customDBPath) db, err := database.Open(dbPath) if err != nil { @@ -98,13 +108,14 @@ func newBaseCtx(versionTag string) (context.DnoteCtx, error) { } // Init initializes the Dnote environment and returns a new dnote context -func Init(versionTag, apiEndpoint string) (*context.DnoteCtx, error) { - ctx, err := newBaseCtx(versionTag) +// apiEndpoint is used when creating a new config file (e.g., from ldflags during tests) +func Init(versionTag, apiEndpoint, dbPath string) (*context.DnoteCtx, error) { + ctx, err := newBaseCtx(versionTag, dbPath) if err != nil { return nil, errors.Wrap(err, "initializing a context") } - if err := InitFiles(ctx, apiEndpoint); err != nil { + if err := initFiles(ctx, apiEndpoint); err != nil { return nil, errors.Wrap(err, "initializing files") } @@ -122,7 +133,7 @@ func Init(versionTag, apiEndpoint string) (*context.DnoteCtx, error) { return nil, errors.Wrap(err, "running migration") } - ctx, err = setupCtx(ctx, apiEndpoint) + ctx, err = setupCtx(ctx) if err != nil { return nil, errors.Wrap(err, "setting up the context") } @@ -134,8 +145,7 @@ func Init(versionTag, apiEndpoint string) (*context.DnoteCtx, error) { // setupCtx enriches the base context with values from config file and database. // This is called after files and database have been initialized. -// If apiEndpoint is provided, it overrides the value from config. -func setupCtx(ctx context.DnoteCtx, apiEndpoint string) (context.DnoteCtx, error) { +func setupCtx(ctx context.DnoteCtx) (context.DnoteCtx, error) { db := ctx.DB var sessionKey string @@ -155,19 +165,13 @@ func setupCtx(ctx context.DnoteCtx, apiEndpoint string) (context.DnoteCtx, error return ctx, errors.Wrap(err, "reading config") } - // Use override if provided, otherwise use config value - endpoint := cf.APIEndpoint - if apiEndpoint != "" { - endpoint = apiEndpoint - } - ret := context.DnoteCtx{ Paths: ctx.Paths, Version: ctx.Version, DB: ctx.DB, SessionKey: sessionKey, SessionKeyExpiry: sessionKeyExpiry, - APIEndpoint: endpoint, + APIEndpoint: cf.APIEndpoint, Editor: cf.Editor, Clock: clock.New(), EnableUpgradeCheck: cf.EnableUpgradeCheck, @@ -367,9 +371,15 @@ func initConfigFile(ctx context.DnoteCtx, apiEndpoint string) error { editor := getEditorCommand() + // Use default API endpoint if none provided + endpoint := apiEndpoint + if endpoint == "" { + endpoint = DefaultAPIEndpoint + } + cf := config.Config{ Editor: editor, - APIEndpoint: apiEndpoint, + APIEndpoint: endpoint, EnableUpgradeCheck: true, } @@ -380,8 +390,8 @@ func initConfigFile(ctx context.DnoteCtx, apiEndpoint string) error { return nil } -// InitFiles creates, if necessary, the dnote directory and files inside -func InitFiles(ctx context.DnoteCtx, apiEndpoint string) error { +// initFiles creates, if necessary, the dnote directory and files inside +func initFiles(ctx context.DnoteCtx, apiEndpoint string) error { if err := initDnoteDir(ctx); err != nil { return errors.Wrap(err, "creating the dnote dir") } diff --git a/pkg/cli/infra/init_test.go b/pkg/cli/infra/init_test.go index cb50a95f..5fff1c93 100644 --- a/pkg/cli/infra/init_test.go +++ b/pkg/cli/infra/init_test.go @@ -95,7 +95,7 @@ func TestInitSystemKV_existing(t *testing.T) { assert.Equal(t, val, "testVal", "system value should not have been updated") } -func TestInit_APIEndpointChange(t *testing.T) { +func TestInit_APIEndpoint(t *testing.T) { // Create a temporary directory for test tmpDir, err := os.MkdirTemp("", "dnote-init-test-*") if err != nil { @@ -108,35 +108,20 @@ func TestInit_APIEndpointChange(t *testing.T) { t.Setenv("XDG_DATA_HOME", fmt.Sprintf("%s/data", tmpDir)) t.Setenv("XDG_CACHE_HOME", fmt.Sprintf("%s/cache", tmpDir)) - // First init. - endpoint1 := "http://127.0.0.1:3001" - ctx, err := Init("test-version", endpoint1) + // Initialize - should create config with default apiEndpoint + ctx, err := Init("test-version", "", "") if err != nil { t.Fatal(errors.Wrap(err, "initializing")) } defer ctx.DB.Close() - assert.Equal(t, ctx.APIEndpoint, endpoint1, "should use endpoint1 API endpoint") - // Test that config was written with endpoint1. + // Read the config that was created cf, err := config.Read(*ctx) if err != nil { t.Fatal(errors.Wrap(err, "reading config")) } - // Second init with different endpoint. - endpoint2 := "http://127.0.0.1:3002" - ctx2, err := Init("test-version", endpoint2) - if err != nil { - t.Fatal(errors.Wrap(err, "initializing with override")) - } - defer ctx2.DB.Close() - // Context must be using that endpoint. - assert.Equal(t, ctx2.APIEndpoint, endpoint2, "should use endpoint2 API endpoint") - - // The config file shouldn't have been modified. - cf2, err := config.Read(*ctx2) - if err != nil { - t.Fatal(errors.Wrap(err, "reading config after override")) - } - assert.Equal(t, cf2.APIEndpoint, cf.APIEndpoint, "config should still have original endpoint, not endpoint2") + // Context should use the apiEndpoint from config + assert.Equal(t, ctx.APIEndpoint, DefaultAPIEndpoint, "context should use apiEndpoint from config") + assert.Equal(t, cf.APIEndpoint, DefaultAPIEndpoint, "context should use apiEndpoint from config") } diff --git a/pkg/cli/main.go b/pkg/cli/main.go index 31c81dd5..1ba97358 100644 --- a/pkg/cli/main.go +++ b/pkg/cli/main.go @@ -46,16 +46,12 @@ var apiEndpoint string var versionTag = "master" func main() { - // Parse flags early to check if --api-endpoint was provided + // Parse flags early to get --dbPath before initializing database root.GetRoot().ParseFlags(os.Args[1:]) + dbPath := root.GetDBPathFlag() - // Use flag value if provided, otherwise use ldflags value - endpoint := apiEndpoint - if flagValue := root.GetAPIEndpointFlag(); flagValue != "" { - endpoint = flagValue - } - - ctx, err := infra.Init(versionTag, endpoint) + // Initialize context - defaultAPIEndpoint is used when creating new config file + ctx, err := infra.Init(versionTag, apiEndpoint, dbPath) if err != nil { panic(errors.Wrap(err, "initializing context")) } diff --git a/pkg/cli/main_test.go b/pkg/cli/main_test.go index 151d4cbf..63a95d87 100644 --- a/pkg/cli/main_test.go +++ b/pkg/cli/main_test.go @@ -501,3 +501,71 @@ func TestRemoveBook(t *testing.T) { }) } } + +func TestDBPathFlag(t *testing.T) { + // Helper function to verify database contents + verifyDatabase := func(t *testing.T, dbPath, expectedBook, expectedNote string) *database.DB { + ok, err := utils.FileExists(dbPath) + if err != nil { + t.Fatal(errors.Wrapf(err, "checking if custom db exists at %s", dbPath)) + } + if !ok { + t.Errorf("custom database was not created at %s", dbPath) + } + + db, err := database.Open(dbPath) + if err != nil { + t.Fatal(errors.Wrapf(err, "opening db at %s", dbPath)) + } + + var noteCount, bookCount int + database.MustScan(t, "counting books", db.QueryRow("SELECT count(*) FROM books"), &bookCount) + database.MustScan(t, "counting notes", db.QueryRow("SELECT count(*) FROM notes"), ¬eCount) + + assert.Equalf(t, bookCount, 1, fmt.Sprintf("%s book count mismatch", dbPath)) + assert.Equalf(t, noteCount, 1, fmt.Sprintf("%s note count mismatch", dbPath)) + + var book database.Book + database.MustScan(t, "getting book", db.QueryRow("SELECT label FROM books"), &book.Label) + assert.Equalf(t, book.Label, expectedBook, fmt.Sprintf("%s book label mismatch", dbPath)) + + var note database.Note + database.MustScan(t, "getting note", db.QueryRow("SELECT body FROM notes"), ¬e.Body) + assert.Equalf(t, note.Body, expectedNote, fmt.Sprintf("%s note body mismatch", dbPath)) + + return db + } + + // Setup - use two different custom database paths + customDBPath1 := "./tmp/custom-test1.db" + customDBPath2 := "./tmp/custom-test2.db" + defer testutils.RemoveDir(t, "./tmp") + + customOpts := testutils.RunDnoteCmdOptions{ + Env: []string{ + fmt.Sprintf("XDG_CONFIG_HOME=%s", testDir), + fmt.Sprintf("XDG_DATA_HOME=%s", testDir), + fmt.Sprintf("XDG_CACHE_HOME=%s", testDir), + }, + } + + // Execute - add different notes to each database + testutils.RunDnoteCmd(t, customOpts, binaryName, "--dbPath", customDBPath1, "add", "db1-book", "-c", "content in db1") + testutils.RunDnoteCmd(t, customOpts, binaryName, "--dbPath", customDBPath2, "add", "db2-book", "-c", "content in db2") + + // Test both databases + db1 := verifyDatabase(t, customDBPath1, "db1-book", "content in db1") + defer db1.Close() + + db2 := verifyDatabase(t, customDBPath2, "db2-book", "content in db2") + defer db2.Close() + + // Verify that the databases are independent + var db1HasDB2Book int + db1.QueryRow("SELECT count(*) FROM books WHERE label = ?", "db2-book").Scan(&db1HasDB2Book) + assert.Equal(t, db1HasDB2Book, 0, "db1 should not have db2's book") + + var db2HasDB1Book int + db2.QueryRow("SELECT count(*) FROM books WHERE label = ?", "db1-book").Scan(&db2HasDB1Book) + assert.Equal(t, db2HasDB1Book, 0, "db2 should not have db1's book") +} diff --git a/pkg/e2e/sync_test.go b/pkg/e2e/sync_test.go index b4fec91f..40fe976c 100644 --- a/pkg/e2e/sync_test.go +++ b/pkg/e2e/sync_test.go @@ -4254,7 +4254,7 @@ func TestSync_EmptyServer(t *testing.T) { clitest.RunDnoteCmd(t, dnoteCmdOpts, cliBinaryName, "add", "js", "-c", "js1") clitest.RunDnoteCmd(t, dnoteCmdOpts, cliBinaryName, "add", "css", "-c", "css1") - clitest.RunDnoteCmd(t, dnoteCmdOpts, cliBinaryName, "--api-endpoint", apiEndpointA, "sync") + clitest.RunDnoteCmd(t, dnoteCmdOpts, cliBinaryName, "sync", "--apiEndpoint", apiEndpointA) // Verify sync to Server A succeeded checkStateWithDB(t, ctx, userA, serverDbA, systemState{ @@ -4278,7 +4278,7 @@ func TestSync_EmptyServer(t *testing.T) { cliDatabase.MustExec(t, "updating session_key_expiry for B", ctx.DB, "UPDATE system SET value = ? WHERE key = ?", sessionB.ExpiresAt.Unix(), consts.SystemSessionKeyExpiry) // Should detect empty server and prompt - clitest.MustWaitDnoteCmd(t, dnoteCmdOpts, clitest.UserConfirmEmptyServerSync, cliBinaryName, "--api-endpoint", apiEndpointB, "sync") + clitest.MustWaitDnoteCmd(t, dnoteCmdOpts, clitest.UserConfirmEmptyServerSync, cliBinaryName, "sync", "--apiEndpoint", apiEndpointB) // Verify Server B now has data checkStateWithDB(t, ctx, userB, serverDbB, systemState{ @@ -4296,7 +4296,7 @@ func TestSync_EmptyServer(t *testing.T) { cliDatabase.MustExec(t, "updating session_key_expiry back to A", ctx.DB, "UPDATE system SET value = ? WHERE key = ?", sessionA.ExpiresAt.Unix(), consts.SystemSessionKeyExpiry) // Should NOT trigger empty server detection (Server A has MaxUSN > 0) - clitest.RunDnoteCmd(t, dnoteCmdOpts, cliBinaryName, "--api-endpoint", apiEndpointA, "sync") + clitest.RunDnoteCmd(t, dnoteCmdOpts, cliBinaryName, "sync", "--apiEndpoint", apiEndpointA) // Verify Server A still has its data checkStateWithDB(t, ctx, userA, serverDbA, systemState{ @@ -4314,7 +4314,7 @@ func TestSync_EmptyServer(t *testing.T) { cliDatabase.MustExec(t, "updating session_key_expiry back to B", ctx.DB, "UPDATE system SET value = ? WHERE key = ?", sessionB.ExpiresAt.Unix(), consts.SystemSessionKeyExpiry) // Should NOT trigger empty server detection (Server B now has MaxUSN > 0 from Step 2) - clitest.RunDnoteCmd(t, dnoteCmdOpts, cliBinaryName, "--api-endpoint", apiEndpointB, "sync") + clitest.RunDnoteCmd(t, dnoteCmdOpts, cliBinaryName, "sync", "--apiEndpoint", apiEndpointB) // Verify both servers maintain independent state checkStateWithDB(t, ctx, userB, serverDbB, systemState{