Skip to content

Commit 092b895

Browse files
authored
fix(gitlab): add missing commit_updated_at column used by MR incremental filter (#9046)
#8959 changed `GetMergeRequestsIterator` to filter merge requests by `GREATEST(gmr.gitlab_updated_at, COALESCE(gmr.commit_updated_at, gmr.gitlab_updated_at))`, but `commit_updated_at` does not exist on `_tool_gitlab_merge_requests`: there is neither a model field nor a migration script, and nothing ever writes the value. As a result every incremental "Collect MR Notes" run (the only caller that passes a stateful collector; the MR commit collector passes nil) aborts with: Error 1054 (42S22): Unknown column 'gmr.commit_updated_at' in 'where clause' This makes the intended behaviour actually work instead of reverting it: * add `CommitUpdatedAt` to `GitlabMergeRequest` plus a migration script * maintain the column in the MR commit extractor, setting it to the latest authored date of the MR's commits, so MRs that received new commits without their own `updated_at` being bumped (e.g. force-pushes) are picked up again * the collector filter is unchanged and now resolves against a real column Verified on MySQL 8 and PostgreSQL 14 (extractor UPDATE and the GREATEST filter produce identical results on both).
1 parent b24379c commit 092b895

5 files changed

Lines changed: 89 additions & 2 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
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 migrationscripts
19+
20+
import (
21+
"time"
22+
23+
"github.com/apache/incubator-devlake/core/context"
24+
"github.com/apache/incubator-devlake/core/errors"
25+
"github.com/apache/incubator-devlake/core/plugin"
26+
)
27+
28+
var _ plugin.MigrationScript = (*addMrCommitUpdatedAt)(nil)
29+
30+
type mrCommitUpdatedAt struct {
31+
CommitUpdatedAt *time.Time
32+
}
33+
34+
func (mrCommitUpdatedAt) TableName() string {
35+
return "_tool_gitlab_merge_requests"
36+
}
37+
38+
type addMrCommitUpdatedAt struct{}
39+
40+
func (*addMrCommitUpdatedAt) Up(basicRes context.BasicRes) errors.Error {
41+
return errors.Convert(basicRes.GetDal().AutoMigrate(&mrCommitUpdatedAt{}))
42+
}
43+
44+
func (*addMrCommitUpdatedAt) Version() uint64 {
45+
return 20260812000001
46+
}
47+
48+
func (*addMrCommitUpdatedAt) Name() string {
49+
return "gitlab: add commit_updated_at to merge requests"
50+
}

backend/plugins/gitlab/models/migrationscripts/register.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,6 @@ func All() []plugin.MigrationScript {
5454
new(addIsChildToPipelines240906),
5555
new(addPrSizeExcludedFileExtensions),
5656
new(addAdditionsDeletionsToMr),
57+
new(addMrCommitUpdatedAt),
5758
}
5859
}

backend/plugins/gitlab/models/mr.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ type GitlabMergeRequest struct {
5353
Component string `gorm:"type:varchar(255)"`
5454
Additions *int `gorm:"comment:Lines added in this MR diff only"`
5555
Deletions *int `gorm:"comment:Lines deleted in this MR diff only"`
56+
// CommitUpdatedAt holds the most recent authored date among the commits of this MR.
57+
// GitLab does not bump `updated_at` for every push (e.g. force-pushes), so this
58+
// timestamp is used in addition to GitlabUpdatedAt to detect MRs that changed
59+
// during incremental collection.
60+
CommitUpdatedAt *time.Time `gorm:"comment:Latest authored date of the commits belonging to this MR"`
5661
common.NoPKModel
5762
}
5863

backend/plugins/gitlab/tasks/mr_commit_extractor.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,37 @@ func ExtractApiMergeRequestsCommits(subtaskCtx plugin.SubTaskContext) errors.Err
104104
return err
105105
}
106106

107-
return extractor.Execute()
107+
if err := extractor.Execute(); err != nil {
108+
return err
109+
}
110+
111+
return updateMrCommitUpdatedAt(subtaskCtx)
112+
}
113+
114+
// updateMrCommitUpdatedAt refreshes `_tool_gitlab_merge_requests.commit_updated_at` with the
115+
// latest authored date of the MR's commits. GitLab does not always bump the MR's own
116+
// `updated_at` when commits are pushed (e.g. force-pushes), so subtasks collecting
117+
// MR sub-entities incrementally would otherwise skip those MRs.
118+
func updateMrCommitUpdatedAt(subtaskCtx plugin.SubTaskContext) errors.Error {
119+
data := subtaskCtx.GetData().(*GitlabTaskData)
120+
return subtaskCtx.GetDal().Exec(`
121+
UPDATE _tool_gitlab_merge_requests AS gmr
122+
SET commit_updated_at = (
123+
SELECT MAX(gmc.commit_authored_date)
124+
FROM _tool_gitlab_mr_commits gmc
125+
WHERE gmc.connection_id = gmr.connection_id
126+
AND gmc.merge_request_id = gmr.gitlab_id
127+
)
128+
WHERE gmr.connection_id = ?
129+
AND gmr.project_id = ?
130+
AND EXISTS (
131+
SELECT 1
132+
FROM _tool_gitlab_mr_commits gmc
133+
WHERE gmc.connection_id = gmr.connection_id
134+
AND gmc.merge_request_id = gmr.gitlab_id
135+
)`,
136+
data.Options.ConnectionId, data.Options.ProjectId,
137+
)
108138
}
109139

110140
// Convert the API response to our DB model instance

backend/plugins/gitlab/tasks/shared.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,8 @@ func GetMergeRequestsIterator(taskCtx plugin.SubTaskContext, apiCollector *api.S
198198
// Filter by the LATER of gitlab_updated_at or commit_updated_at.
199199
// Using only gitlab_updated_at misses MRs where new commits were pushed
200200
// without the MR itself being updated (e.g. force-pushed commits).
201-
// COALESCE handles MRs with no recorded commit_updated_at.
201+
// commit_updated_at is maintained by the MR commit extractor and is NULL
202+
// for MRs without collected commits, hence the COALESCE.
202203
clauses = append(clauses, dal.Where(
203204
`GREATEST(gmr.gitlab_updated_at, COALESCE(gmr.commit_updated_at, gmr.gitlab_updated_at)) > ?`,
204205
*apiCollector.GetSince(),

0 commit comments

Comments
 (0)