klesh opened a new pull request, #2053:
URL: https://github.com/apache/incubator-devlake/pull/2053
# Summary
This PR aims to fix the problems described in #2052
First, all complicated logic has been removed or simplified, the problem we
had was we need to call `GetAsync` inside `GetAsync` callback function which
itself would not release before the nested call return. The solution is simple,
wrap the nested call within newly created function `NextTick` and we are safe:
```go
collector.fetchAsync(&reqDataCopy, func(count int, body
[]byte, res *http.Response) error {
if count < collector.args.PageSize {
return nil
}
apiClient.NextTick(func() error {
reqDataCopy.Pager.Skip +=
collector.args.PageSize
reqDataCopy.Pager.Page += concurrency
return collect()
})
return nil
})
return nil
```
In order to make sure that happens as we expected, a set of Unit Tests is
introduced: (check the `api_collector_test.go` for detail)
```go
// simulate fetching all pages of jira changelogs for 1 issue id with 1
concurrency,
// assuming api doesn't return total number of pages.
// then, we are expecting 2 calls for GetAsync and NextTick each,
otherwise, deadlock happens
getAsyncCounter := 0
mockApi := new(helperMocks.RateLimitedApiClient)
mockApi.On("GetAsync", mock.Anything, mock.Anything, mock.Anything,
mock.Anything).Run(func(args mock.Arguments) {
// fake records for first page, no records for second page
body := "[1,2,3]"
if getAsyncCounter > 0 {
body = "[]"
}
getAsyncCounter += 1
res := &http.Response{
Request: &http.Request{
URL: &url.URL{},
},
Body: ioutil.NopCloser(bytes.NewBufferString(body)),
}
handler := args.Get(3).(common.ApiAsyncCallback)
handler(res)
}).Twice()
mockApi.On("NextTick", mock.Anything).Run(func(args mock.Arguments) {
handler := args.Get(0).(func() error)
assert.Nil(t, handler())
}).Twice()
mockApi.On("HasError").Return(false)
mockApi.On("WaitAsync").Return(nil)
params := struct {
Name string
}{Name: "testparams"}
collector, err := NewApiCollector(ApiCollectorArgs{
RawDataSubTaskArgs: RawDataSubTaskArgs{
Ctx: mockCtx,
Table: "whatever rawtable",
Params: params,
},
ApiClient: mockApi,
Input: mockInput,
UrlTemplate: "whatever url",
Concurrency: 1,
PageSize: 3,
ResponseParser: GetRawMessageArrayFromResponse,
})
assert.Nil(t, err)
assert.Nil(t, collector.Execute())
mockDal.AssertExpectations(t)
```
I had tested it with `go test ./plugins/helper ` on my machine,
## Step to reproduce
1. check out my branch
2. remove other `*_test.go` inside `./plugin/helper` folder, because they
may fail to compile, keep only the `api_collector_test.go`
3. run `go test ./plugins/helper ` at the project root directory.
### Does this close any open issues?
Closes #2052
### Screenshots
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
