[bridge/github] exporter: Improve error handling

[bridge/github] queries: use api v4 for getLabel / createLabel

[bridge/github] add comments to getIdentityClient

3 files changed, 92 insertions, 72 deletions

diff --git a/bridge/github/export.go b/bridge/github/export.go
index 38df9caf..a41efc99 100644
--- a/bridge/github/export.go
+++ b/bridge/github/export.go
@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"time"
 
+	"github.com/pkg/errors"
 	"github.com/shurcooL/githubv4"
 
 	"github.com/MichaelMure/git-bug/bridge/core"
@@ -18,6 +19,10 @@ import (
 	"github.com/MichaelMure/git-bug/util/git"
 )
 
+var (
+	ErrMissingIdentityToken = errors.New("missing identity token")
+)
+
 // githubImporter implement the Importer interface
 type githubExporter struct {
 	conf core.Configuration
@@ -70,6 +75,8 @@ func (ge *githubExporter) allowOrigin(origin string) bool {
 	return false
 }
 
+// getIdentityClient return an identity github api v4 client
+// if no client were found it will initilize it from the known tokens map and cache it for next it use
 func (ge *githubExporter) getIdentityClient(id string) (*githubv4.Client, error) {
 	client, ok := ge.identityClient[id]
 	if ok {
@@ -79,7 +86,7 @@ func (ge *githubExporter) getIdentityClient(id string) (*githubv4.Client, error)
 	// get token
 	token, ok := ge.identityToken[id]
 	if !ok {
-		return nil, fmt.Errorf("unknown identity token")
+		return nil, ErrMissingIdentityToken
 	}
 
 	// create client
@@ -182,28 +189,32 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error {
 		// check that we have a token for operation author
 		client, err := ge.getIdentityClient(author.Id())
 		if err != nil {
-			// if bug is still not exported and user cannot author bug stop the execution
+			// if bug is still not exported and we do not have the author stop the execution
 
-			// TODO: maybe print a warning ?
-			// this is not an error
-			// don't export bug
+			fmt.Println("warning: skipping issue due to missing token for bug creator")
+			// this is not an error, don't export bug
 			return nil
 		}
 
 		// create bug
 		id, url, err := createGithubIssue(client, ge.repositoryID, createOp.Title, createOp.Message)
 		if err != nil {
-			return fmt.Errorf("exporting github issue: %v", err)
+			return errors.Wrap(err, "exporting github issue")
 		}
 
 		hash, err := createOp.Hash()
 		if err != nil {
-			return fmt.Errorf("comment hash: %v", err)
+			return errors.Wrap(err, "comment hash")
 		}
 
 		// mark bug creation operation as exported
 		if err := markOperationAsExported(b, hash, id, url); err != nil {
-			return fmt.Errorf("marking operation as exported: %v", err)
+			return errors.Wrap(err, "marking operation as exported")
+		}
+
+		// commit operation to avoid creating multiple issues with multiple pushes
+		if err := b.CommitAsNeeded(); err != nil {
+			return errors.Wrap(err, "bug commit")
 		}
 
 		// cache bug github ID and URL
@@ -231,7 +242,7 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error {
 		// get operation hash
 		hash, err := op.Hash()
 		if err != nil {
-			return fmt.Errorf("reading operation hash: %v", err)
+			return errors.Wrap(err, "operation hash")
 		}
 
 		// ignore imported (or exported) operations from github
@@ -256,12 +267,12 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error {
 			// send operation to github
 			id, url, err = addCommentGithubIssue(client, bugGithubID, opr.Message)
 			if err != nil {
-				return fmt.Errorf("adding comment: %v", err)
+				return errors.Wrap(err, "adding comment")
 			}
 
 			hash, err = opr.Hash()
 			if err != nil {
-				return fmt.Errorf("comment hash: %v", err)
+				return errors.Wrap(err, "comment hash")
 			}
 
 		case *bug.EditCommentOperation:
@@ -274,7 +285,7 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error {
 
 				// case bug creation operation: we need to edit the Github issue
 				if err := updateGithubIssueBody(client, bugGithubID, opr.Message); err != nil {
-					return fmt.Errorf("editing issue: %v", err)
+					return errors.Wrap(err, "editing issue")
 				}
 
 				id = bugGithubID
@@ -290,7 +301,7 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error {
 
 				eid, eurl, err := editCommentGithubIssue(client, commentID, opr.Message)
 				if err != nil {
-					return fmt.Errorf("editing comment: %v", err)
+					return errors.Wrap(err, "editing comment")
 				}
 
 				// use comment id/url instead of issue id/url
@@ -300,18 +311,18 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error {
 
 			hash, err = opr.Hash()
 			if err != nil {
-				return fmt.Errorf("comment hash: %v", err)
+				return errors.Wrap(err, "comment hash")
 			}
 
 		case *bug.SetStatusOperation:
 			opr := op.(*bug.SetStatusOperation)
 			if err := updateGithubIssueStatus(client, bugGithubID, opr.Status); err != nil {
-				return fmt.Errorf("updating status %v: %v", bugGithubID, err)
+				return errors.Wrap(err, "editing status")
 			}
 
 			hash, err = opr.Hash()
 			if err != nil {
-				return fmt.Errorf("comment hash: %v", err)
+				return errors.Wrap(err, "set status operation hash")
 			}
 
 			id = bugGithubID
@@ -320,12 +331,12 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error {
 		case *bug.SetTitleOperation:
 			opr := op.(*bug.SetTitleOperation)
 			if err := updateGithubIssueTitle(client, bugGithubID, opr.Title); err != nil {
-				return fmt.Errorf("editing comment %v: %v", bugGithubID, err)
+				return errors.Wrap(err, "editing title")
 			}
 
 			hash, err = opr.Hash()
 			if err != nil {
-				return fmt.Errorf("comment hash: %v", err)
+				return errors.Wrap(err, "set title operation hash")
 			}
 
 			id = bugGithubID
@@ -334,12 +345,12 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error {
 		case *bug.LabelChangeOperation:
 			opr := op.(*bug.LabelChangeOperation)
 			if err := ge.updateGithubIssueLabels(client, bugGithubID, opr.Added, opr.Removed); err != nil {
-				return fmt.Errorf("updating labels %v: %v", bugGithubID, err)
+				return errors.Wrap(err, "updating labels")
 			}
 
 			hash, err = opr.Hash()
 			if err != nil {
-				return fmt.Errorf("comment hash: %v", err)
+				return errors.Wrap(err, "label change operation hash")
 			}
 
 			id = bugGithubID
@@ -351,12 +362,12 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error {
 
 		// mark operation as exported
 		if err := markOperationAsExported(b, hash, id, url); err != nil {
-			return fmt.Errorf("marking operation as exported: %v", err)
+			return errors.Wrap(err, "marking operation as exported")
 		}
-	}
 
-	if err := b.CommitAsNeeded(); err != nil {
-		return fmt.Errorf("bug commit: %v", err)
+		if err := b.CommitAsNeeded(); err != nil {
+			return errors.Wrap(err, "bug commit")
+		}
 	}
 
 	return nil
@@ -415,49 +426,17 @@ func markOperationAsExported(b *cache.BugCache, target git.Hash, githubID, githu
 }
 
 // get label from github
-func (ge *githubExporter) getGithubLabelID(label string) (string, error) {
-	url := fmt.Sprintf("%s/repos/%s/%s/labels/%s", githubV3Url, ge.conf[keyOwner], ge.conf[keyProject], label)
-
-	client := &http.Client{
-		Timeout: defaultTimeout,
-	}
-
-	req, err := http.NewRequest("GET", url, nil)
-	if err != nil {
-		return "", err
-	}
-
-	// need the token for private repositories
-	req.Header.Set("Authorization", fmt.Sprintf("token %s", ge.conf[keyToken]))
-
-	resp, err := client.Do(req)
-	if err != nil {
-		return "", err
-	}
-
-	if resp.StatusCode != http.StatusFound {
-		return "", fmt.Errorf("error getting label: status code: %v", resp.StatusCode)
-	}
-
-	aux := struct {
-		ID     string `json:"id"`
-		NodeID string `json:"node_id"`
-		Color  string `json:"color"`
-	}{}
-
-	data, _ := ioutil.ReadAll(resp.Body)
-	defer resp.Body.Close()
-
-	err = json.Unmarshal(data, &aux)
-	if err != nil {
+func (ge *githubExporter) getGithubLabelID(gc *githubv4.Client, label string) (string, error) {
+	q := labelQuery{}
+	variables := map[string]interface{}{"name": label}
+	if err := gc.Query(context.TODO(), &q, variables); err != nil {
 		return "", err
 	}
 
-	return aux.NodeID, nil
+	return q.Repository.Label.ID, nil
 }
 
-// create github label using api v3
-func (ge *githubExporter) createGithubLabel(label, labelColor string) (string, error) {
+func (ge *githubExporter) createGithubLabel(gc *githubv4.Client, label, labelColor string) (string, error) {
 	url := fmt.Sprintf("%s/repos/%s/%s/labels", githubV3Url, ge.conf[keyOwner], ge.conf[keyProject])
 
 	client := &http.Client{
@@ -498,6 +477,22 @@ func (ge *githubExporter) createGithubLabel(label, labelColor string) (string, e
 	return aux.NodeID, nil
 }
 
+// create github label using api v4
+func (ge *githubExporter) createGithubLabelV4(gc *githubv4.Client, label, labelColor string) (string, error) {
+	m := &createLabelMutation{}
+	input := createLabelInput{
+		RepositoryID: ge.repositoryID,
+		Name:         githubv4.String(label),
+		Color:        githubv4.String(labelColor),
+	}
+
+	if err := gc.Mutate(context.TODO(), m, input, nil); err != nil {
+		return "", err
+	}
+
+	return m.CreateLabel.Label.ID, nil
+}
+
 // randomHexColor return a random hex color code
 func randomHexColor() string {
 	bytes := make([]byte, 6)
@@ -508,9 +503,9 @@ func randomHexColor() string {
 	return hex.EncodeToString(bytes)
 }
 
-func (ge *githubExporter) getOrCreateGithubLabelID(repositoryID, label string) (string, error) {
+func (ge *githubExporter) getOrCreateGithubLabelID(gc *githubv4.Client, repositoryID, label string) (string, error) {
 	// try to get label id
-	labelID, err := ge.getGithubLabelID(label)
+	labelID, err := ge.getGithubLabelID(gc, label)
 	if err == nil {
 		return labelID, nil
 	}
@@ -520,7 +515,7 @@ func (ge *githubExporter) getOrCreateGithubLabelID(repositoryID, label string) (
 	color := randomHexColor()
 
 	// create label and return id
-	labelID, err = ge.createGithubLabel(label, color)
+	labelID, err = ge.createGithubLabel(gc, label, color)
 	if err != nil {
 		return "", err
 	}
@@ -528,7 +523,7 @@ func (ge *githubExporter) getOrCreateGithubLabelID(repositoryID, label string) (
 	return labelID, nil
 }
 
-func (ge *githubExporter) getLabelsIDs(repositoryID string, labels []bug.Label) ([]githubv4.ID, error) {
+func (ge *githubExporter) getLabelsIDs(gc *githubv4.Client, repositoryID string, labels []bug.Label) ([]githubv4.ID, error) {
 	ids := make([]githubv4.ID, 0, len(labels))
 	var err error
 
@@ -539,9 +534,9 @@ func (ge *githubExporter) getLabelsIDs(repositoryID string, labels []bug.Label)
 		id, ok := ge.cachedLabels[label]
 		if !ok {
 			// try to query label id
-			id, err = ge.getOrCreateGithubLabelID(repositoryID, label)
+			id, err = ge.getOrCreateGithubLabelID(gc, repositoryID, label)
 			if err != nil {
-				return nil, fmt.Errorf("get or create github label: %v", err)
+				return nil, errors.Wrap(err, "get or create github label")
 			}
 
 			// cache label id
@@ -653,9 +648,9 @@ func updateGithubIssueTitle(gc *githubv4.Client, id, title string) error {
 
 // update github issue labels
 func (ge *githubExporter) updateGithubIssueLabels(gc *githubv4.Client, labelableID string, added, removed []bug.Label) error {
-	addedIDs, err := ge.getLabelsIDs(labelableID, added)
+	addedIDs, err := ge.getLabelsIDs(gc, labelableID, added)
 	if err != nil {
-		return fmt.Errorf("getting added labels ids: %v", err)
+		return errors.Wrap(err, "getting added labels ids")
 	}
 
 	m := &addLabelsToLabelableMutation{}
@@ -669,9 +664,9 @@ func (ge *githubExporter) updateGithubIssueLabels(gc *githubv4.Client, labelable
 		return err
 	}
 
-	removedIDs, err := ge.getLabelsIDs(labelableID, added)
+	removedIDs, err := ge.getLabelsIDs(gc, labelableID, added)
 	if err != nil {
-		return fmt.Errorf("getting added labels ids: %v", err)
+		return errors.Wrap(err, "getting added labels ids")
 	}
 
 	m2 := &removeLabelsFromLabelableMutation{}
diff --git a/bridge/github/export_query.go b/bridge/github/export_query.go
index a527399e..006d2511 100644
--- a/bridge/github/export_query.go
+++ b/bridge/github/export_query.go
@@ -1,5 +1,7 @@
 package github
 
+import "github.com/shurcooL/githubv4"
+
 type createIssueMutation struct {
 	CreateIssue struct {
 		Issue struct {
@@ -36,6 +38,14 @@ type updateIssueCommentMutation struct {
 	} `graphql:"updateIssueComment(input:$input)"`
 }
 
+type createLabelMutation struct {
+	CreateLabel struct {
+		Label struct {
+			ID string `graphql:"id"`
+		} `graphql:"label"`
+	} `graphql:"createLabel(input:{repositoryId: $repositoryId, name: $name, color: $color})"`
+}
+
 type removeLabelsFromLabelableMutation struct {
 	AddLabels struct{} `graphql:"removeLabelsFromLabelable(input:$input)"`
 }
@@ -43,3 +53,10 @@ type removeLabelsFromLabelableMutation struct {
 type addLabelsToLabelableMutation struct {
 	RemoveLabels struct{} `graphql:"addLabelsToLabelable(input:$input)"`
 }
+
+type createLabelInput struct {
+	Color        githubv4.String  `json:"color"`
+	Description  *githubv4.String `json:"description"`
+	Name         githubv4.String  `json:"name"`
+	RepositoryID githubv4.ID      `json:"repositoryId"`
+}
diff --git a/bridge/github/import_query.go b/bridge/github/import_query.go
index 4d5886f6..6c0db5e1 100644
--- a/bridge/github/import_query.go
+++ b/bridge/github/import_query.go
@@ -168,3 +168,11 @@ type userQuery struct {
 		Email     githubv4.String
 	} `graphql:"user(login: $login)"`
 }
+
+type labelQuery struct {
+	Repository struct {
+		Label struct {
+			ID string `graphql:"id"`
+		} `graphql:"label(name: $name)"`
+	} `graphql:"repository(owner: $owner, name: $name)"`
+}