Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(server/v2): remove serverv2.AppI #22446

Merged
merged 4 commits into from
Nov 6, 2024
Merged

refactor(server/v2): remove serverv2.AppI #22446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9dcba88
refactor(server/v2): remove serverv2.AppI
julienrbrt Nov 6, 2024
9d83337
updates
julienrbrt Nov 6, 2024
f3881ef
go mod tidy
julienrbrt Nov 6, 2024
82cd292
build
julienrbrt Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions server/v2/api/grpc/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ func New[T transaction.Tx](
logger log.Logger,
interfaceRegistry server.InterfaceRegistry,
queryHandlers map[string]appmodulev2.Handler,
queryable interface {
Query(ctx context.Context, version uint64, msg transaction.Msg) (transaction.Msg, error)
},
queryable func(ctx context.Context, version uint64, msg transaction.Msg) (transaction.Msg, error),
cfg server.ConfigMap,
cfgOptions ...CfgOption,
) (*Server[T], error) {
Expand All @@ -74,7 +72,7 @@ func New[T transaction.Tx](
// Reflection allows external clients to see what services and methods the gRPC server exposes.
gogoreflection.Register(grpcSrv, slices.Collect(maps.Keys(queryHandlers)), logger.With("sub-module", "grpc-reflection"))

// Register V2
// Register V2 grpc handlers
RegisterServiceServer(grpcSrv, &v2Service{queryHandlers, queryable})

srv.grpcSrv = grpcSrv
Expand All @@ -99,9 +97,9 @@ func (s *Server[T]) StartCmdFlags() *pflag.FlagSet {
return flags
}

func makeUnknownServiceHandler(handlers map[string]appmodulev2.Handler, querier interface {
Query(ctx context.Context, version uint64, msg transaction.Msg) (transaction.Msg, error)
},
func makeUnknownServiceHandler(
handlers map[string]appmodulev2.Handler,
queryable func(ctx context.Context, version uint64, msg transaction.Msg) (transaction.Msg, error),
) grpc.StreamHandler {
getRegistry := sync.OnceValues(gogoproto.MergedRegistry)

Expand Down Expand Up @@ -149,7 +147,7 @@ func makeUnknownServiceHandler(handlers map[string]appmodulev2.Handler, querier
if err != nil {
return status.Errorf(codes.InvalidArgument, "invalid get height from context: %v", err)
}
resp, err := querier.Query(ctx, height, req)
resp, err := queryable(ctx, height, req)
if err != nil {
return err
}
Expand Down
6 changes: 2 additions & 4 deletions server/v2/api/grpc/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ import (
// v2Service implements the gRPC service interface for handling queries and listing handlers.
type v2Service struct {
queryHandlers map[string]appmodulev2.Handler
queryable interface {
Query(ctx context.Context, version uint64, msg transaction.Msg) (transaction.Msg, error)
}
queryable func(ctx context.Context, version uint64, msg transaction.Msg) (transaction.Msg, error)
}

// Query handles incoming query requests by unmarshaling the request, processing it,
Expand All @@ -39,7 +37,7 @@ func (s v2Service) Query(ctx context.Context, request *QueryRequest) (*QueryResp
return nil, status.Errorf(codes.InvalidArgument, "failed to unmarshal request: %v", err)
}

queryResp, err := s.queryable.Query(ctx, 0, protoMsg)
queryResp, err := s.queryable(ctx, 0, protoMsg)
if err != nil {
return nil, status.Errorf(codes.Internal, "query failed: %v", err)
}
Expand Down
7 changes: 2 additions & 5 deletions server/v2/api/grpc/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/core/transaction"
serverv2 "cosmossdk.io/server/v2"
)

type MockRequestMessage struct {
Expand Down Expand Up @@ -42,8 +41,6 @@ func (m *MockResponseMessage) ValidateBasic() error {

type mockApp[T transaction.Tx] struct {
mock.Mock

serverv2.AppI[T]
}

func (m *mockApp[T]) QueryHandlers() map[string]appmodulev2.Handler {
Expand Down Expand Up @@ -139,7 +136,7 @@ func TestQuery(t *testing.T) {
tt.setupMock(mockApp)
}

service := &v2Service{mockApp.QueryHandlers(), mockApp}
service := &v2Service{mockApp.QueryHandlers(), mockApp.Query}
resp, err := service.Query(context.Background(), tt.request)

if tt.expectError {
Expand Down Expand Up @@ -195,7 +192,7 @@ func TestV2Service_ListQueryHandlers(t *testing.T) {
tt.setupMock(mockApp)
}

service := &v2Service{mockApp.QueryHandlers(), mockApp}
service := &v2Service{mockApp.QueryHandlers(), mockApp.Query}
resp, err := service.ListQueryHandlers(context.Background(), &ListQueryHandlersRequest{})

assert.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion server/v2/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ require (
cosmossdk.io/core v1.0.0-alpha.5
cosmossdk.io/core/testing v0.0.0-20240923163230-04da382a9f29
cosmossdk.io/log v1.4.1
cosmossdk.io/schema v0.3.1-0.20241010135032-192601639cac
cosmossdk.io/server/v2/appmanager v0.0.0-00010101000000-000000000000
cosmossdk.io/store/v2 v2.0.0-00010101000000-000000000000
github.com/cosmos/cosmos-proto v1.0.0-beta.5
Expand All @@ -43,6 +42,7 @@ require (

require (
cosmossdk.io/errors/v2 v2.0.0-20240731132947-df72853b3ca5 // indirect
cosmossdk.io/schema v0.3.1-0.20241010135032-192601639cac // indirect
github.com/DataDog/datadog-go v4.8.3+incompatible // indirect
github.com/DataDog/zstd v1.5.5 // indirect
github.com/Microsoft/go-winio v0.6.1 // indirect
Expand Down
30 changes: 10 additions & 20 deletions server/v2/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/stretchr/testify/require"

appmodulev2 "cosmossdk.io/core/appmodule/v2"
coreserver "cosmossdk.io/core/server"
"cosmossdk.io/core/transaction"
"cosmossdk.io/log"
serverv2 "cosmossdk.io/server/v2"
Expand All @@ -21,6 +20,14 @@ import (
storev2 "cosmossdk.io/store/v2"
)

type mockStore struct {
storev2.RootStore
}

func (*mockStore) Close() error {
return nil
}

type mockInterfaceRegistry struct{}

func (*mockInterfaceRegistry) Resolve(typeUrl string) (gogoproto.Message, error) {
Expand All @@ -32,22 +39,6 @@ func (*mockInterfaceRegistry) ListImplementations(ifaceTypeURL string) []string
}
func (*mockInterfaceRegistry) ListAllInterfaces() []string { panic("not implemented") }

type mockApp[T transaction.Tx] struct {
serverv2.AppI[T]
}

func (*mockApp[T]) QueryHandlers() map[string]appmodulev2.Handler {
return map[string]appmodulev2.Handler{}
}

func (*mockApp[T]) InterfaceRegistry() coreserver.InterfaceRegistry {
return &mockInterfaceRegistry{}
}

func (*mockApp[T]) Store() storev2.RootStore {
return nil
}

func TestServer(t *testing.T) {
currentDir, err := os.Getwd()
require.NoError(t, err)
Expand All @@ -62,12 +53,11 @@ func TestServer(t *testing.T) {
logger := log.NewLogger(os.Stdout)

ctx := serverv2.SetServerContext(context.Background(), v, logger)
app := &mockApp[transaction.Tx]{}

grpcServer, err := grpc.New[transaction.Tx](logger, app.InterfaceRegistry(), app.QueryHandlers(), app, cfg)
grpcServer, err := grpc.New[transaction.Tx](logger, &mockInterfaceRegistry{}, map[string]appmodulev2.Handler{}, nil, cfg)
require.NoError(t, err)

storeServer, err := store.New[transaction.Tx](app.Store(), cfg)
storeServer, err := store.New[transaction.Tx](&mockStore{}, cfg)
Comment on lines +57 to +60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance test coverage with additional test cases.

The current test implementation could be improved in several ways:

  1. Add negative test cases (e.g., invalid configurations)
  2. Verify server behavior and interactions
  3. Assert that mockStore is used correctly

Consider adding these test cases:

func TestServer_InvalidConfig(t *testing.T) {
    // Test server initialization with invalid config
}

func TestServer_StoreInteractions(t *testing.T) {
    // Test and verify store interactions
}

require.NoError(t, err)

mockServer := &mockServer{name: "mock-server-1", ch: make(chan string, 100)}
Expand Down
12 changes: 6 additions & 6 deletions server/v2/store/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,18 @@ const ServerName = "store"

// Server manages store config and contains prune & snapshot commands
type Server[T transaction.Tx] struct {
config *root.Config
backend storev2.Backend
config *root.Config
store storev2.RootStore
}

func New[T transaction.Tx](store storev2.Backend, cfg server.ConfigMap) (*Server[T], error) {
func New[T transaction.Tx](store storev2.RootStore, cfg server.ConfigMap) (*Server[T], error) {
config, err := UnmarshalConfig(cfg)
if err != nil {
return nil, err
}
return &Server[T]{
backend: store,
config: config,
store: store,
config: config,
}, nil
}

Expand All @@ -47,7 +47,7 @@ func (s *Server[T]) Start(context.Context) error {
}

func (s *Server[T]) Stop(context.Context) error {
return nil
return s.store.Close()
}

func (s *Server[T]) CLICommands() serverv2.CLIConfig {
Expand Down
26 changes: 0 additions & 26 deletions server/v2/types.go
View file
Open in desktop

This file was deleted.

9 changes: 0 additions & 9 deletions simapp/v2/app_di.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,15 +201,6 @@ func (app *SimApp[T]) Store() store.RootStore {
return app.store
}

// Close overwrites the base Close method to close the stores.
func (app *SimApp[T]) Close() error {
if err := app.store.Close(); err != nil {
return err
}

return app.App.Close()
}

func ProvideRootStoreConfig(config runtime.GlobalConfig) (*root.Config, error) {
return serverstore.UnmarshalConfig(config)
}
2 changes: 1 addition & 1 deletion simapp/v2/simdv2/cmd/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func InitRootCmd[T transaction.Tx](

// build full app!
simApp := deps.SimApp
grpcServer, err := grpc.New[T](logger, simApp.InterfaceRegistry(), simApp.QueryHandlers(), simApp, deps.GlobalConfig)
grpcServer, err := grpc.New[T](logger, simApp.InterfaceRegistry(), simApp.QueryHandlers(), simApp.Query, deps.GlobalConfig)
if err != nil {
return nil, err
}
Expand Down
Loading