|
| 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 tasks |
| 19 | + |
| 20 | +import ( |
| 21 | + "sort" |
| 22 | + "testing" |
| 23 | + |
| 24 | + "github.com/apache/devlake/core/dal" |
| 25 | + "github.com/apache/devlake/core/models/domainlayer" |
| 26 | + "github.com/apache/devlake/core/models/domainlayer/crossdomain" |
| 27 | + "github.com/apache/devlake/core/models/domainlayer/ticket" |
| 28 | + "github.com/apache/devlake/impls/dalgorm" |
| 29 | + "github.com/stretchr/testify/require" |
| 30 | + "gorm.io/driver/sqlite" |
| 31 | + "gorm.io/gorm" |
| 32 | +) |
| 33 | + |
| 34 | +// TestConnectIncidentInScopeProjectFilter reproduces the bug behind "the board |
| 35 | +// still contains incidents that relate to Jira issues that shouldn't be there". |
| 36 | +// |
| 37 | +// ConnectIncidentToDeployment builds project_incident_deployment_relationships |
| 38 | +// (the table the Change Failure Rate metric reads). It used to select incidents |
| 39 | +// by project_mapping on scope_id (= board) ONLY, so on a board shared by several |
| 40 | +// projects, every incident on the board was attached to every project -- the |
| 41 | +// incident objects leaked across projects even though the per-issue slice was |
| 42 | +// correct. |
| 43 | +// |
| 44 | +// Shared board (board1), mapped to both ProjectA and ProjectB. Incidents carry |
| 45 | +// scope_id = board1 (that is all an incident knows). The per-project slice lives |
| 46 | +// on board_issues.project_name: |
| 47 | +// |
| 48 | +// incident/issue 1001 board_issues.project_name = ProjectA |
| 49 | +// incident/issue 1002 board_issues.project_name = ProjectB |
| 50 | +// incident/issue 1003 board_issues.project_name = '' (legacy) |
| 51 | +// |
| 52 | +// Expected selection: |
| 53 | +// ProjectA -> {1001, 1003} (never 1002) |
| 54 | +// ProjectB -> {1002, 1003} (never 1001) |
| 55 | +// |
| 56 | +// On the OLD board-only query, ProjectA would also select 1002 -> the leak. |
| 57 | +func TestConnectIncidentInScopeProjectFilter(t *testing.T) { |
| 58 | + db := newDoraIncidentDB(t) |
| 59 | + |
| 60 | + const board = "board1" |
| 61 | + seedIncident(t, db, "1001", board) |
| 62 | + seedIncident(t, db, "1002", board) |
| 63 | + seedIncident(t, db, "1003", board) |
| 64 | + |
| 65 | + seedDoraBoardIssue(t, db, board, "1001", "ProjectA") |
| 66 | + seedDoraBoardIssue(t, db, board, "1002", "ProjectB") |
| 67 | + // 1003 is an unstamped (empty) row on a board that DOES have real per-project |
| 68 | + // stamps. On such a board the empty/NULL rows are the ambiguous "unfiltered |
| 69 | + // pool" and must NOT be attributed to any project (that was the leak). |
| 70 | + seedDoraBoardIssue(t, db, board, "1003", "") |
| 71 | + |
| 72 | + seedDoraProjectMapping(t, db, "ProjectA", "boards", board) |
| 73 | + seedDoraProjectMapping(t, db, "ProjectB", "boards", board) |
| 74 | + |
| 75 | + projectA := queryConnectIncidentIds(t, db, "ProjectA") |
| 76 | + projectB := queryConnectIncidentIds(t, db, "ProjectB") |
| 77 | + |
| 78 | + require.Equal(t, []string{"1001"}, projectA) |
| 79 | + require.NotContains(t, projectA, "1002", "ProjectA must not attach ProjectB's incident on a shared board") |
| 80 | + require.NotContains(t, projectA, "1003", "unstamped rows on a stamped board must be excluded") |
| 81 | + |
| 82 | + require.Equal(t, []string{"1002"}, projectB) |
| 83 | + require.NotContains(t, projectB, "1001") |
| 84 | + require.NotContains(t, projectB, "1003") |
| 85 | +} |
| 86 | + |
| 87 | +// TestConnectIncidentInScopeExcludesNullPool reproduces the real staging state: |
| 88 | +// ProjectA was collected (its ExtraJQL slice stamped with "ProjectA"), while the |
| 89 | +// rest of the board's incidents are left over from an earlier unfiltered |
| 90 | +// collection and carry a NULL project_name. ProjectA's Change Failure Rate must |
| 91 | +// include ONLY its stamped slice -- the NULL "unfiltered pool" must be excluded, |
| 92 | +// not re-admitted. |
| 93 | +// |
| 94 | +// This is the case the previous fix got wrong: a NULL/empty fallback re-added |
| 95 | +// every NULL row to every project, so ProjectA showed the whole board. |
| 96 | +func TestConnectIncidentInScopeExcludesNullPool(t *testing.T) { |
| 97 | + db := newDoraIncidentDB(t) |
| 98 | + |
| 99 | + const board = "board1" |
| 100 | + // ProjectA's real slice |
| 101 | + seedIncident(t, db, "A1", board) |
| 102 | + seedIncident(t, db, "A2", board) |
| 103 | + seedDoraBoardIssue(t, db, board, "A1", "ProjectA") |
| 104 | + seedDoraBoardIssue(t, db, board, "A2", "ProjectA") |
| 105 | + // leftover unfiltered pool from an earlier collection (NULL project_name) |
| 106 | + seedIncident(t, db, "N1", board) |
| 107 | + seedIncident(t, db, "N2", board) |
| 108 | + seedIncident(t, db, "N3", board) |
| 109 | + seedDoraBoardIssueNull(t, db, board, "N1") |
| 110 | + seedDoraBoardIssueNull(t, db, board, "N2") |
| 111 | + seedDoraBoardIssueNull(t, db, board, "N3") |
| 112 | + |
| 113 | + seedDoraProjectMapping(t, db, "ProjectA", "boards", board) |
| 114 | + |
| 115 | + ids := queryConnectIncidentIds(t, db, "ProjectA") |
| 116 | + require.Equal(t, []string{"A1", "A2"}, ids, "only ProjectA's stamped slice may count; the NULL pool must be excluded") |
| 117 | + require.NotContains(t, ids, "N1") |
| 118 | + require.NotContains(t, ids, "N2") |
| 119 | + require.NotContains(t, ids, "N3") |
| 120 | +} |
| 121 | + |
| 122 | +// TestConnectIncidentInScopeNoShare is the control: a non-shared board with no |
| 123 | +// per-project slice still attaches all of its incidents (no regression). |
| 124 | +func TestConnectIncidentInScopeNoShare(t *testing.T) { |
| 125 | + db := newDoraIncidentDB(t) |
| 126 | + |
| 127 | + const board = "boardSolo" |
| 128 | + seedIncident(t, db, "2001", board) |
| 129 | + seedIncident(t, db, "2002", board) |
| 130 | + seedDoraBoardIssue(t, db, board, "2001", "") |
| 131 | + seedDoraBoardIssue(t, db, board, "2002", "") |
| 132 | + seedDoraProjectMapping(t, db, "SoloProject", "boards", board) |
| 133 | + |
| 134 | + ids := queryConnectIncidentIds(t, db, "SoloProject") |
| 135 | + require.Equal(t, []string{"2001", "2002"}, ids) |
| 136 | +} |
| 137 | + |
| 138 | +// --- helpers --------------------------------------------------------------- |
| 139 | + |
| 140 | +func newDoraIncidentDB(t *testing.T) dal.Dal { |
| 141 | + gormDb, err := gorm.Open(sqlite.Open("file::memory:?cache=shared"), &gorm.Config{}) |
| 142 | + require.NoError(t, err) |
| 143 | + require.NoError(t, gormDb.AutoMigrate(&ticket.Incident{}, &ticket.BoardIssue{}, &crossdomain.ProjectMapping{})) |
| 144 | + require.NoError(t, gormDb.Exec("DELETE FROM incidents").Error) |
| 145 | + require.NoError(t, gormDb.Exec("DELETE FROM board_issues").Error) |
| 146 | + require.NoError(t, gormDb.Exec("DELETE FROM project_mapping").Error) |
| 147 | + return dalgorm.NewDalgorm(gormDb) |
| 148 | +} |
| 149 | + |
| 150 | +func seedIncident(t *testing.T, db dal.Dal, id, boardId string) { |
| 151 | + require.NoError(t, db.Create(&ticket.Incident{ |
| 152 | + DomainEntity: domainlayer.DomainEntity{Id: id}, |
| 153 | + ScopeId: boardId, |
| 154 | + Table: "boards", |
| 155 | + })) |
| 156 | +} |
| 157 | + |
| 158 | +func seedDoraBoardIssue(t *testing.T, db dal.Dal, boardId, issueId, projectName string) { |
| 159 | + require.NoError(t, db.Create(&ticket.BoardIssue{ |
| 160 | + BoardId: boardId, |
| 161 | + IssueId: issueId, |
| 162 | + ProjectName: projectName, |
| 163 | + })) |
| 164 | +} |
| 165 | + |
| 166 | +func seedDoraBoardIssueNull(t *testing.T, db dal.Dal, boardId, issueId string) { |
| 167 | + // Insert a row whose project_name is SQL NULL (the leftover unfiltered pool), |
| 168 | + // which is distinct from the empty-string case. |
| 169 | + require.NoError(t, db.Exec( |
| 170 | + "INSERT INTO board_issues (board_id, issue_id, project_name) VALUES (?, ?, NULL)", |
| 171 | + boardId, issueId, |
| 172 | + )) |
| 173 | +} |
| 174 | + |
| 175 | +func seedDoraProjectMapping(t *testing.T, db dal.Dal, projectName, table, rowId string) { |
| 176 | + require.NoError(t, db.Create(&crossdomain.ProjectMapping{ |
| 177 | + ProjectName: projectName, |
| 178 | + Table: table, |
| 179 | + RowId: rowId, |
| 180 | + })) |
| 181 | +} |
| 182 | + |
| 183 | +func queryConnectIncidentIds(t *testing.T, db dal.Dal, projectName string) []string { |
| 184 | + cursor, err := db.Cursor(connectIncidentInScopeClauses(db, projectName)...) |
| 185 | + require.NoError(t, err) |
| 186 | + defer cursor.Close() |
| 187 | + |
| 188 | + var ids []string |
| 189 | + for cursor.Next() { |
| 190 | + row := &ticket.Incident{} |
| 191 | + require.NoError(t, db.Fetch(cursor, row)) |
| 192 | + ids = append(ids, row.Id) |
| 193 | + } |
| 194 | + sort.Strings(ids) |
| 195 | + return ids |
| 196 | +} |
0 commit comments