Applied a few smaller code changes, cleanups and removed DoNoClose, because a similar function was merged in between

authentication.go

@@ -1,6 +1,8 @@
 package jira
 
-import "fmt"
+import (
+	"fmt"
+)
 
 // AuthenticationService handles authentication for the JIRA instance / API.
 //
@@ -65,9 +67,8 @@ func (s *AuthenticationService) AcquireSessionCookie(username, password string)
 func (s *AuthenticationService) Authenticated() bool {
 	if s != nil {
 		return s.client.session != nil
-	} else {
-		return false
 	}
+	return false
 }
 
 // TODO Missing API Call GET (Returns information about the currently authenticated user's session)

issue.go

@@ -34,7 +34,6 @@ type Attachment struct {
 	Self     string `json:"self,omitempty"`
 	Id       string `json:"id,omitempty"`
 	Filename string `json:"filename,omitempty"`
-	//	TODO Missing fields
 	Author    *Assignee `json:"author,omitempty"`
 	Created   string    `json:"created,omitempty"`
 	Size      int       `json:"size,omitempty"`
@@ -254,9 +253,10 @@ func (s *IssueService) Get(issueID string) (*Issue, *http.Response, error) {
 	return issue, resp, nil
 }
 
-// DownloadAttachment returns an ioReader of an attachment for a given attachment Id
-// The attachment is in the Body of the response
-// The caller should close resp.Body
+// DownloadAttachment returns a http.Response of an attachment for a given attachmentID.
+// The attachment is in the http.Response.Body of the response.
+// This is an io.ReadCloser.
+// The caller should close the resp.Body.
 func (s *IssueService) DownloadAttachment(attachmentID string) (*http.Response, error) {
 	apiEndpoint := fmt.Sprintf("secure/attachment/%s/", attachmentID)
 	req, err := s.client.NewRequest("GET", apiEndpoint, nil)
@@ -264,7 +264,7 @@ func (s *IssueService) DownloadAttachment(attachmentID string) (*http.Response,
 		return nil, err
 	}
 
-	resp, err := s.client.DoNoClose(req, nil)
+	resp, err := s.client.Do(req, nil)
 	if err != nil {
 		return resp, err
 	}
@@ -272,7 +272,7 @@ func (s *IssueService) DownloadAttachment(attachmentID string) (*http.Response,
 	return resp, nil
 }
 
-// PostAttachment uploads an attachment provided as an io.Reader to a given attachment ID
+// PostAttachment uploads r (io.Reader) as an attachment to a given attachmentID
 func (s *IssueService) PostAttachment(attachmentID string, r io.Reader, attachmentName string) (*[]Attachment, *http.Response, error) {
 	apiEndpoint := fmt.Sprintf("rest/api/2/issue/%s/attachments", attachmentID)
 
@@ -307,7 +307,6 @@ func (s *IssueService) PostAttachment(attachmentID string, r io.Reader, attachme
 	}
 
 	return attachment, resp, nil
-
 }
 
 // Create creates an issue or a sub-task from a JSON representation.

jira.go

@@ -89,10 +89,10 @@ func (c *Client) NewRequest(method, urlStr string, body interface{}) (*http.Requ
 	return req, nil
 }
 
-// NewMultiPartRequest creates an API request including a multi-part file
+// NewMultiPartRequest creates an API request including a multi-part file.
 // A relative URL can be provided in urlStr, in which case it is resolved relative to the baseURL of the Client.
 // Relative URLs should always be specified without a preceding slash.
-// If specified, the value pointed to by buf is a multipart form
+// If specified, the value pointed to by buf is a multipart form.
 func (c *Client) NewMultiPartRequest(method, urlStr string, buf *bytes.Buffer) (*http.Request, error) {
 	rel, err := url.Parse(urlStr)
 	if err != nil {
@@ -110,7 +110,7 @@ func (c *Client) NewMultiPartRequest(method, urlStr string, buf *bytes.Buffer) (
 	req.Header.Set("X-Atlassian-Token", "nocheck")
 
 	// Set session cookie if there is one
-	if c.session != nil {
+	if c.Authentication.Authenticated() {
 		req.Header.Set("Cookie", fmt.Sprintf("%s=%s", c.session.Session.Name, c.session.Session.Value))
 	}
 
@@ -141,38 +141,6 @@ func (c *Client) Do(req *http.Request, v interface{}) (*http.Response, error) {
 	return resp, err
 }
 
-// Do sends an API request and returns the API response.
-// The API response is JSON decoded and stored in the value pointed to by v, or returned as an error if an API error has occurred.
-// The caller is expected to consume the Body, and needs to call Body.Close when the response has been handled
-func (c *Client) DoNoClose(req *http.Request, v interface{}) (*http.Response, error) {
-	resp, err := c.client.Do(req)
-	if err != nil {
-		return nil, err
-	}
-
-	err = CheckResponse(resp)
-	if err != nil {
-		// Even though there was an error, we still return the response
-		// in case the caller wants to inspect it further
-		return resp, err
-	}
-
-	if v != nil {
-		err = json.NewDecoder(resp.Body).Decode(v)
-	}
-
-	return resp, err
-}
-
-//// Authenticated reports if the current Client has an authenticated session with JIRA
-//func (c *Client) Authenticated() bool {
-//	if c != nil {
-//		return c.session != nil
-//	} else {
-//		return false
-//	}
-//}
-
 // CheckResponse checks the API response for errors, and returns them if present.
 // A response is considered an error if it has a status code outside the 200 range.
 // API error responses are expected to have either no response body, or a JSON response body that maps to ErrorResponse.

jira_test.go

@@ -257,49 +257,6 @@ func TestDo_HTTPError(t *testing.T) {
 	}
 }
 
-func TestDoNoClose(t *testing.T) {
-	setup()
-	defer teardown()
-
-	type foo struct {
-		A string
-	}
-
-	testMux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
-		if m := "GET"; m != r.Method {
-			t.Errorf("Request method = %v, want %v", r.Method, m)
-		}
-		fmt.Fprint(w, `{"A":"a"}`)
-	})
-
-	req, _ := testClient.NewRequest("GET", "/", nil)
-	body := new(foo)
-	resp, _ := testClient.DoNoClose(req, body)
-	defer resp.Body.Close()
-
-	want := &foo{"a"}
-	if !reflect.DeepEqual(body, want) {
-		t.Errorf("Response body = %v, want %v", body, want)
-	}
-}
-
-func TestDoNoClose_HTTPError(t *testing.T) {
-	setup()
-	defer teardown()
-
-	testMux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
-		http.Error(w, "Bad Request", 400)
-	})
-
-	req, _ := testClient.NewRequest("GET", "/", nil)
-	resp, err := testClient.DoNoClose(req, nil)
-	defer resp.Body.Close()
-
-	if err == nil {
-		t.Error("Expected HTTP 400 error.")
-	}
-}
-
 // Test handling of an error caused by the internal http client's Do() function.
 // A redirect loop is pretty unlikely to occur within the Gerrit API, but does allow us to exercise the right code path.
 func TestDo_RedirectLoop(t *testing.T) {