From e028a0dd0de180b39e9ba5c03b0ff37c4429e944 Mon Sep 17 00:00:00 2001 From: Ignas Bernotas Date: Mon, 15 Apr 2019 15:18:22 +0300 Subject: [PATCH 01/12] NAT status endpoint --- Gopkg.lock | 3 + cmd/di.go | 1 + tequilapi/endpoints/nat.go | 95 ++++++++++++++++++++++++++ tequilapi/endpoints/nat_test.go | 115 ++++++++++++++++++++++++++++++++ 4 files changed, 214 insertions(+) create mode 100644 tequilapi/endpoints/nat.go create mode 100644 tequilapi/endpoints/nat_test.go diff --git a/Gopkg.lock b/Gopkg.lock index 3900ed7a2f..6c95f81ff2 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -607,6 +607,9 @@ "github.com/ethereum/go-ethereum/p2p/nat", "github.com/ethereum/go-ethereum/params", "github.com/gofrs/uuid", + "github.com/huin/goupnp", + "github.com/huin/goupnp/httpu", + "github.com/huin/goupnp/ssdp", "github.com/jackpal/gateway", "github.com/julienschmidt/httprouter", "github.com/mdlayher/wireguardctrl", diff --git a/cmd/di.go b/cmd/di.go index 5f44b4a128..1de813bffc 100644 --- a/cmd/di.go +++ b/cmd/di.go @@ -344,6 +344,7 @@ func (di *Dependencies) bootstrapNodeComponents(nodeOptions node.Options) { tequilapi_endpoints.AddRoutesForServiceSessions(router, di.ServiceSessionStorage) tequilapi_endpoints.AddRoutesForPayout(router, di.IdentityManager, di.SignerFactory, di.MysteriumAPI) tequilapi_endpoints.AddRoutesForAccessPolicies(router, nodeOptions.AccessPolicyEndpointAddress) + tequilapi_endpoints.AddRoutesForNat(router, di.NATTracker) identity_registry.AddIdentityRegistrationEndpoint(router, di.IdentityRegistration, di.IdentityRegistry) corsPolicy := tequilapi.NewMysteriumCorsPolicy() diff --git a/tequilapi/endpoints/nat.go b/tequilapi/endpoints/nat.go new file mode 100644 index 0000000000..4e08051f1d --- /dev/null +++ b/tequilapi/endpoints/nat.go @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2019 The "MysteriumNetwork/node" Authors. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package endpoints + +import ( + "net/http" + + "github.com/julienschmidt/httprouter" + "github.com/mysteriumnetwork/node/nat/traversal" + "github.com/mysteriumnetwork/node/tequilapi/utils" +) + +// swagger:model NatStatusDTO +type NatStatusDTO struct { + Successful bool `json:"successful"` + Error string `json:"error"` +} + +type NatEvents interface { + LastEvent() *traversal.Event +} + +// ServiceEndpoint struct represents management of service resource and it's sub-resources +type NatEndpoint struct { + natEvents NatEvents +} + +// NewNatEndpoint creates and returns nat endpoint +func NewNatEndpoint(natEvents NatEvents) *NatEndpoint { + return &NatEndpoint{ + natEvents: natEvents, + } +} + +// NatStatus provides NAT configuration info +// swagger:operation GET /nat/status Nat NatStatusDTO +// --- +// summary: Shows NAT status +// description: Nat status returns the last known NAT event +// responses: +// 200: +// description: NAT status and/or error +// schema: +// "$ref": "#/definitions/NatStatusDTO" +// 204: +// description: No status available +func (ne *NatEndpoint) NatStatus(resp http.ResponseWriter, _ *http.Request, _ httprouter.Params) { + event := ne.natEvents.LastEvent() + + statusResponse := toNatStatusResponse(event) + if statusResponse == nil { + utils.SendErrorMessage(resp, "No status is available", http.StatusNoContent) + return + } + + utils.WriteAsJSON(statusResponse, resp) +} + +// AddRoutesForService adds service routes to given router +func AddRoutesForNat(router *httprouter.Router, natEvents NatEvents) { + natEndpoint := NewNatEndpoint(natEvents) + + router.GET("/nat/status", natEndpoint.NatStatus) +} + +func toNatStatusResponse(event *traversal.Event) *NatStatusDTO { + if event == nil { + return nil + } + + status := event.Successful + var error string + if event.Error != nil { + error = event.Error.Error() + } + return &NatStatusDTO{ + Successful: status, + Error: error, + } +} diff --git a/tequilapi/endpoints/nat_test.go b/tequilapi/endpoints/nat_test.go new file mode 100644 index 0000000000..06b4c64e29 --- /dev/null +++ b/tequilapi/endpoints/nat_test.go @@ -0,0 +1,115 @@ +/* + * Copyright (C) 2019 The "MysteriumNetwork/node" Authors. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package endpoints + +import ( + "encoding/json" + "github.com/mysteriumnetwork/node/nat/traversal" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "net/http" + "net/http/httptest" + "testing" +) + +var ( + failedEvent = traversal.Event{ + Stage: "hole_punching", + Successful: false, + Error: errors.New("no UPnP or NAT-PMP router discovered"), + } + successfulEvent = traversal.Event{ + Stage: "hole_punching", + Successful: true, + } +) + +func Test_NatEndpoint_Status_WhenStatusIsNotAvailable(t *testing.T) { + expected := errorMessage{Message: "No status is available"} + + testErrorResponse(t, natTrackerMock{hasNoEvent: true}, &expected) +} + +func Test_NatEndpoint_Status_WhenStatusIsFailed(t *testing.T) { + expected := toNatStatusResponse(&failedEvent) + + testResponse(t, natTrackerMock{}, expected) +} + +func Test_NatEndpoint_Status_WhenStatusIsSuccessful(t *testing.T) { + expected := toNatStatusResponse(&successfulEvent) + + testResponse(t, natTrackerMock{hasSuccessfulEvent: true}, expected) +} + +func testErrorResponse(t *testing.T, mockedTracker natTrackerMock, expected interface{}) { + resp, err := makeStatusRequestAndReturnResponse(t, mockedTracker) + assert.Nil(t, err) + + parsedResponse := &errorMessage{} + err = json.Unmarshal(resp.Body.Bytes(), parsedResponse) + assert.Nil(t, err) + assert.EqualValues(t, expected, parsedResponse) +} + +func testResponse(t *testing.T, mockedTracker natTrackerMock, expected interface{}) { + resp, err := makeStatusRequestAndReturnResponse(t, mockedTracker) + assert.Nil(t, err) + + parsedResponse := &NatStatusDTO{} + err = json.Unmarshal(resp.Body.Bytes(), parsedResponse) + assert.Nil(t, err) + assert.EqualValues(t, expected, parsedResponse) +} + +func makeStatusRequestAndReturnResponse(t *testing.T, mockedTracker natTrackerMock) (*httptest.ResponseRecorder, error) { + req, err := http.NewRequest( + http.MethodGet, + "/irrelevant", + nil, + ) + if err != nil { + return nil, err + } + + resp := httptest.NewRecorder() + handlerFunc := NewNatEndpoint(&mockedTracker).NatStatus + handlerFunc(resp, req, nil) + + return resp, nil +} + +type natTrackerMock struct { + hasSuccessfulEvent bool + hasNoEvent bool +} + +type errorMessage struct { + Message string `json:"message"` +} + +func (nt *natTrackerMock) LastEvent() *traversal.Event { + if nt.hasSuccessfulEvent { + return &successfulEvent + } + if nt.hasNoEvent { + return nil + } + + return &failedEvent +} From 725b6ad5a458b2119ef20c4a327c86c2ec4019c5 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Wed, 24 Apr 2019 15:07:49 +0300 Subject: [PATCH 02/12] Fix linter errors, refactor --- tequilapi/endpoints/nat.go | 6 ++-- tequilapi/endpoints/nat_test.go | 63 ++++++++++++++------------------- 2 files changed, 30 insertions(+), 39 deletions(-) diff --git a/tequilapi/endpoints/nat.go b/tequilapi/endpoints/nat.go index 4e08051f1d..6601a98ad1 100644 --- a/tequilapi/endpoints/nat.go +++ b/tequilapi/endpoints/nat.go @@ -25,17 +25,19 @@ import ( "github.com/mysteriumnetwork/node/tequilapi/utils" ) +// NatStatusDTO gives information about NAT traversal success or failure // swagger:model NatStatusDTO type NatStatusDTO struct { Successful bool `json:"successful"` Error string `json:"error"` } +// NatEvents allows retrieving last traversal event type NatEvents interface { LastEvent() *traversal.Event } -// ServiceEndpoint struct represents management of service resource and it's sub-resources +// NatEndpoint struct represents endpoints about NAT traversal type NatEndpoint struct { natEvents NatEvents } @@ -71,7 +73,7 @@ func (ne *NatEndpoint) NatStatus(resp http.ResponseWriter, _ *http.Request, _ ht utils.WriteAsJSON(statusResponse, resp) } -// AddRoutesForService adds service routes to given router +// AddRoutesForNat adds nat routes to given router func AddRoutesForNat(router *httprouter.Router, natEvents NatEvents) { natEndpoint := NewNatEndpoint(natEvents) diff --git a/tequilapi/endpoints/nat_test.go b/tequilapi/endpoints/nat_test.go index 06b4c64e29..44f2d157a7 100644 --- a/tequilapi/endpoints/nat_test.go +++ b/tequilapi/endpoints/nat_test.go @@ -19,65 +19,62 @@ package endpoints import ( "encoding/json" - "github.com/mysteriumnetwork/node/nat/traversal" - "github.com/pkg/errors" - "github.com/stretchr/testify/assert" "net/http" "net/http/httptest" "testing" + + "github.com/mysteriumnetwork/node/nat/traversal" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" ) var ( - failedEvent = traversal.Event{ - Stage: "hole_punching", - Successful: false, - Error: errors.New("no UPnP or NAT-PMP router discovered"), - } - successfulEvent = traversal.Event{ - Stage: "hole_punching", - Successful: true, - } + failedEvent = traversal.BuildFailureEvent( + "hole_punching", + errors.New("no UPnP or NAT-PMP router discovered"), + ) + successfulEvent = traversal.BuildSuccessEvent("hole_punching") ) +func Test_NatEndpoint_Status_WhenStatusIsSuccessful(t *testing.T) { + expected := toNatStatusResponse(&successfulEvent) + + testResponse(t, natTrackerMock{mockLastEvent: &successfulEvent}, expected) +} + func Test_NatEndpoint_Status_WhenStatusIsNotAvailable(t *testing.T) { expected := errorMessage{Message: "No status is available"} - testErrorResponse(t, natTrackerMock{hasNoEvent: true}, &expected) + testErrorResponse(t, natTrackerMock{mockLastEvent: nil}, &expected) } func Test_NatEndpoint_Status_WhenStatusIsFailed(t *testing.T) { expected := toNatStatusResponse(&failedEvent) - testResponse(t, natTrackerMock{}, expected) + testResponse(t, natTrackerMock{mockLastEvent: &failedEvent}, expected) } -func Test_NatEndpoint_Status_WhenStatusIsSuccessful(t *testing.T) { - expected := toNatStatusResponse(&successfulEvent) - - testResponse(t, natTrackerMock{hasSuccessfulEvent: true}, expected) -} - -func testErrorResponse(t *testing.T, mockedTracker natTrackerMock, expected interface{}) { - resp, err := makeStatusRequestAndReturnResponse(t, mockedTracker) +func testResponse(t *testing.T, mockedTracker natTrackerMock, expected interface{}) { + resp, err := makeStatusRequestAndReturnResponse(mockedTracker) assert.Nil(t, err) - parsedResponse := &errorMessage{} + parsedResponse := &NatStatusDTO{} err = json.Unmarshal(resp.Body.Bytes(), parsedResponse) assert.Nil(t, err) assert.EqualValues(t, expected, parsedResponse) } -func testResponse(t *testing.T, mockedTracker natTrackerMock, expected interface{}) { - resp, err := makeStatusRequestAndReturnResponse(t, mockedTracker) +func testErrorResponse(t *testing.T, mockedTracker natTrackerMock, expected interface{}) { + resp, err := makeStatusRequestAndReturnResponse(mockedTracker) assert.Nil(t, err) - parsedResponse := &NatStatusDTO{} + parsedResponse := &errorMessage{} err = json.Unmarshal(resp.Body.Bytes(), parsedResponse) assert.Nil(t, err) assert.EqualValues(t, expected, parsedResponse) } -func makeStatusRequestAndReturnResponse(t *testing.T, mockedTracker natTrackerMock) (*httptest.ResponseRecorder, error) { +func makeStatusRequestAndReturnResponse(mockedTracker natTrackerMock) (*httptest.ResponseRecorder, error) { req, err := http.NewRequest( http.MethodGet, "/irrelevant", @@ -95,8 +92,7 @@ func makeStatusRequestAndReturnResponse(t *testing.T, mockedTracker natTrackerMo } type natTrackerMock struct { - hasSuccessfulEvent bool - hasNoEvent bool + mockLastEvent *traversal.Event } type errorMessage struct { @@ -104,12 +100,5 @@ type errorMessage struct { } func (nt *natTrackerMock) LastEvent() *traversal.Event { - if nt.hasSuccessfulEvent { - return &successfulEvent - } - if nt.hasNoEvent { - return nil - } - - return &failedEvent + return nt.mockLastEvent } From a24e4c6cdf8bcfc8a48b3b93373b9692eff96215 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Thu, 25 Apr 2019 13:21:09 +0300 Subject: [PATCH 03/12] Refactor tests --- tequilapi/endpoints/nat_test.go | 73 +++++++++++++++++---------------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/tequilapi/endpoints/nat_test.go b/tequilapi/endpoints/nat_test.go index 44f2d157a7..7611023ce1 100644 --- a/tequilapi/endpoints/nat_test.go +++ b/tequilapi/endpoints/nat_test.go @@ -23,70 +23,75 @@ import ( "net/http/httptest" "testing" + "github.com/julienschmidt/httprouter" "github.com/mysteriumnetwork/node/nat/traversal" "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) -var ( - failedEvent = traversal.BuildFailureEvent( - "hole_punching", - errors.New("no UPnP or NAT-PMP router discovered"), - ) - successfulEvent = traversal.BuildSuccessEvent("hole_punching") -) - -func Test_NatEndpoint_Status_WhenStatusIsSuccessful(t *testing.T) { - expected := toNatStatusResponse(&successfulEvent) +func Test_NatStatus_ReturnsSuccess_WithSuccessfulEvent(t *testing.T) { + successfulEvent := traversal.BuildSuccessEvent("hole_punching") - testResponse(t, natTrackerMock{mockLastEvent: &successfulEvent}, expected) + testResponse( + t, + natTrackerMock{mockLastEvent: &successfulEvent}, + `{ + "successful": true, + "error": "" + }`, + ) } -func Test_NatEndpoint_Status_WhenStatusIsNotAvailable(t *testing.T) { - expected := errorMessage{Message: "No status is available"} +func Test_NatStatus_ReturnsFailureWithError_WithFailureEvent(t *testing.T) { + failureEvent := traversal.BuildFailureEvent("hole_punching", errors.New("mock error")) - testErrorResponse(t, natTrackerMock{mockLastEvent: nil}, &expected) + testResponse( + t, + natTrackerMock{mockLastEvent: &failureEvent}, + `{ + "successful": false, + "error": "mock error" + }`, + ) } -func Test_NatEndpoint_Status_WhenStatusIsFailed(t *testing.T) { - expected := toNatStatusResponse(&failedEvent) - - testResponse(t, natTrackerMock{mockLastEvent: &failedEvent}, expected) +func Test_NatStatus_ReturnsError_WhenEventIsNotAvailable(t *testing.T) { + testErrorResponse( + t, + natTrackerMock{mockLastEvent: nil}, + `{ + "message": "No status is available" + }`, + ) } -func testResponse(t *testing.T, mockedTracker natTrackerMock, expected interface{}) { +func testResponse(t *testing.T, mockedTracker natTrackerMock, expectedJson string) { resp, err := makeStatusRequestAndReturnResponse(mockedTracker) assert.Nil(t, err) parsedResponse := &NatStatusDTO{} err = json.Unmarshal(resp.Body.Bytes(), parsedResponse) assert.Nil(t, err) - assert.EqualValues(t, expected, parsedResponse) + assert.JSONEq(t, expectedJson, resp.Body.String()) } -func testErrorResponse(t *testing.T, mockedTracker natTrackerMock, expected interface{}) { +func testErrorResponse(t *testing.T, mockedTracker natTrackerMock, expectedJson string) { resp, err := makeStatusRequestAndReturnResponse(mockedTracker) assert.Nil(t, err) - parsedResponse := &errorMessage{} - err = json.Unmarshal(resp.Body.Bytes(), parsedResponse) - assert.Nil(t, err) - assert.EqualValues(t, expected, parsedResponse) + assert.JSONEq(t, expectedJson, resp.Body.String()) } func makeStatusRequestAndReturnResponse(mockedTracker natTrackerMock) (*httptest.ResponseRecorder, error) { - req, err := http.NewRequest( - http.MethodGet, - "/irrelevant", - nil, - ) + req, err := http.NewRequest(http.MethodGet, "/nat/status", nil) if err != nil { return nil, err } resp := httptest.NewRecorder() - handlerFunc := NewNatEndpoint(&mockedTracker).NatStatus - handlerFunc(resp, req, nil) + router := httprouter.New() + AddRoutesForNat(router, &mockedTracker) + router.ServeHTTP(resp, req) return resp, nil } @@ -95,10 +100,6 @@ type natTrackerMock struct { mockLastEvent *traversal.Event } -type errorMessage struct { - Message string `json:"message"` -} - func (nt *natTrackerMock) LastEvent() *traversal.Event { return nt.mockLastEvent } From 480cb72e00bf6cf63f28b302cb7e3253760ab486 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Thu, 25 Apr 2019 13:25:26 +0300 Subject: [PATCH 04/12] Refactor NAT naming to golang style --- cmd/di.go | 2 +- tequilapi/endpoints/nat.go | 46 ++++++++++++++++----------------- tequilapi/endpoints/nat_test.go | 10 +++---- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/cmd/di.go b/cmd/di.go index 1de813bffc..0510df647a 100644 --- a/cmd/di.go +++ b/cmd/di.go @@ -344,7 +344,7 @@ func (di *Dependencies) bootstrapNodeComponents(nodeOptions node.Options) { tequilapi_endpoints.AddRoutesForServiceSessions(router, di.ServiceSessionStorage) tequilapi_endpoints.AddRoutesForPayout(router, di.IdentityManager, di.SignerFactory, di.MysteriumAPI) tequilapi_endpoints.AddRoutesForAccessPolicies(router, nodeOptions.AccessPolicyEndpointAddress) - tequilapi_endpoints.AddRoutesForNat(router, di.NATTracker) + tequilapi_endpoints.AddRoutesForNAT(router, di.NATTracker) identity_registry.AddIdentityRegistrationEndpoint(router, di.IdentityRegistration, di.IdentityRegistry) corsPolicy := tequilapi.NewMysteriumCorsPolicy() diff --git a/tequilapi/endpoints/nat.go b/tequilapi/endpoints/nat.go index 6601a98ad1..2e861788f5 100644 --- a/tequilapi/endpoints/nat.go +++ b/tequilapi/endpoints/nat.go @@ -25,46 +25,46 @@ import ( "github.com/mysteriumnetwork/node/tequilapi/utils" ) -// NatStatusDTO gives information about NAT traversal success or failure -// swagger:model NatStatusDTO -type NatStatusDTO struct { +// NATStatusDTO gives information about NAT traversal success or failure +// swagger:model NATStatusDTO +type NATStatusDTO struct { Successful bool `json:"successful"` Error string `json:"error"` } -// NatEvents allows retrieving last traversal event -type NatEvents interface { +// NATEvents allows retrieving last traversal event +type NATEvents interface { LastEvent() *traversal.Event } -// NatEndpoint struct represents endpoints about NAT traversal -type NatEndpoint struct { - natEvents NatEvents +// NATEndpoint struct represents endpoints about NAT traversal +type NATEndpoint struct { + natEvents NATEvents } -// NewNatEndpoint creates and returns nat endpoint -func NewNatEndpoint(natEvents NatEvents) *NatEndpoint { - return &NatEndpoint{ +// NewNATEndpoint creates and returns nat endpoint +func NewNATEndpoint(natEvents NATEvents) *NATEndpoint { + return &NATEndpoint{ natEvents: natEvents, } } -// NatStatus provides NAT configuration info -// swagger:operation GET /nat/status Nat NatStatusDTO +// NATStatus provides NAT configuration info +// swagger:operation GET /nat/status NAT NATStatusDTO // --- // summary: Shows NAT status -// description: Nat status returns the last known NAT event +// description: NAT status returns the last known NAT event // responses: // 200: // description: NAT status and/or error // schema: -// "$ref": "#/definitions/NatStatusDTO" +// "$ref": "#/definitions/NATStatusDTO" // 204: // description: No status available -func (ne *NatEndpoint) NatStatus(resp http.ResponseWriter, _ *http.Request, _ httprouter.Params) { +func (ne *NATEndpoint) NATStatus(resp http.ResponseWriter, _ *http.Request, _ httprouter.Params) { event := ne.natEvents.LastEvent() - statusResponse := toNatStatusResponse(event) + statusResponse := toNATStatusResponse(event) if statusResponse == nil { utils.SendErrorMessage(resp, "No status is available", http.StatusNoContent) return @@ -73,14 +73,14 @@ func (ne *NatEndpoint) NatStatus(resp http.ResponseWriter, _ *http.Request, _ ht utils.WriteAsJSON(statusResponse, resp) } -// AddRoutesForNat adds nat routes to given router -func AddRoutesForNat(router *httprouter.Router, natEvents NatEvents) { - natEndpoint := NewNatEndpoint(natEvents) +// AddRoutesForNAT adds nat routes to given router +func AddRoutesForNAT(router *httprouter.Router, natEvents NATEvents) { + natEndpoint := NewNATEndpoint(natEvents) - router.GET("/nat/status", natEndpoint.NatStatus) + router.GET("/nat/status", natEndpoint.NATStatus) } -func toNatStatusResponse(event *traversal.Event) *NatStatusDTO { +func toNATStatusResponse(event *traversal.Event) *NATStatusDTO { if event == nil { return nil } @@ -90,7 +90,7 @@ func toNatStatusResponse(event *traversal.Event) *NatStatusDTO { if event.Error != nil { error = event.Error.Error() } - return &NatStatusDTO{ + return &NATStatusDTO{ Successful: status, Error: error, } diff --git a/tequilapi/endpoints/nat_test.go b/tequilapi/endpoints/nat_test.go index 7611023ce1..0409294314 100644 --- a/tequilapi/endpoints/nat_test.go +++ b/tequilapi/endpoints/nat_test.go @@ -29,7 +29,7 @@ import ( "github.com/stretchr/testify/assert" ) -func Test_NatStatus_ReturnsSuccess_WithSuccessfulEvent(t *testing.T) { +func Test_NATStatus_ReturnsSuccess_WithSuccessfulEvent(t *testing.T) { successfulEvent := traversal.BuildSuccessEvent("hole_punching") testResponse( @@ -42,7 +42,7 @@ func Test_NatStatus_ReturnsSuccess_WithSuccessfulEvent(t *testing.T) { ) } -func Test_NatStatus_ReturnsFailureWithError_WithFailureEvent(t *testing.T) { +func Test_NATStatus_ReturnsFailureWithError_WithFailureEvent(t *testing.T) { failureEvent := traversal.BuildFailureEvent("hole_punching", errors.New("mock error")) testResponse( @@ -55,7 +55,7 @@ func Test_NatStatus_ReturnsFailureWithError_WithFailureEvent(t *testing.T) { ) } -func Test_NatStatus_ReturnsError_WhenEventIsNotAvailable(t *testing.T) { +func Test_NATStatus_ReturnsError_WhenEventIsNotAvailable(t *testing.T) { testErrorResponse( t, natTrackerMock{mockLastEvent: nil}, @@ -69,7 +69,7 @@ func testResponse(t *testing.T, mockedTracker natTrackerMock, expectedJson strin resp, err := makeStatusRequestAndReturnResponse(mockedTracker) assert.Nil(t, err) - parsedResponse := &NatStatusDTO{} + parsedResponse := &NATStatusDTO{} err = json.Unmarshal(resp.Body.Bytes(), parsedResponse) assert.Nil(t, err) assert.JSONEq(t, expectedJson, resp.Body.String()) @@ -90,7 +90,7 @@ func makeStatusRequestAndReturnResponse(mockedTracker natTrackerMock) (*httptest resp := httptest.NewRecorder() router := httprouter.New() - AddRoutesForNat(router, &mockedTracker) + AddRoutesForNAT(router, &mockedTracker) router.ServeHTTP(resp, req) return resp, nil From d3258ecb4c0a0facebe6fc7f9ea628ee849ce8ea Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Thu, 25 Apr 2019 13:39:00 +0300 Subject: [PATCH 05/12] Return not_finished/successful/failure NAT status --- tequilapi/endpoints/nat.go | 40 ++++++++++++++++----------------- tequilapi/endpoints/nat_test.go | 26 ++++++--------------- 2 files changed, 27 insertions(+), 39 deletions(-) diff --git a/tequilapi/endpoints/nat.go b/tequilapi/endpoints/nat.go index 2e861788f5..b2cee25a36 100644 --- a/tequilapi/endpoints/nat.go +++ b/tequilapi/endpoints/nat.go @@ -25,11 +25,17 @@ import ( "github.com/mysteriumnetwork/node/tequilapi/utils" ) +const ( + statusNotFinished = "not_finished" + statusSuccessful = "successful" + statusFailure = "failure" +) + // NATStatusDTO gives information about NAT traversal success or failure // swagger:model NATStatusDTO type NATStatusDTO struct { - Successful bool `json:"successful"` - Error string `json:"error"` + Status string `json:"status"` + Error *string `json:"error,omitempty"` } // NATEvents allows retrieving last traversal event @@ -53,23 +59,16 @@ func NewNATEndpoint(natEvents NATEvents) *NATEndpoint { // swagger:operation GET /nat/status NAT NATStatusDTO // --- // summary: Shows NAT status -// description: NAT status returns the last known NAT event +// description: NAT status returns the last known NAT traversal status // responses: // 200: -// description: NAT status and/or error +// description: NAT status ("not_finished"/"successful"/"failed") and optionally error if status is "failed" // schema: // "$ref": "#/definitions/NATStatusDTO" -// 204: -// description: No status available func (ne *NATEndpoint) NATStatus(resp http.ResponseWriter, _ *http.Request, _ httprouter.Params) { event := ne.natEvents.LastEvent() statusResponse := toNATStatusResponse(event) - if statusResponse == nil { - utils.SendErrorMessage(resp, "No status is available", http.StatusNoContent) - return - } - utils.WriteAsJSON(statusResponse, resp) } @@ -80,18 +79,19 @@ func AddRoutesForNAT(router *httprouter.Router, natEvents NATEvents) { router.GET("/nat/status", natEndpoint.NATStatus) } -func toNATStatusResponse(event *traversal.Event) *NATStatusDTO { +func toNATStatusResponse(event *traversal.Event) NATStatusDTO { if event == nil { - return nil + return NATStatusDTO{Status: statusNotFinished} } - status := event.Successful - var error string - if event.Error != nil { - error = event.Error.Error() + if event.Successful { + return NATStatusDTO{Status: statusSuccessful} } - return &NATStatusDTO{ - Successful: status, - Error: error, + + var error *string + if event.Error != nil { + msg := event.Error.Error() + error = &msg } + return NATStatusDTO{Status: statusFailure, Error: error} } diff --git a/tequilapi/endpoints/nat_test.go b/tequilapi/endpoints/nat_test.go index 0409294314..d29ec94116 100644 --- a/tequilapi/endpoints/nat_test.go +++ b/tequilapi/endpoints/nat_test.go @@ -18,7 +18,6 @@ package endpoints import ( - "encoding/json" "net/http" "net/http/httptest" "testing" @@ -29,38 +28,37 @@ import ( "github.com/stretchr/testify/assert" ) -func Test_NATStatus_ReturnsSuccess_WithSuccessfulEvent(t *testing.T) { +func Test_NATStatus_ReturnsStatusSuccessful_WithSuccessfulEvent(t *testing.T) { successfulEvent := traversal.BuildSuccessEvent("hole_punching") testResponse( t, natTrackerMock{mockLastEvent: &successfulEvent}, `{ - "successful": true, - "error": "" + "status": "successful" }`, ) } -func Test_NATStatus_ReturnsFailureWithError_WithFailureEvent(t *testing.T) { +func Test_NATStatus_ReturnsStatusFailureAndError_WithFailureEvent(t *testing.T) { failureEvent := traversal.BuildFailureEvent("hole_punching", errors.New("mock error")) testResponse( t, natTrackerMock{mockLastEvent: &failureEvent}, `{ - "successful": false, + "status": "failure", "error": "mock error" }`, ) } -func Test_NATStatus_ReturnsError_WhenEventIsNotAvailable(t *testing.T) { - testErrorResponse( +func Test_NATStatus_ReturnsStatusNotFinished_WhenEventIsNotAvailable(t *testing.T) { + testResponse( t, natTrackerMock{mockLastEvent: nil}, `{ - "message": "No status is available" + "status": "not_finished" }`, ) } @@ -69,16 +67,6 @@ func testResponse(t *testing.T, mockedTracker natTrackerMock, expectedJson strin resp, err := makeStatusRequestAndReturnResponse(mockedTracker) assert.Nil(t, err) - parsedResponse := &NATStatusDTO{} - err = json.Unmarshal(resp.Body.Bytes(), parsedResponse) - assert.Nil(t, err) - assert.JSONEq(t, expectedJson, resp.Body.String()) -} - -func testErrorResponse(t *testing.T, mockedTracker natTrackerMock, expectedJson string) { - resp, err := makeStatusRequestAndReturnResponse(mockedTracker) - assert.Nil(t, err) - assert.JSONEq(t, expectedJson, resp.Body.String()) } From 96d2f9a86d5345de776202bf87120865c31f7e6e Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Thu, 25 Apr 2019 14:55:09 +0300 Subject: [PATCH 06/12] Extract nat events to separate package --- cmd/di.go | 17 +++++++++-------- nat/mapping/port_mapping.go | 6 +++--- nat/{traversal => natevents}/events_sender.go | 2 +- .../events_sender_test.go | 2 +- nat/{traversal => natevents}/events_tracker.go | 2 +- nat/traversal/pinger.go | 10 ++++++---- services/openvpn/service/manager.go | 4 ++-- session/manager.go | 3 ++- session/manager_test.go | 5 +++-- tequilapi/endpoints/nat.go | 6 +++--- tequilapi/endpoints/nat_test.go | 10 +++++----- 11 files changed, 36 insertions(+), 31 deletions(-) rename nat/{traversal => natevents}/events_sender.go (99%) rename nat/{traversal => natevents}/events_sender_test.go (99%) rename nat/{traversal => natevents}/events_tracker.go (99%) diff --git a/cmd/di.go b/cmd/di.go index 0510df647a..c27e988d90 100644 --- a/cmd/di.go +++ b/cmd/di.go @@ -51,6 +51,7 @@ import ( "github.com/mysteriumnetwork/node/money" "github.com/mysteriumnetwork/node/nat" "github.com/mysteriumnetwork/node/nat/mapping" + "github.com/mysteriumnetwork/node/nat/natevents" "github.com/mysteriumnetwork/node/nat/traversal" "github.com/mysteriumnetwork/node/nat/traversal/config" "github.com/mysteriumnetwork/node/nat/upnp" @@ -95,14 +96,14 @@ type NatPinger interface { // NatEventTracker is responsible for tracking NAT events type NatEventTracker interface { - ConsumeNATEvent(event traversal.Event) - LastEvent() *traversal.Event - WaitForEvent() traversal.Event + ConsumeNATEvent(event natevents.Event) + LastEvent() *natevents.Event + WaitForEvent() natevents.Event } // NatEventSender is responsible for sending NAT events to metrics server type NatEventSender interface { - ConsumeNATEvent(event traversal.Event) + ConsumeNATEvent(event natevents.Event) } // Dependencies is DI container for top level components which is reused in several places @@ -299,11 +300,11 @@ func (di *Dependencies) subscribeEventConsumers() error { } // NAT events - err = di.EventBus.Subscribe(traversal.EventTopic, di.NATEventSender.ConsumeNATEvent) + err = di.EventBus.Subscribe(natevents.EventTopic, di.NATEventSender.ConsumeNATEvent) if err != nil { return err } - return di.EventBus.Subscribe(traversal.EventTopic, di.NATTracker.ConsumeNATEvent) + return di.EventBus.Subscribe(natevents.EventTopic, di.NATTracker.ConsumeNATEvent) } func (di *Dependencies) bootstrapNodeComponents(nodeOptions node.Options) { @@ -486,8 +487,8 @@ func (di *Dependencies) bootstrapMetrics(options node.Options) { } func (di *Dependencies) bootstrapNATComponents(options node.Options) { - di.NATTracker = traversal.NewEventsTracker() - di.NATEventSender = traversal.NewEventsSender(di.MetricsSender, di.IPResolver.GetPublicIP) + di.NATTracker = natevents.NewEventsTracker() + di.NATEventSender = natevents.NewEventsSender(di.MetricsSender, di.IPResolver.GetPublicIP) if options.ExperimentNATPunching { di.NATPinger = traversal.NewPingerFactory( di.NATTracker, diff --git a/nat/mapping/port_mapping.go b/nat/mapping/port_mapping.go index e0a7e7c49f..99fb07b468 100644 --- a/nat/mapping/port_mapping.go +++ b/nat/mapping/port_mapping.go @@ -22,7 +22,7 @@ import ( log "github.com/cihub/seelog" portmap "github.com/ethereum/go-ethereum/p2p/nat" - "github.com/mysteriumnetwork/node/nat/traversal" + "github.com/mysteriumnetwork/node/nat/natevents" ) const logPrefix = "[port mapping] " @@ -88,11 +88,11 @@ func addMapping(m portmap.Interface, protocol string, extPort, intPort int, name log.Debugf("%s Couldn't add port mapping for port %d: %v, retrying with permanent lease", logPrefix, extPort, err) if err := m.AddMapping(protocol, extPort, intPort, name, 0); err != nil { // some gateways support only permanent leases - publisher.Publish(traversal.EventTopic, traversal.BuildFailureEvent(StageName, err)) + publisher.Publish(natevents.EventTopic, natevents.BuildFailureEvent(StageName, err)) log.Debugf("%s Couldn't add port mapping for port %d: %v", logPrefix, extPort, err) return } } - publisher.Publish(traversal.EventTopic, traversal.BuildSuccessEvent(StageName)) + publisher.Publish(natevents.EventTopic, natevents.BuildSuccessEvent(StageName)) log.Info(logPrefix, "Mapped network port: ", extPort) } diff --git a/nat/traversal/events_sender.go b/nat/natevents/events_sender.go similarity index 99% rename from nat/traversal/events_sender.go rename to nat/natevents/events_sender.go index eec2267b0b..9429cc586b 100644 --- a/nat/traversal/events_sender.go +++ b/nat/natevents/events_sender.go @@ -15,7 +15,7 @@ * along with this program. If not, see . */ -package traversal +package natevents import ( log "github.com/cihub/seelog" diff --git a/nat/traversal/events_sender_test.go b/nat/natevents/events_sender_test.go similarity index 99% rename from nat/traversal/events_sender_test.go rename to nat/natevents/events_sender_test.go index 9b819b79dc..1b12233a0f 100644 --- a/nat/traversal/events_sender_test.go +++ b/nat/natevents/events_sender_test.go @@ -15,7 +15,7 @@ * along with this program. If not, see . */ -package traversal +package natevents import ( "errors" diff --git a/nat/traversal/events_tracker.go b/nat/natevents/events_tracker.go similarity index 99% rename from nat/traversal/events_tracker.go rename to nat/natevents/events_tracker.go index 8c9d153cb0..ba4f78707f 100644 --- a/nat/traversal/events_tracker.go +++ b/nat/natevents/events_tracker.go @@ -15,7 +15,7 @@ * along with this program. If not, see . */ -package traversal +package natevents import ( "time" diff --git a/nat/traversal/pinger.go b/nat/traversal/pinger.go index 8fe341e95a..448d78f8f1 100644 --- a/nat/traversal/pinger.go +++ b/nat/traversal/pinger.go @@ -26,15 +26,17 @@ import ( log "github.com/cihub/seelog" "github.com/mysteriumnetwork/node/core/port" + "github.com/mysteriumnetwork/node/nat/natevents" "github.com/mysteriumnetwork/node/services" "github.com/pkg/errors" "golang.org/x/net/ipv4" ) +// StageName represents hole-punching stage of NAT traversal +const StageName = "hole_punching" const prefix = "[NATPinger] " const pingInterval = 200 const pingTimeout = 10000 -const stageName = "hole_punching" // Pinger represents NAT pinger structure type Pinger struct { @@ -53,7 +55,7 @@ type Pinger struct { // NatEventWaiter is responsible for waiting for nat events type NatEventWaiter interface { - WaitForEvent() Event + WaitForEvent() natevents.Event } // ConfigParser is able to parse a config from given raw json @@ -225,11 +227,11 @@ func (p *Pinger) ping(conn *net.UDPConn) error { err := p.sendPingRequest(conn, n) if err != nil { - p.eventPublisher.Publish(EventTopic, BuildFailureEvent(stageName, err)) + p.eventPublisher.Publish(natevents.EventTopic, natevents.BuildFailureEvent(StageName, err)) return err } - p.eventPublisher.Publish(EventTopic, BuildSuccessEvent(stageName)) + p.eventPublisher.Publish(natevents.EventTopic, natevents.BuildSuccessEvent(StageName)) n++ } diff --git a/services/openvpn/service/manager.go b/services/openvpn/service/manager.go index 7d45588c41..9a160ec882 100644 --- a/services/openvpn/service/manager.go +++ b/services/openvpn/service/manager.go @@ -29,7 +29,7 @@ import ( "github.com/mysteriumnetwork/node/market" "github.com/mysteriumnetwork/node/nat" "github.com/mysteriumnetwork/node/nat/mapping" - "github.com/mysteriumnetwork/node/nat/traversal" + "github.com/mysteriumnetwork/node/nat/natevents" "github.com/mysteriumnetwork/node/services" openvpn_service "github.com/mysteriumnetwork/node/services/openvpn" "github.com/mysteriumnetwork/node/session" @@ -58,7 +58,7 @@ type NATPinger interface { // NATEventGetter allows us to fetch the last known NAT event type NATEventGetter interface { - LastEvent() *traversal.Event + LastEvent() *natevents.Event } // Manager represents entrypoint for Openvpn service with top level components diff --git a/session/manager.go b/session/manager.go index 9c240102f8..4d8d50eca5 100644 --- a/session/manager.go +++ b/session/manager.go @@ -26,6 +26,7 @@ import ( log "github.com/cihub/seelog" "github.com/mysteriumnetwork/node/identity" "github.com/mysteriumnetwork/node/market" + "github.com/mysteriumnetwork/node/nat/natevents" "github.com/mysteriumnetwork/node/nat/traversal" ) @@ -74,7 +75,7 @@ type BalanceTrackerFactory func(consumer, provider, issuer identity.Identity) (B // NATEventGetter lets us access the last known traversal event type NATEventGetter interface { - LastEvent() *traversal.Event + LastEvent() *natevents.Event } // NewManager returns new session Manager diff --git a/session/manager_test.go b/session/manager_test.go index 3ac560968a..a8c118e503 100644 --- a/session/manager_test.go +++ b/session/manager_test.go @@ -22,6 +22,7 @@ import ( "github.com/mysteriumnetwork/node/identity" "github.com/mysteriumnetwork/node/market" + "github.com/mysteriumnetwork/node/nat/natevents" "github.com/mysteriumnetwork/node/nat/traversal" "github.com/stretchr/testify/assert" ) @@ -98,6 +99,6 @@ func TestManager_Create_RejectsUnknownProposal(t *testing.T) { type MockNatEventTracker struct { } -func (mnet *MockNatEventTracker) LastEvent() *traversal.Event { - return &traversal.Event{} +func (mnet *MockNatEventTracker) LastEvent() *natevents.Event { + return &natevents.Event{} } diff --git a/tequilapi/endpoints/nat.go b/tequilapi/endpoints/nat.go index b2cee25a36..957f8c6bea 100644 --- a/tequilapi/endpoints/nat.go +++ b/tequilapi/endpoints/nat.go @@ -21,7 +21,7 @@ import ( "net/http" "github.com/julienschmidt/httprouter" - "github.com/mysteriumnetwork/node/nat/traversal" + "github.com/mysteriumnetwork/node/nat/natevents" "github.com/mysteriumnetwork/node/tequilapi/utils" ) @@ -40,7 +40,7 @@ type NATStatusDTO struct { // NATEvents allows retrieving last traversal event type NATEvents interface { - LastEvent() *traversal.Event + LastEvent() *natevents.Event } // NATEndpoint struct represents endpoints about NAT traversal @@ -79,7 +79,7 @@ func AddRoutesForNAT(router *httprouter.Router, natEvents NATEvents) { router.GET("/nat/status", natEndpoint.NATStatus) } -func toNATStatusResponse(event *traversal.Event) NATStatusDTO { +func toNATStatusResponse(event *natevents.Event) NATStatusDTO { if event == nil { return NATStatusDTO{Status: statusNotFinished} } diff --git a/tequilapi/endpoints/nat_test.go b/tequilapi/endpoints/nat_test.go index d29ec94116..6435ba9448 100644 --- a/tequilapi/endpoints/nat_test.go +++ b/tequilapi/endpoints/nat_test.go @@ -23,13 +23,13 @@ import ( "testing" "github.com/julienschmidt/httprouter" - "github.com/mysteriumnetwork/node/nat/traversal" + "github.com/mysteriumnetwork/node/nat/natevents" "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) func Test_NATStatus_ReturnsStatusSuccessful_WithSuccessfulEvent(t *testing.T) { - successfulEvent := traversal.BuildSuccessEvent("hole_punching") + successfulEvent := natevents.BuildSuccessEvent("hole_punching") testResponse( t, @@ -41,7 +41,7 @@ func Test_NATStatus_ReturnsStatusSuccessful_WithSuccessfulEvent(t *testing.T) { } func Test_NATStatus_ReturnsStatusFailureAndError_WithFailureEvent(t *testing.T) { - failureEvent := traversal.BuildFailureEvent("hole_punching", errors.New("mock error")) + failureEvent := natevents.BuildFailureEvent("hole_punching", errors.New("mock error")) testResponse( t, @@ -85,9 +85,9 @@ func makeStatusRequestAndReturnResponse(mockedTracker natTrackerMock) (*httptest } type natTrackerMock struct { - mockLastEvent *traversal.Event + mockLastEvent *natevents.Event } -func (nt *natTrackerMock) LastEvent() *traversal.Event { +func (nt *natTrackerMock) LastEvent() *natevents.Event { return nt.mockLastEvent } From 30e7e73591efc2a51dd84803a569fa81bfbff644 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Thu, 25 Apr 2019 15:11:02 +0300 Subject: [PATCH 07/12] Add nat.StatusTracker to track the status of NAT traversal --- nat/status_tracker.go | 61 ++++++++++++++++++++++++++++++++++++++ nat/status_tracker_test.go | 52 ++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 nat/status_tracker.go create mode 100644 nat/status_tracker_test.go diff --git a/nat/status_tracker.go b/nat/status_tracker.go new file mode 100644 index 0000000000..ebff5c4ce2 --- /dev/null +++ b/nat/status_tracker.go @@ -0,0 +1,61 @@ +/* + * Copyright (C) 2019 The "MysteriumNetwork/node" Authors. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package nat + +import ( + "github.com/mysteriumnetwork/node/nat/natevents" +) + +// StatusTracker keeps status of NAT traversal by consuming NAT events - whether if finished and was it successful. +// It can finish either by successful event from any stage, or by a failure of the last stage. +type StatusTracker struct { + lastStageName string + status string +} + +const ( + statusNotFinished = "not_finished" + statusSuccessful = "successful" + statusFailure = "failure" +) + +// Status returns NAT traversal status - either "not_finished", "successful" or "failure" +func (t *StatusTracker) Status() string { + return t.status +} + +// ConsumeNATEvent processes NAT event to determine NAT traversal status +func (t *StatusTracker) ConsumeNATEvent(event natevents.Event) { + if event.Stage == t.lastStageName && event.Successful == false { + t.status = statusFailure + return + } + + if event.Successful { + t.status = statusSuccessful + return + } +} + +// NewStatusTracker returns new instance of status tracker +func NewStatusTracker(lastStageName string) *StatusTracker { + return &StatusTracker{ + lastStageName: lastStageName, + status: statusNotFinished, + } +} diff --git a/nat/status_tracker_test.go b/nat/status_tracker_test.go new file mode 100644 index 0000000000..8422a18510 --- /dev/null +++ b/nat/status_tracker_test.go @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2019 The "MysteriumNetwork/node" Authors. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package nat + +import ( + "testing" + + "github.com/mysteriumnetwork/node/nat/natevents" + "github.com/stretchr/testify/assert" +) + +func Test_StatusTracker_Status_ReturnsNotFinishedInitially(t *testing.T) { + tracker := NewStatusTracker("last stage") + status := tracker.Status() + assert.Equal(t, "not_finished", status) +} + +func Test_StatusTracker_Status_ReturnsSuccessful_WithSuccessfulEvent(t *testing.T) { + tracker := NewStatusTracker("last stage") + tracker.ConsumeNATEvent(natevents.Event{Successful: true, Stage: "any stage"}) + status := tracker.Status() + assert.Equal(t, "successful", status) +} + +func Test_StatusTracker_Status_ReturnsFailure_WithHolepunchingFailureEvent(t *testing.T) { + tracker := NewStatusTracker("last stage") + tracker.ConsumeNATEvent(natevents.Event{Successful: false, Stage: "last stage"}) + status := tracker.Status() + assert.Equal(t, "failure", status) +} + +func Test_StatusTracker_Status_ReturnsNotFinished_WithPortMappingFailureEvent(t *testing.T) { + tracker := NewStatusTracker("last stage") + tracker.ConsumeNATEvent(natevents.Event{Successful: false, Stage: "first stage"}) + status := tracker.Status() + assert.Equal(t, "not_finished", status) +} From c17ad69532def01b8aa478f2834eb021bb1edfb5 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Thu, 25 Apr 2019 15:40:10 +0300 Subject: [PATCH 08/12] Extend StatusTracker to return error --- nat/status_tracker.go | 18 ++++++++++++------ nat/status_tracker_test.go | 19 ++++++++++++++----- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/nat/status_tracker.go b/nat/status_tracker.go index ebff5c4ce2..d96e98ebad 100644 --- a/nat/status_tracker.go +++ b/nat/status_tracker.go @@ -25,7 +25,7 @@ import ( // It can finish either by successful event from any stage, or by a failure of the last stage. type StatusTracker struct { lastStageName string - status string + status Status } const ( @@ -34,20 +34,26 @@ const ( statusFailure = "failure" ) -// Status returns NAT traversal status - either "not_finished", "successful" or "failure" -func (t *StatusTracker) Status() string { +// Status represents NAT traversal status (either "not_finished", "successful" or "failure") and an optional error. +type Status struct { + Status string + Error error +} + +// Status returns NAT traversal status +func (t *StatusTracker) Status() Status { return t.status } // ConsumeNATEvent processes NAT event to determine NAT traversal status func (t *StatusTracker) ConsumeNATEvent(event natevents.Event) { if event.Stage == t.lastStageName && event.Successful == false { - t.status = statusFailure + t.status = Status{Status: statusFailure, Error: event.Error} return } if event.Successful { - t.status = statusSuccessful + t.status = Status{Status: statusSuccessful} return } } @@ -56,6 +62,6 @@ func (t *StatusTracker) ConsumeNATEvent(event natevents.Event) { func NewStatusTracker(lastStageName string) *StatusTracker { return &StatusTracker{ lastStageName: lastStageName, - status: statusNotFinished, + status: Status{Status: statusNotFinished}, } } diff --git a/nat/status_tracker_test.go b/nat/status_tracker_test.go index 8422a18510..ccec5b7bca 100644 --- a/nat/status_tracker_test.go +++ b/nat/status_tracker_test.go @@ -21,32 +21,41 @@ import ( "testing" "github.com/mysteriumnetwork/node/nat/natevents" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) func Test_StatusTracker_Status_ReturnsNotFinishedInitially(t *testing.T) { tracker := NewStatusTracker("last stage") status := tracker.Status() - assert.Equal(t, "not_finished", status) + + assert.Equal(t, "not_finished", status.Status) + assert.Nil(t, status.Error) } func Test_StatusTracker_Status_ReturnsSuccessful_WithSuccessfulEvent(t *testing.T) { tracker := NewStatusTracker("last stage") tracker.ConsumeNATEvent(natevents.Event{Successful: true, Stage: "any stage"}) status := tracker.Status() - assert.Equal(t, "successful", status) + + assert.Equal(t, "successful", status.Status) + assert.Nil(t, status.Error) } func Test_StatusTracker_Status_ReturnsFailure_WithHolepunchingFailureEvent(t *testing.T) { tracker := NewStatusTracker("last stage") - tracker.ConsumeNATEvent(natevents.Event{Successful: false, Stage: "last stage"}) + tracker.ConsumeNATEvent(natevents.Event{Successful: false, Stage: "last stage", Error: errors.New("test error")}) status := tracker.Status() - assert.Equal(t, "failure", status) + + assert.Equal(t, "failure", status.Status) + assert.EqualError(t, status.Error, "test error") } func Test_StatusTracker_Status_ReturnsNotFinished_WithPortMappingFailureEvent(t *testing.T) { tracker := NewStatusTracker("last stage") tracker.ConsumeNATEvent(natevents.Event{Successful: false, Stage: "first stage"}) status := tracker.Status() - assert.Equal(t, "not_finished", status) + + assert.Equal(t, "not_finished", status.Status) + assert.Nil(t, status.Error) } From 8aaf6e8ff9eebf990850d27c2ace765694d04c5e Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Thu, 25 Apr 2019 17:10:35 +0300 Subject: [PATCH 09/12] Use nat.StatusTracker in NAT endpoint --- cmd/di.go | 32 +++++++++++++++++----- tequilapi/endpoints/nat.go | 37 +++++++++++-------------- tequilapi/endpoints/nat_test.go | 48 +++++++++++++-------------------- 3 files changed, 60 insertions(+), 57 deletions(-) diff --git a/cmd/di.go b/cmd/di.go index c27e988d90..bde4e18d43 100644 --- a/cmd/di.go +++ b/cmd/di.go @@ -106,6 +106,12 @@ type NatEventSender interface { ConsumeNATEvent(event natevents.Event) } +// NATStatusTracker tracks status of NAT traversal by consuming NAT events +type NATStatusTracker interface { + Status() nat.Status + ConsumeNATEvent(event natevents.Event) +} + // Dependencies is DI container for top level components which is reused in several places type Dependencies struct { Node *node.Node @@ -141,9 +147,10 @@ type Dependencies struct { ServiceRegistry *service.Registry ServiceSessionStorage *session.StorageMemory - NATPinger NatPinger - NATTracker NatEventTracker - NATEventSender NatEventSender + NATPinger NatPinger + NATTracker NatEventTracker + NATEventSender NatEventSender + NATStatusTracker NATStatusTracker PortPool *port.Pool @@ -304,7 +311,11 @@ func (di *Dependencies) subscribeEventConsumers() error { if err != nil { return err } - return di.EventBus.Subscribe(natevents.EventTopic, di.NATTracker.ConsumeNATEvent) + err = di.EventBus.Subscribe(natevents.EventTopic, di.NATTracker.ConsumeNATEvent) + if err != nil { + return err + } + return di.EventBus.Subscribe(natevents.EventTopic, di.NATStatusTracker.ConsumeNATEvent) } func (di *Dependencies) bootstrapNodeComponents(nodeOptions node.Options) { @@ -345,7 +356,7 @@ func (di *Dependencies) bootstrapNodeComponents(nodeOptions node.Options) { tequilapi_endpoints.AddRoutesForServiceSessions(router, di.ServiceSessionStorage) tequilapi_endpoints.AddRoutesForPayout(router, di.IdentityManager, di.SignerFactory, di.MysteriumAPI) tequilapi_endpoints.AddRoutesForAccessPolicies(router, nodeOptions.AccessPolicyEndpointAddress) - tequilapi_endpoints.AddRoutesForNAT(router, di.NATTracker) + tequilapi_endpoints.AddRoutesForNAT(router, di.NATStatusTracker.Status) identity_registry.AddIdentityRegistrationEndpoint(router, di.IdentityRegistration, di.IdentityRegistry) corsPolicy := tequilapi.NewMysteriumCorsPolicy() @@ -488,7 +499,6 @@ func (di *Dependencies) bootstrapMetrics(options node.Options) { func (di *Dependencies) bootstrapNATComponents(options node.Options) { di.NATTracker = natevents.NewEventsTracker() - di.NATEventSender = natevents.NewEventsSender(di.MetricsSender, di.IPResolver.GetPublicIP) if options.ExperimentNATPunching { di.NATPinger = traversal.NewPingerFactory( di.NATTracker, @@ -501,4 +511,14 @@ func (di *Dependencies) bootstrapNATComponents(options node.Options) { } else { di.NATPinger = &traversal.NoopPinger{} } + + di.NATEventSender = natevents.NewEventsSender(di.MetricsSender, di.IPResolver.GetPublicIP) + + var lastStageName string + if options.ExperimentNATPunching { + lastStageName = traversal.StageName + } else { + lastStageName = mapping.StageName + } + di.NATStatusTracker = nat.NewStatusTracker(lastStageName) } diff --git a/tequilapi/endpoints/nat.go b/tequilapi/endpoints/nat.go index 957f8c6bea..9acf20ee10 100644 --- a/tequilapi/endpoints/nat.go +++ b/tequilapi/endpoints/nat.go @@ -21,6 +21,7 @@ import ( "net/http" "github.com/julienschmidt/httprouter" + "github.com/mysteriumnetwork/node/nat" "github.com/mysteriumnetwork/node/nat/natevents" "github.com/mysteriumnetwork/node/tequilapi/utils" ) @@ -38,6 +39,8 @@ type NATStatusDTO struct { Error *string `json:"error,omitempty"` } +type natStatusProvider func() nat.Status + // NATEvents allows retrieving last traversal event type NATEvents interface { LastEvent() *natevents.Event @@ -45,13 +48,13 @@ type NATEvents interface { // NATEndpoint struct represents endpoints about NAT traversal type NATEndpoint struct { - natEvents NATEvents + statusProvider natStatusProvider } // NewNATEndpoint creates and returns nat endpoint -func NewNATEndpoint(natEvents NATEvents) *NATEndpoint { +func NewNATEndpoint(statusProvider natStatusProvider) *NATEndpoint { return &NATEndpoint{ - natEvents: natEvents, + statusProvider: statusProvider, } } @@ -66,32 +69,22 @@ func NewNATEndpoint(natEvents NATEvents) *NATEndpoint { // schema: // "$ref": "#/definitions/NATStatusDTO" func (ne *NATEndpoint) NATStatus(resp http.ResponseWriter, _ *http.Request, _ httprouter.Params) { - event := ne.natEvents.LastEvent() - - statusResponse := toNATStatusResponse(event) + status := ne.statusProvider() + statusResponse := toNATStatusResponse(status) utils.WriteAsJSON(statusResponse, resp) } // AddRoutesForNAT adds nat routes to given router -func AddRoutesForNAT(router *httprouter.Router, natEvents NATEvents) { - natEndpoint := NewNATEndpoint(natEvents) +func AddRoutesForNAT(router *httprouter.Router, statusProvider natStatusProvider) { + natEndpoint := NewNATEndpoint(statusProvider) router.GET("/nat/status", natEndpoint.NATStatus) } -func toNATStatusResponse(event *natevents.Event) NATStatusDTO { - if event == nil { - return NATStatusDTO{Status: statusNotFinished} - } - - if event.Successful { - return NATStatusDTO{Status: statusSuccessful} - } - - var error *string - if event.Error != nil { - msg := event.Error.Error() - error = &msg +func toNATStatusResponse(status nat.Status) NATStatusDTO { + if status.Error == nil { + return NATStatusDTO{Status: status.Status} } - return NATStatusDTO{Status: statusFailure, Error: error} + error := status.Error.Error() + return NATStatusDTO{Status: status.Status, Error: &error} } diff --git a/tequilapi/endpoints/nat_test.go b/tequilapi/endpoints/nat_test.go index 6435ba9448..8cc1d4a131 100644 --- a/tequilapi/endpoints/nat_test.go +++ b/tequilapi/endpoints/nat_test.go @@ -23,17 +23,23 @@ import ( "testing" "github.com/julienschmidt/httprouter" - "github.com/mysteriumnetwork/node/nat/natevents" + "github.com/mysteriumnetwork/node/nat" "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) -func Test_NATStatus_ReturnsStatusSuccessful_WithSuccessfulEvent(t *testing.T) { - successfulEvent := natevents.BuildSuccessEvent("hole_punching") +type mockNATStatusProvider struct { + mockStatus nat.Status +} +func (mockProvider *mockNATStatusProvider) Status() nat.Status { + return mockProvider.mockStatus +} + +func Test_NATStatus_ReturnsStatusSuccessful_WithSuccessfulEvent(t *testing.T) { testResponse( t, - natTrackerMock{mockLastEvent: &successfulEvent}, + nat.Status{Status: statusSuccessful}, `{ "status": "successful" }`, @@ -41,11 +47,9 @@ func Test_NATStatus_ReturnsStatusSuccessful_WithSuccessfulEvent(t *testing.T) { } func Test_NATStatus_ReturnsStatusFailureAndError_WithFailureEvent(t *testing.T) { - failureEvent := natevents.BuildFailureEvent("hole_punching", errors.New("mock error")) - testResponse( t, - natTrackerMock{mockLastEvent: &failureEvent}, + nat.Status{Status: statusFailure, Error: errors.New("mock error")}, `{ "status": "failure", "error": "mock error" @@ -56,38 +60,24 @@ func Test_NATStatus_ReturnsStatusFailureAndError_WithFailureEvent(t *testing.T) func Test_NATStatus_ReturnsStatusNotFinished_WhenEventIsNotAvailable(t *testing.T) { testResponse( t, - natTrackerMock{mockLastEvent: nil}, + nat.Status{Status: statusNotFinished}, `{ "status": "not_finished" }`, ) } -func testResponse(t *testing.T, mockedTracker natTrackerMock, expectedJson string) { - resp, err := makeStatusRequestAndReturnResponse(mockedTracker) - assert.Nil(t, err) - - assert.JSONEq(t, expectedJson, resp.Body.String()) -} +func testResponse(t *testing.T, mockStatus nat.Status, expectedJson string) { + provider := mockNATStatusProvider{mockStatus: mockStatus} -func makeStatusRequestAndReturnResponse(mockedTracker natTrackerMock) (*httptest.ResponseRecorder, error) { req, err := http.NewRequest(http.MethodGet, "/nat/status", nil) - if err != nil { - return nil, err - } - + assert.Nil(t, err) resp := httptest.NewRecorder() router := httprouter.New() - AddRoutesForNAT(router, &mockedTracker) - router.ServeHTTP(resp, req) + AddRoutesForNAT(router, provider.Status) - return resp, nil -} - -type natTrackerMock struct { - mockLastEvent *natevents.Event -} + router.ServeHTTP(resp, req) -func (nt *natTrackerMock) LastEvent() *natevents.Event { - return nt.mockLastEvent + assert.Equal(t, http.StatusOK, resp.Code) + assert.JSONEq(t, expectedJson, resp.Body.String()) } From e59c650f6e4c71ee2cb959c8f9777f24036ce0c7 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Fri, 26 Apr 2019 10:03:25 +0300 Subject: [PATCH 10/12] Refactor: remove pointer in NatStatusDTO.Error --- tequilapi/endpoints/nat.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tequilapi/endpoints/nat.go b/tequilapi/endpoints/nat.go index 9acf20ee10..518e87eef1 100644 --- a/tequilapi/endpoints/nat.go +++ b/tequilapi/endpoints/nat.go @@ -35,8 +35,8 @@ const ( // NATStatusDTO gives information about NAT traversal success or failure // swagger:model NATStatusDTO type NATStatusDTO struct { - Status string `json:"status"` - Error *string `json:"error,omitempty"` + Status string `json:"status"` + Error string `json:"error,omitempty"` } type natStatusProvider func() nat.Status @@ -86,5 +86,5 @@ func toNATStatusResponse(status nat.Status) NATStatusDTO { return NATStatusDTO{Status: status.Status} } error := status.Error.Error() - return NATStatusDTO{Status: status.Status, Error: &error} + return NATStatusDTO{Status: status.Status, Error: error} } From e9daed406d696356a8b252668d51685963074713 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Fri, 26 Apr 2019 10:06:43 +0300 Subject: [PATCH 11/12] Rename 'natevents' package to 'event' --- cmd/di.go | 22 +++++++++---------- nat/{natevents => event}/events_sender.go | 2 +- .../events_sender_test.go | 2 +- nat/{natevents => event}/events_tracker.go | 2 +- nat/mapping/port_mapping.go | 6 ++--- nat/status_tracker.go | 4 ++-- nat/status_tracker_test.go | 8 +++---- nat/traversal/pinger.go | 8 +++---- services/openvpn/service/manager.go | 4 ++-- session/manager.go | 4 ++-- session/manager_test.go | 6 ++--- tequilapi/endpoints/nat.go | 4 ++-- 12 files changed, 36 insertions(+), 36 deletions(-) rename nat/{natevents => event}/events_sender.go (99%) rename nat/{natevents => event}/events_sender_test.go (99%) rename nat/{natevents => event}/events_tracker.go (99%) diff --git a/cmd/di.go b/cmd/di.go index bde4e18d43..bc28ce3029 100644 --- a/cmd/di.go +++ b/cmd/di.go @@ -50,8 +50,8 @@ import ( "github.com/mysteriumnetwork/node/metrics" "github.com/mysteriumnetwork/node/money" "github.com/mysteriumnetwork/node/nat" + "github.com/mysteriumnetwork/node/nat/event" "github.com/mysteriumnetwork/node/nat/mapping" - "github.com/mysteriumnetwork/node/nat/natevents" "github.com/mysteriumnetwork/node/nat/traversal" "github.com/mysteriumnetwork/node/nat/traversal/config" "github.com/mysteriumnetwork/node/nat/upnp" @@ -96,20 +96,20 @@ type NatPinger interface { // NatEventTracker is responsible for tracking NAT events type NatEventTracker interface { - ConsumeNATEvent(event natevents.Event) - LastEvent() *natevents.Event - WaitForEvent() natevents.Event + ConsumeNATEvent(event event.Event) + LastEvent() *event.Event + WaitForEvent() event.Event } // NatEventSender is responsible for sending NAT events to metrics server type NatEventSender interface { - ConsumeNATEvent(event natevents.Event) + ConsumeNATEvent(event event.Event) } // NATStatusTracker tracks status of NAT traversal by consuming NAT events type NATStatusTracker interface { Status() nat.Status - ConsumeNATEvent(event natevents.Event) + ConsumeNATEvent(event event.Event) } // Dependencies is DI container for top level components which is reused in several places @@ -307,15 +307,15 @@ func (di *Dependencies) subscribeEventConsumers() error { } // NAT events - err = di.EventBus.Subscribe(natevents.EventTopic, di.NATEventSender.ConsumeNATEvent) + err = di.EventBus.Subscribe(event.EventTopic, di.NATEventSender.ConsumeNATEvent) if err != nil { return err } - err = di.EventBus.Subscribe(natevents.EventTopic, di.NATTracker.ConsumeNATEvent) + err = di.EventBus.Subscribe(event.EventTopic, di.NATTracker.ConsumeNATEvent) if err != nil { return err } - return di.EventBus.Subscribe(natevents.EventTopic, di.NATStatusTracker.ConsumeNATEvent) + return di.EventBus.Subscribe(event.EventTopic, di.NATStatusTracker.ConsumeNATEvent) } func (di *Dependencies) bootstrapNodeComponents(nodeOptions node.Options) { @@ -498,7 +498,7 @@ func (di *Dependencies) bootstrapMetrics(options node.Options) { } func (di *Dependencies) bootstrapNATComponents(options node.Options) { - di.NATTracker = natevents.NewEventsTracker() + di.NATTracker = event.NewEventsTracker() if options.ExperimentNATPunching { di.NATPinger = traversal.NewPingerFactory( di.NATTracker, @@ -512,7 +512,7 @@ func (di *Dependencies) bootstrapNATComponents(options node.Options) { di.NATPinger = &traversal.NoopPinger{} } - di.NATEventSender = natevents.NewEventsSender(di.MetricsSender, di.IPResolver.GetPublicIP) + di.NATEventSender = event.NewEventsSender(di.MetricsSender, di.IPResolver.GetPublicIP) var lastStageName string if options.ExperimentNATPunching { diff --git a/nat/natevents/events_sender.go b/nat/event/events_sender.go similarity index 99% rename from nat/natevents/events_sender.go rename to nat/event/events_sender.go index 9429cc586b..5f0ec00478 100644 --- a/nat/natevents/events_sender.go +++ b/nat/event/events_sender.go @@ -15,7 +15,7 @@ * along with this program. If not, see . */ -package natevents +package event import ( log "github.com/cihub/seelog" diff --git a/nat/natevents/events_sender_test.go b/nat/event/events_sender_test.go similarity index 99% rename from nat/natevents/events_sender_test.go rename to nat/event/events_sender_test.go index 1b12233a0f..c458c85dc3 100644 --- a/nat/natevents/events_sender_test.go +++ b/nat/event/events_sender_test.go @@ -15,7 +15,7 @@ * along with this program. If not, see . */ -package natevents +package event import ( "errors" diff --git a/nat/natevents/events_tracker.go b/nat/event/events_tracker.go similarity index 99% rename from nat/natevents/events_tracker.go rename to nat/event/events_tracker.go index ba4f78707f..1f041d7fca 100644 --- a/nat/natevents/events_tracker.go +++ b/nat/event/events_tracker.go @@ -15,7 +15,7 @@ * along with this program. If not, see . */ -package natevents +package event import ( "time" diff --git a/nat/mapping/port_mapping.go b/nat/mapping/port_mapping.go index 99fb07b468..a20a17248b 100644 --- a/nat/mapping/port_mapping.go +++ b/nat/mapping/port_mapping.go @@ -22,7 +22,7 @@ import ( log "github.com/cihub/seelog" portmap "github.com/ethereum/go-ethereum/p2p/nat" - "github.com/mysteriumnetwork/node/nat/natevents" + "github.com/mysteriumnetwork/node/nat/event" ) const logPrefix = "[port mapping] " @@ -88,11 +88,11 @@ func addMapping(m portmap.Interface, protocol string, extPort, intPort int, name log.Debugf("%s Couldn't add port mapping for port %d: %v, retrying with permanent lease", logPrefix, extPort, err) if err := m.AddMapping(protocol, extPort, intPort, name, 0); err != nil { // some gateways support only permanent leases - publisher.Publish(natevents.EventTopic, natevents.BuildFailureEvent(StageName, err)) + publisher.Publish(event.EventTopic, event.BuildFailureEvent(StageName, err)) log.Debugf("%s Couldn't add port mapping for port %d: %v", logPrefix, extPort, err) return } } - publisher.Publish(natevents.EventTopic, natevents.BuildSuccessEvent(StageName)) + publisher.Publish(event.EventTopic, event.BuildSuccessEvent(StageName)) log.Info(logPrefix, "Mapped network port: ", extPort) } diff --git a/nat/status_tracker.go b/nat/status_tracker.go index d96e98ebad..35e5f890c2 100644 --- a/nat/status_tracker.go +++ b/nat/status_tracker.go @@ -18,7 +18,7 @@ package nat import ( - "github.com/mysteriumnetwork/node/nat/natevents" + "github.com/mysteriumnetwork/node/nat/event" ) // StatusTracker keeps status of NAT traversal by consuming NAT events - whether if finished and was it successful. @@ -46,7 +46,7 @@ func (t *StatusTracker) Status() Status { } // ConsumeNATEvent processes NAT event to determine NAT traversal status -func (t *StatusTracker) ConsumeNATEvent(event natevents.Event) { +func (t *StatusTracker) ConsumeNATEvent(event event.Event) { if event.Stage == t.lastStageName && event.Successful == false { t.status = Status{Status: statusFailure, Error: event.Error} return diff --git a/nat/status_tracker_test.go b/nat/status_tracker_test.go index ccec5b7bca..4ac9600490 100644 --- a/nat/status_tracker_test.go +++ b/nat/status_tracker_test.go @@ -20,7 +20,7 @@ package nat import ( "testing" - "github.com/mysteriumnetwork/node/nat/natevents" + "github.com/mysteriumnetwork/node/nat/event" "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -35,7 +35,7 @@ func Test_StatusTracker_Status_ReturnsNotFinishedInitially(t *testing.T) { func Test_StatusTracker_Status_ReturnsSuccessful_WithSuccessfulEvent(t *testing.T) { tracker := NewStatusTracker("last stage") - tracker.ConsumeNATEvent(natevents.Event{Successful: true, Stage: "any stage"}) + tracker.ConsumeNATEvent(event.Event{Successful: true, Stage: "any stage"}) status := tracker.Status() assert.Equal(t, "successful", status.Status) @@ -44,7 +44,7 @@ func Test_StatusTracker_Status_ReturnsSuccessful_WithSuccessfulEvent(t *testing. func Test_StatusTracker_Status_ReturnsFailure_WithHolepunchingFailureEvent(t *testing.T) { tracker := NewStatusTracker("last stage") - tracker.ConsumeNATEvent(natevents.Event{Successful: false, Stage: "last stage", Error: errors.New("test error")}) + tracker.ConsumeNATEvent(event.Event{Successful: false, Stage: "last stage", Error: errors.New("test error")}) status := tracker.Status() assert.Equal(t, "failure", status.Status) @@ -53,7 +53,7 @@ func Test_StatusTracker_Status_ReturnsFailure_WithHolepunchingFailureEvent(t *te func Test_StatusTracker_Status_ReturnsNotFinished_WithPortMappingFailureEvent(t *testing.T) { tracker := NewStatusTracker("last stage") - tracker.ConsumeNATEvent(natevents.Event{Successful: false, Stage: "first stage"}) + tracker.ConsumeNATEvent(event.Event{Successful: false, Stage: "first stage"}) status := tracker.Status() assert.Equal(t, "not_finished", status.Status) diff --git a/nat/traversal/pinger.go b/nat/traversal/pinger.go index 448d78f8f1..0f221684bb 100644 --- a/nat/traversal/pinger.go +++ b/nat/traversal/pinger.go @@ -26,7 +26,7 @@ import ( log "github.com/cihub/seelog" "github.com/mysteriumnetwork/node/core/port" - "github.com/mysteriumnetwork/node/nat/natevents" + "github.com/mysteriumnetwork/node/nat/event" "github.com/mysteriumnetwork/node/services" "github.com/pkg/errors" "golang.org/x/net/ipv4" @@ -55,7 +55,7 @@ type Pinger struct { // NatEventWaiter is responsible for waiting for nat events type NatEventWaiter interface { - WaitForEvent() natevents.Event + WaitForEvent() event.Event } // ConfigParser is able to parse a config from given raw json @@ -227,11 +227,11 @@ func (p *Pinger) ping(conn *net.UDPConn) error { err := p.sendPingRequest(conn, n) if err != nil { - p.eventPublisher.Publish(natevents.EventTopic, natevents.BuildFailureEvent(StageName, err)) + p.eventPublisher.Publish(event.EventTopic, event.BuildFailureEvent(StageName, err)) return err } - p.eventPublisher.Publish(natevents.EventTopic, natevents.BuildSuccessEvent(StageName)) + p.eventPublisher.Publish(event.EventTopic, event.BuildSuccessEvent(StageName)) n++ } diff --git a/services/openvpn/service/manager.go b/services/openvpn/service/manager.go index 9a160ec882..11ffde9ef7 100644 --- a/services/openvpn/service/manager.go +++ b/services/openvpn/service/manager.go @@ -28,8 +28,8 @@ import ( "github.com/mysteriumnetwork/node/identity" "github.com/mysteriumnetwork/node/market" "github.com/mysteriumnetwork/node/nat" + "github.com/mysteriumnetwork/node/nat/event" "github.com/mysteriumnetwork/node/nat/mapping" - "github.com/mysteriumnetwork/node/nat/natevents" "github.com/mysteriumnetwork/node/services" openvpn_service "github.com/mysteriumnetwork/node/services/openvpn" "github.com/mysteriumnetwork/node/session" @@ -58,7 +58,7 @@ type NATPinger interface { // NATEventGetter allows us to fetch the last known NAT event type NATEventGetter interface { - LastEvent() *natevents.Event + LastEvent() *event.Event } // Manager represents entrypoint for Openvpn service with top level components diff --git a/session/manager.go b/session/manager.go index 4d8d50eca5..cc83058c5b 100644 --- a/session/manager.go +++ b/session/manager.go @@ -26,7 +26,7 @@ import ( log "github.com/cihub/seelog" "github.com/mysteriumnetwork/node/identity" "github.com/mysteriumnetwork/node/market" - "github.com/mysteriumnetwork/node/nat/natevents" + "github.com/mysteriumnetwork/node/nat/event" "github.com/mysteriumnetwork/node/nat/traversal" ) @@ -75,7 +75,7 @@ type BalanceTrackerFactory func(consumer, provider, issuer identity.Identity) (B // NATEventGetter lets us access the last known traversal event type NATEventGetter interface { - LastEvent() *natevents.Event + LastEvent() *event.Event } // NewManager returns new session Manager diff --git a/session/manager_test.go b/session/manager_test.go index a8c118e503..d2c4003d1f 100644 --- a/session/manager_test.go +++ b/session/manager_test.go @@ -22,7 +22,7 @@ import ( "github.com/mysteriumnetwork/node/identity" "github.com/mysteriumnetwork/node/market" - "github.com/mysteriumnetwork/node/nat/natevents" + "github.com/mysteriumnetwork/node/nat/event" "github.com/mysteriumnetwork/node/nat/traversal" "github.com/stretchr/testify/assert" ) @@ -99,6 +99,6 @@ func TestManager_Create_RejectsUnknownProposal(t *testing.T) { type MockNatEventTracker struct { } -func (mnet *MockNatEventTracker) LastEvent() *natevents.Event { - return &natevents.Event{} +func (mnet *MockNatEventTracker) LastEvent() *event.Event { + return &event.Event{} } diff --git a/tequilapi/endpoints/nat.go b/tequilapi/endpoints/nat.go index 518e87eef1..6ecfede5e9 100644 --- a/tequilapi/endpoints/nat.go +++ b/tequilapi/endpoints/nat.go @@ -22,7 +22,7 @@ import ( "github.com/julienschmidt/httprouter" "github.com/mysteriumnetwork/node/nat" - "github.com/mysteriumnetwork/node/nat/natevents" + "github.com/mysteriumnetwork/node/nat/event" "github.com/mysteriumnetwork/node/tequilapi/utils" ) @@ -43,7 +43,7 @@ type natStatusProvider func() nat.Status // NATEvents allows retrieving last traversal event type NATEvents interface { - LastEvent() *natevents.Event + LastEvent() *event.Event } // NATEndpoint struct represents endpoints about NAT traversal From 3d24215db2cd8f859c5033d67be1970bee22e293 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Fri, 26 Apr 2019 10:10:41 +0300 Subject: [PATCH 12/12] Refactor event package - remove redundant 'event' prefixes in names --- cmd/di.go | 10 ++++---- nat/event/{events_sender.go => sender.go} | 22 ++++++++--------- .../{events_sender_test.go => sender_test.go} | 14 +++++------ nat/event/{events_tracker.go => tracker.go} | 24 +++++++++---------- nat/mapping/port_mapping.go | 4 ++-- nat/traversal/pinger.go | 4 ++-- 6 files changed, 39 insertions(+), 39 deletions(-) rename nat/event/{events_sender.go => sender.go} (70%) rename nat/event/{events_sender_test.go => sender_test.go} (89%) rename nat/event/{events_tracker.go => tracker.go} (75%) diff --git a/cmd/di.go b/cmd/di.go index bc28ce3029..f9c0bb0fd4 100644 --- a/cmd/di.go +++ b/cmd/di.go @@ -307,15 +307,15 @@ func (di *Dependencies) subscribeEventConsumers() error { } // NAT events - err = di.EventBus.Subscribe(event.EventTopic, di.NATEventSender.ConsumeNATEvent) + err = di.EventBus.Subscribe(event.Topic, di.NATEventSender.ConsumeNATEvent) if err != nil { return err } - err = di.EventBus.Subscribe(event.EventTopic, di.NATTracker.ConsumeNATEvent) + err = di.EventBus.Subscribe(event.Topic, di.NATTracker.ConsumeNATEvent) if err != nil { return err } - return di.EventBus.Subscribe(event.EventTopic, di.NATStatusTracker.ConsumeNATEvent) + return di.EventBus.Subscribe(event.Topic, di.NATStatusTracker.ConsumeNATEvent) } func (di *Dependencies) bootstrapNodeComponents(nodeOptions node.Options) { @@ -498,7 +498,7 @@ func (di *Dependencies) bootstrapMetrics(options node.Options) { } func (di *Dependencies) bootstrapNATComponents(options node.Options) { - di.NATTracker = event.NewEventsTracker() + di.NATTracker = event.NewTracker() if options.ExperimentNATPunching { di.NATPinger = traversal.NewPingerFactory( di.NATTracker, @@ -512,7 +512,7 @@ func (di *Dependencies) bootstrapNATComponents(options node.Options) { di.NATPinger = &traversal.NoopPinger{} } - di.NATEventSender = event.NewEventsSender(di.MetricsSender, di.IPResolver.GetPublicIP) + di.NATEventSender = event.NewSender(di.MetricsSender, di.IPResolver.GetPublicIP) var lastStageName string if options.ExperimentNATPunching { diff --git a/nat/event/events_sender.go b/nat/event/sender.go similarity index 70% rename from nat/event/events_sender.go rename to nat/event/sender.go index 5f0ec00478..51a5294685 100644 --- a/nat/event/events_sender.go +++ b/nat/event/sender.go @@ -21,10 +21,10 @@ import ( log "github.com/cihub/seelog" ) -const eventsSenderLogPrefix = "[traversal-events-sender] " +const senderLogPrefix = "[traversal-events-sender] " -// EventsSender allows subscribing to NAT events and sends them to server -type EventsSender struct { +// Sender allows subscribing to NAT events and sends them to server +type Sender struct { metricsSender metricsSender ipResolver ipResolver lastIp string @@ -38,16 +38,16 @@ type metricsSender interface { type ipResolver func() (string, error) -// NewEventsSender returns a new instance of events sender -func NewEventsSender(metricsSender metricsSender, ipResolver ipResolver) *EventsSender { - return &EventsSender{metricsSender: metricsSender, ipResolver: ipResolver, lastIp: ""} +// NewSender returns a new instance of events sender +func NewSender(metricsSender metricsSender, ipResolver ipResolver) *Sender { + return &Sender{metricsSender: metricsSender, ipResolver: ipResolver, lastIp: ""} } // ConsumeNATEvent sends received event to server -func (es *EventsSender) ConsumeNATEvent(event Event) { +func (es *Sender) ConsumeNATEvent(event Event) { publicIP, err := es.ipResolver() if err != nil { - log.Warnf(eventsSenderLogPrefix, "resolving public ip failed: ", err) + log.Warnf(senderLogPrefix, "resolving public ip failed: ", err) return } if publicIP == es.lastIp && es.matchesLastEvent(event) { @@ -56,14 +56,14 @@ func (es *EventsSender) ConsumeNATEvent(event Event) { err = es.sendNATEvent(event) if err != nil { - log.Warnf(eventsSenderLogPrefix, "sending event failed: ", err) + log.Warnf(senderLogPrefix, "sending event failed: ", err) } es.lastIp = publicIP es.lastEvent = &event } -func (es *EventsSender) sendNATEvent(event Event) error { +func (es *Sender) sendNATEvent(event Event) error { if event.Successful { return es.metricsSender.SendNATMappingSuccessEvent(event.Stage) } @@ -71,7 +71,7 @@ func (es *EventsSender) sendNATEvent(event Event) error { return es.metricsSender.SendNATMappingFailEvent(event.Stage, event.Error) } -func (es *EventsSender) matchesLastEvent(event Event) bool { +func (es *Sender) matchesLastEvent(event Event) bool { if es.lastEvent == nil { return false } diff --git a/nat/event/events_sender_test.go b/nat/event/sender_test.go similarity index 89% rename from nat/event/events_sender_test.go rename to nat/event/sender_test.go index c458c85dc3..913545dc15 100644 --- a/nat/event/events_sender_test.go +++ b/nat/event/sender_test.go @@ -60,7 +60,7 @@ func (resolver *mockIPResolver) GetPublicIP() (string, error) { func Test_EventsSender_ConsumeNATEvent_SendsSuccessEvent(t *testing.T) { mockMetricsSender := buildMockMetricsSender(nil) mockIPResolver := mockIPResolver{mockIp: "1st ip"} - sender := NewEventsSender(mockMetricsSender, mockIPResolver.GetPublicIP) + sender := NewSender(mockMetricsSender, mockIPResolver.GetPublicIP) sender.ConsumeNATEvent(Event{Stage: "hole_punching", Successful: true}) @@ -71,7 +71,7 @@ func Test_EventsSender_ConsumeNATEvent_SendsSuccessEvent(t *testing.T) { func Test_EventsSender_ConsumeNATEvent_WithSameIp_DoesNotSendSuccessEventAgain(t *testing.T) { mockMetricsSender := buildMockMetricsSender(nil) mockIPResolver := mockIPResolver{mockIp: "1st ip"} - sender := NewEventsSender(mockMetricsSender, mockIPResolver.GetPublicIP) + sender := NewSender(mockMetricsSender, mockIPResolver.GetPublicIP) sender.ConsumeNATEvent(Event{Successful: true}) @@ -84,7 +84,7 @@ func Test_EventsSender_ConsumeNATEvent_WithSameIp_DoesNotSendSuccessEventAgain(t func Test_EventsSender_ConsumeNATEvent_WithDifferentIP_SendsSuccessEventAgain(t *testing.T) { mockMetricsSender := buildMockMetricsSender(nil) mockIPResolver := &mockIPResolver{mockIp: "1st ip"} - sender := NewEventsSender(mockMetricsSender, mockIPResolver.GetPublicIP) + sender := NewSender(mockMetricsSender, mockIPResolver.GetPublicIP) sender.ConsumeNATEvent(Event{Successful: true}) @@ -98,7 +98,7 @@ func Test_EventsSender_ConsumeNATEvent_WithDifferentIP_SendsSuccessEventAgain(t func Test_EventsSender_ConsumeNATEvent_WhenIPResolverFails_DoesNotSendEvent(t *testing.T) { mockMetricsSender := buildMockMetricsSender(nil) mockIPResolver := &mockIPResolver{mockErr: errors.New("mock error")} - sender := NewEventsSender(mockMetricsSender, mockIPResolver.GetPublicIP) + sender := NewSender(mockMetricsSender, mockIPResolver.GetPublicIP) sender.ConsumeNATEvent(Event{Successful: true}) @@ -108,7 +108,7 @@ func Test_EventsSender_ConsumeNATEvent_WhenIPResolverFails_DoesNotSendEvent(t *t func Test_EventsSender_ConsumeNATEvent_SendsFailureEvent(t *testing.T) { mockMetricsSender := buildMockMetricsSender(nil) mockIPResolver := mockIPResolver{mockIp: "1st ip"} - sender := NewEventsSender(mockMetricsSender, mockIPResolver.GetPublicIP) + sender := NewSender(mockMetricsSender, mockIPResolver.GetPublicIP) testErr := errors.New("test error") sender.ConsumeNATEvent(Event{Stage: "hole_punching", Successful: false, Error: testErr}) @@ -120,7 +120,7 @@ func Test_EventsSender_ConsumeNATEvent_SendsFailureEvent(t *testing.T) { func Test_EventsSender_ConsumeNATEvent_WithFailuresOfDifferentStages_SendsBothEvents(t *testing.T) { mockMetricsSender := buildMockMetricsSender(nil) mockIPResolver := &mockIPResolver{mockIp: "1st ip"} - sender := NewEventsSender(mockMetricsSender, mockIPResolver.GetPublicIP) + sender := NewSender(mockMetricsSender, mockIPResolver.GetPublicIP) testErr1 := errors.New("test error 1") sender.ConsumeNATEvent(Event{Successful: false, Error: testErr1, Stage: "test 1"}) @@ -133,7 +133,7 @@ func Test_EventsSender_ConsumeNATEvent_WithFailuresOfDifferentStages_SendsBothEv func Test_EventsSender_ConsumeNATEvent_WithSuccessAndFailureOnSameIp_SendsBothEvents(t *testing.T) { mockMetricsSender := buildMockMetricsSender(nil) mockIPResolver := &mockIPResolver{mockIp: "1st ip"} - sender := NewEventsSender(mockMetricsSender, mockIPResolver.GetPublicIP) + sender := NewSender(mockMetricsSender, mockIPResolver.GetPublicIP) sender.ConsumeNATEvent(Event{Successful: true}) testErr := errors.New("test error") diff --git a/nat/event/events_tracker.go b/nat/event/tracker.go similarity index 75% rename from nat/event/events_tracker.go rename to nat/event/tracker.go index 1f041d7fca..a9504a1acd 100644 --- a/nat/event/events_tracker.go +++ b/nat/event/tracker.go @@ -23,19 +23,19 @@ import ( log "github.com/cihub/seelog" ) -// EventTopic the topic that traversal events are published on -const EventTopic = "Traversal" +// Topic the topic that traversal events are published on +const Topic = "Traversal" const eventsTrackerLogPrefix = "[traversal-events-tracker] " -// EventsTracker is able to track NAT traversal events -type EventsTracker struct { +// Tracker is able to track NAT traversal events +type Tracker struct { lastEvent *Event eventChan chan Event } -// BuildSuccessEvent returns new event for successful NAT traversal -func BuildSuccessEvent(stage string) Event { +// BuildSuccessfulEvent returns new event for successful NAT traversal +func BuildSuccessfulEvent(stage string) Event { return Event{Stage: stage, Successful: true} } @@ -44,13 +44,13 @@ func BuildFailureEvent(stage string, err error) Event { return Event{Stage: stage, Successful: false, Error: err} } -// NewEventsTracker returns a new instance of event tracker -func NewEventsTracker() *EventsTracker { - return &EventsTracker{eventChan: make(chan Event, 1)} +// NewTracker returns a new instance of event tracker +func NewTracker() *Tracker { + return &Tracker{eventChan: make(chan Event, 1)} } // ConsumeNATEvent consumes a NAT event -func (et *EventsTracker) ConsumeNATEvent(event Event) { +func (et *Tracker) ConsumeNATEvent(event Event) { log.Info(eventsTrackerLogPrefix, "got NAT event: ", event) et.lastEvent = &event @@ -61,13 +61,13 @@ func (et *EventsTracker) ConsumeNATEvent(event Event) { } // LastEvent returns the last known event and boolean flag, indicating if such event exists -func (et *EventsTracker) LastEvent() *Event { +func (et *Tracker) LastEvent() *Event { log.Info(eventsTrackerLogPrefix, "getting last NAT event: ", et.lastEvent) return et.lastEvent } // WaitForEvent waits for event to occur -func (et *EventsTracker) WaitForEvent() Event { +func (et *Tracker) WaitForEvent() Event { if et.lastEvent != nil { return *et.lastEvent } diff --git a/nat/mapping/port_mapping.go b/nat/mapping/port_mapping.go index a20a17248b..d4f2ceac34 100644 --- a/nat/mapping/port_mapping.go +++ b/nat/mapping/port_mapping.go @@ -88,11 +88,11 @@ func addMapping(m portmap.Interface, protocol string, extPort, intPort int, name log.Debugf("%s Couldn't add port mapping for port %d: %v, retrying with permanent lease", logPrefix, extPort, err) if err := m.AddMapping(protocol, extPort, intPort, name, 0); err != nil { // some gateways support only permanent leases - publisher.Publish(event.EventTopic, event.BuildFailureEvent(StageName, err)) + publisher.Publish(event.Topic, event.BuildFailureEvent(StageName, err)) log.Debugf("%s Couldn't add port mapping for port %d: %v", logPrefix, extPort, err) return } } - publisher.Publish(event.EventTopic, event.BuildSuccessEvent(StageName)) + publisher.Publish(event.Topic, event.BuildSuccessfulEvent(StageName)) log.Info(logPrefix, "Mapped network port: ", extPort) } diff --git a/nat/traversal/pinger.go b/nat/traversal/pinger.go index 0f221684bb..dd057f8763 100644 --- a/nat/traversal/pinger.go +++ b/nat/traversal/pinger.go @@ -227,11 +227,11 @@ func (p *Pinger) ping(conn *net.UDPConn) error { err := p.sendPingRequest(conn, n) if err != nil { - p.eventPublisher.Publish(event.EventTopic, event.BuildFailureEvent(StageName, err)) + p.eventPublisher.Publish(event.Topic, event.BuildFailureEvent(StageName, err)) return err } - p.eventPublisher.Publish(event.EventTopic, event.BuildSuccessEvent(StageName)) + p.eventPublisher.Publish(event.Topic, event.BuildSuccessfulEvent(StageName)) n++ }