Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wrong type on hooktask to convert typ from char(16) to varchar(16) #14148

Merged
merged 14 commits into from
Jan 6, 2021
40 changes: 40 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package migrations

import (
"context"
"fmt"
"os"
"reflect"
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
6543 marked this conversation as resolved.
Show resolved Hide resolved
// 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))
lafriks marked this conversation as resolved.
Show resolved Hide resolved
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
}
2 changes: 1 addition & 1 deletion models/migrations/v161.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
6543 marked this conversation as resolved.
Show resolved Hide resolved
}
if err := x.Sync2(new(HookTask)); err != nil {
return err
Expand Down
66 changes: 66 additions & 0 deletions models/migrations/v165.go
Original file line number Diff line number Diff line change
@@ -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"
lafriks marked this conversation as resolved.
Show resolved Hide resolved
"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
lafriks marked this conversation as resolved.
Show resolved Hide resolved
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
}
8 changes: 4 additions & 4 deletions models/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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"`
Expand Down
6 changes: 3 additions & 3 deletions services/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,15 @@ 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
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
}

Expand Down Expand Up @@ -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 {
Expand Down