Refactoring of bitbucket provider with betters.

Also doesnt use decode/encode anymore since those methods are more
intended for streams. So this goes to the more standed marshal/unmarshal
of data.

This also adds better error support and will try to give the user
better errors from the api if it can. Or any issues with the bitbucket
service.

builtin/providers/bitbucket/client.go

@@ -2,64 +2,107 @@ package bitbucket
 
 import (
 	"bytes"
+	"encoding/json"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"log"
 	"net/http"
 )
 
+// Error represents a error from the bitbucket api.
+type Error struct {
+	APIError struct {
+		Message string `json:"message,omitempty"`
+	} `json:"error,omitempty"`
+	Type       string `json:"type,omitempty"`
+	StatusCode int
+	Endpoint   string
+}
+
+func (e Error) Error() string {
+	return fmt.Sprintf("API Error: %d %s %s", e.StatusCode, e.Endpoint, e.APIError.Message)
+}
+
+const (
+	// BitbucketEndpoint is the fqdn used to talk to bitbucket
+	BitbucketEndpoint string = "https://api.bitbucket.org/"
+)
+
 type BitbucketClient struct {
-	Username string
+	Username   string
-	Password string
+	Password   string
+	HTTPClient *http.Client
+}
+
+func (c *BitbucketClient) Do(method, endpoint string, payload *bytes.Buffer) (*http.Response, error) {
+
+	absoluteendpoint := BitbucketEndpoint + endpoint
+	log.Printf("[DEBUG] Sending request to %s %s", method, absoluteendpoint)
+
+	var bodyreader io.Reader
+
+	if payload != nil {
+		log.Printf("[DEBUG] With payload %s", payload.String())
+		bodyreader = payload
+	}
+
+	req, err := http.NewRequest(method, absoluteendpoint, bodyreader)
+	if err != nil {
+		return nil, err
+	}
+
+	req.SetBasicAuth(c.Username, c.Password)
+
+	if payload != nil {
+		// Can cause bad request when putting default reviews if set.
+		req.Header.Add("Content-Type", "application/json")
+	}
+
+	req.Close = true
+
+	resp, err := c.HTTPClient.Do(req)
+	log.Printf("[DEBUG] Resp: %v Err: %v", resp, err)
+	if resp.StatusCode >= 400 || resp.StatusCode < 200 {
+		apiError := Error{
+			StatusCode: resp.StatusCode,
+			Endpoint:   endpoint,
+		}
+
+		body, err := ioutil.ReadAll(resp.Body)
+		if err != nil {
+			return nil, err
+		}
+
+		log.Printf("[DEBUG] Resp Body: %s", string(body))
+
+		err = json.Unmarshal(body, &apiError)
+		if err != nil {
+			apiError.APIError.Message = string(body)
+		}
+
+		return resp, error(apiError)
+
+	}
+	return resp, err
 }
 
 func (c *BitbucketClient) Get(endpoint string) (*http.Response, error) {
-	client := &http.Client{}
+	return c.Do("GET", endpoint, nil)
-	req, err := http.NewRequest("GET", "https://api.bitbucket.org/"+endpoint, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	req.SetBasicAuth(c.Username, c.Password)
-	return client.Do(req)
-
 }
 
 func (c *BitbucketClient) Post(endpoint string, jsonpayload *bytes.Buffer) (*http.Response, error) {
-	client := &http.Client{}
+	return c.Do("POST", endpoint, jsonpayload)
-	req, err := http.NewRequest("POST", "https://api.bitbucket.org/"+endpoint, jsonpayload)
-	if err != nil {
-		return nil, err
-	}
-	req.SetBasicAuth(c.Username, c.Password)
-	req.Header.Add("content-type", "application/json")
-	return client.Do(req)
 }
 
 func (c *BitbucketClient) Put(endpoint string, jsonpayload *bytes.Buffer) (*http.Response, error) {
-	client := &http.Client{}
+	return c.Do("PUT", endpoint, jsonpayload)
-	req, err := http.NewRequest("PUT", "https://api.bitbucket.org/"+endpoint, jsonpayload)
-	if err != nil {
-		return nil, err
-	}
-	req.SetBasicAuth(c.Username, c.Password)
-	req.Header.Add("content-type", "application/json")
-	return client.Do(req)
 }
 
 func (c *BitbucketClient) PutOnly(endpoint string) (*http.Response, error) {
-	client := &http.Client{}
+	return c.Do("PUT", endpoint, nil)
-	req, err := http.NewRequest("PUT", "https://api.bitbucket.org/"+endpoint, nil)
-	if err != nil {
-		return nil, err
-	}
-	req.SetBasicAuth(c.Username, c.Password)
-	return client.Do(req)
 }
 
 func (c *BitbucketClient) Delete(endpoint string) (*http.Response, error) {
-	client := &http.Client{}
+	return c.Do("DELETE", endpoint, nil)
-	req, err := http.NewRequest("DELETE", "https://api.bitbucket.org/"+endpoint, nil)
-	if err != nil {
-		return nil, err
-	}
-	req.SetBasicAuth(c.Username, c.Password)
-	return client.Do(req)
 }

builtin/providers/bitbucket/provider.go

@@ -1,6 +1,8 @@
 package bitbucket
 
 import (
+	"net/http"
+
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 )
@@ -30,8 +32,9 @@ func Provider() terraform.ResourceProvider {
 
 func providerConfigure(d *schema.ResourceData) (interface{}, error) {
 	client := &BitbucketClient{
-		Username: d.Get("username").(string),
+		Username:   d.Get("username").(string),
-		Password: d.Get("password").(string),
+		Password:   d.Get("password").(string),
+		HTTPClient: &http.Client{},
 	}
 
 	return client, nil

builtin/providers/bitbucket/resource_default_reviewers.go

@@ -3,6 +3,7 @@ package bitbucket
 import (
 	"encoding/json"
 	"fmt"
+
 	"github.com/hashicorp/terraform/helper/schema"
 )
 
@@ -49,7 +50,7 @@ func resourceDefaultReviewersCreate(d *schema.ResourceData, m interface{}) error
 	client := m.(*BitbucketClient)
 
 	for _, user := range d.Get("reviewers").(*schema.Set).List() {
-		reviewer_resp, err := client.PutOnly(fmt.Sprintf("2.0/repositories/%s/%s/default-reviewers/%s",
+		reviewerResp, err := client.PutOnly(fmt.Sprintf("2.0/repositories/%s/%s/default-reviewers/%s",
 			d.Get("owner").(string),
 			d.Get("repository").(string),
 			user,
@@ -59,11 +60,11 @@ func resourceDefaultReviewersCreate(d *schema.ResourceData, m interface{}) error
 			return err
 		}
 
-		if reviewer_resp.StatusCode != 200 {
+		if reviewerResp.StatusCode != 200 {
-			return fmt.Errorf("Failed to create reviewer %s got code %d", user.(string), reviewer_resp.StatusCode)
+			return fmt.Errorf("Failed to create reviewer %s got code %d", user.(string), reviewerResp.StatusCode)
 		}
 
-		defer reviewer_resp.Body.Close()
+		defer reviewerResp.Body.Close()
 	}
 
 	d.SetId(fmt.Sprintf("%s/%s/reviewers", d.Get("owner").(string), d.Get("repository").(string)))
@@ -72,26 +73,26 @@ func resourceDefaultReviewersCreate(d *schema.ResourceData, m interface{}) error
 func resourceDefaultReviewersRead(d *schema.ResourceData, m interface{}) error {
 	client := m.(*BitbucketClient)
 
-	reviewers_response, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/default-reviewers",
+	reviewersResponse, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/default-reviewers",
 		d.Get("owner").(string),
 		d.Get("repository").(string),
 	))
 
 	var reviewers PaginatedReviewers
 
-	decoder := json.NewDecoder(reviewers_response.Body)
+	decoder := json.NewDecoder(reviewersResponse.Body)
 	err = decoder.Decode(&reviewers)
 	if err != nil {
 		return err
 	}
 
-	terraform_reviewers := make([]string, 0, len(reviewers.Values))
+	terraformReviewers := make([]string, 0, len(reviewers.Values))
 
 	for _, reviewer := range reviewers.Values {
-		terraform_reviewers = append(terraform_reviewers, reviewer.Username)
+		terraformReviewers = append(terraformReviewers, reviewer.Username)
 	}
 
-	d.Set("reviewers", terraform_reviewers)
+	d.Set("reviewers", terraformReviewers)
 
 	return nil
 }

builtin/providers/bitbucket/resource_hook.go

@@ -4,6 +4,10 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io/ioutil"
+	"log"
+	"net/url"
+
 	"github.com/hashicorp/terraform/helper/schema"
 )
 
@@ -81,86 +85,89 @@ func resourceHookCreate(d *schema.ResourceData, m interface{}) error {
 	client := m.(*BitbucketClient)
 	hook := createHook(d)
 
-	var jsonbuffer []byte
+	payload, err := json.Marshal(hook)
-
-	jsonpayload := bytes.NewBuffer(jsonbuffer)
-	enc := json.NewEncoder(jsonpayload)
-	enc.Encode(hook)
-
-	hook_req, err := client.Post(fmt.Sprintf("2.0/repositories/%s/%s/hooks",
-		d.Get("owner").(string),
-		d.Get("repository").(string),
-	), jsonpayload)
-
-	decoder := json.NewDecoder(hook_req.Body)
-	err = decoder.Decode(&hook)
 	if err != nil {
 		return err
 	}
 
+	hook_req, err := client.Post(fmt.Sprintf("2.0/repositories/%s/%s/hooks",
+		d.Get("owner").(string),
+		d.Get("repository").(string),
+	), bytes.NewBuffer(payload))
+
+	if err != nil {
+		return err
+	}
+
+	body, readerr := ioutil.ReadAll(hook_req.Body)
+	if readerr != nil {
+		return readerr
+	}
+
+	decodeerr := json.Unmarshal(body, &hook)
+	if decodeerr != nil {
+		return decodeerr
+	}
+
 	d.SetId(hook.Uuid)
-	d.Set("uuid", hook.Uuid)
 
 	return resourceHookRead(d, m)
 }
 func resourceHookRead(d *schema.ResourceData, m interface{}) error {
 	client := m.(*BitbucketClient)
-	hook_req, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s",
+
+	hook_req, _ := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s",
 		d.Get("owner").(string),
 		d.Get("repository").(string),
-		d.Get("uuid").(string),
+		url.PathEscape(d.Id()),
 	))
 
-	if err != nil {
+	log.Printf("ID: %s", url.PathEscape(d.Id()))
-		return err
+
+	if hook_req.StatusCode == 200 {
+		var hook Hook
+
+		body, readerr := ioutil.ReadAll(hook_req.Body)
+		if readerr != nil {
+			return readerr
+		}
+
+		decodeerr := json.Unmarshal(body, &hook)
+		if decodeerr != nil {
+			return decodeerr
+		}
+
+		d.Set("uuid", hook.Uuid)
+		d.Set("description", hook.Description)
+		d.Set("active", hook.Active)
+		d.Set("url", hook.Url)
+
+		eventsList := make([]string, 0, len(hook.Events))
+
+		for _, event := range hook.Events {
+			eventsList = append(eventsList, event)
+		}
+
+		d.Set("events", eventsList)
 	}
 
-	var hook Hook
-
-	decoder := json.NewDecoder(hook_req.Body)
-	err = decoder.Decode(&hook)
-	if err != nil {
-		return err
-	}
-
-	d.Set("uuid", hook.Uuid)
-	d.Set("description", hook.Description)
-	d.Set("active", hook.Active)
-	d.Set("url", hook.Url)
-
-	eventsList := make([]string, 0, len(hook.Events))
-
-	for _, event := range hook.Events {
-		eventsList = append(eventsList, event)
-	}
-
-	d.Set("events", eventsList)
-
 	return nil
 }
 
 func resourceHookUpdate(d *schema.ResourceData, m interface{}) error {
 	client := m.(*BitbucketClient)
 	hook := createHook(d)
-
+	payload, err := json.Marshal(hook)
-	var jsonbuffer []byte
-
-	jsonpayload := bytes.NewBuffer(jsonbuffer)
-	enc := json.NewEncoder(jsonpayload)
-	enc.Encode(hook)
-
-	hook_req, err := client.Put(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s",
-		d.Get("owner").(string),
-		d.Get("repository").(string),
-		d.Get("uuid").(string),
-	), jsonpayload)
-
 	if err != nil {
 		return err
 	}
 
-	decoder := json.NewDecoder(hook_req.Body)
+	_, err = client.Put(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s",
-	err = decoder.Decode(&hook)
+		d.Get("owner").(string),
+		d.Get("repository").(string),
+		url.PathEscape(d.Id()),
+	), bytes.NewBuffer(payload))
+
 	if err != nil {
 		return err
 	}
@@ -174,7 +181,7 @@ func resourceHookExists(d *schema.ResourceData, m interface{}) (bool, error) {
 		hook_req, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s",
 			d.Get("owner").(string),
 			d.Get("repository").(string),
-			d.Get("uuid").(string),
+			url.PathEscape(d.Id()),
 		))
 
 		if err != nil {
@@ -182,15 +189,14 @@ func resourceHookExists(d *schema.ResourceData, m interface{}) (bool, error) {
 		}
 
 		if hook_req.StatusCode != 200 {
-			d.SetId("")
+			return false, err
-			return false, nil
 		}
 
 		return true, nil
-	} else {
-		return false, nil
 	}
 
+	return false, nil
+
 }
 
 func resourceHookDelete(d *schema.ResourceData, m interface{}) error {
@@ -198,11 +204,9 @@ func resourceHookDelete(d *schema.ResourceData, m interface{}) error {
 	_, err := client.Delete(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s",
 		d.Get("owner").(string),
 		d.Get("repository").(string),
-		d.Get("uuid").(string),
+		url.PathEscape(d.Id()),
 	))
 
-	if err != nil {
+	return err
-		return err
+
-	}
-	return nil
 }

builtin/providers/bitbucket/resource_hook_test.go

@@ -2,6 +2,7 @@ package bitbucket
 
 import (
 	"fmt"
+	"net/url"
 	"os"
 	"testing"
 
@@ -16,7 +17,7 @@ func TestAccBitbucketHook_basic(t *testing.T) {
 	testAccBitbucketHookConfig := fmt.Sprintf(`
 		resource "bitbucket_repository" "test_repo" {
 			owner = "%s"
-			name = "test-repo"
+			name = "test-repo-for-webhook-test"
 		}
 		resource "bitbucket_hook" "test_repo_hook" {
 			owner = "%s"
@@ -51,10 +52,10 @@ func testAccCheckBitbucketHookDestroy(s *terraform.State) error {
 		return fmt.Errorf("Not found %s", "bitbucket_hook.test_repo_hook")
 	}
 
-	response, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s", rs.Primary.Attributes["owner"], rs.Primary.Attributes["repository"], rs.Primary.Attributes["uuid"]))
+	response, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s", rs.Primary.Attributes["owner"], rs.Primary.Attributes["repository"], url.PathEscape(rs.Primary.Attributes["uuid"])))
 
-	if err != nil {
+	if err == nil {
-		return err
+		return fmt.Errorf("The resource was found should have errored")
 	}
 
 	if response.StatusCode != 404 {

builtin/providers/bitbucket/resource_repository.go

@@ -4,8 +4,9 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io/ioutil"
+
 	"github.com/hashicorp/terraform/helper/schema"
-	"log"
 )
 
 type CloneUrl struct {
@@ -131,7 +132,7 @@ func resourceRepositoryUpdate(d *schema.ResourceData, m interface{}) error {
 	enc := json.NewEncoder(jsonpayload)
 	enc.Encode(repository)
 
-	repository_response, err := client.Put(fmt.Sprintf("2.0/repositories/%s/%s",
+	_, err := client.Put(fmt.Sprintf("2.0/repositories/%s/%s",
 		d.Get("owner").(string),
 		d.Get("name").(string),
 	), jsonpayload)
@@ -140,16 +141,6 @@ func resourceRepositoryUpdate(d *schema.ResourceData, m interface{}) error {
 		return err
 	}
 
-	if repository_response.StatusCode == 200 {
-		decoder := json.NewDecoder(repository_response.Body)
-		err = decoder.Decode(&repository)
-		if err != nil {
-			return err
-		}
-	} else {
-		return fmt.Errorf("Failed to put: %d", repository_response.StatusCode)
-	}
-
 	return resourceRepositoryRead(d, m)
 }
 
@@ -157,29 +148,19 @@ func resourceRepositoryCreate(d *schema.ResourceData, m interface{}) error {
 	client := m.(*BitbucketClient)
 	repo := newRepositoryFromResource(d)
 
-	var jsonbuffer []byte
+	bytedata, err := json.Marshal(repo)
 
-	jsonpayload := bytes.NewBuffer(jsonbuffer)
-	enc := json.NewEncoder(jsonpayload)
-	enc.Encode(repo)
-
-	log.Printf("Sending %s \n", jsonpayload)
-
-	repo_req, err := client.Post(fmt.Sprintf("2.0/repositories/%s/%s",
-		d.Get("owner").(string),
-		d.Get("name").(string),
-	), jsonpayload)
-
-	decoder := json.NewDecoder(repo_req.Body)
-	err = decoder.Decode(&repo)
 	if err != nil {
 		return err
 	}
 
-	log.Printf("Received %s \n", repo_req.Body)
+	_, err = client.Post(fmt.Sprintf("2.0/repositories/%s/%s",
+		d.Get("owner").(string),
+		d.Get("name").(string),
+	), bytes.NewBuffer(bytedata))
 
-	if repo_req.StatusCode != 200 {
+	if err != nil {
-		return fmt.Errorf("Failed to create repository got status code %d", repo_req.StatusCode)
+		return err
 	}
 
 	d.SetId(string(fmt.Sprintf("%s/%s", d.Get("owner").(string), d.Get("name").(string))))
@@ -189,39 +170,42 @@ func resourceRepositoryCreate(d *schema.ResourceData, m interface{}) error {
 func resourceRepositoryRead(d *schema.ResourceData, m interface{}) error {
 
 	client := m.(*BitbucketClient)
-	repo_req, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s",
+	repo_req, _ := client.Get(fmt.Sprintf("2.0/repositories/%s/%s",
 		d.Get("owner").(string),
 		d.Get("name").(string),
 	))
 
-	if err != nil {
+	if repo_req.StatusCode == 200 {
-		return err
-	}
 
-	var repo Repository
+		var repo Repository
 
-	decoder := json.NewDecoder(repo_req.Body)
+		body, readerr := ioutil.ReadAll(repo_req.Body)
-	err = decoder.Decode(&repo)
+		if readerr != nil {
-	if err != nil {
+			return readerr
-		return err
+		}
-	}
 
-	d.Set("scm", repo.SCM)
+		decodeerr := json.Unmarshal(body, &repo)
-	d.Set("is_private", repo.IsPrivate)
+		if decodeerr != nil {
-	d.Set("has_wiki", repo.HasWiki)
+			return decodeerr
-	d.Set("has_issues", repo.HasIssues)
+		}
-	d.Set("name", repo.Name)
-	d.Set("language", repo.Language)
-	d.Set("fork_policy", repo.ForkPolicy)
-	d.Set("website", repo.Website)
-	d.Set("description", repo.Description)
-	d.Set("project_key", repo.Project.Key)
 
-	for _, clone_url := range repo.Links.Clone {
+		d.Set("scm", repo.SCM)
-		if clone_url.Name == "https" {
+		d.Set("is_private", repo.IsPrivate)
-			d.Set("clone_https", clone_url.Href)
+		d.Set("has_wiki", repo.HasWiki)
-		} else {
+		d.Set("has_issues", repo.HasIssues)
-			d.Set("clone_ssh", clone_url.Href)
+		d.Set("name", repo.Name)
+		d.Set("language", repo.Language)
+		d.Set("fork_policy", repo.ForkPolicy)
+		d.Set("website", repo.Website)
+		d.Set("description", repo.Description)
+		d.Set("project_key", repo.Project.Key)
+
+		for _, clone_url := range repo.Links.Clone {
+			if clone_url.Name == "https" {
+				d.Set("clone_https", clone_url.Href)
+			} else {
+				d.Set("clone_ssh", clone_url.Href)
+			}
 		}
 	}
 
@@ -230,18 +214,10 @@ func resourceRepositoryRead(d *schema.ResourceData, m interface{}) error {
 
 func resourceRepositoryDelete(d *schema.ResourceData, m interface{}) error {
 	client := m.(*BitbucketClient)
-	delete_response, err := client.Delete(fmt.Sprintf("2.0/repositories/%s/%s",
+	_, err := client.Delete(fmt.Sprintf("2.0/repositories/%s/%s",
 		d.Get("owner").(string),
 		d.Get("name").(string),
 	))
 
-	if err != nil {
+	return err
-		return err
-	}
-
-	if delete_response.StatusCode != 204 {
-		return fmt.Errorf("Failed to delete the repository got status code %d", delete_response.StatusCode)
-	}
-
-	return nil
 }

builtin/providers/bitbucket/resource_repository_test.go

@@ -16,9 +16,9 @@ func TestAccBitbucketRepository_basic(t *testing.T) {
 	testAccBitbucketRepositoryConfig := fmt.Sprintf(`
 		resource "bitbucket_repository" "test_repo" {
 			owner = "%s"
-			name = "%s"
+			name = "test-repo-for-repository-test"
 		}
-	`, testUser, testRepo)
+	`, testUser)
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -42,11 +42,7 @@ func testAccCheckBitbucketRepositoryDestroy(s *terraform.State) error {
 		return fmt.Errorf("Not found %s", "bitbucket_repository.test_repo")
 	}
 
-	response, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s", rs.Primary.Attributes["owner"], rs.Primary.Attributes["name"]))
+	response, _ := client.Get(fmt.Sprintf("2.0/repositories/%s/%s", rs.Primary.Attributes["owner"], rs.Primary.Attributes["name"]))
-
-	if err != nil {
-		return err
-	}
 
 	if response.StatusCode != 404 {
 		return fmt.Errorf("Repository still exists")