Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/internal/migrations/lists.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ var Migrations = []Migration{
v3migrations.MigrateLimiterConfig,
v3migrations.MigrateEnvVarConfig,
v3migrations.MigrateSessionConfig,
v3migrations.MigrateTimeoutConfig,
v3migrations.MigrateReqHeaderParser,
MigrateGoVersion("1.24"),
},
Expand Down
14 changes: 14 additions & 0 deletions cmd/internal/migrations/v3/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,20 @@ func MigrateSessionConfig(cmd *cobra.Command, cwd string, _, _ *semver.Version)
return nil
}

// MigrateTimeoutConfig updates timeout middleware usage to the new Config parameter
func MigrateTimeoutConfig(cmd *cobra.Command, cwd string, _, _ *semver.Version) error {
re := regexp.MustCompile(`timeout\.New\(\s*([^,\n]+)\s*,\s*([^\n)]+)\)`)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current regular expression is not robust enough. It doesn't handle multi-line function calls or ensure idempotency. The regex should allow the first argument to span multiple lines and prevent matching already migrated calls by disallowing { and } in the second argument.

Suggested change
re := regexp.MustCompile(`timeout\.New\(\s*([^,\n]+)\s*,\s*([^\n)]+)\)`)
re := regexp.MustCompile(`timeout\.New\((?s)(.*),\s*([^{}\n)]+)\)`)

err := internal.ChangeFileContent(cwd, func(content string) string {
return re.ReplaceAllString(content, `timeout.New($1, timeout.Config{Timeout: $2})`)
})
if err != nil {
return fmt.Errorf("failed to migrate timeout middleware configs: %w", err)
}

cmd.Println("Migrating timeout middleware configs")
return nil
}

// MigrateAppTestConfig updates app.Test calls to use the new TestConfig parameter
func MigrateAppTestConfig(cmd *cobra.Command, cwd string, _, _ *semver.Version) error {
err := internal.ChangeFileContent(cwd, func(content string) string {
Expand Down
24 changes: 24 additions & 0 deletions cmd/internal/migrations/v3/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -778,3 +778,27 @@ var _ = session.New(session.Config{
assert.NotContains(t, content, "Expiration:")
assert.Contains(t, buf.String(), "Migrating session middleware configs")
}

func Test_MigrateTimeoutConfig(t *testing.T) {
t.Parallel()

dir, err := os.MkdirTemp("", "mtimeout")
require.NoError(t, err)
defer func() { require.NoError(t, os.RemoveAll(dir)) }()

file := writeTempFile(t, dir, `package main
import (
"github.com/gofiber/fiber/v2"
"github.com/gofiber/fiber/v2/middleware/timeout"
"time"
)
var _ = timeout.New(func(c fiber.Ctx) error { return nil }, 2*time.Second)`)

var buf bytes.Buffer
cmd := newCmd(&buf)
require.NoError(t, v3.MigrateTimeoutConfig(cmd, dir, nil, nil))

content := readFile(t, file)
assert.Contains(t, content, `timeout.New(func(c fiber.Ctx) error { return nil }, timeout.Config{Timeout: 2*time.Second})`)
assert.Contains(t, buf.String(), "Migrating timeout middleware configs")
}
Comment on lines +782 to +804
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The test for MigrateTimeoutConfig only covers a simple, single-line case. It's important to test against various code styles and edge cases, including a test case where the timeout.New call is split across multiple lines and a test case to verify idempotency.

Loading