From f70c279c1e8dc7f73e08679f8789c35c1dfdd59d Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sat, 22 Jun 2019 17:10:23 +0200 Subject: [bridge/github] exporter: Improve error handling [bridge/github] queries: use api v4 for getLabel / createLabel [bridge/github] add comments to getIdentityClient --- bridge/github/export.go | 139 ++++++++++++++++++++---------------------- bridge/github/export_query.go | 17 ++++++ bridge/github/import_query.go | 8 +++ 3 files changed, 92 insertions(+), 72 deletions(-) (limited to 'bridge/github') 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)"` +} -- cgit