Skip to content

Commit

Permalink
Merge pull request kubernetes#26564 from AlexNPavel/jira-client-impro…
Browse files Browse the repository at this point in the history 
…vements

jira: general client improvements
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jun 23, 2022
2 parents 4fb2c04 + 0587179 commit 3ef0bc3
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 88 deletions.
153 changes: 77 additions & 76 deletions prow/jira/fakejira/fake.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package fakejira
import (
"context"
"encoding/json"
"errors"
"fmt"
"strconv"
"strings"
Expand All @@ -31,14 +30,17 @@ import (
)

type FakeClient struct {
Issues []*jira.Issue
ExistingLinks map[string][]jira.RemoteLink
NewLinks []jira.RemoteLink
IssueLinks []*jira.IssueLink
GetIssueError error
Transitions []jira.Transition
Users []*jira.User
SearchResponses map[SearchRequest]SearchResponse
Issues []*jira.Issue
ExistingLinks map[string][]jira.RemoteLink
NewLinks []jira.RemoteLink
RemovedLinks []jira.RemoteLink
IssueLinks []*jira.IssueLink
GetIssueError map[string]error
CreateIssueError map[string]error
UpdateIssueError map[string]error
Transitions []jira.Transition
Users []*jira.User
SearchResponses map[SearchRequest]SearchResponse
}

func (f *FakeClient) ListProjects() (*jira.ProjectList, error) {
Expand All @@ -47,7 +49,9 @@ func (f *FakeClient) ListProjects() (*jira.ProjectList, error) {

func (f *FakeClient) GetIssue(id string) (*jira.Issue, error) {
if f.GetIssueError != nil {
return nil, f.GetIssueError
if err, ok := f.GetIssueError[id]; ok {
return nil, err
}
}
for _, existingIssue := range f.Issues {
if existingIssue.ID == id || existingIssue.Key == id {
Expand All @@ -58,7 +62,11 @@ func (f *FakeClient) GetIssue(id string) (*jira.Issue, error) {
}

func (f *FakeClient) GetRemoteLinks(id string) ([]jira.RemoteLink, error) {
return f.ExistingLinks[id], nil
issue, err := f.GetIssue(id)
if err != nil {
return nil, fmt.Errorf("Failed to get issue when chekcing from remote links: %+v", err)
}
return append(f.ExistingLinks[issue.ID], f.ExistingLinks[issue.Key]...), nil
}

func (f *FakeClient) AddRemoteLink(id string, link *jira.RemoteLink) (*jira.RemoteLink, error) {
Expand Down Expand Up @@ -107,6 +115,13 @@ func (f *FakeClient) AddComment(issueID string, comment *jira.Comment) (*jira.Co
if err != nil {
return nil, fmt.Errorf("Issue %s not found: %v", issueID, err)
}
// make sure the fields exist
if issue.Fields == nil {
issue.Fields = &jira.IssueFields{}
}
if issue.Fields.Comments == nil {
issue.Fields.Comments = &jira.Comments{}
}
issue.Fields.Comments.Comments = append(issue.Fields.Comments.Comments, comment)
return comment, nil
}
Expand All @@ -116,44 +131,56 @@ func (f *FakeClient) CreateIssueLink(link *jira.IssueLink) error {
if err != nil {
return fmt.Errorf("failed to get outward link issue: %v", err)
}
outward.Fields.IssueLinks = append(outward.Fields.IssueLinks, link)
// when part of an issue struct, the issue link type does not include the
// short definition of the issue it is in
linkForOutward := *link
linkForOutward.OutwardIssue = nil
outward.Fields.IssueLinks = append(outward.Fields.IssueLinks, &linkForOutward)
inward, err := f.GetIssue(link.InwardIssue.ID)
if err != nil {
return fmt.Errorf("failed to get inward link issue: %v", err)
}
inward.Fields.IssueLinks = append(inward.Fields.IssueLinks, link)
linkForInward := *link
linkForInward.InwardIssue = nil
inward.Fields.IssueLinks = append(inward.Fields.IssueLinks, &linkForInward)
f.IssueLinks = append(f.IssueLinks, link)
return nil
}

func (f *FakeClient) CloneIssue(issue *jira.Issue) (*jira.Issue, error) {
// create deep copy of parent so we can modify key and id for child
data, err := json.Marshal(issue)
if err != nil {
return nil, err
}
issueCopy := &jira.Issue{}
err = json.Unmarshal(data, issueCopy)
if err != nil {
return nil, err
}
// set ID and Key to unused id and key
f.updateIssueIDAndKey(issueCopy)
// run generic cloning function
return jiraclient.CloneIssue(f, issueCopy)
return jiraclient.CloneIssue(f, issue)
}

func (f *FakeClient) CreateIssue(issue *jira.Issue) (*jira.Issue, error) {
// check that there are no ID collisions
for _, currIssue := range f.Issues {
if currIssue.ID == issue.ID {
return nil, fmt.Errorf("Issue ID %s already exists", issue.ID)
if f.CreateIssueError != nil {
if err, ok := f.CreateIssueError[issue.Key]; ok {
return nil, err
}
if currIssue.Key == issue.Key {
return nil, fmt.Errorf("Issue key %s already exists", issue.Key)
}
if issue.Fields == nil {
issue.Fields = &jira.IssueFields{}
}
// find highest issueID and make new issue one higher
highestID := 0
// find highest ID for issues in the same project to make new key one higher
highestKeyID := 0
keyPrefix := issue.Fields.Project.Name + "-"
for _, issue := range f.Issues {
// all IDs are ints, but represented as strings...
intID, _ := strconv.Atoi(issue.ID)
if intID > highestID {
highestID = intID
}
if strings.HasPrefix(issue.Key, keyPrefix) {
stringID := strings.TrimPrefix(issue.Key, keyPrefix)
intID, _ := strconv.Atoi(stringID)
if intID > highestKeyID {
highestKeyID = intID
}
}
}
f.updateIssueIDAndKey(issue)
issue.ID = strconv.Itoa(highestID + 1)
issue.Key = fmt.Sprintf("%s%d", keyPrefix, highestKeyID+1)
f.Issues = append(f.Issues, issue)
return issue, nil
}
Expand Down Expand Up @@ -211,53 +238,22 @@ func (f *FakeClient) DeleteLink(id string) error {
func (f *FakeClient) DeleteRemoteLink(issueID string, linkID int) error {
for index, remoteLink := range f.ExistingLinks[issueID] {
if remoteLink.ID == linkID {
f.ExistingLinks[issueID] = append(f.ExistingLinks[issueID][:index], f.ExistingLinks[issueID][index+1:]...)
f.RemovedLinks = append(f.RemovedLinks, remoteLink)
if len(f.ExistingLinks[issueID]) == index+1 {
f.ExistingLinks[issueID] = f.ExistingLinks[issueID][:index]
} else {
f.ExistingLinks[issueID] = append(f.ExistingLinks[issueID][:index], f.ExistingLinks[issueID][index+1:]...)
}
return nil
}
}
return fmt.Errorf("failed to find link id %d in issue %s", linkID, issueID)
}

func (f *FakeClient) DeleteRemoteLinkViaURL(issueID, url string) error {
func (f *FakeClient) DeleteRemoteLinkViaURL(issueID, url string) (bool, error) {
return jiraclient.DeleteRemoteLinkViaURL(f, issueID, url)
}

func (f *FakeClient) updateIssueIDAndKey(newIssue *jira.Issue) error {
// ensure that a key is set
if newIssue.Key == "" {
return errors.New("Issue key must be set")
}
// ensure key format is correct
splitKey := strings.Split(newIssue.Key, "-")
if len(splitKey) != 2 {
return fmt.Errorf("Invalid issue key: %s", newIssue.Key)
}

// find highest issueID and make new issue one higher
highestID := -1
for _, issue := range f.Issues {
// all IDs are ints, but represented as strings...
intID, _ := strconv.Atoi(issue.ID)
if intID > highestID {
highestID = intID
}
}
newIssue.ID = strconv.Itoa(highestID + 1)
// if there are issues in the same project, make new issue one above those
highestKeyID := 0
keyPrefix := fmt.Sprintf("%s-", splitKey[0])
for _, issue := range f.Issues {
if strings.HasPrefix(issue.Key, keyPrefix) {
stringID := strings.TrimPrefix(issue.Key, keyPrefix)
intID, _ := strconv.Atoi(stringID)
if intID > highestKeyID {
highestKeyID = intID
}
}
}
newIssue.Key = fmt.Sprintf("%s%d", keyPrefix, highestKeyID)
return nil
}

func (f *FakeClient) GetTransitions(issueID string) ([]jira.Transition, error) {
return f.Transitions, nil
}
Expand Down Expand Up @@ -311,6 +307,11 @@ func (f *FakeClient) GetIssueTargetVersion(issue *jira.Issue) (*[]*jira.Version,
}

func (f *FakeClient) UpdateIssue(issue *jira.Issue) (*jira.Issue, error) {
if f.UpdateIssueError != nil {
if err, ok := f.UpdateIssueError[issue.ID]; ok {
return nil, err
}
}
retrievedIssue, err := f.GetIssue(issue.ID)
if err != nil {
return nil, fmt.Errorf("unable to find issue to update: %v", err)
Expand Down Expand Up @@ -339,11 +340,11 @@ func (f *FakeClient) UpdateIssue(issue *jira.Issue) (*jira.Issue, error) {
if err != nil {
return nil, fmt.Errorf("error converting updated issue to json: %v", err)
}
var newFields *jira.IssueFields
if err := json.Unmarshal(updatedIssueBytes, newFields); err != nil {
var newFields jira.IssueFields
if err := json.Unmarshal(updatedIssueBytes, &newFields); err != nil {
return nil, fmt.Errorf("failed converting updated issue to struct: %v", err)
}
retrievedIssue.Fields = newFields
retrievedIssue.Fields = &newFields
return retrievedIssue, nil
}

Expand Down
24 changes: 14 additions & 10 deletions prow/jira/jira.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,10 @@ type Client interface {
DeleteLink(id string) error
DeleteRemoteLink(issueID string, linkID int) error
// DeleteRemoteLinkViaURL identifies and removes a remote link from an issue
// the has the provided URL.
DeleteRemoteLinkViaURL(issueID, url string) error
// the has the provided URL. The returned bool indicates whether a change
// was made during the operation as a remote link with the URL not existing
// is not consider an error for this function.
DeleteRemoteLinkViaURL(issueID, url string) (bool, error)
ForPlugin(plugin string) Client
AddComment(issueID string, comment *jira.Comment) (*jira.Comment, error)
ListProjects() (*jira.ProjectList, error)
Expand Down Expand Up @@ -328,21 +330,23 @@ func (jc *client) DeleteRemoteLink(issueID string, linkID int) error {
}

// DeleteRemoteLinkViaURL identifies and removes a remote link from an issue
// the has the provided URL.
func DeleteRemoteLinkViaURL(jc Client, issueID, url string) error {
// the has the provided URL. The returned bool indicates whether a change
// was made during the operation as a remote link with the URL not existing
// is not consider an error for this function.
func DeleteRemoteLinkViaURL(jc Client, issueID, url string) (bool, error) {
links, err := jc.GetRemoteLinks(issueID)
if err != nil {
return err
return false, err
}
for _, link := range links {
if link.Object.URL == url {
return jc.DeleteRemoteLink(issueID, link.ID)
return true, jc.DeleteRemoteLink(issueID, link.ID)
}
}
return fmt.Errorf("could not find remote link on issue with URL `%s`", url)
return false, fmt.Errorf("could not find remote link on issue with URL `%s`", url)
}

func (jc *client) DeleteRemoteLinkViaURL(issueID, url string) error {
func (jc *client) DeleteRemoteLinkViaURL(issueID, url string) (bool, error) {
return DeleteRemoteLinkViaURL(jc, issueID, url)
}

Expand Down Expand Up @@ -514,7 +518,7 @@ func (jc *client) CloneIssue(parent *jira.Issue) (*jira.Issue, error) {

func unsetProblematicFields(issue *jira.Issue, responseBody string) (*jira.Issue, error) {
// handle unsettable "unknown" fields
processedResponse := createIssueError{}
processedResponse := CreateIssueError{}
if newErr := json.Unmarshal([]byte(responseBody), &processedResponse); newErr != nil {
return nil, fmt.Errorf("Error processing jira error: %w", newErr)
}
Expand Down Expand Up @@ -544,7 +548,7 @@ func unsetProblematicFields(issue *jira.Issue, responseBody string) (*jira.Issue
return &newIssue, nil
}

type createIssueError struct {
type CreateIssueError struct {
ErrorMessages []string `json:"errorMessages"`
Errors map[string]string `json:"errors"`
}
Expand Down
4 changes: 2 additions & 2 deletions prow/plugins/jira/jira_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func TestHandle(t *testing.T) {
event github.GenericCommentEvent
cfg *plugins.Jira
projectCache *threadsafeSet
getIssueClientError error
getIssueClientError map[string]error
existingIssues []jira.Issue
existingLinks map[string][]jira.RemoteLink
expectedNewLinks []jira.RemoteLink
Expand Down Expand Up @@ -328,7 +328,7 @@ func TestHandle(t *testing.T) {
Number: 3,
},
projectCache: &threadsafeSet{},
getIssueClientError: errors.New("error: didn't serve 404 from cache"),
getIssueClientError: map[string]error{"ABC-123": errors.New("error: didn't serve 404 from cache")},
},
}

Expand Down

0 comments on commit 3ef0bc3

Please sign in to comment.