Throw an error on duplicate registration (#7729)
* Panic on registering a service twice * Panic if we register twice * Fix test * Fix test * Add clearer panic message * Add comment * Fix test
This commit is contained in:
parent
1a15713289
commit
9087ffc774
|
@ -54,13 +54,31 @@ func (qrt *GRPCQueryRouter) Route(path string) GRPCQueryHandler {
|
|||
}
|
||||
|
||||
// RegisterService implements the gRPC Server.RegisterService method. sd is a gRPC
|
||||
// service description, handler is an object which implements that gRPC service
|
||||
// service description, handler is an object which implements that gRPC service/
|
||||
//
|
||||
// This functions PANICS:
|
||||
// - if a protobuf service is registered twice.
|
||||
func (qrt *GRPCQueryRouter) RegisterService(sd *grpc.ServiceDesc, handler interface{}) {
|
||||
// adds a top-level query handler based on the gRPC service name
|
||||
for _, method := range sd.Methods {
|
||||
fqName := fmt.Sprintf("/%s/%s", sd.ServiceName, method.MethodName)
|
||||
methodHandler := method.Handler
|
||||
|
||||
// Check that each service is only registered once. If a service is
|
||||
// registered more than once, then we should error. Since we can't
|
||||
// return an error (`Server.RegisterService` interface restriction) we
|
||||
// panic (at startup).
|
||||
_, found := qrt.routes[fqName]
|
||||
if found {
|
||||
panic(
|
||||
fmt.Errorf(
|
||||
"gRPC query service %s has already been registered. Please make sure to only register each service once. "+
|
||||
"This usually means that there are conflicting modules registering the same gRPC query service",
|
||||
fqName,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
qrt.routes[fqName] = func(ctx sdk.Context, req abci.RequestQuery) (abci.ResponseQuery, error) {
|
||||
// call the method handler from the service description with the handler object,
|
||||
// a wrapped sdk.Context with proto-unmarshaled data from the ABCI request data
|
||||
|
|
|
@ -18,7 +18,7 @@ import (
|
|||
// service client.
|
||||
type QueryServiceTestHelper struct {
|
||||
*GRPCQueryRouter
|
||||
ctx sdk.Context
|
||||
Ctx sdk.Context
|
||||
}
|
||||
|
||||
var (
|
||||
|
@ -31,7 +31,7 @@ var (
|
|||
func NewQueryServerTestHelper(ctx sdk.Context, interfaceRegistry types.InterfaceRegistry) *QueryServiceTestHelper {
|
||||
qrt := NewGRPCQueryRouter()
|
||||
qrt.SetInterfaceRegistry(interfaceRegistry)
|
||||
return &QueryServiceTestHelper{GRPCQueryRouter: qrt, ctx: ctx}
|
||||
return &QueryServiceTestHelper{GRPCQueryRouter: qrt, Ctx: ctx}
|
||||
}
|
||||
|
||||
// Invoke implements the grpc ClientConn.Invoke method
|
||||
|
@ -45,7 +45,7 @@ func (q *QueryServiceTestHelper) Invoke(_ gocontext.Context, method string, args
|
|||
return err
|
||||
}
|
||||
|
||||
res, err := querier(q.ctx, abci.RequestQuery{Data: reqBz})
|
||||
res, err := querier(q.Ctx, abci.RequestQuery{Data: reqBz})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
|
@ -1,24 +1,29 @@
|
|||
package baseapp
|
||||
package baseapp_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
"github.com/tendermint/tendermint/libs/log"
|
||||
dbm "github.com/tendermint/tm-db"
|
||||
|
||||
"github.com/cosmos/cosmos-sdk/baseapp"
|
||||
"github.com/cosmos/cosmos-sdk/codec/types"
|
||||
"github.com/cosmos/cosmos-sdk/simapp"
|
||||
"github.com/cosmos/cosmos-sdk/testutil/testdata"
|
||||
sdk "github.com/cosmos/cosmos-sdk/types"
|
||||
)
|
||||
|
||||
func TestGRPCRouter(t *testing.T) {
|
||||
qr := NewGRPCQueryRouter()
|
||||
qr := baseapp.NewGRPCQueryRouter()
|
||||
interfaceRegistry := testdata.NewTestInterfaceRegistry()
|
||||
qr.SetInterfaceRegistry(interfaceRegistry)
|
||||
testdata.RegisterQueryServer(qr, testdata.QueryImpl{})
|
||||
helper := &QueryServiceTestHelper{
|
||||
helper := &baseapp.QueryServiceTestHelper{
|
||||
GRPCQueryRouter: qr,
|
||||
ctx: sdk.Context{}.WithContext(context.Background()),
|
||||
Ctx: sdk.Context{}.WithContext(context.Background()),
|
||||
}
|
||||
client := testdata.NewQueryClient(helper)
|
||||
|
||||
|
@ -44,3 +49,28 @@ func TestGRPCRouter(t *testing.T) {
|
|||
require.NotNil(t, res3)
|
||||
require.Equal(t, spot, res3.HasAnimal.Animal.GetCachedValue())
|
||||
}
|
||||
|
||||
func TestRegisterQueryServiceTwice(t *testing.T) {
|
||||
// Setup baseapp.
|
||||
db := dbm.NewMemDB()
|
||||
encCfg := simapp.MakeTestEncodingConfig()
|
||||
app := baseapp.NewBaseApp("test", log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, encCfg.TxConfig.TxDecoder())
|
||||
app.SetInterfaceRegistry(encCfg.InterfaceRegistry)
|
||||
testdata.RegisterInterfaces(encCfg.InterfaceRegistry)
|
||||
|
||||
// First time registering service shouldn't panic.
|
||||
require.NotPanics(t, func() {
|
||||
testdata.RegisterQueryServer(
|
||||
app.GRPCQueryRouter(),
|
||||
testdata.QueryImpl{},
|
||||
)
|
||||
})
|
||||
|
||||
// Second time should panic.
|
||||
require.Panics(t, func() {
|
||||
testdata.RegisterQueryServer(
|
||||
app.GRPCQueryRouter(),
|
||||
testdata.QueryImpl{},
|
||||
)
|
||||
})
|
||||
}
|
||||
|
|
|
@ -40,8 +40,10 @@ func (msr *MsgServiceRouter) Handler(methodName string) MsgServiceHandler {
|
|||
// RegisterService implements the gRPC Server.RegisterService method. sd is a gRPC
|
||||
// service description, handler is an object which implements that gRPC service.
|
||||
//
|
||||
// This function PANICs if it is called before the service `Msg`s have been
|
||||
// registered using RegisterInterfaces.
|
||||
// This function PANICs:
|
||||
// - if it is called before the service `Msg`s have been registered using
|
||||
// RegisterInterfaces,
|
||||
// - or if a service is being registered twice.
|
||||
func (msr *MsgServiceRouter) RegisterService(sd *grpc.ServiceDesc, handler interface{}) {
|
||||
// Adds a top-level query handler based on the gRPC service name.
|
||||
for _, method := range sd.Methods {
|
||||
|
@ -66,6 +68,21 @@ func (msr *MsgServiceRouter) RegisterService(sd *grpc.ServiceDesc, handler inter
|
|||
)
|
||||
}
|
||||
|
||||
// Check that each service is only registered once. If a service is
|
||||
// registered more than once, then we should error. Since we can't
|
||||
// return an error (`Server.RegisterService` interface restriction) we
|
||||
// panic (at startup).
|
||||
_, found := msr.routes[fqMethod]
|
||||
if found {
|
||||
panic(
|
||||
fmt.Errorf(
|
||||
"msg service %s has already been registered. Please make sure to only register each service once. "+
|
||||
"This usually means that there are conflicting modules registering the same msg service",
|
||||
fqMethod,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
msr.routes[fqMethod] = func(ctx sdk.Context, req sdk.MsgRequest) (*sdk.Result, error) {
|
||||
ctx = ctx.WithEventManager(sdk.NewEventManager())
|
||||
interceptor := func(goCtx context.Context, _ interface{}, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
|
||||
|
|
|
@ -18,7 +18,7 @@ import (
|
|||
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
|
||||
)
|
||||
|
||||
func TestRegisterService(t *testing.T) {
|
||||
func TestRegisterMsgService(t *testing.T) {
|
||||
db := dbm.NewMemDB()
|
||||
|
||||
// Create an encoding config that doesn't register testdata Msg services.
|
||||
|
@ -42,6 +42,31 @@ func TestRegisterService(t *testing.T) {
|
|||
})
|
||||
}
|
||||
|
||||
func TestRegisterMsgServiceTwice(t *testing.T) {
|
||||
// Setup baseapp.
|
||||
db := dbm.NewMemDB()
|
||||
encCfg := simapp.MakeTestEncodingConfig()
|
||||
app := baseapp.NewBaseApp("test", log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, encCfg.TxConfig.TxDecoder())
|
||||
app.SetInterfaceRegistry(encCfg.InterfaceRegistry)
|
||||
testdata.RegisterInterfaces(encCfg.InterfaceRegistry)
|
||||
|
||||
// First time registering service shouldn't panic.
|
||||
require.NotPanics(t, func() {
|
||||
testdata.RegisterMsgServer(
|
||||
app.MsgServiceRouter(),
|
||||
testdata.MsgServerImpl{},
|
||||
)
|
||||
})
|
||||
|
||||
// Second time should panic.
|
||||
require.Panics(t, func() {
|
||||
testdata.RegisterMsgServer(
|
||||
app.MsgServiceRouter(),
|
||||
testdata.MsgServerImpl{},
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
func TestMsgService(t *testing.T) {
|
||||
priv, _, _ := testdata.KeyTestPubAddr()
|
||||
encCfg := simapp.MakeTestEncodingConfig()
|
||||
|
|
|
@ -21,14 +21,9 @@ type IntegrationTestSuite struct {
|
|||
func (s *IntegrationTestSuite) SetupSuite() {
|
||||
app := simapp.Setup(false)
|
||||
|
||||
srv := reflection.NewReflectionServiceServer(app.InterfaceRegistry())
|
||||
|
||||
sdkCtx := app.BaseApp.NewContext(false, tmproto.Header{})
|
||||
queryHelper := baseapp.NewQueryServerTestHelper(sdkCtx, app.InterfaceRegistry())
|
||||
|
||||
reflection.RegisterReflectionServiceServer(queryHelper, srv)
|
||||
queryClient := reflection.NewReflectionServiceClient(queryHelper)
|
||||
|
||||
s.queryClient = queryClient
|
||||
}
|
||||
|
||||
|
|
|
@ -115,6 +115,11 @@ func (registry *interfaceRegistry) RegisterInterface(protoName string, iface int
|
|||
registry.RegisterImplementations(iface, impls...)
|
||||
}
|
||||
|
||||
// RegisterImplementations registers a concrete proto Message which implements
|
||||
// the given interface.
|
||||
//
|
||||
// This function PANICs if different concrete types are registered under the
|
||||
// same typeURL.
|
||||
func (registry *interfaceRegistry) RegisterImplementations(iface interface{}, impls ...proto.Message) {
|
||||
for _, impl := range impls {
|
||||
typeURL := "/" + proto.MessageName(impl)
|
||||
|
@ -122,10 +127,20 @@ func (registry *interfaceRegistry) RegisterImplementations(iface interface{}, im
|
|||
}
|
||||
}
|
||||
|
||||
// RegisterCustomTypeURL registers a concrete type which implements the given
|
||||
// interface under `typeURL`.
|
||||
//
|
||||
// This function PANICs if different concrete types are registered under the
|
||||
// same typeURL.
|
||||
func (registry *interfaceRegistry) RegisterCustomTypeURL(iface interface{}, typeURL string, impl proto.Message) {
|
||||
registry.registerImpl(iface, typeURL, impl)
|
||||
}
|
||||
|
||||
// registerImpl registers a concrete type which implements the given
|
||||
// interface under `typeURL`.
|
||||
//
|
||||
// This function PANICs if different concrete types are registered under the
|
||||
// same typeURL.
|
||||
func (registry *interfaceRegistry) registerImpl(iface interface{}, typeURL string, impl proto.Message) {
|
||||
ityp := reflect.TypeOf(iface).Elem()
|
||||
imap, found := registry.interfaceImpls[ityp]
|
||||
|
@ -138,6 +153,24 @@ func (registry *interfaceRegistry) registerImpl(iface interface{}, typeURL strin
|
|||
panic(fmt.Errorf("type %T doesn't actually implement interface %+v", impl, ityp))
|
||||
}
|
||||
|
||||
// Check if we already registered something under the given typeURL. It's
|
||||
// okay to register the same concrete type again, but if we are registering
|
||||
// a new concrete type under the same typeURL, then we throw an error (here,
|
||||
// we panic).
|
||||
foundImplType, found := imap[typeURL]
|
||||
if found && foundImplType != implType {
|
||||
panic(
|
||||
fmt.Errorf(
|
||||
"concrete type %s has already been registered under typeURL %s, cannot register %s under same typeURL. "+
|
||||
"This usually means that there are conflicting modules registering different concrete types "+
|
||||
"for a same interface implementation",
|
||||
foundImplType,
|
||||
typeURL,
|
||||
implType,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
imap[typeURL] = implType
|
||||
registry.typeURLMap[typeURL] = implType
|
||||
|
||||
|
|
|
@ -48,22 +48,61 @@ type TestI interface {
|
|||
DoSomething()
|
||||
}
|
||||
|
||||
// A struct that has the same typeURL as testdata.Dog, but is actually another
|
||||
// concrete type.
|
||||
type FakeDog struct{}
|
||||
|
||||
var (
|
||||
_ proto.Message = &FakeDog{}
|
||||
_ testdata.Animal = &FakeDog{}
|
||||
)
|
||||
|
||||
// dummy implementation of proto.Message and testdata.Animal
|
||||
func (dog FakeDog) Reset() {}
|
||||
func (dog FakeDog) String() string { return "fakedog" }
|
||||
func (dog FakeDog) ProtoMessage() {}
|
||||
func (dog FakeDog) XXX_MessageName() string { return proto.MessageName(&testdata.Dog{}) }
|
||||
func (dog FakeDog) Greet() string { return "fakedog" }
|
||||
|
||||
func TestRegister(t *testing.T) {
|
||||
registry := types.NewInterfaceRegistry()
|
||||
registry.RegisterInterface("Animal", (*testdata.Animal)(nil))
|
||||
registry.RegisterInterface("TestI", (*TestI)(nil))
|
||||
|
||||
// Happy path.
|
||||
require.NotPanics(t, func() {
|
||||
registry.RegisterImplementations((*testdata.Animal)(nil), &testdata.Dog{})
|
||||
})
|
||||
|
||||
// testdata.Dog doesn't implement TestI
|
||||
require.Panics(t, func() {
|
||||
registry.RegisterImplementations((*TestI)(nil), &testdata.Dog{})
|
||||
})
|
||||
|
||||
// nil proto message
|
||||
require.Panics(t, func() {
|
||||
registry.RegisterImplementations((*TestI)(nil), nil)
|
||||
})
|
||||
|
||||
// Not an interface.
|
||||
require.Panics(t, func() {
|
||||
registry.RegisterInterface("not_an_interface", (*testdata.Dog)(nil))
|
||||
})
|
||||
|
||||
// Duplicate registration with same concrete type.
|
||||
require.NotPanics(t, func() {
|
||||
registry.RegisterImplementations((*testdata.Animal)(nil), &testdata.Dog{})
|
||||
})
|
||||
|
||||
// Duplicate registration with different concrete type on same typeURL.
|
||||
require.PanicsWithError(
|
||||
t,
|
||||
"concrete type *testdata.Dog has already been registered under typeURL /testdata.Dog, cannot register *types_test.FakeDog under same typeURL. "+
|
||||
"This usually means that there are conflicting modules registering different concrete types for a same interface implementation",
|
||||
func() {
|
||||
registry.RegisterImplementations((*testdata.Animal)(nil), &FakeDog{})
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
func TestUnpackInterfaces(t *testing.T) {
|
||||
|
|
Loading…
Reference in New Issue