From db538d3090411f93cebf1531b74775d652692c94 Mon Sep 17 00:00:00 2001 From: DoDiODev Date: Tue, 28 Jul 2026 10:19:23 +0200 Subject: [PATCH 1/3] 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 --- backend/helpers/e2ehelper/migration_db.go | 126 ++++++++++ .../plugins/jira/e2e/migration_schema_test.go | 110 ++++++++ ...7_add_raw_data_columns_to_sprint_report.go | 65 +++++ .../jira/models/migrationscripts/register.go | 1 + .../schema_e2e/migration_schema_test.go | 238 ++++++++++++++++++ ...260727_add_missing_scope_config_columns.go | 61 +++++ .../taiga/models/migrationscripts/register.go | 1 + ...260727_add_missing_scope_config_columns.go | 90 +++++++ .../models/migrationscripts/register.go | 1 + ...260727_add_missing_scope_config_columns.go | 61 +++++ .../models/migrationscripts/register.go | 1 + backend/scripts/build-plugins.sh | 3 +- 12 files changed, 757 insertions(+), 1 deletion(-) create mode 100644 backend/helpers/e2ehelper/migration_db.go create mode 100644 backend/plugins/jira/e2e/migration_schema_test.go create mode 100644 backend/plugins/jira/models/migrationscripts/20260727_add_raw_data_columns_to_sprint_report.go create mode 100644 backend/plugins/schema_e2e/migration_schema_test.go create mode 100644 backend/plugins/taiga/models/migrationscripts/20260727_add_missing_scope_config_columns.go create mode 100644 backend/plugins/teambition/models/migrationscripts/20260727_add_missing_scope_config_columns.go create mode 100644 backend/plugins/testmo/models/migrationscripts/20260727_add_missing_scope_config_columns.go diff --git a/backend/helpers/e2ehelper/migration_db.go b/backend/helpers/e2ehelper/migration_db.go new file mode 100644 index 00000000000..3d7fe2d49e5 --- /dev/null +++ b/backend/helpers/e2ehelper/migration_db.go @@ -0,0 +1,126 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2ehelper + +import ( + "fmt" + "net/url" + "strings" + "testing" + + "github.com/apache/incubator-devlake/core/config" + "github.com/apache/incubator-devlake/core/plugin" + "github.com/apache/incubator-devlake/core/runner" + "github.com/apache/incubator-devlake/impls/dalgorm" + "github.com/apache/incubator-devlake/impls/logruslog" + "gorm.io/gorm" +) + +// NewIsolatedMigrationDb creates a dedicated, empty database next to the one +// referenced by E2E_DB_URL (`_`) and returns a connection to it. +// +// Tests that execute the REAL migration scripts must not share the regular e2e +// database: the other plugin e2e tests seed/AutoMigrate tables (domain layer +// included) without recording anything in `_devlake_migration_history`, so a +// subsequent migration run fails with errors such as +// "Table 'cicd_pipeline_commits' already exists" when it tries to create or +// rename a table that is already there. +// +// The database is dropped again when the test finishes. If E2E_DB_URL is not +// set the test is skipped. +func NewIsolatedMigrationDb(t *testing.T, suffix string) *gorm.DB { + cfg := config.GetConfig() + e2eDbUrl := cfg.GetString("E2E_DB_URL") + if e2eDbUrl == "" { + t.Skip("E2E_DB_URL is not set; skipping migration schema check") + } + u, err := url.Parse(e2eDbUrl) + if err != nil { + t.Fatalf("unable to parse E2E_DB_URL: %v", err) + } + isolatedName := fmt.Sprintf("%s_%s", strings.TrimPrefix(u.Path, "/"), suffix) + quotedName := quoteDbName(u.Scheme, isolatedName) + + gormConf := &gorm.Config{SkipDefaultTransaction: true} + adminDb, err := runner.MakeDbConnection(e2eDbUrl, gormConf) + if err != nil { + t.Fatalf("unable to connect to E2E_DB_URL: %v", err) + } + if err = adminDb.Exec("DROP DATABASE IF EXISTS " + quotedName).Error; err != nil { + t.Fatalf("unable to drop leftover database %s: %v", isolatedName, err) + } + if err = adminDb.Exec("CREATE DATABASE " + quotedName).Error; err != nil { + t.Fatalf("unable to create database %s: %v", isolatedName, err) + } + closeDb(adminDb) + + isolatedUrl := *u + isolatedUrl.Path = "/" + isolatedName + db, err := runner.MakeDbConnection(isolatedUrl.String(), gormConf) + if err != nil { + t.Fatalf("unable to connect to %s: %v", isolatedName, err) + } + + // migration scripts and models read DB_URL from the global config, keep it + // consistent with the connection we hand out and restore it afterwards. + previousDbUrl := cfg.GetString("DB_URL") + cfg.Set("DB_URL", isolatedUrl.String()) + + // Some migration scripts refuse to run without an encryption secret + // (e.g. jira 20220716: "jira v0.11 invalid encKey"). CI does not + // necessarily provide one, so fall back to a deterministic test value. + // dalgorm.Init registers the `encdec` GORM serializer used by connection + // models - without it migrations fail with "invalid serializer type encdec" + // (runner.CreateBasicRes does not register it, only CreateAppBasicRes does). + if cfg.GetString(plugin.EncodeKeyEnvStr) == "" { + cfg.Set(plugin.EncodeKeyEnvStr, "devlake-e2e-test-encryption-secret") + } + dalgorm.Init(cfg.GetString(plugin.EncodeKeyEnvStr)) + + t.Cleanup(func() { + cfg.Set("DB_URL", previousDbUrl) + closeDb(db) + cleanupDb, cleanupErr := runner.MakeDbConnection(e2eDbUrl, gormConf) + if cleanupErr != nil { + t.Logf("unable to connect for dropping %s: %v", isolatedName, cleanupErr) + return + } + defer closeDb(cleanupDb) + if dropErr := cleanupDb.Exec("DROP DATABASE IF EXISTS " + quotedName).Error; dropErr != nil { + t.Logf("unable to drop database %s: %v", isolatedName, dropErr) + } + }) + + logruslog.Global.Info("running migrations against isolated database %s", isolatedName) + return db +} + +func quoteDbName(scheme string, name string) string { + // database names are derived from E2E_DB_URL + a constant suffix, but quote + // them anyway to stay safe with reserved words. + if strings.EqualFold(scheme, "mysql") { + return "`" + strings.ReplaceAll(name, "`", "") + "`" + } + return `"` + strings.ReplaceAll(name, `"`, "") + `"` +} + +func closeDb(db *gorm.DB) { + if sqlDb, err := db.DB(); err == nil { + _ = sqlDb.Close() + } +} diff --git a/backend/plugins/jira/e2e/migration_schema_test.go b/backend/plugins/jira/e2e/migration_schema_test.go new file mode 100644 index 00000000000..619aee6e415 --- /dev/null +++ b/backend/plugins/jira/e2e/migration_schema_test.go @@ -0,0 +1,110 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "sync" + "testing" + + "github.com/apache/incubator-devlake/core/config" + "github.com/apache/incubator-devlake/core/dal" + "github.com/apache/incubator-devlake/core/migration" + coreMigration "github.com/apache/incubator-devlake/core/models/migrationscripts" + "github.com/apache/incubator-devlake/core/runner" + "github.com/apache/incubator-devlake/helpers/e2ehelper" + "github.com/apache/incubator-devlake/impls/dalgorm" + "github.com/apache/incubator-devlake/impls/logruslog" + "github.com/apache/incubator-devlake/plugins/jira/impl" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm/schema" +) + +// TestMigrationSchema guards against schema drift between Jira's migration +// scripts and its runtime GORM models. +// +// Regression test for the Sprint Report bug (introduced by PR #8967/#9010): +// the migration that created `_tool_jira_sprint_reports` used a struct that did +// NOT embed common.NoPKModel, so the `_raw_data_table` / `_raw_data_params` / +// `_raw_data_id` / `_raw_data_remark` columns were missing. The runtime model +// DID embed common.NoPKModel, so the ApiExtractor's cleanup query +// (`WHERE _raw_data_table = ? AND _raw_data_params = ?`) failed at runtime with +// "Error 1054: Unknown column '_raw_data_table' in 'where clause'". +// +// The test runs the REAL migration scripts (framework + jira) to build the +// schema exactly the way a production install would — deliberately NOT via +// AutoMigrate on the runtime model, which would silently hide such drift — and +// then asserts that every column each runtime model expects actually exists in +// the migrated table. Any future migration that forgets to embed +// common.NoPKModel (or otherwise omits a column) will fail this test. +// +// The migrations run against a dedicated, empty database (see +// e2ehelper.NewIsolatedMigrationDb) because the shared e2e database is polluted +// by the other e2e tests, which AutoMigrate tables without recording anything +// in `_devlake_migration_history`. +// +// Requires E2E_DB_URL (runs under `make e2e-test` / `make e2e-test-go-plugins`). +func TestMigrationSchema(t *testing.T) { + var pluginInstance impl.Jira + + db := e2ehelper.NewIsolatedMigrationDb(t, "jira_migration_schema") + dalInstance := dalgorm.NewDalgorm(db) + + // Apply the real migration scripts so the schema matches a production install. + basicRes := runner.CreateBasicRes(config.GetConfig(), logruslog.Global, db) + migrator, err := migration.NewMigrator(basicRes) + require.NoError(t, err) + migrator.Register(coreMigration.All(), "Framework") + migrator.Register(pluginInstance.MigrationScripts(), "jira") + require.NoError(t, migrator.Execute()) + + keepAll := func(dal.ColumnMeta) bool { return true } + + for _, table := range pluginInstance.GetTablesInfo() { + table := table + t.Run(table.TableName(), func(t *testing.T) { + // Columns the runtime GORM model expects. + sch, err := schema.Parse(table, &sync.Map{}, schema.NamingStrategy{}) + require.NoErrorf(t, err, "unable to parse schema for %T", table) + + // Columns that actually exist in the migrated table. + actualColumns, err := dal.GetColumnNames(dalInstance, table, keepAll) + if err != nil || len(actualColumns) == 0 { + // No migration creates this table (e.g. API response models + // that are listed in GetTablesInfo but never persisted) — + // there is no schema to drift from. + t.Skipf("table %q not present after migrations", table.TableName()) + } + existing := make(map[string]struct{}, len(actualColumns)) + for _, c := range actualColumns { + existing[c] = struct{}{} + } + + for _, field := range sch.Fields { + if field.DBName == "" || field.IgnoreMigration { + continue + } + _, ok := existing[field.DBName] + assert.Truef(t, ok, + "table %q is missing column %q expected by model %T — "+ + "did a migration script forget to embed common.NoPKModel (raw-data columns) or add the field?", + table.TableName(), field.DBName, table) + } + }) + } +} diff --git a/backend/plugins/jira/models/migrationscripts/20260727_add_raw_data_columns_to_sprint_report.go b/backend/plugins/jira/models/migrationscripts/20260727_add_raw_data_columns_to_sprint_report.go new file mode 100644 index 00000000000..5ee4d671f70 --- /dev/null +++ b/backend/plugins/jira/models/migrationscripts/20260727_add_raw_data_columns_to_sprint_report.go @@ -0,0 +1,65 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package migrationscripts + +import ( + "github.com/apache/incubator-devlake/core/context" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/models/migrationscripts/archived" + "github.com/apache/incubator-devlake/helpers/migrationhelper" +) + +// jiraSprintReport20260727 mirrors the JiraSprintReport model. The original +// migration (20260722) created _tool_jira_sprint_reports without embedding +// common.NoPKModel, so the _raw_data_table / _raw_data_params / _raw_data_id / +// _raw_data_remark columns (and created_at / updated_at) were missing. The +// runtime model expects them, which made the ApiExtractor's cleanup query +// (WHERE _raw_data_table = ? AND _raw_data_params = ?) fail with +// "Unknown column '_raw_data_table' in 'where clause'". Re-running +// AutoMigrateTables adds the missing columns without dropping existing data. +type jiraSprintReport20260727 struct { + archived.NoPKModel + ConnectionId uint64 `gorm:"primaryKey"` + BoardId uint64 `gorm:"primaryKey"` + SprintId uint64 `gorm:"primaryKey"` + IssueId uint64 `gorm:"primaryKey"` + + IssueKey string `gorm:"type:varchar(255)"` + Bucket string `gorm:"type:varchar(32);index"` + Done bool + StoryPointsAtSprintStart *float64 + StoryPointsAtSprintEnd *float64 +} + +func (jiraSprintReport20260727) TableName() string { + return "_tool_jira_sprint_reports" +} + +type addRawDataColumnsToSprintReport struct{} + +func (script *addRawDataColumnsToSprintReport) Up(basicRes context.BasicRes) errors.Error { + return migrationhelper.AutoMigrateTables(basicRes, &jiraSprintReport20260727{}) +} + +func (*addRawDataColumnsToSprintReport) Version() uint64 { + return 20260727000000 +} + +func (*addRawDataColumnsToSprintReport) Name() string { + return "add missing _raw_data_* columns to _tool_jira_sprint_reports" +} diff --git a/backend/plugins/jira/models/migrationscripts/register.go b/backend/plugins/jira/models/migrationscripts/register.go index 90a3317bd14..e71dee8ea14 100644 --- a/backend/plugins/jira/models/migrationscripts/register.go +++ b/backend/plugins/jira/models/migrationscripts/register.go @@ -59,5 +59,6 @@ func All() []plugin.MigrationScript { new(changeFixVersionsToText20260707), new(addExtraJQLToScopeConfig), new(addSprintReportTable), + new(addRawDataColumnsToSprintReport), } } diff --git a/backend/plugins/schema_e2e/migration_schema_test.go b/backend/plugins/schema_e2e/migration_schema_test.go new file mode 100644 index 00000000000..f1572b4de85 --- /dev/null +++ b/backend/plugins/schema_e2e/migration_schema_test.go @@ -0,0 +1,238 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package schema_e2e contains a cross-plugin regression guard that runs the +// REAL migration scripts of every built-in Go plugin and then asserts that the +// resulting database schema still matches what each runtime GORM model expects. +// +// It lives in an `e2e` package on purpose: it needs a real database +// (E2E_DB_URL) and is therefore only executed by `make e2e-test-go-plugins` +// (scripts/e2e-test-go-plugins.sh selects packages whose import path contains +// "e2e"), and excluded from the DB-less unit test run +// (scripts/unit-test-go.sh skips paths matching "e2e"). +package schema_e2e + +import ( + "os" + "path/filepath" + "sync" + "testing" + + "github.com/apache/incubator-devlake/core/config" + "github.com/apache/incubator-devlake/core/dal" + "github.com/apache/incubator-devlake/core/migration" + coreMigration "github.com/apache/incubator-devlake/core/models/migrationscripts" + "github.com/apache/incubator-devlake/core/plugin" + "github.com/apache/incubator-devlake/core/runner" + "github.com/apache/incubator-devlake/helpers/e2ehelper" + "github.com/apache/incubator-devlake/impls/dalgorm" + "github.com/apache/incubator-devlake/impls/logruslog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm/schema" + + ae "github.com/apache/incubator-devlake/plugins/ae/impl" + argocd "github.com/apache/incubator-devlake/plugins/argocd/impl" + asana "github.com/apache/incubator-devlake/plugins/asana/impl" + azuredevops "github.com/apache/incubator-devlake/plugins/azuredevops_go/impl" + bamboo "github.com/apache/incubator-devlake/plugins/bamboo/impl" + bitbucket "github.com/apache/incubator-devlake/plugins/bitbucket/impl" + bitbucket_server "github.com/apache/incubator-devlake/plugins/bitbucket_server/impl" + circleci "github.com/apache/incubator-devlake/plugins/circleci/impl" + claudeCode "github.com/apache/incubator-devlake/plugins/claude_code/impl" + customize "github.com/apache/incubator-devlake/plugins/customize/impl" + dbt "github.com/apache/incubator-devlake/plugins/dbt/impl" + dora "github.com/apache/incubator-devlake/plugins/dora/impl" + feishu "github.com/apache/incubator-devlake/plugins/feishu/impl" + copilot "github.com/apache/incubator-devlake/plugins/gh-copilot/impl" + gitee "github.com/apache/incubator-devlake/plugins/gitee/impl" + gitextractor "github.com/apache/incubator-devlake/plugins/gitextractor/impl" + github "github.com/apache/incubator-devlake/plugins/github/impl" + githubGraphql "github.com/apache/incubator-devlake/plugins/github_graphql/impl" + gitlab "github.com/apache/incubator-devlake/plugins/gitlab/impl" + icla "github.com/apache/incubator-devlake/plugins/icla/impl" + issueTrace "github.com/apache/incubator-devlake/plugins/issue_trace/impl" + jenkins "github.com/apache/incubator-devlake/plugins/jenkins/impl" + jira "github.com/apache/incubator-devlake/plugins/jira/impl" + linear "github.com/apache/incubator-devlake/plugins/linear/impl" + linker "github.com/apache/incubator-devlake/plugins/linker/impl" + opsgenie "github.com/apache/incubator-devlake/plugins/opsgenie/impl" + org "github.com/apache/incubator-devlake/plugins/org/impl" + pagerduty "github.com/apache/incubator-devlake/plugins/pagerduty/impl" + q_dev "github.com/apache/incubator-devlake/plugins/q_dev/impl" + refdiff "github.com/apache/incubator-devlake/plugins/refdiff/impl" + rootly "github.com/apache/incubator-devlake/plugins/rootly/impl" + slack "github.com/apache/incubator-devlake/plugins/slack/impl" + sonarqube "github.com/apache/incubator-devlake/plugins/sonarqube/impl" + starrocks "github.com/apache/incubator-devlake/plugins/starrocks/impl" + taiga "github.com/apache/incubator-devlake/plugins/taiga/impl" + tapd "github.com/apache/incubator-devlake/plugins/tapd/impl" + teambition "github.com/apache/incubator-devlake/plugins/teambition/impl" + tempo "github.com/apache/incubator-devlake/plugins/tempo/impl" + testmo "github.com/apache/incubator-devlake/plugins/testmo/impl" + trello "github.com/apache/incubator-devlake/plugins/trello/impl" + webhook "github.com/apache/incubator-devlake/plugins/webhook/impl" + zentao "github.com/apache/incubator-devlake/plugins/zentao/impl" +) + +// allGoPlugins lists EVERY built-in Go plugin. Keep it in sync with the plugin +// directories under backend/plugins/ (the TestAllGoPluginsListed guard below +// fails if a new plugin's `impl` package is added but not registered here). +func allGoPlugins() []plugin.PluginMeta { + return []plugin.PluginMeta{ + ae.AE{}, + argocd.ArgoCD{}, + asana.Asana{}, + azuredevops.Azuredevops{}, + bamboo.Bamboo{}, + bitbucket.Bitbucket{}, + bitbucket_server.BitbucketServer{}, + circleci.Circleci{}, + claudeCode.ClaudeCode{}, + customize.Customize{}, + dbt.Dbt{}, + dora.Dora{}, + feishu.Feishu{}, + copilot.GhCopilot{}, + gitee.Gitee{}, + gitextractor.GitExtractor{}, + github.Github{}, + githubGraphql.GithubGraphql{}, + gitlab.Gitlab{}, + icla.Icla{}, + issueTrace.IssueTrace{}, + jenkins.Jenkins{}, + jira.Jira{}, + linear.Linear{}, + linker.Linker{}, + opsgenie.Opsgenie{}, + org.Org{}, + pagerduty.PagerDuty{}, + q_dev.QDev{}, + refdiff.RefDiff{}, + rootly.Rootly{}, + slack.Slack{}, + sonarqube.Sonarqube{}, + starrocks.StarRocks{}, + taiga.Taiga{}, + tapd.Tapd{}, + teambition.Teambition{}, + tempo.Tempo{}, + testmo.Testmo{}, + trello.Trello{}, + webhook.Webhook{}, + zentao.Zentao{}, + } +} + +// TestAllGoPluginsListed guarantees allGoPlugins() stays complete: it counts the +// plugin directories that ship an `impl` package and fails if that number does +// not match the registered list. This makes the schema-drift guard below +// automatically cover any newly added plugin. +func TestAllGoPluginsListed(t *testing.T) { + entries, err := os.ReadDir("..") + require.NoError(t, err) + dirsWithImpl := 0 + for _, e := range entries { + if !e.IsDir() { + continue + } + if info, statErr := os.Stat(filepath.Join("..", e.Name(), "impl")); statErr == nil && info.IsDir() { + dirsWithImpl++ + } + } + assert.Equalf(t, dirsWithImpl, len(allGoPlugins()), + "number of plugin dirs with an impl/ package (%d) != registered plugins (%d); "+ + "add the new plugin to allGoPlugins() in plugins/schema_e2e/migration_schema_test.go", + dirsWithImpl, len(allGoPlugins())) +} + +// TestMigrationSchemaMatchesModels applies the real framework + plugin migration +// scripts and then verifies, for every plugin model, that each column the +// runtime GORM model declares actually exists in the migrated table. +// +// This is a cross-plugin generalization of the Jira Sprint Report regression: +// a migration created `_tool_jira_sprint_reports` without embedding +// common.NoPKModel, so the `_raw_data_*` columns were missing and the +// ApiExtractor cleanup query failed at runtime with +// "Unknown column '_raw_data_table' in 'where clause'". +// +// Tables that no migration creates (e.g. models materialized lazily at runtime) +// are skipped, so the check specifically targets *drift* between an existing +// table and its model — which is exactly the failure mode above. +// +// The migrations run against a dedicated, empty database (see +// e2ehelper.NewIsolatedMigrationDb) because the shared e2e database is polluted +// by the other e2e tests, which AutoMigrate tables without recording anything +// in `_devlake_migration_history`. +func TestMigrationSchemaMatchesModels(t *testing.T) { + db := e2ehelper.NewIsolatedMigrationDb(t, "schema_drift") + dalInstance := dalgorm.NewDalgorm(db) + basicRes := runner.CreateBasicRes(config.GetConfig(), logruslog.Global, db) + + // Apply the migrations exactly the way the server does on startup. + migrator, migErr := migration.NewMigrator(basicRes) + require.NoError(t, migErr) + migrator.Register(coreMigration.All(), "Framework") + for _, p := range allGoPlugins() { + if migratable, ok := p.(plugin.PluginMigration); ok { + migrator.Register(migratable.MigrationScripts(), p.Name()) + } + } + require.NoError(t, migrator.Execute()) + + keepAll := func(dal.ColumnMeta) bool { return true } + + for _, p := range allGoPlugins() { + modeler, ok := p.(plugin.PluginModel) + if !ok { + continue + } + p := p + t.Run(p.Name(), func(t *testing.T) { + for _, table := range modeler.GetTablesInfo() { + table := table + // Columns that actually exist in the migrated table. + actualColumns, colErr := dal.GetColumnNames(dalInstance, table, keepAll) + if colErr != nil || len(actualColumns) == 0 { + // No migration created this table (e.g. runtime-only / + // dynamic model) — nothing to validate for drift. + t.Logf("skip %q: table not present after migrations", table.TableName()) + continue + } + existing := make(map[string]struct{}, len(actualColumns)) + for _, c := range actualColumns { + existing[c] = struct{}{} + } + + // Columns the runtime GORM model expects. + sch, parseErr := schema.Parse(table, &sync.Map{}, schema.NamingStrategy{}) + require.NoErrorf(t, parseErr, "unable to parse schema for %T", table) + for _, field := range sch.Fields { + if field.DBName == "" || field.IgnoreMigration { + continue + } + _, present := existing[field.DBName] + assert.Truef(t, present, + "[%s] table %q is missing column %q expected by model %T — "+ + "did a migration script forget to embed common.NoPKModel (raw-data columns) or add the field?", + p.Name(), table.TableName(), field.DBName, table) + } + } + }) + } +} diff --git a/backend/plugins/taiga/models/migrationscripts/20260727_add_missing_scope_config_columns.go b/backend/plugins/taiga/models/migrationscripts/20260727_add_missing_scope_config_columns.go new file mode 100644 index 00000000000..84fa66cda41 --- /dev/null +++ b/backend/plugins/taiga/models/migrationscripts/20260727_add_missing_scope_config_columns.go @@ -0,0 +1,61 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package migrationscripts + +import ( + "encoding/json" + + "github.com/apache/incubator-devlake/core/context" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/models/migrationscripts/archived" + "github.com/apache/incubator-devlake/helpers/migrationhelper" +) + +// taigaScopeConfig20260727 mirrors models.TaigaScopeConfig. The initial +// migration created `_tool_taiga_scope_configs` without the `type_mappings` +// column, while the runtime model declares it — every read/write of the model +// would fail with "Unknown column 'type_mappings'". +// +// The `uniqueIndex` on `name` is safe to add here: the new column is nullable, +// so pre-existing rows are backfilled with NULL, and both MySQL and PostgreSQL +// allow duplicate NULLs in a unique index (verified against both engines). +type taigaScopeConfig20260727 struct { + archived.Model + Entities []string `gorm:"type:json;serializer:json" json:"entities"` + ConnectionId uint64 `json:"connectionId" gorm:"index"` + Name string `json:"name" gorm:"type:varchar(255);uniqueIndex"` + TypeMappings map[string]json.RawMessage `json:"typeMappings" gorm:"type:json;serializer:json"` +} + +func (taigaScopeConfig20260727) TableName() string { + return "_tool_taiga_scope_configs" +} + +type addMissingScopeConfigColumns struct{} + +func (script *addMissingScopeConfigColumns) Up(basicRes context.BasicRes) errors.Error { + return migrationhelper.AutoMigrateTables(basicRes, &taigaScopeConfig20260727{}) +} + +func (*addMissingScopeConfigColumns) Version() uint64 { + return 20260727000001 +} + +func (*addMissingScopeConfigColumns) Name() string { + return "add missing type_mappings column to _tool_taiga_scope_configs" +} diff --git a/backend/plugins/taiga/models/migrationscripts/register.go b/backend/plugins/taiga/models/migrationscripts/register.go index d2cfd08e269..da91884aa1f 100644 --- a/backend/plugins/taiga/models/migrationscripts/register.go +++ b/backend/plugins/taiga/models/migrationscripts/register.go @@ -26,5 +26,6 @@ func All() []plugin.MigrationScript { return []plugin.MigrationScript{ new(addInitTables20250220), new(addTaskIssueEpicTables20260306), + new(addMissingScopeConfigColumns), } } diff --git a/backend/plugins/teambition/models/migrationscripts/20260727_add_missing_scope_config_columns.go b/backend/plugins/teambition/models/migrationscripts/20260727_add_missing_scope_config_columns.go new file mode 100644 index 00000000000..0b29b9ca044 --- /dev/null +++ b/backend/plugins/teambition/models/migrationscripts/20260727_add_missing_scope_config_columns.go @@ -0,0 +1,90 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package migrationscripts + +import ( + "fmt" + + "github.com/apache/incubator-devlake/core/context" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/models/migrationscripts/archived" + "github.com/apache/incubator-devlake/helpers/migrationhelper" +) + +const teambitionScopeConfigTable20260727 = "_tool_teambition_scope_configs" + +// teambitionScopeConfig20260727 mirrors models.TeambitionScopeConfig. The +// migration that created `_tool_teambition_scope_configs` did not include the +// columns of the embedded common.Model (`id`, `created_at`, `updated_at`), +// which the runtime model expects. +type teambitionScopeConfig20260727 struct { + archived.Model + Entities []string `gorm:"type:json;serializer:json" json:"entities"` + ConnectionId uint64 `json:"connectionId" gorm:"index"` + Name string `json:"name" gorm:"type:varchar(255);uniqueIndex"` + TypeMappings map[string]string `json:"typeMappings" gorm:"serializer:json"` + StatusMappings map[string]string `json:"statusMappings" gorm:"serializer:json"` + BugDueDateField string `json:"bugDueDateField" gorm:"column:bug_due_date_field"` + TaskDueDateField string `json:"taskDueDateField" gorm:"column:task_due_date_field"` + StoryDueDateField string `json:"storyDueDateField" gorm:"column:story_due_date_field"` +} + +func (teambitionScopeConfig20260727) TableName() string { + return teambitionScopeConfigTable20260727 +} + +type addMissingScopeConfigColumns struct{} + +// Up adds the columns of the embedded common.Model that the runtime model +// expects. +// +// `id` is an auto-increment primary key, which GORM's AutoMigrate cannot append +// to an existing table: it emits a plain `ADD COLUMN ... AUTO_INCREMENT`, which +// MySQL rejects with "Incorrect table definition; there can be only one auto +// column and it must be defined as a key". The column is therefore added with +// explicit DDL (the table has no primary key so far), letting the database +// backfill ids for existing rows and keep the sequence/counter in sync. The +// remaining columns (`created_at`, `updated_at`) and the indexes are then +// created by AutoMigrate as usual. +func (script *addMissingScopeConfigColumns) Up(basicRes context.BasicRes) errors.Error { + db := basicRes.GetDal() + if !db.HasColumn(teambitionScopeConfigTable20260727, "id") { + ddl := fmt.Sprintf( + "ALTER TABLE %s ADD COLUMN id BIGSERIAL PRIMARY KEY", + teambitionScopeConfigTable20260727, + ) + if db.Dialect() == "mysql" { + ddl = fmt.Sprintf( + "ALTER TABLE %s ADD COLUMN id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY", + teambitionScopeConfigTable20260727, + ) + } + if err := db.Exec(ddl); err != nil { + return err + } + } + return migrationhelper.AutoMigrateTables(basicRes, &teambitionScopeConfig20260727{}) +} + +func (*addMissingScopeConfigColumns) Version() uint64 { + return 20260727000001 +} + +func (*addMissingScopeConfigColumns) Name() string { + return "add missing id/created_at/updated_at columns to _tool_teambition_scope_configs" +} diff --git a/backend/plugins/teambition/models/migrationscripts/register.go b/backend/plugins/teambition/models/migrationscripts/register.go index f9914e7a12c..d761a15fb70 100644 --- a/backend/plugins/teambition/models/migrationscripts/register.go +++ b/backend/plugins/teambition/models/migrationscripts/register.go @@ -26,5 +26,6 @@ func All() []plugin.MigrationScript { new(reCreateTeambitionConnections), new(addScopeConfigId), new(addAppIdBack), + new(addMissingScopeConfigColumns), } } diff --git a/backend/plugins/testmo/models/migrationscripts/20260727_add_missing_scope_config_columns.go b/backend/plugins/testmo/models/migrationscripts/20260727_add_missing_scope_config_columns.go new file mode 100644 index 00000000000..11048f60a74 --- /dev/null +++ b/backend/plugins/testmo/models/migrationscripts/20260727_add_missing_scope_config_columns.go @@ -0,0 +1,61 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package migrationscripts + +import ( + "github.com/apache/incubator-devlake/core/context" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/models/migrationscripts/archived" + "github.com/apache/incubator-devlake/helpers/migrationhelper" +) + +// testmoScopeConfig20260727 mirrors models.TestmoScopeConfig. The migration +// that created `_tool_testmo_scope_configs` omitted the `connection_id` and +// `name` columns of the embedded common.ScopeConfig, which the runtime model +// expects. +// +// The `uniqueIndex` on `name` is safe to add here: the new column is nullable, +// so pre-existing rows are backfilled with NULL, and both MySQL and PostgreSQL +// allow duplicate NULLs in a unique index (verified against both engines). +type testmoScopeConfig20260727 struct { + archived.Model + Entities []string `gorm:"type:json;serializer:json" json:"entities"` + ConnectionId uint64 `json:"connectionId" gorm:"index"` + Name string `json:"name" gorm:"type:varchar(255);uniqueIndex"` + AcceptanceTestPattern string `json:"acceptanceTestPattern" gorm:"type:varchar(255)"` + SmokeTestPattern string `json:"smokeTestPattern" gorm:"type:varchar(255)"` + TeamPattern string `json:"teamPattern" gorm:"type:varchar(255)"` +} + +func (testmoScopeConfig20260727) TableName() string { + return "_tool_testmo_scope_configs" +} + +type addMissingScopeConfigColumns struct{} + +func (script *addMissingScopeConfigColumns) Up(basicRes context.BasicRes) errors.Error { + return migrationhelper.AutoMigrateTables(basicRes, &testmoScopeConfig20260727{}) +} + +func (*addMissingScopeConfigColumns) Version() uint64 { + return 20260727000001 +} + +func (*addMissingScopeConfigColumns) Name() string { + return "add missing connection_id/name columns to _tool_testmo_scope_configs" +} diff --git a/backend/plugins/testmo/models/migrationscripts/register.go b/backend/plugins/testmo/models/migrationscripts/register.go index 7843d4b84e1..95f2ae48b41 100644 --- a/backend/plugins/testmo/models/migrationscripts/register.go +++ b/backend/plugins/testmo/models/migrationscripts/register.go @@ -25,5 +25,6 @@ func All() []plugin.MigrationScript { new(addScopeConfigIdToProjects), new(replaceTestsWithRuns), new(fixRawTableNamesAndSchemas), + new(addMissingScopeConfigColumns), } } diff --git a/backend/scripts/build-plugins.sh b/backend/scripts/build-plugins.sh index 03fb7b4f9fb..e0ef94bb9b7 100755 --- a/backend/scripts/build-plugins.sh +++ b/backend/scripts/build-plugins.sh @@ -52,7 +52,8 @@ fi if [ -z "$DEVLAKE_PLUGINS" ]; then echo "Building all plugins" - PLUGINS=$(find $PLUGIN_SRC_DIR/* -maxdepth 0 -type d -not -name core -not -name helper -not -name logs -not -empty) + # schema_e2e is not a plugin, it only holds the cross-plugin schema-drift e2e test + PLUGINS=$(find $PLUGIN_SRC_DIR/* -maxdepth 0 -type d -not -name core -not -name helper -not -name logs -not -name schema_e2e -not -empty) else echo "Building the following plugins: $PLUGIN" PLUGINS= From e400f66e2880b0ea029668075791cba4e30283e5 Mon Sep 17 00:00:00 2001 From: DoDiODev Date: Mon, 3 Aug 2026 12:10:35 +0200 Subject: [PATCH 2/3] fix(gh-copilot): add missing AI credit usage breakdown columns The cross-plugin schema-drift guard added by this PR caught a fourth occurrence of the same bug class, introduced by #9019: _tool_copilot_enterprise_ai_credit_usage _tool_copilot_org_ai_credit_usage _tool_copilot_user_ai_credit_usage all lack gross_quantity, discount_quantity, net_quantity, price_per_unit, gross_amount, discount_amount and net_amount, while the runtime models models.GhCopilot{Enterprise,Org,User}AiCreditUsage declare them inline. Writing a record therefore fails with "Unknown column 'gross_quantity' in 'field list'". Root cause: 20260708_add_ai_credit_usage_metrics.go declares those seven columns through an anonymous embedded struct whose TYPE NAME IS UNEXPORTED creditUsageBreakdown20260708 `gorm:"embedded"` and GORM's schema parser skips anonymous fields of unexported types, so AutoMigrate never created the columns. Add a new, additive migration that AutoMigrates the missing columns. It only adds absent columns, so it is a no-op on databases that already have them and safe on populated tables. The original script is left untouched: migration scripts are append-only and its version is already recorded in _devlake_migration_history. Verified with the cross-plugin guard against a fresh database on MySQL 8.4.10 and PostgreSQL 17.2: 44/44 plugins pass (was 43/44 with gh-copilot failing on 21 missing columns). Signed-off-by: DoDiODev --- ...1_fix_ai_credit_usage_breakdown_columns.go | 98 +++++++++++++++++++ .../models/migrationscripts/register.go | 1 + 2 files changed, 99 insertions(+) create mode 100644 backend/plugins/gh-copilot/models/migrationscripts/20260731_fix_ai_credit_usage_breakdown_columns.go diff --git a/backend/plugins/gh-copilot/models/migrationscripts/20260731_fix_ai_credit_usage_breakdown_columns.go b/backend/plugins/gh-copilot/models/migrationscripts/20260731_fix_ai_credit_usage_breakdown_columns.go new file mode 100644 index 00000000000..3365e9653de --- /dev/null +++ b/backend/plugins/gh-copilot/models/migrationscripts/20260731_fix_ai_credit_usage_breakdown_columns.go @@ -0,0 +1,98 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package migrationscripts + +import ( + "github.com/apache/incubator-devlake/core/context" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/models/migrationscripts/archived" + "github.com/apache/incubator-devlake/helpers/migrationhelper" +) + +// The 20260708 migration embedded the credit-usage breakdown columns through an +// *unexported* anonymous struct (`creditUsageBreakdown20260708`). GORM's schema +// parser does not migrate the fields of an unexported embedded type, so the +// gross_/discount_/net_ quantity and amount columns (plus price_per_unit) were +// never created even though the runtime models declare them inline. This +// follow-up migration adds the missing columns. AutoMigrate only adds columns +// that do not yet exist, so it is a no-op on databases that somehow already +// have them. + +type enterpriseAiCreditUsageBreakdown20260731 struct { + GrossQuantity float64 `gorm:"comment:Raw credits consumed"` + DiscountQuantity float64 `gorm:"comment:Credits discounted"` + NetQuantity float64 `gorm:"comment:Credits after discount"` + PricePerUnit float64 `gorm:"comment:Price per credit unit"` + GrossAmount float64 `gorm:"comment:Gross cost before discount"` + DiscountAmount float64 `gorm:"comment:Discount amount"` + NetAmount float64 `gorm:"comment:Net cost after discount"` + archived.NoPKModel +} + +func (enterpriseAiCreditUsageBreakdown20260731) TableName() string { + return "_tool_copilot_enterprise_ai_credit_usage" +} + +type orgAiCreditUsageBreakdown20260731 struct { + GrossQuantity float64 `gorm:"comment:Raw credits consumed"` + DiscountQuantity float64 `gorm:"comment:Credits discounted"` + NetQuantity float64 `gorm:"comment:Credits after discount"` + PricePerUnit float64 `gorm:"comment:Price per credit unit"` + GrossAmount float64 `gorm:"comment:Gross cost before discount"` + DiscountAmount float64 `gorm:"comment:Discount amount"` + NetAmount float64 `gorm:"comment:Net cost after discount"` + archived.NoPKModel +} + +func (orgAiCreditUsageBreakdown20260731) TableName() string { + return "_tool_copilot_org_ai_credit_usage" +} + +type userAiCreditUsageBreakdown20260731 struct { + GrossQuantity float64 `gorm:"comment:Raw credits consumed"` + DiscountQuantity float64 `gorm:"comment:Credits discounted"` + NetQuantity float64 `gorm:"comment:Credits after discount"` + PricePerUnit float64 `gorm:"comment:Price per credit unit"` + GrossAmount float64 `gorm:"comment:Gross cost before discount"` + DiscountAmount float64 `gorm:"comment:Discount amount"` + NetAmount float64 `gorm:"comment:Net cost after discount"` + archived.NoPKModel +} + +func (userAiCreditUsageBreakdown20260731) TableName() string { + return "_tool_copilot_user_ai_credit_usage" +} + +type fixAiCreditUsageBreakdownColumns struct{} + +func (u *fixAiCreditUsageBreakdownColumns) Up(basicRes context.BasicRes) errors.Error { + return migrationhelper.AutoMigrateTables( + basicRes, + &enterpriseAiCreditUsageBreakdown20260731{}, + &orgAiCreditUsageBreakdown20260731{}, + &userAiCreditUsageBreakdown20260731{}, + ) +} + +func (u *fixAiCreditUsageBreakdownColumns) Version() uint64 { + return 20260731000000 +} + +func (u *fixAiCreditUsageBreakdownColumns) Name() string { + return "add missing AI credit usage breakdown columns" +} diff --git a/backend/plugins/gh-copilot/models/migrationscripts/register.go b/backend/plugins/gh-copilot/models/migrationscripts/register.go index 6bf50a37193..8f190264618 100644 --- a/backend/plugins/gh-copilot/models/migrationscripts/register.go +++ b/backend/plugins/gh-copilot/models/migrationscripts/register.go @@ -32,5 +32,6 @@ func All() []plugin.MigrationScript { new(addOrganizationIdToUserMetrics), new(addCopilotMetricsGaps), new(addAiCreditUsageMetrics), + new(fixAiCreditUsageBreakdownColumns), } } From dae92b522273c7e83a320ffd0279f86b8ef2faea Mon Sep 17 00:00:00 2001 From: DoDiODev Date: Mon, 3 Aug 2026 13:28:42 +0200 Subject: [PATCH 3/3] test(schema): guard the migration upgrade path on populated tables Review feedback on #9015: TestMigrationSchemaMatchesModels proves the END STATE of a fresh migration run matches the runtime models, but every table it inspects is empty, so it never exercises the upgrade path of a repair migration on a database that already holds rows -- which is the only situation those migrations exist for. Add TestMigrationUpgradePathOnPopulatedTables, which for every repair migration in this PR 1. recreates the table exactly as the buggy migration left it, 2. inserts rows, 3. runs ONLY that repair script, 4. asserts the columns were added, the rows survived, the table has a primary key and auto-increment ids were backfilled (plus that a subsequent INSERT still works, i.e. the sequence/counter is in sync). Step 4 covers what a column-presence check cannot see. Negative test, with the explicit AUTO_INCREMENT DDL in the teambition script replaced by a plain AutoMigrate: MySQL -> FAIL, migration errors out (Error 1075) PostgreSQL -> FAIL, "table has no primary key after ..." (AutoMigrate happily adds `bigserial` without a key, so this is invisible to the column-only guard) Covered: _tool_jira_sprint_reports, _tool_taiga_scope_configs, _tool_teambition_scope_configs, _tool_testmo_scope_configs and the three _tool_copilot_*_ai_credit_usage tables. The scripts are looked up through each plugin's own MigrationScripts() by version, so the test fails if one is removed or renumbered. Verified on MySQL 8.4.10 and PostgreSQL 17.2: 51/51 subtests pass (44 plugins + 7 upgrade-path cases). Signed-off-by: DoDiODev --- .../schema_e2e/migration_upgrade_path_test.go | 382 ++++++++++++++++++ 1 file changed, 382 insertions(+) create mode 100644 backend/plugins/schema_e2e/migration_upgrade_path_test.go diff --git a/backend/plugins/schema_e2e/migration_upgrade_path_test.go b/backend/plugins/schema_e2e/migration_upgrade_path_test.go new file mode 100644 index 00000000000..60c8de40ffd --- /dev/null +++ b/backend/plugins/schema_e2e/migration_upgrade_path_test.go @@ -0,0 +1,382 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package schema_e2e + +import ( + "fmt" + "testing" + + "github.com/apache/incubator-devlake/core/config" + "github.com/apache/incubator-devlake/core/dal" + "github.com/apache/incubator-devlake/core/models/migrationscripts/archived" + "github.com/apache/incubator-devlake/core/plugin" + "github.com/apache/incubator-devlake/core/runner" + "github.com/apache/incubator-devlake/helpers/e2ehelper" + "github.com/apache/incubator-devlake/impls/dalgorm" + "github.com/apache/incubator-devlake/impls/logruslog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" + + copilotimpl "github.com/apache/incubator-devlake/plugins/gh-copilot/impl" + jiraimpl "github.com/apache/incubator-devlake/plugins/jira/impl" + taigaimpl "github.com/apache/incubator-devlake/plugins/taiga/impl" + teambitionimpl "github.com/apache/incubator-devlake/plugins/teambition/impl" + testmoimpl "github.com/apache/incubator-devlake/plugins/testmo/impl" +) + +// ---------------------------------------------------------------------------- +// Pre-repair table shapes +// +// Each struct reproduces a table EXACTLY as the buggy migration left it, i.e. +// without the columns that the repair migration adds. The repair script is then +// executed against that table *with rows in it*. +// ---------------------------------------------------------------------------- + +// upgradePreJiraSprintReport is _tool_jira_sprint_reports as created by +// 20260722 — no embedded NoPKModel, hence no _raw_data_* / created_at / +// updated_at columns. +type upgradePreJiraSprintReport struct { + ConnectionId uint64 `gorm:"primaryKey"` + BoardId uint64 `gorm:"primaryKey"` + SprintId uint64 `gorm:"primaryKey"` + IssueId uint64 `gorm:"primaryKey"` + + IssueKey string `gorm:"type:varchar(255)"` + Bucket string `gorm:"type:varchar(32);index"` + Done bool + StoryPointsAtSprintStart *float64 + StoryPointsAtSprintEnd *float64 +} + +func (upgradePreJiraSprintReport) TableName() string { return "_tool_jira_sprint_reports" } + +// upgradePreTaigaScopeConfig lacks `type_mappings`. +type upgradePreTaigaScopeConfig struct { + archived.Model + Entities []string `gorm:"type:json;serializer:json"` + ConnectionId uint64 `gorm:"index"` + Name string `gorm:"type:varchar(255);uniqueIndex"` +} + +func (upgradePreTaigaScopeConfig) TableName() string { return "_tool_taiga_scope_configs" } + +// upgradePreTeambitionScopeConfig lacks the embedded common.Model, i.e. the +// table has no primary key at all and no id / created_at / updated_at. +type upgradePreTeambitionScopeConfig struct { + Entities []string `gorm:"type:json;serializer:json"` + ConnectionId uint64 `gorm:"index"` + Name string `gorm:"type:varchar(255)"` + TypeMappings map[string]string `gorm:"serializer:json"` + StatusMappings map[string]string `gorm:"serializer:json"` + BugDueDateField string `gorm:"column:bug_due_date_field"` + TaskDueDateField string `gorm:"column:task_due_date_field"` + StoryDueDateField string `gorm:"column:story_due_date_field"` +} + +func (upgradePreTeambitionScopeConfig) TableName() string { + return "_tool_teambition_scope_configs" +} + +// upgradePreTestmoScopeConfig lacks `connection_id` and `name`. +type upgradePreTestmoScopeConfig struct { + archived.Model + Entities []string `gorm:"type:json;serializer:json"` + AcceptanceTestPattern string `gorm:"type:varchar(255)"` + SmokeTestPattern string `gorm:"type:varchar(255)"` + TeamPattern string `gorm:"type:varchar(255)"` +} + +func (upgradePreTestmoScopeConfig) TableName() string { return "_tool_testmo_scope_configs" } + +// upgradePreCopilotEnterpriseCredits and friends lack the seven credit +// breakdown columns, which 20260708 declared through an unexported embedded +// struct that GORM silently ignores. +type upgradePreCopilotEnterpriseCredits struct { + ConnectionId uint64 `gorm:"primaryKey"` + ScopeId string `gorm:"primaryKey;type:varchar(191)"` + Year int `gorm:"primaryKey"` + Month int `gorm:"primaryKey"` + Day int `gorm:"primaryKey"` + Enterprise string `gorm:"primaryKey;type:varchar(191)"` + Model string `gorm:"primaryKey;type:varchar(191)"` + Product string `gorm:"type:varchar(32)"` + archived.NoPKModel +} + +func (upgradePreCopilotEnterpriseCredits) TableName() string { + return "_tool_copilot_enterprise_ai_credit_usage" +} + +type upgradePreCopilotOrgCredits struct { + ConnectionId uint64 `gorm:"primaryKey"` + ScopeId string `gorm:"primaryKey;type:varchar(191)"` + Year int `gorm:"primaryKey"` + Month int `gorm:"primaryKey"` + Day int `gorm:"primaryKey"` + Organization string `gorm:"primaryKey;type:varchar(191)"` + Model string `gorm:"primaryKey;type:varchar(191)"` + Product string `gorm:"type:varchar(32)"` + archived.NoPKModel +} + +func (upgradePreCopilotOrgCredits) TableName() string { + return "_tool_copilot_org_ai_credit_usage" +} + +type upgradePreCopilotUserCredits struct { + ConnectionId uint64 `gorm:"primaryKey"` + ScopeId string `gorm:"primaryKey;type:varchar(191)"` + Year int `gorm:"primaryKey"` + Month int `gorm:"primaryKey"` + Day int `gorm:"primaryKey"` + User string `gorm:"primaryKey;type:varchar(191)"` + Model string `gorm:"primaryKey;type:varchar(191)"` + Product string `gorm:"type:varchar(32)"` + archived.NoPKModel +} + +func (upgradePreCopilotUserCredits) TableName() string { + return "_tool_copilot_user_ai_credit_usage" +} + +var copilotCreditColumns = []string{ + "gross_quantity", "discount_quantity", "net_quantity", "price_per_unit", + "gross_amount", "discount_amount", "net_amount", +} + +// upgradeCase describes one repair migration and how to exercise it on a table +// that already contains data. +type upgradeCase struct { + // plugin owning the migration script, used to look the script up by version + // instead of duplicating it here (so the test breaks if the script is + // removed or renumbered). + plugin plugin.PluginMeta + version uint64 + // pre is the table as the buggy migration left it. + pre dal.Tabler + // seed rows inserted BEFORE the repair migration runs. + seed []map[string]interface{} + // wantColumns must exist after the repair. + wantColumns []string + // wantPrimaryKey asserts the table has a primary key afterwards. A plain + // AutoMigrate cannot add one, which is invisible to a column-only check + // (and silently accepted by PostgreSQL). + wantPrimaryKey bool + // autoIncColumn, if set, must be backfilled with distinct non-zero values + // for the pre-existing rows, and a subsequent INSERT must still work. + autoIncColumn string +} + +// TestMigrationUpgradePathOnPopulatedTables complements +// TestMigrationSchemaMatchesModels: that guard proves the END STATE of a fresh +// migration run matches the models, but every table it inspects is empty, so it +// cannot exercise the upgrade path of a repair migration on a database that +// already holds rows — which is the only situation those migrations exist for. +// +// For each repair migration this test therefore +// 1. recreates the table exactly as the buggy migration left it, +// 2. inserts rows, +// 3. runs ONLY that repair script, +// 4. asserts the columns were added, the rows survived untouched, the primary +// key exists and auto-increment ids were backfilled. +// +// Step 4 is what a column-presence check on an empty table cannot see: replacing +// the explicit AUTO_INCREMENT DDL in the teambition script with a plain +// AutoMigrate is accepted by PostgreSQL (it adds `bigserial` without a primary +// key), and only this test notices. +func TestMigrationUpgradePathOnPopulatedTables(t *testing.T) { + db := e2ehelper.NewIsolatedMigrationDb(t, "upgrade_path") + dalInstance := dalgorm.NewDalgorm(db) + basicRes := runner.CreateBasicRes(config.GetConfig(), logruslog.Global, db) + + cases := map[string]upgradeCase{ + "jira sprint report raw data columns": { + plugin: jiraimpl.Jira{}, + version: 20260727000000, + pre: upgradePreJiraSprintReport{}, + seed: []map[string]interface{}{ + {"connection_id": 1, "board_id": 10, "sprint_id": 100, "issue_id": 1000, "issue_key": "TEST-1", "bucket": "committed", "done": false}, + {"connection_id": 1, "board_id": 10, "sprint_id": 100, "issue_id": 1001, "issue_key": "TEST-2", "bucket": "completed", "done": true}, + }, + wantColumns: []string{"_raw_data_params", "_raw_data_table", "_raw_data_id", "_raw_data_remark", "created_at", "updated_at"}, + }, + "taiga scope config type_mappings": { + plugin: taigaimpl.Taiga{}, + version: 20260727000001, + pre: upgradePreTaigaScopeConfig{}, + seed: []map[string]interface{}{ + {"id": 1, "connection_id": 1, "name": "taiga-cfg-a"}, + {"id": 2, "connection_id": 1, "name": "taiga-cfg-b"}, + }, + wantColumns: []string{"type_mappings"}, + wantPrimaryKey: true, + }, + "teambition scope config primary key": { + plugin: teambitionimpl.Teambition{}, + version: 20260727000001, + pre: upgradePreTeambitionScopeConfig{}, + seed: []map[string]interface{}{ + {"connection_id": 1, "name": "teambition-cfg-a"}, + {"connection_id": 1, "name": "teambition-cfg-b"}, + }, + wantColumns: []string{"id", "created_at", "updated_at"}, + wantPrimaryKey: true, + autoIncColumn: "id", + }, + "testmo scope config connection_id/name": { + plugin: testmoimpl.Testmo{}, + version: 20260727000001, + pre: upgradePreTestmoScopeConfig{}, + seed: []map[string]interface{}{ + // `name` carries a uniqueIndex in the repaired shape; both rows + // are backfilled with NULL, which MySQL and PostgreSQL accept. + {"id": 1, "acceptance_test_pattern": "a"}, + {"id": 2, "acceptance_test_pattern": "b"}, + }, + wantColumns: []string{"connection_id", "name"}, + wantPrimaryKey: true, + }, + "gh-copilot enterprise credit breakdown": { + plugin: copilotimpl.GhCopilot{}, + version: 20260731000000, + pre: upgradePreCopilotEnterpriseCredits{}, + seed: []map[string]interface{}{ + {"connection_id": 1, "scope_id": "ent-1", "year": 2026, "month": 8, "day": 1, "enterprise": "acme", "model": "gpt-4.1", "product": "copilot"}, + }, + wantColumns: copilotCreditColumns, + }, + "gh-copilot org credit breakdown": { + plugin: copilotimpl.GhCopilot{}, + version: 20260731000000, + pre: upgradePreCopilotOrgCredits{}, + seed: []map[string]interface{}{ + {"connection_id": 1, "scope_id": "org-1", "year": 2026, "month": 8, "day": 1, "organization": "acme", "model": "gpt-4.1", "product": "copilot"}, + }, + wantColumns: copilotCreditColumns, + }, + "gh-copilot user credit breakdown": { + plugin: copilotimpl.GhCopilot{}, + version: 20260731000000, + pre: upgradePreCopilotUserCredits{}, + seed: []map[string]interface{}{ + {"connection_id": 1, "scope_id": "user-1", "year": 2026, "month": 8, "day": 1, "user": "octocat", "model": "gpt-4.1", "product": "copilot"}, + }, + wantColumns: copilotCreditColumns, + }, + } + + for name, c := range cases { + c := c + t.Run(name, func(t *testing.T) { + table := c.pre.TableName() + script := findMigrationScript(t, c.plugin, c.version) + + // 1. table exactly as the buggy migration left it + require.NoError(t, db.Migrator().DropTable(table)) + require.NoError(t, db.Table(table).AutoMigrate(c.pre)) + for _, column := range c.wantColumns { + require.Falsef(t, db.Migrator().HasColumn(c.pre, column), + "precondition failed: %q already has column %q, the pre-repair shape is wrong", + table, column) + } + + // 2. rows, so the migration has to survive real data + for _, row := range c.seed { + require.NoError(t, db.Table(table).Create(row).Error) + } + + // 3. run ONLY the repair script + require.NoErrorf(t, script.Up(basicRes), + "migration %q failed on a populated %q", script.Name(), table) + + // 4a. the columns are there now + actual, colErr := dal.GetColumnNames(dalInstance, dal.DefaultTabler{Name: table}, + func(dal.ColumnMeta) bool { return true }) + require.NoError(t, colErr) + existing := make(map[string]struct{}, len(actual)) + for _, column := range actual { + existing[column] = struct{}{} + } + for _, column := range c.wantColumns { + _, ok := existing[column] + assert.Truef(t, ok, "table %q is still missing column %q after %q", + table, column, script.Name()) + } + + // 4b. no data was lost + var rows int64 + require.NoError(t, db.Table(table).Count(&rows).Error) + assert.EqualValuesf(t, len(c.seed), rows, + "migration %q changed the row count of %q", script.Name(), table) + + // 4c. the primary key survived / was created + if c.wantPrimaryKey { + assert.Truef(t, hasPrimaryKey(t, db, table), + "table %q has no primary key after %q — AutoMigrate cannot add one, "+ + "the script needs explicit DDL", table, script.Name()) + } + + // 4d. auto-increment ids were backfilled and the counter still works + if c.autoIncColumn != "" { + var ids []uint64 + require.NoError(t, db.Table(table).Pluck(c.autoIncColumn, &ids).Error) + require.Len(t, ids, len(c.seed)) + seen := map[uint64]struct{}{} + for _, id := range ids { + assert.NotZerof(t, id, "pre-existing row was not assigned a %q", c.autoIncColumn) + _, dup := seen[id] + assert.Falsef(t, dup, "duplicate %q=%d after backfill", c.autoIncColumn, id) + seen[id] = struct{}{} + } + require.NoErrorf(t, db.Table(table).Create(map[string]interface{}{ + "connection_id": 2, "name": "inserted-after-migration", + }).Error, "INSERT after the migration failed, the sequence/counter is out of sync") + } + }) + } +} + +// findMigrationScript looks the script up through the plugin's own +// MigrationScripts() so the test fails if it is removed or renumbered. +func findMigrationScript(t *testing.T, p plugin.PluginMeta, version uint64) plugin.MigrationScript { + migratable, ok := p.(plugin.PluginMigration) + require.Truef(t, ok, "plugin %s does not implement PluginMigration", p.Name()) + for _, script := range migratable.MigrationScripts() { + if script.Version() == version { + return script + } + } + t.Fatalf("plugin %s has no migration script with version %d", p.Name(), version) + return nil +} + +// hasPrimaryKey works on MySQL and PostgreSQL alike. +func hasPrimaryKey(t *testing.T, db *gorm.DB, table string) bool { + schemaFunc := "current_schema()" + if db.Dialector.Name() == "mysql" { + schemaFunc = "DATABASE()" + } + var count int64 + err := db.Raw(fmt.Sprintf( + `SELECT COUNT(*) FROM information_schema.table_constraints + WHERE constraint_type = 'PRIMARY KEY' AND table_name = ? AND table_schema = %s`, + schemaFunc), table).Scan(&count).Error + require.NoError(t, err) + return count > 0 +}