DoDiODev opened a new pull request, #9015:
URL: https://github.com/apache/devlake/pull/9015
> Starting point is the Jira Sprint Report bug below. The regression guard
added
> for it turned out to catch the same class of bug in three more plugins
> (taiga, teambition, testmo), which are fixed here as well — see
> [§3](#3-additional-schema-drifts-found-by-the-new-cross-plugin-guard).
## Problem
Collecting Jira data fails in the `extractSprintReport` subtask with:
```
subtask extractSprintReport ended unexpectedly
error getting batch from result (500)
Error 1054 (42S22): Unknown column '_raw_data_table' in 'where clause'
```
### Root cause
The Sprint Report feature (PR #8967 / #9010) added the table
`_tool_jira_sprint_reports`. Its migration
(`20260722_add_sprint_report_table.go`) creates the table from a struct that
does **not** embed `common.NoPKModel`:
```go
type jiraSprintReport20260722 struct {
ConnectionId uint64 `gorm:"primaryKey"`
// ... no common.NoPKModel ...
}
```
The runtime model `models.JiraSprintReport` **does** embed `common.NoPKModel`
(→ `common.RawDataOrigin`), so it expects the columns `_raw_data_params`,
`_raw_data_table`, `_raw_data_id`, `_raw_data_remark` (plus `created_at`,
`updated_at`).
During extraction, `api.NewApiExtractor` deletes outdated rows via
`WHERE _raw_data_table = ? AND _raw_data_params = ?`
(`helpers/pluginhelper/api/batch_save_divider.go`). Because the migration
never
created those columns, MySQL rejects the query with error 1054.
## What changed
### 1. Fix for the reported bug (jira)
- **New migration**
`plugins/jira/models/migrationscripts/20260727_add_raw_data_columns_to_sprint_report.go`
re-runs `AutoMigrateTables` on a struct that embeds the raw-data columns,
adding them to existing `_tool_jira_sprint_reports` tables **without data
loss** (GORM `AutoMigrate` only adds missing columns). Registered in
`models/migrationscripts/register.go`.
- The original `20260722_add_sprint_report_table.go` is left **unchanged**:
migration scripts are append-only, and editing it would not repair
databases
that already ran it (its version is already recorded in
`_devlake_migration_history`).
### 2. Regression tests (schema-drift guards)
- **`plugins/jira/e2e/migration_schema_test.go`** — runs the real Jira
migration
chain (framework + jira) and asserts every column each Jira model declares
exists in the migrated table. Directly reproduces and guards the bug above.
- **`plugins/schema_e2e/migration_schema_test.go`** — cross-plugin
generalization:
applies framework + **all** plugin migrations and validates model-vs-table
column parity for every built-in Go plugin. Includes
`TestAllGoPluginsListed`,
which fails if a new plugin directory with an `impl` package is added but
not
registered, so the guard stays complete automatically.
Both deliberately run the **real migration scripts** instead of
`AutoMigrate`-ing
the runtime model — an `AutoMigrate`-based check could never detect this
class of
drift. Both live in `e2e` packages, so they run under `make
e2e-test-go-plugins`
(require `E2E_DB_URL`) and are excluded from the DB-less unit-test run.
Implementation details worth noting:
- Both tests call `dalgorm.Init(...)` to register the `encdec` GORM
serializer.
`runner.CreateBasicRes` does **not** do this (only `CreateAppBasicRes`
does),
so without it the migrations abort with `invalid serializer type encdec`.
- Both fall back to a deterministic `ENCRYPTION_SECRET` if none is
configured;
some migrations (e.g. jira `20220716`) refuse to run without one
(`jira v0.11 invalid encKey`), and CI does not provide a value.
- Models that no migration materializes (pure API-response models such as
`_tool_jira_server_infos`) are skipped — the check targets *drift* between
an
existing table and its model.
### 3. Additional schema drifts found by the new cross-plugin guard
The guard immediately uncovered three pre-existing bugs of exactly the same
class. Each is fixed with a new, additive migration (registered in the
respective `register.go`):
| Table | Missing column(s) |
| --- | --- |
| `_tool_taiga_scope_configs` | `type_mappings` |
| `_tool_teambition_scope_configs` | `id`, `created_at`, `updated_at` |
| `_tool_testmo_scope_configs` | `connection_id`, `name` |
Files:
`plugins/{taiga,teambition,testmo}/models/migrationscripts/20260727_add_missing_scope_config_columns.go`.
All new migration scripts use `core/models/migrationscripts/archived` —
importing
`core/models/common` from a migration script is rejected by
`make migration-script-lint`.
#### Why these repairs are safe on populated tables
The scope-config repairs re-add columns that carry constraints in
`common.ScopeConfig` (`name` has a `uniqueIndex`) and, for teambition, an
AUTO_INCREMENT primary key. Both were verified against live engines rather
than
assumed:
- **`uniqueIndex` on `name`** — GORM adds new columns as *nullable*
(`ALTER TABLE ... ADD COLUMN name VARCHAR(255)`, no `NOT NULL DEFAULT
''`), so
pre-existing rows are backfilled with `NULL`, and both MySQL and PostgreSQL
permit duplicate `NULL`s in a unique index. Creating the index on a table
with
two pre-existing rows succeeded on **MySQL 8.4.10** and **PostgreSQL
17.2**.
The counter-test confirms the distinction: with an explicit
`NOT NULL DEFAULT ''` column the same index fails
(`Error 1062` / `pq: … (23505)`) — which is exactly the case that does
*not*
occur here.
- **AUTO_INCREMENT `id` on the primary-key-less
`_tool_teambition_scope_configs`** —
GORM issues a plain `ADD COLUMN`, and MySQL backfills consecutive ids for
the
existing rows (verified: two pre-existing rows received `id` 1 and 2). No
`Error 1075` ("there can be only one auto column and it must be defined as
a
key").
### 4. Build script
`scripts/build-plugins.sh` builds every directory under `plugins/` with
`-buildmode=plugin`. `plugins/schema_e2e/` is not a plugin (it contains only
the
cross-plugin test and has no `main` package), which made `make build-plugin`
fail
with *"-buildmode=plugin requires exactly one main package"*. The directory
is now
excluded, alongside the existing `core` / `helper` / `logs` exclusions.
## Why a new migration (not editing the old one)
- The buggy migration `20260722` is already on `main`/`upstream/main` and has
already been applied to real databases; its version is recorded, so it will
never re-run. Only a **new** migration can repair those databases.
- Append-only migrations keep the migration history reproducible across
environments.
## Testing
- `make migration-script-lint` — OK.
- `gofmt -l` on all added/changed files — clean.
- `go vet ./plugins/{jira,taiga,teambition,testmo,schema_e2e}/...` — OK.
- **MySQL 8.4.10, fresh database:**
- `go test ./plugins/schema_e2e/` — OK, **42/42 plugins PASS**
(`TestAllGoPluginsListed` included).
- `go test -run TestMigrationSchema ./plugins/jira/e2e/` — OK.
- **PostgreSQL 17.2, fresh database:** both tests OK; the repaired
scope-config
tables contain the added columns.
- **Upgrade path on a populated database** (not just a fresh one): all four
migrations were applied to a real, long-running DevLake instance. They are
recorded in `_devlake_migration_history` and the target tables carry the
expected columns afterwards — e.g. `_tool_teambition_scope_configs` gained
`id`, `created_at`, `updated_at`, and `_tool_jira_sprint_reports` (661
rows)
gained the `_raw_data_*` columns without data loss.
- **Constraint safety** on populated tables (unique index / AUTO_INCREMENT
PK)
verified separately against MySQL 8.4.10 and PostgreSQL 17.2 — see
[§3 "Why these repairs are safe on populated
tables"](#why-these-repairs-are-safe-on-populated-tables).
- **Negative test:** dropping `_raw_data_table` from
`_tool_jira_sprint_reports`
makes the cross-plugin guard fail with
`[jira] table "_tool_jira_sprint_reports" is missing column
"_raw_data_table"`
— i.e. the guard genuinely detects the original regression.
## Known limitations of the guard (intentional)
- It checks **column presence only** — not column type, length, nullability,
primary keys or indexes.
- It runs against the shared E2E database. Other plugin E2E tests use
`FlushTabler` (drop + `AutoMigrate` of the runtime model), so a table
touched by
an earlier test can appear "repaired". On a fresh database (as in CI) this
does
not apply.
- Tables that no migration creates are skipped, so "model listed in
`GetTablesInfo()` but no table at all" is not flagged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
