Skip to content

Commit 21f7945

Browse files
Evgenii Davydovclaude
andcommitted
fix(youtrack): preserve incremental state and replace stale data
Align changed-issue windows for comments and changelogs, keep only the latest issue and workflow-state snapshots on replay, and remove domain comments that were deleted upstream. Make API retries cancellation-aware with a single retry owner, and extend e2e scenarios and fixtures to cover these transitions. Skip 404s from the per-issue comment and activity endpoints so an issue deleted upstream no longer fails the subtask and freezes its bookmark. Delete the scope's domain issue labels and assignees before converting, so they are replaced even when a run emits none. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 57ca469 commit 21f7945

20 files changed

Lines changed: 919 additions & 90 deletions

‎backend/plugins/youtrack/e2e/changelog_test.go‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ import (
4444
func TestYoutrackIssueChangelogDataFlow(t *testing.T) {
4545
var youtrack impl.Youtrack
4646
dataflowTester := e2ehelper.NewDataFlowTester(t, "youtrack", youtrack)
47+
// stateful scenario: the rerun at the end must exercise the real
48+
// config-change transition against persisted state, not the tester's
49+
// forced full sync
50+
beginStatefulScenario(t, dataflowTester)
4751

4852
dataflowTester.ImportCsvIntoRawTable("./raw_tables/_raw_youtrack_issues.csv", "_raw_youtrack_issues")
4953
dataflowTester.ImportCsvIntoRawTable("./raw_tables/_raw_youtrack_issue_changelogs.csv", "_raw_youtrack_issue_changelogs")
@@ -53,8 +57,8 @@ func TestYoutrackIssueChangelogDataFlow(t *testing.T) {
5357

5458
for _, projectId := range []string{"0-1", "0-2"} {
5559
taskData := newTaskData(projectId, zeroConfig())
56-
dataflowTester.Subtask(tasks.ExtractIssuesMeta, taskData)
57-
dataflowTester.Subtask(tasks.ExtractIssueChangelogsMeta, taskData)
60+
runSubtaskPreservingState(t, dataflowTester, tasks.ExtractIssuesMeta, taskData)
61+
runSubtaskPreservingState(t, dataflowTester, tasks.ExtractIssueChangelogsMeta, taskData)
5862
}
5963

6064
dataflowTester.VerifyTableWithOptions(models.YoutrackIssueChangelog{}, e2ehelper.TableOptions{
@@ -115,7 +119,7 @@ func TestYoutrackIssueChangelogDataFlow(t *testing.T) {
115119
dataflowTester.ImportCsvIntoTabler("./snapshot_tables/_tool_youtrack_workflow_states.csv", &models.YoutrackWorkflowState{})
116120
dataflowTester.FlushTabler(&ticket.IssueChangelogs{})
117121
for _, projectId := range []string{"0-1", "0-2"} {
118-
dataflowTester.Subtask(tasks.ConvertIssueChangelogsMeta, newTaskData(projectId, zeroConfig()))
122+
runSubtaskPreservingState(t, dataflowTester, tasks.ConvertIssueChangelogsMeta, newTaskData(projectId, zeroConfig()))
119123
}
120124

121125
dataflowTester.VerifyTableWithOptions(ticket.IssueChangelogs{}, e2ehelper.TableOptions{
@@ -194,14 +198,15 @@ func TestYoutrackIssueChangelogDataFlow(t *testing.T) {
194198
assert.Positive(t, domainCountWhere("author_name != ''"), "AuthorName carries the display name")
195199

196200
// re-walks are idempotent via upsert by activity id: the
197-
// config change flips the extractor to full sync, so every
198-
// raw row is reprocessed — the counts must not move
201+
// config change against the PRESERVED state flips the extractor to full
202+
// sync — no collection involved — so every raw row is
203+
// reprocessed and the counts must not move
199204
reextract := zeroConfig()
200205
reextract.StoryPointField = "Days in stage"
201206
for _, projectId := range []string{"0-1", "0-2"} {
202207
taskData := newTaskData(projectId, reextract)
203-
dataflowTester.Subtask(tasks.ExtractIssueChangelogsMeta, taskData)
204-
dataflowTester.Subtask(tasks.ConvertIssueChangelogsMeta, taskData)
208+
runSubtaskPreservingState(t, dataflowTester, tasks.ExtractIssueChangelogsMeta, taskData)
209+
runSubtaskPreservingState(t, dataflowTester, tasks.ConvertIssueChangelogsMeta, taskData)
205210
}
206211
assert.Equal(t, toolCount, countWhere(&models.YoutrackIssueChangelog{}, "1 = 1"),
207212
"a full re-extraction upserts by activity id — no duplicate tool rows")

‎backend/plugins/youtrack/e2e/comment_test.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,45 @@ func TestYoutrackCommentDataFlow(t *testing.T) {
146146
require.NoError(t, dataflowTester.Db.Model(&crossdomain.Account{}).Count(&domainAccounts).Error)
147147
assert.Positive(t, domainAccounts)
148148
}
149+
150+
// TestYoutrackCommentAllDeletedReplacement covers the replacement edge the
151+
// lazy divider cleanup cannot reach: a scope whose comments have ALL become
152+
// deleted emits zero output rows, and the previously converted domain rows
153+
// must still disappear (deleted comments leave domain metrics).
154+
func TestYoutrackCommentAllDeletedReplacement(t *testing.T) {
155+
var youtrack impl.Youtrack
156+
dataflowTester := e2ehelper.NewDataFlowTester(t, "youtrack", youtrack)
157+
158+
dataflowTester.ImportCsvIntoRawTable("./raw_tables/_raw_youtrack_issues.csv", "_raw_youtrack_issues")
159+
dataflowTester.ImportCsvIntoRawTable("./raw_tables/_raw_youtrack_issue_comments.csv", "_raw_youtrack_issue_comments")
160+
dataflowTester.FlushTabler(&models.YoutrackIssue{})
161+
dataflowTester.FlushTabler(&models.YoutrackIssueComment{})
162+
dataflowTester.FlushTabler(&ticket.IssueComment{})
163+
164+
for _, projectId := range []string{"0-1", "0-2"} {
165+
taskData := newTaskData(projectId, zeroConfig())
166+
dataflowTester.Subtask(tasks.ExtractIssuesMeta, taskData)
167+
dataflowTester.Subtask(tasks.ExtractCommentsMeta, taskData)
168+
}
169+
for _, projectId := range []string{"0-1", "0-2"} {
170+
dataflowTester.Subtask(tasks.ConvertCommentsMeta, newTaskData(projectId, zeroConfig()))
171+
}
172+
173+
domainCount := func() int64 {
174+
var n int64
175+
require.NoError(t, dataflowTester.Db.Model(&ticket.IssueComment{}).Count(&n).Error)
176+
return n
177+
}
178+
require.Positive(t, domainCount(), "populated scope converts comments into the domain layer")
179+
180+
// every comment becomes deleted upstream (the tool layer keeps the rows,
181+
// flag set — exactly what the next incremental collection would extract)
182+
require.NoError(t, dataflowTester.Db.Model(&models.YoutrackIssueComment{}).
183+
Where("connection_id = 1").Update("deleted", true).Error)
184+
185+
for _, projectId := range []string{"0-1", "0-2"} {
186+
dataflowTester.Subtask(tasks.ConvertCommentsMeta, newTaskData(projectId, zeroConfig()))
187+
}
188+
assert.Zero(t, domainCount(),
189+
"conversion emitting zero rows must still replace the scope's previous domain output")
190+
}

‎backend/plugins/youtrack/e2e/issue_mapping_test.go‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ limitations under the License.
1818
package e2e
1919

2020
import (
21+
"encoding/json"
2122
"testing"
23+
"time"
2224

2325
"github.com/apache/devlake/core/models/common"
2426
"github.com/apache/devlake/core/models/domainlayer/ticket"
@@ -130,6 +132,120 @@ func TestYoutrackIssueDataFlowWithMappings(t *testing.T) {
130132
assert.Zero(t, storyAsRequirement, "no row keeps the stale mapping")
131133
}
132134

135+
// TestYoutrackMappingReplayPreservesState covers the transition the
136+
// DataFlowTester.Subtask reruns above cannot: Subtask deletes subtask state
137+
// and forces FullSync, so it can never verify that a config change
138+
// invalidates PERSISTED incremental state. This scenario keeps the state:
139+
//
140+
// run 1 initial extraction (full sync: no prior state)
141+
// run 2 incremental update — one issue re-collected without its tag
142+
// run 3 mapping change — full re-extraction replayed from the raw layer
143+
//
144+
// asserting (a) the incremental run processes only the new raw version,
145+
// (b) the replay never re-hits the API and never resurrects the removed
146+
// tag from the retained old version, and (c) the new mapping applies.
147+
func TestYoutrackMappingReplayPreservesState(t *testing.T) {
148+
var youtrack impl.Youtrack
149+
dataflowTester := e2ehelper.NewDataFlowTester(t, "youtrack", youtrack)
150+
beginStatefulScenario(t, dataflowTester)
151+
152+
dataflowTester.ImportCsvIntoRawTable("./raw_tables/_raw_youtrack_issues.csv", "_raw_youtrack_issues")
153+
dataflowTester.FlushTabler(&models.YoutrackIssue{})
154+
dataflowTester.FlushTabler(&models.YoutrackIssueLabel{})
155+
dataflowTester.FlushTabler(&models.YoutrackAccount{})
156+
157+
// run 1: initial full extraction
158+
for _, projectId := range []string{"0-1", "0-2"} {
159+
runSubtaskPreservingState(t, dataflowTester, tasks.ExtractIssuesMeta, newTaskData(projectId, mappedConfig()))
160+
}
161+
162+
// the fixture's tagged issue in project 0-1 (PROJ1-1, tag "Долго в работе")
163+
const taggedIssue = "2-1"
164+
countLabels := func(issueId string) int64 {
165+
var n int64
166+
require.NoError(t, dataflowTester.Db.Model(&models.YoutrackIssueLabel{}).
167+
Where("connection_id = 1 AND issue_id = ?", issueId).Count(&n).Error)
168+
return n
169+
}
170+
var totalLabels int64
171+
require.NoError(t, dataflowTester.Db.Model(&models.YoutrackIssueLabel{}).
172+
Where("connection_id = 1").Count(&totalLabels).Error)
173+
require.Equal(t, int64(1), countLabels(taggedIssue), "run 1 extracts the fixture's tag")
174+
require.Positive(t, totalLabels)
175+
176+
// the incremental update: the same issue re-collected tagless, updated later
177+
var rawCountBefore int64
178+
require.NoError(t, dataflowTester.Db.Table("_raw_youtrack_issues").Count(&rawCountBefore).Error)
179+
type rawIssueRow struct {
180+
Params string
181+
Data []byte
182+
Url string
183+
}
184+
var rows []rawIssueRow
185+
require.NoError(t, dataflowTester.Db.Table("_raw_youtrack_issues").
186+
Where("params LIKE ?", `%0-1%`).Find(&rows).Error)
187+
var src rawIssueRow
188+
var v2data []byte
189+
for _, row := range rows {
190+
var issue map[string]interface{}
191+
require.NoError(t, json.Unmarshal(row.Data, &issue))
192+
if issue["id"] != taggedIssue {
193+
continue
194+
}
195+
issue["tags"] = []interface{}{}
196+
issue["updated"] = issue["updated"].(float64) + 60000
197+
v2, err := json.Marshal(issue)
198+
require.NoError(t, err)
199+
v2data = v2
200+
src = row
201+
break
202+
}
203+
require.NotNil(t, v2data, "the fixture must contain issue %s in project 0-1", taggedIssue)
204+
require.NoError(t, dataflowTester.Db.Table("_raw_youtrack_issues").Create(map[string]interface{}{
205+
"params": src.Params,
206+
"data": v2data,
207+
"url": src.Url,
208+
"created_at": time.Now(), // inside run 2's incremental window
209+
}).Error)
210+
211+
// run 2: same config, state preserved -> incremental — only the new
212+
// version is processed, the tag disappears
213+
for _, projectId := range []string{"0-1", "0-2"} {
214+
runSubtaskPreservingState(t, dataflowTester, tasks.ExtractIssuesMeta, newTaskData(projectId, mappedConfig()))
215+
}
216+
assert.Zero(t, countLabels(taggedIssue), "incremental re-extraction replaces the issue's labels")
217+
assert.Equal(t, totalLabels-1, func() int64 {
218+
var n int64
219+
require.NoError(t, dataflowTester.Db.Model(&models.YoutrackIssueLabel{}).
220+
Where("connection_id = 1").Count(&n).Error)
221+
return n
222+
}(), "every other issue's labels are untouched")
223+
224+
// run 3: a mapping change must invalidate the persisted incremental
225+
// state -> full re-extraction replayed from the raw layer. The retained
226+
// v1 still carries the tag; only latest-version expansion keeps it dead.
227+
remapped := mappedConfig()
228+
remapped.TypeMappings = map[string]string{"Story": ticket.TASK, "Bug": ticket.BUG}
229+
for _, projectId := range []string{"0-1", "0-2"} {
230+
runSubtaskPreservingState(t, dataflowTester, tasks.ExtractIssuesMeta, newTaskData(projectId, remapped))
231+
}
232+
assert.Zero(t, countLabels(taggedIssue),
233+
"mapping replay must not resurrect the tag from the retained old raw version")
234+
235+
var storyAsTask, storyAsRequirement int64
236+
require.NoError(t, dataflowTester.Db.Model(&models.YoutrackIssue{}).
237+
Where("connection_id = 1 AND type_name = 'Story' AND std_type = ?", ticket.TASK).Count(&storyAsTask).Error)
238+
require.NoError(t, dataflowTester.Db.Model(&models.YoutrackIssue{}).
239+
Where("connection_id = 1 AND type_name = 'Story' AND std_type = ?", ticket.REQUIREMENT).Count(&storyAsRequirement).Error)
240+
assert.Positive(t, storyAsTask, "the changed mapping re-applies from the raw layer")
241+
assert.Zero(t, storyAsRequirement, "no row keeps the stale mapping")
242+
243+
var rawCountAfter int64
244+
require.NoError(t, dataflowTester.Db.Table("_raw_youtrack_issues").Count(&rawCountAfter).Error)
245+
assert.Equal(t, rawCountBefore+1, rawCountAfter,
246+
"the replay read the raw layer only — no collection call (the task data carries no API client)")
247+
}
248+
133249
// TestYoutrackIssueDataFlowMissingField covers the renamed-field trap: a
134250
// configured-but-absent field warns per scope and leaves the dedicated
135251
// columns empty; the run completes (warn, never fail).
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
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 e2e
19+
20+
import (
21+
"testing"
22+
23+
"github.com/apache/devlake/core/models/domainlayer/ticket"
24+
"github.com/apache/devlake/core/utils"
25+
"github.com/apache/devlake/helpers/e2ehelper"
26+
"github.com/apache/devlake/plugins/youtrack/impl"
27+
"github.com/apache/devlake/plugins/youtrack/models"
28+
"github.com/apache/devlake/plugins/youtrack/tasks"
29+
"github.com/stretchr/testify/require"
30+
)
31+
32+
// The divider's lazy cleanup fires only when a converter emits an output row,
33+
// so the domain tables below must be replaced explicitly: once every source
34+
// row disappears, the scope's previous domain output must disappear too.
35+
36+
func countRows(t *testing.T, dataflowTester *e2ehelper.DataFlowTester, model interface{}) int64 {
37+
var n int64
38+
require.NoError(t, dataflowTester.Db.Model(model).Count(&n).Error)
39+
return n
40+
}
41+
42+
// TestYoutrackIssueLabelsAllRemovedReplacement: the project's last tag is
43+
// removed — the extractor wipes the tool labels, the converter emits nothing.
44+
func TestYoutrackIssueLabelsAllRemovedReplacement(t *testing.T) {
45+
var youtrack impl.Youtrack
46+
dataflowTester := e2ehelper.NewDataFlowTester(t, "youtrack", youtrack)
47+
48+
dataflowTester.ImportCsvIntoRawTable("./raw_tables/_raw_youtrack_issues.csv", "_raw_youtrack_issues")
49+
dataflowTester.FlushTabler(&models.YoutrackIssue{})
50+
dataflowTester.FlushTabler(&models.YoutrackIssueLabel{})
51+
dataflowTester.FlushTabler(&ticket.IssueLabel{})
52+
53+
projectIds := []string{"0-1", "0-2"}
54+
for _, projectId := range projectIds {
55+
dataflowTester.Subtask(tasks.ExtractIssuesMeta, newTaskData(projectId, zeroConfig()))
56+
}
57+
for _, projectId := range projectIds {
58+
dataflowTester.Subtask(tasks.ConvertIssueLabelsMeta, newTaskData(projectId, zeroConfig()))
59+
}
60+
require.Positive(t, countRows(t, dataflowTester, &ticket.IssueLabel{}), "populated scope converts labels into the domain layer")
61+
62+
require.NoError(t, dataflowTester.Db.Where("connection_id = 1").Delete(&models.YoutrackIssueLabel{}).Error)
63+
64+
for _, projectId := range projectIds {
65+
dataflowTester.Subtask(tasks.ConvertIssueLabelsMeta, newTaskData(projectId, zeroConfig()))
66+
}
67+
require.Zero(t, countRows(t, dataflowTester, &ticket.IssueLabel{}),
68+
"conversion emitting zero labels must still replace the scope's previous domain labels")
69+
}
70+
71+
// TestYoutrackIssueAssigneesAllRemovedReplacement: every issue of the project
72+
// becomes unassigned — issues still convert, but no assignee row is emitted.
73+
func TestYoutrackIssueAssigneesAllRemovedReplacement(t *testing.T) {
74+
var youtrack impl.Youtrack
75+
dataflowTester := e2ehelper.NewDataFlowTester(t, "youtrack", youtrack)
76+
77+
dataflowTester.ImportCsvIntoTabler("./snapshot_tables/_tool_youtrack_projects.csv", &models.YoutrackProject{})
78+
dataflowTester.ImportCsvIntoTabler("./snapshot_tables/_tool_youtrack_issues_assignee_seed.csv", &models.YoutrackIssue{})
79+
dataflowTester.FlushTabler(&ticket.Issue{})
80+
dataflowTester.FlushTabler(&ticket.BoardIssue{})
81+
dataflowTester.FlushTabler(&ticket.IssueAssignee{})
82+
// the seed carries no raw origin; stamp the one the issue extractor would
83+
// have set, since the converter copies it onto every domain row
84+
require.NoError(t, dataflowTester.Db.Model(&models.YoutrackIssue{}).Where("connection_id = 1").Updates(map[string]interface{}{
85+
"_raw_data_table": "_raw_youtrack_issues",
86+
"_raw_data_params": utils.ToJsonString(models.YoutrackApiParams{ConnectionId: 1, ProjectId: "0-1"}),
87+
}).Error)
88+
89+
dataflowTester.Subtask(tasks.ConvertIssuesMeta, newTaskData("0-1", zeroConfig()))
90+
require.Positive(t, countRows(t, dataflowTester, &ticket.IssueAssignee{}), "an assigned issue converts into issue_assignees")
91+
92+
require.NoError(t, dataflowTester.Db.Model(&models.YoutrackIssue{}).
93+
Where("connection_id = 1").Updates(map[string]interface{}{"assignee_id": "", "assignee_name": ""}).Error)
94+
95+
dataflowTester.Subtask(tasks.ConvertIssuesMeta, newTaskData("0-1", zeroConfig()))
96+
require.Positive(t, countRows(t, dataflowTester, &ticket.Issue{}), "the issue itself still converts")
97+
require.Zero(t, countRows(t, dataflowTester, &ticket.IssueAssignee{}),
98+
"conversion emitting zero assignees must still replace the scope's previous domain assignees")
99+
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
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 e2e
19+
20+
import (
21+
"context"
22+
"testing"
23+
24+
"github.com/apache/devlake/core/models"
25+
"github.com/apache/devlake/core/plugin"
26+
"github.com/apache/devlake/core/runner"
27+
"github.com/apache/devlake/helpers/e2ehelper"
28+
contextimpl "github.com/apache/devlake/impls/context"
29+
"github.com/stretchr/testify/require"
30+
)
31+
32+
// beginStatefulScenario opens a test that exercises the framework's
33+
// incremental/full-sync TRANSITIONS — the thing DataFlowTester.Subtask
34+
// cannot cover, because it deletes _devlake_subtask_state and forces
35+
// FullSync: true on every call (helpers/e2ehelper/data_flow_tester.go).
36+
//
37+
// Within the scenario, subtasks run through runSubtaskPreservingState: the
38+
// subtask state survives between calls and the sync policy permits
39+
// incremental mode, so a stateful extractor's second run goes incremental
40+
// on an unchanged config and full-syncs on a config change — exactly the
41+
// production semantics. The first run still full-syncs (no prior state).
42+
//
43+
// The scenario flushes _devlake_subtask_state at entry because the e2e
44+
// database is shared across tests and state keys (plugin, subtask, params)
45+
// do not include the test name.
46+
func beginStatefulScenario(t *testing.T, dataflowTester *e2ehelper.DataFlowTester) {
47+
t.Helper()
48+
dataflowTester.FlushTabler(&models.SubtaskState{})
49+
}
50+
51+
// runSubtaskPreservingState executes one subtask inside a stateful scenario
52+
// (see beginStatefulScenario): state is neither flushed nor forced into
53+
// full sync. Panics on error, like DataFlowTester.Subtask.
54+
func runSubtaskPreservingState(t *testing.T, dataflowTester *e2ehelper.DataFlowTester, subtaskMeta plugin.SubTaskMeta, taskData interface{}) {
55+
t.Helper()
56+
subtaskCtx := contextimpl.NewStandaloneSubTaskContext(
57+
context.Background(),
58+
runner.CreateBasicRes(dataflowTester.Cfg, dataflowTester.Log, dataflowTester.Db),
59+
dataflowTester.Name,
60+
taskData,
61+
subtaskMeta.Name,
62+
&models.SyncPolicy{},
63+
)
64+
require.NoError(t, subtaskMeta.EntryPoint(subtaskCtx))
65+
}

0 commit comments

Comments
 (0)