diff --git a/backend/helpers/pluginhelper/api/graphql_collector.go b/backend/helpers/pluginhelper/api/graphql_collector.go index 2601e4e4f51..f4f5023a043 100644 --- a/backend/helpers/pluginhelper/api/graphql_collector.go +++ b/backend/helpers/pluginhelper/api/graphql_collector.go @@ -348,8 +348,16 @@ func (collector *GraphqlCollector) HasError() bool { return len(collector.workerErrors) > 0 } +// isIgnorableGraphqlQueryError reports whether err means the queried issue no +// longer exists upstream, so its children can be skipped. GitHub and Linear +// word this differently. func isIgnorableGraphqlQueryError(err error) bool { - return err != nil && strings.Contains(err.Error(), "Could not resolve to an Issue") + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "Could not resolve to an Issue") || + strings.Contains(msg, "Entity not found: Issue") } var _ plugin.SubTask = (*GraphqlCollector)(nil) diff --git a/backend/helpers/pluginhelper/api/graphql_collector_test.go b/backend/helpers/pluginhelper/api/graphql_collector_test.go index fcab4dce098..9c07435e968 100644 --- a/backend/helpers/pluginhelper/api/graphql_collector_test.go +++ b/backend/helpers/pluginhelper/api/graphql_collector_test.go @@ -26,6 +26,8 @@ import ( func TestIsIgnorableGraphqlQueryError(t *testing.T) { assert.True(t, isIgnorableGraphqlQueryError(errors.New("Could not resolve to an Issue with the number of 17."))) + assert.True(t, isIgnorableGraphqlQueryError(errors.New("Entity not found: Issue"))) + assert.False(t, isIgnorableGraphqlQueryError(errors.New("Entity not found: Team"))) assert.False(t, isIgnorableGraphqlQueryError(errors.New("some other graphql error"))) assert.False(t, isIgnorableGraphqlQueryError(nil)) } diff --git a/backend/plugins/linear/tasks/comment_collector.go b/backend/plugins/linear/tasks/comment_collector.go index 137067ca4b0..2d61291e36c 100644 --- a/backend/plugins/linear/tasks/comment_collector.go +++ b/backend/plugins/linear/tasks/comment_collector.go @@ -130,8 +130,7 @@ func CollectComments(taskCtx plugin.SubTaskContext) errors.Error { return query, variables, nil }, GetPageInfo: func(iQuery interface{}, args *helper.GraphqlCollectorArgs) (*helper.GraphqlQueryPageInfo, error) { - query := iQuery.(*GraphqlQueryCommentWrapper) - return query.Issue.Comments.PageInfo, nil + return commentsPageInfo(iQuery.(*GraphqlQueryCommentWrapper)), nil }, ResponseParser: func(queryWrapper interface{}) (messages []json.RawMessage, err errors.Error) { query := queryWrapper.(*GraphqlQueryCommentWrapper) diff --git a/backend/plugins/linear/tasks/issue_history_collector.go b/backend/plugins/linear/tasks/issue_history_collector.go index 847a268626e..7c1bdc4dd2c 100644 --- a/backend/plugins/linear/tasks/issue_history_collector.go +++ b/backend/plugins/linear/tasks/issue_history_collector.go @@ -115,8 +115,7 @@ func CollectIssueHistory(taskCtx plugin.SubTaskContext) errors.Error { return query, variables, nil }, GetPageInfo: func(iQuery interface{}, args *helper.GraphqlCollectorArgs) (*helper.GraphqlQueryPageInfo, error) { - query := iQuery.(*GraphqlQueryHistoryWrapper) - return query.Issue.History.PageInfo, nil + return historyPageInfo(iQuery.(*GraphqlQueryHistoryWrapper)), nil }, ResponseParser: func(queryWrapper interface{}) (messages []json.RawMessage, err errors.Error) { query := queryWrapper.(*GraphqlQueryHistoryWrapper) diff --git a/backend/plugins/linear/tasks/page_info_test.go b/backend/plugins/linear/tasks/page_info_test.go new file mode 100644 index 00000000000..ad738abde5b --- /dev/null +++ b/backend/plugins/linear/tasks/page_info_test.go @@ -0,0 +1,48 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tasks + +import ( + "testing" + + helper "github.com/apache/devlake/helpers/pluginhelper/api" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// A deleted issue comes back as an empty `issue` object: the collector +// skips its "Entity not found: Issue" error, but must not then fail on the +// missing page info. +func TestPageInfo_DeletedIssueEndsPagination(t *testing.T) { + comments := commentsPageInfo(&GraphqlQueryCommentWrapper{}) + require.NotNil(t, comments) + assert.False(t, comments.HasNextPage) + + history := historyPageInfo(&GraphqlQueryHistoryWrapper{}) + require.NotNil(t, history) + assert.False(t, history.HasNextPage) +} + +func TestPageInfo_KeepsLinearPagination(t *testing.T) { + query := &GraphqlQueryCommentWrapper{} + query.Issue.Comments.PageInfo = &helper.GraphqlQueryPageInfo{EndCursor: "c1", HasNextPage: true} + + pageInfo := commentsPageInfo(query) + assert.True(t, pageInfo.HasNextPage) + assert.Equal(t, "c1", pageInfo.EndCursor) +} diff --git a/backend/plugins/linear/tasks/shared.go b/backend/plugins/linear/tasks/shared.go index cc8ee22264a..eb1c6b48c1a 100644 --- a/backend/plugins/linear/tasks/shared.go +++ b/backend/plugins/linear/tasks/shared.go @@ -22,9 +22,28 @@ import ( "github.com/apache/devlake/core/dal" "github.com/apache/devlake/core/models/domainlayer/ticket" + helper "github.com/apache/devlake/helpers/pluginhelper/api" "github.com/apache/devlake/plugins/linear/models" ) +// An issue deleted in Linear returns an empty `issue` object with no page +// info. The collector skips that error, so treat the issue as its own last +// page instead of failing on the missing page info. +func pageInfoOrLast(pageInfo *helper.GraphqlQueryPageInfo) *helper.GraphqlQueryPageInfo { + if pageInfo == nil { + return &helper.GraphqlQueryPageInfo{HasNextPage: false} + } + return pageInfo +} + +func commentsPageInfo(query *GraphqlQueryCommentWrapper) *helper.GraphqlQueryPageInfo { + return pageInfoOrLast(query.Issue.Comments.PageInfo) +} + +func historyPageInfo(query *GraphqlQueryHistoryWrapper) *helper.GraphqlQueryPageInfo { + return pageInfoOrLast(query.Issue.History.PageInfo) +} + // issuesToCollectChildrenClauses builds the cursor clauses that drive per-issue // child collection (comments, history). When `since` is non-nil (an incremental // run), it restricts the sweep to issues updated since the last successful