Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MYST-143 tequila disconnect #71

Merged
merged 5 commits into from Dec 27, 2017
Merged

MYST-143 tequila disconnect #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ae79a92
disconnecting status set in connection manager
shroomist Dec 24, 2017
961651a
disconnecting endpoint test
shroomist Dec 26, 2017
59aaf4e
fix alignment
shroomist Dec 27, 2017
0accb27
Merge branch 'master' of https://github.com/MysteriumNetwork/node int…
shroomist Dec 27, 2017
a02864a
better encapsulation in tests
shroomist Dec 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions client_connection/interface.go
View file
Open in desktop
Expand Up @@ -8,9 +8,10 @@ import (
type State string

const (
NotConnected = State("NotConnected")
Connecting = State("Connecting")
Connected = State("Connected")
NotConnected = State("NotConnected")
Connecting = State("Connecting")
Connected = State("Connected")
Disconnecting = State("Disconnecting")
)

type ConnectionStatus struct {
Expand Down
6 changes: 6 additions & 0 deletions client_connection/manager.go
View file
Open in desktop
Expand Up @@ -82,6 +82,8 @@ func (manager *connectionManager) Status() ConnectionStatus {
}

func (manager *connectionManager) Disconnect() error {
manager.status = statusDisconnecting()
defer func() { manager.status = statusNotConnected() }()
manager.dialog.Close()
return manager.vpnClient.Stop()
}
Expand All @@ -106,6 +108,10 @@ func statusNotConnected() ConnectionStatus {
return ConnectionStatus{NotConnected, "", nil}
}

func statusDisconnecting() ConnectionStatus {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange to nullify sessions ID, while session still exists

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does session have any meaning on disconnecting state?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we dont have real case

return ConnectionStatus{Disconnecting, "", nil}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we using empty strings and not nil in session ids?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's go conventions. But on DTO layer I suggest to have mapping to null in JSON value

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Json encoder can automatically handle empty strings by not rendering fields at all. It's already done by the way.

}

func ConfigureVpnClientFactory(mysteriumApiClient server.Client, vpnClientRuntimeDirectory string) VpnClientFactory {
return func(vpnSession session.VpnSession) (openvpn.Client, error) {
vpnConfig, err := openvpn.NewClientConfigFromString(
Expand Down
38 changes: 37 additions & 1 deletion client_connection/manager_test.go
View file
Open in desktop
Expand Up @@ -20,6 +20,7 @@ type test_context struct {
connManager *connectionManager
fakeDiscoveryClient *server.ClientFake
fakeOpenVpn *fake_openvpn_client
fakeDialog *fake_dialog
}

func (tc *test_context) SetupTest() {
Expand All @@ -29,8 +30,10 @@ func (tc *test_context) SetupTest() {
serviceProposal := service_discovery.NewServiceProposal(identity.FromAddress("vpn-node-1"), dto.Contact{})
tc.fakeDiscoveryClient.NodeRegister(serviceProposal)

tc.fakeDialog = &fake_dialog{make(chan int, 1)}

dialogEstablisherFactory := func(identity identity.Identity) communication.DialogEstablisher {
return &fake_dialog{}
return tc.fakeDialog
}

tc.fakeOpenVpn = &fake_openvpn_client{make(chan int, 1), nil}
Expand Down Expand Up @@ -83,6 +86,33 @@ func (tc *test_context) TestStatusReportsConnectingWhenConnectionIsInProgress()
tc.fakeOpenVpn.resumeStart()
}

func (tc *test_context) TestStatusReportsDisconnectingThenNotConnected() {
tc.fakeOpenVpn.resumeStart()

err := tc.connManager.Connect(identity.FromAddress("identity-1"), "vpn-node-1")

assert.NoError(tc.T(), err)
assert.Equal(tc.T(), ConnectionStatus{Connected, "vpn-session-id", nil}, tc.connManager.Status())

wg := sync.WaitGroup{}
wg.Add(1)
go func() {
wg.Done()
tc.connManager.Disconnect()
}()
wg.Wait()

assert.Equal(tc.T(), ConnectionStatus{Disconnecting, "", nil}, tc.connManager.Status())
wg.Add(1)
go func() {
wg.Done()
tc.fakeDialog.resumeClose()
}()
wg.Wait()

assert.Equal(tc.T(), ConnectionStatus{NotConnected, "", nil}, tc.connManager.Status())
}

func TestConnectionManagerSuite(t *testing.T) {
suite.Run(t, new(test_context))
}
Expand Down Expand Up @@ -110,13 +140,19 @@ func (foc *fake_openvpn_client) Stop() error {
}

type fake_dialog struct {
closeDelay chan int
}

func (fd *fake_dialog) CreateDialog(contact dto.Contact) (communication.Dialog, error) {
return fd, nil
}

func (fd *fake_dialog) resumeClose() {
fd.closeDelay <- 1
}

func (fd *fake_dialog) Close() error {
<-fd.closeDelay
return nil
}

Expand Down
23 changes: 23 additions & 0 deletions tequilapi/endpoints/connection_test.go
View file
Open in desktop
Expand Up @@ -40,6 +40,29 @@ func (fm *fakeManager) Wait() error {
return nil
}

func TestDisconnectingState(t *testing.T) {
var fakeManager = fakeManager{}
fakeManager.onStatusReturn = client_connection.ConnectionStatus{
State: client_connection.Disconnecting,
SessionId: "",
}

connEndpoint := NewConnectionEndpoint(&fakeManager)
req, err := http.NewRequest(http.MethodGet, "/irrelevant", nil)
assert.Nil(t, err)
resp := httptest.NewRecorder()

connEndpoint.Status(resp, req, httprouter.Params{})

assert.Equal(t, http.StatusOK, resp.Code)
assert.JSONEq(
t,
`{
"status" : "Disconnecting"
}`,
resp.Body.String())
}

func TestNotConnectedStateIsReturnedWhenNoConnection(t *testing.T) {
var fakeManager = fakeManager{}
fakeManager.onStatusReturn = client_connection.ConnectionStatus{
Expand Down