Improve consistency of baseURL trailing slash handling.

There were mismatches between the documentation and actual behavior
of how the baseURL trailing slash was handled.

There were also inconsistency of the gerritURL/endpoint parameter name
in the documentation of NewClient and the actual function signature.

This change fixes those and simplifies code.

gerrit.go

@@ -25,8 +25,8 @@ type Client struct {
 	// client is the HTTP client used to communicate with the API.
 	client *http.Client
 
-	// Base URL for API requests.
-	// BaseURL should always be specified with a trailing slash.
+	// baseURL is the base URL of the Gerrit instance for API requests.
+	// It must have a trailing slash.
 	baseURL *url.URL
 
 	// Gerrit service for authentication.
@@ -72,9 +72,10 @@ var (
 	ReParseURL = regexp.MustCompile(`^(http|https)://(.+):(.+)@(.+):(\d+)(.*)$`)
 )
 
-// NewClient returns a new Gerrit API client. The gerritURL argument has to be the
-// HTTP endpoint of the Gerrit instance, http://localhost:8080/ for example. If a nil
-// httpClient is provided, http.DefaultClient will be used.
+// NewClient returns a new Gerrit API client. gerritURL specifies the
+// HTTP endpoint of the Gerrit instance. For example, "http://localhost:8080/".
+// If gerritURL does not have a trailing slash, one is added automatically.
+// If a nil httpClient is provided, http.DefaultClient will be used.
 //
 // The url may contain credentials, http://admin:secret@localhost:8081/ for
 // example. These credentials may either be a user name and password or
@@ -83,12 +84,13 @@ var (
 // returning the client. ErrAuthenticationFailed will be returned if the credentials
 // cannot be validated. The process of validating the credentials is relatively simple and
 // only requires that the provided user have permission to GET /a/accounts/self.
-func NewClient(endpoint string, httpClient *http.Client) (*Client, error) {
+func NewClient(gerritURL string, httpClient *http.Client) (*Client, error) {
 	if httpClient == nil {
 		httpClient = http.DefaultClient
 	}
 
-	if len(endpoint) == 0 {
+	endpoint := gerritURL
+	if endpoint == "" {
 		return nil, ErrNoInstanceGiven
 	}
 
@@ -118,6 +120,9 @@ func NewClient(endpoint string, httpClient *http.Client) (*Client, error) {
 	if err != nil {
 		return nil, err
 	}
+	if !strings.HasSuffix(baseURL.Path, "/") {
+		baseURL.Path += "/"
+	}
 
 	// Note, if we retrieved the URL and password using the regular
 	// expression above then the below code will do nothing.
@@ -314,31 +319,24 @@ func (c *Client) Call(method, u string, body interface{}, v interface{}) (*Respo
 // there has to be "projects/plugin%25Fdelete-project" instead of "projects/plugin/delete-project".
 // The second url will return nothing.
 func (c *Client) buildURLForRequest(urlStr string) (string, error) {
-	u := c.baseURL.String()
-
-	// If there is no / at the end, add one
-	if strings.HasSuffix(u, "/") == false {
-		u += "/"
-	}
-
-	// If there is a "/" at the start, remove it
-	if strings.HasPrefix(urlStr, "/") == true {
-		urlStr = urlStr[1:]
-	}
-
-	// If we are authenticated, lets apply the a/ prefix but only if it has
-	// not already been applied.
-	if c.Authentication.HasAuth() == true && !strings.HasPrefix(urlStr, "a/") {
+	// If there is a "/" at the start, remove it.
+	// TODO: It can be arranged for all callers of buildURLForRequest to never have a "/" prefix,
+	//       which can be ensured via tests. This is how it's done in go-github.
+	//       Then, this run-time check becomes unnecessary and can be removed.
+	urlStr = strings.TrimPrefix(urlStr, "/")
+
+	// If we are authenticated, let's apply the "a/" prefix,
+	// but only if it has not already been applied.
+	if c.Authentication.HasAuth() && !strings.HasPrefix(urlStr, "a/") {
 		urlStr = "a/" + urlStr
 	}
 
 	rel, err := url.Parse(urlStr)
 	if err != nil {
 		return "", err
 	}
-	u += rel.String()
 
-	return u, nil
+	return c.baseURL.String() + rel.String(), nil
 }
 
 // Do sends an API request and returns the API response.
@@ -423,7 +421,6 @@ func (c *Client) addAuthentication(req *http.Request) error {
 		response, err := c.client.Do(digestRequest)
 		if err != nil {
 			return err
-
 		}
 
 		// When the function exits discard the rest of the
@@ -434,7 +431,6 @@ func (c *Client) addAuthentication(req *http.Request) error {
 
 		if response.StatusCode == http.StatusUnauthorized {
 			authorization, err := c.Authentication.digestAuthHeader(response)
-
 			if err != nil {
 				return err
 			}