From 13ec668a855628fb7009082b135126a925ff146e Mon Sep 17 00:00:00 2001 From: Prarit Bhargava Date: Wed, 7 Jun 2023 11:15:21 -0400 Subject: [PATCH 1/2] onpremise: Fix Watchers struct and Get User functions Calling the GetWatchers() function results in a stacktrace [1] This occurs because the Watchers api no longer returns an AccountID for each Watcher and now returns a Key and a Username. The Get User functions also must be updated to support Key (by default) and Username. Signed-off-by: Prarit Bhargava [1] panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x6c0d48] goroutine 1 [running]: github.com/andygrunwald/go-jira/v2/onpremise.(*IssueService).GetWatchers(0xc000334030, {0x9c5eb0, 0xc000028120}, {0xc000029c18?, 0xc0002136a0?}) /home/prarit/go/pkg/mod/github.com/andygrunwald/go-jira/v2@v2.0.0-20230325080157-2e11dffbdb9a/onpremise/issue.go:1373 +0x208 gitlab.com/prarit/rhjira/cmd.GetWatches(0xc000242100) /home/prarit/Other/gitlab/rhjira/cmd/lib.go:72 +0x57 gitlab.com/prarit/rhjira/cmd.glob..func1(0xc6e4e0?, {0xc00019a230?, 0x1?, 0x1?}) /home/prarit/Other/gitlab/rhjira/cmd/dump.go:265 +0x5d2 github.com/spf13/cobra.(*Command).execute(0xc6e4e0, {0xc00019a200, 0x1, 0x1}) /home/prarit/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:944 +0x847 github.com/spf13/cobra.(*Command).ExecuteC(0xc6e200) /home/prarit/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:1068 +0x3bd github.com/spf13/cobra.(*Command).Execute(...) /home/prarit/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:992 gitlab.com/prarit/rhjira/cmd.Execute(0xc000334030?) /home/prarit/Other/gitlab/rhjira/cmd/main.go:25 +0x3e main.main() /home/prarit/Other/gitlab/rhjira/main.go:34 +0x15b Signed-off-by: Prarit Bhargava --- onpremise/issue.go | 6 +++--- onpremise/user.go | 24 ++++++++++-------------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/onpremise/issue.go b/onpremise/issue.go index d1bca7aa..d7227e80 100644 --- a/onpremise/issue.go +++ b/onpremise/issue.go @@ -250,7 +250,7 @@ type Watches struct { type Watcher struct { Self string `json:"self,omitempty" structs:"self,omitempty"` Name string `json:"name,omitempty" structs:"name,omitempty"` - AccountID string `json:"accountId,omitempty" structs:"accountId,omitempty"` + Key string `json:"key,omitempty" structs:"key,omitempty"` DisplayName string `json:"displayName,omitempty" structs:"displayName,omitempty"` Active bool `json:"active,omitempty" structs:"active,omitempty"` } @@ -1364,8 +1364,8 @@ func (s *IssueService) GetWatchers(ctx context.Context, issueID string) (*[]User result := []User{} for _, watcher := range watches.Watchers { var user *User - if watcher.AccountID != "" { - user, resp, err = s.client.User.GetByAccountID(context.Background(), watcher.AccountID) + if watcher.Name != "" { + user, resp, err = s.client.User.Get(context.Background(), watcher.Key) if err != nil { return nil, resp, NewJiraError(resp, err) } diff --git a/onpremise/user.go b/onpremise/user.go index bcbe30aa..23a24dc6 100644 --- a/onpremise/user.go +++ b/onpremise/user.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "html" "net/http" ) @@ -44,14 +45,12 @@ type userSearch []userSearchParam type userSearchF func(userSearch) userSearch -// Get gets user info from Jira using its Account Id +// Get gets user info from Jira using its key // -// Jira API docs: https://developer.atlassian.com/cloud/jira/platform/rest/v2/#api-rest-api-2-user-get +// Jira API doc: https://docs.atlassian.com/software/jira/docs/api/REST/9.9.0/#api/2/user-getUser // -// TODO Double check this method if this works as expected, is using the latest API and the response is complete -// This double check effort is done for v2 - Remove this two lines if this is completed. -func (s *UserService) Get(ctx context.Context, accountId string) (*User, *Response, error) { - apiEndpoint := fmt.Sprintf("/rest/api/2/user?accountId=%s", accountId) +func (s *UserService) Get(ctx context.Context, key string) (*User, *Response, error) { + apiEndpoint := fmt.Sprintf("/rest/api/2/user?key=%s", html.EscapeString(key)) req, err := s.client.NewRequest(ctx, http.MethodGet, apiEndpoint, nil) if err != nil { return nil, nil, err @@ -65,15 +64,12 @@ func (s *UserService) Get(ctx context.Context, accountId string) (*User, *Respon return user, resp, nil } -// GetByAccountID gets user info from Jira -// Searching by another parameter that is not accountId is deprecated, -// but this method is kept for backwards compatibility -// Jira API docs: https://docs.atlassian.com/jira/REST/cloud/#api/2/user-getUser +// GetByUsername gets user info from Jira // -// TODO Double check this method if this works as expected, is using the latest API and the response is complete -// This double check effort is done for v2 - Remove this two lines if this is completed. -func (s *UserService) GetByAccountID(ctx context.Context, accountID string) (*User, *Response, error) { - apiEndpoint := fmt.Sprintf("/rest/api/2/user?accountId=%s", accountID) +// Jira API doc: https://docs.atlassian.com/software/jira/docs/api/REST/9.9.0/#api/2/user-getUser +// +func (s *UserService) GetByUsername(ctx context.Context, username string) (*User, *Response, error) { + apiEndpoint := fmt.Sprintf("/rest/api/2/user?username=%s", html.EscapeString(username)) req, err := s.client.NewRequest(ctx, http.MethodGet, apiEndpoint, nil) if err != nil { return nil, nil, err From 0af6510a1fa848f5415027654083687c9a34a0d7 Mon Sep 17 00:00:00 2001 From: Prarit Bhargava Date: Wed, 7 Jun 2023 20:51:09 -0400 Subject: [PATCH 2/2] onpremise: fix tests for new functions Fix the issue and user tests to use the new user functions. Signed-off-by: Prarit Bhargava --- onpremise/issue_test.go | 20 ++++++++++---------- onpremise/user_test.go | 12 ++++++------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/onpremise/issue_test.go b/onpremise/issue_test.go index 36b6aa31..4e19e4cf 100644 --- a/onpremise/issue_test.go +++ b/onpremise/issue_test.go @@ -1590,14 +1590,14 @@ func TestIssueService_GetWatchers(t *testing.T) { testMethod(t, r, http.MethodGet) testRequestURL(t, r, "/rest/api/2/issue/10002/watchers") - fmt.Fprint(w, `{"self":"http://www.example.com/jira/rest/api/2/issue/EX-1/watchers","isWatching":false,"watchCount":1,"watchers":[{"self":"http://www.example.com/jira/rest/api/2/user?accountId=000000000000000000000000","accountId": "000000000000000000000000","displayName":"Fred F. User","active":false}]}`) + fmt.Fprint(w, `{"self":"http://www.example.com/jira/rest/api/2/issue/EX-1/watchers","isWatching":false,"watchCount":1,"watchers":[{"self":"http://www.example.com/jira/rest/api/2/user?username=fred","name": "fred","displayName":"Fred F. User","active":false}]}`) }) testMux.HandleFunc("/rest/api/2/user", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodGet) - testRequestURL(t, r, "/rest/api/2/user?accountId=000000000000000000000000") + testRequestURL(t, r, "/rest/api/2/user?key=") - fmt.Fprint(w, `{"self":"http://www.example.com/jira/rest/api/2/user?accountId=000000000000000000000000","key":"fred","accountId": "000000000000000000000000", + fmt.Fprint(w, `{"self":"http://www.example.com/jira/rest/api/2/user?username=fred","key":"fred", "name": "fred", "emailAddress":"fred@example.com","avatarUrls":{"48x48":"http://www.example.com/jira/secure/useravatar?size=large&ownerId=fred", "24x24":"http://www.example.com/jira/secure/useravatar?size=small&ownerId=fred","16x16":"http://www.example.com/jira/secure/useravatar?size=xsmall&ownerId=fred", "32x32":"http://www.example.com/jira/secure/useravatar?size=medium&ownerId=fred"},"displayName":"Fred F. User","active":true,"timeZone":"Australia/Sydney","groups":{"size":3,"items":[ @@ -1619,8 +1619,8 @@ func TestIssueService_GetWatchers(t *testing.T) { t.Errorf("Expected 1 watcher, got: %d", len(*watchers)) return } - if (*watchers)[0].AccountID != "000000000000000000000000" { - t.Error("Expected watcher accountId 000000000000000000000000") + if (*watchers)[0].Name != "fred" { + t.Error("Expected watcher accountId fred") } } @@ -1631,14 +1631,14 @@ func TestIssueService_DeprecatedGetWatchers(t *testing.T) { testMethod(t, r, http.MethodGet) testRequestURL(t, r, "/rest/api/2/issue/10002/watchers") - fmt.Fprint(w, `{"self":"http://www.example.com/jira/rest/api/2/issue/EX-1/watchers","isWatching":false,"watchCount":1,"watchers":[{"self":"http://www.example.com/jira/rest/api/2/user?accountId=000000000000000000000000", "accountId": "000000000000000000000000", "displayName":"Fred F. User","active":false}]}`) + fmt.Fprint(w, `{"self":"http://www.example.com/jira/rest/api/2/issue/EX-1/watchers","isWatching":false,"watchCount":1,"watchers":[{"self":"http://www.example.com/jira/rest/api/2/user?username=fred","name": "fred","displayName":"Fred F. User","active":false}]}`) }) testMux.HandleFunc("/rest/api/2/user", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodGet) - testRequestURL(t, r, "/rest/api/2/user?accountId=000000000000000000000000") + testRequestURL(t, r, "/rest/api/2/user?key=") - fmt.Fprint(w, `{"self":"http://www.example.com/jira/rest/api/2/user?accountId=000000000000000000000000", "accountId": "000000000000000000000000", "key": "", "name": "", "emailAddress":"fred@example.com","avatarUrls":{"48x48":"http://www.example.com/jira/secure/useravatar?size=large&ownerId=fred", + fmt.Fprint(w, `{"self":"http://www.example.com/jira/rest/api/2/user?username=fred", "name": "fred", "key": "fred", "emailAddress":"fred@example.com","avatarUrls":{"48x48":"http://www.example.com/jira/secure/useravatar?size=large&ownerId=fred", "24x24":"http://www.example.com/jira/secure/useravatar?size=small&ownerId=fred","16x16":"http://www.example.com/jira/secure/useravatar?size=xsmall&ownerId=fred", "32x32":"http://www.example.com/jira/secure/useravatar?size=medium&ownerId=fred"},"displayName":"Fred F. User","active":true,"timeZone":"Australia/Sydney","groups":{"size":3,"items":[ {"name":"jira-user","self":"http://www.example.com/jira/rest/api/2/group?groupname=jira-user"},{"name":"jira-admin", @@ -1659,8 +1659,8 @@ func TestIssueService_DeprecatedGetWatchers(t *testing.T) { t.Errorf("Expected 1 watcher, got: %d", len(*watchers)) return } - if (*watchers)[0].AccountID != "000000000000000000000000" { - t.Error("Expected accountId 000000000000000000000000") + if (*watchers)[0].Name != "fred" { + t.Error("Expected accountId fred") } } diff --git a/onpremise/user_test.go b/onpremise/user_test.go index 273bc3a0..a223853d 100644 --- a/onpremise/user_test.go +++ b/onpremise/user_test.go @@ -12,7 +12,7 @@ func TestUserService_Get_Success(t *testing.T) { defer teardown() testMux.HandleFunc("/rest/api/2/user", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodGet) - testRequestURL(t, r, "/rest/api/2/user?accountId=000000000000000000000000") + testRequestURL(t, r, "/rest/api/2/user?key=") fmt.Fprint(w, `{"self":"http://www.example.com/jira/rest/api/2/user?username=fred","key":"fred", "name":"fred","emailAddress":"fred@example.com","avatarUrls":{"48x48":"http://www.example.com/jira/secure/useravatar?size=large&ownerId=fred", @@ -23,21 +23,21 @@ func TestUserService_Get_Success(t *testing.T) { }]},"applicationRoles":{"size":1,"items":[]},"expand":"groups,applicationRoles"}`) }) - if user, _, err := testClient.User.Get(context.Background(), "000000000000000000000000"); err != nil { + if user, _, err := testClient.User.Get(context.Background(), "fred"); err != nil { t.Errorf("Error given: %s", err) } else if user == nil { t.Error("Expected user. User is nil") } } -func TestUserService_GetByAccountID_Success(t *testing.T) { +func TestUserService_GetByUsername_Success(t *testing.T) { setup() defer teardown() testMux.HandleFunc("/rest/api/2/user", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodGet) - testRequestURL(t, r, "/rest/api/2/user?accountId=000000000000000000000000") + testRequestURL(t, r, "/rest/api/2/user?username=fred") - fmt.Fprint(w, `{"self":"http://www.example.com/jira/rest/api/2/user?accountId=000000000000000000000000","accountId": "000000000000000000000000", + fmt.Fprint(w, `{"self":"http://www.example.com/jira/rest/api/2/user?username=fred","key":"fred", "name":"fred","emailAddress":"fred@example.com","avatarUrls":{"48x48":"http://www.example.com/jira/secure/useravatar?size=large&ownerId=fred", "24x24":"http://www.example.com/jira/secure/useravatar?size=small&ownerId=fred","16x16":"http://www.example.com/jira/secure/useravatar?size=xsmall&ownerId=fred", "32x32":"http://www.example.com/jira/secure/useravatar?size=medium&ownerId=fred"},"displayName":"Fred F. User","active":true,"timeZone":"Australia/Sydney","groups":{"size":3,"items":[ @@ -46,7 +46,7 @@ func TestUserService_GetByAccountID_Success(t *testing.T) { }]},"applicationRoles":{"size":1,"items":[]},"expand":"groups,applicationRoles"}`) }) - if user, _, err := testClient.User.GetByAccountID(context.Background(), "000000000000000000000000"); err != nil { + if user, _, err := testClient.User.GetByUsername(context.Background(), "fred"); err != nil { t.Errorf("Error given: %s", err) } else if user == nil { t.Error("Expected user. User is nil")