From 9f73cae6358c9647cf64589eb2ce055f3dd07f38 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 6 Jan 2021 23:11:23 +0800 Subject: [PATCH] Fix wrong type on hooktask to convert typ from char(16) to varchar(16) (#14148) * Fix wrong type on hooktask to convert typ from char(16) to varchar(16) * Fix bugs * Improve code * Use different trim function for MSSQL * Fix bug * Removed wrong changed line * Removed wrong changed line * Fix nullable * Fix lint * Ignore sqlite on migration * Fix mssql modify column failure * Move modifyColumn to migrations.go so that other migrate function could use it --- models/migrations/migrations.go | 40 ++++++++++++++++++++ models/migrations/v161.go | 2 +- models/migrations/v165.go | 66 +++++++++++++++++++++++++++++++++ models/webhook.go | 8 ++-- services/webhook/webhook.go | 6 +-- 5 files changed, 114 insertions(+), 8 deletions(-) create mode 100644 models/migrations/v165.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index bb3adccc25..4f3d823b9a 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -6,6 +6,7 @@ package migrations import ( + "context" "fmt" "os" "reflect" @@ -16,6 +17,7 @@ import ( "code.gitea.io/gitea/modules/setting" "xorm.io/xorm" + "xorm.io/xorm/schemas" ) const minDBVersion = 70 // Gitea 1.5.3 @@ -273,6 +275,8 @@ var migrations = []Migration{ NewMigration("Convert topic name from 25 to 50", convertTopicNameFrom25To50), // v164 -> v165 NewMigration("Add scope and nonce columns to oauth2_grant table", addScopeAndNonceColumnsToOAuth2Grant), + // v165 -> v166 + NewMigration("Convert hook task type from char(16) to varchar(16) and trim the column", convertHookTaskTypeToVarcharAndTrim), } // GetCurrentDBVersion returns the current db version @@ -737,3 +741,39 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin return nil } + +// modifyColumn will modify column's type or other propertity. SQLITE is not supported +func modifyColumn(x *xorm.Engine, tableName string, col *schemas.Column) error { + var indexes map[string]*schemas.Index + var err error + // MSSQL have to remove index at first, otherwise alter column will fail + // ref. https://sqlzealots.com/2018/05/09/error-message-the-index-is-dependent-on-column-alter-table-alter-column-failed-because-one-or-more-objects-access-this-column/ + if x.Dialect().URI().DBType == schemas.MSSQL { + indexes, err = x.Dialect().GetIndexes(x.DB(), context.Background(), tableName) + if err != nil { + return err + } + + for _, index := range indexes { + _, err = x.Exec(x.Dialect().DropIndexSQL(tableName, index)) + if err != nil { + return err + } + } + } + + defer func() { + for _, index := range indexes { + _, err = x.Exec(x.Dialect().CreateIndexSQL(tableName, index)) + if err != nil { + log.Error("Create index %s on table %s failed: %v", index.Name, tableName, err) + } + } + }() + + alterSQL := x.Dialect().ModifyColumnSQL(tableName, col) + if _, err := x.Exec(alterSQL); err != nil { + return err + } + return nil +} diff --git a/models/migrations/v161.go b/models/migrations/v161.go index 127dca1500..3eff7df8a3 100644 --- a/models/migrations/v161.go +++ b/models/migrations/v161.go @@ -34,7 +34,7 @@ func convertTaskTypeToString(x *xorm.Engine) error { } type HookTask struct { - Typ string `xorm:"char(16) index"` + Typ string `xorm:"VARCHAR(16) index"` } if err := x.Sync2(new(HookTask)); err != nil { return err diff --git a/models/migrations/v165.go b/models/migrations/v165.go new file mode 100644 index 0000000000..d7df0f07a9 --- /dev/null +++ b/models/migrations/v165.go @@ -0,0 +1,66 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "xorm.io/xorm" + "xorm.io/xorm/schemas" +) + +func convertHookTaskTypeToVarcharAndTrim(x *xorm.Engine) error { + dbType := x.Dialect().URI().DBType + if dbType == schemas.SQLITE { // For SQLITE, varchar or char will always be represented as TEXT + return nil + } + + type HookTask struct { + Typ string `xorm:"VARCHAR(16) index"` + } + + if err := modifyColumn(x, "hook_task", &schemas.Column{ + Name: "typ", + SQLType: schemas.SQLType{ + Name: "VARCHAR", + }, + Length: 16, + Nullable: true, // To keep compatible as nullable + }); err != nil { + return err + } + + var hookTaskTrimSQL string + if dbType == schemas.MSSQL { + hookTaskTrimSQL = "UPDATE hook_task SET typ = RTRIM(LTRIM(typ))" + } else { + hookTaskTrimSQL = "UPDATE hook_task SET typ = TRIM(typ)" + } + if _, err := x.Exec(hookTaskTrimSQL); err != nil { + return err + } + + type Webhook struct { + Type string `xorm:"VARCHAR(16) index"` + } + + if err := modifyColumn(x, "webhook", &schemas.Column{ + Name: "type", + SQLType: schemas.SQLType{ + Name: "VARCHAR", + }, + Length: 16, + Nullable: true, // To keep compatible as nullable + }); err != nil { + return err + } + + var webhookTrimSQL string + if dbType == schemas.MSSQL { + webhookTrimSQL = "UPDATE webhook SET type = RTRIM(LTRIM(type))" + } else { + webhookTrimSQL = "UPDATE webhook SET type = TRIM(type)" + } + _, err := x.Exec(webhookTrimSQL) + return err +} diff --git a/models/webhook.go b/models/webhook.go index 9f1b6131af..e0a75843db 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -113,7 +113,7 @@ type Webhook struct { *HookEvent `xorm:"-"` IsSSL bool `xorm:"is_ssl"` IsActive bool `xorm:"INDEX"` - Type HookTaskType `xorm:"char(16) 'type'"` + Type HookTaskType `xorm:"VARCHAR(16) 'type'"` Meta string `xorm:"TEXT"` // store hook-specific attributes LastStatus HookStatus // Last delivery status @@ -641,9 +641,9 @@ type HookTask struct { RepoID int64 `xorm:"INDEX"` HookID int64 UUID string - Typ HookTaskType - URL string `xorm:"TEXT"` - Signature string `xorm:"TEXT"` + Typ HookTaskType `xorm:"VARCHAR(16) index"` + URL string `xorm:"TEXT"` + Signature string `xorm:"TEXT"` api.Payloader `xorm:"-"` PayloadContent string `xorm:"TEXT"` HTTPMethod string `xorm:"http_method"` diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index b779b38466..7b6bc555f7 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -60,7 +60,7 @@ var ( // RegisterWebhook registers a webhook func RegisterWebhook(name string, webhook *webhook) { - webhooks[models.HookTaskType(strings.TrimSpace(name))] = webhook + webhooks[models.HookTaskType(name)] = webhook } // IsValidHookTaskType returns true if a webhook registered @@ -68,7 +68,7 @@ func IsValidHookTaskType(name string) bool { if name == models.GITEA || name == models.GOGS { return true } - _, ok := webhooks[models.HookTaskType(strings.TrimSpace(name))] + _, ok := webhooks[models.HookTaskType(name)] return ok } @@ -147,7 +147,7 @@ func prepareWebhook(w *models.Webhook, repo *models.Repository, event models.Hoo var payloader api.Payloader var err error - webhook, ok := webhooks[strings.TrimSpace(w.Type)] // NOTICE: w.Type maynot be trimmed before store into database + webhook, ok := webhooks[w.Type] if ok { payloader, err = webhook.payloadCreator(p, event, w.Meta) if err != nil {