From 4a2959b3ec214a954d6b144f0fb9efb848129c8c Mon Sep 17 00:00:00 2001 From: varp0n Date: Wed, 1 May 2024 16:28:44 +0000 Subject: [PATCH] FIX gogs migration if gogs is hosted at a subpath (#3572) Also add a test for GogsDownloaderFactory.New() to make sure that the URL of the source repository is parsed correctly. When the source gogs instance is hosted at a subpath like `https://git.example.com/gogs//` the migration fails. This PR fixes that. Co-authored-by: hecker Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/3572 Reviewed-by: Earl Warren Co-authored-by: varp0n Co-committed-by: varp0n --- release-notes/8.0.0/3572.md | 1 + services/migrations/gogs.go | 15 ++++-- services/migrations/gogs_test.go | 84 ++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 release-notes/8.0.0/3572.md diff --git a/release-notes/8.0.0/3572.md b/release-notes/8.0.0/3572.md new file mode 100644 index 0000000000..c79f9e30fc --- /dev/null +++ b/release-notes/8.0.0/3572.md @@ -0,0 +1 @@ +Fix gogs migration if gogs is hosted at a subpath diff --git a/services/migrations/gogs.go b/services/migrations/gogs.go index 72c52d180b..b31d05fa73 100644 --- a/services/migrations/gogs.go +++ b/services/migrations/gogs.go @@ -38,17 +38,24 @@ func (f *GogsDownloaderFactory) New(ctx context.Context, opts base.MigrateOption return nil, err } - baseURL := u.Scheme + "://" + u.Host repoNameSpace := strings.TrimSuffix(u.Path, ".git") repoNameSpace = strings.Trim(repoNameSpace, "/") fields := strings.Split(repoNameSpace, "/") - if len(fields) < 2 { + numFields := len(fields) + if numFields < 2 { return nil, fmt.Errorf("invalid path: %s", repoNameSpace) } - log.Trace("Create gogs downloader. BaseURL: %s RepoOwner: %s RepoName: %s", baseURL, fields[0], fields[1]) - return NewGogsDownloader(ctx, baseURL, opts.AuthUsername, opts.AuthPassword, opts.AuthToken, fields[0], fields[1]), nil + repoOwner := fields[numFields-2] + repoName := fields[numFields-1] + + u.Path = "" + u = u.JoinPath(fields[:numFields-2]...) + baseURL := u.String() + + log.Trace("Create gogs downloader. BaseURL: %s RepoOwner: %s RepoName: %s", baseURL, repoOwner, repoName) + return NewGogsDownloader(ctx, baseURL, opts.AuthUsername, opts.AuthPassword, opts.AuthToken, repoOwner, repoName), nil } // GitServiceType returns the type of git service diff --git a/services/migrations/gogs_test.go b/services/migrations/gogs_test.go index 610af183de..ca02b4317b 100644 --- a/services/migrations/gogs_test.go +++ b/services/migrations/gogs_test.go @@ -137,3 +137,87 @@ func TestGogsDownloadRepo(t *testing.T) { _, _, err = downloader.GetPullRequests(1, 3) assert.Error(t, err) } + +func TestGogsDownloaderFactory_New(t *testing.T) { + tests := []struct { + name string + args base.MigrateOptions + baseURL string + repoOwner string + repoName string + wantErr bool + }{ + { + name: "Gogs_at_root", + args: base.MigrateOptions{ + CloneAddr: "https://git.example.com/user/repo.git", + AuthUsername: "username", + AuthPassword: "password", + AuthToken: "authtoken", + }, + baseURL: "https://git.example.com/", + repoOwner: "user", + repoName: "repo", + wantErr: false, + }, + { + name: "Gogs_at_sub_path", + args: base.MigrateOptions{ + CloneAddr: "https://git.example.com/subpath/user/repo.git", + AuthUsername: "username", + AuthPassword: "password", + AuthToken: "authtoken", + }, + baseURL: "https://git.example.com/subpath", + repoOwner: "user", + repoName: "repo", + wantErr: false, + }, + { + name: "Gogs_at_2nd_sub_path", + args: base.MigrateOptions{ + CloneAddr: "https://git.example.com/sub1/sub2/user/repo.git", + AuthUsername: "username", + AuthPassword: "password", + AuthToken: "authtoken", + }, + baseURL: "https://git.example.com/sub1/sub2", + repoOwner: "user", + repoName: "repo", + wantErr: false, + }, + { + name: "Gogs_URL_too_short", + args: base.MigrateOptions{ + CloneAddr: "https://git.example.com/repo.git", + AuthUsername: "username", + AuthPassword: "password", + AuthToken: "authtoken", + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &GogsDownloaderFactory{} + opts := base.MigrateOptions{ + CloneAddr: tt.args.CloneAddr, + AuthUsername: tt.args.AuthUsername, + AuthPassword: tt.args.AuthPassword, + AuthToken: tt.args.AuthToken, + } + got, err := f.New(context.Background(), opts) + if (err != nil) != tt.wantErr { + t.Errorf("GogsDownloaderFactory.New() error = %v, wantErr %v", err, tt.wantErr) + return + } else if err != nil { + return + } + + assert.IsType(t, &GogsDownloader{}, got) + assert.EqualValues(t, tt.baseURL, got.(*GogsDownloader).baseURL) + assert.EqualValues(t, tt.repoOwner, got.(*GogsDownloader).repoOwner) + assert.EqualValues(t, tt.repoName, got.(*GogsDownloader).repoName) + }) + } +}