From 61a7336a8c8c069443014c81e346ae792b267cc7 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Tue, 2 Jan 2018 10:52:48 +0200 Subject: [PATCH 1/3] Improve bytescount_client tests --- bytescount_client/middleware.go | 21 +++---- bytescount_client/middleware_test.go | 92 +++++++++++++++++++++++----- bytescount_client/stats_sender.go | 25 ++++++++ client_connection/manager.go | 3 +- 4 files changed, 112 insertions(+), 29 deletions(-) create mode 100644 bytescount_client/stats_sender.go diff --git a/bytescount_client/middleware.go b/bytescount_client/middleware.go index f7ba83066..cb1d33a49 100644 --- a/bytescount_client/middleware.go +++ b/bytescount_client/middleware.go @@ -3,8 +3,6 @@ package bytescount_client import ( "fmt" "github.com/mysterium/node/openvpn" - "github.com/mysterium/node/server" - "github.com/mysterium/node/server/dto" "github.com/mysterium/node/session" "net" "regexp" @@ -13,18 +11,18 @@ import ( ) type middleware struct { - mysteriumClient server.Client - interval time.Duration - sessionId session.SessionId + sessionStatsSender SessionStatsSender + interval time.Duration + sessionId session.SessionId connection net.Conn } -func NewMiddleware(mysteriumClient server.Client, sessionId session.SessionId, interval time.Duration) openvpn.ManagementMiddleware { +func NewMiddleware(sessionStatsSender SessionStatsSender, sessionId session.SessionId, interval time.Duration) openvpn.ManagementMiddleware { return &middleware{ - mysteriumClient: mysteriumClient, - interval: interval, - sessionId: sessionId, + sessionStatsSender: sessionStatsSender, + interval: interval, + sessionId: sessionId, connection: nil, } @@ -66,10 +64,7 @@ func (middleware *middleware) ConsumeLine(line string) (consumed bool, err error return } - err = middleware.mysteriumClient.SendSessionStats(string(middleware.sessionId), dto.SessionStats{ - BytesSent: bytesOut, - BytesReceived: bytesIn, - }) + err = middleware.sessionStatsSender.send(string(middleware.sessionId), bytesOut, bytesIn) return } diff --git a/bytescount_client/middleware_test.go b/bytescount_client/middleware_test.go index abfa902f9..a605a3bac 100644 --- a/bytescount_client/middleware_test.go +++ b/bytescount_client/middleware_test.go @@ -2,36 +2,96 @@ package bytescount_client import ( "errors" - "github.com/mysterium/node/server" "github.com/stretchr/testify/assert" + "net" "testing" "time" ) +type fakeConnection struct { + lastDataWritten []byte +} + +func (conn *fakeConnection) Read(b []byte) (int, error) { + return 0, nil +} + +func (conn *fakeConnection) Write(b []byte) (n int, err error) { + conn.lastDataWritten = b + return 0, nil +} + +func (conn *fakeConnection) Close() error { + return nil +} + +func (conn *fakeConnection) LocalAddr() net.Addr { + return nil +} + +func (conn *fakeConnection) RemoteAddr() net.Addr { + return nil +} + +func (conn *fakeConnection) SetDeadline(t time.Time) error { + return nil +} + +func (conn *fakeConnection) SetReadDeadline(t time.Time) error { + return nil +} + +func (conn *fakeConnection) SetWriteDeadline(t time.Time) error { + return nil +} + +type fakeStatsSender struct { + lastSessionId string + lastBytesSent, lastBytesReceived int +} + +func (sender *fakeStatsSender) send(sessionId string, bytesSent, bytesReceived int) error { + sender.lastSessionId = sessionId + sender.lastBytesSent = bytesSent + sender.lastBytesReceived = bytesReceived + return nil +} + func Test_Factory(t *testing.T) { - middleware := NewMiddleware(server.NewClientFake(), "session-test", 1*time.Minute) + middleware := NewMiddleware(&fakeStatsSender{}, "session-test", 1*time.Minute) assert.NotNil(t, middleware) } +func Test_Start(t *testing.T) { + statsSender := &fakeStatsSender{} + middleware := NewMiddleware(statsSender, "session-test", 1*time.Minute) + connection := &fakeConnection{} + middleware.Start(connection) + assert.Equal(t, []byte("bytecount 60\n"), connection.lastDataWritten) +} + func Test_ConsumeLine(t *testing.T) { var tests = []struct { - line string - expectedConsumed bool - expectedError error + line string + expectedConsumed bool + expectedError error + expectedBytesReceived int + expectedBytesSent int }{ - {">BYTECOUNT:3018,3264", true, nil}, - {">BYTECOUNT:0,3264", true, nil}, - {">BYTECOUNT:3018,", true, errors.New(`strconv.ParseInt: parsing "": invalid syntax`)}, - {">BYTECOUNT:,", true, errors.New(`strconv.ParseInt: parsing "": invalid syntax`)}, - {"OTHER", false, nil}, - {"BYTECOUNT", false, nil}, - {"BYTECOUNT:", false, nil}, - {"BYTECOUNT:3018,3264", false, nil}, - {">BYTECOUNTT:3018,3264", false, nil}, + {">BYTECOUNT:3018,3264", true, nil, 3018, 3264}, + {">BYTECOUNT:0,3264", true, nil, 0, 3264}, + {">BYTECOUNT:3018,", true, errors.New(`strconv.ParseInt: parsing "": invalid syntax`), 0, 0}, + {">BYTECOUNT:,", true, errors.New(`strconv.ParseInt: parsing "": invalid syntax`), 0, 0}, + {"OTHER", false, nil, 0, 0}, + {"BYTECOUNT", false, nil, 0, 0}, + {"BYTECOUNT:", false, nil, 0, 0}, + {"BYTECOUNT:3018,3264", false, nil, 0, 0}, + {">BYTECOUNTT:3018,3264", false, nil, 0, 0}, } - middleware := NewMiddleware(server.NewClientFake(), "session-test", 1*time.Minute) for _, test := range tests { + statsSender := &fakeStatsSender{} + middleware := NewMiddleware(statsSender, "session-test", 1*time.Minute) consumed, err := middleware.ConsumeLine(test.line) if test.expectedError != nil { assert.Error(t, test.expectedError, err.Error(), test.line) @@ -39,5 +99,7 @@ func Test_ConsumeLine(t *testing.T) { assert.NoError(t, err, test.line) } assert.Equal(t, test.expectedConsumed, consumed, test.line) + assert.Equal(t, test.expectedBytesReceived, statsSender.lastBytesReceived) + assert.Equal(t, test.expectedBytesSent, statsSender.lastBytesSent) } } diff --git a/bytescount_client/stats_sender.go b/bytescount_client/stats_sender.go new file mode 100644 index 000000000..5fb81417a --- /dev/null +++ b/bytescount_client/stats_sender.go @@ -0,0 +1,25 @@ +package bytescount_client + +import ( + "github.com/mysterium/node/server" + "github.com/mysterium/node/server/dto" +) + +type SessionStatsSender interface { + send(sessionId string, bytesSent, bytesReceived int) error +} + +type clientSessionStatsSender struct { + mysteriumClient server.Client +} + +func (sender *clientSessionStatsSender) send(sessionId string, bytesSent, bytesReceived int) error { + return sender.mysteriumClient.SendSessionStats(sessionId, dto.SessionStats{ + BytesSent: bytesSent, + BytesReceived: bytesReceived, + }) +} + +func NewSessionStatsSender(mysteriumClient server.Client) SessionStatsSender { + return &clientSessionStatsSender{mysteriumClient} +} diff --git a/client_connection/manager.go b/client_connection/manager.go index ea2a947f6..aa1e96854 100644 --- a/client_connection/manager.go +++ b/client_connection/manager.go @@ -121,8 +121,9 @@ func ConfigureVpnClientFactory(mysteriumApiClient server.Client, vpnClientRuntim return nil, err } + statsSender := bytescount_client.NewSessionStatsSender(mysteriumApiClient) vpnMiddlewares := []openvpn.ManagementMiddleware{ - bytescount_client.NewMiddleware(mysteriumApiClient, vpnSession.Id, 1*time.Minute), + bytescount_client.NewMiddleware(statsSender, vpnSession.Id, 1*time.Minute), } return openvpn.NewClient( vpnConfig, From 6a859b7a5a06394e56b067fd2261a69d7d229d78 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Wed, 3 Jan 2018 10:24:53 +0200 Subject: [PATCH 2/3] Extract sessionId out of middleware into stats sender --- bytescount_client/middleware.go | 7 ++----- bytescount_client/middleware_test.go | 10 ++++------ bytescount_client/stats_sender.go | 11 ++++++----- client_connection/manager.go | 4 ++-- 4 files changed, 14 insertions(+), 18 deletions(-) diff --git a/bytescount_client/middleware.go b/bytescount_client/middleware.go index cb1d33a49..10459e8d2 100644 --- a/bytescount_client/middleware.go +++ b/bytescount_client/middleware.go @@ -3,7 +3,6 @@ package bytescount_client import ( "fmt" "github.com/mysterium/node/openvpn" - "github.com/mysterium/node/session" "net" "regexp" "strconv" @@ -13,16 +12,14 @@ import ( type middleware struct { sessionStatsSender SessionStatsSender interval time.Duration - sessionId session.SessionId connection net.Conn } -func NewMiddleware(sessionStatsSender SessionStatsSender, sessionId session.SessionId, interval time.Duration) openvpn.ManagementMiddleware { +func NewMiddleware(sessionStatsSender SessionStatsSender, interval time.Duration) openvpn.ManagementMiddleware { return &middleware{ sessionStatsSender: sessionStatsSender, interval: interval, - sessionId: sessionId, connection: nil, } @@ -64,7 +61,7 @@ func (middleware *middleware) ConsumeLine(line string) (consumed bool, err error return } - err = middleware.sessionStatsSender.send(string(middleware.sessionId), bytesOut, bytesIn) + err = middleware.sessionStatsSender.send(bytesOut, bytesIn) return } diff --git a/bytescount_client/middleware_test.go b/bytescount_client/middleware_test.go index a605a3bac..8e3a147fd 100644 --- a/bytescount_client/middleware_test.go +++ b/bytescount_client/middleware_test.go @@ -46,25 +46,23 @@ func (conn *fakeConnection) SetWriteDeadline(t time.Time) error { } type fakeStatsSender struct { - lastSessionId string lastBytesSent, lastBytesReceived int } -func (sender *fakeStatsSender) send(sessionId string, bytesSent, bytesReceived int) error { - sender.lastSessionId = sessionId +func (sender *fakeStatsSender) send(bytesSent, bytesReceived int) error { sender.lastBytesSent = bytesSent sender.lastBytesReceived = bytesReceived return nil } func Test_Factory(t *testing.T) { - middleware := NewMiddleware(&fakeStatsSender{}, "session-test", 1*time.Minute) + middleware := NewMiddleware(&fakeStatsSender{}, 1*time.Minute) assert.NotNil(t, middleware) } func Test_Start(t *testing.T) { statsSender := &fakeStatsSender{} - middleware := NewMiddleware(statsSender, "session-test", 1*time.Minute) + middleware := NewMiddleware(statsSender, 1*time.Minute) connection := &fakeConnection{} middleware.Start(connection) assert.Equal(t, []byte("bytecount 60\n"), connection.lastDataWritten) @@ -91,7 +89,7 @@ func Test_ConsumeLine(t *testing.T) { for _, test := range tests { statsSender := &fakeStatsSender{} - middleware := NewMiddleware(statsSender, "session-test", 1*time.Minute) + middleware := NewMiddleware(statsSender, 1*time.Minute) consumed, err := middleware.ConsumeLine(test.line) if test.expectedError != nil { assert.Error(t, test.expectedError, err.Error(), test.line) diff --git a/bytescount_client/stats_sender.go b/bytescount_client/stats_sender.go index 5fb81417a..e2195cef6 100644 --- a/bytescount_client/stats_sender.go +++ b/bytescount_client/stats_sender.go @@ -6,20 +6,21 @@ import ( ) type SessionStatsSender interface { - send(sessionId string, bytesSent, bytesReceived int) error + send(bytesSent, bytesReceived int) error } type clientSessionStatsSender struct { mysteriumClient server.Client + sessionId string } -func (sender *clientSessionStatsSender) send(sessionId string, bytesSent, bytesReceived int) error { - return sender.mysteriumClient.SendSessionStats(sessionId, dto.SessionStats{ +func (sender *clientSessionStatsSender) send(bytesSent, bytesReceived int) error { + return sender.mysteriumClient.SendSessionStats(sender.sessionId, dto.SessionStats{ BytesSent: bytesSent, BytesReceived: bytesReceived, }) } -func NewSessionStatsSender(mysteriumClient server.Client) SessionStatsSender { - return &clientSessionStatsSender{mysteriumClient} +func NewSessionStatsSender(mysteriumClient server.Client, sessionId string) SessionStatsSender { + return &clientSessionStatsSender{mysteriumClient, sessionId} } diff --git a/client_connection/manager.go b/client_connection/manager.go index aa1e96854..57494aeac 100644 --- a/client_connection/manager.go +++ b/client_connection/manager.go @@ -121,9 +121,9 @@ func ConfigureVpnClientFactory(mysteriumApiClient server.Client, vpnClientRuntim return nil, err } - statsSender := bytescount_client.NewSessionStatsSender(mysteriumApiClient) + statsSender := bytescount_client.NewSessionStatsSender(mysteriumApiClient, string(vpnSession.Id)) vpnMiddlewares := []openvpn.ManagementMiddleware{ - bytescount_client.NewMiddleware(statsSender, vpnSession.Id, 1*time.Minute), + bytescount_client.NewMiddleware(statsSender, 1*time.Minute), } return openvpn.NewClient( vpnConfig, From 9bcceba64ab62338732707e3439c05c3062b3b87 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Wed, 3 Jan 2018 14:16:34 +0200 Subject: [PATCH 3/3] Refactor SessionStatsSender to function --- bytescount_client/middleware.go | 2 +- bytescount_client/middleware_test.go | 9 +++++---- bytescount_client/stats_sender.go | 25 +++++++++---------------- client_connection/manager.go | 2 +- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/bytescount_client/middleware.go b/bytescount_client/middleware.go index 10459e8d2..2e6d7451b 100644 --- a/bytescount_client/middleware.go +++ b/bytescount_client/middleware.go @@ -61,7 +61,7 @@ func (middleware *middleware) ConsumeLine(line string) (consumed bool, err error return } - err = middleware.sessionStatsSender.send(bytesOut, bytesIn) + err = middleware.sessionStatsSender(bytesOut, bytesIn) return } diff --git a/bytescount_client/middleware_test.go b/bytescount_client/middleware_test.go index 8e3a147fd..370a7f21a 100644 --- a/bytescount_client/middleware_test.go +++ b/bytescount_client/middleware_test.go @@ -56,13 +56,14 @@ func (sender *fakeStatsSender) send(bytesSent, bytesReceived int) error { } func Test_Factory(t *testing.T) { - middleware := NewMiddleware(&fakeStatsSender{}, 1*time.Minute) + statsSender := fakeStatsSender{} + middleware := NewMiddleware(statsSender.send, 1*time.Minute) assert.NotNil(t, middleware) } func Test_Start(t *testing.T) { - statsSender := &fakeStatsSender{} - middleware := NewMiddleware(statsSender, 1*time.Minute) + statsSender := fakeStatsSender{} + middleware := NewMiddleware(statsSender.send, 1*time.Minute) connection := &fakeConnection{} middleware.Start(connection) assert.Equal(t, []byte("bytecount 60\n"), connection.lastDataWritten) @@ -89,7 +90,7 @@ func Test_ConsumeLine(t *testing.T) { for _, test := range tests { statsSender := &fakeStatsSender{} - middleware := NewMiddleware(statsSender, 1*time.Minute) + middleware := NewMiddleware(statsSender.send, 1*time.Minute) consumed, err := middleware.ConsumeLine(test.line) if test.expectedError != nil { assert.Error(t, test.expectedError, err.Error(), test.line) diff --git a/bytescount_client/stats_sender.go b/bytescount_client/stats_sender.go index e2195cef6..72ef4fa85 100644 --- a/bytescount_client/stats_sender.go +++ b/bytescount_client/stats_sender.go @@ -3,24 +3,17 @@ package bytescount_client import ( "github.com/mysterium/node/server" "github.com/mysterium/node/server/dto" + "github.com/mysterium/node/session" ) -type SessionStatsSender interface { - send(bytesSent, bytesReceived int) error -} - -type clientSessionStatsSender struct { - mysteriumClient server.Client - sessionId string -} +type SessionStatsSender func(bytesSent, bytesReceived int) error -func (sender *clientSessionStatsSender) send(bytesSent, bytesReceived int) error { - return sender.mysteriumClient.SendSessionStats(sender.sessionId, dto.SessionStats{ - BytesSent: bytesSent, - BytesReceived: bytesReceived, +func NewSessionStatsSender(mysteriumClient server.Client, sessionId session.SessionId) SessionStatsSender { + sessionIdString := string(sessionId) + return SessionStatsSender(func(bytesSent, bytesReceived int) error { + return mysteriumClient.SendSessionStats(sessionIdString, dto.SessionStats{ + BytesSent: bytesSent, + BytesReceived: bytesReceived, + }) }) } - -func NewSessionStatsSender(mysteriumClient server.Client, sessionId string) SessionStatsSender { - return &clientSessionStatsSender{mysteriumClient, sessionId} -} diff --git a/client_connection/manager.go b/client_connection/manager.go index 57494aeac..3ffc121b1 100644 --- a/client_connection/manager.go +++ b/client_connection/manager.go @@ -121,7 +121,7 @@ func ConfigureVpnClientFactory(mysteriumApiClient server.Client, vpnClientRuntim return nil, err } - statsSender := bytescount_client.NewSessionStatsSender(mysteriumApiClient, string(vpnSession.Id)) + statsSender := bytescount_client.NewSessionStatsSender(mysteriumApiClient, vpnSession.Id) vpnMiddlewares := []openvpn.ManagementMiddleware{ bytescount_client.NewMiddleware(statsSender, 1*time.Minute), }