Skip to content

Commit

Permalink
Revert "Change hsConn to a map (#50)"
Browse files Browse the repository at this point in the history
This reverts commit 2d2faf9.
  • Loading branch information
Ryanfsdf authored Jul 15, 2020
1 parent 2d2faf9 commit 5bdcf31
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 35 deletions.
11 changes: 4 additions & 7 deletions security/s2a/internal/handshaker/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ import (
)

var (
// mu guards hsConnMap and hsDialer.
// hsConn represents a connection to the S2A handshaker service.
hsConn *grpc.ClientConn
// mu guards hsDialer.
mu sync.Mutex
// hsConnMap represents a mapping from an S2A handshaker service address
// to a corresponding connection to an S2A handshaker service instance.
hsConnMap = make(map[string]*grpc.ClientConn)
// hsDialer will be reassigned in tests.
hsDialer = grpc.Dial
)
Expand All @@ -41,16 +40,14 @@ func Dial(handshakerServiceAddress string) (*grpc.ClientConn, error) {
mu.Lock()
defer mu.Unlock()

hsConn, ok := hsConnMap[handshakerServiceAddress]
if !ok {
if hsConn == nil {
// Create a new connection to the S2A handshaker service. Note that
// this connection stays open until the application is closed.
var err error
hsConn, err = hsDialer(handshakerServiceAddress, grpc.WithInsecure())
if err != nil {
return nil, err
}
hsConnMap[handshakerServiceAddress] = hsConn
}
return hsConn, nil
}
42 changes: 14 additions & 28 deletions security/s2a/internal/handshaker/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import (
)

const (
testAddress = "test_address"
testAddress2 = "test_address_2"
// The address is irrelevant in this test.
testAddress = "some_address"
)

func TestDial(t *testing.T) {
Expand All @@ -40,44 +40,30 @@ func TestDial(t *testing.T) {
}
}()

// First call to Dial, it should create a connection to the server running
// at the given address.
// Ensure that hsConn is nil at first.
hsConn = nil

// First call to Dial, it should create set hsConn.
conn1, err := Dial(testAddress)
if err != nil {
t.Fatalf("first call to Dial(%v) failed: %v", testAddress, err)
t.Fatalf("first call to Dial failed: %v", err)
}
if conn1 == nil {
t.Fatalf("first call to Dial(%v)=(nil, _), want not nil", testAddress)
t.Fatal("first call to Dial(_)=(nil, _), want not nil")
}
if got, want := hsConnMap[testAddress], conn1; got != want {
t.Fatalf("hsConnmap[%v] = %v, want %v", testAddress, got, want)
if got, want := hsConn, conn1; got != want {
t.Fatalf("hsConn=%v, want %v", got, want)
}

// Second call to Dial should return conn1 above.
conn2, err := Dial(testAddress)
if err != nil {
t.Fatalf("second call to Dial(%v) failed: %v", testAddress, err)
t.Fatalf("second call to Dial(_) failed: %v", err)
}
if got, want := conn2, conn1; got != want {
t.Fatalf("second call to Dial(%v)=(%v, _), want (%v, _)", testAddress, got, want)
}
if got, want := hsConnMap[testAddress], conn1; got != want {
t.Fatalf("hsConnMap[%v] = %v, want %v", testAddress, got, want)
}

// Third call to Dial using a different address should create a new
// connection.
conn3, err := Dial(testAddress2)
if err != nil {
t.Fatalf("third call to Dial(%v) failed: %v", testAddress2, err)
}
if conn3 == nil {
t.Fatalf("third call to Dial(%v)=(nil, _), want not nil", testAddress)
}
if got, want := hsConnMap[testAddress2], conn3; got != want {
t.Fatalf("hsConnmap[%v] = %v, want %v", testAddress2, got, want)
t.Fatalf("second call to Dial(_)=(%v, _), want (%v,. _)", got, want)
}
if got, want := conn2 == conn3, false; got != want {
t.Fatalf("(conn2 == conn3) = %v, want %v", got, want)
if got, want := hsConn, conn1; got != want {
t.Fatalf("hsConn=%v, want %v", got, want)
}
}

0 comments on commit 5bdcf31

Please sign in to comment.