From ad7eefc52fe1e4ee747f4356517ab0ffb8c646f2 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 24 Jul 2019 17:28:41 +0200 Subject: [PATCH] API CHANGE: NewClient(clientName, clientVersion) (#21) * API CHANGE: NewClient(clientName, clientVersion) This commit changes the NewClient API to take two arguments, the clientName and the clientVersion, instead of a userAgent. We didn't specify how userAgent was made. In theory we expected it to be composed of `${clientName}/${clientVersion}` but that was not enforced in any way. Per #16, we want the client to identify with the server also using the query string. This is for consistency with what the JavaScript client does. While there, also provide information on the OS and the architecture of the client, again as part of the query string. Closes #16. --- cmd/ndt7-client/main.go | 5 ++-- cmd/ndt7-client/main_test.go | 12 ++++----- example_test.go | 2 +- ndt7.go | 49 ++++++++++++++++++++++++++++-------- ndt7_test.go | 16 ++++++------ 5 files changed, 58 insertions(+), 26 deletions(-) diff --git a/cmd/ndt7-client/main.go b/cmd/ndt7-client/main.go index 7e083c5..3aa7856 100644 --- a/cmd/ndt7-client/main.go +++ b/cmd/ndt7-client/main.go @@ -86,7 +86,8 @@ import ( ) const ( - userAgent = "ndt7-client-go/0.1.0" + clientName = "ndt7-client-go-cmd" + clientVersion = "0.1.0" defaultTimeout = 55 * time.Second ) @@ -163,7 +164,7 @@ func main() { ctx, cancel := context.WithTimeout(context.Background(), *flagTimeout) defer cancel() var r runner - r.client = ndt7.NewClient(userAgent) + r.client = ndt7.NewClient(clientName, clientVersion) r.client.Dialer.TLSClientConfig = &tls.Config{ InsecureSkipVerify: *flagNoVerify, } diff --git a/cmd/ndt7-client/main_test.go b/cmd/ndt7-client/main_test.go index fea739e..fd532ad 100644 --- a/cmd/ndt7-client/main_test.go +++ b/cmd/ndt7-client/main_test.go @@ -103,7 +103,7 @@ func (me mockedEmitter) OnComplete(subtest string) error { // the emitter.OnStarting function fails. func TestRunSubtestOnStartingError(t *testing.T) { runner := runner{ - client: ndt7.NewClient(userAgent), + client: ndt7.NewClient(clientName, clientVersion), emitter: mockedEmitter{ StartingError: errors.New("mocked error"), }, @@ -129,7 +129,7 @@ func TestRunSubtestOnStartingError(t *testing.T) { // the emitter.OnConnected function fails. func TestRunSubtestOnConnectedError(t *testing.T) { runner := runner{ - client: ndt7.NewClient(userAgent), + client: ndt7.NewClient(clientName, clientVersion), emitter: mockedEmitter{ ConnectedError: errors.New("mocked error"), }, @@ -155,7 +155,7 @@ func TestRunSubtestOnConnectedError(t *testing.T) { // the emitter.OnComplete function fails. func TestRunSubtestOnCompleteError(t *testing.T) { runner := runner{ - client: ndt7.NewClient(userAgent), + client: ndt7.NewClient(clientName, clientVersion), emitter: mockedEmitter{ CompleteError: errors.New("mocked error"), }, @@ -181,7 +181,7 @@ func TestRunSubtestOnCompleteError(t *testing.T) { // the emitEvent function fails. func TestRunSubtestEmitEventError(t *testing.T) { runner := runner{ - client: ndt7.NewClient(userAgent), + client: ndt7.NewClient(clientName, clientVersion), emitter: mockedEmitter{}, } code := runner.runSubtest( @@ -213,7 +213,7 @@ func TestBatchEmitterEventsOrderNormal(t *testing.T) { } writer := &mocks.SavingWriter{} runner := runner{ - client: ndt7.NewClient(userAgent), + client: ndt7.NewClient(clientName, clientVersion), emitter: emitter.Batch{Writer: writer}, } code := runner.runSubtest( @@ -269,7 +269,7 @@ func TestBatchEmitterEventsOrderFailure(t *testing.T) { } writer := &mocks.SavingWriter{} runner := runner{ - client: ndt7.NewClient(userAgent), + client: ndt7.NewClient(clientName, clientVersion), emitter: emitter.Batch{Writer: writer}, } runner.client.MLabNSClient.BaseURL = "\t" // URL parser error diff --git a/example_test.go b/example_test.go index dfc5ff8..f7de1cd 100644 --- a/example_test.go +++ b/example_test.go @@ -12,7 +12,7 @@ import ( func Example() { ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() - client := ndt7.NewClient("example/v0.1.0") + client := ndt7.NewClient("ndt7-client-go-example", "0.1.0") ch, err := client.StartDownload(ctx) if err != nil { log.Fatal(err) diff --git a/ndt7.go b/ndt7.go index d6a07af..f9f0b50 100644 --- a/ndt7.go +++ b/ndt7.go @@ -13,6 +13,7 @@ import ( "context" "net/http" "net/url" + "runtime" "time" "github.com/gorilla/websocket" @@ -24,6 +25,14 @@ import ( "github.com/m-lab/ndt7-client-go/spec" ) +const ( + // libraryName is the name of this library + libraryName = "ndt7-client-go" + + // libraryVersion is the version of this library + libraryVersion = "0.1.0" +) + // locateFn is the type of function used to locate a server. type locateFn = func(ctx context.Context, client *mlabns.Client) (string, error) @@ -58,9 +67,13 @@ type Client struct { // defaults in NewClient and you may override it. MLabNSClient *mlabns.Client - // UserAgent is the user-agent that will be used. It's set by + // ClientName is the name of the software running ndt7 tests. It's set by // NewClient; you may want to change this value. - UserAgent string + ClientName string + + // ClientVersion is the version of the software running ndt7 tests. It's + // set by NewClient; you may want to change this value. + ClientVersion string // connect is the function for connecting a specific // websocket cnnection. It's set to its default value by @@ -80,11 +93,18 @@ type Client struct { upload subtestFn } -// NewClient creates a new client instance identified by the -// specified user agent. M-Lab services may reject requests coming -// from clients with empty user agents in the future. -func NewClient(userAgent string) *Client { +// makeUserAgent creates the user agent string +func makeUserAgent(clientName, clientVersion string) string { + return clientName + "/" + clientVersion + " " + libraryName + "/" + libraryVersion +} + +// NewClient creates a new client instance identified by the specified +// clientName and clientVersion. M-Lab services may reject requests coming +// from clients that do not identify themselves properly. +func NewClient(clientName, clientVersion string) *Client { return &Client{ + ClientName: clientName, + ClientVersion: clientVersion, connect: func( dialer websocket.Dialer, ctx context.Context, urlStr string, requestHeader http.Header) (*websocket.Conn, *http.Response, error, @@ -98,9 +118,10 @@ func NewClient(userAgent string) *Client { locate: func(ctx context.Context, c *mlabns.Client) (string, error) { return c.Query(ctx) }, - MLabNSClient: mlabns.NewClient("ndt7", userAgent), - upload: upload.Run, - UserAgent: userAgent, + MLabNSClient: mlabns.NewClient( + "ndt7", makeUserAgent(clientName, clientVersion), + ), + upload: upload.Run, } } @@ -115,9 +136,17 @@ func (c *Client) doConnect(ctx context.Context, URLPath string) (*websocket.Conn URL.Scheme = "wss" URL.Host = c.FQDN URL.Path = URLPath + q := URL.Query() + q.Set("client_arch", runtime.GOARCH) + q.Set("client_library_name", libraryName) + q.Set("client_library_version", libraryVersion) + q.Set("client_name", c.ClientName) + q.Set("client_os", runtime.GOOS) + q.Set("client_version", c.ClientVersion) + URL.RawQuery = q.Encode() headers := http.Header{} headers.Add("Sec-WebSocket-Protocol", params.SecWebSocketProtocol) - headers.Add("User-Agent", c.UserAgent) + headers.Add("User-Agent", makeUserAgent(c.ClientName, c.ClientVersion)) conn, _, err := c.connect(c.Dialer, ctx, URL.String(), headers) return conn, err } diff --git a/ndt7_test.go b/ndt7_test.go index f07b75c..fd0fc6d 100644 --- a/ndt7_test.go +++ b/ndt7_test.go @@ -11,13 +11,15 @@ import ( "github.com/m-lab/ndt7-client-go/spec" ) -// userAgent is the user agent used throughout this file -const userAgent = "ndt7-client-go/0.1.0" +const ( + clientName = "ndt7-client-go-tests" + clientVersion = "0.1.0" +) // newMockedClient returns a mocked client that does nothing // except pretending it is doing something. func newMockedClient(ctx context.Context) *Client { - client := NewClient(userAgent) + client := NewClient(clientName, clientVersion) // Override locate to return a fake IP address client.locate = func(ctx context.Context, c *mlabns.Client) (string, error) { return "127.0.0.1", nil @@ -72,7 +74,7 @@ func TestUploadCase(t *testing.T) { // with an error when discovering a server. func TestStartDiscoverServerError(t *testing.T) { ctx := context.Background() - client := NewClient(userAgent) + client := NewClient(clientName, clientVersion) client.MLabNSClient.BaseURL = "\t" // cause URL parse to fail _, err := client.start(ctx, nil, "") if err == nil { @@ -84,7 +86,7 @@ func TestStartDiscoverServerError(t *testing.T) { // with an error when connecting. func TestStartConnectError(t *testing.T) { ctx := context.Background() - client := NewClient(userAgent) + client := NewClient(clientName, clientVersion) client.FQDN = "\t" // cause URL parse to fail _, err := client.start(ctx, nil, "") if err == nil { @@ -97,7 +99,7 @@ func TestIntegrationDownload(t *testing.T) { if testing.Short() { t.Skip("Skipping test in short mode") } - client := NewClient(userAgent) + client := NewClient(clientName, clientVersion) ch, err := client.StartDownload(context.Background()) if err != nil { t.Fatal(err) @@ -139,7 +141,7 @@ func TestIntegrationUpload(t *testing.T) { if testing.Short() { t.Skip("Skipping test in short mode") } - client := NewClient(userAgent) + client := NewClient(clientName, clientVersion) ch, err := client.StartUpload(context.Background()) if err != nil { t.Fatal(err)