Skip to content

Commit db538d3

Browse files
committed
fix(jira): add missing _raw_data_* columns to _tool_jira_sprint_reports
The Sprint Report migration 20260722_add_sprint_report_table.go creates _tool_jira_sprint_reports from a struct that does not embed common.NoPKModel, while the runtime model models.JiraSprintReport does. The columns _raw_data_params / _raw_data_table / _raw_data_id / _raw_data_remark (plus created_at, updated_at) were therefore never created, so the ApiExtractor cleanup query WHERE _raw_data_table = ? AND _raw_data_params = ? made the extractSprintReport subtask fail with "Error 1054 (42S22): Unknown column '_raw_data_table' in 'where clause'". Add a new, additive migration that re-runs AutoMigrateTables on a struct embedding archived.NoPKModel. The original migration is left untouched: migration scripts are append-only, and editing it would not repair databases that already recorded its version. Add two schema-drift regression guards that run the REAL migration scripts instead of AutoMigrate-ing the runtime model, which would hide this class of drift: * plugins/jira/e2e/migration_schema_test.go - Jira-specific guard. * plugins/schema_e2e/migration_schema_test.go - cross-plugin guard for every built-in Go plugin, including TestAllGoPluginsListed so the guard stays complete when a new plugin is added. Both guards run the migrations against a dedicated, empty database created by the new helper e2ehelper.NewIsolatedMigrationDb: the shared e2e database is polluted by the other e2e tests, which AutoMigrate tables without recording anything in _devlake_migration_history, so running the real scripts against it fails with errors such as "Table 'cicd_pipeline_commits' already exists". The cross-plugin guard immediately uncovered three pre-existing drifts of the same class, each fixed with its own additive migration: * _tool_taiga_scope_configs - missing type_mappings * _tool_teambition_scope_configs - missing id, created_at, updated_at * _tool_testmo_scope_configs - missing connection_id, name The teambition table has no primary key at all, and its missing `id` is an auto-increment primary key, which AutoMigrate cannot append to an existing table (MySQL: "Incorrect table definition; there can be only one auto column and it must be defined as a key"). That column is therefore added with explicit DDL, which also keeps the ids of existing rows and the sequence/counter in sync on both MySQL and PostgreSQL. Finally, exclude plugins/schema_e2e from scripts/build-plugins.sh: it is not a plugin and has no main package, which broke `make build-plugin` with "-buildmode=plugin requires exactly one main package". Signed-off-by: DoDiODev <DoDiDev@proton.me>
1 parent 7f7cf09 commit db538d3

12 files changed

Lines changed: 757 additions & 1 deletion

File tree

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
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 e2ehelper
19+
20+
import (
21+
"fmt"
22+
"net/url"
23+
"strings"
24+
"testing"
25+
26+
"github.com/apache/incubator-devlake/core/config"
27+
"github.com/apache/incubator-devlake/core/plugin"
28+
"github.com/apache/incubator-devlake/core/runner"
29+
"github.com/apache/incubator-devlake/impls/dalgorm"
30+
"github.com/apache/incubator-devlake/impls/logruslog"
31+
"gorm.io/gorm"
32+
)
33+
34+
// NewIsolatedMigrationDb creates a dedicated, empty database next to the one
35+
// referenced by E2E_DB_URL (`<e2e-db>_<suffix>`) and returns a connection to it.
36+
//
37+
// Tests that execute the REAL migration scripts must not share the regular e2e
38+
// database: the other plugin e2e tests seed/AutoMigrate tables (domain layer
39+
// included) without recording anything in `_devlake_migration_history`, so a
40+
// subsequent migration run fails with errors such as
41+
// "Table 'cicd_pipeline_commits' already exists" when it tries to create or
42+
// rename a table that is already there.
43+
//
44+
// The database is dropped again when the test finishes. If E2E_DB_URL is not
45+
// set the test is skipped.
46+
func NewIsolatedMigrationDb(t *testing.T, suffix string) *gorm.DB {
47+
cfg := config.GetConfig()
48+
e2eDbUrl := cfg.GetString("E2E_DB_URL")
49+
if e2eDbUrl == "" {
50+
t.Skip("E2E_DB_URL is not set; skipping migration schema check")
51+
}
52+
u, err := url.Parse(e2eDbUrl)
53+
if err != nil {
54+
t.Fatalf("unable to parse E2E_DB_URL: %v", err)
55+
}
56+
isolatedName := fmt.Sprintf("%s_%s", strings.TrimPrefix(u.Path, "/"), suffix)
57+
quotedName := quoteDbName(u.Scheme, isolatedName)
58+
59+
gormConf := &gorm.Config{SkipDefaultTransaction: true}
60+
adminDb, err := runner.MakeDbConnection(e2eDbUrl, gormConf)
61+
if err != nil {
62+
t.Fatalf("unable to connect to E2E_DB_URL: %v", err)
63+
}
64+
if err = adminDb.Exec("DROP DATABASE IF EXISTS " + quotedName).Error; err != nil {
65+
t.Fatalf("unable to drop leftover database %s: %v", isolatedName, err)
66+
}
67+
if err = adminDb.Exec("CREATE DATABASE " + quotedName).Error; err != nil {
68+
t.Fatalf("unable to create database %s: %v", isolatedName, err)
69+
}
70+
closeDb(adminDb)
71+
72+
isolatedUrl := *u
73+
isolatedUrl.Path = "/" + isolatedName
74+
db, err := runner.MakeDbConnection(isolatedUrl.String(), gormConf)
75+
if err != nil {
76+
t.Fatalf("unable to connect to %s: %v", isolatedName, err)
77+
}
78+
79+
// migration scripts and models read DB_URL from the global config, keep it
80+
// consistent with the connection we hand out and restore it afterwards.
81+
previousDbUrl := cfg.GetString("DB_URL")
82+
cfg.Set("DB_URL", isolatedUrl.String())
83+
84+
// Some migration scripts refuse to run without an encryption secret
85+
// (e.g. jira 20220716: "jira v0.11 invalid encKey"). CI does not
86+
// necessarily provide one, so fall back to a deterministic test value.
87+
// dalgorm.Init registers the `encdec` GORM serializer used by connection
88+
// models - without it migrations fail with "invalid serializer type encdec"
89+
// (runner.CreateBasicRes does not register it, only CreateAppBasicRes does).
90+
if cfg.GetString(plugin.EncodeKeyEnvStr) == "" {
91+
cfg.Set(plugin.EncodeKeyEnvStr, "devlake-e2e-test-encryption-secret")
92+
}
93+
dalgorm.Init(cfg.GetString(plugin.EncodeKeyEnvStr))
94+
95+
t.Cleanup(func() {
96+
cfg.Set("DB_URL", previousDbUrl)
97+
closeDb(db)
98+
cleanupDb, cleanupErr := runner.MakeDbConnection(e2eDbUrl, gormConf)
99+
if cleanupErr != nil {
100+
t.Logf("unable to connect for dropping %s: %v", isolatedName, cleanupErr)
101+
return
102+
}
103+
defer closeDb(cleanupDb)
104+
if dropErr := cleanupDb.Exec("DROP DATABASE IF EXISTS " + quotedName).Error; dropErr != nil {
105+
t.Logf("unable to drop database %s: %v", isolatedName, dropErr)
106+
}
107+
})
108+
109+
logruslog.Global.Info("running migrations against isolated database %s", isolatedName)
110+
return db
111+
}
112+
113+
func quoteDbName(scheme string, name string) string {
114+
// database names are derived from E2E_DB_URL + a constant suffix, but quote
115+
// them anyway to stay safe with reserved words.
116+
if strings.EqualFold(scheme, "mysql") {
117+
return "`" + strings.ReplaceAll(name, "`", "") + "`"
118+
}
119+
return `"` + strings.ReplaceAll(name, `"`, "") + `"`
120+
}
121+
122+
func closeDb(db *gorm.DB) {
123+
if sqlDb, err := db.DB(); err == nil {
124+
_ = sqlDb.Close()
125+
}
126+
}
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
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+
"sync"
22+
"testing"
23+
24+
"github.com/apache/incubator-devlake/core/config"
25+
"github.com/apache/incubator-devlake/core/dal"
26+
"github.com/apache/incubator-devlake/core/migration"
27+
coreMigration "github.com/apache/incubator-devlake/core/models/migrationscripts"
28+
"github.com/apache/incubator-devlake/core/runner"
29+
"github.com/apache/incubator-devlake/helpers/e2ehelper"
30+
"github.com/apache/incubator-devlake/impls/dalgorm"
31+
"github.com/apache/incubator-devlake/impls/logruslog"
32+
"github.com/apache/incubator-devlake/plugins/jira/impl"
33+
"github.com/stretchr/testify/assert"
34+
"github.com/stretchr/testify/require"
35+
"gorm.io/gorm/schema"
36+
)
37+
38+
// TestMigrationSchema guards against schema drift between Jira's migration
39+
// scripts and its runtime GORM models.
40+
//
41+
// Regression test for the Sprint Report bug (introduced by PR #8967/#9010):
42+
// the migration that created `_tool_jira_sprint_reports` used a struct that did
43+
// NOT embed common.NoPKModel, so the `_raw_data_table` / `_raw_data_params` /
44+
// `_raw_data_id` / `_raw_data_remark` columns were missing. The runtime model
45+
// DID embed common.NoPKModel, so the ApiExtractor's cleanup query
46+
// (`WHERE _raw_data_table = ? AND _raw_data_params = ?`) failed at runtime with
47+
// "Error 1054: Unknown column '_raw_data_table' in 'where clause'".
48+
//
49+
// The test runs the REAL migration scripts (framework + jira) to build the
50+
// schema exactly the way a production install would — deliberately NOT via
51+
// AutoMigrate on the runtime model, which would silently hide such drift — and
52+
// then asserts that every column each runtime model expects actually exists in
53+
// the migrated table. Any future migration that forgets to embed
54+
// common.NoPKModel (or otherwise omits a column) will fail this test.
55+
//
56+
// The migrations run against a dedicated, empty database (see
57+
// e2ehelper.NewIsolatedMigrationDb) because the shared e2e database is polluted
58+
// by the other e2e tests, which AutoMigrate tables without recording anything
59+
// in `_devlake_migration_history`.
60+
//
61+
// Requires E2E_DB_URL (runs under `make e2e-test` / `make e2e-test-go-plugins`).
62+
func TestMigrationSchema(t *testing.T) {
63+
var pluginInstance impl.Jira
64+
65+
db := e2ehelper.NewIsolatedMigrationDb(t, "jira_migration_schema")
66+
dalInstance := dalgorm.NewDalgorm(db)
67+
68+
// Apply the real migration scripts so the schema matches a production install.
69+
basicRes := runner.CreateBasicRes(config.GetConfig(), logruslog.Global, db)
70+
migrator, err := migration.NewMigrator(basicRes)
71+
require.NoError(t, err)
72+
migrator.Register(coreMigration.All(), "Framework")
73+
migrator.Register(pluginInstance.MigrationScripts(), "jira")
74+
require.NoError(t, migrator.Execute())
75+
76+
keepAll := func(dal.ColumnMeta) bool { return true }
77+
78+
for _, table := range pluginInstance.GetTablesInfo() {
79+
table := table
80+
t.Run(table.TableName(), func(t *testing.T) {
81+
// Columns the runtime GORM model expects.
82+
sch, err := schema.Parse(table, &sync.Map{}, schema.NamingStrategy{})
83+
require.NoErrorf(t, err, "unable to parse schema for %T", table)
84+
85+
// Columns that actually exist in the migrated table.
86+
actualColumns, err := dal.GetColumnNames(dalInstance, table, keepAll)
87+
if err != nil || len(actualColumns) == 0 {
88+
// No migration creates this table (e.g. API response models
89+
// that are listed in GetTablesInfo but never persisted) —
90+
// there is no schema to drift from.
91+
t.Skipf("table %q not present after migrations", table.TableName())
92+
}
93+
existing := make(map[string]struct{}, len(actualColumns))
94+
for _, c := range actualColumns {
95+
existing[c] = struct{}{}
96+
}
97+
98+
for _, field := range sch.Fields {
99+
if field.DBName == "" || field.IgnoreMigration {
100+
continue
101+
}
102+
_, ok := existing[field.DBName]
103+
assert.Truef(t, ok,
104+
"table %q is missing column %q expected by model %T — "+
105+
"did a migration script forget to embed common.NoPKModel (raw-data columns) or add the field?",
106+
table.TableName(), field.DBName, table)
107+
}
108+
})
109+
}
110+
}
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 migrationscripts
19+
20+
import (
21+
"github.com/apache/incubator-devlake/core/context"
22+
"github.com/apache/incubator-devlake/core/errors"
23+
"github.com/apache/incubator-devlake/core/models/migrationscripts/archived"
24+
"github.com/apache/incubator-devlake/helpers/migrationhelper"
25+
)
26+
27+
// jiraSprintReport20260727 mirrors the JiraSprintReport model. The original
28+
// migration (20260722) created _tool_jira_sprint_reports without embedding
29+
// common.NoPKModel, so the _raw_data_table / _raw_data_params / _raw_data_id /
30+
// _raw_data_remark columns (and created_at / updated_at) were missing. The
31+
// runtime model expects them, which made the ApiExtractor's cleanup query
32+
// (WHERE _raw_data_table = ? AND _raw_data_params = ?) fail with
33+
// "Unknown column '_raw_data_table' in 'where clause'". Re-running
34+
// AutoMigrateTables adds the missing columns without dropping existing data.
35+
type jiraSprintReport20260727 struct {
36+
archived.NoPKModel
37+
ConnectionId uint64 `gorm:"primaryKey"`
38+
BoardId uint64 `gorm:"primaryKey"`
39+
SprintId uint64 `gorm:"primaryKey"`
40+
IssueId uint64 `gorm:"primaryKey"`
41+
42+
IssueKey string `gorm:"type:varchar(255)"`
43+
Bucket string `gorm:"type:varchar(32);index"`
44+
Done bool
45+
StoryPointsAtSprintStart *float64
46+
StoryPointsAtSprintEnd *float64
47+
}
48+
49+
func (jiraSprintReport20260727) TableName() string {
50+
return "_tool_jira_sprint_reports"
51+
}
52+
53+
type addRawDataColumnsToSprintReport struct{}
54+
55+
func (script *addRawDataColumnsToSprintReport) Up(basicRes context.BasicRes) errors.Error {
56+
return migrationhelper.AutoMigrateTables(basicRes, &jiraSprintReport20260727{})
57+
}
58+
59+
func (*addRawDataColumnsToSprintReport) Version() uint64 {
60+
return 20260727000000
61+
}
62+
63+
func (*addRawDataColumnsToSprintReport) Name() string {
64+
return "add missing _raw_data_* columns to _tool_jira_sprint_reports"
65+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,5 +59,6 @@ func All() []plugin.MigrationScript {
5959
new(changeFixVersionsToText20260707),
6060
new(addExtraJQLToScopeConfig),
6161
new(addSprintReportTable),
62+
new(addRawDataColumnsToSprintReport),
6263
}
6364
}

0 commit comments

Comments
 (0)