From 10d96216570419d13854e627ff3e93cb6aa9ab7f Mon Sep 17 00:00:00 2001 From: Evgeniy Kulikov Date: Wed, 27 Nov 2019 02:46:40 +0300 Subject: [PATCH] web: update servers + test coverage Signed-off-by: Evgeniy Kulikov --- web/servers.go | 127 +++++++++++++++++++++++++----------- web/servers_test.go | 156 +++++++++++++++++++++++++++++++++----------- 2 files changed, 207 insertions(+), 76 deletions(-) diff --git a/web/servers.go b/web/servers.go index f79e5da..f2fdbdc 100644 --- a/web/servers.go +++ b/web/servers.go @@ -4,12 +4,12 @@ import ( "net/http" "net/http/pprof" - "github.com/chapsuk/mserv" - "github.com/im-kulikov/helium/logger" "github.com/im-kulikov/helium/module" "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/spf13/viper" "go.uber.org/dig" + "go.uber.org/zap" + "google.golang.org/grpc" ) type ( @@ -18,7 +18,7 @@ type ( dig.In Config *viper.Viper - Logger logger.StdLogger + Logger *zap.Logger Handler http.Handler `optional:"true"` } @@ -26,31 +26,39 @@ type ( MultiServerParams struct { dig.In - Logger logger.StdLogger - Servers []mserv.Server `group:"web_server"` + Logger *zap.Logger + Servers []Service `group:"services"` } // ServerResult struct ServerResult struct { dig.Out - Server mserv.Server `group:"web_server"` + Server Service `group:"services"` } profileParams struct { dig.In - Handler http.Handler `name:"profile_handler" optional:"true"` + Logger *zap.Logger Viper *viper.Viper - Logger logger.StdLogger + Handler http.Handler `name:"profile_handler" optional:"true"` } metricParams struct { dig.In - Handler http.Handler `name:"metric_handler" optional:"true"` + Logger *zap.Logger Viper *viper.Viper - Logger logger.StdLogger + Handler http.Handler `name:"metric_handler" optional:"true"` + } + + grpcParams struct { + dig.In + + Viper *viper.Viper + Key string `name:"grpc_config" optional:"true"` + Server *grpc.Server `name:"grpc_server" optional:"true"` } ) @@ -65,11 +73,9 @@ var ( ) // NewMultiServer returns new multi servers group -func NewMultiServer(params MultiServerParams) mserv.Server { - return mserv.New(params.Servers...) -} +func NewMultiServer(p MultiServerParams) (Service, error) { return New(p.Logger, p.Servers...) } -func newProfileServer(p profileParams) ServerResult { +func newProfileServer(p profileParams) (ServerResult, error) { mux := http.NewServeMux() mux.HandleFunc("/debug/pprof/", pprof.Index) mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) @@ -82,7 +88,7 @@ func newProfileServer(p profileParams) ServerResult { return NewHTTPServer(p.Viper, "pprof", mux, p.Logger) } -func newMetricServer(p metricParams) ServerResult { +func newMetricServer(p metricParams) (ServerResult, error) { mux := http.NewServeMux() mux.Handle("/metrics", promhttp.Handler()) if p.Handler != nil { @@ -91,37 +97,80 @@ func newMetricServer(p metricParams) ServerResult { return NewHTTPServer(p.Viper, "metrics", mux, p.Logger) } +func newDefaultGRPCServer(p grpcParams) (ServerResult, error) { + if p.Server == nil || p.Viper.IsSet(p.Key+".disabled") { + return ServerResult{}, nil + } + + options := []GRPCOption{ + GRPCListenAddress(p.Viper.GetString(p.Key + ".address")), + GRPCShutdownTimeout(p.Viper.GetDuration(p.Key + ".shutdown_timeout")), + } + + if p.Viper.GetBool(p.Key + ".skip_errors") { + options = append(options, GRPCSkipErrors()) + } + + if p.Viper.IsSet(p.Key + ".network") { + options = append(options, GRPCListenNetwork(p.Viper.GetString(p.Key+".network"))) + } + + serve, err := NewGRPCService(p.Server, options...) + + return ServerResult{Server: serve}, err +} + // NewAPIServer creates api server by http.Handler from DI container -func NewAPIServer(v *viper.Viper, l logger.StdLogger, h http.Handler) ServerResult { +func NewAPIServer(v *viper.Viper, l *zap.Logger, h http.Handler) (ServerResult, error) { return NewHTTPServer(v, "api", h, l) } // NewHTTPServer creates http-server that will be embedded into multi-server -func NewHTTPServer(v *viper.Viper, key string, h http.Handler, l logger.StdLogger) ServerResult { +func NewHTTPServer(v *viper.Viper, key string, h http.Handler, l *zap.Logger) (ServerResult, error) { switch { case h == nil: - l.Printf("Empty handler for %s server, skip", key) - return ServerResult{} + l.Info("Empty handler, skip", + zap.String("name", key)) + return ServerResult{}, nil case v.GetBool(key + ".disabled"): - l.Printf("Server %s disabled", key) - return ServerResult{} + l.Info("Server disabled", + zap.String("name", key)) + return ServerResult{}, nil case !v.IsSet(key + ".address"): - l.Printf("Empty bind address for %s server, skip", key) - return ServerResult{} - } - - l.Printf("Create %s http server, bind address: %s", key, v.GetString(key+".address")) - return ServerResult{ - Server: mserv.NewHTTPServer( - &http.Server{ - Addr: v.GetString(key + ".address"), - Handler: h, - ReadTimeout: v.GetDuration(key + ".read_timeout"), - ReadHeaderTimeout: v.GetDuration(key + ".read_header_timeout"), - WriteTimeout: v.GetDuration(key + ".write_timeout"), - IdleTimeout: v.GetDuration(key + ".idle_timeout"), - MaxHeaderBytes: v.GetInt(key + ".max_header_bytes"), - }, - mserv.HTTPShutdownTimeout(v.GetDuration(key+".shutdown_timeout")), - )} + l.Info("Empty bind address, skip", + zap.String("name", key)) + return ServerResult{}, nil + } + + options := []HTTPOption{ + HTTPListenAddress(v.GetString(key + ".address")), + HTTPShutdownTimeout(v.GetDuration(key + ".shutdown_timeout")), + } + + if v.IsSet(key + ".network") { + options = append(options, HTTPListenNetwork(v.GetString(key+".network"))) + } + + if v.IsSet(key + ".skip_errors") { + options = append(options, HTTPSkipErrors()) + } + + serve, err := NewHTTPService( + &http.Server{ + Handler: h, + Addr: v.GetString(key + ".address"), + ReadTimeout: v.GetDuration(key + ".read_timeout"), + ReadHeaderTimeout: v.GetDuration(key + ".read_header_timeout"), + WriteTimeout: v.GetDuration(key + ".write_timeout"), + IdleTimeout: v.GetDuration(key + ".idle_timeout"), + MaxHeaderBytes: v.GetInt(key + ".max_header_bytes"), + }, + options..., + ) + + l.Info("Creates http server", + zap.String("name", key), + zap.String("address", v.GetString(key+".address"))) + + return ServerResult{Server: serve}, err } diff --git a/web/servers_test.go b/web/servers_test.go index 15e2f9b..b8fffd5 100644 --- a/web/servers_test.go +++ b/web/servers_test.go @@ -5,16 +5,47 @@ import ( "net" "net/http" "testing" + "time" - "github.com/chapsuk/mserv" - "github.com/im-kulikov/helium/logger" "github.com/im-kulikov/helium/module" "github.com/spf13/viper" "github.com/stretchr/testify/require" "go.uber.org/dig" "go.uber.org/zap" + "golang.org/x/net/context" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + gt "google.golang.org/grpc/test/grpc_testing" ) +type ( + testGRPC struct { + gt.TestServiceServer + } + + grpcResult struct { + dig.Out + + Config string `name:"grpc_config"` + Server *grpc.Server `name:"grpc_server"` + } +) + +var ( + _ = ListenerSkipErrors + _ = ListenerIgnoreError + _ = ListenerShutdownTimeout +) + +func (t testGRPC) EmptyCall(context.Context, *gt.Empty) (*gt.Empty, error) { + return new(gt.Empty), nil +} + +func (t testGRPC) UnaryCall(context.Context, *gt.SimpleRequest) (*gt.SimpleResponse, error) { + return nil, status.Error(codes.AlreadyExists, codes.AlreadyExists.String()) +} + var expectResult = []byte("OK") func testHTTPHandler(assert *require.Assertions) http.Handler { @@ -26,12 +57,17 @@ func testHTTPHandler(assert *require.Assertions) http.Handler { return mux } +func testGRPCServer(_ *require.Assertions) *grpc.Server { + s := grpc.NewServer() + gt.RegisterTestServiceServer(s, testGRPC{}) + return s +} + func TestServers(t *testing.T) { var ( - z = zap.L() + l = zap.L() di = dig.New() v = viper.New() - l = logger.NewStdLogger(z) ) t.Run("check pprof server", func(t *testing.T) { @@ -40,7 +76,8 @@ func TestServers(t *testing.T) { Viper: v, Logger: l, } - serve := newProfileServer(params) + serve, err := newProfileServer(params) + require.NoError(t, err) require.Nil(t, serve.Server) }) @@ -50,9 +87,10 @@ func TestServers(t *testing.T) { Viper: v, Logger: l, } - serve := newProfileServer(params) + serve, err := newProfileServer(params) + require.NoError(t, err) require.NotNil(t, serve.Server) - require.IsType(t, &mserv.HTTPServer{}, serve.Server) + require.IsType(t, &httpService{}, serve.Server) }) }) @@ -62,7 +100,8 @@ func TestServers(t *testing.T) { Viper: v, Logger: l, } - serve := newMetricServer(params) + serve, err := newMetricServer(params) + require.NoError(t, err) require.Nil(t, serve.Server) }) @@ -72,9 +111,10 @@ func TestServers(t *testing.T) { Viper: v, Logger: l, } - serve := newMetricServer(params) + serve, err := newMetricServer(params) + require.NoError(t, err) require.NotNil(t, serve.Server) - require.IsType(t, &mserv.HTTPServer{}, serve.Server) + require.IsType(t, &httpService{}, serve.Server) }) }) @@ -86,32 +126,34 @@ func TestServers(t *testing.T) { z, err := zap.NewDevelopment() is.NoError(err) - l := logger.NewStdLogger(z) - testHTTPHandler(is) - serve := NewHTTPServer(v, "test-api", testHTTPHandler(is), l) + serve, err := NewHTTPServer(v, "test-api", testHTTPHandler(is), z) + require.NoError(t, err) is.Nil(serve.Server) }) t.Run("check api server", func(t *testing.T) { t.Run("without config", func(t *testing.T) { - serve := NewAPIServer(v, l, nil) + serve, err := NewAPIServer(v, l, nil) + require.NoError(t, err) require.Nil(t, serve.Server) }) t.Run("without handler", func(t *testing.T) { v.SetDefault("api.address", ":8090") - serve := NewAPIServer(v, l, nil) + serve, err := NewAPIServer(v, l, nil) + require.NoError(t, err) require.Nil(t, serve.Server) }) t.Run("should be ok", func(t *testing.T) { assert := require.New(t) v.SetDefault("api.address", ":8090") - serve := NewAPIServer(v, l, testHTTPHandler(assert)) + serve, err := NewAPIServer(v, l, testHTTPHandler(assert)) + assert.NoError(err) assert.NotNil(serve.Server) - assert.IsType(&mserv.HTTPServer{}, serve.Server) + assert.IsType(&httpService{}, serve.Server) }) }) @@ -123,6 +165,7 @@ func TestServers(t *testing.T) { "pprof.address": nil, "metrics.address": nil, "api.address": nil, + "grpc.address": nil, } ) @@ -131,14 +174,20 @@ func TestServers(t *testing.T) { servers[name], err = net.Listen("tcp", "127.0.0.1:0") assert.NoError(err) assert.NoError(servers[name].Close()) - v.SetDefault(name, servers[name].Addr().String()) } mod := module.Module{ + {Constructor: newDefaultGRPCServer}, + {Constructor: func() *zap.Logger { return l }}, {Constructor: func() *viper.Viper { return v }}, - {Constructor: func() logger.StdLogger { return l }}, {Constructor: func() http.Handler { return testHTTPHandler(assert) }}, + {Constructor: func() grpcResult { + return grpcResult{ + Config: "grpc", + Server: testGRPCServer(assert), + } + }}, { Constructor: func() http.Handler { return testHTTPHandler(assert) }, @@ -155,30 +204,63 @@ func TestServers(t *testing.T) { assert.NoError(module.Provide(di, mod)) - err = di.Invoke(func(serve mserv.Server) { - assert.IsType(&mserv.MultiServer{}, serve) - + err = di.Invoke(func(serve Service) { + assert.NotNil(serve) assert.NoError(serve.Start()) - }) - assert.NoError(err) - for name, lis := range servers { - { - t.Logf("check for %q on %q", name, lis.Addr()) + for name, lis := range servers { + t.Run(name, func(t *testing.T) { + t.Logf("check for %q on %q", name, lis.Addr()) + + switch name { + case "grpc.address": + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + + conn, err := grpc.DialContext(ctx, lis.Addr().String(), + grpc.WithBlock(), + grpc.WithInsecure()) + + assert.NoError(err) + + cli := gt.NewTestServiceClient(conn) - resp, err := http.Get("http://" + lis.Addr().String() + "/test") - assert.NoError(err) + { // EmptyCall + res, err := cli.EmptyCall(ctx, >.Empty{}) + assert.NoError(err) + assert.NotNil(res) + } - defer func() { - err := resp.Body.Close() - assert.NoError(err) - }() + { // UnaryCall + res, err := cli.UnaryCall(ctx, >.SimpleRequest{}) + assert.Nil(res) + assert.Error(err) - data, err := ioutil.ReadAll(resp.Body) - assert.NoError(err) + st, ok := status.FromError(err) + assert.True(ok) + assert.Equal(codes.AlreadyExists, st.Code()) + assert.Equal(codes.AlreadyExists.String(), st.Message()) + } - assert.Equal(expectResult, data) + default: + resp, err := http.Get("http://" + lis.Addr().String() + "/test") + assert.NoError(err) + + defer func() { + err := resp.Body.Close() + assert.NoError(err) + }() + + data, err := ioutil.ReadAll(resp.Body) + assert.NoError(err) + + assert.Equal(expectResult, data) + } + }) } - } + + assert.NoError(serve.Stop()) + }) + assert.NoError(err) }) }