mirror of
https://github.com/interviewstreet/go-jira.git
synced 2025-04-25 12:14:56 +02:00
fix: paging with load balancer going to endless loop
When used with a load balanced Jira, the SearchPages method would end up in an endless loop. This was caused by a bug where Jira would not handle the MaxResults=50 that is sent by defaul properly, thus retur- ning no issues. The SearchPages method didn't check for empty results and ended up in an endless loop. Fixed this by 1. Pre-escaping '&maxResults' to '&MaxResults'. 2. Adding a check in SearchPages to see if the issues array is empty before going into the endless 'for'. Also fixed the appropriate tests. Fixes issue #260.
This commit is contained in:
Dick Appel
Wes McNamee
parent
436469b62d
commit
19d3fc0aec
6
issue.go
6
issue.go
@ -965,7 +965,7 @@ func (s *IssueService) Search(jql string, options *SearchOptions) ([]Issue, *Res
|
||||
u += fmt.Sprintf("&startAt=%d", options.StartAt)
|
||||
}
|
||||
if options.MaxResults != 0 {
|
||||
u += fmt.Sprintf("&maxResults=%d", options.MaxResults)
|
||||
u += fmt.Sprintf("&maxResults=%d", options.MaxResults)
|
||||
}
|
||||
if options.Expand != "" {
|
||||
u += fmt.Sprintf("&expand=%s", options.Expand)
|
||||
@ -1011,6 +1011,10 @@ func (s *IssueService) SearchPages(jql string, options *SearchOptions, f func(Is
|
||||
return err
|
||||
}
|
||||
|
||||
if len(issues) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
for {
|
||||
for _, issue := range issues {
|
||||
err = f(issue)
|
||||
|
||||
@ -3,7 +3,6 @@ package jira
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"github.com/google/go-cmp/cmp"
|
||||
"io"
|
||||
"io/ioutil"
|
||||
"net/http"
|
||||
@ -11,6 +10,8 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/google/go-cmp/cmp"
|
||||
|
||||
"time"
|
||||
|
||||
"github.com/trivago/tgo/tcontainer"
|
||||
@ -596,7 +597,7 @@ func TestIssueService_Search(t *testing.T) {
|
||||
defer teardown()
|
||||
testMux.HandleFunc("/rest/api/2/search", func(w http.ResponseWriter, r *http.Request) {
|
||||
testMethod(t, r, "GET")
|
||||
testRequestURL(t, r, "/rest/api/2/search?jql=something&startAt=1&maxResults=40&expand=foo")
|
||||
testRequestURL(t, r, "/rest/api/2/search?jql=something&startAt=1&maxResults=40&expand=foo")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
fmt.Fprint(w, `{"expand": "schema,names","startAt": 1,"maxResults": 40,"total": 6,"issues": [{"expand": "html","id": "10230","self": "http://kelpie9:8081/rest/api/2/issue/BULK-62","key": "BULK-62","fields": {"summary": "testing","timetracking": null,"issuetype": {"self": "http://kelpie9:8081/rest/api/2/issuetype/5","id": "5","description": "The sub-task of the issue","iconUrl": "http://kelpie9:8081/images/icons/issue_subtask.gif","name": "Sub-task","subtask": true},"customfield_10071": null}},{"expand": "html","id": "10004","self": "http://kelpie9:8081/rest/api/2/issue/BULK-47","key": "BULK-47","fields": {"summary": "Cheese v1 2.0 issue","timetracking": null,"issuetype": {"self": "http://kelpie9:8081/rest/api/2/issuetype/3","id": "3","description": "A task that needs to be done.","iconUrl": "http://kelpie9:8081/images/icons/task.gif","name": "Task","subtask": false}}}]}`)
|
||||
})
|
||||
@ -657,15 +658,15 @@ func TestIssueService_SearchPages(t *testing.T) {
|
||||
defer teardown()
|
||||
testMux.HandleFunc("/rest/api/2/search", func(w http.ResponseWriter, r *http.Request) {
|
||||
testMethod(t, r, "GET")
|
||||
if r.URL.String() == "/rest/api/2/search?jql=something&startAt=1&maxResults=2&expand=foo&validateQuery=warn" {
|
||||
if r.URL.String() == "/rest/api/2/search?jql=something&startAt=1&maxResults=2&expand=foo&validateQuery=warn" {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
fmt.Fprint(w, `{"expand": "schema,names","startAt": 1,"maxResults": 2,"total": 6,"issues": [{"expand": "html","id": "10230","self": "http://kelpie9:8081/rest/api/2/issue/BULK-62","key": "BULK-62","fields": {"summary": "testing","timetracking": null,"issuetype": {"self": "http://kelpie9:8081/rest/api/2/issuetype/5","id": "5","description": "The sub-task of the issue","iconUrl": "http://kelpie9:8081/images/icons/issue_subtask.gif","name": "Sub-task","subtask": true},"customfield_10071": null}},{"expand": "html","id": "10004","self": "http://kelpie9:8081/rest/api/2/issue/BULK-47","key": "BULK-47","fields": {"summary": "Cheese v1 2.0 issue","timetracking": null,"issuetype": {"self": "http://kelpie9:8081/rest/api/2/issuetype/3","id": "3","description": "A task that needs to be done.","iconUrl": "http://kelpie9:8081/images/icons/task.gif","name": "Task","subtask": false}}}]}`)
|
||||
return
|
||||
} else if r.URL.String() == "/rest/api/2/search?jql=something&startAt=3&maxResults=2&expand=foo&validateQuery=warn" {
|
||||
} else if r.URL.String() == "/rest/api/2/search?jql=something&startAt=3&maxResults=2&expand=foo&validateQuery=warn" {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
fmt.Fprint(w, `{"expand": "schema,names","startAt": 3,"maxResults": 2,"total": 6,"issues": [{"expand": "html","id": "10230","self": "http://kelpie9:8081/rest/api/2/issue/BULK-62","key": "BULK-62","fields": {"summary": "testing","timetracking": null,"issuetype": {"self": "http://kelpie9:8081/rest/api/2/issuetype/5","id": "5","description": "The sub-task of the issue","iconUrl": "http://kelpie9:8081/images/icons/issue_subtask.gif","name": "Sub-task","subtask": true},"customfield_10071": null}},{"expand": "html","id": "10004","self": "http://kelpie9:8081/rest/api/2/issue/BULK-47","key": "BULK-47","fields": {"summary": "Cheese v1 2.0 issue","timetracking": null,"issuetype": {"self": "http://kelpie9:8081/rest/api/2/issuetype/3","id": "3","description": "A task that needs to be done.","iconUrl": "http://kelpie9:8081/images/icons/task.gif","name": "Task","subtask": false}}}]}`)
|
||||
return
|
||||
} else if r.URL.String() == "/rest/api/2/search?jql=something&startAt=5&maxResults=2&expand=foo&validateQuery=warn" {
|
||||
} else if r.URL.String() == "/rest/api/2/search?jql=something&startAt=5&maxResults=2&expand=foo&validateQuery=warn" {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
fmt.Fprint(w, `{"expand": "schema,names","startAt": 5,"maxResults": 2,"total": 6,"issues": [{"expand": "html","id": "10230","self": "http://kelpie9:8081/rest/api/2/issue/BULK-62","key": "BULK-62","fields": {"summary": "testing","timetracking": null,"issuetype": {"self": "http://kelpie9:8081/rest/api/2/issuetype/5","id": "5","description": "The sub-task of the issue","iconUrl": "http://kelpie9:8081/images/icons/issue_subtask.gif","name": "Sub-task","subtask": true},"customfield_10071": null}}]}`)
|
||||
return
|
||||
@ -690,6 +691,34 @@ func TestIssueService_SearchPages(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestIssueService_SearchPages_EmptyResult(t *testing.T) {
|
||||
setup()
|
||||
defer teardown()
|
||||
testMux.HandleFunc("/rest/api/2/search", func(w http.ResponseWriter, r *http.Request) {
|
||||
testMethod(t, r, "GET")
|
||||
if r.URL.String() == "/rest/api/2/search?jql=something&startAt=1&maxResults=50&expand=foo&validateQuery=warn" {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
// This is what Jira outputs when the &maxResult= issue occurs. It used to cause SearchPages to go into an endless loop.
|
||||
fmt.Fprint(w, `{"expand": "schema,names","startAt": 0,"maxResults": 0,"total": 6,"issues": []}`)
|
||||
return
|
||||
}
|
||||
|
||||
t.Errorf("Unexpected URL: %v", r.URL)
|
||||
})
|
||||
|
||||
opt := &SearchOptions{StartAt: 1, MaxResults: 50, Expand: "foo", ValidateQuery: "warn"}
|
||||
issues := make([]Issue, 0)
|
||||
err := testClient.Issue.SearchPages("something", opt, func(issue Issue) error {
|
||||
issues = append(issues, issue)
|
||||
return nil
|
||||
})
|
||||
|
||||
if err != nil {
|
||||
t.Errorf("Error given: %s", err)
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
func TestIssueService_GetCustomFields(t *testing.T) {
|
||||
setup()
|
||||
defer teardown()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user