Repository: go-kratos/kratos
Pull request #1729 (2021-12-29 13:04:25)

feat(transport):add grpc and http with listen
<!--
🎉 Thanks for sending a pull request to Kratos! Here are some tips for you:

1. If this is your first time contributing to Kratos, please read our contribution guide: https://go-kratos.dev/en/docs/community/contribution/
2. Ensure you have added or ran the appropriate tests and lint for your PR, please use `make lint` and `make test` before filing your PR, use `make clean` to tidy your go mod.
3. If the PR is unfinished, you may need mark it as a WIP(Work In Progress) PR or draft PR
4. Please use a conventional commits format title: `<type>[optional scope]: <description>`
    
    Some suggestion on <type>:

    fix: A bug fix
    feat: A new feature
    test: Adding missing tests or correcting existing tests
    refactor: A code change that neither fixes a bug nor adds a feature
    break: Changes has break change

    docs: Documentation only changes
    deps: Changes external dependencies
    style: Changes that do not affect the meaning of the code (white-space, formatting, etc)
    chore Daily work, examples, etc.
    ci: Changes to our CI configuration files and scripts
-->

#### Description (what this PR does / why we need it):
<!--
* The description should include the motivation for this PR or contrast this with previous behavior
-->


#### Which issue(s) this PR fixes (resolves / be part of):
<!--
* Automatically closes linked issue when PR is merged.
* If you PR is not fully resolved issue, please use `part of #<issue number>` instead.

Usage: `fixes/resolves #<issue number>`, or `fixes/resolves (paste link of issue)`.
-->
fixes #

#### Other special notes for reviewer:
<!--
* Somethings that need extra attention for reviewer
* Some additional notes, TODO list, etc.
-->


codecov-commenter: # [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/1729?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) Report
> Merging [#1729](https://codecov.io/gh/go-kratos/kratos/pull/1729?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (80ff403) into [main](https://codecov.io/gh/go-kratos/kratos/commit/b4213892270edb9274d9fd7360c1032292d5483c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (b421389) will **decrease** coverage by `0.04%`.
> The diff coverage is `56.25%`.

[![Impacted file tree graph](https://codecov.io/gh/go-kratos/kratos/pull/1729/graphs/tree.svg?width=650&height=150&src=pr&token=XwnmBTz78x&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)](https://codecov.io/gh/go-kratos/kratos/pull/1729?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)

```diff
@@            Coverage Diff             @@
##             main    #1729      +/-   ##
==========================================
- Coverage   81.17%   81.12%   -0.05%     
==========================================
  Files          78       78              
  Lines        3468     3476       +8     
==========================================
+ Hits         2815     2820       +5     
- Misses        449      452       +3     
  Partials      204      204              
```


| [Impacted Files](https://codecov.io/gh/go-kratos/kratos/pull/1729?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) | Coverage Δ | |
|---|---|---|
| [transport/http/server.go](https://codecov.io/gh/go-kratos/kratos/pull/1729/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-dHJhbnNwb3J0L2h0dHAvc2VydmVyLmdv) | `75.00% <50.00%> (ø)` | |
| [transport/grpc/server.go](https://codecov.io/gh/go-kratos/kratos/pull/1729/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-dHJhbnNwb3J0L2dycGMvc2VydmVyLmdv) | `90.47% <62.50%> (-1.61%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/go-kratos/kratos/pull/1729?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/1729?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Last update [b421389...80ff403](https://codecov.io/gh/go-kratos/kratos/pull/1729?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos).

letian0805: Hey! This is the third PR about the side effects of Listener and Endpoint, right?
The first one is https://github.com/go-kratos/kratos/pull/1470
haiyux: > Hey! This is the third PR about the side effects of Listener and Endpoint, right? The first one is #1470

Your PR does not eliminate the side effects of endpoint, it still listen in the endpoint function.
letian0805: > > Hey! This is the third PR about the side effects of Listener and Endpoint, right? The first one is #1470
> 
> Your PR does not eliminate the side effects of endpoint, it still listen in the endpoint function.

Yes i know, so longxboy created another one to fix side effects. https://github.com/go-kratos/kratos/pull/1497
haiyux: feat #1714

Patch:
diff --git a/transport/grpc/server.go b/transport/grpc/server.go
index 748302c2..0ae458d6 100644
--- a/transport/grpc/server.go
+++ b/transport/grpc/server.go
@@ -5,7 +5,6 @@ import (
 	"crypto/tls"
 	"net"
 	"net/url"
-	"sync"
 	"time"
 
 	"github.com/go-kratos/kratos/v2/internal/endpoint"
@@ -75,6 +74,13 @@ func TLSConfig(c *tls.Config) ServerOption {
 	}
 }
 
+// Listener with server lis
+func Listener(lis net.Listener) ServerOption {
+	return func(s *Server) {
+		s.lis = lis
+	}
+}
+
 // UnaryInterceptor returns a ServerOption that sets the UnaryServerInterceptor for the server.
 func UnaryInterceptor(in ...grpc.UnaryServerInterceptor) ServerOption {
 	return func(s *Server) {
@@ -95,7 +101,6 @@ type Server struct {
 	baseCtx    context.Context
 	tlsConf    *tls.Config
 	lis        net.Listener
-	once       sync.Once
 	err        error
 	network    string
 	address    string
@@ -139,6 +144,8 @@ func NewServer(opts ...ServerOption) *Server {
 	}
 	srv.Server = grpc.NewServer(grpcOpts...)
 	srv.metadata = apimd.NewServer(srv.Server)
+	// listen and endpoint
+	srv.err = srv.listenAndEndpoint()
 	// internal register
 	grpc_health_v1.RegisterHealthServer(srv.Server, srv.health)
 	apimd.RegisterMetadataServer(srv.Server, srv.metadata)
@@ -150,21 +157,6 @@ func NewServer(opts ...ServerOption) *Server {
 // examples:
 //   grpc://127.0.0.1:9000?isSecure=false
 func (s *Server) Endpoint() (*url.URL, error) {
-	s.once.Do(func() {
-		lis, err := net.Listen(s.network, s.address)
-		if err != nil {
-			s.err = err
-			return
-		}
-		addr, err := host.Extract(s.address, lis)
-		if err != nil {
-			_ = lis.Close()
-			s.err = err
-			return
-		}
-		s.lis = lis
-		s.endpoint = endpoint.NewEndpoint("grpc", addr, s.tlsConf != nil)
-	})
 	if s.err != nil {
 		return nil, s.err
 	}
@@ -173,8 +165,8 @@ func (s *Server) Endpoint() (*url.URL, error) {
 
 // Start start the gRPC server.
 func (s *Server) Start(ctx context.Context) error {
-	if _, err := s.Endpoint(); err != nil {
-		return err
+	if s.err != nil {
+		return s.err
 	}
 	s.baseCtx = ctx
 	s.log.Infof("[gRPC] server listening on: %s", s.lis.Addr().String())
@@ -190,6 +182,23 @@ func (s *Server) Stop(ctx context.Context) error {
 	return nil
 }
 
+func (s *Server) listenAndEndpoint() error {
+	if s.lis == nil {
+		lis, err := net.Listen(s.network, s.address)
+		if err != nil {
+			return err
+		}
+		s.lis = lis
+	}
+	addr, err := host.Extract(s.address, s.lis)
+	if err != nil {
+		_ = s.lis.Close()
+		return err
+	}
+	s.endpoint = endpoint.NewEndpoint("grpc", addr, s.tlsConf != nil)
+	return nil
+}
+
 func (s *Server) unaryServerInterceptor() grpc.UnaryServerInterceptor {
 	return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
 		ctx, cancel := ic.Merge(ctx, s.baseCtx)
diff --git a/transport/http/server.go b/transport/http/server.go
index da8a29fe..f30268cf 100644
--- a/transport/http/server.go
+++ b/transport/http/server.go
@@ -7,7 +7,6 @@ import (
 	"net"
 	"net/http"
 	"net/url"
-	"sync"
 	"time"
 
 	"github.com/go-kratos/kratos/v2/internal/endpoint"
@@ -107,12 +106,18 @@ func StrictSlash(strictSlash bool) ServerOption {
 	}
 }
 
+// Listener with server lis
+func Listener(lis net.Listener) ServerOption {
+	return func(s *Server) {
+		s.lis = lis
+	}
+}
+
 // Server is an HTTP server wrapper.
 type Server struct {
 	*http.Server
 	lis         net.Listener
 	tlsConf     *tls.Config
-	once        sync.Once
 	endpoint    *url.URL
 	err         error
 	network     string
@@ -149,6 +154,7 @@ func NewServer(opts ...ServerOption) *Server {
 		Handler:   FilterChain(srv.filters...)(srv.router),
 		TLSConfig: srv.tlsConf,
 	}
+	srv.err = srv.listenAndEndpoint()
 	return srv
 }
 
@@ -219,22 +225,6 @@ func (s *Server) filter() mux.MiddlewareFunc {
 // examples:
 //   http://127.0.0.1:8000?isSecure=false
 func (s *Server) Endpoint() (*url.URL, error) {
-	s.once.Do(func() {
-		lis, err := net.Listen(s.network, s.address)
-		if err != nil {
-			s.err = err
-			return
-		}
-		addr, err := host.Extract(s.address, lis)
-		if err != nil {
-			lis.Close()
-			s.err = err
-			return
-		}
-		s.lis = lis
-
-		s.endpoint = endpoint.NewEndpoint("http", addr, s.tlsConf != nil)
-	})
 	if s.err != nil {
 		return nil, s.err
 	}
@@ -243,8 +233,8 @@ func (s *Server) Endpoint() (*url.URL, error) {
 
 // Start start the HTTP server.
 func (s *Server) Start(ctx context.Context) error {
-	if _, err := s.Endpoint(); err != nil {
-		return err
+	if s.err != nil {
+		return s.err
 	}
 	s.BaseContext = func(net.Listener) context.Context {
 		return ctx
@@ -267,3 +257,20 @@ func (s *Server) Stop(ctx context.Context) error {
 	s.log.Info("[HTTP] server stopping")
 	return s.Shutdown(ctx)
 }
+
+func (s *Server) listenAndEndpoint() error {
+	if s.lis == nil {
+		lis, err := net.Listen(s.network, s.address)
+		if err != nil {
+			return err
+		}
+		s.lis = lis
+	}
+	addr, err := host.Extract(s.address, s.lis)
+	if err != nil {
+		_ = s.lis.Close()
+		return err
+	}
+	s.endpoint = endpoint.NewEndpoint("http", addr, s.tlsConf != nil)
+	return nil
+}


Test patch:
diff --git a/transport/grpc/server_test.go b/transport/grpc/server_test.go
index 87c3483c..14079ce4 100644
--- a/transport/grpc/server_test.go
+++ b/transport/grpc/server_test.go
@@ -117,11 +117,9 @@ func TestNetwork(t *testing.T) {
 }
 
 func TestAddress(t *testing.T) {
-	o := &Server{}
 	v := "abc"
-	Address(v)(o)
+	o := NewServer(Address(v))
 	assert.Equal(t, v, o.address)
-
 	u, err := o.Endpoint()
 	assert.NotNil(t, err)
 	assert.Nil(t, u)