Merge branch 'jasonob-attachment'

* jasonob-attachment:
  go lint
  go fmt
  Applied a few smaller code changes, cleanups and removed DoNoClose, because a similar function was merged in between
  Increase test coverage slightly and add some robustness
  Add test cases for DoNoClose()
  Add test cases for PostAttachment() and DownloadAttachment(). Fix PostAttachment() to handle response as a JSON array of Attachments. Add Author *Assignee and Thumbnail definition to Attachment structure. Clarify comments on DoNoClose() call
  Ensure Authenticated test validates the path where the AuthenticationService hasn't been initialized
  Moved Authentication() test into AuthenticationService, and add test case to validate it operates correctly
  * Add ability to add and download attachments, including multi-part form handling * Add method to report if the current session is authenticated or not

authentication.go

@@ -63,6 +63,14 @@ func (s *AuthenticationService) AcquireSessionCookie(username, password string)
 	return true, nil
 }
 
+// Authenticated reports if the current Client has an authenticated session with JIRA
+func (s *AuthenticationService) Authenticated() bool {
+	if s != nil {
+		return s.client.session != nil
+	}
+	return false
+}
+
 // TODO Missing API Call GET (Returns information about the currently authenticated user's session)
 // See https://docs.atlassian.com/jira/REST/latest/#auth/1/session
 // TODO Missing API Call DELETE (Logs the current user out of JIRA, destroying the existing session, if any.)

authentication_test.go

@@ -36,6 +36,10 @@ func TestAcquireSessionCookie_Fail(t *testing.T) {
 	if res == true {
 		t.Error("Expected error, but result was true")
 	}
+
+	if testClient.Authentication.Authenticated() != false {
+		t.Error("Expected false, but result was true")
+	}
 }
 
 func TestAcquireSessionCookie_Success(t *testing.T) {
@@ -65,4 +69,18 @@ func TestAcquireSessionCookie_Success(t *testing.T) {
 	if res == false {
 		t.Error("Expected result was true. Got false")
 	}
+
+	if testClient.Authentication.Authenticated() != true {
+		t.Error("Expected true, but result was false")
+	}
+}
+
+func TestAuthenticated_NotInit(t *testing.T) {
+	// Skip setup() because we don't want a fully setup client
+	testClient = new(Client)
+
+	// Test before we've attempted to authenticate
+	if testClient.Authentication.Authenticated() != false {
+		t.Error("Expected false, but result was true")
+	}
 }

issue.go

@@ -1,7 +1,10 @@
 package jira
 
 import (
+	"bytes"
 	"fmt"
+	"io"
+	"mime/multipart"
 	"net/http"
 )
 
@@ -26,6 +29,19 @@ type Issue struct {
 	Fields *IssueFields `json:"fields,omitempty"`
 }
 
+// Attachment represents a JIRA attachment
+type Attachment struct {
+	Self      string    `json:"self,omitempty"`
+	ID        string    `json:"id,omitempty"`
+	Filename  string    `json:"filename,omitempty"`
+	Author    *Assignee `json:"author,omitempty"`
+	Created   string    `json:"created,omitempty"`
+	Size      int       `json:"size,omitempty"`
+	MimeType  string    `json:"mimeType,omitempty"`
+	Content   string    `json:"content,omitempty"`
+	Thumbnail string    `json:"thumbnail,omitempty"`
+}
+
 // IssueFields represents single fields of a JIRA issue.
 // Every JIRA issue has several fields attached.
 type IssueFields struct {
@@ -38,7 +54,6 @@ type IssueFields struct {
 	//	* "aggregatetimeoriginalestimate": null,
 	//	* "timeoriginalestimate": null,
 	//	* "timetracking": {},
-	//	* "attachment": [],
 	//	* "aggregatetimeestimate": null,
 	//	* "subtasks": [],
 	//	* "environment": null,
@@ -65,6 +80,7 @@ type IssueFields struct {
 	Comments          []*Comment    `json:"comment.comments,omitempty"`
 	FixVersions       []*FixVersion `json:"fixVersions,omitempty"`
 	Labels            []string      `json:"labels,omitempty"`
+	Attachments       []*Attachment `json:"attachment,omitempty"`
 }
 
 // IssueType represents a type of a JIRA issue.
@@ -186,14 +202,14 @@ type IssueLinkType struct {
 
 // Comment represents a comment by a person to an issue in JIRA.
 type Comment struct {
-	Self         string            `json:"self"`
-	Name         string            `json:"name"`
-	Author       Assignee          `json:"author"`
-	Body         string            `json:"body"`
-	UpdateAuthor Assignee          `json:"updateAuthor"`
-	Updated      string            `json:"updated"`
-	Created      string            `json:"created"`
-	Visibility   CommentVisibility `json:"visibility"`
+	Self         string            `json:"self,omitempty"`
+	Name         string            `json:"name,omitempty"`
+	Author       Assignee          `json:"author,omitempty"`
+	Body         string            `json:"body,omitempty"`
+	UpdateAuthor Assignee          `json:"updateAuthor,omitempty"`
+	Updated      string            `json:"updated,omitempty"`
+	Created      string            `json:"created,omitempty"`
+	Visibility   CommentVisibility `json:"visibility,omitempty"`
 }
 
 // FixVersion represents a software release in which an issue is fixed.
@@ -211,8 +227,8 @@ type FixVersion struct {
 // CommentVisibility represents he visibility of a comment.
 // E.g. Type could be "role" and Value "Administrators"
 type CommentVisibility struct {
-	Type  string `json:"type"`
-	Value string `json:"value"`
+	Type  string `json:"type,omitempty"`
+	Value string `json:"value,omitempty"`
 }
 
 // Get returns a full representation of the issue for the given issue key.
@@ -237,6 +253,62 @@ func (s *IssueService) Get(issueID string) (*Issue, *http.Response, error) {
 	return issue, resp, nil
 }
 
+// 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)
+	if err != nil {
+		return nil, err
+	}
+
+	resp, err := s.client.Do(req, nil)
+	if err != nil {
+		return resp, err
+	}
+
+	return resp, nil
+}
+
+// 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)
+
+	b := new(bytes.Buffer)
+	writer := multipart.NewWriter(b)
+
+	fw, err := writer.CreateFormFile("file", attachmentName)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	if r != nil {
+		// Copy the file
+		if _, err = io.Copy(fw, r); err != nil {
+			return nil, nil, err
+		}
+	}
+	writer.Close()
+
+	req, err := s.client.NewMultiPartRequest("POST", apiEndpoint, b)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	req.Header.Set("Content-Type", writer.FormDataContentType())
+
+	// PostAttachment response returns a JSON array (as multiple attachments can be posted)
+	attachment := new([]Attachment)
+	resp, err := s.client.Do(req, attachment)
+	if err != nil {
+		return nil, resp, err
+	}
+
+	return attachment, resp, nil
+}
+
 // Create creates an issue or a sub-task from a JSON representation.
 // Creating a sub-task is similar to creating a regular issue, with two important differences:
 // The issueType field must correspond to a sub-task issue type and you must provide a parent field in the issue create request containing the id or key of the parent issue.

issue_test.go

@@ -2,8 +2,10 @@ package jira
 
 import (
 	"fmt"
+	"io/ioutil"
 	"net/http"
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -135,6 +137,174 @@ func TestIssueFields(t *testing.T) {
 	if !reflect.DeepEqual(issue.Fields.Labels, []string{"test"}) {
 		t.Error("Expected labels for the returned issue")
 	}
+
+	if err != nil {
+		t.Errorf("Error given: %s", err)
+	}
+}
+
+func TestIssueDownloadAttachment(t *testing.T) {
+	var testAttachment = "Here is an attachment"
+
+	setup()
+	defer teardown()
+	testMux.HandleFunc("/secure/attachment/", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		testRequestURL(t, r, "/secure/attachment/10000/")
+
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(testAttachment))
+	})
+
+	resp, err := testClient.Issue.DownloadAttachment("10000")
+	if resp == nil {
+		t.Error("Expected response. Response is nil")
+	}
+	defer resp.Body.Close()
+
+	attachment, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		t.Error("Expected attachment text", err)
+	}
+	if string(attachment) != testAttachment {
+		t.Errorf("Expecting an attachment: %s", string(attachment))
+	}
+
+	if resp.StatusCode != 200 {
+		t.Errorf("Expected Status code 200. Given %d", resp.StatusCode)
+	}
+	if err != nil {
+		t.Errorf("Error given: %s", err)
+	}
+}
+
+func TestIssueDownloadAttachment_BadStatus(t *testing.T) {
+
+	setup()
+	defer teardown()
+	testMux.HandleFunc("/secure/attachment/", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		testRequestURL(t, r, "/secure/attachment/10000/")
+
+		w.WriteHeader(http.StatusForbidden)
+	})
+
+	resp, err := testClient.Issue.DownloadAttachment("10000")
+	if resp == nil {
+		t.Error("Expected response. Response is nil")
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusForbidden {
+		t.Errorf("Expected Status code %d. Given %d", http.StatusForbidden, resp.StatusCode)
+	}
+	if err == nil {
+		t.Errorf("Error expected")
+	}
+}
+
+func TestIssuePostAttachment(t *testing.T) {
+	var testAttachment = "Here is an attachment"
+
+	setup()
+	defer teardown()
+	testMux.HandleFunc("/rest/api/2/issue/10000/attachments", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "POST")
+		testRequestURL(t, r, "/rest/api/2/issue/10000/attachments")
+		status := http.StatusOK
+
+		file, _, err := r.FormFile("file")
+		if err != nil {
+			status = http.StatusNotAcceptable
+		}
+		if file == nil {
+			status = http.StatusNoContent
+		} else {
+
+			// Read the file into memory
+			data, err := ioutil.ReadAll(file)
+			if err != nil {
+				status = http.StatusInternalServerError
+			}
+			if string(data) != testAttachment {
+				status = http.StatusNotAcceptable
+			}
+
+			w.WriteHeader(status)
+			fmt.Fprint(w, `[{"self":"http://jira/jira/rest/api/2/attachment/228924","id":"228924","filename":"example.jpg","author":{"self":"http://jira/jira/rest/api/2/user?username=test","name":"test","emailAddress":"test@test.com","avatarUrls":{"16x16":"http://jira/jira/secure/useravatar?size=small&avatarId=10082","48x48":"http://jira/jira/secure/useravatar?avatarId=10082"},"displayName":"Tester","active":true},"created":"2016-05-24T00:25:17.000-0700","size":32280,"mimeType":"image/jpeg","content":"http://jira/jira/secure/attachment/228924/example.jpg","thumbnail":"http://jira/jira/secure/thumbnail/228924/_thumb_228924.png"}]`)
+			file.Close()
+		}
+	})
+
+	reader := strings.NewReader(testAttachment)
+
+	issue, resp, err := testClient.Issue.PostAttachment("10000", reader, "attachment")
+
+	if issue == nil {
+		t.Error("Expected response. Response is nil")
+	}
+
+	if resp.StatusCode != 200 {
+		t.Errorf("Expected Status code 200. Given %d", resp.StatusCode)
+	}
+
+	if err != nil {
+		t.Errorf("Error given: %s", err)
+	}
+}
+
+func TestIssuePostAttachment_NoResponse(t *testing.T) {
+	var testAttachment = "Here is an attachment"
+
+	setup()
+	defer teardown()
+	testMux.HandleFunc("/rest/api/2/issue/10000/attachments", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "POST")
+		testRequestURL(t, r, "/rest/api/2/issue/10000/attachments")
+		w.WriteHeader(http.StatusOK)
+	})
+	reader := strings.NewReader(testAttachment)
+
+	_, _, err := testClient.Issue.PostAttachment("10000", reader, "attachment")
+
+	if err == nil {
+		t.Errorf("Error expected: %s", err)
+	}
+}
+
+func TestIssuePostAttachment_NoFilename(t *testing.T) {
+	var testAttachment = "Here is an attachment"
+
+	setup()
+	defer teardown()
+	testMux.HandleFunc("/rest/api/2/issue/10000/attachments", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "POST")
+		testRequestURL(t, r, "/rest/api/2/issue/10000/attachments")
+		w.WriteHeader(http.StatusOK)
+		fmt.Fprint(w, `[{"self":"http://jira/jira/rest/api/2/attachment/228924","id":"228924","filename":"example.jpg","author":{"self":"http://jira/jira/rest/api/2/user?username=test","name":"test","emailAddress":"test@test.com","avatarUrls":{"16x16":"http://jira/jira/secure/useravatar?size=small&avatarId=10082","48x48":"http://jira/jira/secure/useravatar?avatarId=10082"},"displayName":"Tester","active":true},"created":"2016-05-24T00:25:17.000-0700","size":32280,"mimeType":"image/jpeg","content":"http://jira/jira/secure/attachment/228924/example.jpg","thumbnail":"http://jira/jira/secure/thumbnail/228924/_thumb_228924.png"}]`)
+	})
+	reader := strings.NewReader(testAttachment)
+
+	_, _, err := testClient.Issue.PostAttachment("10000", reader, "")
+
+	if err != nil {
+		t.Errorf("Error expected: %s", err)
+	}
+}
+
+func TestIssuePostAttachment_NoAttachment(t *testing.T) {
+
+	setup()
+	defer teardown()
+	testMux.HandleFunc("/rest/api/2/issue/10000/attachments", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "POST")
+		testRequestURL(t, r, "/rest/api/2/issue/10000/attachments")
+		w.WriteHeader(http.StatusOK)
+		fmt.Fprint(w, `[{"self":"http://jira/jira/rest/api/2/attachment/228924","id":"228924","filename":"example.jpg","author":{"self":"http://jira/jira/rest/api/2/user?username=test","name":"test","emailAddress":"test@test.com","avatarUrls":{"16x16":"http://jira/jira/secure/useravatar?size=small&avatarId=10082","48x48":"http://jira/jira/secure/useravatar?avatarId=10082"},"displayName":"Tester","active":true},"created":"2016-05-24T00:25:17.000-0700","size":32280,"mimeType":"image/jpeg","content":"http://jira/jira/secure/attachment/228924/example.jpg","thumbnail":"http://jira/jira/secure/thumbnail/228924/_thumb_228924.png"}]`)
+	})
+
+	_, _, err := testClient.Issue.PostAttachment("10000", nil, "attachment")
+
 	if err != nil {
 		t.Errorf("Error given: %s", err)
 	}

jira.go

@@ -89,6 +89,34 @@ func (c *Client) NewRequest(method, urlStr string, body interface{}) (*http.Requ
 	return req, nil
 }
 
+// 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.
+func (c *Client) NewMultiPartRequest(method, urlStr string, buf *bytes.Buffer) (*http.Request, error) {
+	rel, err := url.Parse(urlStr)
+	if err != nil {
+		return nil, err
+	}
+
+	u := c.baseURL.ResolveReference(rel)
+
+	req, err := http.NewRequest(method, u.String(), buf)
+	if err != nil {
+		return nil, err
+	}
+
+	// Set required headers
+	req.Header.Set("X-Atlassian-Token", "nocheck")
+
+	// Set session cookie if there is one
+	if c.Authentication.Authenticated() {
+		req.Header.Set("Cookie", fmt.Sprintf("%s=%s", c.session.Session.Name, c.session.Session.Value))
+	}
+
+	return req, nil
+}
+
 // 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.
 func (c *Client) Do(req *http.Request, v interface{}) (*http.Response, error) {