Skip to content

Commit 2c3a04e

Browse files
committed
fix(github): fix zerotime issues for GraphQL
1 parent 04a2532 commit 2c3a04e

File tree

12 files changed

+129
-41
lines changed

12 files changed

+129
-41
lines changed

backend/core/utils/time.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
package utils
19+
20+
import "time"
21+
22+
// NilIfZeroTime returns nil if t is nil or represents the zero time (0001-01-01...).
23+
// Otherwise, it returns t unchanged.
24+
func NilIfZeroTime(t *time.Time) *time.Time {
25+
if t == nil {
26+
return nil
27+
}
28+
if t.IsZero() {
29+
return nil
30+
}
31+
return t
32+
}

backend/core/utils/time_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
package utils
18+
19+
import (
20+
"testing"
21+
"time"
22+
23+
"github.com/stretchr/testify/assert"
24+
)
25+
26+
func TestNilIfZeroTime(t *testing.T) {
27+
type args struct {
28+
t *time.Time
29+
}
30+
tests := []struct {
31+
name string
32+
args args
33+
want *time.Time
34+
}{
35+
{
36+
name: "Empty date should be nil",
37+
args: args{nil},
38+
want: nil,
39+
},
40+
{
41+
name: "Zero date should be nil",
42+
args: args{&time.Time{}},
43+
want: nil,
44+
},
45+
}
46+
for _, tt := range tests {
47+
t.Run(tt.name, func(t *testing.T) {
48+
assert.Equalf(t, tt.want, NilIfZeroTime(tt.args.t), "NilIfZeroTime(%v)", tt.args.t)
49+
})
50+
}
51+
}

backend/plugins/github/models/release.go

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,25 @@ import (
2525

2626
type GithubRelease struct {
2727
common.NoPKModel `json:"-" mapstructure:"-"`
28-
ConnectionId uint64 `json:"connection_id" gorm:"primaryKey"`
29-
GithubId int `json:"github_id"`
30-
Id string `json:"id" gorm:"type:varchar(255);primaryKey"`
31-
AuthorName string `json:"authorName"`
32-
AuthorID string `json:"authorId"`
33-
CreatedAt time.Time `json:"createdAt"`
34-
DatabaseID int `json:"databaseId"`
35-
Description string `json:"description"`
36-
DescriptionHTML string `json:"descriptionHTML"`
37-
IsDraft bool `json:"isDraft"`
38-
IsLatest bool `json:"isLatest"`
39-
IsPrerelease bool `json:"isPrerelease"`
40-
Name string `json:"name"`
41-
PublishedAt time.Time `json:"publishedAt"`
42-
ResourcePath string `json:"resourcePath"`
43-
TagName string `json:"tagName"`
44-
UpdatedAt time.Time `json:"updatedAt"`
45-
CommitSha string `json:"commit_sha"`
46-
URL string `json:"url"`
28+
ConnectionId uint64 `json:"connection_id" gorm:"primaryKey"`
29+
GithubId int `json:"github_id"`
30+
Id string `json:"id" gorm:"type:varchar(255);primaryKey"`
31+
AuthorName string `json:"authorName"`
32+
AuthorID string `json:"authorId"`
33+
CreatedAt time.Time `json:"createdAt"`
34+
DatabaseID int `json:"databaseId"`
35+
Description string `json:"description"`
36+
DescriptionHTML string `json:"descriptionHTML"`
37+
IsDraft bool `json:"isDraft"`
38+
IsLatest bool `json:"isLatest"`
39+
IsPrerelease bool `json:"isPrerelease"`
40+
Name string `json:"name"`
41+
PublishedAt *time.Time `json:"publishedAt"`
42+
ResourcePath string `json:"resourcePath"`
43+
TagName string `json:"tagName"`
44+
UpdatedAt time.Time `json:"updatedAt"`
45+
CommitSha string `json:"commit_sha"`
46+
URL string `json:"url"`
4747
}
4848

4949
func (GithubRelease) TableName() string {

backend/plugins/github/tasks/release_convertor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func ConvertRelease(taskCtx plugin.SubTaskContext) errors.Error {
7373
DomainEntity: domainlayer.DomainEntity{
7474
Id: releaseIdGen.Generate(githubRelease.ConnectionId, githubRelease.Id),
7575
},
76-
PublishedAt: githubRelease.PublishedAt,
76+
PublishedAt: *githubRelease.PublishedAt,
7777
CicdScopeId: releaseScopeIdGen.Generate(githubRelease.ConnectionId, githubRelease.GithubId),
7878
Name: githubRelease.Name,
7979
DisplayTitle: githubRelease.Name,

backend/plugins/github_graphql/tasks/account_collector.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,13 @@ type GraphqlQueryAccountWrapper struct {
4040
}
4141

4242
type GraphqlQueryAccount struct {
43-
Login string
44-
Id int `graphql:"databaseId"`
45-
Name string
46-
Company string
47-
Email string
48-
AvatarUrl string
49-
HtmlUrl string `graphql:"url"`
50-
//Type string
43+
Login string
44+
Id int `graphql:"databaseId"`
45+
Name string
46+
Company string
47+
Email string
48+
AvatarUrl string
49+
HtmlUrl string `graphql:"url"`
5150
Organizations struct {
5251
Nodes []struct {
5352
Email string

backend/plugins/github_graphql/tasks/account_graphql_pre_extractor.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ type GithubAccountEdge struct {
2929
Email string
3030
AvatarUrl string
3131
HtmlUrl string `graphql:"url"`
32-
//Type string
3332
}
3433
type GraphqlInlineAccountQuery struct {
3534
GithubAccountEdge `graphql:"... on User"`

backend/plugins/github_graphql/tasks/issue_collector.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/apache/incubator-devlake/core/dal"
2727
"github.com/apache/incubator-devlake/core/errors"
2828
"github.com/apache/incubator-devlake/core/plugin"
29+
"github.com/apache/incubator-devlake/core/utils"
2930
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
3031
"github.com/apache/incubator-devlake/plugins/github/models"
3132
githubTasks "github.com/apache/incubator-devlake/plugins/github/tasks"
@@ -134,6 +135,7 @@ func CollectIssues(taskCtx plugin.SubTaskContext) errors.Error {
134135
query := queryWrapper.(*GraphqlQueryIssueWrapper)
135136
issues := query.Repository.IssueList.Issues
136137
for _, rawL := range issues {
138+
rawL.ClosedAt = utils.NilIfZeroTime(rawL.ClosedAt)
137139
if since != nil && since.After(rawL.UpdatedAt) {
138140
return messages, api.ErrFinishCollect
139141
}

backend/plugins/github_graphql/tasks/issue_extractor.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/apache/incubator-devlake/core/errors"
2626
"github.com/apache/incubator-devlake/core/models/domainlayer/ticket"
2727
"github.com/apache/incubator-devlake/core/plugin"
28+
"github.com/apache/incubator-devlake/core/utils"
2829
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
2930
"github.com/apache/incubator-devlake/plugins/github/models"
3031
githubTasks "github.com/apache/incubator-devlake/plugins/github/tasks"
@@ -68,6 +69,8 @@ func ExtractIssues(taskCtx plugin.SubTaskContext) errors.Error {
6869
if err != nil {
6970
return nil, err
7071
}
72+
// Normalize zero-date to nil for closedAt
73+
issue.ClosedAt = utils.NilIfZeroTime(issue.ClosedAt)
7174
results := make([]interface{}, 0, 1)
7275
githubIssue, err := convertGithubIssue(milestoneMap, issue, data.Options.ConnectionId, data.Options.GithubId)
7376
if err != nil {

backend/plugins/github_graphql/tasks/job_collector.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@ package tasks
1919

2020
import (
2121
"encoding/json"
22-
"fmt"
2322
"reflect"
2423
"time"
2524

2625
"github.com/apache/incubator-devlake/core/dal"
2726
"github.com/apache/incubator-devlake/core/errors"
2827
"github.com/apache/incubator-devlake/core/plugin"
28+
"github.com/apache/incubator-devlake/core/utils"
2929
helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
3030
"github.com/apache/incubator-devlake/plugins/github/models"
3131
githubTasks "github.com/apache/incubator-devlake/plugins/github/tasks"
@@ -202,14 +202,10 @@ func CollectJobs(taskCtx plugin.SubTaskContext) errors.Error {
202202
RunId: runId,
203203
GraphqlQueryCheckRun: &checkRun,
204204
}
205-
// A checkRun without a startedAt time is a run that was never started (skipped)
206-
// akwardly, GitHub assigns a completedAt time to such runs, which is the time when the run was skipped
207-
// TODO: Decide if we want to skip those runs or should we assign the startedAt time to the completedAt time
208-
if dbCheckRun.StartedAt == nil || dbCheckRun.StartedAt.IsZero() {
209-
debug := fmt.Sprintf("collector: checkRun.StartedAt is nil or zero: %s", dbCheckRun.Id)
210-
taskCtx.GetLogger().Debug(debug, "Collector: CheckRun started at is nil or zero")
211-
continue
212-
}
205+
// A checkRun without a startedAt time is a run that was never started (skipped), GitHub returns
206+
// a ZeroTime (Due to the GO implementation) for startedAt, so we need to check for that here.
207+
dbCheckRun.StartedAt = utils.NilIfZeroTime(dbCheckRun.StartedAt)
208+
dbCheckRun.CompletedAt = utils.NilIfZeroTime(dbCheckRun.CompletedAt)
213209
updatedAt := dbCheckRun.StartedAt
214210
if dbCheckRun.CompletedAt != nil {
215211
updatedAt = dbCheckRun.CompletedAt

backend/plugins/github_graphql/tasks/job_extractor.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/apache/incubator-devlake/core/errors"
2525
"github.com/apache/incubator-devlake/core/models/domainlayer/devops"
2626
"github.com/apache/incubator-devlake/core/plugin"
27+
"github.com/apache/incubator-devlake/core/utils"
2728
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
2829
"github.com/apache/incubator-devlake/plugins/github/models"
2930
githubTasks "github.com/apache/incubator-devlake/plugins/github/tasks"
@@ -57,7 +58,9 @@ func ExtractJobs(taskCtx plugin.SubTaskContext) errors.Error {
5758
return nil, err
5859
}
5960
results := make([]interface{}, 0, 1)
60-
61+
// Normalize zero-date times to nil
62+
checkRun.StartedAt = utils.NilIfZeroTime(checkRun.StartedAt)
63+
checkRun.CompletedAt = utils.NilIfZeroTime(checkRun.CompletedAt)
6164
paramsBytes, marshalError := json.Marshal(checkRun.Steps.Nodes)
6265
err = errors.Convert(marshalError)
6366
if err != nil {

0 commit comments

Comments
 (0)