From 6f51c3569f9fa6df4a2ddef551f3789e51729171 Mon Sep 17 00:00:00 2001 From: Phil Adams Date: Sat, 7 Sep 2019 09:31:07 -0500 Subject: [PATCH] feat: Defer service URL validation Fixes arf/planning-sdk-squad#1011 Why: In order to support parameterized URLs, we need to defer the validation of the service URL value (i.e. myservice.Service.Options.URL) to the point at which it is first needed, instead of when the BaseService struct is constructed. What: This PR contains changes to support the above, plus the following: 1) Added comments here and there to some public structs and methods. 2) Made certain elements within BaseService private since they're not needed outside BaseService. 3) Removed the Version field from the ServiceOptions struct. Instead, the Go generator will be changed to define Version as a field of the generated service struct (e.g. AssistantV1) only when the service is detected to use a "version param". The Go core has no use for the Version field as it is only ever referenced by the generated service code, so better to move the definition of the field there as well. Plus, we only generate the Version-related code if we find that the API definition defines a "version param". --- core/base_service.go | 63 ++++++--- core/base_service_test.go | 172 +++++++++++++++--------- core/basic_authenticator_test.go | 7 +- core/bearer_token_authenticator_test.go | 7 +- core/cp4d_authenticator.go | 5 +- core/iam_authenticator.go | 8 +- core/noauth_authenticator_test.go | 7 +- core/request_builder.go | 18 ++- core/request_builder_test.go | 91 ++++++++----- 9 files changed, 245 insertions(+), 133 deletions(-) diff --git a/core/base_service.go b/core/base_service.go index b46a01a..05775b4 100644 --- a/core/base_service.go +++ b/core/base_service.go @@ -26,29 +26,44 @@ import ( "time" ) -// common constants for core const ( - USER_AGENT = "User-Agent" - SDK_NAME = "ibm-go-sdk-core" - UNKNOWN_ERROR = "Unknown Error" + header_name_USER_AGENT = "User-Agent" + sdk_name = "ibm-go-sdk-core" ) -// ServiceOptions Service options +// ServiceOptions : This struct contains the options supported by the BaseService methods. type ServiceOptions struct { - Version string - URL string + // This is the base URL associated with the service instance. + // This value will be combined with the path for each operation to form the request URL. + URL string + + // This field holds the authenticator for the service instance. + // The authenticator will "authenticate" each outbound request by adding additional + // information to the request, typically in the form of the "Authorization" http header. Authenticator Authenticator } -// BaseService Base Service +// BaseService : This struct defines a common "service" object that is used by each generated service +// to manage requests and responses, perform authentication, etc. type BaseService struct { - Options *ServiceOptions + + // The options related to the base service. + Options *ServiceOptions + + // A set of "default" http headers to be included with each outbound request. + // This can be set by the SDK user. DefaultHeaders http.Header - Client *http.Client - UserAgent string + + // The HTTP Client used to send requests and receive responses. + Client *http.Client + + // The value to be used for the "User-Agent" HTTP header that is added to each outbound request. + // If this value is not set, then a default value will be used for the header. + UserAgent string } -// NewBaseService Instantiate a Base Service +// NewBaseService : This function will construct a new instance of the BaseService struct, while +// performing validation on input parameters and service options. func NewBaseService(options *ServiceOptions, serviceName, displayName string) (*BaseService, error) { if HasBadFirstOrLastChar(options.URL) { return nil, fmt.Errorf(ERRORMSG_PROP_INVALID, "URL") @@ -71,7 +86,7 @@ func NewBaseService(options *ServiceOptions, serviceName, displayName string) (* } // Set a default value for the User-Agent http header. - service.SetUserAgent(service.BuildUserAgent()) + service.SetUserAgent(service.buildUserAgent()) // Try to load service properties from external config. serviceProps, err := getServiceProperties(serviceName) @@ -110,7 +125,14 @@ func NewBaseService(options *ServiceOptions, serviceName, displayName string) (* } // SetURL sets the service URL +// +// Deprecated: use SetServiceURL instead. func (service *BaseService) SetURL(url string) error { + return service.SetServiceURL(url) +} + +// SetServiceURL sets the service URL +func (service *BaseService) SetServiceURL(url string) error { if HasBadFirstOrLastChar(url) { return fmt.Errorf(ERRORMSG_PROP_INVALID, "URL") } @@ -119,6 +141,11 @@ func (service *BaseService) SetURL(url string) error { return nil } +// GetServiceURL returns the service URL +func (service *BaseService) GetServiceURL() string { + return service.Options.URL +} + // SetDefaultHeaders sets HTTP headers to be sent in every request. func (service *BaseService) SetDefaultHeaders(headers http.Header) { service.DefaultHeaders = headers @@ -138,14 +165,14 @@ func (service *BaseService) DisableSSLVerification() { } // BuildUserAgent : Builds the user agent string -func (service *BaseService) BuildUserAgent() string { - return fmt.Sprintf("%s-%s %s", SDK_NAME, __VERSION__, SystemInfo()) +func (service *BaseService) buildUserAgent() string { + return fmt.Sprintf("%s-%s %s", sdk_name, __VERSION__, SystemInfo()) } // SetUserAgent : Sets the user agent value func (service *BaseService) SetUserAgent(userAgentString string) { if userAgentString == "" { - service.UserAgent = service.BuildUserAgent() + service.UserAgent = service.buildUserAgent() } service.UserAgent = userAgentString } @@ -160,9 +187,9 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta } // Add the default User-Agent header if not already present. - userAgent := req.Header.Get(USER_AGENT) + userAgent := req.Header.Get(header_name_USER_AGENT) if userAgent == "" { - req.Header.Add(USER_AGENT, service.UserAgent) + req.Header.Add(header_name_USER_AGENT, service.UserAgent) } // Add authentication to the outbound request. diff --git a/core/base_service_test.go b/core/base_service_test.go index b06b22d..381a8eb 100644 --- a/core/base_service_test.go +++ b/core/base_service_test.go @@ -41,9 +41,10 @@ func TestGoodResponseJSON(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("POST"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("POST") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -81,9 +82,10 @@ func TestGoodResponseJSONExtraFields(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -108,9 +110,10 @@ func TestGoodResponseNonJSON(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -149,9 +152,10 @@ func TestGoodResponseNonJSONNoContentType(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -189,9 +193,10 @@ func TestGoodResponseJSONDeserFailure(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -218,9 +223,10 @@ func TestGoodResponseNoBody(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("POST"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -294,9 +300,10 @@ func TestErrorResponseJSON(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -327,9 +334,10 @@ func TestErrorResponseJSONDeserError(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -356,9 +364,10 @@ func TestErrorResponseNotJSON(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -385,9 +394,10 @@ func TestErrorResponseNoBody(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("POST"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -430,9 +440,10 @@ func TestRequestForDefaultUserAgent(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -453,9 +464,10 @@ func TestRequestForProvidedUserAgent(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09") req, _ := builder.Build() @@ -516,12 +528,13 @@ func TestBasicAuth1(t *testing.T) { assert.NotNil(t, service.Options.Authenticator) assert.Equal(t, AUTHTYPE_BASIC, service.Options.Authenticator.AuthenticationType()) - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddQuery("Version", "2018-22-09") + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddQuery("Version", "2018-22-09") req, _ := builder.Build() - _, err := service.Request(req, new(Foo)) + _, err = service.Request(req, new(Foo)) assert.Nil(t, err) } @@ -543,9 +556,10 @@ func TestBasicAuth2(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddQuery("Version", "2018-22-09") + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddQuery("Version", "2018-22-09") req, _ := builder.Build() options := &ServiceOptions{ @@ -594,9 +608,10 @@ func TestNoAuth1(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddQuery("Version", "2018-22-09") + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddQuery("Version", "2018-22-09") req, _ := builder.Build() options := &ServiceOptions{ @@ -621,9 +636,10 @@ func TestNoAuth2(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddQuery("Version", "2018-22-09") + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddQuery("Version", "2018-22-09") req, _ := builder.Build() options := &ServiceOptions{ @@ -667,9 +683,10 @@ func TestIAMAuth(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddQuery("Version", "2018-22-09") + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddQuery("Version", "2018-22-09") req, _ := builder.Build() options := &ServiceOptions{ @@ -699,9 +716,10 @@ func TestIAMFailure(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddQuery("Version", "2018-22-09") + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddQuery("Version", "2018-22-09") req, _ := builder.Build() options := &ServiceOptions{ @@ -743,9 +761,10 @@ func TestIAMWithIdSecret(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddQuery("Version", "2018-22-09") + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddQuery("Version", "2018-22-09") req, _ := builder.Build() options := &ServiceOptions{ @@ -828,9 +847,10 @@ func TestCP4DAuth(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddQuery("Version", "2018-22-09") + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddQuery("Version", "2018-22-09") req, _ := builder.Build() options := &ServiceOptions{ @@ -857,9 +877,10 @@ func TestCP4DFail(t *testing.T) { })) defer server.Close() - builder := NewRequestBuilder("GET"). - ConstructHTTPURL(server.URL, nil, nil). - AddQuery("Version", "2018-22-09") + builder := NewRequestBuilder("GET") + _, err := builder.ConstructHTTPURL(server.URL, nil, nil) + assert.Nil(t, err) + builder.AddQuery("Version", "2018-22-09") req, _ := builder.Build() options := &ServiceOptions{ @@ -879,6 +900,7 @@ func TestCP4DFail(t *testing.T) { assert.Equal(t, "Sorry you are forbidden", err.Error()) } +// Test for the deprecated SetURL method. func TestSetURL(t *testing.T) { service, err := NewBaseService( &ServiceOptions{ @@ -893,6 +915,28 @@ func TestSetURL(t *testing.T) { assert.NotNil(t, err) } +func TestSetServiceURL(t *testing.T) { + service, err := NewBaseService( + &ServiceOptions{ + Authenticator: &NoAuthAuthenticator{}, + }, "watson", "watson") + assert.Nil(t, err) + assert.NotNil(t, service) + + err = service.SetServiceURL("{bad url}") + assert.NotNil(t, err) + + err = service.SetServiceURL("") + assert.Nil(t, err) + assert.Equal(t, "", service.Options.URL) + assert.Equal(t, "", service.GetServiceURL()) + + err = service.SetServiceURL("https://myserver.com/api/baseurl") + assert.Nil(t, err) + assert.Equal(t, "https://myserver.com/api/baseurl", service.Options.URL) + assert.Equal(t, "https://myserver.com/api/baseurl", service.GetServiceURL()) +} + func TestExtConfigFromCredentialFile(t *testing.T) { pwd, _ := os.Getwd() credentialFilePath := path.Join(pwd, "/../resources/my-credentials.env") diff --git a/core/basic_authenticator_test.go b/core/basic_authenticator_test.go index 3dc628c..e3806a5 100644 --- a/core/basic_authenticator_test.go +++ b/core/basic_authenticator_test.go @@ -69,9 +69,10 @@ func TestBasicAuthAuthenticate(t *testing.T) { assert.Equal(t, authenticator.AuthenticationType(), AUTHTYPE_BASIC) // Create a new Request object. - request, err := NewRequestBuilder("GET"). - ConstructHTTPURL("https://localhost/placeholder/url", nil, nil). - Build() + builder, err := NewRequestBuilder("GET").ConstructHTTPURL("https://localhost/placeholder/url", nil, nil) + assert.Nil(t, err) + + request, err := builder.Build() assert.Nil(t, err) assert.NotNil(t, request) diff --git a/core/bearer_token_authenticator_test.go b/core/bearer_token_authenticator_test.go index 7c90a87..29995c3 100644 --- a/core/bearer_token_authenticator_test.go +++ b/core/bearer_token_authenticator_test.go @@ -40,9 +40,10 @@ func TestBearerTokenAuthenticate(t *testing.T) { assert.Equal(t, authenticator.AuthenticationType(), AUTHTYPE_BEARER_TOKEN) // Create a new Request object. - request, err := NewRequestBuilder("GET"). - ConstructHTTPURL("https://localhost/placeholder/url", nil, nil). - Build() + builder, err := NewRequestBuilder("GET").ConstructHTTPURL("https://localhost/placeholder/url", nil, nil) + assert.Nil(t, err) + + request, err := builder.Build() assert.Nil(t, err) assert.NotNil(t, request) diff --git a/core/cp4d_authenticator.go b/core/cp4d_authenticator.go index 2983793..a6396b5 100644 --- a/core/cp4d_authenticator.go +++ b/core/cp4d_authenticator.go @@ -157,7 +157,10 @@ func (authenticator *CloudPakForDataAuthenticator) requestToken() (*cp4dTokenSer url = fmt.Sprintf("%s%s", url, PRE_AUTH_PATH) } - builder := NewRequestBuilder(GET).ConstructHTTPURL(url, nil, nil) + builder, err := NewRequestBuilder(GET).ConstructHTTPURL(url, nil, nil) + if err != nil { + return nil, err + } // Add user-defined headers to request. for headerName, headerValue := range authenticator.Headers { diff --git a/core/iam_authenticator.go b/core/iam_authenticator.go index 06b7b37..88384bd 100644 --- a/core/iam_authenticator.go +++ b/core/iam_authenticator.go @@ -176,8 +176,12 @@ func (authenticator *IamAuthenticator) requestToken() (*iamTokenServerResponse, } builder := NewRequestBuilder(POST) - builder.ConstructHTTPURL(url, nil, nil). - AddHeader(CONTENT_TYPE, DEFAULT_CONTENT_TYPE). + _, err := builder.ConstructHTTPURL(url, nil, nil) + if err != nil { + return nil, err + } + + builder.AddHeader(CONTENT_TYPE, DEFAULT_CONTENT_TYPE). AddHeader(Accept, APPLICATION_JSON). AddFormData("grant_type", "", "", REQUEST_TOKEN_GRANT_TYPE). AddFormData("apikey", "", "", authenticator.ApiKey). diff --git a/core/noauth_authenticator_test.go b/core/noauth_authenticator_test.go index 31bcf98..4723f2f 100644 --- a/core/noauth_authenticator_test.go +++ b/core/noauth_authenticator_test.go @@ -28,9 +28,10 @@ func TestNoAuthAuthenticate(t *testing.T) { assert.Equal(t, authenticator.AuthenticationType(), AUTHTYPE_NOAUTH) // Create a new Request object. - request, err := NewRequestBuilder("GET"). - ConstructHTTPURL("https://localhost/placeholder/url", nil, nil). - Build() + builder, err := NewRequestBuilder("GET").ConstructHTTPURL("https://localhost/placeholder/url", nil, nil) + assert.Nil(t, err) + + request, err := builder.Build() assert.Nil(t, err) assert.NotNil(t, request) diff --git a/core/request_builder.go b/core/request_builder.go index 8fba1ec..2be57f6 100644 --- a/core/request_builder.go +++ b/core/request_builder.go @@ -45,6 +45,9 @@ const ( CONTENT_DISPOSITION = "Content-Disposition" CONTENT_TYPE = "Content-Type" FORM_URL_ENCODED_HEADER = "application/x-www-form-urlencoded" + + ERRORMSG_SERVICE_URL_MISSING = "The service URL is required." + ERRORMSG_SERVICE_URL_INVALID = "There was an error parsing the service URL: %s" ) // A FormData stores information for form data @@ -74,13 +77,18 @@ func NewRequestBuilder(method string) *RequestBuilder { } } -// ConstructHTTPURL creates a properly encoded URL with path parameters. -func (requestBuilder *RequestBuilder) ConstructHTTPURL(endPoint string, pathSegments []string, pathParameters []string) *RequestBuilder { +// ConstructHTTPURL creates a properly-encoded URL with path parameters. +// This function returns an error if the serviceURL is "" or is an +// invalid URL string (e.g. ":"). +func (requestBuilder *RequestBuilder) ConstructHTTPURL(serviceURL string, pathSegments []string, pathParameters []string) (*RequestBuilder, error) { + if serviceURL == "" { + return requestBuilder, fmt.Errorf(ERRORMSG_SERVICE_URL_MISSING) + } var URL *url.URL - URL, err := url.Parse(endPoint) + URL, err := url.Parse(serviceURL) if err != nil { - panic(err) + return requestBuilder, fmt.Errorf(ERRORMSG_SERVICE_URL_INVALID, err.Error()) } for i, pathSegment := range pathSegments { @@ -90,7 +98,7 @@ func (requestBuilder *RequestBuilder) ConstructHTTPURL(endPoint string, pathSegm } } requestBuilder.URL = URL - return requestBuilder + return requestBuilder, nil } // AddQuery adds Query name and value diff --git a/core/request_builder_test.go b/core/request_builder_test.go index a136d03..2499176 100644 --- a/core/request_builder_test.go +++ b/core/request_builder_test.go @@ -18,6 +18,7 @@ import ( "bytes" "io" "os" + "strings" "testing" assert "github.com/stretchr/testify/assert" @@ -38,8 +39,9 @@ func TestConstructHTTPURL(t *testing.T) { pathParameters := []string{"xxxxx"} request := setup() want := "https://gateway.watsonplatform.net/assistant/api/v1/workspaces/xxxxx/message" - request.ConstructHTTPURL(endPoint, pathSegments, pathParameters) - assert.Equal(t, want, request.URL.String(), "Invalid comstruction of url") + _, err := request.ConstructHTTPURL(endPoint, pathSegments, pathParameters) + assert.Nil(t, err) + assert.Equal(t, want, request.URL.String(), "Invalid construction of url") } func TestConstructHTTPURLWithNoPathParam(t *testing.T) { @@ -47,8 +49,23 @@ func TestConstructHTTPURLWithNoPathParam(t *testing.T) { pathSegments := []string{"v1/workspaces"} request := setup() want := "https://gateway.watsonplatform.net/assistant/api/v1/workspaces" - request.ConstructHTTPURL(endPoint, pathSegments, nil) - assert.Equal(t, want, request.URL.String(), "Invalid comstruction of url") + _, err := request.ConstructHTTPURL(endPoint, pathSegments, nil) + assert.Nil(t, err) + assert.Equal(t, want, request.URL.String(), "Invalid construction of url") +} + +func TestConstructHTTPURLMissingURL(t *testing.T) { + request := setup() + _, err := request.ConstructHTTPURL("", nil, nil) + assert.NotNil(t, err) + assert.Equal(t, ERRORMSG_SERVICE_URL_MISSING, err.Error()) +} + +func TestConstructHTTPURLInvalidURL(t *testing.T) { + request := setup() + _, err := request.ConstructHTTPURL(":", nil, nil) + assert.NotNil(t, err) + assert.Equal(t, true, strings.HasPrefix(err.Error(), "There was an error parsing the service URL:")) } func TestAddQuery(t *testing.T) { @@ -148,14 +165,16 @@ func TestSetBodyContentError(t *testing.T) { } func TestBuildWithMultipartFormEmptyFileName(t *testing.T) { - request := NewRequestBuilder("POST"). - ConstructHTTPURL("test.com", nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("POST") + _, err := builder.ConstructHTTPURL("test.com", nil, nil) + assert.Nil(t, err) + + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09"). AddFormData("hello1", "", "text/plain", "Hello GO SDK"). AddFormData("hello2", "", "", "Hello GO SDK again") - req, _ := request.Build() - assert.NotNil(t, req.Body, "Couldnt build successfully") + request, _ := builder.Build() + assert.NotNil(t, request.Body, "Couldnt build successfully") } func TestBuildWithMultipartForm(t *testing.T) { @@ -166,9 +185,11 @@ func TestBuildWithMultipartForm(t *testing.T) { json2 := make(map[string]interface{}) json2["name2"] = "test name2" - request := NewRequestBuilder("POST"). - ConstructHTTPURL("test.com", nil, nil). - AddHeader("Content-Type", "Application/json"). + builder := NewRequestBuilder("POST") + _, err := builder.ConstructHTTPURL("test.com", nil, nil) + assert.Nil(t, err) + + builder.AddHeader("Content-Type", "Application/json"). AddQuery("Version", "2018-22-09"). AddFormData("name1", "json1.json", "application/json", json1). AddFormData("name2", "json2.json", "application/json", json2). @@ -177,29 +198,31 @@ func TestBuildWithMultipartForm(t *testing.T) { pwd, _ := os.Getwd() var testFile io.ReadCloser - testFile, err := os.Open(pwd + "/../resources/test_file.txt") + testFile, err = os.Open(pwd + "/../resources/test_file.txt") assert.Nil(t, err, "Could not open file") - request.AddFormData("test_file1", "test_file.txt", "application/octet-stream", testFile) - request.AddFormData("test_file2", "test_file.txt", "application/octet-stream", &testFile) + builder.AddFormData("test_file1", "test_file.txt", "application/octet-stream", testFile) + builder.AddFormData("test_file2", "test_file.txt", "application/octet-stream", &testFile) - _, err = request.Build() + request, err := builder.Build() assert.Nil(t, err, "Couldnt build successfully") + assert.NotNil(t, request) assert.NotNil(t, request.Body) defer testFile.Close() } func TestURLEncodedForm(t *testing.T) { - request := NewRequestBuilder("POST"). - ConstructHTTPURL("test.com", nil, nil). - AddHeader("Content-Type", FORM_URL_ENCODED_HEADER). + builder := NewRequestBuilder("POST") + _, err := builder.ConstructHTTPURL("test.com", nil, nil) + assert.Nil(t, err) + + builder.AddHeader("Content-Type", FORM_URL_ENCODED_HEADER). AddQuery("Version", "2018-22-09"). AddFormData("grant_type", "", "", "lalalala"). AddFormData("apikey", "", "", "xxxx") - _, err := request.Build() - if err != nil { - t.Errorf("Couldnt build successfully") - } + request, err := builder.Build() + assert.Nil(t, err) + assert.NotNil(t, request) } func TestBuild(t *testing.T) { @@ -214,17 +237,17 @@ func TestBuild(t *testing.T) { body := make(map[string]interface{}) body["name"] = testStructure.Name - request := NewRequestBuilder("POST"). - ConstructHTTPURL(endPoint, pathParameters, pathSegments). - AddHeader("Content-Type", "Application/json"). - AddQuery("Version", "2018-22-09") + builder := NewRequestBuilder("POST") + _, err := builder.ConstructHTTPURL(endPoint, pathParameters, pathSegments) + assert.Nil(t, err) - request, _ = request.SetBodyContentJSON(body) - req, err := request.Build() - if err != nil { - t.Errorf("Couldnt build successfully") - } + builder.AddHeader("Content-Type", "Application/json"). + AddQuery("Version", "2018-22-09") - assert.Equal(t, req.URL.String(), wantURL) - assert.Equal(t, req.Header["Content-Type"][0], "Application/json") + _, _ = builder.SetBodyContentJSON(body) + request, err := builder.Build() + assert.Nil(t, err) + assert.NotNil(t, request) + assert.Equal(t, wantURL, request.URL.String()) + assert.Equal(t, "Application/json", request.Header["Content-Type"][0]) }