Skip to content

Commit

Permalink
Added ErrMultiRegister on multiple Register calls
Browse files Browse the repository at this point in the history
  • Loading branch information
Fontinalis authored and jprobinson committed Mar 30, 2018
1 parent c2394f1 commit 912a07e
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 11 deletions.
10 changes: 10 additions & 0 deletions server/rpc_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
// RPCServer is an experimental server that serves a gRPC server on one
// port and the same endpoints via JSON on another port.
type RPCServer struct {
// tracks if the Register function is already called or not
registered bool

cfg *Config

// exit chan for graceful shutdown
Expand Down Expand Up @@ -67,6 +70,13 @@ func NewRPCServer(cfg *Config) *RPCServer {
// Register will attempt to register the given RPCService with the server.
// If any other types are passed, Register will panic.
func (r *RPCServer) Register(svc Service) error {
// check multiple register call error
if r.registered {
return ErrMultiRegister
}
// set registered to true because we called it
r.registered = true

rpcsvc, ok := svc.(RPCService)
if !ok {
Log.Fatalf("invalid service type for rpc server: %T", svc)
Expand Down
5 changes: 4 additions & 1 deletion server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (
"syscall"
"time"

"github.com/sirupsen/logrus"
"github.com/go-kit/kit/metrics/provider"
"github.com/nu7hatch/gouuid"
"github.com/sirupsen/logrus"

"github.com/NYTimes/gizmo/config/metrics"
"github.com/NYTimes/gizmo/web"
Expand All @@ -31,6 +31,9 @@ type Server interface {
}

var (
// ErrMultiRegister occurs when a Register method is called multiple times
ErrMultiRegister = errors.New("register method has been called multiple times")

// Name is used for status and logging.
Name = "nyt-awesome-go-server"
// Log is the global logger for the server. It will take care of logrotate
Expand Down
10 changes: 10 additions & 0 deletions server/simple_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
// SimpleServer is a basic http Server implementation for
// serving SimpleService, JSONService or MixedService implementations.
type SimpleServer struct {
// tracks if the Register function is already called or not
registered bool

cfg *Config

// exit chan for graceful shutdown
Expand Down Expand Up @@ -217,6 +220,13 @@ func (s *SimpleServer) Stop() error {

// Register will accept and register SimpleServer, JSONService or MixedService implementations.
func (s *SimpleServer) Register(svcI Service) error {
// check multiple register call error
if s.registered {
return ErrMultiRegister
}
// set registered to true because we called it
s.registered = true

s.svc = svcI
prefix := svcI.Prefix()
// quick fix for backwards compatibility
Expand Down
34 changes: 24 additions & 10 deletions server/simple_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,20 +367,34 @@ func TestFactory(*testing.T) {
}

func TestBasicRegistration(t *testing.T) {
s := NewSimpleServer(nil)
services := []Service{
&benchmarkSimpleService{},
&benchmarkJSONService{},
&testMixedService{},
&benchmarkContextService{},
tests := []struct {
server *SimpleServer
service Service
}{
{
NewSimpleServer(nil),
&benchmarkSimpleService{},
},
{
NewSimpleServer(nil),
&benchmarkJSONService{},
},
{
NewSimpleServer(nil),
&testMixedService{},
},
{
NewSimpleServer(nil),
&benchmarkContextService{},
},
}
for _, svc := range services {
if err := s.Register(svc); err != nil {
for _, test := range tests {
if err := test.server.Register(test.service); err != nil {
t.Errorf("Basic registration of services should not encounter an error: %s\n", err)
}
}

if err := s.Register(&testInvalidService{}); err == nil {
invServer := NewSimpleServer(nil)
if err := invServer.Register(&testInvalidService{}); err == nil {
t.Error("Invalid services should produce an error in service registration")
}
}
Expand Down

0 comments on commit 912a07e

Please sign in to comment.