From 2f30459bac6f3c9be335fb2db305702c3745a94e Mon Sep 17 00:00:00 2001 From: Youngteac Hong Date: Fri, 15 Dec 2023 10:45:48 +0900 Subject: [PATCH] Simplify converter interface --- admin/client.go | 6 +-- api/converter/from_pb.go | 8 ++-- api/converter/to_pb.go | 53 +++++++++---------------- cmd/yorkie/document/list.go | 2 +- cmd/yorkie/document/remove.go | 2 +- cmd/yorkie/history.go | 2 +- cmd/yorkie/login.go | 2 +- cmd/yorkie/project/create.go | 2 +- cmd/yorkie/project/list.go | 2 +- cmd/yorkie/project/update.go | 2 +- server/rpc/admin_server.go | 56 ++++----------------------- test/bench/grpc_bench_test.go | 2 +- test/helper/helper.go | 4 +- test/integration/admin_test.go | 2 +- test/integration/auth_webhook_test.go | 10 ++--- test/integration/document_test.go | 2 +- test/integration/history_test.go | 2 +- test/integration/retention_test.go | 2 +- test/integration/user_test.go | 2 +- 19 files changed, 51 insertions(+), 112 deletions(-) diff --git a/admin/client.go b/admin/client.go index 96fcd6c24..2a4c6b0d2 100644 --- a/admin/client.go +++ b/admin/client.go @@ -128,10 +128,8 @@ func (c *Client) Dial(rpcAddr string) error { } // Close closes the connection to the admin service. -func (c *Client) Close() error { +func (c *Client) Close() { c.conn.CloseIdleConnections() - - return nil } // LogIn logs in a user. @@ -167,7 +165,7 @@ func (c *Client) SignUp( return nil, err } - return converter.FromUser(response.Msg.User) + return converter.FromUser(response.Msg.User), nil } // CreateProject creates a new project. diff --git a/api/converter/from_pb.go b/api/converter/from_pb.go index 953e1528c..9119bbc1c 100644 --- a/api/converter/from_pb.go +++ b/api/converter/from_pb.go @@ -30,14 +30,12 @@ import ( ) // FromUser converts the given Protobuf formats to model format. -func FromUser(pbUser *api.User) (*types.User, error) { - createdAt := pbUser.CreatedAt.AsTime() - +func FromUser(pbUser *api.User) *types.User { return &types.User{ ID: types.ID(pbUser.Id), Username: pbUser.Username, - CreatedAt: createdAt, - }, nil + CreatedAt: pbUser.CreatedAt.AsTime(), + } } // FromProjects converts the given Protobuf formats to model format. diff --git a/api/converter/to_pb.go b/api/converter/to_pb.go index 43cd21268..b2d29778f 100644 --- a/api/converter/to_pb.go +++ b/api/converter/to_pb.go @@ -33,35 +33,26 @@ import ( ) // ToUser converts the given model format to Protobuf format. -func ToUser(user *types.User) (*api.User, error) { - pbCreatedAt := timestamppb.New(user.CreatedAt) - +func ToUser(user *types.User) *api.User { return &api.User{ Id: user.ID.String(), Username: user.Username, - CreatedAt: pbCreatedAt, - }, nil + CreatedAt: timestamppb.New(user.CreatedAt), + } } // ToProjects converts the given model to Protobuf. -func ToProjects(projects []*types.Project) ([]*api.Project, error) { +func ToProjects(projects []*types.Project) []*api.Project { var pbProjects []*api.Project for _, project := range projects { - pbProject, err := ToProject(project) - if err != nil { - return nil, err - } - pbProjects = append(pbProjects, pbProject) + pbProjects = append(pbProjects, ToProject(project)) } - return pbProjects, nil + return pbProjects } // ToProject converts the given model to Protobuf. -func ToProject(project *types.Project) (*api.Project, error) { - pbCreatedAt := timestamppb.New(project.CreatedAt) - pbUpdatedAt := timestamppb.New(project.UpdatedAt) - +func ToProject(project *types.Project) *api.Project { return &api.Project{ Id: project.ID.String(), Name: project.Name, @@ -70,38 +61,30 @@ func ToProject(project *types.Project) (*api.Project, error) { ClientDeactivateThreshold: project.ClientDeactivateThreshold, PublicKey: project.PublicKey, SecretKey: project.SecretKey, - CreatedAt: pbCreatedAt, - UpdatedAt: pbUpdatedAt, - }, nil + CreatedAt: timestamppb.New(project.CreatedAt), + UpdatedAt: timestamppb.New(project.UpdatedAt), + } } // ToDocumentSummaries converts the given model to Protobuf. -func ToDocumentSummaries(summaries []*types.DocumentSummary) ([]*api.DocumentSummary, error) { +func ToDocumentSummaries(summaries []*types.DocumentSummary) []*api.DocumentSummary { var pbSummaries []*api.DocumentSummary for _, summary := range summaries { - pbSummary, err := ToDocumentSummary(summary) - if err != nil { - return nil, err - } - pbSummaries = append(pbSummaries, pbSummary) + pbSummaries = append(pbSummaries, ToDocumentSummary(summary)) } - return pbSummaries, nil + return pbSummaries } // ToDocumentSummary converts the given model to Protobuf format. -func ToDocumentSummary(summary *types.DocumentSummary) (*api.DocumentSummary, error) { - pbCreatedAt := timestamppb.New(summary.CreatedAt) - pbAccessedAt := timestamppb.New(summary.AccessedAt) - pbUpdatedAt := timestamppb.New(summary.UpdatedAt) - +func ToDocumentSummary(summary *types.DocumentSummary) *api.DocumentSummary { return &api.DocumentSummary{ Id: summary.ID.String(), Key: summary.Key.String(), - CreatedAt: pbCreatedAt, - AccessedAt: pbAccessedAt, - UpdatedAt: pbUpdatedAt, + CreatedAt: timestamppb.New(summary.CreatedAt), + AccessedAt: timestamppb.New(summary.AccessedAt), + UpdatedAt: timestamppb.New(summary.UpdatedAt), Snapshot: summary.Snapshot, - }, nil + } } // ToPresences converts the given model to Protobuf format. diff --git a/cmd/yorkie/document/list.go b/cmd/yorkie/document/list.go index 4989c9d86..71d886e87 100644 --- a/cmd/yorkie/document/list.go +++ b/cmd/yorkie/document/list.go @@ -59,7 +59,7 @@ func newListCommand() *cobra.Command { return err } defer func() { - _ = cli.Close() + cli.Close() }() ctx := context.Background() diff --git a/cmd/yorkie/document/remove.go b/cmd/yorkie/document/remove.go index f79d8f2e2..a682eae48 100644 --- a/cmd/yorkie/document/remove.go +++ b/cmd/yorkie/document/remove.go @@ -54,7 +54,7 @@ func newRemoveCommand() *cobra.Command { return err } defer func() { - _ = cli.Close() + cli.Close() }() ctx := context.Background() diff --git a/cmd/yorkie/history.go b/cmd/yorkie/history.go index 8634ee266..73782bbe2 100644 --- a/cmd/yorkie/history.go +++ b/cmd/yorkie/history.go @@ -56,7 +56,7 @@ func newHistoryCmd() *cobra.Command { return err } defer func() { - _ = cli.Close() + cli.Close() }() ctx := context.Background() diff --git a/cmd/yorkie/login.go b/cmd/yorkie/login.go index 034577906..da636f0c8 100644 --- a/cmd/yorkie/login.go +++ b/cmd/yorkie/login.go @@ -43,7 +43,7 @@ func newLoginCmd() *cobra.Command { return err } defer func() { - _ = cli.Close() + cli.Close() }() ctx := context.Background() diff --git a/cmd/yorkie/project/create.go b/cmd/yorkie/project/create.go index 89ba8a024..5e1f0416b 100644 --- a/cmd/yorkie/project/create.go +++ b/cmd/yorkie/project/create.go @@ -53,7 +53,7 @@ func newCreateCommand() *cobra.Command { return err } defer func() { - _ = cli.Close() + cli.Close() }() ctx := context.Background() diff --git a/cmd/yorkie/project/list.go b/cmd/yorkie/project/list.go index b2721e75a..14a9ecbaf 100644 --- a/cmd/yorkie/project/list.go +++ b/cmd/yorkie/project/list.go @@ -46,7 +46,7 @@ func newListCommand() *cobra.Command { return err } defer func() { - _ = cli.Close() + cli.Close() }() ctx := context.Background() diff --git a/cmd/yorkie/project/update.go b/cmd/yorkie/project/update.go index c273f2e4e..c26eb94be 100644 --- a/cmd/yorkie/project/update.go +++ b/cmd/yorkie/project/update.go @@ -62,7 +62,7 @@ func newUpdateCommand() *cobra.Command { return err } defer func() { - _ = cli.Close() + cli.Close() }() ctx := context.Background() diff --git a/server/rpc/admin_server.go b/server/rpc/admin_server.go index 98f9a7bd7..63f7fbe2b 100644 --- a/server/rpc/admin_server.go +++ b/server/rpc/admin_server.go @@ -65,13 +65,8 @@ func (s *adminServer) SignUp( return nil, err } - pbUser, err := converter.ToUser(user) - if err != nil { - return nil, err - } - return connect.NewResponse(&api.SignUpResponse{ - User: pbUser, + User: converter.ToUser(user), }), nil } @@ -116,13 +111,8 @@ func (s *adminServer) CreateProject( return nil, err } - pbProject, err := converter.ToProject(project) - if err != nil { - return nil, err - } - return connect.NewResponse(&api.CreateProjectResponse{ - Project: pbProject, + Project: converter.ToProject(project), }), nil } @@ -137,13 +127,8 @@ func (s *adminServer) ListProjects( return nil, err } - pbProjects, err := converter.ToProjects(projectList) - if err != nil { - return nil, err - } - return connect.NewResponse(&api.ListProjectsResponse{ - Projects: pbProjects, + Projects: converter.ToProjects(projectList), }), nil } @@ -158,13 +143,8 @@ func (s *adminServer) GetProject( return nil, err } - pbProject, err := converter.ToProject(project) - if err != nil { - return nil, err - } - return connect.NewResponse(&api.GetProjectResponse{ - Project: pbProject, + Project: converter.ToProject(project), }), nil } @@ -193,13 +173,8 @@ func (s *adminServer) UpdateProject( return nil, err } - pbProject, err := converter.ToProject(project) - if err != nil { - return nil, err - } - return connect.NewResponse(&api.UpdateProjectResponse{ - Project: pbProject, + Project: converter.ToProject(project), }), nil } @@ -224,13 +199,8 @@ func (s *adminServer) GetDocument( return nil, err } - pbDocument, err := converter.ToDocumentSummary(document) - if err != nil { - return nil, err - } - return connect.NewResponse(&api.GetDocumentResponse{ - Document: pbDocument, + Document: converter.ToDocumentSummary(document), }), nil } @@ -293,13 +263,8 @@ func (s *adminServer) ListDocuments( return nil, err } - pbDocuments, err := converter.ToDocumentSummaries(docs) - if err != nil { - return nil, err - } - return connect.NewResponse(&api.ListDocumentsResponse{ - Documents: pbDocuments, + Documents: converter.ToDocumentSummaries(docs), }), nil } @@ -325,14 +290,9 @@ func (s *adminServer) SearchDocuments( return nil, err } - pbDocuments, err := converter.ToDocumentSummaries(result.Elements) - if err != nil { - return nil, err - } - return connect.NewResponse(&api.SearchDocumentsResponse{ TotalCount: int32(result.TotalCount), - Documents: pbDocuments, + Documents: converter.ToDocumentSummaries(result.Elements), }), nil } diff --git a/test/bench/grpc_bench_test.go b/test/bench/grpc_bench_test.go index bc9099395..39bb924ca 100644 --- a/test/bench/grpc_bench_test.go +++ b/test/bench/grpc_bench_test.go @@ -301,7 +301,7 @@ func BenchmarkRPC(b *testing.B) { b.Run("adminCli to server", func(b *testing.B) { adminCli := helper.CreateAdminCli(b, defaultServer.RPCAddr()) - defer func() { assert.NoError(b, adminCli.Close()) }() + defer func() { adminCli.Close() }() ctx := context.Background() for i := 0; i < b.N; i++ { diff --git a/test/helper/helper.go b/test/helper/helper.go index 87a3c86b2..bd90c5d4b 100644 --- a/test/helper/helper.go +++ b/test/helper/helper.go @@ -49,9 +49,9 @@ var testStartedAt int64 // Below are the values of the Yorkie config used in the test. var ( - RPCPort = 21101 + RPCPort = 11101 - ProfilingPort = 21102 + ProfilingPort = 11102 AdminUser = server.DefaultAdminUser AdminPassword = server.DefaultAdminPassword diff --git a/test/integration/admin_test.go b/test/integration/admin_test.go index dfb535d5a..b11e18f25 100644 --- a/test/integration/admin_test.go +++ b/test/integration/admin_test.go @@ -43,7 +43,7 @@ func TestAdmin(t *testing.T) { _, err = adminCli.LogIn(ctx, "admin", "admin") assert.NoError(t, err) defer func() { - assert.NoError(t, adminCli.Close()) + adminCli.Close() }() clients := activeClients(t, 1) diff --git a/test/integration/auth_webhook_test.go b/test/integration/auth_webhook_test.go index de57180e7..8f196b8bf 100644 --- a/test/integration/auth_webhook_test.go +++ b/test/integration/auth_webhook_test.go @@ -84,7 +84,7 @@ func TestProjectAuthWebhook(t *testing.T) { defer func() { assert.NoError(t, svr.Shutdown(true)) }() adminCli := helper.CreateAdminCli(t, svr.RPCAddr()) - defer func() { assert.NoError(t, adminCli.Close()) }() + defer func() { adminCli.Close() }() project, err := adminCli.CreateProject(context.Background(), "auth-webhook-test") assert.NoError(t, err) @@ -199,7 +199,7 @@ func TestAuthWebhook(t *testing.T) { defer func() { assert.NoError(t, svr.Shutdown(true)) }() adminCli := helper.CreateAdminCli(t, svr.RPCAddr()) - defer func() { assert.NoError(t, adminCli.Close()) }() + defer func() { adminCli.Close() }() project, err := adminCli.CreateProject(context.Background(), "success-webhook-after-retries") assert.NoError(t, err) project.AuthWebhookURL = authServer.URL @@ -241,7 +241,7 @@ func TestAuthWebhook(t *testing.T) { defer func() { assert.NoError(t, svr.Shutdown(true)) }() adminCli := helper.CreateAdminCli(t, svr.RPCAddr()) - defer func() { assert.NoError(t, adminCli.Close()) }() + defer func() { adminCli.Close() }() project, err := adminCli.CreateProject(context.Background(), "fail-webhook-after-retries") assert.NoError(t, err) project.AuthWebhookURL = authServer.URL @@ -294,7 +294,7 @@ func TestAuthWebhook(t *testing.T) { defer func() { assert.NoError(t, svr.Shutdown(true)) }() adminCli := helper.CreateAdminCli(t, svr.RPCAddr()) - defer func() { assert.NoError(t, adminCli.Close()) }() + defer func() { adminCli.Close() }() project, err := adminCli.CreateProject(context.Background(), "auth-request-cache") assert.NoError(t, err) project.AuthWebhookURL = authServer.URL @@ -370,7 +370,7 @@ func TestAuthWebhook(t *testing.T) { defer func() { assert.NoError(t, svr.Shutdown(true)) }() adminCli := helper.CreateAdminCli(t, svr.RPCAddr()) - defer func() { assert.NoError(t, adminCli.Close()) }() + defer func() { adminCli.Close() }() project, err := adminCli.CreateProject(context.Background(), "unauth-request-cache") assert.NoError(t, err) project.AuthWebhookURL = authServer.URL diff --git a/test/integration/document_test.go b/test/integration/document_test.go index 84c49d66d..e69ae49a1 100644 --- a/test/integration/document_test.go +++ b/test/integration/document_test.go @@ -672,7 +672,7 @@ func TestDocument(t *testing.T) { func TestDocumentWithProjects(t *testing.T) { ctx := context.Background() adminCli := helper.CreateAdminCli(t, defaultServer.RPCAddr()) - defer func() { assert.NoError(t, adminCli.Close()) }() + defer func() { adminCli.Close() }() project1, err := adminCli.CreateProject(ctx, "project1") assert.NoError(t, err) diff --git a/test/integration/history_test.go b/test/integration/history_test.go index 24134ab06..52c059518 100644 --- a/test/integration/history_test.go +++ b/test/integration/history_test.go @@ -36,7 +36,7 @@ func TestHistory(t *testing.T) { defer deactivateAndCloseClients(t, clients) adminCli := helper.CreateAdminCli(t, defaultServer.RPCAddr()) - defer func() { assert.NoError(t, adminCli.Close()) }() + defer func() { adminCli.Close() }() t.Run("history test", func(t *testing.T) { ctx := context.Background() diff --git a/test/integration/retention_test.go b/test/integration/retention_test.go index fc36fa2df..863005698 100644 --- a/test/integration/retention_test.go +++ b/test/integration/retention_test.go @@ -76,7 +76,7 @@ func TestRetention(t *testing.T) { adminCli := helper.CreateAdminCli(t, testServer.RPCAddr()) assert.NoError(t, err) - defer func() { assert.NoError(t, adminCli.Close()) }() + defer func() { adminCli.Close() }() ctx := context.Background() diff --git a/test/integration/user_test.go b/test/integration/user_test.go index 3f95f0141..304f6c168 100644 --- a/test/integration/user_test.go +++ b/test/integration/user_test.go @@ -30,7 +30,7 @@ import ( func TestUser(t *testing.T) { adminCli := helper.CreateAdminCli(t, defaultServer.RPCAddr()) - defer func() { assert.NoError(t, adminCli.Close()) }() + defer func() { adminCli.Close() }() t.Run("user test", func(t *testing.T) { ctx := context.Background()