From b3e3a1ffc306cced81d6fa12e46feedaa1b67678 Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Thu, 13 Jun 2024 08:21:53 +0200 Subject: [PATCH] Remove error from constructor if never error occurs --- pkg/datastore/bootstrap.go | 10 +--- pkg/datastore/migrate.go | 5 +- pkg/datastore/postgres.go | 4 +- pkg/datastore/postgres_benchmark_test.go | 2 +- pkg/datastore/postgres_test.go | 63 ++++++++++-------------- pkg/service/project.go | 19 ++----- pkg/service/projectmember.go | 19 ++----- pkg/service/tenant.go | 14 ++---- pkg/service/tenant_test.go | 22 ++++----- pkg/service/tenantmember.go | 14 ++---- server/main.go | 25 ++-------- 11 files changed, 64 insertions(+), 133 deletions(-) diff --git a/pkg/datastore/bootstrap.go b/pkg/datastore/bootstrap.go index 6c16b15..91d0c17 100644 --- a/pkg/datastore/bootstrap.go +++ b/pkg/datastore/bootstrap.go @@ -31,19 +31,13 @@ func Initdb(log *slog.Logger, db *sqlx.DB, healthServer *health.Server, dir stri return err } - ts, err := New(log, db, &v1.Tenant{}) - if err != nil { - return err - } + ts := New(log, db, &v1.Tenant{}) tbs := &bootstrap[*v1.Tenant]{ log: log, ds: ts, } - ps, err := New(log, db, &v1.Project{}) - if err != nil { - return err - } + ps := New(log, db, &v1.Project{}) pbs := &bootstrap[*v1.Project]{ log: log, ds: ps, diff --git a/pkg/datastore/migrate.go b/pkg/datastore/migrate.go index 0fb4609..ca8fe00 100644 --- a/pkg/datastore/migrate.go +++ b/pkg/datastore/migrate.go @@ -22,10 +22,7 @@ func MigrateDB(log *slog.Logger, db *sqlx.DB, healthServer *health.Server) error &migrator.Migration{ Name: "Sample Migration for Tenant", Func: func(tx *sql.Tx) error { - ts, err := New(log, db, &v1.Tenant{}) - if err != nil { - return err - } + ts := New(log, db, &v1.Tenant{}) tenants, _, err := ts.Find(context.Background(), nil, nil) if err != nil { diff --git a/pkg/datastore/postgres.go b/pkg/datastore/postgres.go index 6b5d54b..40b8dd5 100644 --- a/pkg/datastore/postgres.go +++ b/pkg/datastore/postgres.go @@ -82,7 +82,7 @@ func NewPostgresDB(logger *slog.Logger, host, port, user, password, dbname, sslm } // New creates a new Storage which uses the given database abstraction. -func New[E Entity](logger *slog.Logger, db *sqlx.DB, e E) (Storage[E], error) { +func New[E Entity](logger *slog.Logger, db *sqlx.DB, e E) Storage[E] { ds := &datastore[E]{ log: logger, db: db, @@ -91,7 +91,7 @@ func New[E Entity](logger *slog.Logger, db *sqlx.DB, e E) (Storage[E], error) { tableName: e.TableName(), historyTableName: fmt.Sprintf("%s_history", e.TableName()), } - return ds, nil + return ds } // Create a entity diff --git a/pkg/datastore/postgres_benchmark_test.go b/pkg/datastore/postgres_benchmark_test.go index e99d539..2fe0177 100644 --- a/pkg/datastore/postgres_benchmark_test.go +++ b/pkg/datastore/postgres_benchmark_test.go @@ -18,7 +18,7 @@ var ( func init() { _, db, _ = createPostgresConnection() - ds, _ = New(slog.Default(), db, &v1.Tenant{}) + ds = New(slog.Default(), db, &v1.Tenant{}) } func BenchmarkGetTenant(b *testing.B) { diff --git a/pkg/datastore/postgres_test.go b/pkg/datastore/postgres_test.go index fbf1b71..2460fba 100644 --- a/pkg/datastore/postgres_test.go +++ b/pkg/datastore/postgres_test.go @@ -53,8 +53,7 @@ func TestMain(m *testing.M) { } func TestCRUD(t *testing.T) { - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) assert.NotNil(t, tenantDS, "Datastore must not be nil") ctx := context.Background() tcr := &v1.Tenant{ @@ -63,7 +62,7 @@ func TestCRUD(t *testing.T) { Description: "A very important Tenant", } - err = tenantDS.Create(ctx, tcr) + err := tenantDS.Create(ctx, tcr) require.NoError(t, err) assert.NotNil(t, tcr) // specified id is persisted @@ -134,8 +133,7 @@ func TestCRUD(t *testing.T) { } func TestUpdateOptimisticLock(t *testing.T) { - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) assert.NotNil(t, tenantDS, "Datastore must not be nil") ctx := context.Background() tcr := &v1.Tenant{ @@ -144,7 +142,7 @@ func TestUpdateOptimisticLock(t *testing.T) { Description: "A very important Tenant", } - err = tenantDS.Create(ctx, tcr) + err := tenantDS.Create(ctx, tcr) require.NoError(t, err) assert.NotNil(t, tcr) assert.Equal(t, int64(0), tcr.Meta.Version) @@ -180,8 +178,7 @@ func TestUpdateOptimisticLock(t *testing.T) { func TestCreate(t *testing.T) { const t1 = "t1" - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) assert.NotNil(t, tenantDS, "Datastore must not be nil") ctx := context.Background() @@ -191,7 +188,7 @@ func TestCreate(t *testing.T) { } // meta is nil - err = tenantDS.Create(ctx, tcr1) + err := tenantDS.Create(ctx, tcr1) require.Error(t, err) require.EqualError(t, err, "create of type:tenant failed, meta is nil") @@ -271,8 +268,7 @@ func TestCreate(t *testing.T) { func TestUpdate(t *testing.T) { const t3 = "t3" - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) assert.NotNil(t, tenantDS, "Datastore must not be nil") ctx := context.Background() @@ -281,7 +277,7 @@ func TestUpdate(t *testing.T) { Name: "ctenant", Description: "C Tenant", } - err = tenantDS.Update(ctx, tcr1) + err := tenantDS.Update(ctx, tcr1) require.Error(t, err) require.EqualError(t, err, "update of type:tenant failed, meta is nil") @@ -344,20 +340,18 @@ func TestUpdate(t *testing.T) { //nolint:unparam func checkHistoryCreated(ctx context.Context, t *testing.T, id string, name string, desc string) { - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) var tgrhc v1.Tenant - err = tenantDS.GetHistoryCreated(ctx, id, &tgrhc) + err := tenantDS.GetHistoryCreated(ctx, id, &tgrhc) require.NoError(t, err) assert.Equal(t, name, tgrhc.Name) assert.Equal(t, desc, tgrhc.GetDescription()) } func checkHistory(ctx context.Context, t *testing.T, id string, tm time.Time, name string, desc string) { - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) var tgrh v1.Tenant - err = tenantDS.GetHistory(ctx, id, tm, &tgrh) + err := tenantDS.GetHistory(ctx, id, tm, &tgrh) require.NoError(t, err) assert.Equal(t, name, tgrh.Name) assert.Equal(t, desc, tgrh.GetDescription()) @@ -365,12 +359,11 @@ func checkHistory(ctx context.Context, t *testing.T, id string, tm time.Time, na func TestGet(t *testing.T) { const t4 = "t4" - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) assert.NotNil(t, tenantDS, "Datastore must not be nil") ctx := context.Background() // unknown id - _, err = tenantDS.Get(ctx, "unknown-id") + _, err := tenantDS.Get(ctx, "unknown-id") require.Error(t, err) require.EqualError(t, err, "tenant with id:unknown-id not found sql: no rows in result set") @@ -396,8 +389,7 @@ func TestGet(t *testing.T) { func TestGetHistory(t *testing.T) { const t5 = "t5" - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) assert.NotNil(t, tenantDS, "Datastore must not be nil") ctx := context.Background() @@ -405,7 +397,7 @@ func TestGetHistory(t *testing.T) { // unknown id var tgr1 v1.Tenant - err = tenantDS.GetHistory(ctx, "unknown-id", tsNow, &tgr1) + err := tenantDS.GetHistory(ctx, "unknown-id", tsNow, &tgr1) require.Error(t, err) require.EqualError(t, err, "entity of type:tenant with predicate:[map[id:unknown-id] map[created_at:2020-04-30 18:00:00 +0000 UTC]] not found") @@ -487,8 +479,7 @@ func TestGetHistory(t *testing.T) { func TestFind(t *testing.T) { const t6 = "t6" - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) assert.NotNil(t, tenantDS, "Datastore must not be nil") ctx := context.Background() @@ -498,7 +489,7 @@ func TestFind(t *testing.T) { Name: "ftenant", Description: "F Tenant", } - err = tenantDS.Create(ctx, tcr1) + err := tenantDS.Create(ctx, tcr1) require.NoError(t, err) assert.Equal(t, t6, tcr1.GetMeta().GetId()) assert.Equal(t, "ftenant", tcr1.GetName()) @@ -554,10 +545,9 @@ func TestFind(t *testing.T) { } func TestFindWithPaging(t *testing.T) { - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) // prevent side effects db.MustExec("DELETE from tenants") - require.NoError(t, err) assert.NotNil(t, tenantDS, "Datastore must not be nil") ctx := context.Background() @@ -595,8 +585,7 @@ func TestFindWithPaging(t *testing.T) { func TestDelete(t *testing.T) { const t9 = "t9" - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) assert.NotNil(t, tenantDS, "Datastore must not be nil") ctx := context.Background() @@ -604,7 +593,7 @@ func TestDelete(t *testing.T) { tdr1 := &v1.Tenant{ Meta: &v1.Meta{Id: "unknown-id"}, } - err = tenantDS.Delete(ctx, tdr1.Meta.Id) + err := tenantDS.Delete(ctx, tdr1.Meta.Id) require.Error(t, err) require.EqualError(t, err, "tenant with id:unknown-id not found sql: no rows in result set") @@ -642,8 +631,7 @@ func TestDeleteAll(t *testing.T) { t11 = "t11" t10 = "t10" ) - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) assert.NotNil(t, tenantDS, "Datastore must not be nil") ctx := context.Background() @@ -651,7 +639,7 @@ func TestDeleteAll(t *testing.T) { tdr1 := &v1.Tenant{ Meta: &v1.Meta{Id: "unknown-id"}, } - err = tenantDS.DeleteAll(ctx, tdr1.Meta.Id) + err := tenantDS.DeleteAll(ctx, tdr1.Meta.Id) require.Error(t, err) require.EqualError(t, err, "tenant with id:unknown-id not found sql: no rows in result set") @@ -702,8 +690,7 @@ func TestDeleteAll(t *testing.T) { } func TestAnnotationsAndLabels(t *testing.T) { - tenantDS, err := New(slog.Default(), db, &v1.Tenant{}) - require.NoError(t, err) + tenantDS := New(slog.Default(), db, &v1.Tenant{}) assert.NotNil(t, tenantDS, "Datastore must not be nil") ctx := context.Background() tcr := &v1.Tenant{ @@ -721,7 +708,7 @@ func TestAnnotationsAndLabels(t *testing.T) { Description: "A very important Tenant", } - err = tenantDS.Create(ctx, tcr) + err := tenantDS.Create(ctx, tcr) require.NoError(t, err) assert.NotNil(t, tcr) assert.Equal(t, int64(0), tcr.Meta.Version) diff --git a/pkg/service/project.go b/pkg/service/project.go index 31918e8..edb8dde 100644 --- a/pkg/service/project.go +++ b/pkg/service/project.go @@ -19,25 +19,16 @@ type projectService struct { log *slog.Logger } -func NewProjectService(db *sqlx.DB, l *slog.Logger) (*projectService, error) { - ps, err := datastore.New(l, db, &v1.Project{}) - if err != nil { - return nil, err - } - ts, err := datastore.New(l, db, &v1.Tenant{}) - if err != nil { - return nil, err - } - pms, err := datastore.New(l, db, &v1.ProjectMember{}) - if err != nil { - return nil, err - } +func NewProjectService(db *sqlx.DB, l *slog.Logger) *projectService { + ps := datastore.New(l, db, &v1.Project{}) + ts := datastore.New(l, db, &v1.Tenant{}) + pms := datastore.New(l, db, &v1.ProjectMember{}) return &projectService{ projectStore: NewStorageStatusWrapper(ps), projectMemberStore: NewStorageStatusWrapper(pms), tenantStore: NewStorageStatusWrapper(ts), log: l, - }, nil + } } func (s *projectService) Create(ctx context.Context, req *v1.ProjectCreateRequest) (*v1.ProjectResponse, error) { diff --git a/pkg/service/projectmember.go b/pkg/service/projectmember.go index a14e2dc..77a44fc 100644 --- a/pkg/service/projectmember.go +++ b/pkg/service/projectmember.go @@ -19,25 +19,16 @@ type projectMemberService struct { log *slog.Logger } -func NewProjectMemberService(db *sqlx.DB, l *slog.Logger) (*projectMemberService, error) { - pms, err := datastore.New(l, db, &v1.ProjectMember{}) - if err != nil { - return nil, err - } - ts, err := datastore.New(l, db, &v1.Tenant{}) - if err != nil { - return nil, err - } - ps, err := datastore.New(l, db, &v1.Project{}) - if err != nil { - return nil, err - } +func NewProjectMemberService(db *sqlx.DB, l *slog.Logger) *projectMemberService { + pms := datastore.New(l, db, &v1.ProjectMember{}) + ts := datastore.New(l, db, &v1.Tenant{}) + ps := datastore.New(l, db, &v1.Project{}) return &projectMemberService{ projectMemberStore: NewStorageStatusWrapper(pms), tenantStore: NewStorageStatusWrapper(ts), projectStore: NewStorageStatusWrapper(ps), log: l, - }, nil + } } func (s *projectMemberService) Create(ctx context.Context, req *v1.ProjectMemberCreateRequest) (*v1.ProjectMemberResponse, error) { diff --git a/pkg/service/tenant.go b/pkg/service/tenant.go index 24c3157..d401f7c 100644 --- a/pkg/service/tenant.go +++ b/pkg/service/tenant.go @@ -26,21 +26,15 @@ var ( tenants = datastore.Entity(&v1.Tenant{}) ) -func NewTenantService(db *sqlx.DB, l *slog.Logger) (*tenantService, error) { - ts, err := datastore.New(l, db, &v1.Tenant{}) - if err != nil { - return nil, err - } - tms, err := datastore.New(l, db, &v1.TenantMember{}) - if err != nil { - return nil, err - } +func NewTenantService(db *sqlx.DB, l *slog.Logger) *tenantService { + ts := datastore.New(l, db, &v1.Tenant{}) + tms := datastore.New(l, db, &v1.TenantMember{}) return &tenantService{ db: db, tenantStore: NewStorageStatusWrapper(ts), tenantMemberStore: NewStorageStatusWrapper(tms), log: l, - }, nil + } } func (s *tenantService) Create(ctx context.Context, req *v1.TenantCreateRequest) (*v1.TenantResponse, error) { diff --git a/pkg/service/tenant_test.go b/pkg/service/tenant_test.go index 672f1ed..773e828 100644 --- a/pkg/service/tenant_test.go +++ b/pkg/service/tenant_test.go @@ -208,9 +208,9 @@ func Test_tenantService_FindParticipatingProjects(t *testing.T) { } var ( - projectStore, _ = datastore.New(log, db, &v1.Project{}) - tenantMemberStore, _ = datastore.New(log, db, &v1.TenantMember{}) - projectMemberStore, _ = datastore.New(log, db, &v1.ProjectMember{}) + projectStore = datastore.New(log, db, &v1.Project{}) + tenantMemberStore = datastore.New(log, db, &v1.TenantMember{}) + projectMemberStore = datastore.New(log, db, &v1.ProjectMember{}) ) tests := []struct { @@ -453,10 +453,10 @@ func Test_tenantService_FindParticipatingTenants(t *testing.T) { } var ( - projectStore, _ = datastore.New(log, db, &v1.Project{}) - tenantMemberStore, _ = datastore.New(log, db, &v1.TenantMember{}) - projectMemberStore, _ = datastore.New(log, db, &v1.ProjectMember{}) - tenantStore, _ = datastore.New(log, db, &v1.Tenant{}) + projectStore = datastore.New(log, db, &v1.Project{}) + tenantMemberStore = datastore.New(log, db, &v1.TenantMember{}) + projectMemberStore = datastore.New(log, db, &v1.ProjectMember{}) + tenantStore = datastore.New(log, db, &v1.Tenant{}) ) tests := []struct { @@ -680,10 +680,10 @@ func Test_tenantService_ListTenantMembers(t *testing.T) { } var ( - projectStore, _ = datastore.New(log, db, &v1.Project{}) - tenantMemberStore, _ = datastore.New(log, db, &v1.TenantMember{}) - projectMemberStore, _ = datastore.New(log, db, &v1.ProjectMember{}) - tenantStore, _ = datastore.New(log, db, &v1.Tenant{}) + projectStore = datastore.New(log, db, &v1.Project{}) + tenantMemberStore = datastore.New(log, db, &v1.TenantMember{}) + projectMemberStore = datastore.New(log, db, &v1.ProjectMember{}) + tenantStore = datastore.New(log, db, &v1.Tenant{}) ) tests := []struct { diff --git a/pkg/service/tenantmember.go b/pkg/service/tenantmember.go index a7dadb0..93fa12c 100644 --- a/pkg/service/tenantmember.go +++ b/pkg/service/tenantmember.go @@ -18,20 +18,14 @@ type tenantMemberService struct { log *slog.Logger } -func NewTenantMemberService(db *sqlx.DB, l *slog.Logger) (*tenantMemberService, error) { - tms, err := datastore.New(l, db, &v1.TenantMember{}) - if err != nil { - return nil, err - } - ts, err := datastore.New(l, db, &v1.Tenant{}) - if err != nil { - return nil, err - } +func NewTenantMemberService(db *sqlx.DB, l *slog.Logger) *tenantMemberService { + tms := datastore.New(l, db, &v1.TenantMember{}) + ts := datastore.New(l, db, &v1.Tenant{}) return &tenantMemberService{ tenantMemberStore: NewStorageStatusWrapper(tms), tenantStore: NewStorageStatusWrapper(ts), log: l, - }, nil + } } func (s *tenantMemberService) Create(ctx context.Context, req *v1.TenantMemberCreateRequest) (*v1.TenantMemberResponse, error) { diff --git a/server/main.go b/server/main.go index 19cd586..4e2352b 100644 --- a/server/main.go +++ b/server/main.go @@ -244,27 +244,10 @@ func run() { logger.Error("unable to apply migrate db", "error", err) } - projectService, err := service.NewProjectService(db, logger) - if err != nil { - logger.Error("unable to create project service", "error", err) - panic(err) - } - projectMemberService, err := service.NewProjectMemberService(db, logger) - if err != nil { - logger.Error("unable to create project member service", "error", err) - panic(err) - - } - tenantService, err := service.NewTenantService(db, logger) - if err != nil { - logger.Error("unable to create tenant service", "error", err) - panic(err) - } - tenantMemberService, err := service.NewTenantMemberService(db, logger) - if err != nil { - logger.Error("unable to create tenant member service", "error", err) - panic(err) - } + projectService := service.NewProjectService(db, logger) + projectMemberService := service.NewProjectMemberService(db, logger) + tenantService := service.NewTenantService(db, logger) + tenantMemberService := service.NewTenantMemberService(db, logger) apiv1.RegisterProjectServiceServer(grpcServer, projectService) apiv1.RegisterProjectMemberServiceServer(grpcServer, projectMemberService)