From 85f94023150c3e553872313edaf70c0fd37cb131 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Tue, 17 Mar 2026 16:41:25 -0500 Subject: [PATCH 1/3] refactor(phase1): remove legacy type definitions and unused imports Remove type definitions that are only used by deprecated create-database, create-user, and update-user commands. These include DatabaseParams, SingleDBParams, GrantDBParams, CreateUserParams, CreateUserRouteParams, CreateSingleDBResult, CreateGrantResult, and CreateDatabaseResult. Also remove unused imports (Either, IO, Array, Option) after deletion of their dependent code. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- database.go | 227 ---------------------------------------------------- logger.go | 53 ------------ main.go | 112 -------------------------- types.go | 48 ----------- user.go | 198 --------------------------------------------- 5 files changed, 638 deletions(-) diff --git a/database.go b/database.go index 43e07e7..3396c12 100644 --- a/database.go +++ b/database.go @@ -4,41 +4,14 @@ import ( "context" "fmt" - A "github.com/IBM/fp-go/v2/array" fperrors "github.com/IBM/fp-go/v2/errors" F "github.com/IBM/fp-go/v2/function" IOE "github.com/IBM/fp-go/v2/ioeither" - O "github.com/IBM/fp-go/v2/option" P "github.com/IBM/fp-go/v2/pair" driver "github.com/arangodb/go-driver" "github.com/urfave/cli/v3" ) -// CreateDatabase creates one or more databases with optional user and grants -func CreateDatabase(_ context.Context, cmd *cli.Command) error { - logger := newLogger(cmd) - return F.Pipe6( - cmd, - connParamsFromCmd, - createArangoClient, - IOE.Map[error](func(client driver.Client) DatabaseParams { - return DatabaseParams{ - WithClient: WithClient{ - Client: client, - Logger: logger, - }, - Username: cmd.String("user"), - Password: cmd.String("password"), - Grant: cmd.String("grant"), - Databases: cmd.StringSlice("database"), - } - }), - IOE.Chain(createDatabasePipeline), - IOE.ChainFirstIOK[error](logCreateDatabase(logger)), - foldIOE[CreateDatabaseResult], - ) -} - // EnsureDatabase ensures a single database exists, creating it if missing. func EnsureDatabase(ctx context.Context, cmd *cli.Command) error { logger := newLogger(cmd) @@ -123,203 +96,3 @@ func handleNewDatabase( ) } -// createDatabasePipeline creates databases and optionally creates a user with grants. -func createDatabasePipeline( - p DatabaseParams, -) IOE.IOEither[error, CreateDatabaseResult] { - return F.Pipe2( - F.Pipe1( - p.Databases, - A.Map(func(dbname string) SingleDBParams { - return SingleDBParams{ - Client: p.Client, - Logger: p.Logger, - Dbname: dbname, - } - }), - ), - IOE.TraverseArraySeq(createSingleDatabase), - IOE.Chain(maybeCreateUserAndGrant(p)), - ) -} - -// createSingleDatabase creates a single database if needed and returns creation status. -func createSingleDatabase(p SingleDBParams) IOE.IOEither[error, CreateSingleDBResult] { - return F.Pipe2( - IOE.TryCatchError(func() (bool, error) { - return p.Client.DatabaseExists(context.Background(), p.Dbname) - }), - IOE.MapLeft[bool](fperrors.OnError( - fmt.Sprintf("error checking for database %s", p.Dbname), - )), - IOE.Chain(F.Ternary( - F.Identity[bool], - F.Constant1[bool]( - IOE.Of[error](P.MakePair(false, p.Dbname)), - ), - F.Constant1[bool](createDatabase(p)), - )), - ) -} - -// createDatabase creates a single database and returns creation status. -func createDatabase(p SingleDBParams) IOE.IOEither[error, CreateSingleDBResult] { - return F.Pipe1( - IOE.TryCatchError(func() (CreateSingleDBResult, error) { - _, err := p.Client.CreateDatabase( - context.Background(), - p.Dbname, - nil, - ) - return P.MakePair(true, p.Dbname), err - }), - IOE.MapLeft[CreateSingleDBResult](fperrors.OnError( - fmt.Sprintf("error creating database %s", p.Dbname), - )), - ) -} - -// maybeCreateUserAndGrant returns a Kleisli arrow that creates a user and -// grants access if Username is non-empty, otherwise returns database outcomes. -func maybeCreateUserAndGrant( - p DatabaseParams, -) func([]CreateSingleDBResult) IOE.IOEither[error, CreateDatabaseResult] { - return func( - dbResults []CreateSingleDBResult, - ) IOE.IOEither[error, CreateDatabaseResult] { - return F.Pipe2( - p.Username, - O.FromPredicate(func(s string) bool { return len(s) > 0 }), - O.Fold( - func() IOE.IOEither[error, CreateDatabaseResult] { - return IOE.Of[error](CreateDatabaseResult{ - Databases: dbResults, - HasUser: false, - }) - }, - func(_ string) IOE.IOEither[error, CreateDatabaseResult] { - return F.Pipe1( - createUserAndGrant(p), - IOE.Map[error](func( - result P.Pair[CreateUserResult, []CreateGrantResult], - ) CreateDatabaseResult { - userResult := P.First(result) - return CreateDatabaseResult{ - Databases: dbResults, - HasUser: true, - User: userResult, - Grants: P.Second(result), - } - }), - ) - }, - ), - ) - } -} - -// createUserAndGrant creates/gets a user and grants access to all databases. -func createUserAndGrant( - p DatabaseParams, -) IOE.IOEither[error, P.Pair[CreateUserResult, []CreateGrantResult]] { - return F.Pipe3( - IOE.TryCatchError(func() (bool, error) { - return p.Client.UserExists(context.Background(), p.Username) - }), - IOE.MapLeft[bool](fperrors.OnError( - fmt.Sprintf("error checking for user %s", p.Username), - )), - IOE.Chain(F.Ternary( - F.Identity[bool], - func(_ bool) IOE.IOEither[error, CreateUserResult] { - return F.Pipe2( - IOE.TryCatchError(func() (driver.User, error) { - return p.Client.User(context.Background(), p.Username) - }), - IOE.MapLeft[driver.User](fperrors.OnError( - fmt.Sprintf("error fetching user %s", p.Username), - )), - IOE.Map[error](withCreateStatus(false)), - ) - }, - func(_ bool) IOE.IOEither[error, CreateUserResult] { - return F.Pipe2( - IOE.TryCatchError(func() (driver.User, error) { - return p.Client.CreateUser( - context.Background(), - p.Username, - &driver.UserOptions{Password: p.Password}, - ) - }), - IOE.MapLeft[driver.User](fperrors.OnError( - fmt.Sprintf("error creating user %s", p.Username), - )), - IOE.Map[error](withCreateStatus(true)), - ) - }, - )), - IOE.Chain(func( - userResult CreateUserResult, - ) IOE.IOEither[error, P.Pair[CreateUserResult, []CreateGrantResult]] { - return F.Pipe2( - F.Pipe1( - p.Databases, - A.Map(func(dbname string) GrantDBParams { - return GrantDBParams{ - Client: p.Client, - Logger: p.Logger, - Dbname: dbname, - Grant: p.Grant, - User: P.Second(userResult), - } - }), - ), - IOE.TraverseArraySeq(grantSingleDatabase), - IOE.Map[error](func( - grants []CreateGrantResult, - ) P.Pair[CreateUserResult, []CreateGrantResult] { - return P.MakePair(userResult, grants) - }), - ) - }), - ) -} - -// grantSingleDatabase grants access to a single database for a user -func grantSingleDatabase(g GrantDBParams) IOE.IOEither[error, CreateGrantResult] { - return F.Pipe2( - IOE.TryCatchError(func() (driver.Database, error) { - return g.Client.Database( - context.Background(), - g.Dbname, - ) - }), - IOE.MapLeft[driver.Database](fperrors.OnError( - fmt.Sprintf("error getting database %s", g.Dbname), - )), - IOE.Chain(func( - db driver.Database, - ) IOE.IOEither[error, CreateGrantResult] { - return F.Pipe1( - IOE.TryCatchError(func() (CreateGrantResult, error) { - err := g.User.SetDatabaseAccess( - context.Background(), - db, - getGrant(g.Grant), - ) - if err != nil { - var zero CreateGrantResult - return zero, err - } - return P.MakePair(g.Dbname, g.Grant), nil - }), - IOE.MapLeft[CreateGrantResult](fperrors.OnError( - fmt.Sprintf( - "error granting access to database %s", - g.Dbname, - ), - )), - ) - }), - ) -} diff --git a/logger.go b/logger.go index 9d714ad..4b7badc 100644 --- a/logger.go +++ b/logger.go @@ -41,34 +41,6 @@ func parseLogLevel(levelStr string) slog.Level { } } -func logCreateUserOutcome(logger *slog.Logger, result CreateUserResult) { - logger.Info( - "user status", - "username", - F.Pipe2(result, P.Second, userName), - "status", - F.Pipe2(result, P.First, statusFromCreated), - ) -} - -func logCreateDatabaseOutcome(logger *slog.Logger, result CreateDatabaseResult) { - for _, dbResult := range result.Databases { - logger.Info( - "database status", - "database", - P.Second(dbResult), - "status", - F.Pipe2(dbResult, P.First, statusFromCreated), - ) - } - - if !result.HasUser { - return - } - - logCreateUserOutcome(logger, result.User) -} - func logEnsureUserOutcome(logger *slog.Logger, result EnsureUserResult) { logger.Info( "user status", @@ -112,31 +84,6 @@ func statusFromCreated(created bool) string { func userName(u driver.User) string { return u.Name() } -func logUserUpdated(logger *slog.Logger, username string) IO.IO[F.Void] { - return func() F.Void { - logger.Info("user status", "username", username, "status", "updated") - return F.VOID - } -} - -func logCreateUser(logger *slog.Logger) func(CreateUserResult) IO.IO[F.Void] { - return func(result CreateUserResult) IO.IO[F.Void] { - return func() F.Void { - logCreateUserOutcome(logger, result) - return F.VOID - } - } -} - -func logCreateDatabase(logger *slog.Logger) func(CreateDatabaseResult) IO.IO[F.Void] { - return func(result CreateDatabaseResult) IO.IO[F.Void] { - return func() F.Void { - logCreateDatabaseOutcome(logger, result) - return F.VOID - } - } -} - func logEnsureUser(logger *slog.Logger) func(EnsureUserResult) IO.IO[F.Void] { return func(result EnsureUserResult) IO.IO[F.Void] { return func() F.Void { diff --git a/main.go b/main.go index b3417e1..b5ad1c9 100644 --- a/main.go +++ b/main.go @@ -15,9 +15,6 @@ func main() { Version: "1.0.0", Flags: globalFlags(), Commands: []*cli.Command{ - createDatabaseCommand(), - createUserCommand(), - updateUserCommand(), ensureUserCommand(), ensureDatabaseCommand(), ensureGrantCommand(), @@ -60,115 +57,6 @@ func globalFlags() []cli.Flag { } } -func createDatabaseCommand() *cli.Command { - return &cli.Command{ - Name: "create-database", - Usage: "create a new arangodb database", - Action: CreateDatabase, - Flags: []cli.Flag{ - &cli.StringFlag{ - Name: "admin-user", - Aliases: []string{"au"}, - Usage: "arangodb admin user", - Value: "root", - }, - &cli.StringFlag{ - Name: "admin-password", - Aliases: []string{"ap"}, - Usage: "arangodb admin password", - }, - &cli.StringSliceFlag{ - Name: "database", - Aliases: []string{"db"}, - Usage: "name of arangodb database", - Required: true, - }, - &cli.StringFlag{ - Name: "user", - Aliases: []string{"u"}, - Usage: "arangodb user", - }, - &cli.StringFlag{ - Name: "password", - Aliases: []string{"pw"}, - Usage: "arangodb password for new user", - }, - &cli.StringFlag{ - Name: "grant", - Aliases: []string{"g"}, - Usage: "level of access for arangodb user", - Value: "rw", - }, - }, - } -} - -func createUserCommand() *cli.Command { - return &cli.Command{ - Name: "create-user", - Usage: "create a new user for accessing arangodb", - Action: CreateUser, - Flags: []cli.Flag{ - &cli.StringFlag{ - Name: "admin-user", - Aliases: []string{"au"}, - Usage: "arangodb admin user", - Value: "root", - }, - &cli.StringFlag{ - Name: "admin-password", - Aliases: []string{"ap"}, - Usage: "arangodb admin password", - }, - &cli.StringFlag{ - Name: "user", - Aliases: []string{"u"}, - Usage: "arangodb user", - Required: true, - }, - &cli.StringFlag{ - Name: "password", - Aliases: []string{"pw"}, - Usage: "arangodb password for new user", - Required: true, - }, - }, - } -} - -func updateUserCommand() *cli.Command { - return &cli.Command{ - Name: "update-user", - Usage: "update an existing user's password for accessing arangodb", - Action: UpdateUser, - Flags: []cli.Flag{ - &cli.StringFlag{ - Name: "admin-user", - Aliases: []string{"au"}, - Usage: "arangodb admin user", - Value: "root", - }, - &cli.StringFlag{ - Name: "admin-password", - Aliases: []string{"ap"}, - Usage: "arangodb admin password", - }, - &cli.StringFlag{ - Name: "user", - Aliases: []string{"u"}, - Usage: "arangodb user", - Required: true, - }, - &cli.StringFlag{ - Name: "password", - Aliases: []string{"pw"}, - Usage: "new arangodb password for the user", - Required: true, - }, - }, - } -} - func ensureUserCommand() *cli.Command { return &cli.Command{ Name: "ensure-user", diff --git a/types.go b/types.go index f12eb5b..7d5f744 100644 --- a/types.go +++ b/types.go @@ -34,18 +34,9 @@ type UserParams struct { Username, Password string } -// CreateUserParams is a narrower input for the create-user pipeline. -type CreateUserParams struct { - Client driver.Client - Username, Password string -} - // CreateUserResult carries whether the user was newly created and the user itself. type CreateUserResult = P.Pair[bool, driver.User] -// CreateUserRouteParams carries user existence state alongside create-user params. -type CreateUserRouteParams = P.Pair[bool, CreateUserParams] - // EnsureUserParams carries inputs for the ensure-user command. type EnsureUserParams struct { Context context.Context @@ -67,45 +58,6 @@ const ( // EnsureUserResult carries the final outcome status and the driver.User. type EnsureUserResult = P.Pair[EnsureUserStatus, driver.User] -// DatabaseParams contains create-database command inputs and dependencies. -type DatabaseParams struct { - WithClient - Databases []string - Username, Password string // optional user creation - Grant string -} - -// SingleDBParams carries what's needed to create one database -type SingleDBParams struct { - Client driver.Client - Logger *slog.Logger - Dbname string -} - -// GrantDBParams carries what's needed to grant a user access to one database -type GrantDBParams struct { - Client driver.Client - Logger *slog.Logger - Dbname string - Grant string - User driver.User -} - -// CreateSingleDBResult carries whether a database was newly created and its name. -type CreateSingleDBResult = P.Pair[bool, string] - -// CreateGrantResult carries database name and applied grant. -type CreateGrantResult = P.Pair[string, string] - -// CreateDatabaseResult carries outcomes for database creation, optional user creation, -// and grants applied during create-database command execution. -type CreateDatabaseResult struct { - Databases []CreateSingleDBResult - HasUser bool - User CreateUserResult - Grants []CreateGrantResult -} - // EnsureDatabaseParams carries inputs for the ensure-database command. type EnsureDatabaseParams struct { Context context.Context diff --git a/user.go b/user.go index fe9582c..2ee40d2 100644 --- a/user.go +++ b/user.go @@ -4,15 +4,12 @@ import ( "context" "fmt" - E "github.com/IBM/fp-go/v2/either" fperrors "github.com/IBM/fp-go/v2/errors" F "github.com/IBM/fp-go/v2/function" - IO "github.com/IBM/fp-go/v2/io" IOE "github.com/IBM/fp-go/v2/ioeither" P "github.com/IBM/fp-go/v2/pair" PR "github.com/IBM/fp-go/v2/predicate" driver "github.com/arangodb/go-driver" - "github.com/urfave/cli/v3" ) type UserForUpdate struct { @@ -20,201 +17,6 @@ type UserForUpdate struct { User driver.User } -// CreateUser adds a new user with pre-specified privileges to ArangoDB -func CreateUser(_ context.Context, cmd *cli.Command) error { - logger := newLogger(cmd) - return F.Pipe6( - cmd, - connParamsFromCmd, - createArangoClient, - IOE.Map[error](func(client driver.Client) CreateUserParams { - return CreateUserParams{ - Client: client, - Username: cmd.String("user"), - Password: cmd.String("password"), - } - }), - IOE.Chain(createUserPipeline), - IOE.ChainFirstIOK[error](logCreateUser(logger)), - foldIOE[CreateUserResult], - ) -} - -// createUserPipeline creates a user if they don't exist or returns the existing user. -func createUserPipeline( - p CreateUserParams, -) IOE.IOEither[error, CreateUserResult] { - return F.Pipe3( - p, - checkUserExistence, - IOE.Map[error](func(exists bool) CreateUserRouteParams { - return P.MakePair(exists, p) - }), - IOE.Chain(routeUserCreation), - ) -} - -// checkUserExistence checks whether the user exists in ArangoDB -func checkUserExistence(p CreateUserParams) IOE.IOEither[error, bool] { - return F.Pipe1( - IOE.TryCatchError(func() (bool, error) { - return p.Client.UserExists(context.Background(), p.Username) - }), - IOE.MapLeft[bool](fperrors.OnError( - fmt.Sprintf("error checking for user %s", p.Username), - )), - ) -} - -// routeUserCreation routes to either fetching an existing user or creating a new one. -func routeUserCreation( - params CreateUserRouteParams, -) IOE.IOEither[error, CreateUserResult] { - return F.Pipe1( - params, - F.Ternary( - P.First[bool, CreateUserParams], - handleExistingUser, - handleNewUser, - ), - ) -} - -func handleExistingUser( - params CreateUserRouteParams, -) IOE.IOEither[error, CreateUserResult] { - return F.Pipe2( - P.Second(params), - fetchExistingUser, - IOE.Map[error](withCreateStatus(false)), - ) -} - -func handleNewUser( - params CreateUserRouteParams, -) IOE.IOEither[error, CreateUserResult] { - return F.Pipe2( - P.Second(params), - createNewUser, - IOE.Map[error](withCreateStatus(true)), - ) -} - -func fetchExistingUser(p CreateUserParams) IOE.IOEither[error, driver.User] { - return F.Pipe1( - IOE.TryCatchError(func() (driver.User, error) { - return p.Client.User(context.Background(), p.Username) - }), - IOE.MapLeft[driver.User](fperrors.OnError( - fmt.Sprintf("error fetching user %s", p.Username), - )), - ) -} - -func withCreateStatus(created bool) func(driver.User) CreateUserResult { - return func(user driver.User) CreateUserResult { - return P.MakePair(created, user) - } -} - -// createNewUser creates a new user in ArangoDB. -func createNewUser(p CreateUserParams) IOE.IOEither[error, driver.User] { - return F.Pipe1( - IOE.TryCatchError(func() (driver.User, error) { - return p.Client.CreateUser( - context.Background(), - p.Username, - &driver.UserOptions{Password: p.Password}, - ) - }), - IOE.MapLeft[driver.User](fperrors.OnError( - fmt.Sprintf("error creating user %s", p.Username), - )), - ) -} - -// UpdateUser updates the password of an existing user in ArangoDB -func UpdateUser(_ context.Context, cmd *cli.Command) error { - return F.Pipe5( - cmd, - connParamsFromCmd, - createArangoClient, - IOE.Map[error](func(client driver.Client) UserParams { - return UserParams{ - WithClient: WithClient{ - Client: client, - Logger: newLogger(cmd), - }, - Username: cmd.String("user"), - Password: cmd.String("password"), - } - }), - IOE.Chain(updateUserPipeline), - foldIOE[string], - ) -} - -// updateUserPipeline updates a user's password if they exist -func updateUserPipeline(p UserParams) IOE.IOEither[error, string] { - return F.Pipe3( - p, - checkUserExists, - IOE.Chain(getExistingUser), - IOE.Chain(updateExistingUser), - ) -} - -func checkUserExists(p UserParams) IOE.IOEither[error, UserParams] { - return F.Pipe3( - IOE.TryCatchError(func() (bool, error) { - return p.Client.UserExists(context.Background(), p.Username) - }), - IOE.MapLeft[bool](fperrors.OnError( - fmt.Sprintf("error checking for user %s", p.Username), - )), - IOE.ChainEitherK(E.FromPredicate( - F.Identity[bool], - func(_ bool) error { return fmt.Errorf("user %s does not exist", p.Username) }, - )), - IOE.Map[error](F.Constant1[bool](p)), - ) -} - -func getExistingUser(p UserParams) IOE.IOEither[error, UserForUpdate] { - return F.Pipe2( - IOE.TryCatchError(func() (driver.User, error) { - return p.Client.User(context.Background(), p.Username) - }), - IOE.MapLeft[driver.User]( - fperrors.OnError(fmt.Sprintf("error fetching user %s", p.Username)), - ), - IOE.Map[error](withExistingUser(p)), - ) -} - -func withExistingUser(p UserParams) func(driver.User) UserForUpdate { - return func(user driver.User) UserForUpdate { - return UserForUpdate{Params: p, User: user} - } -} - -func updateExistingUser(u UserForUpdate) IOE.IOEither[error, string] { - return F.Pipe2( - IOE.TryCatchError(func() (string, error) { - return u.Params.Username, u.User.Update( - context.Background(), - driver.UserOptions{Password: u.Params.Password}, - ) - }), - IOE.MapLeft[string](fperrors.OnError( - fmt.Sprintf("error updating user %s", u.Params.Username), - )), - IOE.ChainFirstIOK[error](func(username string) IO.IO[F.Void] { - return logUserUpdated(u.Params.Logger, username) - }), - ) -} - type EnsureExistingUserPolicyInput struct { Params EnsureUserParams User driver.User From a36e3821c8593c9c96af599c3718b2746e223699 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Tue, 17 Mar 2026 16:43:17 -0500 Subject: [PATCH 2/3] refactor(phase4): remove legacy test functions Delete test functions for deprecated create-user, update-user, and create-database commands from user_test.go, database_test.go, and logger_test.go. These tests are no longer relevant as the legacy handlers and functions have been removed. Removed tests: - user_test.go: TestCreateUser, TestUpdateUser, TestCreateUserPipelineNewUser, TestCreateUserPipelineIdempotent, TestUpdateUserPipelineSuccess, TestUpdateUserPipelineNonExistent - database_test.go: TestCreateDatabase, TestCreateDatabaseWithUser, TestCreateSingleDatabaseNew, TestCreateSingleDatabaseIdempotent, TestGrantSingleDatabase, TestGrantSingleDatabaseFailure, TestCreateDatabasePipelineWithUser, TestCreateUserAndGrantErrors - logger_test.go: TestLogCreateUserOutcomeIncludesStatus, TestLogUserUpdatedIncludesStatus, TestLogCreateDatabaseOutcomeIncludesStatuses Co-Authored-By: Claude Sonnet 4.6 (1M context) --- database_test.go | 289 ----------------------------------------------- logger_test.go | 73 ------------ user_test.go | 212 ---------------------------------- 3 files changed, 574 deletions(-) diff --git a/database_test.go b/database_test.go index e8cc8ee..809bbf4 100644 --- a/database_test.go +++ b/database_test.go @@ -14,258 +14,6 @@ import ( "github.com/urfave/cli/v3" ) -func TestCreateDatabase(t *testing.T) { - assert := assert.New(t) - ctx := context.Background() - - cmd := &cli.Command{ - Flags: globalFlags(), - Commands: []*cli.Command{ - createDatabaseCommand(), - }, - } - - dbName := "testdb" - args := []string{ - "arangoadmin", - "--host", arangoHost, - "--port", arangoPort, - "create-database", - "--admin-password", arangoPassword, - "--database", dbName, - } - - err := cmd.Run(ctx, args) - assert.NoError(err) - - // Verify database exists - client, err := getTestClient() - assert.NoError(err) - - ok, err := client.DatabaseExists(ctx, dbName) - assert.NoError(err) - assert.True(ok, "database should exist") - - // Test multiple databases - dbNames := []string{"testdb2", "testdb3"} - args = []string{ - "arangoadmin", - "--host", arangoHost, - "--port", arangoPort, - "create-database", - "--admin-password", arangoPassword, - "--database", dbNames[0], - "--database", dbNames[1], - } - err = cmd.Run(ctx, args) - assert.NoError(err) - - for _, name := range dbNames { - ok, err := client.DatabaseExists(ctx, name) - assert.NoError(err) - assert.True(ok, "database %s should exist", name) - } -} - -func TestCreateDatabaseWithUser(t *testing.T) { - assert := assert.New(t) - ctx := context.Background() - - cmd := &cli.Command{ - Flags: globalFlags(), - Commands: []*cli.Command{ - createDatabaseCommand(), - }, - } - - dbName := "testdb_user" - user := "testuser" - pass := "testpass" - args := []string{ - "arangoadmin", - "--host", arangoHost, - "--port", arangoPort, - "create-database", - "--admin-password", arangoPassword, - "--database", dbName, - "--user", user, - "--password", pass, - } - - err := cmd.Run(ctx, args) - assert.NoError(err) - - client, err := getTestClient() - assert.NoError(err) - - // Verify user exists - ok, err := client.UserExists(ctx, user) - assert.NoError(err) - assert.True(ok, "user should exist") - - // Verify database exists - ok, err = client.DatabaseExists(ctx, dbName) - assert.NoError(err) - assert.True(ok, "database should exist") -} - -// Unit tests for database pipelines -func TestCreateSingleDatabaseNew(t *testing.T) { - require := require.New(t) - ctx := context.Background() - - client, err := getClient(&ClientParams{ - Host: arangoHost, - Port: arangoPort, - User: "root", - Pass: arangoPassword, - IsSecure: false, - }) - require.NoError(err) - - logger := slog.New(slog.NewTextHandler(os.Stderr, nil)) - p := SingleDBParams{ - Client: client, - Logger: logger, - Dbname: "fptest_newdb", - } - result := toEither(createSingleDatabase(p)) - require.True(E.IsRight(result), "createSingleDatabase should succeed for new database") - - ok, err := client.DatabaseExists(ctx, "fptest_newdb") - require.NoError(err) - require.True(ok, "database should exist after creation") -} - -func TestCreateSingleDatabaseIdempotent(t *testing.T) { - require := require.New(t) - - client, err := getClient(&ClientParams{ - Host: arangoHost, - Port: arangoPort, - User: "root", - Pass: arangoPassword, - IsSecure: false, - }) - require.NoError(err) - - logger := slog.New(slog.NewTextHandler(os.Stderr, nil)) - p := SingleDBParams{ - Client: client, - Logger: logger, - Dbname: "fptest_idempotentdb", - } - r1 := toEither(createSingleDatabase(p)) - require.True(E.IsRight(r1)) - r2 := toEither(createSingleDatabase(p)) - require.True(E.IsRight(r2), "creating same database twice should succeed (idempotent)") -} - -func TestGrantSingleDatabase(t *testing.T) { - require := require.New(t) - ctx := context.Background() - - client, err := getClient(&ClientParams{ - Host: arangoHost, - Port: arangoPort, - User: "root", - Pass: arangoPassword, - IsSecure: false, - }) - require.NoError(err) - - // Pre-create database and user - dbName := "fptest_grantdb" - userName := "fptest_grantusr" - _, _ = client.CreateDatabase(ctx, dbName, nil) - // If it already exists, that's fine for this test - user, err := client.CreateUser(ctx, userName, &driver.UserOptions{Password: "p"}) - if err != nil { - user, err = client.User(ctx, userName) - require.NoError(err) - } - - logger := slog.New(slog.NewTextHandler(os.Stderr, nil)) - g := GrantDBParams{ - Client: client, - Logger: logger, - Dbname: dbName, - Grant: "rw", - User: user, - } - result := toEither(grantSingleDatabase(g)) - require.True(E.IsRight(result), "grantSingleDatabase should succeed") -} - -func TestGrantSingleDatabaseFailure(t *testing.T) { - require := require.New(t) - ctx := context.Background() - - client, err := getClient(&ClientParams{ - Host: arangoHost, - Port: arangoPort, - User: "root", - Pass: arangoPassword, - IsSecure: false, - }) - require.NoError(err) - - // Pre-create database - dbName := "fptest_grantdb_fail" - _, _ = client.CreateDatabase(ctx, dbName, nil) - // Ignore error if it already exists - - logger := slog.New(slog.NewTextHandler(os.Stderr, nil)) - g := GrantDBParams{ - Client: client, - Logger: logger, - Dbname: dbName, - Grant: "rw", - User: errorUser{}, // This will fail SetDatabaseAccess - } - result := toEither(grantSingleDatabase(g)) - require.True(E.IsLeft(result), "grantSingleDatabase should fail when User.SetDatabaseAccess fails") - _, err = E.UnwrapError(result) - require.Contains(err.Error(), "error granting access to database") -} - -func TestCreateDatabasePipelineWithUser(t *testing.T) { - require := require.New(t) - ctx := context.Background() - - client, err := getClient(&ClientParams{ - Host: arangoHost, - Port: arangoPort, - User: "root", - Pass: arangoPassword, - IsSecure: false, - }) - require.NoError(err) - - databases := []string{"fptest_grantdb1", "fptest_grantdb2"} - logger := slog.New(slog.NewTextHandler(os.Stderr, nil)) - p := DatabaseParams{ - WithClient: WithClient{Client: client, Logger: logger}, - Databases: databases, - Username: "fptest_grantuser", - Password: "pass", - Grant: "rw", - } - result := toEither(createDatabasePipeline(p)) - require.True(E.IsRight(result), "database pipeline should succeed") - - // Verify databases exist - for _, db := range databases { - ok, nerr := client.DatabaseExists(ctx, db) - require.NoError(nerr) - require.True(ok, "database %s should exist", db) - } - // Verify user exists - ok, nerr := client.UserExists(ctx, p.Username) - require.NoError(nerr) - require.True(ok, "user should exist") -} - func TestEnsureDatabase(t *testing.T) { assert := assert.New(t) ctx := context.Background() @@ -353,40 +101,3 @@ func (m *mockClient) Database(_ context.Context, _ string) (driver.Database, err return nil, nil // Not used for this test } -func TestCreateUserAndGrantErrors(t *testing.T) { - assert := assert.New(t) - require := require.New(t) - - // 1. UserExists fails - m1 := &mockClient{userExistsError: fmt.Errorf("user exists fail")} - p1 := DatabaseParams{ - WithClient: WithClient{Client: m1}, - Username: "test", - } - res1 := toEither(createUserAndGrant(p1)) - assert.True(E.IsLeft(res1)) - _, err1 := E.UnwrapError(res1) - require.Contains(err1.Error(), "user exists fail") - - // 2. User exists but fetching it fails - m2 := &mockClient{userExists: true, userError: fmt.Errorf("user fetch fail")} - p2 := DatabaseParams{ - WithClient: WithClient{Client: m2}, - Username: "test", - } - res2 := toEither(createUserAndGrant(p2)) - assert.True(E.IsLeft(res2)) - _, err2 := E.UnwrapError(res2) - require.Contains(err2.Error(), "user fetch fail") - - // 3. User does not exist and creation fails - m3 := &mockClient{userExists: false, createUserError: fmt.Errorf("user create fail")} - p3 := DatabaseParams{ - WithClient: WithClient{Client: m3}, - Username: "test", - } - res3 := toEither(createUserAndGrant(p3)) - assert.True(E.IsLeft(res3)) - _, err3 := E.UnwrapError(res3) - require.Contains(err3.Error(), "user create fail") -} diff --git a/logger_test.go b/logger_test.go index f20f903..3c4c626 100644 --- a/logger_test.go +++ b/logger_test.go @@ -84,79 +84,6 @@ func (u testUser) GrantReadWriteAccess(_ context.Context, _ driver.Database) err func (u testUser) RevokeAccess(_ context.Context, _ driver.Database) error { return nil } -func TestLogCreateUserOutcomeIncludesStatus(t *testing.T) { - require := require.New(t) - var buf bytes.Buffer - logger := slog.New(slog.NewTextHandler(&buf, nil)) - - logCreateUserOutcome(logger, P.MakePair[bool, driver.User](true, testUser{ - name: "created-user", - active: true, - })) - output := buf.String() - require.Contains(output, "msg=\"user status\"") - require.Contains(output, "username=created-user") - require.Contains(output, "status=created") - - buf.Reset() - - logCreateUserOutcome(logger, P.MakePair[bool, driver.User](false, testUser{ - name: "existing-user", - active: true, - })) - output = buf.String() - require.Contains(output, "msg=\"user status\"") - require.Contains(output, "username=existing-user") - require.Contains(output, "status=existing") -} - -func TestLogUserUpdatedIncludesStatus(t *testing.T) { - require := require.New(t) - var buf bytes.Buffer - logger := slog.New(slog.NewTextHandler(&buf, nil)) - - logUserUpdated(logger, "updated-user")() - - output := buf.String() - require.Contains(output, "msg=\"user status\"") - require.Contains(output, "username=updated-user") - require.Contains(output, "status=updated") -} - -func TestLogCreateDatabaseOutcomeIncludesStatuses(t *testing.T) { - require := require.New(t) - var buf bytes.Buffer - logger := slog.New(slog.NewTextHandler(&buf, nil)) - - userResult := P.MakePair[bool, driver.User](true, testUser{ - name: "db-user", - active: true, - }) - result := CreateDatabaseResult{ - Databases: []CreateSingleDBResult{ - P.MakePair(true, "db_created"), - P.MakePair(false, "db_existing"), - }, - HasUser: true, - User: userResult, - Grants: []CreateGrantResult{ - P.MakePair("db_created", "rw"), - P.MakePair("db_existing", "rw"), - }, - } - - logCreateDatabaseOutcome(logger, result) - output := buf.String() - - require.Contains(output, "msg=\"database status\"") - require.Contains(output, "database=db_created") - require.Contains(output, "status=created") - require.Contains(output, "database=db_existing") - require.Contains(output, "status=existing") - require.Contains(output, "msg=\"user status\"") - require.Contains(output, "username=db-user") -} - func TestLogEnsureUserOutcome(t *testing.T) { require := require.New(t) var buf bytes.Buffer diff --git a/user_test.go b/user_test.go index 860e078..1d4e08a 100644 --- a/user_test.go +++ b/user_test.go @@ -14,218 +14,6 @@ import ( "github.com/urfave/cli/v3" ) -func TestCreateUser(t *testing.T) { - assert := assert.New(t) - ctx := context.Background() - - cmd := &cli.Command{ - Flags: globalFlags(), - Commands: []*cli.Command{ - createUserCommand(), - }, - } - - user := "newuser" - pass := "newpass" - args := []string{ - "arangoadmin", - "--host", arangoHost, - "--port", arangoPort, - "create-user", - "--admin-password", arangoPassword, - "--user", user, - "--password", pass, - } - - err := cmd.Run(ctx, args) - assert.NoError(err) - - client, err := getTestClient() - assert.NoError(err) - - ok, err := client.UserExists(ctx, user) - assert.NoError(err) - assert.True(ok, "user should exist") -} - -func TestUpdateUser(t *testing.T) { - assert := assert.New(t) - ctx := context.Background() - - cmd := &cli.Command{ - Flags: globalFlags(), - Commands: []*cli.Command{ - createDatabaseCommand(), - updateUserCommand(), - }, - } - - user := "updateuser" - pass := "initialpass" - dbName := "updatetestdb" - // Create user with database first to ensure it has some permissions - args := []string{ - "arangoadmin", - "--host", arangoHost, - "--port", arangoPort, - "create-database", - "--admin-password", arangoPassword, - "--database", dbName, - "--user", user, - "--password", pass, - } - err := cmd.Run(ctx, args) - assert.NoError(err) - - // Update password - newPass := "updatedpass" - args = []string{ - "arangoadmin", - "--host", arangoHost, - "--port", arangoPort, - "update-user", - "--admin-password", arangoPassword, - "--user", user, - "--password", newPass, - } - err = cmd.Run(ctx, args) - assert.NoError(err) - - // Verify we can connect with new password - client, err := getClient(&ClientParams{ - Host: arangoHost, - Port: arangoPort, - User: user, - Pass: newPass, - IsSecure: false, - }) - assert.NoError(err) - // Try to get the database we have access to - _, err = client.Database(ctx, dbName) - assert.NoError(err, "should be able to access database with new password") - - // Verify we cannot connect with old password - clientOld, err := getClient(&ClientParams{ - Host: arangoHost, - Port: arangoPort, - User: user, - Pass: pass, - IsSecure: false, - }) - assert.NoError(err) - _, err = clientOld.Database(ctx, dbName) - assert.Error(err, "should not be able to access database with old password") -} - -// Unit tests for createUserPipeline -func TestCreateUserPipelineNewUser(t *testing.T) { - require := require.New(t) - ctx := context.Background() - - client, err := getClient(&ClientParams{ - Host: arangoHost, - Port: arangoPort, - User: "root", - Pass: arangoPassword, - IsSecure: false, - }) - require.NoError(err) - - p := CreateUserParams{ - Client: client, - Username: "fptest_newuser", - Password: "testpass", - } - result, err := toTuple(createUserPipeline(p)) - require.NoError(err, "createUserPipeline should succeed for new user") - require.True(P.First(result), "true should mean newly created") - require.Equal("fptest_newuser", P.Second(result).Name()) - - // Verify user was actually created - ok, err := client.UserExists(ctx, "fptest_newuser") - require.NoError(err) - require.True(ok, "user should exist after creation") -} - -func TestCreateUserPipelineIdempotent(t *testing.T) { - require := require.New(t) - - client, err := getClient(&ClientParams{ - Host: arangoHost, - Port: arangoPort, - User: "root", - Pass: arangoPassword, - IsSecure: false, - }) - require.NoError(err) - - p := CreateUserParams{ - Client: client, - Username: "fptest_idempotent", - Password: "testpass", - } - // Create once - result1, err := toTuple(createUserPipeline(p)) - require.NoError(err) - require.True(P.First(result1), "first creation should report created=true") - require.Equal("fptest_idempotent", P.Second(result1).Name()) - // Create again — should succeed (idempotent, logs "exists") - result2, err := toTuple(createUserPipeline(p)) - require.NoError(err) - require.False(P.First(result2), "second creation should report created=false") - require.Equal("fptest_idempotent", P.Second(result2).Name()) -} - -// Unit tests for updateUserPipeline -func TestUpdateUserPipelineSuccess(t *testing.T) { - require := require.New(t) - ctx := context.Background() - - client, err := getClient(&ClientParams{ - Host: arangoHost, - Port: arangoPort, - User: "root", - Pass: arangoPassword, - IsSecure: false, - }) - require.NoError(err) - - // Pre-create user - _, err = client.CreateUser(ctx, "fptest_update", &driver.UserOptions{Password: "oldpass"}) - require.NoError(err) - - logger := slog.New(slog.NewTextHandler(os.Stderr, nil)) - p := UserParams{ - WithClient: WithClient{Client: client, Logger: logger}, - Username: "fptest_update", - Password: "newpass", - } - result := toEither(updateUserPipeline(p)) - require.True(E.IsRight(result), "updateUserPipeline should succeed for existing user") -} - -func TestUpdateUserPipelineNonExistent(t *testing.T) { - require := require.New(t) - - client, err := getClient(&ClientParams{ - Host: arangoHost, - Port: arangoPort, - User: "root", - Pass: arangoPassword, - IsSecure: false, - }) - require.NoError(err) - - logger := slog.New(slog.NewTextHandler(os.Stderr, nil)) - p := UserParams{ - WithClient: WithClient{Client: client, Logger: logger}, - Username: "fptest_ghost", - Password: "pass", - } - result := toEither(updateUserPipeline(p)) - require.True(E.IsLeft(result), "updateUserPipeline should fail for non-existent user") -} - func TestEnsureUser(t *testing.T) { require := require.New(t) From 1f974a0b4fa9611aa2927f523cbab3a0709b2aad Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Tue, 17 Mar 2026 19:39:42 -0500 Subject: [PATCH 3/3] refactor(phase5): remove orphaned types, mocks, and unused imports Delete WithClient, UserParams, and UserForUpdate structs that were only used by the removed legacy commands. Remove the mockClient test helper that was only used by the deleted TestCreateUserAndGrantErrors test. Clean up unused imports across database_test.go, user_test.go, and types.go introduced by the legacy code deletions. Migrate TestConnectionFailure from create-user to ensure-user command to preserve the connection error coverage without the legacy command. Remove TestCreateUserAuthFailure which tested the deleted create-user command. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- database_test.go | 29 ----------------------------- types.go | 14 -------------- user.go | 6 +----- user_test.go | 28 ++-------------------------- 4 files changed, 3 insertions(+), 74 deletions(-) diff --git a/database_test.go b/database_test.go index 809bbf4..04a7e92 100644 --- a/database_test.go +++ b/database_test.go @@ -2,15 +2,9 @@ package main import ( "context" - "fmt" - "log/slog" - "os" "testing" - E "github.com/IBM/fp-go/v2/either" - driver "github.com/arangodb/go-driver" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/urfave/cli/v3" ) @@ -77,27 +71,4 @@ func TestEnsureDatabaseError(t *testing.T) { assert.Error(err, "should fail with invalid connection params") } -type mockClient struct { - driver.Client - userExistsError error - userError error - createUserError error - userExists bool -} - -func (m *mockClient) UserExists(_ context.Context, _ string) (bool, error) { - return m.userExists, m.userExistsError -} - -func (m *mockClient) User(_ context.Context, _ string) (driver.User, error) { - return nil, m.userError -} - -func (m *mockClient) CreateUser(_ context.Context, _ string, _ *driver.UserOptions) (driver.User, error) { - return nil, m.createUserError -} - -func (m *mockClient) Database(_ context.Context, _ string) (driver.Database, error) { - return nil, nil // Not used for this test -} diff --git a/types.go b/types.go index 7d5f744..c4de04a 100644 --- a/types.go +++ b/types.go @@ -2,7 +2,6 @@ package main import ( "context" - "log/slog" P "github.com/IBM/fp-go/v2/pair" driver "github.com/arangodb/go-driver" @@ -21,19 +20,6 @@ type WithConnection struct { Conn driver.Connection } -// WithClient enriches connection params with an ArangoDB client and logger. -type WithClient struct { - ConnectionParams - Client driver.Client - Logger *slog.Logger -} - -// UserParams contains CLI-provided user values plus client dependencies. -type UserParams struct { - WithClient - Username, Password string -} - // CreateUserResult carries whether the user was newly created and the user itself. type CreateUserResult = P.Pair[bool, driver.User] diff --git a/user.go b/user.go index 2ee40d2..130012a 100644 --- a/user.go +++ b/user.go @@ -10,13 +10,9 @@ import ( P "github.com/IBM/fp-go/v2/pair" PR "github.com/IBM/fp-go/v2/predicate" driver "github.com/arangodb/go-driver" + "github.com/urfave/cli/v3" ) -type UserForUpdate struct { - Params UserParams - User driver.User -} - type EnsureExistingUserPolicyInput struct { Params EnsureUserParams User driver.User diff --git a/user_test.go b/user_test.go index 1d4e08a..9805f08 100644 --- a/user_test.go +++ b/user_test.go @@ -2,11 +2,8 @@ package main import ( "context" - "log/slog" - "os" "testing" - E "github.com/IBM/fp-go/v2/either" P "github.com/IBM/fp-go/v2/pair" driver "github.com/arangodb/go-driver" "github.com/stretchr/testify/assert" @@ -139,27 +136,6 @@ func TestGetGrant(t *testing.T) { assert.Equal(driver.GrantNone, getGrant("invalid")) } -func TestCreateUserAuthFailure(t *testing.T) { - require := require.New(t) - cmd := &cli.Command{ - Flags: globalFlags(), - Commands: []*cli.Command{ - createUserCommand(), - }, - } - args := []string{ - "arangoadmin", - "--host", arangoHost, - "--port", arangoPort, - "create-user", - "--admin-password", "wrong-password", - "--user", "failuser", - "--password", "failpass", - } - err := cmd.Run(context.Background(), args) - require.Error(err) - require.Contains(err.Error(), "not authorized") -} func TestEnsureUserAuthFailure(t *testing.T) { require := require.New(t) @@ -188,14 +164,14 @@ func TestConnectionFailure(t *testing.T) { cmd := &cli.Command{ Flags: globalFlags(), Commands: []*cli.Command{ - createUserCommand(), + ensureUserCommand(), }, } args := []string{ "arangoadmin", "--host", "nonexistent-host", "--port", "8529", - "create-user", + "ensure-user", "--admin-password", arangoPassword, "--user", "failuser", "--password", "failpass",