From 476781de1ef18ce45772f7d2379218ec2f5c60f2 Mon Sep 17 00:00:00 2001 From: Donatas Kucinskas Date: Thu, 25 Jan 2018 11:26:34 +0200 Subject: [PATCH] Move "/ip" endpoint to "/connection/ip", refactor "connection_test.go" --- cmd/mysterium_client/run/command.go | 4 +- tequilapi/client/client.go | 2 +- tequilapi/endpoints/connection.go | 33 +++++- tequilapi/endpoints/connection_test.go | 136 ++++++++++++++++++++----- tequilapi/endpoints/current_ip.go | 35 ------- tequilapi/endpoints/current_ip_test.go | 49 --------- tequilapi/http_routes.go | 4 - 7 files changed, 140 insertions(+), 123 deletions(-) delete mode 100644 tequilapi/endpoints/current_ip.go delete mode 100644 tequilapi/endpoints/current_ip_test.go diff --git a/cmd/mysterium_client/run/command.go b/cmd/mysterium_client/run/command.go index 67df0d1948..edb0e4bfdf 100644 --- a/cmd/mysterium_client/run/command.go +++ b/cmd/mysterium_client/run/command.go @@ -8,6 +8,7 @@ import ( nats_dialog "github.com/mysterium/node/communication/nats/dialog" nats_discovery "github.com/mysterium/node/communication/nats/discovery" "github.com/mysterium/node/identity" + "github.com/mysterium/node/ipify" "github.com/mysterium/node/openvpn" "github.com/mysterium/node/server" "github.com/mysterium/node/tequilapi" @@ -48,7 +49,8 @@ func NewCommandWith( router := tequilapi.NewAPIRouter() tequilapi_endpoints.AddRoutesForIdentities(router, identityManager, mysteriumClient, signerFactory) - tequilapi_endpoints.AddRoutesForConnection(router, connectionManager) + ipResolver := ipify.NewClient().GetPublicIP + tequilapi_endpoints.AddRoutesForConnection(router, connectionManager, ipResolver) tequilapi_endpoints.AddRoutesForProposals(router, mysteriumClient) httpAPIServer := tequilapi.NewServer(options.TequilapiAddress, options.TequilapiPort, router) diff --git a/tequilapi/client/client.go b/tequilapi/client/client.go index c18c10666b..6eb3e5baa0 100644 --- a/tequilapi/client/client.go +++ b/tequilapi/client/client.go @@ -111,7 +111,7 @@ func (client *Client) Status() (status StatusDto, err error) { } func (client *Client) GetIP() (string, error) { - response, err := client.http.Get("ip", url.Values{}) + response, err := client.http.Get("connection/ip", url.Values{}) if err != nil { return "", err } diff --git a/tequilapi/endpoints/connection.go b/tequilapi/endpoints/connection.go index 1ab2b1a837..1b45eb5fd7 100644 --- a/tequilapi/endpoints/connection.go +++ b/tequilapi/endpoints/connection.go @@ -21,11 +21,17 @@ type statusResponse struct { } type connectionEndpoint struct { - manager client_connection.Manager + manager client_connection.Manager + ipResolver ipResolver } -func NewConnectionEndpoint(manager client_connection.Manager) *connectionEndpoint { - return &connectionEndpoint{manager} +type ipResolver func() (string, error) + +func NewConnectionEndpoint(manager client_connection.Manager, ipResolver ipResolver) *connectionEndpoint { + return &connectionEndpoint{ + manager: manager, + ipResolver: ipResolver, + } } func (ce *connectionEndpoint) Status(resp http.ResponseWriter, req *http.Request, params httprouter.Params) { @@ -61,11 +67,28 @@ func (ce *connectionEndpoint) Kill(resp http.ResponseWriter, req *http.Request, resp.WriteHeader(http.StatusAccepted) } -func AddRoutesForConnection(router *httprouter.Router, manager client_connection.Manager) { - connectionEndpoint := NewConnectionEndpoint(manager) +// GetIP responds with current ip, using its ip resolver +func (ce *connectionEndpoint) GetIP(writer http.ResponseWriter, request *http.Request, params httprouter.Params) { + ip, err := ce.ipResolver() + if err != nil { + utils.SendError(writer, err, http.StatusInternalServerError) + return + } + response := struct { + IP string `json:"ip"` + }{ + IP: ip, + } + utils.WriteAsJSON(response, writer) +} + +// TODO: Uppercase IPResolver? +func AddRoutesForConnection(router *httprouter.Router, manager client_connection.Manager, ipResolver ipResolver) { + connectionEndpoint := NewConnectionEndpoint(manager, ipResolver) router.GET("/connection", connectionEndpoint.Status) router.PUT("/connection", connectionEndpoint.Create) router.DELETE("/connection", connectionEndpoint.Kill) + router.GET("/connection/ip", connectionEndpoint.GetIP) } func toConnectionRequest(req *http.Request) (*connectionRequest, error) { diff --git a/tequilapi/endpoints/connection_test.go b/tequilapi/endpoints/connection_test.go index 64377e43ca..3845600dce 100644 --- a/tequilapi/endpoints/connection_test.go +++ b/tequilapi/endpoints/connection_test.go @@ -1,13 +1,14 @@ package endpoints import ( - "bytes" + "errors" "github.com/julienschmidt/httprouter" "github.com/mysterium/node/client_connection" "github.com/mysterium/node/identity" "github.com/stretchr/testify/assert" "net/http" "net/http/httptest" + "strings" "testing" ) @@ -40,6 +41,53 @@ func (fm *fakeManager) Wait() error { return nil } +func TestAddRoutesForConnectionAddsRoutes(t *testing.T) { + router := httprouter.New() + fakeManager := fakeManager{} + ipResolver := func() (string, error) { + return "123.123.123.123", nil + } + + AddRoutesForConnection(router, &fakeManager, ipResolver) + + tests := []struct { + method string + path string + body string + expectedStatus int + expectedJSON string + }{ + { + http.MethodGet, "/connection", "", + http.StatusOK, `{"status": ""}`, + }, + { + http.MethodPut, "/connection", `{"identity": "identity", "nodeKey": "nodeKey"}`, + http.StatusCreated, `{"status": ""}`, + }, + { + http.MethodDelete, "/connection", "", + http.StatusAccepted, "", + }, + { + http.MethodGet, "/connection/ip", "", + http.StatusOK, `{"ip": "123.123.123.123"}`, + }, + } + + for _, test := range tests { + resp := httptest.NewRecorder() + req := httptest.NewRequest(test.method, test.path, strings.NewReader(test.body)) + router.ServeHTTP(resp, req) + assert.Equal(t, test.expectedStatus, resp.Code) + if test.expectedJSON != "" { + assert.JSONEq(t, test.expectedJSON, resp.Body.String()) + } else { + assert.Equal(t, "", resp.Body.String()) + } + } +} + func TestDisconnectingState(t *testing.T) { var fakeManager = fakeManager{} fakeManager.onStatusReturn = client_connection.ConnectionStatus{ @@ -47,12 +95,11 @@ func TestDisconnectingState(t *testing.T) { SessionID: "", } - connEndpoint := NewConnectionEndpoint(&fakeManager) - req, err := http.NewRequest(http.MethodGet, "/irrelevant", nil) - assert.Nil(t, err) + connEndpoint := NewConnectionEndpoint(&fakeManager, nil) + req := httptest.NewRequest(http.MethodGet, "/irrelevant", nil) resp := httptest.NewRecorder() - connEndpoint.Status(resp, req, httprouter.Params{}) + connEndpoint.Status(resp, req, nil) assert.Equal(t, http.StatusOK, resp.Code) assert.JSONEq( @@ -70,9 +117,8 @@ func TestNotConnectedStateIsReturnedWhenNoConnection(t *testing.T) { SessionID: "", } - connEndpoint := NewConnectionEndpoint(&fakeManager) - req, err := http.NewRequest(http.MethodGet, "/connection", nil) - assert.Nil(t, err) + connEndpoint := NewConnectionEndpoint(&fakeManager, nil) + req := httptest.NewRequest(http.MethodGet, "/irrelevant", nil) resp := httptest.NewRecorder() connEndpoint.Status(resp, req, httprouter.Params{}) @@ -93,9 +139,8 @@ func TestStateConnectingIsReturnedWhenIsConnectionInProgress(t *testing.T) { State: client_connection.Connecting, } - connEndpoint := NewConnectionEndpoint(&fakeManager) - req, err := http.NewRequest(http.MethodGet, "/connection", nil) - assert.Nil(t, err) + connEndpoint := NewConnectionEndpoint(&fakeManager, nil) + req := httptest.NewRequest(http.MethodGet, "/irrelevant", nil) resp := httptest.NewRecorder() connEndpoint.Status(resp, req, httprouter.Params{}) @@ -117,9 +162,8 @@ func TestConnectedStateAndSessionIdIsReturnedWhenIsConnected(t *testing.T) { SessionID: "My-super-session", } - connEndpoint := NewConnectionEndpoint(&fakeManager) - req, err := http.NewRequest(http.MethodGet, "/connection", nil) - assert.Nil(t, err) + connEndpoint := NewConnectionEndpoint(&fakeManager, nil) + req := httptest.NewRequest(http.MethodGet, "/irrelevant", nil) resp := httptest.NewRecorder() connEndpoint.Status(resp, req, httprouter.Params{}) @@ -138,9 +182,8 @@ func TestConnectedStateAndSessionIdIsReturnedWhenIsConnected(t *testing.T) { func TestPutReturns400ErrorIfRequestBodyIsNotJson(t *testing.T) { fakeManager := fakeManager{} - connEndpoint := NewConnectionEndpoint(&fakeManager) - req, err := http.NewRequest(http.MethodPut, "/connection", bytes.NewBufferString("a")) - assert.Nil(t, err) + connEndpoint := NewConnectionEndpoint(&fakeManager, nil) + req := httptest.NewRequest(http.MethodPut, "/irrelevant", strings.NewReader("a")) resp := httptest.NewRecorder() connEndpoint.Create(resp, req, httprouter.Params{}) @@ -158,9 +201,8 @@ func TestPutReturns400ErrorIfRequestBodyIsNotJson(t *testing.T) { func TestPutReturns422ErrorIfRequestBodyIsMissingFieldValues(t *testing.T) { fakeManager := fakeManager{} - connEndpoint := NewConnectionEndpoint(&fakeManager) - req, err := http.NewRequest(http.MethodPut, "/connection", bytes.NewBufferString("{}")) - assert.Nil(t, err) + connEndpoint := NewConnectionEndpoint(&fakeManager, nil) + req := httptest.NewRequest(http.MethodPut, "/irrelevant", strings.NewReader("{}")) resp := httptest.NewRecorder() connEndpoint.Create(resp, req, httprouter.Params{}) @@ -181,16 +223,15 @@ func TestPutReturns422ErrorIfRequestBodyIsMissingFieldValues(t *testing.T) { func TestPutWithValidBodyCreatesConnection(t *testing.T) { fakeManager := fakeManager{} - connEndpoint := NewConnectionEndpoint(&fakeManager) - req, err := http.NewRequest( + connEndpoint := NewConnectionEndpoint(&fakeManager, nil) + req := httptest.NewRequest( http.MethodPut, - "/connection", - bytes.NewBufferString( + "/irrelevant", + strings.NewReader( `{ "identity" : "my-identity", "nodeKey" : "required-node" }`)) - assert.Nil(t, err) resp := httptest.NewRecorder() connEndpoint.Create(resp, req, httprouter.Params{}) @@ -205,9 +246,8 @@ func TestPutWithValidBodyCreatesConnection(t *testing.T) { func TestDeleteCallsDisconnect(t *testing.T) { fakeManager := fakeManager{} - connEndpoint := NewConnectionEndpoint(&fakeManager) - req, err := http.NewRequest(http.MethodDelete, "/connection", nil) - assert.Nil(t, err) + connEndpoint := NewConnectionEndpoint(&fakeManager, nil) + req := httptest.NewRequest(http.MethodDelete, "/irrelevant", nil) resp := httptest.NewRecorder() connEndpoint.Kill(resp, req, httprouter.Params{}) @@ -216,3 +256,43 @@ func TestDeleteCallsDisconnect(t *testing.T) { assert.Equal(t, fakeManager.disconnectCount, 1) } + +func TestGetIPEndpointSucceeds(t *testing.T) { + manager := fakeManager{} + ipResolver := func() (string, error) { + return "123.123.123.123", nil + } + connEndpoint := NewConnectionEndpoint(&manager, ipResolver) + resp := httptest.NewRecorder() + + connEndpoint.GetIP(resp, nil, nil) + + assert.Equal(t, http.StatusOK, resp.Code) + assert.JSONEq( + t, + `{ + "ip": "123.123.123.123" + }`, + resp.Body.String(), + ) +} + +func TestGetIPEndpointReturnsErrorWhenIPDetectionFails(t *testing.T) { + manager := fakeManager{} + ipResolver := func() (string, error) { + return "", errors.New("fake error") + } + connEndpoint := NewConnectionEndpoint(&manager, ipResolver) + resp := httptest.NewRecorder() + + connEndpoint.GetIP(resp, nil, nil) + + assert.Equal(t, http.StatusInternalServerError, resp.Code) + assert.JSONEq( + t, + `{ + "message": "fake error" + }`, + resp.Body.String(), + ) +} diff --git a/tequilapi/endpoints/current_ip.go b/tequilapi/endpoints/current_ip.go deleted file mode 100644 index 0158dd82e2..0000000000 --- a/tequilapi/endpoints/current_ip.go +++ /dev/null @@ -1,35 +0,0 @@ -package endpoints - -import ( - "github.com/julienschmidt/httprouter" - "github.com/mysterium/node/tequilapi/utils" - "net/http" -) - -type currentIPEndpoint struct { - ipResolver ipResolver -} - -type ipResolver func() (string, error) - -// NewCurrentIPEndpoint returns endpoint for detecting current ip -func NewCurrentIPEndpoint(ipResolver ipResolver) *currentIPEndpoint { - return ¤tIPEndpoint{ - ipResolver: ipResolver, - } -} - -// CurrentIP responds with current ip, using its ip resolver -func (ipe *currentIPEndpoint) CurrentIP(writer http.ResponseWriter, request *http.Request, params httprouter.Params) { - ip, err := ipe.ipResolver() - if err != nil { - utils.SendError(writer, err, http.StatusInternalServerError) - return - } - response := struct { - IP string `json:"ip"` - }{ - IP: ip, - } - utils.WriteAsJSON(response, writer) -} diff --git a/tequilapi/endpoints/current_ip_test.go b/tequilapi/endpoints/current_ip_test.go deleted file mode 100644 index cb39c6e4cd..0000000000 --- a/tequilapi/endpoints/current_ip_test.go +++ /dev/null @@ -1,49 +0,0 @@ -package endpoints - -import ( - "github.com/julienschmidt/httprouter" - "github.com/pkg/errors" - "github.com/stretchr/testify/assert" - "net/http" - "net/http/httptest" - "testing" -) - -func TestCurrentIPEndpointSucceeds(t *testing.T) { - endpoint := NewCurrentIPEndpoint(func() (string, error) { - return "123.123.123.123", nil - }) - - resp := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/irrelevant", nil) - endpoint.CurrentIP(resp, req, httprouter.Params{}) - - assert.JSONEq( - t, - `{ - "ip": "123.123.123.123" - }`, - resp.Body.String(), - ) -} - -func TestCurrentIPEndpointReturnsErrorWhenIPDetectionFails(t *testing.T) { - endpoint := NewCurrentIPEndpoint(func() (string, error) { - return "", errors.New("fake error") - }) - - resp := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/irrelevant", nil) - endpoint.CurrentIP(resp, req, httprouter.Params{}) - - // TODO: status code - - assert.Equal(t, http.StatusInternalServerError, resp.Code) - assert.JSONEq( - t, - `{ - "message": "fake error" - }`, - resp.Body.String(), - ) -} diff --git a/tequilapi/http_routes.go b/tequilapi/http_routes.go index 881839e32a..11d5c7ecb0 100644 --- a/tequilapi/http_routes.go +++ b/tequilapi/http_routes.go @@ -2,7 +2,6 @@ package tequilapi import ( "github.com/julienschmidt/httprouter" - "github.com/mysterium/node/ipify" "github.com/mysterium/node/tequilapi/endpoints" "os" "time" @@ -15,8 +14,5 @@ func NewAPIRouter() *httprouter.Router { router.GET("/healthcheck", endpoints.HealthCheckEndpointFactory(time.Now, os.Getpid).HealthCheck) - ipifyClient := ipify.NewClient() - router.GET("/ip", endpoints.NewCurrentIPEndpoint(ipifyClient.GetPublicIP).CurrentIP) - return router }