Merge branch 'attachment' of https://github.com/jasonob/go-jira into jasonob-attachment

* 'attachment' of https://github.com/jasonob/go-jira:
  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

@@ -1,8 +1,6 @@
 package jira
 
-import (
-	"fmt"
-)
+import "fmt"
 
 // AuthenticationService handles authentication for the JIRA instance / API.
 //
@@ -63,6 +61,15 @@ 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
+	} else {
+		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,20 @@ 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"`
+	//	TODO Missing fields
+	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 +55,6 @@ type IssueFields struct {
 	//	* "aggregatetimeoriginalestimate": null,
 	//	* "timeoriginalestimate": null,
 	//	* "timetracking": {},
-	//	* "attachment": [],
 	//	* "aggregatetimeestimate": null,
 	//	* "subtasks": [],
 	//	* "environment": null,
@@ -65,6 +81,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 +203,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 +228,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 +254,62 @@ 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
+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.DoNoClose(req, nil)
+	if err != nil {
+		return resp, err
+	}
+
+	return resp, nil
+}
+
+// PostAttachment uploads an attachment provided as an io.Reader to a given attachment ID
+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", 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.session != nil {
+		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) {
@@ -113,6 +141,38 @@ 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,6 +257,49 @@ 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) {